Octavian Purdila writes:
Hi everybody,
Hi Octavian.
I hope this RFC is a good starting point to discuss support for
hardware fifo in IIO. The main reason to support it is to reduce the
power consumtion, by allowing the CPU to enter deep sleep states for
longer periods of time.
A worthwhile aim indeed.
Don't get discourage by the large number of patches most of them are
refactors in the bmc150 driver, to make it easier to add support for
the hardware fifo (basically to make adding interrupts and
events/triggers easier).
For discussing the hardware fifo stuff, only the first and last
patches are important: the first adds new IIO attributes so that we
can expose the hardware fifo and the last implements hadware fifo for
IIO (as an example of how would a device use the exposed attributes).
Note that the attributes can be exposed on a per device or per channel
basis, since it seems both types of hardware fifos exists: those that
store all data in a single fifo (temperature, accelerometer,
magnetometer, etc.) and those that have separate fifos for
accelerometer, gyroscope, etc. Thankfully, at the driver level we just
need to use the appropriate sharing level to support one mode or the
other.
If this is the case, then the buffered outputs will have to be separate
so we know what is coming. E.g. it will have to be effectively treated
as separate iio_buffers. Now we have devices that already do (see some
of the more interesting SOC ADCs)
Is this actually the case for the bmc150? If you could point at
devices where it is then it would be great (as mentioned above one
or two SOC ADCs have a bonkers level of flexibility).
My reading of the datasheet is that the data is interleaved in the
buffer for any channels enable.
Also note that this patch is orthogonal to the software watermark /
batching patch send on the list a while back.
I'm not so sure this is orthogonal at all. That proposal was actually
about hardware support. I pushed back that I wanted any new interface
to work whether or not there was hardware support. In a sense that is
a more complex problem - hence the discussion became a little bit focused
on that case.
The intent there was to hide the implementation details of exactly this
sort of hardware/software buffer interaction.
Perhaps some history is in order.
We had exactly what you propose here a long time back with the sca3000
accelerometer (which is why there is still core support for hardware
buffers). The interface was precisely the watershed type you have
here.
A review by Arnd Bergman pointed out that this was seriously clunky.
It puts data related to the in-band data flow into our out-of-band channel.
His suggestion was to allow for watershed interrupts using one of the
more interesting POLL types leaving the main poll for the data flow.
Arnd also suggested the bits about using anonymous chardevs for the event
reporting etc. The unusual form of poll bit never got implemented,
but in the interests of moving forward with the common case and because
they were on there way out anyway the watershed interrupts were dropped.
The more recent discussion came to the conclusion that there was no need
to use the weird forms of poll. We could simply have an attribute to
control when poll was fired. The only bit that caused friction was whether
there should be a timeout or not.
Now, when then ti_am35xx driver came along it became clear that it wasn't
actually terribly useful exposing the hardware buffers directly to
user space. The buffers tend not to be terribly long (in your bma160) I
see they are only 32 elements. As such the conclusion was that it makes
more sense to use the buffers as temporary storage to smooth out the
data flow. Thus that driver fills a software buffer from a hardware
buffer. This seems the method that is most likely to work long term.
I note this is effectively what you have here, though I'm not sure of
the advantage of the explicit software flush over just reading the data
whenever the interrupt fires.
Hence we started with something that at least superficially (I haven't
had a chance to go through the implementation in detail yet)
looks similar to what you have but ended up changing the method of
signalling to and from userspace.
Hardware buffer -> Software buffer -> user space.
Userspace watershed control -> Software buffer watershed control
-> Hardware buffer watershed control.
If userspace sets the watershed to say 16 then, as well as setting
that level in the software buffer it should be passed on to the
device driver allowing the watershed there to be set appropriately.
Now things get interesting if userspace sets the watershed to a value
that makes no sense for the hardware (say 17 on a device that does
power of 2 values only) as then it will have to fall back to
grabbing every one (Watershed of 1). Perhaps we can provide 'hints'
for this?
Anyhow, having given this general comment, I'll give some feedback on
the implementation details.
Jonathan
Octavian Purdila (8):
iio: add support for hardware fifo
iio: bmc150: refactor slope duration and threshold update
iio: bmc150: refactor interrupt enabling
iio: bmc150: exit early if event / trigger state is not changed
iio: bmc150: introduce bmc150_accel_interrupt
iio: bmc150: introduce bmc150_accel_trigger
iio: bmc150: introduce bmc150_accel_event
iio: bmc150: add support for hardware fifo
Documentation/ABI/testing/sysfs-bus-iio | 51 ++
drivers/iio/accel/bmc150-accel.c | 976 ++++++++++++++++++++++----------
drivers/iio/industrialio-core.c | 2 +
drivers/iio/industrialio-event.c | 2 +
include/linux/iio/iio.h | 17 +
include/linux/iio/types.h | 2 +
6 files changed, 739 insertions(+), 311 deletions(-)
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
