Re: [PATCH v4 13/14] gpio: ad7768: Add AD7768 GPIO auxiliary driver
Jonathan Cameron <[email protected]>
| Newsgroups | org.kernel.vger.linux-iio,org.kernel.vger.linux-devicetree,org.kernel.vger.linux-doc,org.kernel.vger.linux-gpio,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <20260823203947.3ecdc448@jic23-huawei> |
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. > > Signed-off-by: Janani Sunil <[email protected]> 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: [email protected] > 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? > +}