linux-kernel - Re: [PATCH v3 1/9] mfd: max77705: max77705_charger: move active discharge setting to mfd parent

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <rwnwaen5hqoeieiubishsrzn3wnmhhpbaurx6oukos7ysjoppm@uh6m22r2cdps>
Date: Mon, 15 Sep 2025 17:42:51 +0200
From: Sebastian Reichel <sebastian.reichel@...labora.com>
To: Dzmitry Sankouski <dsankouski@...il.com>, Lee Jones <lee@...nel.org>
Cc: Chanwoo Choi <cw00.choi@...sung.com>, 
	Krzysztof Kozlowski <krzk@...nel.org>, Luca Ceresoli <luca.ceresoli@...tlin.com>, 
	Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>, linux-kernel@...r.kernel.org, linux-pm@...r.kernel.org
Subject: Re: [PATCH v3 1/9] mfd: max77705: max77705_charger: move active
 discharge setting to mfd parent

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@...il.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
> 

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)