Re:Re: [PATCH v6 08/13] gpio: regmap: support write_data_after_dir and girq

Hi Andy,

Thanks for the review.

Are you going to fix this HW in the next version of the SoC?

No — Ambarella does not use write_data_after_dir. It only preserves the
existing ARM PL061 quirk already documented in gpio-pl061 (data writes
ignored while the pin is still an input). Only pl061_arm sets the flag.

I will also make the first gpio_regmap_set() conditional so the quirk
path writes once after direction_output, not twice.

This needs to be in a separate update. Also we need to understand why
it is required.

OK, girq will be a separate patch. PL061 keeps a custom chained
irqchip (not regmap-irq); gpio_regmap today only takes irq_domain or
regmap_irq_chip, so we passed girq to keep the usual gpio_irq_chip +
gpiochip_add flow. Open to using a caller-created irq_domain instead if
you prefer.

Is pahole happy with the layout?

Will check and reorder if needed.

Best regards,
Long Zhao

At 2026-09-04 19:55:45, "Andy Shevchenko" [off-list ref] wrote:

On Fri, Sep 04, 2026 at 02:38:15PM +0800, Long Zhao via B4 Relay wrote:

Thanks for this change, my comments below.

quoted

Add an optional write-after-direction-output quirk for controllers
that ignore data-register writes while a line is still configured as
input,

The chips that require this are buggy. Are you going to fix this HW in the next
version of the SoC?

quoted

and allow drivers to pass an existing gpio_irq_chip through
gpio_regmap_register() so IRQ setup can stay with the caller.

This needs to be in a separate update. Also we need to understand why
it is required. The caller should be able to create the respective IRQ
domain which is important.

...

quoted

struct gpio_regmap {
 	unsigned int reg_dir_out_base;
 	unsigned long *fixed_direction_mask;
 	unsigned long *fixed_direction_output;
+	bool write_data_after_dir;

Is `pahole` happy with the layout?

quoted

 #ifdef CONFIG_REGMAP_IRQ
 	int regmap_irq_line;

...

quoted

static int gpio_regmap_direction_output(struct gpio_chip *chip,

quoted

 	gpio_regmap_set(chip, offset, value);

^^^ (see below)

quoted

 
-	return gpio_regmap_set_direction(chip, offset, true);
+	ret = gpio_regmap_set_direction(chip, offset, true);
+	if (ret)
+		return ret;
+
+	/* Some controllers ignore data writes while the line is still an input. */
+	if (gpio->write_data_after_dir)
+		gpio_regmap_set(chip, offset, value);

This can lead to two writes to the same register, the above needs to be
conditional as well.

-- 
With Best Regards,
Andy Shevchenko