Skip to contentSkip to conversation

iio: mma8452 enhancements

PATCH Superseded Martin Fuzzey 3 participants 18 messages Jul 23, 2014 – Jul 28, 2014 IIO

v1 of 5 Superseded Latest is v5 →

lore
Share & tools
lore

Martin Fuzzey

author
#0 lore raw

cover iio: mma8452 enhancements

This series adds some additional features to the mma8452 accelerometer driver
* debugfs register access
* transient threshold events
* highpass filter

The highpass filter attributes are added to the core.

In addition a latent bug in the device initialisation is fixed.

author
#1 lore raw

PATCH 1/8 iio: mma8452: Initialise before activating

Many of the hardware configuration registers may only be modified while the device is inactive.

Currently the probe code first activates the device and then modifies the registers (eg to set the scale). This doesn't actually work but is not noticed since the scale used is the default value.

While at it also issue a hardware reset command at probe time.

Signed-off-by Martin Fuzzey <[email protected]>
---
 drivers/iio/accel/mma8452.c |   39 ++++++++++++++++++++++++++++++++++-----
 1 file changed, 34 insertions(+), 5 deletions(-)
drivers/iio/accel/mma8452.c +34 -5
index 3c12d49..04a4f34 100644
@@ -32,6 +32,7 @@
 #define MMA8452_OFF_Z 0x31
 #define MMA8452_CTRL_REG1 0x2a
 #define MMA8452_CTRL_REG2 0x2b
+#define MMA8452_CTRL_REG2_RST		BIT(6)
 
 #define MMA8452_STATUS_DRDY (BIT(2) | BIT(1) | BIT(0))
 

@@ -111,7 +112,7 @@ static const int mma8452_samp_freq[8][2] = {
 	{6, 250000}, {1, 560000}
 };
 
-/* 
+/*
  * Hardware has fullscale of -2G, -4G, -8G corresponding to raw value -2048
  * The userspace interface uses m/s^2 and we declare micro units
  * So scale factor is given by:

@@ -335,6 +336,30 @@ static const struct iio_info mma8452_info = {
 
 static const unsigned long mma8452_scan_masks[] = {0x7, 0};
 
+static int mma8452_reset(struct i2c_client *client)
+{
+	int i;
+	int ret;
+
+	ret = i2c_smbus_write_byte_data(client,	MMA8452_CTRL_REG2,
+						MMA8452_CTRL_REG2_RST);
+	if (ret)
+		return ret;
+
+	for (i = 0; i < 10; i++) {
+		udelay(100);
+		ret = i2c_smbus_read_byte_data(client, MMA8452_CTRL_REG2);
+		if (ret == -EIO)
+			continue; /* I2C comm reset */
+		if (ret < 0)
+			return ret;
+		if (!(ret & MMA8452_CTRL_REG2_RST))
+			return 0;
+	}
+
+	return -ETIMEDOUT;
+}
+
 static int mma8452_probe(struct i2c_client *client,
 			 const struct i2c_device_id *id)
 {

@@ -365,10 +390,7 @@ static int mma8452_probe(struct i2c_client *client,
 	indio_dev->num_channels = ARRAY_SIZE(mma8452_channels);
 	indio_dev->available_scan_masks = mma8452_scan_masks;
 
-	data->ctrl_reg1 = MMA8452_CTRL_ACTIVE |
-		(MMA8452_CTRL_DR_DEFAULT << MMA8452_CTRL_DR_SHIFT);
-	ret = i2c_smbus_write_byte_data(client, MMA8452_CTRL_REG1,
-		data->ctrl_reg1);
+	ret = mma8452_reset(client);
 	if (ret < 0)
 		return ret;
 

@@ -378,6 +400,13 @@ static int mma8452_probe(struct i2c_client *client,
 	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,
+		data->ctrl_reg1);
+	if (ret < 0)
+		return ret;
+
 	ret = iio_triggered_buffer_setup(indio_dev, NULL,
 		mma8452_trigger_handler, NULL);
 	if (ret < 0)
author
#4 lore raw

PATCH 2/8 iio: mma8452: Add access to registers via DebugFS

In reply to Martin Fuzzey
Signed-off-by Martin Fuzzey <[email protected]>
---
 drivers/iio/accel/mma8452.c |   25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)
drivers/iio/accel/mma8452.c +25 -0
index 04a4f34..c46df4e 100644
@@ -34,6 +34,8 @@
 #define MMA8452_CTRL_REG2 0x2b
 #define MMA8452_CTRL_REG2_RST		BIT(6)
 
+#define MMA8452_MAX_REG 0x31
+
 #define MMA8452_STATUS_DRDY (BIT(2) | BIT(1) | BIT(0))
 
 #define MMA8452_CTRL_DR_MASK (BIT(5) | BIT(4) | BIT(3))

@@ -292,6 +294,28 @@ done:
 	return IRQ_HANDLED;
 }
 
+static int mma8452_reg_access_dbg(struct iio_dev *indio_dev,
+			      unsigned reg, unsigned writeval,
+			      unsigned *readval)
+{
+	int ret;
+	struct mma8452_data *data = iio_priv(indio_dev);
+
+	if (reg > MMA8452_MAX_REG)
+		return -EINVAL;
+
+	if (readval == NULL) {
+		ret = mma8452_change_config(data, reg, writeval);
+	} else {
+		ret = i2c_smbus_read_byte_data(data->client, reg);
+		if (ret < 0)
+			return ret;
+		*readval = ret;
+		ret = 0;
+	}
+	return ret;
+}
+
 #define MMA8452_CHANNEL(axis, idx) { \
 	.type = IIO_ACCEL, \
 	.modified = 1, \

@@ -331,6 +355,7 @@ static const struct iio_info mma8452_info = {
 	.attrs = &mma8452_group,
 	.read_raw = &mma8452_read_raw,
 	.write_raw = &mma8452_write_raw,
+	.debugfs_reg_access = &mma8452_reg_access_dbg,
 	.driver_module = THIS_MODULE,
 };
author
#5 lore raw

PATCH 3/8 iio: mma8452: Basic support for transient events.

In reply to Martin Fuzzey

The event is triggered when the highpass filtered absolute acceleration exceeds the threshold.

Signed-off-by Martin Fuzzey <[email protected]>
---
 drivers/iio/accel/mma8452.c |  203 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 202 insertions(+), 1 deletion(-)
drivers/iio/accel/mma8452.c +202 -1
index c46df4e..8129c0b 100644
@@ -9,7 +9,7 @@
  *
  * 7-bit I2C slave address 0x1c/0x1d (pin selectable)
  *
- * TODO: interrupt, thresholding, orientation / freefall events, autosleep
+ * TODO: orientation / freefall events, autosleep
  */
 
 #include <linux/module.h>

@@ -19,20 +19,33 @@
 #include <linux/iio/trigger_consumer.h>
 #include <linux/iio/buffer.h>
 #include <linux/iio/triggered_buffer.h>
+#include <linux/iio/events.h>
 #include <linux/delay.h>
 
 #define MMA8452_STATUS 0x00
 #define MMA8452_OUT_X 0x01 /* MSB first, 12-bit  */
 #define MMA8452_OUT_Y 0x03
 #define MMA8452_OUT_Z 0x05
+#define MMA8452_INT_SRC 0x0c
 #define MMA8452_WHO_AM_I 0x0d
 #define MMA8452_DATA_CFG 0x0e
+#define MMA8452_TRANSIENT_CFG 0x1d
+#define MMA8452_TRANSIENT_CFG_ELE		BIT(4)
+#define MMA8452_TRANSIENT_CFG_CHAN_MASK(chan)	(BIT(1) << chan)
+#define MMA8452_TRANSIENT_SRC 0x1e
+#define MMA8452_TRANSIENT_SRC_XTRANSE		BIT(1)
+#define MMA8452_TRANSIENT_SRC_YTRANSE		BIT(3)
+#define MMA8452_TRANSIENT_SRC_ZTRANSE		BIT(5)
+#define MMA8452_TRANSIENT_THS 0x1f
+#define MMA8452_TRANSIENT_COUNT 0x20
 #define MMA8452_OFF_X 0x2f
 #define MMA8452_OFF_Y 0x30
 #define MMA8452_OFF_Z 0x31
 #define MMA8452_CTRL_REG1 0x2a
 #define MMA8452_CTRL_REG2 0x2b
 #define MMA8452_CTRL_REG2_RST		BIT(6)
+#define MMA8452_CTRL_REG4 0x2d
+#define MMA8452_CTRL_REG5 0x2e
 
 #define MMA8452_MAX_REG 0x31
 

@@ -48,6 +61,13 @@
 #define MMA8452_DATA_CFG_FS_4G 1
 #define MMA8452_DATA_CFG_FS_8G 2
 
+#define MMA8452_INT_DRDY	BIT(0)
+#define MMA8452_INT_FF_MT	BIT(2)
+#define MMA8452_INT_PULSE	BIT(3)
+#define MMA8452_INT_LNDPRT	BIT(4)
+#define MMA8452_INT_TRANS	BIT(5)
+#define MMA8452_INT_ASLP	BIT(7)
+
 #define MMA8452_DEVICE_ID 0x2a
 
 struct mma8452_data {

@@ -274,6 +294,119 @@ static int mma8452_write_raw(struct iio_dev *indio_dev,
 	}
 }
 
+static int mma8452_read_thresh(struct iio_dev *indio_dev,
+	const struct iio_chan_spec *chan, enum iio_event_type type,
+	enum iio_event_direction dir, enum iio_event_info info, int *val,
+	int *val2)
+{
+	struct mma8452_data *data = iio_priv(indio_dev);
+	int ret;
+
+	ret = i2c_smbus_read_byte_data(data->client, MMA8452_TRANSIENT_THS);
+	if (ret < 0)
+		return ret;
+
+	*val = ret & 0x7f;
+
+	return IIO_VAL_INT;
+}
+
+static int mma8452_write_thresh(struct iio_dev *indio_dev,
+	const struct iio_chan_spec *chan, enum iio_event_type type,
+	enum iio_event_direction dir, enum iio_event_info info, int val,
+	int val2)
+{
+	struct mma8452_data *data = iio_priv(indio_dev);
+
+	return mma8452_change_config(data, MMA8452_TRANSIENT_THS, val & 0x7f);
+}
+
+static int mma8452_read_event_config(struct iio_dev *indio_dev,
+	const struct iio_chan_spec *chan, enum iio_event_type type,
+	enum iio_event_direction dir)
+{
+	struct mma8452_data *data = iio_priv(indio_dev);
+	int ret;
+
+	ret = i2c_smbus_read_byte_data(data->client, MMA8452_TRANSIENT_CFG);
+	if (ret < 0)
+		return ret;
+
+	return ret & MMA8452_TRANSIENT_CFG_CHAN_MASK(chan->scan_index) ? 1 : 0;
+}
+
+static int mma8452_write_event_config(struct iio_dev *indio_dev,
+	const struct iio_chan_spec *chan, enum iio_event_type type,
+	enum iio_event_direction dir, int state)
+{
+	struct mma8452_data *data = iio_priv(indio_dev);
+	int val;
+
+	val = i2c_smbus_read_byte_data(data->client, MMA8452_TRANSIENT_CFG);
+	if (val < 0)
+		return val;
+
+	if (state)
+		val |= MMA8452_TRANSIENT_CFG_CHAN_MASK(chan->scan_index);
+	else
+		val &= ~MMA8452_TRANSIENT_CFG_CHAN_MASK(chan->scan_index);
+
+	val |= MMA8452_TRANSIENT_CFG_ELE;
+
+	return mma8452_change_config(data, MMA8452_TRANSIENT_CFG, val);
+}
+
+static void mma8452_push_transient_event(struct iio_dev *indio_dev,
+						int chan, u64 ts)
+{
+	iio_push_event(indio_dev,
+		IIO_UNMOD_EVENT_CODE(IIO_ACCEL,
+			chan,
+			IIO_EV_TYPE_THRESH,
+			IIO_EV_DIR_RISING),
+		ts);
+}
+
+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;
+
+	src = i2c_smbus_read_byte_data(data->client, MMA8452_TRANSIENT_SRC);
+	if (src < 0)
+		return;
+
+	if (src & MMA8452_TRANSIENT_SRC_XTRANSE)
+		mma8452_push_transient_event(indio_dev, 0, ts);
+
+	if (src & MMA8452_TRANSIENT_SRC_YTRANSE)
+		mma8452_push_transient_event(indio_dev, 1, ts);
+
+	if (src & MMA8452_TRANSIENT_SRC_ZTRANSE)
+		mma8452_push_transient_event(indio_dev, 2, ts);
+}
+
+static irqreturn_t mma8452_interrupt(int irq, void *p)
+{
+	struct iio_dev *indio_dev = p;
+	struct mma8452_data *data = iio_priv(indio_dev);
+	int ret = IRQ_NONE;
+	int src;
+
+	src = i2c_smbus_read_byte_data(data->client, MMA8452_INT_SRC);
+	if (src < 0)
+		goto out;
+
+	if (src & MMA8452_INT_TRANS) {
+		mma8452_transient_interrupt(indio_dev);
+		ret = IRQ_HANDLED;
+	}
+
+out:
+	return ret;
+}
+
 static irqreturn_t mma8452_trigger_handler(int irq, void *p)
 {
 	struct iio_poll_func *pf = p;

@@ -316,6 +449,31 @@ static int mma8452_reg_access_dbg(struct iio_dev *indio_dev,
 	return ret;
 }
 
+static const struct iio_event_spec mma8452_transient_event[] = {
+	{
+		.type = IIO_EV_TYPE_THRESH,
+		.dir = IIO_EV_DIR_RISING,
+		.mask_separate = BIT(IIO_EV_INFO_ENABLE),
+		.mask_shared_by_type = BIT(IIO_EV_INFO_VALUE),
+	},
+};
+
+/**
+ * Threshold is configured in fixed 8G/127 steps regardless of
+ * currently selected scale for measurement.
+ */
+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,
+	NULL,
+};
+
+static struct attribute_group mma8452_event_attribute_group = {
+	.attrs = mma8452_event_attributes,
+	.name = "events",
+};
+
 #define MMA8452_CHANNEL(axis, idx) { \
 	.type = IIO_ACCEL, \
 	.modified = 1, \

@@ -332,6 +490,8 @@ static int mma8452_reg_access_dbg(struct iio_dev *indio_dev,
 		.shift = 4, \
 		.endianness = IIO_BE, \
 	}, \
+	.event_spec = mma8452_transient_event, \
+	.num_event_specs = ARRAY_SIZE(mma8452_transient_event), \
 }
 
 static const struct iio_chan_spec mma8452_channels[] = {

@@ -355,6 +515,11 @@ static const struct iio_info mma8452_info = {
 	.attrs = &mma8452_group,
 	.read_raw = &mma8452_read_raw,
 	.write_raw = &mma8452_write_raw,
+	.event_attrs = &mma8452_event_attribute_group,
+	.read_event_value = &mma8452_read_thresh,
+	.write_event_value = &mma8452_write_thresh,
+	.read_event_config = &mma8452_read_event_config,
+	.write_event_config = &mma8452_write_event_config,
 	.debugfs_reg_access = &mma8452_reg_access_dbg,
 	.driver_module = THIS_MODULE,
 };

@@ -425,6 +590,29 @@ static int mma8452_probe(struct i2c_client *client,
 	if (ret < 0)
 		return ret;
 
+	/* By default set transient threshold to max to avoid events if
+	 * enabling without configuring threshold */
+	ret = i2c_smbus_write_byte_data(client, MMA8452_TRANSIENT_THS, 0x7f);
+	if (ret < 0)
+		return ret;
+
+	if (client->irq) {
+		int supported_interrupts = MMA8452_INT_TRANS;
+
+		/* Assume wired to INT1 pin */
+		ret = i2c_smbus_write_byte_data(client,
+						MMA8452_CTRL_REG5,
+						supported_interrupts);
+		if (ret < 0)
+			return ret;
+
+		ret = i2c_smbus_write_byte_data(client,
+						MMA8452_CTRL_REG4,
+						supported_interrupts);
+		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,

@@ -440,8 +628,21 @@ static int mma8452_probe(struct i2c_client *client,
 	ret = iio_device_register(indio_dev);
 	if (ret < 0)
 		goto buffer_cleanup;
+
+	if (client->irq) {
+		ret = devm_request_threaded_irq(&client->dev,
+						client->irq,
+						NULL, mma8452_interrupt,
+						IRQF_TRIGGER_LOW | IRQF_ONESHOT,
+						client->name, indio_dev);
+		if (ret)
+			goto device_cleanup;
+	}
 	return 0;
 
+device_cleanup:
+	iio_device_unregister(indio_dev);
+
 buffer_cleanup:
 	iio_triggered_buffer_cleanup(indio_dev);
 	return ret;
author
#9 lore raw

PATCH 4/8 iio: mma8452: Add support for transient event debouncing

In reply to Martin Fuzzey

Allow the debouce counter for transient events to be configured using the sysfs attribute events/in_accel_thresh_rising_period

Signed-off-by Martin Fuzzey <[email protected]>
---
 drivers/iio/accel/mma8452.c |   75 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 73 insertions(+), 2 deletions(-)
drivers/iio/accel/mma8452.c +73 -2
index 8129c0b..7e0c29c 100644
@@ -129,6 +129,12 @@ static int mma8452_get_int_plus_micros_index(const int (*vals)[2], int n,
 	return -EINVAL;
 }
 
+static int mma8452_get_odr_index(struct mma8452_data *data)
+{
+	return (data->ctrl_reg1 & MMA8452_CTRL_DR_MASK) >>
+			MMA8452_CTRL_DR_SHIFT;
+}
+
 static const int mma8452_samp_freq[8][2] = {
 	{800, 0}, {400, 0}, {200, 0}, {100, 0}, {50, 0}, {12, 500000},
 	{6, 250000}, {1, 560000}

@@ -144,6 +150,18 @@ static const int mma8452_scales[3][2] = {
 	{0, 9577}, {0, 19154}, {0, 38307}
 };
 
+/* Datasheet table 35  (step time vs sample frequency) */
+static const int mma8452_transient_time_step_us[8] = {
+	1250,
+	2500,
+	5000,
+	10000,
+	20000,
+	20000,
+	20000,
+	20000
+};
+
 static ssize_t mma8452_show_samp_freq_avail(struct device *dev,
 				struct device_attribute *attr, char *buf)
 {

@@ -203,8 +221,7 @@ static int mma8452_read_raw(struct iio_dev *indio_dev,
 		*val2 = mma8452_scales[i][1];
 		return IIO_VAL_INT_PLUS_MICRO;
 	case IIO_CHAN_INFO_SAMP_FREQ:
-		i = (data->ctrl_reg1 & MMA8452_CTRL_DR_MASK) >>
-			MMA8452_CTRL_DR_SHIFT;
+		i = mma8452_get_odr_index(data);
 		*val = mma8452_samp_freq[i][0];
 		*val2 = mma8452_samp_freq[i][1];
 		return IIO_VAL_INT_PLUS_MICRO;

@@ -356,6 +373,52 @@ static int mma8452_write_event_config(struct iio_dev *indio_dev,
 	return mma8452_change_config(data, MMA8452_TRANSIENT_CFG, val);
 }
 
+static ssize_t mma8452_query_transient_period(struct device *dev,
+						struct device_attribute *attr,
+						char *buf)
+{
+	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+	struct mma8452_data *data = iio_priv(indio_dev);
+	int us, steps;
+
+	steps = i2c_smbus_read_byte_data(data->client, MMA8452_TRANSIENT_COUNT);
+	if (steps < 0)
+		return steps;
+
+	us = mma8452_transient_time_step_us[mma8452_get_odr_index(data)]
+								* steps;
+
+	return sprintf(buf, "%ld.%06ld\n",
+			us / USEC_PER_SEC, us % USEC_PER_SEC);
+}
+
+static ssize_t mma8452_set_transient_period(struct device *dev,
+						struct device_attribute *attr,
+						const char *buf,
+						size_t len)
+{
+	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+	struct mma8452_data *data = iio_priv(indio_dev);
+	int i, f, us, steps, ret;
+
+	ret = iio_str_to_fixpoint(buf, USEC_PER_SEC / 10, &i, &f);
+	if (ret)
+		return ret;
+
+	us = i * USEC_PER_SEC + f;
+	steps = us / mma8452_transient_time_step_us[
+				mma8452_get_odr_index(data)];
+
+	if (steps < 0 || steps > 0xFF)
+		return -EINVAL;
+
+	ret = mma8452_change_config(data, MMA8452_TRANSIENT_COUNT, steps);
+	if (ret)
+		return ret;
+
+	return len;
+}
+
 static void mma8452_push_transient_event(struct iio_dev *indio_dev,
 						int chan, u64 ts)
 {

@@ -464,8 +527,16 @@ static const struct iio_event_spec mma8452_transient_event[] = {
  */
 static IIO_CONST_ATTR_NAMED(accel_transient_scale, in_accel_scale, "0.617742");
 
+static IIO_DEVICE_ATTR_NAMED(accel_transient_period,
+			     in_accel_thresh_rising_period,
+			     S_IRUGO | S_IWUSR,
+			     mma8452_query_transient_period,
+			     mma8452_set_transient_period,
+			     0);
+
 static struct attribute *mma8452_event_attributes[] = {
 	&iio_const_attr_accel_transient_scale.dev_attr.attr,
+	&iio_dev_attr_accel_transient_period.dev_attr.attr,
 	NULL,
 };
author
#12 lore raw

PATCH 5/8 iio: core: add high pass filter attributes

In reply to Martin Fuzzey
Signed-off-by Martin Fuzzey <[email protected]>
---
 Documentation/ABI/testing/sysfs-bus-iio |   10 ++++++++++
 drivers/iio/industrialio-core.c         |    2 ++
 include/linux/iio/iio.h                 |    1 +
 3 files changed, 13 insertions(+)
Documentation/ABI/testing/sysfs-bus-iio +10 -0
index a9757dc..4825139 100644
@@ -340,6 +340,16 @@ Description:
 		to the underlying data channel, then this parameter
 		gives the 3dB frequency of the filter in Hz.
 
+What:		/sys/.../in_accel_filter_high_pass_3db_frequency
+What:		/sys/.../in_magn_filter_high_pass_3db_frequency
+What:		/sys/.../in_anglvel_filter_high_pass_3db_frequency
+KernelVersion:	3.17
+Contact:	[email protected]
+Description:
+		If a known or controllable high pass filter is applied
+		to the underlying data channel, then this parameter
+		gives the 3dB frequency of the filter in Hz.
+
 What:		/sys/bus/iio/devices/iio:deviceX/out_voltageY_raw
 What:		/sys/bus/iio/devices/iio:deviceX/out_altvoltageY_raw
 KernelVersion:	2.6.37
drivers/iio/industrialio-core.c +2 -0
index fa06197..967468d 100644
@@ -103,6 +103,8 @@ static const char * const iio_chan_info_postfix[] = {
 	[IIO_CHAN_INFO_AVERAGE_RAW] = "mean_raw",
 	[IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY]
 	= "filter_low_pass_3db_frequency",
+	[IIO_CHAN_INFO_HIGH_PASS_FILTER_3DB_FREQUENCY]
+	= "filter_high_pass_3db_frequency",
 	[IIO_CHAN_INFO_SAMP_FREQ] = "sampling_frequency",
 	[IIO_CHAN_INFO_FREQUENCY] = "frequency",
 	[IIO_CHAN_INFO_PHASE] = "phase",
include/linux/iio/iio.h +1 -0
index ccde917..88a255d 100644
@@ -31,6 +31,7 @@ enum iio_chan_info_enum {
 	IIO_CHAN_INFO_QUADRATURE_CORRECTION_RAW,
 	IIO_CHAN_INFO_AVERAGE_RAW,
 	IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY,
+	IIO_CHAN_INFO_HIGH_PASS_FILTER_3DB_FREQUENCY,
 	IIO_CHAN_INFO_SAMP_FREQ,
 	IIO_CHAN_INFO_FREQUENCY,
 	IIO_CHAN_INFO_PHASE,
author
#13 lore raw

PATCH 6/8 io: mma8452: Add highpass filter configuration.

In reply to Martin Fuzzey

Allow the cutoff frequency of the high pass filter to be configured.

Signed-off-by Martin Fuzzey <[email protected]>
---
 drivers/iio/accel/mma8452.c |   65 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 63 insertions(+), 2 deletions(-)
drivers/iio/accel/mma8452.c +63 -2
index 7e0c29c..66933b2 100644
@@ -29,6 +29,8 @@
 #define MMA8452_INT_SRC 0x0c
 #define MMA8452_WHO_AM_I 0x0d
 #define MMA8452_DATA_CFG 0x0e
+#define MMA8452_HP_FILTER_CUTOFF 0x0f
+#define MMA8452_HP_FILTER_CUTOFF_SEL_MASK	(BIT(0) | BIT(1))
 #define MMA8452_TRANSIENT_CFG 0x1d
 #define MMA8452_TRANSIENT_CFG_ELE		BIT(4)
 #define MMA8452_TRANSIENT_CFG_CHAN_MASK(chan)	(BIT(1) << chan)

@@ -162,6 +164,18 @@ static const int mma8452_transient_time_step_us[8] = {
 	20000
 };
 
+/* Datasheet table 18 (normal mode) */
+static const int mma8452_hp_filter_cutoff[8][4][2] = {
+	{ {16, 0}, {8, 0}, {4, 0}, {2, 0} },		/* 800 Hz sample */
+	{ {16, 0}, {8, 0}, {4, 0}, {2, 0} },		/* 400 Hz sample */
+	{ {8, 0}, {4, 0}, {2, 0}, {1, 0} },		/* 200 Hz sample */
+	{ {4, 0}, {2, 0}, {1, 0}, {0, 500000} },	/* 100 Hz sample */
+	{ {2, 0}, {1, 0}, {0, 500000}, {0, 250000} },	/* 50 Hz sample */
+	{ {2, 0}, {1, 0}, {0, 500000}, {0, 250000} },	/* 12.5 Hz sample */
+	{ {2, 0}, {1, 0}, {0, 500000}, {0, 250000} },	/* 6.25 Hz sample */
+	{ {2, 0}, {1, 0}, {0, 500000}, {0, 250000} }	/* 1.56 Hz sample */
+};
+
 static ssize_t mma8452_show_samp_freq_avail(struct device *dev,
 				struct device_attribute *attr, char *buf)
 {

@@ -176,9 +190,22 @@ static ssize_t mma8452_show_scale_avail(struct device *dev,
 		ARRAY_SIZE(mma8452_scales));
 }
 
+static ssize_t mma8452_show_hp_cutoff_avail(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+	struct mma8452_data *data = iio_priv(indio_dev);
+	int i = mma8452_get_odr_index(data);
+
+	return mma8452_show_int_plus_micros(buf, mma8452_hp_filter_cutoff[i],
+		ARRAY_SIZE(mma8452_hp_filter_cutoff[0]));
+}
+
 static IIO_DEV_ATTR_SAMP_FREQ_AVAIL(mma8452_show_samp_freq_avail);
 static IIO_DEVICE_ATTR(in_accel_scale_available, S_IRUGO,
 	mma8452_show_scale_avail, NULL, 0);
+static IIO_DEVICE_ATTR(in_accel_filter_high_pass_3db_frequency_available,
+			S_IRUGO, mma8452_show_hp_cutoff_avail, NULL, 0);
 
 static int mma8452_get_samp_freq_index(struct mma8452_data *data,
 	int val, int val2)

@@ -194,6 +221,15 @@ static int mma8452_get_scale_index(struct mma8452_data *data,
 		ARRAY_SIZE(mma8452_scales), val, val2);
 }
 
+static int mma8452_get_hp_filter_index(struct mma8452_data *data,
+	int val, int val2)
+{
+	int i = mma8452_get_odr_index(data);
+
+	return mma8452_get_int_plus_micros_index(mma8452_hp_filter_cutoff[i],
+		ARRAY_SIZE(mma8452_scales[0]), val, val2);
+}
+
 static int mma8452_read_raw(struct iio_dev *indio_dev,
 			    struct iio_chan_spec const *chan,
 			    int *val, int *val2, long mask)

@@ -232,6 +268,16 @@ static int mma8452_read_raw(struct iio_dev *indio_dev,
 			return ret;
 		*val = sign_extend32(ret, 7);
 		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_HIGH_PASS_FILTER_3DB_FREQUENCY:
+		ret = i2c_smbus_read_byte_data(data->client,
+						MMA8452_HP_FILTER_CUTOFF);
+		if (ret < 0)
+			return ret;
+		i = mma8452_get_odr_index(data);
+		ret &= MMA8452_HP_FILTER_CUTOFF_SEL_MASK;
+		*val = mma8452_hp_filter_cutoff[i][ret][0];
+		*val2 = mma8452_hp_filter_cutoff[i][ret][1];
+		return IIO_VAL_INT_PLUS_MICRO;
 	}
 	return -EINVAL;
 }

@@ -278,7 +324,7 @@ static int mma8452_write_raw(struct iio_dev *indio_dev,
 			     int val, int val2, long mask)
 {
 	struct mma8452_data *data = iio_priv(indio_dev);
-	int i;
+	int i, reg;
 
 	if (iio_buffer_enabled(indio_dev))
 		return -EBUSY;

@@ -306,6 +352,19 @@ static int mma8452_write_raw(struct iio_dev *indio_dev,
 			return -EINVAL;
 		return mma8452_change_config(data, MMA8452_OFF_X +
 			chan->scan_index, val);
+	case IIO_CHAN_INFO_HIGH_PASS_FILTER_3DB_FREQUENCY:
+		i = mma8452_get_hp_filter_index(data, val, val2);
+		if (i < 0)
+			return -EINVAL;
+
+		reg = i2c_smbus_read_byte_data(data->client,
+						MMA8452_HP_FILTER_CUTOFF);
+		if (reg < 0)
+			return reg;
+		reg &= ~MMA8452_HP_FILTER_CUTOFF_SEL_MASK;
+		reg |= i;
+		return mma8452_change_config(data,
+				MMA8452_HP_FILTER_CUTOFF, reg);
 	default:
 		return -EINVAL;
 	}

@@ -552,7 +611,8 @@ static struct attribute_group mma8452_event_attribute_group = {
 	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
 		BIT(IIO_CHAN_INFO_CALIBBIAS), \
 	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SAMP_FREQ) | \
-		BIT(IIO_CHAN_INFO_SCALE), \
+		BIT(IIO_CHAN_INFO_SCALE) | \
+		BIT(IIO_CHAN_INFO_HIGH_PASS_FILTER_3DB_FREQUENCY), \
 	.scan_index = idx, \
 	.scan_type = { \
 		.sign = 's', \

@@ -575,6 +635,7 @@ static const struct iio_chan_spec mma8452_channels[] = {
 static struct attribute *mma8452_attributes[] = {
 	&iio_dev_attr_sampling_frequency_available.dev_attr.attr,
 	&iio_dev_attr_in_accel_scale_available.dev_attr.attr,
+	&iio_dev_attr_in_accel_filter_high_pass_3db_frequency_available.dev_attr.attr,
 	NULL
 };
author
#14 lore raw

PATCH 7/8 iio: mma8452: add an attribute to enable the highpass filter

In reply to Martin Fuzzey

The hardware contains a single configurable highpass filter which is normally used for transient detection (event).

However it is also possible to enable this filter for normal channel reading. Add a new attribute in_accel_high_pass_filter_en to do this.

Signed-off-by Martin Fuzzey <[email protected]>
---
 drivers/iio/accel/mma8452.c |  115 +++++++++++++++++++++++++++++--------------
 1 file changed, 78 insertions(+), 37 deletions(-)
drivers/iio/accel/mma8452.c +78 -37
index 66933b2..c56d34e 100644
@@ -62,6 +62,7 @@
 #define MMA8452_DATA_CFG_FS_2G 0
 #define MMA8452_DATA_CFG_FS_4G 1
 #define MMA8452_DATA_CFG_FS_8G 2
+#define MMA8452_DATA_CFG_HPF_MASK BIT(4)
 
 #define MMA8452_INT_DRDY	BIT(0)
 #define MMA8452_INT_FF_MT	BIT(2)

@@ -106,6 +107,43 @@ static int mma8452_read(struct mma8452_data *data, __be16 buf[3])
 		MMA8452_OUT_X, 3 * sizeof(__be16), (u8 *) buf);
 }
 
+static int mma8452_standby(struct mma8452_data *data)
+{
+	return i2c_smbus_write_byte_data(data->client, MMA8452_CTRL_REG1,
+		data->ctrl_reg1 & ~MMA8452_CTRL_ACTIVE);
+}
+
+static int mma8452_active(struct mma8452_data *data)
+{
+	return i2c_smbus_write_byte_data(data->client, MMA8452_CTRL_REG1,
+		data->ctrl_reg1);
+}
+
+static int mma8452_change_config(struct mma8452_data *data, u8 reg, u8 val)
+{
+	int ret;
+
+	mutex_lock(&data->lock);
+
+	/* config can only be changed when in standby */
+	ret = mma8452_standby(data);
+	if (ret < 0)
+		goto fail;
+
+	ret = i2c_smbus_write_byte_data(data->client, reg, val);
+	if (ret < 0)
+		goto fail;
+
+	ret = mma8452_active(data);
+	if (ret < 0)
+		goto fail;
+
+	ret = 0;
+fail:
+	mutex_unlock(&data->lock);
+	return ret;
+}
+
 static ssize_t mma8452_show_int_plus_micros(char *buf,
 	const int (*vals)[2], int n)
 {

@@ -201,11 +239,50 @@ static ssize_t mma8452_show_hp_cutoff_avail(struct device *dev,
 		ARRAY_SIZE(mma8452_hp_filter_cutoff[0]));
 }
 
+static ssize_t mma8452_show_hp_cutoff_en(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+	struct mma8452_data *data = iio_priv(indio_dev);
+
+	return sprintf(buf, "%d\n",
+		data->data_cfg & MMA8452_DATA_CFG_HPF_MASK ? 1 : 0);
+}
+
+static ssize_t mma8452_store_hp_cutoff_en(struct device *dev,
+				struct device_attribute *attr,
+				const char *buf, size_t len)
+{
+	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+	struct mma8452_data *data = iio_priv(indio_dev);
+	bool state;
+	int ret;
+
+	ret = strtobool(buf, &state);
+	if (ret < 0)
+		return ret;
+
+	if (state)
+		data->data_cfg |= MMA8452_DATA_CFG_HPF_MASK;
+	else
+		data->data_cfg &= ~MMA8452_DATA_CFG_HPF_MASK;
+
+	ret = mma8452_change_config(data, MMA8452_DATA_CFG, data->data_cfg);
+	if (ret)
+		return ret;
+
+	return len;
+}
+
 static IIO_DEV_ATTR_SAMP_FREQ_AVAIL(mma8452_show_samp_freq_avail);
 static IIO_DEVICE_ATTR(in_accel_scale_available, S_IRUGO,
 	mma8452_show_scale_avail, NULL, 0);
 static IIO_DEVICE_ATTR(in_accel_filter_high_pass_3db_frequency_available,
 			S_IRUGO, mma8452_show_hp_cutoff_avail, NULL, 0);
+static IIO_DEVICE_ATTR(in_accel_filter_high_pass_en,
+			 S_IRUGO | S_IWUSR,
+			 mma8452_show_hp_cutoff_en,
+			 mma8452_store_hp_cutoff_en, 0);
 
 static int mma8452_get_samp_freq_index(struct mma8452_data *data,
 	int val, int val2)

@@ -282,43 +359,6 @@ static int mma8452_read_raw(struct iio_dev *indio_dev,
 	return -EINVAL;
 }
 
-static int mma8452_standby(struct mma8452_data *data)
-{
-	return i2c_smbus_write_byte_data(data->client, MMA8452_CTRL_REG1,
-		data->ctrl_reg1 & ~MMA8452_CTRL_ACTIVE);
-}
-
-static int mma8452_active(struct mma8452_data *data)
-{
-	return i2c_smbus_write_byte_data(data->client, MMA8452_CTRL_REG1,
-		data->ctrl_reg1);
-}
-
-static int mma8452_change_config(struct mma8452_data *data, u8 reg, u8 val)
-{
-	int ret;
-
-	mutex_lock(&data->lock);
-
-	/* config can only be changed when in standby */
-	ret = mma8452_standby(data);
-	if (ret < 0)
-		goto fail;
-
-	ret = i2c_smbus_write_byte_data(data->client, reg, val);
-	if (ret < 0)
-		goto fail;
-
-	ret = mma8452_active(data);
-	if (ret < 0)
-		goto fail;
-
-	ret = 0;
-fail:
-	mutex_unlock(&data->lock);
-	return ret;
-}
-
 static int mma8452_write_raw(struct iio_dev *indio_dev,
 			     struct iio_chan_spec const *chan,
 			     int val, int val2, long mask)

@@ -636,6 +676,7 @@ static struct attribute *mma8452_attributes[] = {
 	&iio_dev_attr_sampling_frequency_available.dev_attr.attr,
 	&iio_dev_attr_in_accel_scale_available.dev_attr.attr,
 	&iio_dev_attr_in_accel_filter_high_pass_3db_frequency_available.dev_attr.attr,
+	&iio_dev_attr_in_accel_filter_high_pass_en.dev_attr.attr,
 	NULL
 };
author
#17 lore raw

PATCH 8/8 iio: mma8452: Add support for interrupt driven triggers.

In reply to Martin Fuzzey

Implement interrupt driven trigger for data ready. This allows more efficient access to the sample data.

Signed-off-by Martin Fuzzey <[email protected]>
---
 drivers/iio/accel/mma8452.c |   86 +++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 82 insertions(+), 4 deletions(-)
drivers/iio/accel/mma8452.c +82 -4
index c56d34e..f5ee5f2 100644
@@ -18,6 +18,8 @@
 #include <linux/iio/sysfs.h>
 #include <linux/iio/trigger_consumer.h>
 #include <linux/iio/buffer.h>
+#include <linux/iio/trigger.h>
+#include <linux/iio/trigger_consumer.h>
 #include <linux/iio/triggered_buffer.h>
 #include <linux/iio/events.h>
 #include <linux/delay.h>

@@ -560,6 +562,11 @@ static irqreturn_t mma8452_interrupt(int irq, void *p)
 	if (src < 0)
 		goto out;
 
+	if (src & MMA8452_INT_DRDY) {
+		iio_trigger_poll_chained(indio_dev->trig, iio_get_time_ns());
+		ret = IRQ_HANDLED;
+	}
+
 	if (src & MMA8452_INT_TRANS) {
 		mma8452_transient_interrupt(indio_dev);
 		ret = IRQ_HANDLED;

@@ -699,6 +706,67 @@ static const struct iio_info mma8452_info = {
 
 static const unsigned long mma8452_scan_masks[] = {0x7, 0};
 
+static int mma8452_data_rdy_trigger_set_state(struct iio_trigger *trig,
+		bool state)
+{
+	struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig);
+	struct mma8452_data *data = iio_priv(indio_dev);
+	int reg;
+
+	reg = i2c_smbus_read_byte_data(data->client, MMA8452_CTRL_REG4);
+	if (reg < 0)
+		return reg;
+
+	if (state)
+		reg |= MMA8452_INT_DRDY;
+	else
+		reg &= ~MMA8452_INT_DRDY;
+
+	return mma8452_change_config(data, MMA8452_CTRL_REG4, reg);
+}
+
+static const struct iio_trigger_ops mma8452_trigger_ops = {
+	.set_trigger_state = mma8452_data_rdy_trigger_set_state,
+	.owner = THIS_MODULE,
+};
+
+static int mma8452_trigger_setup(struct iio_dev *indio_dev)
+{
+	struct mma8452_data *data = iio_priv(indio_dev);
+	struct iio_trigger *trig;
+	int ret;
+
+	trig = iio_trigger_alloc("%s-dev%d", indio_dev->name,
+						indio_dev->id);
+	if (!trig)
+		return -ENOMEM;
+
+	trig->dev.parent = &data->client->dev;
+	trig->ops = &mma8452_trigger_ops;
+	iio_trigger_set_drvdata(trig, indio_dev);
+
+	ret = iio_trigger_register(trig);
+	if (ret)
+		goto err_trigger_free;
+
+	indio_dev->trig = trig;
+
+	return 0;
+
+err_trigger_free:
+	iio_trigger_free(trig);
+
+	return ret;
+}
+
+static void mma8452_trigger_cleanup(struct iio_dev *indio_dev)
+{
+	if (indio_dev->trig) {
+		iio_trigger_unregister(indio_dev->trig);
+		iio_trigger_free(indio_dev->trig);
+	}
+}
+
 static int mma8452_reset(struct i2c_client *client)
 {
 	int i;

@@ -770,7 +838,8 @@ static int mma8452_probe(struct i2c_client *client,
 		return ret;
 
 	if (client->irq) {
-		int supported_interrupts = MMA8452_INT_TRANS;
+		int supported_interrupts = MMA8452_INT_DRDY | MMA8452_INT_TRANS;
+		int enabled_interrupts = MMA8452_INT_TRANS;
 
 		/* Assume wired to INT1 pin */
 		ret = i2c_smbus_write_byte_data(client,

@@ -781,7 +850,11 @@ static int mma8452_probe(struct i2c_client *client,
 
 		ret = i2c_smbus_write_byte_data(client,
 						MMA8452_CTRL_REG4,
-						supported_interrupts);
+						enabled_interrupts);
+		if (ret < 0)
+			return ret;
+
+		ret = mma8452_trigger_setup(indio_dev);
 		if (ret < 0)
 			return ret;
 	}

@@ -791,12 +864,12 @@ static int mma8452_probe(struct i2c_client *client,
 	ret = i2c_smbus_write_byte_data(client, MMA8452_CTRL_REG1,
 		data->ctrl_reg1);
 	if (ret < 0)
-		return ret;
+		goto trigger_cleanup;
 
 	ret = iio_triggered_buffer_setup(indio_dev, NULL,
 		mma8452_trigger_handler, NULL);
 	if (ret < 0)
-		return ret;
+		goto trigger_cleanup;
 
 	ret = iio_device_register(indio_dev);
 	if (ret < 0)

@@ -818,6 +891,10 @@ device_cleanup:
 
 buffer_cleanup:
 	iio_triggered_buffer_cleanup(indio_dev);
+
+trigger_cleanup:
+	mma8452_trigger_cleanup(indio_dev);
+
 	return ret;
 }
 

@@ -827,6 +904,7 @@ static int mma8452_remove(struct i2c_client *client)
 
 	iio_device_unregister(indio_dev);
 	iio_triggered_buffer_cleanup(indio_dev);
+	mma8452_trigger_cleanup(indio_dev);
 	mma8452_standby(iio_priv(indio_dev));
 
 	return 0;
#2 lore raw

Re: [PATCH 1/8] iio: mma8452: Initialise before activating

In reply to Martin Fuzzey

Hello Martin,

Quoted 2 lines

Many of the hardware configuration registers may only be modified while the device is inactive.

so this means that writing INFO_SAMP_FREQ or INFO_SCALE won't work without deactivating / reactivating the device?

I'm pretty sure I tried this...

p.

Quoted 95 lines

Currently the probe code first activates the device and then modifies the registers (eg to set the scale). This doesn't actually work but is not noticed since the scale used is the default value.

While at it also issue a hardware reset command at probe time.

Signed-off-by Martin Fuzzey <[email protected]>
---
 drivers/iio/accel/mma8452.c |   39 ++++++++++++++++++++++++++++++++++-----
 1 file changed, 34 insertions(+), 5 deletions(-)
drivers/iio/accel/mma8452.c +34 -6
index 3c12d49..04a4f34 100644
@@ -32,6 +32,7 @@
 #define MMA8452_OFF_Z 0x31
 #define MMA8452_CTRL_REG1 0x2a
 #define MMA8452_CTRL_REG2 0x2b
+#define MMA8452_CTRL_REG2_RST		BIT(6)
 
 #define MMA8452_STATUS_DRDY (BIT(2) | BIT(1) | BIT(0))
 

@@ -111,7 +112,7 @@ static const int mma8452_samp_freq[8][2] = {
 	{6, 250000}, {1, 560000}
 };
 
-/* 
+/*
  * Hardware has fullscale of -2G, -4G, -8G corresponding to raw value -2048
  * The userspace interface uses m/s^2 and we declare micro units
  * So scale factor is given by:

@@ -335,6 +336,30 @@ static const struct iio_info mma8452_info = {
 
 static const unsigned long mma8452_scan_masks[] = {0x7, 0};
 
+static int mma8452_reset(struct i2c_client *client)
+{
+	int i;
+	int ret;
+
+	ret = i2c_smbus_write_byte_data(client,	MMA8452_CTRL_REG2,
+						MMA8452_CTRL_REG2_RST);
+	if (ret)
+		return ret;
+
+	for (i = 0; i < 10; i++) {
+		udelay(100);
+		ret = i2c_smbus_read_byte_data(client, MMA8452_CTRL_REG2);
+		if (ret == -EIO)
+			continue; /* I2C comm reset */
+		if (ret < 0)
+			return ret;
+		if (!(ret & MMA8452_CTRL_REG2_RST))
+			return 0;
+	}
+
+	return -ETIMEDOUT;
+}
+
 static int mma8452_probe(struct i2c_client *client,
 			 const struct i2c_device_id *id)
 {

@@ -365,10 +390,7 @@ static int mma8452_probe(struct i2c_client *client,
 	indio_dev->num_channels = ARRAY_SIZE(mma8452_channels);
 	indio_dev->available_scan_masks = mma8452_scan_masks;
 
-	data->ctrl_reg1 = MMA8452_CTRL_ACTIVE |
-		(MMA8452_CTRL_DR_DEFAULT << MMA8452_CTRL_DR_SHIFT);
-	ret = i2c_smbus_write_byte_data(client, MMA8452_CTRL_REG1,
-		data->ctrl_reg1);
+	ret = mma8452_reset(client);
 	if (ret < 0)
 		return ret;
 

@@ -378,6 +400,13 @@ static int mma8452_probe(struct i2c_client *client,
 	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,
+		data->ctrl_reg1);
+	if (ret < 0)
+		return ret;
+
 	ret = iio_triggered_buffer_setup(indio_dev, NULL,
 		mma8452_trigger_handler, NULL);
 	if (ret < 0)
 
--

To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Signature · 2 lines
Peter Meerwald
+43-664-2444418 (mobile)
#6 lore raw

Re: [PATCH 3/8] iio: mma8452: Basic support for transient events.

In reply to Martin Fuzzey
Quoted 2 lines

The event is triggered when the highpass filtered absolute acceleration exceeds the threshold.

comments inline

Quoted 25 lines
Signed-off-by Martin Fuzzey <[email protected]>
---
 drivers/iio/accel/mma8452.c |  203 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 202 insertions(+), 1 deletion(-)
drivers/iio/accel/mma8452.c +2 -1
index c46df4e..8129c0b 100644
@@ -9,7 +9,7 @@
  *
  * 7-bit I2C slave address 0x1c/0x1d (pin selectable)
  *
- * TODO: interrupt, thresholding, orientation / freefall events, autosleep
+ * TODO: orientation / freefall events, autosleep
  */
 
 #include <linux/module.h>

@@ -19,20 +19,33 @@
 #include <linux/iio/trigger_consumer.h>
 #include <linux/iio/buffer.h>
 #include <linux/iio/triggered_buffer.h>
+#include <linux/iio/events.h>
 #include <linux/delay.h>

drivers/iio/accel/mma8452.c line 34

Quoted 10 lines
drivers/iio/accel/mma8452.c +4 -0
 #define MMA8452_STATUS 0x00
 #define MMA8452_OUT_X 0x01 /* MSB first, 12-bit  */
 #define MMA8452_OUT_Y 0x03
 #define MMA8452_OUT_Z 0x05
+#define MMA8452_INT_SRC 0x0c
 #define MMA8452_WHO_AM_I 0x0d
 #define MMA8452_DATA_CFG 0x0e
+#define MMA8452_TRANSIENT_CFG 0x1d
+#define MMA8452_TRANSIENT_CFG_ELE		BIT(4)
+#define MMA8452_TRANSIENT_CFG_CHAN_MASK(chan)	(BIT(1) << chan)

could use GENMASK() macro

Quoted 124 lines
drivers/iio/accel/mma8452.c +104 -0
+#define MMA8452_TRANSIENT_SRC 0x1e
+#define MMA8452_TRANSIENT_SRC_XTRANSE		BIT(1)
+#define MMA8452_TRANSIENT_SRC_YTRANSE		BIT(3)
+#define MMA8452_TRANSIENT_SRC_ZTRANSE		BIT(5)
+#define MMA8452_TRANSIENT_THS 0x1f
+#define MMA8452_TRANSIENT_COUNT 0x20
 #define MMA8452_OFF_X 0x2f
 #define MMA8452_OFF_Y 0x30
 #define MMA8452_OFF_Z 0x31
 #define MMA8452_CTRL_REG1 0x2a
 #define MMA8452_CTRL_REG2 0x2b
 #define MMA8452_CTRL_REG2_RST		BIT(6)
+#define MMA8452_CTRL_REG4 0x2d
+#define MMA8452_CTRL_REG5 0x2e
 
 #define MMA8452_MAX_REG 0x31
 

@@ -48,6 +61,13 @@
 #define MMA8452_DATA_CFG_FS_4G 1
 #define MMA8452_DATA_CFG_FS_8G 2
 
+#define MMA8452_INT_DRDY	BIT(0)
+#define MMA8452_INT_FF_MT	BIT(2)
+#define MMA8452_INT_PULSE	BIT(3)
+#define MMA8452_INT_LNDPRT	BIT(4)
+#define MMA8452_INT_TRANS	BIT(5)
+#define MMA8452_INT_ASLP	BIT(7)
+
 #define MMA8452_DEVICE_ID 0x2a
 
 struct mma8452_data {

@@ -274,6 +294,119 @@ static int mma8452_write_raw(struct iio_dev *indio_dev,
 	}
 }
 
+static int mma8452_read_thresh(struct iio_dev *indio_dev,
+	const struct iio_chan_spec *chan, enum iio_event_type type,
+	enum iio_event_direction dir, enum iio_event_info info, int *val,
+	int *val2)
+{
+	struct mma8452_data *data = iio_priv(indio_dev);
+	int ret;
+
+	ret = i2c_smbus_read_byte_data(data->client, MMA8452_TRANSIENT_THS);
+	if (ret < 0)
+		return ret;
+
+	*val = ret & 0x7f;
+
+	return IIO_VAL_INT;
+}
+
+static int mma8452_write_thresh(struct iio_dev *indio_dev,
+	const struct iio_chan_spec *chan, enum iio_event_type type,
+	enum iio_event_direction dir, enum iio_event_info info, int val,
+	int val2)
+{
+	struct mma8452_data *data = iio_priv(indio_dev);
+
+	return mma8452_change_config(data, MMA8452_TRANSIENT_THS, val & 0x7f);
+}
+
+static int mma8452_read_event_config(struct iio_dev *indio_dev,
+	const struct iio_chan_spec *chan, enum iio_event_type type,
+	enum iio_event_direction dir)
+{
+	struct mma8452_data *data = iio_priv(indio_dev);
+	int ret;
+
+	ret = i2c_smbus_read_byte_data(data->client, MMA8452_TRANSIENT_CFG);
+	if (ret < 0)
+		return ret;
+
+	return ret & MMA8452_TRANSIENT_CFG_CHAN_MASK(chan->scan_index) ? 1 : 0;
+}
+
+static int mma8452_write_event_config(struct iio_dev *indio_dev,
+	const struct iio_chan_spec *chan, enum iio_event_type type,
+	enum iio_event_direction dir, int state)
+{
+	struct mma8452_data *data = iio_priv(indio_dev);
+	int val;
+
+	val = i2c_smbus_read_byte_data(data->client, MMA8452_TRANSIENT_CFG);
+	if (val < 0)
+		return val;
+
+	if (state)
+		val |= MMA8452_TRANSIENT_CFG_CHAN_MASK(chan->scan_index);
+	else
+		val &= ~MMA8452_TRANSIENT_CFG_CHAN_MASK(chan->scan_index);
+
+	val |= MMA8452_TRANSIENT_CFG_ELE;
+
+	return mma8452_change_config(data, MMA8452_TRANSIENT_CFG, val);
+}
+
+static void mma8452_push_transient_event(struct iio_dev *indio_dev,
+						int chan, u64 ts)
+{
+	iio_push_event(indio_dev,
+		IIO_UNMOD_EVENT_CODE(IIO_ACCEL,
+			chan,
+			IIO_EV_TYPE_THRESH,
+			IIO_EV_DIR_RISING),
+		ts);
+}
+
+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;
+
+	src = i2c_smbus_read_byte_data(data->client, MMA8452_TRANSIENT_SRC);
+	if (src < 0)
+		return;
+
+	if (src & MMA8452_TRANSIENT_SRC_XTRANSE)
+		mma8452_push_transient_event(indio_dev, 0, ts);
+
+	if (src & MMA8452_TRANSIENT_SRC_YTRANSE)
+		mma8452_push_transient_event(indio_dev, 1, ts);
+

drivers/iio/accel/mma8452.c static int mma8452_write_raw(struct iio_dev *indio_dev, · line 395

Quoted 10 lines
drivers/iio/accel/mma8452.c +10 -0
+	if (src & MMA8452_TRANSIENT_SRC_ZTRANSE)
+		mma8452_push_transient_event(indio_dev, 2, ts);
+}
+
+static irqreturn_t mma8452_interrupt(int irq, void *p)
+{
+	struct iio_dev *indio_dev = p;
+	struct mma8452_data *data = iio_priv(indio_dev);
+	int ret = IRQ_NONE;
+	int src;

save either ret or src

drivers/iio/accel/mma8452.c static int mma8452_write_raw(struct iio_dev *indio_dev, · line 399

Quoted 4 lines
drivers/iio/accel/mma8452.c +4 -0
+
+	src = i2c_smbus_read_byte_data(data->client, MMA8452_INT_SRC);
+	if (src < 0)
+		goto out;

return directly with IRQ_NONE

drivers/iio/accel/mma8452.c static int mma8452_write_raw(struct iio_dev *indio_dev, · line 403

Quoted 4 lines
drivers/iio/accel/mma8452.c +4 -0
+
+	if (src & MMA8452_INT_TRANS) {
+		mma8452_transient_interrupt(indio_dev);
+		ret = IRQ_HANDLED;

return directly with IRQ_HANDLED

drivers/iio/accel/mma8452.c static int mma8452_write_raw(struct iio_dev *indio_dev, · line 406

Quoted 3 lines
drivers/iio/accel/mma8452.c +3 -0
+	}
+
+out:

no need for goto/label

Quoted 60 lines
drivers/iio/accel/mma8452.c +35 -0
+	return ret;
+}
+
 static irqreturn_t mma8452_trigger_handler(int irq, void *p)
 {
 	struct iio_poll_func *pf = p;

@@ -316,6 +449,31 @@ static int mma8452_reg_access_dbg(struct iio_dev *indio_dev,
 	return ret;
 }
 
+static const struct iio_event_spec mma8452_transient_event[] = {
+	{
+		.type = IIO_EV_TYPE_THRESH,
+		.dir = IIO_EV_DIR_RISING,
+		.mask_separate = BIT(IIO_EV_INFO_ENABLE),
+		.mask_shared_by_type = BIT(IIO_EV_INFO_VALUE),
+	},
+};
+
+/**
+ * Threshold is configured in fixed 8G/127 steps regardless of
+ * currently selected scale for measurement.
+ */
+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,
+	NULL,
+};
+
+static struct attribute_group mma8452_event_attribute_group = {
+	.attrs = mma8452_event_attributes,
+	.name = "events",
+};
+
 #define MMA8452_CHANNEL(axis, idx) { \
 	.type = IIO_ACCEL, \
 	.modified = 1, \

@@ -332,6 +490,8 @@ static int mma8452_reg_access_dbg(struct iio_dev *indio_dev,
 		.shift = 4, \
 		.endianness = IIO_BE, \
 	}, \
+	.event_spec = mma8452_transient_event, \
+	.num_event_specs = ARRAY_SIZE(mma8452_transient_event), \
 }
 
 static const struct iio_chan_spec mma8452_channels[] = {

@@ -355,6 +515,11 @@ static const struct iio_info mma8452_info = {
 	.attrs = &mma8452_group,
 	.read_raw = &mma8452_read_raw,
 	.write_raw = &mma8452_write_raw,
+	.event_attrs = &mma8452_event_attribute_group,
+	.read_event_value = &mma8452_read_thresh,
+	.write_event_value = &mma8452_write_thresh,
+	.read_event_config = &mma8452_read_event_config,
+	.write_event_config = &mma8452_write_event_config,
 	.debugfs_reg_access = &mma8452_reg_access_dbg,
 	.driver_module = THIS_MODULE,
 };

drivers/iio/accel/mma8452.c static int mma8452_probe(struct i2c_client *client, · line 594

Quoted 5 lines
drivers/iio/accel/mma8452.c +2 -0
@@ -425,6 +590,29 @@ static int mma8452_probe(struct i2c_client *client,
 	if (ret < 0)
 		return ret;
 
+	/* By default set transient threshold to max to avoid events if
+	 * enabling without configuring threshold */

not a proper multiline comment

drivers/iio/accel/mma8452.c static int mma8452_probe(struct i2c_client *client, · line 599

Quoted 5 lines
drivers/iio/accel/mma8452.c +5 -0
+	ret = i2c_smbus_write_byte_data(client, MMA8452_TRANSIENT_THS, 0x7f);
+	if (ret < 0)
+		return ret;
+
+	if (client->irq) {

I am always confused if 0 is a valid irq or not; I think it is, so client->irq >= 0

Quoted 47 lines
drivers/iio/accel/mma8452.c +29 -1
+		int supported_interrupts = MMA8452_INT_TRANS;
+
+		/* Assume wired to INT1 pin */
+		ret = i2c_smbus_write_byte_data(client,
+						MMA8452_CTRL_REG5,
+						supported_interrupts);
+		if (ret < 0)
+			return ret;
+
+		ret = i2c_smbus_write_byte_data(client,
+						MMA8452_CTRL_REG4,
+						supported_interrupts);
+		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,

@@ -440,8 +628,21 @@ static int mma8452_probe(struct i2c_client *client,
 	ret = iio_device_register(indio_dev);
 	if (ret < 0)
 		goto buffer_cleanup;
+
+	if (client->irq) {
+		ret = devm_request_threaded_irq(&client->dev,
+						client->irq,
+						NULL, mma8452_interrupt,
+						IRQF_TRIGGER_LOW | IRQF_ONESHOT,
+						client->name, indio_dev);
+		if (ret)
+			goto device_cleanup;
+	}
 	return 0;
 
+device_cleanup:
+	iio_device_unregister(indio_dev);
+
 buffer_cleanup:
 	iio_triggered_buffer_cleanup(indio_dev);
 	return ret;
 
--

To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Signature · 2 lines
Peter Meerwald
+43-664-2444418 (mobile)
#10 lore raw

Re: [PATCH 4/8] iio: mma8452: Add support for transient event debouncing

In reply to Martin Fuzzey
Quoted 3 lines

Allow the debouce counter for transient events to be configured using the sysfs attribute events/in_accel_thresh_rising_period

comments inline

Quoted 82 lines
Signed-off-by Martin Fuzzey <[email protected]>
---
 drivers/iio/accel/mma8452.c |   75 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 73 insertions(+), 2 deletions(-)
drivers/iio/accel/mma8452.c +46 -2
index 8129c0b..7e0c29c 100644
@@ -129,6 +129,12 @@ static int mma8452_get_int_plus_micros_index(const int (*vals)[2], int n,
 	return -EINVAL;
 }
 
+static int mma8452_get_odr_index(struct mma8452_data *data)
+{
+	return (data->ctrl_reg1 & MMA8452_CTRL_DR_MASK) >>
+			MMA8452_CTRL_DR_SHIFT;
+}
+
 static const int mma8452_samp_freq[8][2] = {
 	{800, 0}, {400, 0}, {200, 0}, {100, 0}, {50, 0}, {12, 500000},
 	{6, 250000}, {1, 560000}

@@ -144,6 +150,18 @@ static const int mma8452_scales[3][2] = {
 	{0, 9577}, {0, 19154}, {0, 38307}
 };
 
+/* Datasheet table 35  (step time vs sample frequency) */
+static const int mma8452_transient_time_step_us[8] = {
+	1250,
+	2500,
+	5000,
+	10000,
+	20000,
+	20000,
+	20000,
+	20000
+};
+
 static ssize_t mma8452_show_samp_freq_avail(struct device *dev,
 				struct device_attribute *attr, char *buf)
 {

@@ -203,8 +221,7 @@ static int mma8452_read_raw(struct iio_dev *indio_dev,
 		*val2 = mma8452_scales[i][1];
 		return IIO_VAL_INT_PLUS_MICRO;
 	case IIO_CHAN_INFO_SAMP_FREQ:
-		i = (data->ctrl_reg1 & MMA8452_CTRL_DR_MASK) >>
-			MMA8452_CTRL_DR_SHIFT;
+		i = mma8452_get_odr_index(data);
 		*val = mma8452_samp_freq[i][0];
 		*val2 = mma8452_samp_freq[i][1];
 		return IIO_VAL_INT_PLUS_MICRO;

@@ -356,6 +373,52 @@ static int mma8452_write_event_config(struct iio_dev *indio_dev,
 	return mma8452_change_config(data, MMA8452_TRANSIENT_CFG, val);
 }
 
+static ssize_t mma8452_query_transient_period(struct device *dev,
+						struct device_attribute *attr,
+						char *buf)
+{
+	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+	struct mma8452_data *data = iio_priv(indio_dev);
+	int us, steps;
+
+	steps = i2c_smbus_read_byte_data(data->client, MMA8452_TRANSIENT_COUNT);
+	if (steps < 0)
+		return steps;
+
+	us = mma8452_transient_time_step_us[mma8452_get_odr_index(data)]
+								* steps;
+
+	return sprintf(buf, "%ld.%06ld\n",
+			us / USEC_PER_SEC, us % USEC_PER_SEC);
+}
+
+static ssize_t mma8452_set_transient_period(struct device *dev,
+						struct device_attribute *attr,
+						const char *buf,
+						size_t len)
+{
+	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+	struct mma8452_data *data = iio_priv(indio_dev);
+	int i, f, us, steps, ret;

drivers/iio/accel/mma8452.c static int mma8452_write_event_config(struct iio_dev *indio_dev, · line 412

Quoted 10 lines
drivers/iio/accel/mma8452.c +10 -0
+
+	ret = iio_str_to_fixpoint(buf, USEC_PER_SEC / 10, &i, &f);
+	if (ret)
+		return ret;
+
+	us = i * USEC_PER_SEC + f;
+	steps = us / mma8452_transient_time_step_us[
+				mma8452_get_odr_index(data)];
+
+	if (steps < 0 || steps > 0xFF)

perhaps do the check for negative i earlier; use lowercase hex-values

Quoted 35 lines
drivers/iio/accel/mma8452.c +17 -1
+		return -EINVAL;
+
+	ret = mma8452_change_config(data, MMA8452_TRANSIENT_COUNT, steps);
+	if (ret)
+		return ret;
+
+	return len;
+}
+
 static void mma8452_push_transient_event(struct iio_dev *indio_dev,
 						int chan, u64 ts)
 {

@@ -464,8 +527,16 @@ static const struct iio_event_spec mma8452_transient_event[] = {
  */
 static IIO_CONST_ATTR_NAMED(accel_transient_scale, in_accel_scale, "0.617742");
 
+static IIO_DEVICE_ATTR_NAMED(accel_transient_period,
+			     in_accel_thresh_rising_period,
+			     S_IRUGO | S_IWUSR,
+			     mma8452_query_transient_period,
+			     mma8452_set_transient_period,
+			     0);
+
 static struct attribute *mma8452_event_attributes[] = {
 	&iio_const_attr_accel_transient_scale.dev_attr.attr,
+	&iio_dev_attr_accel_transient_period.dev_attr.attr,
 	NULL,
 };
 
 
--

To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Signature · 2 lines
Peter Meerwald
+43-664-2444418 (mobile)
#15 lore raw

Re: [PATCH 7/8] iio: mma8452: add an attribute to enable the highpass filter

In reply to Martin Fuzzey
Quoted 6 lines

The hardware contains a single configurable highpass filter which is normally used for transient detection (event).

However it is also possible to enable this filter for normal channel reading. Add a new attribute in_accel_high_pass_filter_en to do this.

the patch moves around many lines of code, can likely be made much more compact

Quoted 170 lines
Signed-off-by Martin Fuzzey <[email protected]>
---
 drivers/iio/accel/mma8452.c |  115 +++++++++++++++++++++++++++++--------------
 1 file changed, 78 insertions(+), 37 deletions(-)
drivers/iio/accel/mma8452.c +78 -38
index 66933b2..c56d34e 100644
@@ -62,6 +62,7 @@
 #define MMA8452_DATA_CFG_FS_2G 0
 #define MMA8452_DATA_CFG_FS_4G 1
 #define MMA8452_DATA_CFG_FS_8G 2
+#define MMA8452_DATA_CFG_HPF_MASK BIT(4)
 
 #define MMA8452_INT_DRDY	BIT(0)
 #define MMA8452_INT_FF_MT	BIT(2)

@@ -106,6 +107,43 @@ static int mma8452_read(struct mma8452_data *data, __be16 buf[3])
 		MMA8452_OUT_X, 3 * sizeof(__be16), (u8 *) buf);
 }
 
+static int mma8452_standby(struct mma8452_data *data)
+{
+	return i2c_smbus_write_byte_data(data->client, MMA8452_CTRL_REG1,
+		data->ctrl_reg1 & ~MMA8452_CTRL_ACTIVE);
+}
+
+static int mma8452_active(struct mma8452_data *data)
+{
+	return i2c_smbus_write_byte_data(data->client, MMA8452_CTRL_REG1,
+		data->ctrl_reg1);
+}
+
+static int mma8452_change_config(struct mma8452_data *data, u8 reg, u8 val)
+{
+	int ret;
+
+	mutex_lock(&data->lock);
+
+	/* config can only be changed when in standby */
+	ret = mma8452_standby(data);
+	if (ret < 0)
+		goto fail;
+
+	ret = i2c_smbus_write_byte_data(data->client, reg, val);
+	if (ret < 0)
+		goto fail;
+
+	ret = mma8452_active(data);
+	if (ret < 0)
+		goto fail;
+
+	ret = 0;
+fail:
+	mutex_unlock(&data->lock);
+	return ret;
+}
+
 static ssize_t mma8452_show_int_plus_micros(char *buf,
 	const int (*vals)[2], int n)
 {

@@ -201,11 +239,50 @@ static ssize_t mma8452_show_hp_cutoff_avail(struct device *dev,
 		ARRAY_SIZE(mma8452_hp_filter_cutoff[0]));
 }
 
+static ssize_t mma8452_show_hp_cutoff_en(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+	struct mma8452_data *data = iio_priv(indio_dev);
+
+	return sprintf(buf, "%d\n",
+		data->data_cfg & MMA8452_DATA_CFG_HPF_MASK ? 1 : 0);
+}
+
+static ssize_t mma8452_store_hp_cutoff_en(struct device *dev,
+				struct device_attribute *attr,
+				const char *buf, size_t len)
+{
+	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+	struct mma8452_data *data = iio_priv(indio_dev);
+	bool state;
+	int ret;
+
+	ret = strtobool(buf, &state);
+	if (ret < 0)
+		return ret;
+
+	if (state)
+		data->data_cfg |= MMA8452_DATA_CFG_HPF_MASK;
+	else
+		data->data_cfg &= ~MMA8452_DATA_CFG_HPF_MASK;
+
+	ret = mma8452_change_config(data, MMA8452_DATA_CFG, data->data_cfg);
+	if (ret)
+		return ret;
+
+	return len;
+}
+
 static IIO_DEV_ATTR_SAMP_FREQ_AVAIL(mma8452_show_samp_freq_avail);
 static IIO_DEVICE_ATTR(in_accel_scale_available, S_IRUGO,
 	mma8452_show_scale_avail, NULL, 0);
 static IIO_DEVICE_ATTR(in_accel_filter_high_pass_3db_frequency_available,
 			S_IRUGO, mma8452_show_hp_cutoff_avail, NULL, 0);
+static IIO_DEVICE_ATTR(in_accel_filter_high_pass_en,
+			 S_IRUGO | S_IWUSR,
+			 mma8452_show_hp_cutoff_en,
+			 mma8452_store_hp_cutoff_en, 0);
 
 static int mma8452_get_samp_freq_index(struct mma8452_data *data,
 	int val, int val2)

@@ -282,43 +359,6 @@ static int mma8452_read_raw(struct iio_dev *indio_dev,
 	return -EINVAL;
 }
 
-static int mma8452_standby(struct mma8452_data *data)
-{
-	return i2c_smbus_write_byte_data(data->client, MMA8452_CTRL_REG1,
-		data->ctrl_reg1 & ~MMA8452_CTRL_ACTIVE);
-}
-
-static int mma8452_active(struct mma8452_data *data)
-{
-	return i2c_smbus_write_byte_data(data->client, MMA8452_CTRL_REG1,
-		data->ctrl_reg1);
-}
-
-static int mma8452_change_config(struct mma8452_data *data, u8 reg, u8 val)
-{
-	int ret;
-
-	mutex_lock(&data->lock);
-
-	/* config can only be changed when in standby */
-	ret = mma8452_standby(data);
-	if (ret < 0)
-		goto fail;
-
-	ret = i2c_smbus_write_byte_data(data->client, reg, val);
-	if (ret < 0)
-		goto fail;
-
-	ret = mma8452_active(data);
-	if (ret < 0)
-		goto fail;
-
-	ret = 0;
-fail:
-	mutex_unlock(&data->lock);
-	return ret;
-}
-
 static int mma8452_write_raw(struct iio_dev *indio_dev,
 			     struct iio_chan_spec const *chan,
 			     int val, int val2, long mask)

@@ -636,6 +676,7 @@ static struct attribute *mma8452_attributes[] = {
 	&iio_dev_attr_sampling_frequency_available.dev_attr.attr,
 	&iio_dev_attr_in_accel_scale_available.dev_attr.attr,
 	&iio_dev_attr_in_accel_filter_high_pass_3db_frequency_available.dev_attr.attr,
+	&iio_dev_attr_in_accel_filter_high_pass_en.dev_attr.attr,
 	NULL
 };
 
 
--

To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Signature · 2 lines
Peter Meerwald
+43-664-2444418 (mobile)
author
#7 lore raw

Re: [PATCH 3/8] iio: mma8452: Basic support for transient events.

In reply to Peter Meerwald

Hi Peter and thank you for the review.

Quoted 2 lines · Peter Meerwald

+#define MMA8452_TRANSIENT_CFG_CHAN_MASK(chan) (BIT(1) << chan)
could use GENMASK() macro

That would be

#define MMA8452_TRANSIENT_CFG_CHAN_MASK(chan) (GENMASK(chan + 1, chan + 1))

I don't really see that being better in the single bit case here.

Quoted 8 lines
drivers/iio/accel/mma8452.c +7 -0
+
+static irqreturn_t mma8452_interrupt(int irq, void *p)
+{
+	struct iio_dev *indio_dev = p;
+	struct mma8452_data *data = iio_priv(indio_dev);
+	int ret = IRQ_NONE;
+	int src;

save either ret or src

Ok, done for V2

Quoted 5 lines

drivers/iio/accel/mma8452.c static int mma8452_write_raw(struct iio_dev *indio_dev, · line 399

Quoted 4 lines
drivers/iio/accel/mma8452.c +4 -0
+
+	src = i2c_smbus_read_byte_data(data->client, MMA8452_INT_SRC);
+	if (src < 0)
+		goto out;

return directly with IRQ_NONE

Ok, done for V2

Quoted 5 lines

drivers/iio/accel/mma8452.c static int mma8452_write_raw(struct iio_dev *indio_dev, · line 403

Quoted 4 lines
drivers/iio/accel/mma8452.c +4 -0
+
+	if (src & MMA8452_INT_TRANS) {
+		mma8452_transient_interrupt(indio_dev);
+		ret = IRQ_HANDLED;

return directly with IRQ_HANDLED

Ok, done for V2

Quoted 4 lines

drivers/iio/accel/mma8452.c static int mma8452_write_raw(struct iio_dev *indio_dev, · line 406

Quoted 3 lines
drivers/iio/accel/mma8452.c +3 -0
+	}
+
+out:

no need for goto/label

Ok, done for V2

Quoted 3 lines
drivers/iio/accel/mma8452.c +2 -0
+	/* By default set transient threshold to max to avoid events if
+	 * enabling without configuring threshold */

not a proper multiline comment

Ok, fixed for V2

Quoted 7 lines

drivers/iio/accel/mma8452.c static int mma8452_probe(struct i2c_client *client, · line 599

Quoted 5 lines
drivers/iio/accel/mma8452.c +5 -0
+	ret = i2c_smbus_write_byte_data(client, MMA8452_TRANSIENT_THS, 0x7f);
+	if (ret < 0)
+		return ret;
+
+	if (client->irq) {

I am always confused if 0 is a valid irq or not; I think it is, so client->irq >= 0

On my DT platform when I remove the interrupt definition from the device tree I get 0 for client->irq so I think it is not.

author
#11 lore raw

Re: [PATCH 4/8] iio: mma8452: Add support for transient event debouncing

In reply to Peter Meerwald
Quoted 18 lines · Peter Meerwald
drivers/iio/accel/mma8452.c +18 -0
+static ssize_t mma8452_set_transient_period(struct device *dev,
+						struct device_attribute *attr,
+						const char *buf,
+						size_t len)
+{
+	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+	struct mma8452_data *data = iio_priv(indio_dev);
+	int i, f, us, steps, ret;
+
+	ret = iio_str_to_fixpoint(buf, USEC_PER_SEC / 10, &i, &f);
+	if (ret)
+		return ret;
+
+	us = i * USEC_PER_SEC + f;
+	steps = us / mma8452_transient_time_step_us[
+				mma8452_get_odr_index(data)];
+
+	if (steps < 0 || steps > 0xFF)
Quoted 2 lines

perhaps do the check for negative i earlier; use lowercase hex-values

Ok done for V2

author
#16 lore raw

Re: [PATCH 7/8] iio: mma8452: add an attribute to enable the highpass filter

In reply to Peter Meerwald
Quoted 7 lines · Peter Meerwald
Quoted 5 lines

The hardware contains a single configurable highpass filter which is normally used for transient detection (event).

However it is also possible to enable this filter for normal channel reading. Add a new attribute in_accel_high_pass_filter_en to do this.

the patch moves around many lines of code, can likely be made much more compact

Well yes it does move some code around but that is because new function mma8452_store_hp_cutoff_en(), introduced as main purpose of this patch, requires access to mma8452_change_config(), and indirectly to mma8452_standby() and mma8452_active()

So this patch moves those three functions earlier in the file.

The only ways I can to see to make it smaller would be:

1) Add a forward declaration for mma8452_change_config()
2) Place mma8452_store_hp_cutoff_en() later in the file.

But for 1) AFAIKT in the kernel forward declarations are avoided when possible to avoid introducing double maintenance points.

And for 2) that would break the logical grouping of keeping the attribute accessor functions together.

I didn't think the aim was necessarilly to minimize the patch size (providing it remains reviewable) but to maximize the quality of the resulting code once the patch is applied.

So, if no strong objections I'd like to keep this as is.

Quoted 171 lines
Quoted 171 lines
Signed-off-by Martin Fuzzey <[email protected]>
---
  drivers/iio/accel/mma8452.c |  115 +++++++++++++++++++++++++++++--------------
  1 file changed, 78 insertions(+), 37 deletions(-)
drivers/iio/accel/mma8452.c +78 -38
index 66933b2..c56d34e 100644
@@ -62,6 +62,7 @@
  #define MMA8452_DATA_CFG_FS_2G 0
  #define MMA8452_DATA_CFG_FS_4G 1
  #define MMA8452_DATA_CFG_FS_8G 2
+#define MMA8452_DATA_CFG_HPF_MASK BIT(4)
 
  #define MMA8452_INT_DRDY	BIT(0)
  #define MMA8452_INT_FF_MT	BIT(2)

@@ -106,6 +107,43 @@ static int mma8452_read(struct mma8452_data *data, __be16 buf[3])
  		MMA8452_OUT_X, 3 * sizeof(__be16), (u8 *) buf);
  }
 
+static int mma8452_standby(struct mma8452_data *data)
+{
+	return i2c_smbus_write_byte_data(data->client, MMA8452_CTRL_REG1,
+		data->ctrl_reg1 & ~MMA8452_CTRL_ACTIVE);
+}
+
+static int mma8452_active(struct mma8452_data *data)
+{
+	return i2c_smbus_write_byte_data(data->client, MMA8452_CTRL_REG1,
+		data->ctrl_reg1);
+}
+
+static int mma8452_change_config(struct mma8452_data *data, u8 reg, u8 val)
+{
+	int ret;
+
+	mutex_lock(&data->lock);
+
+	/* config can only be changed when in standby */
+	ret = mma8452_standby(data);
+	if (ret < 0)
+		goto fail;
+
+	ret = i2c_smbus_write_byte_data(data->client, reg, val);
+	if (ret < 0)
+		goto fail;
+
+	ret = mma8452_active(data);
+	if (ret < 0)
+		goto fail;
+
+	ret = 0;
+fail:
+	mutex_unlock(&data->lock);
+	return ret;
+}
+
  static ssize_t mma8452_show_int_plus_micros(char *buf,
  	const int (*vals)[2], int n)
  {

@@ -201,11 +239,50 @@ static ssize_t mma8452_show_hp_cutoff_avail(struct device *dev,
  		ARRAY_SIZE(mma8452_hp_filter_cutoff[0]));
  }
 
+static ssize_t mma8452_show_hp_cutoff_en(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+	struct mma8452_data *data = iio_priv(indio_dev);
+
+	return sprintf(buf, "%d\n",
+		data->data_cfg & MMA8452_DATA_CFG_HPF_MASK ? 1 : 0);
+}
+
+static ssize_t mma8452_store_hp_cutoff_en(struct device *dev,
+				struct device_attribute *attr,
+				const char *buf, size_t len)
+{
+	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+	struct mma8452_data *data = iio_priv(indio_dev);
+	bool state;
+	int ret;
+
+	ret = strtobool(buf, &state);
+	if (ret < 0)
+		return ret;
+
+	if (state)
+		data->data_cfg |= MMA8452_DATA_CFG_HPF_MASK;
+	else
+		data->data_cfg &= ~MMA8452_DATA_CFG_HPF_MASK;
+
+	ret = mma8452_change_config(data, MMA8452_DATA_CFG, data->data_cfg);
+	if (ret)
+		return ret;
+
+	return len;
+}
+
  static IIO_DEV_ATTR_SAMP_FREQ_AVAIL(mma8452_show_samp_freq_avail);
  static IIO_DEVICE_ATTR(in_accel_scale_available, S_IRUGO,
  	mma8452_show_scale_avail, NULL, 0);
  static IIO_DEVICE_ATTR(in_accel_filter_high_pass_3db_frequency_available,
  			S_IRUGO, mma8452_show_hp_cutoff_avail, NULL, 0);
+static IIO_DEVICE_ATTR(in_accel_filter_high_pass_en,
+			 S_IRUGO | S_IWUSR,
+			 mma8452_show_hp_cutoff_en,
+			 mma8452_store_hp_cutoff_en, 0);
 
  static int mma8452_get_samp_freq_index(struct mma8452_data *data,
  	int val, int val2)

@@ -282,43 +359,6 @@ static int mma8452_read_raw(struct iio_dev *indio_dev,
  	return -EINVAL;
  }
 
-static int mma8452_standby(struct mma8452_data *data)
-{
-	return i2c_smbus_write_byte_data(data->client, MMA8452_CTRL_REG1,
-		data->ctrl_reg1 & ~MMA8452_CTRL_ACTIVE);
-}
-
-static int mma8452_active(struct mma8452_data *data)
-{
-	return i2c_smbus_write_byte_data(data->client, MMA8452_CTRL_REG1,
-		data->ctrl_reg1);
-}
-
-static int mma8452_change_config(struct mma8452_data *data, u8 reg, u8 val)
-{
-	int ret;
-
-	mutex_lock(&data->lock);
-
-	/* config can only be changed when in standby */
-	ret = mma8452_standby(data);
-	if (ret < 0)
-		goto fail;
-
-	ret = i2c_smbus_write_byte_data(data->client, reg, val);
-	if (ret < 0)
-		goto fail;
-
-	ret = mma8452_active(data);
-	if (ret < 0)
-		goto fail;
-
-	ret = 0;
-fail:
-	mutex_unlock(&data->lock);
-	return ret;
-}
-
  static int mma8452_write_raw(struct iio_dev *indio_dev,
  			     struct iio_chan_spec const *chan,
  			     int val, int val2, long mask)

@@ -636,6 +676,7 @@ static struct attribute *mma8452_attributes[] = {
  	&iio_dev_attr_sampling_frequency_available.dev_attr.attr,
  	&iio_dev_attr_in_accel_scale_available.dev_attr.attr,
  	&iio_dev_attr_in_accel_filter_high_pass_3db_frequency_available.dev_attr.attr,
+	&iio_dev_attr_in_accel_filter_high_pass_en.dev_attr.attr,
  	NULL
  };
 
 
--

To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

author
#3 lore raw

Re: [PATCH 1/8] iio: mma8452: Initialise before activating

In reply to Peter Meerwald

Hi Peter,

Quoted 8 lines · Peter Meerwald

Hello Martin,

Quoted 2 lines

Many of the hardware configuration registers may only be modified while the device is inactive.

so this means that writing INFO_SAMP_FREQ or INFO_SCALE won't work without deactivating / reactivating the device?

I'm pretty sure I tried this...

I've tried it and it doesn't work.

Applying this (demo only, not for merging) patch on top of my series to make the debugfs code not deactivate / reactivate on writes:

@@ -608,7 +608,7 @@ static int mma8452_reg_access_dbg(struct iio_dev *indio_dev,

                return -EINVAL;

        if (readval == NULL) {
-               ret = mma8452_change_config(data, reg, writeval);
+               ret = i2c_smbus_write_byte_data(data->client, reg, writeval);
        } else {
                ret = i2c_smbus_read_byte_data(data->client, reg);
                if (ret < 0)

Then:

# cd /sys/kernel/debug/iio/iio:device0
# echo 0x0e > direct_reg_access
# cat direct_reg_access
0x0
# echo 0x0e 0x01 >direct_reg_access
# cat direct_reg_access
0x0

So the write is not changing anything.

Removing the demo patch so that mma8452_change_config() is used causes it to work as expected:

# echo 0x0e 0x01 > direct_reg_access
# cat direct_reg_access
0x1
maintainer
#8 lore raw

Re: [PATCH 3/8] iio: mma8452: Basic support for transient events.

In reply to Martin Fuzzey
Quoted 52 lines · Martin Fuzzey

Hi Peter and thank you for the review.

Quoted 2 lines · Peter Meerwald

+#define MMA8452_TRANSIENT_CFG_CHAN_MASK(chan) (BIT(1) << chan)
could use GENMASK() macro

That would be

#define MMA8452_TRANSIENT_CFG_CHAN_MASK(chan) (GENMASK(chan + 1, chan +
1))

I don't really see that being better in the single bit case here.

Quoted 8 lines
drivers/iio/accel/mma8452.c +7 -0
+
+static irqreturn_t mma8452_interrupt(int irq, void *p)
+{
+	struct iio_dev *indio_dev = p;
+	struct mma8452_data *data = iio_priv(indio_dev);
+	int ret = IRQ_NONE;
+	int src;

save either ret or src

Ok, done for V2

Quoted 5 lines

drivers/iio/accel/mma8452.c static int mma8452_write_raw(struct iio_dev *indio_dev, · line 399

Quoted 4 lines
drivers/iio/accel/mma8452.c +4 -0
+
+	src = i2c_smbus_read_byte_data(data->client, MMA8452_INT_SRC);
+	if (src < 0)
+		goto out;

return directly with IRQ_NONE

Ok, done for V2

Quoted 5 lines

drivers/iio/accel/mma8452.c static int mma8452_write_raw(struct iio_dev *indio_dev, · line 403

Quoted 4 lines
drivers/iio/accel/mma8452.c +4 -0
+
+	if (src & MMA8452_INT_TRANS) {
+		mma8452_transient_interrupt(indio_dev);
+		ret = IRQ_HANDLED;

return directly with IRQ_HANDLED

Ok, done for V2

Quoted 4 lines

drivers/iio/accel/mma8452.c static int mma8452_write_raw(struct iio_dev *indio_dev, · line 406

Quoted 3 lines
drivers/iio/accel/mma8452.c +3 -0
+	}
+
+out:

no need for goto/label

Ok, done for V2

Quoted 3 lines
drivers/iio/accel/mma8452.c +2 -0
+	/* By default set transient threshold to max to avoid events if
+	 * enabling without configuring threshold */

not a proper multiline comment

Ok, fixed for V2

Quoted 1 line

Review

Quoted 1 line
(quoted diff) +1 -0
+	ret = i2c_smbus_write_byte_data(client, MMA8452_TRANSIENT_THS,

0x7f);

Quoted 6 lines

drivers/iio/accel/mma8452.c static int mma8452_probe(struct i2c_client *client, · line 599

Quoted 4 lines
drivers/iio/accel/mma8452.c +4 -0
+	if (ret < 0)
+		return ret;
+
+	if (client->irq) {

I am always confused if 0 is a valid irq or not; I think it is, so client->irq >= 0

Quoted 4 lines

On my DT platform when I remove the interrupt definition from the device
tree
I get 0 for client->irq so I think it is not.

Used to be on Arm. A lot of effort went into clearing that up a while back.

J

Quoted 5 lines

--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Signature · 1 line
Sent from my Android phone with K-9 Mail. Please excuse my brevity.