mbox series

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

Message ID 20241017113942.139712-1-prabhakar.mahadev-lad.rj@bp.renesas.com
Headers show
Series pinctrl: rzg2l: Fix releasing of IRQ and status reported in pinmux-pins | expand

Message

Lad, Prabhakar Oct. 17, 2024, 11:39 a.m. UTC
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(-)

Comments

Linus Walleij Oct. 19, 2024, 6:21 p.m. UTC | #1
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
Lad, Prabhakar Oct. 21, 2024, 8:03 a.m. UTC | #2
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