[PATCH] irqchip/stm32: Keep pinctrl block clock enabled when LEVEL IRQ requested
Fabien DESSENNE
fabien.dessenne at foss.st.com
Fri Apr 22 07:28:49 PDT 2022
Hi Marek
Thank you for the patch.
Like Linus did for your other patch, I would prefer the subject prefix
to be "pinctrl: stm32:"
Apart from that I added my RB below
BR
Fabien
On 21/04/2022 16:08, Marek Vasut wrote:
> The current EOI handler for LEVEL triggered interrupts calls clk_enable(),
> register IO, clk_disable(). The clock manipulation requires locking which
> happens with IRQs disabled in clk_enable_lock(). Instead of turning the
> clock on and off all the time, enable the clock in case LEVEL interrupt is
> requested and keep the clock enabled until all LEVEL interrupts are freed.
> The LEVEL interrupts are an exception on this platform and seldom used, so
> this does not affect the common case.
>
> This simplifies the LEVEL interrupt handling considerably and also fixes
> the following splat found when using preempt-rt:
> ------------[ cut here ]------------
> WARNING: CPU: 0 PID: 0 at kernel/locking/rtmutex.c:2040 __rt_mutex_trylock+0x37/0x62
> Modules linked in:
> CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.10.109-rt65-stable-standard-00068-g6a5afc4b1217 #85
> Hardware name: STM32 (Device Tree Support)
> [<c010a45d>] (unwind_backtrace) from [<c010766f>] (show_stack+0xb/0xc)
> [<c010766f>] (show_stack) from [<c06353ab>] (dump_stack+0x6f/0x84)
> [<c06353ab>] (dump_stack) from [<c01145e3>] (__warn+0x7f/0xa4)
> [<c01145e3>] (__warn) from [<c063386f>] (warn_slowpath_fmt+0x3b/0x74)
> [<c063386f>] (warn_slowpath_fmt) from [<c063b43d>] (__rt_mutex_trylock+0x37/0x62)
> [<c063b43d>] (__rt_mutex_trylock) from [<c063c053>] (rt_spin_trylock+0x7/0x16)
> [<c063c053>] (rt_spin_trylock) from [<c036a2f3>] (clk_enable_lock+0xb/0x80)
> [<c036a2f3>] (clk_enable_lock) from [<c036ba69>] (clk_core_enable_lock+0x9/0x18)
> [<c036ba69>] (clk_core_enable_lock) from [<c034e9f3>] (stm32_gpio_get+0x11/0x24)
> [<c034e9f3>] (stm32_gpio_get) from [<c034ef43>] (stm32_gpio_irq_trigger+0x1f/0x48)
> [<c034ef43>] (stm32_gpio_irq_trigger) from [<c014aa53>] (handle_fasteoi_irq+0x71/0xa8)
> [<c014aa53>] (handle_fasteoi_irq) from [<c0147111>] (generic_handle_irq+0x19/0x22)
> [<c0147111>] (generic_handle_irq) from [<c014752d>] (__handle_domain_irq+0x55/0x64)
> [<c014752d>] (__handle_domain_irq) from [<c0346f13>] (gic_handle_irq+0x53/0x64)
> [<c0346f13>] (gic_handle_irq) from [<c0100ba5>] (__irq_svc+0x65/0xc0)
> Exception stack(0xc0e01f18 to 0xc0e01f60)
> 1f00: 0000300c 00000000
> 1f20: 0000300c c010ff01 00000000 00000000 c0e00000 c0e07714 00000001 c0e01f78
> 1f40: c0e07758 00000000 ef7cd0ff c0e01f68 c010554b c0105542 40000033 ffffffff
> [<c0100ba5>] (__irq_svc) from [<c0105542>] (arch_cpu_idle+0xc/0x1e)
> [<c0105542>] (arch_cpu_idle) from [<c063be95>] (default_idle_call+0x21/0x3c)
> [<c063be95>] (default_idle_call) from [<c01324f7>] (do_idle+0xe3/0x1e4)
> [<c01324f7>] (do_idle) from [<c01327b3>] (cpu_startup_entry+0x13/0x14)
> [<c01327b3>] (cpu_startup_entry) from [<c0a00c13>] (start_kernel+0x397/0x3d4)
> [<c0a00c13>] (start_kernel) from [<00000000>] (0x0)
> ---[ end trace 0000000000000002 ]---
>
> Power consumption measured on STM32MP157C DHCOM SoM is not increased or
> is below noise threshold.
>
> Fixes: 47beed513a85b ("pinctrl: stm32: Add level interrupt support to gpio irq chip")
> Signed-off-by: Marek Vasut <marex at denx.de>
Reviewed-by: Fabien Dessenne <fabien.dessenne at foss.st.com>
> Cc: Alexandre Torgue <alexandre.torgue at foss.st.com>
> Cc: Fabien Dessenne <fabien.dessenne at foss.st.com>
> Cc: Linus Walleij <linus.walleij at linaro.org>
> Cc: Marc Zyngier <maz at kernel.org>
> Cc: linux-stm32 at st-md-mailman.stormreply.com
> Cc: linux-arm-kernel at lists.infradead.org
> To: linux-gpio at vger.kernel.org
> ---
> drivers/pinctrl/stm32/pinctrl-stm32.c | 19 +++++++++++++++++--
> 1 file changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pinctrl/stm32/pinctrl-stm32.c b/drivers/pinctrl/stm32/pinctrl-stm32.c
> index 242d1c37c6e4..7aecd0efde07 100644
> --- a/drivers/pinctrl/stm32/pinctrl-stm32.c
> +++ b/drivers/pinctrl/stm32/pinctrl-stm32.c
> @@ -226,6 +226,13 @@ static void stm32_gpio_free(struct gpio_chip *chip, unsigned offset)
> pinctrl_gpio_free(chip->base + offset);
> }
>
> +static int stm32_gpio_get_noclk(struct gpio_chip *chip, unsigned int offset)
> +{
> + struct stm32_gpio_bank *bank = gpiochip_get_data(chip);
> +
> + return !!(readl_relaxed(bank->base + STM32_GPIO_IDR) & BIT(offset));
> +}
> +
> static int stm32_gpio_get(struct gpio_chip *chip, unsigned offset)
> {
> struct stm32_gpio_bank *bank = gpiochip_get_data(chip);
> @@ -233,7 +240,7 @@ static int stm32_gpio_get(struct gpio_chip *chip, unsigned offset)
>
> clk_enable(bank->clk);
>
> - ret = !!(readl_relaxed(bank->base + STM32_GPIO_IDR) & BIT(offset));
> + ret = stm32_gpio_get_noclk(chip, offset);
>
> clk_disable(bank->clk);
>
> @@ -317,7 +324,7 @@ static void stm32_gpio_irq_trigger(struct irq_data *d)
> return;
>
> /* If level interrupt type then retrig */
> - level = stm32_gpio_get(&bank->gpio_chip, d->hwirq);
> + level = stm32_gpio_get_noclk(&bank->gpio_chip, d->hwirq);
> if ((level == 0 && bank->irq_type[d->hwirq] == IRQ_TYPE_LEVEL_LOW) ||
> (level == 1 && bank->irq_type[d->hwirq] == IRQ_TYPE_LEVEL_HIGH))
> irq_chip_retrigger_hierarchy(d);
> @@ -359,6 +366,7 @@ static int stm32_gpio_irq_request_resources(struct irq_data *irq_data)
> {
> struct stm32_gpio_bank *bank = irq_data->domain->host_data;
> struct stm32_pinctrl *pctl = dev_get_drvdata(bank->gpio_chip.parent);
> + unsigned long flags;
> int ret;
>
> ret = stm32_gpio_direction_input(&bank->gpio_chip, irq_data->hwirq);
> @@ -372,6 +380,10 @@ static int stm32_gpio_irq_request_resources(struct irq_data *irq_data)
> return ret;
> }
>
> + flags = irqd_get_trigger_type(irq_data);
> + if (flags & IRQ_TYPE_LEVEL_MASK)
> + clk_enable(bank->clk);
> +
> return 0;
> }
>
> @@ -379,6 +391,9 @@ static void stm32_gpio_irq_release_resources(struct irq_data *irq_data)
> {
> struct stm32_gpio_bank *bank = irq_data->domain->host_data;
>
> + if (bank->irq_type[irq_data->hwirq] & IRQ_TYPE_LEVEL_MASK)
> + clk_disable(bank->clk);
> +
> gpiochip_unlock_as_irq(&bank->gpio_chip, irq_data->hwirq);
> }
>
More information about the linux-arm-kernel
mailing list