From: Claudiu Beznea claudiu.beznea.uj@bp.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@vger.kernel.org Signed-off-by: Claudiu Beznea claudiu.beznea.uj@bp.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
drivers/pinctrl/renesas/pinctrl-rzg2l.c | 71 +++++++++++++++---------- 1 file changed, 44 insertions(+), 27 deletions(-)
diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c index f524af6f586f..c360e473488c 100644 --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c @@ -359,7 +359,7 @@ struct rzg2l_pinctrl { spinlock_t bitmap_lock; /* protect tint_slot bitmap */ unsigned int hwirq[RZG2L_TINT_MAX_INTERRUPT];
- spinlock_t lock; /* lock read/write registers */ + raw_spinlock_t lock; /* lock read/write registers */ struct mutex mutex; /* serialize adding groups and functions */
struct rzg2l_pinctrl_pin_settings *settings; @@ -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)); @@ -567,7 +567,7 @@ static void rzg2l_pinctrl_set_pfc_mode(struct rzg2l_pinctrl *pctrl,
pctrl->data->pwpr_pfc_lock_unlock(pctrl, true);
- spin_unlock_irqrestore(&pctrl->lock, flags); + raw_spin_unlock_irqrestore(&pctrl->lock, flags); };
static int rzg2l_pinctrl_set_mux(struct pinctrl_dev *pctldev, @@ -882,10 +882,10 @@ static void rzg2l_rmw_pin_config(struct rzg2l_pinctrl *pctrl, u32 offset, addr += 4; }
- spin_lock_irqsave(&pctrl->lock, flags); + raw_spin_lock_irqsave(&pctrl->lock, flags); reg = readl(addr) & ~(mask << (bit * 8)); writel(reg | (val << (bit * 8)), addr); - spin_unlock_irqrestore(&pctrl->lock, flags); + raw_spin_unlock_irqrestore(&pctrl->lock, flags); }
static int rzg2l_caps_to_pwr_reg(const struct rzg2l_register_offsets *regs, u32 caps) @@ -1121,7 +1121,7 @@ static int rzg2l_write_oen(struct rzg2l_pinctrl *pctrl, unsigned int _pin, u8 oe if (bit < 0) return -EINVAL;
- spin_lock_irqsave(&pctrl->lock, flags); + raw_spin_lock_irqsave(&pctrl->lock, flags); val = readb(pctrl->base + oen_offset); if (oen) val &= ~BIT(bit); @@ -1134,7 +1134,7 @@ static int rzg2l_write_oen(struct rzg2l_pinctrl *pctrl, unsigned int _pin, u8 oe writeb(val, pctrl->base + oen_offset); if (pctrl->data->hwcfg->oen_pwpr_lock) writeb(pwpr & ~PWPR_REGWE_B, pctrl->base + regs->pwpr); - spin_unlock_irqrestore(&pctrl->lock, flags); + raw_spin_unlock_irqrestore(&pctrl->lock, flags);
return 0; } @@ -1687,14 +1687,14 @@ static int rzg2l_gpio_request(struct gpio_chip *chip, unsigned int offset) if (ret) return ret;
- spin_lock_irqsave(&pctrl->lock, flags); + raw_spin_lock_irqsave(&pctrl->lock, flags);
/* Select GPIO mode in PMC Register */ reg8 = readb(pctrl->base + PMC(off)); reg8 &= ~BIT(bit); pctrl->data->pmc_writeb(pctrl, reg8, PMC(off));
- spin_unlock_irqrestore(&pctrl->lock, flags); + raw_spin_unlock_irqrestore(&pctrl->lock, flags);
return 0; } @@ -1709,7 +1709,7 @@ static void rzg2l_gpio_set_direction(struct rzg2l_pinctrl *pctrl, u32 offset, unsigned long flags; u16 reg16;
- spin_lock_irqsave(&pctrl->lock, flags); + raw_spin_lock_irqsave(&pctrl->lock, flags);
reg16 = readw(pctrl->base + PM(off)); reg16 &= ~(PM_MASK << (bit * 2)); @@ -1717,7 +1717,7 @@ static void rzg2l_gpio_set_direction(struct rzg2l_pinctrl *pctrl, u32 offset, reg16 |= (output ? PM_OUTPUT : PM_INPUT) << (bit * 2); writew(reg16, pctrl->base + PM(off));
- spin_unlock_irqrestore(&pctrl->lock, flags); + raw_spin_unlock_irqrestore(&pctrl->lock, flags); }
static int rzg2l_gpio_get_direction(struct gpio_chip *chip, unsigned int offset) @@ -1761,7 +1761,7 @@ static int rzg2l_gpio_set(struct gpio_chip *chip, unsigned int offset, unsigned long flags; u8 reg8;
- spin_lock_irqsave(&pctrl->lock, flags); + raw_spin_lock_irqsave(&pctrl->lock, flags);
reg8 = readb(pctrl->base + P(off));
@@ -1770,7 +1770,7 @@ static int rzg2l_gpio_set(struct gpio_chip *chip, unsigned int offset, else writeb(reg8 & ~BIT(bit), pctrl->base + P(off));
- spin_unlock_irqrestore(&pctrl->lock, flags); + raw_spin_unlock_irqrestore(&pctrl->lock, flags);
return 0; } @@ -2429,14 +2429,13 @@ static int rzg2l_gpio_get_gpioint(unsigned int virq, struct rzg2l_pinctrl *pctrl return gpioint; }
-static void rzg2l_gpio_irq_endisable(struct rzg2l_pinctrl *pctrl, - unsigned int hwirq, bool enable) +static void __rzg2l_gpio_irq_endisable(struct rzg2l_pinctrl *pctrl, + unsigned int hwirq, bool enable) { const struct pinctrl_pin_desc *pin_desc = &pctrl->desc.pins[hwirq]; u64 *pin_data = pin_desc->drv_data; u32 off = RZG2L_PIN_CFG_TO_PORT_OFFSET(*pin_data); u8 bit = RZG2L_PIN_ID_TO_PIN(hwirq); - unsigned long flags; void __iomem *addr;
addr = pctrl->base + ISEL(off); @@ -2445,12 +2444,20 @@ static void rzg2l_gpio_irq_endisable(struct rzg2l_pinctrl *pctrl, addr += 4; }
- spin_lock_irqsave(&pctrl->lock, flags); if (enable) writel(readl(addr) | BIT(bit * 8), addr); else writel(readl(addr) & ~BIT(bit * 8), addr); - spin_unlock_irqrestore(&pctrl->lock, flags); +} + +static void rzg2l_gpio_irq_endisable(struct rzg2l_pinctrl *pctrl, + unsigned int hwirq, bool enable) +{ + unsigned long flags; + + raw_spin_lock_irqsave(&pctrl->lock, flags); + __rzg2l_gpio_irq_endisable(pctrl, hwirq, enable); + raw_spin_unlock_irqrestore(&pctrl->lock, flags); }
static void rzg2l_gpio_irq_disable(struct irq_data *d) @@ -2462,15 +2469,25 @@ static void rzg2l_gpio_irq_disable(struct irq_data *d) gpiochip_disable_irq(gc, hwirq); }
-static void rzg2l_gpio_irq_enable(struct irq_data *d) +static void __rzg2l_gpio_irq_enable(struct irq_data *d, bool lock) { 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 hwirq = irqd_to_hwirq(d);
gpiochip_enable_irq(gc, hwirq); + if (lock) + rzg2l_gpio_irq_endisable(pctrl, hwirq, true); + else + __rzg2l_gpio_irq_endisable(pctrl, hwirq, true); irq_chip_enable_parent(d); }
+static void rzg2l_gpio_irq_enable(struct irq_data *d) +{ + __rzg2l_gpio_irq_enable(d, true); +} + static int rzg2l_gpio_irq_set_type(struct irq_data *d, unsigned int type) { return irq_chip_set_type_parent(d, type); @@ -2616,11 +2633,11 @@ static void rzg2l_gpio_irq_restore(struct rzg2l_pinctrl *pctrl) * This has to be atomically executed to protect against a concurrent * interrupt. */ - spin_lock_irqsave(&pctrl->lock, flags); + raw_spin_lock_irqsave(&pctrl->lock, flags); ret = rzg2l_gpio_irq_set_type(data, irqd_get_trigger_type(data)); if (!ret && !irqd_irq_disabled(data)) - rzg2l_gpio_irq_enable(data); - spin_unlock_irqrestore(&pctrl->lock, flags); + __rzg2l_gpio_irq_enable(data, false); + raw_spin_unlock_irqrestore(&pctrl->lock, flags);
if (ret) dev_crit(pctrl->dev, "Failed to set IRQ type for virq=%u\n", virq); @@ -2950,7 +2967,7 @@ static int rzg2l_pinctrl_probe(struct platform_device *pdev) "failed to enable GPIO clk\n"); }
- spin_lock_init(&pctrl->lock); + raw_spin_lock_init(&pctrl->lock); spin_lock_init(&pctrl->bitmap_lock); mutex_init(&pctrl->mutex); atomic_set(&pctrl->wakeup_path, 0); @@ -3093,7 +3110,7 @@ static void rzg2l_pinctrl_pm_setup_pfc(struct rzg2l_pinctrl *pctrl) u32 nports = pctrl->data->n_port_pins / RZG2L_PINS_PER_PORT; unsigned long flags;
- spin_lock_irqsave(&pctrl->lock, flags); + raw_spin_lock_irqsave(&pctrl->lock, flags); pctrl->data->pwpr_pfc_lock_unlock(pctrl, false);
/* Restore port registers. */ @@ -3138,7 +3155,7 @@ static void rzg2l_pinctrl_pm_setup_pfc(struct rzg2l_pinctrl *pctrl) }
pctrl->data->pwpr_pfc_lock_unlock(pctrl, true); - spin_unlock_irqrestore(&pctrl->lock, flags); + raw_spin_unlock_irqrestore(&pctrl->lock, flags); }
static int rzg2l_pinctrl_suspend_noirq(struct device *dev) @@ -3187,14 +3204,14 @@ static int rzg2l_pinctrl_resume_noirq(struct device *dev)
writeb(cache->qspi, pctrl->base + QSPI); if (pctrl->data->hwcfg->oen_pwpr_lock) { - spin_lock_irqsave(&pctrl->lock, flags); + raw_spin_lock_irqsave(&pctrl->lock, flags); pwpr = readb(pctrl->base + regs->pwpr); writeb(pwpr | PWPR_REGWE_B, pctrl->base + regs->pwpr); } writeb(cache->oen, pctrl->base + pctrl->data->hwcfg->regs.oen); if (pctrl->data->hwcfg->oen_pwpr_lock) { writeb(pwpr & ~PWPR_REGWE_B, pctrl->base + regs->pwpr); - spin_unlock_irqrestore(&pctrl->lock, flags); + raw_spin_unlock_irqrestore(&pctrl->lock, flags); } for (u8 i = 0; i < 2; i++) { if (regs->sd_ch)
linux-stable-mirror@lists.linaro.org