| 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: <ef82c610-0571-4665-a5d1-07a9ed9fb8d3@tuxon.dev>
Date: Mon, 29 Sep 2025 15:10:00 +0300
From: Claudiu Beznea <claudiu.beznea@...on.dev>
To: Geert Uytterhoeven <geert@...ux-m68k.org>
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, Geert,
On 9/29/25 15:03, Geert Uytterhoeven wrote:
> 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.
This is right. Thank you! I'm going to give it also a try (on actual HW) a
bit later. I'll let you know.
Thank you,
Claudiu
>
> Gr{oetje,eeting}s,
>
> Geert
>
Powered by blists - more mailing lists