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

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 <[email protected]>

...

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