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! ~~
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.