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

[Matti Vaittinen]

On 12/08/2026 11:25, Andreas Kemnade wrote:
> On Wed, 12 Aug 2026 11:17:36 +0300
> Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:
>
> > On Wed, Aug 12, 2026 at 10:10:50AM +0200, Andreas Kemnade wrote:
> > > On Mon, 10 Aug 2026 13:09:07 +0300
> > > Matti Vaittinen <matti.vaittinen@xxxxxxxxx> wrote:
> >
> > ...
> >
> > > 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
> >
> > Do you mean existing sign_extend32() from bitops.h?
>
> I don't think it does what is needed. Here we have
>
> 0000 0000 0000 0001 -> 1
> Usually we have
>
> 1111 1111 1111 1111 -> -1
>
> but here
>
> 1000 0000 0000 0001 -> -1
>
> If I do understand things correctly. I not find something useful
> in bitops.h.

Thanks for the review and testing Andreas! We share the same 
(mis?)understanding. ;)

I am not sure this is so widely used it'd warrant a generic macro. I am 
afraid that if someone encountered similar design, he would not expect a 
generic macro to exist. My gut feeling is that this is too marginal to 
be widely useful. I am not against it though, if someone wants to 
implement it.

What comes to complexity - I agree. The code is messy, but mostly 
because of "clever hacks" done so values are in arrays indexed by loop.

I, however, would like to see a minimal backportable fix, and 
potentially, larger clean-up which can then go in as a feature, without 
any rush.

Yours,
	-- Matti

--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

~~ When things go utterly wrong vim users can always type :help! ~~