[PATCH 1/3] iio: mma8452: add freefall detection for Freescale's accelerometers

[PATCH 1/3] iio: mma8452: add freefall detection for Freescale's accelerometers

[Martin Kepplinger]

This adds freefall event detection to the supported devices. It adds
the in_accel_x&y&z_mag_falling_en iio event attribute, which activates
freefall mode.

In freefall mode, the current acceleration values of all activated axis
are added and if the *sum* falls *under* the threshold specified
(in_accel_mag_falling_value), the appropriate IIO event code
is generated.

By enabling freefall mode (in_accel_x&y&z_mag_falling_en)
all 3 axis are enabled too as this describes a classic freefall
detection. Of course the user is free to disable one or more directions.

The values of rising and falling versions of various sysfs files are
shared, which is compliant to the IIO specification.

This is what the sysfs "events" directory for these devices looks
like after this change:

-rw-r--r--    4096 Oct 23 08:45 in_accel_mag_falling_period
-rw-r--r--    4096 Oct 23 08:45 in_accel_mag_falling_value
-rw-r--r--    4096 Oct 23 08:45 in_accel_mag_rising_period
-rw-r--r--    4096 Oct 23 08:45 in_accel_mag_rising_value
-r--r--r--    4096 Oct 23 08:45 in_accel_scale
-rw-r--r--    4096 Oct 23 08:45 in_accel_x&y&z_mag_falling_en
-rw-r--r--    4096 Oct 23 08:45 in_accel_x_mag_falling_en
-rw-r--r--    4096 Oct 23 08:45 in_accel_x_mag_rising_en
-rw-r--r--    4096 Oct 23 08:45 in_accel_y_mag_falling_en
-rw-r--r--    4096 Oct 23 08:45 in_accel_y_mag_rising_en
-rw-r--r--    4096 Oct 23 08:45 in_accel_z_mag_falling_en
-rw-r--r--    4096 Oct 23 08:45 in_accel_z_mag_rising_en

Signed-off-by: Martin Kepplinger <martin.kepplinger@theobroma-systems.com>
Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com>
---
 drivers/iio/accel/mma8452.c | 114 ++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 111 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 116a6e4..dedcc1d 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -15,7 +15,7 @@
  *
  * 7-bit I2C slave address 0x1c/0x1d (pin selectable)
  *
- * TODO: orientation / freefall events, autosleep
+ * TODO: orientation events, autosleep
  */
 
 #include <linux/module.h>
@@ -144,6 +144,12 @@ struct mma_chip_info {
 	u8 ev_count;
 };
 
+enum {
+	axis_x,
+	axis_y,
+	axis_z,
+};
+
 static int mma8452_drdy(struct mma8452_data *data)
 {
 	int tries = 150;
@@ -410,6 +416,74 @@ fail:
 	return ret;
 }
 
+static ssize_t mma8452_get_freefall_mode(struct device *dev,
+					 struct device_attribute *attr,
+					 char *buf)
+{
+	int val;
+	struct mma8452_data *data = iio_priv(i2c_get_clientdata(
+					     to_i2c_client(dev)));
+	const struct mma_chip_info *chip = data->chip_info;
+
+	mutex_lock(&data->lock);
+	val = i2c_smbus_read_byte_data(data->client, chip->ev_cfg);
+	mutex_unlock(&data->lock);
+	if (val < 0)
+		return val;
+
+	return scnprintf(buf, PAGE_SIZE, "%d\n",
+			 !(val & MMA8452_FF_MT_CFG_OAE));
+}
+
+static ssize_t mma8452_set_freefall_mode(struct device *dev,
+					 struct device_attribute *attr,
+					 const char *buf,
+					 size_t len)
+{
+	int val, ret;
+	u8 user_val;
+	struct mma8452_data *data = iio_priv(i2c_get_clientdata(
+					     to_i2c_client(dev)));
+	const struct mma_chip_info *chip = data->chip_info;
+
+	ret = kstrtou8(buf, 10, &user_val);
+	if (ret)
+		goto err;
+
+	val = i2c_smbus_read_byte_data(data->client, chip->ev_cfg);
+	if (val < 0) {
+		ret = val;
+		goto err;
+	}
+
+	if (user_val && (val & MMA8452_FF_MT_CFG_OAE)) {
+		val |= BIT(axis_x + chip->ev_cfg_chan_shift);
+		val |= BIT(axis_y + chip->ev_cfg_chan_shift);
+		val |= BIT(axis_z + chip->ev_cfg_chan_shift);
+		val &= ~MMA8452_FF_MT_CFG_OAE;
+	} else if (!user_val && !(val & MMA8452_FF_MT_CFG_OAE)) {
+		val &= ~BIT(axis_x + chip->ev_cfg_chan_shift);
+		val &= ~BIT(axis_y + chip->ev_cfg_chan_shift);
+		val &= ~BIT(axis_z + chip->ev_cfg_chan_shift);
+		val |= MMA8452_FF_MT_CFG_OAE;
+	}
+
+	ret = mma8452_change_config(data, chip->ev_cfg, val);
+	if (ret)
+		goto err;
+
+	return len;
+err:
+	return ret;
+}
+
+static IIO_DEVICE_ATTR_NAMED(accel_xayaz_mag_falling_en,
+			     in_accel_x&y&z_mag_falling_en,
+			     S_IRUGO | S_IWUSR,
+			     mma8452_get_freefall_mode,
+			     mma8452_set_freefall_mode,
+			     0);
+
 static int mma8452_set_hp_filter_frequency(struct mma8452_data *data,
 					   int val, int val2)
 {
@@ -631,7 +705,6 @@ static int mma8452_write_event_config(struct iio_dev *indio_dev,
 		val &= ~BIT(chan->scan_index + chip->ev_cfg_chan_shift);
 
 	val |= chip->ev_cfg_ele;
-	val |= MMA8452_FF_MT_CFG_OAE;
 
 	return mma8452_change_config(data, chip->ev_cfg, val);
 }
@@ -640,12 +713,26 @@ static void mma8452_transient_interrupt(struct iio_dev *indio_dev)
 {
 	struct mma8452_data *data = iio_priv(indio_dev);
 	s64 ts = iio_get_time_ns();
-	int src;
+	int src, cfg;
 
 	src = i2c_smbus_read_byte_data(data->client, data->chip_info->ev_src);
 	if (src < 0)
 		return;
 
+	cfg = i2c_smbus_read_byte_data(data->client, data->chip_info->ev_cfg);
+	if (cfg < 0)
+		return;
+
+	if (!(cfg & MMA8452_FF_MT_CFG_OAE)) {
+		iio_push_event(indio_dev,
+			       IIO_MOD_EVENT_CODE(IIO_ACCEL, 0,
+						  IIO_MOD_X_AND_Y_AND_Z,
+						  IIO_EV_TYPE_MAG,
+						  IIO_EV_DIR_FALLING),
+			       ts);
+		return;
+	}
+
 	if (src & data->chip_info->ev_src_xe)
 		iio_push_event(indio_dev,
 			       IIO_MOD_EVENT_CODE(IIO_ACCEL, 0, IIO_MOD_X,
@@ -758,6 +845,13 @@ static const struct iio_event_spec mma8452_motion_event[] = {
 		.mask_shared_by_type = BIT(IIO_EV_INFO_VALUE) |
 					BIT(IIO_EV_INFO_PERIOD)
 	},
+	{
+		.type = IIO_EV_TYPE_MAG,
+		.dir = IIO_EV_DIR_FALLING,
+		.mask_separate = BIT(IIO_EV_INFO_ENABLE),
+		.mask_shared_by_type = BIT(IIO_EV_INFO_VALUE) |
+					BIT(IIO_EV_INFO_PERIOD)
+	},
 };
 
 /*
@@ -768,6 +862,7 @@ static IIO_CONST_ATTR_NAMED(accel_transient_scale, in_accel_scale, "0.617742");
 
 static struct attribute *mma8452_event_attributes[] = {
 	&iio_const_attr_accel_transient_scale.dev_attr.attr,
+	&iio_dev_attr_accel_xayaz_mag_falling_en.dev_attr.attr,
 	NULL,
 };
 
@@ -1057,6 +1152,7 @@ static int mma8452_probe(struct i2c_client *client,
 	struct mma8452_data *data;
 	struct iio_dev *indio_dev;
 	int ret;
+	u8 val;
 	const struct of_device_id *match;
 
 	match = of_match_device(mma8452_dt_ids, &client->dev);
@@ -1158,6 +1254,18 @@ static int mma8452_probe(struct i2c_client *client,
 			return ret;
 	}
 
+	/* don't activate freefall mode on startup */
+	ret = i2c_smbus_read_byte_data(data->client, data->chip_info->ev_cfg);
+	if (ret < 0)
+		return ret;
+
+	val = ret;
+	ret = i2c_smbus_write_byte_data(client,
+					data->chip_info->ev_cfg,
+					val | MMA8452_FF_MT_CFG_OAE);
+	if (ret < 0)
+		return ret;
+
 	data->ctrl_reg1 = MMA8452_CTRL_ACTIVE |
 			  (MMA8452_CTRL_DR_DEFAULT << MMA8452_CTRL_DR_SHIFT);
 	ret = i2c_smbus_write_byte_data(client, MMA8452_CTRL_REG1,
-- 
2.1.4

[PATCH 2/3] iio: mma8452: use enum for numbering the axis

[Martin Kepplinger]

Use the newly added enum that numbers x, y and z axis.

Signed-off-by: Martin Kepplinger <martin.kepplinger@theobroma-systems.com>
Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com>
---
 drivers/iio/accel/mma8452.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index dedcc1d..38652e1 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -912,30 +912,30 @@ static struct attribute_group mma8452_event_attribute_group = {
 }
 
 static const struct iio_chan_spec mma8452_channels[] = {
-	MMA8452_CHANNEL(X, 0, 12),
-	MMA8452_CHANNEL(Y, 1, 12),
-	MMA8452_CHANNEL(Z, 2, 12),
+	MMA8452_CHANNEL(X, axis_x, 12),
+	MMA8452_CHANNEL(Y, axis_y, 12),
+	MMA8452_CHANNEL(Z, axis_z, 12),
 	IIO_CHAN_SOFT_TIMESTAMP(3),
 };
 
 static const struct iio_chan_spec mma8453_channels[] = {
-	MMA8452_CHANNEL(X, 0, 10),
-	MMA8452_CHANNEL(Y, 1, 10),
-	MMA8452_CHANNEL(Z, 2, 10),
+	MMA8452_CHANNEL(X, axis_x, 10),
+	MMA8452_CHANNEL(Y, axis_y, 10),
+	MMA8452_CHANNEL(Z, axis_z, 10),
 	IIO_CHAN_SOFT_TIMESTAMP(3),
 };
 
 static const struct iio_chan_spec mma8652_channels[] = {
-	MMA8652_CHANNEL(X, 0, 12),
-	MMA8652_CHANNEL(Y, 1, 12),
-	MMA8652_CHANNEL(Z, 2, 12),
+	MMA8652_CHANNEL(X, axis_x, 12),
+	MMA8652_CHANNEL(Y, axis_y, 12),
+	MMA8652_CHANNEL(Z, axis_z, 12),
 	IIO_CHAN_SOFT_TIMESTAMP(3),
 };
 
 static const struct iio_chan_spec mma8653_channels[] = {
-	MMA8652_CHANNEL(X, 0, 10),
-	MMA8652_CHANNEL(Y, 1, 10),
-	MMA8652_CHANNEL(Z, 2, 10),
+	MMA8652_CHANNEL(X, axis_x, 10),
+	MMA8652_CHANNEL(Y, axis_y, 10),
+	MMA8652_CHANNEL(Z, axis_z, 10),
 	IIO_CHAN_SOFT_TIMESTAMP(3),
 };
 
-- 
2.1.4

[PATCH 3/3] iio: mma8452: remove unused register description

[Martin Kepplinger]

Signed-off-by: Martin Kepplinger <martin.kepplinger@theobroma-systems.com>
Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com>
---
 drivers/iio/accel/mma8452.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 38652e1..eb2b2c2 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -58,7 +58,6 @@
 #define MMA8452_FF_MT_COUNT			0x18
 #define MMA8452_TRANSIENT_CFG			0x1d
 #define  MMA8452_TRANSIENT_CFG_HPF_BYP		BIT(0)
-#define  MMA8452_TRANSIENT_CFG_CHAN(chan)	BIT(chan + 1)
 #define  MMA8452_TRANSIENT_CFG_ELE		BIT(4)
 #define MMA8452_TRANSIENT_SRC			0x1e
 #define  MMA8452_TRANSIENT_SRC_XTRANSE		BIT(1)
-- 
2.1.4

Re: [PATCH 1/3] iio: mma8452: add freefall detection for Freescale's accelerometers

[Lars-Peter Clausen]

On 11/11/2015 07:38 PM, Martin Kepplinger wrote:
> This adds freefall event detection to the supported devices. It adds the 
> in_accel_x&y&z_mag_falling_en iio event attribute, which activates 
> freefall mode.
> 

Hi,

Thanks for the patch, looks pretty good. Just a few things.

> In freefall mode, the current acceleration values of all activated axis 
> are added and if the *sum* falls *under* the threshold specified

Are you sure its not the magnitude of the vector rather than the sum of the
individual components?

> (in_accel_mag_falling_value), the appropriate IIO event code is 
> generated.
[...]
> +enum { +	axis_x, +	axis_y, +	axis_z, +};

That should be part of patch two I guess.

[...]
> +static IIO_DEVICE_ATTR_NAMED(accel_xayaz_mag_falling_en, + 
> in_accel_x&y&z_mag_falling_en, +			     S_IRUGO | S_IWUSR, + 
> mma8452_get_freefall_mode, +			     mma8452_set_freefall_mode, + 0);

This should be created by the core based on the event spec, shouldn't it?

Re: [PATCH 1/3] iio: mma8452: add freefall detection for Freescale's accelerometers

[Martin Kepplinger]

Am 2015-11-11 um 20:08 schrieb Lars-Peter Clausen:
> On 11/11/2015 07:38 PM, Martin Kepplinger wrote:
>> This adds freefall event detection to the supported devices. It adds the 
>> in_accel_x&y&z_mag_falling_en iio event attribute, which activates 
>> freefall mode.
>>
> 
> Hi,
> 
> Thanks for the patch, looks pretty good. Just a few things.
> 
>> In freefall mode, the current acceleration values of all activated axis 
>> are added and if the *sum* falls *under* the threshold specified
> 
> Are you sure its not the magnitude of the vector rather than the sum of the
> individual components?

It's the acceleration magnitude, logical AND combination of the values.
Will correct this for v2.

> 
>> (in_accel_mag_falling_value), the appropriate IIO event code is 
>> generated.
> [...]
>> +enum { +	axis_x, +	axis_y, +	axis_z, +};
> 
> That should be part of patch two I guess.

Will do.

> 
> [...]
>> +static IIO_DEVICE_ATTR_NAMED(accel_xayaz_mag_falling_en, + 
>> in_accel_x&y&z_mag_falling_en, +			     S_IRUGO | S_IWUSR, + 
>> mma8452_get_freefall_mode, +			     mma8452_set_freefall_mode, + 0);
> 
> This should be created by the core based on the event spec, shouldn't it?
> 

I'll check if this works when I get to it.

thanks for the review!

                                  martin

Re: [PATCH 1/3] iio: mma8452: add freefall detection for Freescale's accelerometers

[Jonathan Cameron]

On 11/11/15 18:38, Martin Kepplinger wrote:
> This adds freefall event detection to the supported devices. It adds
> the in_accel_x&y&z_mag_falling_en iio event attribute, which activates
> freefall mode.
> 
> In freefall mode, the current acceleration values of all activated axis
> are added and if the *sum* falls *under* the threshold specified
> (in_accel_mag_falling_value), the appropriate IIO event code
> is generated.
> 
> By enabling freefall mode (in_accel_x&y&z_mag_falling_en)
> all 3 axis are enabled too as this describes a classic freefall
> detection. Of course the user is free to disable one or more directions.
> 
> The values of rising and falling versions of various sysfs files are
> shared, which is compliant to the IIO specification.
> 
> This is what the sysfs "events" directory for these devices looks
> like after this change:
> 
> -rw-r--r--    4096 Oct 23 08:45 in_accel_mag_falling_period
> -rw-r--r--    4096 Oct 23 08:45 in_accel_mag_falling_value
> -rw-r--r--    4096 Oct 23 08:45 in_accel_mag_rising_period
> -rw-r--r--    4096 Oct 23 08:45 in_accel_mag_rising_value
> -r--r--r--    4096 Oct 23 08:45 in_accel_scale
> -rw-r--r--    4096 Oct 23 08:45 in_accel_x&y&z_mag_falling_en
> -rw-r--r--    4096 Oct 23 08:45 in_accel_x_mag_falling_en
> -rw-r--r--    4096 Oct 23 08:45 in_accel_x_mag_rising_en
> -rw-r--r--    4096 Oct 23 08:45 in_accel_y_mag_falling_en
> -rw-r--r--    4096 Oct 23 08:45 in_accel_y_mag_rising_en
> -rw-r--r--    4096 Oct 23 08:45 in_accel_z_mag_falling_en
> -rw-r--r--    4096 Oct 23 08:45 in_accel_z_mag_rising_en
> 
> Signed-off-by: Martin Kepplinger <martin.kepplinger@theobroma-systems.com>
> Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com>
Looks pretty good to me (other than obviously the bits Lars already
picked up on!)

My only real comment was that you could do the rest of the combined
possibilities whilst you are here (if you want to!)  You've
picked the mostly obviously useful one though so maybe leave it
at that.

Jonathan
> ---
>  drivers/iio/accel/mma8452.c | 114 ++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 111 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
> index 116a6e4..dedcc1d 100644
> --- a/drivers/iio/accel/mma8452.c
> +++ b/drivers/iio/accel/mma8452.c
> @@ -15,7 +15,7 @@
>   *
>   * 7-bit I2C slave address 0x1c/0x1d (pin selectable)
>   *
> - * TODO: orientation / freefall events, autosleep
> + * TODO: orientation events, autosleep
>   */
>  
>  #include <linux/module.h>
> @@ -144,6 +144,12 @@ struct mma_chip_info {
>  	u8 ev_count;
>  };
>  
> +enum {
> +	axis_x,
> +	axis_y,
> +	axis_z,
> +};
> +
>  static int mma8452_drdy(struct mma8452_data *data)
>  {
>  	int tries = 150;
> @@ -410,6 +416,74 @@ fail:
>  	return ret;
>  }
>  
> +static ssize_t mma8452_get_freefall_mode(struct device *dev,
> +					 struct device_attribute *attr,
> +					 char *buf)
> +{
> +	int val;
> +	struct mma8452_data *data = iio_priv(i2c_get_clientdata(
> +					     to_i2c_client(dev)));
> +	const struct mma_chip_info *chip = data->chip_info;
> +
> +	mutex_lock(&data->lock);
> +	val = i2c_smbus_read_byte_data(data->client, chip->ev_cfg);
> +	mutex_unlock(&data->lock);
> +	if (val < 0)
> +		return val;
> +
> +	return scnprintf(buf, PAGE_SIZE, "%d\n",
> +			 !(val & MMA8452_FF_MT_CFG_OAE));
> +}
> +
> +static ssize_t mma8452_set_freefall_mode(struct device *dev,
> +					 struct device_attribute *attr,
> +					 const char *buf,
> +					 size_t len)
> +{
> +	int val, ret;
> +	u8 user_val;
> +	struct mma8452_data *data = iio_priv(i2c_get_clientdata(
> +					     to_i2c_client(dev)));
> +	const struct mma_chip_info *chip = data->chip_info;
> +
> +	ret = kstrtou8(buf, 10, &user_val);
> +	if (ret)
> +		goto err;
> +
> +	val = i2c_smbus_read_byte_data(data->client, chip->ev_cfg);
> +	if (val < 0) {
> +		ret = val;
> +		goto err;
> +	}
> +
> +	if (user_val && (val & MMA8452_FF_MT_CFG_OAE)) {
> +		val |= BIT(axis_x + chip->ev_cfg_chan_shift);
> +		val |= BIT(axis_y + chip->ev_cfg_chan_shift);
> +		val |= BIT(axis_z + chip->ev_cfg_chan_shift);
> +		val &= ~MMA8452_FF_MT_CFG_OAE;
> +	} else if (!user_val && !(val & MMA8452_FF_MT_CFG_OAE)) {
> +		val &= ~BIT(axis_x + chip->ev_cfg_chan_shift);
> +		val &= ~BIT(axis_y + chip->ev_cfg_chan_shift);
> +		val &= ~BIT(axis_z + chip->ev_cfg_chan_shift);
> +		val |= MMA8452_FF_MT_CFG_OAE;
> +	}
It would be a pain to do perhaps and of limited interest, but
you could support all the combinations as well as separate
events... (clearly enabling one disables another, but that's fine
under the ABI)

(There's a reason we have x&y, x&z, etc :)  Maybe it's one
to leave for an intersted party to pick up in future if they
care.

> +
> +	ret = mma8452_change_config(data, chip->ev_cfg, val);
> +	if (ret)
> +		goto err;
> +
> +	return len;
> +err:
> +	return ret;
> +}
> +
> +static IIO_DEVICE_ATTR_NAMED(accel_xayaz_mag_falling_en,
> +			     in_accel_x&y&z_mag_falling_en,
> +			     S_IRUGO | S_IWUSR,
> +			     mma8452_get_freefall_mode,
> +			     mma8452_set_freefall_mode,
> +			     0);
> +
Lars is correct on this one. If it's not getting created something is
going wrong in an odd fashion!

>  static int mma8452_set_hp_filter_frequency(struct mma8452_data *data,
>  					   int val, int val2)
>  {
> @@ -631,7 +705,6 @@ static int mma8452_write_event_config(struct iio_dev *indio_dev,
>  		val &= ~BIT(chan->scan_index + chip->ev_cfg_chan_shift);
>  
>  	val |= chip->ev_cfg_ele;
> -	val |= MMA8452_FF_MT_CFG_OAE;
>  
>  	return mma8452_change_config(data, chip->ev_cfg, val);
>  }
> @@ -640,12 +713,26 @@ static void mma8452_transient_interrupt(struct iio_dev *indio_dev)
>  {
>  	struct mma8452_data *data = iio_priv(indio_dev);
>  	s64 ts = iio_get_time_ns();
> -	int src;
> +	int src, cfg;
>  
>  	src = i2c_smbus_read_byte_data(data->client, data->chip_info->ev_src);
>  	if (src < 0)
>  		return;
>  
> +	cfg = i2c_smbus_read_byte_data(data->client, data->chip_info->ev_cfg);
> +	if (cfg < 0)
> +		return;
> +
> +	if (!(cfg & MMA8452_FF_MT_CFG_OAE)) {
> +		iio_push_event(indio_dev,
> +			       IIO_MOD_EVENT_CODE(IIO_ACCEL, 0,
> +						  IIO_MOD_X_AND_Y_AND_Z,
> +						  IIO_EV_TYPE_MAG,
> +						  IIO_EV_DIR_FALLING),
> +			       ts);
> +		return;
> +	}
> +
>  	if (src & data->chip_info->ev_src_xe)
>  		iio_push_event(indio_dev,
>  			       IIO_MOD_EVENT_CODE(IIO_ACCEL, 0, IIO_MOD_X,
> @@ -758,6 +845,13 @@ static const struct iio_event_spec mma8452_motion_event[] = {
>  		.mask_shared_by_type = BIT(IIO_EV_INFO_VALUE) |
>  					BIT(IIO_EV_INFO_PERIOD)
>  	},
> +	{
> +		.type = IIO_EV_TYPE_MAG,
> +		.dir = IIO_EV_DIR_FALLING,
> +		.mask_separate = BIT(IIO_EV_INFO_ENABLE),
> +		.mask_shared_by_type = BIT(IIO_EV_INFO_VALUE) |
> +					BIT(IIO_EV_INFO_PERIOD)
> +	},
>  };
>  
>  /*
> @@ -768,6 +862,7 @@ static IIO_CONST_ATTR_NAMED(accel_transient_scale, in_accel_scale, "0.617742");
>  
>  static struct attribute *mma8452_event_attributes[] = {
>  	&iio_const_attr_accel_transient_scale.dev_attr.attr,
> +	&iio_dev_attr_accel_xayaz_mag_falling_en.dev_attr.attr,
>  	NULL,
>  };
>  
> @@ -1057,6 +1152,7 @@ static int mma8452_probe(struct i2c_client *client,
>  	struct mma8452_data *data;
>  	struct iio_dev *indio_dev;
>  	int ret;
> +	u8 val;
>  	const struct of_device_id *match;
>  
>  	match = of_match_device(mma8452_dt_ids, &client->dev);
> @@ -1158,6 +1254,18 @@ static int mma8452_probe(struct i2c_client *client,
>  			return ret;
>  	}
>  
> +	/* don't activate freefall mode on startup */
> +	ret = i2c_smbus_read_byte_data(data->client, data->chip_info->ev_cfg);
> +	if (ret < 0)
> +		return ret;
> +
> +	val = ret;
> +	ret = i2c_smbus_write_byte_data(client,
> +					data->chip_info->ev_cfg,
> +					val | MMA8452_FF_MT_CFG_OAE);
> +	if (ret < 0)
> +		return ret;
> +
>  	data->ctrl_reg1 = MMA8452_CTRL_ACTIVE |
>  			  (MMA8452_CTRL_DR_DEFAULT << MMA8452_CTRL_DR_SHIFT);
>  	ret = i2c_smbus_write_byte_data(client, MMA8452_CTRL_REG1,
> 

Re: [PATCH 1/3] iio: mma8452: add freefall detection for Freescale's accelerometers

[Martin Kepplinger]

Am 2015-11-14 um 19:03 schrieb Jonathan Cameron:
> On 11/11/15 18:38, Martin Kepplinger wrote:
>> This adds freefall event detection to the supported devices. It adds
>> the in_accel_x&y&z_mag_falling_en iio event attribute, which activates
>> freefall mode.
>>
>> In freefall mode, the current acceleration values of all activated axis
>> are added and if the *sum* falls *under* the threshold specified
>> (in_accel_mag_falling_value), the appropriate IIO event code
>> is generated.
>>
>> By enabling freefall mode (in_accel_x&y&z_mag_falling_en)
>> all 3 axis are enabled too as this describes a classic freefall
>> detection. Of course the user is free to disable one or more directions.
>>
>> The values of rising and falling versions of various sysfs files are
>> shared, which is compliant to the IIO specification.
>>
>> This is what the sysfs "events" directory for these devices looks
>> like after this change:
>>
>> -rw-r--r--    4096 Oct 23 08:45 in_accel_mag_falling_period
>> -rw-r--r--    4096 Oct 23 08:45 in_accel_mag_falling_value
>> -rw-r--r--    4096 Oct 23 08:45 in_accel_mag_rising_period
>> -rw-r--r--    4096 Oct 23 08:45 in_accel_mag_rising_value
>> -r--r--r--    4096 Oct 23 08:45 in_accel_scale
>> -rw-r--r--    4096 Oct 23 08:45 in_accel_x&y&z_mag_falling_en
>> -rw-r--r--    4096 Oct 23 08:45 in_accel_x_mag_falling_en
>> -rw-r--r--    4096 Oct 23 08:45 in_accel_x_mag_rising_en
>> -rw-r--r--    4096 Oct 23 08:45 in_accel_y_mag_falling_en
>> -rw-r--r--    4096 Oct 23 08:45 in_accel_y_mag_rising_en
>> -rw-r--r--    4096 Oct 23 08:45 in_accel_z_mag_falling_en
>> -rw-r--r--    4096 Oct 23 08:45 in_accel_z_mag_rising_en
>>
>> Signed-off-by: Martin Kepplinger <martin.kepplinger@theobroma-systems.com>
>> Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com>
> Looks pretty good to me (other than obviously the bits Lars already
> picked up on!)
> 
> My only real comment was that you could do the rest of the combined
> possibilities whilst you are here (if you want to!)  You've
> picked the mostly obviously useful one though so maybe leave it
> at that.

I'm not sure what you mean. There is only in_accel_x&y&z_mag_falling_en
documented. So my guess was to have this to enable freefall mode, taking
into account the currently enabled falling axis
(in_accel_x_mag_falling_en, ...).

Plus enabling all of them. The user has to check it anyways, but maybe I
should leave that out (which is a slightly different matter from what
you meant).

> 
> Jonathan
>> ---
>>  drivers/iio/accel/mma8452.c | 114 ++++++++++++++++++++++++++++++++++++++++++--
>>  1 file changed, 111 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
>> index 116a6e4..dedcc1d 100644
>> --- a/drivers/iio/accel/mma8452.c
>> +++ b/drivers/iio/accel/mma8452.c
>> @@ -15,7 +15,7 @@
>>   *
>>   * 7-bit I2C slave address 0x1c/0x1d (pin selectable)
>>   *
>> - * TODO: orientation / freefall events, autosleep
>> + * TODO: orientation events, autosleep
>>   */
>>  
>>  #include <linux/module.h>
>> @@ -144,6 +144,12 @@ struct mma_chip_info {
>>  	u8 ev_count;
>>  };
>>  
>> +enum {
>> +	axis_x,
>> +	axis_y,
>> +	axis_z,
>> +};
>> +
>>  static int mma8452_drdy(struct mma8452_data *data)
>>  {
>>  	int tries = 150;
>> @@ -410,6 +416,74 @@ fail:
>>  	return ret;
>>  }
>>  
>> +static ssize_t mma8452_get_freefall_mode(struct device *dev,
>> +					 struct device_attribute *attr,
>> +					 char *buf)
>> +{
>> +	int val;
>> +	struct mma8452_data *data = iio_priv(i2c_get_clientdata(
>> +					     to_i2c_client(dev)));
>> +	const struct mma_chip_info *chip = data->chip_info;
>> +
>> +	mutex_lock(&data->lock);
>> +	val = i2c_smbus_read_byte_data(data->client, chip->ev_cfg);
>> +	mutex_unlock(&data->lock);
>> +	if (val < 0)
>> +		return val;
>> +
>> +	return scnprintf(buf, PAGE_SIZE, "%d\n",
>> +			 !(val & MMA8452_FF_MT_CFG_OAE));
>> +}
>> +
>> +static ssize_t mma8452_set_freefall_mode(struct device *dev,
>> +					 struct device_attribute *attr,
>> +					 const char *buf,
>> +					 size_t len)
>> +{
>> +	int val, ret;
>> +	u8 user_val;
>> +	struct mma8452_data *data = iio_priv(i2c_get_clientdata(
>> +					     to_i2c_client(dev)));
>> +	const struct mma_chip_info *chip = data->chip_info;
>> +
>> +	ret = kstrtou8(buf, 10, &user_val);
>> +	if (ret)
>> +		goto err;
>> +
>> +	val = i2c_smbus_read_byte_data(data->client, chip->ev_cfg);
>> +	if (val < 0) {
>> +		ret = val;
>> +		goto err;
>> +	}
>> +
>> +	if (user_val && (val & MMA8452_FF_MT_CFG_OAE)) {
>> +		val |= BIT(axis_x + chip->ev_cfg_chan_shift);
>> +		val |= BIT(axis_y + chip->ev_cfg_chan_shift);
>> +		val |= BIT(axis_z + chip->ev_cfg_chan_shift);
>> +		val &= ~MMA8452_FF_MT_CFG_OAE;
>> +	} else if (!user_val && !(val & MMA8452_FF_MT_CFG_OAE)) {
>> +		val &= ~BIT(axis_x + chip->ev_cfg_chan_shift);
>> +		val &= ~BIT(axis_y + chip->ev_cfg_chan_shift);
>> +		val &= ~BIT(axis_z + chip->ev_cfg_chan_shift);
>> +		val |= MMA8452_FF_MT_CFG_OAE;
>> +	}
> It would be a pain to do perhaps and of limited interest, but
> you could support all the combinations as well as separate
> events... (clearly enabling one disables another, but that's fine
> under the ABI)
> 
> (There's a reason we have x&y, x&z, etc :)  Maybe it's one
> to leave for an intersted party to pick up in future if they
> care.
> 
>> +
>> +	ret = mma8452_change_config(data, chip->ev_cfg, val);
>> +	if (ret)
>> +		goto err;
>> +
>> +	return len;
>> +err:
>> +	return ret;
>> +}
>> +
>> +static IIO_DEVICE_ATTR_NAMED(accel_xayaz_mag_falling_en,
>> +			     in_accel_x&y&z_mag_falling_en,
>> +			     S_IRUGO | S_IWUSR,
>> +			     mma8452_get_freefall_mode,
>> +			     mma8452_set_freefall_mode,
>> +			     0);
>> +
> Lars is correct on this one. If it's not getting created something is
> going wrong in an odd fashion!
> 
>>  static int mma8452_set_hp_filter_frequency(struct mma8452_data *data,
>>  					   int val, int val2)
>>  {
>> @@ -631,7 +705,6 @@ static int mma8452_write_event_config(struct iio_dev *indio_dev,
>>  		val &= ~BIT(chan->scan_index + chip->ev_cfg_chan_shift);
>>  
>>  	val |= chip->ev_cfg_ele;
>> -	val |= MMA8452_FF_MT_CFG_OAE;
>>  
>>  	return mma8452_change_config(data, chip->ev_cfg, val);
>>  }
>> @@ -640,12 +713,26 @@ static void mma8452_transient_interrupt(struct iio_dev *indio_dev)
>>  {
>>  	struct mma8452_data *data = iio_priv(indio_dev);
>>  	s64 ts = iio_get_time_ns();
>> -	int src;
>> +	int src, cfg;
>>  
>>  	src = i2c_smbus_read_byte_data(data->client, data->chip_info->ev_src);
>>  	if (src < 0)
>>  		return;
>>  
>> +	cfg = i2c_smbus_read_byte_data(data->client, data->chip_info->ev_cfg);
>> +	if (cfg < 0)
>> +		return;
>> +
>> +	if (!(cfg & MMA8452_FF_MT_CFG_OAE)) {
>> +		iio_push_event(indio_dev,
>> +			       IIO_MOD_EVENT_CODE(IIO_ACCEL, 0,
>> +						  IIO_MOD_X_AND_Y_AND_Z,
>> +						  IIO_EV_TYPE_MAG,
>> +						  IIO_EV_DIR_FALLING),
>> +			       ts);
>> +		return;
>> +	}
>> +
>>  	if (src & data->chip_info->ev_src_xe)
>>  		iio_push_event(indio_dev,
>>  			       IIO_MOD_EVENT_CODE(IIO_ACCEL, 0, IIO_MOD_X,
>> @@ -758,6 +845,13 @@ static const struct iio_event_spec mma8452_motion_event[] = {
>>  		.mask_shared_by_type = BIT(IIO_EV_INFO_VALUE) |
>>  					BIT(IIO_EV_INFO_PERIOD)
>>  	},
>> +	{
>> +		.type = IIO_EV_TYPE_MAG,
>> +		.dir = IIO_EV_DIR_FALLING,
>> +		.mask_separate = BIT(IIO_EV_INFO_ENABLE),
>> +		.mask_shared_by_type = BIT(IIO_EV_INFO_VALUE) |
>> +					BIT(IIO_EV_INFO_PERIOD)
>> +	},
>>  };
>>  
>>  /*
>> @@ -768,6 +862,7 @@ static IIO_CONST_ATTR_NAMED(accel_transient_scale, in_accel_scale, "0.617742");
>>  
>>  static struct attribute *mma8452_event_attributes[] = {
>>  	&iio_const_attr_accel_transient_scale.dev_attr.attr,
>> +	&iio_dev_attr_accel_xayaz_mag_falling_en.dev_attr.attr,
>>  	NULL,
>>  };
>>  
>> @@ -1057,6 +1152,7 @@ static int mma8452_probe(struct i2c_client *client,
>>  	struct mma8452_data *data;
>>  	struct iio_dev *indio_dev;
>>  	int ret;
>> +	u8 val;
>>  	const struct of_device_id *match;
>>  
>>  	match = of_match_device(mma8452_dt_ids, &client->dev);
>> @@ -1158,6 +1254,18 @@ static int mma8452_probe(struct i2c_client *client,
>>  			return ret;
>>  	}
>>  
>> +	/* don't activate freefall mode on startup */
>> +	ret = i2c_smbus_read_byte_data(data->client, data->chip_info->ev_cfg);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	val = ret;
>> +	ret = i2c_smbus_write_byte_data(client,
>> +					data->chip_info->ev_cfg,
>> +					val | MMA8452_FF_MT_CFG_OAE);
>> +	if (ret < 0)
>> +		return ret;
>> +
>>  	data->ctrl_reg1 = MMA8452_CTRL_ACTIVE |
>>  			  (MMA8452_CTRL_DR_DEFAULT << MMA8452_CTRL_DR_SHIFT);
>>  	ret = i2c_smbus_write_byte_data(client, MMA8452_CTRL_REG1,
>>
> 

Re: [PATCH 1/3] iio: mma8452: add freefall detection for Freescale's accelerometers

[Jonathan Cameron]

On 14/11/15 18:45, Martin Kepplinger wrote:
> Am 2015-11-14 um 19:03 schrieb Jonathan Cameron:
>> On 11/11/15 18:38, Martin Kepplinger wrote:
>>> This adds freefall event detection to the supported devices. It adds
>>> the in_accel_x&y&z_mag_falling_en iio event attribute, which activates
>>> freefall mode.
>>>
>>> In freefall mode, the current acceleration values of all activated axis
>>> are added and if the *sum* falls *under* the threshold specified
>>> (in_accel_mag_falling_value), the appropriate IIO event code
>>> is generated.
>>>
>>> By enabling freefall mode (in_accel_x&y&z_mag_falling_en)
>>> all 3 axis are enabled too as this describes a classic freefall
>>> detection. Of course the user is free to disable one or more directions.
>>>
>>> The values of rising and falling versions of various sysfs files are
>>> shared, which is compliant to the IIO specification.
>>>
>>> This is what the sysfs "events" directory for these devices looks
>>> like after this change:
>>>
>>> -rw-r--r--    4096 Oct 23 08:45 in_accel_mag_falling_period
>>> -rw-r--r--    4096 Oct 23 08:45 in_accel_mag_falling_value
>>> -rw-r--r--    4096 Oct 23 08:45 in_accel_mag_rising_period
>>> -rw-r--r--    4096 Oct 23 08:45 in_accel_mag_rising_value
>>> -r--r--r--    4096 Oct 23 08:45 in_accel_scale
>>> -rw-r--r--    4096 Oct 23 08:45 in_accel_x&y&z_mag_falling_en
>>> -rw-r--r--    4096 Oct 23 08:45 in_accel_x_mag_falling_en
>>> -rw-r--r--    4096 Oct 23 08:45 in_accel_x_mag_rising_en
>>> -rw-r--r--    4096 Oct 23 08:45 in_accel_y_mag_falling_en
>>> -rw-r--r--    4096 Oct 23 08:45 in_accel_y_mag_rising_en
>>> -rw-r--r--    4096 Oct 23 08:45 in_accel_z_mag_falling_en
>>> -rw-r--r--    4096 Oct 23 08:45 in_accel_z_mag_rising_en
>>>
>>> Signed-off-by: Martin Kepplinger <martin.kepplinger@theobroma-systems.com>
>>> Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com>
>> Looks pretty good to me (other than obviously the bits Lars already
>> picked up on!)
>>
>> My only real comment was that you could do the rest of the combined
>> possibilities whilst you are here (if you want to!)  You've
>> picked the mostly obviously useful one though so maybe leave it
>> at that.
> 
> I'm not sure what you mean. There is only in_accel_x&y&z_mag_falling_en
> documented. So my guess was to have this to enable freefall mode, taking
> into account the currently enabled falling axis
> (in_accel_x_mag_falling_en, ...).
Sure, but the set of modifiers allows for
in_accel_x&y_mag_falling_* etc and they aren't documented in this particular
form simply because no driver has used them yet ;)

> 
> Plus enabling all of them. The user has to check it anyways, but maybe I
> should leave that out (which is a slightly different matter from what
> you meant).
> 
>>
>> Jonathan
>>> ---
>>>  drivers/iio/accel/mma8452.c | 114 ++++++++++++++++++++++++++++++++++++++++++--
>>>  1 file changed, 111 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
>>> index 116a6e4..dedcc1d 100644
>>> --- a/drivers/iio/accel/mma8452.c
>>> +++ b/drivers/iio/accel/mma8452.c
>>> @@ -15,7 +15,7 @@
>>>   *
>>>   * 7-bit I2C slave address 0x1c/0x1d (pin selectable)
>>>   *
>>> - * TODO: orientation / freefall events, autosleep
>>> + * TODO: orientation events, autosleep
>>>   */
>>>  
>>>  #include <linux/module.h>
>>> @@ -144,6 +144,12 @@ struct mma_chip_info {
>>>  	u8 ev_count;
>>>  };
>>>  
>>> +enum {
>>> +	axis_x,
>>> +	axis_y,
>>> +	axis_z,
>>> +};
>>> +
>>>  static int mma8452_drdy(struct mma8452_data *data)
>>>  {
>>>  	int tries = 150;
>>> @@ -410,6 +416,74 @@ fail:
>>>  	return ret;
>>>  }
>>>  
>>> +static ssize_t mma8452_get_freefall_mode(struct device *dev,
>>> +					 struct device_attribute *attr,
>>> +					 char *buf)
>>> +{
>>> +	int val;
>>> +	struct mma8452_data *data = iio_priv(i2c_get_clientdata(
>>> +					     to_i2c_client(dev)));
>>> +	const struct mma_chip_info *chip = data->chip_info;
>>> +
>>> +	mutex_lock(&data->lock);
>>> +	val = i2c_smbus_read_byte_data(data->client, chip->ev_cfg);
>>> +	mutex_unlock(&data->lock);
>>> +	if (val < 0)
>>> +		return val;
>>> +
>>> +	return scnprintf(buf, PAGE_SIZE, "%d\n",
>>> +			 !(val & MMA8452_FF_MT_CFG_OAE));
>>> +}
>>> +
>>> +static ssize_t mma8452_set_freefall_mode(struct device *dev,
>>> +					 struct device_attribute *attr,
>>> +					 const char *buf,
>>> +					 size_t len)
>>> +{
>>> +	int val, ret;
>>> +	u8 user_val;
>>> +	struct mma8452_data *data = iio_priv(i2c_get_clientdata(
>>> +					     to_i2c_client(dev)));
>>> +	const struct mma_chip_info *chip = data->chip_info;
>>> +
>>> +	ret = kstrtou8(buf, 10, &user_val);
>>> +	if (ret)
>>> +		goto err;
>>> +
>>> +	val = i2c_smbus_read_byte_data(data->client, chip->ev_cfg);
>>> +	if (val < 0) {
>>> +		ret = val;
>>> +		goto err;
>>> +	}
>>> +
>>> +	if (user_val && (val & MMA8452_FF_MT_CFG_OAE)) {
>>> +		val |= BIT(axis_x + chip->ev_cfg_chan_shift);
>>> +		val |= BIT(axis_y + chip->ev_cfg_chan_shift);
>>> +		val |= BIT(axis_z + chip->ev_cfg_chan_shift);
>>> +		val &= ~MMA8452_FF_MT_CFG_OAE;
>>> +	} else if (!user_val && !(val & MMA8452_FF_MT_CFG_OAE)) {
>>> +		val &= ~BIT(axis_x + chip->ev_cfg_chan_shift);
>>> +		val &= ~BIT(axis_y + chip->ev_cfg_chan_shift);
>>> +		val &= ~BIT(axis_z + chip->ev_cfg_chan_shift);
>>> +		val |= MMA8452_FF_MT_CFG_OAE;
>>> +	}
>> It would be a pain to do perhaps and of limited interest, but
>> you could support all the combinations as well as separate
>> events... (clearly enabling one disables another, but that's fine
>> under the ABI)
>>
>> (There's a reason we have x&y, x&z, etc :)  Maybe it's one
>> to leave for an intersted party to pick up in future if they
>> care.
>>
>>> +
>>> +	ret = mma8452_change_config(data, chip->ev_cfg, val);
>>> +	if (ret)
>>> +		goto err;
>>> +
>>> +	return len;
>>> +err:
>>> +	return ret;
>>> +}
>>> +
>>> +static IIO_DEVICE_ATTR_NAMED(accel_xayaz_mag_falling_en,
>>> +			     in_accel_x&y&z_mag_falling_en,
>>> +			     S_IRUGO | S_IWUSR,
>>> +			     mma8452_get_freefall_mode,
>>> +			     mma8452_set_freefall_mode,
>>> +			     0);
>>> +
>> Lars is correct on this one. If it's not getting created something is
>> going wrong in an odd fashion!
>>
>>>  static int mma8452_set_hp_filter_frequency(struct mma8452_data *data,
>>>  					   int val, int val2)
>>>  {
>>> @@ -631,7 +705,6 @@ static int mma8452_write_event_config(struct iio_dev *indio_dev,
>>>  		val &= ~BIT(chan->scan_index + chip->ev_cfg_chan_shift);
>>>  
>>>  	val |= chip->ev_cfg_ele;
>>> -	val |= MMA8452_FF_MT_CFG_OAE;
>>>  
>>>  	return mma8452_change_config(data, chip->ev_cfg, val);
>>>  }
>>> @@ -640,12 +713,26 @@ static void mma8452_transient_interrupt(struct iio_dev *indio_dev)
>>>  {
>>>  	struct mma8452_data *data = iio_priv(indio_dev);
>>>  	s64 ts = iio_get_time_ns();
>>> -	int src;
>>> +	int src, cfg;
>>>  
>>>  	src = i2c_smbus_read_byte_data(data->client, data->chip_info->ev_src);
>>>  	if (src < 0)
>>>  		return;
>>>  
>>> +	cfg = i2c_smbus_read_byte_data(data->client, data->chip_info->ev_cfg);
>>> +	if (cfg < 0)
>>> +		return;
>>> +
>>> +	if (!(cfg & MMA8452_FF_MT_CFG_OAE)) {
>>> +		iio_push_event(indio_dev,
>>> +			       IIO_MOD_EVENT_CODE(IIO_ACCEL, 0,
>>> +						  IIO_MOD_X_AND_Y_AND_Z,
>>> +						  IIO_EV_TYPE_MAG,
>>> +						  IIO_EV_DIR_FALLING),
>>> +			       ts);
>>> +		return;
>>> +	}
>>> +
>>>  	if (src & data->chip_info->ev_src_xe)
>>>  		iio_push_event(indio_dev,
>>>  			       IIO_MOD_EVENT_CODE(IIO_ACCEL, 0, IIO_MOD_X,
>>> @@ -758,6 +845,13 @@ static const struct iio_event_spec mma8452_motion_event[] = {
>>>  		.mask_shared_by_type = BIT(IIO_EV_INFO_VALUE) |
>>>  					BIT(IIO_EV_INFO_PERIOD)
>>>  	},
>>> +	{
>>> +		.type = IIO_EV_TYPE_MAG,
>>> +		.dir = IIO_EV_DIR_FALLING,
>>> +		.mask_separate = BIT(IIO_EV_INFO_ENABLE),
>>> +		.mask_shared_by_type = BIT(IIO_EV_INFO_VALUE) |
>>> +					BIT(IIO_EV_INFO_PERIOD)
>>> +	},
>>>  };
>>>  
>>>  /*
>>> @@ -768,6 +862,7 @@ static IIO_CONST_ATTR_NAMED(accel_transient_scale, in_accel_scale, "0.617742");
>>>  
>>>  static struct attribute *mma8452_event_attributes[] = {
>>>  	&iio_const_attr_accel_transient_scale.dev_attr.attr,
>>> +	&iio_dev_attr_accel_xayaz_mag_falling_en.dev_attr.attr,
>>>  	NULL,
>>>  };
>>>  
>>> @@ -1057,6 +1152,7 @@ static int mma8452_probe(struct i2c_client *client,
>>>  	struct mma8452_data *data;
>>>  	struct iio_dev *indio_dev;
>>>  	int ret;
>>> +	u8 val;
>>>  	const struct of_device_id *match;
>>>  
>>>  	match = of_match_device(mma8452_dt_ids, &client->dev);
>>> @@ -1158,6 +1254,18 @@ static int mma8452_probe(struct i2c_client *client,
>>>  			return ret;
>>>  	}
>>>  
>>> +	/* don't activate freefall mode on startup */
>>> +	ret = i2c_smbus_read_byte_data(data->client, data->chip_info->ev_cfg);
>>> +	if (ret < 0)
>>> +		return ret;
>>> +
>>> +	val = ret;
>>> +	ret = i2c_smbus_write_byte_data(client,
>>> +					data->chip_info->ev_cfg,
>>> +					val | MMA8452_FF_MT_CFG_OAE);
>>> +	if (ret < 0)
>>> +		return ret;
>>> +
>>>  	data->ctrl_reg1 = MMA8452_CTRL_ACTIVE |
>>>  			  (MMA8452_CTRL_DR_DEFAULT << MMA8452_CTRL_DR_SHIFT);
>>>  	ret = i2c_smbus_write_byte_data(client, MMA8452_CTRL_REG1,
>>>
>>
> 
> --
> 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
> 

Re: [PATCH 1/3] iio: mma8452: add freefall detection for Freescale's accelerometers

[Martin Kepplinger]

Am 2015-11-14 um 20:03 schrieb Jonathan Cameron:
> On 14/11/15 18:45, Martin Kepplinger wrote:
>> Am 2015-11-14 um 19:03 schrieb Jonathan Cameron:
>>> On 11/11/15 18:38, Martin Kepplinger wrote:
>>>> This adds freefall event detection to the supported devices. It adds
>>>> the in_accel_x&y&z_mag_falling_en iio event attribute, which activates
>>>> freefall mode.
>>>>
>>>> In freefall mode, the current acceleration values of all activated axis
>>>> are added and if the *sum* falls *under* the threshold specified
>>>> (in_accel_mag_falling_value), the appropriate IIO event code
>>>> is generated.
>>>>
>>>> By enabling freefall mode (in_accel_x&y&z_mag_falling_en)
>>>> all 3 axis are enabled too as this describes a classic freefall
>>>> detection. Of course the user is free to disable one or more directions.
>>>>
>>>> The values of rising and falling versions of various sysfs files are
>>>> shared, which is compliant to the IIO specification.
>>>>
>>>> This is what the sysfs "events" directory for these devices looks
>>>> like after this change:
>>>>
>>>> -rw-r--r--    4096 Oct 23 08:45 in_accel_mag_falling_period
>>>> -rw-r--r--    4096 Oct 23 08:45 in_accel_mag_falling_value
>>>> -rw-r--r--    4096 Oct 23 08:45 in_accel_mag_rising_period
>>>> -rw-r--r--    4096 Oct 23 08:45 in_accel_mag_rising_value
>>>> -r--r--r--    4096 Oct 23 08:45 in_accel_scale
>>>> -rw-r--r--    4096 Oct 23 08:45 in_accel_x&y&z_mag_falling_en
>>>> -rw-r--r--    4096 Oct 23 08:45 in_accel_x_mag_falling_en
>>>> -rw-r--r--    4096 Oct 23 08:45 in_accel_x_mag_rising_en
>>>> -rw-r--r--    4096 Oct 23 08:45 in_accel_y_mag_falling_en
>>>> -rw-r--r--    4096 Oct 23 08:45 in_accel_y_mag_rising_en
>>>> -rw-r--r--    4096 Oct 23 08:45 in_accel_z_mag_falling_en
>>>> -rw-r--r--    4096 Oct 23 08:45 in_accel_z_mag_rising_en
>>>>
>>>> Signed-off-by: Martin Kepplinger <martin.kepplinger@theobroma-systems.com>
>>>> Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com>
>>> Looks pretty good to me (other than obviously the bits Lars already
>>> picked up on!)
>>>
>>> My only real comment was that you could do the rest of the combined
>>> possibilities whilst you are here (if you want to!)  You've
>>> picked the mostly obviously useful one though so maybe leave it
>>> at that.
>>
>> I'm not sure what you mean. There is only in_accel_x&y&z_mag_falling_en
>> documented. So my guess was to have this to enable freefall mode, taking
>> into account the currently enabled falling axis
>> (in_accel_x_mag_falling_en, ...).
> Sure, but the set of modifiers allows for
> in_accel_x&y_mag_falling_* etc and they aren't documented in this particular
> form simply because no driver has used them yet ;)
> 



I don't see how to do it in iio_event_spec only. Could you point me in
the right direction, could well be I overlook something...

I *can* add a channel for freefall purposes, something like:

static const struct iio_chan_spec mma8653_channels[] = {
        MMA8652_CHANNEL(IIO_MOD_X, idx_axis_x, 10),
        MMA8652_CHANNEL(IIO_MOD_Y, idx_axis_y, 10),
        MMA8652_CHANNEL(IIO_MOD_Z, idx_axis_z, 10),
        MMA8652_CHANNEL(IIO_MOD_X_AND_Y_AND_Z, idx_axis_xyz, 10),
        IIO_CHAN_SOFT_TIMESTAMP(idx_timestamp),
};


just as an example, but then of course I get more than just events: In
the device-directory:

in_accel_x&y&z_calibbias
in_accel_x&y&z_raw
in_accel_x_calibbias
in_accel_x_raw
in_accel_y_calibbias
in_accel_y_raw
in_accel_z_calibbias
in_accel_z_raw


calibbias and raw are not (really) available for x&y&z. I could
calculate a "raw" value, but calibbias doesn't make sense here. ...But
again, maybe I'm on the wrong path!!

thanks a lot!

                              martin

Re: [PATCH 1/3] iio: mma8452: add freefall detection for Freescale's accelerometers

[Martin Kepplinger]

Am 2015-11-17 um 20:24 schrieb Martin Kepplinger:
> Am 2015-11-14 um 20:03 schrieb Jonathan Cameron:
>> On 14/11/15 18:45, Martin Kepplinger wrote:
>>> Am 2015-11-14 um 19:03 schrieb Jonathan Cameron:
>>>> On 11/11/15 18:38, Martin Kepplinger wrote:
>>>>> This adds freefall event detection to the supported devices. It adds
>>>>> the in_accel_x&y&z_mag_falling_en iio event attribute, which activates
>>>>> freefall mode.
>>>>>
>>>>> In freefall mode, the current acceleration values of all activated axis
>>>>> are added and if the *sum* falls *under* the threshold specified
>>>>> (in_accel_mag_falling_value), the appropriate IIO event code
>>>>> is generated.
>>>>>
>>>>> By enabling freefall mode (in_accel_x&y&z_mag_falling_en)
>>>>> all 3 axis are enabled too as this describes a classic freefall
>>>>> detection. Of course the user is free to disable one or more directions.
>>>>>
>>>>> The values of rising and falling versions of various sysfs files are
>>>>> shared, which is compliant to the IIO specification.
>>>>>
>>>>> This is what the sysfs "events" directory for these devices looks
>>>>> like after this change:
>>>>>
>>>>> -rw-r--r--    4096 Oct 23 08:45 in_accel_mag_falling_period
>>>>> -rw-r--r--    4096 Oct 23 08:45 in_accel_mag_falling_value
>>>>> -rw-r--r--    4096 Oct 23 08:45 in_accel_mag_rising_period
>>>>> -rw-r--r--    4096 Oct 23 08:45 in_accel_mag_rising_value
>>>>> -r--r--r--    4096 Oct 23 08:45 in_accel_scale
>>>>> -rw-r--r--    4096 Oct 23 08:45 in_accel_x&y&z_mag_falling_en
>>>>> -rw-r--r--    4096 Oct 23 08:45 in_accel_x_mag_falling_en
>>>>> -rw-r--r--    4096 Oct 23 08:45 in_accel_x_mag_rising_en
>>>>> -rw-r--r--    4096 Oct 23 08:45 in_accel_y_mag_falling_en
>>>>> -rw-r--r--    4096 Oct 23 08:45 in_accel_y_mag_rising_en
>>>>> -rw-r--r--    4096 Oct 23 08:45 in_accel_z_mag_falling_en
>>>>> -rw-r--r--    4096 Oct 23 08:45 in_accel_z_mag_rising_en
>>>>>
>>>>> Signed-off-by: Martin Kepplinger <martin.kepplinger@theobroma-systems.com>
>>>>> Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com>
>>>> Looks pretty good to me (other than obviously the bits Lars already
>>>> picked up on!)
>>>>
>>>> My only real comment was that you could do the rest of the combined
>>>> possibilities whilst you are here (if you want to!)  You've
>>>> picked the mostly obviously useful one though so maybe leave it
>>>> at that.
>>>
>>> I'm not sure what you mean. There is only in_accel_x&y&z_mag_falling_en
>>> documented. So my guess was to have this to enable freefall mode, taking
>>> into account the currently enabled falling axis
>>> (in_accel_x_mag_falling_en, ...).
>> Sure, but the set of modifiers allows for
>> in_accel_x&y_mag_falling_* etc and they aren't documented in this particular
>> form simply because no driver has used them yet ;)
>>
> 
> 
> 
> I don't see how to do it in iio_event_spec only. Could you point me in
> the right direction, could well be I overlook something...
> 
> I *can* add a channel for freefall purposes, something like:
> 
> static const struct iio_chan_spec mma8653_channels[] = {
>         MMA8652_CHANNEL(IIO_MOD_X, idx_axis_x, 10),
>         MMA8652_CHANNEL(IIO_MOD_Y, idx_axis_y, 10),
>         MMA8652_CHANNEL(IIO_MOD_Z, idx_axis_z, 10),
>         MMA8652_CHANNEL(IIO_MOD_X_AND_Y_AND_Z, idx_axis_xyz, 10),
>         IIO_CHAN_SOFT_TIMESTAMP(idx_timestamp),
> };
> 
> 
> just as an example, but then of course I get more than just events: In
> the device-directory:
> 
> in_accel_x&y&z_calibbias
> in_accel_x&y&z_raw
> in_accel_x_calibbias
> in_accel_x_raw
> in_accel_y_calibbias
> in_accel_y_raw
> in_accel_z_calibbias
> in_accel_z_raw
> 
> 
> calibbias and raw are not (really) available for x&y&z. I could
> calculate a "raw" value, but calibbias doesn't make sense here. ...But
> again, maybe I'm on the wrong path!!
> 
> thanks a lot!
> 
>                               martin
> 

Would this be the way to go? And just not support the generated file
that don't apply?

thanks,

                             martin

Re: [PATCH 1/3] iio: mma8452: add freefall detection for Freescale's accelerometers

[Lars-Peter Clausen]

On 11/24/2015 06:20 AM, Martin Kepplinger wrote:
> Am 2015-11-17 um 20:24 schrieb Martin Kepplinger:
>> Am 2015-11-14 um 20:03 schrieb Jonathan Cameron:
>>> On 14/11/15 18:45, Martin Kepplinger wrote:
>>>> Am 2015-11-14 um 19:03 schrieb Jonathan Cameron:
>>>>> On 11/11/15 18:38, Martin Kepplinger wrote:
>>>>>> This adds freefall event detection to the supported devices. It adds
>>>>>> the in_accel_x&y&z_mag_falling_en iio event attribute, which activates
>>>>>> freefall mode.
>>>>>>
>>>>>> In freefall mode, the current acceleration values of all activated axis
>>>>>> are added and if the *sum* falls *under* the threshold specified
>>>>>> (in_accel_mag_falling_value), the appropriate IIO event code
>>>>>> is generated.
>>>>>>
>>>>>> By enabling freefall mode (in_accel_x&y&z_mag_falling_en)
>>>>>> all 3 axis are enabled too as this describes a classic freefall
>>>>>> detection. Of course the user is free to disable one or more directions.
>>>>>>
>>>>>> The values of rising and falling versions of various sysfs files are
>>>>>> shared, which is compliant to the IIO specification.
>>>>>>
>>>>>> This is what the sysfs "events" directory for these devices looks
>>>>>> like after this change:
>>>>>>
>>>>>> -rw-r--r--    4096 Oct 23 08:45 in_accel_mag_falling_period
>>>>>> -rw-r--r--    4096 Oct 23 08:45 in_accel_mag_falling_value
>>>>>> -rw-r--r--    4096 Oct 23 08:45 in_accel_mag_rising_period
>>>>>> -rw-r--r--    4096 Oct 23 08:45 in_accel_mag_rising_value
>>>>>> -r--r--r--    4096 Oct 23 08:45 in_accel_scale
>>>>>> -rw-r--r--    4096 Oct 23 08:45 in_accel_x&y&z_mag_falling_en
>>>>>> -rw-r--r--    4096 Oct 23 08:45 in_accel_x_mag_falling_en
>>>>>> -rw-r--r--    4096 Oct 23 08:45 in_accel_x_mag_rising_en
>>>>>> -rw-r--r--    4096 Oct 23 08:45 in_accel_y_mag_falling_en
>>>>>> -rw-r--r--    4096 Oct 23 08:45 in_accel_y_mag_rising_en
>>>>>> -rw-r--r--    4096 Oct 23 08:45 in_accel_z_mag_falling_en
>>>>>> -rw-r--r--    4096 Oct 23 08:45 in_accel_z_mag_rising_en
>>>>>>
>>>>>> Signed-off-by: Martin Kepplinger <martin.kepplinger@theobroma-systems.com>
>>>>>> Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com>
>>>>> Looks pretty good to me (other than obviously the bits Lars already
>>>>> picked up on!)
>>>>>
>>>>> My only real comment was that you could do the rest of the combined
>>>>> possibilities whilst you are here (if you want to!)  You've
>>>>> picked the mostly obviously useful one though so maybe leave it
>>>>> at that.
>>>>
>>>> I'm not sure what you mean. There is only in_accel_x&y&z_mag_falling_en
>>>> documented. So my guess was to have this to enable freefall mode, taking
>>>> into account the currently enabled falling axis
>>>> (in_accel_x_mag_falling_en, ...).
>>> Sure, but the set of modifiers allows for
>>> in_accel_x&y_mag_falling_* etc and they aren't documented in this particular
>>> form simply because no driver has used them yet ;)
>>>
>>
>>
>>
>> I don't see how to do it in iio_event_spec only. Could you point me in
>> the right direction, could well be I overlook something...
>>
>> I *can* add a channel for freefall purposes, something like:
>>
>> static const struct iio_chan_spec mma8653_channels[] = {
>>         MMA8652_CHANNEL(IIO_MOD_X, idx_axis_x, 10),
>>         MMA8652_CHANNEL(IIO_MOD_Y, idx_axis_y, 10),
>>         MMA8652_CHANNEL(IIO_MOD_Z, idx_axis_z, 10),
>>         MMA8652_CHANNEL(IIO_MOD_X_AND_Y_AND_Z, idx_axis_xyz, 10),
>>         IIO_CHAN_SOFT_TIMESTAMP(idx_timestamp),
>> };
>>
>>
>> just as an example, but then of course I get more than just events: In
>> the device-directory:
>>
>> in_accel_x&y&z_calibbias
>> in_accel_x&y&z_raw
>> in_accel_x_calibbias
>> in_accel_x_raw
>> in_accel_y_calibbias
>> in_accel_y_raw
>> in_accel_z_calibbias
>> in_accel_z_raw
>>
>>
>> calibbias and raw are not (really) available for x&y&z. I could
>> calculate a "raw" value, but calibbias doesn't make sense here. ...But
>> again, maybe I'm on the wrong path!!
>>
>> thanks a lot!
>>
>>                               martin
>>
> 
> Would this be the way to go? And just not support the generated file
> that don't apply?

The calibbias and raw properties are added because the MMA8652_CHANNEL()
macro defines them. If you don't use that macro and don't set any of the
info masks for your x&y&z channel, but instead set the the event spec you
get a channel that only has events.

- Lars