| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAMuHMdXv1-w0SE7FZy5k3jg2FO-a-RB2w1WB=VM_UFEA9zjWDw@mail.gmail.com>
Date: Mon, 29 Sep 2025 14:03:25 +0200
From: Geert Uytterhoeven <geert@...ux-m68k.org>
To: Claudiu <claudiu.beznea@...on.dev>
Cc: linus.walleij@...aro.org, biju.das.jz@...renesas.com,
linux-renesas-soc@...r.kernel.org, linux-gpio@...r.kernel.org,
linux-kernel@...r.kernel.org,
Claudiu Beznea <claudiu.beznea.uj@...renesas.com>, stable@...r.kernel.org
Subject: Re: [PATCH v2] pinctrl: renesas: rzg2l: Fix ISEL restore on resume
Hi Claudiu,
On Fri, 12 Sept 2025 at 11:53, Claudiu <claudiu.beznea@...on.dev> wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@...renesas.com>
>
> Commit 1d2da79708cb ("pinctrl: renesas: rzg2l: Avoid configuring ISEL in
> gpio_irq_{en,dis}able*()") dropped the configuration of ISEL from
> struct irq_chip::{irq_enable, irq_disable} APIs and moved it to
> struct gpio_chip::irq::{child_to_parent_hwirq,
> child_irq_domain_ops::free} APIs to fix spurious IRQs.
>
> After commit 1d2da79708cb ("pinctrl: renesas: rzg2l: Avoid configuring ISEL
> in gpio_irq_{en,dis}able*()"), ISEL was no longer configured properly on
> resume. This is because the pinctrl resume code used
> struct irq_chip::irq_enable (called from rzg2l_gpio_irq_restore()) to
> reconfigure the wakeup interrupts. Some drivers (e.g. Ethernet) may also
> reconfigure non-wakeup interrupts on resume through their own code,
> eventually calling struct irq_chip::irq_enable.
>
> Fix this by adding ISEL configuration back into the
> struct irq_chip::irq_enable API and on resume path for wakeup interrupts.
>
> As struct irq_chip::irq_enable needs now to lock to update the ISEL,
> convert the struct rzg2l_pinctrl::lock to a raw spinlock and replace the
> locking API calls with the raw variants. Otherwise the lockdep reports
> invalid wait context when probing the adv7511 module on RZ/G2L:
>
> [ BUG: Invalid wait context ]
> 6.17.0-rc5-next-20250911-00001-gfcfac22533c9 #18 Not tainted
> -----------------------------
> (udev-worker)/165 is trying to lock:
> ffff00000e3664a8 (&pctrl->lock){....}-{3:3}, at: rzg2l_gpio_irq_enable+0x38/0x78
> other info that might help us debug this:
> context-{5:5}
> 3 locks held by (udev-worker)/165:
> #0: ffff00000e890108 (&dev->mutex){....}-{4:4}, at: __driver_attach+0x90/0x1ac
> #1: ffff000011c07240 (request_class){+.+.}-{4:4}, at: __setup_irq+0xb4/0x6dc
> #2: ffff000011c070c8 (lock_class){....}-{2:2}, at: __setup_irq+0xdc/0x6dc
> stack backtrace:
> CPU: 1 UID: 0 PID: 165 Comm: (udev-worker) Not tainted 6.17.0-rc5-next-20250911-00001-gfcfac22533c9 #18 PREEMPT
> Hardware name: Renesas SMARC EVK based on r9a07g044l2 (DT)
> Call trace:
> show_stack+0x18/0x24 (C)
> dump_stack_lvl+0x90/0xd0
> dump_stack+0x18/0x24
> __lock_acquire+0xa14/0x20b4
> lock_acquire+0x1c8/0x354
> _raw_spin_lock_irqsave+0x60/0x88
> rzg2l_gpio_irq_enable+0x38/0x78
> irq_enable+0x40/0x8c
> __irq_startup+0x78/0xa4
> irq_startup+0x108/0x16c
> __setup_irq+0x3c0/0x6dc
> request_threaded_irq+0xec/0x1ac
> devm_request_threaded_irq+0x80/0x134
> adv7511_probe+0x928/0x9a4 [adv7511]
> i2c_device_probe+0x22c/0x3dc
> really_probe+0xbc/0x2a0
> __driver_probe_device+0x78/0x12c
> driver_probe_device+0x40/0x164
> __driver_attach+0x9c/0x1ac
> bus_for_each_dev+0x74/0xd0
> driver_attach+0x24/0x30
> bus_add_driver+0xe4/0x208
> driver_register+0x60/0x128
> i2c_register_driver+0x48/0xd0
> adv7511_init+0x5c/0x1000 [adv7511]
> do_one_initcall+0x64/0x30c
> do_init_module+0x58/0x23c
> load_module+0x1bcc/0x1d40
> init_module_from_file+0x88/0xc4
> idempotent_init_module+0x188/0x27c
> __arm64_sys_finit_module+0x68/0xac
> invoke_syscall+0x48/0x110
> el0_svc_common.constprop.0+0xc0/0xe0
> do_el0_svc+0x1c/0x28
> el0_svc+0x4c/0x160
> el0t_64_sync_handler+0xa0/0xe4
> el0t_64_sync+0x198/0x19c
>
> Having ISEL configuration back into the struct irq_chip::irq_enable API
> should be safe with respect to spurious IRQs, as in the probe case IRQs
> are enabled anyway in struct gpio_chip::irq::child_to_parent_hwirq. No
> spurious IRQs were detected on suspend/resume, boot, ethernet link
> insert/remove tests (executed on RZ/G3S). Boot, ethernet link
> insert/remove tests were also executed successfully on RZ/G2L.
>
> Fixes: 1d2da79708cb ("pinctrl: renesas: rzg2l: Avoid configuring ISEL in gpio_irq_{en,dis}able*(")
> Cc: stable@...r.kernel.org
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@...renesas.com>
> ---
>
> Changes in v2:
> - changed the implementation approach by dropping the spinlock in
> rzg2l_gpio_irq_endisable(), renaming it to
> __rzg2l_gpio_irq_endisable() and using it in
> rzg2l_gpio_irq_endisable(), the newly introduced
> __rzg2l_gpio_irq_enable() and rzg2l_gpio_irq_restore()
> - convert struct rzg2l_pinctrl::lock to raw_spinlock_t
LGTM, so
Reviewed-by: Geert Uytterhoeven <geert+renesas@...der.be>
i.e. will queue in renesas-pinctrl for v6.19.
> --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> @@ -543,7 +543,7 @@ static void rzg2l_pinctrl_set_pfc_mode(struct rzg2l_pinctrl *pctrl,
> unsigned long flags;
> u32 reg;
>
> - spin_lock_irqsave(&pctrl->lock, flags);
> + raw_spin_lock_irqsave(&pctrl->lock, flags);
>
> /* Set pin to 'Non-use (Hi-Z input protection)' */
> reg = readw(pctrl->base + PM(off));
This conflicts with commit d57183d06851bae4 ("pinctrl: renesas: rzg2l:
Drop unnecessary pin configurations"), which I have already queued
in renesas-drivers/renesas-pinctrl-for-v6.19. Hence I am replacing
the above hunk by:
/* Switching to GPIO is not required if reset value is
same as func */
reg = readb(pctrl->base + PMC(off));
- spin_lock_irqsave(&pctrl->lock, flags);
+ raw_spin_lock_irqsave(&pctrl->lock, flags);
pfc = readl(pctrl->base + PFC(off));
if ((reg & BIT(pin)) && (((pfc >> (pin * 4)) & PFC_MASK) == func)) {
- spin_unlock_irqrestore(&pctrl->lock, flags);
+ raw_spin_unlock_irqrestore(&pctrl->lock, flags);
return;
}
while applying.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-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
Powered by blists - more mailing lists