Re: [PATCH v4 13/14] gpio: ad7768: Add AD7768 GPIO auxiliary driver

[Jonathan Cameron]

On Fri, 21 Aug 2026 16:07:06 +0200
Janani Sunil <janani.sunil@xxxxxxxxxx> wrote:

> The AD7768 provides five GPIOs controlled through registers shared
> with the parent IIO device. Register an auxiliary gpio-regmap driver
> and use the parent device for runtime PM.
> 
> The device has separate input-state and output-latch registers. Add a
> reg_mask_xlate() callback that checks the line direction and reads the
> programmed output latch for output lines while retaining input-state
> reads for input lines.
> 
> Signed-off-by: Janani Sunil <janani.sunil@xxxxxxxxxx>

Linus W, for I think that (when you are happy) you can pick up this
and the previous two enabling patches via the gpio tree and I'll take
the rest via IIO.  One small wrinkle around the maintainers entry that
we can fudge in various ways.

> ---
>  MAINTAINERS                |   1 +
>  drivers/gpio/Kconfig       |  12 +++++
>  drivers/gpio/Makefile      |   1 +
>  drivers/gpio/gpio-ad7768.c | 118 +++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 132 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index dc94e7803a7c..3de7ebcc4ee7 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1639,6 +1639,7 @@ L:	linux-iio@xxxxxxxxxxxxxxx
>  S:	Supported
>  W:	https://ez.analog.com/linux-software-drivers
>  F:	Documentation/devicetree/bindings/iio/adc/adi,ad7768.yaml
> +F:	drivers/gpio/gpio-ad7768.c

This may be the only reason we can't merge the gpio driver via
the gpio tree, independent of the rest.

Maybe drop this for now, or break it out as a separately patch that I can
pick up via IIO.

>  F:	drivers/iio/adc/ad7768.c
>  
>  ANALOG DEVICES INC AD7780 DRIVER

> diff --git a/drivers/gpio/gpio-ad7768.c b/drivers/gpio/gpio-ad7768.c
> new file mode 100644
> index 000000000000..710739e33dff
> --- /dev/null
> +++ b/drivers/gpio/gpio-ad7768.c

> +static int ad7768_gpio_probe(struct auxiliary_device *adev,
> +			     const struct auxiliary_device_id *id)
> +{
> +	struct device *parent = adev->dev.parent;
> +	struct gpio_regmap_config config = {
> +		.parent = &adev->dev,
> +		.label = dev_name(parent),
> +		.ngpio = AD7768_NUM_GPIOS,
> +		.reg_dat_base = AD7768_REG_GPIO_READ,
> +		.reg_set_base = AD7768_REG_GPIO_WRITE,
> +		.reg_dir_out_base = AD7768_REG_GPIO_CONTROL,
> +		.pm_dev = parent,
> +		.reg_mask_xlate = ad7768_gpio_reg_mask_xlate,
> +		.init_valid_mask = ad7768_gpio_init_valid_mask,
> +	};
> +	struct gpio_regmap *gpio;
> +	struct regmap *map;
> +	int ret;
> +
> +	map = dev_get_regmap(parent, NULL);
> +	if (!map)
> +		return -ENODEV;
> +
> +	PM_RUNTIME_ACQUIRE_AUTOSUSPEND(parent, pm);
> +	ret = PM_RUNTIME_ACQUIRE_ERR(&pm);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_set_bits(map, AD7768_REG_GPIO_CONTROL, AD7768_GPIO_UGPIO_ENABLE);
> +	if (ret)
> +		return ret;
> +
> +	config.regmap = map;
> +	config.drvdata = map;

Why not do all this in one place rather than init then later
modify?

	config = (struct gpio_regmap_config) {
		.parent = &adev->dev,
		.regmap = map,
		.label = dev_name(parent),
		.ngpio = AD7768_NUM_GPIOS,
		.reg_dat_base = AD7768_REG_GPIO_READ,
		.reg_set_base = AD7768_REG_GPIO_WRITE,
		.reg_dir_out_base = AD7768_REG_GPIO_CONTROL,
		.pm_dev = parent,
		.reg_mask_xlate = ad7768_gpio_reg_mask_xlate,
		.init_valid_mask = ad7768_gpio_init_valid_mask,
		.drvdata = map,
	};

> +	gpio = devm_gpio_regmap_register(&adev->dev, &config);
> +	return PTR_ERR_OR_ZERO(gpio);
Could do
	return PTR_ERR_OR_ZERO(devm_gpio_regmap_register(&adev->dev, &config));

Though maybe it looses some readability?

> +}