linux-iio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] iio: ad7266: Convert to use GPIO descriptors
@ 2019-12-02  9:38 Linus Walleij
  2019-12-02 14:39 ` Ardelean, Alexandru
  0 siblings, 1 reply; 3+ messages in thread
From: Linus Walleij @ 2019-12-02  9:38 UTC (permalink / raw)
  To: Jonathan Cameron, linux-iio
  Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler,
	Linus Walleij, Alison Schofield

The AD7266 have no in-tree users making use of the platform
data mechanism to pass address GPIO lines when not using
a fixed address, so we can easily convert this to use
GPIO descriptors instead of the platform data integers
currently passed.

Lowercase the labels "ad0".."ad2" as this will make a better
fit for platform descriptions like device tree that prefer
lowercase names such as "ad0-gpios" rather than "AD0-gpios".

Board files and other static users of this device can pass
the same GPIO descriptors using machine descriptor
tables if need be.

Cc: Alison Schofield <amsfield22@gmail.com>
Cc: Lars-Peter Clausen <lars@metafoo.de>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/iio/adc/ad7266.c             | 29 ++++++++++++----------------
 include/linux/platform_data/ad7266.h |  3 ---
 2 files changed, 12 insertions(+), 20 deletions(-)

diff --git a/drivers/iio/adc/ad7266.c b/drivers/iio/adc/ad7266.c
index c31b8eabb894..c8524f098883 100644
--- a/drivers/iio/adc/ad7266.c
+++ b/drivers/iio/adc/ad7266.c
@@ -11,7 +11,7 @@
 #include <linux/spi/spi.h>
 #include <linux/regulator/consumer.h>
 #include <linux/err.h>
-#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
 #include <linux/module.h>
 
 #include <linux/interrupt.h>
@@ -34,7 +34,7 @@ struct ad7266_state {
 	enum ad7266_range	range;
 	enum ad7266_mode	mode;
 	bool			fixed_addr;
-	struct gpio		gpios[3];
+	struct gpio_desc	*gpios[3];
 
 	/*
 	 * DMA (thus cache coherency maintenance) requires the
@@ -117,7 +117,7 @@ static void ad7266_select_input(struct ad7266_state *st, unsigned int nr)
 	}
 
 	for (i = 0; i < 3; ++i)
-		gpio_set_value(st->gpios[i].gpio, (bool)(nr & BIT(i)));
+		gpiod_set_value(st->gpios[i], (bool)(nr & BIT(i)));
 }
 
 static int ad7266_update_scan_mode(struct iio_dev *indio_dev,
@@ -376,7 +376,7 @@ static void ad7266_init_channels(struct iio_dev *indio_dev)
 }
 
 static const char * const ad7266_gpio_labels[] = {
-	"AD0", "AD1", "AD2",
+	"ad0", "ad1", "ad2",
 };
 
 static int ad7266_probe(struct spi_device *spi)
@@ -419,14 +419,14 @@ static int ad7266_probe(struct spi_device *spi)
 
 		if (!st->fixed_addr) {
 			for (i = 0; i < ARRAY_SIZE(st->gpios); ++i) {
-				st->gpios[i].gpio = pdata->addr_gpios[i];
-				st->gpios[i].flags = GPIOF_OUT_INIT_LOW;
-				st->gpios[i].label = ad7266_gpio_labels[i];
+				st->gpios[i] = devm_gpiod_get(&spi->dev,
+						      ad7266_gpio_labels[i],
+						      GPIOD_OUT_LOW);
+				if (IS_ERR(st->gpios[i])) {
+					ret = PTR_ERR(st->gpios[i]);
+					goto error_disable_reg;
+				}
 			}
-			ret = gpio_request_array(st->gpios,
-				ARRAY_SIZE(st->gpios));
-			if (ret)
-				goto error_disable_reg;
 		}
 	} else {
 		st->fixed_addr = true;
@@ -465,7 +465,7 @@ static int ad7266_probe(struct spi_device *spi)
 	ret = iio_triggered_buffer_setup(indio_dev, &iio_pollfunc_store_time,
 		&ad7266_trigger_handler, &iio_triggered_buffer_setup_ops);
 	if (ret)
-		goto error_free_gpios;
+		goto error_disable_reg;
 
 	ret = iio_device_register(indio_dev);
 	if (ret)
@@ -475,9 +475,6 @@ static int ad7266_probe(struct spi_device *spi)
 
 error_buffer_cleanup:
 	iio_triggered_buffer_cleanup(indio_dev);
-error_free_gpios:
-	if (!st->fixed_addr)
-		gpio_free_array(st->gpios, ARRAY_SIZE(st->gpios));
 error_disable_reg:
 	if (!IS_ERR(st->reg))
 		regulator_disable(st->reg);
@@ -492,8 +489,6 @@ static int ad7266_remove(struct spi_device *spi)
 
 	iio_device_unregister(indio_dev);
 	iio_triggered_buffer_cleanup(indio_dev);
-	if (!st->fixed_addr)
-		gpio_free_array(st->gpios, ARRAY_SIZE(st->gpios));
 	if (!IS_ERR(st->reg))
 		regulator_disable(st->reg);
 
diff --git a/include/linux/platform_data/ad7266.h b/include/linux/platform_data/ad7266.h
index 7de6c16122df..f0652567afba 100644
--- a/include/linux/platform_data/ad7266.h
+++ b/include/linux/platform_data/ad7266.h
@@ -40,14 +40,11 @@ enum ad7266_mode {
  * @range: Reference voltage range the device is configured for
  * @mode: Sample mode the device is configured for
  * @fixed_addr: Whether the address pins are hard-wired
- * @addr_gpios: GPIOs used for controlling the address pins, only used if
- *		fixed_addr is set to false.
  */
 struct ad7266_platform_data {
 	enum ad7266_range range;
 	enum ad7266_mode mode;
 	bool fixed_addr;
-	unsigned int addr_gpios[3];
 };
 
 #endif
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] iio: ad7266: Convert to use GPIO descriptors
  2019-12-02  9:38 [PATCH] iio: ad7266: Convert to use GPIO descriptors Linus Walleij
@ 2019-12-02 14:39 ` Ardelean, Alexandru
  2019-12-07 10:15   ` Jonathan Cameron
  0 siblings, 1 reply; 3+ messages in thread
From: Ardelean, Alexandru @ 2019-12-02 14:39 UTC (permalink / raw)
  To: jic23, linux-iio, linus.walleij; +Cc: knaack.h, lars, pmeerw, amsfield22

On Mon, 2019-12-02 at 10:38 +0100, Linus Walleij wrote:
> The AD7266 have no in-tree users making use of the platform
> data mechanism to pass address GPIO lines when not using
> a fixed address, so we can easily convert this to use
> GPIO descriptors instead of the platform data integers
> currently passed.
> 
> Lowercase the labels "ad0".."ad2" as this will make a better
> fit for platform descriptions like device tree that prefer
> lowercase names such as "ad0-gpios" rather than "AD0-gpios".
> 
> Board files and other static users of this device can pass
> the same GPIO descriptors using machine descriptor
> tables if need be.
> 

Reviewed-by: Alexandru Ardelean <alexandru.ardelean@analog.com>

> Cc: Alison Schofield <amsfield22@gmail.com>
> Cc: Lars-Peter Clausen <lars@metafoo.de>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  drivers/iio/adc/ad7266.c             | 29 ++++++++++++----------------
>  include/linux/platform_data/ad7266.h |  3 ---
>  2 files changed, 12 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad7266.c b/drivers/iio/adc/ad7266.c
> index c31b8eabb894..c8524f098883 100644
> --- a/drivers/iio/adc/ad7266.c
> +++ b/drivers/iio/adc/ad7266.c
> @@ -11,7 +11,7 @@
>  #include <linux/spi/spi.h>
>  #include <linux/regulator/consumer.h>
>  #include <linux/err.h>
> -#include <linux/gpio.h>
> +#include <linux/gpio/consumer.h>
>  #include <linux/module.h>
>  
>  #include <linux/interrupt.h>
> @@ -34,7 +34,7 @@ struct ad7266_state {
>  	enum ad7266_range	range;
>  	enum ad7266_mode	mode;
>  	bool			fixed_addr;
> -	struct gpio		gpios[3];
> +	struct gpio_desc	*gpios[3];
>  
>  	/*
>  	 * DMA (thus cache coherency maintenance) requires the
> @@ -117,7 +117,7 @@ static void ad7266_select_input(struct ad7266_state
> *st, unsigned int nr)
>  	}
>  
>  	for (i = 0; i < 3; ++i)
> -		gpio_set_value(st->gpios[i].gpio, (bool)(nr & BIT(i)));
> +		gpiod_set_value(st->gpios[i], (bool)(nr & BIT(i)));
>  }
>  
>  static int ad7266_update_scan_mode(struct iio_dev *indio_dev,
> @@ -376,7 +376,7 @@ static void ad7266_init_channels(struct iio_dev
> *indio_dev)
>  }
>  
>  static const char * const ad7266_gpio_labels[] = {
> -	"AD0", "AD1", "AD2",
> +	"ad0", "ad1", "ad2",
>  };
>  
>  static int ad7266_probe(struct spi_device *spi)
> @@ -419,14 +419,14 @@ static int ad7266_probe(struct spi_device *spi)
>  
>  		if (!st->fixed_addr) {
>  			for (i = 0; i < ARRAY_SIZE(st->gpios); ++i) {
> -				st->gpios[i].gpio = pdata->addr_gpios[i];
> -				st->gpios[i].flags = GPIOF_OUT_INIT_LOW;
> -				st->gpios[i].label = ad7266_gpio_labels[i];
> +				st->gpios[i] = devm_gpiod_get(&spi->dev,
> +						      ad7266_gpio_labels[i]
> ,
> +						      GPIOD_OUT_LOW);
> +				if (IS_ERR(st->gpios[i])) {
> +					ret = PTR_ERR(st->gpios[i]);
> +					goto error_disable_reg;
> +				}
>  			}
> -			ret = gpio_request_array(st->gpios,
> -				ARRAY_SIZE(st->gpios));
> -			if (ret)
> -				goto error_disable_reg;
>  		}
>  	} else {
>  		st->fixed_addr = true;
> @@ -465,7 +465,7 @@ static int ad7266_probe(struct spi_device *spi)
>  	ret = iio_triggered_buffer_setup(indio_dev,
> &iio_pollfunc_store_time,
>  		&ad7266_trigger_handler, &iio_triggered_buffer_setup_ops);
>  	if (ret)
> -		goto error_free_gpios;
> +		goto error_disable_reg;
>  
>  	ret = iio_device_register(indio_dev);
>  	if (ret)
> @@ -475,9 +475,6 @@ static int ad7266_probe(struct spi_device *spi)
>  
>  error_buffer_cleanup:
>  	iio_triggered_buffer_cleanup(indio_dev);
> -error_free_gpios:
> -	if (!st->fixed_addr)
> -		gpio_free_array(st->gpios, ARRAY_SIZE(st->gpios));
>  error_disable_reg:
>  	if (!IS_ERR(st->reg))
>  		regulator_disable(st->reg);
> @@ -492,8 +489,6 @@ static int ad7266_remove(struct spi_device *spi)
>  
>  	iio_device_unregister(indio_dev);
>  	iio_triggered_buffer_cleanup(indio_dev);
> -	if (!st->fixed_addr)
> -		gpio_free_array(st->gpios, ARRAY_SIZE(st->gpios));
>  	if (!IS_ERR(st->reg))
>  		regulator_disable(st->reg);
>  
> diff --git a/include/linux/platform_data/ad7266.h
> b/include/linux/platform_data/ad7266.h
> index 7de6c16122df..f0652567afba 100644
> --- a/include/linux/platform_data/ad7266.h
> +++ b/include/linux/platform_data/ad7266.h
> @@ -40,14 +40,11 @@ enum ad7266_mode {
>   * @range: Reference voltage range the device is configured for
>   * @mode: Sample mode the device is configured for
>   * @fixed_addr: Whether the address pins are hard-wired
> - * @addr_gpios: GPIOs used for controlling the address pins, only used
> if
> - *		fixed_addr is set to false.
>   */
>  struct ad7266_platform_data {
>  	enum ad7266_range range;
>  	enum ad7266_mode mode;
>  	bool fixed_addr;
> -	unsigned int addr_gpios[3];
>  };
>  
>  #endif

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] iio: ad7266: Convert to use GPIO descriptors
  2019-12-02 14:39 ` Ardelean, Alexandru
@ 2019-12-07 10:15   ` Jonathan Cameron
  0 siblings, 0 replies; 3+ messages in thread
From: Jonathan Cameron @ 2019-12-07 10:15 UTC (permalink / raw)
  To: Ardelean, Alexandru
  Cc: linux-iio, linus.walleij, knaack.h, lars, pmeerw, amsfield22

On Mon, 2 Dec 2019 14:39:01 +0000
"Ardelean, Alexandru" <alexandru.Ardelean@analog.com> wrote:

> On Mon, 2019-12-02 at 10:38 +0100, Linus Walleij wrote:
> > The AD7266 have no in-tree users making use of the platform
> > data mechanism to pass address GPIO lines when not using
> > a fixed address, so we can easily convert this to use
> > GPIO descriptors instead of the platform data integers
> > currently passed.
> > 
> > Lowercase the labels "ad0".."ad2" as this will make a better
> > fit for platform descriptions like device tree that prefer
> > lowercase names such as "ad0-gpios" rather than "AD0-gpios".
> > 
> > Board files and other static users of this device can pass
> > the same GPIO descriptors using machine descriptor
> > tables if need be.
> >   
> 
> Reviewed-by: Alexandru Ardelean <alexandru.ardelean@analog.com>

This could do with some DT work when someone has time and consideration
of relaxing the fact these gpios are only available from platform data
at the moment (that had me confused for a while as I though this
would break DT).

Doesn't look like a big job to tidy this up.

Applied,

Thanks,

Jonathan


> 
> > Cc: Alison Schofield <amsfield22@gmail.com>
> > Cc: Lars-Peter Clausen <lars@metafoo.de>
> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> > ---
> >  drivers/iio/adc/ad7266.c             | 29 ++++++++++++----------------
> >  include/linux/platform_data/ad7266.h |  3 ---
> >  2 files changed, 12 insertions(+), 20 deletions(-)
> > 
> > diff --git a/drivers/iio/adc/ad7266.c b/drivers/iio/adc/ad7266.c
> > index c31b8eabb894..c8524f098883 100644
> > --- a/drivers/iio/adc/ad7266.c
> > +++ b/drivers/iio/adc/ad7266.c
> > @@ -11,7 +11,7 @@
> >  #include <linux/spi/spi.h>
> >  #include <linux/regulator/consumer.h>
> >  #include <linux/err.h>
> > -#include <linux/gpio.h>
> > +#include <linux/gpio/consumer.h>
> >  #include <linux/module.h>
> >  
> >  #include <linux/interrupt.h>
> > @@ -34,7 +34,7 @@ struct ad7266_state {
> >  	enum ad7266_range	range;
> >  	enum ad7266_mode	mode;
> >  	bool			fixed_addr;
> > -	struct gpio		gpios[3];
> > +	struct gpio_desc	*gpios[3];
> >  
> >  	/*
> >  	 * DMA (thus cache coherency maintenance) requires the
> > @@ -117,7 +117,7 @@ static void ad7266_select_input(struct ad7266_state
> > *st, unsigned int nr)
> >  	}
> >  
> >  	for (i = 0; i < 3; ++i)
> > -		gpio_set_value(st->gpios[i].gpio, (bool)(nr & BIT(i)));
> > +		gpiod_set_value(st->gpios[i], (bool)(nr & BIT(i)));
> >  }
> >  
> >  static int ad7266_update_scan_mode(struct iio_dev *indio_dev,
> > @@ -376,7 +376,7 @@ static void ad7266_init_channels(struct iio_dev
> > *indio_dev)
> >  }
> >  
> >  static const char * const ad7266_gpio_labels[] = {
> > -	"AD0", "AD1", "AD2",
> > +	"ad0", "ad1", "ad2",
> >  };
> >  
> >  static int ad7266_probe(struct spi_device *spi)
> > @@ -419,14 +419,14 @@ static int ad7266_probe(struct spi_device *spi)
> >  
> >  		if (!st->fixed_addr) {
> >  			for (i = 0; i < ARRAY_SIZE(st->gpios); ++i) {
> > -				st->gpios[i].gpio = pdata->addr_gpios[i];
> > -				st->gpios[i].flags = GPIOF_OUT_INIT_LOW;
> > -				st->gpios[i].label = ad7266_gpio_labels[i];
> > +				st->gpios[i] = devm_gpiod_get(&spi->dev,
> > +						      ad7266_gpio_labels[i]
> > ,
> > +						      GPIOD_OUT_LOW);
> > +				if (IS_ERR(st->gpios[i])) {
> > +					ret = PTR_ERR(st->gpios[i]);
> > +					goto error_disable_reg;
> > +				}
> >  			}
> > -			ret = gpio_request_array(st->gpios,
> > -				ARRAY_SIZE(st->gpios));
> > -			if (ret)
> > -				goto error_disable_reg;
> >  		}
> >  	} else {
> >  		st->fixed_addr = true;
> > @@ -465,7 +465,7 @@ static int ad7266_probe(struct spi_device *spi)
> >  	ret = iio_triggered_buffer_setup(indio_dev,
> > &iio_pollfunc_store_time,
> >  		&ad7266_trigger_handler, &iio_triggered_buffer_setup_ops);
> >  	if (ret)
> > -		goto error_free_gpios;
> > +		goto error_disable_reg;
> >  
> >  	ret = iio_device_register(indio_dev);
> >  	if (ret)
> > @@ -475,9 +475,6 @@ static int ad7266_probe(struct spi_device *spi)
> >  
> >  error_buffer_cleanup:
> >  	iio_triggered_buffer_cleanup(indio_dev);
> > -error_free_gpios:
> > -	if (!st->fixed_addr)
> > -		gpio_free_array(st->gpios, ARRAY_SIZE(st->gpios));
> >  error_disable_reg:
> >  	if (!IS_ERR(st->reg))
> >  		regulator_disable(st->reg);
> > @@ -492,8 +489,6 @@ static int ad7266_remove(struct spi_device *spi)
> >  
> >  	iio_device_unregister(indio_dev);
> >  	iio_triggered_buffer_cleanup(indio_dev);
> > -	if (!st->fixed_addr)
> > -		gpio_free_array(st->gpios, ARRAY_SIZE(st->gpios));
> >  	if (!IS_ERR(st->reg))
> >  		regulator_disable(st->reg);
> >  
> > diff --git a/include/linux/platform_data/ad7266.h
> > b/include/linux/platform_data/ad7266.h
> > index 7de6c16122df..f0652567afba 100644
> > --- a/include/linux/platform_data/ad7266.h
> > +++ b/include/linux/platform_data/ad7266.h
> > @@ -40,14 +40,11 @@ enum ad7266_mode {
> >   * @range: Reference voltage range the device is configured for
> >   * @mode: Sample mode the device is configured for
> >   * @fixed_addr: Whether the address pins are hard-wired
> > - * @addr_gpios: GPIOs used for controlling the address pins, only used
> > if
> > - *		fixed_addr is set to false.
> >   */
> >  struct ad7266_platform_data {
> >  	enum ad7266_range range;
> >  	enum ad7266_mode mode;
> >  	bool fixed_addr;
> > -	unsigned int addr_gpios[3];
> >  };
> >  
> >  #endif  


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2019-12-07 10:15 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-02  9:38 [PATCH] iio: ad7266: Convert to use GPIO descriptors Linus Walleij
2019-12-02 14:39 ` Ardelean, Alexandru
2019-12-07 10:15   ` Jonathan Cameron

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).