Re: [PATCH 3/3] iio: light: vl6180: Add support for Continuous Mode

[Jonathan Cameron <jic23@kernel.org>]

On Sat, 5 Oct 2024 22:42:43 +0530
Abhash jha <abhashkumarjha123@gmail.com> wrote:

> > Hi Abhash,
> >
> > Some comments below.
> >  
> Hi Jonathan,
> I will do the fixes and send a v3.
> 
> I have a question though:
> The device has a 8 x 16-bit HW-buffer.
> I want to implement the HW buffer support. Where in this driver should
> I read the hardware buffer?

If it were a fifo there are lots of examples in tree, but those tend
to 'empty' on read. From your description I'm not sure this one does.

> How is that exposed to userspace? Is it even exposed?
> There is no buffer-full interrupt, It just has the latest 16 range
> measurements and
> latest 8 ALS measurements.

Ah. Can we tell if the data is new vs data we have already read?
From a quick glance looks like you can clear it, so maybe we can use that
though we'll have to be careful about races.  Do we have to stop
continuous mode to clear it? Sort of looks like that's the case from
the description of the clear not occuring until a start_stop write.

However we'll have to dead reckon the timing without an interrupt.
That should be fine, as just configure it to max say 3/4 of the
time to fill it.

> 
> There is also a SYSTEM_HISTORY_CTRL register, which configures the HW buffer,
> like setting which data to capture (ALS/RANGE) as well as turning the
> buffer on/off.
> Where should all this configuration be done?
> Should it be default or have some sysfs attribute associated with it?

So if it were a conventional fifo it would  mostly be hidden behind the
software fifo and just act as an optimization of the data capture.
A few things are exposed though as can make a difference to how you configure
the device such as the size of the hardware fifo and the hwfifo threshold
(affects latency of data capture).  However sounds like you don't have
that here.

So challenging feature to support.
> 
> Thanks,
> Abhash