Re: [PATCH RESEND v3 12/17] iio: adc: ad7768-1: Add GPIO controller support

[Jonathan Cameron <jic23@kernel.org>]

On Wed, 12 Feb 2025 15:18:04 -0300
Jonathan Santos <Jonathan.Santos@analog.com> wrote:

> From: Sergiu Cuciurean <sergiu.cuciurean@analog.com>
> 
> The AD7768-1 has the ability to control other local hardware (such as gain
> stages),to power down other blocks in the signal chain, or read local
> status signals over the SPI interface.
> 
> This change exports the AD7768-1's four gpios and makes them accessible
> at an upper layer.
> 
> Signed-off-by: Sergiu Cuciurean <sergiu.cuciurean@analog.com>
> Co-developed-by: Jonathan Santos <Jonathan.Santos@analog.com>
> Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com>

Similar to previous.  +CC the gpio driver maintainers.
Whilst small part of this much larger driver, good to make sure
they have visibility and can choose whether to review or not.

+CC Linus, Bartosz and the linux-gpio list.
Probably also worth calling out in this patch description the reason we can't set
the GPIO state when in buffered mode as that won't be obvious to
anyone just reviewing the GPIO part of the driver.

> ---
> v3 Changes:
> * Fixed SoB order.
> * Added mising iio_device_release_direct_mode().
> * Simplified some regmap writes.
> * Removed ad7768_gpio_request() callback.
> * Fixed line wrapping.
> 
> v2 Changes:
> * Replaced mutex for iio_device_claim_direct_mode().
> * Use gpio-controller property to conditionally enable the
>   GPIO support.
> * OBS: when the GPIO is configured as output, we should read 
>   the current state value from AD7768_REG_GPIO_WRITE.
> ---
>  drivers/iio/adc/ad7768-1.c | 143 ++++++++++++++++++++++++++++++++++++-
>  1 file changed, 141 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad7768-1.c b/drivers/iio/adc/ad7768-1.c
> index a359cd8fceca..afcc8678cf83 100644
> --- a/drivers/iio/adc/ad7768-1.c
> +++ b/drivers/iio/adc/ad7768-1.c
> @@ -9,6 +9,8 @@
>  #include <linux/delay.h>
>  #include <linux/device.h>
>  #include <linux/err.h>
> +#include <linux/gpio.h>
> +#include <linux/gpio/driver.h>
>  #include <linux/gpio/consumer.h>
>  #include <linux/kernel.h>
>  #include <linux/module.h>
> @@ -86,6 +88,16 @@
>  #define AD7768_REG_ANALOG2_VCM_MSK	GENMASK(2, 0)
>  #define AD7768_REG_ANALOG2_VCM(x)	FIELD_PREP(AD7768_REG_ANALOG2_VCM_MSK, x)
>  
> +/* AD7768_REG_GPIO_CONTROL */
> +#define AD7768_GPIO_UNIVERSAL_EN	BIT(7)
> +#define AD7768_GPIO_CONTROL_MSK		GENMASK(3, 0)
> +
> +/* AD7768_REG_GPIO_WRITE */
> +#define AD7768_GPIO_WRITE_MSK		GENMASK(3, 0)
> +
> +/* AD7768_REG_GPIO_READ */
> +#define AD7768_GPIO_READ_MSK		GENMASK(3, 0)
> +
>  #define AD7768_RD_FLAG_MSK(x)		(BIT(6) | ((x) & 0x3F))
>  #define AD7768_WR_FLAG_MSK(x)		((x) & 0x3F)
>  
> @@ -170,6 +182,7 @@ struct ad7768_state {
>  	struct regulator *vref;
>  	struct regulator_dev *vcm_rdev;
>  	struct clk *mclk;
> +	struct gpio_chip gpiochip;
>  	unsigned int mclk_freq;
>  	unsigned int samp_freq;
>  	struct completion completion;
> @@ -349,6 +362,124 @@ static int ad7768_set_dig_fil(struct ad7768_state *st,
>  	return 0;
>  }
>  
> +static int ad7768_gpio_direction_input(struct gpio_chip *chip, unsigned int offset)
> +{
> +	struct iio_dev *indio_dev = gpiochip_get_data(chip);
> +	struct ad7768_state *st = iio_priv(indio_dev);
> +	int ret;
> +
> +	ret = iio_device_claim_direct_mode(indio_dev);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_clear_bits(st->regmap, AD7768_REG_GPIO_CONTROL,
> +				BIT(offset));
> +	iio_device_release_direct_mode(indio_dev);
> +
> +	return ret;
> +}
> +
> +static int ad7768_gpio_direction_output(struct gpio_chip *chip,
> +					unsigned int offset, int value)
> +{
> +	struct iio_dev *indio_dev = gpiochip_get_data(chip);
> +	struct ad7768_state *st = iio_priv(indio_dev);
> +	int ret;
> +
> +	ret = iio_device_claim_direct_mode(indio_dev);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_set_bits(st->regmap, AD7768_REG_GPIO_CONTROL,
> +			      BIT(offset));
> +	iio_device_release_direct_mode(indio_dev);
> +
> +	return ret;
> +}
> +
> +static int ad7768_gpio_get(struct gpio_chip *chip, unsigned int offset)
> +{
> +	struct iio_dev *indio_dev = gpiochip_get_data(chip);
> +	struct ad7768_state *st = iio_priv(indio_dev);
> +	unsigned int val;
> +	int ret;
> +
> +	ret = iio_device_claim_direct_mode(indio_dev);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_read(st->regmap, AD7768_REG_GPIO_CONTROL, &val);
> +	if (ret)
> +		goto err_release;
> +
> +	/*
> +	 * If the GPIO is configured as an output, read the current value from
> +	 * AD7768_REG_GPIO_WRITE. Otherwise, read the input value from
> +	 * AD7768_REG_GPIO_READ.
> +	 */
> +	if (val & BIT(offset))
> +		ret = regmap_read(st->regmap, AD7768_REG_GPIO_WRITE, &val);
> +	else
> +		ret = regmap_read(st->regmap, AD7768_REG_GPIO_READ, &val);
> +	if (ret)
> +		goto err_release;
> +
> +	ret = !!(val & BIT(offset));
> +err_release:
> +	iio_device_release_direct_mode(indio_dev);
> +
> +	return ret;
> +}
> +
> +static void ad7768_gpio_set(struct gpio_chip *chip, unsigned int offset, int value)
> +{
> +	struct iio_dev *indio_dev = gpiochip_get_data(chip);
> +	struct ad7768_state *st = iio_priv(indio_dev);
> +	unsigned int val;
> +	int ret;
> +
> +	ret = iio_device_claim_direct_mode(indio_dev);
> +	if (ret)
> +		return;
> +
> +	ret = regmap_read(st->regmap, AD7768_REG_GPIO_CONTROL, &val);
> +	if (ret)
> +		goto err_release;
> +
> +	if (val & BIT(offset))
> +		regmap_update_bits(st->regmap, AD7768_REG_GPIO_WRITE,
> +				   BIT(offset), value << offset);
> +
> +err_release:
> +	iio_device_release_direct_mode(indio_dev);
> +}
> +
> +static int ad7768_gpio_init(struct iio_dev *indio_dev)
> +{
> +	struct ad7768_state *st = iio_priv(indio_dev);
> +	int ret;
> +
> +	ret = regmap_write(st->regmap, AD7768_REG_GPIO_CONTROL,
> +			   AD7768_GPIO_UNIVERSAL_EN);
> +	if (ret)
> +		return ret;
> +
> +	st->gpiochip = (struct gpio_chip) {
> +		.label = "ad7768_1_gpios",
> +		.base = -1,
> +		.ngpio = 4,
> +		.parent = &st->spi->dev,
> +		.can_sleep = true,
> +		.direction_input = ad7768_gpio_direction_input,
> +		.direction_output = ad7768_gpio_direction_output,
> +		.get = ad7768_gpio_get,
> +		.set = ad7768_gpio_set,
> +		.owner = THIS_MODULE,
> +	};
> +
> +	return gpiochip_add_data(&st->gpiochip, indio_dev);
> +}
> +
>  static int ad7768_set_freq(struct ad7768_state *st,
>  			   unsigned int freq)
>  {
> @@ -492,8 +623,9 @@ static const struct iio_info ad7768_info = {
>  	.debugfs_reg_access = &ad7768_reg_access,
>  };
>  
> -static int ad7768_setup(struct ad7768_state *st)
> +static int ad7768_setup(struct iio_dev *indio_dev)
>  {
> +	struct ad7768_state *st = iio_priv(indio_dev);
>  	int ret;
>  
>  	st->gpio_reset = devm_gpiod_get_optional(&st->spi->dev, "reset",
> @@ -526,6 +658,13 @@ static int ad7768_setup(struct ad7768_state *st)
>  	if (IS_ERR(st->gpio_sync_in))
>  		return PTR_ERR(st->gpio_sync_in);
>  
> +	/* Only create a Chip GPIO if flagged for it */
> +	if (device_property_read_bool(&st->spi->dev, "gpio-controller")) {
> +		ret = ad7768_gpio_init(indio_dev);
> +		if (ret < 0)
> +			return ret;
> +	}
> +
>  	/* Set the default sampling frequency to 32000 kSPS */
>  	return ad7768_set_freq(st, 32000);
>  }
> @@ -845,7 +984,7 @@ static int ad7768_probe(struct spi_device *spi)
>  	if (ret)
>  		return ret;
>  
> -	ret = ad7768_setup(st);
> +	ret = ad7768_setup(indio_dev);
>  	if (ret < 0) {
>  		dev_err(&spi->dev, "AD7768 setup failed\n");
>  		return ret;