Hi Mehdi, Glad to see you active here :) Le Mon 02 Dec 24, 15:55, Mehdi Djait a écrit : > Hello Paul :) > > On Mon, Dec 02, 2024 at 11:06:59AM +0000, Jonathan Cameron wrote: > > On Sun, 1 Dec 2024 18:56:30 +0100 > > Paul Kocialkowski <paulk@xxxxxxxxxxx> wrote: > > > > > Hi Jonathan, > > > > > > Le Sun 01 Dec 24, 11:55, Jonathan Cameron a écrit : > > > > On Sat, 30 Nov 2024 18:42:12 +0100 > > > > Paul Kocialkowski <paulk@xxxxxxxxxxx> wrote: > > > > > > > > > The Texas Instruments OPT4048 is a XYZ tristimulus color sensor, > > > > > with an additional wide (visible + IR) channel. > > > > > > > > > > This driver implements support for all channels, with configurable > > > > > integration time and auto-gain. Both direct reading and > > > > > triggered-buffer modes are supported. > > > > > > > > > > Note that the Y channel is also reported as a separate illuminance > > > > > channel, for which a scale is provided (following the datasheet) to > > > > > convert it to lux units. Falling and rising thresholds are supported > > > > > for this channel. > > > > > > > > > > The device's interrupt can be used to sample all channels at the end > > > > > of conversion and is optional. > > > > > > > > > > Signed-off-by: Paul Kocialkowski <paulk@xxxxxxxxxxx> > > > > Hi Paul, > > > > > > > > Various comments inline. Most significant is that this seems to be > > > > suitable for a simple dataready trigger that will make your various > > > > interrupt and non interrupt flows more similar. > > > > > > And thanks for the fast review and insightful comments! > > > > > > I considered implementing a trigger in the driver, but the issue I found > > > is that the trigger is expected to be called from hard irq context, > > > while the new values are read in the bottom half. > > > > The trigger can be called from either the hard irq context or from > > a thread. See iio_trigger_poll_nested() > > There is a quirk that you then don't end up calling the registered > > hard irq handler for the trigger so sometimes a bit of fiddly code > > is needed to ensure timestamps etc are grabbed. Not sure that matters > > here. > > > > If the timestamps do matter: here is a (maybe relevant?) discussion for > an issue I faced with timestamps for a driver that supports both FIFO > and triggered buffer mode > > Please note that iio_trigger_poll_nested() was called > iio_trigger_poll_chained() back in that discussion. > > https://lore.kernel.org/linux-iio/Y+6QoBLh1k82cJVN@carbian/ Thanks for the hint! I'll definitely look it up for details. Cheers, Paul > > > I understand the triggered > > > buffer callbacks are executed as a thread as well, so there would be race > > > between the two which could result in previous values being returned. > > > > With the above nested call it is all run in the same thread > > See handle_nested_irq() in particular the function docs. > > https://elixir.bootlin.com/linux/v6.12.1/source/kernel/irq/chip.c#L459 > > > > > So I concluded that it was more beneficial to preserve the synchronous reading > > > mechanism over implementing the trigger. > > > > Definite preference for a trigger approach, but I may well still be missing > > a detail. > > -- > Kind Regards > Mehdi Djait -- Paul Kocialkowski, Independent contractor - sys-base - https://www.sys-base.io/ Free software developer - https://www.paulk.fr/ Expert in multimedia, graphics and embedded hardware support with Linux.
Attachment:
signature.asc
Description: PGP signature
