* [PATCH] iio: adc: ti-adc161s626: add regulator support
@ 2016-09-16 5:32 Matt Ranostay
2016-09-18 11:20 ` Jonathan Cameron
0 siblings, 1 reply; 3+ messages in thread
From: Matt Ranostay @ 2016-09-16 5:32 UTC (permalink / raw)
To: linux-iio; +Cc: jic23, Matt Ranostay
Allow IIO_CHAN_INFO_SCALE and IIO_CHAN_INFO_OFFSET attributes for
processing by checking voltage from a regulator.
Signed-off-by: Matt Ranostay <matt@ranostay.consulting>
---
.../devicetree/bindings/iio/adc/ti-adc161s626.txt | 2 +
drivers/iio/adc/ti-adc161s626.c | 60 ++++++++++++++++++----
2 files changed, 51 insertions(+), 11 deletions(-)
diff --git a/Documentation/devicetree/bindings/iio/adc/ti-adc161s626.txt b/Documentation/devicetree/bindings/iio/adc/ti-adc161s626.txt
index 2bdad2d13d6f..0a50ee9541c8 100644
--- a/Documentation/devicetree/bindings/iio/adc/ti-adc161s626.txt
+++ b/Documentation/devicetree/bindings/iio/adc/ti-adc161s626.txt
@@ -3,6 +3,7 @@
Required properties:
- compatible: Should be "ti,adc141s626" or "ti,adc161s626"
- reg: spi chip select number for the device
+ - vdda-supply: supply voltage to VDDA pin
Recommended properties:
- spi-max-frequency: Definition as per
@@ -11,6 +12,7 @@ Recommended properties:
Example:
adc@0 {
compatible = "ti,adc161s626";
+ vdda-supply = <&vdda_fixed>;
reg = <0>;
spi-max-frequency = <4300000>;
};
diff --git a/drivers/iio/adc/ti-adc161s626.c b/drivers/iio/adc/ti-adc161s626.c
index f94b69f9c288..d192951056cf 100644
--- a/drivers/iio/adc/ti-adc161s626.c
+++ b/drivers/iio/adc/ti-adc161s626.c
@@ -27,6 +27,7 @@
#include <linux/iio/buffer.h>
#include <linux/iio/trigger_consumer.h>
#include <linux/iio/triggered_buffer.h>
+#include <linux/regulator/consumer.h>
#define TI_ADC_DRV_NAME "ti-adc161s626"
@@ -39,7 +40,9 @@ static const struct iio_chan_spec ti_adc141s626_channels[] = {
{
.type = IIO_VOLTAGE,
.channel = 0,
- .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+ BIT(IIO_CHAN_INFO_SCALE) |
+ BIT(IIO_CHAN_INFO_OFFSET),
.scan_index = 0,
.scan_type = {
.sign = 's',
@@ -54,7 +57,9 @@ static const struct iio_chan_spec ti_adc161s626_channels[] = {
{
.type = IIO_VOLTAGE,
.channel = 0,
- .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+ BIT(IIO_CHAN_INFO_SCALE) |
+ BIT(IIO_CHAN_INFO_OFFSET),
.scan_index = 0,
.scan_type = {
.sign = 's',
@@ -68,6 +73,8 @@ static const struct iio_chan_spec ti_adc161s626_channels[] = {
struct ti_adc_data {
struct iio_dev *indio_dev;
struct spi_device *spi;
+ struct regulator *ref;
+
u8 read_size;
u8 shift;
@@ -135,18 +142,32 @@ static int ti_adc_read_raw(struct iio_dev *indio_dev,
struct ti_adc_data *data = iio_priv(indio_dev);
int ret;
- if (mask != IIO_CHAN_INFO_RAW)
- return -EINVAL;
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW:
+ ret = iio_device_claim_direct_mode(indio_dev);
+ if (ret)
+ return ret;
- ret = iio_device_claim_direct_mode(indio_dev);
- if (ret)
- return ret;
+ ret = ti_adc_read_measurement(data, chan, val);
+ iio_device_release_direct_mode(indio_dev);
- ret = ti_adc_read_measurement(data, chan, val);
- iio_device_release_direct_mode(indio_dev);
+ if (ret)
+ return ret;
- if (!ret)
return IIO_VAL_INT;
+ case IIO_CHAN_INFO_SCALE:
+ ret = regulator_get_voltage(data->ref);
+ if (ret < 0)
+ return ret;
+
+ *val = ret / 1000;
+ *val2 = chan->scan_type.realbits;
+
+ return IIO_VAL_FRACTIONAL_LOG2;
+ case IIO_CHAN_INFO_OFFSET:
+ *val = 1 << (chan->scan_type.realbits - 1);
+ return IIO_VAL_INT;
+ }
return 0;
}
@@ -191,10 +212,22 @@ static int ti_adc_probe(struct spi_device *spi)
break;
}
+ data->ref = devm_regulator_get(&spi->dev, "vdda");
+ if (!IS_ERR(data->ref)) {
+ ret = regulator_enable(data->ref);
+ if (ret < 0)
+ return ret;
+
+ ret = regulator_get_voltage(data->ref);
+ if (ret < 0)
+ goto error_regulator_disable;
+ } else
+ return IS_ERR(data->ref);
+
ret = iio_triggered_buffer_setup(indio_dev, NULL,
ti_adc_trigger_handler, NULL);
if (ret)
- return ret;
+ goto error_regulator_disable;
ret = iio_device_register(indio_dev);
if (ret)
@@ -205,15 +238,20 @@ static int ti_adc_probe(struct spi_device *spi)
error_unreg_buffer:
iio_triggered_buffer_cleanup(indio_dev);
+error_regulator_disable:
+ regulator_disable(data->ref);
+
return ret;
}
static int ti_adc_remove(struct spi_device *spi)
{
struct iio_dev *indio_dev = spi_get_drvdata(spi);
+ struct ti_adc_data *data = iio_priv(indio_dev);
iio_device_unregister(indio_dev);
iio_triggered_buffer_cleanup(indio_dev);
+ regulator_disable(data->ref);
return 0;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] iio: adc: ti-adc161s626: add regulator support
2016-09-16 5:32 [PATCH] iio: adc: ti-adc161s626: add regulator support Matt Ranostay
@ 2016-09-18 11:20 ` Jonathan Cameron
2016-09-18 22:08 ` Matt Ranostay
0 siblings, 1 reply; 3+ messages in thread
From: Jonathan Cameron @ 2016-09-18 11:20 UTC (permalink / raw)
To: Matt Ranostay, linux-iio; +Cc: Matt Ranostay
On 16/09/16 06:32, Matt Ranostay wrote:
> Allow IIO_CHAN_INFO_SCALE and IIO_CHAN_INFO_OFFSET attributes for
> processing by checking voltage from a regulator.
>
> Signed-off-by: Matt Ranostay <matt@ranostay.consulting>
Comments inline.
> ---
> .../devicetree/bindings/iio/adc/ti-adc161s626.txt | 2 +
> drivers/iio/adc/ti-adc161s626.c | 60 ++++++++++++++++++----
> 2 files changed, 51 insertions(+), 11 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/iio/adc/ti-adc161s626.txt b/Documentation/devicetree/bindings/iio/adc/ti-adc161s626.txt
> index 2bdad2d13d6f..0a50ee9541c8 100644
> --- a/Documentation/devicetree/bindings/iio/adc/ti-adc161s626.txt
> +++ b/Documentation/devicetree/bindings/iio/adc/ti-adc161s626.txt
> @@ -3,6 +3,7 @@
> Required properties:
> - compatible: Should be "ti,adc141s626" or "ti,adc161s626"
> - reg: spi chip select number for the device
> + - vdda-supply: supply voltage to VDDA pin
>
> Recommended properties:
> - spi-max-frequency: Definition as per
> @@ -11,6 +12,7 @@ Recommended properties:
> Example:
> adc@0 {
> compatible = "ti,adc161s626";
> + vdda-supply = <&vdda_fixed>;
> reg = <0>;
> spi-max-frequency = <4300000>;
> };
> diff --git a/drivers/iio/adc/ti-adc161s626.c b/drivers/iio/adc/ti-adc161s626.c
> index f94b69f9c288..d192951056cf 100644
> --- a/drivers/iio/adc/ti-adc161s626.c
> +++ b/drivers/iio/adc/ti-adc161s626.c
> @@ -27,6 +27,7 @@
> #include <linux/iio/buffer.h>
> #include <linux/iio/trigger_consumer.h>
> #include <linux/iio/triggered_buffer.h>
> +#include <linux/regulator/consumer.h>
>
> #define TI_ADC_DRV_NAME "ti-adc161s626"
>
> @@ -39,7 +40,9 @@ static const struct iio_chan_spec ti_adc141s626_channels[] = {
> {
> .type = IIO_VOLTAGE,
> .channel = 0,
> - .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> + BIT(IIO_CHAN_INFO_SCALE) |
> + BIT(IIO_CHAN_INFO_OFFSET),
> .scan_index = 0,
> .scan_type = {
> .sign = 's',
> @@ -54,7 +57,9 @@ static const struct iio_chan_spec ti_adc161s626_channels[] = {
> {
> .type = IIO_VOLTAGE,
> .channel = 0,
> - .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> + BIT(IIO_CHAN_INFO_SCALE) |
> + BIT(IIO_CHAN_INFO_OFFSET),
> .scan_index = 0,
> .scan_type = {
> .sign = 's',
> @@ -68,6 +73,8 @@ static const struct iio_chan_spec ti_adc161s626_channels[] = {
> struct ti_adc_data {
> struct iio_dev *indio_dev;
> struct spi_device *spi;
> + struct regulator *ref;
> +
> u8 read_size;
> u8 shift;
>
> @@ -135,18 +142,32 @@ static int ti_adc_read_raw(struct iio_dev *indio_dev,
> struct ti_adc_data *data = iio_priv(indio_dev);
> int ret;
>
> - if (mask != IIO_CHAN_INFO_RAW)
> - return -EINVAL;
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + ret = iio_device_claim_direct_mode(indio_dev);
> + if (ret)
> + return ret;
>
> - ret = iio_device_claim_direct_mode(indio_dev);
> - if (ret)
> - return ret;
> + ret = ti_adc_read_measurement(data, chan, val);
> + iio_device_release_direct_mode(indio_dev);
>
> - ret = ti_adc_read_measurement(data, chan, val);
> - iio_device_release_direct_mode(indio_dev);
> + if (ret)
> + return ret;
>
> - if (!ret)
> return IIO_VAL_INT;
> + case IIO_CHAN_INFO_SCALE:
> + ret = regulator_get_voltage(data->ref);
> + if (ret < 0)
> + return ret;
> +
> + *val = ret / 1000;
> + *val2 = chan->scan_type.realbits;
> +
> + return IIO_VAL_FRACTIONAL_LOG2;
> + case IIO_CHAN_INFO_OFFSET:
> + *val = 1 << (chan->scan_type.realbits - 1);
> + return IIO_VAL_INT;
> + }
>
> return 0;
> }
> @@ -191,10 +212,22 @@ static int ti_adc_probe(struct spi_device *spi)
> break;
> }
>
> + data->ref = devm_regulator_get(&spi->dev, "vdda");
> + if (!IS_ERR(data->ref)) {
> + ret = regulator_enable(data->ref);
> + if (ret < 0)
> + return ret;
> +
> + ret = regulator_get_voltage(data->ref);
> + if (ret < 0)
> + goto error_regulator_disable;
> + } else
> + return IS_ERR(data->ref);
This is an overly complex way of organizing things.
Check the error and return first.
Then the good path just becomes normal flow.
There is also the irritating case of the regulator not being specified.
Then we get a dummy which is fine until we ask it what voltage it
is supplying..
In that case I think we get a return -EINVAL.
I'd have no trouble with us returning -EINVAL on an attempt to read
the scale attribute if the regulator isn't specified, but refusing to
probe seems harsh. Particularly as before this support was added it
would have probed fine.
> +
> ret = iio_triggered_buffer_setup(indio_dev, NULL,
> ti_adc_trigger_handler, NULL);
> if (ret)
> - return ret;
> + goto error_regulator_disable;
>
> ret = iio_device_register(indio_dev);
> if (ret)
> @@ -205,15 +238,20 @@ static int ti_adc_probe(struct spi_device *spi)
> error_unreg_buffer:
> iio_triggered_buffer_cleanup(indio_dev);
>
> +error_regulator_disable:
> + regulator_disable(data->ref);
> +
> return ret;
> }
>
> static int ti_adc_remove(struct spi_device *spi)
> {
> struct iio_dev *indio_dev = spi_get_drvdata(spi);
> + struct ti_adc_data *data = iio_priv(indio_dev);
>
> iio_device_unregister(indio_dev);
> iio_triggered_buffer_cleanup(indio_dev);
> + regulator_disable(data->ref);
>
> return 0;
> }
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] iio: adc: ti-adc161s626: add regulator support
2016-09-18 11:20 ` Jonathan Cameron
@ 2016-09-18 22:08 ` Matt Ranostay
0 siblings, 0 replies; 3+ messages in thread
From: Matt Ranostay @ 2016-09-18 22:08 UTC (permalink / raw)
To: Jonathan Cameron; +Cc: linux-iio, Matt Ranostay
On Sun, Sep 18, 2016 at 4:20 AM, Jonathan Cameron <jic23@kernel.org> wrote:
> On 16/09/16 06:32, Matt Ranostay wrote:
>> Allow IIO_CHAN_INFO_SCALE and IIO_CHAN_INFO_OFFSET attributes for
>> processing by checking voltage from a regulator.
>>
>> Signed-off-by: Matt Ranostay <matt@ranostay.consulting>
> Comments inline.
>> ---
>> .../devicetree/bindings/iio/adc/ti-adc161s626.txt | 2 +
>> drivers/iio/adc/ti-adc161s626.c | 60 ++++++++++++++++++----
>> 2 files changed, 51 insertions(+), 11 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/iio/adc/ti-adc161s626.txt b/Documentation/devicetree/bindings/iio/adc/ti-adc161s626.txt
>> index 2bdad2d13d6f..0a50ee9541c8 100644
>> --- a/Documentation/devicetree/bindings/iio/adc/ti-adc161s626.txt
>> +++ b/Documentation/devicetree/bindings/iio/adc/ti-adc161s626.txt
>> @@ -3,6 +3,7 @@
>> Required properties:
>> - compatible: Should be "ti,adc141s626" or "ti,adc161s626"
>> - reg: spi chip select number for the device
>> + - vdda-supply: supply voltage to VDDA pin
>>
>> Recommended properties:
>> - spi-max-frequency: Definition as per
>> @@ -11,6 +12,7 @@ Recommended properties:
>> Example:
>> adc@0 {
>> compatible = "ti,adc161s626";
>> + vdda-supply = <&vdda_fixed>;
>> reg = <0>;
>> spi-max-frequency = <4300000>;
>> };
>> diff --git a/drivers/iio/adc/ti-adc161s626.c b/drivers/iio/adc/ti-adc161s626.c
>> index f94b69f9c288..d192951056cf 100644
>> --- a/drivers/iio/adc/ti-adc161s626.c
>> +++ b/drivers/iio/adc/ti-adc161s626.c
>> @@ -27,6 +27,7 @@
>> #include <linux/iio/buffer.h>
>> #include <linux/iio/trigger_consumer.h>
>> #include <linux/iio/triggered_buffer.h>
>> +#include <linux/regulator/consumer.h>
>>
>> #define TI_ADC_DRV_NAME "ti-adc161s626"
>>
>> @@ -39,7 +40,9 @@ static const struct iio_chan_spec ti_adc141s626_channels[] = {
>> {
>> .type = IIO_VOLTAGE,
>> .channel = 0,
>> - .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
>> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
>> + BIT(IIO_CHAN_INFO_SCALE) |
>> + BIT(IIO_CHAN_INFO_OFFSET),
>> .scan_index = 0,
>> .scan_type = {
>> .sign = 's',
>> @@ -54,7 +57,9 @@ static const struct iio_chan_spec ti_adc161s626_channels[] = {
>> {
>> .type = IIO_VOLTAGE,
>> .channel = 0,
>> - .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
>> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
>> + BIT(IIO_CHAN_INFO_SCALE) |
>> + BIT(IIO_CHAN_INFO_OFFSET),
>> .scan_index = 0,
>> .scan_type = {
>> .sign = 's',
>> @@ -68,6 +73,8 @@ static const struct iio_chan_spec ti_adc161s626_channels[] = {
>> struct ti_adc_data {
>> struct iio_dev *indio_dev;
>> struct spi_device *spi;
>> + struct regulator *ref;
>> +
>> u8 read_size;
>> u8 shift;
>>
>> @@ -135,18 +142,32 @@ static int ti_adc_read_raw(struct iio_dev *indio_dev,
>> struct ti_adc_data *data = iio_priv(indio_dev);
>> int ret;
>>
>> - if (mask != IIO_CHAN_INFO_RAW)
>> - return -EINVAL;
>> + switch (mask) {
>> + case IIO_CHAN_INFO_RAW:
>> + ret = iio_device_claim_direct_mode(indio_dev);
>> + if (ret)
>> + return ret;
>>
>> - ret = iio_device_claim_direct_mode(indio_dev);
>> - if (ret)
>> - return ret;
>> + ret = ti_adc_read_measurement(data, chan, val);
>> + iio_device_release_direct_mode(indio_dev);
>>
>> - ret = ti_adc_read_measurement(data, chan, val);
>> - iio_device_release_direct_mode(indio_dev);
>> + if (ret)
>> + return ret;
>>
>> - if (!ret)
>> return IIO_VAL_INT;
>> + case IIO_CHAN_INFO_SCALE:
>> + ret = regulator_get_voltage(data->ref);
>> + if (ret < 0)
>> + return ret;
>> +
>> + *val = ret / 1000;
>> + *val2 = chan->scan_type.realbits;
>> +
>> + return IIO_VAL_FRACTIONAL_LOG2;
>> + case IIO_CHAN_INFO_OFFSET:
>> + *val = 1 << (chan->scan_type.realbits - 1);
>> + return IIO_VAL_INT;
>> + }
>>
>> return 0;
>> }
>> @@ -191,10 +212,22 @@ static int ti_adc_probe(struct spi_device *spi)
>> break;
>> }
>>
>> + data->ref = devm_regulator_get(&spi->dev, "vdda");
>> + if (!IS_ERR(data->ref)) {
>> + ret = regulator_enable(data->ref);
>> + if (ret < 0)
>> + return ret;
>> +
>> + ret = regulator_get_voltage(data->ref);
>> + if (ret < 0)
>> + goto error_regulator_disable;
>> + } else
>> + return IS_ERR(data->ref);
> This is an overly complex way of organizing things.
> Check the error and return first.
>
> Then the good path just becomes normal flow.
>
> There is also the irritating case of the regulator not being specified.
> Then we get a dummy which is fine until we ask it what voltage it
> is supplying..
>
> In that case I think we get a return -EINVAL.
> I'd have no trouble with us returning -EINVAL on an attempt to read
> the scale attribute if the regulator isn't specified, but refusing to
> probe seems harsh. Particularly as before this support was added it
> would have probed fine.
Ok will fix in v2
>> +
>> ret = iio_triggered_buffer_setup(indio_dev, NULL,
>> ti_adc_trigger_handler, NULL);
>> if (ret)
>> - return ret;
>> + goto error_regulator_disable;
>>
>> ret = iio_device_register(indio_dev);
>> if (ret)
>> @@ -205,15 +238,20 @@ static int ti_adc_probe(struct spi_device *spi)
>> error_unreg_buffer:
>> iio_triggered_buffer_cleanup(indio_dev);
>>
>> +error_regulator_disable:
>> + regulator_disable(data->ref);
>> +
>> return ret;
>> }
>>
>> static int ti_adc_remove(struct spi_device *spi)
>> {
>> struct iio_dev *indio_dev = spi_get_drvdata(spi);
>> + struct ti_adc_data *data = iio_priv(indio_dev);
>>
>> iio_device_unregister(indio_dev);
>> iio_triggered_buffer_cleanup(indio_dev);
>> + regulator_disable(data->ref);
>>
>> return 0;
>> }
>>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-09-18 22:08 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-16 5:32 [PATCH] iio: adc: ti-adc161s626: add regulator support Matt Ranostay
2016-09-18 11:20 ` Jonathan Cameron
2016-09-18 22:08 ` Matt Ranostay
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.