Re: [PATCH v8 08/18] gpiolib: regmap: add write_data_after_dir quirk

[Linus Walleij]

Hi Long,

this is starting to look good!

On Mon, Sep 21, 2026 at 6:23 AM Long Zhao via B4 Relay
<devnull+longzhao.ambarella.com@xxxxxxxxxx> wrote:

> From: Long Zhao <longzhao@xxxxxxxxxxxxx>
>
> 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@xxxxxxxxxxxxx>
(...)

> +++ 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;

1. You have put the kerneldoc in a completely different place in the order
of thing than where you put the flag in the struct.

2. Put a comment above the behaviour flags because there will
invariably be more of them! Something like:
/* Regmap GPIO behaviour flags */

3. Put a newline before & after the flags so they end up in a separate
section of the struct, so people know where to add them.

I would put them:

        struct device *parent;
        struct regmap *regmap;
        struct fwnode_handle *fwnode;

        const char *label;
        int ngpio;
        const char *const *names;

HERE

        unsigned int reg_dat_base;
        unsigned int reg_set_base;

Yours,
Linus Walleij