[PATCH 0/7] pinctrl: rzg2l: Fix releasing of IRQ and status reported in pinmux-pins

[PATCH 0/7] pinctrl: rzg2l: Fix releasing of IRQ and status reported in pinmux-pins

[Prabhakar]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Hi All,

This patch series aims to fix the reporting of pin status in the
`pinmux-pins` file and properly free up the IRQ line when using a GPIO 
pin as an interrupt via the `interrupts`/`interrupts-extended` property.

Testing on SMARC RZ/G2L:
------------------------
	keys {
		compatible = "gpio-keys";

		key-1 {
			interrupt-parent = <&pinctrl>;
			gpios = <&pinctrl RZG2L_GPIO(41, 0) GPIO_ACTIVE_HIGH>;
			linux,code = <KEY_1>;
			label = "USER_SW1";
			wakeup-source;
			debounce-interval = <20>;
		};

		key-3 {
			interrupts-extended = <&pinctrl RZG2L_GPIO(43, 1) IRQ_TYPE_EDGE_RISING>;
			linux,code = <KEY_3>;
			label = "USER_SW3";
			wakeup-source;
			debounce-interval = <20>;
		};
	};

Before this patch series, the pin status was incorrectly reported in the
`pinmux-pins` file, and the IRQ was not released. As seen below, `P43_1` is
reported as `UNCLAIMED`, and after unloading the `gpio_keys` module, the IRQ
is not released, unlike `P41_0`, which was used via the `gpios` property.
---------------------------------------------------------------------------------
root@smarc-rzg2l:~# cat /proc/interrupts | grep pinctrl
 78:          1          0 11030000.pinctrl  17 Edge      1-003d
 91:          0          0 11030000.pinctrl 345 Edge      USER_SW3
 92:          0          0 11030000.pinctrl 328 Edge      USER_SW1
root@smarc-rzg2l:~# cat /sys/kernel/debug/gpio | grep 41_0
 gpio-840 (P41_0               |USER_SW1            ) in  lo IRQ
root@smarc-rzg2l:~# cat /sys/kernel/debug/gpio | grep 43_1
 gpio-857 (P43_1               |interrupt           ) in  lo IRQ
root@smarc-rzg2l:~# cat /sys/kernel/debug/pinctrl/11030000.pinctrl-pinctrl-rzg2l/pinmux-pins | grep 43_1
pin 345 (P43_1): UNCLAIMED
root@smarc-rzg2l:~# cat /sys/kernel/debug/pinctrl/11030000.pinctrl-pinctrl-rzg2l/pinmux-pins | grep 41_0
pin 328 (P41_0): GPIO 11030000.pinctrl:840
root@smarc-rzg2l:~#
root@smarc-rzg2l:~# ls /sys/kernel/debug/irq/irqs/91
/sys/kernel/debug/irq/irqs/91
root@smarc-rzg2l:~# ls /sys/kernel/debug/irq/irqs/92
/sys/kernel/debug/irq/irqs/92
root@smarc-rzg2l:~#
root@smarc-rzg2l:~# lsmod
Module                  Size  Used by
gpio_keys              20480  0
root@smarc-rzg2l:~#
root@smarc-rzg2l:~# modprobe -r gpio_keys
root@smarc-rzg2l:~#
root@smarc-rzg2l:~# cat /proc/interrupts | grep pinctrl
 78:          1          0 11030000.pinctrl  17 Edge      1-003d
root@smarc-rzg2l:~# cat /sys/kernel/debug/gpio | grep 41_0
 gpio-840 (P41_0               )
root@smarc-rzg2l:~# cat /sys/kernel/debug/gpio | grep 43_1
 gpio-857 (P43_1               )
root@smarc-rzg2l:~#
root@smarc-rzg2l:~# cat /sys/kernel/debug/pinctrl/11030000.pinctrl-pinctrl-rzg2l/pinmux-pins | grep 41_0
pin 328 (P41_0): UNCLAIMED
root@smarc-rzg2l:~# cat /sys/kernel/debug/pinctrl/11030000.pinctrl-pinctrl-rzg2l/pinmux-pins | grep 43_1
pin 345 (P43_1): UNCLAIMED
root@smarc-rzg2l:~#
root@smarc-rzg2l:~# ls /sys/kernel/debug/irq/irqs/91
/sys/kernel/debug/irq/irqs/91
root@smarc-rzg2l:~# ls /sys/kernel/debug/irq/irqs/92
ls: /sys/kernel/debug/irq/irqs/92: No such file or directory
root@smarc-rzg2l:~#
root@smarc-rzg2l:~#


After applying this patch series, the pin status is correctly reported in
the `pinmux-pins` file, and the IRQ is properly released. As shown below,
the status for pin `P43_1` is now reported correctly, and the IRQ is now
released, unlike in the previous logs above.
---------------------------------------------------------------------------
root@smarc-rzg2l:~# cat /proc/interrupts | grep pinctrl
 78:          1          0 11030000.pinctrl  17 Edge      1-003d
 91:          0          0 11030000.pinctrl 345 Edge      USER_SW3
 92:          0          0 11030000.pinctrl 328 Edge      USER_SW1
root@smarc-rzg2l:~# cat /sys/kernel/debug/gpio | grep 41_0
 gpio-840 (P41_0               |USER_SW1            ) in  lo IRQ
root@smarc-rzg2l:~# cat /sys/kernel/debug/gpio | grep 43_1
 gpio-857 (P43_1               |interrupt           ) in  lo IRQ
root@smarc-rzg2l:~# cat /sys/kernel/debug/pinctrl/11030000.pinctrl-pinctrl-rzg2l/pinmux-pins | grep P43_1
pin 345 (P43_1): GPIO 11030000.pinctrl:857
root@smarc-rzg2l:~# cat /sys/kernel/debug/pinctrl/11030000.pinctrl-pinctrl-rzg2l/pinmux-pins | grep P41_0
pin 328 (P41_0): GPIO 11030000.pinctrl:840
root@smarc-rzg2l:~#
root@smarc-rzg2l:~# ls /sys/kernel/debug/irq/irqs/91
/sys/kernel/debug/irq/irqs/91
root@smarc-rzg2l:~# ls /sys/kernel/debug/irq/irqs/92
/sys/kernel/debug/irq/irqs/92
root@smarc-rzg2l:~# lsmod
Module                  Size  Used by
gpio_keys              20480  0
root@smarc-rzg2l:~# modprobe -r gpio_keys
root@smarc-rzg2l:~# cat /proc/interrupts | grep pinctrl
 78:          1          0 11030000.pinctrl  17 Edge      1-003d
root@smarc-rzg2l:~#
root@smarc-rzg2l:~# cat /sys/kernel/debug/gpio | grep P43_1
 gpio-857 (P43_1               )
root@smarc-rzg2l:~# cat /sys/kernel/debug/gpio | grep P41_0
 gpio-840 (P41_0               )
root@smarc-rzg2l:~# cat /sys/kernel/debug/pinctrl/11030000.pinctrl-pinctrl-rzg2l/pinmux-pins | grep P43_1
pin 345 (P43_1): UNCLAIMED
root@smarc-rzg2l:~# cat /sys/kernel/debug/pinctrl/11030000.pinctrl-pinctrl-rzg2l/pinmux-pins | grep P41_0
pin 328 (P41_0): UNCLAIMED
root@smarc-rzg2l:~# ls /sys/kernel/debug/irq/irqs/91
ls: /sys/kernel/debug/irq/irqs/91: No such file or directory
root@smarc-rzg2l:~# ls /sys/kernel/debug/irq/irqs/92
ls: /sys/kernel/debug/irq/irqs/92: No such file or directory
root@smarc-rzg2l:~#

Note:
- On RZ/G3S SMARC, prior to this patch series, we hogged the GPIO pins as
  inputs before using them as GPIO interrupts via the
  `interrupts`/`interrupts-extended` property. These patches do not break
  older DTBs, i.e., functionality remains the same. However, after
  unloading the `gpio_keys` module, the `pinmux-pins` file in sysfs
  reports the pin as UNCLAIMED.
  
Cheers,
Prabhakar

Lad Prabhakar (7):
  arm64: dts: renesas: rzg3s-smarc: Drop hogging of GPIO pins
  pinctrl: renesas: rzg2l: Use gpiochip_populate_parent_fwspec_twocell
    helper
  pinctrl: renesas: rzg2l: Release all the virq's in free callback
  pinctrl: renesas: rzg2l: Reorder function call in rzg2l_gpio_free()
  pinctrl: renesas: rzg2l: Drop calling rzg2l_gpio_request()
  pinctrl: pinmux: Introduce API to check if a pin is requested
  pinctrl: renesas: pinctrl-rzg2l: Override
    irq_request/release_resources

 .../boot/dts/renesas/rzg3s-smarc-som.dtsi     |  18 ---
 arch/arm64/boot/dts/renesas/rzg3s-smarc.dtsi  |  21 ---
 drivers/pinctrl/pinmux.c                      |  14 ++
 drivers/pinctrl/pinmux.h                      |   5 +
 drivers/pinctrl/renesas/pinctrl-rzg2l.c       | 128 ++++++++----------
 5 files changed, 79 insertions(+), 107 deletions(-)

-- 
2.43.0

[PATCH 1/7] arm64: dts: renesas: rzg3s-smarc: Drop hogging of GPIO pins

[Prabhakar]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Hogging of gpio pins used as interrupts is not required, as the rzg2l
pinctrl driver sets the direction as input when the pin is requested
as GPIO interrupt.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 .../boot/dts/renesas/rzg3s-smarc-som.dtsi     | 18 ----------------
 arch/arm64/boot/dts/renesas/rzg3s-smarc.dtsi  | 21 -------------------
 2 files changed, 39 deletions(-)

diff --git a/arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi b/arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi
index 71424e69939e..6e95933cd7ef 100644
--- a/arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi
+++ b/arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi
@@ -196,15 +196,6 @@ &sdhi2 {
 #endif
 
 &pinctrl {
-#if SW_CONFIG3 == SW_ON
-	eth0-phy-irq-hog {
-		gpio-hog;
-		gpios = <RZG2L_GPIO(12, 0) GPIO_ACTIVE_LOW>;
-		input;
-		line-name = "eth0-phy-irq";
-	};
-#endif
-
 	eth0_pins: eth0 {
 		txc {
 			pinmux = <RZG2L_PORT_PINMUX(1, 0, 1)>;  /* ET0_TXC */
@@ -239,15 +230,6 @@ mux {
 		};
 	};
 
-#if SW_CONFIG3 == SW_ON
-	eth1-phy-irq-hog {
-		gpio-hog;
-		gpios = <RZG2L_GPIO(12, 1) GPIO_ACTIVE_LOW>;
-		input;
-		line-name = "eth1-phy-irq";
-	};
-#endif
-
 	eth1_pins: eth1 {
 		txc {
 			pinmux = <RZG2L_PORT_PINMUX(7, 0, 1)>;	/* ET1_TXC */
diff --git a/arch/arm64/boot/dts/renesas/rzg3s-smarc.dtsi b/arch/arm64/boot/dts/renesas/rzg3s-smarc.dtsi
index 4509151344c4..baeca5ff5d61 100644
--- a/arch/arm64/boot/dts/renesas/rzg3s-smarc.dtsi
+++ b/arch/arm64/boot/dts/renesas/rzg3s-smarc.dtsi
@@ -71,27 +71,6 @@ &i2c0 {
 };
 
 &pinctrl {
-	key-1-gpio-hog {
-		gpio-hog;
-		gpios = <RZG2L_GPIO(18, 0) GPIO_ACTIVE_LOW>;
-		input;
-		line-name = "key-1-gpio-irq";
-	};
-
-	key-2-gpio-hog {
-		gpio-hog;
-		gpios = <RZG2L_GPIO(0, 1) GPIO_ACTIVE_LOW>;
-		input;
-		line-name = "key-2-gpio-irq";
-	};
-
-	key-3-gpio-hog {
-		gpio-hog;
-		gpios = <RZG2L_GPIO(0, 3) GPIO_ACTIVE_LOW>;
-		input;
-		line-name = "key-3-gpio-irq";
-	};
-
 	scif0_pins: scif0 {
 		pinmux = <RZG2L_PORT_PINMUX(6, 3, 1)>, /* RXD */
 			 <RZG2L_PORT_PINMUX(6, 4, 1)>; /* TXD */
-- 
2.43.0

[PATCH 2/7] pinctrl: renesas: rzg2l: Use gpiochip_populate_parent_fwspec_twocell helper

[Prabhakar]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 drivers/pinctrl/renesas/pinctrl-rzg2l.c | 17 +----------------
 1 file changed, 1 insertion(+), 16 deletions(-)

diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
index 5eefbace7d91..8ffb9430a134 100644
--- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
+++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
@@ -2433,21 +2433,6 @@ static int rzg2l_gpio_child_to_parent_hwirq(struct gpio_chip *gc,
 	return ret;
 }
 
-static int rzg2l_gpio_populate_parent_fwspec(struct gpio_chip *chip,
-					     union gpio_irq_fwspec *gfwspec,
-					     unsigned int parent_hwirq,
-					     unsigned int parent_type)
-{
-	struct irq_fwspec *fwspec = &gfwspec->fwspec;
-
-	fwspec->fwnode = chip->irq.parent_domain->fwnode;
-	fwspec->param_count = 2;
-	fwspec->param[0] = parent_hwirq;
-	fwspec->param[1] = parent_type;
-
-	return 0;
-}
-
 static void rzg2l_gpio_irq_restore(struct rzg2l_pinctrl *pctrl)
 {
 	struct irq_domain *domain = pctrl->gpio_chip.irq.domain;
@@ -2649,7 +2634,7 @@ static int rzg2l_gpio_register(struct rzg2l_pinctrl *pctrl)
 	girq->fwnode = dev_fwnode(pctrl->dev);
 	girq->parent_domain = parent_domain;
 	girq->child_to_parent_hwirq = rzg2l_gpio_child_to_parent_hwirq;
-	girq->populate_parent_alloc_arg = rzg2l_gpio_populate_parent_fwspec;
+	girq->populate_parent_alloc_arg = gpiochip_populate_parent_fwspec_twocell;
 	girq->child_irq_domain_ops.free = rzg2l_gpio_irq_domain_free;
 	girq->init_valid_mask = rzg2l_init_irq_valid_mask;
 
-- 
2.43.0

Re: [PATCH 2/7] pinctrl: renesas: rzg2l: Use gpiochip_populate_parent_fwspec_twocell helper

[Geert Uytterhoeven]

On Thu, Oct 17, 2024 at 1:39 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

"Because rzg2l_gpio_populate_parent_fwspec() and
 gpiochip_populate_parent_fwspec_twocell() are identical."

> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
i.e. will queue in renesas-pinctrl for v6.13.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 2/7] pinctrl: renesas: rzg2l: Use gpiochip_populate_parent_fwspec_twocell helper

[Lad, Prabhakar]

Hi Geert,

Thank you for the review.

On Thu, Oct 17, 2024 at 2:04 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> On Thu, Oct 17, 2024 at 1:39 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> "Because rzg2l_gpio_populate_parent_fwspec() and
>  gpiochip_populate_parent_fwspec_twocell() are identical."
>
Ouch, thanks for taking care of it.

Cheers,
Prabhakar

> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> i.e. will queue in renesas-pinctrl for v6.13.
>
> Gr{oetje,eeting}s,
>
>                         Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds

[PATCH 3/7] pinctrl: renesas: rzg2l: Release all the virq's in free callback

[Prabhakar]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

The `free` callback in `struct irq_domain_ops` expects to free
nr_irqs interrupts starting from virq.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 drivers/pinctrl/renesas/pinctrl-rzg2l.c | 39 ++++++++++++++-----------
 1 file changed, 22 insertions(+), 17 deletions(-)

diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
index 8ffb9430a134..1a09ccbfebad 100644
--- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
+++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
@@ -2477,25 +2477,30 @@ static void rzg2l_gpio_irq_restore(struct rzg2l_pinctrl *pctrl)
 static void rzg2l_gpio_irq_domain_free(struct irq_domain *domain, unsigned int virq,
 				       unsigned int nr_irqs)
 {
+	struct rzg2l_pinctrl *pctrl;
+	irq_hw_number_t hwirq;
+	struct gpio_chip *gc;
+	unsigned long flags;
 	struct irq_data *d;
+	unsigned int i, j;
 
-	d = irq_domain_get_irq_data(domain, virq);
-	if (d) {
-		struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
-		struct rzg2l_pinctrl *pctrl = container_of(gc, struct rzg2l_pinctrl, gpio_chip);
-		irq_hw_number_t hwirq = irqd_to_hwirq(d);
-		unsigned long flags;
-		unsigned int i;
-
-		for (i = 0; i < RZG2L_TINT_MAX_INTERRUPT; i++) {
-			if (pctrl->hwirq[i] == hwirq) {
-				rzg2l_gpio_irq_endisable(pctrl, hwirq, false);
-				rzg2l_gpio_free(gc, hwirq);
-				spin_lock_irqsave(&pctrl->bitmap_lock, flags);
-				bitmap_release_region(pctrl->tint_slot, i, get_order(1));
-				spin_unlock_irqrestore(&pctrl->bitmap_lock, flags);
-				pctrl->hwirq[i] = 0;
-				break;
+	for (j = 0; j < nr_irqs; j++) {
+		d = irq_domain_get_irq_data(domain, virq + j);
+		if (d) {
+			gc = irq_data_get_irq_chip_data(d);
+			pctrl = container_of(gc, struct rzg2l_pinctrl, gpio_chip);
+			hwirq = irqd_to_hwirq(d);
+
+			for (i = 0; i < RZG2L_TINT_MAX_INTERRUPT; i++) {
+				if (pctrl->hwirq[i] == hwirq) {
+					rzg2l_gpio_irq_endisable(pctrl, hwirq, false);
+					rzg2l_gpio_free(gc, hwirq);
+					spin_lock_irqsave(&pctrl->bitmap_lock, flags);
+					bitmap_release_region(pctrl->tint_slot, i, get_order(1));
+					spin_unlock_irqrestore(&pctrl->bitmap_lock, flags);
+					pctrl->hwirq[i] = 0;
+					break;
+				}
 			}
 		}
 	}
-- 
2.43.0

[PATCH 4/7] pinctrl: renesas: rzg2l: Reorder function call in rzg2l_gpio_free()

[Prabhakar]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

The order of function calls in the gpio free operation should be the
reverse of that in the gpio request operation. Thus move
pinctrl_gpio_free() call at the very end in rzg2l_gpio_free()

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 drivers/pinctrl/renesas/pinctrl-rzg2l.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
index 1a09ccbfebad..d8b942fbf537 100644
--- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
+++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
@@ -1774,8 +1774,6 @@ static void rzg2l_gpio_free(struct gpio_chip *chip, unsigned int offset)
 {
 	unsigned int virq;
 
-	pinctrl_gpio_free(chip, offset);
-
 	virq = irq_find_mapping(chip->irq.domain, offset);
 	if (virq)
 		irq_dispose_mapping(virq);
@@ -1785,6 +1783,8 @@ static void rzg2l_gpio_free(struct gpio_chip *chip, unsigned int offset)
 	 * drive the GPIO pin as an output.
 	 */
 	rzg2l_gpio_direction_input(chip, offset);
+
+	pinctrl_gpio_free(chip, offset);
 }
 
 static const char * const rzg2l_gpio_names[] = {
-- 
2.43.0

[PATCH 5/7] pinctrl: renesas: rzg2l: Drop calling rzg2l_gpio_request()

[Prabhakar]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Drop calling rzg2l_gpio_request() in rzg2l_gpio_interrupt_input_mode()
this was added to handle special case of bootloader setting
the same gpio pin as function.

When GPIO pin is requested as interrupt through `gpios` DT property
the gpio_request() is called through the code path and when releasing
GPIO pin it goes through the gpio_free() path, so drop calling
gpio_request() in rzg2l_gpio_child_to_parent_hwirq() path and also
drop rzg2l_gpio_free() in rzg2l_gpio_irq_domain_free(). This fixes
case where rzg2l_gpio_free() was being called twice after the GPIO
interrupt pin is freed (after unbinding the module).

When GPIO pin is requested as interrupt through `interrupt` DT property
this doesn't go through gpio_request()/gpio_free() code path.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 drivers/pinctrl/renesas/pinctrl-rzg2l.c | 33 +++----------------------
 1 file changed, 3 insertions(+), 30 deletions(-)

diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
index d8b942fbf537..b9a8bf43a92a 100644
--- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
+++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
@@ -2372,26 +2372,6 @@ static const struct irq_chip rzg2l_gpio_irqchip = {
 	GPIOCHIP_IRQ_RESOURCE_HELPERS,
 };
 
-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;
-	}
-
-	return rzg2l_gpio_direction_input(chip, offset);
-}
-
 static int rzg2l_gpio_child_to_parent_hwirq(struct gpio_chip *gc,
 					    unsigned int child,
 					    unsigned int child_type,
@@ -2407,17 +2387,15 @@ static int rzg2l_gpio_child_to_parent_hwirq(struct gpio_chip *gc,
 	if (gpioint < 0)
 		return gpioint;
 
-	ret = rzg2l_gpio_interrupt_input_mode(gc, child);
+	ret = rzg2l_gpio_direction_input(gc, child);
 	if (ret)
 		return ret;
 
 	spin_lock_irqsave(&pctrl->bitmap_lock, flags);
 	irq = bitmap_find_free_region(pctrl->tint_slot, RZG2L_TINT_MAX_INTERRUPT, get_order(1));
 	spin_unlock_irqrestore(&pctrl->bitmap_lock, flags);
-	if (irq < 0) {
-		ret = -ENOSPC;
-		goto err;
-	}
+	if (irq < 0)
+		return -ENOSPC;
 
 	rzg2l_gpio_irq_endisable(pctrl, child, true);
 	pctrl->hwirq[irq] = child;
@@ -2427,10 +2405,6 @@ static int rzg2l_gpio_child_to_parent_hwirq(struct gpio_chip *gc,
 	*parent_type = IRQ_TYPE_LEVEL_HIGH;
 	*parent = RZG2L_PACK_HWIRQ(gpioint, irq);
 	return 0;
-
-err:
-	rzg2l_gpio_free(gc, child);
-	return ret;
 }
 
 static void rzg2l_gpio_irq_restore(struct rzg2l_pinctrl *pctrl)
@@ -2494,7 +2468,6 @@ static void rzg2l_gpio_irq_domain_free(struct irq_domain *domain, unsigned int v
 			for (i = 0; i < RZG2L_TINT_MAX_INTERRUPT; i++) {
 				if (pctrl->hwirq[i] == hwirq) {
 					rzg2l_gpio_irq_endisable(pctrl, hwirq, false);
-					rzg2l_gpio_free(gc, hwirq);
 					spin_lock_irqsave(&pctrl->bitmap_lock, flags);
 					bitmap_release_region(pctrl->tint_slot, i, get_order(1));
 					spin_unlock_irqrestore(&pctrl->bitmap_lock, flags);
-- 
2.43.0

[PATCH 6/7] pinctrl: pinmux: Introduce API to check if a pin is requested

[Prabhakar]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Introduce `pin_requested` API to check if a pin is currently requested.
This API allows pinctrl drivers to verify whether a pin is requested or
not by checking if the pin is owned by either `gpio_owner` or `mux_owner`.

GPIO pins used as interrupts through the `interrupts` DT property do not
follow the usual `gpio_request`/`pin_request` path, unlike GPIO pins used
as interrupts via the `gpios` property. As a result, such pins were
reported as `UNCLAIMED` in the `pinmux-pins` sysfs file, even though they
were in use as interrupts.

With the newly introduced API, pinctrl drivers can check if a pin is
already requested by the pinctrl core and ensure that pin is requested
during when using as irq. This helps to ensure that the `pinmux-pins`
sysfs file reflects the correct status of the pin.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 drivers/pinctrl/pinmux.c | 14 ++++++++++++++
 drivers/pinctrl/pinmux.h |  5 +++++
 2 files changed, 19 insertions(+)

diff --git a/drivers/pinctrl/pinmux.c b/drivers/pinctrl/pinmux.c
index 02033ea1c643..6c3d18b162ad 100644
--- a/drivers/pinctrl/pinmux.c
+++ b/drivers/pinctrl/pinmux.c
@@ -99,6 +99,20 @@ bool pinmux_can_be_used_for_gpio(struct pinctrl_dev *pctldev, unsigned int pin)
 	return !(ops->strict && !!desc->gpio_owner);
 }
 
+bool pin_requested(struct pinctrl_dev *pctldev, int pin)
+{
+	struct pin_desc *desc;
+
+	desc = pin_desc_get(pctldev, pin);
+	if (!desc)
+		return false;
+
+	if (!desc->gpio_owner && !desc->mux_owner)
+		return false;
+
+	return true;
+}
+
 /**
  * pin_request() - request a single pin to be muxed in, typically for GPIO
  * @pctldev: the associated pin controller device
diff --git a/drivers/pinctrl/pinmux.h b/drivers/pinctrl/pinmux.h
index 2965ec20b77f..6d854eebaad4 100644
--- a/drivers/pinctrl/pinmux.h
+++ b/drivers/pinctrl/pinmux.h
@@ -42,6 +42,7 @@ int pinmux_map_to_setting(const struct pinctrl_map *map,
 void pinmux_free_setting(const struct pinctrl_setting *setting);
 int pinmux_enable_setting(const struct pinctrl_setting *setting);
 void pinmux_disable_setting(const struct pinctrl_setting *setting);
+bool pin_requested(struct pinctrl_dev *pctldev, int pin);
 
 #else
 
@@ -100,6 +101,10 @@ static inline void pinmux_disable_setting(const struct pinctrl_setting *setting)
 {
 }
 
+bool pin_requested(struct pinctrl_dev *pctldev, int pin)
+{
+	return false;
+}
 #endif
 
 #if defined(CONFIG_PINMUX) && defined(CONFIG_DEBUG_FS)
-- 
2.43.0

Re: [PATCH 6/7] pinctrl: pinmux: Introduce API to check if a pin is requested

[Linus Walleij]

On Thu, Oct 17, 2024 at 1:39 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:

> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Introduce `pin_requested` API to check if a pin is currently requested.
> This API allows pinctrl drivers to verify whether a pin is requested or
> not by checking if the pin is owned by either `gpio_owner` or `mux_owner`.
>
> GPIO pins used as interrupts through the `interrupts` DT property do not
> follow the usual `gpio_request`/`pin_request` path, unlike GPIO pins used
> as interrupts via the `gpios` property. As a result, such pins were
> reported as `UNCLAIMED` in the `pinmux-pins` sysfs file, even though they
> were in use as interrupts.
>
> With the newly introduced API, pinctrl drivers can check if a pin is
> already requested by the pinctrl core and ensure that pin is requested
> during when using as irq. This helps to ensure that the `pinmux-pins`
> sysfs file reflects the correct status of the pin.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Provided that Geert applies 7/7:
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCH 6/7] pinctrl: pinmux: Introduce API to check if a pin is requested

[kernel test robot]

Hi Prabhakar,

kernel test robot noticed the following build warnings:

[auto build test WARNING on geert-renesas-drivers/renesas-pinctrl]
[also build test WARNING on linusw-pinctrl/devel linusw-pinctrl/for-next geert-renesas-devel/next linus/master v6.12-rc3 next-20241018]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Prabhakar/arm64-dts-renesas-rzg3s-smarc-Drop-hogging-of-GPIO-pins/20241017-194200
base:   https://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git renesas-pinctrl
patch link:    https://lore.kernel.org/r/20241017113942.139712-7-prabhakar.mahadev-lad.rj%40bp.renesas.com
patch subject: [PATCH 6/7] pinctrl: pinmux: Introduce API to check if a pin is requested
config: arm-pxa3xx_defconfig (https://download.01.org/0day-ci/archive/20241019/202410190448.yDDAKyxt-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 13.3.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241019/202410190448.yDDAKyxt-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410190448.yDDAKyxt-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from drivers/pinctrl/core.c:38:
>> drivers/pinctrl/pinmux.h:104:6: warning: no previous prototype for 'pin_requested' [-Wmissing-prototypes]
     104 | bool pin_requested(struct pinctrl_dev *pctldev, int pin)
         |      ^~~~~~~~~~~~~


vim +/pin_requested +104 drivers/pinctrl/pinmux.h

   103	
 > 104	bool pin_requested(struct pinctrl_dev *pctldev, int pin)
   105	{
   106		return false;
   107	}
   108	#endif
   109	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 6/7] pinctrl: pinmux: Introduce API to check if a pin is requested

[Linus Walleij]

On Thu, Oct 17, 2024 at 1:39 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:

> @@ -100,6 +101,10 @@ static inline void pinmux_disable_setting(const struct pinctrl_setting *setting)
>  {
>  }
>
> +bool pin_requested(struct pinctrl_dev *pctldev, int pin)
> +{
> +       return false;
> +}
>  #endif

You need "static inline" in front of the stub, that's why the robot is
complaining.

Yours,
Linus Walleij

Re: [PATCH 6/7] pinctrl: pinmux: Introduce API to check if a pin is requested

[Lad, Prabhakar]

Hi Linus,

On Sat, Oct 19, 2024 at 7:22 PM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Thu, Oct 17, 2024 at 1:39 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
>
> > @@ -100,6 +101,10 @@ static inline void pinmux_disable_setting(const struct pinctrl_setting *setting)
> >  {
> >  }
> >
> > +bool pin_requested(struct pinctrl_dev *pctldev, int pin)
> > +{
> > +       return false;
> > +}
> >  #endif
>
> You need "static inline" in front of the stub, that's why the robot is
> complaining.
>
Agreed, If there are no other comments for the rest of the patches
I'll just send an updated patch for this or include it as part of the
whole series.

Cheers,
Prabhakar

Re: [PATCH 6/7] pinctrl: pinmux: Introduce API to check if a pin is requested

[kernel test robot]

Hi Prabhakar,

kernel test robot noticed the following build warnings:

[auto build test WARNING on geert-renesas-drivers/renesas-pinctrl]
[also build test WARNING on linusw-pinctrl/devel linusw-pinctrl/for-next geert-renesas-devel/next linus/master v6.12-rc4 next-20241018]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Prabhakar/arm64-dts-renesas-rzg3s-smarc-Drop-hogging-of-GPIO-pins/20241017-194200
base:   https://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git renesas-pinctrl
patch link:    https://lore.kernel.org/r/20241017113942.139712-7-prabhakar.mahadev-lad.rj%40bp.renesas.com
patch subject: [PATCH 6/7] pinctrl: pinmux: Introduce API to check if a pin is requested
config: hexagon-randconfig-001-20241021 (https://download.01.org/0day-ci/archive/20241021/202410211007.sFKERNFY-lkp@intel.com/config)
compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project bfe84f7085d82d06d61c632a7bad1e692fd159e4)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241021/202410211007.sFKERNFY-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410211007.sFKERNFY-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from drivers/pinctrl/core.c:28:
   In file included from include/linux/gpio/driver.h:8:
   In file included from include/linux/irqchip/chained_irq.h:10:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:14:
   In file included from arch/hexagon/include/asm/io.h:328:
   include/asm-generic/io.h:548:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     548 |         val = __raw_readb(PCI_IOBASE + addr);
         |                           ~~~~~~~~~~ ^
   include/asm-generic/io.h:561:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     561 |         val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
         |                                                         ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
      37 | #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
         |                                                   ^
   In file included from drivers/pinctrl/core.c:28:
   In file included from include/linux/gpio/driver.h:8:
   In file included from include/linux/irqchip/chained_irq.h:10:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:14:
   In file included from arch/hexagon/include/asm/io.h:328:
   include/asm-generic/io.h:574:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     574 |         val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
         |                                                         ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
      35 | #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
         |                                                   ^
   In file included from drivers/pinctrl/core.c:28:
   In file included from include/linux/gpio/driver.h:8:
   In file included from include/linux/irqchip/chained_irq.h:10:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:14:
   In file included from arch/hexagon/include/asm/io.h:328:
   include/asm-generic/io.h:585:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     585 |         __raw_writeb(value, PCI_IOBASE + addr);
         |                             ~~~~~~~~~~ ^
   include/asm-generic/io.h:595:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     595 |         __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
         |                                                       ~~~~~~~~~~ ^
   include/asm-generic/io.h:605:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     605 |         __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
         |                                                       ~~~~~~~~~~ ^
   In file included from drivers/pinctrl/core.c:38:
>> drivers/pinctrl/pinmux.h:104:6: warning: no previous prototype for function 'pin_requested' [-Wmissing-prototypes]
     104 | bool pin_requested(struct pinctrl_dev *pctldev, int pin)
         |      ^
   drivers/pinctrl/pinmux.h:104:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
     104 | bool pin_requested(struct pinctrl_dev *pctldev, int pin)
         | ^
         | static 
   7 warnings generated.

Kconfig warnings: (for reference only)
   WARNING: unmet direct dependencies detected for MODVERSIONS
   Depends on [n]: MODULES [=y] && !COMPILE_TEST [=y]
   Selected by [y]:
   - RANDSTRUCT_FULL [=y] && (CC_HAS_RANDSTRUCT [=y] || GCC_PLUGINS [=n]) && MODULES [=y]


vim +/pin_requested +104 drivers/pinctrl/pinmux.h

   103	
 > 104	bool pin_requested(struct pinctrl_dev *pctldev, int pin)
   105	{
   106		return false;
   107	}
   108	#endif
   109	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[PATCH 7/7] pinctrl: renesas: pinctrl-rzg2l: Override irq_request/release_resources

[Prabhakar]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Override the default `irq_request_resources` and `irq_release_resources`
functions with `rzg2l_gpio_irq_request_resources` and
`rzg2l_gpio_irq_release_resources` in the RZ/G2L pinctrl driver.

The `rzg2l_gpio_irq_request_resources()` function now ensures that the pin
is requested by the pinctrl core before locking the GPIO as an IRQ. This
ensures that the `pinmux-pins` file in sysfs correctly reports the pin as
claimed. Additionally, the `rzg2l_gpio_direction_input()` call is moved
into the `rzg2l_gpio_irq_request_resources()` callback, as it makes sense
to configure the GPIO pin as an input after it has been requested.

The `rzg2l_gpio_irq_release_resources()` function unlocks the GPIO as an
IRQ and then frees the GPIO, ensuring proper cleanup when the IRQ is no
longer needed. This guarantees that the `pinmux-pins` file in sysfs
correctly reports the pin as unclaimed.

Also add a `pin_requested()` check in `rzg2l_gpio_free()` to return early
if the pin is already released.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 drivers/pinctrl/renesas/pinctrl-rzg2l.c | 41 +++++++++++++++++++++----
 1 file changed, 35 insertions(+), 6 deletions(-)

diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
index b9a8bf43a92a..47b3e296d094 100644
--- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
+++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
@@ -1772,8 +1772,12 @@ static int rzg2l_gpio_get(struct gpio_chip *chip, unsigned int offset)
 
 static void rzg2l_gpio_free(struct gpio_chip *chip, unsigned int offset)
 {
+	struct rzg2l_pinctrl *pctrl = gpiochip_get_data(chip);
 	unsigned int virq;
 
+	if (!pin_requested(pctrl->pctl, offset))
+		return;
+
 	virq = irq_find_mapping(chip->irq.domain, offset);
 	if (virq)
 		irq_dispose_mapping(virq);
@@ -2357,6 +2361,35 @@ static int rzg2l_gpio_irq_set_wake(struct irq_data *data, unsigned int on)
 	return 0;
 }
 
+static int rzg2l_gpio_irq_request_resources(struct irq_data *d)
+{
+	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+	struct rzg2l_pinctrl *pctrl = container_of(gc, struct rzg2l_pinctrl, gpio_chip);
+	unsigned int child = irqd_to_hwirq(d);
+	int ret;
+
+	if (!pin_requested(pctrl->pctl, child)) {
+		ret = rzg2l_gpio_request(gc, child);
+		if (ret)
+			return ret;
+	}
+
+	ret = rzg2l_gpio_direction_input(gc, child);
+	if (ret)
+		return ret;
+
+	return gpiochip_irq_reqres(d);
+}
+
+static void rzg2l_gpio_irq_release_resources(struct irq_data *d)
+{
+	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+	unsigned int child = irqd_to_hwirq(d);
+
+	gpiochip_irq_relres(d);
+	rzg2l_gpio_free(gc, child);
+}
+
 static const struct irq_chip rzg2l_gpio_irqchip = {
 	.name = "rzg2l-gpio",
 	.irq_disable = rzg2l_gpio_irq_disable,
@@ -2368,8 +2401,9 @@ static const struct irq_chip rzg2l_gpio_irqchip = {
 	.irq_print_chip = rzg2l_gpio_irq_print_chip,
 	.irq_set_affinity = irq_chip_set_affinity_parent,
 	.irq_set_wake = rzg2l_gpio_irq_set_wake,
+	.irq_request_resources = rzg2l_gpio_irq_request_resources,
+	.irq_release_resources = rzg2l_gpio_irq_release_resources,
 	.flags = IRQCHIP_IMMUTABLE,
-	GPIOCHIP_IRQ_RESOURCE_HELPERS,
 };
 
 static int rzg2l_gpio_child_to_parent_hwirq(struct gpio_chip *gc,
@@ -2381,16 +2415,11 @@ static int rzg2l_gpio_child_to_parent_hwirq(struct gpio_chip *gc,
 	struct rzg2l_pinctrl *pctrl = gpiochip_get_data(gc);
 	unsigned long flags;
 	int gpioint, irq;
-	int ret;
 
 	gpioint = rzg2l_gpio_get_gpioint(child, pctrl);
 	if (gpioint < 0)
 		return gpioint;
 
-	ret = rzg2l_gpio_direction_input(gc, child);
-	if (ret)
-		return ret;
-
 	spin_lock_irqsave(&pctrl->bitmap_lock, flags);
 	irq = bitmap_find_free_region(pctrl->tint_slot, RZG2L_TINT_MAX_INTERRUPT, get_order(1));
 	spin_unlock_irqrestore(&pctrl->bitmap_lock, flags);
-- 
2.43.0

Re: [PATCH 7/7] pinctrl: renesas: pinctrl-rzg2l: Override irq_request/release_resources

[Linus Walleij]

On Thu, Oct 17, 2024 at 1:39 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:

> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Override the default `irq_request_resources` and `irq_release_resources`
> functions with `rzg2l_gpio_irq_request_resources` and
> `rzg2l_gpio_irq_release_resources` in the RZ/G2L pinctrl driver.
>
> The `rzg2l_gpio_irq_request_resources()` function now ensures that the pin
> is requested by the pinctrl core before locking the GPIO as an IRQ. This
> ensures that the `pinmux-pins` file in sysfs correctly reports the pin as
> claimed. Additionally, the `rzg2l_gpio_direction_input()` call is moved
> into the `rzg2l_gpio_irq_request_resources()` callback, as it makes sense
> to configure the GPIO pin as an input after it has been requested.
>
> The `rzg2l_gpio_irq_release_resources()` function unlocks the GPIO as an
> IRQ and then frees the GPIO, ensuring proper cleanup when the IRQ is no
> longer needed. This guarantees that the `pinmux-pins` file in sysfs
> correctly reports the pin as unclaimed.
>
> Also add a `pin_requested()` check in `rzg2l_gpio_free()` to return early
> if the pin is already released.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Hm! I can live with this I think, if Geert also think it is the
right thing to do.

Yours,
Linus Walleij