Re: [PATCH 2/5] power: supply: bd71828: Fix current direction

[Andreas Kemnade <andreas@kemnade.info>]

On Mon, 10 Aug 2026 13:09:07 +0300
Matti Vaittinen <matti.vaittinen@linux.dev> wrote:

> From: Matti Vaittinen <mazziesaccount@gmail.com>
> 
> Both the current and the average current registers contain same
> 'direction' -bit to denote the direction of the current. The code
> reading these registers incorrectly caches the direction read from the
> first register, and uses it also for the second.
> 
> Fix this by initializing the direction bit for both register reads.
> 
> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
> Fixes: 5bff79dad20a ("power: supply: Add bd718(15/28/78) charger driver")

Weird. I reported that issue after you have submitted an RFC.
current_avg just changing direction after disconnecting power supply.

Then I had a fix to enable current_now property.
Somehow we thought that averaging current in the chip ist just weird.

But now it makes sense.

Tested-by: Andreas Kemnade <andreas@kemnade.info> # Kobo Clara 2e rev B

> ---
>  drivers/power/supply/bd71828-power.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/power/supply/bd71828-power.c b/drivers/power/supply/bd71828-power.c
> index e1ae5fe52e1d..9927a7e8934a 100644
> --- a/drivers/power/supply/bd71828-power.c
> +++ b/drivers/power/supply/bd71828-power.c
> @@ -192,12 +192,13 @@ static int bd71828_get_current_ds_adc(struct bd71828_power *pwr, int *curr, int
>  {
>  	__be16 tmp_curr;
>  	char *tmp = (char *)&tmp_curr;
> -	int dir = 1;
>  	int regs[] = { pwr->regs->ibat, pwr->regs->ibat_avg };
>  	int *vals[] = { curr, curr_avg };
>  	int ret, i;
>  
> -	for (dir = 1, i = 0; i < ARRAY_SIZE(regs); i++) {
> +	for (i = 0; i < ARRAY_SIZE(regs); i++) {
> +		int dir = 1;
> +
>  		ret = regmap_bulk_read(pwr->regmap, regs[i], &tmp_curr,
>  				       sizeof(tmp_curr));
>  		if (ret)

That code is complicated. What about a generic 16bit non_2complemented sign
to 2-complemented sign function/macro. Such would have prevented the error
in the first places. So &vals[i] = signbit_to_2complement(be16_to_cpu(tmp_curr), 15) * pwr->curr_factor

Regards,
Andreas