Re: [PATCH v4 13/14] gpio: ad7768: Add AD7768 GPIO auxiliary driver
From: Andy Shevchenko
Date: Mon Aug 24 2026 - 04:14:29 EST
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@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.
...
> > +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