From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f52.google.com (mail-wr1-f52.google.com [209.85.221.52]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 2B7252BF000 for ; Mon, 29 Sep 2025 12:10:03 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.221.52 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1759147807; cv=none; b=XhwuBcUK/+eO0keeuXj9PhhP6IBDcjT0S5LTnL4cyWSNTMqctHcBNHf6QzUEpR/d4tf/f2qSCd/4N4bepPdG6i5tyLLqamvZusc0Lnsus6KT9sl3+KBb9m7lJOWyjUFpBkAoeRbiKZlTro1bz7y7HAy+eCOWxJAzMwJ7eWzkTTw= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1759147807; c=relaxed/simple; bh=xp73wE0iogPFWbLCN8f/Njrke1JukbnUCU8WyG4rMoQ=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=HSt0sftyDp5owx8v36YatPVF6c3fcPHKGSQ4yKIl3XVtFWdcEMlj2emM99mV5SmYcZBp3CF4TFqjjQT4Rm1030W7TVxxQx7PHfMknEsaQsF5qPvW9+D2hI87oKPGm4DlzeW9iOQ1xN+SNp1mDDoNwoFO3PDpG4XBNkDylg+bU7c= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=tuxon.dev; spf=pass smtp.mailfrom=tuxon.dev; dkim=pass (2048-bit key) header.d=tuxon.dev header.i=@tuxon.dev header.b=KwG4vMWs; arc=none smtp.client-ip=209.85.221.52 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=tuxon.dev Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=tuxon.dev Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=tuxon.dev header.i=@tuxon.dev header.b="KwG4vMWs" Received: by mail-wr1-f52.google.com with SMTP id ffacd0b85a97d-3f99ac9acc4so3771360f8f.3 for ; Mon, 29 Sep 2025 05:10:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=tuxon.dev; s=google; t=1759147802; x=1759752602; darn=vger.kernel.org; h=content-transfer-encoding:in-reply-to:content-language:from :references:cc:to:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=/A1N+ty2qApZBdj5/inaifpCvqdtitLGQdcgdQAh7Zc=; b=KwG4vMWspA2Bn1gXCL5Mpb2Dyvj7rwwLpccdrEyDsZfQUvYSXBydIuWgXUoFV7QIsi LGfsfd2k2boNqkTmkRagxw0LXvHEAzmGRuNGdfkJe/EAnb/hqlzxUtvvb0ry3iqyz90B ZDn+v6vgyXjYQAPyNOvhXb4oQsFEbJrbQ7olZJUhDatSBCRc7vX9eNisPPaHkSRrCHDy PE87+TvAIKXIaqt0uD7BP+bUcopb+I2Su1nJMS2Guc3bRn0abY3AyGC+I9zezqKhhBeJ sZ4shgjV/69blOTRjIhKStX01IxQL8ZG6yTuGGxBPUri3pCZFlDKYzZ5RPTiOzE6B/cr vemQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1759147802; x=1759752602; h=content-transfer-encoding:in-reply-to:content-language:from :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=/A1N+ty2qApZBdj5/inaifpCvqdtitLGQdcgdQAh7Zc=; b=Np++fwmA7eIK7v/cfiylZaITeoQBYiGJ2lV1743VGI0QsTttSwKqmjaw20CLFhjSb0 LWunx5ZPW+RTfRwgnDadwSk+oabQrXHceniZS6fFnDTi1p7OrnhCjEF03XAifu2xvjot 3dFPuD1WZPVJ2bfglMdfMHn2JzH/PdWwD+K8IPEJ9FgxpAKo6NW74iKEqnzpZWBOld8t 3e9hNAjEyKU0mTi+wUpZP+SKfbvvihv5JM3itfjHYAui8lYU9rEfU6rdUNp6uZ2J9nrF 5fO8TdyduynfURgl39XWhBlHWIEKEoNoJNZbneR82dSS0WSafM3b1lSwxBUvBJlOVyHO h+Dg== X-Forwarded-Encrypted: i=1; AJvYcCW3RozJjnaSK1tJ5mkizKCGVGxVEwRgAD6xy1eJbuLKrA66bSKXZY0wSyfRJKy7lhVjAGKrDok=@vger.kernel.org X-Gm-Message-State: AOJu0Yx3HlDUdF51FksIU6KptH3qrEZBm1Il/Phey5UE+ivCj+eOPyb5 QyfHrv8P7RSnn/eAH79B3Fw9xNGJ9YykNAcMCpR9XEq3TbOO7+Yp+I4LWxpYbGLcMl8= X-Gm-Gg: ASbGncvrXNz65wsE2a8WBYSfTSoUI1IiJ5qIR/ssjCBR3V+H4OznscJcfxSyZVmGltQ UCiJ+xlfSDXNgBQaSeD84eIg+7X//rRxRib1IW5yP/SF9yQfGTRD4HZdbUCmrdNPt39evtR8qN2 8T/oCm4j7LlugUadjrnW70E8njGG8r3BD0Q1SeKxN3E/TMejbqb1PBJC3zVtXpK31CLbIh/Waqg acoupfS8oLrw9c8d25y2Rk1JB7BUvcm3WoSidpl2yWdgtlN8j+pNAFxfGxWXOQowj/Gs9lduoFV ICRJlSVkhZt+Xj8+tCf9ljmsvhL/ASYX5a6FuWT1ThZH1Qqm43DK0a2d+XNQ9YtvcOQXdNk2K0o L5MjbLJLFoJXeDH3xfzQcc+yC/vp2L5rzvvEkmrrFLw== X-Google-Smtp-Source: AGHT+IGOI3s83JZpf8iF/U2faBc1EGkYgirVAtNe1lZ2DI3XBN2xkajove5bIaa7373Vb0l2Wjr1lw== X-Received: by 2002:a05:6000:3105:b0:3f8:6e37:376 with SMTP id ffacd0b85a97d-40e499acbecmr16060096f8f.45.1759147802117; Mon, 29 Sep 2025 05:10:02 -0700 (PDT) Received: from [192.168.50.4] ([82.78.167.111]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-46e56f3d754sm11135015e9.4.2025.09.29.05.10.00 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 29 Sep 2025 05:10:01 -0700 (PDT) Message-ID: Date: Mon, 29 Sep 2025 15:10:00 +0300 Precedence: bulk X-Mailing-List: stable@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2] pinctrl: renesas: rzg2l: Fix ISEL restore on resume To: Geert Uytterhoeven Cc: linus.walleij@linaro.org, biju.das.jz@bp.renesas.com, linux-renesas-soc@vger.kernel.org, linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org, Claudiu Beznea , stable@vger.kernel.org References: <20250912095308.3603704-1-claudiu.beznea.uj@bp.renesas.com> From: Claudiu Beznea Content-Language: en-US In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Hi, Geert, On 9/29/25 15:03, Geert Uytterhoeven wrote: > Hi Claudiu, > > On Fri, 12 Sept 2025 at 11:53, Claudiu wrote: >> From: Claudiu Beznea >> >> 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@vger.kernel.org >> Signed-off-by: Claudiu Beznea >> --- >> >> 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 > 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 >