On Mon, May 27, 2019 at 2:18 AM Jonathan Cameron <jic23@xxxxxxxxxxxxxxxxxxxxx> wrote: > > On Fri, 24 May 2019 17:28:42 -0700 > Gwendal Grignou <gwendal@xxxxxxxxxxxx> wrote: > > > On Sun, May 5, 2019 at 8:36 AM Jonathan Cameron > > <jic23@xxxxxxxxxxxxxxxxxxxxx> wrote: > > > > > > On Fri, 3 May 2019 12:54:46 +0200 > > > Fabien Lahoudere <fabien.lahoudere@xxxxxxxxxxxxx> wrote: > > > > > > > From: Gwendal Grignou <gwendal@xxxxxxxxxxxx> > > > > > > > > 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@xxxxxxxxxxxx> > > > > Signed-off-by: Fabien Lahoudere <fabien.lahoudere@xxxxxxxxxxxxx> > > > 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@xxxxxxxxxxxxxxx > > > > +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@xxxxxxxxxxxxxxx > > > > +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@xxxxxxxxxxxxxxx > > > > +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? Yes [https://source.android.com/devices/sensors/hal-interface#sensors_event_t] > These aren't events in the IIO sense of 'thresholds' > being passed? Correct. > > 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. That make sense: hwfifo_watermark_available will be set to a single value, which match hwfifo_watermak_max. Cros EC has a limited support: - the size can not be changed - the EC will seldom fill the hwfifo, it fires interrupt to a hard coded threshold to avoid losing samples - the hwfifo is shared by all sensors - sampling_frequency attribute is used to force the EC to generate interrupt before the hwfifo is full. Replacing max_events with hwfifo_watermark_max works for me. Thanks, Gwendal. > > Jonathan > >
