RE: [PATCH v2 6/9] iio: adc: rzt2h: expose sampling frequency

[Cosmin-Gabriel Tanislav]

> From: Jonathan Cameron <jic23@xxxxxxxxxx>
> Sent: Monday, September 14, 2026 12:13 AM
> 
> > Expose the sampling frequency as a per-channel IIO_CHAN_INFO_SAMP_FREQ
> > to let userspace control conversion time.
> >
> > Each channel conversion takes a fixed 13 ADCLK cycles plus the sample
> > time programmed in ADSSTRn, giving a rate of ADCLK / (13 + ADSSTRn).
> >
> > Read the ADCLK rate from the "adclk" clock to derive the frequency.
> >
> > Claim direct mode while writing so the rate cannot change during a
> > capture.
> >
> > Program the sample time into ADSSTRn for each enabled channel on single
> > reads and on buffer enable.
> >
> > Wait for the maximum amount of time a conversion can take + 1 jiffy for
> > the completion event to come after triggering a single read.
> >
> > Signed-off-by: Cosmin Tanislav <cosmin-gabriel.tanislav.xa@xxxxxxxxxxx>
> 
> There is a sashiko comment on this one (See below) but I think it's
> in the category of things that we don't are about if a timeout
> has occured.  Please sanity check but I'll not hold series for that
> one.
> 
> >
> > diff --git a/drivers/iio/adc/rzt2h_adc.c b/drivers/iio/adc/rzt2h_adc.c
> > index 0460dffe189e..8ba7b0fde8a1 100644
> > --- a/drivers/iio/adc/rzt2h_adc.c
> > +++ b/drivers/iio/adc/rzt2h_adc.c
> > @@ -2,6 +2,7 @@
> >
> >  #include <linux/bitfield.h>
> >  #include <linux/cleanup.h>
> > +#include <linux/clk.h>
> >  #include <linux/completion.h>
> >  #include <linux/delay.h>
> >  #include <linux/dma-mapping.h>
> 
> ..
> 
> > -	ret = wait_for_completion_timeout(&adc->completion, usecs_to_jiffies(1));
> > +	ret = wait_for_completion_timeout(&adc->completion, usecs_to_jiffies(5) + 1);
> >  	if (!ret) {
> >  		ret = -ETIMEDOUT;
> >  		goto disable;
> 
> Sashiko:
> [Severity: High]
> This is a pre-existing issue, but does branching to the disable path on a
> timeout leave the driver vulnerable to a race with the interrupt handler?
> 
> If the timeout expires and the hardware interrupt fires immediately after,
> the ISR can execute locklessly and call complete(&adc->completion).
> 
> Since the disable path calls rzt2h_adc_stop(), which only masks the interrupt
> at the device level without waiting for executing handlers via
> synchronize_irq(), could the delayed interrupt complete the structure after
> the next read operation calls reinit_completion() in rzt2h_adc_read_single()?
> 
> If this sequence occurs, the next read could return immediately with stale
> data, and its own triggered conversion could eventually interrupt and corrupt
> the subsequent read, leading to a persistent desynchronization.
> -
> With a timeout we never expect to see I don't think it makes sense
> to care too much about stale data.
> 

If a timeout occurs with 1-2 jiffies of leeway at standard CONFIG_HZ
values (plus however much time it takes for the user to read the next
value), something is terribly wrong. I don't think we need to handle
this situation.