Re: [PATCH 5/6] gpio: ad7768: Add AD7768 GPIO auxiliary driver

[Jonathan Cameron <jic23@kernel.org>]

On Thu, 9 Jul 2026 10:50:16 +0200
Janani Sunil <janani.sunil@analog.com> wrote:

> The AD7768/AD7768-4 ADC exposes 5 general-purpose I/O pins that can be
> independently configured as inputs or outputs. Add an auxiliary bus driver
> to expose these pins as a GPIO chip, registered by the parent IIO driver.
> 
> The driver uses the parent's regmap for register access and delegates
> runtime power management to the parent device.
> 
> Signed-off-by: Janani Sunil <janani.sunil@analog.com>
A few things inline.

> diff --git a/drivers/gpio/gpio-ad7768.c b/drivers/gpio/gpio-ad7768.c
> new file mode 100644
> index 000000000000..c2f01b1abd7c
> --- /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 *dev = &adev->dev;
> +	const char *label = dev_get_platdata(dev);
> +	struct ad7768_gpio_state *st;
> +	struct gpio_chip *gc;
> +	int ret;
> +
> +	st = devm_kzalloc(dev, sizeof(*st), GFP_KERNEL);
> +	if (!st)
> +		return -ENOMEM;
> +
> +	st->parent = dev->parent;
> +	st->regmap = dev_get_regmap(dev->parent, NULL);
> +
> +	ret = devm_mutex_init(dev, &st->lock);
> +	if (ret)
> +		return ret;
> +
> +	ret = pm_runtime_resume_and_get(st->parent);
I'd use the ACQUIRE stuff you used else where and not worry about
maybe keeping power on a tiny bit long.


> +	if (ret < 0)
> +		return ret;
> +
> +	ret = regmap_update_bits(st->regmap, AD7768_REG_GPIO_CONTROL,
> +				 AD7768_GPIO_UGPIO_ENABLE,
> +				 AD7768_GPIO_UGPIO_ENABLE);

set_bits()

> +
> +	pm_runtime_mark_last_busy(st->parent);

As before this is wrong.

> +	pm_runtime_put_autosuspend(st->parent);
> +
> +	if (ret < 0)
> +		return ret;
> +
> +	gc = &st->gc;
> +	gc->label = label;
> +	gc->base = -1;
> +	gc->ngpio = AD7768_NUM_GPIOS;
> +	gc->parent = dev;
> +	gc->owner = THIS_MODULE;
> +	gc->can_sleep = true;
> +	gc->get_direction = ad7768_gpio_get_direction;
> +	gc->direction_input = ad7768_gpio_direction_input;
> +	gc->direction_output = ad7768_gpio_direction_output;
> +	gc->get = ad7768_gpio_get;
> +	gc->set = ad7768_gpio_set;
> +
> +	return devm_gpiochip_add_data(dev, &st->gc, st);
use gc given you have it.

> +}

>