Re: [PATCH] pinctrl: renesas: rzg2l: Always call rzg2l_gpio_request() for interrupt pins

[Lad, Prabhakar]

Hi Claudiu,

On Wed, Oct 9, 2024 at 9:11 AM claudiu beznea <claudiu.beznea@xxxxxxxxx> wrote:
>
> Hi, Prabhakar,
>
> On 03.10.2024 16:16, Prabhakar wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
> >
> > Ensure that rzg2l_gpio_request() is called for GPIO pins configured as
> > interrupts, regardless of whether they are muxed in u-boot. This
> > guarantees that the pinctrl core is aware of the GPIO pin usage via
> > pinctrl_gpio_request(), which is invoked through rzg2l_gpio_request().
> >
> > Fixes: 2fd4fe19d0150 ("pinctrl: renesas: rzg2l: Configure interrupt input mode")
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
> > ---
> > Output before this patch on G2L/SMARC:
> > root@smarc-rzg2l:~# cat /sys/kernel/debug/pinctrl/11030000.pinctrl-pinctrl-rzg2l/pinmux-pins | grep P2_1
> > pin 17 (P2_1): UNCLAIMED
> >
> > Output after this patch G2L/SMARC:
> > root@smarc-rzg2l:~# cat /sys/kernel/debug/pinctrl/11030000.pinctrl-pinctrl-rzg2l/pinmux-pins | grep P2_1
> > pin 17 (P2_1): GPIO 11030000.pinctrl:529
> > ---
> >  drivers/pinctrl/renesas/pinctrl-rzg2l.c | 15 +++------------
> >  1 file changed, 3 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > index 60ef20ca3ccf..1dceaf8290ea 100644
> > --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > @@ -2368,20 +2368,11 @@ static const struct irq_chip rzg2l_gpio_irqchip = {
> >
> >  static int rzg2l_gpio_interrupt_input_mode(struct gpio_chip *chip, unsigned int offset)
> >  {
> > -     struct rzg2l_pinctrl *pctrl = gpiochip_get_data(chip);
> > -     const struct pinctrl_pin_desc *pin_desc = &pctrl->desc.pins[offset];
> > -     u64 *pin_data = pin_desc->drv_data;
> > -     u32 off = RZG2L_PIN_CFG_TO_PORT_OFFSET(*pin_data);
> > -     u8 bit = RZG2L_PIN_ID_TO_PIN(offset);
> > -     u8 reg8;
> >       int ret;
> >
> > -     reg8 = readb(pctrl->base + PMC(off));
> > -     if (reg8 & BIT(bit)) {
> > -             ret = rzg2l_gpio_request(chip, offset);
> > -             if (ret)
> > -                     return ret;
> > -     }
> > +     ret = rzg2l_gpio_request(chip, offset);
> > +     if (ret)
> > +             return ret;
> >
>
> With this approach I'm getting the following on RZ/G3S SMARC Carrier II board:
>
> [    0.368129] pinctrl-rzg2l 11030000.pinctrl: pinctrl-rzg2l support registered
> [    0.390426] 1004b800.serial: ttySC0 at MMIO 0x1004b800 (irq = 42,
> base_baud = 0) is a scif
> [    0.390558] printk: legacy console [ttySC0] enabled
> [    1.601991] pinctrl-rzg2l 11030000.pinctrl: pin P12_0 already requested
> by 11030000.pinctrl:608; cannot claim for 11030000.pinctrl:608
> [    1.614208] pinctrl-rzg2l 11030000.pinctrl: error -EINVAL: pin-96
> (11030000.pinctrl:608)
> [    1.622313] gpio gpiochip0: (11030000.pinctrl): can't look up hwirq 96
> [    1.631801] ravb 11c30000.ethernet eth0: Base address at 0x11c30000,
> d2:7b:7f:8f:d8:52, IRQ 46.
> [    1.645752] pinctrl-rzg2l 11030000.pinctrl: pin P12_1 already requested
> by 11030000.pinctrl:609; cannot claim for 11030000.pinctrl:609
> [    1.657923] pinctrl-rzg2l 11030000.pinctrl: error -EINVAL: pin-97
> (11030000.pinctrl:609)
> [    1.666035] gpio gpiochip0: (11030000.pinctrl): can't look up hwirq 97
> [    1.675573] ravb 11c40000.ethernet eth1: Base address at 0x11c40000,
> d2:7b:7f:8f:d8:52, IRQ 47.
> [    1.700907] pinctrl-rzg2l 11030000.pinctrl: pin P18_0 already requested
> by 11030000.pinctrl:656; cannot claim for 11030000.pinctrl:656
> [    1.713272] pinctrl-rzg2l 11030000.pinctrl: error -EINVAL: pin-144
> (11030000.pinctrl:656)
> [    1.721496] gpio gpiochip0: (11030000.pinctrl): can't look up hwirq 144
> [    1.729209] pinctrl-rzg2l 11030000.pinctrl: pin P0_1 already requested
> by 11030000.pinctrl:513; cannot claim for 11030000.pinctrl:513
> [    1.741345] pinctrl-rzg2l 11030000.pinctrl: error -EINVAL: pin-1
> (11030000.pinctrl:513)
> [    1.749432] gpio gpiochip0: (11030000.pinctrl): can't look up hwirq 1
> [    1.756285] pinctrl-rzg2l 11030000.pinctrl: pin P0_3 already requested
> by 11030000.pinctrl:515; cannot claim for 11030000.pinctrl:515
> [    1.768475] pinctrl-rzg2l 11030000.pinctrl: error -EINVAL: pin-3
> (11030000.pinctrl:515)
> [    1.776524] gpio gpiochip0: (11030000.pinctrl): can't look up hwirq 3
> [    1.783124] gpio-keys keys: Found button without gpio or irq
> [    1.788851] renesas_sdhi_internal_dmac 11c00000.mmc: mmc0 base at
> 0x0000000011c00000, max clock rate 125 MHz
> [    1.798791] gpio-keys keys: probe with driver gpio-keys failed with
> error -22
>
Thanks for the test, I noticed the same.
>
> All these ports are hogs to configure them as input. Removing the hog
> property make this patch work but I'm not sure this is the right approach
> (see below diff).
>
I have dropped a query [0] to GPIO maintainers to check on the correct approach.

https://lore.kernel.org/all/CA+V-a8vxUjTWccV-wLgy5CJiFYfEMsx-f+8weCJDP6uD_dh4AA@xxxxxxxxxxxxxx/

Cheers,
Prabhakar