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

[Andy Shevchenko <andriy.shevchenko@intel.com>]

On Sun, Aug 23, 2026 at 08:39:47PM +0100, Jonathan Cameron wrote:
> On Fri, 21 Aug 2026 16:07:06 +0200
> Janani Sunil <janani.sunil@analog.com> 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.

...

> > +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?

See above the answer :-)

...

> > +	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?

Fine with me. Taking into account my previous suggestion (in another reply)

	return PTR_ERR_OR_ZERO(devm_gpio_regmap_register(dev, &config));

which bumps readability back to track a bit.

> > +}

-- 
With Best Regards,
Andy Shevchenko