Re: [PATCH] iio: ad7266: Convert to use GPIO descriptors

[Jonathan Cameron <jic23@kernel.org>]

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