Messages in this thread | | | Date | Mon, 12 Jul 2021 13:56:49 +0000 | From | Vincent Pelletier <> | Subject | Re: [PATCH v5] regulator: da9063: Add support for full-current mode. |
| |
Hello,
Thanks for the review.
On Mon, 12 Jul 2021 10:42:26 +0000, Adam Thomson <Adam.Thomson.Opensource@diasemi.com> wrote: > On 08 July 2021 11:33, Vincent Pelletier wrote: > > + if (ret < 0) > > + /* attempt to restore original overdrive state, ignore failure- > > + * on-failure > > + */ > > + regmap_update_bits(regl->hw->regmap, > > DA9063_REG_CONFIG_H, > > + overdrive_mask, orig_overdrive); > > If I2C is failing here I'm not sure this is going to go through and you have > bigger problems. Not sure if it's really worth trying to roll-back at this point > but maybe Mark has another view. Personally I'd be tempted to just ditch this > and just always set the OD bit in this function, rather than trying an roll-back. > Will be much simpler code.
What I have in mind here is regulator_set_current_limit_regmap rejecting the change not because of a bus issue, but rather because of an unusable min_uA..max_uA range. I add this to the error handling path comment to make the intent clearer.
But your remark indeed fully applies in the case of da9063_buck_set_limit_clear_overdrive. I will keep the roll-back codepath for the next patch iteration, but I will drop it if the consensus is against its presence.
Regards, -- Vincent Pelletier GPG fingerprint 983A E8B7 3B91 1598 7A92 3845 CAC9 3691 4257 B0C1
| |