Re: [PATCH v7 08/15] gpiolib: regmap: add write_data_after_dir quirk

["Michael Walle" <michael@walle.cc>]

[-- Attachment #1: Type: text/plain, Size: 3724 bytes --]

On Tue Sep 15, 2026 at 1:15 PM CEST, Long Zhao via B4 Relay wrote:
> From: Long Zhao <longzhao@ambarella.com>
>
> Some controllers ignore data-register writes while a line is still an
> input. Optionally write the output value again after switching the
> direction, matching the existing PL061 behaviour.
>
> Signed-off-by: Long Zhao <longzhao@ambarella.com>
> ---
>  drivers/gpio/gpio-regmap.c  | 20 ++++++++++++++++++--
>  include/linux/gpio/regmap.h |  8 ++++++++
>  2 files changed, 26 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpio/gpio-regmap.c b/drivers/gpio/gpio-regmap.c
> index 51b4d69b8740..dfcc1f1122ec 100644
> --- a/drivers/gpio/gpio-regmap.c
> +++ b/drivers/gpio/gpio-regmap.c
> @@ -31,6 +31,7 @@ struct gpio_regmap {
>  	unsigned int reg_clr_base;
>  	unsigned int reg_dir_in_base;
>  	unsigned int reg_dir_out_base;
> +	bool write_data_after_dir;


Can we just have some quirk flags for these? First, this actually
describe what it is - a quirk - and in the future, we don't have to
copy all the flags individually in _register().

Also maybe QUIRK_SET_AFTER_DIR. The whole driver doesn't mention a
data register.

>  	unsigned long *fixed_direction_mask;
>  	unsigned long *fixed_direction_output;
>  
> @@ -271,9 +272,22 @@ static int gpio_regmap_direction_output(struct gpio_chip *chip,
>  			return ret;
>  	}
>  
> -	gpio_regmap_set(chip, offset, value);
> +	ret = gpio_regmap_set(chip, offset, value);
> +	if (ret)
> +		return ret;
> +
> +	ret = gpio_regmap_set_direction(chip, offset, true);
> +	if (ret)
> +		return ret;
>  
> -	return gpio_regmap_set_direction(chip, offset, true);
> +	/*
> +	 * gpio value is set again, because pl061 doesn't allow to set value of
> +	 * a gpio pin before configuring it in OUT mode.

Is this copied from the old code? Not sure, why pl061 have to be
referenced here.

-michael

> +	 */
> +	if (gpio->write_data_after_dir)
> +		return gpio_regmap_set(chip, offset, value);
> +
> +	return 0;
>  }
>  
>  void *gpio_regmap_get_drvdata(struct gpio_regmap *gpio)
> @@ -376,6 +390,8 @@ struct gpio_regmap *gpio_regmap_register(const struct gpio_regmap_config *config
>  			    config->fixed_direction_output, chip->ngpio);
>  	}
>  
> +	gpio->write_data_after_dir = config->write_data_after_dir;
> +
>  	/* if not set, assume there is only one register */
>  	gpio->ngpio_per_reg = config->ngpio_per_reg;
>  	if (!gpio->ngpio_per_reg)
> diff --git a/include/linux/gpio/regmap.h b/include/linux/gpio/regmap.h
> index 06255756710d..d7ffc12b00a2 100644
> --- a/include/linux/gpio/regmap.h
> +++ b/include/linux/gpio/regmap.h
> @@ -48,6 +48,13 @@ struct regmap;
>   *			(Optional) Bitmap representing the fixed direction of
>   *			the GPIO lines. Useful when there are GPIO lines with a
>   *			fixed direction mixed together in the same register.
> + * @write_data_after_dir:
> + *			(Optional) Write the output value again after
> + *			switching a line to output in ->direction_output().
> + *			Needed for hardware which ignores data register
> + *			writes while the line is configured as an input.
> + *			This is a legacy quirk (e.g. ARM PL061); new hardware
> + *			must not use it. Direction changes will glitch.
>   * @drvdata:		(Optional) Pointer to driver specific data which is
>   *			not used by gpio-remap but is provided "as is" to the
>   *			driver callback(s).
> @@ -94,6 +101,7 @@ struct gpio_regmap_config {
>  	unsigned int reg_dir_out_base;
>  	int reg_stride;
>  	int ngpio_per_reg;
> +	bool write_data_after_dir;
>  	struct irq_domain *irq_domain;
>  	unsigned long *fixed_direction_mask;
>  	unsigned long *fixed_direction_output;


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 297 bytes --]