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

[Jonathan Cameron <jic23@kernel.org>]

On Sun, 10 May 2026 22:34:35 +0300
Md Shofiqul Islam <shofiqtest@gmail.com> wrote:

> At the lowest supported data rate of 250Hz, one conversion period is
> 4ms, not 40ms. The 50ms timeout is deliberately conservative to allow
> for kernel scheduling latency, which can be significant under load or
> on slow machines.
> 
> Fix the comment to state the correct conversion time, use "lowest sample
> rate" for clarity, and explain that the extra margin exists to absorb
> scheduling latency so that no one is tempted to shrink the timeout to
> match the conversion period.
> 
> Also drop the redundant ret variable assignment by using the return value
> of wait_for_completion_timeout() directly in the if() condition.
> 
> Signed-off-by: Md Shofiqul Islam <shofiqtest@gmail.com>
Hi. Please slow down a bit and perhaps more importantly be careful
with your various series numbers etc.  I see the same patch in a series
label v4 then as a patch on it's own labelled v2, one labelled v3 and
2 patches labeled v4 sent a few minute apart

I'm going to pick up the most recent one probably but that might
well be the wrong one!


> ---
> Changes in v4:
>  - Drop the ret assignment for wait_for_completion_timeout(); use the
>    return value directly in the if() condition (Andy Shevchenko)
> 
> Changes in v3:
>  - Fix multi-line comment style: move comment text to its own line
>    after '/*' (Andy Shevchenko)
> 
> Changes in v2:
>  - Correct conversion time to 4ms (was wrong 40ms), add scheduling
>    latency explanation, clarify "lowest sample rate" (David Lechner,
>    Andy Shevchenko)
> 
>  drivers/iio/adc/ti-ads1298.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/adc/ti-ads1298.c b/drivers/iio/adc/ti-ads1298.c
> index ae30b47e45..25261163d3 100644
> --- a/drivers/iio/adc/ti-ads1298.c
> +++ b/drivers/iio/adc/ti-ads1298.c
> @@ -210,9 +210,11 @@ static int ads1298_read_one(struct ads1298_private *priv, int chan_index)
>  		return ret;
>  	}
>  
> -	/* Cannot take longer than 40ms (250Hz) */
> -	ret = wait_for_completion_timeout(&priv->completion, msecs_to_jiffies(50));
> -	if (!ret)
> +	/*
> +	 * One conversion takes at most 4ms at the lowest sample rate (250Hz).
> +	 * Use 50ms to allow for kernel scheduling latency.
> +	 */
> +	if (!wait_for_completion_timeout(&priv->completion, msecs_to_jiffies(50)))
>  		return -ETIMEDOUT;
>  
>  	return 0;