Re: [PATCH v6 3/3] gpio: ad7768: Add AD7768 GPIO auxiliary driver

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

On Thu, Sep 03, 2026 at 02:01:28PM +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/bitmap.h>

Hmm... as far as I can see this is an overkill and bitops.h should suffice.

...

> +static int ad7768_gpio_init_valid_mask(struct gpio_chip *gc,
> +				       unsigned long *valid_mask,
> +				       unsigned int ngpios)
> +{
> +	if (ngpios > AD7768_FILTER_GPIO &&
> +	    device_property_match_string(gc->parent->parent, "clock-names",
> +					 "mclk") < 0)
> +		bitmap_clear(valid_mask, AD7768_FILTER_GPIO, 1);

This is a single bit, we have __clear_bit() for that in bitops.h.

> +	return 0;
> +}

...

> +static int ad7768_gpio_probe(struct auxiliary_device *adev,
> +			     const struct auxiliary_device_id *id)
> +{
> +	struct gpio_regmap_config config;
> +	struct device *dev = &adev->dev;

> +	struct device *parent;

Assign it here, this callback must not be called with dev == NULL, so it's fine
to dereference it without validation.

	struct device *parent = dev->parent;

> +	struct regmap *map;
> +	int ret;

> +	parent = dev->parent;

And drop this.

> +	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 = (struct gpio_regmap_config) {
> +		.parent = 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,
> +	};
> +
> +	return PTR_ERR_OR_ZERO(devm_gpio_regmap_register(dev, &config));
> +}

-- 
With Best Regards,
Andy Shevchenko