Re: [RFC] iio: light: tcs3472: implementing wait time TODO

[Aldo Conte]

On 4/26/26 12:48, Jonathan Cameron wrote:
> On Sat, 25 Apr 2026 18:11:23 -0500
> David Lechner <dlechner@xxxxxxxxxxxx> wrote:
>
> > On 4/25/26 2:00 PM, Aldo Conte wrote:
> > > On 4/25/26 18:49, David Lechner wrote:
> > > > On 4/25/26 11:28 AM, Aldo Conte wrote:
> > > > > Hi all,
> > > > >
> > > > > I'd like to resolve the wait time TODO in tcs3472.c.
> > > >
> > > > Do you actually have this hardware to test it?
> > > >
> > > > What is the application that needs to make use of this feature?
> > > >   
> > > I'm currently participating in the 2026 linux kernel mentorship program. I don't really have an application that would use this feature, but in any case, I have the hardware (Adafruit TCS34725 breakout board to test with a Raspberry Pi 3B) to test everything out.
> >
> > Do you have a logic analyzer or oscilloscope too? Having a tool like that
> > is important to be able to see that the timing of the signals on the hardware
> > are matching what we have requested.
On the logic analyzer: I do have one, so I can monitor the I2C
bus directly.
> > I guess if not, there is the gpio-sloppy-logic-analyzer in the kernel. :-)
> > Real tools are of course much nicer.
> >
> > > As part of this experience, I'm focusing on “light” and, while looking through the TODO, i noticed this and wanted to explore it further.
> > >
> > >    
> > > > > The TCS3472 has a WTIME register and WEN bit that insert a low-power
> > > >
> > > > What about WLONG?
> > > >   
> > > > > wait state between RGBC cycles. The register is already defined in the driver but never used.
> > > > > I noticed that tsl2772.c enables wait with a fixed default and no
> > > > > userspace control. However, I think exposing the wait time to
> > > > > userspace would be more useful to tune the power/responsiveness tradeoff.
> > > >
> > > > I assume this would affect the effective sample rate?
> > > yes
> > > >   
> > > > > My plan would be to expose it via an ext_info attribute in
> > > > > microseconds, following the same convention as integration_time.
> > > > > Does that sound acceptable, or would you prefer a simpler approach
> > > > > with just a fixed default like tsl2772?
> > > >
> > > > So perhaps we could just use the standard sampling_frequency attribute?
> > >
> > > Just want to make sure I understand the sampling_frequency approach
> > > correctly before implementing.
> > >
> > > The total cycle time of the chip is:
> > >
> > >    cycle = wait_time + rgbc_init + integration_time
> > >          = (256 - WTIME) * 2.4ms + 2.4ms + (256 - ATIME) * 2.4ms
> > >
> > > So sampling_frequency = 1 / cycle.
> > >
> > > On a write, the driver would keep ATIME fixed (since the user
> > > controls it independently via integration_time) and solve for WTIME:
> > >
> > >    wait_time = (1 / requested_freq) - 2.4ms - atime_duration
> > >    WTIME = 256 - (wait_time / 2.4ms)
> > >
> > > For very low frequencies where the wait exceeds 614 ms, the driver
> > > would automatically enable WLONG in the CONFIG register to extend
> > > the step from 2.4 ms to 28.8 ms.
> > >
> > > But i wanto to clear my self this: changing integration_time would implicitly change
> > > the effective sampling_frequency since both contribute to the cycle
> > > time. Is that acceptable, or should the driver recalculate WTIME
> > > when ATIME changes to maintain the current sampling_frequency?
> >
> > This sounds like the right way to do it. It is normal for changing one
> > attribute to affect another attribute like this, so that is not a problem.
> >
> > Either way of handling WTIME is acceptable, but I think that recalculating
> > WTIME to keep as close as possible to requested_freq when ATIME changes is
> > more user-friendly.
>
> Agreed - given it is fairly easy in this case the user friendly route is
> the way to go even though the approach of one ABI element modifying the
> value of another is allowed we don't need to use that flexibility much
> here except at the boundaries where say increasing integration time
> means we can't meet a previous sampling_frequency target and hence have
> to increase it.
okay, then I'll proceed this way
> > >    
> > > > > I also plan a following patch converting the driver to devm.
> > > >
> > > > Would be better to do this first before adding new features.
> > > >   
> > > ok i could bring it up in the series assuming instead that the WTIME new feature is actually wanted, since I don't have a real-world application.
> >
> > If it would require a custom attribute, I would say wait until we really
> > need it. But this sounds fairly straight-forward, so I would say go for it.
> > You never know who might want to make use of that feature. :-)
>
> So if I follow this correctly it is only useful if some sort of continuous sampling
> is going on. E.g. events?  If so that's fine as I assume it trades off
> power usage against responsiveness to light level changes and that's a reasonable
> thing to want to control.
On continuous sampling: yes, the driver currently operates in
continuous mode (PON=1, AEN=1) and the chip cycles autonomously
through RGBC conversions back-to-back. The wait state inserts a
low-power pause between cycles, trading responsiveness for power
savings. This is relevant both for polled reads and for threshold
event monitoring.

I can verify the effect with the logic analyzer by capturing the
I2C traffic during triggered buffer reads


I'll start with the devm conversion and follow up with the
sampling_frequency implementation.

Is it ok to make a patch series?
> > > > > Thanks,
> > > > > Aldo
> > > > >   
> > > >   
> > >