Re: [PATCH v3 3/7] gpio: regmap: Add gpio_regmap_operation and write-enable support

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

On Tue, May 12, 2026 at 11:33:13AM +0800, Yu-Chun Lin wrote:
> Extend the reg_mask_xlate callback with an operation type parameter
> (gpio_regmap_operation) to allow drivers to return different
> register/mask combinations for different GPIO operations.
> 
> Also add write-enable mechanism for hardware that requires setting a
> write-enable bit before modifying GPIO control registers.
> 
> Consequently, update all existing drivers utilizing the gpio-regmap
> framework (across drivers/gpio, drivers/iio, and drivers/pinctrl)
> to accommodate the new reg_mask_xlate function signature.

Dunno if we want per-driver patches (in that case it will be a new name and
callback, conversion per driver, removal old name, and, if required, renaming
back). In any case looks reasonable change.

...

> -static int idi_48_reg_mask_xlate(struct gpio_regmap *gpio, unsigned int base,
> -				 unsigned int offset, unsigned int *reg,
> -				 unsigned int *mask)
> +static int idi_48_reg_mask_xlate(struct gpio_regmap *gpio,
> +				 enum gpio_regmap_operation op,
> +				 unsigned int base, unsigned int offset,
> +				 unsigned int *reg, unsigned int *mask)

In every case, use this logical split.

...

> -static int i8255_reg_mask_xlate(struct gpio_regmap *gpio, unsigned int base,
> -				unsigned int offset, unsigned int *reg,
> +static int i8255_reg_mask_xlate(struct gpio_regmap *gpio, enum gpio_regmap_operation op,
> +				unsigned int base, unsigned int offset, unsigned int *reg,
>  				unsigned int *mask)

Exempli gratia, this one looks illogical, harder to read.

...

> +	ret = gpio->reg_mask_xlate(gpio, GPIO_REGMAP_SET_DIR_WREN_OP, base, offset, &reg,
> +				   &wren_mask);

Ditto. Easier to follow when

	ret = gpio->reg_mask_xlate(gpio, GPIO_REGMAP_SET_DIR_WREN_OP, base, offset,
				   &reg, &wren_mask);

>  	if (ret)
>  		return ret;

...

> +/**
> + * enum gpio_regmap_operation - Operation type for reg_mask_xlate callback
> + *
> + * This enum is used to distinguish between different types of GPIO operations
> + * so that the reg_mask_xlate callback can return the appropriate mask for each
> + * operation type.
> + *
> + * Value operations:

Have you checked the rendered text (HTML, PDF)? I believe this will look awfully wrong.

> + * @GPIO_REGMAP_GET_OP: Mask for reading direction to detect if GPIO is input or output.
> + *                      Used in gpio_regmap_get() to determine the GPIO direction.
> + * @GPIO_REGMAP_IN: Mask for reading input value. Used when GPIO is configured as input.
> + * @GPIO_REGMAP_OUT: Mask for reading output value. Used when GPIO is configured as output.
> + *
> + * Output operations:
> + * @GPIO_REGMAP_SET_OP: Mask for setting GPIO output value.
> + * @GPIO_REGMAP_SET_WITH_CLEAR_OP: Mask for setting/clearing GPIO using separate registers.
> + * @GPIO_REGMAP_SET_WREN_OP: Write-enable mask for output operations. May be used to enable
> + *                           writes to protected registers.
> + *
> + * Direction operations:
> + * @GPIO_REGMAP_GET_DIR_OP: Mask for reading GPIO direction (input/output).
> + * @GPIO_REGMAP_SET_DIR_OP: Mask for setting GPIO direction (input/output).
> + * @GPIO_REGMAP_SET_DIR_WREN_OP: Write-enable mask for direction operations. May be used to
> + *                               enable writes to protected direction registers.
> + */

-- 
With Best Regards,
Andy Shevchenko