Re: [PATCH v2 5/7] gpio: regmap: Add runtime PM and read_output_reg_set support

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

On Thu, Aug 06, 2026 at 05:41:24PM +0200, Janani Sunil wrote:
> The new pm_dev field in gpio_regmap_config allows a driver to supply a
> device for runtime PM. All operations call pm_runtime_resume_and_get()
> before accessing the regmap and pm_runtime_put_autosuspend() on return.
> 
> The new read_output_reg_set flag when set, gpio_regmap_get() checks the
> pin direction first and reads from reg_set_base instead of reg_dat_base
> for output pins. Requires both reg_dat_base and reg_set_base to be
> configured.

...

> struct gpio_regmap {

>  	unsigned int reg_clr_base;
>  	unsigned int reg_dir_in_base;
>  	unsigned int reg_dir_out_base;
> +	struct device *pm_dev;
> +	bool read_output_reg_set;
>  	unsigned long *fixed_direction_mask;
>  	unsigned long *fixed_direction_output;

>  }

Have you run `pahole`? Is it okay to put bool there and not after one of
unsigned int:s (just from the given context, it might be even better location)?

...

> +static int gpio_regmap_runtime_get(struct gpio_regmap *gpio)
> +{
> +	if (!gpio->pm_dev)
> +		return 0;
> +
> +	return pm_runtime_resume_and_get(gpio->pm_dev);
> +}
> +
> +static void gpio_regmap_runtime_put(struct gpio_regmap *gpio)
> +{

> +	if (gpio->pm_dev)
> +		pm_runtime_put_autosuspend(gpio->pm_dev);

Same pattern as per above.

> +}

...

> +static int gpio_regmap_get_direction(struct gpio_chip *chip,
> +				     unsigned int offset);

Can this be avoidable?

...

>  	/* ensure we don't spoil any register cache with pin input values */
>  	if (gpio->reg_dat_base == gpio->reg_set_base)
>  		ret = regmap_read_bypassed(gpio->regmap, reg, &val);
>  	else
>  		ret = regmap_read(gpio->regmap, reg, &val);
> -	if (ret)
> -		return ret;
> +	if (!ret)
> +		ret = !!(val & mask);
>  
> -	return !!(val & mask);

You want regmap_test_bits() in one case, and the regular pattern in the other.

> +out_pm:

labels should be marked better to show what will be when goto them.

out_pm_put:

> +	gpio_regmap_runtime_put(gpio);
> +	return ret;

...

> +out_pm:
> +	gpio_regmap_runtime_put(gpio);
>  	return ret;

Ditto.

...

>  	ret = regmap_read(gpio->regmap, reg, &val);
>  	if (ret)
> -		return ret;
> +		goto out_pm;
>  
>  	if (!!(val & mask) ^ invert)
> -		return GPIO_LINE_DIRECTION_OUT;
> +		ret = GPIO_LINE_DIRECTION_OUT;
>  	else
> -		return GPIO_LINE_DIRECTION_IN;
> +		ret = GPIO_LINE_DIRECTION_IN;

Also can be transformed to use regmap_test_bits() (but in a separate change).

...

> struct gpio_regmap_config {

>  	unsigned int reg_clr_base;
>  	unsigned int reg_dir_in_base;
>  	unsigned int reg_dir_out_base;
> +	struct device *pm_dev;
> +	bool read_output_reg_set;
>  	int reg_stride;
>  	int ngpio_per_reg;
>  	struct irq_domain *irq_domain;

`pahole`?

-- 
With Best Regards,
Andy Shevchenko