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

On Wed, 12 Aug 2026 12:40:47 +0300
Matti Vaittinen <[email protected]> wrote:

> On 12/08/2026 11:25, Andreas Kemnade wrote:
> > On Wed, 12 Aug 2026 11:17:36 +0300
> > Andy Shevchenko <[email protected]> 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 <[email protected]> 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.

here is the special case that the sign bit is detached from the rest
Bit6/14 is unused. The other problem would be the name of the macro.
Probably nothing more meaningful than the code.

> 
> 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.
>
Looking at it: The relationship between tmp and tmp_curr.
IMHO reading into an u8 array and doing get_unaligned_be16()
would be the best improvment of readablity. 

> 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.
> 
Yes, that is a good point. 

Regards,
Andreas