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

[Vaittinen, Matti]

On 2/26/23 15:52, Jonathan Cameron wrote:
> On Fri, 24 Feb 2023 12:41:46 +0200
> Matti Vaittinen <mazziesaccount@xxxxxxxxx> 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 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

-- 
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

~~ When things go utterly wrong vim users can always type :help! ~~