[PATCH v5 0/8] iio: mma8452: improve driver and support more chips

[PATCH v5 0/8] iio: mma8452: improve driver and support more chips

[Martin Kepplinger]

Before I go away for holidays, this is version 5 of the mma8452 driver
improvements. This version removes the unnessasary DRIVER_NAME define and
improves the DT property "use_int2" to be extendible for any future additions
to the driver.

This is based on today's -next.


These changes add support for motion interrupts and 3 more accelerometer
chips, two of which use them because they don't support the until now
included transient interrupt sources:

MMA8453Q, MMA8652FC and MMA8653FC; datasheets are in the commit messages.
The driver and module name remains the same, seperating it from the device
names it now supports.

On top of this, there are minor documentation additions, and more notably,
it allows to use the driver, no matter how the interrupt pins are wired
on your board.

Please review and test if you can. For MMA8452Q, nothing should have
changed.

revision history
----------------
v5   DRIVER_NAME define removed; more flexible DT property
v4   cleanup; one bugfix patch removed from series; DT people added
v3   adds one patch to allow all possible pin wirings; adds more email
recipients
v2   splits the work into a series of smaller pieces
v1   initial post

[PATCH v5 0/8] iio: mma8452: improve driver and support more chips

[Martin Kepplinger]

Before I go away for holidays, this is version 5 of the mma8452 driver
improvements. This version removes the unnessasary DRIVER_NAME define and
improves the DT property "use_int2" to be extendible for any future additions
to the driver.

This is based on today's -next.


These changes add support for motion interrupts and 3 more accelerometer
chips, two of which use them because they don't support the until now
included transient interrupt sources:

MMA8453Q, MMA8652FC and MMA8653FC; datasheets are in the commit messages.
The driver and module name remains the same, seperating it from the device
names it now supports.

On top of this, there are minor documentation additions, and more notably,
it allows to use the driver, no matter how the interrupt pins are wired
on your board.

Please review and test if you can. For MMA8452Q, nothing should have
changed.

revision history
----------------
v5   DRIVER_NAME define removed; more flexible DT property
v4   cleanup; one bugfix patch removed from series; DT people added
v3   adds one patch to allow all possible pin wirings; adds more email
recipients
v2   splits the work into a series of smaller pieces
v1   initial post


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/8] iio: mma8452: refactor for seperating chip specific data

[Martin Kepplinger]

This adds a struct mma_chip_info to hold data that will remain specific to
the chip in use. It is provided during probe() and linked in
struct of_device_id.

Also this suggests that the driver is called "mma8452" and now handles the
MMA8452Q device, but is not limited to it.

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 | 185 ++++++++++++++++++++++++++++++++------------
 1 file changed, 136 insertions(+), 49 deletions(-)

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 13ea1ea..8965517 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -23,6 +23,7 @@
 #include <linux/iio/triggered_buffer.h>
 #include <linux/iio/events.h>
 #include <linux/delay.h>
+#include <linux/of_device.h>
 
 #define MMA8452_STATUS 0x00
 #define MMA8452_OUT_X 0x01 /* MSB first, 12-bit  */
@@ -78,6 +79,52 @@ struct mma8452_data {
 	struct mutex lock;
 	u8 ctrl_reg1;
 	u8 data_cfg;
+	const struct mma_chip_info *chip_info;
+};
+
+/**
+ * struct mma_chip_info - chip specific data for Freescale's accelerometers
+ * @chip_id:			WHO_AM_I register's value
+ * @channels:			struct iio_chan_spec matching the device's
+ *				capabilities
+ * @num_channels:		number of channels
+ * @mma_scales:			scale factors for converting register values
+ *				to m/s^2; 3 modes: 2g, 4g, 8g; 2 integers
+ *				per mode: m/s^2 and micro m/s^2
+ * @ev_cfg:			event config register address
+ * @ev_cfg_ele:			latch bit in event config register
+ * @ev_cfg_chan_shift:		number of the bit to enable events in X
+ *				direction; in event config register
+ * @ev_src:			event source register address
+ * @ev_src_xe:			bit in event source register that indicates
+ *				an event in X direction
+ * @ev_src_ye:			bit in event source register that indicates
+ *				an event in Y direction
+ * @ev_src_ze:			bit in event source register that indicates
+ *				an event in Z direction
+ * @ev_ths:			event threshold register address
+ * @ev_ths_mask:		mask for the threshold value
+ * @ev_count:			event count (period) register address
+ *
+ * Since not all chips supported by the driver support comparing high pass
+ * filtered data for events (interrupts), different interrupt sources are
+ * used for different chips and the relevant registers are included here.
+ */
+struct mma_chip_info {
+	u8 chip_id;
+	const struct iio_chan_spec *channels;
+	int num_channels;
+	const int mma_scales[3][2];
+	u8 ev_cfg;
+	u8 ev_cfg_ele;
+	u8 ev_cfg_chan_shift;
+	u8 ev_src;
+	u8 ev_src_xe;
+	u8 ev_src_ye;
+	u8 ev_src_ze;
+	u8 ev_ths;
+	u8 ev_ths_mask;
+	u8 ev_count;
 };
 
 static int mma8452_drdy(struct mma8452_data *data)
@@ -143,16 +190,6 @@ 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:
- * 	g * N * 1000000 / 2048 for N = 2, 4, 8 and g=9.80665
- */
-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,
@@ -187,8 +224,11 @@ static ssize_t mma8452_show_samp_freq_avail(struct device *dev,
 static ssize_t mma8452_show_scale_avail(struct device *dev,
 				struct device_attribute *attr, char *buf)
 {
-	return mma8452_show_int_plus_micros(buf, mma8452_scales,
-		ARRAY_SIZE(mma8452_scales));
+	struct mma8452_data *data = iio_priv(i2c_get_clientdata(
+					     to_i2c_client(dev)));
+
+	return mma8452_show_int_plus_micros(buf, data->chip_info->mma_scales,
+		ARRAY_SIZE(data->chip_info->mma_scales));
 }
 
 static ssize_t mma8452_show_hp_cutoff_avail(struct device *dev,
@@ -219,8 +259,8 @@ static int mma8452_get_samp_freq_index(struct mma8452_data *data,
 static int mma8452_get_scale_index(struct mma8452_data *data,
 	int val, int val2)
 {
-	return mma8452_get_int_plus_micros_index(mma8452_scales,
-		ARRAY_SIZE(mma8452_scales), val, val2);
+	return mma8452_get_int_plus_micros_index(data->chip_info->mma_scales,
+		ARRAY_SIZE(data->chip_info->mma_scales), val, val2);
 }
 
 static int mma8452_get_hp_filter_index(struct mma8452_data *data,
@@ -229,7 +269,7 @@ static int mma8452_get_hp_filter_index(struct mma8452_data *data,
 	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);
+		ARRAY_SIZE(data->chip_info->mma_scales[0]), val, val2);
 }
 
 static int mma8452_read_hp_filter(struct mma8452_data *data, int *hz, int *uHz)
@@ -266,13 +306,14 @@ static int mma8452_read_raw(struct iio_dev *indio_dev,
 		mutex_unlock(&data->lock);
 		if (ret < 0)
 			return ret;
-		*val = sign_extend32(
-			be16_to_cpu(buffer[chan->scan_index]) >> 4, 11);
+		*val = sign_extend32(be16_to_cpu(
+			buffer[chan->scan_index]) >> chan->scan_type.shift,
+			chan->scan_type.realbits - 1);
 		return IIO_VAL_INT;
 	case IIO_CHAN_INFO_SCALE:
 		i = data->data_cfg & MMA8452_DATA_CFG_FS_MASK;
-		*val = mma8452_scales[i][0];
-		*val2 = mma8452_scales[i][1];
+		*val = data->chip_info->mma_scales[i][0];
+		*val2 = data->chip_info->mma_scales[i][1];
 		return IIO_VAL_INT_PLUS_MICRO;
 	case IIO_CHAN_INFO_SAMP_FREQ:
 		i = mma8452_get_odr_index(data);
@@ -420,16 +461,16 @@ static int mma8452_read_thresh(struct iio_dev *indio_dev,
 	switch (info) {
 	case IIO_EV_INFO_VALUE:
 		ret = i2c_smbus_read_byte_data(data->client,
-					       MMA8452_TRANSIENT_THS);
+					       data->chip_info->ev_ths);
 		if (ret < 0)
 			return ret;
 
-		*val = ret & MMA8452_TRANSIENT_THS_MASK;
+		*val = ret & data->chip_info->ev_ths_mask;
 		return IIO_VAL_INT;
 
 	case IIO_EV_INFO_PERIOD:
 		ret = i2c_smbus_read_byte_data(data->client,
-					       MMA8452_TRANSIENT_COUNT);
+					       data->chip_info->ev_count);
 		if (ret < 0)
 			return ret;
 
@@ -472,8 +513,11 @@ static int mma8452_write_thresh(struct iio_dev *indio_dev,
 
 	switch (info) {
 	case IIO_EV_INFO_VALUE:
-		return mma8452_change_config(data, MMA8452_TRANSIENT_THS,
-					     val & MMA8452_TRANSIENT_THS_MASK);
+		if (val < 0 || val > 127) /* LSB 0.6178 m/s^2 */
+			return -EINVAL;
+
+		return mma8452_change_config(data, data->chip_info->ev_ths,
+					     val);
 
 	case IIO_EV_INFO_PERIOD:
 		steps = (val * USEC_PER_SEC + val2) /
@@ -483,7 +527,7 @@ static int mma8452_write_thresh(struct iio_dev *indio_dev,
 		if (steps > 0xff)
 			return -EINVAL;
 
-		return mma8452_change_config(data, MMA8452_TRANSIENT_COUNT,
+		return mma8452_change_config(data, data->chip_info->ev_count,
 					     steps);
 	case IIO_EV_INFO_HIGH_PASS_FILTER_3DB:
 		reg = i2c_smbus_read_byte_data(data->client,
@@ -512,13 +556,15 @@ static int mma8452_read_event_config(struct iio_dev *indio_dev,
 				     enum iio_event_direction dir)
 {
 	struct mma8452_data *data = iio_priv(indio_dev);
+	const struct mma_chip_info *chip = data->chip_info;
 	int ret;
 
-	ret = i2c_smbus_read_byte_data(data->client, MMA8452_TRANSIENT_CFG);
+	ret = i2c_smbus_read_byte_data(data->client,
+				       data->chip_info->ev_cfg);
 	if (ret < 0)
 		return ret;
 
-	return ret & MMA8452_TRANSIENT_CFG_CHAN(chan->scan_index) ? 1 : 0;
+	return !!(ret & BIT(chan->scan_index + chip->ev_cfg_chan_shift));
 }
 
 static int mma8452_write_event_config(struct iio_dev *indio_dev,
@@ -528,20 +574,21 @@ static int mma8452_write_event_config(struct iio_dev *indio_dev,
 				      int state)
 {
 	struct mma8452_data *data = iio_priv(indio_dev);
+	const struct mma_chip_info *chip = data->chip_info;
 	int val;
 
-	val = i2c_smbus_read_byte_data(data->client, MMA8452_TRANSIENT_CFG);
+	val = i2c_smbus_read_byte_data(data->client, chip->ev_cfg);
 	if (val < 0)
 		return val;
 
 	if (state)
-		val |= MMA8452_TRANSIENT_CFG_CHAN(chan->scan_index);
+		val |= BIT(chan->scan_index + chip->ev_cfg_chan_shift);
 	else
-		val &= ~MMA8452_TRANSIENT_CFG_CHAN(chan->scan_index);
+		val &= ~BIT(chan->scan_index + chip->ev_cfg_chan_shift);
 
 	val |= MMA8452_TRANSIENT_CFG_ELE;
 
-	return mma8452_change_config(data, MMA8452_TRANSIENT_CFG, val);
+	return mma8452_change_config(data, chip->ev_cfg, val);
 }
 
 static void mma8452_transient_interrupt(struct iio_dev *indio_dev)
@@ -550,25 +597,25 @@ static void mma8452_transient_interrupt(struct iio_dev *indio_dev)
 	s64 ts = iio_get_time_ns();
 	int src;
 
-	src = i2c_smbus_read_byte_data(data->client, MMA8452_TRANSIENT_SRC);
+	src = i2c_smbus_read_byte_data(data->client, data->chip_info->ev_src);
 	if (src < 0)
 		return;
 
-	if (src & MMA8452_TRANSIENT_SRC_XTRANSE)
+	if (src & data->chip_info->ev_src_xe)
 		iio_push_event(indio_dev,
 			       IIO_MOD_EVENT_CODE(IIO_ACCEL, 0, IIO_MOD_X,
 						  IIO_EV_TYPE_MAG,
 						  IIO_EV_DIR_RISING),
 			       ts);
 
-	if (src & MMA8452_TRANSIENT_SRC_YTRANSE)
+	if (src & data->chip_info->ev_src_ye)
 		iio_push_event(indio_dev,
 			       IIO_MOD_EVENT_CODE(IIO_ACCEL, 0, IIO_MOD_Y,
 						  IIO_EV_TYPE_MAG,
 						  IIO_EV_DIR_RISING),
 			       ts);
 
-	if (src & MMA8452_TRANSIENT_SRC_ZTRANSE)
+	if (src & data->chip_info->ev_src_ze)
 		iio_push_event(indio_dev,
 			       IIO_MOD_EVENT_CODE(IIO_ACCEL, 0, IIO_MOD_Z,
 						  IIO_EV_TYPE_MAG,
@@ -669,7 +716,7 @@ static struct attribute_group mma8452_event_attribute_group = {
 	.name = "events",
 };
 
-#define MMA8452_CHANNEL(axis, idx) { \
+#define MMA8452_CHANNEL(axis, idx, bits) { \
 	.type = IIO_ACCEL, \
 	.modified = 1, \
 	.channel2 = IIO_MOD_##axis, \
@@ -681,9 +728,9 @@ static struct attribute_group mma8452_event_attribute_group = {
 	.scan_index = idx, \
 	.scan_type = { \
 		.sign = 's', \
-		.realbits = 12, \
+		.realbits = (bits), \
 		.storagebits = 16, \
-		.shift = 4, \
+		.shift = 16 - (bits), \
 		.endianness = IIO_BE, \
 	}, \
 	.event_spec = mma8452_transient_event, \
@@ -691,12 +738,41 @@ static struct attribute_group mma8452_event_attribute_group = {
 }
 
 static const struct iio_chan_spec mma8452_channels[] = {
-	MMA8452_CHANNEL(X, 0),
-	MMA8452_CHANNEL(Y, 1),
-	MMA8452_CHANNEL(Z, 2),
+	MMA8452_CHANNEL(X, 0, 12),
+	MMA8452_CHANNEL(Y, 1, 12),
+	MMA8452_CHANNEL(Z, 2, 12),
 	IIO_CHAN_SOFT_TIMESTAMP(3),
 };
 
+enum {
+	mma8452,
+};
+
+static const struct mma_chip_info mma_chip_info_table[] = {
+	[mma8452] = {
+		.chip_id = MMA8452_DEVICE_ID,
+		.channels = mma8452_channels,
+		.num_channels = ARRAY_SIZE(mma8452_channels),
+		/*
+		 * Hardware has fullscale of -2G, -4G, -8G corresponding to
+		 * raw value -2048 for 12 bit or -512 for 10 bit.
+		 * The userspace interface uses m/s^2 and we declare micro units
+		 * So scale factor for 12 bit here is given by:
+		 *	g * N * 1000000 / 2048 for N = 2, 4, 8 and g=9.80665
+		 */
+		.mma_scales = { {0, 9577}, {0, 19154}, {0, 38307} },
+		.ev_cfg = MMA8452_TRANSIENT_CFG,
+		.ev_cfg_ele = MMA8452_TRANSIENT_CFG_ELE,
+		.ev_cfg_chan_shift = 1,
+		.ev_src = MMA8452_TRANSIENT_SRC,
+		.ev_src_xe = MMA8452_TRANSIENT_SRC_XTRANSE,
+		.ev_src_ye = MMA8452_TRANSIENT_SRC_YTRANSE,
+		.ev_src_ze = MMA8452_TRANSIENT_SRC_ZTRANSE,
+		.ev_ths = MMA8452_TRANSIENT_THS,
+		.ev_ths_mask = MMA8452_TRANSIENT_THS_MASK,
+		.ev_count = MMA8452_TRANSIENT_COUNT,
+	},
+};
 static struct attribute *mma8452_attributes[] = {
 	&iio_dev_attr_sampling_frequency_available.dev_attr.attr,
 	&iio_dev_attr_in_accel_scale_available.dev_attr.attr,
@@ -813,12 +889,18 @@ static int mma8452_reset(struct i2c_client *client)
 	return -ETIMEDOUT;
 }
 
+static const struct of_device_id mma8452_dt_ids[] = {
+	{ .compatible = "fsl,mma8452", .data = &mma_chip_info_table[mma8452] },
+	{ }
+};
+
 static int mma8452_probe(struct i2c_client *client,
 			 const struct i2c_device_id *id)
 {
 	struct mma8452_data *data;
 	struct iio_dev *indio_dev;
 	int ret;
+	const struct of_device_id *match;
 
 	ret = i2c_smbus_read_byte_data(client, MMA8452_WHO_AM_I);
 	if (ret < 0)
@@ -826,6 +908,12 @@ static int mma8452_probe(struct i2c_client *client,
 	if (ret != MMA8452_DEVICE_ID)
 		return -ENODEV;
 
+	match = of_match_device(mma8452_dt_ids, &client->dev);
+	if (!match) {
+		dev_err(&client->dev, "unknown device model\n");
+		return -ENODEV;
+	}
+
 	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
 	if (!indio_dev)
 		return -ENOMEM;
@@ -833,14 +921,18 @@ static int mma8452_probe(struct i2c_client *client,
 	data = iio_priv(indio_dev);
 	data->client = client;
 	mutex_init(&data->lock);
+	data->chip_info = match->data;
+
+	dev_info(&client->dev, "registering %s accelerometer; ID 0x%x\n",
+		 match->compatible, data->chip_info->chip_id);
 
 	i2c_set_clientdata(client, indio_dev);
 	indio_dev->info = &mma8452_info;
 	indio_dev->name = id->name;
 	indio_dev->dev.parent = &client->dev;
 	indio_dev->modes = INDIO_DIRECT_MODE;
-	indio_dev->channels = mma8452_channels;
-	indio_dev->num_channels = ARRAY_SIZE(mma8452_channels);
+	indio_dev->channels = data->chip_info->channels;
+	indio_dev->num_channels = data->chip_info->num_channels;
 	indio_dev->available_scan_masks = mma8452_scan_masks;
 
 	ret = mma8452_reset(client);
@@ -959,16 +1051,11 @@ static SIMPLE_DEV_PM_OPS(mma8452_pm_ops, mma8452_suspend, mma8452_resume);
 #endif
 
 static const struct i2c_device_id mma8452_id[] = {
-	{ "mma8452", 0 },
+	{ "mma8452", mma8452 },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, mma8452_id);
 
-static const struct of_device_id mma8452_dt_ids[] = {
-	{ .compatible = "fsl,mma8452" },
-	{ }
-};
-
 static struct i2c_driver mma8452_driver = {
 	.driver = {
 		.name	= "mma8452",
-- 
2.1.4

[PATCH 2/8] iio: mma8452: add support for MMA8453Q accelerometer chip

[Martin Kepplinger]

This adds support for the 10 bit version if Freescale's accelerometers
of this series. The datasheet is available at Freescale's website:

http://cache.freescale.com/files/sensors/doc/data_sheet/MMA8453Q.pdf

Signed-off-by: Martin Kepplinger <martin.kepplinger@theobroma-systems.com>
Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com>
---
 drivers/iio/accel/Kconfig   |  6 +++---
 drivers/iio/accel/mma8452.c | 38 +++++++++++++++++++++++++++++++++++---
 2 files changed, 38 insertions(+), 6 deletions(-)

diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
index 00e7bcb..dc80dc9 100644
--- a/drivers/iio/accel/Kconfig
+++ b/drivers/iio/accel/Kconfig
@@ -87,13 +87,13 @@ config KXSD9
 	  will be called kxsd9.
 
 config MMA8452
-	tristate "Freescale MMA8452Q Accelerometer Driver"
+	tristate "Freescale MMA8452Q and similar Accelerometers Driver"
 	depends on I2C
 	select IIO_BUFFER
 	select IIO_TRIGGERED_BUFFER
 	help
-	  Say yes here to build support for the Freescale MMA8452Q 3-axis
-	  accelerometer.
+	  Say yes here to build support for the following Freescale 3-axis
+	  accelerometers: MMA8452Q, MMA8453Q.
 
 	  To compile this driver as a module, choose M here: the module
 	  will be called mma8452.
diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 8965517..57b12eb 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -1,5 +1,8 @@
 /*
- * mma8452.c - Support for Freescale MMA8452Q 3-axis 12-bit accelerometer
+ * mma8452.c - Support for following Freescale 3-axis accelerometers:
+ *
+ *	MMA8452Q
+ *	MMA8453Q
  *
  * Copyright 2014 Peter Meerwald <pmeerw@pmeerw.net>
  *
@@ -26,7 +29,7 @@
 #include <linux/of_device.h>
 
 #define MMA8452_STATUS 0x00
-#define MMA8452_OUT_X 0x01 /* MSB first, 12-bit  */
+#define MMA8452_OUT_X 0x01 /* MSB first, 10 or 12-bit  */
 #define MMA8452_OUT_Y 0x03
 #define MMA8452_OUT_Z 0x05
 #define MMA8452_INT_SRC 0x0c
@@ -73,6 +76,7 @@
 #define MMA8452_INT_TRANS	BIT(5)
 
 #define MMA8452_DEVICE_ID 0x2a
+#define MMA8453_DEVICE_ID 0x3a
 
 struct mma8452_data {
 	struct i2c_client *client;
@@ -744,8 +748,16 @@ static const struct iio_chan_spec mma8452_channels[] = {
 	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),
+	IIO_CHAN_SOFT_TIMESTAMP(3),
+};
+
 enum {
 	mma8452,
+	mma8453,
 };
 
 static const struct mma_chip_info mma_chip_info_table[] = {
@@ -772,7 +784,24 @@ static const struct mma_chip_info mma_chip_info_table[] = {
 		.ev_ths_mask = MMA8452_TRANSIENT_THS_MASK,
 		.ev_count = MMA8452_TRANSIENT_COUNT,
 	},
+	[mma8453] = {
+		.chip_id = MMA8453_DEVICE_ID,
+		.channels = mma8453_channels,
+		.num_channels = ARRAY_SIZE(mma8453_channels),
+		.mma_scales = { {0, 38307}, {0, 76614}, {0, 153228} },
+		.ev_cfg = MMA8452_TRANSIENT_CFG,
+		.ev_cfg_ele = MMA8452_TRANSIENT_CFG_ELE,
+		.ev_cfg_chan_shift = 1,
+		.ev_src = MMA8452_TRANSIENT_SRC,
+		.ev_src_xe = MMA8452_TRANSIENT_SRC_XTRANSE,
+		.ev_src_ye = MMA8452_TRANSIENT_SRC_YTRANSE,
+		.ev_src_ze = MMA8452_TRANSIENT_SRC_ZTRANSE,
+		.ev_ths = MMA8452_TRANSIENT_THS,
+		.ev_ths_mask = MMA8452_TRANSIENT_THS_MASK,
+		.ev_count = MMA8452_TRANSIENT_COUNT,
+	},
 };
+
 static struct attribute *mma8452_attributes[] = {
 	&iio_dev_attr_sampling_frequency_available.dev_attr.attr,
 	&iio_dev_attr_in_accel_scale_available.dev_attr.attr,
@@ -891,6 +920,7 @@ static int mma8452_reset(struct i2c_client *client)
 
 static const struct of_device_id mma8452_dt_ids[] = {
 	{ .compatible = "fsl,mma8452", .data = &mma_chip_info_table[mma8452] },
+	{ .compatible = "fsl,mma8453", .data = &mma_chip_info_table[mma8453] },
 	{ }
 };
 
@@ -905,7 +935,8 @@ static int mma8452_probe(struct i2c_client *client,
 	ret = i2c_smbus_read_byte_data(client, MMA8452_WHO_AM_I);
 	if (ret < 0)
 		return ret;
-	if (ret != MMA8452_DEVICE_ID)
+
+	if (ret != MMA8452_DEVICE_ID && ret != MMA8453_DEVICE_ID)
 		return -ENODEV;
 
 	match = of_match_device(mma8452_dt_ids, &client->dev);
@@ -1052,6 +1083,7 @@ static SIMPLE_DEV_PM_OPS(mma8452_pm_ops, mma8452_suspend, mma8452_resume);
 
 static const struct i2c_device_id mma8452_id[] = {
 	{ "mma8452", mma8452 },
+	{ "mma8453", mma8453 },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, mma8452_id);
-- 
2.1.4

[PATCH 3/8] iio: mma8452: add freefall / motion interrupt source

[Martin Kepplinger]

This adds the freefall / motion interrupt source definitions to the driver.
It is used in this series' next patch, for chips that don't support the
transient interrupt source.

The iio event type is IIO_EV_TYPE_MAG since the threshold for comparison
is no signed value. An interrupt occurs on positive and negative values
when the magnitude crosses the threshold value.

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 | 45 ++++++++++++++++++++++++++++++++++++---------
 1 file changed, 36 insertions(+), 9 deletions(-)

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 57b12eb..d51ce63 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -37,9 +37,18 @@
 #define MMA8452_DATA_CFG 0x0e
 #define MMA8452_HP_FILTER_CUTOFF 0x0f
 #define MMA8452_HP_FILTER_CUTOFF_SEL_MASK	(BIT(0) | BIT(1))
+#define MMA8452_FF_MT_CFG 0x15
+#define MMA8452_FF_MT_CFG_OAE			BIT(6)
+#define MMA8452_FF_MT_CFG_ELE			BIT(7)
+#define MMA8452_FF_MT_SRC 0x16
+#define MMA8452_FF_MT_SRC_XHE			BIT(1)
+#define MMA8452_FF_MT_SRC_YHE			BIT(3)
+#define MMA8452_FF_MT_SRC_ZHE			BIT(5)
+#define MMA8452_FF_MT_THS 0x17
+#define MMA8452_FF_MT_THS_MASK 0x7f
+#define MMA8452_FF_MT_COUNT 0x18
 #define MMA8452_TRANSIENT_CFG 0x1d
 #define MMA8452_TRANSIENT_CFG_ELE		BIT(4)
-#define MMA8452_TRANSIENT_CFG_CHAN(chan)	BIT(chan + 1)
 #define MMA8452_TRANSIENT_CFG_HPF_BYP		BIT(0)
 #define MMA8452_TRANSIENT_SRC 0x1e
 #define MMA8452_TRANSIENT_SRC_XTRANSE		BIT(1)
@@ -73,6 +82,7 @@
 #define MMA8452_DATA_CFG_HPF_MASK BIT(4)
 
 #define MMA8452_INT_DRDY	BIT(0)
+#define MMA8452_INT_FF_MT	BIT(2)
 #define MMA8452_INT_TRANS	BIT(5)
 
 #define MMA8452_DEVICE_ID 0x2a
@@ -590,7 +600,8 @@ static int mma8452_write_event_config(struct iio_dev *indio_dev,
 	else
 		val &= ~BIT(chan->scan_index + chip->ev_cfg_chan_shift);
 
-	val |= MMA8452_TRANSIENT_CFG_ELE;
+	val |= chip->ev_cfg_ele;
+	val |= MMA8452_FF_MT_CFG_OAE;
 
 	return mma8452_change_config(data, chip->ev_cfg, val);
 }
@@ -631,6 +642,7 @@ static irqreturn_t mma8452_interrupt(int irq, void *p)
 {
 	struct iio_dev *indio_dev = p;
 	struct mma8452_data *data = iio_priv(indio_dev);
+	const struct mma_chip_info *chip = data->chip_info;
 	int ret = IRQ_NONE;
 	int src;
 
@@ -643,7 +655,10 @@ static irqreturn_t mma8452_interrupt(int irq, void *p)
 		ret = IRQ_HANDLED;
 	}
 
-	if (src & MMA8452_INT_TRANS) {
+	if ((src & MMA8452_INT_TRANS &&
+	     chip->ev_src == MMA8452_TRANSIENT_SRC) ||
+	    (src & MMA8452_INT_FF_MT &&
+	     chip->ev_src == MMA8452_FF_MT_SRC)) {
 		mma8452_transient_interrupt(indio_dev);
 		ret = IRQ_HANDLED;
 	}
@@ -704,6 +719,16 @@ static const struct iio_event_spec mma8452_transient_event[] = {
 	},
 };
 
+static const struct iio_event_spec mma8452_motion_event[] = {
+	{
+		.type = IIO_EV_TYPE_MAG,
+		.dir = IIO_EV_DIR_RISING,
+		.mask_separate = BIT(IIO_EV_INFO_ENABLE),
+		.mask_shared_by_type = BIT(IIO_EV_INFO_VALUE) |
+					BIT(IIO_EV_INFO_PERIOD)
+	},
+};
+
 /*
  * Threshold is configured in fixed 8G/127 steps regardless of
  * currently selected scale for measurement.
@@ -987,13 +1012,15 @@ static int mma8452_probe(struct i2c_client *client,
 
 	if (client->irq) {
 		/*
-		 * Although we enable the transient interrupt source once and
-		 * for all here the transient event detection itself is not
-		 * enabled until userspace asks for it by
-		 * mma8452_write_event_config()
+		 * Although we enable the interrupt sources once and for
+		 * all here the event detection itself is not enabled until
+		 * userspace asks for it by mma8452_write_event_config()
 		 */
-		int supported_interrupts = MMA8452_INT_DRDY | MMA8452_INT_TRANS;
-		int enabled_interrupts = MMA8452_INT_TRANS;
+		int supported_interrupts = MMA8452_INT_DRDY |
+					   MMA8452_INT_TRANS |
+					   MMA8452_INT_FF_MT;
+		int enabled_interrupts = MMA8452_INT_TRANS |
+					 MMA8452_INT_FF_MT;
 
 		/* Assume wired to INT1 pin */
 		ret = i2c_smbus_write_byte_data(client,
-- 
2.1.4

[PATCH 3/8] iio: mma8452: add freefall / motion interrupt source

[Martin Kepplinger]

This adds the freefall / motion interrupt source definitions to the driver.
It is used in this series' next patch, for chips that don't support the
transient interrupt source.

The iio event type is IIO_EV_TYPE_MAG since the threshold for comparison
is no signed value. An interrupt occurs on positive and negative values
when the magnitude crosses the threshold value.

Signed-off-by: Martin Kepplinger <martin.kepplinger-SN7IsUiht6C/RdPyistoZJqQE7yCjDx5@public.gmane.org>
Signed-off-by: Christoph Muellner <christoph.muellner-SN7IsUiht6C/RdPyistoZJqQE7yCjDx5@public.gmane.org>
---
 drivers/iio/accel/mma8452.c | 45 ++++++++++++++++++++++++++++++++++++---------
 1 file changed, 36 insertions(+), 9 deletions(-)

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 57b12eb..d51ce63 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -37,9 +37,18 @@
 #define MMA8452_DATA_CFG 0x0e
 #define MMA8452_HP_FILTER_CUTOFF 0x0f
 #define MMA8452_HP_FILTER_CUTOFF_SEL_MASK	(BIT(0) | BIT(1))
+#define MMA8452_FF_MT_CFG 0x15
+#define MMA8452_FF_MT_CFG_OAE			BIT(6)
+#define MMA8452_FF_MT_CFG_ELE			BIT(7)
+#define MMA8452_FF_MT_SRC 0x16
+#define MMA8452_FF_MT_SRC_XHE			BIT(1)
+#define MMA8452_FF_MT_SRC_YHE			BIT(3)
+#define MMA8452_FF_MT_SRC_ZHE			BIT(5)
+#define MMA8452_FF_MT_THS 0x17
+#define MMA8452_FF_MT_THS_MASK 0x7f
+#define MMA8452_FF_MT_COUNT 0x18
 #define MMA8452_TRANSIENT_CFG 0x1d
 #define MMA8452_TRANSIENT_CFG_ELE		BIT(4)
-#define MMA8452_TRANSIENT_CFG_CHAN(chan)	BIT(chan + 1)
 #define MMA8452_TRANSIENT_CFG_HPF_BYP		BIT(0)
 #define MMA8452_TRANSIENT_SRC 0x1e
 #define MMA8452_TRANSIENT_SRC_XTRANSE		BIT(1)
@@ -73,6 +82,7 @@
 #define MMA8452_DATA_CFG_HPF_MASK BIT(4)
 
 #define MMA8452_INT_DRDY	BIT(0)
+#define MMA8452_INT_FF_MT	BIT(2)
 #define MMA8452_INT_TRANS	BIT(5)
 
 #define MMA8452_DEVICE_ID 0x2a
@@ -590,7 +600,8 @@ static int mma8452_write_event_config(struct iio_dev *indio_dev,
 	else
 		val &= ~BIT(chan->scan_index + chip->ev_cfg_chan_shift);
 
-	val |= MMA8452_TRANSIENT_CFG_ELE;
+	val |= chip->ev_cfg_ele;
+	val |= MMA8452_FF_MT_CFG_OAE;
 
 	return mma8452_change_config(data, chip->ev_cfg, val);
 }
@@ -631,6 +642,7 @@ static irqreturn_t mma8452_interrupt(int irq, void *p)
 {
 	struct iio_dev *indio_dev = p;
 	struct mma8452_data *data = iio_priv(indio_dev);
+	const struct mma_chip_info *chip = data->chip_info;
 	int ret = IRQ_NONE;
 	int src;
 
@@ -643,7 +655,10 @@ static irqreturn_t mma8452_interrupt(int irq, void *p)
 		ret = IRQ_HANDLED;
 	}
 
-	if (src & MMA8452_INT_TRANS) {
+	if ((src & MMA8452_INT_TRANS &&
+	     chip->ev_src == MMA8452_TRANSIENT_SRC) ||
+	    (src & MMA8452_INT_FF_MT &&
+	     chip->ev_src == MMA8452_FF_MT_SRC)) {
 		mma8452_transient_interrupt(indio_dev);
 		ret = IRQ_HANDLED;
 	}
@@ -704,6 +719,16 @@ static const struct iio_event_spec mma8452_transient_event[] = {
 	},
 };
 
+static const struct iio_event_spec mma8452_motion_event[] = {
+	{
+		.type = IIO_EV_TYPE_MAG,
+		.dir = IIO_EV_DIR_RISING,
+		.mask_separate = BIT(IIO_EV_INFO_ENABLE),
+		.mask_shared_by_type = BIT(IIO_EV_INFO_VALUE) |
+					BIT(IIO_EV_INFO_PERIOD)
+	},
+};
+
 /*
  * Threshold is configured in fixed 8G/127 steps regardless of
  * currently selected scale for measurement.
@@ -987,13 +1012,15 @@ static int mma8452_probe(struct i2c_client *client,
 
 	if (client->irq) {
 		/*
-		 * Although we enable the transient interrupt source once and
-		 * for all here the transient event detection itself is not
-		 * enabled until userspace asks for it by
-		 * mma8452_write_event_config()
+		 * Although we enable the interrupt sources once and for
+		 * all here the event detection itself is not enabled until
+		 * userspace asks for it by mma8452_write_event_config()
 		 */
-		int supported_interrupts = MMA8452_INT_DRDY | MMA8452_INT_TRANS;
-		int enabled_interrupts = MMA8452_INT_TRANS;
+		int supported_interrupts = MMA8452_INT_DRDY |
+					   MMA8452_INT_TRANS |
+					   MMA8452_INT_FF_MT;
+		int enabled_interrupts = MMA8452_INT_TRANS |
+					 MMA8452_INT_FF_MT;
 
 		/* Assume wired to INT1 pin */
 		ret = i2c_smbus_write_byte_data(client,
-- 
2.1.4

[PATCH 4/8] iio: mma8452: add support for MMA8652FC and MMA8653FC accelerometers

[Martin Kepplinger]

MMA8652FC and MMA8653FC don't provide the transient interrupt source, so
the motion interrupt source is used by providing a new iio_chan_spec
definition, so that other supported devices are not affected by this.

Datasheets for the newly supported devices are available at Freescale's
website:

http://cache.freescale.com/files/sensors/doc/data_sheet/MMA8652FC.pdf
http://cache.freescale.com/files/sensors/doc/data_sheet/MMA8653FC.pdf

Signed-off-by: Martin Kepplinger <martin.kepplinger@theobroma-systems.com>
Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com>
---
 drivers/iio/accel/Kconfig   |  2 +-
 drivers/iio/accel/mma8452.c | 98 +++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 92 insertions(+), 8 deletions(-)

diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
index dc80dc9..fdb59e0 100644
--- a/drivers/iio/accel/Kconfig
+++ b/drivers/iio/accel/Kconfig
@@ -93,7 +93,7 @@ config MMA8452
 	select IIO_TRIGGERED_BUFFER
 	help
 	  Say yes here to build support for the following Freescale 3-axis
-	  accelerometers: MMA8452Q, MMA8453Q.
+	  accelerometers: MMA8452Q, MMA8453Q, MMA8652FC, MMA8653FC.
 
 	  To compile this driver as a module, choose M here: the module
 	  will be called mma8452.
diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index d51ce63..5a8c738 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -3,6 +3,8 @@
  *
  *	MMA8452Q
  *	MMA8453Q
+ *	MMA8652FC
+ *	MMA8653FC
  *
  * Copyright 2014 Peter Meerwald <pmeerw@pmeerw.net>
  *
@@ -87,6 +89,8 @@
 
 #define MMA8452_DEVICE_ID 0x2a
 #define MMA8453_DEVICE_ID 0x3a
+#define MMA8652_DEVICE_ID 0x4a
+#define MMA8653_DEVICE_ID 0x5a
 
 struct mma8452_data {
 	struct i2c_client *client;
@@ -766,6 +770,26 @@ static struct attribute_group mma8452_event_attribute_group = {
 	.num_event_specs = ARRAY_SIZE(mma8452_transient_event), \
 }
 
+#define MMA8652_CHANNEL(axis, idx, bits) { \
+	.type = IIO_ACCEL, \
+	.modified = 1, \
+	.channel2 = IIO_MOD_##axis, \
+	.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), \
+	.scan_index = idx, \
+	.scan_type = { \
+		.sign = 's', \
+		.realbits = (bits), \
+		.storagebits = 16, \
+		.shift = 16 - (bits), \
+		.endianness = IIO_BE, \
+	}, \
+	.event_spec = mma8452_motion_event, \
+	.num_event_specs = ARRAY_SIZE(mma8452_motion_event), \
+}
+
 static const struct iio_chan_spec mma8452_channels[] = {
 	MMA8452_CHANNEL(X, 0, 12),
 	MMA8452_CHANNEL(Y, 1, 12),
@@ -780,9 +804,25 @@ static const struct iio_chan_spec mma8453_channels[] = {
 	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),
+	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),
+	IIO_CHAN_SOFT_TIMESTAMP(3),
+};
+
 enum {
 	mma8452,
 	mma8453,
+	mma8652,
+	mma8653,
 };
 
 static const struct mma_chip_info mma_chip_info_table[] = {
@@ -825,6 +865,38 @@ static const struct mma_chip_info mma_chip_info_table[] = {
 		.ev_ths_mask = MMA8452_TRANSIENT_THS_MASK,
 		.ev_count = MMA8452_TRANSIENT_COUNT,
 	},
+	[mma8652] = {
+		.chip_id = MMA8652_DEVICE_ID,
+		.channels = mma8652_channels,
+		.num_channels = ARRAY_SIZE(mma8652_channels),
+		.mma_scales = { {0, 9577}, {0, 19154}, {0, 38307} },
+		.ev_cfg = MMA8452_FF_MT_CFG,
+		.ev_cfg_ele = MMA8452_FF_MT_CFG_ELE,
+		.ev_cfg_chan_shift = 3,
+		.ev_src = MMA8452_FF_MT_SRC,
+		.ev_src_xe = MMA8452_FF_MT_SRC_XHE,
+		.ev_src_ye = MMA8452_FF_MT_SRC_YHE,
+		.ev_src_ze = MMA8452_FF_MT_SRC_ZHE,
+		.ev_ths = MMA8452_FF_MT_THS,
+		.ev_ths_mask = MMA8452_FF_MT_THS_MASK,
+		.ev_count = MMA8452_FF_MT_COUNT,
+	},
+	[mma8653] = {
+		.chip_id = MMA8653_DEVICE_ID,
+		.channels = mma8653_channels,
+		.num_channels = ARRAY_SIZE(mma8653_channels),
+		.mma_scales = { {0, 38307}, {0, 76614}, {0, 153228} },
+		.ev_cfg = MMA8452_FF_MT_CFG,
+		.ev_cfg_ele = MMA8452_FF_MT_CFG_ELE,
+		.ev_cfg_chan_shift = 3,
+		.ev_src = MMA8452_FF_MT_SRC,
+		.ev_src_xe = MMA8452_FF_MT_SRC_XHE,
+		.ev_src_ye = MMA8452_FF_MT_SRC_YHE,
+		.ev_src_ze = MMA8452_FF_MT_SRC_ZHE,
+		.ev_ths = MMA8452_FF_MT_THS,
+		.ev_ths_mask = MMA8452_FF_MT_THS_MASK,
+		.ev_count = MMA8452_FF_MT_COUNT,
+	},
 };
 
 static struct attribute *mma8452_attributes[] = {
@@ -946,6 +1018,8 @@ static int mma8452_reset(struct i2c_client *client)
 static const struct of_device_id mma8452_dt_ids[] = {
 	{ .compatible = "fsl,mma8452", .data = &mma_chip_info_table[mma8452] },
 	{ .compatible = "fsl,mma8453", .data = &mma_chip_info_table[mma8453] },
+	{ .compatible = "fsl,mma8652", .data = &mma_chip_info_table[mma8652] },
+	{ .compatible = "fsl,mma8653", .data = &mma_chip_info_table[mma8653] },
 	{ }
 };
 
@@ -957,13 +1031,6 @@ static int mma8452_probe(struct i2c_client *client,
 	int ret;
 	const struct of_device_id *match;
 
-	ret = i2c_smbus_read_byte_data(client, MMA8452_WHO_AM_I);
-	if (ret < 0)
-		return ret;
-
-	if (ret != MMA8452_DEVICE_ID && ret != MMA8453_DEVICE_ID)
-		return -ENODEV;
-
 	match = of_match_device(mma8452_dt_ids, &client->dev);
 	if (!match) {
 		dev_err(&client->dev, "unknown device model\n");
@@ -979,6 +1046,21 @@ static int mma8452_probe(struct i2c_client *client,
 	mutex_init(&data->lock);
 	data->chip_info = match->data;
 
+	ret = i2c_smbus_read_byte_data(client, MMA8452_WHO_AM_I);
+	if (ret < 0)
+		return ret;
+
+	switch (ret) {
+	case MMA8452_DEVICE_ID:
+	case MMA8453_DEVICE_ID:
+	case MMA8652_DEVICE_ID:
+	case MMA8653_DEVICE_ID:
+		if (ret == data->chip_info->chip_id)
+			break;
+	default:
+		return -ENODEV;
+	}
+
 	dev_info(&client->dev, "registering %s accelerometer; ID 0x%x\n",
 		 match->compatible, data->chip_info->chip_id);
 
@@ -1111,6 +1193,8 @@ static SIMPLE_DEV_PM_OPS(mma8452_pm_ops, mma8452_suspend, mma8452_resume);
 static const struct i2c_device_id mma8452_id[] = {
 	{ "mma8452", mma8452 },
 	{ "mma8453", mma8453 },
+	{ "mma8652", mma8652 },
+	{ "mma8653", mma8653 },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, mma8452_id);
-- 
2.1.4

[PATCH 4/8] iio: mma8452: add support for MMA8652FC and MMA8653FC accelerometers

[Martin Kepplinger]

MMA8652FC and MMA8653FC don't provide the transient interrupt source, so
the motion interrupt source is used by providing a new iio_chan_spec
definition, so that other supported devices are not affected by this.

Datasheets for the newly supported devices are available at Freescale's
website:

http://cache.freescale.com/files/sensors/doc/data_sheet/MMA8652FC.pdf
http://cache.freescale.com/files/sensors/doc/data_sheet/MMA8653FC.pdf

Signed-off-by: Martin Kepplinger <martin.kepplinger-SN7IsUiht6C/RdPyistoZJqQE7yCjDx5@public.gmane.org>
Signed-off-by: Christoph Muellner <christoph.muellner-SN7IsUiht6C/RdPyistoZJqQE7yCjDx5@public.gmane.org>
---
 drivers/iio/accel/Kconfig   |  2 +-
 drivers/iio/accel/mma8452.c | 98 +++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 92 insertions(+), 8 deletions(-)

diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
index dc80dc9..fdb59e0 100644
--- a/drivers/iio/accel/Kconfig
+++ b/drivers/iio/accel/Kconfig
@@ -93,7 +93,7 @@ config MMA8452
 	select IIO_TRIGGERED_BUFFER
 	help
 	  Say yes here to build support for the following Freescale 3-axis
-	  accelerometers: MMA8452Q, MMA8453Q.
+	  accelerometers: MMA8452Q, MMA8453Q, MMA8652FC, MMA8653FC.
 
 	  To compile this driver as a module, choose M here: the module
 	  will be called mma8452.
diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index d51ce63..5a8c738 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -3,6 +3,8 @@
  *
  *	MMA8452Q
  *	MMA8453Q
+ *	MMA8652FC
+ *	MMA8653FC
  *
  * Copyright 2014 Peter Meerwald <pmeerw-jW+XmwGofnusTnJN9+BGXg@public.gmane.org>
  *
@@ -87,6 +89,8 @@
 
 #define MMA8452_DEVICE_ID 0x2a
 #define MMA8453_DEVICE_ID 0x3a
+#define MMA8652_DEVICE_ID 0x4a
+#define MMA8653_DEVICE_ID 0x5a
 
 struct mma8452_data {
 	struct i2c_client *client;
@@ -766,6 +770,26 @@ static struct attribute_group mma8452_event_attribute_group = {
 	.num_event_specs = ARRAY_SIZE(mma8452_transient_event), \
 }
 
+#define MMA8652_CHANNEL(axis, idx, bits) { \
+	.type = IIO_ACCEL, \
+	.modified = 1, \
+	.channel2 = IIO_MOD_##axis, \
+	.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), \
+	.scan_index = idx, \
+	.scan_type = { \
+		.sign = 's', \
+		.realbits = (bits), \
+		.storagebits = 16, \
+		.shift = 16 - (bits), \
+		.endianness = IIO_BE, \
+	}, \
+	.event_spec = mma8452_motion_event, \
+	.num_event_specs = ARRAY_SIZE(mma8452_motion_event), \
+}
+
 static const struct iio_chan_spec mma8452_channels[] = {
 	MMA8452_CHANNEL(X, 0, 12),
 	MMA8452_CHANNEL(Y, 1, 12),
@@ -780,9 +804,25 @@ static const struct iio_chan_spec mma8453_channels[] = {
 	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),
+	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),
+	IIO_CHAN_SOFT_TIMESTAMP(3),
+};
+
 enum {
 	mma8452,
 	mma8453,
+	mma8652,
+	mma8653,
 };
 
 static const struct mma_chip_info mma_chip_info_table[] = {
@@ -825,6 +865,38 @@ static const struct mma_chip_info mma_chip_info_table[] = {
 		.ev_ths_mask = MMA8452_TRANSIENT_THS_MASK,
 		.ev_count = MMA8452_TRANSIENT_COUNT,
 	},
+	[mma8652] = {
+		.chip_id = MMA8652_DEVICE_ID,
+		.channels = mma8652_channels,
+		.num_channels = ARRAY_SIZE(mma8652_channels),
+		.mma_scales = { {0, 9577}, {0, 19154}, {0, 38307} },
+		.ev_cfg = MMA8452_FF_MT_CFG,
+		.ev_cfg_ele = MMA8452_FF_MT_CFG_ELE,
+		.ev_cfg_chan_shift = 3,
+		.ev_src = MMA8452_FF_MT_SRC,
+		.ev_src_xe = MMA8452_FF_MT_SRC_XHE,
+		.ev_src_ye = MMA8452_FF_MT_SRC_YHE,
+		.ev_src_ze = MMA8452_FF_MT_SRC_ZHE,
+		.ev_ths = MMA8452_FF_MT_THS,
+		.ev_ths_mask = MMA8452_FF_MT_THS_MASK,
+		.ev_count = MMA8452_FF_MT_COUNT,
+	},
+	[mma8653] = {
+		.chip_id = MMA8653_DEVICE_ID,
+		.channels = mma8653_channels,
+		.num_channels = ARRAY_SIZE(mma8653_channels),
+		.mma_scales = { {0, 38307}, {0, 76614}, {0, 153228} },
+		.ev_cfg = MMA8452_FF_MT_CFG,
+		.ev_cfg_ele = MMA8452_FF_MT_CFG_ELE,
+		.ev_cfg_chan_shift = 3,
+		.ev_src = MMA8452_FF_MT_SRC,
+		.ev_src_xe = MMA8452_FF_MT_SRC_XHE,
+		.ev_src_ye = MMA8452_FF_MT_SRC_YHE,
+		.ev_src_ze = MMA8452_FF_MT_SRC_ZHE,
+		.ev_ths = MMA8452_FF_MT_THS,
+		.ev_ths_mask = MMA8452_FF_MT_THS_MASK,
+		.ev_count = MMA8452_FF_MT_COUNT,
+	},
 };
 
 static struct attribute *mma8452_attributes[] = {
@@ -946,6 +1018,8 @@ static int mma8452_reset(struct i2c_client *client)
 static const struct of_device_id mma8452_dt_ids[] = {
 	{ .compatible = "fsl,mma8452", .data = &mma_chip_info_table[mma8452] },
 	{ .compatible = "fsl,mma8453", .data = &mma_chip_info_table[mma8453] },
+	{ .compatible = "fsl,mma8652", .data = &mma_chip_info_table[mma8652] },
+	{ .compatible = "fsl,mma8653", .data = &mma_chip_info_table[mma8653] },
 	{ }
 };
 
@@ -957,13 +1031,6 @@ static int mma8452_probe(struct i2c_client *client,
 	int ret;
 	const struct of_device_id *match;
 
-	ret = i2c_smbus_read_byte_data(client, MMA8452_WHO_AM_I);
-	if (ret < 0)
-		return ret;
-
-	if (ret != MMA8452_DEVICE_ID && ret != MMA8453_DEVICE_ID)
-		return -ENODEV;
-
 	match = of_match_device(mma8452_dt_ids, &client->dev);
 	if (!match) {
 		dev_err(&client->dev, "unknown device model\n");
@@ -979,6 +1046,21 @@ static int mma8452_probe(struct i2c_client *client,
 	mutex_init(&data->lock);
 	data->chip_info = match->data;
 
+	ret = i2c_smbus_read_byte_data(client, MMA8452_WHO_AM_I);
+	if (ret < 0)
+		return ret;
+
+	switch (ret) {
+	case MMA8452_DEVICE_ID:
+	case MMA8453_DEVICE_ID:
+	case MMA8652_DEVICE_ID:
+	case MMA8653_DEVICE_ID:
+		if (ret == data->chip_info->chip_id)
+			break;
+	default:
+		return -ENODEV;
+	}
+
 	dev_info(&client->dev, "registering %s accelerometer; ID 0x%x\n",
 		 match->compatible, data->chip_info->chip_id);
 
@@ -1111,6 +1193,8 @@ static SIMPLE_DEV_PM_OPS(mma8452_pm_ops, mma8452_suspend, mma8452_resume);
 static const struct i2c_device_id mma8452_id[] = {
 	{ "mma8452", mma8452 },
 	{ "mma8453", mma8453 },
+	{ "mma8652", mma8652 },
+	{ "mma8653", mma8653 },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, mma8452_id);
-- 
2.1.4

[PATCH 5/8] iio: mma8452: add devicetree binding document

[Martin Kepplinger]

As we support more chips now, add a binding document and remove it from
i2c trivial-devices.txt list.

The binding document is further extended in a later patch of this series.

Signed-off-by: Martin Kepplinger <martin.kepplinger@theobroma-systems.com>
Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com>
---
 .../devicetree/bindings/i2c/trivial-devices.txt     |  1 -
 .../devicetree/bindings/iio/accel/mma8452.txt       | 21 +++++++++++++++++++++
 2 files changed, 21 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/iio/accel/mma8452.txt

diff --git a/Documentation/devicetree/bindings/i2c/trivial-devices.txt b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
index 00f8652..094238a 100644
--- a/Documentation/devicetree/bindings/i2c/trivial-devices.txt
+++ b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
@@ -54,7 +54,6 @@ epson,rx8581		I2C-BUS INTERFACE REAL TIME CLOCK MODULE
 fsl,mag3110		MAG3110: Xtrinsic High Accuracy, 3D Magnetometer
 fsl,mc13892		MC13892: Power Management Integrated Circuit (PMIC) for i.MX35/51
 fsl,mma8450		MMA8450Q: Xtrinsic Low-power, 3-axis Xtrinsic Accelerometer
-fsl,mma8452		MMA8452Q: 3-axis 12-bit / 8-bit Digital Accelerometer
 fsl,mpr121		MPR121: Proximity Capacitive Touch Sensor Controller
 fsl,sgtl5000		SGTL5000: Ultra Low-Power Audio Codec
 gmt,g751		G751: Digital Temperature Sensor and Thermal Watchdog with Two-Wire Interface
diff --git a/Documentation/devicetree/bindings/iio/accel/mma8452.txt b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
new file mode 100644
index 0000000..8d98e05
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
@@ -0,0 +1,21 @@
+Freescale MMA8452Q, MMA8453Q, MMA8652FC or MMA8653FC triaxial accelerometer
+
+Required properties:
+
+  - compatible: should be "fsl,mma8653", "fsl,mma8652", "fsl,mma8453" or
+    "fsl,mma8452" respectively.
+  - reg: the I2C address of the chip
+
+Optional properties:
+
+  - interrupt-parent: should be the phandle for the interrupt controller
+  - interrupts: interrupt mapping for GPIO IRQ
+
+Example:
+
+	mma8653fc@1d {
+		compatible = "fsl,mma8653";
+		reg = <0x1d>;
+		interrupt-parent = <&gpio1>;
+		interrupts = <5 0>;
+	};
-- 
2.1.4

Re: [PATCH 5/8] iio: mma8452: add devicetree binding document

[Mark Rutland]

On Mon, Jul 27, 2015 at 03:08:12PM +0100, Martin Kepplinger wrote:
> As we support more chips now, add a binding document and remove it from
> i2c trivial-devices.txt list.
> 
> The binding document is further extended in a later patch of this series.
> 
> Signed-off-by: Martin Kepplinger <martin.kepplinger@theobroma-systems.com>
> Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com>
> ---
>  .../devicetree/bindings/i2c/trivial-devices.txt     |  1 -
>  .../devicetree/bindings/iio/accel/mma8452.txt       | 21 +++++++++++++++++++++
>  2 files changed, 21 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/devicetree/bindings/iio/accel/mma8452.txt
> 
> diff --git a/Documentation/devicetree/bindings/i2c/trivial-devices.txt b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
> index 00f8652..094238a 100644
> --- a/Documentation/devicetree/bindings/i2c/trivial-devices.txt
> +++ b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
> @@ -54,7 +54,6 @@ epson,rx8581		I2C-BUS INTERFACE REAL TIME CLOCK MODULE
>  fsl,mag3110		MAG3110: Xtrinsic High Accuracy, 3D Magnetometer
>  fsl,mc13892		MC13892: Power Management Integrated Circuit (PMIC) for i.MX35/51
>  fsl,mma8450		MMA8450Q: Xtrinsic Low-power, 3-axis Xtrinsic Accelerometer
> -fsl,mma8452		MMA8452Q: 3-axis 12-bit / 8-bit Digital Accelerometer
>  fsl,mpr121		MPR121: Proximity Capacitive Touch Sensor Controller
>  fsl,sgtl5000		SGTL5000: Ultra Low-Power Audio Codec
>  gmt,g751		G751: Digital Temperature Sensor and Thermal Watchdog with Two-Wire Interface
> diff --git a/Documentation/devicetree/bindings/iio/accel/mma8452.txt b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
> new file mode 100644
> index 0000000..8d98e05
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
> @@ -0,0 +1,21 @@
> +Freescale MMA8452Q, MMA8453Q, MMA8652FC or MMA8653FC triaxial accelerometer
> +
> +Required properties:
> +
> +  - compatible: should be "fsl,mma8653", "fsl,mma8652", "fsl,mma8453" or
> +    "fsl,mma8452" respectively.


This would be easier to read/search (and easier to extend in future) if
formatted as a list, e.g.

 - compatible: should contain one of:
   * "fsl,mma8653"
   * "fsl,mma8652"
   * "fsl,mma8453"
   * "fsl,mma8452"

Mark.

> +  - reg: the I2C address of the chip
> +
> +Optional properties:
> +
> +  - interrupt-parent: should be the phandle for the interrupt controller
> +  - interrupts: interrupt mapping for GPIO IRQ
> +
> +Example:
> +
> +	mma8653fc@1d {
> +		compatible = "fsl,mma8653";
> +		reg = <0x1d>;
> +		interrupt-parent = <&gpio1>;
> +		interrupts = <5 0>;
> +	};
> -- 
> 2.1.4
> 

Re: [PATCH 5/8] iio: mma8452: add devicetree binding document

[Mark Rutland]

On Mon, Jul 27, 2015 at 03:08:12PM +0100, Martin Kepplinger wrote:
> As we support more chips now, add a binding document and remove it from
> i2c trivial-devices.txt list.
> 
> The binding document is further extended in a later patch of this series.
> 
> Signed-off-by: Martin Kepplinger <martin.kepplinger@theobroma-systems.com>
> Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com>
> ---
>  .../devicetree/bindings/i2c/trivial-devices.txt     |  1 -
>  .../devicetree/bindings/iio/accel/mma8452.txt       | 21 +++++++++++++++++++++
>  2 files changed, 21 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/devicetree/bindings/iio/accel/mma8452.txt
> 
> diff --git a/Documentation/devicetree/bindings/i2c/trivial-devices.txt b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
> index 00f8652..094238a 100644
> --- a/Documentation/devicetree/bindings/i2c/trivial-devices.txt
> +++ b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
> @@ -54,7 +54,6 @@ epson,rx8581		I2C-BUS INTERFACE REAL TIME CLOCK MODULE
>  fsl,mag3110		MAG3110: Xtrinsic High Accuracy, 3D Magnetometer
>  fsl,mc13892		MC13892: Power Management Integrated Circuit (PMIC) for i.MX35/51
>  fsl,mma8450		MMA8450Q: Xtrinsic Low-power, 3-axis Xtrinsic Accelerometer
> -fsl,mma8452		MMA8452Q: 3-axis 12-bit / 8-bit Digital Accelerometer
>  fsl,mpr121		MPR121: Proximity Capacitive Touch Sensor Controller
>  fsl,sgtl5000		SGTL5000: Ultra Low-Power Audio Codec
>  gmt,g751		G751: Digital Temperature Sensor and Thermal Watchdog with Two-Wire Interface
> diff --git a/Documentation/devicetree/bindings/iio/accel/mma8452.txt b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
> new file mode 100644
> index 0000000..8d98e05
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
> @@ -0,0 +1,21 @@
> +Freescale MMA8452Q, MMA8453Q, MMA8652FC or MMA8653FC triaxial accelerometer
> +
> +Required properties:
> +
> +  - compatible: should be "fsl,mma8653", "fsl,mma8652", "fsl,mma8453" or
> +    "fsl,mma8452" respectively.


This would be easier to read/search (and easier to extend in future) if
formatted as a list, e.g.

 - compatible: should contain one of:
   * "fsl,mma8653"
   * "fsl,mma8652"
   * "fsl,mma8453"
   * "fsl,mma8452"

Mark.

> +  - reg: the I2C address of the chip
> +
> +Optional properties:
> +
> +  - interrupt-parent: should be the phandle for the interrupt controller
> +  - interrupts: interrupt mapping for GPIO IRQ
> +
> +Example:
> +
> +	mma8653fc@1d {
> +		compatible = "fsl,mma8653";
> +		reg = <0x1d>;
> +		interrupt-parent = <&gpio1>;
> +		interrupts = <5 0>;
> +	};
> -- 
> 2.1.4
> 

[PATCHv2 5/8] iio: mma8452: add devicetree binding document

[Martin Kepplinger]

As we support more chips now, add a binding document and remove it from
i2c trivial-devices.txt list.

The binding document is further extended in a later patch of this series.

Signed-off-by: Martin Kepplinger <martin.kepplinger@theobroma-systems.com>
Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com>
---
builds a nice list of supported compatible strings.

thanks!


 .../devicetree/bindings/i2c/trivial-devices.txt    |  1 -
 .../devicetree/bindings/iio/accel/mma8452.txt      | 24 ++++++++++++++++++++++
 2 files changed, 24 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/iio/accel/mma8452.txt

diff --git a/Documentation/devicetree/bindings/i2c/trivial-devices.txt b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
index 00f8652..094238a 100644
--- a/Documentation/devicetree/bindings/i2c/trivial-devices.txt
+++ b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
@@ -54,7 +54,6 @@ epson,rx8581		I2C-BUS INTERFACE REAL TIME CLOCK MODULE
 fsl,mag3110		MAG3110: Xtrinsic High Accuracy, 3D Magnetometer
 fsl,mc13892		MC13892: Power Management Integrated Circuit (PMIC) for i.MX35/51
 fsl,mma8450		MMA8450Q: Xtrinsic Low-power, 3-axis Xtrinsic Accelerometer
-fsl,mma8452		MMA8452Q: 3-axis 12-bit / 8-bit Digital Accelerometer
 fsl,mpr121		MPR121: Proximity Capacitive Touch Sensor Controller
 fsl,sgtl5000		SGTL5000: Ultra Low-Power Audio Codec
 gmt,g751		G751: Digital Temperature Sensor and Thermal Watchdog with Two-Wire Interface
diff --git a/Documentation/devicetree/bindings/iio/accel/mma8452.txt b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
new file mode 100644
index 0000000..57feb16
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
@@ -0,0 +1,24 @@
+Freescale MMA8452Q, MMA8453Q, MMA8652FC or MMA8653FC triaxial accelerometer
+
+Required properties:
+
+  - compatible: should contain one of
+    * "fsl,mma8653"
+    * "fsl,mma8652"
+    * "fsl,mma8453"
+    * "fsl,mma8452"
+  - reg: the I2C address of the chip
+
+Optional properties:
+
+  - interrupt-parent: should be the phandle for the interrupt controller
+  - interrupts: interrupt mapping for GPIO IRQ
+
+Example:
+
+	mma8653fc@1d {
+		compatible = "fsl,mma8653";
+		reg = <0x1d>;
+		interrupt-parent = <&gpio1>;
+		interrupts = <5 0>;
+	};
-- 
2.1.4

[PATCHv2 5/8] iio: mma8452: add devicetree binding document

[Martin Kepplinger]

As we support more chips now, add a binding document and remove it from
i2c trivial-devices.txt list.

The binding document is further extended in a later patch of this series.

Signed-off-by: Martin Kepplinger <martin.kepplinger-SN7IsUiht6C/RdPyistoZJqQE7yCjDx5@public.gmane.org>
Signed-off-by: Christoph Muellner <christoph.muellner-SN7IsUiht6C/RdPyistoZJqQE7yCjDx5@public.gmane.org>
---
builds a nice list of supported compatible strings.

thanks!


 .../devicetree/bindings/i2c/trivial-devices.txt    |  1 -
 .../devicetree/bindings/iio/accel/mma8452.txt      | 24 ++++++++++++++++++++++
 2 files changed, 24 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/iio/accel/mma8452.txt

diff --git a/Documentation/devicetree/bindings/i2c/trivial-devices.txt b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
index 00f8652..094238a 100644
--- a/Documentation/devicetree/bindings/i2c/trivial-devices.txt
+++ b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
@@ -54,7 +54,6 @@ epson,rx8581		I2C-BUS INTERFACE REAL TIME CLOCK MODULE
 fsl,mag3110		MAG3110: Xtrinsic High Accuracy, 3D Magnetometer
 fsl,mc13892		MC13892: Power Management Integrated Circuit (PMIC) for i.MX35/51
 fsl,mma8450		MMA8450Q: Xtrinsic Low-power, 3-axis Xtrinsic Accelerometer
-fsl,mma8452		MMA8452Q: 3-axis 12-bit / 8-bit Digital Accelerometer
 fsl,mpr121		MPR121: Proximity Capacitive Touch Sensor Controller
 fsl,sgtl5000		SGTL5000: Ultra Low-Power Audio Codec
 gmt,g751		G751: Digital Temperature Sensor and Thermal Watchdog with Two-Wire Interface
diff --git a/Documentation/devicetree/bindings/iio/accel/mma8452.txt b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
new file mode 100644
index 0000000..57feb16
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
@@ -0,0 +1,24 @@
+Freescale MMA8452Q, MMA8453Q, MMA8652FC or MMA8653FC triaxial accelerometer
+
+Required properties:
+
+  - compatible: should contain one of
+    * "fsl,mma8653"
+    * "fsl,mma8652"
+    * "fsl,mma8453"
+    * "fsl,mma8452"
+  - reg: the I2C address of the chip
+
+Optional properties:
+
+  - interrupt-parent: should be the phandle for the interrupt controller
+  - interrupts: interrupt mapping for GPIO IRQ
+
+Example:
+
+	mma8653fc@1d {
+		compatible = "fsl,mma8653";
+		reg = <0x1d>;
+		interrupt-parent = <&gpio1>;
+		interrupts = <5 0>;
+	};
-- 
2.1.4

[PATCH 6/8] iio: mma8452: add copyright notice comment

[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 insertion(+)

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 5a8c738..dc66815 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -6,6 +6,7 @@
  *	MMA8652FC
  *	MMA8653FC
  *
+ * Copyright 2015 Martin Kepplinger <martin.kepplinger@theobroma-systems.com>
  * Copyright 2014 Peter Meerwald <pmeerw@pmeerw.net>
  *
  * This file is subject to the terms and conditions of version 2 of
-- 
2.1.4

[PATCH 7/8] iio: mma8452: leave sysfs namings to the iio core

[Martin Kepplinger]

This doesn't actually change anything since the core names the sysfs folder
for the iio event attributes "events" anyways. It only leaves the job to the
core.

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 dc66815..918ab59 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -747,7 +747,6 @@ static struct attribute *mma8452_event_attributes[] = {
 
 static struct attribute_group mma8452_event_attribute_group = {
 	.attrs = mma8452_event_attributes,
-	.name = "events",
 };
 
 #define MMA8452_CHANNEL(axis, idx, bits) { \
-- 
2.1.4

[PATCH 8/8] iio: mma8452: add devicetree property to allow all pin wirings

[Martin Kepplinger]

For the devices supported by the mma8452 driver, two interrupt pins are
available to route the interrupt signals to. By default INT1 is assumed.

This adds a bitmask DT property for users to configure interrupt sources
for INT2, if that is the wired interrupt pin for them.

This is important for everyone to be able to use this driver, no matter
how their chip is wired. At the moment, only 0xff for using INT2 for all
available interrupt sources is supported. See the devicetree documentation
file for more details.

Since this doesn't change the default behaviour, it doesn't break anything
for existing users.

Signed-off-by: Martin Kepplinger <martin.kepplinger@theobroma-systems.com>
Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com>
---
 .../devicetree/bindings/iio/accel/mma8452.txt        |  4 ++++
 drivers/iio/accel/mma8452.c                          | 20 +++++++++++++-------
 2 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/accel/mma8452.txt b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
index 8d98e05..738a430 100644
--- a/Documentation/devicetree/bindings/iio/accel/mma8452.txt
+++ b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
@@ -10,6 +10,9 @@ Optional properties:
 
   - interrupt-parent: should be the phandle for the interrupt controller
   - interrupts: interrupt mapping for GPIO IRQ
+  - use_int2: bitmask to choose interrupt sources assumed to be wired to
+    interrupt pin INT2 instead of INT1. Only 0xff (INT2 for every interrupt
+    source) is supported at the moment.
 
 Example:
 
@@ -18,4 +21,5 @@ Example:
 		reg = <0x1d>;
 		interrupt-parent = <&gpio1>;
 		interrupts = <5 0>;
+		use_int2 = /bits/ 8 <0xff>;
 	};
diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 918ab59..a03836b1 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -1028,8 +1028,9 @@ static int mma8452_probe(struct i2c_client *client,
 {
 	struct mma8452_data *data;
 	struct iio_dev *indio_dev;
-	int ret;
 	const struct of_device_id *match;
+	int ret;
+	u8 int2;
 
 	match = of_match_device(mma8452_dt_ids, &client->dev);
 	if (!match) {
@@ -1104,12 +1105,17 @@ static int mma8452_probe(struct i2c_client *client,
 		int enabled_interrupts = MMA8452_INT_TRANS |
 					 MMA8452_INT_FF_MT;
 
-		/* Assume wired to INT1 pin */
-		ret = i2c_smbus_write_byte_data(client,
-						MMA8452_CTRL_REG5,
-						supported_interrupts);
-		if (ret < 0)
-			return ret;
+		of_property_read_u8(client->dev.of_node, "use_int2", &int2);
+		if (int2 == 0xff) {
+			dev_dbg(&client->dev, "use interrupt line INT2\n");
+		} else {
+			dev_dbg(&client->dev, "use interrupt line INT1\n");
+			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,
-- 
2.1.4

Re: [PATCH 8/8] iio: mma8452: add devicetree property to allow all pin wirings

[Mark Rutland]

On Mon, Jul 27, 2015 at 03:08:15PM +0100, Martin Kepplinger wrote:
> For the devices supported by the mma8452 driver, two interrupt pins are
> available to route the interrupt signals to. By default INT1 is assumed.
> 
> This adds a bitmask DT property for users to configure interrupt sources
> for INT2, if that is the wired interrupt pin for them.

This sounds like configureation rather than a HW property. Why does this
need to be in the DT?

> This is important for everyone to be able to use this driver, no matter
> how their chip is wired. At the moment, only 0xff for using INT2 for all
> available interrupt sources is supported. See the devicetree documentation
> file for more details.
> 
> Since this doesn't change the default behaviour, it doesn't break anything
> for existing users.
> 
> Signed-off-by: Martin Kepplinger <martin.kepplinger@theobroma-systems.com>
> Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com>
> ---
>  .../devicetree/bindings/iio/accel/mma8452.txt        |  4 ++++
>  drivers/iio/accel/mma8452.c                          | 20 +++++++++++++-------
>  2 files changed, 17 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/iio/accel/mma8452.txt b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
> index 8d98e05..738a430 100644
> --- a/Documentation/devicetree/bindings/iio/accel/mma8452.txt
> +++ b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
> @@ -10,6 +10,9 @@ Optional properties:
>  
>    - interrupt-parent: should be the phandle for the interrupt controller
>    - interrupts: interrupt mapping for GPIO IRQ
> +  - use_int2: bitmask to choose interrupt sources assumed to be wired to
> +    interrupt pin INT2 instead of INT1. Only 0xff (INT2 for every interrupt
> +    source) is supported at the moment.

s/_/-/ in property names, please.

We generally avoid bitmasks in properties, and we also usually exepct a
full cell even if data is smaller. The fact that you expect /bits/ 8 
must be documented here if that's truly necessary.

Thanks,
Mark

>  
>  Example:
>  
> @@ -18,4 +21,5 @@ Example:
>  		reg = <0x1d>;
>  		interrupt-parent = <&gpio1>;
>  		interrupts = <5 0>;
> +		use_int2 = /bits/ 8 <0xff>;
>  	};
> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
> index 918ab59..a03836b1 100644
> --- a/drivers/iio/accel/mma8452.c
> +++ b/drivers/iio/accel/mma8452.c
> @@ -1028,8 +1028,9 @@ static int mma8452_probe(struct i2c_client *client,
>  {
>  	struct mma8452_data *data;
>  	struct iio_dev *indio_dev;
> -	int ret;
>  	const struct of_device_id *match;
> +	int ret;
> +	u8 int2;
>  
>  	match = of_match_device(mma8452_dt_ids, &client->dev);
>  	if (!match) {
> @@ -1104,12 +1105,17 @@ static int mma8452_probe(struct i2c_client *client,
>  		int enabled_interrupts = MMA8452_INT_TRANS |
>  					 MMA8452_INT_FF_MT;
>  
> -		/* Assume wired to INT1 pin */
> -		ret = i2c_smbus_write_byte_data(client,
> -						MMA8452_CTRL_REG5,
> -						supported_interrupts);
> -		if (ret < 0)
> -			return ret;
> +		of_property_read_u8(client->dev.of_node, "use_int2", &int2);
> +		if (int2 == 0xff) {
> +			dev_dbg(&client->dev, "use interrupt line INT2\n");
> +		} else {
> +			dev_dbg(&client->dev, "use interrupt line INT1\n");
> +			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,
> -- 
> 2.1.4
> 

Re: [PATCH 8/8] iio: mma8452: add devicetree property to allow all pin wirings

[Mark Rutland]

On Mon, Jul 27, 2015 at 03:08:15PM +0100, Martin Kepplinger wrote:
> For the devices supported by the mma8452 driver, two interrupt pins are
> available to route the interrupt signals to. By default INT1 is assumed.
> 
> This adds a bitmask DT property for users to configure interrupt sources
> for INT2, if that is the wired interrupt pin for them.

This sounds like configureation rather than a HW property. Why does this
need to be in the DT?

> This is important for everyone to be able to use this driver, no matter
> how their chip is wired. At the moment, only 0xff for using INT2 for all
> available interrupt sources is supported. See the devicetree documentation
> file for more details.
> 
> Since this doesn't change the default behaviour, it doesn't break anything
> for existing users.
> 
> Signed-off-by: Martin Kepplinger <martin.kepplinger@theobroma-systems.com>
> Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com>
> ---
>  .../devicetree/bindings/iio/accel/mma8452.txt        |  4 ++++
>  drivers/iio/accel/mma8452.c                          | 20 +++++++++++++-------
>  2 files changed, 17 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/iio/accel/mma8452.txt b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
> index 8d98e05..738a430 100644
> --- a/Documentation/devicetree/bindings/iio/accel/mma8452.txt
> +++ b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
> @@ -10,6 +10,9 @@ Optional properties:
>  
>    - interrupt-parent: should be the phandle for the interrupt controller
>    - interrupts: interrupt mapping for GPIO IRQ
> +  - use_int2: bitmask to choose interrupt sources assumed to be wired to
> +    interrupt pin INT2 instead of INT1. Only 0xff (INT2 for every interrupt
> +    source) is supported at the moment.

s/_/-/ in property names, please.

We generally avoid bitmasks in properties, and we also usually exepct a
full cell even if data is smaller. The fact that you expect /bits/ 8 
must be documented here if that's truly necessary.

Thanks,
Mark

>  
>  Example:
>  
> @@ -18,4 +21,5 @@ Example:
>  		reg = <0x1d>;
>  		interrupt-parent = <&gpio1>;
>  		interrupts = <5 0>;
> +		use_int2 = /bits/ 8 <0xff>;
>  	};
> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
> index 918ab59..a03836b1 100644
> --- a/drivers/iio/accel/mma8452.c
> +++ b/drivers/iio/accel/mma8452.c
> @@ -1028,8 +1028,9 @@ static int mma8452_probe(struct i2c_client *client,
>  {
>  	struct mma8452_data *data;
>  	struct iio_dev *indio_dev;
> -	int ret;
>  	const struct of_device_id *match;
> +	int ret;
> +	u8 int2;
>  
>  	match = of_match_device(mma8452_dt_ids, &client->dev);
>  	if (!match) {
> @@ -1104,12 +1105,17 @@ static int mma8452_probe(struct i2c_client *client,
>  		int enabled_interrupts = MMA8452_INT_TRANS |
>  					 MMA8452_INT_FF_MT;
>  
> -		/* Assume wired to INT1 pin */
> -		ret = i2c_smbus_write_byte_data(client,
> -						MMA8452_CTRL_REG5,
> -						supported_interrupts);
> -		if (ret < 0)
> -			return ret;
> +		of_property_read_u8(client->dev.of_node, "use_int2", &int2);
> +		if (int2 == 0xff) {
> +			dev_dbg(&client->dev, "use interrupt line INT2\n");
> +		} else {
> +			dev_dbg(&client->dev, "use interrupt line INT1\n");
> +			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,
> -- 
> 2.1.4
> 

Re: [PATCH 8/8] iio: mma8452: add devicetree property to allow all pin wirings

[Martin Kepplinger]

Am 2015-07-27 um 16:23 schrieb Mark Rutland:
> On Mon, Jul 27, 2015 at 03:08:15PM +0100, Martin Kepplinger wrote:
>> For the devices supported by the mma8452 driver, two interrupt pins are
>> available to route the interrupt signals to. By default INT1 is assumed.
>>
>> This adds a bitmask DT property for users to configure interrupt sources
>> for INT2, if that is the wired interrupt pin for them.
> 
> This sounds like configureation rather than a HW property. Why does this
> need to be in the DT?

It's a hardware property of the board that uses the device. There might
be boards that connect just one of them at random, which is the reason
for this DT property. There also might be exotic users who will want
to use both pins to route different interrupt sources to (not yet
supported, but no problem with such a bitmask).

> 
>> This is important for everyone to be able to use this driver, no matter
>> how their chip is wired. At the moment, only 0xff for using INT2 for all
>> available interrupt sources is supported. See the devicetree documentation
>> file for more details.
>>
>> Since this doesn't change the default behaviour, it doesn't break anything
>> for existing users.
>>
>> Signed-off-by: Martin Kepplinger <martin.kepplinger@theobroma-systems.com>
>> Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com>
>> ---
>>  .../devicetree/bindings/iio/accel/mma8452.txt        |  4 ++++
>>  drivers/iio/accel/mma8452.c                          | 20 +++++++++++++-------
>>  2 files changed, 17 insertions(+), 7 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/iio/accel/mma8452.txt b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
>> index 8d98e05..738a430 100644
>> --- a/Documentation/devicetree/bindings/iio/accel/mma8452.txt
>> +++ b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
>> @@ -10,6 +10,9 @@ Optional properties:
>>  
>>    - interrupt-parent: should be the phandle for the interrupt controller
>>    - interrupts: interrupt mapping for GPIO IRQ
>> +  - use_int2: bitmask to choose interrupt sources assumed to be wired to
>> +    interrupt pin INT2 instead of INT1. Only 0xff (INT2 for every interrupt
>> +    source) is supported at the moment.
> 
> s/_/-/ in property names, please.

ok. If I don't do a version 6 really soon, I'll reply with this patch
corrected here.

> 
> We generally avoid bitmasks in properties, and we also usually exepct a
> full cell even if data is smaller. The fact that you expect /bits/ 8 
> must be documented here if that's truly necessary.

It's not truly necessary. It's just a nice fit. There is one 8 bit
(device memory) register that basically could (in the future) be
exposed through this DT property.

For now it's just 0xff or nothing. We only don't want to create an
interface that could restrict us from implementing more in the future
without breaking anything.

> 
> Thanks,
> Mark
> 
>>  
>>  Example:
>>  
>> @@ -18,4 +21,5 @@ Example:
>>  		reg = <0x1d>;
>>  		interrupt-parent = <&gpio1>;
>>  		interrupts = <5 0>;
>> +		use_int2 = /bits/ 8 <0xff>;
>>  	};
>> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
>> index 918ab59..a03836b1 100644
>> --- a/drivers/iio/accel/mma8452.c
>> +++ b/drivers/iio/accel/mma8452.c
>> @@ -1028,8 +1028,9 @@ static int mma8452_probe(struct i2c_client *client,
>>  {
>>  	struct mma8452_data *data;
>>  	struct iio_dev *indio_dev;
>> -	int ret;
>>  	const struct of_device_id *match;
>> +	int ret;
>> +	u8 int2;
>>  
>>  	match = of_match_device(mma8452_dt_ids, &client->dev);
>>  	if (!match) {
>> @@ -1104,12 +1105,17 @@ static int mma8452_probe(struct i2c_client *client,
>>  		int enabled_interrupts = MMA8452_INT_TRANS |
>>  					 MMA8452_INT_FF_MT;
>>  
>> -		/* Assume wired to INT1 pin */
>> -		ret = i2c_smbus_write_byte_data(client,
>> -						MMA8452_CTRL_REG5,
>> -						supported_interrupts);
>> -		if (ret < 0)
>> -			return ret;
>> +		of_property_read_u8(client->dev.of_node, "use_int2", &int2);
>> +		if (int2 == 0xff) {
>> +			dev_dbg(&client->dev, "use interrupt line INT2\n");
>> +		} else {
>> +			dev_dbg(&client->dev, "use interrupt line INT1\n");
>> +			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,
>> -- 
>> 2.1.4
>>

Re: [PATCH 8/8] iio: mma8452: add devicetree property to allow all pin wirings

[Martin Kepplinger]

Am 2015-07-27 um 16:23 schrieb Mark Rutland:
> On Mon, Jul 27, 2015 at 03:08:15PM +0100, Martin Kepplinger wrote:
>> For the devices supported by the mma8452 driver, two interrupt pins are
>> available to route the interrupt signals to. By default INT1 is assumed.
>>
>> This adds a bitmask DT property for users to configure interrupt sources
>> for INT2, if that is the wired interrupt pin for them.
> 
> This sounds like configureation rather than a HW property. Why does this
> need to be in the DT?

It's a hardware property of the board that uses the device. There might
be boards that connect just one of them at random, which is the reason
for this DT property. There also might be exotic users who will want
to use both pins to route different interrupt sources to (not yet
supported, but no problem with such a bitmask).

> 
>> This is important for everyone to be able to use this driver, no matter
>> how their chip is wired. At the moment, only 0xff for using INT2 for all
>> available interrupt sources is supported. See the devicetree documentation
>> file for more details.
>>
>> Since this doesn't change the default behaviour, it doesn't break anything
>> for existing users.
>>
>> Signed-off-by: Martin Kepplinger <martin.kepplinger@theobroma-systems.com>
>> Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com>
>> ---
>>  .../devicetree/bindings/iio/accel/mma8452.txt        |  4 ++++
>>  drivers/iio/accel/mma8452.c                          | 20 +++++++++++++-------
>>  2 files changed, 17 insertions(+), 7 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/iio/accel/mma8452.txt b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
>> index 8d98e05..738a430 100644
>> --- a/Documentation/devicetree/bindings/iio/accel/mma8452.txt
>> +++ b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
>> @@ -10,6 +10,9 @@ Optional properties:
>>  
>>    - interrupt-parent: should be the phandle for the interrupt controller
>>    - interrupts: interrupt mapping for GPIO IRQ
>> +  - use_int2: bitmask to choose interrupt sources assumed to be wired to
>> +    interrupt pin INT2 instead of INT1. Only 0xff (INT2 for every interrupt
>> +    source) is supported at the moment.
> 
> s/_/-/ in property names, please.

ok. If I don't do a version 6 really soon, I'll reply with this patch
corrected here.

> 
> We generally avoid bitmasks in properties, and we also usually exepct a
> full cell even if data is smaller. The fact that you expect /bits/ 8 
> must be documented here if that's truly necessary.

It's not truly necessary. It's just a nice fit. There is one 8 bit
(device memory) register that basically could (in the future) be
exposed through this DT property.

For now it's just 0xff or nothing. We only don't want to create an
interface that could restrict us from implementing more in the future
without breaking anything.

> 
> Thanks,
> Mark
> 
>>  
>>  Example:
>>  
>> @@ -18,4 +21,5 @@ Example:
>>  		reg = <0x1d>;
>>  		interrupt-parent = <&gpio1>;
>>  		interrupts = <5 0>;
>> +		use_int2 = /bits/ 8 <0xff>;
>>  	};
>> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
>> index 918ab59..a03836b1 100644
>> --- a/drivers/iio/accel/mma8452.c
>> +++ b/drivers/iio/accel/mma8452.c
>> @@ -1028,8 +1028,9 @@ static int mma8452_probe(struct i2c_client *client,
>>  {
>>  	struct mma8452_data *data;
>>  	struct iio_dev *indio_dev;
>> -	int ret;
>>  	const struct of_device_id *match;
>> +	int ret;
>> +	u8 int2;
>>  
>>  	match = of_match_device(mma8452_dt_ids, &client->dev);
>>  	if (!match) {
>> @@ -1104,12 +1105,17 @@ static int mma8452_probe(struct i2c_client *client,
>>  		int enabled_interrupts = MMA8452_INT_TRANS |
>>  					 MMA8452_INT_FF_MT;
>>  
>> -		/* Assume wired to INT1 pin */
>> -		ret = i2c_smbus_write_byte_data(client,
>> -						MMA8452_CTRL_REG5,
>> -						supported_interrupts);
>> -		if (ret < 0)
>> -			return ret;
>> +		of_property_read_u8(client->dev.of_node, "use_int2", &int2);
>> +		if (int2 == 0xff) {
>> +			dev_dbg(&client->dev, "use interrupt line INT2\n");
>> +		} else {
>> +			dev_dbg(&client->dev, "use interrupt line INT1\n");
>> +			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,
>> -- 
>> 2.1.4
>>

Re: [PATCH 8/8] iio: mma8452: add devicetree property to allow all pin wirings

[Martin Kepplinger]

Am 2015-07-27 um 16:23 schrieb Mark Rutland:
> On Mon, Jul 27, 2015 at 03:08:15PM +0100, Martin Kepplinger wrote:
>> For the devices supported by the mma8452 driver, two interrupt pins are
>> available to route the interrupt signals to. By default INT1 is assumed.
>>
>> This adds a bitmask DT property for users to configure interrupt sources
>> for INT2, if that is the wired interrupt pin for them.
> 
> This sounds like configureation rather than a HW property. Why does this
> need to be in the DT?

It's a hardware property of the board that uses the device. There might
be boards that connect just one of them at random, which is the reason
for this DT property. There also might be exotic users who will want
to use both pins to route different interrupt sources to (not yet
supported, but no problem with such a bitmask).

> 
>> This is important for everyone to be able to use this driver, no matter
>> how their chip is wired. At the moment, only 0xff for using INT2 for all
>> available interrupt sources is supported. See the devicetree documentation
>> file for more details.
>>
>> Since this doesn't change the default behaviour, it doesn't break anything
>> for existing users.
>>
>> Signed-off-by: Martin Kepplinger <martin.kepplinger-SN7IsUiht6C/RdPyistoZJqQE7yCjDx5@public.gmane.org>
>> Signed-off-by: Christoph Muellner <christoph.muellner-SN7IsUiht6C/RdPyistoZJqQE7yCjDx5@public.gmane.org>
>> ---
>>  .../devicetree/bindings/iio/accel/mma8452.txt        |  4 ++++
>>  drivers/iio/accel/mma8452.c                          | 20 +++++++++++++-------
>>  2 files changed, 17 insertions(+), 7 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/iio/accel/mma8452.txt b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
>> index 8d98e05..738a430 100644
>> --- a/Documentation/devicetree/bindings/iio/accel/mma8452.txt
>> +++ b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
>> @@ -10,6 +10,9 @@ Optional properties:
>>  
>>    - interrupt-parent: should be the phandle for the interrupt controller
>>    - interrupts: interrupt mapping for GPIO IRQ
>> +  - use_int2: bitmask to choose interrupt sources assumed to be wired to
>> +    interrupt pin INT2 instead of INT1. Only 0xff (INT2 for every interrupt
>> +    source) is supported at the moment.
> 
> s/_/-/ in property names, please.

ok. If I don't do a version 6 really soon, I'll reply with this patch
corrected here.

> 
> We generally avoid bitmasks in properties, and we also usually exepct a
> full cell even if data is smaller. The fact that you expect /bits/ 8 
> must be documented here if that's truly necessary.

It's not truly necessary. It's just a nice fit. There is one 8 bit
(device memory) register that basically could (in the future) be
exposed through this DT property.

For now it's just 0xff or nothing. We only don't want to create an
interface that could restrict us from implementing more in the future
without breaking anything.

> 
> Thanks,
> Mark
> 
>>  
>>  Example:
>>  
>> @@ -18,4 +21,5 @@ Example:
>>  		reg = <0x1d>;
>>  		interrupt-parent = <&gpio1>;
>>  		interrupts = <5 0>;
>> +		use_int2 = /bits/ 8 <0xff>;
>>  	};
>> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
>> index 918ab59..a03836b1 100644
>> --- a/drivers/iio/accel/mma8452.c
>> +++ b/drivers/iio/accel/mma8452.c
>> @@ -1028,8 +1028,9 @@ static int mma8452_probe(struct i2c_client *client,
>>  {
>>  	struct mma8452_data *data;
>>  	struct iio_dev *indio_dev;
>> -	int ret;
>>  	const struct of_device_id *match;
>> +	int ret;
>> +	u8 int2;
>>  
>>  	match = of_match_device(mma8452_dt_ids, &client->dev);
>>  	if (!match) {
>> @@ -1104,12 +1105,17 @@ static int mma8452_probe(struct i2c_client *client,
>>  		int enabled_interrupts = MMA8452_INT_TRANS |
>>  					 MMA8452_INT_FF_MT;
>>  
>> -		/* Assume wired to INT1 pin */
>> -		ret = i2c_smbus_write_byte_data(client,
>> -						MMA8452_CTRL_REG5,
>> -						supported_interrupts);
>> -		if (ret < 0)
>> -			return ret;
>> +		of_property_read_u8(client->dev.of_node, "use_int2", &int2);
>> +		if (int2 == 0xff) {
>> +			dev_dbg(&client->dev, "use interrupt line INT2\n");
>> +		} else {
>> +			dev_dbg(&client->dev, "use interrupt line INT1\n");
>> +			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,
>> -- 
>> 2.1.4
>>

Re: [PATCH 8/8] iio: mma8452: add devicetree property to allow all pin wirings

[Mark Rutland]

On Mon, Jul 27, 2015 at 03:37:48PM +0100, Martin Kepplinger wrote:
> Am 2015-07-27 um 16:23 schrieb Mark Rutland:
> > On Mon, Jul 27, 2015 at 03:08:15PM +0100, Martin Kepplinger wrote:
> >> For the devices supported by the mma8452 driver, two interrupt pins are
> >> available to route the interrupt signals to. By default INT1 is assumed.
> >>
> >> This adds a bitmask DT property for users to configure interrupt sources
> >> for INT2, if that is the wired interrupt pin for them.
> > 
> > This sounds like configureation rather than a HW property. Why does this
> > need to be in the DT?
> 
> It's a hardware property of the board that uses the device. There might
> be boards that connect just one of them at random, which is the reason
> for this DT property. There also might be exotic users who will want
> to use both pins to route different interrupt sources to (not yet
> supported, but no problem with such a bitmask).

Ok, so I'm somewhat confused as to what the hardware looks like and what
this means.

Could you elaborate on how INT1 and INT2 are used? It looks like they're
used as output pins, and so interrupt-names would seem appropriate for
describing the combination which is wired up.

w.r.t. configuring the choice of output(s), that sounds like a runtime
decision rather than something which needs to be configured statically.

> >> This is important for everyone to be able to use this driver, no matter
> >> how their chip is wired. At the moment, only 0xff for using INT2 for all
> >> available interrupt sources is supported. See the devicetree documentation
> >> file for more details.
> >>
> >> Since this doesn't change the default behaviour, it doesn't break anything
> >> for existing users.
> >>
> >> Signed-off-by: Martin Kepplinger <martin.kepplinger@theobroma-systems.com>
> >> Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com>
> >> ---
> >>  .../devicetree/bindings/iio/accel/mma8452.txt        |  4 ++++
> >>  drivers/iio/accel/mma8452.c                          | 20 +++++++++++++-------
> >>  2 files changed, 17 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/iio/accel/mma8452.txt b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
> >> index 8d98e05..738a430 100644
> >> --- a/Documentation/devicetree/bindings/iio/accel/mma8452.txt
> >> +++ b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
> >> @@ -10,6 +10,9 @@ Optional properties:
> >>  
> >>    - interrupt-parent: should be the phandle for the interrupt controller
> >>    - interrupts: interrupt mapping for GPIO IRQ
> >> +  - use_int2: bitmask to choose interrupt sources assumed to be wired to
> >> +    interrupt pin INT2 instead of INT1. Only 0xff (INT2 for every interrupt
> >> +    source) is supported at the moment.
> > 
> > s/_/-/ in property names, please.
> 
> ok. If I don't do a version 6 really soon, I'll reply with this patch
> corrected here.
> 
> > 
> > We generally avoid bitmasks in properties, and we also usually exepct a
> > full cell even if data is smaller. The fact that you expect /bits/ 8 
> > must be documented here if that's truly necessary.
> 
> It's not truly necessary. It's just a nice fit. There is one 8 bit
> (device memory) register that basically could (in the future) be
> exposed through this DT property.
> 
> For now it's just 0xff or nothing. We only don't want to create an
> interface that could restrict us from implementing more in the future
> without breaking anything.

It sounds like you wouldn't need this (at least for now) if you were to
use interrupt-names to describe whether INT1 and/or INT2 were wired up.

Thanks,
Mark.

Re: [PATCH 8/8] iio: mma8452: add devicetree property to allow all pin wirings

[Mark Rutland]

On Mon, Jul 27, 2015 at 03:37:48PM +0100, Martin Kepplinger wrote:
> Am 2015-07-27 um 16:23 schrieb Mark Rutland:
> > On Mon, Jul 27, 2015 at 03:08:15PM +0100, Martin Kepplinger wrote:
> >> For the devices supported by the mma8452 driver, two interrupt pins are
> >> available to route the interrupt signals to. By default INT1 is assumed.
> >>
> >> This adds a bitmask DT property for users to configure interrupt sources
> >> for INT2, if that is the wired interrupt pin for them.
> > 
> > This sounds like configureation rather than a HW property. Why does this
> > need to be in the DT?
> 
> It's a hardware property of the board that uses the device. There might
> be boards that connect just one of them at random, which is the reason
> for this DT property. There also might be exotic users who will want
> to use both pins to route different interrupt sources to (not yet
> supported, but no problem with such a bitmask).

Ok, so I'm somewhat confused as to what the hardware looks like and what
this means.

Could you elaborate on how INT1 and INT2 are used? It looks like they're
used as output pins, and so interrupt-names would seem appropriate for
describing the combination which is wired up.

w.r.t. configuring the choice of output(s), that sounds like a runtime
decision rather than something which needs to be configured statically.

> >> This is important for everyone to be able to use this driver, no matter
> >> how their chip is wired. At the moment, only 0xff for using INT2 for all
> >> available interrupt sources is supported. See the devicetree documentation
> >> file for more details.
> >>
> >> Since this doesn't change the default behaviour, it doesn't break anything
> >> for existing users.
> >>
> >> Signed-off-by: Martin Kepplinger <martin.kepplinger@theobroma-systems.com>
> >> Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com>
> >> ---
> >>  .../devicetree/bindings/iio/accel/mma8452.txt        |  4 ++++
> >>  drivers/iio/accel/mma8452.c                          | 20 +++++++++++++-------
> >>  2 files changed, 17 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/iio/accel/mma8452.txt b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
> >> index 8d98e05..738a430 100644
> >> --- a/Documentation/devicetree/bindings/iio/accel/mma8452.txt
> >> +++ b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
> >> @@ -10,6 +10,9 @@ Optional properties:
> >>  
> >>    - interrupt-parent: should be the phandle for the interrupt controller
> >>    - interrupts: interrupt mapping for GPIO IRQ
> >> +  - use_int2: bitmask to choose interrupt sources assumed to be wired to
> >> +    interrupt pin INT2 instead of INT1. Only 0xff (INT2 for every interrupt
> >> +    source) is supported at the moment.
> > 
> > s/_/-/ in property names, please.
> 
> ok. If I don't do a version 6 really soon, I'll reply with this patch
> corrected here.
> 
> > 
> > We generally avoid bitmasks in properties, and we also usually exepct a
> > full cell even if data is smaller. The fact that you expect /bits/ 8 
> > must be documented here if that's truly necessary.
> 
> It's not truly necessary. It's just a nice fit. There is one 8 bit
> (device memory) register that basically could (in the future) be
> exposed through this DT property.
> 
> For now it's just 0xff or nothing. We only don't want to create an
> interface that could restrict us from implementing more in the future
> without breaking anything.

It sounds like you wouldn't need this (at least for now) if you were to
use interrupt-names to describe whether INT1 and/or INT2 were wired up.

Thanks,
Mark.

Re: [PATCH 8/8] iio: mma8452: add devicetree property to allow all pin wirings

[Mark Rutland]

On Mon, Jul 27, 2015 at 03:37:48PM +0100, Martin Kepplinger wrote:
> Am 2015-07-27 um 16:23 schrieb Mark Rutland:
> > On Mon, Jul 27, 2015 at 03:08:15PM +0100, Martin Kepplinger wrote:
> >> For the devices supported by the mma8452 driver, two interrupt pins are
> >> available to route the interrupt signals to. By default INT1 is assumed.
> >>
> >> This adds a bitmask DT property for users to configure interrupt sources
> >> for INT2, if that is the wired interrupt pin for them.
> > 
> > This sounds like configureation rather than a HW property. Why does this
> > need to be in the DT?
> 
> It's a hardware property of the board that uses the device. There might
> be boards that connect just one of them at random, which is the reason
> for this DT property. There also might be exotic users who will want
> to use both pins to route different interrupt sources to (not yet
> supported, but no problem with such a bitmask).

Ok, so I'm somewhat confused as to what the hardware looks like and what
this means.

Could you elaborate on how INT1 and INT2 are used? It looks like they're
used as output pins, and so interrupt-names would seem appropriate for
describing the combination which is wired up.

w.r.t. configuring the choice of output(s), that sounds like a runtime
decision rather than something which needs to be configured statically.

> >> This is important for everyone to be able to use this driver, no matter
> >> how their chip is wired. At the moment, only 0xff for using INT2 for all
> >> available interrupt sources is supported. See the devicetree documentation
> >> file for more details.
> >>
> >> Since this doesn't change the default behaviour, it doesn't break anything
> >> for existing users.
> >>
> >> Signed-off-by: Martin Kepplinger <martin.kepplinger-SN7IsUiht6C/RdPyistoZJqQE7yCjDx5@public.gmane.org>
> >> Signed-off-by: Christoph Muellner <christoph.muellner-SN7IsUiht6C/RdPyistoZJqQE7yCjDx5@public.gmane.org>
> >> ---
> >>  .../devicetree/bindings/iio/accel/mma8452.txt        |  4 ++++
> >>  drivers/iio/accel/mma8452.c                          | 20 +++++++++++++-------
> >>  2 files changed, 17 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/iio/accel/mma8452.txt b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
> >> index 8d98e05..738a430 100644
> >> --- a/Documentation/devicetree/bindings/iio/accel/mma8452.txt
> >> +++ b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
> >> @@ -10,6 +10,9 @@ Optional properties:
> >>  
> >>    - interrupt-parent: should be the phandle for the interrupt controller
> >>    - interrupts: interrupt mapping for GPIO IRQ
> >> +  - use_int2: bitmask to choose interrupt sources assumed to be wired to
> >> +    interrupt pin INT2 instead of INT1. Only 0xff (INT2 for every interrupt
> >> +    source) is supported at the moment.
> > 
> > s/_/-/ in property names, please.
> 
> ok. If I don't do a version 6 really soon, I'll reply with this patch
> corrected here.
> 
> > 
> > We generally avoid bitmasks in properties, and we also usually exepct a
> > full cell even if data is smaller. The fact that you expect /bits/ 8 
> > must be documented here if that's truly necessary.
> 
> It's not truly necessary. It's just a nice fit. There is one 8 bit
> (device memory) register that basically could (in the future) be
> exposed through this DT property.
> 
> For now it's just 0xff or nothing. We only don't want to create an
> interface that could restrict us from implementing more in the future
> without breaking anything.

It sounds like you wouldn't need this (at least for now) if you were to
use interrupt-names to describe whether INT1 and/or INT2 were wired up.

Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 8/8] iio: mma8452: add devicetree property to allow all pin wirings

[Martin Kepplinger]

On 2015-07-27 19:33, Mark Rutland wrote:
> On Mon, Jul 27, 2015 at 03:37:48PM +0100, Martin Kepplinger wrote:
>> Am 2015-07-27 um 16:23 schrieb Mark Rutland:
>>> On Mon, Jul 27, 2015 at 03:08:15PM +0100, Martin Kepplinger wrote:
>>>> For the devices supported by the mma8452 driver, two interrupt pins are
>>>> available to route the interrupt signals to. By default INT1 is assumed.
>>>>
>>>> This adds a bitmask DT property for users to configure interrupt sources
>>>> for INT2, if that is the wired interrupt pin for them.
>>>
>>> This sounds like configureation rather than a HW property. Why does this
>>> need to be in the DT?
>>
>> It's a hardware property of the board that uses the device. There might
>> be boards that connect just one of them at random, which is the reason
>> for this DT property. There also might be exotic users who will want
>> to use both pins to route different interrupt sources to (not yet
>> supported, but no problem with such a bitmask).
> 
> Ok, so I'm somewhat confused as to what the hardware looks like and what
> this means.
> 
> Could you elaborate on how INT1 and INT2 are used? It looks like they're
> used as output pins, and so interrupt-names would seem appropriate for
> describing the combination which is wired up.

They are just the chip's two possible interrupt lines for us to get
notified about event.

You build a board, you use one of these 4 chips, wiring up just one of
the 2 interrupt pins. By far most people won't ever need both pins.

DT describes your hardware, right? So you describe how you built your
board (wired the accelerometer chip) with this DT property.

> 
> w.r.t. configuring the choice of output(s), that sounds like a runtime
> decision rather than something which needs to be configured statically.

This won't be useful during runtime. (De)activating events is what you
do in iio sysfs.

Even in the rare case (maybe supported in the future) when you want one
interrupt source on one pin and another source on the other pin, that
describes your hardware. You wire, say, data-ready to Linux and
motion-detection to some strange alarm system. When you change your
hardware (say, use Linux for both pins), I think it would justify
changing a DT property.

Btw, we are talking about very theoretical stuff here. For now (and even
possibly forever) we just don't ever want to break a DT propery we
introduce here, thus the bitmask.

> 
>>>> This is important for everyone to be able to use this driver, no matter
>>>> how their chip is wired. At the moment, only 0xff for using INT2 for all
>>>> available interrupt sources is supported. See the devicetree documentation
>>>> file for more details.
>>>>
>>>> Since this doesn't change the default behaviour, it doesn't break anything
>>>> for existing users.
>>>>
>>>> Signed-off-by: Martin Kepplinger <martin.kepplinger@theobroma-systems.com>
>>>> Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com>
>>>> ---
>>>>  .../devicetree/bindings/iio/accel/mma8452.txt        |  4 ++++
>>>>  drivers/iio/accel/mma8452.c                          | 20 +++++++++++++-------
>>>>  2 files changed, 17 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/iio/accel/mma8452.txt b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
>>>> index 8d98e05..738a430 100644
>>>> --- a/Documentation/devicetree/bindings/iio/accel/mma8452.txt
>>>> +++ b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
>>>> @@ -10,6 +10,9 @@ Optional properties:
>>>>  
>>>>    - interrupt-parent: should be the phandle for the interrupt controller
>>>>    - interrupts: interrupt mapping for GPIO IRQ
>>>> +  - use_int2: bitmask to choose interrupt sources assumed to be wired to
>>>> +    interrupt pin INT2 instead of INT1. Only 0xff (INT2 for every interrupt
>>>> +    source) is supported at the moment.
>>>
>>> s/_/-/ in property names, please.
>>
>> ok. If I don't do a version 6 really soon, I'll reply with this patch
>> corrected here.
>>
>>>
>>> We generally avoid bitmasks in properties, and we also usually exepct a
>>> full cell even if data is smaller. The fact that you expect /bits/ 8 
>>> must be documented here if that's truly necessary.
>>
>> It's not truly necessary. It's just a nice fit. There is one 8 bit
>> (device memory) register that basically could (in the future) be
>> exposed through this DT property.
>>
>> For now it's just 0xff or nothing. We only don't want to create an
>> interface that could restrict us from implementing more in the future
>> without breaking anything.
> 
> It sounds like you wouldn't need this (at least for now) if you were to
> use interrupt-names to describe whether INT1 and/or INT2 were wired up.
> 
> Thanks,
> Mark.
> 

Re: [PATCH 8/8] iio: mma8452: add devicetree property to allow all pin wirings

[Martin Kepplinger]

On 2015-07-27 19:33, Mark Rutland wrote:
> On Mon, Jul 27, 2015 at 03:37:48PM +0100, Martin Kepplinger wrote:
>> Am 2015-07-27 um 16:23 schrieb Mark Rutland:
>>> On Mon, Jul 27, 2015 at 03:08:15PM +0100, Martin Kepplinger wrote:
>>>> For the devices supported by the mma8452 driver, two interrupt pins are
>>>> available to route the interrupt signals to. By default INT1 is assumed.
>>>>
>>>> This adds a bitmask DT property for users to configure interrupt sources
>>>> for INT2, if that is the wired interrupt pin for them.
>>>
>>> This sounds like configureation rather than a HW property. Why does this
>>> need to be in the DT?
>>
>> It's a hardware property of the board that uses the device. There might
>> be boards that connect just one of them at random, which is the reason
>> for this DT property. There also might be exotic users who will want
>> to use both pins to route different interrupt sources to (not yet
>> supported, but no problem with such a bitmask).
> 
> Ok, so I'm somewhat confused as to what the hardware looks like and what
> this means.
> 
> Could you elaborate on how INT1 and INT2 are used? It looks like they're
> used as output pins, and so interrupt-names would seem appropriate for
> describing the combination which is wired up.

They are just the chip's two possible interrupt lines for us to get
notified about event.

You build a board, you use one of these 4 chips, wiring up just one of
the 2 interrupt pins. By far most people won't ever need both pins.

DT describes your hardware, right? So you describe how you built your
board (wired the accelerometer chip) with this DT property.

> 
> w.r.t. configuring the choice of output(s), that sounds like a runtime
> decision rather than something which needs to be configured statically.

This won't be useful during runtime. (De)activating events is what you
do in iio sysfs.

Even in the rare case (maybe supported in the future) when you want one
interrupt source on one pin and another source on the other pin, that
describes your hardware. You wire, say, data-ready to Linux and
motion-detection to some strange alarm system. When you change your
hardware (say, use Linux for both pins), I think it would justify
changing a DT property.

Btw, we are talking about very theoretical stuff here. For now (and even
possibly forever) we just don't ever want to break a DT propery we
introduce here, thus the bitmask.

> 
>>>> This is important for everyone to be able to use this driver, no matter
>>>> how their chip is wired. At the moment, only 0xff for using INT2 for all
>>>> available interrupt sources is supported. See the devicetree documentation
>>>> file for more details.
>>>>
>>>> Since this doesn't change the default behaviour, it doesn't break anything
>>>> for existing users.
>>>>
>>>> Signed-off-by: Martin Kepplinger <martin.kepplinger@theobroma-systems.com>
>>>> Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com>
>>>> ---
>>>>  .../devicetree/bindings/iio/accel/mma8452.txt        |  4 ++++
>>>>  drivers/iio/accel/mma8452.c                          | 20 +++++++++++++-------
>>>>  2 files changed, 17 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/iio/accel/mma8452.txt b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
>>>> index 8d98e05..738a430 100644
>>>> --- a/Documentation/devicetree/bindings/iio/accel/mma8452.txt
>>>> +++ b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
>>>> @@ -10,6 +10,9 @@ Optional properties:
>>>>  
>>>>    - interrupt-parent: should be the phandle for the interrupt controller
>>>>    - interrupts: interrupt mapping for GPIO IRQ
>>>> +  - use_int2: bitmask to choose interrupt sources assumed to be wired to
>>>> +    interrupt pin INT2 instead of INT1. Only 0xff (INT2 for every interrupt
>>>> +    source) is supported at the moment.
>>>
>>> s/_/-/ in property names, please.
>>
>> ok. If I don't do a version 6 really soon, I'll reply with this patch
>> corrected here.
>>
>>>
>>> We generally avoid bitmasks in properties, and we also usually exepct a
>>> full cell even if data is smaller. The fact that you expect /bits/ 8 
>>> must be documented here if that's truly necessary.
>>
>> It's not truly necessary. It's just a nice fit. There is one 8 bit
>> (device memory) register that basically could (in the future) be
>> exposed through this DT property.
>>
>> For now it's just 0xff or nothing. We only don't want to create an
>> interface that could restrict us from implementing more in the future
>> without breaking anything.
> 
> It sounds like you wouldn't need this (at least for now) if you were to
> use interrupt-names to describe whether INT1 and/or INT2 were wired up.
> 
> Thanks,
> Mark.
> 

Re: [PATCH 8/8] iio: mma8452: add devicetree property to allow all pin wirings

[Martin Kepplinger]

On 2015-07-27 19:33, Mark Rutland wrote:
> On Mon, Jul 27, 2015 at 03:37:48PM +0100, Martin Kepplinger wrote:
>> Am 2015-07-27 um 16:23 schrieb Mark Rutland:
>>> On Mon, Jul 27, 2015 at 03:08:15PM +0100, Martin Kepplinger wrote:
>>>> For the devices supported by the mma8452 driver, two interrupt pins are
>>>> available to route the interrupt signals to. By default INT1 is assumed.
>>>>
>>>> This adds a bitmask DT property for users to configure interrupt sources
>>>> for INT2, if that is the wired interrupt pin for them.
>>>
>>> This sounds like configureation rather than a HW property. Why does this
>>> need to be in the DT?
>>
>> It's a hardware property of the board that uses the device. There might
>> be boards that connect just one of them at random, which is the reason
>> for this DT property. There also might be exotic users who will want
>> to use both pins to route different interrupt sources to (not yet
>> supported, but no problem with such a bitmask).
> 
> Ok, so I'm somewhat confused as to what the hardware looks like and what
> this means.
> 
> Could you elaborate on how INT1 and INT2 are used? It looks like they're
> used as output pins, and so interrupt-names would seem appropriate for
> describing the combination which is wired up.

They are just the chip's two possible interrupt lines for us to get
notified about event.

You build a board, you use one of these 4 chips, wiring up just one of
the 2 interrupt pins. By far most people won't ever need both pins.

DT describes your hardware, right? So you describe how you built your
board (wired the accelerometer chip) with this DT property.

> 
> w.r.t. configuring the choice of output(s), that sounds like a runtime
> decision rather than something which needs to be configured statically.

This won't be useful during runtime. (De)activating events is what you
do in iio sysfs.

Even in the rare case (maybe supported in the future) when you want one
interrupt source on one pin and another source on the other pin, that
describes your hardware. You wire, say, data-ready to Linux and
motion-detection to some strange alarm system. When you change your
hardware (say, use Linux for both pins), I think it would justify
changing a DT property.

Btw, we are talking about very theoretical stuff here. For now (and even
possibly forever) we just don't ever want to break a DT propery we
introduce here, thus the bitmask.

> 
>>>> This is important for everyone to be able to use this driver, no matter
>>>> how their chip is wired. At the moment, only 0xff for using INT2 for all
>>>> available interrupt sources is supported. See the devicetree documentation
>>>> file for more details.
>>>>
>>>> Since this doesn't change the default behaviour, it doesn't break anything
>>>> for existing users.
>>>>
>>>> Signed-off-by: Martin Kepplinger <martin.kepplinger-SN7IsUiht6C/RdPyistoZJqQE7yCjDx5@public.gmane.org>
>>>> Signed-off-by: Christoph Muellner <christoph.muellner-SN7IsUiht6C/RdPyistoZJqQE7yCjDx5@public.gmane.org>
>>>> ---
>>>>  .../devicetree/bindings/iio/accel/mma8452.txt        |  4 ++++
>>>>  drivers/iio/accel/mma8452.c                          | 20 +++++++++++++-------
>>>>  2 files changed, 17 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/iio/accel/mma8452.txt b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
>>>> index 8d98e05..738a430 100644
>>>> --- a/Documentation/devicetree/bindings/iio/accel/mma8452.txt
>>>> +++ b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
>>>> @@ -10,6 +10,9 @@ Optional properties:
>>>>  
>>>>    - interrupt-parent: should be the phandle for the interrupt controller
>>>>    - interrupts: interrupt mapping for GPIO IRQ
>>>> +  - use_int2: bitmask to choose interrupt sources assumed to be wired to
>>>> +    interrupt pin INT2 instead of INT1. Only 0xff (INT2 for every interrupt
>>>> +    source) is supported at the moment.
>>>
>>> s/_/-/ in property names, please.
>>
>> ok. If I don't do a version 6 really soon, I'll reply with this patch
>> corrected here.
>>
>>>
>>> We generally avoid bitmasks in properties, and we also usually exepct a
>>> full cell even if data is smaller. The fact that you expect /bits/ 8 
>>> must be documented here if that's truly necessary.
>>
>> It's not truly necessary. It's just a nice fit. There is one 8 bit
>> (device memory) register that basically could (in the future) be
>> exposed through this DT property.
>>
>> For now it's just 0xff or nothing. We only don't want to create an
>> interface that could restrict us from implementing more in the future
>> without breaking anything.
> 
> It sounds like you wouldn't need this (at least for now) if you were to
> use interrupt-names to describe whether INT1 and/or INT2 were wired up.
> 
> Thanks,
> Mark.
> 

Re: [PATCH 8/8] iio: mma8452: add devicetree property to allow all pin wirings

[Mark Rutland]

On Tue, Jul 28, 2015 at 10:11:29AM +0100, Martin Kepplinger wrote:
> 
> 
> On 2015-07-27 19:33, Mark Rutland wrote:
> > On Mon, Jul 27, 2015 at 03:37:48PM +0100, Martin Kepplinger wrote:
> >> Am 2015-07-27 um 16:23 schrieb Mark Rutland:
> >>> On Mon, Jul 27, 2015 at 03:08:15PM +0100, Martin Kepplinger wrote:
> >>>> For the devices supported by the mma8452 driver, two interrupt pins are
> >>>> available to route the interrupt signals to. By default INT1 is assumed.
> >>>>
> >>>> This adds a bitmask DT property for users to configure interrupt sources
> >>>> for INT2, if that is the wired interrupt pin for them.
> >>>
> >>> This sounds like configureation rather than a HW property. Why does this
> >>> need to be in the DT?
> >>
> >> It's a hardware property of the board that uses the device. There might
> >> be boards that connect just one of them at random, which is the reason
> >> for this DT property. There also might be exotic users who will want
> >> to use both pins to route different interrupt sources to (not yet
> >> supported, but no problem with such a bitmask).
> > 
> > Ok, so I'm somewhat confused as to what the hardware looks like and what
> > this means.
> > 
> > Could you elaborate on how INT1 and INT2 are used? It looks like they're
> > used as output pins, and so interrupt-names would seem appropriate for
> > describing the combination which is wired up.
> 
> They are just the chip's two possible interrupt lines for us to get
> notified about event.

Ok. So that matches my understanding.

> You build a board, you use one of these 4 chips, wiring up just one of
> the 2 interrupt pins. By far most people won't ever need both pins.
> 
> DT describes your hardware, right? So you describe how you built your
> board (wired the accelerometer chip) with this DT property.

Ok.

> > w.r.t. configuring the choice of output(s), that sounds like a runtime
> > decision rather than something which needs to be configured statically.
> 
> This won't be useful during runtime. (De)activating events is what you
> do in iio sysfs.
> 
> Even in the rare case (maybe supported in the future) when you want one
> interrupt source on one pin and another source on the other pin, that
> describes your hardware. You wire, say, data-ready to Linux and
> motion-detection to some strange alarm system. When you change your
> hardware (say, use Linux for both pins), I think it would justify
> changing a DT property.

In that case you would need additional properties anyway.

> Btw, we are talking about very theoretical stuff here. For now (and even
> possibly forever) we just don't ever want to break a DT propery we
> introduce here, thus the bitmask.

I don't think you need the bitmask.

I think all you need is interrupt-names, e.g.

dev1 {
	/* both wired up */
	interrupts = <&some_ic 0 47>, <&some_ic 5 62>;
	interrupt-names = "INT1", "INT2";
}

dev2 {
	/* only INT2 wired up */
	interrupts = <&some_ic 3 96>;
	interrupt-names = "INT2";
}

You can figure out which interrupts are wired up by trying to acquire
them by name, then falling back to acquiting an anonymouos interrupt
(assuming it's INT1) to keep compatible with existing DTBs. You can
choose which to use arbitrarily, try to load balance, or whatever you'd
like.

If it's later necessary to route some interrupts to another device,
additional properties can be added to accomodate that. We already know
that the bitmask alone is not sufficient for that case.

Thanks,
Mark.

Re: [PATCH 8/8] iio: mma8452: add devicetree property to allow all pin wirings

[Mark Rutland]

On Tue, Jul 28, 2015 at 10:11:29AM +0100, Martin Kepplinger wrote:
> 
> 
> On 2015-07-27 19:33, Mark Rutland wrote:
> > On Mon, Jul 27, 2015 at 03:37:48PM +0100, Martin Kepplinger wrote:
> >> Am 2015-07-27 um 16:23 schrieb Mark Rutland:
> >>> On Mon, Jul 27, 2015 at 03:08:15PM +0100, Martin Kepplinger wrote:
> >>>> For the devices supported by the mma8452 driver, two interrupt pins are
> >>>> available to route the interrupt signals to. By default INT1 is assumed.
> >>>>
> >>>> This adds a bitmask DT property for users to configure interrupt sources
> >>>> for INT2, if that is the wired interrupt pin for them.
> >>>
> >>> This sounds like configureation rather than a HW property. Why does this
> >>> need to be in the DT?
> >>
> >> It's a hardware property of the board that uses the device. There might
> >> be boards that connect just one of them at random, which is the reason
> >> for this DT property. There also might be exotic users who will want
> >> to use both pins to route different interrupt sources to (not yet
> >> supported, but no problem with such a bitmask).
> > 
> > Ok, so I'm somewhat confused as to what the hardware looks like and what
> > this means.
> > 
> > Could you elaborate on how INT1 and INT2 are used? It looks like they're
> > used as output pins, and so interrupt-names would seem appropriate for
> > describing the combination which is wired up.
> 
> They are just the chip's two possible interrupt lines for us to get
> notified about event.

Ok. So that matches my understanding.

> You build a board, you use one of these 4 chips, wiring up just one of
> the 2 interrupt pins. By far most people won't ever need both pins.
> 
> DT describes your hardware, right? So you describe how you built your
> board (wired the accelerometer chip) with this DT property.

Ok.

> > w.r.t. configuring the choice of output(s), that sounds like a runtime
> > decision rather than something which needs to be configured statically.
> 
> This won't be useful during runtime. (De)activating events is what you
> do in iio sysfs.
> 
> Even in the rare case (maybe supported in the future) when you want one
> interrupt source on one pin and another source on the other pin, that
> describes your hardware. You wire, say, data-ready to Linux and
> motion-detection to some strange alarm system. When you change your
> hardware (say, use Linux for both pins), I think it would justify
> changing a DT property.

In that case you would need additional properties anyway.

> Btw, we are talking about very theoretical stuff here. For now (and even
> possibly forever) we just don't ever want to break a DT propery we
> introduce here, thus the bitmask.

I don't think you need the bitmask.

I think all you need is interrupt-names, e.g.

dev1 {
	/* both wired up */
	interrupts = <&some_ic 0 47>, <&some_ic 5 62>;
	interrupt-names = "INT1", "INT2";
}

dev2 {
	/* only INT2 wired up */
	interrupts = <&some_ic 3 96>;
	interrupt-names = "INT2";
}

You can figure out which interrupts are wired up by trying to acquire
them by name, then falling back to acquiting an anonymouos interrupt
(assuming it's INT1) to keep compatible with existing DTBs. You can
choose which to use arbitrarily, try to load balance, or whatever you'd
like.

If it's later necessary to route some interrupts to another device,
additional properties can be added to accomodate that. We already know
that the bitmask alone is not sufficient for that case.

Thanks,
Mark.

Re: [PATCH 8/8] iio: mma8452: add devicetree property to allow all pin wirings

[Martin Kepplinger]

Am 2015-07-28 um 11:28 schrieb Mark Rutland:
> On Tue, Jul 28, 2015 at 10:11:29AM +0100, Martin Kepplinger wrote:
>>
>>
>> On 2015-07-27 19:33, Mark Rutland wrote:
>>> On Mon, Jul 27, 2015 at 03:37:48PM +0100, Martin Kepplinger wrote:
>>>> Am 2015-07-27 um 16:23 schrieb Mark Rutland:
>>>>> On Mon, Jul 27, 2015 at 03:08:15PM +0100, Martin Kepplinger wrote:
>>>>>> For the devices supported by the mma8452 driver, two interrupt pins are
>>>>>> available to route the interrupt signals to. By default INT1 is assumed.
>>>>>>
>>>>>> This adds a bitmask DT property for users to configure interrupt sources
>>>>>> for INT2, if that is the wired interrupt pin for them.
>>>>>
>>>>> This sounds like configureation rather than a HW property. Why does this
>>>>> need to be in the DT?
>>>>
>>>> It's a hardware property of the board that uses the device. There might
>>>> be boards that connect just one of them at random, which is the reason
>>>> for this DT property. There also might be exotic users who will want
>>>> to use both pins to route different interrupt sources to (not yet
>>>> supported, but no problem with such a bitmask).
>>>
>>> Ok, so I'm somewhat confused as to what the hardware looks like and what
>>> this means.
>>>
>>> Could you elaborate on how INT1 and INT2 are used? It looks like they're
>>> used as output pins, and so interrupt-names would seem appropriate for
>>> describing the combination which is wired up.
>>
>> They are just the chip's two possible interrupt lines for us to get
>> notified about event.
> 
> Ok. So that matches my understanding.
> 
>> You build a board, you use one of these 4 chips, wiring up just one of
>> the 2 interrupt pins. By far most people won't ever need both pins.
>>
>> DT describes your hardware, right? So you describe how you built your
>> board (wired the accelerometer chip) with this DT property.
> 
> Ok.
> 
>>> w.r.t. configuring the choice of output(s), that sounds like a runtime
>>> decision rather than something which needs to be configured statically.
>>
>> This won't be useful during runtime. (De)activating events is what you
>> do in iio sysfs.
>>
>> Even in the rare case (maybe supported in the future) when you want one
>> interrupt source on one pin and another source on the other pin, that
>> describes your hardware. You wire, say, data-ready to Linux and
>> motion-detection to some strange alarm system. When you change your
>> hardware (say, use Linux for both pins), I think it would justify
>> changing a DT property.
> 
> In that case you would need additional properties anyway.
> 
>> Btw, we are talking about very theoretical stuff here. For now (and even
>> possibly forever) we just don't ever want to break a DT propery we
>> introduce here, thus the bitmask.
> 
> I don't think you need the bitmask.
> 
> I think all you need is interrupt-names, e.g.
> 
> dev1 {
> 	/* both wired up */
> 	interrupts = <&some_ic 0 47>, <&some_ic 5 62>;
> 	interrupt-names = "INT1", "INT2";
> }
> 
> dev2 {
> 	/* only INT2 wired up */
> 	interrupts = <&some_ic 3 96>;
> 	interrupt-names = "INT2";
> }
> 
> You can figure out which interrupts are wired up by trying to acquire
> them by name, then falling back to acquiting an anonymouos interrupt
> (assuming it's INT1) to keep compatible with existing DTBs. You can
> choose which to use arbitrarily, try to load balance, or whatever you'd
> like.
> 
> If it's later necessary to route some interrupts to another device,
> additional properties can be added to accomodate that. We already know
> that the bitmask alone is not sufficient for that case.
> 

Yes, this sounds reasonable indeed. I like the idea. I'm sorry I won't
rewrite patch 8/8 now. Relocation and a lot to do before holidays. I'll
be happy to write and test this properly in one month from now, if not
done by somebody until then.

Until then, since patches 1-7 only introduce a bindings document, they
shouldn't be problematic for devicetree people.


So if Jonathan and IIO people find anybody for review, feel free to take
patches 1-7. In any case, there is direct register access via debugfs to
at least somehow make the driver work for everybody ;)

so long, thanks.
                    martin

> Thanks,
> Mark.
> 

Re: [PATCH 8/8] iio: mma8452: add devicetree property to allow all pin wirings

[Martin Kepplinger]

Am 2015-07-28 um 11:28 schrieb Mark Rutland:
> On Tue, Jul 28, 2015 at 10:11:29AM +0100, Martin Kepplinger wrote:
>>
>>
>> On 2015-07-27 19:33, Mark Rutland wrote:
>>> On Mon, Jul 27, 2015 at 03:37:48PM +0100, Martin Kepplinger wrote:
>>>> Am 2015-07-27 um 16:23 schrieb Mark Rutland:
>>>>> On Mon, Jul 27, 2015 at 03:08:15PM +0100, Martin Kepplinger wrote:
>>>>>> For the devices supported by the mma8452 driver, two interrupt pins are
>>>>>> available to route the interrupt signals to. By default INT1 is assumed.
>>>>>>
>>>>>> This adds a bitmask DT property for users to configure interrupt sources
>>>>>> for INT2, if that is the wired interrupt pin for them.
>>>>>
>>>>> This sounds like configureation rather than a HW property. Why does this
>>>>> need to be in the DT?
>>>>
>>>> It's a hardware property of the board that uses the device. There might
>>>> be boards that connect just one of them at random, which is the reason
>>>> for this DT property. There also might be exotic users who will want
>>>> to use both pins to route different interrupt sources to (not yet
>>>> supported, but no problem with such a bitmask).
>>>
>>> Ok, so I'm somewhat confused as to what the hardware looks like and what
>>> this means.
>>>
>>> Could you elaborate on how INT1 and INT2 are used? It looks like they're
>>> used as output pins, and so interrupt-names would seem appropriate for
>>> describing the combination which is wired up.
>>
>> They are just the chip's two possible interrupt lines for us to get
>> notified about event.
> 
> Ok. So that matches my understanding.
> 
>> You build a board, you use one of these 4 chips, wiring up just one of
>> the 2 interrupt pins. By far most people won't ever need both pins.
>>
>> DT describes your hardware, right? So you describe how you built your
>> board (wired the accelerometer chip) with this DT property.
> 
> Ok.
> 
>>> w.r.t. configuring the choice of output(s), that sounds like a runtime
>>> decision rather than something which needs to be configured statically.
>>
>> This won't be useful during runtime. (De)activating events is what you
>> do in iio sysfs.
>>
>> Even in the rare case (maybe supported in the future) when you want one
>> interrupt source on one pin and another source on the other pin, that
>> describes your hardware. You wire, say, data-ready to Linux and
>> motion-detection to some strange alarm system. When you change your
>> hardware (say, use Linux for both pins), I think it would justify
>> changing a DT property.
> 
> In that case you would need additional properties anyway.
> 
>> Btw, we are talking about very theoretical stuff here. For now (and even
>> possibly forever) we just don't ever want to break a DT propery we
>> introduce here, thus the bitmask.
> 
> I don't think you need the bitmask.
> 
> I think all you need is interrupt-names, e.g.
> 
> dev1 {
> 	/* both wired up */
> 	interrupts = <&some_ic 0 47>, <&some_ic 5 62>;
> 	interrupt-names = "INT1", "INT2";
> }
> 
> dev2 {
> 	/* only INT2 wired up */
> 	interrupts = <&some_ic 3 96>;
> 	interrupt-names = "INT2";
> }
> 
> You can figure out which interrupts are wired up by trying to acquire
> them by name, then falling back to acquiting an anonymouos interrupt
> (assuming it's INT1) to keep compatible with existing DTBs. You can
> choose which to use arbitrarily, try to load balance, or whatever you'd
> like.
> 
> If it's later necessary to route some interrupts to another device,
> additional properties can be added to accomodate that. We already know
> that the bitmask alone is not sufficient for that case.
> 

Yes, this sounds reasonable indeed. I like the idea. I'm sorry I won't
rewrite patch 8/8 now. Relocation and a lot to do before holidays. I'll
be happy to write and test this properly in one month from now, if not
done by somebody until then.

Until then, since patches 1-7 only introduce a bindings document, they
shouldn't be problematic for devicetree people.


So if Jonathan and IIO people find anybody for review, feel free to take
patches 1-7. In any case, there is direct register access via debugfs to
at least somehow make the driver work for everybody ;)

so long, thanks.
                    martin

> Thanks,
> Mark.
> 

Re: [PATCH 8/8] iio: mma8452: add devicetree property to allow all pin wirings

[Martin Kepplinger]

Am 2015-07-28 um 11:28 schrieb Mark Rutland:
> On Tue, Jul 28, 2015 at 10:11:29AM +0100, Martin Kepplinger wrote:
>>
>>
>> On 2015-07-27 19:33, Mark Rutland wrote:
>>> On Mon, Jul 27, 2015 at 03:37:48PM +0100, Martin Kepplinger wrote:
>>>> Am 2015-07-27 um 16:23 schrieb Mark Rutland:
>>>>> On Mon, Jul 27, 2015 at 03:08:15PM +0100, Martin Kepplinger wrote:
>>>>>> For the devices supported by the mma8452 driver, two interrupt pins are
>>>>>> available to route the interrupt signals to. By default INT1 is assumed.
>>>>>>
>>>>>> This adds a bitmask DT property for users to configure interrupt sources
>>>>>> for INT2, if that is the wired interrupt pin for them.
>>>>>
>>>>> This sounds like configureation rather than a HW property. Why does this
>>>>> need to be in the DT?
>>>>
>>>> It's a hardware property of the board that uses the device. There might
>>>> be boards that connect just one of them at random, which is the reason
>>>> for this DT property. There also might be exotic users who will want
>>>> to use both pins to route different interrupt sources to (not yet
>>>> supported, but no problem with such a bitmask).
>>>
>>> Ok, so I'm somewhat confused as to what the hardware looks like and what
>>> this means.
>>>
>>> Could you elaborate on how INT1 and INT2 are used? It looks like they're
>>> used as output pins, and so interrupt-names would seem appropriate for
>>> describing the combination which is wired up.
>>
>> They are just the chip's two possible interrupt lines for us to get
>> notified about event.
> 
> Ok. So that matches my understanding.
> 
>> You build a board, you use one of these 4 chips, wiring up just one of
>> the 2 interrupt pins. By far most people won't ever need both pins.
>>
>> DT describes your hardware, right? So you describe how you built your
>> board (wired the accelerometer chip) with this DT property.
> 
> Ok.
> 
>>> w.r.t. configuring the choice of output(s), that sounds like a runtime
>>> decision rather than something which needs to be configured statically.
>>
>> This won't be useful during runtime. (De)activating events is what you
>> do in iio sysfs.
>>
>> Even in the rare case (maybe supported in the future) when you want one
>> interrupt source on one pin and another source on the other pin, that
>> describes your hardware. You wire, say, data-ready to Linux and
>> motion-detection to some strange alarm system. When you change your
>> hardware (say, use Linux for both pins), I think it would justify
>> changing a DT property.
> 
> In that case you would need additional properties anyway.
> 
>> Btw, we are talking about very theoretical stuff here. For now (and even
>> possibly forever) we just don't ever want to break a DT propery we
>> introduce here, thus the bitmask.
> 
> I don't think you need the bitmask.
> 
> I think all you need is interrupt-names, e.g.
> 
> dev1 {
> 	/* both wired up */
> 	interrupts = <&some_ic 0 47>, <&some_ic 5 62>;
> 	interrupt-names = "INT1", "INT2";
> }
> 
> dev2 {
> 	/* only INT2 wired up */
> 	interrupts = <&some_ic 3 96>;
> 	interrupt-names = "INT2";
> }
> 
> You can figure out which interrupts are wired up by trying to acquire
> them by name, then falling back to acquiting an anonymouos interrupt
> (assuming it's INT1) to keep compatible with existing DTBs. You can
> choose which to use arbitrarily, try to load balance, or whatever you'd
> like.
> 
> If it's later necessary to route some interrupts to another device,
> additional properties can be added to accomodate that. We already know
> that the bitmask alone is not sufficient for that case.
> 

Yes, this sounds reasonable indeed. I like the idea. I'm sorry I won't
rewrite patch 8/8 now. Relocation and a lot to do before holidays. I'll
be happy to write and test this properly in one month from now, if not
done by somebody until then.

Until then, since patches 1-7 only introduce a bindings document, they
shouldn't be problematic for devicetree people.


So if Jonathan and IIO people find anybody for review, feel free to take
patches 1-7. In any case, there is direct register access via debugfs to
at least somehow make the driver work for everybody ;)

so long, thanks.
                    martin

> Thanks,
> Mark.
> 

Re: [PATCH 8/8] iio: mma8452: add devicetree property to allow all pin wirings

[Jonathan Cameron]

On 29/07/15 00:12, Martin Kepplinger wrote:
> Am 2015-07-28 um 11:28 schrieb Mark Rutland:
>> On Tue, Jul 28, 2015 at 10:11:29AM +0100, Martin Kepplinger wrote:
>>>
>>>
>>> On 2015-07-27 19:33, Mark Rutland wrote:
>>>> On Mon, Jul 27, 2015 at 03:37:48PM +0100, Martin Kepplinger wrote:
>>>>> Am 2015-07-27 um 16:23 schrieb Mark Rutland:
>>>>>> On Mon, Jul 27, 2015 at 03:08:15PM +0100, Martin Kepplinger wrote:
>>>>>>> For the devices supported by the mma8452 driver, two interrupt pins are
>>>>>>> available to route the interrupt signals to. By default INT1 is assumed.
>>>>>>>
>>>>>>> This adds a bitmask DT property for users to configure interrupt sources
>>>>>>> for INT2, if that is the wired interrupt pin for them.
>>>>>>
>>>>>> This sounds like configureation rather than a HW property. Why does this
>>>>>> need to be in the DT?
>>>>>
>>>>> It's a hardware property of the board that uses the device. There might
>>>>> be boards that connect just one of them at random, which is the reason
>>>>> for this DT property. There also might be exotic users who will want
>>>>> to use both pins to route different interrupt sources to (not yet
>>>>> supported, but no problem with such a bitmask).
>>>>
>>>> Ok, so I'm somewhat confused as to what the hardware looks like and what
>>>> this means.
>>>>
>>>> Could you elaborate on how INT1 and INT2 are used? It looks like they're
>>>> used as output pins, and so interrupt-names would seem appropriate for
>>>> describing the combination which is wired up.
>>>
>>> They are just the chip's two possible interrupt lines for us to get
>>> notified about event.
>>
>> Ok. So that matches my understanding.
>>
>>> You build a board, you use one of these 4 chips, wiring up just one of
>>> the 2 interrupt pins. By far most people won't ever need both pins.
>>>
>>> DT describes your hardware, right? So you describe how you built your
>>> board (wired the accelerometer chip) with this DT property.
>>
>> Ok.
>>
>>>> w.r.t. configuring the choice of output(s), that sounds like a runtime
>>>> decision rather than something which needs to be configured statically.
>>>
>>> This won't be useful during runtime. (De)activating events is what you
>>> do in iio sysfs.
>>>
>>> Even in the rare case (maybe supported in the future) when you want one
>>> interrupt source on one pin and another source on the other pin, that
>>> describes your hardware. You wire, say, data-ready to Linux and
>>> motion-detection to some strange alarm system. When you change your
>>> hardware (say, use Linux for both pins), I think it would justify
>>> changing a DT property.
>>
>> In that case you would need additional properties anyway.
>>
>>> Btw, we are talking about very theoretical stuff here. For now (and even
>>> possibly forever) we just don't ever want to break a DT propery we
>>> introduce here, thus the bitmask.
>>
>> I don't think you need the bitmask.
>>
>> I think all you need is interrupt-names, e.g.
>>
>> dev1 {
>> 	/* both wired up */
>> 	interrupts = <&some_ic 0 47>, <&some_ic 5 62>;
>> 	interrupt-names = "INT1", "INT2";
>> }
>>
>> dev2 {
>> 	/* only INT2 wired up */
>> 	interrupts = <&some_ic 3 96>;
>> 	interrupt-names = "INT2";
>> }
>>
>> You can figure out which interrupts are wired up by trying to acquire
>> them by name, then falling back to acquiting an anonymouos interrupt
>> (assuming it's INT1) to keep compatible with existing DTBs. You can
>> choose which to use arbitrarily, try to load balance, or whatever you'd
>> like.
>>
>> If it's later necessary to route some interrupts to another device,
>> additional properties can be added to accomodate that. We already know
>> that the bitmask alone is not sufficient for that case.
>>
> 
> Yes, this sounds reasonable indeed. I like the idea. I'm sorry I won't
> rewrite patch 8/8 now. Relocation and a lot to do before holidays. I'll
> be happy to write and test this properly in one month from now, if not
> done by somebody until then.
> 
> Until then, since patches 1-7 only introduce a bindings document, they
> shouldn't be problematic for devicetree people.
> 
> 
> So if Jonathan and IIO people find anybody for review, feel free to take
> patches 1-7. In any case, there is direct register access via debugfs to
> at least somehow make the driver work for everybody ;)
> 
> so long, thanks.
>                     martin
Cool, will kick this one into the long grass until you get back most
likely!

Have a good holiday when you get to it!

Jonathan

Re: [PATCH 8/8] iio: mma8452: add devicetree property to allow all pin wirings

[Jonathan Cameron]

On 29/07/15 00:12, Martin Kepplinger wrote:
> Am 2015-07-28 um 11:28 schrieb Mark Rutland:
>> On Tue, Jul 28, 2015 at 10:11:29AM +0100, Martin Kepplinger wrote:
>>>
>>>
>>> On 2015-07-27 19:33, Mark Rutland wrote:
>>>> On Mon, Jul 27, 2015 at 03:37:48PM +0100, Martin Kepplinger wrote:
>>>>> Am 2015-07-27 um 16:23 schrieb Mark Rutland:
>>>>>> On Mon, Jul 27, 2015 at 03:08:15PM +0100, Martin Kepplinger wrote:
>>>>>>> For the devices supported by the mma8452 driver, two interrupt pins are
>>>>>>> available to route the interrupt signals to. By default INT1 is assumed.
>>>>>>>
>>>>>>> This adds a bitmask DT property for users to configure interrupt sources
>>>>>>> for INT2, if that is the wired interrupt pin for them.
>>>>>>
>>>>>> This sounds like configureation rather than a HW property. Why does this
>>>>>> need to be in the DT?
>>>>>
>>>>> It's a hardware property of the board that uses the device. There might
>>>>> be boards that connect just one of them at random, which is the reason
>>>>> for this DT property. There also might be exotic users who will want
>>>>> to use both pins to route different interrupt sources to (not yet
>>>>> supported, but no problem with such a bitmask).
>>>>
>>>> Ok, so I'm somewhat confused as to what the hardware looks like and what
>>>> this means.
>>>>
>>>> Could you elaborate on how INT1 and INT2 are used? It looks like they're
>>>> used as output pins, and so interrupt-names would seem appropriate for
>>>> describing the combination which is wired up.
>>>
>>> They are just the chip's two possible interrupt lines for us to get
>>> notified about event.
>>
>> Ok. So that matches my understanding.
>>
>>> You build a board, you use one of these 4 chips, wiring up just one of
>>> the 2 interrupt pins. By far most people won't ever need both pins.
>>>
>>> DT describes your hardware, right? So you describe how you built your
>>> board (wired the accelerometer chip) with this DT property.
>>
>> Ok.
>>
>>>> w.r.t. configuring the choice of output(s), that sounds like a runtime
>>>> decision rather than something which needs to be configured statically.
>>>
>>> This won't be useful during runtime. (De)activating events is what you
>>> do in iio sysfs.
>>>
>>> Even in the rare case (maybe supported in the future) when you want one
>>> interrupt source on one pin and another source on the other pin, that
>>> describes your hardware. You wire, say, data-ready to Linux and
>>> motion-detection to some strange alarm system. When you change your
>>> hardware (say, use Linux for both pins), I think it would justify
>>> changing a DT property.
>>
>> In that case you would need additional properties anyway.
>>
>>> Btw, we are talking about very theoretical stuff here. For now (and even
>>> possibly forever) we just don't ever want to break a DT propery we
>>> introduce here, thus the bitmask.
>>
>> I don't think you need the bitmask.
>>
>> I think all you need is interrupt-names, e.g.
>>
>> dev1 {
>> 	/* both wired up */
>> 	interrupts = <&some_ic 0 47>, <&some_ic 5 62>;
>> 	interrupt-names = "INT1", "INT2";
>> }
>>
>> dev2 {
>> 	/* only INT2 wired up */
>> 	interrupts = <&some_ic 3 96>;
>> 	interrupt-names = "INT2";
>> }
>>
>> You can figure out which interrupts are wired up by trying to acquire
>> them by name, then falling back to acquiting an anonymouos interrupt
>> (assuming it's INT1) to keep compatible with existing DTBs. You can
>> choose which to use arbitrarily, try to load balance, or whatever you'd
>> like.
>>
>> If it's later necessary to route some interrupts to another device,
>> additional properties can be added to accomodate that. We already know
>> that the bitmask alone is not sufficient for that case.
>>
> 
> Yes, this sounds reasonable indeed. I like the idea. I'm sorry I won't
> rewrite patch 8/8 now. Relocation and a lot to do before holidays. I'll
> be happy to write and test this properly in one month from now, if not
> done by somebody until then.
> 
> Until then, since patches 1-7 only introduce a bindings document, they
> shouldn't be problematic for devicetree people.
> 
> 
> So if Jonathan and IIO people find anybody for review, feel free to take
> patches 1-7. In any case, there is direct register access via debugfs to
> at least somehow make the driver work for everybody ;)
> 
> so long, thanks.
>                     martin
Cool, will kick this one into the long grass until you get back most
likely!

Have a good holiday when you get to it!

Jonathan

Re: [PATCH 8/8] iio: mma8452: add devicetree property to allow all pin wirings

[Jonathan Cameron]

On 29/07/15 00:12, Martin Kepplinger wrote:
> Am 2015-07-28 um 11:28 schrieb Mark Rutland:
>> On Tue, Jul 28, 2015 at 10:11:29AM +0100, Martin Kepplinger wrote:
>>>
>>>
>>> On 2015-07-27 19:33, Mark Rutland wrote:
>>>> On Mon, Jul 27, 2015 at 03:37:48PM +0100, Martin Kepplinger wrote:
>>>>> Am 2015-07-27 um 16:23 schrieb Mark Rutland:
>>>>>> On Mon, Jul 27, 2015 at 03:08:15PM +0100, Martin Kepplinger wrote:
>>>>>>> For the devices supported by the mma8452 driver, two interrupt pins are
>>>>>>> available to route the interrupt signals to. By default INT1 is assumed.
>>>>>>>
>>>>>>> This adds a bitmask DT property for users to configure interrupt sources
>>>>>>> for INT2, if that is the wired interrupt pin for them.
>>>>>>
>>>>>> This sounds like configureation rather than a HW property. Why does this
>>>>>> need to be in the DT?
>>>>>
>>>>> It's a hardware property of the board that uses the device. There might
>>>>> be boards that connect just one of them at random, which is the reason
>>>>> for this DT property. There also might be exotic users who will want
>>>>> to use both pins to route different interrupt sources to (not yet
>>>>> supported, but no problem with such a bitmask).
>>>>
>>>> Ok, so I'm somewhat confused as to what the hardware looks like and what
>>>> this means.
>>>>
>>>> Could you elaborate on how INT1 and INT2 are used? It looks like they're
>>>> used as output pins, and so interrupt-names would seem appropriate for
>>>> describing the combination which is wired up.
>>>
>>> They are just the chip's two possible interrupt lines for us to get
>>> notified about event.
>>
>> Ok. So that matches my understanding.
>>
>>> You build a board, you use one of these 4 chips, wiring up just one of
>>> the 2 interrupt pins. By far most people won't ever need both pins.
>>>
>>> DT describes your hardware, right? So you describe how you built your
>>> board (wired the accelerometer chip) with this DT property.
>>
>> Ok.
>>
>>>> w.r.t. configuring the choice of output(s), that sounds like a runtime
>>>> decision rather than something which needs to be configured statically.
>>>
>>> This won't be useful during runtime. (De)activating events is what you
>>> do in iio sysfs.
>>>
>>> Even in the rare case (maybe supported in the future) when you want one
>>> interrupt source on one pin and another source on the other pin, that
>>> describes your hardware. You wire, say, data-ready to Linux and
>>> motion-detection to some strange alarm system. When you change your
>>> hardware (say, use Linux for both pins), I think it would justify
>>> changing a DT property.
>>
>> In that case you would need additional properties anyway.
>>
>>> Btw, we are talking about very theoretical stuff here. For now (and even
>>> possibly forever) we just don't ever want to break a DT propery we
>>> introduce here, thus the bitmask.
>>
>> I don't think you need the bitmask.
>>
>> I think all you need is interrupt-names, e.g.
>>
>> dev1 {
>> 	/* both wired up */
>> 	interrupts = <&some_ic 0 47>, <&some_ic 5 62>;
>> 	interrupt-names = "INT1", "INT2";
>> }
>>
>> dev2 {
>> 	/* only INT2 wired up */
>> 	interrupts = <&some_ic 3 96>;
>> 	interrupt-names = "INT2";
>> }
>>
>> You can figure out which interrupts are wired up by trying to acquire
>> them by name, then falling back to acquiting an anonymouos interrupt
>> (assuming it's INT1) to keep compatible with existing DTBs. You can
>> choose which to use arbitrarily, try to load balance, or whatever you'd
>> like.
>>
>> If it's later necessary to route some interrupts to another device,
>> additional properties can be added to accomodate that. We already know
>> that the bitmask alone is not sufficient for that case.
>>
> 
> Yes, this sounds reasonable indeed. I like the idea. I'm sorry I won't
> rewrite patch 8/8 now. Relocation and a lot to do before holidays. I'll
> be happy to write and test this properly in one month from now, if not
> done by somebody until then.
> 
> Until then, since patches 1-7 only introduce a bindings document, they
> shouldn't be problematic for devicetree people.
> 
> 
> So if Jonathan and IIO people find anybody for review, feel free to take
> patches 1-7. In any case, there is direct register access via debugfs to
> at least somehow make the driver work for everybody ;)
> 
> so long, thanks.
>                     martin
Cool, will kick this one into the long grass until you get back most
likely!

Have a good holiday when you get to it!

Jonathan

[PATCHv2 8/8] iio: mma8452: add devicetree property to allow all pin wirings

[Martin Kepplinger]

For the devices supported by the mma8452 driver, two interrupt pins are
available to route the interrupt signals to. By default INT1 is assumed.

This adds a bitmask DT property for users to configure interrupt sources
for INT2, if that is the wired interrupt pin for them.

This is important for everyone to be able to use this driver, no matter
how their chip is wired. At the moment, only 0xff for using INT2 for all
available interrupt sources is supported. See the devicetree documentation
file for more details.

Since this doesn't change the default behaviour, it doesn't break anything
for existing users.

Signed-off-by: Martin Kepplinger <martin.kepplinger@theobroma-systems.com>
Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com>
---
PATCH v2 of the series' 5th version. DT cleanup and a little clearer
documentation.


 .../devicetree/bindings/iio/accel/mma8452.txt        |  3 +++
 drivers/iio/accel/mma8452.c                          | 20 +++++++++++++-------
 2 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/accel/mma8452.txt b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
index 57feb16..32f137e 100644
--- a/Documentation/devicetree/bindings/iio/accel/mma8452.txt
+++ b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
@@ -13,6 +13,8 @@ Optional properties:
 
   - interrupt-parent: should be the phandle for the interrupt controller
   - interrupts: interrupt mapping for GPIO IRQ
+  - use-int2: To use interrupt pin INT2 instead of INT1 (default), use
+    "/bits/ 8 <0xff>" here. More options might be available in the future.
 
 Example:
 
@@ -21,4 +23,5 @@ Example:
 		reg = <0x1d>;
 		interrupt-parent = <&gpio1>;
 		interrupts = <5 0>;
+		use-int2 = /bits/ 8 <0xff>;
 	};
diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 918ab59..ab40fa9 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -1028,8 +1028,9 @@ static int mma8452_probe(struct i2c_client *client,
 {
 	struct mma8452_data *data;
 	struct iio_dev *indio_dev;
-	int ret;
 	const struct of_device_id *match;
+	int ret;
+	u8 int2;
 
 	match = of_match_device(mma8452_dt_ids, &client->dev);
 	if (!match) {
@@ -1104,12 +1105,17 @@ static int mma8452_probe(struct i2c_client *client,
 		int enabled_interrupts = MMA8452_INT_TRANS |
 					 MMA8452_INT_FF_MT;
 
-		/* Assume wired to INT1 pin */
-		ret = i2c_smbus_write_byte_data(client,
-						MMA8452_CTRL_REG5,
-						supported_interrupts);
-		if (ret < 0)
-			return ret;
+		of_property_read_u8(client->dev.of_node, "use-int2", &int2);
+		if (int2 == 0xff) {
+			dev_dbg(&client->dev, "use interrupt line INT2\n");
+		} else {
+			dev_dbg(&client->dev, "use interrupt line INT1\n");
+			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,
-- 
2.1.4

[PATCHv2 8/8] iio: mma8452: add devicetree property to allow all pin wirings

[Martin Kepplinger]

For the devices supported by the mma8452 driver, two interrupt pins are
available to route the interrupt signals to. By default INT1 is assumed.

This adds a bitmask DT property for users to configure interrupt sources
for INT2, if that is the wired interrupt pin for them.

This is important for everyone to be able to use this driver, no matter
how their chip is wired. At the moment, only 0xff for using INT2 for all
available interrupt sources is supported. See the devicetree documentation
file for more details.

Since this doesn't change the default behaviour, it doesn't break anything
for existing users.

Signed-off-by: Martin Kepplinger <martin.kepplinger-SN7IsUiht6C/RdPyistoZJqQE7yCjDx5@public.gmane.org>
Signed-off-by: Christoph Muellner <christoph.muellner-SN7IsUiht6C/RdPyistoZJqQE7yCjDx5@public.gmane.org>
---
PATCH v2 of the series' 5th version. DT cleanup and a little clearer
documentation.


 .../devicetree/bindings/iio/accel/mma8452.txt        |  3 +++
 drivers/iio/accel/mma8452.c                          | 20 +++++++++++++-------
 2 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/accel/mma8452.txt b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
index 57feb16..32f137e 100644
--- a/Documentation/devicetree/bindings/iio/accel/mma8452.txt
+++ b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
@@ -13,6 +13,8 @@ Optional properties:
 
   - interrupt-parent: should be the phandle for the interrupt controller
   - interrupts: interrupt mapping for GPIO IRQ
+  - use-int2: To use interrupt pin INT2 instead of INT1 (default), use
+    "/bits/ 8 <0xff>" here. More options might be available in the future.
 
 Example:
 
@@ -21,4 +23,5 @@ Example:
 		reg = <0x1d>;
 		interrupt-parent = <&gpio1>;
 		interrupts = <5 0>;
+		use-int2 = /bits/ 8 <0xff>;
 	};
diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 918ab59..ab40fa9 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -1028,8 +1028,9 @@ static int mma8452_probe(struct i2c_client *client,
 {
 	struct mma8452_data *data;
 	struct iio_dev *indio_dev;
-	int ret;
 	const struct of_device_id *match;
+	int ret;
+	u8 int2;
 
 	match = of_match_device(mma8452_dt_ids, &client->dev);
 	if (!match) {
@@ -1104,12 +1105,17 @@ static int mma8452_probe(struct i2c_client *client,
 		int enabled_interrupts = MMA8452_INT_TRANS |
 					 MMA8452_INT_FF_MT;
 
-		/* Assume wired to INT1 pin */
-		ret = i2c_smbus_write_byte_data(client,
-						MMA8452_CTRL_REG5,
-						supported_interrupts);
-		if (ret < 0)
-			return ret;
+		of_property_read_u8(client->dev.of_node, "use-int2", &int2);
+		if (int2 == 0xff) {
+			dev_dbg(&client->dev, "use interrupt line INT2\n");
+		} else {
+			dev_dbg(&client->dev, "use interrupt line INT1\n");
+			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,
-- 
2.1.4

Re: [PATCH v5 0/8] iio: mma8452: improve driver and support more chips

[Jonathan Cameron]

Hi Martin,

Sorry to say this has crossed with a cleanup set from Hartmut.
I'm afraid I applied his first.  This will all need rebasing.

There weren't any fundamental changes in Hartmut's series, but
I'd feel nervous to try and apply yours on top without hardware to
sanity check I hadn't messed it up.

Jonathan

On 27/07/15 15:08, Martin Kepplinger wrote:
> Before I go away for holidays, this is version 5 of the mma8452 driver
> improvements. This version removes the unnessasary DRIVER_NAME define and
> improves the DT property "use_int2" to be extendible for any future additions
> to the driver.
> 
> This is based on today's -next.
> 
> 
> These changes add support for motion interrupts and 3 more accelerometer
> chips, two of which use them because they don't support the until now
> included transient interrupt sources:
> 
> MMA8453Q, MMA8652FC and MMA8653FC; datasheets are in the commit messages.
> The driver and module name remains the same, seperating it from the device
> names it now supports.
> 
> On top of this, there are minor documentation additions, and more notably,
> it allows to use the driver, no matter how the interrupt pins are wired
> on your board.
> 
> Please review and test if you can. For MMA8452Q, nothing should have
> changed.
> 
> revision history
> ----------------
> v5   DRIVER_NAME define removed; more flexible DT property
> v4   cleanup; one bugfix patch removed from series; DT people added
> v3   adds one patch to allow all possible pin wirings; adds more email
> recipients
> v2   splits the work into a series of smaller pieces
> v1   initial post
> 
> 
> --
> 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 v5 0/8] iio: mma8452: improve driver and support more chips

[Jonathan Cameron]

Hi Martin,

Sorry to say this has crossed with a cleanup set from Hartmut.
I'm afraid I applied his first.  This will all need rebasing.

There weren't any fundamental changes in Hartmut's series, but
I'd feel nervous to try and apply yours on top without hardware to
sanity check I hadn't messed it up.

Jonathan

On 27/07/15 15:08, Martin Kepplinger wrote:
> Before I go away for holidays, this is version 5 of the mma8452 driver
> improvements. This version removes the unnessasary DRIVER_NAME define and
> improves the DT property "use_int2" to be extendible for any future additions
> to the driver.
> 
> This is based on today's -next.
> 
> 
> These changes add support for motion interrupts and 3 more accelerometer
> chips, two of which use them because they don't support the until now
> included transient interrupt sources:
> 
> MMA8453Q, MMA8652FC and MMA8653FC; datasheets are in the commit messages.
> The driver and module name remains the same, seperating it from the device
> names it now supports.
> 
> On top of this, there are minor documentation additions, and more notably,
> it allows to use the driver, no matter how the interrupt pins are wired
> on your board.
> 
> Please review and test if you can. For MMA8452Q, nothing should have
> changed.
> 
> revision history
> ----------------
> v5   DRIVER_NAME define removed; more flexible DT property
> v4   cleanup; one bugfix patch removed from series; DT people added
> v3   adds one patch to allow all possible pin wirings; adds more email
> recipients
> v2   splits the work into a series of smaller pieces
> v1   initial post
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

[PATCH v4 0/8] iio:mma8452: improve driver and support more chips

[Martin Kepplinger]

This is version 4 of the mma8452 driver improvements. This version removes
one patch that changed the iio event type for existing users. It can be
dealt with seperately and this series applies without it.

Also, changes according to Jonathan's review are applied and a few
devicetree maintainers are added as recipients.


These changes add support for motion interrupts and 3 more accelerometer
chips, two of which use them because they don't support the until now
included transient interrupt sources:

MMA8453Q, MMA8652FC and MMA8653FC; datasheets are in the commit messages.

The driver and module name remains the same, seperating it from the device
names it now supports.

On top of this, there are minor documentation additions, and more notably,
 it allows to use the driver, no matter how the interrupt pins are wired
on your board.

Please review and test if you can. For MMA8452Q, nothing should have
changed.

revision history
----------------
v4   cleanup; one bugfix patch removed from series; DT people added
v3   adds one patch to allow all possible pin wirings; adds more email
     recipients
v2   splits the work into a series of smaller pieces
v1   initial post

[PATCH 8/8] iio: mma8452: add devicetree property to allow all pin wirings

[Martin Kepplinger]

For the devices supported by the mma8452 driver, two interrupt pins are
available to route the interrupt signals to. By default INT1 is assumed.

This adds a simple boolean DT property, for users to configure it for
INT2, if that is the wired interrupt pin for them.

This is important for everyone to be able to use this driver, no matter
how their chip is wired.

Since this doesn't change the default behaviour, it doesn't break anything
for existing users.

Signed-off-by: Martin Kepplinger <martin.kepplinger@theobroma-systems.com>
Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com>
---
 Documentation/devicetree/bindings/iio/accel/mma8452.txt |  2 ++
 drivers/iio/accel/mma8452.c                             | 14 ++++++++------
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/accel/mma8452.txt b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
index 8d98e05..9bad1fc 100644
--- a/Documentation/devicetree/bindings/iio/accel/mma8452.txt
+++ b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
@@ -10,6 +10,7 @@ Optional properties:
 
   - interrupt-parent: should be the phandle for the interrupt controller
   - interrupts: interrupt mapping for GPIO IRQ
+  - use_int2: assume interrupt pin wired to INT2 instead of INT1
 
 Example:
 
@@ -18,4 +19,5 @@ Example:
 		reg = <0x1d>;
 		interrupt-parent = <&gpio1>;
 		interrupts = <5 0>;
+		use_int2;
 	};
diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 2b8ed67..f8ba146 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -1105,12 +1105,14 @@ static int mma8452_probe(struct i2c_client *client,
 		int enabled_interrupts = MMA8452_INT_TRANS |
 					 MMA8452_INT_FF_MT;
 
-		/* Assume wired to INT1 pin */
-		ret = i2c_smbus_write_byte_data(client,
-						MMA8452_CTRL_REG5,
-						supported_interrupts);
-		if (ret < 0)
-			return ret;
+		/* Assume wired to INT1 pin, except "use_int2" is found in DT */
+		if (!of_property_read_bool(client->dev.of_node, "use_int2")) {
+			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,
-- 
2.1.4

[PATCH 8/8] iio: mma8452: add devicetree property to allow all pin wirings

[Martin Kepplinger]

For the devices supported by the mma8452 driver, two interrupt pins are
available to route the interrupt signals to. By default INT1 is assumed.

This adds a simple boolean DT property, for users to configure it for
INT2, if that is the wired interrupt pin for them.

This is important for everyone to be able to use this driver, no matter
how their chip is wired.

Since this doesn't change the default behaviour, it doesn't break anything
for existing users.

Signed-off-by: Martin Kepplinger <martin.kepplinger-SN7IsUiht6C/RdPyistoZJqQE7yCjDx5@public.gmane.org>
Signed-off-by: Christoph Muellner <christoph.muellner-SN7IsUiht6C/RdPyistoZJqQE7yCjDx5@public.gmane.org>
---
 Documentation/devicetree/bindings/iio/accel/mma8452.txt |  2 ++
 drivers/iio/accel/mma8452.c                             | 14 ++++++++------
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/accel/mma8452.txt b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
index 8d98e05..9bad1fc 100644
--- a/Documentation/devicetree/bindings/iio/accel/mma8452.txt
+++ b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
@@ -10,6 +10,7 @@ Optional properties:
 
   - interrupt-parent: should be the phandle for the interrupt controller
   - interrupts: interrupt mapping for GPIO IRQ
+  - use_int2: assume interrupt pin wired to INT2 instead of INT1
 
 Example:
 
@@ -18,4 +19,5 @@ Example:
 		reg = <0x1d>;
 		interrupt-parent = <&gpio1>;
 		interrupts = <5 0>;
+		use_int2;
 	};
diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 2b8ed67..f8ba146 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -1105,12 +1105,14 @@ static int mma8452_probe(struct i2c_client *client,
 		int enabled_interrupts = MMA8452_INT_TRANS |
 					 MMA8452_INT_FF_MT;
 
-		/* Assume wired to INT1 pin */
-		ret = i2c_smbus_write_byte_data(client,
-						MMA8452_CTRL_REG5,
-						supported_interrupts);
-		if (ret < 0)
-			return ret;
+		/* Assume wired to INT1 pin, except "use_int2" is found in DT */
+		if (!of_property_read_bool(client->dev.of_node, "use_int2")) {
+			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,
-- 
2.1.4

Re: [PATCH 8/8] iio: mma8452: add devicetree property to allow all pin wirings

[Jonathan Cameron]

On 06/07/15 13:34, Martin Kepplinger wrote:
> For the devices supported by the mma8452 driver, two interrupt pins are
> available to route the interrupt signals to. By default INT1 is assumed.
> 
> This adds a simple boolean DT property, for users to configure it for
> INT2, if that is the wired interrupt pin for them.
> 
> This is important for everyone to be able to use this driver, no matter
> how their chip is wired.
> 
> Since this doesn't change the default behaviour, it doesn't break anything
> for existing users.
> 
> Signed-off-by: Martin Kepplinger <martin.kepplinger@theobroma-systems.com>
> Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com>
The whole series looks good to me.  Just want those acks from Peter and / or
Martin before I apply it.

Thanks,

Jonathan
> ---
>  Documentation/devicetree/bindings/iio/accel/mma8452.txt |  2 ++
>  drivers/iio/accel/mma8452.c                             | 14 ++++++++------
>  2 files changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/iio/accel/mma8452.txt b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
> index 8d98e05..9bad1fc 100644
> --- a/Documentation/devicetree/bindings/iio/accel/mma8452.txt
> +++ b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
> @@ -10,6 +10,7 @@ Optional properties:
>  
>    - interrupt-parent: should be the phandle for the interrupt controller
>    - interrupts: interrupt mapping for GPIO IRQ
> +  - use_int2: assume interrupt pin wired to INT2 instead of INT1
>  
>  Example:
>  
> @@ -18,4 +19,5 @@ Example:
>  		reg = <0x1d>;
>  		interrupt-parent = <&gpio1>;
>  		interrupts = <5 0>;
> +		use_int2;
>  	};
> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
> index 2b8ed67..f8ba146 100644
> --- a/drivers/iio/accel/mma8452.c
> +++ b/drivers/iio/accel/mma8452.c
> @@ -1105,12 +1105,14 @@ static int mma8452_probe(struct i2c_client *client,
>  		int enabled_interrupts = MMA8452_INT_TRANS |
>  					 MMA8452_INT_FF_MT;
>  
> -		/* Assume wired to INT1 pin */
> -		ret = i2c_smbus_write_byte_data(client,
> -						MMA8452_CTRL_REG5,
> -						supported_interrupts);
> -		if (ret < 0)
> -			return ret;
> +		/* Assume wired to INT1 pin, except "use_int2" is found in DT */
> +		if (!of_property_read_bool(client->dev.of_node, "use_int2")) {
> +			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,
> 

Re: [PATCH 8/8] iio: mma8452: add devicetree property to allow all pin wirings

[Jonathan Cameron]

On 06/07/15 13:34, Martin Kepplinger wrote:
> For the devices supported by the mma8452 driver, two interrupt pins are
> available to route the interrupt signals to. By default INT1 is assumed.
> 
> This adds a simple boolean DT property, for users to configure it for
> INT2, if that is the wired interrupt pin for them.
> 
> This is important for everyone to be able to use this driver, no matter
> how their chip is wired.
> 
> Since this doesn't change the default behaviour, it doesn't break anything
> for existing users.
> 
> Signed-off-by: Martin Kepplinger <martin.kepplinger-SN7IsUiht6C/RdPyistoZJqQE7yCjDx5@public.gmane.org>
> Signed-off-by: Christoph Muellner <christoph.muellner-SN7IsUiht6C/RdPyistoZJqQE7yCjDx5@public.gmane.org>
The whole series looks good to me.  Just want those acks from Peter and / or
Martin before I apply it.

Thanks,

Jonathan
> ---
>  Documentation/devicetree/bindings/iio/accel/mma8452.txt |  2 ++
>  drivers/iio/accel/mma8452.c                             | 14 ++++++++------
>  2 files changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/iio/accel/mma8452.txt b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
> index 8d98e05..9bad1fc 100644
> --- a/Documentation/devicetree/bindings/iio/accel/mma8452.txt
> +++ b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
> @@ -10,6 +10,7 @@ Optional properties:
>  
>    - interrupt-parent: should be the phandle for the interrupt controller
>    - interrupts: interrupt mapping for GPIO IRQ
> +  - use_int2: assume interrupt pin wired to INT2 instead of INT1
>  
>  Example:
>  
> @@ -18,4 +19,5 @@ Example:
>  		reg = <0x1d>;
>  		interrupt-parent = <&gpio1>;
>  		interrupts = <5 0>;
> +		use_int2;
>  	};
> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
> index 2b8ed67..f8ba146 100644
> --- a/drivers/iio/accel/mma8452.c
> +++ b/drivers/iio/accel/mma8452.c
> @@ -1105,12 +1105,14 @@ static int mma8452_probe(struct i2c_client *client,
>  		int enabled_interrupts = MMA8452_INT_TRANS |
>  					 MMA8452_INT_FF_MT;
>  
> -		/* Assume wired to INT1 pin */
> -		ret = i2c_smbus_write_byte_data(client,
> -						MMA8452_CTRL_REG5,
> -						supported_interrupts);
> -		if (ret < 0)
> -			return ret;
> +		/* Assume wired to INT1 pin, except "use_int2" is found in DT */
> +		if (!of_property_read_bool(client->dev.of_node, "use_int2")) {
> +			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,
> 

Re: [PATCH 8/8] iio: mma8452: add devicetree property to allow all pin wirings

[Martin Kepplinger]

Am 2015-07-19 um 15:47 schrieb Jonathan Cameron:
> On 06/07/15 13:34, Martin Kepplinger wrote:
>> For the devices supported by the mma8452 driver, two interrupt pins are
>> available to route the interrupt signals to. By default INT1 is assumed.
>>
>> This adds a simple boolean DT property, for users to configure it for
>> INT2, if that is the wired interrupt pin for them.
>>
>> This is important for everyone to be able to use this driver, no matter
>> how their chip is wired.
>>
>> Since this doesn't change the default behaviour, it doesn't break anything
>> for existing users.
>>
>> Signed-off-by: Martin Kepplinger <martin.kepplinger@theobroma-systems.com>
>> Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com>
> The whole series looks good to me.  Just want those acks from Peter and / or
> Martin before I apply it.
> 
> Thanks,
> 
> Jonathan

Peter at least replied to the first version of these patches, so he
should be around. I suspect that holidays get in the way now.

While I'm at it: I'll be on holidays pretty much all of august. In
general I could maintain the driver and have more improvements planned
on top of this patchset. Maybe I'll include a maintainers file entry
with those (later this year).

And: It shouldn't apply cleanly at least on your fixes-togreg branch of
iio.git. I don't know if this is in -next but in case you don't want to
resolve the diff, please feel free to ask for a clean version of the
whole patchset against -next at any time.

thanks,

                  martin

>> ---
>>  Documentation/devicetree/bindings/iio/accel/mma8452.txt |  2 ++
>>  drivers/iio/accel/mma8452.c                             | 14 ++++++++------
>>  2 files changed, 10 insertions(+), 6 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/iio/accel/mma8452.txt b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
>> index 8d98e05..9bad1fc 100644
>> --- a/Documentation/devicetree/bindings/iio/accel/mma8452.txt
>> +++ b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
>> @@ -10,6 +10,7 @@ Optional properties:
>>  
>>    - interrupt-parent: should be the phandle for the interrupt controller
>>    - interrupts: interrupt mapping for GPIO IRQ
>> +  - use_int2: assume interrupt pin wired to INT2 instead of INT1
>>  
>>  Example:
>>  
>> @@ -18,4 +19,5 @@ Example:
>>  		reg = <0x1d>;
>>  		interrupt-parent = <&gpio1>;
>>  		interrupts = <5 0>;
>> +		use_int2;
>>  	};
>> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
>> index 2b8ed67..f8ba146 100644
>> --- a/drivers/iio/accel/mma8452.c
>> +++ b/drivers/iio/accel/mma8452.c
>> @@ -1105,12 +1105,14 @@ static int mma8452_probe(struct i2c_client *client,
>>  		int enabled_interrupts = MMA8452_INT_TRANS |
>>  					 MMA8452_INT_FF_MT;
>>  
>> -		/* Assume wired to INT1 pin */
>> -		ret = i2c_smbus_write_byte_data(client,
>> -						MMA8452_CTRL_REG5,
>> -						supported_interrupts);
>> -		if (ret < 0)
>> -			return ret;
>> +		/* Assume wired to INT1 pin, except "use_int2" is found in DT */
>> +		if (!of_property_read_bool(client->dev.of_node, "use_int2")) {
>> +			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,
>>
> 

Re: [PATCH 8/8] iio: mma8452: add devicetree property to allow all pin wirings

[Martin Kepplinger]

Am 2015-07-19 um 15:47 schrieb Jonathan Cameron:
> On 06/07/15 13:34, Martin Kepplinger wrote:
>> For the devices supported by the mma8452 driver, two interrupt pins are
>> available to route the interrupt signals to. By default INT1 is assumed.
>>
>> This adds a simple boolean DT property, for users to configure it for
>> INT2, if that is the wired interrupt pin for them.
>>
>> This is important for everyone to be able to use this driver, no matter
>> how their chip is wired.
>>
>> Since this doesn't change the default behaviour, it doesn't break anything
>> for existing users.
>>
>> Signed-off-by: Martin Kepplinger <martin.kepplinger-SN7IsUiht6C/RdPyistoZJqQE7yCjDx5@public.gmane.org>
>> Signed-off-by: Christoph Muellner <christoph.muellner-SN7IsUiht6C/RdPyistoZJqQE7yCjDx5@public.gmane.org>
> The whole series looks good to me.  Just want those acks from Peter and / or
> Martin before I apply it.
> 
> Thanks,
> 
> Jonathan

Peter at least replied to the first version of these patches, so he
should be around. I suspect that holidays get in the way now.

While I'm at it: I'll be on holidays pretty much all of august. In
general I could maintain the driver and have more improvements planned
on top of this patchset. Maybe I'll include a maintainers file entry
with those (later this year).

And: It shouldn't apply cleanly at least on your fixes-togreg branch of
iio.git. I don't know if this is in -next but in case you don't want to
resolve the diff, please feel free to ask for a clean version of the
whole patchset against -next at any time.

thanks,

                  martin

>> ---
>>  Documentation/devicetree/bindings/iio/accel/mma8452.txt |  2 ++
>>  drivers/iio/accel/mma8452.c                             | 14 ++++++++------
>>  2 files changed, 10 insertions(+), 6 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/iio/accel/mma8452.txt b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
>> index 8d98e05..9bad1fc 100644
>> --- a/Documentation/devicetree/bindings/iio/accel/mma8452.txt
>> +++ b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
>> @@ -10,6 +10,7 @@ Optional properties:
>>  
>>    - interrupt-parent: should be the phandle for the interrupt controller
>>    - interrupts: interrupt mapping for GPIO IRQ
>> +  - use_int2: assume interrupt pin wired to INT2 instead of INT1
>>  
>>  Example:
>>  
>> @@ -18,4 +19,5 @@ Example:
>>  		reg = <0x1d>;
>>  		interrupt-parent = <&gpio1>;
>>  		interrupts = <5 0>;
>> +		use_int2;
>>  	};
>> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
>> index 2b8ed67..f8ba146 100644
>> --- a/drivers/iio/accel/mma8452.c
>> +++ b/drivers/iio/accel/mma8452.c
>> @@ -1105,12 +1105,14 @@ static int mma8452_probe(struct i2c_client *client,
>>  		int enabled_interrupts = MMA8452_INT_TRANS |
>>  					 MMA8452_INT_FF_MT;
>>  
>> -		/* Assume wired to INT1 pin */
>> -		ret = i2c_smbus_write_byte_data(client,
>> -						MMA8452_CTRL_REG5,
>> -						supported_interrupts);
>> -		if (ret < 0)
>> -			return ret;
>> +		/* Assume wired to INT1 pin, except "use_int2" is found in DT */
>> +		if (!of_property_read_bool(client->dev.of_node, "use_int2")) {
>> +			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,
>>
> 

Re: [PATCH 8/8] iio: mma8452: add devicetree property to allow all pin wirings

[Martin Fuzzey]

On 19/07/15 15:47, Jonathan Cameron wrote:
> On 06/07/15 13:34, Martin Kepplinger wrote:
>> For the devices supported by the mma8452 driver, two interrupt pins are
>> available to route the interrupt signals to. By default INT1 is assumed.
>>
>> This adds a simple boolean DT property, for users to configure it for
>> INT2, if that is the wired interrupt pin for them.
>>
>> This is important for everyone to be able to use this driver, no matter
>> how their chip is wired.
>>
>> Since this doesn't change the default behaviour, it doesn't break anything
>> for existing users.
A remark about this.

The hardware allows each of the interrupt sources (threshold, freefall, 
data available, wakeup, ...) to be individiually mapped to one of the 
two available interrupt pins.
This patch only allows one pin or the other one to be selected for all 
the sources.

Granted this will certainly cover 99% of use cases (even if both are 
wired that means we can use either).
However there is one possible case not covered which is when one of the 
interrupt signals is connected to some other circuitry (alarm, 
power,...) in addition to being used as an interrupt to the CPU.

Normally for code I'd just say let's cross that bridge when we come to 
it given that it's pretty unlikely.
However since this affects the DT binding and they can be awkward to 
change while keeping compatibility maybe it would be better to use a 
bitmask DT property rather than a boolean to actually describe the full 
hardware capabilities and keep that door open?

Of course, actually supporting arbitary mappings would be a bit more 
work in the driver (both irqs would need to requested...).
However it would be possible to use a bitmask in the DT but, for the 
moment, only accept all zeros or all ones, leaving the rest for a future 
driver update if and when it is needed.

I don't have a strong objection to doing it the current way, it's 
certainly the simplest way and it's likely to be all we ever need just 
pointing out a possible shortcoming.

I think It would be good to have the DT maintainers view on this.

>> Signed-off-by: Martin Kepplinger <martin.kepplinger@theobroma-systems.com>
>> Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com>
> The whole series looks good to me.  Just want those acks from Peter and / or
> Martin before I apply it.
I'll try to give it a spin this week. Can't promise though


Regards,

Martin

Re: [PATCH 8/8] iio: mma8452: add devicetree property to allow all pin wirings

[Martin Fuzzey]

On 19/07/15 15:47, Jonathan Cameron wrote:
> On 06/07/15 13:34, Martin Kepplinger wrote:
>> For the devices supported by the mma8452 driver, two interrupt pins are
>> available to route the interrupt signals to. By default INT1 is assumed.
>>
>> This adds a simple boolean DT property, for users to configure it for
>> INT2, if that is the wired interrupt pin for them.
>>
>> This is important for everyone to be able to use this driver, no matter
>> how their chip is wired.
>>
>> Since this doesn't change the default behaviour, it doesn't break anything
>> for existing users.
A remark about this.

The hardware allows each of the interrupt sources (threshold, freefall, 
data available, wakeup, ...) to be individiually mapped to one of the 
two available interrupt pins.
This patch only allows one pin or the other one to be selected for all 
the sources.

Granted this will certainly cover 99% of use cases (even if both are 
wired that means we can use either).
However there is one possible case not covered which is when one of the 
interrupt signals is connected to some other circuitry (alarm, 
power,...) in addition to being used as an interrupt to the CPU.

Normally for code I'd just say let's cross that bridge when we come to 
it given that it's pretty unlikely.
However since this affects the DT binding and they can be awkward to 
change while keeping compatibility maybe it would be better to use a 
bitmask DT property rather than a boolean to actually describe the full 
hardware capabilities and keep that door open?

Of course, actually supporting arbitary mappings would be a bit more 
work in the driver (both irqs would need to requested...).
However it would be possible to use a bitmask in the DT but, for the 
moment, only accept all zeros or all ones, leaving the rest for a future 
driver update if and when it is needed.

I don't have a strong objection to doing it the current way, it's 
certainly the simplest way and it's likely to be all we ever need just 
pointing out a possible shortcoming.

I think It would be good to have the DT maintainers view on this.

>> Signed-off-by: Martin Kepplinger <martin.kepplinger-SN7IsUiht6C/RdPyistoZJqQE7yCjDx5@public.gmane.org>
>> Signed-off-by: Christoph Muellner <christoph.muellner-SN7IsUiht6C/RdPyistoZJqQE7yCjDx5@public.gmane.org>
> The whole series looks good to me.  Just want those acks from Peter and / or
> Martin before I apply it.
I'll try to give it a spin this week. Can't promise though


Regards,

Martin

Re: [PATCH 8/8] iio: mma8452: add devicetree property to allow all pin wirings

[Martin Kepplinger]

On 2015-07-20 10:38, Martin Fuzzey wrote:
> On 19/07/15 15:47, Jonathan Cameron wrote:
>> On 06/07/15 13:34, Martin Kepplinger wrote:
>>> For the devices supported by the mma8452 driver, two interrupt pins are
>>> available to route the interrupt signals to. By default INT1 is assumed.
>>>
>>> This adds a simple boolean DT property, for users to configure it for
>>> INT2, if that is the wired interrupt pin for them.
>>>
>>> This is important for everyone to be able to use this driver, no matter
>>> how their chip is wired.
>>>
>>> Since this doesn't change the default behaviour, it doesn't break
>>> anything
>>> for existing users.
> A remark about this.
> 
> The hardware allows each of the interrupt sources (threshold, freefall,
> data available, wakeup, ...) to be individiually mapped to one of the
> two available interrupt pins.
> This patch only allows one pin or the other one to be selected for all
> the sources.
> 
> Granted this will certainly cover 99% of use cases (even if both are
> wired that means we can use either).
> However there is one possible case not covered which is when one of the
> interrupt signals is connected to some other circuitry (alarm,
> power,...) in addition to being used as an interrupt to the CPU.
> 
> Normally for code I'd just say let's cross that bridge when we come to
> it given that it's pretty unlikely.
> However since this affects the DT binding and they can be awkward to
> change while keeping compatibility maybe it would be better to use a
> bitmask DT property rather than a boolean to actually describe the full
> hardware capabilities and keep that door open?
> 
> Of course, actually supporting arbitary mappings would be a bit more
> work in the driver (both irqs would need to requested...).
> However it would be possible to use a bitmask in the DT but, for the
> moment, only accept all zeros or all ones, leaving the rest for a future
> driver update if and when it is needed.
> 
> I don't have a strong objection to doing it the current way, it's
> certainly the simplest way and it's likely to be all we ever need just
> pointing out a possible shortcoming.
> 
> I think It would be good to have the DT maintainers view on this.

Your're right. I thought about it, and maybe the gain in simplicity is
not worth losing the theoretical option of supporting "weird" users.

I'll probably do another version after your or Peter's review and will
change this.

> 
>>> Signed-off-by: Martin Kepplinger
>>> <martin.kepplinger@theobroma-systems.com>
>>> Signed-off-by: Christoph Muellner
>>> <christoph.muellner@theobroma-systems.com>
>> The whole series looks good to me.  Just want those acks from Peter
>> and / or
>> Martin before I apply it.
> I'll try to give it a spin this week. Can't promise though
> 

Thanks so far. It *really* won't take you long to review ;)

> 
> Regards,
> 
> Martin
> 

Re: [PATCH 8/8] iio: mma8452: add devicetree property to allow all pin wirings

[Martin Kepplinger]

On 2015-07-20 10:38, Martin Fuzzey wrote:
> On 19/07/15 15:47, Jonathan Cameron wrote:
>> On 06/07/15 13:34, Martin Kepplinger wrote:
>>> For the devices supported by the mma8452 driver, two interrupt pins are
>>> available to route the interrupt signals to. By default INT1 is assumed.
>>>
>>> This adds a simple boolean DT property, for users to configure it for
>>> INT2, if that is the wired interrupt pin for them.
>>>
>>> This is important for everyone to be able to use this driver, no matter
>>> how their chip is wired.
>>>
>>> Since this doesn't change the default behaviour, it doesn't break
>>> anything
>>> for existing users.
> A remark about this.
> 
> The hardware allows each of the interrupt sources (threshold, freefall,
> data available, wakeup, ...) to be individiually mapped to one of the
> two available interrupt pins.
> This patch only allows one pin or the other one to be selected for all
> the sources.
> 
> Granted this will certainly cover 99% of use cases (even if both are
> wired that means we can use either).
> However there is one possible case not covered which is when one of the
> interrupt signals is connected to some other circuitry (alarm,
> power,...) in addition to being used as an interrupt to the CPU.
> 
> Normally for code I'd just say let's cross that bridge when we come to
> it given that it's pretty unlikely.
> However since this affects the DT binding and they can be awkward to
> change while keeping compatibility maybe it would be better to use a
> bitmask DT property rather than a boolean to actually describe the full
> hardware capabilities and keep that door open?
> 
> Of course, actually supporting arbitary mappings would be a bit more
> work in the driver (both irqs would need to requested...).
> However it would be possible to use a bitmask in the DT but, for the
> moment, only accept all zeros or all ones, leaving the rest for a future
> driver update if and when it is needed.
> 
> I don't have a strong objection to doing it the current way, it's
> certainly the simplest way and it's likely to be all we ever need just
> pointing out a possible shortcoming.
> 
> I think It would be good to have the DT maintainers view on this.

Your're right. I thought about it, and maybe the gain in simplicity is
not worth losing the theoretical option of supporting "weird" users.

I'll probably do another version after your or Peter's review and will
change this.

> 
>>> Signed-off-by: Martin Kepplinger
>>> <martin.kepplinger-SN7IsUiht6C/RdPyistoZJqQE7yCjDx5@public.gmane.org>
>>> Signed-off-by: Christoph Muellner
>>> <christoph.muellner-SN7IsUiht6C/RdPyistoZJqQE7yCjDx5@public.gmane.org>
>> The whole series looks good to me.  Just want those acks from Peter
>> and / or
>> Martin before I apply it.
> I'll try to give it a spin this week. Can't promise though
> 

Thanks so far. It *really* won't take you long to review ;)

> 
> Regards,
> 
> Martin
>