Re: [RFC 1/1] iio: common: cros_ec_sensors: add extra sensor API

[Jonathan Cameron <jic23@jic23.retrosnub.co.uk>]

On Fri, 24 May 2019 17:28:42 -0700
Gwendal Grignou <gwendal@chromium.org> wrote:

> On Sun, May 5, 2019 at 8:36 AM Jonathan Cameron
> <jic23@jic23.retrosnub.co.uk> wrote:
> >
> > On Fri,  3 May 2019 12:54:46 +0200
> > Fabien Lahoudere <fabien.lahoudere@collabora.com> wrote:
> >  
> > > From: Gwendal Grignou <gwendal@chromium.org>
> > >
> > > Version 3 of the EC protocol provides min and max frequencies and fifo
> > > size for EC sensors.
> > > The driver allows EC to set parameters for version 3 and set default values
> > > for earlier versions.
> > > The sysfs entries are read only and cannot be used to impact on values.
> > >
> > > Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
> > > Signed-off-by: Fabien Lahoudere <fabien.lahoudere@collabora.com>  
> > Mostly changes in here will follow from comments on the cover letter
> > but a few other things inline.
> >
> > Thanks,
> >
> > Jonathan  
> > > ---
> > >  .../ABI/testing/sysfs-bus-iio-cros-ec         |  24 ++++
> > >  .../cros_ec_sensors/cros_ec_sensors_core.c    | 126 +++++++++++++++++-
> > >  .../linux/iio/common/cros_ec_sensors_core.h   |   7 +
> > >  include/linux/mfd/cros_ec_commands.h          |  21 +++
> > >  4 files changed, 177 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/Documentation/ABI/testing/sysfs-bus-iio-cros-ec b/Documentation/ABI/testing/sysfs-bus-iio-cros-ec
> > > index 0e95c2ca105c..85d266f4a57e 100644
> > > --- a/Documentation/ABI/testing/sysfs-bus-iio-cros-ec
> > > +++ b/Documentation/ABI/testing/sysfs-bus-iio-cros-ec
> > > @@ -26,3 +26,27 @@ Description:
> > >               driver and represents the sensor ID as exposed by the EC. This
> > >               ID is used by the Android sensor service hardware abstraction
> > >               layer (sensor HAL) through the Android container on ChromeOS.
> > > +
> > > +What:                /sys/bus/iio/devices/iio:deviceX/min_frequency
> > > +Date:                April 2019
> > > +KernelVersion:       5.0
> > > +Contact:     linux-iio@vger.kernel.org
> > > +Description:
> > > +             This attribute is exposed by CrOS EC sensors. It defines the
> > > +             minimum frequency used by the sensor.
> > > +
> > > +What:                /sys/bus/iio/devices/iio:deviceX/max_frequency
> > > +Date:                April 2019
> > > +KernelVersion:       5.0
> > > +Contact:     linux-iio@vger.kernel.org
> > > +Description:
> > > +             This attribute is exposed by CrOS EC sensors. It defines the
> > > +             maximum frequency used by the sensor.
> > > +  
> > See reply to the cover letter on these.  Bonus points if you implement them
> > with the read_avail callback.
> >  
> > > +What:                /sys/bus/iio/devices/iio:deviceX/max_events
> > > +Date:                April 2019
> > > +KernelVersion:       5.0
> > > +Contact:     linux-iio@vger.kernel.org
> > > +Description:
> > > +             This attribute is exposed by CrOS EC sensors. It defines the
> > > +             maximum number of events in the fifo.  
> >
> > Why does userspace care?  
> Is use to report the size of the hardware FIFO to Android, to enable
> batching mode:
> https://source.android.com/devices/sensors/batching.
> It is reported in fifoReservedEventCount or fifoMaxEventCount in the
> sensor_t structure described at
> https://source.android.com/devices/sensors/hal-interface#sensor_t.
> Having said that, batching is not terribly useful for tablet or
> laptop, but is critical for wearable.

Ok. So let me just check, in android terms, events are just readings
from the sensor?  These aren't events in the IIO sense of 'thresholds'
being passed?

If so can we map this to hwfifo_watermark[_available] using the range
descriptor? We have a bit of legacy around that with  hwfifo_watermark_max
and hwfifo_watermark_min also defined, but should probably move towards
the more standard _available form and deprecate the other two.

Jonathan