Re: [PATCH v9 07/15] gpiolib: regmap: add GPIO_REGMAP_QUIRK_SET_AFTER_DIR

From: Andy Shevchenko

Date: Fri Oct 02 2026 - 03:49:56 EST


On Thu, Oct 01, 2026 at 08:40:56PM +0800, Long Zhao via B4 Relay wrote:

> Some controllers ignore output writes while a line is still an input.
> Add a behaviour flag so the output value is written after the direction
> change. This is a legacy quirk. New hardware must not use it.

A nit-pick below.
In general looks good to me.
Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>

...

> struct gpio_regmap {

> unsigned int reg_clr_base;
> unsigned int reg_dir_in_base;
> unsigned int reg_dir_out_base;
> + unsigned long quirks;

I think I have told at some point that this location is not the best, I woul put it...

> unsigned long *fixed_direction_mask;
> unsigned long *fixed_direction_output;

...here, after the group of the direction related offsets and masks.

> - gpio_regmap_set(chip, offset, value);
> + if (!(gpio->quirks & GPIO_REGMAP_QUIRK_SET_AFTER_DIR))
> + gpio_regmap_set(chip, offset, value);
>
> - return gpio_regmap_set_direction(chip, offset, true);
> + ret = gpio_regmap_set_direction(chip, offset, true);
> + if (ret)
> + return ret;
> +
> + if (gpio->quirks & GPIO_REGMAP_QUIRK_SET_AFTER_DIR)
> + gpio_regmap_set(chip, offset, value);
> +
> + return 0;
> }

...

> /**
> * struct gpio_regmap_config - Description of a generic regmap gpio_chip.
> * @parent: The parent device

> * @ngpio: (Optional) Number of GPIOs
> * @names: (Optional) Array of names for gpios
> + * @quirks: (Optional) Behaviour flags, OR of GPIO_REGMAP_QUIRK_*.
> * @reg_dat_base: (Optional) (in) register base address
> * @reg_set_base: (Optional) set register base address
> * @reg_clr_base: (Optional) clear register base address

> int ngpio;
> const char *const *names;
>
> + /* Regmap GPIO behaviour flags */
> + unsigned long quirks;
> +
> unsigned int reg_dat_base;
> unsigned int reg_set_base;
> unsigned int reg_clr_base;

I would follow the same location as it's in the above struct, id est after

unsigned long *fixed_direction_output;

member.


--
With Best Regards,
Andy Shevchenko