Re: [PATCH v4 2/3] iio: adc: ti-ads1298: Fix incorrect timeout comment

[David Lechner]

On 5/9/26 10:19 AM, Md Shofiqul Islam wrote:
> At the lowest supported data rate of 250Hz, one conversion period is
> 4ms, not 40ms. Fix the comment to correctly reflect the timing.
> The 50ms timeout value itself is correct as a conservative margin.
> 
> Signed-off-by: Md Shofiqul Islam <shofiqtest@xxxxxxxxx>
> ---
>  drivers/iio/adc/ti-ads1298.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/adc/ti-ads1298.c b/drivers/iio/adc/ti-ads1298.c
> index cf5f954206..186bda3087 100644
> --- a/drivers/iio/adc/ti-ads1298.c
> +++ b/drivers/iio/adc/ti-ads1298.c
> @@ -210,7 +210,7 @@ static int ads1298_read_one(struct ads1298_private *priv, int chan_index)
>  		return ret;
>  	}
>  
> -	/* Cannot take longer than 40ms (250Hz) */
> +	/* Cannot take longer than 4ms at the lowest rate (250Hz) */
>  	ret = wait_for_completion_timeout(&priv->completion, msecs_to_jiffies(50));

I would say "lowest sample rate" so we know which rate it is talking about.

However, there could be latency in the kernel delaying the interrupt from
firing. The kernel latency can be much larger (I've seen 100s of ms on old
single core ARM CPUs). So I think we should mention that in the comment as
well so that no one is tempted to set it to msecs_to_jiffies(5) (or 4). Even
if that works most of the time on a fast machine, we may need the longer
timeout on slower machines.


>  	if (!ret)
>  		return -ETIMEDOUT;