Re: [PATCH v4 13/14] gpio: ad7768: Add AD7768 GPIO auxiliary driver
From: Andy Shevchenko
Date: Mon Aug 24 2026 - 04:19:44 EST
On Fri, Aug 21, 2026 at 04:07:06PM +0200, Janani Sunil 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.
Nice driver!
See a few nit-picks below and after addressing them
Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxx>
...
> +#include <linux/auxiliary_bus.h>
> +#include <linux/bitmap.h>
> +#include <linux/bits.h>
bitmap.h covers bitops.h that covers bits.h.
So bitmap.h is superior of those.
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/gpio/driver.h>
> +#include <linux/gpio/regmap.h>
> +#include <linux/module.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/regmap.h>
...
> +static int ad7768_gpio_probe(struct auxiliary_device *adev,
> + const struct auxiliary_device_id *id)
> +{
struct device *dev = &adev->dev;
> + struct device *parent = adev->dev.parent;
struct device *parent = dev->parent;
> + struct gpio_regmap_config config = {
> + .parent = &adev->dev,
.parent = 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;
> +
> + gpio = devm_gpio_regmap_register(&adev->dev, &config);
gpio = devm_gpio_regmap_register(dev, &config);
> + return PTR_ERR_OR_ZERO(gpio);
> +}
--
With Best Regards,
Andy Shevchenko