* [RFC] iio: light: tcs3472: implementing wait time TODO @ 2026-04-25 16:28 Aldo Conte 2026-04-25 16:49 ` David Lechner 0 siblings, 1 reply; 7+ messages in thread From: Aldo Conte @ 2026-04-25 16:28 UTC (permalink / raw) To: linux-iio Cc: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko, linux-kernel Hi all, I'd like to resolve the wait time TODO in tcs3472.c. The TCS3472 has a WTIME register and WEN bit that insert a low-power 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. 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? I also plan a following patch converting the driver to devm. Thanks, Aldo ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC] iio: light: tcs3472: implementing wait time TODO 2026-04-25 16:28 [RFC] iio: light: tcs3472: implementing wait time TODO Aldo Conte @ 2026-04-25 16:49 ` David Lechner 2026-04-25 19:00 ` Aldo Conte 0 siblings, 1 reply; 7+ messages in thread From: David Lechner @ 2026-04-25 16:49 UTC (permalink / raw) To: Aldo Conte, linux-iio Cc: Jonathan Cameron, Nuno Sá, Andy Shevchenko, linux-kernel 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? > > 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? > > 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? > > I also plan a following patch converting the driver to devm. Would be better to do this first before adding new features. > > Thanks, > Aldo > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC] iio: light: tcs3472: implementing wait time TODO 2026-04-25 16:49 ` David Lechner @ 2026-04-25 19:00 ` Aldo Conte 2026-04-25 23:11 ` David Lechner 0 siblings, 1 reply; 7+ messages in thread From: Aldo Conte @ 2026-04-25 19:00 UTC (permalink / raw) To: David Lechner, linux-iio Cc: Jonathan Cameron, Nuno Sá, Andy Shevchenko, linux-kernel 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. 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? >> >> 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. >> >> Thanks, >> Aldo >> > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC] iio: light: tcs3472: implementing wait time TODO 2026-04-25 19:00 ` Aldo Conte @ 2026-04-25 23:11 ` David Lechner 2026-04-26 10:48 ` Jonathan Cameron 0 siblings, 1 reply; 7+ messages in thread From: David Lechner @ 2026-04-25 23:11 UTC (permalink / raw) To: Aldo Conte, linux-iio Cc: Jonathan Cameron, Nuno Sá, Andy Shevchenko, linux-kernel 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. 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. > >>> >>> 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. :-) >>> >>> Thanks, >>> Aldo >>> >> > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC] iio: light: tcs3472: implementing wait time TODO 2026-04-25 23:11 ` David Lechner @ 2026-04-26 10:48 ` Jonathan Cameron 2026-04-26 14:37 ` Aldo Conte 0 siblings, 1 reply; 7+ messages in thread From: Jonathan Cameron @ 2026-04-26 10:48 UTC (permalink / raw) To: David Lechner Cc: Aldo Conte, linux-iio, Nuno Sá, Andy Shevchenko, linux-kernel 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. > > 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. > > > > >>> > >>> 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. > > >>> > >>> Thanks, > >>> Aldo > >>> > >> > > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC] iio: light: tcs3472: implementing wait time TODO 2026-04-26 10:48 ` Jonathan Cameron @ 2026-04-26 14:37 ` Aldo Conte 2026-04-26 15:51 ` David Lechner 0 siblings, 1 reply; 7+ messages in thread From: Aldo Conte @ 2026-04-26 14:37 UTC (permalink / raw) To: Jonathan Cameron, David Lechner Cc: linux-iio, Nuno Sá, Andy Shevchenko, linux-kernel 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 >>>>> >>>> >>> >> > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC] iio: light: tcs3472: implementing wait time TODO 2026-04-26 14:37 ` Aldo Conte @ 2026-04-26 15:51 ` David Lechner 0 siblings, 0 replies; 7+ messages in thread From: David Lechner @ 2026-04-26 15:51 UTC (permalink / raw) To: Aldo Conte, Jonathan Cameron Cc: linux-iio, Nuno Sá, Andy Shevchenko, linux-kernel On 4/26/26 9:37 AM, Aldo Conte wrote: > 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: ... > I'll start with the devm conversion and follow up with the > sampling_frequency implementation. > > Is it ok to make a patch series? Sure. :-) If it was me, I would send the devm one first and work on the new feature while I was waiting for review on the first one. But maybe you are really fast and will get it done all in one day. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-04-26 15:51 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-04-25 16:28 [RFC] iio: light: tcs3472: implementing wait time TODO Aldo Conte 2026-04-25 16:49 ` David Lechner 2026-04-25 19:00 ` Aldo Conte 2026-04-25 23:11 ` David Lechner 2026-04-26 10:48 ` Jonathan Cameron 2026-04-26 14:37 ` Aldo Conte 2026-04-26 15:51 ` David Lechner
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox