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

Andy Shevchenko <[email protected]>
Newsgroups org.kernel.vger.linux-doc,org.kernel.vger.linux-devicetree,org.kernel.vger.linux-gpio,org.kernel.vger.linux-iio,org.kernel.vger.linux-kernel
Organization Intel Finland Oy - BIC 0357606-4 - c/o Alberga Business Park, 6 krs, Bertel Jungin Aukio 5, 02600 Espoo
Message-ID <[email protected]>
On Sun, Aug 23, 2026 at 08:39:47PM +0100, Jonathan Cameron wrote:
> On Fri, 21 Aug 2026 16:07:06 +0200
> Janani Sunil <[email protected]> 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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.