Re: [PATCH 2/5] power: supply: bd71828: Fix current direction
Matti Vaittinen <[email protected]>
| Newsgroups | org.kernel.vger.linux-pm,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
Thanks for the review(s) Andy.
On 10/08/2026 20:18, Andy Shevchenko wrote:
> On Mon, Aug 10, 2026 at 01:09:07PM +0300, Matti Vaittinen wrote:
>
>> 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.
>
> ...
>
>> 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++) {
>
>
> While at it
>
> for (unsigned int i = 0; i < ARRAY_SIZE(regs); i++) {
>
>> + int dir = 1;
>
>
> ...and drop it from the top declaration.
Nope. I'm not a fan of introducing variables inside loop conditions.
It's too easy for me to miss them. And, even if I liked the idea (and I
don't), it is not a change related to the actual fix - and making it for
this loop only would make the style inconsistent in this file.
Yours,
-- Matti
--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland
~~ When things go utterly wrong vim users can always type :help! ~~