[PATCH 0/3] Add ChromeOS EC Activity Sensors driver

[PATCH 0/3] Add ChromeOS EC Activity Sensors driver

[Thierry Escande]

Hi,

This patchset adds support for an activity sensors driver handling
activity/gesture recognition coming from the EC. Only significant motion
is currently supported. Moreover it is an incomplete driver: activity can
be set and ring buffer must be used to receive interruption.

This patchset depends on [1] to apply (which is merged into linux-next).

[1] https://lkml.org/lkml/2016/8/1/141

Regards,
 Thierry

Gwendal Grignou (3):
  mfd: cros_ec: update MOTIONSENSE definitions and commands
  iio: core: Add double tap as possible gesture
  iio: cros_ec_activity: add ChromeOS EC Activity Sensors

 drivers/iio/common/cros_ec_sensors/Kconfig         |  10 +
 drivers/iio/common/cros_ec_sensors/Makefile        |   1 +
 .../iio/common/cros_ec_sensors/cros_ec_activity.c  | 300 +++++++++++++++++++++
 .../common/cros_ec_sensors/cros_ec_sensors_core.c  |  24 ++
 .../common/cros_ec_sensors/cros_ec_sensors_core.h  |   1 +
 drivers/iio/industrialio-core.c                    |   1 +
 include/linux/mfd/cros_ec_commands.h               |  35 +++
 include/uapi/linux/iio/types.h                     |   1 +
 8 files changed, 373 insertions(+)
 create mode 100644 drivers/iio/common/cros_ec_sensors/cros_ec_activity.c

-- 
2.7.4

[PATCH 1/3] mfd: cros_ec: Add activity motion sense definitions

[Thierry Escande]

From: Gwendal Grignou <gwendal@chromium.org>

This adds motion sense definitions to the commands header related to the
activity sensors attached behind the ChromeOS Embedded Controller.
Supported activities are MOTION and DOUBLE_TAP.

Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
---
 include/linux/mfd/cros_ec_commands.h | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/include/linux/mfd/cros_ec_commands.h b/include/linux/mfd/cros_ec_commands.h
index 1683003..80e6060 100644
--- a/include/linux/mfd/cros_ec_commands.h
+++ b/include/linux/mfd/cros_ec_commands.h
@@ -1417,6 +1417,18 @@ enum motionsense_command {
 	 */
 	MOTIONSENSE_CMD_SENSOR_OFFSET = 11,
 
+	/*
+	 * List available activities for a MOTION sensor.
+	 * Indicates if they are enabled or disabled.
+	 */
+	MOTIONSENSE_CMD_LIST_ACTIVITIES = 12,
+
+	/*
+	 * Activity management
+	 * Enable/Disable activity recognition.
+	 */
+	MOTIONSENSE_CMD_SET_ACTIVITY = 13,
+
 	/* Number of motionsense sub-commands. */
 	MOTIONSENSE_NUM_CMDS
 };
@@ -1494,6 +1506,21 @@ struct ec_response_motion_sensor_data {
 	};
 } __packed;
 
+/* List supported activity recognition */
+enum motionsensor_activity {
+	MOTIONSENSE_ACTIVITY_RESERVED = 0,
+	MOTIONSENSE_ACTIVITY_SIG_MOTION = 1,
+	MOTIONSENSE_ACTIVITY_DOUBLE_TAP = 2,
+};
+
+struct ec_motion_sense_activity {
+	uint8_t sensor_num;
+	uint8_t activity;       /* one of enum motionsensor_activity */
+	uint8_t enable;         /* 1: enable, 0: disable */
+	uint8_t reserved;
+	uint16_t parameters[3]; /* activity dependent parameters */
+};
+
 struct ec_params_motion_sense {
 	uint8_t cmd;
 	union {
@@ -1561,6 +1588,8 @@ struct ec_params_motion_sense {
 			/* Data to set or EC_MOTION_SENSE_NO_VALUE to read. */
 			int32_t data;
 		} sensor_odr, sensor_range;
+
+		struct ec_motion_sense_activity set_activity;
 	};
 } __packed;
 
@@ -1611,6 +1640,12 @@ struct ec_response_motion_sense {
 			int16_t temp;
 			int16_t offset[3];
 		} sensor_offset, perform_calib;
+
+		struct {
+			uint16_t reserved;
+			uint32_t enabled;
+			uint32_t disabled;
+		} __packed list_activities;
 	};
 } __packed;
 
-- 
2.7.4

Re: [PATCH 1/3] mfd: cros_ec: Add activity motion sense definitions

[Lee Jones]

On Mon, 05 Dec 2016, Thierry Escande wrote:

> From: Gwendal Grignou <gwendal@chromium.org>
> 
> This adds motion sense definitions to the commands header related to the
> activity sensors attached behind the ChromeOS Embedded Controller.
> Supported activities are MOTION and DOUBLE_TAP.
> 
> Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
> Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
> ---
>  include/linux/mfd/cros_ec_commands.h | 35 +++++++++++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)

I only see this patch?  Where are the other 2?

Do they depend on this one?

In the mean time:

For my own reference:
  Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>

> diff --git a/include/linux/mfd/cros_ec_commands.h b/include/linux/mfd/cros_ec_commands.h
> index 1683003..80e6060 100644
> --- a/include/linux/mfd/cros_ec_commands.h
> +++ b/include/linux/mfd/cros_ec_commands.h
> @@ -1417,6 +1417,18 @@ enum motionsense_command {
>  	 */
>  	MOTIONSENSE_CMD_SENSOR_OFFSET = 11,
>  
> +	/*
> +	 * List available activities for a MOTION sensor.
> +	 * Indicates if they are enabled or disabled.
> +	 */
> +	MOTIONSENSE_CMD_LIST_ACTIVITIES = 12,
> +
> +	/*
> +	 * Activity management
> +	 * Enable/Disable activity recognition.
> +	 */
> +	MOTIONSENSE_CMD_SET_ACTIVITY = 13,
> +
>  	/* Number of motionsense sub-commands. */
>  	MOTIONSENSE_NUM_CMDS
>  };
> @@ -1494,6 +1506,21 @@ struct ec_response_motion_sensor_data {
>  	};
>  } __packed;
>  
> +/* List supported activity recognition */
> +enum motionsensor_activity {
> +	MOTIONSENSE_ACTIVITY_RESERVED = 0,
> +	MOTIONSENSE_ACTIVITY_SIG_MOTION = 1,
> +	MOTIONSENSE_ACTIVITY_DOUBLE_TAP = 2,
> +};
> +
> +struct ec_motion_sense_activity {
> +	uint8_t sensor_num;
> +	uint8_t activity;       /* one of enum motionsensor_activity */
> +	uint8_t enable;         /* 1: enable, 0: disable */
> +	uint8_t reserved;
> +	uint16_t parameters[3]; /* activity dependent parameters */
> +};
> +
>  struct ec_params_motion_sense {
>  	uint8_t cmd;
>  	union {
> @@ -1561,6 +1588,8 @@ struct ec_params_motion_sense {
>  			/* Data to set or EC_MOTION_SENSE_NO_VALUE to read. */
>  			int32_t data;
>  		} sensor_odr, sensor_range;
> +
> +		struct ec_motion_sense_activity set_activity;
>  	};
>  } __packed;
>  
> @@ -1611,6 +1640,12 @@ struct ec_response_motion_sense {
>  			int16_t temp;
>  			int16_t offset[3];
>  		} sensor_offset, perform_calib;
> +
> +		struct {
> +			uint16_t reserved;
> +			uint32_t enabled;
> +			uint32_t disabled;
> +		} __packed list_activities;
>  	};
>  } __packed;
>  

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH 2/3] iio: core: Add double tap as possible gesture

[Thierry Escande]

From: Gwendal Grignou <gwendal@chromium.org>

This adds the IIO_MOD_DOUBLE_TAP entry to the iio_modifier enum and the
corresponding "double_tap" string to the iio_modifier_names array.

Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
---
 drivers/iio/industrialio-core.c | 1 +
 include/uapi/linux/iio/types.h  | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index fc340ed..82ce05f 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -120,6 +120,7 @@ static const char * const iio_modifier_names[] = {
 	[IIO_MOD_Q] = "q",
 	[IIO_MOD_CO2] = "co2",
 	[IIO_MOD_VOC] = "voc",
+	[IIO_MOD_DOUBLE_TAP] = "double_tap",
 };
 
 /* relies on pairs of these shared then separate */
diff --git a/include/uapi/linux/iio/types.h b/include/uapi/linux/iio/types.h
index 22e5e58..47d2768 100644
--- a/include/uapi/linux/iio/types.h
+++ b/include/uapi/linux/iio/types.h
@@ -80,6 +80,7 @@ enum iio_modifier {
 	IIO_MOD_CO2,
 	IIO_MOD_VOC,
 	IIO_MOD_LIGHT_UV,
+	IIO_MOD_DOUBLE_TAP,
 };
 
 enum iio_event_type {
-- 
2.7.4

Re: [PATCH 2/3] iio: core: Add double tap as possible gesture

[Peter Meerwald-Stadler]

> This adds the IIO_MOD_DOUBLE_TAP entry to the iio_modifier enum and the
> corresponding "double_tap" string to the iio_modifier_names array.

I don't think we should have gestures as channel modifiers

probably a middle layer between IIO and input subsystem is needed (could 
be in userspace?)
 
> Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
> Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
> ---
>  drivers/iio/industrialio-core.c | 1 +
>  include/uapi/linux/iio/types.h  | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> index fc340ed..82ce05f 100644
> --- a/drivers/iio/industrialio-core.c
> +++ b/drivers/iio/industrialio-core.c
> @@ -120,6 +120,7 @@ static const char * const iio_modifier_names[] = {
>  	[IIO_MOD_Q] = "q",
>  	[IIO_MOD_CO2] = "co2",
>  	[IIO_MOD_VOC] = "voc",
> +	[IIO_MOD_DOUBLE_TAP] = "double_tap",
>  };
>  
>  /* relies on pairs of these shared then separate */
> diff --git a/include/uapi/linux/iio/types.h b/include/uapi/linux/iio/types.h
> index 22e5e58..47d2768 100644
> --- a/include/uapi/linux/iio/types.h
> +++ b/include/uapi/linux/iio/types.h
> @@ -80,6 +80,7 @@ enum iio_modifier {
>  	IIO_MOD_CO2,
>  	IIO_MOD_VOC,
>  	IIO_MOD_LIGHT_UV,
> +	IIO_MOD_DOUBLE_TAP,
>  };
>  
>  enum iio_event_type {
> 

-- 

Peter Meerwald-Stadler
+43-664-2444418 (mobile)

Re: [PATCH 2/3] iio: core: Add double tap as possible gesture

[Jonathan Cameron]

On 06/12/16 11:25, Peter Meerwald-Stadler wrote:
> 
>> This adds the IIO_MOD_DOUBLE_TAP entry to the iio_modifier enum and the
>> corresponding "double_tap" string to the iio_modifier_names array.
> 
> I don't think we should have gestures as channel modifiers
Agreed, though treating it like other activities (walking, running etc) gave me
the thought that maybe there are exercise classes out there somewhere in
'double tapping'.

We don't actually have a good way of handling this at the moment, but it's
definitely an 'event' in IIO parlance rather than a modifier of a channel type.

It's irritatingly non general, but perhaps an event type is needed for
gestures with some abuse of the event code format when one turns up to allow
us to encode the type?

> probably a middle layer between IIO and input subsystem is needed (could 
> be in userspace?)
Could do it in userspace, but we probably ultimately want an in kernel
consumer interface for IIO events, and do the translation in a input
bridge using that...

Certainly bridging buffered data in userspace didn't turn out as neatly
as the in kernel version.

>  
>> Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
>> Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
>> ---
>>  drivers/iio/industrialio-core.c | 1 +
>>  include/uapi/linux/iio/types.h  | 1 +
>>  2 files changed, 2 insertions(+)
>>
>> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
>> index fc340ed..82ce05f 100644
>> --- a/drivers/iio/industrialio-core.c
>> +++ b/drivers/iio/industrialio-core.c
>> @@ -120,6 +120,7 @@ static const char * const iio_modifier_names[] = {
>>  	[IIO_MOD_Q] = "q",
>>  	[IIO_MOD_CO2] = "co2",
>>  	[IIO_MOD_VOC] = "voc",
>> +	[IIO_MOD_DOUBLE_TAP] = "double_tap",
>>  };
>>  
>>  /* relies on pairs of these shared then separate */
>> diff --git a/include/uapi/linux/iio/types.h b/include/uapi/linux/iio/types.h
>> index 22e5e58..47d2768 100644
>> --- a/include/uapi/linux/iio/types.h
>> +++ b/include/uapi/linux/iio/types.h
>> @@ -80,6 +80,7 @@ enum iio_modifier {
>>  	IIO_MOD_CO2,
>>  	IIO_MOD_VOC,
>>  	IIO_MOD_LIGHT_UV,
>> +	IIO_MOD_DOUBLE_TAP,
>>  };
>>  
>>  enum iio_event_type {
>>
> 

Re: [PATCH 2/3] iio: core: Add double tap as possible gesture

[Gwendal Grignou]

[+Nick]
Looking into it again, given that overloading IIO_ACTIVITY is not an
option, we need another channel type for these type of events. (single
tap, double tap).
Also to reduce the CPU load at idle, instead of polling an
accelerometer to detect orientation change, we are looking at using
accelerometer built-in capability to detect it. (portrait, landscape,
inverted portrait/landscape).

I am proposing to add a new event type for tap: IIO_EV_TYPE_TAP
IIO_MOD_EVENT_CODE(IIO_ACCEL,
                                                x,
                                                IIO_MOD_Z,   /* if the
tap is on the Z axis, if the sensor can tell */
                                                IIO_EV_TYPE_TAP,
                                                IIO_EV_DIR_NONE)
where x == 1 : single tap, 2 : double tap, 3 : .. triple tap ....

For screen rotation detection, another event: IIO_EV_TYPE_ROT, this
time, x will be an enum similar to the W3C  Screen Orientation API
(https://www.w3.org/TR/screen-orientation/#screenorientation-interface):

enum iio_orientation_type {
    PORTRAIT_PRIMARY,
    PORTRAIT_SECONDARY,
    LANDSCAPE_PRIMARY,
    LANDSCAPE_SECONDARY
};

Gwendal

On Mon, Dec 19, 2016 at 1:11 PM, Jonathan Cameron <jic23@kernel.org> wrote:
> On 06/12/16 11:25, Peter Meerwald-Stadler wrote:
>>
>>> This adds the IIO_MOD_DOUBLE_TAP entry to the iio_modifier enum and the
>>> corresponding "double_tap" string to the iio_modifier_names array.
>>
>> I don't think we should have gestures as channel modifiers
> Agreed, though treating it like other activities (walking, running etc) gave me
> the thought that maybe there are exercise classes out there somewhere in
> 'double tapping'.
>
> We don't actually have a good way of handling this at the moment, but it's
> definitely an 'event' in IIO parlance rather than a modifier of a channel type.
>
> It's irritatingly non general, but perhaps an event type is needed for
> gestures with some abuse of the event code format when one turns up to allow
> us to encode the type?
>
>> probably a middle layer between IIO and input subsystem is needed (could
>> be in userspace?)
> Could do it in userspace, but we probably ultimately want an in kernel
> consumer interface for IIO events, and do the translation in a input
> bridge using that...
>
> Certainly bridging buffered data in userspace didn't turn out as neatly
> as the in kernel version.
>
>>
>>> Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
>>> Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
>>> ---
>>>  drivers/iio/industrialio-core.c | 1 +
>>>  include/uapi/linux/iio/types.h  | 1 +
>>>  2 files changed, 2 insertions(+)
>>>
>>> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
>>> index fc340ed..82ce05f 100644
>>> --- a/drivers/iio/industrialio-core.c
>>> +++ b/drivers/iio/industrialio-core.c
>>> @@ -120,6 +120,7 @@ static const char * const iio_modifier_names[] = {
>>>      [IIO_MOD_Q] = "q",
>>>      [IIO_MOD_CO2] = "co2",
>>>      [IIO_MOD_VOC] = "voc",
>>> +    [IIO_MOD_DOUBLE_TAP] = "double_tap",
>>>  };
>>>
>>>  /* relies on pairs of these shared then separate */
>>> diff --git a/include/uapi/linux/iio/types.h b/include/uapi/linux/iio/types.h
>>> index 22e5e58..47d2768 100644
>>> --- a/include/uapi/linux/iio/types.h
>>> +++ b/include/uapi/linux/iio/types.h
>>> @@ -80,6 +80,7 @@ enum iio_modifier {
>>>      IIO_MOD_CO2,
>>>      IIO_MOD_VOC,
>>>      IIO_MOD_LIGHT_UV,
>>> +    IIO_MOD_DOUBLE_TAP,
>>>  };
>>>
>>>  enum iio_event_type {
>>>
>>
>

Re: [PATCH 2/3] iio: core: Add double tap as possible gesture

[Jonathan Cameron]

On Wed, 7 Jun 2017 18:10:02 -0700
Gwendal Grignou <gwendal@chromium.org> wrote:

Hi Gwendal,

> [+Nick]
> Looking into it again, given that overloading IIO_ACTIVITY is not an
> option, we need another channel type for these type of events. (single
> tap, double tap).
Could represent them as an any access event...
> Also to reduce the CPU load at idle, instead of polling an
> accelerometer to detect orientation change, we are looking at using
> accelerometer built-in capability to detect it. (portrait, landscape,
> inverted portrait/landscape).
Makes sense - I was wondering when someone would propose something for
that given the number of devices supporting it is increasing.
> 
> I am proposing to add a new event type for tap: IIO_EV_TYPE_TAP
> IIO_MOD_EVENT_CODE(IIO_ACCEL,
>                                                 x,
>                                                 IIO_MOD_Z,   /* if the
> tap is on the Z axis, if the sensor can tell */
>                                                 IIO_EV_TYPE_TAP,
>                                                 IIO_EV_DIR_NONE)
> where x == 1 : single tap, 2 : double tap, 3 : .. triple tap ....
No on the use of the index field for the number of taps.
That's a channel index and there are dual accelerometer devices
out there which have parallel accelerometers and use this index.
Not to mention we want to keep it generic if we can. 

Do we get directional taps?  I.d. would direction ever be set?  Seems
possible in future that a device might detect this so I suppose we
can't abuse the direction field and that wouldn't be any better
than the index field.

We have 8 bits for the event type.  Perhaps we should just use a few
values to represent these tap counts?  So have
IIO_EV_TYPE_TAP_SINGLE
IIO_EV_TYPE_TAP_DOUBLE
etc.

> 
> For screen rotation detection, another event: IIO_EV_TYPE_ROT, this
> time, x will be an enum similar to the W3C  Screen Orientation API
> (https://www.w3.org/TR/screen-orientation/#screenorientation-interface):
> 
> enum iio_orientation_type {
>     PORTRAIT_PRIMARY,
>     PORTRAIT_SECONDARY,
>     LANDSCAPE_PRIMARY,
>     LANDSCAPE_SECONDARY
> };
Hmm. This rather feels like bludgeoning a continuous value into an
encoded event. As before I don't like the reuse of the index field...
I also don't really get the whole primary / secondary in the w3c spec...

What we are really looking at here is bounded regions in rotation space that
are detected as the various orientations.  It would be lovely to describe
them as such, but I fear we'll never know enough about most devices to know
how they are defining these regions - and I suspect a lot are much simpler
than they would like us to think ;)

So another option would be to do these in a similar fashion to the
gestures - thus allowing a wide degree of flexibility.

So we'd have a set of channels representing the 'probability' that we
are at a given orientation with a new channel type.  These could be polled at
any time. in_orientation_portrait 0 or 100 (%) on most devices - one
day we'll get a probabalistic output reflecting the certainty that the
underlying algorithm has on the orientation - it might stop the
jumpiness to you get on landscape / portrait on phones when they are
near flat by providing the information that the phone isn't all
that sure...

To achieve the event type interface, do the same as we did for gestures and use
RISING and FALLING events to indicate coming into and out of various orientations.

This should also allow us to cope with the more 'interesting' orientations
one gets with jointed devices.

Does that fit reasonably well?

Jonathan
> 
> Gwendal
> 
> On Mon, Dec 19, 2016 at 1:11 PM, Jonathan Cameron <jic23@kernel.org> wrote:
> > On 06/12/16 11:25, Peter Meerwald-Stadler wrote:  
> >>  
> >>> This adds the IIO_MOD_DOUBLE_TAP entry to the iio_modifier enum and the
> >>> corresponding "double_tap" string to the iio_modifier_names array.  
> >>
> >> I don't think we should have gestures as channel modifiers  
> > Agreed, though treating it like other activities (walking, running etc) gave me
> > the thought that maybe there are exercise classes out there somewhere in
> > 'double tapping'.
> >
> > We don't actually have a good way of handling this at the moment, but it's
> > definitely an 'event' in IIO parlance rather than a modifier of a channel type.
> >
> > It's irritatingly non general, but perhaps an event type is needed for
> > gestures with some abuse of the event code format when one turns up to allow
> > us to encode the type?
> >  
> >> probably a middle layer between IIO and input subsystem is needed (could
> >> be in userspace?)  
> > Could do it in userspace, but we probably ultimately want an in kernel
> > consumer interface for IIO events, and do the translation in a input
> > bridge using that...
> >
> > Certainly bridging buffered data in userspace didn't turn out as neatly
> > as the in kernel version.
> >  
> >>  
> >>> Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
> >>> Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
> >>> ---
> >>>  drivers/iio/industrialio-core.c | 1 +
> >>>  include/uapi/linux/iio/types.h  | 1 +
> >>>  2 files changed, 2 insertions(+)
> >>>
> >>> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> >>> index fc340ed..82ce05f 100644
> >>> --- a/drivers/iio/industrialio-core.c
> >>> +++ b/drivers/iio/industrialio-core.c
> >>> @@ -120,6 +120,7 @@ static const char * const iio_modifier_names[] = {
> >>>      [IIO_MOD_Q] = "q",
> >>>      [IIO_MOD_CO2] = "co2",
> >>>      [IIO_MOD_VOC] = "voc",
> >>> +    [IIO_MOD_DOUBLE_TAP] = "double_tap",
> >>>  };
> >>>
> >>>  /* relies on pairs of these shared then separate */
> >>> diff --git a/include/uapi/linux/iio/types.h b/include/uapi/linux/iio/types.h
> >>> index 22e5e58..47d2768 100644
> >>> --- a/include/uapi/linux/iio/types.h
> >>> +++ b/include/uapi/linux/iio/types.h
> >>> @@ -80,6 +80,7 @@ enum iio_modifier {
> >>>      IIO_MOD_CO2,
> >>>      IIO_MOD_VOC,
> >>>      IIO_MOD_LIGHT_UV,
> >>> +    IIO_MOD_DOUBLE_TAP,
> >>>  };
> >>>
> >>>  enum iio_event_type {
> >>>  
> >>  
> >  
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/3] iio: cros_ec_activity: add ChromeOS EC Activity Sensors

[Thierry Escande]

From: Gwendal Grignou <gwendal@chromium.org>

This patch adds a driver for handling activity/gesture recognition
coming from the EC. Only significant motion is currently supported. It
is an incomplete driver: activity can be set, but ring buffer must be
used to receive interruption.

Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
---
 drivers/iio/common/cros_ec_sensors/Kconfig         |  10 +
 drivers/iio/common/cros_ec_sensors/Makefile        |   1 +
 .../iio/common/cros_ec_sensors/cros_ec_activity.c  | 300 +++++++++++++++++++++
 .../common/cros_ec_sensors/cros_ec_sensors_core.c  |  24 ++
 .../common/cros_ec_sensors/cros_ec_sensors_core.h  |   1 +
 5 files changed, 336 insertions(+)
 create mode 100644 drivers/iio/common/cros_ec_sensors/cros_ec_activity.c

diff --git a/drivers/iio/common/cros_ec_sensors/Kconfig b/drivers/iio/common/cros_ec_sensors/Kconfig
index 3349c9d..0a64928 100644
--- a/drivers/iio/common/cros_ec_sensors/Kconfig
+++ b/drivers/iio/common/cros_ec_sensors/Kconfig
@@ -21,3 +21,13 @@ config IIO_CROS_EC_SENSORS
 	  Accelerometers, Gyroscope and Magnetometer that are
 	  presented by the ChromeOS EC Sensor hub.
 	  Creates an IIO device for each functions.
+
+config IIO_CROS_EC_ACTIVITY
+	tristate "ChromeOS EC Activity Sensors"
+	select IIO_CROS_EC_SENSORS_CORE
+	help
+	  Module to handle activity events detections presented by the ChromeOS
+	  EC Sensor hub.
+	  Activities can be simple (low/no motion) or more complex (riding train).
+	  They are being reported by physical devices or the EC itself.
+	  Creates an IIO device to manage all activities.
diff --git a/drivers/iio/common/cros_ec_sensors/Makefile b/drivers/iio/common/cros_ec_sensors/Makefile
index ec716ff..a4a2d6c 100644
--- a/drivers/iio/common/cros_ec_sensors/Makefile
+++ b/drivers/iio/common/cros_ec_sensors/Makefile
@@ -4,3 +4,4 @@
 
 obj-$(CONFIG_IIO_CROS_EC_SENSORS_CORE) += cros_ec_sensors_core.o
 obj-$(CONFIG_IIO_CROS_EC_SENSORS) += cros_ec_sensors.o
+obj-$(CONFIG_IIO_CROS_EC_ACTIVITY) += cros_ec_activity.o
diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_activity.c b/drivers/iio/common/cros_ec_sensors/cros_ec_activity.c
new file mode 100644
index 0000000..f14c3d3
--- /dev/null
+++ b/drivers/iio/common/cros_ec_sensors/cros_ec_activity.c
@@ -0,0 +1,300 @@
+/*
+ * cros_ec_sensors_activity - Driver for activities/gesture recognition.
+ *
+ * Copyright (C) 2015 Google, Inc
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * This driver uses the cros-ec interface to communicate with the Chrome OS
+ * EC about accelerometer data. Accelerometer access is presented through
+ * iio sysfs.
+ */
+
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/iio/iio.h>
+#include <linux/kernel.h>
+#include <linux/mfd/cros_ec.h>
+#include <linux/mfd/cros_ec_commands.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/platform_device.h>
+
+#include "cros_ec_sensors_core.h"
+
+#define DRV_NAME "cros-ec-activity"
+
+/* st data for ec_sensors iio driver. */
+struct cros_ec_sensors_state {
+	/* Shared by all sensors */
+	struct cros_ec_sensors_core_state core;
+
+	struct iio_chan_spec *channels;
+	unsigned int nb_activities;
+};
+
+static const struct iio_event_spec cros_ec_activity_single_shot[] = {
+	{
+		.type = IIO_EV_TYPE_CHANGE,
+		/* significant motion trigger when we get out of still. */
+		.dir = IIO_EV_DIR_FALLING,
+		.mask_separate = BIT(IIO_EV_INFO_ENABLE),
+	 },
+};
+
+static int ec_sensors_read(struct iio_dev *indio_dev,
+			   struct iio_chan_spec const *chan,
+			   int *val, int *val2, long mask)
+{
+	dev_warn(&indio_dev->dev, "%s: Not Expected: %d\n", __func__,
+		 chan->channel2);
+
+	return -EPERM;
+}
+
+static int ec_sensors_write(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan,
+			    int val, int val2, long mask)
+{
+	dev_warn(&indio_dev->dev, "%s: Not Expected: %d\n", __func__,
+		 chan->channel2);
+
+	return -EPERM;
+}
+
+static int cros_ec_read_event_config(struct iio_dev *indio_dev,
+				     const struct iio_chan_spec *chan,
+				     enum iio_event_type type,
+				     enum iio_event_direction dir)
+{
+	struct cros_ec_sensors_state *st = iio_priv(indio_dev);
+	int ret;
+
+	if (chan->type != IIO_ACTIVITY)
+		return -EINVAL;
+
+	mutex_lock(&st->core.cmd_lock);
+
+	st->core.param.cmd = MOTIONSENSE_CMD_LIST_ACTIVITIES;
+
+	if (cros_ec_motion_send_host_cmd(&st->core, 0) != EC_RES_SUCCESS) {
+		ret = -EIO;
+		goto exit;
+	}
+
+	switch (chan->channel2) {
+	case IIO_MOD_STILL:
+		ret = !!(st->core.resp->list_activities.enabled &
+			 (1 << MOTIONSENSE_ACTIVITY_SIG_MOTION));
+		break;
+	case IIO_MOD_DOUBLE_TAP:
+		ret = !!(st->core.resp->list_activities.enabled &
+			 (1 << MOTIONSENSE_ACTIVITY_DOUBLE_TAP));
+		break;
+	default:
+		dev_warn(&indio_dev->dev, "Unknown activity: %d\n",
+			 chan->channel2);
+		ret = -EINVAL;
+	}
+
+exit:
+	mutex_unlock(&st->core.cmd_lock);
+
+	return ret;
+}
+
+static int cros_ec_write_event_config(struct iio_dev *indio_dev,
+				      const struct iio_chan_spec *chan,
+				      enum iio_event_type type,
+				      enum iio_event_direction dir, int state)
+{
+	struct cros_ec_sensors_state *st = iio_priv(indio_dev);
+	int ret;
+
+	if (chan->type != IIO_ACTIVITY)
+		return -EINVAL;
+
+	mutex_lock(&st->core.cmd_lock);
+	st->core.param.cmd = MOTIONSENSE_CMD_SET_ACTIVITY;
+
+	switch (chan->channel2) {
+	case IIO_MOD_STILL:
+		st->core.param.set_activity.activity =
+			MOTIONSENSE_ACTIVITY_SIG_MOTION;
+		break;
+	case IIO_MOD_DOUBLE_TAP:
+		st->core.param.set_activity.activity =
+			MOTIONSENSE_ACTIVITY_DOUBLE_TAP;
+		break;
+	default:
+		dev_warn(&indio_dev->dev, "Unknown activity: %d\n",
+			 chan->channel2);
+	}
+
+	st->core.param.set_activity.enable = state;
+
+	ret = cros_ec_motion_send_host_cmd(&st->core, 0);
+
+	mutex_unlock(&st->core.cmd_lock);
+
+	return ret;
+}
+
+/* Not implemented */
+static int cros_ec_read_event_value(struct iio_dev *indio_dev,
+				    const struct iio_chan_spec *chan,
+				    enum iio_event_type type,
+				    enum iio_event_direction dir,
+				    enum iio_event_info info,
+				    int *val, int *val2)
+{
+	dev_warn(&indio_dev->dev, "%s: Not Expected: %d\n", __func__,
+		 chan->channel2);
+
+	return -EPERM;
+}
+
+static int cros_ec_write_event_value(struct iio_dev *indio_dev,
+				     const struct iio_chan_spec *chan,
+				     enum iio_event_type type,
+				     enum iio_event_direction dir,
+				     enum iio_event_info info,
+				     int val, int val2)
+{
+	dev_warn(&indio_dev->dev, "%s: Not Expected: %d\n", __func__,
+		 chan->channel2);
+
+	return -EPERM;
+}
+
+static const struct iio_info ec_sensors_info = {
+	.read_raw = &ec_sensors_read,
+	.write_raw = &ec_sensors_write,
+	.read_event_config = cros_ec_read_event_config,
+	.write_event_config = cros_ec_write_event_config,
+	.read_event_value = cros_ec_read_event_value,
+	.write_event_value = cros_ec_write_event_value,
+};
+
+static int cros_ec_sensors_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct cros_ec_dev *ec_dev = dev_get_drvdata(dev->parent);
+	struct cros_ec_device *ec_device;
+	struct iio_dev *indio_dev;
+	struct cros_ec_sensors_state *st;
+	struct iio_chan_spec *channel;
+	unsigned long activities;
+	int i, index, ret, nb_activities;
+
+	if (!ec_dev || !ec_dev->ec_dev) {
+		dev_warn(&pdev->dev, "No CROS EC device found.\n");
+		return -EINVAL;
+	}
+
+	ec_device = ec_dev->ec_dev;
+
+	indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*st));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	ret = cros_ec_sensors_core_init(pdev, indio_dev, true);
+	if (ret)
+		return ret;
+
+	indio_dev->info = &ec_sensors_info;
+	st = iio_priv(indio_dev);
+	st->core.type = st->core.resp->info.type;
+	st->core.loc = st->core.resp->info.location;
+
+	/* List all available activities */
+	st->core.param.cmd = MOTIONSENSE_CMD_LIST_ACTIVITIES;
+	ret = cros_ec_motion_send_host_cmd(&st->core, 0);
+	if (ret)
+		return ret;
+
+	activities = st->core.resp->list_activities.enabled |
+		     st->core.resp->list_activities.disabled;
+	nb_activities = hweight_long(activities) + 1;
+
+	if (!activities)
+		return -ENODEV;
+
+	/* Allocate a channel per activity and one for timestamp */
+	st->channels = devm_kcalloc(&pdev->dev, nb_activities,
+				    sizeof(*st->channels), GFP_KERNEL);
+	if (!st->channels)
+		return -ENOMEM;
+
+	channel = &st->channels[0];
+	index = 0;
+	for_each_set_bit(i, &activities, BITS_PER_LONG) {
+		channel->scan_index = index;
+
+		/* List all available activities */
+		channel->type = IIO_ACTIVITY;
+		channel->modified = 1;
+		channel->event_spec = cros_ec_activity_single_shot;
+		channel->num_event_specs =
+				ARRAY_SIZE(cros_ec_activity_single_shot);
+		switch (i) {
+		case MOTIONSENSE_ACTIVITY_SIG_MOTION:
+			channel->channel2 = IIO_MOD_STILL;
+			break;
+		case MOTIONSENSE_ACTIVITY_DOUBLE_TAP:
+			channel->channel2 = IIO_MOD_DOUBLE_TAP;
+			break;
+		default:
+			dev_warn(&pdev->dev, "Unknown activity: %d\n", i);
+			continue;
+		}
+		channel->ext_info = cros_ec_sensors_limited_info;
+		channel++;
+		index++;
+	}
+
+	/* Timestamp */
+	channel->scan_index = index;
+	channel->type = IIO_TIMESTAMP;
+	channel->channel = -1;
+	channel->scan_type.sign = 's';
+	channel->scan_type.realbits = 64;
+	channel->scan_type.storagebits = 64;
+
+	indio_dev->channels = st->channels;
+	indio_dev->num_channels = index + 1;
+
+	st->core.read_ec_sensors_data = cros_ec_sensors_read_cmd;
+
+	/* Driver is incomplete: by itself, no way to get event directly */
+	return iio_device_register(indio_dev);
+}
+
+static int cros_ec_sensors_remove(struct platform_device *pdev)
+{
+	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
+
+	iio_device_unregister(indio_dev);
+
+	return 0;
+}
+
+static struct platform_driver cros_ec_sensors_platform_driver = {
+	.driver = {
+		.name	= DRV_NAME,
+	},
+	.probe		= cros_ec_sensors_probe,
+	.remove		= cros_ec_sensors_remove,
+};
+module_platform_driver(cros_ec_sensors_platform_driver);
+
+MODULE_DESCRIPTION("ChromeOS EC activity sensors driver");
+MODULE_ALIAS("platform:" DRV_NAME);
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
index a3be799..6010718 100644
--- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
+++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
@@ -135,6 +135,15 @@ static ssize_t cros_ec_sensors_calibrate(struct iio_dev *indio_dev,
 	return ret ? ret : len;
 }
 
+static ssize_t cros_ec_sensors_id(struct iio_dev *indio_dev,
+		uintptr_t private, const struct iio_chan_spec *chan,
+		char *buf)
+{
+	struct cros_ec_sensors_core_state *st = iio_priv(indio_dev);
+
+	return snprintf(buf, PAGE_SIZE, "%d\n", st->param.info.sensor_num);
+}
+
 static ssize_t cros_ec_sensors_loc(struct iio_dev *indio_dev,
 		uintptr_t private, const struct iio_chan_spec *chan,
 		char *buf)
@@ -159,6 +168,21 @@ const struct iio_chan_spec_ext_info cros_ec_sensors_ext_info[] = {
 };
 EXPORT_SYMBOL_GPL(cros_ec_sensors_ext_info);
 
+const struct iio_chan_spec_ext_info cros_ec_sensors_limited_info[] = {
+	{
+		.name = "id",
+		.shared = IIO_SHARED_BY_ALL,
+		.read = cros_ec_sensors_id
+	},
+	{
+		.name = "location",
+		.shared = IIO_SHARED_BY_ALL,
+		.read = cros_ec_sensors_loc
+	},
+	{ },
+};
+EXPORT_SYMBOL_GPL(cros_ec_sensors_limited_info);
+
 /**
  * cros_ec_sensors_idx_to_reg - convert index into offset in shared memory
  * @st:		pointer to state information for device
diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.h b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.h
index 8bc2ca3..b785f2b 100644
--- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.h
+++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.h
@@ -171,5 +171,6 @@ int cros_ec_sensors_core_write(struct cros_ec_sensors_core_state *st,
 
 /* List of extended channel specification for all sensors */
 extern const struct iio_chan_spec_ext_info cros_ec_sensors_ext_info[];
+extern const struct iio_chan_spec_ext_info cros_ec_sensors_limited_info[];
 
 #endif  /* __CROS_EC_SENSORS_CORE_H */
-- 
2.7.4

Re: [PATCH 3/3] iio: cros_ec_activity: add ChromeOS EC Activity Sensors

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 948 bytes --]

Hi Gwendal,

[auto build test WARNING on iio/togreg]
[also build test WARNING on next-20161206]
[cannot apply to v4.9-rc8]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Thierry-Escande/mfd-cros_ec-Add-activity-motion-sense-definitions/20161206-002239
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
config: um-allyesconfig (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=um 

All warnings (new ones prefixed by >>):

warning: (IIO_CROS_EC_ACTIVITY) selects IIO_CROS_EC_SENSORS_CORE which has unmet direct dependencies (IIO && SYSFS && MFD_CROS_EC)

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 18581 bytes --]

Re: [PATCH 3/3] iio: cros_ec_activity: add ChromeOS EC Activity Sensors

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 3980 bytes --]

Hi Gwendal,

[auto build test ERROR on iio/togreg]
[also build test ERROR on next-20161216]
[cannot apply to v4.9]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Thierry-Escande/mfd-cros_ec-Add-activity-motion-sense-definitions/20161206-002239
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
config: um-allmodconfig (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=um 

All errors (new ones prefixed by >>):

   ERROR: "devm_gpiod_get_optional" [net/rfkill/rfkill-gpio.ko] undefined!
   ERROR: "devm_gpio_request_one" [drivers/regulator/tps62360-regulator.ko] undefined!
   ERROR: "devm_gpiod_get_optional" [drivers/regulator/pwm-regulator.ko] undefined!
   ERROR: "devm_gpio_request_one" [drivers/regulator/max8973-regulator.ko] undefined!
   ERROR: "devm_gpio_request_one" [drivers/regulator/max8952.ko] undefined!
   ERROR: "devm_gpio_request_one" [drivers/regulator/lp872x.ko] undefined!
   ERROR: "devm_gpio_request" [drivers/pps/clients/pps-gpio.ko] undefined!
   ERROR: "devm_gpiod_get_optional" [drivers/power/supply/sbs-battery.ko] undefined!
   ERROR: "devm_gpio_request" [drivers/power/supply/max8903_charger.ko] undefined!
   ERROR: "devm_gpiod_get_index" [drivers/power/supply/bq25890_charger.ko] undefined!
   ERROR: "devm_gpiod_get_optional" [drivers/power/supply/bq24735-charger.ko] undefined!
   ERROR: "devm_gpiod_get_optional" [drivers/power/supply/bq24257_charger.ko] undefined!
   ERROR: "devm_gpio_request_one" [drivers/nfc/st21nfca/st21nfca_i2c.ko] undefined!
   ERROR: "devm_gpio_request_one" [drivers/nfc/st-nci/st-nci_i2c.ko] undefined!
   ERROR: "devm_gpio_request_one" [drivers/nfc/nxp-nci/nxp-nci_i2c.ko] undefined!
   ERROR: "devm_gpio_free" [drivers/nfc/nfcmrvl/nfcmrvl.ko] undefined!
   ERROR: "devm_gpio_request_one" [drivers/nfc/nfcmrvl/nfcmrvl.ko] undefined!
   ERROR: "devm_gpiod_get" [drivers/nfc/fdp/fdp_i2c.ko] undefined!
   ERROR: "devm_gpio_request" [drivers/net/phy/mdio-gpio.ko] undefined!
   ERROR: "devm_gpiod_get_optional" [drivers/net/phy/at803x.ko] undefined!
   ERROR: "devm_gpiod_get_optional" [drivers/net/dsa/mv88e6xxx/mv88e6xxx.ko] undefined!
   ERROR: "devm_gpio_request_one" [drivers/leds/leds-lt3593.ko] undefined!
   ERROR: "devm_gpiod_get_optional" [drivers/leds/leds-lp8860.ko] undefined!
   ERROR: "devm_gpio_request_one" [drivers/leds/leds-lp55xx-common.ko] undefined!
   ERROR: "devm_gpiod_get" [drivers/leds/leds-ktd2692.ko] undefined!
   ERROR: "devm_get_gpiod_from_child" [drivers/leds/leds-gpio.ko] undefined!
   ERROR: "devm_gpio_request_one" [drivers/leds/leds-gpio.ko] undefined!
   ERROR: "devm_gpiod_get_index" [drivers/iio/proximity/sx9500.ko] undefined!
   ERROR: "devm_gpiod_get_index" [drivers/iio/pressure/hp03.ko] undefined!
   ERROR: "devm_gpiod_get" [drivers/iio/pressure/bmp280.ko] undefined!
   ERROR: "devm_gpio_request_one" [drivers/iio/magnetometer/ak8975.ko] undefined!
   ERROR: "devm_gpiod_get_optional" [drivers/iio/dac/ad5592r-base.ko] undefined!
>> ERROR: "cros_ec_cmd_xfer_status" [drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.ko] undefined!
   ERROR: "devm_gpiod_get_index" [drivers/iio/accel/mma9551.ko] undefined!
   ERROR: "devm_gpiod_get_optional" [drivers/i2c/muxes/i2c-mux-pca954x.ko] undefined!
   ERROR: "devm_gpiod_get" [drivers/extcon/extcon-usb-gpio.ko] undefined!
   ERROR: "devm_gpiod_get" [drivers/extcon/extcon-max3355.ko] undefined!
   ERROR: "devm_gpio_request_one" [drivers/extcon/extcon-gpio.ko] undefined!
   ERROR: "devm_gpiod_get" [drivers/bluetooth/hci_uart.ko] undefined!
   ERROR: "devm_ioremap_resource" [drivers/auxdisplay/img-ascii-lcd.ko] undefined!

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 18719 bytes --]