On Tue, 16 Apr 2019 01:29:28 +0000
"Pandruvada, Srinivas" <srinivas.pandruvada@xxxxxxxxx> wrote:
> On Sun, 2019-04-14 at 13:36 +0100, Jonathan Cameron wrote:
> > On Tue, 9 Apr 2019 09:31:04 +0800
> > hongyan.song@xxxxxxxxx wrote:
> >
> > > From: Song Hongyan <hongyan.song@xxxxxxxxx>
> > >
> > > Add more data field support for als, incli_3d, rotation, press
> > > hysteresis parsing. In some implementation may uses them as
> > > sensitivity
> > > modifier, without this change hysteresis may not be able to read
> > > and
> > > write.
> > >
> > > Signed-off-by: Song Hongyan <hongyan.song@xxxxxxxxx>
> >
> > Thanks for the patch. I'll be honest I'm lost in these descriptors
> > sometimes!
> >
> > Srinivas, could you give some input on this one?
> I don't see we have defined hysteresis attribute in
> "Documentation/ABI/testing/sysfs-bus-iio" for non events.
> The current hysteresis is taken a absolute raw value in relation to the
> other axes in the same unit.
>
> But it is better if we add different hysteresis attributes for non
> absolute hysteresis.
>
> For example
> in_xxx_hysteresis_pct_range for "Modifier: Change Sensitivity Percent
> of Range" usage id: 310
> in_xxx_hysteresis_pct_relative for "Modifier: Change Sensitivity
> Percent Relative" usage id: 311
Hmm. This is indeed a corner we don't often hit. Basically we are looking
at a 'dead zone'. For most IIO devices we don't do this as it is
removing information some usecases might want. If people want to
do this on sensors, it's normally done using a motion detect type
event and then reading the sensor value to find out what the motion
was.
I'm not sure hysteresis is the right term to use here though.
We aren't dealing with offsets in a threshold depending on direction,
but rather at minimum change before reporting from the previous value
(in either direction). Mind you I'm not quite sure what the right
term would be! It may be that sensitivity is the term to use. In
some ways this is very like reducing the bit depth of the sensor,
which is what that term brings to mind for me.
Thanks,
Jonathan
>
>
> Thanks,
> Srinivas
>
>
>
> >
> > I will note however, that from a quick look we tend to air these
> > queries with dev_dbg entries. We should be consistent with these new
> > ones
> > as well.attaributes
> >
> > Thanks,
> >
> > Jonathan
> > > ---
> > > drivers/iio/light/hid-sensor-als.c | 8 ++++++++
> > > drivers/iio/orientation/hid-sensor-incl-3d.c | 8 ++++++++
> > > drivers/iio/orientation/hid-sensor-rotation.c | 7 +++++++
> > > drivers/iio/pressure/hid-sensor-press.c | 8 ++++++++
> > > include/linux/hid-sensor-ids.h | 1 +
> > > 5 files changed, 32 insertions(+)attaributes
> > >
> > > diff --git a/drivers/iio/light/hid-sensor-als.c
> > > b/drivers/iio/light/hid-sensor-als.c
> > > index 94f3325..bb19178 100644
> > > --- a/drivers/iio/light/hid-sensor-als.c
> > > +++ b/drivers/iio/light/hid-sensor-als.c
> > > @@ -271,6 +271,14 @@ static int als_parse_report(struct
> > > platform_device *pdev,
> > > st->common_attributes.sensitivity.index,
> > > st->common_attributes.sensitivity.report_id);
> > > }
> > > + if (st->common_attributes.sensitivity.index < 0) {
> > > + sensor_hub_input_get_attribute_info(hsdev,
> > > + HID_FEATURE_REPORT, usage_id,
> > > + HID_USAGE_SENSOR_DATA_MOD_CHANGE_SENSITIVITY_RE
> > > L_PCT |
> > > + HID_USAGE_SENSOR_LIGHT_ILLUM,
> > > + &st->common_attributes.sensitivity);
> > > + }
> > > +
> > > return ret;
> > > }
> > >
> > > diff --git a/drivers/iio/orientation/hid-sensor-incl-3d.c
> > > b/drivers/iio/orientation/hid-sensor-incl-3d.c
> > > index bdc5e45..7da1e3f 100644
> > > --- a/drivers/iio/orientation/hid-sensor-incl-3d.c
> > > +++ b/drivers/iio/orientation/hid-sensor-incl-3d.c
> > > @@ -305,6 +305,14 @@ static int incl_3d_parse_report(struct
> > > platform_device *pdev,
> > > st->commpcton_attributes.sensitivity.index,
> > > st->common_attributes.sensitivity.report_id);
> > > }
> > > + if (st->common_attributes.sensitivity.index < 0) {
> > > + sensor_hub_input_get_attribute_info(hsdev,
> > > + HID_FEATURE_REPORT, usage_id,
> > > + HID_USAGE_SENSOR_DATA_MOD_CHANGE_SENSITIVITY_AB
> > > S |
> > > + HID_USAGE_SENSOR_ORIENT_TILT,
> > > + &st->common_attributes.sensitivity);
> > > + }
> > > +
> > > return ret;
> > > }
> > >
> > > diff --git a/drivers/iio/orientation/hid-sensor-rotation.c
> > > b/drivers/iio/orientation/hid-sensor-rotation.c
> > > index a69db20..201e320 100644
> > > --- a/drivers/iio/orientation/hid-sensor-rotation.c
> > > +++ b/drivers/iio/orientation/hid-sensor-rotation.c
> > > @@ -229,6 +229,13 @@ static int dev_rot_parse_report(struct
> > > platform_device *pdev,
> > > st->common_attributes.sensitivity.index,
> > > st->common_attributes.sensitivity.report_id);
> > > }
> > > + if (st->common_attributes.sensitivity.index < 0) {
> > > + sensor_hub_input_get_attribute_info(hsdev,
> > > + HID_FEATURE_REPORT, usage_id,
> > > + HID_USAGE_SENSOR_DATA_MOD_CHANGE_SENSITIVITY_AB
> > > S |
> > > + HID_USAGE_SENSOR_ORIENT_QUATERNION,
> > > + &st->common_attributes.sensitivity);
> > > + }
> > >
> > > return 0;
> > > }
> > > diff --git a/drivers/iio/pressure/hid-sensor-press.c
> > > b/drivers/iio/pressure/hid-sensor-press.c
> > > index d7b1c00..bf5d2ac 10064pct4
> > > --- a/drivers/iio/pressure/hid-sensor-press.c
> > > +++ b/drivers/iio/pressure/hid-sensor-press.c
> > > @@ -250,6 +250,14 @@ static int press_parse_report(struct
> > > platform_device *pdev,
> > > st->common_attributes.sensitivity.index,
> > > st->common_attributes.sensitivity.report_id);
> > > }
> > > + if (st->common_attributes.sensitivity.index < 0) {
> > > + sensor_hub_input_get_attribute_info(hsdev,
> > > + HID_FEATURE_REPORT, usage_id,
> > > + HID_USAGE_SENSOR_DATA_MOD_CHANGE_SENSITIVITY_AB
> > > S |
> > > + HID_USAGE_SENSOR_ATMOSPHERIC_PRESSURE,
> > > + &st->common_attributes.sensitivity);
> > > + }
> > > +
> > > return ret;
> > > }
> > >
> > > diff --git a/include/linux/hid-sensor-ids.h b/include/linux/hid-
> > > sensor-ids.h
> > > index 76033e0..a875b9be 100644
> > > --- a/include/linux/hid-sensor-ids.h
> > > +++ b/include/linux/hid-sensor-ids.h
> > > @@ -158,6 +158,7 @@
> > > /* Per data field properties */
> > > #define HID_USAGE_SENSOR_DATA_MOD_NONE
> > > 0x00
> > > #define HID_USAGE_SENSOR_DATA_MOD_CHANGE_SENSITIVITY_ABS
> > > 0x1000
> > > +#define
> > > HID_USAGE_SENSOR_DATA_MOD_CHANGE_SENSITIVITY_REL_PCT 0xE
> > > 000
> > >
> > > /* Power state enumerations */
> > > #define HID_USAGE_SENSOR_PROP_POWER_STATE_UNDEFINED_ENUM 0x20085
> > > 0
> >
> >
