RE: [PATCH v2 3/4] gpio: realtek: Add driver for Realtek DHC RTD1625 SoC

Yu-Chun Lin [林祐君]
2026-04-30T09:59:47Z

Hi Linus and Michael,

> Hi,
>
> On Sun Apr 19, 2026 at 11:19 PM CEST, Linus Walleij wrote:
> > Hi Yu-Chun,
> >
> > On Fri, Apr 10, 2026 at 11:39 AM Yu-Chun Lin [林祐君]
> > <eleanor.lin@realtek.com> wrote:
> >
> >> We did look into gpio-mmio and gpio-regmap, but they are not quite suitable for
> >> our platform due to the specific hardware design:
> >>
> >> 1. Per-GPIO Dedicated Registers: Unlike typical GPIO controllers that pack 32 pins
> >> into a single 32-bit register (1 bit per pin), our hardware uses a dedicated 32-bit
> >> register for each individual GPIO. This single register controls the
> >> input/output state, direction, and interrupt trigger type for that specific pin.
> >
> > Isn't that attainable by:
> >
> > - setting .ngpio_per_reg to 1 in struct gpio_regmap_config
>
> Which is just used by the gpio_regmap_simple_xlate() anyway. So it
> doesn't really matter. But yeah, 1 would be the correct value here,
> assuming that the registers are consecutive.
>
> > - extend .reg_mask_xlate callback with an enum for each operation
> > (need to change all users of the .reg_mask_xlate callback but
> > who cares, they are not many):
> >
> > e.g.
> >
> > enum gpio_regmap_operation {
> > GPIO_REGMAP_GET_OP,
> > GPIO_REGMAP_SET_OP,
> > GPIO_REGMAP_SET_WITH_CLEAR_OP,
> > GPIO_REGMAP_GET_DIR_OP,
> > GPIO_REGMAP_SET_DIR_OP,
> > };
> >
> > int (*reg_mask_xlate)(struct gpio_regmap *gpio,
> > enum_gpio_regmap_operation op,
> > unsigned int base,
> > unsigned int offset, unsigned int *reg,
> > unsigned int *mask);
> >
> > This way .reg_mask_xlate() can hit different bits in the returned
> > *mask depending on operation and it will be find to pack all of
> > the bits into one 32bit register.
> >
> > Added Michael Walle to the the thread, he will know if this is a
> > good idea.
>
> Nice idea, though the information is then redundant in the usual
> case, i.e. drivers which need to translate specific registers
> will do a "switch (base)" at the moment. These should be converted
> to "switch (op)" just to keep all the drivers aligned and prevent
> new drivers from using the old method. You'd need to touch them
> anyway.
>
> I was briefly thinking about making it somewhat possible to embed
> the op into the base, if it would otherwise be all the same. That
> way, you could gpio-regmap as is. A special case like
> GPIO_REGMAP_ADDR_ZERO, that could be used by these kind of drivers,
> but that is probably too hacky.
>
> I'm fine with either way.
>
> >> 2. Write-Enable (WREN) Mask Mechanism: Our hardware requires a specific Write-Enable
> >> mask to be written simultaneously when updating the register values.
> >
> > Which is to just set bit 31.
> >
> > With the above scheme your .reg_mask_xlate callback can just set bit 31
> > no matter what operating you're doing. Piece of cake.
>
> Keep in mind, that this will make reading and writing somewhat
> different. reading assumes there is only one bit set in mask,
> because of the "!!(val & mask)" op, which is hardcoded. I'm not
> against using the write like that though.
>
> -michael
>

I understand the proposed approach for extending '.reg_mask_xlate' with
'enum gpio_regmap_operation', and I have started implementing it.

> >> 3. Hardware Debounce: We also need to support hardware debounce settings per pin,
> >> which requires custom configuration via set_config mapped to these specific per-pin
> >> registers.
> >
> > Just add a version of an optional .set_config() call to gpio-regmap.c
> > to handle this using .reg_mask_xlate() per above and add a new
> > GPIO_REGMAP_CONFIG_OP to the above enum, problem solved.
> >
> > If it seems too hard I can write patch 1 & 2 adding this infrastructure
> > but I bet you can easily see what can be done with gpio-regmap.c
> > here provided Michael W approves the idea.
> >

Our .set_config requires mapping specific debounce time values to
hardware-specific enums and applying a Write-Enable bit.

Would it be better to allow drivers to assign a custom '.set_config' callback
directly within 'struct gpio_regmap_config'?

Additionally, I didn't mention this in my previous email. we also need to
implement GPIO interrupts. Our hardware design does not fit well with
'regmap_irq_chip'. Therefore, I am planning to create our own 'irqdomain' and
'irqchip' ops to handle our cascaded interrupts.

Because of this custom IRQ implementation, I would need to use 'readl()/writel()'
for the IRQ callbacks in the rtd1625 GPIO driver instead of the regmap APIs.
Do you have any suggestions on how to handle this gracefully while keeping it
aligned with the regmap infrastructure?

Best Regards,
Yu-Chun

> > Yours,
> > Linus Walleij