Re: [RFC PATCH 5/6] iio: light: ROHM BU27034 Ambient Light Sensor

[Jonathan Cameron <jic23@kernel.org>]

On Tue, 28 Feb 2023 08:43:28 +0000
"Vaittinen, Matti" <Matti.Vaittinen@fi.rohmeurope.com> wrote:

> On 2/26/23 15:52, Jonathan Cameron wrote:
> > On Fri, 24 Feb 2023 12:41:46 +0200
> > Matti Vaittinen <mazziesaccount@gmail.com> wrote:
> >   
> >> On 2/22/23 18:15, Matti Vaittinen wrote:
> >>
> >> Well, this "works on my machine" - but I am slightly unhappy with this.
> >> I have a feeling I am effectively making a poor, reduced version of data
> >> buffering here. I am starting to think that I should
> >>
> >> a) Not start measurement at chip init. (saves power)
> >> b) Start measurement at raw-read and just block for damn long for each
> >> raw-read. Yep, it probably means that users who want to raw-read all
> >> channels will be blocking 4 * measurement time when they are reading all
> >> channels one after another. Yes, this is in worst case 4 * 400 mS.
> >> Horrible. But see (c) below.  
> > 
> > Hmm. Light sensors tend to be slow in some modes, but rarely do people actually
> > have such low light levels that they are using them with 400mS integration times.
> >   
> >> c) Implement triggered_buffer mode. Here my lack of IIO-experience shows
> >> up again. I have no idea if there is - or what is - the "de facto" way
> >> for implementing this when our device has no IRQ? I could cook-up some
> >> 'tiny bit shorter than the measurement time' period timer which would
> >> kick the driver to poll the VALID-bit - or, because we need anyways to
> >> poll the valid bit from process context - just a kthread which polls the
> >> VALID-bit. Naturally the thread/timer should be only activated when the
> >> trigger is enabled.  
> > 
> > Firstly you don't have to have a trigger. In a case where it's a bit hacky
> > and unlikely to be particularly useful for other devices, you can just implement
> > a buffer directly.  
> 
> This is the approach I took for the next attempt. I just used the 
> iio_kfifo_buffer.
> 
> > There are various options that exist..
> > 1) iio-trig-loop - this is nasty but occasionally useful approach.  You then
> >     make the iio_poll_func wait on the flag.  
> 
> I actually did take a look at this. The loop trigger had pretty much 
> everything I would have needed - except configurability from the driver. 

It's purpose was a originally a bit different, so I'm not surprised it
didn't really fit.  The target was a sensor that needed explicit triggering
but then took a while to get the data.  Aim was to grab data as quick as we
could.  So there were no problems with alignment.

> It had the enable/disable with protected start of the thread and the 
> thread stopping all in place. Really, as you said, the only thing that 
> was missing was "hinting the timing". For a moment I was playing with a 
> thought of trying to implement a simple generic thread-loop code which 
> could take the sleep-time + callback for 'ensuring we slept long enough' 
> + a callback for code to execute (collect data +  push to buffers) - but 
> it felt like re-implementing existing mechanisms. Besides, as you said, 
> I don't probably need a trigger
> 
> I'll do some clean-ups and look through the feedback and try to get the 
> v2 out still during this week.
> 
> Yours,
> 	-- Matti
>