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

[Aldo Conte <aldocontelk@gmail.com>]

On 4/26/26 12:48, Jonathan Cameron wrote:
> On Sat, 25 Apr 2026 18:11:23 -0500
> David Lechner <dlechner@baylibre.com> 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
>>>>>   
>>>>   
>>>    
>>
>