Re: [PATCH 5.10.y-cip] pinctrl: renesas: rzg2l: Fix ISEL restore on resume

[Pavel Machek <pavel@nabladev.com>]

[-- Attachment #1: Type: text/plain, Size: 2258 bytes --]

Hi!

> commit 44bf66122c12ef6d3382a9b84b9be1802e5f0e95 upstream.
> 
> 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:

> +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
...
> -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);
>  }
>

Conditional locking. That's something that should normally be avoided.

Anyway, it is already in mainline, and patch looks good to me, so

Reviewed-by: Pavel Machek <pavel@nabladev.com>

I can apply this if it passes testing.

Best regards,
                                                                Pavel
-- 
In cooperation with Nabla.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]