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

[Andy Shevchenko <andriy.shevchenko@intel.com>]

On Thu, Aug 13, 2026 at 03:57: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.

...

> +#include <linux/auxiliary_bus.h>
> +#include <linux/bits.h>
> +#include <linux/cleanup.h>

Is it used?

> +#include <linux/device.h>
> +#include <linux/err.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 = dev->parent;

> +	struct gpio_regmap_config config = {
> +		.parent = dev,
> +		.label = dev_name(dev->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 = dev->parent,

parent

> +		.reg_mask_xlate = ad7768_gpio_reg_mask_xlate,
> +	};
> +	struct gpio_regmap *gpio;
> +	struct regmap *regmap;

Name it map, it will shorten some lines (see below).

> +	int ret;
> +
> +	regmap = dev_get_regmap(dev->parent, NULL);

parent

> +	if (!regmap)
> +		return -ENODEV;
> +
> +	PM_RUNTIME_ACQUIRE_IF_ENABLED_AUTOSUSPEND(dev->parent, pm);

parent

> +	ret = PM_RUNTIME_ACQUIRE_ERR(&pm);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_set_bits(regmap, AD7768_REG_GPIO_CONTROL,
> +			      AD7768_GPIO_UGPIO_ENABLE);

I would dare to put it on a single line

	ret = regmap_set_bits(map, AD7768_REG_GPIO_CONTROL, AD7768_GPIO_UGPIO_ENABLE);

(out of only 86 characters).

> +	if (ret)
> +		return ret;
> +
> +	config.regmap = regmap;
> +	config.drvdata = regmap;

> +	gpio = devm_gpio_regmap_register(dev, &config);
> +
> +	return PTR_ERR_OR_ZERO(gpio);

I would swap blank line as the below is tightly coupled.

	config.regmap = regmap;
	config.drvdata = regmap;

	gpio = devm_gpio_regmap_register(dev, &config);
	return PTR_ERR_OR_ZERO(gpio);

> +}

-- 
With Best Regards,
Andy Shevchenko