[PATCH v4 0/3] iio: bmc150: refactoring in preparation for hardware fifo support

[PATCH v4 0/3] iio: bmc150: refactoring in preparation for hardware fifo support

[Octavian Purdila]

Hi Jonathan,

Here is a new version for the remaining bmc150 refactor patches that
were not merged. I've also added to this set a fix from Sathya which
has been sitting in our tree.

As you requested, I've split the refactoring series from the hardware
fifo series. I will send the v4 hwfifo series shortly.

Thanks,
Tavi

Changes since v3:

* added enums for interrupts and triggers

* added fix from Sathya



Octavian Purdila (2):
  iio: bmc150: introduce bmc150_accel_interrupt
  iio: bmc150: introduce bmc150_accel_trigger

Sathyanarayanan Kuppuswamy (1):
  iio: bmc150: change sampling frequency

 drivers/iio/accel/bmc150-accel.c | 294 ++++++++++++++++++++++++---------------
 1 file changed, 178 insertions(+), 116 deletions(-)

-- 
1.9.1

[PATCH v4 1/3] iio: bmc150: change sampling frequency

[Octavian Purdila]

From: Sathyanarayanan Kuppuswamy <sathyanarayanan.kuppuswamy@intel.com>

Currently driver reports device bandwidth list as available
sampling frequency. But sampling frequency is actually twice
the device bandwidth. This patch fixes this issue.

Signed-off-by: Sathyanarayanan Kuppuswamy <sathyanarayanan.kuppuswamy@intel.com>
---
 drivers/iio/accel/bmc150-accel.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/iio/accel/bmc150-accel.c b/drivers/iio/accel/bmc150-accel.c
index 8004ce3..74ee54e 100644
--- a/drivers/iio/accel/bmc150-accel.c
+++ b/drivers/iio/accel/bmc150-accel.c
@@ -168,14 +168,14 @@ static const struct {
 	int val;
 	int val2;
 	u8 bw_bits;
-} bmc150_accel_samp_freq_table[] = { {7, 810000, 0x08},
-				     {15, 630000, 0x09},
-				     {31, 250000, 0x0A},
-				     {62, 500000, 0x0B},
-				     {125, 0, 0x0C},
-				     {250, 0, 0x0D},
-				     {500, 0, 0x0E},
-				     {1000, 0, 0x0F} };
+} bmc150_accel_samp_freq_table[] = { {15, 620000, 0x08},
+				     {31, 260000, 0x09},
+				     {62, 500000, 0x0A},
+				     {125, 0, 0x0B},
+				     {250, 0, 0x0C},
+				     {500, 0, 0x0D},
+				     {1000, 0, 0x0E},
+				     {2000, 0, 0x0F} };
 
 static const struct {
 	int bw_bits;
@@ -783,7 +783,7 @@ static int bmc150_accel_validate_trigger(struct iio_dev *indio_dev,
 }
 
 static IIO_CONST_ATTR_SAMP_FREQ_AVAIL(
-		"7.810000 15.630000 31.250000 62.500000 125 250 500 1000");
+		"15.620000 31.260000 62.50000 125 250 500 1000 2000");
 
 static struct attribute *bmc150_accel_attributes[] = {
 	&iio_const_attr_sampling_frequency_available.dev_attr.attr,
-- 
1.9.1

[PATCH v4 2/3] iio: bmc150: introduce bmc150_accel_interrupt

[Octavian Purdila]

Since both triggers and events can share an interrupt, add a data
structure that tracks the users of an interrupt so that it enables or
disables it only for the first users and respectively last user.

This will allows us to easily add more events or triggers.

The patch also adds an interrupt enabled counter, so that we can
easily know if we need to put the device in normal mode when the
resume callback is issued.

Signed-off-by: Octavian Purdila <octavian.purdila@intel.com>
---
 drivers/iio/accel/bmc150-accel.c | 86 ++++++++++++++++++++++++----------------
 1 file changed, 51 insertions(+), 35 deletions(-)

diff --git a/drivers/iio/accel/bmc150-accel.c b/drivers/iio/accel/bmc150-accel.c
index 74ee54e..70497c3 100644
--- a/drivers/iio/accel/bmc150-accel.c
+++ b/drivers/iio/accel/bmc150-accel.c
@@ -147,10 +147,24 @@ struct bmc150_accel_chip_info {
 	const struct bmc150_scale_info scale_table[4];
 };
 
+struct bmc150_accel_interrupt {
+	const struct bmc150_accel_interrupt_info *info;
+	atomic_t users;
+};
+
+enum bmc150_accel_interrupt_id {
+	BMC150_ACCEL_INT_DATA_READY,
+	BMC150_ACCEL_INT_ANY_MOTION,
+	BMC150_ACCEL_INT_WATERMARK,
+	BMC150_ACCEL_INTERRUPTS,
+};
+
 struct bmc150_accel_data {
 	struct i2c_client *client;
+	struct bmc150_accel_interrupt interrupts[BMC150_ACCEL_INTERRUPTS];
 	struct iio_trigger *dready_trig;
 	struct iio_trigger *motion_trig;
+	atomic_t active_intr;
 	struct mutex mutex;
 	s16 buffer[8];
 	u8 bw_bits;
@@ -421,7 +435,7 @@ static const struct bmc150_accel_interrupt_info {
 	u8 map_bitmask;
 	u8 en_reg;
 	u8 en_bitmask;
-} bmc150_accel_interrupts[] = {
+} bmc150_accel_interrupts[BMC150_ACCEL_INTERRUPTS] = {
 	{ /* data ready interrupt */
 		.map_reg = BMC150_ACCEL_REG_INT_MAP_1,
 		.map_bitmask = BMC150_ACCEL_INT_MAP_1_BIT_DATA,
@@ -438,12 +452,30 @@ static const struct bmc150_accel_interrupt_info {
 	},
 };
 
-static int bmc150_accel_set_interrupt(struct bmc150_accel_data *data,
-				const struct bmc150_accel_interrupt_info *info,
+static void bmc150_accel_interrupts_setup(struct iio_dev *indio_dev,
+					  struct bmc150_accel_data *data)
+{
+	int i;
+
+	for (i = 0; i < BMC150_ACCEL_INTERRUPTS; i++)
+		data->interrupts[i].info = &bmc150_accel_interrupts[i];
+}
+
+static int bmc150_accel_set_interrupt(struct bmc150_accel_data *data, int i,
 				      bool state)
 {
+	struct bmc150_accel_interrupt *intr = &data->interrupts[i];
+	const struct bmc150_accel_interrupt_info *info = intr->info;
 	int ret;
 
+	if (state) {
+		if (atomic_inc_return(&intr->users) > 1)
+			return 0;
+	} else {
+		if (atomic_dec_return(&intr->users) > 0)
+			return 0;
+	}
+
 	/*
 	 * We will expect the enable and disable to do operation in
 	 * in reverse order. This will happen here anyway as our
@@ -493,6 +525,11 @@ static int bmc150_accel_set_interrupt(struct bmc150_accel_data *data,
 		goto out_fix_power_state;
 	}
 
+	if (state)
+		atomic_inc(&data->active_intr);
+	else
+		atomic_dec(&data->active_intr);
+
 	return 0;
 
 out_fix_power_state:
@@ -500,20 +537,6 @@ out_fix_power_state:
 	return ret;
 }
 
-static int bmc150_accel_setup_any_motion_interrupt(
-					struct bmc150_accel_data *data,
-					bool status)
-{
-	return bmc150_accel_set_interrupt(data, &bmc150_accel_interrupts[1],
-					  status);
-}
-
-static int bmc150_accel_setup_new_data_interrupt(struct bmc150_accel_data *data,
-						 bool status)
-{
-	return bmc150_accel_set_interrupt(data, &bmc150_accel_interrupts[0],
-					  status);
-}
 
 static int bmc150_accel_set_scale(struct bmc150_accel_data *data, int val)
 {
@@ -753,13 +776,8 @@ static int bmc150_accel_write_event_config(struct iio_dev *indio_dev,
 
 	mutex_lock(&data->mutex);
 
-	if (!state && data->motion_trigger_on) {
-		data->ev_enable_state = 0;
-		mutex_unlock(&data->mutex);
-		return 0;
-	}
-
-	ret =  bmc150_accel_setup_any_motion_interrupt(data, state);
+	ret = bmc150_accel_set_interrupt(data, BMC150_ACCEL_INT_ANY_MOTION,
+					 state);
 	if (ret < 0) {
 		mutex_unlock(&data->mutex);
 		return ret;
@@ -996,19 +1014,16 @@ static int bmc150_accel_data_rdy_trigger_set_state(struct iio_trigger *trig,
 		}
 	}
 
-	if (!state && data->ev_enable_state && data->motion_trigger_on) {
-		data->motion_trigger_on = false;
-		mutex_unlock(&data->mutex);
-		return 0;
-	}
-
 	if (data->motion_trig == trig) {
 		ret = bmc150_accel_update_slope(data);
 		if (!ret)
-			ret = bmc150_accel_setup_any_motion_interrupt(data,
-								      state);
+			ret = bmc150_accel_set_interrupt(data,
+						 BMC150_ACCEL_INT_ANY_MOTION,
+							 state);
 	} else {
-		ret = bmc150_accel_setup_new_data_interrupt(data, state);
+		ret = bmc150_accel_set_interrupt(data,
+						 BMC150_ACCEL_INT_DATA_READY,
+						 state);
 	}
 	if (ret < 0) {
 		mutex_unlock(&data->mutex);
@@ -1206,6 +1221,8 @@ static int bmc150_accel_probe(struct i2c_client *client,
 			return ret;
 		}
 
+		bmc150_accel_interrupts_setup(indio_dev, data);
+
 		data->dready_trig = devm_iio_trigger_alloc(&client->dev,
 							   "%s-dev%d",
 							   indio_dev->name,
@@ -1321,8 +1338,7 @@ static int bmc150_accel_resume(struct device *dev)
 	struct bmc150_accel_data *data = iio_priv(indio_dev);
 
 	mutex_lock(&data->mutex);
-	if (data->dready_trigger_on || data->motion_trigger_on ||
-							data->ev_enable_state)
+	if (atomic_read(&data->active_intr))
 		bmc150_accel_set_mode(data, BMC150_ACCEL_SLEEP_MODE_NORMAL, 0);
 	mutex_unlock(&data->mutex);
 
-- 
1.9.1

[PATCH v4 3/3] iio: bmc150: introduce bmc150_accel_trigger

[Octavian Purdila]

Add a separate structure for triggers and add the infrastructure to
support an arbitrary number of triggers. Each trigger is associated
with an interrupt and has an enabled/disabled state.

Signed-off-by: Octavian Purdila <octavian.purdila@intel.com>
Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
 drivers/iio/accel/bmc150-accel.c | 204 ++++++++++++++++++++++++---------------
 1 file changed, 125 insertions(+), 79 deletions(-)

diff --git a/drivers/iio/accel/bmc150-accel.c b/drivers/iio/accel/bmc150-accel.c
index 70497c3..d826394 100644
--- a/drivers/iio/accel/bmc150-accel.c
+++ b/drivers/iio/accel/bmc150-accel.c
@@ -152,6 +152,14 @@ struct bmc150_accel_interrupt {
 	atomic_t users;
 };
 
+struct bmc150_accel_trigger {
+	struct bmc150_accel_data *data;
+	struct iio_trigger *indio_trig;
+	int (*setup)(struct bmc150_accel_trigger *t, bool state);
+	int intr;
+	bool enabled;
+};
+
 enum bmc150_accel_interrupt_id {
 	BMC150_ACCEL_INT_DATA_READY,
 	BMC150_ACCEL_INT_ANY_MOTION,
@@ -159,12 +167,17 @@ enum bmc150_accel_interrupt_id {
 	BMC150_ACCEL_INTERRUPTS,
 };
 
+enum bmc150_accel_trigger_id {
+	BMC150_ACCEL_TRIGGER_DATA_READY,
+	BMC150_ACCEL_TRIGGER_ANY_MOTION,
+	BMC150_ACCEL_TRIGGERS,
+};
+
 struct bmc150_accel_data {
 	struct i2c_client *client;
 	struct bmc150_accel_interrupt interrupts[BMC150_ACCEL_INTERRUPTS];
-	struct iio_trigger *dready_trig;
-	struct iio_trigger *motion_trig;
 	atomic_t active_intr;
+	struct bmc150_accel_trigger triggers[BMC150_ACCEL_TRIGGERS];
 	struct mutex mutex;
 	s16 buffer[8];
 	u8 bw_bits;
@@ -172,8 +185,6 @@ struct bmc150_accel_data {
 	u32 slope_thres;
 	u32 range;
 	int ev_enable_state;
-	bool dready_trigger_on;
-	bool motion_trigger_on;
 	int64_t timestamp;
 	const struct bmc150_accel_chip_info *chip_info;
 };
@@ -314,6 +325,15 @@ static int bmc150_accel_update_slope(struct bmc150_accel_data *data)
 	return ret;
 }
 
+static int bmc150_accel_any_motion_setup(struct bmc150_accel_trigger *t,
+					 bool state)
+{
+	if (state)
+		return bmc150_accel_update_slope(t->data);
+
+	return 0;
+}
+
 static int bmc150_accel_chip_init(struct bmc150_accel_data *data)
 {
 	int ret;
@@ -793,11 +813,14 @@ static int bmc150_accel_validate_trigger(struct iio_dev *indio_dev,
 				   struct iio_trigger *trig)
 {
 	struct bmc150_accel_data *data = iio_priv(indio_dev);
+	int i;
 
-	if (data->dready_trig != trig && data->motion_trig != trig)
-		return -EINVAL;
+	for (i = 0; i < BMC150_ACCEL_TRIGGERS; i++) {
+		if (data->triggers[i].indio_trig == trig)
+			return 0;
+	}
 
-	return 0;
+	return -EINVAL;
 }
 
 static IIO_CONST_ATTR_SAMP_FREQ_AVAIL(
@@ -969,12 +992,12 @@ err_read:
 
 static int bmc150_accel_trig_try_reen(struct iio_trigger *trig)
 {
-	struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig);
-	struct bmc150_accel_data *data = iio_priv(indio_dev);
+	struct bmc150_accel_trigger *t = iio_trigger_get_drvdata(trig);
+	struct bmc150_accel_data *data = t->data;
 	int ret;
 
 	/* new data interrupts don't need ack */
-	if (data->dready_trigger_on)
+	if (t == &t->data->triggers[BMC150_ACCEL_TRIGGER_DATA_READY])
 		return 0;
 
 	mutex_lock(&data->mutex);
@@ -993,46 +1016,35 @@ static int bmc150_accel_trig_try_reen(struct iio_trigger *trig)
 	return 0;
 }
 
-static int bmc150_accel_data_rdy_trigger_set_state(struct iio_trigger *trig,
+static int bmc150_accel_trigger_set_state(struct iio_trigger *trig,
 						   bool state)
 {
-	struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig);
-	struct bmc150_accel_data *data = iio_priv(indio_dev);
+	struct bmc150_accel_trigger *t = iio_trigger_get_drvdata(trig);
+	struct bmc150_accel_data *data = t->data;
 	int ret;
 
 	mutex_lock(&data->mutex);
 
-	if (data->motion_trig == trig) {
-		if (data->motion_trigger_on == state) {
-			mutex_unlock(&data->mutex);
-			return 0;
-		}
-	} else {
-		if (data->dready_trigger_on == state) {
+	if (t->enabled == state) {
+		mutex_unlock(&data->mutex);
+		return 0;
+	}
+
+	if (t->setup) {
+		ret = t->setup(t, state);
+		if (ret < 0) {
 			mutex_unlock(&data->mutex);
-			return 0;
+			return ret;
 		}
 	}
 
-	if (data->motion_trig == trig) {
-		ret = bmc150_accel_update_slope(data);
-		if (!ret)
-			ret = bmc150_accel_set_interrupt(data,
-						 BMC150_ACCEL_INT_ANY_MOTION,
-							 state);
-	} else {
-		ret = bmc150_accel_set_interrupt(data,
-						 BMC150_ACCEL_INT_DATA_READY,
-						 state);
-	}
+	ret = bmc150_accel_set_interrupt(data, t->intr, state);
 	if (ret < 0) {
 		mutex_unlock(&data->mutex);
 		return ret;
 	}
-	if (data->motion_trig == trig)
-		data->motion_trigger_on = state;
-	else
-		data->dready_trigger_on = state;
+
+	t->enabled = state;
 
 	mutex_unlock(&data->mutex);
 
@@ -1040,7 +1052,7 @@ static int bmc150_accel_data_rdy_trigger_set_state(struct iio_trigger *trig,
 }
 
 static const struct iio_trigger_ops bmc150_accel_trigger_ops = {
-	.set_trigger_state = bmc150_accel_data_rdy_trigger_set_state,
+	.set_trigger_state = bmc150_accel_trigger_set_state,
 	.try_reenable = bmc150_accel_trig_try_reen,
 	.owner = THIS_MODULE,
 };
@@ -1086,7 +1098,7 @@ static irqreturn_t bmc150_accel_event_handler(int irq, void *private)
 							dir),
 							data->timestamp);
 ack_intr_status:
-	if (!data->dready_trigger_on)
+	if (!data->triggers[BMC150_ACCEL_TRIGGER_DATA_READY].enabled)
 		ret = i2c_smbus_write_byte_data(data->client,
 					BMC150_ACCEL_REG_INT_RST_LATCH,
 					BMC150_ACCEL_INT_MODE_LATCH_INT |
@@ -1099,13 +1111,16 @@ static irqreturn_t bmc150_accel_data_rdy_trig_poll(int irq, void *private)
 {
 	struct iio_dev *indio_dev = private;
 	struct bmc150_accel_data *data = iio_priv(indio_dev);
+	int i;
 
 	data->timestamp = iio_get_time_ns();
 
-	if (data->dready_trigger_on)
-		iio_trigger_poll(data->dready_trig);
-	else if (data->motion_trigger_on)
-		iio_trigger_poll(data->motion_trig);
+	for (i = 0; i < BMC150_ACCEL_TRIGGERS; i++) {
+		if (data->triggers[i].enabled) {
+			iio_trigger_poll(data->triggers[i].indio_trig);
+			break;
+		}
+	}
 
 	if (data->ev_enable_state)
 		return IRQ_WAKE_THREAD;
@@ -1153,6 +1168,70 @@ static int bmc150_accel_gpio_probe(struct i2c_client *client,
 	return ret;
 }
 
+static const struct {
+	int intr;
+	const char *name;
+	int (*setup)(struct bmc150_accel_trigger *t, bool state);
+} bmc150_accel_triggers[BMC150_ACCEL_TRIGGERS] = {
+	{
+		.intr = 0,
+		.name = "%s-dev%d",
+	},
+	{
+		.intr = 1,
+		.name = "%s-any-motion-dev%d",
+		.setup = bmc150_accel_any_motion_setup,
+	},
+};
+
+static void bmc150_accel_unregister_triggers(struct bmc150_accel_data *data,
+					     int from)
+{
+	int i;
+
+	for (i = from; i >= 0; i++) {
+		if (data->triggers[i].indio_trig) {
+			iio_trigger_unregister(data->triggers[i].indio_trig);
+			data->triggers[i].indio_trig = NULL;
+		}
+	}
+}
+
+static int bmc150_accel_triggers_setup(struct iio_dev *indio_dev,
+				       struct bmc150_accel_data *data)
+{
+	int i, ret;
+
+	for (i = 0; i < BMC150_ACCEL_TRIGGERS; i++) {
+		struct bmc150_accel_trigger *t = &data->triggers[i];
+
+		t->indio_trig = devm_iio_trigger_alloc(&data->client->dev,
+					       bmc150_accel_triggers[i].name,
+						       indio_dev->name,
+						       indio_dev->id);
+		if (!t->indio_trig) {
+			ret = -ENOMEM;
+			break;
+		}
+
+		t->indio_trig->dev.parent = &data->client->dev;
+		t->indio_trig->ops = &bmc150_accel_trigger_ops;
+		t->intr = bmc150_accel_triggers[i].intr;
+		t->data = data;
+		t->setup = bmc150_accel_triggers[i].setup;
+		iio_trigger_set_drvdata(t->indio_trig, t);
+
+		ret = iio_trigger_register(t->indio_trig);
+		if (ret)
+			break;
+	}
+
+	if (ret)
+		bmc150_accel_unregister_triggers(data, i - 1);
+
+	return ret;
+}
+
 static int bmc150_accel_probe(struct i2c_client *client,
 			      const struct i2c_device_id *id)
 {
@@ -1223,36 +1302,10 @@ static int bmc150_accel_probe(struct i2c_client *client,
 
 		bmc150_accel_interrupts_setup(indio_dev, data);
 
-		data->dready_trig = devm_iio_trigger_alloc(&client->dev,
-							   "%s-dev%d",
-							   indio_dev->name,
-							   indio_dev->id);
-		if (!data->dready_trig)
-			return -ENOMEM;
-
-		data->motion_trig = devm_iio_trigger_alloc(&client->dev,
-							  "%s-any-motion-dev%d",
-							  indio_dev->name,
-							  indio_dev->id);
-		if (!data->motion_trig)
-			return -ENOMEM;
-
-		data->dready_trig->dev.parent = &client->dev;
-		data->dready_trig->ops = &bmc150_accel_trigger_ops;
-		iio_trigger_set_drvdata(data->dready_trig, indio_dev);
-		ret = iio_trigger_register(data->dready_trig);
+		ret = bmc150_accel_triggers_setup(indio_dev, data);
 		if (ret)
 			return ret;
 
-		data->motion_trig->dev.parent = &client->dev;
-		data->motion_trig->ops = &bmc150_accel_trigger_ops;
-		iio_trigger_set_drvdata(data->motion_trig, indio_dev);
-		ret = iio_trigger_register(data->motion_trig);
-		if (ret) {
-			data->motion_trig = NULL;
-			goto err_trigger_unregister;
-		}
-
 		ret = iio_triggered_buffer_setup(indio_dev,
 						 &iio_pollfunc_store_time,
 						 bmc150_accel_trigger_handler,
@@ -1284,13 +1337,10 @@ static int bmc150_accel_probe(struct i2c_client *client,
 err_iio_unregister:
 	iio_device_unregister(indio_dev);
 err_buffer_cleanup:
-	if (data->dready_trig)
+	if (indio_dev->pollfunc)
 		iio_triggered_buffer_cleanup(indio_dev);
 err_trigger_unregister:
-	if (data->dready_trig)
-		iio_trigger_unregister(data->dready_trig);
-	if (data->motion_trig)
-		iio_trigger_unregister(data->motion_trig);
+	bmc150_accel_unregister_triggers(data, BMC150_ACCEL_TRIGGERS - 1);
 
 	return ret;
 }
@@ -1306,11 +1356,7 @@ static int bmc150_accel_remove(struct i2c_client *client)
 
 	iio_device_unregister(indio_dev);
 
-	if (data->dready_trig) {
-		iio_triggered_buffer_cleanup(indio_dev);
-		iio_trigger_unregister(data->dready_trig);
-		iio_trigger_unregister(data->motion_trig);
-	}
+	bmc150_accel_unregister_triggers(data, BMC150_ACCEL_TRIGGERS - 1);
 
 	mutex_lock(&data->mutex);
 	bmc150_accel_set_mode(data, BMC150_ACCEL_SLEEP_MODE_DEEP_SUSPEND, 0);
-- 
1.9.1

Re: [PATCH v4 1/3] iio: bmc150: change sampling frequency

[Jonathan Cameron]

On 03/03/15 16:17, Octavian Purdila wrote:
> From: Sathyanarayanan Kuppuswamy <sathyanarayanan.kuppuswamy@intel.com>
> 
> Currently driver reports device bandwidth list as available
> sampling frequency. But sampling frequency is actually twice
> the device bandwidth. This patch fixes this issue.
> 
> Signed-off-by: Sathyanarayanan Kuppuswamy <sathyanarayanan.kuppuswamy@intel.com>
I was going to reject this on the basis that it should be using the
low_pass_filter_3db_bandwidth rather than sampling frequency but in
this device the two are tied together in a fashion not often seen.

Anyhow, fair enough.

It's probably going to make life messy with the rest of the series, but this
is clearly a fix that wants to stable as well.

Note Octavian that as you handled the patch it ought to have your sign off
as well as Sathyanarayanan's.

Jonathan
> ---
>  drivers/iio/accel/bmc150-accel.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/iio/accel/bmc150-accel.c b/drivers/iio/accel/bmc150-accel.c
> index 8004ce3..74ee54e 100644
> --- a/drivers/iio/accel/bmc150-accel.c
> +++ b/drivers/iio/accel/bmc150-accel.c
> @@ -168,14 +168,14 @@ static const struct {
>  	int val;
>  	int val2;
>  	u8 bw_bits;
> -} bmc150_accel_samp_freq_table[] = { {7, 810000, 0x08},
> -				     {15, 630000, 0x09},
> -				     {31, 250000, 0x0A},
> -				     {62, 500000, 0x0B},
> -				     {125, 0, 0x0C},
> -				     {250, 0, 0x0D},
> -				     {500, 0, 0x0E},
> -				     {1000, 0, 0x0F} };
> +} bmc150_accel_samp_freq_table[] = { {15, 620000, 0x08},
> +				     {31, 260000, 0x09},
> +				     {62, 500000, 0x0A},
> +				     {125, 0, 0x0B},
> +				     {250, 0, 0x0C},
> +				     {500, 0, 0x0D},
> +				     {1000, 0, 0x0E},
> +				     {2000, 0, 0x0F} };
>  
>  static const struct {
>  	int bw_bits;
> @@ -783,7 +783,7 @@ static int bmc150_accel_validate_trigger(struct iio_dev *indio_dev,
>  }
>  
>  static IIO_CONST_ATTR_SAMP_FREQ_AVAIL(
> -		"7.810000 15.630000 31.250000 62.500000 125 250 500 1000");
> +		"15.620000 31.260000 62.50000 125 250 500 1000 2000");
>  
>  static struct attribute *bmc150_accel_attributes[] = {
>  	&iio_const_attr_sampling_frequency_available.dev_attr.attr,
> 

Re: [PATCH v4 2/3] iio: bmc150: introduce bmc150_accel_interrupt

[Jonathan Cameron]

On 03/03/15 16:17, Octavian Purdila wrote:
> Since both triggers and events can share an interrupt, add a data
> structure that tracks the users of an interrupt so that it enables or
> disables it only for the first users and respectively last user.
> 
> This will allows us to easily add more events or triggers.
> 
> The patch also adds an interrupt enabled counter, so that we can
> easily know if we need to put the device in normal mode when the
> resume callback is issued.
> 
> Signed-off-by: Octavian Purdila <octavian.purdila@intel.com>
As I stated before I'd like an Ack from Srinivas on this
(obviously Srinivas, if you've handed the driver maintenance over
to Octavian just let me know and I'll stop pestering you ;)

Jonathan
> ---
>  drivers/iio/accel/bmc150-accel.c | 86 ++++++++++++++++++++++++----------------
>  1 file changed, 51 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/iio/accel/bmc150-accel.c b/drivers/iio/accel/bmc150-accel.c
> index 74ee54e..70497c3 100644
> --- a/drivers/iio/accel/bmc150-accel.c
> +++ b/drivers/iio/accel/bmc150-accel.c
> @@ -147,10 +147,24 @@ struct bmc150_accel_chip_info {
>  	const struct bmc150_scale_info scale_table[4];
>  };
>  
> +struct bmc150_accel_interrupt {
> +	const struct bmc150_accel_interrupt_info *info;
> +	atomic_t users;
> +};
> +
> +enum bmc150_accel_interrupt_id {
> +	BMC150_ACCEL_INT_DATA_READY,
> +	BMC150_ACCEL_INT_ANY_MOTION,
> +	BMC150_ACCEL_INT_WATERMARK,
> +	BMC150_ACCEL_INTERRUPTS,
> +};
> +
>  struct bmc150_accel_data {
>  	struct i2c_client *client;
> +	struct bmc150_accel_interrupt interrupts[BMC150_ACCEL_INTERRUPTS];
>  	struct iio_trigger *dready_trig;
>  	struct iio_trigger *motion_trig;
> +	atomic_t active_intr;
>  	struct mutex mutex;
>  	s16 buffer[8];
>  	u8 bw_bits;
> @@ -421,7 +435,7 @@ static const struct bmc150_accel_interrupt_info {
>  	u8 map_bitmask;
>  	u8 en_reg;
>  	u8 en_bitmask;
> -} bmc150_accel_interrupts[] = {
> +} bmc150_accel_interrupts[BMC150_ACCEL_INTERRUPTS] = {
>  	{ /* data ready interrupt */
>  		.map_reg = BMC150_ACCEL_REG_INT_MAP_1,
>  		.map_bitmask = BMC150_ACCEL_INT_MAP_1_BIT_DATA,
> @@ -438,12 +452,30 @@ static const struct bmc150_accel_interrupt_info {
>  	},
>  };
>  
> -static int bmc150_accel_set_interrupt(struct bmc150_accel_data *data,
> -				const struct bmc150_accel_interrupt_info *info,
> +static void bmc150_accel_interrupts_setup(struct iio_dev *indio_dev,
> +					  struct bmc150_accel_data *data)
> +{
> +	int i;
> +
> +	for (i = 0; i < BMC150_ACCEL_INTERRUPTS; i++)
> +		data->interrupts[i].info = &bmc150_accel_interrupts[i];
> +}
> +
> +static int bmc150_accel_set_interrupt(struct bmc150_accel_data *data, int i,
>  				      bool state)
>  {
> +	struct bmc150_accel_interrupt *intr = &data->interrupts[i];
> +	const struct bmc150_accel_interrupt_info *info = intr->info;
>  	int ret;
>  
> +	if (state) {
> +		if (atomic_inc_return(&intr->users) > 1)
> +			return 0;
> +	} else {
> +		if (atomic_dec_return(&intr->users) > 0)
> +			return 0;
> +	}
> +
>  	/*
>  	 * We will expect the enable and disable to do operation in
>  	 * in reverse order. This will happen here anyway as our
> @@ -493,6 +525,11 @@ static int bmc150_accel_set_interrupt(struct bmc150_accel_data *data,
>  		goto out_fix_power_state;
>  	}
>  
> +	if (state)
> +		atomic_inc(&data->active_intr);
> +	else
> +		atomic_dec(&data->active_intr);
> +
>  	return 0;
>  
>  out_fix_power_state:
> @@ -500,20 +537,6 @@ out_fix_power_state:
>  	return ret;
>  }
>  
> -static int bmc150_accel_setup_any_motion_interrupt(
> -					struct bmc150_accel_data *data,
> -					bool status)
> -{
> -	return bmc150_accel_set_interrupt(data, &bmc150_accel_interrupts[1],
> -					  status);
> -}
> -
> -static int bmc150_accel_setup_new_data_interrupt(struct bmc150_accel_data *data,
> -						 bool status)
> -{
> -	return bmc150_accel_set_interrupt(data, &bmc150_accel_interrupts[0],
> -					  status);
> -}
>  
>  static int bmc150_accel_set_scale(struct bmc150_accel_data *data, int val)
>  {
> @@ -753,13 +776,8 @@ static int bmc150_accel_write_event_config(struct iio_dev *indio_dev,
>  
>  	mutex_lock(&data->mutex);
>  
> -	if (!state && data->motion_trigger_on) {
> -		data->ev_enable_state = 0;
> -		mutex_unlock(&data->mutex);
> -		return 0;
> -	}
> -
> -	ret =  bmc150_accel_setup_any_motion_interrupt(data, state);
> +	ret = bmc150_accel_set_interrupt(data, BMC150_ACCEL_INT_ANY_MOTION,
> +					 state);
>  	if (ret < 0) {
>  		mutex_unlock(&data->mutex);
>  		return ret;
> @@ -996,19 +1014,16 @@ static int bmc150_accel_data_rdy_trigger_set_state(struct iio_trigger *trig,
>  		}
>  	}
>  
> -	if (!state && data->ev_enable_state && data->motion_trigger_on) {
> -		data->motion_trigger_on = false;
> -		mutex_unlock(&data->mutex);
> -		return 0;
> -	}
> -
>  	if (data->motion_trig == trig) {
>  		ret = bmc150_accel_update_slope(data);
>  		if (!ret)
> -			ret = bmc150_accel_setup_any_motion_interrupt(data,
> -								      state);
> +			ret = bmc150_accel_set_interrupt(data,
> +						 BMC150_ACCEL_INT_ANY_MOTION,
> +							 state);
>  	} else {
> -		ret = bmc150_accel_setup_new_data_interrupt(data, state);
> +		ret = bmc150_accel_set_interrupt(data,
> +						 BMC150_ACCEL_INT_DATA_READY,
> +						 state);
>  	}
>  	if (ret < 0) {
>  		mutex_unlock(&data->mutex);
> @@ -1206,6 +1221,8 @@ static int bmc150_accel_probe(struct i2c_client *client,
>  			return ret;
>  		}
>  
> +		bmc150_accel_interrupts_setup(indio_dev, data);
> +
>  		data->dready_trig = devm_iio_trigger_alloc(&client->dev,
>  							   "%s-dev%d",
>  							   indio_dev->name,
> @@ -1321,8 +1338,7 @@ static int bmc150_accel_resume(struct device *dev)
>  	struct bmc150_accel_data *data = iio_priv(indio_dev);
>  
>  	mutex_lock(&data->mutex);
> -	if (data->dready_trigger_on || data->motion_trigger_on ||
> -							data->ev_enable_state)
> +	if (atomic_read(&data->active_intr))
>  		bmc150_accel_set_mode(data, BMC150_ACCEL_SLEEP_MODE_NORMAL, 0);
>  	mutex_unlock(&data->mutex);
>  
> 

Re: [PATCH v4 1/3] iio: bmc150: change sampling frequency

[Octavian Purdila]

On Sun, Mar 8, 2015 at 1:17 PM, Jonathan Cameron <jic23@kernel.org> wrote:
> On 03/03/15 16:17, Octavian Purdila wrote:
>> From: Sathyanarayanan Kuppuswamy <sathyanarayanan.kuppuswamy@intel.com>
>>
>> Currently driver reports device bandwidth list as available
>> sampling frequency. But sampling frequency is actually twice
>> the device bandwidth. This patch fixes this issue.
>>
>> Signed-off-by: Sathyanarayanan Kuppuswamy <sathyanarayanan.kuppuswamy@intel.com>
> I was going to reject this on the basis that it should be using the
> low_pass_filter_3db_bandwidth rather than sampling frequency but in
> this device the two are tied together in a fashion not often seen.
>
> Anyhow, fair enough.
>
> It's probably going to make life messy with the rest of the series, but this
> is clearly a fix that wants to stable as well.
>
> Note Octavian that as you handled the patch it ought to have your sign off
> as well as Sathyanarayanan's.
>

Yes, I missed that, sorry about that. Here is the sign off, I assume
there is no need to send the series again just for this:

Signed-off-by: Octavian Purdila <octavian.purdila@intel.com>

> Jonathan
>> ---
>>  drivers/iio/accel/bmc150-accel.c | 18 +++++++++---------
>>  1 file changed, 9 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/iio/accel/bmc150-accel.c b/drivers/iio/accel/bmc150-accel.c
>> index 8004ce3..74ee54e 100644
>> --- a/drivers/iio/accel/bmc150-accel.c
>> +++ b/drivers/iio/accel/bmc150-accel.c
>> @@ -168,14 +168,14 @@ static const struct {
>>       int val;
>>       int val2;
>>       u8 bw_bits;
>> -} bmc150_accel_samp_freq_table[] = { {7, 810000, 0x08},
>> -                                  {15, 630000, 0x09},
>> -                                  {31, 250000, 0x0A},
>> -                                  {62, 500000, 0x0B},
>> -                                  {125, 0, 0x0C},
>> -                                  {250, 0, 0x0D},
>> -                                  {500, 0, 0x0E},
>> -                                  {1000, 0, 0x0F} };
>> +} bmc150_accel_samp_freq_table[] = { {15, 620000, 0x08},
>> +                                  {31, 260000, 0x09},
>> +                                  {62, 500000, 0x0A},
>> +                                  {125, 0, 0x0B},
>> +                                  {250, 0, 0x0C},
>> +                                  {500, 0, 0x0D},
>> +                                  {1000, 0, 0x0E},
>> +                                  {2000, 0, 0x0F} };
>>
>>  static const struct {
>>       int bw_bits;
>> @@ -783,7 +783,7 @@ static int bmc150_accel_validate_trigger(struct iio_dev *indio_dev,
>>  }
>>
>>  static IIO_CONST_ATTR_SAMP_FREQ_AVAIL(
>> -             "7.810000 15.630000 31.250000 62.500000 125 250 500 1000");
>> +             "15.620000 31.260000 62.50000 125 250 500 1000 2000");
>>
>>  static struct attribute *bmc150_accel_attributes[] = {
>>       &iio_const_attr_sampling_frequency_available.dev_attr.attr,
>>
>

Re: [PATCH v4 2/3] iio: bmc150: introduce bmc150_accel_interrupt

[Srinivas Pandruvada]

On Sun, 2015-03-08 at 11:22 +0000, Jonathan Cameron wrote:
> On 03/03/15 16:17, Octavian Purdila wrote:
> > Since both triggers and events can share an interrupt, add a data
> > structure that tracks the users of an interrupt so that it enables or
> > disables it only for the first users and respectively last user.
> > 
> > This will allows us to easily add more events or triggers.
> > 
> > The patch also adds an interrupt enabled counter, so that we can
> > easily know if we need to put the device in normal mode when the
> > resume callback is issued.
> > 
> > Signed-off-by: Octavian Purdila <octavian.purdila@intel.com>
> As I stated before I'd like an Ack from Srinivas on this
> (obviously Srinivas, if you've handed the driver maintenance over
> to Octavian just let me know and I'll stop pestering you ;)

Reviewed-by:: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>


I prefer to test before Acks. Since Octavian is adding Fifo support and
many changes, If Octavian wants he can take over the maintenance of this
driver.


Thanks,
Srinivas

> 

> Jonathan
> > ---
> >  drivers/iio/accel/bmc150-accel.c | 86 ++++++++++++++++++++++++----------------
> >  1 file changed, 51 insertions(+), 35 deletions(-)
> > 
> > diff --git a/drivers/iio/accel/bmc150-accel.c b/drivers/iio/accel/bmc150-accel.c
> > index 74ee54e..70497c3 100644
> > --- a/drivers/iio/accel/bmc150-accel.c
> > +++ b/drivers/iio/accel/bmc150-accel.c
> > @@ -147,10 +147,24 @@ struct bmc150_accel_chip_info {
> >  	const struct bmc150_scale_info scale_table[4];
> >  };
> >  
> > +struct bmc150_accel_interrupt {
> > +	const struct bmc150_accel_interrupt_info *info;
> > +	atomic_t users;
> > +};
> > +
> > +enum bmc150_accel_interrupt_id {
> > +	BMC150_ACCEL_INT_DATA_READY,
> > +	BMC150_ACCEL_INT_ANY_MOTION,
> > +	BMC150_ACCEL_INT_WATERMARK,
> > +	BMC150_ACCEL_INTERRUPTS,
> > +};
> > +
> >  struct bmc150_accel_data {
> >  	struct i2c_client *client;
> > +	struct bmc150_accel_interrupt interrupts[BMC150_ACCEL_INTERRUPTS];
> >  	struct iio_trigger *dready_trig;
> >  	struct iio_trigger *motion_trig;
> > +	atomic_t active_intr;
> >  	struct mutex mutex;
> >  	s16 buffer[8];
> >  	u8 bw_bits;
> > @@ -421,7 +435,7 @@ static const struct bmc150_accel_interrupt_info {
> >  	u8 map_bitmask;
> >  	u8 en_reg;
> >  	u8 en_bitmask;
> > -} bmc150_accel_interrupts[] = {
> > +} bmc150_accel_interrupts[BMC150_ACCEL_INTERRUPTS] = {
> >  	{ /* data ready interrupt */
> >  		.map_reg = BMC150_ACCEL_REG_INT_MAP_1,
> >  		.map_bitmask = BMC150_ACCEL_INT_MAP_1_BIT_DATA,
> > @@ -438,12 +452,30 @@ static const struct bmc150_accel_interrupt_info {
> >  	},
> >  };
> >  
> > -static int bmc150_accel_set_interrupt(struct bmc150_accel_data *data,
> > -				const struct bmc150_accel_interrupt_info *info,
> > +static void bmc150_accel_interrupts_setup(struct iio_dev *indio_dev,
> > +					  struct bmc150_accel_data *data)
> > +{
> > +	int i;
> > +
> > +	for (i = 0; i < BMC150_ACCEL_INTERRUPTS; i++)
> > +		data->interrupts[i].info = &bmc150_accel_interrupts[i];
> > +}
> > +
> > +static int bmc150_accel_set_interrupt(struct bmc150_accel_data *data, int i,
> >  				      bool state)
> >  {
> > +	struct bmc150_accel_interrupt *intr = &data->interrupts[i];
> > +	const struct bmc150_accel_interrupt_info *info = intr->info;
> >  	int ret;
> >  
> > +	if (state) {
> > +		if (atomic_inc_return(&intr->users) > 1)
> > +			return 0;
> > +	} else {
> > +		if (atomic_dec_return(&intr->users) > 0)
> > +			return 0;
> > +	}
> > +
> >  	/*
> >  	 * We will expect the enable and disable to do operation in
> >  	 * in reverse order. This will happen here anyway as our
> > @@ -493,6 +525,11 @@ static int bmc150_accel_set_interrupt(struct bmc150_accel_data *data,
> >  		goto out_fix_power_state;
> >  	}
> >  
> > +	if (state)
> > +		atomic_inc(&data->active_intr);
> > +	else
> > +		atomic_dec(&data->active_intr);
> > +
> >  	return 0;
> >  
> >  out_fix_power_state:
> > @@ -500,20 +537,6 @@ out_fix_power_state:
> >  	return ret;
> >  }
> >  
> > -static int bmc150_accel_setup_any_motion_interrupt(
> > -					struct bmc150_accel_data *data,
> > -					bool status)
> > -{
> > -	return bmc150_accel_set_interrupt(data, &bmc150_accel_interrupts[1],
> > -					  status);
> > -}
> > -
> > -static int bmc150_accel_setup_new_data_interrupt(struct bmc150_accel_data *data,
> > -						 bool status)
> > -{
> > -	return bmc150_accel_set_interrupt(data, &bmc150_accel_interrupts[0],
> > -					  status);
> > -}
> >  
> >  static int bmc150_accel_set_scale(struct bmc150_accel_data *data, int val)
> >  {
> > @@ -753,13 +776,8 @@ static int bmc150_accel_write_event_config(struct iio_dev *indio_dev,
> >  
> >  	mutex_lock(&data->mutex);
> >  
> > -	if (!state && data->motion_trigger_on) {
> > -		data->ev_enable_state = 0;
> > -		mutex_unlock(&data->mutex);
> > -		return 0;
> > -	}
> > -
> > -	ret =  bmc150_accel_setup_any_motion_interrupt(data, state);
> > +	ret = bmc150_accel_set_interrupt(data, BMC150_ACCEL_INT_ANY_MOTION,
> > +					 state);
> >  	if (ret < 0) {
> >  		mutex_unlock(&data->mutex);
> >  		return ret;
> > @@ -996,19 +1014,16 @@ static int bmc150_accel_data_rdy_trigger_set_state(struct iio_trigger *trig,
> >  		}
> >  	}
> >  
> > -	if (!state && data->ev_enable_state && data->motion_trigger_on) {
> > -		data->motion_trigger_on = false;
> > -		mutex_unlock(&data->mutex);
> > -		return 0;
> > -	}
> > -
> >  	if (data->motion_trig == trig) {
> >  		ret = bmc150_accel_update_slope(data);
> >  		if (!ret)
> > -			ret = bmc150_accel_setup_any_motion_interrupt(data,
> > -								      state);
> > +			ret = bmc150_accel_set_interrupt(data,
> > +						 BMC150_ACCEL_INT_ANY_MOTION,
> > +							 state);
> >  	} else {
> > -		ret = bmc150_accel_setup_new_data_interrupt(data, state);
> > +		ret = bmc150_accel_set_interrupt(data,
> > +						 BMC150_ACCEL_INT_DATA_READY,
> > +						 state);
> >  	}
> >  	if (ret < 0) {
> >  		mutex_unlock(&data->mutex);
> > @@ -1206,6 +1221,8 @@ static int bmc150_accel_probe(struct i2c_client *client,
> >  			return ret;
> >  		}
> >  
> > +		bmc150_accel_interrupts_setup(indio_dev, data);
> > +
> >  		data->dready_trig = devm_iio_trigger_alloc(&client->dev,
> >  							   "%s-dev%d",
> >  							   indio_dev->name,
> > @@ -1321,8 +1338,7 @@ static int bmc150_accel_resume(struct device *dev)
> >  	struct bmc150_accel_data *data = iio_priv(indio_dev);
> >  
> >  	mutex_lock(&data->mutex);
> > -	if (data->dready_trigger_on || data->motion_trigger_on ||
> > -							data->ev_enable_state)
> > +	if (atomic_read(&data->active_intr))
> >  		bmc150_accel_set_mode(data, BMC150_ACCEL_SLEEP_MODE_NORMAL, 0);
> >  	mutex_unlock(&data->mutex);
> >  
> > 
> 

Re: [PATCH v4 2/3] iio: bmc150: introduce bmc150_accel_interrupt

[Jonathan Cameron]

On 09/03/15 20:40, Srinivas Pandruvada wrote:
> On Sun, 2015-03-08 at 11:22 +0000, Jonathan Cameron wrote:
>> On 03/03/15 16:17, Octavian Purdila wrote:
>>> Since both triggers and events can share an interrupt, add a data
>>> structure that tracks the users of an interrupt so that it enables or
>>> disables it only for the first users and respectively last user.
>>>
>>> This will allows us to easily add more events or triggers.
>>>
>>> The patch also adds an interrupt enabled counter, so that we can
>>> easily know if we need to put the device in normal mode when the
>>> resume callback is issued.
>>>
>>> Signed-off-by: Octavian Purdila <octavian.purdila@intel.com>
>> As I stated before I'd like an Ack from Srinivas on this
>> (obviously Srinivas, if you've handed the driver maintenance over
>> to Octavian just let me know and I'll stop pestering you ;)
> 
> Reviewed-by:: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> 
> 
> I prefer to test before Acks. Since Octavian is adding Fifo support and
> many changes, If Octavian wants he can take over the maintenance of this
> driver.
> 
cool. Applied to the togreg branch of iio.git - initially pushed out as
testing for the autobuilders to play with it.

Thanks,

Jonathan
> 
> Thanks,
> Srinivas
> 
>>
> 
>> Jonathan
>>> ---
>>>  drivers/iio/accel/bmc150-accel.c | 86 ++++++++++++++++++++++++----------------
>>>  1 file changed, 51 insertions(+), 35 deletions(-)
>>>
>>> diff --git a/drivers/iio/accel/bmc150-accel.c b/drivers/iio/accel/bmc150-accel.c
>>> index 74ee54e..70497c3 100644
>>> --- a/drivers/iio/accel/bmc150-accel.c
>>> +++ b/drivers/iio/accel/bmc150-accel.c
>>> @@ -147,10 +147,24 @@ struct bmc150_accel_chip_info {
>>>  	const struct bmc150_scale_info scale_table[4];
>>>  };
>>>  
>>> +struct bmc150_accel_interrupt {
>>> +	const struct bmc150_accel_interrupt_info *info;
>>> +	atomic_t users;
>>> +};
>>> +
>>> +enum bmc150_accel_interrupt_id {
>>> +	BMC150_ACCEL_INT_DATA_READY,
>>> +	BMC150_ACCEL_INT_ANY_MOTION,
>>> +	BMC150_ACCEL_INT_WATERMARK,
>>> +	BMC150_ACCEL_INTERRUPTS,
>>> +};
>>> +
>>>  struct bmc150_accel_data {
>>>  	struct i2c_client *client;
>>> +	struct bmc150_accel_interrupt interrupts[BMC150_ACCEL_INTERRUPTS];
>>>  	struct iio_trigger *dready_trig;
>>>  	struct iio_trigger *motion_trig;
>>> +	atomic_t active_intr;
>>>  	struct mutex mutex;
>>>  	s16 buffer[8];
>>>  	u8 bw_bits;
>>> @@ -421,7 +435,7 @@ static const struct bmc150_accel_interrupt_info {
>>>  	u8 map_bitmask;
>>>  	u8 en_reg;
>>>  	u8 en_bitmask;
>>> -} bmc150_accel_interrupts[] = {
>>> +} bmc150_accel_interrupts[BMC150_ACCEL_INTERRUPTS] = {
>>>  	{ /* data ready interrupt */
>>>  		.map_reg = BMC150_ACCEL_REG_INT_MAP_1,
>>>  		.map_bitmask = BMC150_ACCEL_INT_MAP_1_BIT_DATA,
>>> @@ -438,12 +452,30 @@ static const struct bmc150_accel_interrupt_info {
>>>  	},
>>>  };
>>>  
>>> -static int bmc150_accel_set_interrupt(struct bmc150_accel_data *data,
>>> -				const struct bmc150_accel_interrupt_info *info,
>>> +static void bmc150_accel_interrupts_setup(struct iio_dev *indio_dev,
>>> +					  struct bmc150_accel_data *data)
>>> +{
>>> +	int i;
>>> +
>>> +	for (i = 0; i < BMC150_ACCEL_INTERRUPTS; i++)
>>> +		data->interrupts[i].info = &bmc150_accel_interrupts[i];
>>> +}
>>> +
>>> +static int bmc150_accel_set_interrupt(struct bmc150_accel_data *data, int i,
>>>  				      bool state)
>>>  {
>>> +	struct bmc150_accel_interrupt *intr = &data->interrupts[i];
>>> +	const struct bmc150_accel_interrupt_info *info = intr->info;
>>>  	int ret;
>>>  
>>> +	if (state) {
>>> +		if (atomic_inc_return(&intr->users) > 1)
>>> +			return 0;
>>> +	} else {
>>> +		if (atomic_dec_return(&intr->users) > 0)
>>> +			return 0;
>>> +	}
>>> +
>>>  	/*
>>>  	 * We will expect the enable and disable to do operation in
>>>  	 * in reverse order. This will happen here anyway as our
>>> @@ -493,6 +525,11 @@ static int bmc150_accel_set_interrupt(struct bmc150_accel_data *data,
>>>  		goto out_fix_power_state;
>>>  	}
>>>  
>>> +	if (state)
>>> +		atomic_inc(&data->active_intr);
>>> +	else
>>> +		atomic_dec(&data->active_intr);
>>> +
>>>  	return 0;
>>>  
>>>  out_fix_power_state:
>>> @@ -500,20 +537,6 @@ out_fix_power_state:
>>>  	return ret;
>>>  }
>>>  
>>> -static int bmc150_accel_setup_any_motion_interrupt(
>>> -					struct bmc150_accel_data *data,
>>> -					bool status)
>>> -{
>>> -	return bmc150_accel_set_interrupt(data, &bmc150_accel_interrupts[1],
>>> -					  status);
>>> -}
>>> -
>>> -static int bmc150_accel_setup_new_data_interrupt(struct bmc150_accel_data *data,
>>> -						 bool status)
>>> -{
>>> -	return bmc150_accel_set_interrupt(data, &bmc150_accel_interrupts[0],
>>> -					  status);
>>> -}
>>>  
>>>  static int bmc150_accel_set_scale(struct bmc150_accel_data *data, int val)
>>>  {
>>> @@ -753,13 +776,8 @@ static int bmc150_accel_write_event_config(struct iio_dev *indio_dev,
>>>  
>>>  	mutex_lock(&data->mutex);
>>>  
>>> -	if (!state && data->motion_trigger_on) {
>>> -		data->ev_enable_state = 0;
>>> -		mutex_unlock(&data->mutex);
>>> -		return 0;
>>> -	}
>>> -
>>> -	ret =  bmc150_accel_setup_any_motion_interrupt(data, state);
>>> +	ret = bmc150_accel_set_interrupt(data, BMC150_ACCEL_INT_ANY_MOTION,
>>> +					 state);
>>>  	if (ret < 0) {
>>>  		mutex_unlock(&data->mutex);
>>>  		return ret;
>>> @@ -996,19 +1014,16 @@ static int bmc150_accel_data_rdy_trigger_set_state(struct iio_trigger *trig,
>>>  		}
>>>  	}
>>>  
>>> -	if (!state && data->ev_enable_state && data->motion_trigger_on) {
>>> -		data->motion_trigger_on = false;
>>> -		mutex_unlock(&data->mutex);
>>> -		return 0;
>>> -	}
>>> -
>>>  	if (data->motion_trig == trig) {
>>>  		ret = bmc150_accel_update_slope(data);
>>>  		if (!ret)
>>> -			ret = bmc150_accel_setup_any_motion_interrupt(data,
>>> -								      state);
>>> +			ret = bmc150_accel_set_interrupt(data,
>>> +						 BMC150_ACCEL_INT_ANY_MOTION,
>>> +							 state);
>>>  	} else {
>>> -		ret = bmc150_accel_setup_new_data_interrupt(data, state);
>>> +		ret = bmc150_accel_set_interrupt(data,
>>> +						 BMC150_ACCEL_INT_DATA_READY,
>>> +						 state);
>>>  	}
>>>  	if (ret < 0) {
>>>  		mutex_unlock(&data->mutex);
>>> @@ -1206,6 +1221,8 @@ static int bmc150_accel_probe(struct i2c_client *client,
>>>  			return ret;
>>>  		}
>>>  
>>> +		bmc150_accel_interrupts_setup(indio_dev, data);
>>> +
>>>  		data->dready_trig = devm_iio_trigger_alloc(&client->dev,
>>>  							   "%s-dev%d",
>>>  							   indio_dev->name,
>>> @@ -1321,8 +1338,7 @@ static int bmc150_accel_resume(struct device *dev)
>>>  	struct bmc150_accel_data *data = iio_priv(indio_dev);
>>>  
>>>  	mutex_lock(&data->mutex);
>>> -	if (data->dready_trigger_on || data->motion_trigger_on ||
>>> -							data->ev_enable_state)
>>> +	if (atomic_read(&data->active_intr))
>>>  		bmc150_accel_set_mode(data, BMC150_ACCEL_SLEEP_MODE_NORMAL, 0);
>>>  	mutex_unlock(&data->mutex);
>>>  
>>>
>>
> 
> 

Re: [PATCH v4 3/3] iio: bmc150: introduce bmc150_accel_trigger

[Jonathan Cameron]

On 03/03/15 16:17, Octavian Purdila wrote:
> Add a separate structure for triggers and add the infrastructure to
> support an arbitrary number of triggers. Each trigger is associated
> with an interrupt and has an enabled/disabled state.
> 
> Signed-off-by: Octavian Purdila <octavian.purdila@intel.com>
> Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Applied to the togreg branch of iio.git. 
Initially pushed out as testing for the autobuilders to play with it

Thanks,

Jonathan
> ---
>  drivers/iio/accel/bmc150-accel.c | 204 ++++++++++++++++++++++++---------------
>  1 file changed, 125 insertions(+), 79 deletions(-)
> 
> diff --git a/drivers/iio/accel/bmc150-accel.c b/drivers/iio/accel/bmc150-accel.c
> index 70497c3..d826394 100644
> --- a/drivers/iio/accel/bmc150-accel.c
> +++ b/drivers/iio/accel/bmc150-accel.c
> @@ -152,6 +152,14 @@ struct bmc150_accel_interrupt {
>  	atomic_t users;
>  };
>  
> +struct bmc150_accel_trigger {
> +	struct bmc150_accel_data *data;
> +	struct iio_trigger *indio_trig;
> +	int (*setup)(struct bmc150_accel_trigger *t, bool state);
> +	int intr;
> +	bool enabled;
> +};
> +
>  enum bmc150_accel_interrupt_id {
>  	BMC150_ACCEL_INT_DATA_READY,
>  	BMC150_ACCEL_INT_ANY_MOTION,
> @@ -159,12 +167,17 @@ enum bmc150_accel_interrupt_id {
>  	BMC150_ACCEL_INTERRUPTS,
>  };
>  
> +enum bmc150_accel_trigger_id {
> +	BMC150_ACCEL_TRIGGER_DATA_READY,
> +	BMC150_ACCEL_TRIGGER_ANY_MOTION,
> +	BMC150_ACCEL_TRIGGERS,
> +};
> +
>  struct bmc150_accel_data {
>  	struct i2c_client *client;
>  	struct bmc150_accel_interrupt interrupts[BMC150_ACCEL_INTERRUPTS];
> -	struct iio_trigger *dready_trig;
> -	struct iio_trigger *motion_trig;
>  	atomic_t active_intr;
> +	struct bmc150_accel_trigger triggers[BMC150_ACCEL_TRIGGERS];
>  	struct mutex mutex;
>  	s16 buffer[8];
>  	u8 bw_bits;
> @@ -172,8 +185,6 @@ struct bmc150_accel_data {
>  	u32 slope_thres;
>  	u32 range;
>  	int ev_enable_state;
> -	bool dready_trigger_on;
> -	bool motion_trigger_on;
>  	int64_t timestamp;
>  	const struct bmc150_accel_chip_info *chip_info;
>  };
> @@ -314,6 +325,15 @@ static int bmc150_accel_update_slope(struct bmc150_accel_data *data)
>  	return ret;
>  }
>  
> +static int bmc150_accel_any_motion_setup(struct bmc150_accel_trigger *t,
> +					 bool state)
> +{
> +	if (state)
> +		return bmc150_accel_update_slope(t->data);
> +
> +	return 0;
> +}
> +
>  static int bmc150_accel_chip_init(struct bmc150_accel_data *data)
>  {
>  	int ret;
> @@ -793,11 +813,14 @@ static int bmc150_accel_validate_trigger(struct iio_dev *indio_dev,
>  				   struct iio_trigger *trig)
>  {
>  	struct bmc150_accel_data *data = iio_priv(indio_dev);
> +	int i;
>  
> -	if (data->dready_trig != trig && data->motion_trig != trig)
> -		return -EINVAL;
> +	for (i = 0; i < BMC150_ACCEL_TRIGGERS; i++) {
> +		if (data->triggers[i].indio_trig == trig)
> +			return 0;
> +	}
>  
> -	return 0;
> +	return -EINVAL;
>  }
>  
>  static IIO_CONST_ATTR_SAMP_FREQ_AVAIL(
> @@ -969,12 +992,12 @@ err_read:
>  
>  static int bmc150_accel_trig_try_reen(struct iio_trigger *trig)
>  {
> -	struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig);
> -	struct bmc150_accel_data *data = iio_priv(indio_dev);
> +	struct bmc150_accel_trigger *t = iio_trigger_get_drvdata(trig);
> +	struct bmc150_accel_data *data = t->data;
>  	int ret;
>  
>  	/* new data interrupts don't need ack */
> -	if (data->dready_trigger_on)
> +	if (t == &t->data->triggers[BMC150_ACCEL_TRIGGER_DATA_READY])
>  		return 0;
>  
>  	mutex_lock(&data->mutex);
> @@ -993,46 +1016,35 @@ static int bmc150_accel_trig_try_reen(struct iio_trigger *trig)
>  	return 0;
>  }
>  
> -static int bmc150_accel_data_rdy_trigger_set_state(struct iio_trigger *trig,
> +static int bmc150_accel_trigger_set_state(struct iio_trigger *trig,
>  						   bool state)
>  {
> -	struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig);
> -	struct bmc150_accel_data *data = iio_priv(indio_dev);
> +	struct bmc150_accel_trigger *t = iio_trigger_get_drvdata(trig);
> +	struct bmc150_accel_data *data = t->data;
>  	int ret;
>  
>  	mutex_lock(&data->mutex);
>  
> -	if (data->motion_trig == trig) {
> -		if (data->motion_trigger_on == state) {
> -			mutex_unlock(&data->mutex);
> -			return 0;
> -		}
> -	} else {
> -		if (data->dready_trigger_on == state) {
> +	if (t->enabled == state) {
> +		mutex_unlock(&data->mutex);
> +		return 0;
> +	}
> +
> +	if (t->setup) {
> +		ret = t->setup(t, state);
> +		if (ret < 0) {
>  			mutex_unlock(&data->mutex);
> -			return 0;
> +			return ret;
>  		}
>  	}
>  
> -	if (data->motion_trig == trig) {
> -		ret = bmc150_accel_update_slope(data);
> -		if (!ret)
> -			ret = bmc150_accel_set_interrupt(data,
> -						 BMC150_ACCEL_INT_ANY_MOTION,
> -							 state);
> -	} else {
> -		ret = bmc150_accel_set_interrupt(data,
> -						 BMC150_ACCEL_INT_DATA_READY,
> -						 state);
> -	}
> +	ret = bmc150_accel_set_interrupt(data, t->intr, state);
>  	if (ret < 0) {
>  		mutex_unlock(&data->mutex);
>  		return ret;
>  	}
> -	if (data->motion_trig == trig)
> -		data->motion_trigger_on = state;
> -	else
> -		data->dready_trigger_on = state;
> +
> +	t->enabled = state;
>  
>  	mutex_unlock(&data->mutex);
>  
> @@ -1040,7 +1052,7 @@ static int bmc150_accel_data_rdy_trigger_set_state(struct iio_trigger *trig,
>  }
>  
>  static const struct iio_trigger_ops bmc150_accel_trigger_ops = {
> -	.set_trigger_state = bmc150_accel_data_rdy_trigger_set_state,
> +	.set_trigger_state = bmc150_accel_trigger_set_state,
>  	.try_reenable = bmc150_accel_trig_try_reen,
>  	.owner = THIS_MODULE,
>  };
> @@ -1086,7 +1098,7 @@ static irqreturn_t bmc150_accel_event_handler(int irq, void *private)
>  							dir),
>  							data->timestamp);
>  ack_intr_status:
> -	if (!data->dready_trigger_on)
> +	if (!data->triggers[BMC150_ACCEL_TRIGGER_DATA_READY].enabled)
>  		ret = i2c_smbus_write_byte_data(data->client,
>  					BMC150_ACCEL_REG_INT_RST_LATCH,
>  					BMC150_ACCEL_INT_MODE_LATCH_INT |
> @@ -1099,13 +1111,16 @@ static irqreturn_t bmc150_accel_data_rdy_trig_poll(int irq, void *private)
>  {
>  	struct iio_dev *indio_dev = private;
>  	struct bmc150_accel_data *data = iio_priv(indio_dev);
> +	int i;
>  
>  	data->timestamp = iio_get_time_ns();
>  
> -	if (data->dready_trigger_on)
> -		iio_trigger_poll(data->dready_trig);
> -	else if (data->motion_trigger_on)
> -		iio_trigger_poll(data->motion_trig);
> +	for (i = 0; i < BMC150_ACCEL_TRIGGERS; i++) {
> +		if (data->triggers[i].enabled) {
> +			iio_trigger_poll(data->triggers[i].indio_trig);
> +			break;
> +		}
> +	}
>  
>  	if (data->ev_enable_state)
>  		return IRQ_WAKE_THREAD;
> @@ -1153,6 +1168,70 @@ static int bmc150_accel_gpio_probe(struct i2c_client *client,
>  	return ret;
>  }
>  
> +static const struct {
> +	int intr;
> +	const char *name;
> +	int (*setup)(struct bmc150_accel_trigger *t, bool state);
> +} bmc150_accel_triggers[BMC150_ACCEL_TRIGGERS] = {
> +	{
> +		.intr = 0,
> +		.name = "%s-dev%d",
> +	},
> +	{
> +		.intr = 1,
> +		.name = "%s-any-motion-dev%d",
> +		.setup = bmc150_accel_any_motion_setup,
> +	},
> +};
> +
> +static void bmc150_accel_unregister_triggers(struct bmc150_accel_data *data,
> +					     int from)
> +{
> +	int i;
> +
> +	for (i = from; i >= 0; i++) {
> +		if (data->triggers[i].indio_trig) {
> +			iio_trigger_unregister(data->triggers[i].indio_trig);
> +			data->triggers[i].indio_trig = NULL;
> +		}
> +	}
> +}
> +
> +static int bmc150_accel_triggers_setup(struct iio_dev *indio_dev,
> +				       struct bmc150_accel_data *data)
> +{
> +	int i, ret;
> +
> +	for (i = 0; i < BMC150_ACCEL_TRIGGERS; i++) {
> +		struct bmc150_accel_trigger *t = &data->triggers[i];
> +
> +		t->indio_trig = devm_iio_trigger_alloc(&data->client->dev,
> +					       bmc150_accel_triggers[i].name,
> +						       indio_dev->name,
> +						       indio_dev->id);
> +		if (!t->indio_trig) {
> +			ret = -ENOMEM;
> +			break;
> +		}
> +
> +		t->indio_trig->dev.parent = &data->client->dev;
> +		t->indio_trig->ops = &bmc150_accel_trigger_ops;
> +		t->intr = bmc150_accel_triggers[i].intr;
> +		t->data = data;
> +		t->setup = bmc150_accel_triggers[i].setup;
> +		iio_trigger_set_drvdata(t->indio_trig, t);
> +
> +		ret = iio_trigger_register(t->indio_trig);
> +		if (ret)
> +			break;
> +	}
> +
> +	if (ret)
> +		bmc150_accel_unregister_triggers(data, i - 1);
> +
> +	return ret;
> +}
> +
>  static int bmc150_accel_probe(struct i2c_client *client,
>  			      const struct i2c_device_id *id)
>  {
> @@ -1223,36 +1302,10 @@ static int bmc150_accel_probe(struct i2c_client *client,
>  
>  		bmc150_accel_interrupts_setup(indio_dev, data);
>  
> -		data->dready_trig = devm_iio_trigger_alloc(&client->dev,
> -							   "%s-dev%d",
> -							   indio_dev->name,
> -							   indio_dev->id);
> -		if (!data->dready_trig)
> -			return -ENOMEM;
> -
> -		data->motion_trig = devm_iio_trigger_alloc(&client->dev,
> -							  "%s-any-motion-dev%d",
> -							  indio_dev->name,
> -							  indio_dev->id);
> -		if (!data->motion_trig)
> -			return -ENOMEM;
> -
> -		data->dready_trig->dev.parent = &client->dev;
> -		data->dready_trig->ops = &bmc150_accel_trigger_ops;
> -		iio_trigger_set_drvdata(data->dready_trig, indio_dev);
> -		ret = iio_trigger_register(data->dready_trig);
> +		ret = bmc150_accel_triggers_setup(indio_dev, data);
>  		if (ret)
>  			return ret;
>  
> -		data->motion_trig->dev.parent = &client->dev;
> -		data->motion_trig->ops = &bmc150_accel_trigger_ops;
> -		iio_trigger_set_drvdata(data->motion_trig, indio_dev);
> -		ret = iio_trigger_register(data->motion_trig);
> -		if (ret) {
> -			data->motion_trig = NULL;
> -			goto err_trigger_unregister;
> -		}
> -
>  		ret = iio_triggered_buffer_setup(indio_dev,
>  						 &iio_pollfunc_store_time,
>  						 bmc150_accel_trigger_handler,
> @@ -1284,13 +1337,10 @@ static int bmc150_accel_probe(struct i2c_client *client,
>  err_iio_unregister:
>  	iio_device_unregister(indio_dev);
>  err_buffer_cleanup:
> -	if (data->dready_trig)
> +	if (indio_dev->pollfunc)
>  		iio_triggered_buffer_cleanup(indio_dev);
>  err_trigger_unregister:
> -	if (data->dready_trig)
> -		iio_trigger_unregister(data->dready_trig);
> -	if (data->motion_trig)
> -		iio_trigger_unregister(data->motion_trig);
> +	bmc150_accel_unregister_triggers(data, BMC150_ACCEL_TRIGGERS - 1);
>  
>  	return ret;
>  }
> @@ -1306,11 +1356,7 @@ static int bmc150_accel_remove(struct i2c_client *client)
>  
>  	iio_device_unregister(indio_dev);
>  
> -	if (data->dready_trig) {
> -		iio_triggered_buffer_cleanup(indio_dev);
> -		iio_trigger_unregister(data->dready_trig);
> -		iio_trigger_unregister(data->motion_trig);
> -	}
> +	bmc150_accel_unregister_triggers(data, BMC150_ACCEL_TRIGGERS - 1);
>  
>  	mutex_lock(&data->mutex);
>  	bmc150_accel_set_mode(data, BMC150_ACCEL_SLEEP_MODE_DEEP_SUSPEND, 0);
>