Re: [PATCH v3 2/2] Add GPIO support for Realtek DHC(Digital Home Center) RTD SoCs.

From: Michael Walle
Date: Thu Dec 14 2023 - 09:35:25 EST


Hi,

>> >> This driver enables configuration of GPIO direction, GPIO values,
>> >> GPIO debounce settings and handles GPIO interrupts.
>> >
>> >Why gpio-regmap can't be used?
>>
>> I will try to use gpio-remap in the next version.
>
>If it appears that it makes code uglier / complicated, please add the note
>somewhere to answer the above question.

I've traced the gpio-regmap.c file. It appears that for the driver to register
gpio_irq_chip, it must create the irq_domain and add it into gpio_regmap_config.
Additionally, the driver needs to register the irq handler by itself.
However, this process can be managed by the gpiolib if the driver fills in the struct
gpio_irq_chip inside struct gpio_chip before invoking gpiochip_add_data.

Hmm... I thought this is solvable issue.
Michael, is there a limitation in GPIO regmap that this driver can't be converted?

gpio-regmap is designed that regmap-irq (drivers/base/regmap/irq.c) can be
used. So, if regmap-irq fit this driver, then it can be used together with
gpio-regmap.

From a quick glance at the patch, it looks like the gpio portion might fit
gpio-regmap.

Moreover, apart from managing the registers for gpio direction and value, there
are several other registers that require access(interrupt enable, debounce...).
The GPIO IRQ status registers are located at different base addresses and are
not contiguous. It may need to create an additional regmap and assign the access
table to this regmap.

AFAIK this is not a problem as you can provide your own xlate function that
will take care about register mapping.

Just for the gpio part. IIRC regmap has it own translation (regmap fields).

With the above consideration, I tend to keep using the existing method.

I would like to hear from Michael if it's indeed a big obstacle.

So, regarding the irq portion, again, it must fit the regmap-irq. For the
additional requirement to set the debounce, you can add a .set_config to
gpio_regmap_config and supply your own set_config callback. See also [1].

-michael

[1] https://lore.kernel.org/linux-gpio/d4a6a640c373b6d939e147691efa596c@xxxxxxxx/