[PATCH v3 00/11] iio: adc: ti-ads1015: fixes, cleanups, and threshold event support

[PATCH v3 00/11] iio: adc: ti-ads1015: fixes, cleanups, and threshold event support

[Akinobu Mita]

This series contains some fixes and cleanups, and a threshold event support
for ti-ads1015 driver.

The first six patches are bug fixes, next four are cleanups, and the last
one adds support threshold event.

Changes from v2 (All changes are done in the last patch)
- Add an error checking for conv register read in interrupt handler
- Fix comparator polarity field setting in config register

Changes from v1
- Split the patch "enable conversion when CONFIG_PM is not set" into a pure
  fix and a cleanup
- Add new patch "don't return invalid value from buffer setup callbacks"
- Add new patch "add adequate wait time to get correct conversion"
- Support traditional comparator mode in addition to window comparator mode,
  add a comment for default threshold settings, and allow setting the state
  that has already been set in threshold event support patch

Akinobu Mita (11):
  iio: adc: ti-ads1015: fix incorrect data rate setting update
  iio: adc: ti-ads1015: fix scale information for ADS1115
  iio: adc: ti-ads1015: enable conversion when CONFIG_PM is not set
  iio: adc: ti-ads1015: avoid getting stale result after runtime resume
  iio: adc: ti-ads1015: don't return invalid value from buffer setup
    callbacks
  iio: adc: ti-ads1015: add adequate wait time to get correct conversion
  iio: adc: ti-ads1015: remove unnecessary config register update
  iio: adc: ti-ads1015: add helper to set conversion mode
  iio: adc: ti-ads1015: use devm_iio_triggered_buffer_setup
  iio: adc: ti-ads1015: use iio_device_claim_direct_mode()
  iio: adc: ti-ads1015: add threshold event support

 drivers/iio/adc/ti-ads1015.c | 594 ++++++++++++++++++++++++++++++++++++-------
 1 file changed, 500 insertions(+), 94 deletions(-)

Cc: Daniel Baluta <daniel.baluta@gmail.com>
Cc: Jonathan Cameron <jic23@kernel.org>
-- 
2.7.4

[PATCH v3 01/11] iio: adc: ti-ads1015: fix incorrect data rate setting update

[Akinobu Mita]

The ti-ads1015 driver has eight iio voltage channels and each iio channel
can hold own sampling frequency information.

The ADS1015 device only have a single config register which contains an
input multiplexer selection, PGA and data rate settings.  So the driver
should load the correct settings when the input multiplexer selection is
changed.

However, regardless of which channlel is currently selected, changing any
iio channel's sampling frequency information immediately overwrites the
current data rate setting in the config register.

It breaks the current data rate setting if the different channel's sampling
frequency information is changed because the data rate setting is not
reloaded when the input multiplexer is switched.

This removes the unexpected config register update and correctly load the
data rate setting before getting adc result.

Cc: Daniel Baluta <daniel.baluta@gmail.com>
Cc: Jonathan Cameron <jic23@kernel.org>
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
---
 drivers/iio/adc/ti-ads1015.c | 27 ++++++++++-----------------
 1 file changed, 10 insertions(+), 17 deletions(-)

diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
index 884b8e46..443444c 100644
--- a/drivers/iio/adc/ti-ads1015.c
+++ b/drivers/iio/adc/ti-ads1015.c
@@ -252,9 +252,11 @@ int ads1015_get_adc_result(struct ads1015_data *data, int chan, int *val)
 
 	ret = regmap_update_bits_check(data->regmap, ADS1015_CFG_REG,
 				       ADS1015_CFG_MUX_MASK |
-				       ADS1015_CFG_PGA_MASK,
+				       ADS1015_CFG_PGA_MASK |
+				       ADS1015_CFG_DR_MASK,
 				       chan << ADS1015_CFG_MUX_SHIFT |
-				       pga << ADS1015_CFG_PGA_SHIFT,
+				       pga << ADS1015_CFG_PGA_SHIFT |
+				       dr << ADS1015_CFG_DR_SHIFT,
 				       &change);
 	if (ret < 0)
 		return ret;
@@ -325,25 +327,16 @@ static int ads1015_set_scale(struct ads1015_data *data, int chan,
 
 static int ads1015_set_data_rate(struct ads1015_data *data, int chan, int rate)
 {
-	int i, ret, rindex = -1;
+	int i;
 
-	for (i = 0; i < ARRAY_SIZE(ads1015_data_rate); i++)
+	for (i = 0; i < ARRAY_SIZE(ads1015_data_rate); i++) {
 		if (data->data_rate[i] == rate) {
-			rindex = i;
-			break;
+			data->channel_data[chan].data_rate = i;
+			return 0;
 		}
-	if (rindex < 0)
-		return -EINVAL;
-
-	ret = regmap_update_bits(data->regmap, ADS1015_CFG_REG,
-				 ADS1015_CFG_DR_MASK,
-				 rindex << ADS1015_CFG_DR_SHIFT);
-	if (ret < 0)
-		return ret;
-
-	data->channel_data[chan].data_rate = rindex;
+	}
 
-	return 0;
+	return -EINVAL;
 }
 
 static int ads1015_read_raw(struct iio_dev *indio_dev,
-- 
2.7.4

Re: [PATCH v3 01/11] iio: adc: ti-ads1015: fix incorrect data rate setting update

[Jonathan Cameron]

On Mon, 31 Jul 2017 00:12:45 +0900
Akinobu Mita <akinobu.mita@gmail.com> wrote:

> The ti-ads1015 driver has eight iio voltage channels and each iio channel
> can hold own sampling frequency information.
> 
> The ADS1015 device only have a single config register which contains an
> input multiplexer selection, PGA and data rate settings.  So the driver
> should load the correct settings when the input multiplexer selection is
> changed.
> 
> However, regardless of which channlel is currently selected, changing any
> iio channel's sampling frequency information immediately overwrites the
> current data rate setting in the config register.
> 
> It breaks the current data rate setting if the different channel's sampling
> frequency information is changed because the data rate setting is not
> reloaded when the input multiplexer is switched.
> 
> This removes the unexpected config register update and correctly load the
> data rate setting before getting adc result.
> 
> Cc: Daniel Baluta <daniel.baluta@gmail.com>
> Cc: Jonathan Cameron <jic23@kernel.org>
> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
Applied to the fixes-togreg branch of iio.git and marked
for stable.

The only minor thing that you could have done to make
this series even better would be to have figured out
appropriate Fixes: tags to make the job of the stable
maintainers a little easier. 

Mostly I think it is a case of 'if it will apply
then it should' here though so I haven't dug into it
enough to tag them all.

Thanks,

Jonathan
> ---
>  drivers/iio/adc/ti-ads1015.c | 27 ++++++++++-----------------
>  1 file changed, 10 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
> index 884b8e46..443444c 100644
> --- a/drivers/iio/adc/ti-ads1015.c
> +++ b/drivers/iio/adc/ti-ads1015.c
> @@ -252,9 +252,11 @@ int ads1015_get_adc_result(struct ads1015_data *data, int chan, int *val)
>  
>  	ret = regmap_update_bits_check(data->regmap, ADS1015_CFG_REG,
>  				       ADS1015_CFG_MUX_MASK |
> -				       ADS1015_CFG_PGA_MASK,
> +				       ADS1015_CFG_PGA_MASK |
> +				       ADS1015_CFG_DR_MASK,
>  				       chan << ADS1015_CFG_MUX_SHIFT |
> -				       pga << ADS1015_CFG_PGA_SHIFT,
> +				       pga << ADS1015_CFG_PGA_SHIFT |
> +				       dr << ADS1015_CFG_DR_SHIFT,
>  				       &change);
>  	if (ret < 0)
>  		return ret;
> @@ -325,25 +327,16 @@ static int ads1015_set_scale(struct ads1015_data *data, int chan,
>  
>  static int ads1015_set_data_rate(struct ads1015_data *data, int chan, int rate)
>  {
> -	int i, ret, rindex = -1;
> +	int i;
>  
> -	for (i = 0; i < ARRAY_SIZE(ads1015_data_rate); i++)
> +	for (i = 0; i < ARRAY_SIZE(ads1015_data_rate); i++) {
>  		if (data->data_rate[i] == rate) {
> -			rindex = i;
> -			break;
> +			data->channel_data[chan].data_rate = i;
> +			return 0;
>  		}
> -	if (rindex < 0)
> -		return -EINVAL;
> -
> -	ret = regmap_update_bits(data->regmap, ADS1015_CFG_REG,
> -				 ADS1015_CFG_DR_MASK,
> -				 rindex << ADS1015_CFG_DR_SHIFT);
> -	if (ret < 0)
> -		return ret;
> -
> -	data->channel_data[chan].data_rate = rindex;
> +	}
>  
> -	return 0;
> +	return -EINVAL;
>  }
>  
>  static int ads1015_read_raw(struct iio_dev *indio_dev,

[PATCH v3 02/11] iio: adc: ti-ads1015: fix scale information for ADS1115

[Akinobu Mita]

The ti-ads1015 driver supports ADS1015 and ADS1115 devices.  The same
scale information is used for both devices in this driver, however they
have actually different values and the ADS1115's one is not correct.

These devices have the same full-scale input voltage range for each PGA
selection.  So instead of adding another hardcoded scale information,
compute a correct scale on demand from each device's resolution.

Cc: Daniel Baluta <daniel.baluta@gmail.com>
Cc: Jonathan Cameron <jic23@kernel.org>
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
---
 drivers/iio/adc/ti-ads1015.c | 48 ++++++++++++++++++++++----------------------
 1 file changed, 24 insertions(+), 24 deletions(-)

diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
index 443444c..f32d046 100644
--- a/drivers/iio/adc/ti-ads1015.c
+++ b/drivers/iio/adc/ti-ads1015.c
@@ -81,18 +81,12 @@ static const unsigned int ads1115_data_rate[] = {
 	8, 16, 32, 64, 128, 250, 475, 860
 };
 
-static const struct {
-	int scale;
-	int uscale;
-} ads1015_scale[] = {
-	{3, 0},
-	{2, 0},
-	{1, 0},
-	{0, 500000},
-	{0, 250000},
-	{0, 125000},
-	{0, 125000},
-	{0, 125000},
+/*
+ * Translation from PGA bits to full-scale positive and negative input voltage
+ * range in mV
+ */
+static int ads1015_fullscale_range[] = {
+	6144, 4096, 2048, 1024, 512, 256, 256, 256
 };
 
 #define ADS1015_V_CHAN(_chan, _addr) {				\
@@ -300,17 +294,20 @@ static irqreturn_t ads1015_trigger_handler(int irq, void *p)
 	return IRQ_HANDLED;
 }
 
-static int ads1015_set_scale(struct ads1015_data *data, int chan,
+static int ads1015_set_scale(struct ads1015_data *data,
+			     struct iio_chan_spec const *chan,
 			     int scale, int uscale)
 {
 	int i, ret, rindex = -1;
+	int fullscale = div_s64((scale * 1000000LL + uscale) <<
+				(chan->scan_type.realbits - 1), 1000000);
 
-	for (i = 0; i < ARRAY_SIZE(ads1015_scale); i++)
-		if (ads1015_scale[i].scale == scale &&
-		    ads1015_scale[i].uscale == uscale) {
+	for (i = 0; i < ARRAY_SIZE(ads1015_fullscale_range); i++) {
+		if (ads1015_fullscale_range[i] == fullscale) {
 			rindex = i;
 			break;
 		}
+	}
 	if (rindex < 0)
 		return -EINVAL;
 
@@ -320,7 +317,7 @@ static int ads1015_set_scale(struct ads1015_data *data, int chan,
 	if (ret < 0)
 		return ret;
 
-	data->channel_data[chan].pga = rindex;
+	data->channel_data[chan->address].pga = rindex;
 
 	return 0;
 }
@@ -378,9 +375,9 @@ static int ads1015_read_raw(struct iio_dev *indio_dev,
 	}
 	case IIO_CHAN_INFO_SCALE:
 		idx = data->channel_data[chan->address].pga;
-		*val = ads1015_scale[idx].scale;
-		*val2 = ads1015_scale[idx].uscale;
-		ret = IIO_VAL_INT_PLUS_MICRO;
+		*val = ads1015_fullscale_range[idx];
+		*val2 = chan->scan_type.realbits - 1;
+		ret = IIO_VAL_FRACTIONAL_LOG2;
 		break;
 	case IIO_CHAN_INFO_SAMP_FREQ:
 		idx = data->channel_data[chan->address].data_rate;
@@ -407,7 +404,7 @@ static int ads1015_write_raw(struct iio_dev *indio_dev,
 	mutex_lock(&data->lock);
 	switch (mask) {
 	case IIO_CHAN_INFO_SCALE:
-		ret = ads1015_set_scale(data, chan->address, val, val2);
+		ret = ads1015_set_scale(data, chan, val, val2);
 		break;
 	case IIO_CHAN_INFO_SAMP_FREQ:
 		ret = ads1015_set_data_rate(data, chan->address, val);
@@ -439,7 +436,10 @@ static const struct iio_buffer_setup_ops ads1015_buffer_setup_ops = {
 	.validate_scan_mask = &iio_validate_scan_mask_onehot,
 };
 
-static IIO_CONST_ATTR(scale_available, "3 2 1 0.5 0.25 0.125");
+static IIO_CONST_ATTR_NAMED(ads1015_scale_available, scale_available,
+	"3 2 1 0.5 0.25 0.125");
+static IIO_CONST_ATTR_NAMED(ads1115_scale_available, scale_available,
+	"0.1875 0.125 0.0625 0.03125 0.015625 0.007813");
 
 static IIO_CONST_ATTR_NAMED(ads1015_sampling_frequency_available,
 	sampling_frequency_available, "128 250 490 920 1600 2400 3300");
@@ -447,7 +447,7 @@ static IIO_CONST_ATTR_NAMED(ads1115_sampling_frequency_available,
 	sampling_frequency_available, "8 16 32 64 128 250 475 860");
 
 static struct attribute *ads1015_attributes[] = {
-	&iio_const_attr_scale_available.dev_attr.attr,
+	&iio_const_attr_ads1015_scale_available.dev_attr.attr,
 	&iio_const_attr_ads1015_sampling_frequency_available.dev_attr.attr,
 	NULL,
 };
@@ -457,7 +457,7 @@ static const struct attribute_group ads1015_attribute_group = {
 };
 
 static struct attribute *ads1115_attributes[] = {
-	&iio_const_attr_scale_available.dev_attr.attr,
+	&iio_const_attr_ads1115_scale_available.dev_attr.attr,
 	&iio_const_attr_ads1115_sampling_frequency_available.dev_attr.attr,
 	NULL,
 };
-- 
2.7.4

Re: [PATCH v3 02/11] iio: adc: ti-ads1015: fix scale information for ADS1115

[Jonathan Cameron]

On Mon, 31 Jul 2017 00:12:46 +0900
Akinobu Mita <akinobu.mita@gmail.com> wrote:

> The ti-ads1015 driver supports ADS1015 and ADS1115 devices.  The same
> scale information is used for both devices in this driver, however they
> have actually different values and the ADS1115's one is not correct.
> 
> These devices have the same full-scale input voltage range for each PGA
> selection.  So instead of adding another hardcoded scale information,
> compute a correct scale on demand from each device's resolution.
> 
> Cc: Daniel Baluta <daniel.baluta@gmail.com>
> Cc: Jonathan Cameron <jic23@kernel.org>
> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
Applied to the fixes-togreg branch of iio.git and marked for stable.

Thanks,

Jonathan
> ---
>  drivers/iio/adc/ti-ads1015.c | 48 ++++++++++++++++++++++----------------------
>  1 file changed, 24 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
> index 443444c..f32d046 100644
> --- a/drivers/iio/adc/ti-ads1015.c
> +++ b/drivers/iio/adc/ti-ads1015.c
> @@ -81,18 +81,12 @@ static const unsigned int ads1115_data_rate[] = {
>  	8, 16, 32, 64, 128, 250, 475, 860
>  };
>  
> -static const struct {
> -	int scale;
> -	int uscale;
> -} ads1015_scale[] = {
> -	{3, 0},
> -	{2, 0},
> -	{1, 0},
> -	{0, 500000},
> -	{0, 250000},
> -	{0, 125000},
> -	{0, 125000},
> -	{0, 125000},
> +/*
> + * Translation from PGA bits to full-scale positive and negative input voltage
> + * range in mV
> + */
> +static int ads1015_fullscale_range[] = {
> +	6144, 4096, 2048, 1024, 512, 256, 256, 256
>  };
>  
>  #define ADS1015_V_CHAN(_chan, _addr) {				\
> @@ -300,17 +294,20 @@ static irqreturn_t ads1015_trigger_handler(int irq, void *p)
>  	return IRQ_HANDLED;
>  }
>  
> -static int ads1015_set_scale(struct ads1015_data *data, int chan,
> +static int ads1015_set_scale(struct ads1015_data *data,
> +			     struct iio_chan_spec const *chan,
>  			     int scale, int uscale)
>  {
>  	int i, ret, rindex = -1;
> +	int fullscale = div_s64((scale * 1000000LL + uscale) <<
> +				(chan->scan_type.realbits - 1), 1000000);
>  
> -	for (i = 0; i < ARRAY_SIZE(ads1015_scale); i++)
> -		if (ads1015_scale[i].scale == scale &&
> -		    ads1015_scale[i].uscale == uscale) {
> +	for (i = 0; i < ARRAY_SIZE(ads1015_fullscale_range); i++) {
> +		if (ads1015_fullscale_range[i] == fullscale) {
>  			rindex = i;
>  			break;
>  		}
> +	}
>  	if (rindex < 0)
>  		return -EINVAL;
>  
> @@ -320,7 +317,7 @@ static int ads1015_set_scale(struct ads1015_data *data, int chan,
>  	if (ret < 0)
>  		return ret;
>  
> -	data->channel_data[chan].pga = rindex;
> +	data->channel_data[chan->address].pga = rindex;
>  
>  	return 0;
>  }
> @@ -378,9 +375,9 @@ static int ads1015_read_raw(struct iio_dev *indio_dev,
>  	}
>  	case IIO_CHAN_INFO_SCALE:
>  		idx = data->channel_data[chan->address].pga;
> -		*val = ads1015_scale[idx].scale;
> -		*val2 = ads1015_scale[idx].uscale;
> -		ret = IIO_VAL_INT_PLUS_MICRO;
> +		*val = ads1015_fullscale_range[idx];
> +		*val2 = chan->scan_type.realbits - 1;
> +		ret = IIO_VAL_FRACTIONAL_LOG2;
>  		break;
>  	case IIO_CHAN_INFO_SAMP_FREQ:
>  		idx = data->channel_data[chan->address].data_rate;
> @@ -407,7 +404,7 @@ static int ads1015_write_raw(struct iio_dev *indio_dev,
>  	mutex_lock(&data->lock);
>  	switch (mask) {
>  	case IIO_CHAN_INFO_SCALE:
> -		ret = ads1015_set_scale(data, chan->address, val, val2);
> +		ret = ads1015_set_scale(data, chan, val, val2);
>  		break;
>  	case IIO_CHAN_INFO_SAMP_FREQ:
>  		ret = ads1015_set_data_rate(data, chan->address, val);
> @@ -439,7 +436,10 @@ static const struct iio_buffer_setup_ops ads1015_buffer_setup_ops = {
>  	.validate_scan_mask = &iio_validate_scan_mask_onehot,
>  };
>  
> -static IIO_CONST_ATTR(scale_available, "3 2 1 0.5 0.25 0.125");
> +static IIO_CONST_ATTR_NAMED(ads1015_scale_available, scale_available,
> +	"3 2 1 0.5 0.25 0.125");
> +static IIO_CONST_ATTR_NAMED(ads1115_scale_available, scale_available,
> +	"0.1875 0.125 0.0625 0.03125 0.015625 0.007813");
>  
>  static IIO_CONST_ATTR_NAMED(ads1015_sampling_frequency_available,
>  	sampling_frequency_available, "128 250 490 920 1600 2400 3300");
> @@ -447,7 +447,7 @@ static IIO_CONST_ATTR_NAMED(ads1115_sampling_frequency_available,
>  	sampling_frequency_available, "8 16 32 64 128 250 475 860");
>  
>  static struct attribute *ads1015_attributes[] = {
> -	&iio_const_attr_scale_available.dev_attr.attr,
> +	&iio_const_attr_ads1015_scale_available.dev_attr.attr,
>  	&iio_const_attr_ads1015_sampling_frequency_available.dev_attr.attr,
>  	NULL,
>  };
> @@ -457,7 +457,7 @@ static const struct attribute_group ads1015_attribute_group = {
>  };
>  
>  static struct attribute *ads1115_attributes[] = {
> -	&iio_const_attr_scale_available.dev_attr.attr,
> +	&iio_const_attr_ads1115_scale_available.dev_attr.attr,
>  	&iio_const_attr_ads1115_sampling_frequency_available.dev_attr.attr,
>  	NULL,
>  };

[PATCH v3 03/11] iio: adc: ti-ads1015: enable conversion when CONFIG_PM is not set

[Akinobu Mita]

The ADS1015 device have two operating modes, continuous conversion mode
and single-shot mode.  This driver assumes that the continuous conversion
mode is selected by runtime resume callback when the ADC result is
requested.

If CONFIG_PM is disabled, the device is always in the default single-shot
mode and no one begins a single conversion.  So the conversion register
doesn't contain valid ADC result.  Fix it by changing the continuous mode
in probe function.

Cc: Daniel Baluta <daniel.baluta@gmail.com>
Cc: Jonathan Cameron <jic23@kernel.org>
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
---
 drivers/iio/adc/ti-ads1015.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
index f32d046..fcfa570 100644
--- a/drivers/iio/adc/ti-ads1015.c
+++ b/drivers/iio/adc/ti-ads1015.c
@@ -623,6 +623,13 @@ static int ads1015_probe(struct i2c_client *client,
 		dev_err(&client->dev, "iio triggered buffer setup failed\n");
 		return ret;
 	}
+
+	ret = regmap_update_bits(data->regmap, ADS1015_CFG_REG,
+				ADS1015_CFG_MOD_MASK,
+				ADS1015_CONTINUOUS << ADS1015_CFG_MOD_SHIFT);
+	if (ret)
+		return ret;
+
 	ret = pm_runtime_set_active(&client->dev);
 	if (ret)
 		goto err_buffer_cleanup;
-- 
2.7.4

Re: [PATCH v3 03/11] iio: adc: ti-ads1015: enable conversion when CONFIG_PM is not set

[Jonathan Cameron]

On Mon, 31 Jul 2017 00:12:47 +0900
Akinobu Mita <akinobu.mita@gmail.com> wrote:

> The ADS1015 device have two operating modes, continuous conversion mode
> and single-shot mode.  This driver assumes that the continuous conversion
> mode is selected by runtime resume callback when the ADC result is
> requested.
> 
> If CONFIG_PM is disabled, the device is always in the default single-shot
> mode and no one begins a single conversion.  So the conversion register
> doesn't contain valid ADC result.  Fix it by changing the continuous mode
> in probe function.
> 
> Cc: Daniel Baluta <daniel.baluta@gmail.com>
> Cc: Jonathan Cameron <jic23@kernel.org>
> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
Applied to the fixes-togreg branch of iio.git.

thanks,

Jonathan
> ---
>  drivers/iio/adc/ti-ads1015.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
> index f32d046..fcfa570 100644
> --- a/drivers/iio/adc/ti-ads1015.c
> +++ b/drivers/iio/adc/ti-ads1015.c
> @@ -623,6 +623,13 @@ static int ads1015_probe(struct i2c_client *client,
>  		dev_err(&client->dev, "iio triggered buffer setup failed\n");
>  		return ret;
>  	}
> +
> +	ret = regmap_update_bits(data->regmap, ADS1015_CFG_REG,
> +				ADS1015_CFG_MOD_MASK,
> +				ADS1015_CONTINUOUS << ADS1015_CFG_MOD_SHIFT);
> +	if (ret)
> +		return ret;
> +
>  	ret = pm_runtime_set_active(&client->dev);
>  	if (ret)
>  		goto err_buffer_cleanup;

[PATCH v3 04/11] iio: adc: ti-ads1015: avoid getting stale result after runtime resume

[Akinobu Mita]

This driver assumes that the device is operating in the continuous
conversion mode which performs the conversion continuously.  So this driver
doesn't insert a wait time before reading the conversion register if the
configuration is not changed from a previous request.

This assumption is broken if the device is runtime suspended and entered
a power-down state.  The forthcoming request causes reading a stale result
from the conversion register as the device is runtime resumed just before.

Fix it by adding a flag to detect that condition and insert a necessary
wait time.

Cc: Daniel Baluta <daniel.baluta@gmail.com>
Cc: Jonathan Cameron <jic23@kernel.org>
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
---
 drivers/iio/adc/ti-ads1015.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
index fcfa570..8905f0d 100644
--- a/drivers/iio/adc/ti-ads1015.c
+++ b/drivers/iio/adc/ti-ads1015.c
@@ -177,6 +177,12 @@ struct ads1015_data {
 	struct ads1015_channel_data channel_data[ADS1015_CHANNELS];
 
 	unsigned int *data_rate;
+	/*
+	 * Set to true when the ADC is switched to the continuous-conversion
+	 * mode and exits from a power-down state.  This flag is used to avoid
+	 * getting the stale result from the conversion register.
+	 */
+	bool conv_invalid;
 };
 
 static bool ads1015_is_writeable_reg(struct device *dev, unsigned int reg)
@@ -255,9 +261,10 @@ int ads1015_get_adc_result(struct ads1015_data *data, int chan, int *val)
 	if (ret < 0)
 		return ret;
 
-	if (change) {
+	if (change || data->conv_invalid) {
 		conv_time = DIV_ROUND_UP(USEC_PER_SEC, data->data_rate[dr]);
 		usleep_range(conv_time, conv_time + 1);
+		data->conv_invalid = false;
 	}
 
 	return regmap_read(data->regmap, ADS1015_CONV_REG, val);
@@ -630,6 +637,8 @@ static int ads1015_probe(struct i2c_client *client,
 	if (ret)
 		return ret;
 
+	data->conv_invalid = true;
+
 	ret = pm_runtime_set_active(&client->dev);
 	if (ret)
 		goto err_buffer_cleanup;
@@ -685,10 +694,15 @@ static int ads1015_runtime_resume(struct device *dev)
 {
 	struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
 	struct ads1015_data *data = iio_priv(indio_dev);
+	int ret;
 
-	return regmap_update_bits(data->regmap, ADS1015_CFG_REG,
+	ret = regmap_update_bits(data->regmap, ADS1015_CFG_REG,
 				  ADS1015_CFG_MOD_MASK,
 				  ADS1015_CONTINUOUS << ADS1015_CFG_MOD_SHIFT);
+	if (!ret)
+		data->conv_invalid = true;
+
+	return ret;
 }
 #endif
 
-- 
2.7.4

Re: [PATCH v3 04/11] iio: adc: ti-ads1015: avoid getting stale result after runtime resume

[Jonathan Cameron]

On Mon, 31 Jul 2017 00:12:48 +0900
Akinobu Mita <akinobu.mita@gmail.com> wrote:

> This driver assumes that the device is operating in the continuous
> conversion mode which performs the conversion continuously.  So this driver
> doesn't insert a wait time before reading the conversion register if the
> configuration is not changed from a previous request.
> 
> This assumption is broken if the device is runtime suspended and entered
> a power-down state.  The forthcoming request causes reading a stale result
> from the conversion register as the device is runtime resumed just before.
> 
> Fix it by adding a flag to detect that condition and insert a necessary
> wait time.
> 
> Cc: Daniel Baluta <daniel.baluta@gmail.com>
> Cc: Jonathan Cameron <jic23@kernel.org>
> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
Applied to the fixes-togreg branch of iio.git and marked for stable.

Thanks,

Jonathan
> ---
>  drivers/iio/adc/ti-ads1015.c | 18 ++++++++++++++++--
>  1 file changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
> index fcfa570..8905f0d 100644
> --- a/drivers/iio/adc/ti-ads1015.c
> +++ b/drivers/iio/adc/ti-ads1015.c
> @@ -177,6 +177,12 @@ struct ads1015_data {
>  	struct ads1015_channel_data channel_data[ADS1015_CHANNELS];
>  
>  	unsigned int *data_rate;
> +	/*
> +	 * Set to true when the ADC is switched to the continuous-conversion
> +	 * mode and exits from a power-down state.  This flag is used to avoid
> +	 * getting the stale result from the conversion register.
> +	 */
> +	bool conv_invalid;
>  };
>  
>  static bool ads1015_is_writeable_reg(struct device *dev, unsigned int reg)
> @@ -255,9 +261,10 @@ int ads1015_get_adc_result(struct ads1015_data *data, int chan, int *val)
>  	if (ret < 0)
>  		return ret;
>  
> -	if (change) {
> +	if (change || data->conv_invalid) {
>  		conv_time = DIV_ROUND_UP(USEC_PER_SEC, data->data_rate[dr]);
>  		usleep_range(conv_time, conv_time + 1);
> +		data->conv_invalid = false;
>  	}
>  
>  	return regmap_read(data->regmap, ADS1015_CONV_REG, val);
> @@ -630,6 +637,8 @@ static int ads1015_probe(struct i2c_client *client,
>  	if (ret)
>  		return ret;
>  
> +	data->conv_invalid = true;
> +
>  	ret = pm_runtime_set_active(&client->dev);
>  	if (ret)
>  		goto err_buffer_cleanup;
> @@ -685,10 +694,15 @@ static int ads1015_runtime_resume(struct device *dev)
>  {
>  	struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
>  	struct ads1015_data *data = iio_priv(indio_dev);
> +	int ret;
>  
> -	return regmap_update_bits(data->regmap, ADS1015_CFG_REG,
> +	ret = regmap_update_bits(data->regmap, ADS1015_CFG_REG,
>  				  ADS1015_CFG_MOD_MASK,
>  				  ADS1015_CONTINUOUS << ADS1015_CFG_MOD_SHIFT);
> +	if (!ret)
> +		data->conv_invalid = true;
> +
> +	return ret;
>  }
>  #endif
>  

[PATCH v3 05/11] iio: adc: ti-ads1015: don't return invalid value from buffer setup callbacks

[Akinobu Mita]

pm_runtime_get_sync() and pm_runtime_put_autosuspend() return 0 on
success, 1 if the device's runtime PM status was already requested status
or error code on failure.  So a positive return value doesn't indicate an
error condition.

However, any non-zero return values from buffer preenable and postdisable
callbacks are recognized as an error and this driver reuses the return
value from pm_runtime_get_sync() and pm_runtime_put_autosuspend() in
these callbacks.  This change fixes the false error detections.

Cc: Daniel Baluta <daniel.baluta@gmail.com>
Cc: Jonathan Cameron <jic23@kernel.org>
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
---
 drivers/iio/adc/ti-ads1015.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
index 8905f0d..1c475e2 100644
--- a/drivers/iio/adc/ti-ads1015.c
+++ b/drivers/iio/adc/ti-ads1015.c
@@ -235,7 +235,7 @@ static int ads1015_set_power_state(struct ads1015_data *data, bool on)
 		ret = pm_runtime_put_autosuspend(dev);
 	}
 
-	return ret;
+	return ret < 0 ? ret : 0;
 }
 
 static
-- 
2.7.4

Re: [PATCH v3 05/11] iio: adc: ti-ads1015: don't return invalid value from buffer setup callbacks

[Jonathan Cameron]

On Mon, 31 Jul 2017 00:12:49 +0900
Akinobu Mita <akinobu.mita@gmail.com> wrote:

> pm_runtime_get_sync() and pm_runtime_put_autosuspend() return 0 on
> success, 1 if the device's runtime PM status was already requested status
> or error code on failure.  So a positive return value doesn't indicate an
> error condition.
> 
> However, any non-zero return values from buffer preenable and postdisable
> callbacks are recognized as an error and this driver reuses the return
> value from pm_runtime_get_sync() and pm_runtime_put_autosuspend() in
> these callbacks.  This change fixes the false error detections.
> 
> Cc: Daniel Baluta <daniel.baluta@gmail.com>
> Cc: Jonathan Cameron <jic23@kernel.org>
> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
Applied to the fixes-togreg branch of iio.git marked for
stable.

Thanks,

Jonathan
> ---
>  drivers/iio/adc/ti-ads1015.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
> index 8905f0d..1c475e2 100644
> --- a/drivers/iio/adc/ti-ads1015.c
> +++ b/drivers/iio/adc/ti-ads1015.c
> @@ -235,7 +235,7 @@ static int ads1015_set_power_state(struct ads1015_data *data, bool on)
>  		ret = pm_runtime_put_autosuspend(dev);
>  	}
>  
> -	return ret;
> +	return ret < 0 ? ret : 0;
>  }
>  
>  static

[PATCH v3 06/11] iio: adc: ti-ads1015: add adequate wait time to get correct conversion

[Akinobu Mita]

This driver assumes that the device is operating in the continuous
conversion mode which performs the conversion continuously.  So this driver
inserts a wait time before reading the conversion register if the
configuration is changed from a previous request.

Currently, the wait time is only the period required for a single
conversion that is calculated as the reciprocal of the sampling frequency.
However we also need to wait for the the previous conversion to complete.
Otherwise we probably get the conversion result for the previous
configuration when the sampling frequency is lower.

Cc: Daniel Baluta <daniel.baluta@gmail.com>
Cc: Jonathan Cameron <jic23@kernel.org>
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
---
 drivers/iio/adc/ti-ads1015.c | 31 +++++++++++++++++++------------
 1 file changed, 19 insertions(+), 12 deletions(-)

diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
index 1c475e2..9c501e5 100644
--- a/drivers/iio/adc/ti-ads1015.c
+++ b/drivers/iio/adc/ti-ads1015.c
@@ -242,27 +242,34 @@ static
 int ads1015_get_adc_result(struct ads1015_data *data, int chan, int *val)
 {
 	int ret, pga, dr, conv_time;
-	bool change;
+	unsigned int old, mask, cfg;
 
 	if (chan < 0 || chan >= ADS1015_CHANNELS)
 		return -EINVAL;
 
+	ret = regmap_read(data->regmap, ADS1015_CFG_REG, &old);
+	if (ret)
+		return ret;
+
 	pga = data->channel_data[chan].pga;
 	dr = data->channel_data[chan].data_rate;
+	mask = ADS1015_CFG_MUX_MASK | ADS1015_CFG_PGA_MASK |
+		ADS1015_CFG_DR_MASK;
+	cfg = chan << ADS1015_CFG_MUX_SHIFT | pga << ADS1015_CFG_PGA_SHIFT |
+		dr << ADS1015_CFG_DR_SHIFT;
 
-	ret = regmap_update_bits_check(data->regmap, ADS1015_CFG_REG,
-				       ADS1015_CFG_MUX_MASK |
-				       ADS1015_CFG_PGA_MASK |
-				       ADS1015_CFG_DR_MASK,
-				       chan << ADS1015_CFG_MUX_SHIFT |
-				       pga << ADS1015_CFG_PGA_SHIFT |
-				       dr << ADS1015_CFG_DR_SHIFT,
-				       &change);
-	if (ret < 0)
+	cfg = (old & ~mask) | (cfg & mask);
+
+	ret = regmap_write(data->regmap, ADS1015_CFG_REG, cfg);
+	if (ret)
 		return ret;
 
-	if (change || data->conv_invalid) {
-		conv_time = DIV_ROUND_UP(USEC_PER_SEC, data->data_rate[dr]);
+	if (old != cfg || data->conv_invalid) {
+		int dr_old = (old & ADS1015_CFG_DR_MASK) >>
+				ADS1015_CFG_DR_SHIFT;
+
+		conv_time = DIV_ROUND_UP(USEC_PER_SEC, data->data_rate[dr_old]);
+		conv_time += DIV_ROUND_UP(USEC_PER_SEC, data->data_rate[dr]);
 		usleep_range(conv_time, conv_time + 1);
 		data->conv_invalid = false;
 	}
-- 
2.7.4

Re: [PATCH v3 06/11] iio: adc: ti-ads1015: add adequate wait time to get correct conversion

[Jonathan Cameron]

On Mon, 31 Jul 2017 00:12:50 +0900
Akinobu Mita <akinobu.mita@gmail.com> wrote:

> This driver assumes that the device is operating in the continuous
> conversion mode which performs the conversion continuously.  So this driver
> inserts a wait time before reading the conversion register if the
> configuration is changed from a previous request.
> 
> Currently, the wait time is only the period required for a single
> conversion that is calculated as the reciprocal of the sampling frequency.
> However we also need to wait for the the previous conversion to complete.
> Otherwise we probably get the conversion result for the previous
> configuration when the sampling frequency is lower.
> 
> Cc: Daniel Baluta <daniel.baluta@gmail.com>
> Cc: Jonathan Cameron <jic23@kernel.org>
> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
Applied to the fixes-togreg branch of iio.git.

Unfortunately the rest of the series will have to wait for these
to work there way through Greg, to mainline and back to the iio.git
togreg branch which will take a week or two probably.

Please give me a bump if I seem to have forgotten about them.

Thanks,

Jonathan
> ---
>  drivers/iio/adc/ti-ads1015.c | 31 +++++++++++++++++++------------
>  1 file changed, 19 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
> index 1c475e2..9c501e5 100644
> --- a/drivers/iio/adc/ti-ads1015.c
> +++ b/drivers/iio/adc/ti-ads1015.c
> @@ -242,27 +242,34 @@ static
>  int ads1015_get_adc_result(struct ads1015_data *data, int chan, int *val)
>  {
>  	int ret, pga, dr, conv_time;
> -	bool change;
> +	unsigned int old, mask, cfg;
>  
>  	if (chan < 0 || chan >= ADS1015_CHANNELS)
>  		return -EINVAL;
>  
> +	ret = regmap_read(data->regmap, ADS1015_CFG_REG, &old);
> +	if (ret)
> +		return ret;
> +
>  	pga = data->channel_data[chan].pga;
>  	dr = data->channel_data[chan].data_rate;
> +	mask = ADS1015_CFG_MUX_MASK | ADS1015_CFG_PGA_MASK |
> +		ADS1015_CFG_DR_MASK;
> +	cfg = chan << ADS1015_CFG_MUX_SHIFT | pga << ADS1015_CFG_PGA_SHIFT |
> +		dr << ADS1015_CFG_DR_SHIFT;
>  
> -	ret = regmap_update_bits_check(data->regmap, ADS1015_CFG_REG,
> -				       ADS1015_CFG_MUX_MASK |
> -				       ADS1015_CFG_PGA_MASK |
> -				       ADS1015_CFG_DR_MASK,
> -				       chan << ADS1015_CFG_MUX_SHIFT |
> -				       pga << ADS1015_CFG_PGA_SHIFT |
> -				       dr << ADS1015_CFG_DR_SHIFT,
> -				       &change);
> -	if (ret < 0)
> +	cfg = (old & ~mask) | (cfg & mask);
> +
> +	ret = regmap_write(data->regmap, ADS1015_CFG_REG, cfg);
> +	if (ret)
>  		return ret;
>  
> -	if (change || data->conv_invalid) {
> -		conv_time = DIV_ROUND_UP(USEC_PER_SEC, data->data_rate[dr]);
> +	if (old != cfg || data->conv_invalid) {
> +		int dr_old = (old & ADS1015_CFG_DR_MASK) >>
> +				ADS1015_CFG_DR_SHIFT;
> +
> +		conv_time = DIV_ROUND_UP(USEC_PER_SEC, data->data_rate[dr_old]);
> +		conv_time += DIV_ROUND_UP(USEC_PER_SEC, data->data_rate[dr]);
>  		usleep_range(conv_time, conv_time + 1);
>  		data->conv_invalid = false;
>  	}

[PATCH v3 07/11] iio: adc: ti-ads1015: remove unnecessary config register update

[Akinobu Mita]

The ti-ads1015 driver has eight iio voltage channels and each iio channel
can hold own scale information.

The ADS1015 device only have a single config register which contains an
input multiplexer selection, PGA and data rate settings.  So the driver
should load the correct settings when the input multiplexer selection is
changed.

However, regardless of which channlel is currently selected, changing any
iio channel's scale information immediately overwrites the current PGA
setting in the config register.

It is harmless because the correct PGA settings are reloaded just before
getting adc result anyway.  But it is unnecessary register update and
should be removed.

Cc: Daniel Baluta <daniel.baluta@gmail.com>
Cc: Jonathan Cameron <jic23@kernel.org>
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
---
 drivers/iio/adc/ti-ads1015.c | 18 ++++--------------
 1 file changed, 4 insertions(+), 14 deletions(-)

diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
index 9c501e5..e83cebc 100644
--- a/drivers/iio/adc/ti-ads1015.c
+++ b/drivers/iio/adc/ti-ads1015.c
@@ -312,28 +312,18 @@ static int ads1015_set_scale(struct ads1015_data *data,
 			     struct iio_chan_spec const *chan,
 			     int scale, int uscale)
 {
-	int i, ret, rindex = -1;
+	int i;
 	int fullscale = div_s64((scale * 1000000LL + uscale) <<
 				(chan->scan_type.realbits - 1), 1000000);
 
 	for (i = 0; i < ARRAY_SIZE(ads1015_fullscale_range); i++) {
 		if (ads1015_fullscale_range[i] == fullscale) {
-			rindex = i;
-			break;
+			data->channel_data[chan->address].pga = i;
+			return 0;
 		}
 	}
-	if (rindex < 0)
-		return -EINVAL;
-
-	ret = regmap_update_bits(data->regmap, ADS1015_CFG_REG,
-				 ADS1015_CFG_PGA_MASK,
-				 rindex << ADS1015_CFG_PGA_SHIFT);
-	if (ret < 0)
-		return ret;
-
-	data->channel_data[chan->address].pga = rindex;
 
-	return 0;
+	return -EINVAL;
 }
 
 static int ads1015_set_data_rate(struct ads1015_data *data, int chan, int rate)
-- 
2.7.4

[PATCH v3 08/11] iio: adc: ti-ads1015: add helper to set conversion mode

[Akinobu Mita]

This adds a helper function to set conversion mode as there are a fair
number of users.

Cc: Daniel Baluta <daniel.baluta@gmail.com>
Cc: Jonathan Cameron <jic23@kernel.org>
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
---
 drivers/iio/adc/ti-ads1015.c | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
index e83cebc..9f3f0c9 100644
--- a/drivers/iio/adc/ti-ads1015.c
+++ b/drivers/iio/adc/ti-ads1015.c
@@ -570,6 +570,13 @@ static void ads1015_get_channels_config(struct i2c_client *client)
 	}
 }
 
+static int ads1015_set_conv_mode(struct ads1015_data *data, int mode)
+{
+	return regmap_update_bits(data->regmap, ADS1015_CFG_REG,
+				  ADS1015_CFG_MOD_MASK,
+				  mode << ADS1015_CFG_MOD_SHIFT);
+}
+
 static int ads1015_probe(struct i2c_client *client,
 			 const struct i2c_device_id *id)
 {
@@ -628,9 +635,7 @@ static int ads1015_probe(struct i2c_client *client,
 		return ret;
 	}
 
-	ret = regmap_update_bits(data->regmap, ADS1015_CFG_REG,
-				ADS1015_CFG_MOD_MASK,
-				ADS1015_CONTINUOUS << ADS1015_CFG_MOD_SHIFT);
+	ret = ads1015_set_conv_mode(data, ADS1015_CONTINUOUS);
 	if (ret)
 		return ret;
 
@@ -671,9 +676,7 @@ static int ads1015_remove(struct i2c_client *client)
 	iio_triggered_buffer_cleanup(indio_dev);
 
 	/* power down single shot mode */
-	return regmap_update_bits(data->regmap, ADS1015_CFG_REG,
-				  ADS1015_CFG_MOD_MASK,
-				  ADS1015_SINGLESHOT << ADS1015_CFG_MOD_SHIFT);
+	return ads1015_set_conv_mode(data, ADS1015_SINGLESHOT);
 }
 
 #ifdef CONFIG_PM
@@ -682,9 +685,7 @@ static int ads1015_runtime_suspend(struct device *dev)
 	struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
 	struct ads1015_data *data = iio_priv(indio_dev);
 
-	return regmap_update_bits(data->regmap, ADS1015_CFG_REG,
-				  ADS1015_CFG_MOD_MASK,
-				  ADS1015_SINGLESHOT << ADS1015_CFG_MOD_SHIFT);
+	return ads1015_set_conv_mode(data, ADS1015_SINGLESHOT);
 }
 
 static int ads1015_runtime_resume(struct device *dev)
@@ -693,9 +694,7 @@ static int ads1015_runtime_resume(struct device *dev)
 	struct ads1015_data *data = iio_priv(indio_dev);
 	int ret;
 
-	ret = regmap_update_bits(data->regmap, ADS1015_CFG_REG,
-				  ADS1015_CFG_MOD_MASK,
-				  ADS1015_CONTINUOUS << ADS1015_CFG_MOD_SHIFT);
+	ret = ads1015_set_conv_mode(data, ADS1015_CONTINUOUS);
 	if (!ret)
 		data->conv_invalid = true;
 
-- 
2.7.4

[PATCH v3 09/11] iio: adc: ti-ads1015: use devm_iio_triggered_buffer_setup

[Akinobu Mita]

Use devm_iio_triggered_buffer_setup to simplify the error path in the
probe() and remove() function.

This changes the remove order, but the end result of remove function
actually does the reverse of probe.

Cc: Daniel Baluta <daniel.baluta@gmail.com>
Cc: Jonathan Cameron <jic23@kernel.org>
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
---
 drivers/iio/adc/ti-ads1015.c | 17 +++++------------
 1 file changed, 5 insertions(+), 12 deletions(-)

diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
index 9f3f0c9..da25780 100644
--- a/drivers/iio/adc/ti-ads1015.c
+++ b/drivers/iio/adc/ti-ads1015.c
@@ -627,9 +627,9 @@ static int ads1015_probe(struct i2c_client *client,
 		return PTR_ERR(data->regmap);
 	}
 
-	ret = iio_triggered_buffer_setup(indio_dev, NULL,
-					 ads1015_trigger_handler,
-					 &ads1015_buffer_setup_ops);
+	ret = devm_iio_triggered_buffer_setup(&client->dev, indio_dev, NULL,
+					      ads1015_trigger_handler,
+					      &ads1015_buffer_setup_ops);
 	if (ret < 0) {
 		dev_err(&client->dev, "iio triggered buffer setup failed\n");
 		return ret;
@@ -643,7 +643,7 @@ static int ads1015_probe(struct i2c_client *client,
 
 	ret = pm_runtime_set_active(&client->dev);
 	if (ret)
-		goto err_buffer_cleanup;
+		return ret;
 	pm_runtime_set_autosuspend_delay(&client->dev, ADS1015_SLEEP_DELAY_MS);
 	pm_runtime_use_autosuspend(&client->dev);
 	pm_runtime_enable(&client->dev);
@@ -651,15 +651,10 @@ static int ads1015_probe(struct i2c_client *client,
 	ret = iio_device_register(indio_dev);
 	if (ret < 0) {
 		dev_err(&client->dev, "Failed to register IIO device\n");
-		goto err_buffer_cleanup;
+		return ret;
 	}
 
 	return 0;
-
-err_buffer_cleanup:
-	iio_triggered_buffer_cleanup(indio_dev);
-
-	return ret;
 }
 
 static int ads1015_remove(struct i2c_client *client)
@@ -673,8 +668,6 @@ static int ads1015_remove(struct i2c_client *client)
 	pm_runtime_set_suspended(&client->dev);
 	pm_runtime_put_noidle(&client->dev);
 
-	iio_triggered_buffer_cleanup(indio_dev);
-
 	/* power down single shot mode */
 	return ads1015_set_conv_mode(data, ADS1015_SINGLESHOT);
 }
-- 
2.7.4

[PATCH v3 10/11] iio: adc: ti-ads1015: use iio_device_claim_direct_mode()

[Akinobu Mita]

While the iio buffer for the ti-ads1015 driver is enabled, reading the
raw ADC channel data is restricted.  We usually use the
iio_device_claim_direct_mode()/iio_device_release_direct_mode() pair for
that.

This change consequently reverses the locking order for the driver's
private lock and indio_dev->mlock which acquired by
iio_device_claim_direct_mode() internally. But it's safe because there is
no other dependency between these locks.

Cc: Daniel Baluta <daniel.baluta@gmail.com>
Cc: Jonathan Cameron <jic23@kernel.org>
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
---
 drivers/iio/adc/ti-ads1015.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
index da25780..5568be4 100644
--- a/drivers/iio/adc/ti-ads1015.c
+++ b/drivers/iio/adc/ti-ads1015.c
@@ -347,34 +347,34 @@ static int ads1015_read_raw(struct iio_dev *indio_dev,
 	int ret, idx;
 	struct ads1015_data *data = iio_priv(indio_dev);
 
-	mutex_lock(&indio_dev->mlock);
 	mutex_lock(&data->lock);
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW: {
 		int shift = chan->scan_type.shift;
 
-		if (iio_buffer_enabled(indio_dev)) {
-			ret = -EBUSY;
+		ret = iio_device_claim_direct_mode(indio_dev);
+		if (ret)
 			break;
-		}
 
 		ret = ads1015_set_power_state(data, true);
 		if (ret < 0)
-			break;
+			goto release_direct;
 
 		ret = ads1015_get_adc_result(data, chan->address, val);
 		if (ret < 0) {
 			ads1015_set_power_state(data, false);
-			break;
+			goto release_direct;
 		}
 
 		*val = sign_extend32(*val >> shift, 15 - shift);
 
 		ret = ads1015_set_power_state(data, false);
 		if (ret < 0)
-			break;
+			goto release_direct;
 
 		ret = IIO_VAL_INT;
+release_direct:
+		iio_device_release_direct_mode(indio_dev);
 		break;
 	}
 	case IIO_CHAN_INFO_SCALE:
@@ -393,7 +393,6 @@ static int ads1015_read_raw(struct iio_dev *indio_dev,
 		break;
 	}
 	mutex_unlock(&data->lock);
-	mutex_unlock(&indio_dev->mlock);
 
 	return ret;
 }
-- 
2.7.4

[PATCH v3 11/11] iio: adc: ti-ads1015: add threshold event support

[Akinobu Mita]

The ADS1015 device provides programmable comparator that can issue an
interrupt on the ALERT pin.  This change adds the iio threshold event
support for that feature.

The ADS1015 device only have a single config register which contains an
input multiplexer selection, comparator settings, and etc.  So only a
single event channel can be enabled at a time.  Also enabling both buffer
and event are prohibited for simplicity.

Cc: Daniel Baluta <daniel.baluta@gmail.com>
Cc: Jonathan Cameron <jic23@kernel.org>
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
---
 drivers/iio/adc/ti-ads1015.c | 408 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 406 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
index 5568be4..a6f578c 100644
--- a/drivers/iio/adc/ti-ads1015.c
+++ b/drivers/iio/adc/ti-ads1015.c
@@ -17,6 +17,7 @@
 #include <linux/module.h>
 #include <linux/of_device.h>
 #include <linux/init.h>
+#include <linux/irq.h>
 #include <linux/i2c.h>
 #include <linux/regmap.h>
 #include <linux/pm_runtime.h>
@@ -28,6 +29,7 @@
 #include <linux/iio/iio.h>
 #include <linux/iio/types.h>
 #include <linux/iio/sysfs.h>
+#include <linux/iio/events.h>
 #include <linux/iio/buffer.h>
 #include <linux/iio/triggered_buffer.h>
 #include <linux/iio/trigger_consumer.h>
@@ -36,17 +38,38 @@
 
 #define ADS1015_CONV_REG	0x00
 #define ADS1015_CFG_REG		0x01
+#define ADS1015_LO_THRESH_REG	0x02
+#define ADS1015_HI_THRESH_REG	0x03
 
+#define ADS1015_CFG_COMP_QUE_SHIFT	0
+#define ADS1015_CFG_COMP_LAT_SHIFT	2
+#define ADS1015_CFG_COMP_POL_SHIFT	3
+#define ADS1015_CFG_COMP_MODE_SHIFT	4
 #define ADS1015_CFG_DR_SHIFT	5
 #define ADS1015_CFG_MOD_SHIFT	8
 #define ADS1015_CFG_PGA_SHIFT	9
 #define ADS1015_CFG_MUX_SHIFT	12
 
+#define ADS1015_CFG_COMP_QUE_MASK	GENMASK(1, 0)
+#define ADS1015_CFG_COMP_LAT_MASK	BIT(2)
+#define ADS1015_CFG_COMP_POL_MASK	BIT(3)
+#define ADS1015_CFG_COMP_MODE_MASK	BIT(4)
 #define ADS1015_CFG_DR_MASK	GENMASK(7, 5)
 #define ADS1015_CFG_MOD_MASK	BIT(8)
 #define ADS1015_CFG_PGA_MASK	GENMASK(11, 9)
 #define ADS1015_CFG_MUX_MASK	GENMASK(14, 12)
 
+/* Comparator queue and disable field */
+#define ADS1015_CFG_COMP_DISABLE	3
+
+/* Comparator polarity field */
+#define ADS1015_CFG_COMP_POL_LOW	0
+#define ADS1015_CFG_COMP_POL_HIGH	1
+
+/* Comparator mode field */
+#define ADS1015_CFG_COMP_MODE_TRAD	0
+#define ADS1015_CFG_COMP_MODE_WINDOW	1
+
 /* device operating modes */
 #define ADS1015_CONTINUOUS	0
 #define ADS1015_SINGLESHOT	1
@@ -89,6 +112,30 @@ static int ads1015_fullscale_range[] = {
 	6144, 4096, 2048, 1024, 512, 256, 256, 256
 };
 
+/*
+ * Translation from COMP_QUE field value to the number of successive readings
+ * exceed the threshold values before an interrupt is generated
+ */
+static const int ads1015_comp_queue[] = { 1, 2, 4 };
+
+static const struct iio_event_spec ads1015_events[] = {
+	{
+		.type = IIO_EV_TYPE_THRESH,
+		.dir = IIO_EV_DIR_RISING,
+		.mask_separate = BIT(IIO_EV_INFO_VALUE) |
+				BIT(IIO_EV_INFO_ENABLE),
+	}, {
+		.type = IIO_EV_TYPE_THRESH,
+		.dir = IIO_EV_DIR_FALLING,
+		.mask_separate = BIT(IIO_EV_INFO_VALUE),
+	}, {
+		.type = IIO_EV_TYPE_THRESH,
+		.dir = IIO_EV_DIR_EITHER,
+		.mask_separate = BIT(IIO_EV_INFO_ENABLE) |
+				BIT(IIO_EV_INFO_PERIOD),
+	},
+};
+
 #define ADS1015_V_CHAN(_chan, _addr) {				\
 	.type = IIO_VOLTAGE,					\
 	.indexed = 1,						\
@@ -105,6 +152,8 @@ static int ads1015_fullscale_range[] = {
 		.shift = 4,					\
 		.endianness = IIO_CPU,				\
 	},							\
+	.event_spec = ads1015_events,				\
+	.num_event_specs = ARRAY_SIZE(ads1015_events),		\
 	.datasheet_name = "AIN"#_chan,				\
 }
 
@@ -126,6 +175,8 @@ static int ads1015_fullscale_range[] = {
 		.shift = 4,					\
 		.endianness = IIO_CPU,				\
 	},							\
+	.event_spec = ads1015_events,				\
+	.num_event_specs = ARRAY_SIZE(ads1015_events),		\
 	.datasheet_name = "AIN"#_chan"-AIN"#_chan2,		\
 }
 
@@ -144,6 +195,8 @@ static int ads1015_fullscale_range[] = {
 		.storagebits = 16,				\
 		.endianness = IIO_CPU,				\
 	},							\
+	.event_spec = ads1015_events,				\
+	.num_event_specs = ARRAY_SIZE(ads1015_events),		\
 	.datasheet_name = "AIN"#_chan,				\
 }
 
@@ -164,9 +217,17 @@ static int ads1015_fullscale_range[] = {
 		.storagebits = 16,				\
 		.endianness = IIO_CPU,				\
 	},							\
+	.event_spec = ads1015_events,				\
+	.num_event_specs = ARRAY_SIZE(ads1015_events),		\
 	.datasheet_name = "AIN"#_chan"-AIN"#_chan2,		\
 }
 
+struct ads1015_thresh_data {
+	unsigned int comp_queue;
+	int high_thresh;
+	int low_thresh;
+};
+
 struct ads1015_data {
 	struct regmap *regmap;
 	/*
@@ -176,6 +237,10 @@ struct ads1015_data {
 	struct mutex lock;
 	struct ads1015_channel_data channel_data[ADS1015_CHANNELS];
 
+	unsigned int event_channel;
+	unsigned int comp_mode;
+	struct ads1015_thresh_data thresh_data[ADS1015_CHANNELS];
+
 	unsigned int *data_rate;
 	/*
 	 * Set to true when the ADC is switched to the continuous-conversion
@@ -185,15 +250,41 @@ struct ads1015_data {
 	bool conv_invalid;
 };
 
+static bool ads1015_event_channel_enabled(struct ads1015_data *data)
+{
+	return (data->event_channel != ADS1015_CHANNELS);
+}
+
+static void ads1015_event_channel_enable(struct ads1015_data *data, int chan,
+					 int comp_mode)
+{
+	WARN_ON(ads1015_event_channel_enabled(data));
+
+	data->event_channel = chan;
+	data->comp_mode = comp_mode;
+}
+
+static void ads1015_event_channel_disable(struct ads1015_data *data, int chan)
+{
+	data->event_channel = ADS1015_CHANNELS;
+}
+
 static bool ads1015_is_writeable_reg(struct device *dev, unsigned int reg)
 {
-	return (reg == ADS1015_CFG_REG);
+	switch (reg) {
+	case ADS1015_CFG_REG:
+	case ADS1015_LO_THRESH_REG:
+	case ADS1015_HI_THRESH_REG:
+		return true;
+	default:
+		return false;
+	}
 }
 
 static const struct regmap_config ads1015_regmap_config = {
 	.reg_bits = 8,
 	.val_bits = 16,
-	.max_register = ADS1015_CFG_REG,
+	.max_register = ADS1015_HI_THRESH_REG,
 	.writeable_reg = ads1015_is_writeable_reg,
 };
 
@@ -258,6 +349,14 @@ int ads1015_get_adc_result(struct ads1015_data *data, int chan, int *val)
 	cfg = chan << ADS1015_CFG_MUX_SHIFT | pga << ADS1015_CFG_PGA_SHIFT |
 		dr << ADS1015_CFG_DR_SHIFT;
 
+	if (ads1015_event_channel_enabled(data)) {
+		mask |= ADS1015_CFG_COMP_QUE_MASK | ADS1015_CFG_COMP_MODE_MASK;
+		cfg |= data->thresh_data[chan].comp_queue <<
+				ADS1015_CFG_COMP_QUE_SHIFT |
+			data->comp_mode <<
+				ADS1015_CFG_COMP_MODE_SHIFT;
+	}
+
 	cfg = (old & ~mask) | (cfg & mask);
 
 	ret = regmap_write(data->regmap, ADS1015_CFG_REG, cfg);
@@ -356,6 +455,12 @@ static int ads1015_read_raw(struct iio_dev *indio_dev,
 		if (ret)
 			break;
 
+		if (ads1015_event_channel_enabled(data) &&
+				data->event_channel != chan->address) {
+			ret = -EBUSY;
+			goto release_direct;
+		}
+
 		ret = ads1015_set_power_state(data, true);
 		if (ret < 0)
 			goto release_direct;
@@ -421,8 +526,251 @@ static int ads1015_write_raw(struct iio_dev *indio_dev,
 	return ret;
 }
 
+static int ads1015_read_event(struct iio_dev *indio_dev,
+	const struct iio_chan_spec *chan, enum iio_event_type type,
+	enum iio_event_direction dir, enum iio_event_info info, int *val,
+	int *val2)
+{
+	struct ads1015_data *data = iio_priv(indio_dev);
+	int ret;
+	unsigned int comp_queue;
+	int period;
+	int dr;
+
+	mutex_lock(&data->lock);
+
+	switch (info) {
+	case IIO_EV_INFO_VALUE:
+		*val = (dir == IIO_EV_DIR_RISING) ?
+			data->thresh_data[chan->address].high_thresh :
+			data->thresh_data[chan->address].low_thresh;
+		ret = IIO_VAL_INT;
+		break;
+	case IIO_EV_INFO_PERIOD:
+		dr = data->channel_data[chan->address].data_rate;
+		comp_queue = data->thresh_data[chan->address].comp_queue;
+		period = ads1015_comp_queue[comp_queue] *
+			USEC_PER_SEC / data->data_rate[dr];
+
+		*val = period / USEC_PER_SEC;
+		*val2 = period % USEC_PER_SEC;
+		ret = IIO_VAL_INT_PLUS_MICRO;
+		break;
+	default:
+		ret = -EINVAL;
+		break;
+	}
+
+	mutex_unlock(&data->lock);
+
+	return ret;
+}
+
+static int ads1015_write_event(struct iio_dev *indio_dev,
+	const struct iio_chan_spec *chan, enum iio_event_type type,
+	enum iio_event_direction dir, enum iio_event_info info, int val,
+	int val2)
+{
+	struct ads1015_data *data = iio_priv(indio_dev);
+	int realbits = chan->scan_type.realbits;
+	int ret = 0;
+	long long period;
+	int i;
+	int dr;
+
+	mutex_lock(&data->lock);
+
+	switch (info) {
+	case IIO_EV_INFO_VALUE:
+		if (val >= 1 << (realbits - 1) || val < -1 << (realbits - 1)) {
+			ret = -EINVAL;
+			break;
+		}
+		if (dir == IIO_EV_DIR_RISING)
+			data->thresh_data[chan->address].high_thresh = val;
+		else
+			data->thresh_data[chan->address].low_thresh = val;
+		break;
+	case IIO_EV_INFO_PERIOD:
+		dr = data->channel_data[chan->address].data_rate;
+		period = val * USEC_PER_SEC + val2;
+
+		for (i = 0; i < ARRAY_SIZE(ads1015_comp_queue) - 1; i++) {
+			if (period <= ads1015_comp_queue[i] *
+					USEC_PER_SEC / data->data_rate[dr])
+				break;
+		}
+		data->thresh_data[chan->address].comp_queue = i;
+		break;
+	default:
+		ret = -EINVAL;
+		break;
+	}
+
+	mutex_unlock(&data->lock);
+
+	return ret;
+}
+
+static int ads1015_read_event_config(struct iio_dev *indio_dev,
+	const struct iio_chan_spec *chan, enum iio_event_type type,
+	enum iio_event_direction dir)
+{
+	struct ads1015_data *data = iio_priv(indio_dev);
+	int ret = 0;
+
+	mutex_lock(&data->lock);
+	if (data->event_channel == chan->address) {
+		switch (dir) {
+		case IIO_EV_DIR_RISING:
+			ret = 1;
+			break;
+		case IIO_EV_DIR_EITHER:
+			ret = (data->comp_mode == ADS1015_CFG_COMP_MODE_WINDOW);
+			break;
+		default:
+			ret = -EINVAL;
+			break;
+		}
+	}
+	mutex_unlock(&data->lock);
+
+	return ret;
+}
+
+static int ads1015_enable_event_config(struct ads1015_data *data,
+	const struct iio_chan_spec *chan, int comp_mode)
+{
+	int low_thresh = data->thresh_data[chan->address].low_thresh;
+	int high_thresh = data->thresh_data[chan->address].high_thresh;
+	int ret;
+	unsigned int val;
+
+	if (ads1015_event_channel_enabled(data)) {
+		if (data->event_channel != chan->address ||
+			(data->comp_mode == ADS1015_CFG_COMP_MODE_TRAD &&
+				comp_mode == ADS1015_CFG_COMP_MODE_WINDOW))
+			return -EBUSY;
+
+		return 0;
+	}
+
+	if (comp_mode == ADS1015_CFG_COMP_MODE_TRAD) {
+		low_thresh = max(-1 << (chan->scan_type.realbits - 1),
+				high_thresh - 1);
+	}
+	ret = regmap_write(data->regmap, ADS1015_LO_THRESH_REG,
+			low_thresh << chan->scan_type.shift);
+	if (ret)
+		return ret;
+
+	ret = regmap_write(data->regmap, ADS1015_HI_THRESH_REG,
+			high_thresh << chan->scan_type.shift);
+	if (ret)
+		return ret;
+
+	ret = ads1015_set_power_state(data, true);
+	if (ret < 0)
+		return ret;
+
+	ads1015_event_channel_enable(data, chan->address, comp_mode);
+
+	ret = ads1015_get_adc_result(data, chan->address, &val);
+	if (ret) {
+		ads1015_event_channel_disable(data, chan->address);
+		ads1015_set_power_state(data, false);
+	}
+
+	return ret;
+}
+
+static int ads1015_disable_event_config(struct ads1015_data *data,
+	const struct iio_chan_spec *chan, int comp_mode)
+{
+	int ret;
+
+	if (!ads1015_event_channel_enabled(data))
+		return 0;
+
+	if (data->event_channel != chan->address)
+		return 0;
+
+	if (data->comp_mode == ADS1015_CFG_COMP_MODE_TRAD &&
+			comp_mode == ADS1015_CFG_COMP_MODE_WINDOW)
+		return 0;
+
+	ret = regmap_update_bits(data->regmap, ADS1015_CFG_REG,
+				ADS1015_CFG_COMP_QUE_MASK,
+				ADS1015_CFG_COMP_DISABLE <<
+					ADS1015_CFG_COMP_QUE_SHIFT);
+	if (ret)
+		return ret;
+
+	ads1015_event_channel_disable(data, chan->address);
+
+	return ads1015_set_power_state(data, false);
+}
+
+static int ads1015_write_event_config(struct iio_dev *indio_dev,
+	const struct iio_chan_spec *chan, enum iio_event_type type,
+	enum iio_event_direction dir, int state)
+{
+	struct ads1015_data *data = iio_priv(indio_dev);
+	int ret;
+	int comp_mode = (dir == IIO_EV_DIR_EITHER) ?
+		ADS1015_CFG_COMP_MODE_WINDOW : ADS1015_CFG_COMP_MODE_TRAD;
+
+	mutex_lock(&data->lock);
+
+	/* Prevent from enabling both buffer and event at a time */
+	ret = iio_device_claim_direct_mode(indio_dev);
+	if (ret) {
+		mutex_unlock(&data->lock);
+		return ret;
+	}
+
+	if (state)
+		ret = ads1015_enable_event_config(data, chan, comp_mode);
+	else
+		ret = ads1015_disable_event_config(data, chan, comp_mode);
+
+	iio_device_release_direct_mode(indio_dev);
+	mutex_unlock(&data->lock);
+
+	return ret;
+}
+
+static irqreturn_t ads1015_event_handler(int irq, void *priv)
+{
+	struct iio_dev *indio_dev = priv;
+	struct ads1015_data *data = iio_priv(indio_dev);
+	int ret;
+	int val;
+
+	/* Clear the latched ALERT/RDY pin */
+	ret = regmap_read(data->regmap, ADS1015_CONV_REG, &val);
+	if (!ret && ads1015_event_channel_enabled(data)) {
+		enum iio_event_direction dir;
+		u64 code;
+
+		dir = data->comp_mode == ADS1015_CFG_COMP_MODE_TRAD ?
+					IIO_EV_DIR_RISING : IIO_EV_DIR_EITHER;
+		code = IIO_UNMOD_EVENT_CODE(IIO_VOLTAGE, data->event_channel,
+					IIO_EV_TYPE_THRESH, dir);
+		iio_push_event(indio_dev, code, iio_get_time_ns(indio_dev));
+	}
+
+	return IRQ_HANDLED;
+}
+
 static int ads1015_buffer_preenable(struct iio_dev *indio_dev)
 {
+	struct ads1015_data *data = iio_priv(indio_dev);
+
+	/* Prevent from enabling both buffer and event at a time */
+	if (ads1015_event_channel_enabled(data))
+		return -EBUSY;
+
 	return ads1015_set_power_state(iio_priv(indio_dev), true);
 }
 
@@ -473,6 +821,10 @@ static const struct iio_info ads1015_info = {
 	.driver_module	= THIS_MODULE,
 	.read_raw	= ads1015_read_raw,
 	.write_raw	= ads1015_write_raw,
+	.read_event_value = ads1015_read_event,
+	.write_event_value = ads1015_write_event,
+	.read_event_config = ads1015_read_event_config,
+	.write_event_config = ads1015_write_event_config,
 	.attrs          = &ads1015_attribute_group,
 };
 
@@ -480,6 +832,10 @@ static const struct iio_info ads1115_info = {
 	.driver_module	= THIS_MODULE,
 	.read_raw	= ads1015_read_raw,
 	.write_raw	= ads1015_write_raw,
+	.read_event_value = ads1015_read_event,
+	.write_event_value = ads1015_write_event,
+	.read_event_config = ads1015_read_event_config,
+	.write_event_config = ads1015_write_event_config,
 	.attrs          = &ads1115_attribute_group,
 };
 
@@ -583,6 +939,7 @@ static int ads1015_probe(struct i2c_client *client,
 	struct ads1015_data *data;
 	int ret;
 	enum chip_ids chip;
+	int i;
 
 	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
 	if (!indio_dev)
@@ -617,6 +974,18 @@ static int ads1015_probe(struct i2c_client *client,
 		break;
 	}
 
+	data->event_channel = ADS1015_CHANNELS;
+	/*
+	 * Set default lower and upper threshold to min and max value
+	 * respectively.
+	 */
+	for (i = 0; i < ADS1015_CHANNELS; i++) {
+		int realbits = indio_dev->channels[i].scan_type.realbits;
+
+		data->thresh_data[i].low_thresh = -1 << (realbits - 1);
+		data->thresh_data[i].high_thresh = (1 << (realbits - 1)) - 1;
+	}
+
 	/* we need to keep this ABI the same as used by hwmon ADS1015 driver */
 	ads1015_get_channels_config(client);
 
@@ -634,6 +1003,41 @@ static int ads1015_probe(struct i2c_client *client,
 		return ret;
 	}
 
+	if (client->irq) {
+		unsigned long irq_trig =
+			irqd_get_trigger_type(irq_get_irq_data(client->irq));
+		unsigned int cfg_comp_mask = ADS1015_CFG_COMP_QUE_MASK |
+			ADS1015_CFG_COMP_LAT_MASK | ADS1015_CFG_COMP_POL_MASK;
+		unsigned int cfg_comp =
+			ADS1015_CFG_COMP_DISABLE << ADS1015_CFG_COMP_QUE_SHIFT |
+			1 << ADS1015_CFG_COMP_LAT_SHIFT;
+
+		switch (irq_trig) {
+		case IRQF_TRIGGER_LOW:
+			cfg_comp |= ADS1015_CFG_COMP_POL_LOW <<
+					ADS1015_CFG_COMP_POL_SHIFT;
+			break;
+		case IRQF_TRIGGER_HIGH:
+			cfg_comp |= ADS1015_CFG_COMP_POL_HIGH <<
+					ADS1015_CFG_COMP_POL_SHIFT;
+			break;
+		default:
+			return -EINVAL;
+		}
+
+		ret = regmap_update_bits(data->regmap, ADS1015_CFG_REG,
+					cfg_comp_mask, cfg_comp);
+		if (ret)
+			return ret;
+
+		ret = devm_request_threaded_irq(&client->dev, client->irq,
+						NULL, ads1015_event_handler,
+						irq_trig | IRQF_ONESHOT,
+						client->name, indio_dev);
+		if (ret)
+			return ret;
+	}
+
 	ret = ads1015_set_conv_mode(data, ADS1015_CONTINUOUS);
 	if (ret)
 		return ret;
-- 
2.7.4

Re: [PATCH v3 11/11] iio: adc: ti-ads1015: add threshold event support

[Jonathan Cameron]

On Mon, 31 Jul 2017 00:12:55 +0900
Akinobu Mita <akinobu.mita@gmail.com> wrote:

> The ADS1015 device provides programmable comparator that can issue an
> interrupt on the ALERT pin.  This change adds the iio threshold event
> support for that feature.
> 
> The ADS1015 device only have a single config register which contains an
> input multiplexer selection, comparator settings, and etc.  So only a
> single event channel can be enabled at a time.  Also enabling both buffer
> and event are prohibited for simplicity.
> 
> Cc: Daniel Baluta <daniel.baluta@gmail.com>
> Cc: Jonathan Cameron <jic23@kernel.org>
> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
Very nice.  I pretty much assumed there would be a locking problem in here
but as far as I can see you got it spot on!

This will obviously be waiting on the fixes to work their way through the
fixes tree.

I'm also happy with the rest of the series before this.

Please give me a bump if it looks like I've lost this
somehow (it's been known to happen if the cycle of getting
changes back from the fixes tree takes too long)

Jonathan
> ---
>  drivers/iio/adc/ti-ads1015.c | 408 ++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 406 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
> index 5568be4..a6f578c 100644
> --- a/drivers/iio/adc/ti-ads1015.c
> +++ b/drivers/iio/adc/ti-ads1015.c
> @@ -17,6 +17,7 @@
>  #include <linux/module.h>
>  #include <linux/of_device.h>
>  #include <linux/init.h>
> +#include <linux/irq.h>
>  #include <linux/i2c.h>
>  #include <linux/regmap.h>
>  #include <linux/pm_runtime.h>
> @@ -28,6 +29,7 @@
>  #include <linux/iio/iio.h>
>  #include <linux/iio/types.h>
>  #include <linux/iio/sysfs.h>
> +#include <linux/iio/events.h>
>  #include <linux/iio/buffer.h>
>  #include <linux/iio/triggered_buffer.h>
>  #include <linux/iio/trigger_consumer.h>
> @@ -36,17 +38,38 @@
>  
>  #define ADS1015_CONV_REG	0x00
>  #define ADS1015_CFG_REG		0x01
> +#define ADS1015_LO_THRESH_REG	0x02
> +#define ADS1015_HI_THRESH_REG	0x03
>  
> +#define ADS1015_CFG_COMP_QUE_SHIFT	0
> +#define ADS1015_CFG_COMP_LAT_SHIFT	2
> +#define ADS1015_CFG_COMP_POL_SHIFT	3
> +#define ADS1015_CFG_COMP_MODE_SHIFT	4
>  #define ADS1015_CFG_DR_SHIFT	5
>  #define ADS1015_CFG_MOD_SHIFT	8
>  #define ADS1015_CFG_PGA_SHIFT	9
>  #define ADS1015_CFG_MUX_SHIFT	12
>  
> +#define ADS1015_CFG_COMP_QUE_MASK	GENMASK(1, 0)
> +#define ADS1015_CFG_COMP_LAT_MASK	BIT(2)
> +#define ADS1015_CFG_COMP_POL_MASK	BIT(3)
> +#define ADS1015_CFG_COMP_MODE_MASK	BIT(4)
>  #define ADS1015_CFG_DR_MASK	GENMASK(7, 5)
>  #define ADS1015_CFG_MOD_MASK	BIT(8)
>  #define ADS1015_CFG_PGA_MASK	GENMASK(11, 9)
>  #define ADS1015_CFG_MUX_MASK	GENMASK(14, 12)
>  
> +/* Comparator queue and disable field */
> +#define ADS1015_CFG_COMP_DISABLE	3
> +
> +/* Comparator polarity field */
> +#define ADS1015_CFG_COMP_POL_LOW	0
> +#define ADS1015_CFG_COMP_POL_HIGH	1
> +
> +/* Comparator mode field */
> +#define ADS1015_CFG_COMP_MODE_TRAD	0
> +#define ADS1015_CFG_COMP_MODE_WINDOW	1
> +
>  /* device operating modes */
>  #define ADS1015_CONTINUOUS	0
>  #define ADS1015_SINGLESHOT	1
> @@ -89,6 +112,30 @@ static int ads1015_fullscale_range[] = {
>  	6144, 4096, 2048, 1024, 512, 256, 256, 256
>  };
>  
> +/*
> + * Translation from COMP_QUE field value to the number of successive readings
> + * exceed the threshold values before an interrupt is generated
> + */
> +static const int ads1015_comp_queue[] = { 1, 2, 4 };
> +
> +static const struct iio_event_spec ads1015_events[] = {
> +	{
> +		.type = IIO_EV_TYPE_THRESH,
> +		.dir = IIO_EV_DIR_RISING,
> +		.mask_separate = BIT(IIO_EV_INFO_VALUE) |
> +				BIT(IIO_EV_INFO_ENABLE),
> +	}, {
> +		.type = IIO_EV_TYPE_THRESH,
> +		.dir = IIO_EV_DIR_FALLING,
> +		.mask_separate = BIT(IIO_EV_INFO_VALUE),
> +	}, {
> +		.type = IIO_EV_TYPE_THRESH,
> +		.dir = IIO_EV_DIR_EITHER,
> +		.mask_separate = BIT(IIO_EV_INFO_ENABLE) |
> +				BIT(IIO_EV_INFO_PERIOD),
> +	},
> +};
> +
>  #define ADS1015_V_CHAN(_chan, _addr) {				\
>  	.type = IIO_VOLTAGE,					\
>  	.indexed = 1,						\
> @@ -105,6 +152,8 @@ static int ads1015_fullscale_range[] = {
>  		.shift = 4,					\
>  		.endianness = IIO_CPU,				\
>  	},							\
> +	.event_spec = ads1015_events,				\
> +	.num_event_specs = ARRAY_SIZE(ads1015_events),		\
>  	.datasheet_name = "AIN"#_chan,				\
>  }
>  
> @@ -126,6 +175,8 @@ static int ads1015_fullscale_range[] = {
>  		.shift = 4,					\
>  		.endianness = IIO_CPU,				\
>  	},							\
> +	.event_spec = ads1015_events,				\
> +	.num_event_specs = ARRAY_SIZE(ads1015_events),		\
>  	.datasheet_name = "AIN"#_chan"-AIN"#_chan2,		\
>  }
>  
> @@ -144,6 +195,8 @@ static int ads1015_fullscale_range[] = {
>  		.storagebits = 16,				\
>  		.endianness = IIO_CPU,				\
>  	},							\
> +	.event_spec = ads1015_events,				\
> +	.num_event_specs = ARRAY_SIZE(ads1015_events),		\
>  	.datasheet_name = "AIN"#_chan,				\
>  }
>  
> @@ -164,9 +217,17 @@ static int ads1015_fullscale_range[] = {
>  		.storagebits = 16,				\
>  		.endianness = IIO_CPU,				\
>  	},							\
> +	.event_spec = ads1015_events,				\
> +	.num_event_specs = ARRAY_SIZE(ads1015_events),		\
>  	.datasheet_name = "AIN"#_chan"-AIN"#_chan2,		\
>  }
>  
> +struct ads1015_thresh_data {
> +	unsigned int comp_queue;
> +	int high_thresh;
> +	int low_thresh;
> +};
> +
>  struct ads1015_data {
>  	struct regmap *regmap;
>  	/*
> @@ -176,6 +237,10 @@ struct ads1015_data {
>  	struct mutex lock;
>  	struct ads1015_channel_data channel_data[ADS1015_CHANNELS];
>  
> +	unsigned int event_channel;
> +	unsigned int comp_mode;
> +	struct ads1015_thresh_data thresh_data[ADS1015_CHANNELS];
> +
>  	unsigned int *data_rate;
>  	/*
>  	 * Set to true when the ADC is switched to the continuous-conversion
> @@ -185,15 +250,41 @@ struct ads1015_data {
>  	bool conv_invalid;
>  };
>  
> +static bool ads1015_event_channel_enabled(struct ads1015_data *data)
> +{
> +	return (data->event_channel != ADS1015_CHANNELS);
> +}
> +
> +static void ads1015_event_channel_enable(struct ads1015_data *data, int chan,
> +					 int comp_mode)
> +{
> +	WARN_ON(ads1015_event_channel_enabled(data));
> +
> +	data->event_channel = chan;
> +	data->comp_mode = comp_mode;
> +}
> +
> +static void ads1015_event_channel_disable(struct ads1015_data *data, int chan)
> +{
> +	data->event_channel = ADS1015_CHANNELS;
> +}
> +
>  static bool ads1015_is_writeable_reg(struct device *dev, unsigned int reg)
>  {
> -	return (reg == ADS1015_CFG_REG);
> +	switch (reg) {
> +	case ADS1015_CFG_REG:
> +	case ADS1015_LO_THRESH_REG:
> +	case ADS1015_HI_THRESH_REG:
> +		return true;
> +	default:
> +		return false;
> +	}
>  }
>  
>  static const struct regmap_config ads1015_regmap_config = {
>  	.reg_bits = 8,
>  	.val_bits = 16,
> -	.max_register = ADS1015_CFG_REG,
> +	.max_register = ADS1015_HI_THRESH_REG,
>  	.writeable_reg = ads1015_is_writeable_reg,
>  };
>  
> @@ -258,6 +349,14 @@ int ads1015_get_adc_result(struct ads1015_data *data, int chan, int *val)
>  	cfg = chan << ADS1015_CFG_MUX_SHIFT | pga << ADS1015_CFG_PGA_SHIFT |
>  		dr << ADS1015_CFG_DR_SHIFT;
>  
> +	if (ads1015_event_channel_enabled(data)) {
> +		mask |= ADS1015_CFG_COMP_QUE_MASK | ADS1015_CFG_COMP_MODE_MASK;
> +		cfg |= data->thresh_data[chan].comp_queue <<
> +				ADS1015_CFG_COMP_QUE_SHIFT |
> +			data->comp_mode <<
> +				ADS1015_CFG_COMP_MODE_SHIFT;
> +	}
> +
>  	cfg = (old & ~mask) | (cfg & mask);
>  
>  	ret = regmap_write(data->regmap, ADS1015_CFG_REG, cfg);
> @@ -356,6 +455,12 @@ static int ads1015_read_raw(struct iio_dev *indio_dev,
>  		if (ret)
>  			break;
>  
> +		if (ads1015_event_channel_enabled(data) &&
> +				data->event_channel != chan->address) {
> +			ret = -EBUSY;
> +			goto release_direct;
> +		}
> +
>  		ret = ads1015_set_power_state(data, true);
>  		if (ret < 0)
>  			goto release_direct;
> @@ -421,8 +526,251 @@ static int ads1015_write_raw(struct iio_dev *indio_dev,
>  	return ret;
>  }
>  
> +static int ads1015_read_event(struct iio_dev *indio_dev,
> +	const struct iio_chan_spec *chan, enum iio_event_type type,
> +	enum iio_event_direction dir, enum iio_event_info info, int *val,
> +	int *val2)
> +{
> +	struct ads1015_data *data = iio_priv(indio_dev);
> +	int ret;
> +	unsigned int comp_queue;
> +	int period;
> +	int dr;
> +
> +	mutex_lock(&data->lock);
> +
> +	switch (info) {
> +	case IIO_EV_INFO_VALUE:
> +		*val = (dir == IIO_EV_DIR_RISING) ?
> +			data->thresh_data[chan->address].high_thresh :
> +			data->thresh_data[chan->address].low_thresh;
> +		ret = IIO_VAL_INT;
> +		break;
> +	case IIO_EV_INFO_PERIOD:
> +		dr = data->channel_data[chan->address].data_rate;
> +		comp_queue = data->thresh_data[chan->address].comp_queue;
> +		period = ads1015_comp_queue[comp_queue] *
> +			USEC_PER_SEC / data->data_rate[dr];
> +
> +		*val = period / USEC_PER_SEC;
> +		*val2 = period % USEC_PER_SEC;
> +		ret = IIO_VAL_INT_PLUS_MICRO;
> +		break;
> +	default:
> +		ret = -EINVAL;
> +		break;
> +	}
> +
> +	mutex_unlock(&data->lock);
> +
> +	return ret;
> +}
> +
> +static int ads1015_write_event(struct iio_dev *indio_dev,
> +	const struct iio_chan_spec *chan, enum iio_event_type type,
> +	enum iio_event_direction dir, enum iio_event_info info, int val,
> +	int val2)
> +{
> +	struct ads1015_data *data = iio_priv(indio_dev);
> +	int realbits = chan->scan_type.realbits;
> +	int ret = 0;
> +	long long period;
> +	int i;
> +	int dr;
> +
> +	mutex_lock(&data->lock);
> +
> +	switch (info) {
> +	case IIO_EV_INFO_VALUE:
> +		if (val >= 1 << (realbits - 1) || val < -1 << (realbits - 1)) {
> +			ret = -EINVAL;
> +			break;
> +		}
> +		if (dir == IIO_EV_DIR_RISING)
> +			data->thresh_data[chan->address].high_thresh = val;
> +		else
> +			data->thresh_data[chan->address].low_thresh = val;
> +		break;
> +	case IIO_EV_INFO_PERIOD:
> +		dr = data->channel_data[chan->address].data_rate;
> +		period = val * USEC_PER_SEC + val2;
> +
> +		for (i = 0; i < ARRAY_SIZE(ads1015_comp_queue) - 1; i++) {
> +			if (period <= ads1015_comp_queue[i] *
> +					USEC_PER_SEC / data->data_rate[dr])
> +				break;
> +		}
> +		data->thresh_data[chan->address].comp_queue = i;
> +		break;
> +	default:
> +		ret = -EINVAL;
> +		break;
> +	}
> +
> +	mutex_unlock(&data->lock);
> +
> +	return ret;
> +}
> +
> +static int ads1015_read_event_config(struct iio_dev *indio_dev,
> +	const struct iio_chan_spec *chan, enum iio_event_type type,
> +	enum iio_event_direction dir)
> +{
> +	struct ads1015_data *data = iio_priv(indio_dev);
> +	int ret = 0;
> +
> +	mutex_lock(&data->lock);
> +	if (data->event_channel == chan->address) {
> +		switch (dir) {
> +		case IIO_EV_DIR_RISING:
> +			ret = 1;
> +			break;
> +		case IIO_EV_DIR_EITHER:
> +			ret = (data->comp_mode == ADS1015_CFG_COMP_MODE_WINDOW);
> +			break;
> +		default:
> +			ret = -EINVAL;
> +			break;
> +		}
> +	}
> +	mutex_unlock(&data->lock);
> +
> +	return ret;
> +}
> +
> +static int ads1015_enable_event_config(struct ads1015_data *data,
> +	const struct iio_chan_spec *chan, int comp_mode)
> +{
> +	int low_thresh = data->thresh_data[chan->address].low_thresh;
> +	int high_thresh = data->thresh_data[chan->address].high_thresh;
> +	int ret;
> +	unsigned int val;
> +
> +	if (ads1015_event_channel_enabled(data)) {
> +		if (data->event_channel != chan->address ||
> +			(data->comp_mode == ADS1015_CFG_COMP_MODE_TRAD &&
> +				comp_mode == ADS1015_CFG_COMP_MODE_WINDOW))
> +			return -EBUSY;
> +
> +		return 0;
> +	}
> +
> +	if (comp_mode == ADS1015_CFG_COMP_MODE_TRAD) {
> +		low_thresh = max(-1 << (chan->scan_type.realbits - 1),
> +				high_thresh - 1);
> +	}
> +	ret = regmap_write(data->regmap, ADS1015_LO_THRESH_REG,
> +			low_thresh << chan->scan_type.shift);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_write(data->regmap, ADS1015_HI_THRESH_REG,
> +			high_thresh << chan->scan_type.shift);
> +	if (ret)
> +		return ret;
> +
> +	ret = ads1015_set_power_state(data, true);
> +	if (ret < 0)
> +		return ret;
> +
> +	ads1015_event_channel_enable(data, chan->address, comp_mode);
> +
> +	ret = ads1015_get_adc_result(data, chan->address, &val);
> +	if (ret) {
> +		ads1015_event_channel_disable(data, chan->address);
> +		ads1015_set_power_state(data, false);
> +	}
> +
> +	return ret;
> +}
> +
> +static int ads1015_disable_event_config(struct ads1015_data *data,
> +	const struct iio_chan_spec *chan, int comp_mode)
> +{
> +	int ret;
> +
> +	if (!ads1015_event_channel_enabled(data))
> +		return 0;
> +
> +	if (data->event_channel != chan->address)
> +		return 0;
> +
> +	if (data->comp_mode == ADS1015_CFG_COMP_MODE_TRAD &&
> +			comp_mode == ADS1015_CFG_COMP_MODE_WINDOW)
> +		return 0;
> +
> +	ret = regmap_update_bits(data->regmap, ADS1015_CFG_REG,
> +				ADS1015_CFG_COMP_QUE_MASK,
> +				ADS1015_CFG_COMP_DISABLE <<
> +					ADS1015_CFG_COMP_QUE_SHIFT);
> +	if (ret)
> +		return ret;
> +
> +	ads1015_event_channel_disable(data, chan->address);
> +
> +	return ads1015_set_power_state(data, false);
> +}
> +
> +static int ads1015_write_event_config(struct iio_dev *indio_dev,
> +	const struct iio_chan_spec *chan, enum iio_event_type type,
> +	enum iio_event_direction dir, int state)
> +{
> +	struct ads1015_data *data = iio_priv(indio_dev);
> +	int ret;
> +	int comp_mode = (dir == IIO_EV_DIR_EITHER) ?
> +		ADS1015_CFG_COMP_MODE_WINDOW : ADS1015_CFG_COMP_MODE_TRAD;
> +
> +	mutex_lock(&data->lock);
> +
> +	/* Prevent from enabling both buffer and event at a time */
> +	ret = iio_device_claim_direct_mode(indio_dev);
> +	if (ret) {
> +		mutex_unlock(&data->lock);
> +		return ret;
> +	}
> +
> +	if (state)
> +		ret = ads1015_enable_event_config(data, chan, comp_mode);
> +	else
> +		ret = ads1015_disable_event_config(data, chan, comp_mode);
> +
> +	iio_device_release_direct_mode(indio_dev);
> +	mutex_unlock(&data->lock);
> +
> +	return ret;
> +}
> +
> +static irqreturn_t ads1015_event_handler(int irq, void *priv)
> +{
> +	struct iio_dev *indio_dev = priv;
> +	struct ads1015_data *data = iio_priv(indio_dev);
> +	int ret;
> +	int val;
> +
> +	/* Clear the latched ALERT/RDY pin */
> +	ret = regmap_read(data->regmap, ADS1015_CONV_REG, &val);
> +	if (!ret && ads1015_event_channel_enabled(data)) {
> +		enum iio_event_direction dir;
> +		u64 code;
> +
> +		dir = data->comp_mode == ADS1015_CFG_COMP_MODE_TRAD ?
> +					IIO_EV_DIR_RISING : IIO_EV_DIR_EITHER;
> +		code = IIO_UNMOD_EVENT_CODE(IIO_VOLTAGE, data->event_channel,
> +					IIO_EV_TYPE_THRESH, dir);
> +		iio_push_event(indio_dev, code, iio_get_time_ns(indio_dev));
> +	}
> +
> +	return IRQ_HANDLED;
> +}
> +
>  static int ads1015_buffer_preenable(struct iio_dev *indio_dev)
>  {
> +	struct ads1015_data *data = iio_priv(indio_dev);
> +
> +	/* Prevent from enabling both buffer and event at a time */
> +	if (ads1015_event_channel_enabled(data))
> +		return -EBUSY;
> +
>  	return ads1015_set_power_state(iio_priv(indio_dev), true);
>  }
>  
> @@ -473,6 +821,10 @@ static const struct iio_info ads1015_info = {
>  	.driver_module	= THIS_MODULE,
>  	.read_raw	= ads1015_read_raw,
>  	.write_raw	= ads1015_write_raw,
> +	.read_event_value = ads1015_read_event,
> +	.write_event_value = ads1015_write_event,
> +	.read_event_config = ads1015_read_event_config,
> +	.write_event_config = ads1015_write_event_config,
>  	.attrs          = &ads1015_attribute_group,
>  };
>  
> @@ -480,6 +832,10 @@ static const struct iio_info ads1115_info = {
>  	.driver_module	= THIS_MODULE,
>  	.read_raw	= ads1015_read_raw,
>  	.write_raw	= ads1015_write_raw,
> +	.read_event_value = ads1015_read_event,
> +	.write_event_value = ads1015_write_event,
> +	.read_event_config = ads1015_read_event_config,
> +	.write_event_config = ads1015_write_event_config,
>  	.attrs          = &ads1115_attribute_group,
>  };
>  
> @@ -583,6 +939,7 @@ static int ads1015_probe(struct i2c_client *client,
>  	struct ads1015_data *data;
>  	int ret;
>  	enum chip_ids chip;
> +	int i;
>  
>  	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
>  	if (!indio_dev)
> @@ -617,6 +974,18 @@ static int ads1015_probe(struct i2c_client *client,
>  		break;
>  	}
>  
> +	data->event_channel = ADS1015_CHANNELS;
> +	/*
> +	 * Set default lower and upper threshold to min and max value
> +	 * respectively.
> +	 */
> +	for (i = 0; i < ADS1015_CHANNELS; i++) {
> +		int realbits = indio_dev->channels[i].scan_type.realbits;
> +
> +		data->thresh_data[i].low_thresh = -1 << (realbits - 1);
> +		data->thresh_data[i].high_thresh = (1 << (realbits - 1)) - 1;
> +	}
> +
>  	/* we need to keep this ABI the same as used by hwmon ADS1015 driver */
>  	ads1015_get_channels_config(client);
>  
> @@ -634,6 +1003,41 @@ static int ads1015_probe(struct i2c_client *client,
>  		return ret;
>  	}
>  
> +	if (client->irq) {
> +		unsigned long irq_trig =
> +			irqd_get_trigger_type(irq_get_irq_data(client->irq));
> +		unsigned int cfg_comp_mask = ADS1015_CFG_COMP_QUE_MASK |
> +			ADS1015_CFG_COMP_LAT_MASK | ADS1015_CFG_COMP_POL_MASK;
> +		unsigned int cfg_comp =
> +			ADS1015_CFG_COMP_DISABLE << ADS1015_CFG_COMP_QUE_SHIFT |
> +			1 << ADS1015_CFG_COMP_LAT_SHIFT;
> +
> +		switch (irq_trig) {
> +		case IRQF_TRIGGER_LOW:
> +			cfg_comp |= ADS1015_CFG_COMP_POL_LOW <<
> +					ADS1015_CFG_COMP_POL_SHIFT;
> +			break;
> +		case IRQF_TRIGGER_HIGH:
> +			cfg_comp |= ADS1015_CFG_COMP_POL_HIGH <<
> +					ADS1015_CFG_COMP_POL_SHIFT;
> +			break;
> +		default:
> +			return -EINVAL;
> +		}
> +
> +		ret = regmap_update_bits(data->regmap, ADS1015_CFG_REG,
> +					cfg_comp_mask, cfg_comp);
> +		if (ret)
> +			return ret;
> +
> +		ret = devm_request_threaded_irq(&client->dev, client->irq,
> +						NULL, ads1015_event_handler,
> +						irq_trig | IRQF_ONESHOT,
> +						client->name, indio_dev);
> +		if (ret)
> +			return ret;
> +	}
> +
>  	ret = ads1015_set_conv_mode(data, ADS1015_CONTINUOUS);
>  	if (ret)
>  		return ret;

Re: [PATCH v3 11/11] iio: adc: ti-ads1015: add threshold event support

[Jonathan Cameron]

On Sun, 30 Jul 2017 17:58:06 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> On Mon, 31 Jul 2017 00:12:55 +0900
> Akinobu Mita <akinobu.mita@gmail.com> wrote:
> 
> > The ADS1015 device provides programmable comparator that can issue an
> > interrupt on the ALERT pin.  This change adds the iio threshold event
> > support for that feature.
> > 
> > The ADS1015 device only have a single config register which contains an
> > input multiplexer selection, comparator settings, and etc.  So only a
> > single event channel can be enabled at a time.  Also enabling both buffer
> > and event are prohibited for simplicity.
> > 
> > Cc: Daniel Baluta <daniel.baluta@gmail.com>
> > Cc: Jonathan Cameron <jic23@kernel.org>
> > Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>  
> Very nice.  I pretty much assumed there would be a locking problem in here
> but as far as I can see you got it spot on!
> 
> This will obviously be waiting on the fixes to work their way through the
> fixes tree.
> 
> I'm also happy with the rest of the series before this.
> 
> Please give me a bump if it looks like I've lost this
> somehow (it's been known to happen if the cycle of getting
> changes back from the fixes tree takes too long)

I've just been revisiting my patches to apply set and discovered I actually
took v2 of many if not all of these patches a week before this set was posted.
Could you sanity check the results currently in tree?

Thanks,

Jonathan
> 
> Jonathan
> > ---
> >  drivers/iio/adc/ti-ads1015.c | 408 ++++++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 406 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
> > index 5568be4..a6f578c 100644
> > --- a/drivers/iio/adc/ti-ads1015.c
> > +++ b/drivers/iio/adc/ti-ads1015.c
> > @@ -17,6 +17,7 @@
> >  #include <linux/module.h>
> >  #include <linux/of_device.h>
> >  #include <linux/init.h>
> > +#include <linux/irq.h>
> >  #include <linux/i2c.h>
> >  #include <linux/regmap.h>
> >  #include <linux/pm_runtime.h>
> > @@ -28,6 +29,7 @@
> >  #include <linux/iio/iio.h>
> >  #include <linux/iio/types.h>
> >  #include <linux/iio/sysfs.h>
> > +#include <linux/iio/events.h>
> >  #include <linux/iio/buffer.h>
> >  #include <linux/iio/triggered_buffer.h>
> >  #include <linux/iio/trigger_consumer.h>
> > @@ -36,17 +38,38 @@
> >  
> >  #define ADS1015_CONV_REG	0x00
> >  #define ADS1015_CFG_REG		0x01
> > +#define ADS1015_LO_THRESH_REG	0x02
> > +#define ADS1015_HI_THRESH_REG	0x03
> >  
> > +#define ADS1015_CFG_COMP_QUE_SHIFT	0
> > +#define ADS1015_CFG_COMP_LAT_SHIFT	2
> > +#define ADS1015_CFG_COMP_POL_SHIFT	3
> > +#define ADS1015_CFG_COMP_MODE_SHIFT	4
> >  #define ADS1015_CFG_DR_SHIFT	5
> >  #define ADS1015_CFG_MOD_SHIFT	8
> >  #define ADS1015_CFG_PGA_SHIFT	9
> >  #define ADS1015_CFG_MUX_SHIFT	12
> >  
> > +#define ADS1015_CFG_COMP_QUE_MASK	GENMASK(1, 0)
> > +#define ADS1015_CFG_COMP_LAT_MASK	BIT(2)
> > +#define ADS1015_CFG_COMP_POL_MASK	BIT(3)
> > +#define ADS1015_CFG_COMP_MODE_MASK	BIT(4)
> >  #define ADS1015_CFG_DR_MASK	GENMASK(7, 5)
> >  #define ADS1015_CFG_MOD_MASK	BIT(8)
> >  #define ADS1015_CFG_PGA_MASK	GENMASK(11, 9)
> >  #define ADS1015_CFG_MUX_MASK	GENMASK(14, 12)
> >  
> > +/* Comparator queue and disable field */
> > +#define ADS1015_CFG_COMP_DISABLE	3
> > +
> > +/* Comparator polarity field */
> > +#define ADS1015_CFG_COMP_POL_LOW	0
> > +#define ADS1015_CFG_COMP_POL_HIGH	1
> > +
> > +/* Comparator mode field */
> > +#define ADS1015_CFG_COMP_MODE_TRAD	0
> > +#define ADS1015_CFG_COMP_MODE_WINDOW	1
> > +
> >  /* device operating modes */
> >  #define ADS1015_CONTINUOUS	0
> >  #define ADS1015_SINGLESHOT	1
> > @@ -89,6 +112,30 @@ static int ads1015_fullscale_range[] = {
> >  	6144, 4096, 2048, 1024, 512, 256, 256, 256
> >  };
> >  
> > +/*
> > + * Translation from COMP_QUE field value to the number of successive readings
> > + * exceed the threshold values before an interrupt is generated
> > + */
> > +static const int ads1015_comp_queue[] = { 1, 2, 4 };
> > +
> > +static const struct iio_event_spec ads1015_events[] = {
> > +	{
> > +		.type = IIO_EV_TYPE_THRESH,
> > +		.dir = IIO_EV_DIR_RISING,
> > +		.mask_separate = BIT(IIO_EV_INFO_VALUE) |
> > +				BIT(IIO_EV_INFO_ENABLE),
> > +	}, {
> > +		.type = IIO_EV_TYPE_THRESH,
> > +		.dir = IIO_EV_DIR_FALLING,
> > +		.mask_separate = BIT(IIO_EV_INFO_VALUE),
> > +	}, {
> > +		.type = IIO_EV_TYPE_THRESH,
> > +		.dir = IIO_EV_DIR_EITHER,
> > +		.mask_separate = BIT(IIO_EV_INFO_ENABLE) |
> > +				BIT(IIO_EV_INFO_PERIOD),
> > +	},
> > +};
> > +
> >  #define ADS1015_V_CHAN(_chan, _addr) {				\
> >  	.type = IIO_VOLTAGE,					\
> >  	.indexed = 1,						\
> > @@ -105,6 +152,8 @@ static int ads1015_fullscale_range[] = {
> >  		.shift = 4,					\
> >  		.endianness = IIO_CPU,				\
> >  	},							\
> > +	.event_spec = ads1015_events,				\
> > +	.num_event_specs = ARRAY_SIZE(ads1015_events),		\
> >  	.datasheet_name = "AIN"#_chan,				\
> >  }
> >  
> > @@ -126,6 +175,8 @@ static int ads1015_fullscale_range[] = {
> >  		.shift = 4,					\
> >  		.endianness = IIO_CPU,				\
> >  	},							\
> > +	.event_spec = ads1015_events,				\
> > +	.num_event_specs = ARRAY_SIZE(ads1015_events),		\
> >  	.datasheet_name = "AIN"#_chan"-AIN"#_chan2,		\
> >  }
> >  
> > @@ -144,6 +195,8 @@ static int ads1015_fullscale_range[] = {
> >  		.storagebits = 16,				\
> >  		.endianness = IIO_CPU,				\
> >  	},							\
> > +	.event_spec = ads1015_events,				\
> > +	.num_event_specs = ARRAY_SIZE(ads1015_events),		\
> >  	.datasheet_name = "AIN"#_chan,				\
> >  }
> >  
> > @@ -164,9 +217,17 @@ static int ads1015_fullscale_range[] = {
> >  		.storagebits = 16,				\
> >  		.endianness = IIO_CPU,				\
> >  	},							\
> > +	.event_spec = ads1015_events,				\
> > +	.num_event_specs = ARRAY_SIZE(ads1015_events),		\
> >  	.datasheet_name = "AIN"#_chan"-AIN"#_chan2,		\
> >  }
> >  
> > +struct ads1015_thresh_data {
> > +	unsigned int comp_queue;
> > +	int high_thresh;
> > +	int low_thresh;
> > +};
> > +
> >  struct ads1015_data {
> >  	struct regmap *regmap;
> >  	/*
> > @@ -176,6 +237,10 @@ struct ads1015_data {
> >  	struct mutex lock;
> >  	struct ads1015_channel_data channel_data[ADS1015_CHANNELS];
> >  
> > +	unsigned int event_channel;
> > +	unsigned int comp_mode;
> > +	struct ads1015_thresh_data thresh_data[ADS1015_CHANNELS];
> > +
> >  	unsigned int *data_rate;
> >  	/*
> >  	 * Set to true when the ADC is switched to the continuous-conversion
> > @@ -185,15 +250,41 @@ struct ads1015_data {
> >  	bool conv_invalid;
> >  };
> >  
> > +static bool ads1015_event_channel_enabled(struct ads1015_data *data)
> > +{
> > +	return (data->event_channel != ADS1015_CHANNELS);
> > +}
> > +
> > +static void ads1015_event_channel_enable(struct ads1015_data *data, int chan,
> > +					 int comp_mode)
> > +{
> > +	WARN_ON(ads1015_event_channel_enabled(data));
> > +
> > +	data->event_channel = chan;
> > +	data->comp_mode = comp_mode;
> > +}
> > +
> > +static void ads1015_event_channel_disable(struct ads1015_data *data, int chan)
> > +{
> > +	data->event_channel = ADS1015_CHANNELS;
> > +}
> > +
> >  static bool ads1015_is_writeable_reg(struct device *dev, unsigned int reg)
> >  {
> > -	return (reg == ADS1015_CFG_REG);
> > +	switch (reg) {
> > +	case ADS1015_CFG_REG:
> > +	case ADS1015_LO_THRESH_REG:
> > +	case ADS1015_HI_THRESH_REG:
> > +		return true;
> > +	default:
> > +		return false;
> > +	}
> >  }
> >  
> >  static const struct regmap_config ads1015_regmap_config = {
> >  	.reg_bits = 8,
> >  	.val_bits = 16,
> > -	.max_register = ADS1015_CFG_REG,
> > +	.max_register = ADS1015_HI_THRESH_REG,
> >  	.writeable_reg = ads1015_is_writeable_reg,
> >  };
> >  
> > @@ -258,6 +349,14 @@ int ads1015_get_adc_result(struct ads1015_data *data, int chan, int *val)
> >  	cfg = chan << ADS1015_CFG_MUX_SHIFT | pga << ADS1015_CFG_PGA_SHIFT |
> >  		dr << ADS1015_CFG_DR_SHIFT;
> >  
> > +	if (ads1015_event_channel_enabled(data)) {
> > +		mask |= ADS1015_CFG_COMP_QUE_MASK | ADS1015_CFG_COMP_MODE_MASK;
> > +		cfg |= data->thresh_data[chan].comp_queue <<
> > +				ADS1015_CFG_COMP_QUE_SHIFT |
> > +			data->comp_mode <<
> > +				ADS1015_CFG_COMP_MODE_SHIFT;
> > +	}
> > +
> >  	cfg = (old & ~mask) | (cfg & mask);
> >  
> >  	ret = regmap_write(data->regmap, ADS1015_CFG_REG, cfg);
> > @@ -356,6 +455,12 @@ static int ads1015_read_raw(struct iio_dev *indio_dev,
> >  		if (ret)
> >  			break;
> >  
> > +		if (ads1015_event_channel_enabled(data) &&
> > +				data->event_channel != chan->address) {
> > +			ret = -EBUSY;
> > +			goto release_direct;
> > +		}
> > +
> >  		ret = ads1015_set_power_state(data, true);
> >  		if (ret < 0)
> >  			goto release_direct;
> > @@ -421,8 +526,251 @@ static int ads1015_write_raw(struct iio_dev *indio_dev,
> >  	return ret;
> >  }
> >  
> > +static int ads1015_read_event(struct iio_dev *indio_dev,
> > +	const struct iio_chan_spec *chan, enum iio_event_type type,
> > +	enum iio_event_direction dir, enum iio_event_info info, int *val,
> > +	int *val2)
> > +{
> > +	struct ads1015_data *data = iio_priv(indio_dev);
> > +	int ret;
> > +	unsigned int comp_queue;
> > +	int period;
> > +	int dr;
> > +
> > +	mutex_lock(&data->lock);
> > +
> > +	switch (info) {
> > +	case IIO_EV_INFO_VALUE:
> > +		*val = (dir == IIO_EV_DIR_RISING) ?
> > +			data->thresh_data[chan->address].high_thresh :
> > +			data->thresh_data[chan->address].low_thresh;
> > +		ret = IIO_VAL_INT;
> > +		break;
> > +	case IIO_EV_INFO_PERIOD:
> > +		dr = data->channel_data[chan->address].data_rate;
> > +		comp_queue = data->thresh_data[chan->address].comp_queue;
> > +		period = ads1015_comp_queue[comp_queue] *
> > +			USEC_PER_SEC / data->data_rate[dr];
> > +
> > +		*val = period / USEC_PER_SEC;
> > +		*val2 = period % USEC_PER_SEC;
> > +		ret = IIO_VAL_INT_PLUS_MICRO;
> > +		break;
> > +	default:
> > +		ret = -EINVAL;
> > +		break;
> > +	}
> > +
> > +	mutex_unlock(&data->lock);
> > +
> > +	return ret;
> > +}
> > +
> > +static int ads1015_write_event(struct iio_dev *indio_dev,
> > +	const struct iio_chan_spec *chan, enum iio_event_type type,
> > +	enum iio_event_direction dir, enum iio_event_info info, int val,
> > +	int val2)
> > +{
> > +	struct ads1015_data *data = iio_priv(indio_dev);
> > +	int realbits = chan->scan_type.realbits;
> > +	int ret = 0;
> > +	long long period;
> > +	int i;
> > +	int dr;
> > +
> > +	mutex_lock(&data->lock);
> > +
> > +	switch (info) {
> > +	case IIO_EV_INFO_VALUE:
> > +		if (val >= 1 << (realbits - 1) || val < -1 << (realbits - 1)) {
> > +			ret = -EINVAL;
> > +			break;
> > +		}
> > +		if (dir == IIO_EV_DIR_RISING)
> > +			data->thresh_data[chan->address].high_thresh = val;
> > +		else
> > +			data->thresh_data[chan->address].low_thresh = val;
> > +		break;
> > +	case IIO_EV_INFO_PERIOD:
> > +		dr = data->channel_data[chan->address].data_rate;
> > +		period = val * USEC_PER_SEC + val2;
> > +
> > +		for (i = 0; i < ARRAY_SIZE(ads1015_comp_queue) - 1; i++) {
> > +			if (period <= ads1015_comp_queue[i] *
> > +					USEC_PER_SEC / data->data_rate[dr])
> > +				break;
> > +		}
> > +		data->thresh_data[chan->address].comp_queue = i;
> > +		break;
> > +	default:
> > +		ret = -EINVAL;
> > +		break;
> > +	}
> > +
> > +	mutex_unlock(&data->lock);
> > +
> > +	return ret;
> > +}
> > +
> > +static int ads1015_read_event_config(struct iio_dev *indio_dev,
> > +	const struct iio_chan_spec *chan, enum iio_event_type type,
> > +	enum iio_event_direction dir)
> > +{
> > +	struct ads1015_data *data = iio_priv(indio_dev);
> > +	int ret = 0;
> > +
> > +	mutex_lock(&data->lock);
> > +	if (data->event_channel == chan->address) {
> > +		switch (dir) {
> > +		case IIO_EV_DIR_RISING:
> > +			ret = 1;
> > +			break;
> > +		case IIO_EV_DIR_EITHER:
> > +			ret = (data->comp_mode == ADS1015_CFG_COMP_MODE_WINDOW);
> > +			break;
> > +		default:
> > +			ret = -EINVAL;
> > +			break;
> > +		}
> > +	}
> > +	mutex_unlock(&data->lock);
> > +
> > +	return ret;
> > +}
> > +
> > +static int ads1015_enable_event_config(struct ads1015_data *data,
> > +	const struct iio_chan_spec *chan, int comp_mode)
> > +{
> > +	int low_thresh = data->thresh_data[chan->address].low_thresh;
> > +	int high_thresh = data->thresh_data[chan->address].high_thresh;
> > +	int ret;
> > +	unsigned int val;
> > +
> > +	if (ads1015_event_channel_enabled(data)) {
> > +		if (data->event_channel != chan->address ||
> > +			(data->comp_mode == ADS1015_CFG_COMP_MODE_TRAD &&
> > +				comp_mode == ADS1015_CFG_COMP_MODE_WINDOW))
> > +			return -EBUSY;
> > +
> > +		return 0;
> > +	}
> > +
> > +	if (comp_mode == ADS1015_CFG_COMP_MODE_TRAD) {
> > +		low_thresh = max(-1 << (chan->scan_type.realbits - 1),
> > +				high_thresh - 1);
> > +	}
> > +	ret = regmap_write(data->regmap, ADS1015_LO_THRESH_REG,
> > +			low_thresh << chan->scan_type.shift);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = regmap_write(data->regmap, ADS1015_HI_THRESH_REG,
> > +			high_thresh << chan->scan_type.shift);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = ads1015_set_power_state(data, true);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	ads1015_event_channel_enable(data, chan->address, comp_mode);
> > +
> > +	ret = ads1015_get_adc_result(data, chan->address, &val);
> > +	if (ret) {
> > +		ads1015_event_channel_disable(data, chan->address);
> > +		ads1015_set_power_state(data, false);
> > +	}
> > +
> > +	return ret;
> > +}
> > +
> > +static int ads1015_disable_event_config(struct ads1015_data *data,
> > +	const struct iio_chan_spec *chan, int comp_mode)
> > +{
> > +	int ret;
> > +
> > +	if (!ads1015_event_channel_enabled(data))
> > +		return 0;
> > +
> > +	if (data->event_channel != chan->address)
> > +		return 0;
> > +
> > +	if (data->comp_mode == ADS1015_CFG_COMP_MODE_TRAD &&
> > +			comp_mode == ADS1015_CFG_COMP_MODE_WINDOW)
> > +		return 0;
> > +
> > +	ret = regmap_update_bits(data->regmap, ADS1015_CFG_REG,
> > +				ADS1015_CFG_COMP_QUE_MASK,
> > +				ADS1015_CFG_COMP_DISABLE <<
> > +					ADS1015_CFG_COMP_QUE_SHIFT);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ads1015_event_channel_disable(data, chan->address);
> > +
> > +	return ads1015_set_power_state(data, false);
> > +}
> > +
> > +static int ads1015_write_event_config(struct iio_dev *indio_dev,
> > +	const struct iio_chan_spec *chan, enum iio_event_type type,
> > +	enum iio_event_direction dir, int state)
> > +{
> > +	struct ads1015_data *data = iio_priv(indio_dev);
> > +	int ret;
> > +	int comp_mode = (dir == IIO_EV_DIR_EITHER) ?
> > +		ADS1015_CFG_COMP_MODE_WINDOW : ADS1015_CFG_COMP_MODE_TRAD;
> > +
> > +	mutex_lock(&data->lock);
> > +
> > +	/* Prevent from enabling both buffer and event at a time */
> > +	ret = iio_device_claim_direct_mode(indio_dev);
> > +	if (ret) {
> > +		mutex_unlock(&data->lock);
> > +		return ret;
> > +	}
> > +
> > +	if (state)
> > +		ret = ads1015_enable_event_config(data, chan, comp_mode);
> > +	else
> > +		ret = ads1015_disable_event_config(data, chan, comp_mode);
> > +
> > +	iio_device_release_direct_mode(indio_dev);
> > +	mutex_unlock(&data->lock);
> > +
> > +	return ret;
> > +}
> > +
> > +static irqreturn_t ads1015_event_handler(int irq, void *priv)
> > +{
> > +	struct iio_dev *indio_dev = priv;
> > +	struct ads1015_data *data = iio_priv(indio_dev);
> > +	int ret;
> > +	int val;
> > +
> > +	/* Clear the latched ALERT/RDY pin */
> > +	ret = regmap_read(data->regmap, ADS1015_CONV_REG, &val);
> > +	if (!ret && ads1015_event_channel_enabled(data)) {
> > +		enum iio_event_direction dir;
> > +		u64 code;
> > +
> > +		dir = data->comp_mode == ADS1015_CFG_COMP_MODE_TRAD ?
> > +					IIO_EV_DIR_RISING : IIO_EV_DIR_EITHER;
> > +		code = IIO_UNMOD_EVENT_CODE(IIO_VOLTAGE, data->event_channel,
> > +					IIO_EV_TYPE_THRESH, dir);
> > +		iio_push_event(indio_dev, code, iio_get_time_ns(indio_dev));
> > +	}
> > +
> > +	return IRQ_HANDLED;
> > +}
> > +
> >  static int ads1015_buffer_preenable(struct iio_dev *indio_dev)
> >  {
> > +	struct ads1015_data *data = iio_priv(indio_dev);
> > +
> > +	/* Prevent from enabling both buffer and event at a time */
> > +	if (ads1015_event_channel_enabled(data))
> > +		return -EBUSY;
> > +
> >  	return ads1015_set_power_state(iio_priv(indio_dev), true);
> >  }
> >  
> > @@ -473,6 +821,10 @@ static const struct iio_info ads1015_info = {
> >  	.driver_module	= THIS_MODULE,
> >  	.read_raw	= ads1015_read_raw,
> >  	.write_raw	= ads1015_write_raw,
> > +	.read_event_value = ads1015_read_event,
> > +	.write_event_value = ads1015_write_event,
> > +	.read_event_config = ads1015_read_event_config,
> > +	.write_event_config = ads1015_write_event_config,
> >  	.attrs          = &ads1015_attribute_group,
> >  };
> >  
> > @@ -480,6 +832,10 @@ static const struct iio_info ads1115_info = {
> >  	.driver_module	= THIS_MODULE,
> >  	.read_raw	= ads1015_read_raw,
> >  	.write_raw	= ads1015_write_raw,
> > +	.read_event_value = ads1015_read_event,
> > +	.write_event_value = ads1015_write_event,
> > +	.read_event_config = ads1015_read_event_config,
> > +	.write_event_config = ads1015_write_event_config,
> >  	.attrs          = &ads1115_attribute_group,
> >  };
> >  
> > @@ -583,6 +939,7 @@ static int ads1015_probe(struct i2c_client *client,
> >  	struct ads1015_data *data;
> >  	int ret;
> >  	enum chip_ids chip;
> > +	int i;
> >  
> >  	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
> >  	if (!indio_dev)
> > @@ -617,6 +974,18 @@ static int ads1015_probe(struct i2c_client *client,
> >  		break;
> >  	}
> >  
> > +	data->event_channel = ADS1015_CHANNELS;
> > +	/*
> > +	 * Set default lower and upper threshold to min and max value
> > +	 * respectively.
> > +	 */
> > +	for (i = 0; i < ADS1015_CHANNELS; i++) {
> > +		int realbits = indio_dev->channels[i].scan_type.realbits;
> > +
> > +		data->thresh_data[i].low_thresh = -1 << (realbits - 1);
> > +		data->thresh_data[i].high_thresh = (1 << (realbits - 1)) - 1;
> > +	}
> > +
> >  	/* we need to keep this ABI the same as used by hwmon ADS1015 driver */
> >  	ads1015_get_channels_config(client);
> >  
> > @@ -634,6 +1003,41 @@ static int ads1015_probe(struct i2c_client *client,
> >  		return ret;
> >  	}
> >  
> > +	if (client->irq) {
> > +		unsigned long irq_trig =
> > +			irqd_get_trigger_type(irq_get_irq_data(client->irq));
> > +		unsigned int cfg_comp_mask = ADS1015_CFG_COMP_QUE_MASK |
> > +			ADS1015_CFG_COMP_LAT_MASK | ADS1015_CFG_COMP_POL_MASK;
> > +		unsigned int cfg_comp =
> > +			ADS1015_CFG_COMP_DISABLE << ADS1015_CFG_COMP_QUE_SHIFT |
> > +			1 << ADS1015_CFG_COMP_LAT_SHIFT;
> > +
> > +		switch (irq_trig) {
> > +		case IRQF_TRIGGER_LOW:
> > +			cfg_comp |= ADS1015_CFG_COMP_POL_LOW <<
> > +					ADS1015_CFG_COMP_POL_SHIFT;
> > +			break;
> > +		case IRQF_TRIGGER_HIGH:
> > +			cfg_comp |= ADS1015_CFG_COMP_POL_HIGH <<
> > +					ADS1015_CFG_COMP_POL_SHIFT;
> > +			break;
> > +		default:
> > +			return -EINVAL;
> > +		}
> > +
> > +		ret = regmap_update_bits(data->regmap, ADS1015_CFG_REG,
> > +					cfg_comp_mask, cfg_comp);
> > +		if (ret)
> > +			return ret;
> > +
> > +		ret = devm_request_threaded_irq(&client->dev, client->irq,
> > +						NULL, ads1015_event_handler,
> > +						irq_trig | IRQF_ONESHOT,
> > +						client->name, indio_dev);
> > +		if (ret)
> > +			return ret;
> > +	}
> > +
> >  	ret = ads1015_set_conv_mode(data, ADS1015_CONTINUOUS);
> >  	if (ret)
> >  		return ret;  
> 
> --
> 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 v3 11/11] iio: adc: ti-ads1015: add threshold event support

[Akinobu Mita]

2017-10-01 2:32 GMT+09:00 Jonathan Cameron <jic23@jic23.retrosnub.co.uk>:
> On Sun, 30 Jul 2017 17:58:06 +0100
> Jonathan Cameron <jic23@kernel.org> wrote:
>
>> On Mon, 31 Jul 2017 00:12:55 +0900
>> Akinobu Mita <akinobu.mita@gmail.com> wrote:
>>
>> > The ADS1015 device provides programmable comparator that can issue an
>> > interrupt on the ALERT pin.  This change adds the iio threshold event
>> > support for that feature.
>> >
>> > The ADS1015 device only have a single config register which contains an
>> > input multiplexer selection, comparator settings, and etc.  So only a
>> > single event channel can be enabled at a time.  Also enabling both buffer
>> > and event are prohibited for simplicity.
>> >
>> > Cc: Daniel Baluta <daniel.baluta@gmail.com>
>> > Cc: Jonathan Cameron <jic23@kernel.org>
>> > Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
>> Very nice.  I pretty much assumed there would be a locking problem in here
>> but as far as I can see you got it spot on!
>>
>> This will obviously be waiting on the fixes to work their way through the
>> fixes tree.
>>
>> I'm also happy with the rest of the series before this.
>>
>> Please give me a bump if it looks like I've lost this
>> somehow (it's been known to happen if the cycle of getting
>> changes back from the fixes tree takes too long)
>
> I've just been revisiting my patches to apply set and discovered I actually
> took v2 of many if not all of these patches a week before this set was posted.
> Could you sanity check the results currently in tree?

I've already realized and sent an patch that was not included in v2.
You have already added it to the fixes-togreg-post-rc1 branch of iio.git :)
Please take a look at the email thread "[PATCH v2 11/11] iio: adc:
ti-ads1015: add threshold event support".

The patch "iio: adc: ti-ads1015: fix comparator polarity setting" is
now added to Greg's staging-linus branch.  So if the patch will be merged,
everything will be fine.