Active discharge setting is a part of MFD top level i2c device, hence
cannot be controlled by charger. Writing to MAX77705_PMIC_REG_MAINCTRL1
register from charger driver is a mistake.
Move active discharge setting to MFD parent driver.
Fixes: a6a494c8e3ce ("power: supply: max77705: Add charger driver for Maxim 77705")
Signed-off-by: Dzmitry Sankouski <dsankouski@gmail.com>
---
drivers/mfd/max77705.c | 3 +++
drivers/power/supply/max77705_charger.c | 3 ---
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/mfd/max77705.c b/drivers/mfd/max77705.c
index 6b263bacb8c2..ff07d0e0d5f8 100644
--- a/drivers/mfd/max77705.c
+++ b/drivers/mfd/max77705.c
@@ -108,6 +108,9 @@ static int max77705_i2c_probe(struct i2c_client *i2c)
if (pmic_rev != MAX77705_PASS3)
return dev_err_probe(dev, -ENODEV, "Rev.0x%x is not tested\n", pmic_rev);
+ /* Active Discharge Enable */
+ regmap_update_bits(max77705->regmap, MAX77705_PMIC_REG_MAINCTRL1, 1, 1);
+
ret = devm_regmap_add_irq_chip(dev, max77705->regmap,
i2c->irq,
IRQF_ONESHOT | IRQF_SHARED, 0,
diff --git a/drivers/power/supply/max77705_charger.c b/drivers/power/supply/max77705_charger.c
index 329b430d0e50..3b75c82b9b9e 100644
--- a/drivers/power/supply/max77705_charger.c
+++ b/drivers/power/supply/max77705_charger.c
@@ -487,9 +487,6 @@ static void max77705_charger_initialize(struct max77705_charger_data *chg)
regmap_update_bits(regmap, MAX77705_CHG_REG_CNFG_00,
MAX77705_WDTEN_MASK, 0);
- /* Active Discharge Enable */
- regmap_update_bits(regmap, MAX77705_PMIC_REG_MAINCTRL1, 1, 1);
-
/* VBYPSET=5.0V */
regmap_update_bits(regmap, MAX77705_CHG_REG_CNFG_11, MAX77705_VBYPSET_MASK, 0);
--
2.39.5
On Thu, 11 Sep 2025, Dzmitry Sankouski wrote: > Active discharge setting is a part of MFD top level i2c device, hence > cannot be controlled by charger. Writing to MAX77705_PMIC_REG_MAINCTRL1 > register from charger driver is a mistake. > > Move active discharge setting to MFD parent driver. > > Fixes: a6a494c8e3ce ("power: supply: max77705: Add charger driver for Maxim 77705") > > Signed-off-by: Dzmitry Sankouski <dsankouski@gmail.com> > --- > drivers/mfd/max77705.c | 3 +++ > drivers/power/supply/max77705_charger.c | 3 --- > 2 files changed, 3 insertions(+), 3 deletions(-) Acked-by: Lee Jones <lee@kernel.org> > diff --git a/drivers/mfd/max77705.c b/drivers/mfd/max77705.c > index 6b263bacb8c2..ff07d0e0d5f8 100644 > --- a/drivers/mfd/max77705.c > +++ b/drivers/mfd/max77705.c > @@ -108,6 +108,9 @@ static int max77705_i2c_probe(struct i2c_client *i2c) > if (pmic_rev != MAX77705_PASS3) > return dev_err_probe(dev, -ENODEV, "Rev.0x%x is not tested\n", pmic_rev); > > + /* Active Discharge Enable */ > + regmap_update_bits(max77705->regmap, MAX77705_PMIC_REG_MAINCTRL1, 1, 1); > + > ret = devm_regmap_add_irq_chip(dev, max77705->regmap, > i2c->irq, > IRQF_ONESHOT | IRQF_SHARED, 0, > diff --git a/drivers/power/supply/max77705_charger.c b/drivers/power/supply/max77705_charger.c > index 329b430d0e50..3b75c82b9b9e 100644 > --- a/drivers/power/supply/max77705_charger.c > +++ b/drivers/power/supply/max77705_charger.c > @@ -487,9 +487,6 @@ static void max77705_charger_initialize(struct max77705_charger_data *chg) > regmap_update_bits(regmap, MAX77705_CHG_REG_CNFG_00, > MAX77705_WDTEN_MASK, 0); > > - /* Active Discharge Enable */ > - regmap_update_bits(regmap, MAX77705_PMIC_REG_MAINCTRL1, 1, 1); > - > /* VBYPSET=5.0V */ > regmap_update_bits(regmap, MAX77705_CHG_REG_CNFG_11, MAX77705_VBYPSET_MASK, 0); > > > -- > 2.39.5 > -- Lee Jones [李琼斯]
Hi, On Thu, Sep 11, 2025 at 08:57:09PM +0300, Dzmitry Sankouski wrote: > Active discharge setting is a part of MFD top level i2c device, hence > cannot be controlled by charger. Writing to MAX77705_PMIC_REG_MAINCTRL1 > register from charger driver is a mistake. > > Move active discharge setting to MFD parent driver. This is quite confusing to read without knowing the driver. I think it's betterly described like this: Active discharge setting is a part of MFD top level register range, hence cannot be controlled by the charger regmap, which only offers access to the charger registers. Move active discharge setting to MFD parent driver to fix the issue. > > Fixes: a6a494c8e3ce ("power: supply: max77705: Add charger driver for Maxim 77705") > No newline after Fixes: > Signed-off-by: Dzmitry Sankouski <dsankouski@gmail.com> > --- > drivers/mfd/max77705.c | 3 +++ > drivers/power/supply/max77705_charger.c | 3 --- > 2 files changed, 3 insertions(+), 3 deletions(-) I think it's sensible to use 'mfd: max77705:' prefix for this patch, otherwise Lee easily misses that this patchset has anything to do with his subsystem :) Greetings, -- Sebastian > > diff --git a/drivers/mfd/max77705.c b/drivers/mfd/max77705.c > index 6b263bacb8c2..ff07d0e0d5f8 100644 > --- a/drivers/mfd/max77705.c > +++ b/drivers/mfd/max77705.c > @@ -108,6 +108,9 @@ static int max77705_i2c_probe(struct i2c_client *i2c) > if (pmic_rev != MAX77705_PASS3) > return dev_err_probe(dev, -ENODEV, "Rev.0x%x is not tested\n", pmic_rev); > > + /* Active Discharge Enable */ > + regmap_update_bits(max77705->regmap, MAX77705_PMIC_REG_MAINCTRL1, 1, 1); > + > ret = devm_regmap_add_irq_chip(dev, max77705->regmap, > i2c->irq, > IRQF_ONESHOT | IRQF_SHARED, 0, > diff --git a/drivers/power/supply/max77705_charger.c b/drivers/power/supply/max77705_charger.c > index 329b430d0e50..3b75c82b9b9e 100644 > --- a/drivers/power/supply/max77705_charger.c > +++ b/drivers/power/supply/max77705_charger.c > @@ -487,9 +487,6 @@ static void max77705_charger_initialize(struct max77705_charger_data *chg) > regmap_update_bits(regmap, MAX77705_CHG_REG_CNFG_00, > MAX77705_WDTEN_MASK, 0); > > - /* Active Discharge Enable */ > - regmap_update_bits(regmap, MAX77705_PMIC_REG_MAINCTRL1, 1, 1); > - > /* VBYPSET=5.0V */ > regmap_update_bits(regmap, MAX77705_CHG_REG_CNFG_11, MAX77705_VBYPSET_MASK, 0); > > > -- > 2.39.5 >
On Mon, 15 Sep 2025, Sebastian Reichel wrote: > Hi, > > On Thu, Sep 11, 2025 at 08:57:09PM +0300, Dzmitry Sankouski wrote: > > Active discharge setting is a part of MFD top level i2c device, hence > > cannot be controlled by charger. Writing to MAX77705_PMIC_REG_MAINCTRL1 > > register from charger driver is a mistake. > > > > Move active discharge setting to MFD parent driver. > > This is quite confusing to read without knowing the driver. I think > it's betterly described like this: > > Active discharge setting is a part of MFD top level register range, > hence cannot be controlled by the charger regmap, which only offers > access to the charger registers. > > Move active discharge setting to MFD parent driver to fix the issue. > > > > > Fixes: a6a494c8e3ce ("power: supply: max77705: Add charger driver for Maxim 77705") > > > > No newline after Fixes: > > > Signed-off-by: Dzmitry Sankouski <dsankouski@gmail.com> > > --- > > drivers/mfd/max77705.c | 3 +++ > > drivers/power/supply/max77705_charger.c | 3 --- > > 2 files changed, 3 insertions(+), 3 deletions(-) > > I think it's sensible to use 'mfd: max77705:' prefix for this patch, > otherwise Lee easily misses that this patchset has anything to do > with his subsystem :) =:-) -- Lee Jones [李琼斯]
© 2016 - 2025 Red Hat, Inc.