On piątek, 18 marca 2022 09:22:16 CET Krzysztof Kozlowski wrote:
> On 18/03/2022 01:10, Sebastian Krzyszkowiak wrote:
> > Unlike other models, max17055 doesn't require cell characterization
> > data and operates on smaller amount of input variables (DesignCap,
> > VEmpty, IChgTerm and ModelCfg). Input data can already be filled in
> > by max17042_override_por_values, however model refresh bit has to be
> > set after adjusting input variables in order to make them apply.
> >
> > Signed-off-by: Sebastian Krzyszkowiak <sebastian.krzyszkowiak@xxxxxxx>
> > ---
> >
> > drivers/power/supply/max17042_battery.c | 73 +++++++++++++++----------
> > include/linux/power/max17042_battery.h | 3 +
> > 2 files changed, 48 insertions(+), 28 deletions(-)
> >
> > diff --git a/drivers/power/supply/max17042_battery.c
> > b/drivers/power/supply/max17042_battery.c index
> > c019d6c52363..c39250349a1d 100644
> > --- a/drivers/power/supply/max17042_battery.c
> > +++ b/drivers/power/supply/max17042_battery.c
> > @@ -806,6 +806,13 @@ static inline void
> > max17042_override_por_values(struct max17042_chip *chip)>
> > (chip->chip_type == MAXIM_DEVICE_TYPE_MAX17055)) {
> >
> > max17042_override_por(map, MAX17047_V_empty, config-
>vempty);
> >
> > }
> >
> > +
> > + if (chip->chip_type == MAXIM_DEVICE_TYPE_MAX17055) {
> > + max17042_override_por(map, MAX17055_ModelCfg, config-
>model_cfg);
> > + // VChg is 1 by default, so allow it to be set to 0
>
> Consistent comment, so /* */
>
> I actually do not understand fully the comment and the code. You write
> entire model_cfg to MAX17055_ModelCfg and then immediately do it again,
> but with smaller mask. Why?
That's because VChg is 1 on POR, and max17042_override_por doesn't do anything
when value equals 0 - which means that if the whole config->model_cfg is 0,
VChg won't get unset (which is needed for 4.2V batteries).
This could actually be replaced with a single regmap_write.
> > + regmap_update_bits(map, MAX17055_ModelCfg,
> > + MAX17055_MODELCFG_VCHG_BIT,
config->model_cfg);
>
> Can you align the continued line with previous line? Same in other
> places if it is not aligned.
>
> > + }
> >
> > }
> >
> > static int max17042_init_chip(struct max17042_chip *chip)
> >
> > @@ -814,44 +821,54 @@ static int max17042_init_chip(struct max17042_chip
> > *chip)>
> > int ret;
>
> Best regards,
> Krzysztof
Attachment:
signature.asc
Description: This is a digitally signed message part.
