[PATCH v2 0/4] Adding scale support to the lpc32xx ADC driver

[PATCH v2 0/4] Adding scale support to the lpc32xx ADC driver

[Gregory CLEMENT]

Hello,

This is the second version of a series adding the support of the scale
feature to the lpc32xx ADC driver. In order to use it we need to have
the voltage reference as a device tree property, however in order to
be backward compatible, this property is optional and do not prevent
to use the driver if missing.

I updated the binding documentation accordingly.

Gregory

Changelog:
v1 -> v2:

 - "dt-bindings: iio: adc: move lpc32xx-adc out of staging" and
   "iio:adc:lpc32xx use SPDX-License-Identifier" has been merged

 - split "iio:adc:lpc32xx use SPDX-License-Identifier" in 2 patches
   one foe sorting and the second one for removal

 - add reviewed-by from Rob Herring on the binding documentation patch

 - add default in the switch/case in lpc32xx_read_raw()

 - no more use scan_type.realbits and directly put the value in the
   IIO_CHAN_INFO_SCALE case

 - keep the const for the struct iio_chan_spec and declare 2 of them.

 - remove the unrelated and bad whitespace change.

Gregory CLEMENT (4):
  iio: adc: lpc32xx: Sort headers
  iio: adc: lpc32xx: Cleanup headers
  iio: adc: lpc32xx: Add scale feature
  dt-bindings: iio: adc: lpc32xx-adc: Document vref-supply

 .../bindings/iio/adc/lpc32xx-adc.txt          |  5 ++
 drivers/iio/adc/lpc32xx_adc.c                 | 59 +++++++++++++------
 2 files changed, 47 insertions(+), 17 deletions(-)

-- 
2.20.1

[PATCH v2 1/4] iio: adc: lpc32xx: Sort headers

[Gregory CLEMENT]

Sort the headers in alphabetic order in order to ease the maintenance for
this part.

Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com>
---
 drivers/iio/adc/lpc32xx_adc.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/iio/adc/lpc32xx_adc.c b/drivers/iio/adc/lpc32xx_adc.c
index e361c1532a75..7084f804504e 100644
--- a/drivers/iio/adc/lpc32xx_adc.c
+++ b/drivers/iio/adc/lpc32xx_adc.c
@@ -7,20 +7,19 @@
  *  Copyright (C) 2011, 2012 Roland Stigge <stigge@antcom.de>
  */
 
-#include <linux/module.h>
-#include <linux/platform_device.h>
-#include <linux/interrupt.h>
-#include <linux/device.h>
-#include <linux/kernel.h>
-#include <linux/slab.h>
-#include <linux/io.h>
 #include <linux/clk.h>
-#include <linux/err.h>
 #include <linux/completion.h>
-#include <linux/of.h>
-
+#include <linux/device.h>
+#include <linux/err.h>
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
 
 /*
  * LPC32XX registers definitions
-- 
2.20.1

[PATCH v2 2/4] iio: adc: lpc32xx: Cleanup headers

[Gregory CLEMENT]

A few headers is useless: remove them.

Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com>
---
 drivers/iio/adc/lpc32xx_adc.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/iio/adc/lpc32xx_adc.c b/drivers/iio/adc/lpc32xx_adc.c
index 7084f804504e..f391c1e10136 100644
--- a/drivers/iio/adc/lpc32xx_adc.c
+++ b/drivers/iio/adc/lpc32xx_adc.c
@@ -9,17 +9,11 @@
 
 #include <linux/clk.h>
 #include <linux/completion.h>
-#include <linux/device.h>
-#include <linux/err.h>
 #include <linux/iio/iio.h>
-#include <linux/iio/sysfs.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
-#include <linux/kernel.h>
 #include <linux/module.h>
-#include <linux/of.h>
 #include <linux/platform_device.h>
-#include <linux/slab.h>
 
 /*
  * LPC32XX registers definitions
-- 
2.20.1

[PATCH v2 3/4] iio: adc: lpc32xx: Add scale feature

[Gregory CLEMENT]

Until now this driver only exposed the raw value of the channels. With
this patch, the scale value is also exposed.

It depends of a regulator supply, and unlike most of the other driver, do
not having this regulator won't prevent to use the driver. The reason for
it is to allow to continue to use this driver with an old device tree. If
there is no regulator supply then the scale won't be exposed.

Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com>
---
 drivers/iio/adc/lpc32xx_adc.c | 44 ++++++++++++++++++++++++++++++-----
 1 file changed, 38 insertions(+), 6 deletions(-)

diff --git a/drivers/iio/adc/lpc32xx_adc.c b/drivers/iio/adc/lpc32xx_adc.c
index f391c1e10136..5d1c76e3aad8 100644
--- a/drivers/iio/adc/lpc32xx_adc.c
+++ b/drivers/iio/adc/lpc32xx_adc.c
@@ -14,6 +14,7 @@
 #include <linux/io.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
+#include <linux/regulator/consumer.h>
 
 /*
  * LPC32XX registers definitions
@@ -45,6 +46,7 @@ struct lpc32xx_adc_state {
 	void __iomem *adc_base;
 	struct clk *clk;
 	struct completion completion;
+	struct regulator *vref;
 
 	u32 value;
 };
@@ -57,7 +59,9 @@ static int lpc32xx_read_raw(struct iio_dev *indio_dev,
 {
 	struct lpc32xx_adc_state *st = iio_priv(indio_dev);
 	int ret;
-	if (mask == IIO_CHAN_INFO_RAW) {
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
 		mutex_lock(&indio_dev->mlock);
 		ret = clk_prepare_enable(st->clk);
 		if (ret) {
@@ -77,22 +81,36 @@ static int lpc32xx_read_raw(struct iio_dev *indio_dev,
 		mutex_unlock(&indio_dev->mlock);
 
 		return IIO_VAL_INT;
-	}
 
-	return -EINVAL;
+	case IIO_CHAN_INFO_SCALE:
+		*val = regulator_get_voltage(st->vref) / 1000;
+		*val2 =  10;
+
+		return IIO_VAL_FRACTIONAL_LOG2;
+	default:
+		return -EINVAL;
+	}
 }
 
 static const struct iio_info lpc32xx_adc_iio_info = {
 	.read_raw = &lpc32xx_read_raw,
 };
 
-#define LPC32XX_ADC_CHANNEL(_index) {			\
+#define LPC32XX_ADC_CHANNEL_BASE(_index)		\
 	.type = IIO_VOLTAGE,				\
 	.indexed = 1,					\
 	.channel = _index,				\
 	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),	\
 	.address = LPC32XXAD_IN * _index,		\
-	.scan_index = _index,				\
+	.scan_index = _index,
+
+#define LPC32XX_ADC_CHANNEL(_index) {		\
+	LPC32XX_ADC_CHANNEL_BASE(_index)	\
+}
+
+#define LPC32XX_ADC_SCALE_CHANNEL(_index) {			\
+	LPC32XX_ADC_CHANNEL_BASE(_index)			\
+	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE)	\
 }
 
 static const struct iio_chan_spec lpc32xx_adc_iio_channels[] = {
@@ -101,6 +119,12 @@ static const struct iio_chan_spec lpc32xx_adc_iio_channels[] = {
 	LPC32XX_ADC_CHANNEL(2),
 };
 
+static const struct iio_chan_spec lpc32xx_adc_iio_scale_channels[] = {
+	LPC32XX_ADC_SCALE_CHANNEL(0),
+	LPC32XX_ADC_SCALE_CHANNEL(1),
+	LPC32XX_ADC_SCALE_CHANNEL(2),
+};
+
 static irqreturn_t lpc32xx_adc_isr(int irq, void *dev_id)
 {
 	struct lpc32xx_adc_state *st = dev_id;
@@ -159,6 +183,15 @@ static int lpc32xx_adc_probe(struct platform_device *pdev)
 		return retval;
 	}
 
+	st->vref = devm_regulator_get(&pdev->dev, "vref");
+	if (IS_ERR(st->vref)) {
+		iodev->channels = lpc32xx_adc_iio_channels;
+		dev_err(&pdev->dev,
+			"Missing vref regulator: No scaling available\n");
+	} else {
+		iodev->channels = lpc32xx_adc_iio_scale_channels;
+	}
+
 	platform_set_drvdata(pdev, iodev);
 
 	init_completion(&st->completion);
@@ -167,7 +200,6 @@ static int lpc32xx_adc_probe(struct platform_device *pdev)
 	iodev->dev.parent = &pdev->dev;
 	iodev->info = &lpc32xx_adc_iio_info;
 	iodev->modes = INDIO_DIRECT_MODE;
-	iodev->channels = lpc32xx_adc_iio_channels;
 	iodev->num_channels = ARRAY_SIZE(lpc32xx_adc_iio_channels);
 
 	retval = devm_iio_device_register(&pdev->dev, iodev);
-- 
2.20.1

[PATCH v2 4/4] dt-bindings: iio: adc: lpc32xx-adc: Document vref-supply

[Gregory CLEMENT]

As most of the other ADC the lpc32xx one use a vref-supply property:
document it.

Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com>
---
 Documentation/devicetree/bindings/iio/adc/lpc32xx-adc.txt | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/adc/lpc32xx-adc.txt b/Documentation/devicetree/bindings/iio/adc/lpc32xx-adc.txt
index b3629d3a9adf..3a1bc669bd51 100644
--- a/Documentation/devicetree/bindings/iio/adc/lpc32xx-adc.txt
+++ b/Documentation/devicetree/bindings/iio/adc/lpc32xx-adc.txt
@@ -6,6 +6,10 @@ Required properties:
   region.
 - interrupts: The ADC interrupt
 
+Optional:
+ - vref-supply: The regulator supply ADC reference voltage, optional
+   for legacy reason, but highly encouraging to us in new device tree
+
 Example:
 
 	adc@40048000 {
@@ -13,4 +17,5 @@ Example:
 		reg = <0x40048000 0x1000>;
 		interrupt-parent = <&mic>;
 		interrupts = <39 0>;
+		vref-supply = <&vcc>;
 	};
-- 
2.20.1

Re: [PATCH v2 0/4] Adding scale support to the lpc32xx ADC driver

[Jonathan Cameron]

On Fri, 15 Mar 2019 10:52:27 +0100
Gregory CLEMENT <gregory.clement@bootlin.com> wrote:

> Hello,
> 
> This is the second version of a series adding the support of the scale
> feature to the lpc32xx ADC driver. In order to use it we need to have
> the voltage reference as a device tree property, however in order to
> be backward compatible, this property is optional and do not prevent
> to use the driver if missing.
> 
> I updated the binding documentation accordingly.
> 
> Gregory
> 
> Changelog:
> v1 -> v2:
> 
>  - "dt-bindings: iio: adc: move lpc32xx-adc out of staging" and
>    "iio:adc:lpc32xx use SPDX-License-Identifier" has been merged
> 
>  - split "iio:adc:lpc32xx use SPDX-License-Identifier" in 2 patches
>    one foe sorting and the second one for removal
Hohum. I'm reviewing cover letters now. Must have too much time on 
my hands ;)

This is talking about the wrong patch.

> 
>  - add reviewed-by from Rob Herring on the binding documentation patch
> 
>  - add default in the switch/case in lpc32xx_read_raw()
> 
>  - no more use scan_type.realbits and directly put the value in the
>    IIO_CHAN_INFO_SCALE case
> 
>  - keep the const for the struct iio_chan_spec and declare 2 of them.
> 
>  - remove the unrelated and bad whitespace change.
> 
> Gregory CLEMENT (4):
>   iio: adc: lpc32xx: Sort headers
>   iio: adc: lpc32xx: Cleanup headers
>   iio: adc: lpc32xx: Add scale feature
>   dt-bindings: iio: adc: lpc32xx-adc: Document vref-supply
> 
>  .../bindings/iio/adc/lpc32xx-adc.txt          |  5 ++
>  drivers/iio/adc/lpc32xx_adc.c                 | 59 +++++++++++++------
>  2 files changed, 47 insertions(+), 17 deletions(-)
> 

Re: [PATCH v2 1/4] iio: adc: lpc32xx: Sort headers

[Jonathan Cameron]

On Fri, 15 Mar 2019 10:52:28 +0100
Gregory CLEMENT <gregory.clement@bootlin.com> wrote:

> Sort the headers in alphabetic order in order to ease the maintenance for
> this part.
> 
> Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com>
Applied,

Thanks,

Jonathan

> ---
>  drivers/iio/adc/lpc32xx_adc.c | 19 +++++++++----------
>  1 file changed, 9 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/iio/adc/lpc32xx_adc.c b/drivers/iio/adc/lpc32xx_adc.c
> index e361c1532a75..7084f804504e 100644
> --- a/drivers/iio/adc/lpc32xx_adc.c
> +++ b/drivers/iio/adc/lpc32xx_adc.c
> @@ -7,20 +7,19 @@
>   *  Copyright (C) 2011, 2012 Roland Stigge <stigge@antcom.de>
>   */
>  
> -#include <linux/module.h>
> -#include <linux/platform_device.h>
> -#include <linux/interrupt.h>
> -#include <linux/device.h>
> -#include <linux/kernel.h>
> -#include <linux/slab.h>
> -#include <linux/io.h>
>  #include <linux/clk.h>
> -#include <linux/err.h>
>  #include <linux/completion.h>
> -#include <linux/of.h>
> -
> +#include <linux/device.h>
> +#include <linux/err.h>
>  #include <linux/iio/iio.h>
>  #include <linux/iio/sysfs.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
>  
>  /*
>   * LPC32XX registers definitions

Re: [PATCH v2 2/4] iio: adc: lpc32xx: Cleanup headers

[Jonathan Cameron]

On Fri, 15 Mar 2019 10:52:29 +0100
Gregory CLEMENT <gregory.clement@bootlin.com> wrote:

> A few headers is useless: remove them.
> 
> Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com>
> ---
>  drivers/iio/adc/lpc32xx_adc.c | 6 ------
>  1 file changed, 6 deletions(-)
> 
> diff --git a/drivers/iio/adc/lpc32xx_adc.c b/drivers/iio/adc/lpc32xx_adc.c
> index 7084f804504e..f391c1e10136 100644
> --- a/drivers/iio/adc/lpc32xx_adc.c
> +++ b/drivers/iio/adc/lpc32xx_adc.c
> @@ -9,17 +9,11 @@
>  
>  #include <linux/clk.h>
>  #include <linux/completion.h>
> -#include <linux/device.h>
> -#include <linux/err.h>
 -EINVAL comes from asm/errno.h I believe. IS_ERR
is also in this header.  Otherwise, they are fine, so
I'll just leave this one.

>  #include <linux/iio/iio.h>
> -#include <linux/iio/sysfs.h>
>  #include <linux/interrupt.h>
>  #include <linux/io.h>
> -#include <linux/kernel.h>
>  #include <linux/module.h>
> -#include <linux/of.h>
>  #include <linux/platform_device.h>
> -#include <linux/slab.h>
>  
>  /*
>   * LPC32XX registers definitions

Re: [PATCH v2 3/4] iio: adc: lpc32xx: Add scale feature

[Jonathan Cameron]

On Fri, 15 Mar 2019 10:52:30 +0100
Gregory CLEMENT <gregory.clement@bootlin.com> wrote:

> Until now this driver only exposed the raw value of the channels. With
> this patch, the scale value is also exposed.
> 
> It depends of a regulator supply, and unlike most of the other driver, do
> not having this regulator won't prevent to use the driver. The reason for
> it is to allow to continue to use this driver with an old device tree. If
> there is no regulator supply then the scale won't be exposed.
> 
> Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com>

I made a minor tweak to downgrade the message if the regulator isn't
specified.

Applied to the togreg branch of iio.git and pushed out as testing
for the autobuilders to play with it.

Thanks,

Jonathan

> ---
>  drivers/iio/adc/lpc32xx_adc.c | 44 ++++++++++++++++++++++++++++++-----
>  1 file changed, 38 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/iio/adc/lpc32xx_adc.c b/drivers/iio/adc/lpc32xx_adc.c
> index f391c1e10136..5d1c76e3aad8 100644
> --- a/drivers/iio/adc/lpc32xx_adc.c
> +++ b/drivers/iio/adc/lpc32xx_adc.c
> @@ -14,6 +14,7 @@
>  #include <linux/io.h>
>  #include <linux/module.h>
>  #include <linux/platform_device.h>
> +#include <linux/regulator/consumer.h>
>  
>  /*
>   * LPC32XX registers definitions
> @@ -45,6 +46,7 @@ struct lpc32xx_adc_state {
>  	void __iomem *adc_base;
>  	struct clk *clk;
>  	struct completion completion;
> +	struct regulator *vref;
>  
>  	u32 value;
>  };
> @@ -57,7 +59,9 @@ static int lpc32xx_read_raw(struct iio_dev *indio_dev,
>  {
>  	struct lpc32xx_adc_state *st = iio_priv(indio_dev);
>  	int ret;
> -	if (mask == IIO_CHAN_INFO_RAW) {
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
>  		mutex_lock(&indio_dev->mlock);
>  		ret = clk_prepare_enable(st->clk);
>  		if (ret) {
> @@ -77,22 +81,36 @@ static int lpc32xx_read_raw(struct iio_dev *indio_dev,
>  		mutex_unlock(&indio_dev->mlock);
>  
>  		return IIO_VAL_INT;
> -	}
>  
> -	return -EINVAL;
> +	case IIO_CHAN_INFO_SCALE:
> +		*val = regulator_get_voltage(st->vref) / 1000;
> +		*val2 =  10;
> +
> +		return IIO_VAL_FRACTIONAL_LOG2;
> +	default:
> +		return -EINVAL;
> +	}
>  }
>  
>  static const struct iio_info lpc32xx_adc_iio_info = {
>  	.read_raw = &lpc32xx_read_raw,
>  };
>  
> -#define LPC32XX_ADC_CHANNEL(_index) {			\
> +#define LPC32XX_ADC_CHANNEL_BASE(_index)		\
>  	.type = IIO_VOLTAGE,				\
>  	.indexed = 1,					\
>  	.channel = _index,				\
>  	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),	\
>  	.address = LPC32XXAD_IN * _index,		\
> -	.scan_index = _index,				\
> +	.scan_index = _index,
> +
> +#define LPC32XX_ADC_CHANNEL(_index) {		\
> +	LPC32XX_ADC_CHANNEL_BASE(_index)	\
> +}
> +
> +#define LPC32XX_ADC_SCALE_CHANNEL(_index) {			\
> +	LPC32XX_ADC_CHANNEL_BASE(_index)			\
> +	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE)	\
>  }
>  
>  static const struct iio_chan_spec lpc32xx_adc_iio_channels[] = {
> @@ -101,6 +119,12 @@ static const struct iio_chan_spec lpc32xx_adc_iio_channels[] = {
>  	LPC32XX_ADC_CHANNEL(2),
>  };
>  
> +static const struct iio_chan_spec lpc32xx_adc_iio_scale_channels[] = {
> +	LPC32XX_ADC_SCALE_CHANNEL(0),
> +	LPC32XX_ADC_SCALE_CHANNEL(1),
> +	LPC32XX_ADC_SCALE_CHANNEL(2),
> +};
> +
>  static irqreturn_t lpc32xx_adc_isr(int irq, void *dev_id)
>  {
>  	struct lpc32xx_adc_state *st = dev_id;
> @@ -159,6 +183,15 @@ static int lpc32xx_adc_probe(struct platform_device *pdev)
>  		return retval;
>  	}
>  
> +	st->vref = devm_regulator_get(&pdev->dev, "vref");
> +	if (IS_ERR(st->vref)) {
> +		iodev->channels = lpc32xx_adc_iio_channels;
> +		dev_err(&pdev->dev,
> +			"Missing vref regulator: No scaling available\n");
It isn't an error, so downgraded to dev_info.

> +	} else {
> +		iodev->channels = lpc32xx_adc_iio_scale_channels;
> +	}
> +
>  	platform_set_drvdata(pdev, iodev);
>  
>  	init_completion(&st->completion);
> @@ -167,7 +200,6 @@ static int lpc32xx_adc_probe(struct platform_device *pdev)
>  	iodev->dev.parent = &pdev->dev;
>  	iodev->info = &lpc32xx_adc_iio_info;
>  	iodev->modes = INDIO_DIRECT_MODE;
> -	iodev->channels = lpc32xx_adc_iio_channels;
>  	iodev->num_channels = ARRAY_SIZE(lpc32xx_adc_iio_channels);
>  
>  	retval = devm_iio_device_register(&pdev->dev, iodev);

Re: [PATCH v2 4/4] dt-bindings: iio: adc: lpc32xx-adc: Document vref-supply

[Jonathan Cameron]

On Fri, 15 Mar 2019 10:52:31 +0100
Gregory CLEMENT <gregory.clement@bootlin.com> wrote:

> As most of the other ADC the lpc32xx one use a vref-supply property:
> document it.
> 
> Reviewed-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com>
Applied.

Thanks,

Jonathan

> ---
>  Documentation/devicetree/bindings/iio/adc/lpc32xx-adc.txt | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/lpc32xx-adc.txt b/Documentation/devicetree/bindings/iio/adc/lpc32xx-adc.txt
> index b3629d3a9adf..3a1bc669bd51 100644
> --- a/Documentation/devicetree/bindings/iio/adc/lpc32xx-adc.txt
> +++ b/Documentation/devicetree/bindings/iio/adc/lpc32xx-adc.txt
> @@ -6,6 +6,10 @@ Required properties:
>    region.
>  - interrupts: The ADC interrupt
>  
> +Optional:
> + - vref-supply: The regulator supply ADC reference voltage, optional
> +   for legacy reason, but highly encouraging to us in new device tree
> +
>  Example:
>  
>  	adc@40048000 {
> @@ -13,4 +17,5 @@ Example:
>  		reg = <0x40048000 0x1000>;
>  		interrupt-parent = <&mic>;
>  		interrupts = <39 0>;
> +		vref-supply = <&vcc>;
>  	};