Re: [PATCH v3 02/11] hwmon: chipcap2: fix off-by-one in command mode retry loop

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] hwmon: chipcap2: premature timeout due to 1-jiffy wait_for_completion_timeout
--

commit 529f4f489744495e9cdde8072b4894cc25895048
Author: Javier Carrasco <javier.carrasco.cruz@gmail.com>

hwmon: chipcap2: fix off-by-one in command mode retry loop

i == CC2_CM_RETRIES can never be true inside the loop, since it
only runs while i < CC2_CM_RETRIES. Use CC2_CM_RETRIES - 1 so the
last failed retry breaks out immediately instead of doing a useless
disable + 80 ms power-cycle delay before the loop condition ends it
anyway.

> diff --git a/drivers/hwmon/chipcap2.c b/drivers/hwmon/chipcap2.c
> index 8550578928523..8102b6851d3d1 100644
> --- a/drivers/hwmon/chipcap2.c
> +++ b/drivers/hwmon/chipcap2.c
> @@ -226,7 +226,7 @@ static int cc2_command_mode_start(struct cc2_data *data)
>  				     2 * CC2_RESP_START_CM_US);
>  		}
>  		ret = cc2_read_command_status(data->client);
> -		if (ret != -ETIMEDOUT || i == CC2_CM_RETRIES)
> +		if (ret != -ETIMEDOUT || i == CC2_CM_RETRIES - 1)

[Severity: High]
Can this still cause premature timeouts during device initialization and reads?

Looking at cc2_command_mode_start() just above this check, the timeout
calculation evaluates to exactly 1 jiffy on standard HZ configurations
(e.g., HZ=100 or HZ=250), because 200 microseconds is significantly less
than one tick:

	timeout = usecs_to_jiffies(2 * CC2_RESP_START_CM_US);
	ret = wait_for_completion_timeout(&data->complete, timeout);

When a 1-jiffy timeout is passed to wait_for_completion_timeout(), it can
expire almost instantly if the next timer interrupt occurs immediately after
the call.

This can cause the driver to prematurely abort the device initialization
without triggering a retry, returning -ETIMEDOUT sporadically. This leads
to random failures in device communication and user-space reads even if
the device properly signals completion within its 100-microsecond window.

This same 1-jiffy timeout issue also affects the EEPROM read sequence in
cc2_read_reg():

	timeout = usecs_to_jiffies(2 * CC2_RESP_EEPROM_R_US);
	ret = wait_for_completion_timeout(&data->complete, timeout);

>  			break;
>  
>  		/* command window missed, prepare for a retry */

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014-chipcap2_locks-v3-0-27ebfc2c5b9e@gmail.com?part=2