linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH]  iio: Introduce activity channel
@ 2014-10-02 13:43 Daniel Baluta
  2014-10-02 13:43 ` [RFC PATCH 1/8] iio: dummy: Introduce virtual registers for dummy device Daniel Baluta
                   ` (7 more replies)
  0 siblings, 8 replies; 27+ messages in thread
From: Daniel Baluta @ 2014-10-02 13:43 UTC (permalink / raw)
  To: jic23, linux-iio, linux-kernel; +Cc: daniel.baluta, irina.tirdea

This patchset introduces a new interface for supporting some of the composite
sensors in Android [1]. First supported sensors are motion and pedometer.

This is a follow up of the discussion about adding new channels to IIO
initiated some time ago on IIO list [2].

A device that has the motion/pedometer functionality is Freescale's MMA9553L ([3]).

Because we don't have yet the hardware, to demonstrate this new channel we 
update iio_dummy kernel module and iio_event_monitor test program. We want
to get an early feedback about extending the IIO interface in the correct
direction.

[1] http://source.android.com/devices/sensors/composite_sensors.html
[2] http://marc.info/?l=linux-iio&m=138374342831057&w=2
[3] http://www.freescale.com/files/sensors/doc/ref_manual/MMA9553LSWRM.pdf

Daniel Baluta (5):
  iio: dummy: Introduce virtual registers for dummy device
  iio: core: Introduce IIO_ACTIVITY channel
  iio: core: Introduce new MOTION event
  iio: dummy: Demonstrate the usage of activity channel
  iio: event_monitor: Add support for activity channel

Irina Tirdea (3):
  iio: core: Introduce pedometer STEP counter modifier
  iio: core: Introduce ENABLE channel info mask
  iio: core: Introduce new STEP_DETECT event

 Documentation/ABI/testing/sysfs-bus-iio            | 28 +++++++
 drivers/iio/industrialio-core.c                    |  7 ++
 drivers/iio/industrialio-event.c                   |  2 +
 .../staging/iio/Documentation/iio_event_monitor.c  | 16 ++++
 drivers/staging/iio/iio_dummy_evgen.c              | 16 ++++
 drivers/staging/iio/iio_dummy_evgen.h              |  7 ++
 drivers/staging/iio/iio_simple_dummy.c             | 87 ++++++++++++++++++++--
 drivers/staging/iio/iio_simple_dummy.h             |  4 +
 drivers/staging/iio/iio_simple_dummy_events.c      | 37 +++++++--
 include/linux/iio/iio.h                            |  1 +
 include/linux/iio/types.h                          | 10 ++-
 11 files changed, 201 insertions(+), 14 deletions(-)

-- 
1.9.1


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [RFC PATCH 1/8] iio: dummy: Introduce virtual registers for dummy device
  2014-10-02 13:43 [RFC PATCH] iio: Introduce activity channel Daniel Baluta
@ 2014-10-02 13:43 ` Daniel Baluta
  2014-10-04 12:48   ` Jonathan Cameron
  2014-10-19 20:30   ` Hartmut Knaack
  2014-10-02 13:43 ` [RFC PATCH 2/8] iio: core: Introduce IIO_ACTIVITY channel Daniel Baluta
                   ` (6 subsequent siblings)
  7 siblings, 2 replies; 27+ messages in thread
From: Daniel Baluta @ 2014-10-02 13:43 UTC (permalink / raw)
  To: jic23, linux-iio, linux-kernel; +Cc: daniel.baluta, irina.tirdea

We need a way to store events generated by iio_dummy_evgen module,
in order to correctly process IRQs in iio_simple_dummy_events.

For the moment, we add two registers:

* id_reg  - ID register, stores the source of the event
* id_data - DATA register, stores the type of the event

e.g echo 4 > /sys/bus/iio/devices/iio_evgen/poke2

id_reg 0x02, id_data 0x04

This means, event of type 4 was generated by fake device 2.

We currently use a hardcoded mapping of virtual events to IIO events.

Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
---
 drivers/staging/iio/iio_dummy_evgen.c         | 16 ++++++++++++++++
 drivers/staging/iio/iio_dummy_evgen.h         |  7 +++++++
 drivers/staging/iio/iio_simple_dummy.h        |  2 ++
 drivers/staging/iio/iio_simple_dummy_events.c | 23 ++++++++++++++++++-----
 4 files changed, 43 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/iio/iio_dummy_evgen.c b/drivers/staging/iio/iio_dummy_evgen.c
index 5a804f1..d44f138 100644
--- a/drivers/staging/iio/iio_dummy_evgen.c
+++ b/drivers/staging/iio/iio_dummy_evgen.c
@@ -33,6 +33,7 @@
  * @base: base of irq range
  * @enabled: mask of which irqs are enabled
  * @inuse: mask of which irqs are connected
+ * @regs: irq regs we are faking
  * @lock: protect the evgen state
  */
 struct iio_dummy_eventgen {
@@ -40,6 +41,7 @@ struct iio_dummy_eventgen {
 	int base;
 	bool enabled[IIO_EVENTGEN_NO];
 	bool inuse[IIO_EVENTGEN_NO];
+	struct iio_dummy_regs regs[IIO_EVENTGEN_NO];
 	struct mutex lock;
 };
 
@@ -136,6 +138,12 @@ int iio_dummy_evgen_release_irq(int irq)
 }
 EXPORT_SYMBOL_GPL(iio_dummy_evgen_release_irq);
 
+struct iio_dummy_regs *iio_dummy_evgen_get_regs(int irq)
+{
+	return &iio_evgen->regs[irq - iio_evgen->base];
+}
+EXPORT_SYMBOL_GPL(iio_dummy_evgen_get_regs);
+
 static void iio_dummy_evgen_free(void)
 {
 	irq_free_descs(iio_evgen->base, IIO_EVENTGEN_NO);
@@ -153,6 +161,14 @@ static ssize_t iio_evgen_poke(struct device *dev,
 			      size_t len)
 {
 	struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
+	unsigned long event, ret;
+
+	ret = kstrtoul(buf, 10, &event);
+	if (ret)
+		return ret;
+
+	iio_evgen->regs[this_attr->address].reg_id   = this_attr->address;
+	iio_evgen->regs[this_attr->address].reg_data = event;
 
 	if (iio_evgen->enabled[this_attr->address])
 		handle_nested_irq(iio_evgen->base + this_attr->address);
diff --git a/drivers/staging/iio/iio_dummy_evgen.h b/drivers/staging/iio/iio_dummy_evgen.h
index d8845e2..5273478 100644
--- a/drivers/staging/iio/iio_dummy_evgen.h
+++ b/drivers/staging/iio/iio_dummy_evgen.h
@@ -1,2 +1,9 @@
+struct iio_dummy_regs {
+	u32 reg_id;
+	u32 reg_data;
+};
+
+struct iio_dummy_regs *iio_dummy_evgen_get_regs(int irq);
 int iio_dummy_evgen_get_irq(void);
 int iio_dummy_evgen_release_irq(int irq);
+
diff --git a/drivers/staging/iio/iio_simple_dummy.h b/drivers/staging/iio/iio_simple_dummy.h
index b126196..1a74e26 100644
--- a/drivers/staging/iio/iio_simple_dummy.h
+++ b/drivers/staging/iio/iio_simple_dummy.h
@@ -11,6 +11,7 @@
 #include <linux/kernel.h>
 
 struct iio_dummy_accel_calibscale;
+struct iio_dummy_regs;
 
 /**
  * struct iio_dummy_state - device instance specific state.
@@ -33,6 +34,7 @@ struct iio_dummy_state {
 	int accel_calibbias;
 	const struct iio_dummy_accel_calibscale *accel_calibscale;
 	struct mutex lock;
+	struct iio_dummy_regs *regs;
 #ifdef CONFIG_IIO_SIMPLE_DUMMY_EVENTS
 	int event_irq;
 	int event_val;
diff --git a/drivers/staging/iio/iio_simple_dummy_events.c b/drivers/staging/iio/iio_simple_dummy_events.c
index 64b45b0..719dfa5 100644
--- a/drivers/staging/iio/iio_simple_dummy_events.c
+++ b/drivers/staging/iio/iio_simple_dummy_events.c
@@ -148,12 +148,23 @@ int iio_simple_dummy_write_event_value(struct iio_dev *indio_dev,
 static irqreturn_t iio_simple_dummy_event_handler(int irq, void *private)
 {
 	struct iio_dev *indio_dev = private;
+	struct iio_dummy_state *st = iio_priv(indio_dev);
+
+	dev_dbg(&indio_dev->dev, "id %x event %x\n",
+		st->regs->reg_id, st->regs->reg_data);
+
+	switch (st->regs->reg_data) {
+	case 0:
+		iio_push_event(indio_dev,
+			       IIO_EVENT_CODE(IIO_VOLTAGE, 0, 0,
+					      IIO_EV_DIR_RISING,
+					      IIO_EV_TYPE_THRESH, 0, 0, 0),
+			       iio_get_time_ns());
+		break;
+	default:
+		break;
+	}
 
-	iio_push_event(indio_dev,
-		       IIO_EVENT_CODE(IIO_VOLTAGE, 0, 0,
-				      IIO_EV_DIR_RISING,
-				      IIO_EV_TYPE_THRESH, 0, 0, 0),
-		       iio_get_time_ns());
 	return IRQ_HANDLED;
 }
 
@@ -179,6 +190,8 @@ int iio_simple_dummy_events_register(struct iio_dev *indio_dev)
 		ret = st->event_irq;
 		goto error_ret;
 	}
+	st->regs = iio_dummy_evgen_get_regs(st->event_irq);
+
 	ret = request_threaded_irq(st->event_irq,
 				   NULL,
 				   &iio_simple_dummy_event_handler,
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [RFC PATCH 2/8] iio: core: Introduce IIO_ACTIVITY channel
  2014-10-02 13:43 [RFC PATCH] iio: Introduce activity channel Daniel Baluta
  2014-10-02 13:43 ` [RFC PATCH 1/8] iio: dummy: Introduce virtual registers for dummy device Daniel Baluta
@ 2014-10-02 13:43 ` Daniel Baluta
  2014-10-04 13:00   ` Jonathan Cameron
  2014-10-02 13:43 ` [RFC PATCH 3/8] iio: core: Introduce new MOTION event Daniel Baluta
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 27+ messages in thread
From: Daniel Baluta @ 2014-10-02 13:43 UTC (permalink / raw)
  To: jic23, linux-iio, linux-kernel; +Cc: daniel.baluta, irina.tirdea

This channel will be used for exposing information about
some activity composite sensors including:
	* motion (running, jogging, walking, still).
	* step counter
	* step detector

This will offer an interface for Android composite sensors
defined here:
http://source.android.com/devices/sensors/composite_sensors.html

This sensors are supported by Freescale's MMA9553 device.
http://freescale.com/files/sensors/doc/ref_manual/MMA9553LSWRM.pdf

Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
---
 drivers/iio/industrialio-core.c | 1 +
 include/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 af3e76d..67e8561 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -70,6 +70,7 @@ static const char * const iio_chan_type_name_spec[] = {
 	[IIO_CCT] = "cct",
 	[IIO_PRESSURE] = "pressure",
 	[IIO_HUMIDITYRELATIVE] = "humidityrelative",
+	[IIO_ACTIVITY] = "activity",
 };
 
 static const char * const iio_modifier_names[] = {
diff --git a/include/linux/iio/types.h b/include/linux/iio/types.h
index 4a2af8a..d58769a 100644
--- a/include/linux/iio/types.h
+++ b/include/linux/iio/types.h
@@ -30,6 +30,7 @@ enum iio_chan_type {
 	IIO_CCT,
 	IIO_PRESSURE,
 	IIO_HUMIDITYRELATIVE,
+	IIO_ACTIVITY,
 };
 
 enum iio_modifier {
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [RFC PATCH 3/8] iio: core: Introduce new MOTION event
  2014-10-02 13:43 [RFC PATCH] iio: Introduce activity channel Daniel Baluta
  2014-10-02 13:43 ` [RFC PATCH 1/8] iio: dummy: Introduce virtual registers for dummy device Daniel Baluta
  2014-10-02 13:43 ` [RFC PATCH 2/8] iio: core: Introduce IIO_ACTIVITY channel Daniel Baluta
@ 2014-10-02 13:43 ` Daniel Baluta
  2014-10-04 13:12   ` Jonathan Cameron
  2014-10-02 13:43 ` [RFC PATCH 4/8] iio: core: Introduce pedometer STEP counter modifier Daniel Baluta
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 27+ messages in thread
From: Daniel Baluta @ 2014-10-02 13:43 UTC (permalink / raw)
  To: jic23, linux-iio, linux-kernel; +Cc: daniel.baluta, irina.tirdea

This is to be used by drivers to signal detection of motion. We also
add some possible values for motion as IIO events modifiers:
	* running
	* jogging
	* walking
	* still

These values are supported by Frescale's MMA9553 sensor:

http://freescale.com/files/sensors/doc/ref_manual/MMA9553LSWRM.pdf

Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
---
 Documentation/ABI/testing/sysfs-bus-iio | 7 +++++++
 drivers/iio/industrialio-core.c         | 4 ++++
 drivers/iio/industrialio-event.c        | 1 +
 include/linux/iio/types.h               | 7 ++++++-
 4 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
index d760b02..070346d 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio
+++ b/Documentation/ABI/testing/sysfs-bus-iio
@@ -808,6 +808,13 @@ Description:
 		number or direction is not specified, applies to all channels of
 		this type.
 
+What:		/sys/.../events/in_activity_motion_either_en
+KernelVersion:	3.17
+Contact:	linux-iio@vger.kernel.org
+Description:
+		Enables or disables motion detection. Each time motion is detected an
+		event of this type will be generated.
+
 What:		/sys/bus/iio/devices/iio:deviceX/trigger/current_trigger
 KernelVersion:	2.6.35
 Contact:	linux-iio@vger.kernel.org
diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index 67e8561..e453ef9 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -92,6 +92,10 @@ static const char * const iio_modifier_names[] = {
 	[IIO_MOD_NORTH_TRUE] = "from_north_true",
 	[IIO_MOD_NORTH_MAGN_TILT_COMP] = "from_north_magnetic_tilt_comp",
 	[IIO_MOD_NORTH_TRUE_TILT_COMP] = "from_north_true_tilt_comp",
+	[IIO_MOD_RUNNING] = "running",
+	[IIO_MOD_JOGGING] = "jogging",
+	[IIO_MOD_WALKING] = "walking",
+	[IIO_MOD_STILL] = "still",
 };
 
 /* relies on pairs of these shared then separate */
diff --git a/drivers/iio/industrialio-event.c b/drivers/iio/industrialio-event.c
index 0c1e37e..eca5af2 100644
--- a/drivers/iio/industrialio-event.c
+++ b/drivers/iio/industrialio-event.c
@@ -197,6 +197,7 @@ static const char * const iio_ev_type_text[] = {
 	[IIO_EV_TYPE_ROC] = "roc",
 	[IIO_EV_TYPE_THRESH_ADAPTIVE] = "thresh_adaptive",
 	[IIO_EV_TYPE_MAG_ADAPTIVE] = "mag_adaptive",
+	[IIO_EV_TYPE_MOTION] = "motion",
 };
 
 static const char * const iio_ev_dir_text[] = {
diff --git a/include/linux/iio/types.h b/include/linux/iio/types.h
index d58769a..003638d 100644
--- a/include/linux/iio/types.h
+++ b/include/linux/iio/types.h
@@ -60,7 +60,11 @@ enum iio_modifier {
 	IIO_MOD_NORTH_MAGN,
 	IIO_MOD_NORTH_TRUE,
 	IIO_MOD_NORTH_MAGN_TILT_COMP,
-	IIO_MOD_NORTH_TRUE_TILT_COMP
+	IIO_MOD_NORTH_TRUE_TILT_COMP,
+	IIO_MOD_RUNNING,
+	IIO_MOD_JOGGING,
+	IIO_MOD_WALKING,
+	IIO_MOD_STILL,
 };
 
 enum iio_event_type {
@@ -69,6 +73,7 @@ enum iio_event_type {
 	IIO_EV_TYPE_ROC,
 	IIO_EV_TYPE_THRESH_ADAPTIVE,
 	IIO_EV_TYPE_MAG_ADAPTIVE,
+	IIO_EV_TYPE_MOTION,
 };
 
 enum iio_event_info {
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [RFC PATCH 4/8] iio: core: Introduce pedometer STEP counter modifier
  2014-10-02 13:43 [RFC PATCH] iio: Introduce activity channel Daniel Baluta
                   ` (2 preceding siblings ...)
  2014-10-02 13:43 ` [RFC PATCH 3/8] iio: core: Introduce new MOTION event Daniel Baluta
@ 2014-10-02 13:43 ` Daniel Baluta
  2014-10-04 12:53   ` Jonathan Cameron
  2014-10-02 13:43 ` [RFC PATCH 5/8] iio: core: Introduce ENABLE channel info mask Daniel Baluta
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 27+ messages in thread
From: Daniel Baluta @ 2014-10-02 13:43 UTC (permalink / raw)
  To: jic23, linux-iio, linux-kernel; +Cc: daniel.baluta, irina.tirdea

From: Irina Tirdea <irina.tirdea@intel.com>

One of the functionalities of a pedometer is a step counter.
The step counter needs to be enabled and then it will count the steps
in its hardware register. Whenever the applications need to check
the step count, they will read the step counter register.

To support this functionality we need a steps attribute that
will export the number of steps.

For more information on the pedometer requirements for Android see
http://source.android.com/devices/sensors/composite_sensors.html#counter.

A device that has the pedometer functionality this interface needs to
support is Freescale's MMA9553L:
http://www.freescale.com/files/sensors/doc/ref_manual/MMA9553LSWRM.pdf

Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
---
 Documentation/ABI/testing/sysfs-bus-iio | 7 +++++++
 drivers/iio/industrialio-core.c         | 1 +
 include/linux/iio/types.h               | 1 +
 3 files changed, 9 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
index 070346d..feacb45 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio
+++ b/Documentation/ABI/testing/sysfs-bus-iio
@@ -949,6 +949,13 @@ Description:
 		and the relevant _type attributes to establish the data storage
 		format.
 
+What:		/sys/.../iio:deviceX/in_activity_steps_raw
+KernelVersion:	3.17
+Contact:	linux-iio@vger.kernel.org
+Description:
+		This attribute is used to read the number of steps taken by the user
+		since the last reboot while activated.
+
 What:		/sys/.../iio:deviceX/in_anglvel_z_quadrature_correction_raw
 KernelVersion:	2.6.38
 Contact:	linux-iio@vger.kernel.org
diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index e453ef9..935a8a1 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -96,6 +96,7 @@ static const char * const iio_modifier_names[] = {
 	[IIO_MOD_JOGGING] = "jogging",
 	[IIO_MOD_WALKING] = "walking",
 	[IIO_MOD_STILL] = "still",
+	[IIO_MOD_PED_STEPS] = "steps",
 };
 
 /* relies on pairs of these shared then separate */
diff --git a/include/linux/iio/types.h b/include/linux/iio/types.h
index 003638d..ae51780 100644
--- a/include/linux/iio/types.h
+++ b/include/linux/iio/types.h
@@ -65,6 +65,7 @@ enum iio_modifier {
 	IIO_MOD_JOGGING,
 	IIO_MOD_WALKING,
 	IIO_MOD_STILL,
+	IIO_MOD_PED_STEPS,
 };
 
 enum iio_event_type {
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [RFC PATCH 5/8] iio: core: Introduce ENABLE channel info mask
  2014-10-02 13:43 [RFC PATCH] iio: Introduce activity channel Daniel Baluta
                   ` (3 preceding siblings ...)
  2014-10-02 13:43 ` [RFC PATCH 4/8] iio: core: Introduce pedometer STEP counter modifier Daniel Baluta
@ 2014-10-02 13:43 ` Daniel Baluta
  2014-10-02 13:43 ` [RFC PATCH 6/8] iio: core: Introduce new STEP_DETECT event Daniel Baluta
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 27+ messages in thread
From: Daniel Baluta @ 2014-10-02 13:43 UTC (permalink / raw)
  To: jic23, linux-iio, linux-kernel; +Cc: daniel.baluta, irina.tirdea

From: Irina Tirdea <irina.tirdea@intel.com>

This change is needed for the step counter functionality of a pedometer.
The step counter needs to be enabled and then it will count the steps
in its hardware register. Whenever the applications need to check
the step count, they will read the step counter register.

Since the pedometer needs to be enabled once so that the hardware
can count and store the steps, we need a specific enable attribute.

For more information on the pedometer requirements for Android see
http://source.android.com/devices/sensors/composite_sensors.html#counter.

A device that has the pedometer functionality this interface needs to
support is Freescale's MMA9553L:
http://www.freescale.com/files/sensors/doc/ref_manual/MMA9553LSWRM.pdf

Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
---
 Documentation/ABI/testing/sysfs-bus-iio | 7 +++++++
 drivers/iio/industrialio-core.c         | 1 +
 include/linux/iio/iio.h                 | 1 +
 3 files changed, 9 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
index feacb45..c02785d 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio
+++ b/Documentation/ABI/testing/sysfs-bus-iio
@@ -949,6 +949,13 @@ Description:
 		and the relevant _type attributes to establish the data storage
 		format.
 
+What:		/sys/.../iio:deviceX/in_activity_enable
+KernelVersion:	3.17
+Contact:	linux-iio@vger.kernel.org
+Description:
+		Activates the step counter and all dependent features
+		(e.g.: walked distance, activity detection like running, walking, etc.).
+
 What:		/sys/.../iio:deviceX/in_activity_steps_raw
 KernelVersion:	3.17
 Contact:	linux-iio@vger.kernel.org
diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index 935a8a1..936df49 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -119,6 +119,7 @@ static const char * const iio_chan_info_postfix[] = {
 	[IIO_CHAN_INFO_HARDWAREGAIN] = "hardwaregain",
 	[IIO_CHAN_INFO_HYSTERESIS] = "hysteresis",
 	[IIO_CHAN_INFO_INT_TIME] = "integration_time",
+	[IIO_CHAN_INFO_ENABLE] = "enable",
 };
 
 /**
diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
index 15dc6bc..3d3f06f 100644
--- a/include/linux/iio/iio.h
+++ b/include/linux/iio/iio.h
@@ -37,6 +37,7 @@ enum iio_chan_info_enum {
 	IIO_CHAN_INFO_HARDWAREGAIN,
 	IIO_CHAN_INFO_HYSTERESIS,
 	IIO_CHAN_INFO_INT_TIME,
+	IIO_CHAN_INFO_ENABLE,
 };
 
 enum iio_shared_by {
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [RFC PATCH 6/8] iio: core: Introduce new STEP_DETECT event
  2014-10-02 13:43 [RFC PATCH] iio: Introduce activity channel Daniel Baluta
                   ` (4 preceding siblings ...)
  2014-10-02 13:43 ` [RFC PATCH 5/8] iio: core: Introduce ENABLE channel info mask Daniel Baluta
@ 2014-10-02 13:43 ` Daniel Baluta
  2014-10-04 12:56   ` Jonathan Cameron
  2014-10-02 13:43 ` [RFC PATCH 7/8] iio: dummy: Demonstrate the usage of activity channel Daniel Baluta
  2014-10-02 13:43 ` [RFC PATCH 8/8] iio: event_monitor: Add support for " Daniel Baluta
  7 siblings, 1 reply; 27+ messages in thread
From: Daniel Baluta @ 2014-10-02 13:43 UTC (permalink / raw)
  To: jic23, linux-iio, linux-kernel; +Cc: daniel.baluta, irina.tirdea

From: Irina Tirdea <irina.tirdea@intel.com>

This event is needed for the step detection functionality of a pedometer:
an interrupt is generated by the hardware device each time
a step is detected.

To support this, we add a new iio event.

For more information on the pedometer requirements for Android see
http://source.android.com/devices/sensors/composite_sensors.html#detector.

A device that has the pedometer functionality this interface needs to
support is Freescale's MMA9553L:
http://www.freescale.com/files/sensors/doc/ref_manual/MMA9553LSWRM.pdf

Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
---
 Documentation/ABI/testing/sysfs-bus-iio | 7 +++++++
 drivers/iio/industrialio-event.c        | 1 +
 include/linux/iio/types.h               | 1 +
 3 files changed, 9 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
index c02785d..fd66073 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio
+++ b/Documentation/ABI/testing/sysfs-bus-iio
@@ -815,6 +815,13 @@ Description:
 		Enables or disables motion detection. Each time motion is detected an
 		event of this type will be generated.
 
+What:		/sys/.../events/in_activity_step_detect_either_en
+KernelVersion:	3.17
+Contact:	linux-iio@vger.kernel.org
+Description:
+		Enables or disables step detection. Each time the user takes a step an
+		event of this type will be generated.
+
 What:		/sys/bus/iio/devices/iio:deviceX/trigger/current_trigger
 KernelVersion:	2.6.35
 Contact:	linux-iio@vger.kernel.org
diff --git a/drivers/iio/industrialio-event.c b/drivers/iio/industrialio-event.c
index eca5af2..c2ade1f 100644
--- a/drivers/iio/industrialio-event.c
+++ b/drivers/iio/industrialio-event.c
@@ -198,6 +198,7 @@ static const char * const iio_ev_type_text[] = {
 	[IIO_EV_TYPE_THRESH_ADAPTIVE] = "thresh_adaptive",
 	[IIO_EV_TYPE_MAG_ADAPTIVE] = "mag_adaptive",
 	[IIO_EV_TYPE_MOTION] = "motion",
+	[IIO_EV_TYPE_STEP_DETECT] = "step_detect",
 };
 
 static const char * const iio_ev_dir_text[] = {
diff --git a/include/linux/iio/types.h b/include/linux/iio/types.h
index ae51780..83768a6 100644
--- a/include/linux/iio/types.h
+++ b/include/linux/iio/types.h
@@ -75,6 +75,7 @@ enum iio_event_type {
 	IIO_EV_TYPE_THRESH_ADAPTIVE,
 	IIO_EV_TYPE_MAG_ADAPTIVE,
 	IIO_EV_TYPE_MOTION,
+	IIO_EV_TYPE_STEP_DETECT,
 };
 
 enum iio_event_info {
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [RFC PATCH 7/8] iio: dummy: Demonstrate the usage of activity channel
  2014-10-02 13:43 [RFC PATCH] iio: Introduce activity channel Daniel Baluta
                   ` (5 preceding siblings ...)
  2014-10-02 13:43 ` [RFC PATCH 6/8] iio: core: Introduce new STEP_DETECT event Daniel Baluta
@ 2014-10-02 13:43 ` Daniel Baluta
  2014-10-02 13:43 ` [RFC PATCH 8/8] iio: event_monitor: Add support for " Daniel Baluta
  7 siblings, 0 replies; 27+ messages in thread
From: Daniel Baluta @ 2014-10-02 13:43 UTC (permalink / raw)
  To: jic23, linux-iio, linux-kernel; +Cc: daniel.baluta, irina.tirdea

Adds support for the activity channel in the dummy driver:
- a new channel IIO_ACTIVITY
- support for reading and writing pedometer step counter
- step detect event
- motion detect event

Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
---
 drivers/staging/iio/iio_simple_dummy.c        | 87 ++++++++++++++++++++++++---
 drivers/staging/iio/iio_simple_dummy.h        |  2 +
 drivers/staging/iio/iio_simple_dummy_events.c | 14 +++++
 3 files changed, 95 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/iio/iio_simple_dummy.c b/drivers/staging/iio/iio_simple_dummy.c
index bf78e6f..b3becee 100644
--- a/drivers/staging/iio/iio_simple_dummy.c
+++ b/drivers/staging/iio/iio_simple_dummy.c
@@ -69,6 +69,20 @@ static const struct iio_event_spec iio_dummy_event = {
 	.mask_separate = BIT(IIO_EV_INFO_VALUE) | BIT(IIO_EV_INFO_ENABLE),
 };
 
+/*
+ * simple activity events:
+ * motion: triggered when an activity (walking, running, etc.) is detected
+ * step detect: triggered when a step is detected
+ */
+static const struct iio_event_spec activity_events[] = {
+	{
+		.type = IIO_EV_TYPE_MOTION,
+		.mask_separate = BIT(IIO_EV_INFO_ENABLE),
+	}, {
+		.type = IIO_EV_TYPE_STEP_DETECT,
+		.mask_separate = BIT(IIO_EV_INFO_ENABLE),
+	},
+};
 #endif
 
 /*
@@ -215,6 +229,20 @@ static const struct iio_chan_spec iio_dummy_channels[] = {
 		.indexed = 1,
 		.channel = 0,
 	},
+	{
+		.type = IIO_ACTIVITY,
+		.modified = 1,
+		.channel2 = IIO_MOD_PED_STEPS,
+		.info_mask_shared_by_type =  BIT(IIO_CHAN_INFO_ENABLE),
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+	},
+#ifdef CONFIG_IIO_SIMPLE_DUMMY_EVENTS
+	{
+		.type = IIO_ACTIVITY,
+		.event_spec = activity_events,
+		.num_event_specs = ARRAY_SIZE(activity_events),
+	},
+#endif /* CONFIG_IIO_SIMPLE_DUMMY_EVENTS */
 };
 
 /**
@@ -259,6 +287,14 @@ static int iio_dummy_read_raw(struct iio_dev *indio_dev,
 			*val = st->accel_val;
 			ret = IIO_VAL_INT;
 			break;
+		case IIO_ACTIVITY:
+			switch (chan->channel2) {
+			case IIO_MOD_PED_STEPS:
+				*val = st->ped_steps;
+				ret = IIO_VAL_INT;
+				break;
+			}
+			break;
 		default:
 			break;
 		}
@@ -298,6 +334,16 @@ static int iio_dummy_read_raw(struct iio_dev *indio_dev,
 		*val2 = 33;
 		ret = IIO_VAL_INT_PLUS_NANO;
 		break;
+	case IIO_CHAN_INFO_ENABLE:
+		switch (chan->type) {
+		case IIO_ACTIVITY:
+			*val = st->ped_enabled;
+			ret = IIO_VAL_INT;
+			break;
+		default:
+			break;
+		}
+		break;
 	default:
 		break;
 	}
@@ -330,14 +376,29 @@ static int iio_dummy_write_raw(struct iio_dev *indio_dev,
 
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
-		if (chan->output == 0)
+		switch (chan->type) {
+		case IIO_VOLTAGE:
+			if (chan->output == 0)
+				return -EINVAL;
+
+			/* Locking not required as writing single value */
+			mutex_lock(&st->lock);
+			st->dac_val = val;
+			mutex_unlock(&st->lock);
+			return 0;
+		case IIO_ACTIVITY:
+			switch (chan->channel2) {
+			case IIO_MOD_PED_STEPS:
+				mutex_lock(&st->lock);
+				st->ped_steps = val;
+				mutex_unlock(&st->lock);
+				return 0;
+			default:
+				return -EINVAL;
+			}
+		default:
 			return -EINVAL;
-
-		/* Locking not required as writing single value */
-		mutex_lock(&st->lock);
-		st->dac_val = val;
-		mutex_unlock(&st->lock);
-		return 0;
+		}
 	case IIO_CHAN_INFO_CALIBSCALE:
 		mutex_lock(&st->lock);
 		/* Compare against table - hard matching here */
@@ -356,7 +417,16 @@ static int iio_dummy_write_raw(struct iio_dev *indio_dev,
 		st->accel_calibbias = val;
 		mutex_unlock(&st->lock);
 		return 0;
-
+	case IIO_CHAN_INFO_ENABLE:
+		switch (chan->type) {
+		case IIO_ACTIVITY:
+			mutex_lock(&st->lock);
+			st->ped_enabled = val;
+			mutex_unlock(&st->lock);
+			return 0;
+		default:
+			return -EINVAL;
+		}
 	default:
 		return -EINVAL;
 	}
@@ -395,6 +465,7 @@ static int iio_dummy_init_device(struct iio_dev *indio_dev)
 	st->accel_val = 34;
 	st->accel_calibbias = -7;
 	st->accel_calibscale = &dummy_scales[0];
+	st->ped_steps = 47;
 
 	return 0;
 }
diff --git a/drivers/staging/iio/iio_simple_dummy.h b/drivers/staging/iio/iio_simple_dummy.h
index 1a74e26..1996e3f 100644
--- a/drivers/staging/iio/iio_simple_dummy.h
+++ b/drivers/staging/iio/iio_simple_dummy.h
@@ -34,6 +34,8 @@ struct iio_dummy_state {
 	int accel_calibbias;
 	const struct iio_dummy_accel_calibscale *accel_calibscale;
 	struct mutex lock;
+	int ped_enabled;
+	int ped_steps;
 	struct iio_dummy_regs *regs;
 #ifdef CONFIG_IIO_SIMPLE_DUMMY_EVENTS
 	int event_irq;
diff --git a/drivers/staging/iio/iio_simple_dummy_events.c b/drivers/staging/iio/iio_simple_dummy_events.c
index 719dfa5..94de22a 100644
--- a/drivers/staging/iio/iio_simple_dummy_events.c
+++ b/drivers/staging/iio/iio_simple_dummy_events.c
@@ -161,6 +161,20 @@ static irqreturn_t iio_simple_dummy_event_handler(int irq, void *private)
 					      IIO_EV_TYPE_THRESH, 0, 0, 0),
 			       iio_get_time_ns());
 		break;
+	case 1:
+		iio_push_event(indio_dev,
+			       IIO_EVENT_CODE(IIO_ACTIVITY, 0, IIO_MOD_RUNNING,
+					      IIO_EV_DIR_RISING,
+					      IIO_EV_TYPE_MOTION, 0, 0, 0),
+			       iio_get_time_ns());
+		break;
+	case 2:
+		iio_push_event(indio_dev,
+			       IIO_EVENT_CODE(IIO_ACTIVITY, 0, IIO_NO_MOD,
+					      IIO_EV_DIR_EITHER,
+					      IIO_EV_TYPE_STEP_DETECT, 0, 0, 0),
+			       iio_get_time_ns());
+		break;
 	default:
 		break;
 	}
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [RFC PATCH 8/8] iio: event_monitor: Add support for activity channel
  2014-10-02 13:43 [RFC PATCH] iio: Introduce activity channel Daniel Baluta
                   ` (6 preceding siblings ...)
  2014-10-02 13:43 ` [RFC PATCH 7/8] iio: dummy: Demonstrate the usage of activity channel Daniel Baluta
@ 2014-10-02 13:43 ` Daniel Baluta
  7 siblings, 0 replies; 27+ messages in thread
From: Daniel Baluta @ 2014-10-02 13:43 UTC (permalink / raw)
  To: jic23, linux-iio, linux-kernel; +Cc: daniel.baluta, irina.tirdea

We have the following testing scenario:

$ insmod iio_dummy_evgen.ko
$ insmod iio_dummy.ko

$ ./iio_event_monitor /dev/iio:device0
Event: time: 1412254756207353948, type: voltage, channel: 0, evtype: thresh, direction: rising
Event: time: 1412254762455172908, type: activity(running), channel: 0, evtype: motion, direction: rising
Event: time: 1412254770823830774, type: activity, channel: 0, evtype: step_detect, direction: rising

$ echo 0 > /sys/bus/iio/devices/iio_evgen/poke_ev0
$ echo 1 > /sys/bus/iio/devices/iio_evgen/poke_ev0
$ echo 2 > /sys/bus/iio/devices/iio_evgen/poke_ev0

Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
---
 drivers/staging/iio/Documentation/iio_event_monitor.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/staging/iio/Documentation/iio_event_monitor.c b/drivers/staging/iio/Documentation/iio_event_monitor.c
index 569d6f8..5df7662 100644
--- a/drivers/staging/iio/Documentation/iio_event_monitor.c
+++ b/drivers/staging/iio/Documentation/iio_event_monitor.c
@@ -49,6 +49,7 @@ static const char * const iio_chan_type_name_spec[] = {
 	[IIO_CCT] = "cct",
 	[IIO_PRESSURE] = "pressure",
 	[IIO_HUMIDITYRELATIVE] = "humidityrelative",
+	[IIO_ACTIVITY] = "activity",
 };
 
 static const char * const iio_ev_type_text[] = {
@@ -57,6 +58,8 @@ static const char * const iio_ev_type_text[] = {
 	[IIO_EV_TYPE_ROC] = "roc",
 	[IIO_EV_TYPE_THRESH_ADAPTIVE] = "thresh_adaptive",
 	[IIO_EV_TYPE_MAG_ADAPTIVE] = "mag_adaptive",
+	[IIO_EV_TYPE_MOTION] = "motion",
+	[IIO_EV_TYPE_STEP_DETECT] = "step_detect",
 };
 
 static const char * const iio_ev_dir_text[] = {
@@ -79,6 +82,11 @@ static const char * const iio_modifier_names[] = {
 	[IIO_MOD_LIGHT_RED] = "red",
 	[IIO_MOD_LIGHT_GREEN] = "green",
 	[IIO_MOD_LIGHT_BLUE] = "blue",
+	[IIO_MOD_RUNNING] = "running",
+	[IIO_MOD_JOGGING] = "jogging",
+	[IIO_MOD_WALKING] = "walking",
+	[IIO_MOD_STILL] = "still",
+	[IIO_MOD_PED_STEPS] = "steps",
 };
 
 static bool event_is_known(struct iio_event_data *event)
@@ -108,6 +116,7 @@ static bool event_is_known(struct iio_event_data *event)
 	case IIO_CCT:
 	case IIO_PRESSURE:
 	case IIO_HUMIDITYRELATIVE:
+	case IIO_ACTIVITY:
 		break;
 	default:
 		return false;
@@ -126,6 +135,11 @@ static bool event_is_known(struct iio_event_data *event)
 	case IIO_MOD_LIGHT_RED:
 	case IIO_MOD_LIGHT_GREEN:
 	case IIO_MOD_LIGHT_BLUE:
+	case IIO_MOD_RUNNING:
+	case IIO_MOD_JOGGING:
+	case IIO_MOD_WALKING:
+	case IIO_MOD_STILL:
+	case IIO_MOD_PED_STEPS:
 		break;
 	default:
 		return false;
@@ -137,6 +151,8 @@ static bool event_is_known(struct iio_event_data *event)
 	case IIO_EV_TYPE_ROC:
 	case IIO_EV_TYPE_THRESH_ADAPTIVE:
 	case IIO_EV_TYPE_MAG_ADAPTIVE:
+	case IIO_EV_TYPE_MOTION:
+	case IIO_EV_TYPE_STEP_DETECT:
 		break;
 	default:
 		return false;
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* Re: [RFC PATCH 1/8] iio: dummy: Introduce virtual registers for dummy device
  2014-10-02 13:43 ` [RFC PATCH 1/8] iio: dummy: Introduce virtual registers for dummy device Daniel Baluta
@ 2014-10-04 12:48   ` Jonathan Cameron
  2014-10-06 11:17     ` Daniel Baluta
  2014-10-19 20:30   ` Hartmut Knaack
  1 sibling, 1 reply; 27+ messages in thread
From: Jonathan Cameron @ 2014-10-04 12:48 UTC (permalink / raw)
  To: Daniel Baluta, linux-iio, linux-kernel; +Cc: irina.tirdea

On 02/10/14 14:43, Daniel Baluta wrote:
> We need a way to store events generated by iio_dummy_evgen module,
> in order to correctly process IRQs in iio_simple_dummy_events.
>
> For the moment, we add two registers:
>
> * id_reg  - ID register, stores the source of the event
> * id_data - DATA register, stores the type of the event
>
> e.g echo 4 > /sys/bus/iio/devices/iio_evgen/poke2
>
> id_reg 0x02, id_data 0x04
>
> This means, event of type 4 was generated by fake device 2.
>
> We currently use a hardcoded mapping of virtual events to IIO events.
>
> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
> Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
Does the job and will look enough like a normal interrupt that
it allows the dummy driver to act as example code.

> ---
>  drivers/staging/iio/iio_dummy_evgen.c         | 16 ++++++++++++++++
>  drivers/staging/iio/iio_dummy_evgen.h         |  7 +++++++
>  drivers/staging/iio/iio_simple_dummy.h        |  2 ++
>  drivers/staging/iio/iio_simple_dummy_events.c | 23 ++++++++++++++++++-----
>  4 files changed, 43 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/staging/iio/iio_dummy_evgen.c b/drivers/staging/iio/iio_dummy_evgen.c
> index 5a804f1..d44f138 100644
> --- a/drivers/staging/iio/iio_dummy_evgen.c
> +++ b/drivers/staging/iio/iio_dummy_evgen.c
> @@ -33,6 +33,7 @@
>   * @base: base of irq range
>   * @enabled: mask of which irqs are enabled
>   * @inuse: mask of which irqs are connected
> + * @regs: irq regs we are faking
>   * @lock: protect the evgen state
>   */
>  struct iio_dummy_eventgen {
> @@ -40,6 +41,7 @@ struct iio_dummy_eventgen {
>  	int base;
>  	bool enabled[IIO_EVENTGEN_NO];
>  	bool inuse[IIO_EVENTGEN_NO];
> +	struct iio_dummy_regs regs[IIO_EVENTGEN_NO];
>  	struct mutex lock;
>  };
>
> @@ -136,6 +138,12 @@ int iio_dummy_evgen_release_irq(int irq)
>  }
>  EXPORT_SYMBOL_GPL(iio_dummy_evgen_release_irq);
>
> +struct iio_dummy_regs *iio_dummy_evgen_get_regs(int irq)
> +{
> +	return &iio_evgen->regs[irq - iio_evgen->base];
> +}
> +EXPORT_SYMBOL_GPL(iio_dummy_evgen_get_regs);
> +
>  static void iio_dummy_evgen_free(void)
>  {
>  	irq_free_descs(iio_evgen->base, IIO_EVENTGEN_NO);
> @@ -153,6 +161,14 @@ static ssize_t iio_evgen_poke(struct device *dev,
>  			      size_t len)
>  {
>  	struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
> +	unsigned long event, ret;
> +
> +	ret = kstrtoul(buf, 10, &event);
> +	if (ret)
> +		return ret;
> +
> +	iio_evgen->regs[this_attr->address].reg_id   = this_attr->address;
> +	iio_evgen->regs[this_attr->address].reg_data = event;
>
>  	if (iio_evgen->enabled[this_attr->address])
>  		handle_nested_irq(iio_evgen->base + this_attr->address);
> diff --git a/drivers/staging/iio/iio_dummy_evgen.h b/drivers/staging/iio/iio_dummy_evgen.h
> index d8845e2..5273478 100644
> --- a/drivers/staging/iio/iio_dummy_evgen.h
> +++ b/drivers/staging/iio/iio_dummy_evgen.h
> @@ -1,2 +1,9 @@
> +struct iio_dummy_regs {
> +	u32 reg_id;
> +	u32 reg_data;
> +};
> +
> +struct iio_dummy_regs *iio_dummy_evgen_get_regs(int irq);
>  int iio_dummy_evgen_get_irq(void);
>  int iio_dummy_evgen_release_irq(int irq);
> +
> diff --git a/drivers/staging/iio/iio_simple_dummy.h b/drivers/staging/iio/iio_simple_dummy.h
> index b126196..1a74e26 100644
> --- a/drivers/staging/iio/iio_simple_dummy.h
> +++ b/drivers/staging/iio/iio_simple_dummy.h
> @@ -11,6 +11,7 @@
>  #include <linux/kernel.h>
>
>  struct iio_dummy_accel_calibscale;
> +struct iio_dummy_regs;
>
>  /**
>   * struct iio_dummy_state - device instance specific state.
> @@ -33,6 +34,7 @@ struct iio_dummy_state {
>  	int accel_calibbias;
>  	const struct iio_dummy_accel_calibscale *accel_calibscale;
>  	struct mutex lock;
> +	struct iio_dummy_regs *regs;
>  #ifdef CONFIG_IIO_SIMPLE_DUMMY_EVENTS
>  	int event_irq;
>  	int event_val;
> diff --git a/drivers/staging/iio/iio_simple_dummy_events.c b/drivers/staging/iio/iio_simple_dummy_events.c
> index 64b45b0..719dfa5 100644
> --- a/drivers/staging/iio/iio_simple_dummy_events.c
> +++ b/drivers/staging/iio/iio_simple_dummy_events.c
> @@ -148,12 +148,23 @@ int iio_simple_dummy_write_event_value(struct iio_dev *indio_dev,
>  static irqreturn_t iio_simple_dummy_event_handler(int irq, void *private)
>  {
>  	struct iio_dev *indio_dev = private;
> +	struct iio_dummy_state *st = iio_priv(indio_dev);
> +
> +	dev_dbg(&indio_dev->dev, "id %x event %x\n",
> +		st->regs->reg_id, st->regs->reg_data);
> +
> +	switch (st->regs->reg_data) {
> +	case 0:
> +		iio_push_event(indio_dev,
> +			       IIO_EVENT_CODE(IIO_VOLTAGE, 0, 0,
> +					      IIO_EV_DIR_RISING,
> +					      IIO_EV_TYPE_THRESH, 0, 0, 0),
> +			       iio_get_time_ns());
> +		break;
> +	default:
> +		break;
> +	}
>
> -	iio_push_event(indio_dev,
> -		       IIO_EVENT_CODE(IIO_VOLTAGE, 0, 0,
> -				      IIO_EV_DIR_RISING,
> -				      IIO_EV_TYPE_THRESH, 0, 0, 0),
> -		       iio_get_time_ns());
>  	return IRQ_HANDLED;
>  }
>
> @@ -179,6 +190,8 @@ int iio_simple_dummy_events_register(struct iio_dev *indio_dev)
>  		ret = st->event_irq;
>  		goto error_ret;
>  	}
> +	st->regs = iio_dummy_evgen_get_regs(st->event_irq);
> +
>  	ret = request_threaded_irq(st->event_irq,
>  				   NULL,
>  				   &iio_simple_dummy_event_handler,
>

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [RFC PATCH 4/8] iio: core: Introduce pedometer STEP counter modifier
  2014-10-02 13:43 ` [RFC PATCH 4/8] iio: core: Introduce pedometer STEP counter modifier Daniel Baluta
@ 2014-10-04 12:53   ` Jonathan Cameron
  2014-10-06 13:50     ` Tirdea, Irina
  0 siblings, 1 reply; 27+ messages in thread
From: Jonathan Cameron @ 2014-10-04 12:53 UTC (permalink / raw)
  To: Daniel Baluta, linux-iio, linux-kernel; +Cc: irina.tirdea

On 02/10/14 14:43, Daniel Baluta wrote:
> From: Irina Tirdea <irina.tirdea@intel.com>
> 
> One of the functionalities of a pedometer is a step counter.
> The step counter needs to be enabled and then it will count the steps
> in its hardware register. Whenever the applications need to check
> the step count, they will read the step counter register.
> 
> To support this functionality we need a steps attribute that
> will export the number of steps.
> 
> For more information on the pedometer requirements for Android see
> http://source.android.com/devices/sensors/composite_sensors.html#counter.
> 
> A device that has the pedometer functionality this interface needs to
> support is Freescale's MMA9553L:
> http://www.freescale.com/files/sensors/doc/ref_manual/MMA9553LSWRM.pdf
> 
> Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
I'm not keen on multiplexing different types of data onto a single activity type.
Steps is well enough defined on it's own to have it's own channel type.

in_steps_input would be fine by me.  I suppose steps might mean something else
though...

> ---
>  Documentation/ABI/testing/sysfs-bus-iio | 7 +++++++
>  drivers/iio/industrialio-core.c         | 1 +
>  include/linux/iio/types.h               | 1 +
>  3 files changed, 9 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
> index 070346d..feacb45 100644
> --- a/Documentation/ABI/testing/sysfs-bus-iio
> +++ b/Documentation/ABI/testing/sysfs-bus-iio
> @@ -949,6 +949,13 @@ Description:
>  		and the relevant _type attributes to establish the data storage
>  		format.
>  
> +What:		/sys/.../iio:deviceX/in_activity_steps_raw
> +KernelVersion:	3.17
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		This attribute is used to read the number of steps taken by the user
> +		since the last reboot while activated.
> +
>  What:		/sys/.../iio:deviceX/in_anglvel_z_quadrature_correction_raw
>  KernelVersion:	2.6.38
>  Contact:	linux-iio@vger.kernel.org
> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> index e453ef9..935a8a1 100644
> --- a/drivers/iio/industrialio-core.c
> +++ b/drivers/iio/industrialio-core.c
> @@ -96,6 +96,7 @@ static const char * const iio_modifier_names[] = {
>  	[IIO_MOD_JOGGING] = "jogging",
>  	[IIO_MOD_WALKING] = "walking",
>  	[IIO_MOD_STILL] = "still",
> +	[IIO_MOD_PED_STEPS] = "steps",
>  };
>  
>  /* relies on pairs of these shared then separate */
> diff --git a/include/linux/iio/types.h b/include/linux/iio/types.h
> index 003638d..ae51780 100644
> --- a/include/linux/iio/types.h
> +++ b/include/linux/iio/types.h
> @@ -65,6 +65,7 @@ enum iio_modifier {
>  	IIO_MOD_JOGGING,
>  	IIO_MOD_WALKING,
>  	IIO_MOD_STILL,
> +	IIO_MOD_PED_STEPS,
>  };
>  
>  enum iio_event_type {
> 

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [RFC PATCH 6/8] iio: core: Introduce new STEP_DETECT event
  2014-10-02 13:43 ` [RFC PATCH 6/8] iio: core: Introduce new STEP_DETECT event Daniel Baluta
@ 2014-10-04 12:56   ` Jonathan Cameron
  0 siblings, 0 replies; 27+ messages in thread
From: Jonathan Cameron @ 2014-10-04 12:56 UTC (permalink / raw)
  To: Daniel Baluta, linux-iio, linux-kernel; +Cc: irina.tirdea

On 02/10/14 14:43, Daniel Baluta wrote:
> From: Irina Tirdea <irina.tirdea@intel.com>
> 
> This event is needed for the step detection functionality of a pedometer:
> an interrupt is generated by the hardware device each time
> a step is detected.
> 
> To support this, we add a new iio event.
> 
> For more information on the pedometer requirements for Android see
> http://source.android.com/devices/sensors/composite_sensors.html#detector.
> 
> A device that has the pedometer functionality this interface needs to
> support is Freescale's MMA9553L:
> http://www.freescale.com/files/sensors/doc/ref_manual/MMA9553LSWRM.pdf
> 
> Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
This could be more generic.  Perhaps an 'instance' event?
If applied to our step type it would
be in_step_instance_en etc?


> ---
>  Documentation/ABI/testing/sysfs-bus-iio | 7 +++++++
>  drivers/iio/industrialio-event.c        | 1 +
>  include/linux/iio/types.h               | 1 +
>  3 files changed, 9 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
> index c02785d..fd66073 100644
> --- a/Documentation/ABI/testing/sysfs-bus-iio
> +++ b/Documentation/ABI/testing/sysfs-bus-iio
> @@ -815,6 +815,13 @@ Description:
>  		Enables or disables motion detection. Each time motion is detected an
>  		event of this type will be generated.
>  
> +What:		/sys/.../events/in_activity_step_detect_either_en
> +KernelVersion:	3.17
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		Enables or disables step detection. Each time the user takes a step an
> +		event of this type will be generated.
> +
>  What:		/sys/bus/iio/devices/iio:deviceX/trigger/current_trigger
>  KernelVersion:	2.6.35
>  Contact:	linux-iio@vger.kernel.org
> diff --git a/drivers/iio/industrialio-event.c b/drivers/iio/industrialio-event.c
> index eca5af2..c2ade1f 100644
> --- a/drivers/iio/industrialio-event.c
> +++ b/drivers/iio/industrialio-event.c
> @@ -198,6 +198,7 @@ static const char * const iio_ev_type_text[] = {
>  	[IIO_EV_TYPE_THRESH_ADAPTIVE] = "thresh_adaptive",
>  	[IIO_EV_TYPE_MAG_ADAPTIVE] = "mag_adaptive",
>  	[IIO_EV_TYPE_MOTION] = "motion",
> +	[IIO_EV_TYPE_STEP_DETECT] = "step_detect",
>  };
>  
>  static const char * const iio_ev_dir_text[] = {
> diff --git a/include/linux/iio/types.h b/include/linux/iio/types.h
> index ae51780..83768a6 100644
> --- a/include/linux/iio/types.h
> +++ b/include/linux/iio/types.h
> @@ -75,6 +75,7 @@ enum iio_event_type {
>  	IIO_EV_TYPE_THRESH_ADAPTIVE,
>  	IIO_EV_TYPE_MAG_ADAPTIVE,
>  	IIO_EV_TYPE_MOTION,
> +	IIO_EV_TYPE_STEP_DETECT,
>  };
>  
>  enum iio_event_info {
> 

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [RFC PATCH 2/8] iio: core: Introduce IIO_ACTIVITY channel
  2014-10-02 13:43 ` [RFC PATCH 2/8] iio: core: Introduce IIO_ACTIVITY channel Daniel Baluta
@ 2014-10-04 13:00   ` Jonathan Cameron
  0 siblings, 0 replies; 27+ messages in thread
From: Jonathan Cameron @ 2014-10-04 13:00 UTC (permalink / raw)
  To: Daniel Baluta, linux-iio, linux-kernel; +Cc: irina.tirdea

On 02/10/14 14:43, Daniel Baluta wrote:
> This channel will be used for exposing information about
> some activity composite sensors including:
> 	* motion (running, jogging, walking, still).
> 	* step counter
> 	* step detector
> 
> This will offer an interface for Android composite sensors
> defined here:
> http://source.android.com/devices/sensors/composite_sensors.html
> 
> This sensors are supported by Freescale's MMA9553 device.
> http://freescale.com/files/sensors/doc/ref_manual/MMA9553LSWRM.pdf
> 
> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
> Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
This interface is interesting.... As I state on later patches,
get the less fuzzy stuff (step counting) out of here into
it's own type.

I'd also merge this at least with the next patch as it has little meaning
without that...

> ---
>  drivers/iio/industrialio-core.c | 1 +
>  include/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 af3e76d..67e8561 100644
> --- a/drivers/iio/industrialio-core.c
> +++ b/drivers/iio/industrialio-core.c
> @@ -70,6 +70,7 @@ static const char * const iio_chan_type_name_spec[] = {
>  	[IIO_CCT] = "cct",
>  	[IIO_PRESSURE] = "pressure",
>  	[IIO_HUMIDITYRELATIVE] = "humidityrelative",
> +	[IIO_ACTIVITY] = "activity",
>  };
>  
>  static const char * const iio_modifier_names[] = {
> diff --git a/include/linux/iio/types.h b/include/linux/iio/types.h
> index 4a2af8a..d58769a 100644
> --- a/include/linux/iio/types.h
> +++ b/include/linux/iio/types.h
> @@ -30,6 +30,7 @@ enum iio_chan_type {
>  	IIO_CCT,
>  	IIO_PRESSURE,
>  	IIO_HUMIDITYRELATIVE,
> +	IIO_ACTIVITY,
>  };
>  
>  enum iio_modifier {
> 

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [RFC PATCH 3/8] iio: core: Introduce new MOTION event
  2014-10-02 13:43 ` [RFC PATCH 3/8] iio: core: Introduce new MOTION event Daniel Baluta
@ 2014-10-04 13:12   ` Jonathan Cameron
  2014-10-06 14:17     ` Daniel Baluta
  2014-10-07 10:48     ` Daniel Baluta
  0 siblings, 2 replies; 27+ messages in thread
From: Jonathan Cameron @ 2014-10-04 13:12 UTC (permalink / raw)
  To: Daniel Baluta, linux-iio, linux-kernel; +Cc: irina.tirdea

On 02/10/14 14:43, Daniel Baluta wrote:
> This is to be used by drivers to signal detection of motion. We also
> add some possible values for motion as IIO events modifiers:
> 	* running
> 	* jogging
> 	* walking
> 	* still
> 
> These values are supported by Frescale's MMA9553 sensor:
> 
> http://freescale.com/files/sensors/doc/ref_manual/MMA9553LSWRM.pdf
> 
> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
> Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
Hmm.. This is the interesting one.
Not immediately obvious how best to represent this stuff.
> ---
>  Documentation/ABI/testing/sysfs-bus-iio | 7 +++++++
>  drivers/iio/industrialio-core.c         | 4 ++++
>  drivers/iio/industrialio-event.c        | 1 +
>  include/linux/iio/types.h               | 7 ++++++-
>  4 files changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
> index d760b02..070346d 100644
> --- a/Documentation/ABI/testing/sysfs-bus-iio
> +++ b/Documentation/ABI/testing/sysfs-bus-iio
> @@ -808,6 +808,13 @@ Description:
>  		number or direction is not specified, applies to all channels of
>  		this type.
>  
> +What:		/sys/.../events/in_activity_motion_either_en
> +KernelVersion:	3.17
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		Enables or disables motion detection. Each time motion is detected an
> +		event of this type will be generated.
> +
The either bit seems a bit random but I can see there is no particularly obvious
alternative.

We really need a clean way of representing a multilevel 'state change' like this.

Looking at the event code, I almost wonder if we would be better using the
direction element for running, walking etc rather than a modifier.

Having said that we will probably also get devices where this is polled rather than
event.  'What activity is currently going on?'
If we take that view modifiers make sense as it becomes
'Is the user running?'  Perhaps even offering a confidence interval, e.g units as
percentage
in_activity_running_input 0..100
in_activity_walking_input 0..100
etc

Then our event becomes a state change event (yup we'll need to add that)

/events/in_activity_walking_rising_en  will then cause events when the percentage
confidence on a state rises above the provided threshold or goes above it
(default of 50% perhaps on devices which only report one state).

/events/in_activity_walking_falling_en will do the leaving case.

Note these are just some quick initial thoughts on alternative methods.
I'll want to think on this more and get responses from more interested
parties!

>  What:		/sys/bus/iio/devices/iio:deviceX/trigger/current_trigger
>  KernelVersion:	2.6.35
>  Contact:	linux-iio@vger.kernel.org
> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> index 67e8561..e453ef9 100644
> --- a/drivers/iio/industrialio-core.c
> +++ b/drivers/iio/industrialio-core.c
> @@ -92,6 +92,10 @@ static const char * const iio_modifier_names[] = {
>  	[IIO_MOD_NORTH_TRUE] = "from_north_true",
>  	[IIO_MOD_NORTH_MAGN_TILT_COMP] = "from_north_magnetic_tilt_comp",
>  	[IIO_MOD_NORTH_TRUE_TILT_COMP] = "from_north_true_tilt_comp",
> +	[IIO_MOD_RUNNING] = "running",
> +	[IIO_MOD_JOGGING] = "jogging",
> +	[IIO_MOD_WALKING] = "walking",
> +	[IIO_MOD_STILL] = "still",
>  };
>  
>  /* relies on pairs of these shared then separate */
> diff --git a/drivers/iio/industrialio-event.c b/drivers/iio/industrialio-event.c
> index 0c1e37e..eca5af2 100644
> --- a/drivers/iio/industrialio-event.c
> +++ b/drivers/iio/industrialio-event.c
> @@ -197,6 +197,7 @@ static const char * const iio_ev_type_text[] = {
>  	[IIO_EV_TYPE_ROC] = "roc",
>  	[IIO_EV_TYPE_THRESH_ADAPTIVE] = "thresh_adaptive",
>  	[IIO_EV_TYPE_MAG_ADAPTIVE] = "mag_adaptive",
> +	[IIO_EV_TYPE_MOTION] = "motion",
>  };
>  
>  static const char * const iio_ev_dir_text[] = {
> diff --git a/include/linux/iio/types.h b/include/linux/iio/types.h
> index d58769a..003638d 100644
> --- a/include/linux/iio/types.h
> +++ b/include/linux/iio/types.h
> @@ -60,7 +60,11 @@ enum iio_modifier {
>  	IIO_MOD_NORTH_MAGN,
>  	IIO_MOD_NORTH_TRUE,
>  	IIO_MOD_NORTH_MAGN_TILT_COMP,
> -	IIO_MOD_NORTH_TRUE_TILT_COMP
> +	IIO_MOD_NORTH_TRUE_TILT_COMP,
> +	IIO_MOD_RUNNING,
> +	IIO_MOD_JOGGING,
> +	IIO_MOD_WALKING,
> +	IIO_MOD_STILL,
>  };
>  
>  enum iio_event_type {
> @@ -69,6 +73,7 @@ enum iio_event_type {
>  	IIO_EV_TYPE_ROC,
>  	IIO_EV_TYPE_THRESH_ADAPTIVE,
>  	IIO_EV_TYPE_MAG_ADAPTIVE,
> +	IIO_EV_TYPE_MOTION,
>  };
>  
>  enum iio_event_info {
> 

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [RFC PATCH 1/8] iio: dummy: Introduce virtual registers for dummy device
  2014-10-04 12:48   ` Jonathan Cameron
@ 2014-10-06 11:17     ` Daniel Baluta
  2014-10-09 19:28       ` Jonathan Cameron
  0 siblings, 1 reply; 27+ messages in thread
From: Daniel Baluta @ 2014-10-06 11:17 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Daniel Baluta, linux-iio, Linux Kernel Mailing List, irina.tirdea

On Sat, Oct 4, 2014 at 3:48 PM, Jonathan Cameron <jic23@kernel.org> wrote:
> On 02/10/14 14:43, Daniel Baluta wrote:
>> We need a way to store events generated by iio_dummy_evgen module,
>> in order to correctly process IRQs in iio_simple_dummy_events.
>>
>> For the moment, we add two registers:
>>
>> * id_reg  - ID register, stores the source of the event
>> * id_data - DATA register, stores the type of the event
>>
>> e.g echo 4 > /sys/bus/iio/devices/iio_evgen/poke2
>>
>> id_reg 0x02, id_data 0x04
>>
>> This means, event of type 4 was generated by fake device 2.
>>
>> We currently use a hardcoded mapping of virtual events to IIO events.
>>
>> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
>> Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
> Does the job and will look enough like a normal interrupt that
> it allows the dummy driver to act as example code.

Hi Jonathan,

Thanks for the reviews.

This patch is self contained and it could be merged
separately if you think it's stable enough.

We are going over your reviews for the other patches and will send v2
as soon as possible.

Daniel.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* RE: [RFC PATCH 4/8] iio: core: Introduce pedometer STEP counter modifier
  2014-10-04 12:53   ` Jonathan Cameron
@ 2014-10-06 13:50     ` Tirdea, Irina
  2014-10-06 16:31       ` Jonathan Cameron
  0 siblings, 1 reply; 27+ messages in thread
From: Tirdea, Irina @ 2014-10-06 13:50 UTC (permalink / raw)
  To: Jonathan Cameron; +Cc: Baluta, Daniel, linux-iio, linux-kernel

> From: Jonathan Cameron [mailto:jic23@kernel.org]
> On 02/10/14 14:43, Daniel Baluta wrote:
> > From: Irina Tirdea <irina.tirdea@intel.com>
> >
> > One of the functionalities of a pedometer is a step counter.
> > The step counter needs to be enabled and then it will count the steps
> > in its hardware register. Whenever the applications need to check
> > the step count, they will read the step counter register.
> >
> > To support this functionality we need a steps attribute that
> > will export the number of steps.
> >
> I'm not keen on multiplexing different types of data onto a single activity type.
> Steps is well enough defined on it's own to have it's own channel type.
> 
> in_steps_input would be fine by me.  I suppose steps might mean something else
> though...
> 

Hi Jonathan,

Thanks for the review.

Moving the pedometer part to a new type sounds good to me.
However, I would prefer to add a new type called pedometer and keep steps as a modifier, generating names like in_ped_steps_input for the attribute and in_ped_steps_instance_en for the event.
The reason for this is that for supporting Freescale's MMA9553L we will need additional attributes (distance, speed, calories, height, weight) that we can add as modifiers to this pedometer type. To keep things simple, I did not add these additional attributes to the RFC series, but I could do that if you think it would be useful. For this device, the motion events (walking, running, jogging, still) also depend on the height attribute being set, but we intend to deal with this dependency in the driver (using the pedometer's height attribute).

What do you think?

Thanks,
Irina


> > --- a/drivers/iio/industrialio-core.c
> > +++ b/drivers/iio/industrialio-core.c
> > @@ -96,6 +96,7 @@ static const char * const iio_modifier_names[] = {
> >  	[IIO_MOD_JOGGING] = "jogging",
> >  	[IIO_MOD_WALKING] = "walking",
> >  	[IIO_MOD_STILL] = "still",
> > +	[IIO_MOD_PED_STEPS] = "steps",
> >  };
> >
> >  /* relies on pairs of these shared then separate */
> > diff --git a/include/linux/iio/types.h b/include/linux/iio/types.h
> > index 003638d..ae51780 100644
> > --- a/include/linux/iio/types.h
> > +++ b/include/linux/iio/types.h
> > @@ -65,6 +65,7 @@ enum iio_modifier {
> >  	IIO_MOD_JOGGING,
> >  	IIO_MOD_WALKING,
> >  	IIO_MOD_STILL,
> > +	IIO_MOD_PED_STEPS,
> >  };
> >
> >  enum iio_event_type {
> >

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [RFC PATCH 3/8] iio: core: Introduce new MOTION event
  2014-10-04 13:12   ` Jonathan Cameron
@ 2014-10-06 14:17     ` Daniel Baluta
  2014-10-09 19:31       ` Jonathan Cameron
  2014-10-07 10:48     ` Daniel Baluta
  1 sibling, 1 reply; 27+ messages in thread
From: Daniel Baluta @ 2014-10-06 14:17 UTC (permalink / raw)
  To: Jonathan Cameron; +Cc: linux-iio, linux-kernel, irina.tirdea



On 10/04/2014 04:12 PM, Jonathan Cameron wrote:
> On 02/10/14 14:43, Daniel Baluta wrote:
>> This is to be used by drivers to signal detection of motion. We also
>> add some possible values for motion as IIO events modifiers:
>> 	* running
>> 	* jogging
>> 	* walking
>> 	* still
>>
>> These values are supported by Frescale's MMA9553 sensor:
>>
>> http://freescale.com/files/sensors/doc/ref_manual/MMA9553LSWRM.pdf
>>
>> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
>> Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
> Hmm.. This is the interesting one.
> Not immediately obvious how best to represent this stuff.
>> ---
>>   Documentation/ABI/testing/sysfs-bus-iio | 7 +++++++
>>   drivers/iio/industrialio-core.c         | 4 ++++
>>   drivers/iio/industrialio-event.c        | 1 +
>>   include/linux/iio/types.h               | 7 ++++++-
>>   4 files changed, 18 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
>> index d760b02..070346d 100644
>> --- a/Documentation/ABI/testing/sysfs-bus-iio
>> +++ b/Documentation/ABI/testing/sysfs-bus-iio
>> @@ -808,6 +808,13 @@ Description:
>>   		number or direction is not specified, applies to all channels of
>>   		this type.
>>
>> +What:		/sys/.../events/in_activity_motion_either_en
>> +KernelVersion:	3.17
>> +Contact:	linux-iio@vger.kernel.org
>> +Description:
>> +		Enables or disables motion detection. Each time motion is detected an
>> +		event of this type will be generated.
>> +
> The either bit seems a bit random but I can see there is no particularly obvious
> alternative.

I wonder if introducing a new IIO_EV_DIR_NONE event direction type would 
make sense. In this case the sysfs attribute will drop event direction 
text from its name (e.g /sys/.../events/in_activity_motion_en)

>
> We really need a clean way of representing a multilevel 'state change' like this.
>
> Looking at the event code, I almost wonder if we would be better using the
> direction element for running, walking etc rather than a modifier.

When pushing events code to userspace the modifier seemed to be the only 
option.

>
> Having said that we will probably also get devices where this is polled rather than
> event.  'What activity is currently going on?'

Adding IIO_EV_INFO_VALUE bit, would create an attribute 
/sys/.../events/in_activity_motion_either_value that could expose the 
current activity going on.

> If we take that view modifiers make sense as it becomes
> 'Is the user running?'  Perhaps even offering a confidence interval, e.g units as
> percentage
> in_activity_running_input 0..100
> in_activity_walking_input 0..100
> etc
>
> Then our event becomes a state change event (yup we'll need to add that)
>
> /events/in_activity_walking_rising_en  will then cause events when the percentage
> confidence on a state rises above the provided threshold or goes above it
> (default of 50% perhaps on devices which only report one state).
>
> /events/in_activity_walking_falling_en will do the leaving case.

This is a very nice idea and it will also offer more flexibility. I am 
not sure about the use case of confidence interval but using 0 and 100 
will do the trick for us.

We will use this interface for implementation of significant motion in 
Android's HAL. [1]

I will experiment more with how IIO attributes work and I will send a v2
using direction instead of modifier for activity type (running, walking 
etc).


>
> Note these are just some quick initial thoughts on alternative methods.
> I'll want to think on this more and get responses from more interested
> parties!

Thanks a lot for your time!

Daniel.

[1] https://source.android.com/devices/sensors/composite_sensors.html

^ permalink raw reply	[flat|nested] 27+ messages in thread

* RE: [RFC PATCH 4/8] iio: core: Introduce pedometer STEP counter modifier
  2014-10-06 13:50     ` Tirdea, Irina
@ 2014-10-06 16:31       ` Jonathan Cameron
  2014-10-07 13:54         ` Tirdea, Irina
  0 siblings, 1 reply; 27+ messages in thread
From: Jonathan Cameron @ 2014-10-06 16:31 UTC (permalink / raw)
  To: Tirdea, Irina; +Cc: Baluta, Daniel, linux-iio, linux-kernel



On October 6, 2014 2:50:13 PM GMT+01:00, "Tirdea, Irina" <irina.tirdea@intel.com> wrote:
>> From: Jonathan Cameron [mailto:jic23@kernel.org]
>> On 02/10/14 14:43, Daniel Baluta wrote:
>> > From: Irina Tirdea <irina.tirdea@intel.com>
>> >
>> > One of the functionalities of a pedometer is a step counter.
>> > The step counter needs to be enabled and then it will count the
>steps
>> > in its hardware register. Whenever the applications need to check
>> > the step count, they will read the step counter register.
>> >
>> > To support this functionality we need a steps attribute that
>> > will export the number of steps.
>> >
>> I'm not keen on multiplexing different types of data onto a single
>activity type.
>> Steps is well enough defined on it's own to have it's own channel
>type.
>> 
>> in_steps_input would be fine by me.  I suppose steps might mean
>something else
>> though...
>> 
>
>Hi Jonathan,
>
>Thanks for the review.
>
>Moving the pedometer part to a new type sounds good to me.
>However, I would prefer to add a new type called pedometer and keep
>steps as a modifier, generating names like in_ped_steps_input for the
>attribute and in_ped_steps_instance_en for the event.
>The reason for this is that for supporting Freescale's MMA9553L we will
>need additional attributes (distance, speed, calories, height, weight)
>that we can add as modifiers to this pedometer type. To keep things
>simple, I did not add these additional attributes to the RFC series,
>but I could do that if you think it would be useful. For this device,
>the motion events (walking, running, jogging, still) also depend on the
>height attribute being set, but we intend to deal with this dependency
>in the driver (using the pedometer's height attribute).
>
>What do you think?
I think I would rather each of these was included as a type rather than a modifier.
There are lots of other ways to measure speed for starters and often user space
won't care where it comes from...

In_speed_raw etc...

Trick where possible is to think about what is measured rather than how. Tends to give
 more consistent interfaces.
>
>Thanks,
>Irina
>
>
>> > --- a/drivers/iio/industrialio-core.c
>> > +++ b/drivers/iio/industrialio-core.c
>> > @@ -96,6 +96,7 @@ static const char * const iio_modifier_names[] =
>{
>> >  	[IIO_MOD_JOGGING] = "jogging",
>> >  	[IIO_MOD_WALKING] = "walking",
>> >  	[IIO_MOD_STILL] = "still",
>> > +	[IIO_MOD_PED_STEPS] = "steps",
>> >  };
>> >
>> >  /* relies on pairs of these shared then separate */
>> > diff --git a/include/linux/iio/types.h b/include/linux/iio/types.h
>> > index 003638d..ae51780 100644
>> > --- a/include/linux/iio/types.h
>> > +++ b/include/linux/iio/types.h
>> > @@ -65,6 +65,7 @@ enum iio_modifier {
>> >  	IIO_MOD_JOGGING,
>> >  	IIO_MOD_WALKING,
>> >  	IIO_MOD_STILL,
>> > +	IIO_MOD_PED_STEPS,
>> >  };
>> >
>> >  enum iio_event_type {
>> >

-- 
Sent from my Android phone with K-9 Mail. Please excuse my brevity.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [RFC PATCH 3/8] iio: core: Introduce new MOTION event
  2014-10-04 13:12   ` Jonathan Cameron
  2014-10-06 14:17     ` Daniel Baluta
@ 2014-10-07 10:48     ` Daniel Baluta
  2014-10-09 19:37       ` Jonathan Cameron
  1 sibling, 1 reply; 27+ messages in thread
From: Daniel Baluta @ 2014-10-07 10:48 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Daniel Baluta, linux-iio, Linux Kernel Mailing List, irina.tirdea

On Sat, Oct 4, 2014 at 4:12 PM, Jonathan Cameron <jic23@kernel.org> wrote:
> On 02/10/14 14:43, Daniel Baluta wrote:
>> This is to be used by drivers to signal detection of motion. We also
>> add some possible values for motion as IIO events modifiers:
>>       * running
>>       * jogging
>>       * walking
>>       * still
>>
>> These values are supported by Frescale's MMA9553 sensor:
>>
>> http://freescale.com/files/sensors/doc/ref_manual/MMA9553LSWRM.pdf
>>
>> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
>> Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
> Hmm.. This is the interesting one.
> Not immediately obvious how best to represent this stuff.
>> ---
>>  Documentation/ABI/testing/sysfs-bus-iio | 7 +++++++
>>  drivers/iio/industrialio-core.c         | 4 ++++
>>  drivers/iio/industrialio-event.c        | 1 +
>>  include/linux/iio/types.h               | 7 ++++++-
>>  4 files changed, 18 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
>> index d760b02..070346d 100644
>> --- a/Documentation/ABI/testing/sysfs-bus-iio
>> +++ b/Documentation/ABI/testing/sysfs-bus-iio
>> @@ -808,6 +808,13 @@ Description:
>>               number or direction is not specified, applies to all channels of
>>               this type.
>>
>> +What:                /sys/.../events/in_activity_motion_either_en
>> +KernelVersion:       3.17
>> +Contact:     linux-iio@vger.kernel.org
>> +Description:
>> +             Enables or disables motion detection. Each time motion is detected an
>> +             event of this type will be generated.
>> +
> The either bit seems a bit random but I can see there is no particularly obvious
> alternative.
>
> We really need a clean way of representing a multilevel 'state change' like this.
>
> Looking at the event code, I almost wonder if we would be better using the
> direction element for running, walking etc rather than a modifier.
>
> Having said that we will probably also get devices where this is polled rather than
> event.  'What activity is currently going on?'
> If we take that view modifiers make sense as it becomes
> 'Is the user running?'  Perhaps even offering a confidence interval, e.g units as
> percentage
> in_activity_running_input 0..100
> in_activity_walking_input 0..100
> etc
>

This is clear.

> Then our event becomes a state change event (yup we'll need to add that)

So this means we will have a single event type named "state_change"?
Or better "transition".

>
> /events/in_activity_walking_rising_en  will then cause events when the percentage
> confidence on a state rises above the provided threshold or goes above it
> (default of 50% perhaps on devices which only report one state).

Then here the attribute name will be:
/events/in_activity_state_change_walking_rising_en

or if we call the event type "transition" this will be:
/events/in_activity_transition_walking_rising_en

I am a little bit confused :).

>
> /events/in_activity_walking_falling_en will do the leaving case.
>
> Note these are just some quick initial thoughts on alternative methods.
> I'll want to think on this more and get responses from more interested
> parties!
>
>>  What:                /sys/bus/iio/devices/iio:deviceX/trigger/current_trigger
>>  KernelVersion:       2.6.35
>>  Contact:     linux-iio@vger.kernel.org
>> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
>> index 67e8561..e453ef9 100644
>> --- a/drivers/iio/industrialio-core.c
>> +++ b/drivers/iio/industrialio-core.c
>> @@ -92,6 +92,10 @@ static const char * const iio_modifier_names[] = {
>>       [IIO_MOD_NORTH_TRUE] = "from_north_true",
>>       [IIO_MOD_NORTH_MAGN_TILT_COMP] = "from_north_magnetic_tilt_comp",
>>       [IIO_MOD_NORTH_TRUE_TILT_COMP] = "from_north_true_tilt_comp",
>> +     [IIO_MOD_RUNNING] = "running",
>> +     [IIO_MOD_JOGGING] = "jogging",
>> +     [IIO_MOD_WALKING] = "walking",
>> +     [IIO_MOD_STILL] = "still",
>>  };
>>
>>  /* relies on pairs of these shared then separate */
>> diff --git a/drivers/iio/industrialio-event.c b/drivers/iio/industrialio-event.c
>> index 0c1e37e..eca5af2 100644
>> --- a/drivers/iio/industrialio-event.c
>> +++ b/drivers/iio/industrialio-event.c
>> @@ -197,6 +197,7 @@ static const char * const iio_ev_type_text[] = {
>>       [IIO_EV_TYPE_ROC] = "roc",
>>       [IIO_EV_TYPE_THRESH_ADAPTIVE] = "thresh_adaptive",
>>       [IIO_EV_TYPE_MAG_ADAPTIVE] = "mag_adaptive",
>> +     [IIO_EV_TYPE_MOTION] = "motion",
>>  };
>>
>>  static const char * const iio_ev_dir_text[] = {
>> diff --git a/include/linux/iio/types.h b/include/linux/iio/types.h
>> index d58769a..003638d 100644
>> --- a/include/linux/iio/types.h
>> +++ b/include/linux/iio/types.h
>> @@ -60,7 +60,11 @@ enum iio_modifier {
>>       IIO_MOD_NORTH_MAGN,
>>       IIO_MOD_NORTH_TRUE,
>>       IIO_MOD_NORTH_MAGN_TILT_COMP,
>> -     IIO_MOD_NORTH_TRUE_TILT_COMP
>> +     IIO_MOD_NORTH_TRUE_TILT_COMP,
>> +     IIO_MOD_RUNNING,
>> +     IIO_MOD_JOGGING,
>> +     IIO_MOD_WALKING,
>> +     IIO_MOD_STILL,
>>  };
>>
>>  enum iio_event_type {
>> @@ -69,6 +73,7 @@ enum iio_event_type {
>>       IIO_EV_TYPE_ROC,
>>       IIO_EV_TYPE_THRESH_ADAPTIVE,
>>       IIO_EV_TYPE_MAG_ADAPTIVE,
>> +     IIO_EV_TYPE_MOTION,
>>  };
>>
>>  enum iio_event_info {
>>
> --
> 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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* RE: [RFC PATCH 4/8] iio: core: Introduce pedometer STEP counter modifier
  2014-10-06 16:31       ` Jonathan Cameron
@ 2014-10-07 13:54         ` Tirdea, Irina
  0 siblings, 0 replies; 27+ messages in thread
From: Tirdea, Irina @ 2014-10-07 13:54 UTC (permalink / raw)
  To: Jonathan Cameron; +Cc: Baluta, Daniel, linux-iio, linux-kernel

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 2608 bytes --]



> -----Original Message-----
> From: linux-iio-owner@vger.kernel.org [mailto:linux-iio-owner@vger.kernel.org] On Behalf Of Jonathan Cameron
> On October 6, 2014 2:50:13 PM GMT+01:00, "Tirdea, Irina" <irina.tirdea@intel.com> wrote:
> >> From: Jonathan Cameron [mailto:jic23@kernel.org]
> >> On 02/10/14 14:43, Daniel Baluta wrote:
> >> > From: Irina Tirdea <irina.tirdea@intel.com>
> >> >
> >> > One of the functionalities of a pedometer is a step counter.
> >> > The step counter needs to be enabled and then it will count the
> >steps
> >> > in its hardware register. Whenever the applications need to check
> >> > the step count, they will read the step counter register.
> >> >
> >> > To support this functionality we need a steps attribute that
> >> > will export the number of steps.
> >> >
> >> I'm not keen on multiplexing different types of data onto a single
> >activity type.
> >> Steps is well enough defined on it's own to have it's own channel
> >type.
> >>
> >> in_steps_input would be fine by me.  I suppose steps might mean
> >something else
> >> though...
> >>
> >
> >Hi Jonathan,
> >
> >Thanks for the review.
> >
> >Moving the pedometer part to a new type sounds good to me.
> >However, I would prefer to add a new type called pedometer and keep
> >steps as a modifier, generating names like in_ped_steps_input for the
> >attribute and in_ped_steps_instance_en for the event.
> >The reason for this is that for supporting Freescale's MMA9553L we will
> >need additional attributes (distance, speed, calories, height, weight)
> >that we can add as modifiers to this pedometer type. To keep things
> >simple, I did not add these additional attributes to the RFC series,
> >but I could do that if you think it would be useful. For this device,
> >the motion events (walking, running, jogging, still) also depend on the
> >height attribute being set, but we intend to deal with this dependency
> >in the driver (using the pedometer's height attribute).
> >
> >What do you think?
> I think I would rather each of these was included as a type rather than a modifier.
> There are lots of other ways to measure speed for starters and often user space
> won't care where it comes from...
> 
> In_speed_raw etc...
> 
> Trick where possible is to think about what is measured rather than how. Tends to give
>  more consistent interfaces.

That makes sense. Will do the changes and also add one of these other types in v2.

Thanks,
Irina

ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [RFC PATCH 1/8] iio: dummy: Introduce virtual registers for dummy device
  2014-10-06 11:17     ` Daniel Baluta
@ 2014-10-09 19:28       ` Jonathan Cameron
  0 siblings, 0 replies; 27+ messages in thread
From: Jonathan Cameron @ 2014-10-09 19:28 UTC (permalink / raw)
  To: Daniel Baluta; +Cc: linux-iio, Linux Kernel Mailing List, irina.tirdea

On 06/10/14 12:17, Daniel Baluta wrote:
> On Sat, Oct 4, 2014 at 3:48 PM, Jonathan Cameron <jic23@kernel.org> wrote:
>> On 02/10/14 14:43, Daniel Baluta wrote:
>>> We need a way to store events generated by iio_dummy_evgen module,
>>> in order to correctly process IRQs in iio_simple_dummy_events.
>>>
>>> For the moment, we add two registers:
>>>
>>> * id_reg  - ID register, stores the source of the event
>>> * id_data - DATA register, stores the type of the event
>>>
>>> e.g echo 4 > /sys/bus/iio/devices/iio_evgen/poke2
>>>
>>> id_reg 0x02, id_data 0x04
>>>
>>> This means, event of type 4 was generated by fake device 2.
>>>
>>> We currently use a hardcoded mapping of virtual events to IIO events.
>>>
>>> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
>>> Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
>> Does the job and will look enough like a normal interrupt that
>> it allows the dummy driver to act as example code.
> 
> Hi Jonathan,
> 
> Thanks for the reviews.
> 
> This patch is self contained and it could be merged
> separately if you think it's stable enough.
Given there's no rush I'll wait for the whole series.
> 
> We are going over your reviews for the other patches and will send v2
> as soon as possible.
> 
> Daniel.
> 

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [RFC PATCH 3/8] iio: core: Introduce new MOTION event
  2014-10-06 14:17     ` Daniel Baluta
@ 2014-10-09 19:31       ` Jonathan Cameron
  2014-10-11  9:47         ` Daniel Baluta
  0 siblings, 1 reply; 27+ messages in thread
From: Jonathan Cameron @ 2014-10-09 19:31 UTC (permalink / raw)
  To: Daniel Baluta; +Cc: linux-iio, linux-kernel, irina.tirdea

On 06/10/14 15:17, Daniel Baluta wrote:
> 
> 
> On 10/04/2014 04:12 PM, Jonathan Cameron wrote:
>> On 02/10/14 14:43, Daniel Baluta wrote:
>>> This is to be used by drivers to signal detection of motion. We also
>>> add some possible values for motion as IIO events modifiers:
>>>     * running
>>>     * jogging
>>>     * walking
>>>     * still
>>>
>>> These values are supported by Frescale's MMA9553 sensor:
>>>
>>> http://freescale.com/files/sensors/doc/ref_manual/MMA9553LSWRM.pdf
>>>
>>> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
>>> Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
>> Hmm.. This is the interesting one.
>> Not immediately obvious how best to represent this stuff.
>>> ---
>>>   Documentation/ABI/testing/sysfs-bus-iio | 7 +++++++
>>>   drivers/iio/industrialio-core.c         | 4 ++++
>>>   drivers/iio/industrialio-event.c        | 1 +
>>>   include/linux/iio/types.h               | 7 ++++++-
>>>   4 files changed, 18 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Documentation/ABI/testing/sysfs-bus-iio
>>> b/Documentation/ABI/testing/sysfs-bus-iio
>>> index d760b02..070346d 100644
>>> --- a/Documentation/ABI/testing/sysfs-bus-iio
>>> +++ b/Documentation/ABI/testing/sysfs-bus-iio
>>> @@ -808,6 +808,13 @@ Description:
>>>           number or direction is not specified, applies to all channels of
>>>           this type.
>>>
>>> +What:        /sys/.../events/in_activity_motion_either_en
>>> +KernelVersion:    3.17
>>> +Contact:    linux-iio@vger.kernel.org
>>> +Description:
>>> +        Enables or disables motion detection. Each time motion is detected an
>>> +        event of this type will be generated.
>>> +
>> The either bit seems a bit random but I can see there is no particularly obvious
>> alternative.
> 
> I wonder if introducing a new IIO_EV_DIR_NONE event direction type would make
> sense. In this case the sysfs attribute will drop event direction text from its
> name (e.g /sys/.../events/in_activity_motion_en)
> 
>>
>> We really need a clean way of representing a multilevel 'state change' like this.
>>
>> Looking at the event code, I almost wonder if we would be better using the
>> direction element for running, walking etc rather than a modifier.
> 
> When pushing events code to userspace the modifier seemed to be the only option.
> 
>>
>> Having said that we will probably also get devices where this is polled rather
>> than
>> event.  'What activity is currently going on?'
> 
> Adding IIO_EV_INFO_VALUE bit, would create an attribute
> /sys/.../events/in_activity_motion_either_value that could expose the current
> activity going on.
> 
>> If we take that view modifiers make sense as it becomes
>> 'Is the user running?'  Perhaps even offering a confidence interval, e.g units as
>> percentage
>> in_activity_running_input 0..100
>> in_activity_walking_input 0..100
>> etc
>>
>> Then our event becomes a state change event (yup we'll need to add that)
>>
>> /events/in_activity_walking_rising_en  will then cause events when the percentage
>> confidence on a state rises above the provided threshold or goes above it
>> (default of 50% perhaps on devices which only report one state).
>>
>> /events/in_activity_walking_falling_en will do the leaving case.
> 
> This is a very nice idea and it will also offer more flexibility. I am not sure
> about the use case of confidence interval but using 0 and 100 will do the trick
> for us.
Sure, feel free to propose something else.  We could define a confidence interval
that counts as 'we think it is this'.  Basically just use values of 0 or 100 when
there is no explicit indication of the confidence available.  Not sure what
you do get ;)
> 
> We will use this interface for implementation of significant motion in Android's
> HAL. [1]
> 
> I will experiment more with how IIO attributes work and I will send a v2
> using direction instead of modifier for activity type (running, walking etc).
> 
> 
>>
>> Note these are just some quick initial thoughts on alternative methods.
>> I'll want to think on this more and get responses from more interested
>> parties!
> 
> Thanks a lot for your time!
You are welcome.  Funnily enough I rather enjoy trying to think of ways to
handle new 'weird' hardware in a consistent fashion :)
> 
> Daniel.
> 
> [1] https://source.android.com/devices/sensors/composite_sensors.html

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [RFC PATCH 3/8] iio: core: Introduce new MOTION event
  2014-10-07 10:48     ` Daniel Baluta
@ 2014-10-09 19:37       ` Jonathan Cameron
  0 siblings, 0 replies; 27+ messages in thread
From: Jonathan Cameron @ 2014-10-09 19:37 UTC (permalink / raw)
  To: Daniel Baluta; +Cc: linux-iio, Linux Kernel Mailing List, irina.tirdea

On 07/10/14 11:48, Daniel Baluta wrote:
> On Sat, Oct 4, 2014 at 4:12 PM, Jonathan Cameron <jic23@kernel.org> wrote:
>> On 02/10/14 14:43, Daniel Baluta wrote:
>>> This is to be used by drivers to signal detection of motion. We also
>>> add some possible values for motion as IIO events modifiers:
>>>       * running
>>>       * jogging
>>>       * walking
>>>       * still
>>>
>>> These values are supported by Frescale's MMA9553 sensor:
>>>
>>> http://freescale.com/files/sensors/doc/ref_manual/MMA9553LSWRM.pdf
>>>
>>> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
>>> Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
>> Hmm.. This is the interesting one.
>> Not immediately obvious how best to represent this stuff.
>>> ---
>>>  Documentation/ABI/testing/sysfs-bus-iio | 7 +++++++
>>>  drivers/iio/industrialio-core.c         | 4 ++++
>>>  drivers/iio/industrialio-event.c        | 1 +
>>>  include/linux/iio/types.h               | 7 ++++++-
>>>  4 files changed, 18 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
>>> index d760b02..070346d 100644
>>> --- a/Documentation/ABI/testing/sysfs-bus-iio
>>> +++ b/Documentation/ABI/testing/sysfs-bus-iio
>>> @@ -808,6 +808,13 @@ Description:
>>>               number or direction is not specified, applies to all channels of
>>>               this type.
>>>
>>> +What:                /sys/.../events/in_activity_motion_either_en
>>> +KernelVersion:       3.17
>>> +Contact:     linux-iio@vger.kernel.org
>>> +Description:
>>> +             Enables or disables motion detection. Each time motion is detected an
>>> +             event of this type will be generated.
>>> +
>> The either bit seems a bit random but I can see there is no particularly obvious
>> alternative.
>>
>> We really need a clean way of representing a multilevel 'state change' like this.
>>
>> Looking at the event code, I almost wonder if we would be better using the
>> direction element for running, walking etc rather than a modifier.
>>
>> Having said that we will probably also get devices where this is polled rather than
>> event.  'What activity is currently going on?'
>> If we take that view modifiers make sense as it becomes
>> 'Is the user running?'  Perhaps even offering a confidence interval, e.g units as
>> percentage
>> in_activity_running_input 0..100
>> in_activity_walking_input 0..100
>> etc
>>
> 
> This is clear.
> 
>> Then our event becomes a state change event (yup we'll need to add that)
> 
> So this means we will have a single event type named "state_change"?
> Or better "transition".
I was being rather incoherent here.  I think we are mostly covered
simple rising type events for the confidence interval measurements for
each of the activity_running, activity_walking etc 'modified channels'
we have above.

Don't need a state change.  If we actually have a device that interrupts on
simply the event has change, then the event handling will need to check what
the new state is and fire the relevant event.
> 
>>
>> /events/in_activity_walking_rising_en  will then cause events when the percentage
>> confidence on a state rises above the provided threshold or goes above it
>> (default of 50% perhaps on devices which only report one state).
> 
> Then here the attribute name will be:
> /events/in_activity_state_change_walking_rising_en
> 
> or if we call the event type "transition" this will be:
> /events/in_activity_transition_walking_rising_en
> 
> I am a little bit confused :).
I confused myself as well.

we should end up with
/events/in_activity_walking_rising_en
/events/in_activity_running_rising_en
/events/in_activity_hopping_rising_en
(I'll stop there before things get silly)

Each of which claims to based on the confidence level of that state, even if that
only ever takes values of 0 and 100%
> 
>>
>> /events/in_activity_walking_falling_en will do the leaving case.
>>
>> Note these are just some quick initial thoughts on alternative methods.
>> I'll want to think on this more and get responses from more interested
>> parties!
>>
>>>  What:                /sys/bus/iio/devices/iio:deviceX/trigger/current_trigger
>>>  KernelVersion:       2.6.35
>>>  Contact:     linux-iio@vger.kernel.org
>>> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
>>> index 67e8561..e453ef9 100644
>>> --- a/drivers/iio/industrialio-core.c
>>> +++ b/drivers/iio/industrialio-core.c
>>> @@ -92,6 +92,10 @@ static const char * const iio_modifier_names[] = {
>>>       [IIO_MOD_NORTH_TRUE] = "from_north_true",
>>>       [IIO_MOD_NORTH_MAGN_TILT_COMP] = "from_north_magnetic_tilt_comp",
>>>       [IIO_MOD_NORTH_TRUE_TILT_COMP] = "from_north_true_tilt_comp",
>>> +     [IIO_MOD_RUNNING] = "running",
>>> +     [IIO_MOD_JOGGING] = "jogging",
>>> +     [IIO_MOD_WALKING] = "walking",
>>> +     [IIO_MOD_STILL] = "still",
>>>  };
>>>
>>>  /* relies on pairs of these shared then separate */
>>> diff --git a/drivers/iio/industrialio-event.c b/drivers/iio/industrialio-event.c
>>> index 0c1e37e..eca5af2 100644
>>> --- a/drivers/iio/industrialio-event.c
>>> +++ b/drivers/iio/industrialio-event.c
>>> @@ -197,6 +197,7 @@ static const char * const iio_ev_type_text[] = {
>>>       [IIO_EV_TYPE_ROC] = "roc",
>>>       [IIO_EV_TYPE_THRESH_ADAPTIVE] = "thresh_adaptive",
>>>       [IIO_EV_TYPE_MAG_ADAPTIVE] = "mag_adaptive",
>>> +     [IIO_EV_TYPE_MOTION] = "motion",
>>>  };
>>>
>>>  static const char * const iio_ev_dir_text[] = {
>>> diff --git a/include/linux/iio/types.h b/include/linux/iio/types.h
>>> index d58769a..003638d 100644
>>> --- a/include/linux/iio/types.h
>>> +++ b/include/linux/iio/types.h
>>> @@ -60,7 +60,11 @@ enum iio_modifier {
>>>       IIO_MOD_NORTH_MAGN,
>>>       IIO_MOD_NORTH_TRUE,
>>>       IIO_MOD_NORTH_MAGN_TILT_COMP,
>>> -     IIO_MOD_NORTH_TRUE_TILT_COMP
>>> +     IIO_MOD_NORTH_TRUE_TILT_COMP,
>>> +     IIO_MOD_RUNNING,
>>> +     IIO_MOD_JOGGING,
>>> +     IIO_MOD_WALKING,
>>> +     IIO_MOD_STILL,
>>>  };
>>>
>>>  enum iio_event_type {
>>> @@ -69,6 +73,7 @@ enum iio_event_type {
>>>       IIO_EV_TYPE_ROC,
>>>       IIO_EV_TYPE_THRESH_ADAPTIVE,
>>>       IIO_EV_TYPE_MAG_ADAPTIVE,
>>> +     IIO_EV_TYPE_MOTION,
>>>  };
>>>
>>>  enum iio_event_info {
>>>
>> --
>> 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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [RFC PATCH 3/8] iio: core: Introduce new MOTION event
  2014-10-09 19:31       ` Jonathan Cameron
@ 2014-10-11  9:47         ` Daniel Baluta
  2014-10-13  9:46           ` Karol Wrona
  0 siblings, 1 reply; 27+ messages in thread
From: Daniel Baluta @ 2014-10-11  9:47 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Daniel Baluta, linux-iio, Linux Kernel Mailing List,
	irina.tirdea, Karol Wrona

On Thu, Oct 9, 2014 at 10:31 PM, Jonathan Cameron <jic23@kernel.org> wrote:
> On 06/10/14 15:17, Daniel Baluta wrote:
>>
>>
>> On 10/04/2014 04:12 PM, Jonathan Cameron wrote:
>>> On 02/10/14 14:43, Daniel Baluta wrote:
>>>> This is to be used by drivers to signal detection of motion. We also
>>>> add some possible values for motion as IIO events modifiers:
>>>>     * running
>>>>     * jogging
>>>>     * walking
>>>>     * still
>>>>
>>>> These values are supported by Frescale's MMA9553 sensor:
>>>>
>>>> http://freescale.com/files/sensors/doc/ref_manual/MMA9553LSWRM.pdf
>>>>
>>>> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
>>>> Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
>>> Hmm.. This is the interesting one.
>>> Not immediately obvious how best to represent this stuff.
>>>> ---
>>>>   Documentation/ABI/testing/sysfs-bus-iio | 7 +++++++
>>>>   drivers/iio/industrialio-core.c         | 4 ++++
>>>>   drivers/iio/industrialio-event.c        | 1 +
>>>>   include/linux/iio/types.h               | 7 ++++++-
>>>>   4 files changed, 18 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/Documentation/ABI/testing/sysfs-bus-iio
>>>> b/Documentation/ABI/testing/sysfs-bus-iio
>>>> index d760b02..070346d 100644
>>>> --- a/Documentation/ABI/testing/sysfs-bus-iio
>>>> +++ b/Documentation/ABI/testing/sysfs-bus-iio
>>>> @@ -808,6 +808,13 @@ Description:
>>>>           number or direction is not specified, applies to all channels of
>>>>           this type.
>>>>
>>>> +What:        /sys/.../events/in_activity_motion_either_en
>>>> +KernelVersion:    3.17
>>>> +Contact:    linux-iio@vger.kernel.org
>>>> +Description:
>>>> +        Enables or disables motion detection. Each time motion is detected an
>>>> +        event of this type will be generated.
>>>> +
>>> The either bit seems a bit random but I can see there is no particularly obvious
>>> alternative.
>>
>> I wonder if introducing a new IIO_EV_DIR_NONE event direction type would make
>> sense. In this case the sysfs attribute will drop event direction text from its
>> name (e.g /sys/.../events/in_activity_motion_en)
>>
>>>
>>> We really need a clean way of representing a multilevel 'state change' like this.
>>>
>>> Looking at the event code, I almost wonder if we would be better using the
>>> direction element for running, walking etc rather than a modifier.
>>
>> When pushing events code to userspace the modifier seemed to be the only option.
>>
>>>
>>> Having said that we will probably also get devices where this is polled rather
>>> than
>>> event.  'What activity is currently going on?'
>>
>> Adding IIO_EV_INFO_VALUE bit, would create an attribute
>> /sys/.../events/in_activity_motion_either_value that could expose the current
>> activity going on.
>>
>>> If we take that view modifiers make sense as it becomes
>>> 'Is the user running?'  Perhaps even offering a confidence interval, e.g units as
>>> percentage
>>> in_activity_running_input 0..100
>>> in_activity_walking_input 0..100
>>> etc
>>>
>>> Then our event becomes a state change event (yup we'll need to add that)
>>>
>>> /events/in_activity_walking_rising_en  will then cause events when the percentage
>>> confidence on a state rises above the provided threshold or goes above it
>>> (default of 50% perhaps on devices which only report one state).
>>>
>>> /events/in_activity_walking_falling_en will do the leaving case.
>>
>> This is a very nice idea and it will also offer more flexibility. I am not sure
>> about the use case of confidence interval but using 0 and 100 will do the trick
>> for us.
> Sure, feel free to propose something else.  We could define a confidence interval
> that counts as 'we think it is this'.  Basically just use values of 0 or 100 when
> there is no explicit indication of the confidence available.  Not sure what
> you do get ;)
>>
>> We will use this interface for implementation of significant motion in Android's
>> HAL. [1]
>>
>> I will experiment more with how IIO attributes work and I will send a v2
>> using direction instead of modifier for activity type (running, walking etc).
>>
>>
>>>
>>> Note these are just some quick initial thoughts on alternative methods.
>>> I'll want to think on this more and get responses from more interested
>>> parties!
>>
>> Thanks a lot for your time!
> You are welcome.  Funnily enough I rather enjoy trying to think of ways to
> handle new 'weird' hardware in a consistent fashion :)

We have already sent a second proposal :).

http://marc.info/?l=linux-iio&m=141285801717857&w=2

We are also hoping to get more opinions from other parties.
CC'ing Karol from Samsung :).

Daniel.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [RFC PATCH 3/8] iio: core: Introduce new MOTION event
  2014-10-11  9:47         ` Daniel Baluta
@ 2014-10-13  9:46           ` Karol Wrona
  0 siblings, 0 replies; 27+ messages in thread
From: Karol Wrona @ 2014-10-13  9:46 UTC (permalink / raw)
  To: Daniel Baluta, Jonathan Cameron
  Cc: linux-iio, Linux Kernel Mailing List, irina.tirdea

On 10/11/2014 11:47 AM, Daniel Baluta wrote:
> On Thu, Oct 9, 2014 at 10:31 PM, Jonathan Cameron <jic23@kernel.org> wrote:
>> On 06/10/14 15:17, Daniel Baluta wrote:
>>>
>>> On 10/04/2014 04:12 PM, Jonathan Cameron wrote:
>>>> On 02/10/14 14:43, Daniel Baluta wrote:
>>>>> This is to be used by drivers to signal detection of motion. We also
>>>>> add some possible values for motion as IIO events modifiers:
>>>>>      * running
>>>>>      * jogging
>>>>>      * walking
>>>>>      * still
>>>>>
>>>>> These values are supported by Frescale's MMA9553 sensor:
>>>>>
>>>>> http://freescale.com/files/sensors/doc/ref_manual/MMA9553LSWRM.pdf
>>>>>
>>>>> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
>>>>> Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
>>>> Hmm.. This is the interesting one.
>>>> Not immediately obvious how best to represent this stuff.
>>>>> ---
>>>>>    Documentation/ABI/testing/sysfs-bus-iio | 7 +++++++
>>>>>    drivers/iio/industrialio-core.c         | 4 ++++
>>>>>    drivers/iio/industrialio-event.c        | 1 +
>>>>>    include/linux/iio/types.h               | 7 ++++++-
>>>>>    4 files changed, 18 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/Documentation/ABI/testing/sysfs-bus-iio
>>>>> b/Documentation/ABI/testing/sysfs-bus-iio
>>>>> index d760b02..070346d 100644
>>>>> --- a/Documentation/ABI/testing/sysfs-bus-iio
>>>>> +++ b/Documentation/ABI/testing/sysfs-bus-iio
>>>>> @@ -808,6 +808,13 @@ Description:
>>>>>            number or direction is not specified, applies to all channels of
>>>>>            this type.
>>>>>
>>>>> +What:        /sys/.../events/in_activity_motion_either_en
>>>>> +KernelVersion:    3.17
>>>>> +Contact:    linux-iio@vger.kernel.org
>>>>> +Description:
>>>>> +        Enables or disables motion detection. Each time motion is detected an
>>>>> +        event of this type will be generated.
>>>>> +
>>>> The either bit seems a bit random but I can see there is no particularly obvious
>>>> alternative.
>>> I wonder if introducing a new IIO_EV_DIR_NONE event direction type would make
>>> sense. In this case the sysfs attribute will drop event direction text from its
>>> name (e.g /sys/.../events/in_activity_motion_en)
>>>
>>>> We really need a clean way of representing a multilevel 'state change' like this.
>>>>
>>>> Looking at the event code, I almost wonder if we would be better using the
>>>> direction element for running, walking etc rather than a modifier.
>>> When pushing events code to userspace the modifier seemed to be the only option.
>>>
>>>> Having said that we will probably also get devices where this is polled rather
>>>> than
>>>> event.  'What activity is currently going on?'
>>> Adding IIO_EV_INFO_VALUE bit, would create an attribute
>>> /sys/.../events/in_activity_motion_either_value that could expose the current
>>> activity going on.
>>>
>>>> If we take that view modifiers make sense as it becomes
>>>> 'Is the user running?'  Perhaps even offering a confidence interval, e.g units as
>>>> percentage
>>>> in_activity_running_input 0..100
>>>> in_activity_walking_input 0..100
>>>> etc
>>>>
>>>> Then our event becomes a state change event (yup we'll need to add that)
>>>>
>>>> /events/in_activity_walking_rising_en  will then cause events when the percentage
>>>> confidence on a state rises above the provided threshold or goes above it
>>>> (default of 50% perhaps on devices which only report one state).
>>>>
>>>> /events/in_activity_walking_falling_en will do the leaving case.
>>> This is a very nice idea and it will also offer more flexibility. I am not sure
>>> about the use case of confidence interval but using 0 and 100 will do the trick
>>> for us.
>> Sure, feel free to propose something else.  We could define a confidence interval
>> that counts as 'we think it is this'.  Basically just use values of 0 or 100 when
>> there is no explicit indication of the confidence available.  Not sure what
>> you do get ;)
>>> We will use this interface for implementation of significant motion in Android's
>>> HAL. [1]
>>>
>>> I will experiment more with how IIO attributes work and I will send a v2
>>> using direction instead of modifier for activity type (running, walking etc).
>>>
>>>
>>>> Note these are just some quick initial thoughts on alternative methods.
>>>> I'll want to think on this more and get responses from more interested
>>>> parties!
>>> Thanks a lot for your time!
>> You are welcome.  Funnily enough I rather enjoy trying to think of ways to
>> handle new 'weird' hardware in a consistent fashion :)
> We have already sent a second proposal :).
>
> http://marc.info/?l=linux-iio&m=141285801717857&w=2
>
> We are also hoping to get more opinions from other parties.
> CC'ing Karol from Samsung :).
>
> Daniel.
>
Thanks for the info.
In my case I have to figure out what is really needed between hw and 
driver first.
The existing android driver has given a data blob to user space as 
concerns new
sensor classes. Please, give a few days.

Karol

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [RFC PATCH 1/8] iio: dummy: Introduce virtual registers for dummy device
  2014-10-02 13:43 ` [RFC PATCH 1/8] iio: dummy: Introduce virtual registers for dummy device Daniel Baluta
  2014-10-04 12:48   ` Jonathan Cameron
@ 2014-10-19 20:30   ` Hartmut Knaack
  2014-10-19 20:39     ` Daniel Baluta
  1 sibling, 1 reply; 27+ messages in thread
From: Hartmut Knaack @ 2014-10-19 20:30 UTC (permalink / raw)
  To: Daniel Baluta, jic23, linux-iio, linux-kernel; +Cc: irina.tirdea

Daniel Baluta schrieb am 02.10.2014 15:43:
> We need a way to store events generated by iio_dummy_evgen module,
> in order to correctly process IRQs in iio_simple_dummy_events.
> 
> For the moment, we add two registers:
> 
> * id_reg  - ID register, stores the source of the event
> * id_data - DATA register, stores the type of the event
> 
> e.g echo 4 > /sys/bus/iio/devices/iio_evgen/poke2
> 
> id_reg 0x02, id_data 0x04
> 
> This means, event of type 4 was generated by fake device 2.
> 
> We currently use a hardcoded mapping of virtual events to IIO events.
> 
> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
> Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
> ---
>  drivers/staging/iio/iio_dummy_evgen.c         | 16 ++++++++++++++++
>  drivers/staging/iio/iio_dummy_evgen.h         |  7 +++++++
>  drivers/staging/iio/iio_simple_dummy.h        |  2 ++
>  drivers/staging/iio/iio_simple_dummy_events.c | 23 ++++++++++++++++++-----
>  4 files changed, 43 insertions(+), 5 deletions(-)
> 
<...>
> @@ -153,6 +161,14 @@ static ssize_t iio_evgen_poke(struct device *dev,
>  			      size_t len)
>  {
>  	struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
> +	unsigned long event, ret;
int ret
> +
> +	ret = kstrtoul(buf, 10, &event);
> +	if (ret)
> +		return ret;
> +
> +	iio_evgen->regs[this_attr->address].reg_id   = this_attr->address;
> +	iio_evgen->regs[this_attr->address].reg_data = event;
>  
>  	if (iio_evgen->enabled[this_attr->address])
>  		handle_nested_irq(iio_evgen->base + this_attr->address);
<...>

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [RFC PATCH 1/8] iio: dummy: Introduce virtual registers for dummy device
  2014-10-19 20:30   ` Hartmut Knaack
@ 2014-10-19 20:39     ` Daniel Baluta
  0 siblings, 0 replies; 27+ messages in thread
From: Daniel Baluta @ 2014-10-19 20:39 UTC (permalink / raw)
  To: Hartmut Knaack
  Cc: Daniel Baluta, Jonathan Cameron, linux-iio,
	Linux Kernel Mailing List, irina.tirdea

On Sun, Oct 19, 2014 at 11:30 PM, Hartmut Knaack <knaack.h@gmx.de> wrote:
> Daniel Baluta schrieb am 02.10.2014 15:43:
>> We need a way to store events generated by iio_dummy_evgen module,
>> in order to correctly process IRQs in iio_simple_dummy_events.
>>
>> For the moment, we add two registers:
>>
>> * id_reg  - ID register, stores the source of the event
>> * id_data - DATA register, stores the type of the event
>>
>> e.g echo 4 > /sys/bus/iio/devices/iio_evgen/poke2
>>
>> id_reg 0x02, id_data 0x04
>>
>> This means, event of type 4 was generated by fake device 2.
>>
>> We currently use a hardcoded mapping of virtual events to IIO events.
>>
>> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
>> Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
>> ---
>>  drivers/staging/iio/iio_dummy_evgen.c         | 16 ++++++++++++++++
>>  drivers/staging/iio/iio_dummy_evgen.h         |  7 +++++++
>>  drivers/staging/iio/iio_simple_dummy.h        |  2 ++
>>  drivers/staging/iio/iio_simple_dummy_events.c | 23 ++++++++++++++++++-----
>>  4 files changed, 43 insertions(+), 5 deletions(-)
>>
> <...>
>> @@ -153,6 +161,14 @@ static ssize_t iio_evgen_poke(struct device *dev,
>>                             size_t len)
>>  {
>>       struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
>> +     unsigned long event, ret;
> int ret

Good catch. Will fix in v3.

Daniel.

^ permalink raw reply	[flat|nested] 27+ messages in thread

end of thread, other threads:[~2014-10-19 20:39 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-10-02 13:43 [RFC PATCH] iio: Introduce activity channel Daniel Baluta
2014-10-02 13:43 ` [RFC PATCH 1/8] iio: dummy: Introduce virtual registers for dummy device Daniel Baluta
2014-10-04 12:48   ` Jonathan Cameron
2014-10-06 11:17     ` Daniel Baluta
2014-10-09 19:28       ` Jonathan Cameron
2014-10-19 20:30   ` Hartmut Knaack
2014-10-19 20:39     ` Daniel Baluta
2014-10-02 13:43 ` [RFC PATCH 2/8] iio: core: Introduce IIO_ACTIVITY channel Daniel Baluta
2014-10-04 13:00   ` Jonathan Cameron
2014-10-02 13:43 ` [RFC PATCH 3/8] iio: core: Introduce new MOTION event Daniel Baluta
2014-10-04 13:12   ` Jonathan Cameron
2014-10-06 14:17     ` Daniel Baluta
2014-10-09 19:31       ` Jonathan Cameron
2014-10-11  9:47         ` Daniel Baluta
2014-10-13  9:46           ` Karol Wrona
2014-10-07 10:48     ` Daniel Baluta
2014-10-09 19:37       ` Jonathan Cameron
2014-10-02 13:43 ` [RFC PATCH 4/8] iio: core: Introduce pedometer STEP counter modifier Daniel Baluta
2014-10-04 12:53   ` Jonathan Cameron
2014-10-06 13:50     ` Tirdea, Irina
2014-10-06 16:31       ` Jonathan Cameron
2014-10-07 13:54         ` Tirdea, Irina
2014-10-02 13:43 ` [RFC PATCH 5/8] iio: core: Introduce ENABLE channel info mask Daniel Baluta
2014-10-02 13:43 ` [RFC PATCH 6/8] iio: core: Introduce new STEP_DETECT event Daniel Baluta
2014-10-04 12:56   ` Jonathan Cameron
2014-10-02 13:43 ` [RFC PATCH 7/8] iio: dummy: Demonstrate the usage of activity channel Daniel Baluta
2014-10-02 13:43 ` [RFC PATCH 8/8] iio: event_monitor: Add support for " Daniel Baluta

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).