Re: [PATCH v5] iio: tcs3472: implement wait time and sampling frequency

[Jonathan Cameron]

On Tue,  9 Jun 2026 17:55:15 +0200
Aldo Conte <aldocontelk@xxxxxxxxx> wrote:

I tweaked the patch title to include light so we know what type of
sensor this is as well.

> The TCS3472 has a wait state controlled by the WEN bit in the ENABLE
> register and the WAIT register, with an additional WLONG bit in CONFIG
> that if set multiplies the wait step by 12. The driver previously
> defined TCS3472_WTIME but never used it leaving the TODO comment on
> the top of the source file.
> 
> Implement control of the wait time through IIO_CHAN_INFO_SAMP_FREQ:
> 
>   - Reading sampling_frequency returns the chip's current cycle time,
>     computed as the sum of ATIME, the fixed RGBC initialization time
>     and the wait time (which depends on WEN and WLONG).
> 
>   - Writing sampling_frequency programs WTIME so that the resulting
>     cycle period approximates the requested frequency. If the
>     requested frequency cannot be reached with any
>     non-zero wait time, WEN is disabled and the chip runs
>     back-to-back conversions at the maximum rate allowed by ATIME.
>     If the requested period exceeds the maximum WTIME range, WLONG
>     is enabled to extend the wait step from 2.4 ms to 28.8 ms.
> 
>   - The user's last requested frequency is stored in the driver's
>     private data so that subsequent changes to integration_time
>     recompute WTIME and preserve the requested sampling rate as
>     closely as possible.
> 
> Add TCS3472_ENABLE_WEN, TCS3472_ENABLE_RUN and TCS3472_CONFIG_WLONG
> bit definitions. TCS3472_ENABLE_RUN bundles the bits
> (AEN | PON | WEN) that are simultaneously set when the chip is in
> running state and cleared during powerdown, and is used by
> tcs3472_probe(), tcs3472_powerdown().
> 
> Add a u8 enable_pre_suspend field to struct tcs3472_data:
> tcs3472_powerdown() snapshots data->enable into it, and
> tcs3472_resume() restores enable register content from the snapshot.
> This preserves the user's WEN choice across suspend/resume.
> 
> Bound tcs3472_req_data() polling to the worst-case cycle time
> (~8 seconds with ATIME=0x00, WTIME=0x00, WLONG=1).
> 
> Fix the event period calculation in tcs3472_read_event() and
> tcs3472_write_event() to use tcs3472_cycle_time_us() instead of
> ATIME alone. With WEN enabled, the chip cycle includes the wait
> time now.
> 
> Remove the "TODO: wait time" comment at the top of the file.
> 
> Reported-by: Sashiko <sashiko@xxxxxxxxxxxxxxx>
> Closes: https://sashiko.dev/#/patchset/20260607112713.299968-1-aldocontelk%40gmail.com

These are (I think) for tweaks to a patch we hadn't merged.
As such they shouldn't be here. Thus stuff belongs in a patch
that simply addresses some feedback from the bot (and nothing else!)

I tweaked as Andy suggested + the stuff mentioned above whilst applying

Applied to the testing branch of iio.git. Note this will be rebased on rc1 once
available before it turns up in linux-next etc.

Thanks,

Jonathan


> Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxx>
> Signed-off-by: Aldo Conte <aldocontelk@xxxxxxxxx>