FAILED: patch "[PATCH] pinctrl: qcom: Assign irq_eoi conditionally" failed to apply to 5.4-stable tree

PATCH [email protected] 2 participants 3 messages Mar 16, 2020 – Mar 18, 2020

Subsystems (5)

Stable branch Arm Qualcomm mailing list Pin control Pin controller - Qualcomm +1 more subsystems

lore

Share & tools

lore

[email protected]

author maintainer

2020-03-16 11:34 UTC #0 lore raw

The patch below does not apply to the 5.4-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From 1cada2f307665e208a486d7ac2294ed9a6f74a6f Mon Sep 17 00:00:00 2001
From: Linus Walleij <[email protected]>
Date: Mon, 9 Mar 2020 16:26:04 +0100
Subject: [PATCH] pinctrl: qcom: Assign irq_eoi conditionally

The hierarchical parts of MSM pinctrl/GPIO is only used when the device tree has a "wakeup-parent" as a phandle, but the .irq_eoi is anyway assigned leading to semantic problems on elder Qualcomm chipsets.

When the drivers/mfd/qcom-pm8xxx.c driver calls chained_irq_exit() that call will in turn call chip->irq_eoi() which is set to irq_chip_eoi_parent() by default on a hierachical IRQ chip, and the parent is pinctrl-msm.c so that will in turn unconditionally call irq_chip_eoi_parent() again, but its parent is invalid so we get the following crash:

Unnable to handle kernel NULL pointer dereference at virtual address 00000010
pgd = (ptrval)
[00000010] *pgd=00000000

 Internal error: Oops: 5 [#1] PREEMPT SMP ARM
 (...)
 PC is at irq_chip_eoi_parent+0x4/0x10
 LR is at pm8xxx_irq_handler+0x1b4/0x2d8

If we solve this crash by avoiding to call up to irq_chip_eoi_parent(), the machine will hang and get reset by the watchdog, because of semantic issues, probably inside irq_chip.

As a solution, just assign the .irq_eoi conditionally if we are actually using a wakeup parent.

Cc David Heidelberg <[email protected]>

Cc Bjorn Andersson <[email protected]>

Cc Lina Iyer <[email protected]>

Cc Stephen Boyd <[email protected]>

Cc [email protected]

Fixes e35a6ae0eb3a ("pinctrl/msm: Setup GPIO chip in hierarchy")

Link https://lore.kernel.org/r/[email protected] open here

Tested-by David Heidelberg <[email protected]>

Acked-by Marc Zyngier <[email protected]>

Signed-off-by Linus Walleij <[email protected]>

drivers/pinctrl/qcom/pinctrl-msm.c +1 -2

index 9a8daa256a32..1a948c3f54b7 100644
@@ -1104,7 +1104,6 @@ static int msm_gpio_init(struct msm_pinctrl *pctrl)
 	pctrl->irq_chip.irq_mask = msm_gpio_irq_mask;
 	pctrl->irq_chip.irq_unmask = msm_gpio_irq_unmask;
 	pctrl->irq_chip.irq_ack = msm_gpio_irq_ack;
-	pctrl->irq_chip.irq_eoi = irq_chip_eoi_parent;
 	pctrl->irq_chip.irq_set_type = msm_gpio_irq_set_type;
 	pctrl->irq_chip.irq_set_wake = msm_gpio_irq_set_wake;
 	pctrl->irq_chip.irq_request_resources = msm_gpio_irq_reqres;

@@ -1118,7 +1117,7 @@ static int msm_gpio_init(struct msm_pinctrl *pctrl)
 		if (!chip->irq.parent_domain)
 			return -EPROBE_DEFER;
 		chip->irq.child_to_parent_hwirq = msm_gpio_wakeirq;
-
+		pctrl->irq_chip.irq_eoi = irq_chip_eoi_parent;
 		/*
 		 * Let's skip handling the GPIOs, if the parent irqchip
 		 * is handling the direct connect IRQ of the GPIO.

Sasha Levin

maintainer

2020-03-18 16:40 UTC #1 lore raw

Quoted 47 lines · On Mon, Mar 16, 2020 at 12:34:09PM +0100, [email protected] wrote:

The patch below does not apply to the 5.4-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
From 1cada2f307665e208a486d7ac2294ed9a6f74a6f Mon Sep 17 00:00:00 2001
From: Linus Walleij <[email protected]>
Date: Mon, 9 Mar 2020 16:26:04 +0100
Subject: [PATCH] pinctrl: qcom: Assign irq_eoi conditionally
The hierarchical parts of MSM pinctrl/GPIO is only used when the device tree has a "wakeup-parent" as a phandle, but the .irq_eoi is anyway assigned leading to semantic problems on elder Qualcomm chipsets.
When the drivers/mfd/qcom-pm8xxx.c driver calls chained_irq_exit() that call will in turn call chip->irq_eoi() which is set to irq_chip_eoi_parent() by default on a hierachical IRQ chip, and the parent is pinctrl-msm.c so that will in turn unconditionally call irq_chip_eoi_parent() again, but its parent is invalid so we get the following crash:
Unnable to handle kernel NULL pointer dereference at virtual address 00000010
pgd = (ptrval)
[00000010] *pgd=00000000
Internal error: Oops: 5 [#1] PREEMPT SMP ARM
(...)
PC is at irq_chip_eoi_parent+0x4/0x10
LR is at pm8xxx_irq_handler+0x1b4/0x2d8
If we solve this crash by avoiding to call up to irq_chip_eoi_parent(), the machine will hang and get reset by the watchdog, because of semantic issues, probably inside irq_chip.
As a solution, just assign the .irq_eoi conditionally if we are actually using a wakeup parent.

Quoted 6 lines

Cc David Heidelberg <[email protected]>
Cc Bjorn Andersson <[email protected]>
Cc Lina Iyer <[email protected]>
Cc Stephen Boyd <[email protected]>
Cc [email protected]
Fixes e35a6ae0eb3a ("pinctrl/msm: Setup GPIO chip in hierarchy")

This shouldn't go in 5.4, e35a6ae0eb3a is not there.

Signature · 2 lines

Thanks,
Sasha

Greg KH

author maintainer

2020-03-18 16:53 UTC #2 lore raw

Quoted 48 lines · Sasha Levin

Quoted 47 lines · On Mon, Mar 16, 2020 at 12:34:09PM +0100, [email protected] wrote:
The patch below does not apply to the 5.4-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
Quoted 1 line
From 1cada2f307665e208a486d7ac2294ed9a6f74a6f Mon Sep 17 00:00:00 2001
From: Linus Walleij <[email protected]>
Date: Mon, 9 Mar 2020 16:26:04 +0100
Subject: [PATCH] pinctrl: qcom: Assign irq_eoi conditionally
The hierarchical parts of MSM pinctrl/GPIO is only used when the device tree has a "wakeup-parent" as a phandle, but the .irq_eoi is anyway assigned leading to semantic problems on elder Qualcomm chipsets.
When the drivers/mfd/qcom-pm8xxx.c driver calls chained_irq_exit() that call will in turn call chip->irq_eoi() which is set to irq_chip_eoi_parent() by default on a hierachical IRQ chip, and the parent is pinctrl-msm.c so that will in turn unconditionally call irq_chip_eoi_parent() again, but its parent is invalid so we get the following crash:
Unnable to handle kernel NULL pointer dereference at virtual address 00000010
pgd = (ptrval)
[00000010] *pgd=00000000
Internal error: Oops: 5 [#1] PREEMPT SMP ARM
(...)
PC is at irq_chip_eoi_parent+0x4/0x10
LR is at pm8xxx_irq_handler+0x1b4/0x2d8
If we solve this crash by avoiding to call up to irq_chip_eoi_parent(), the machine will hang and get reset by the watchdog, because of semantic issues, probably inside irq_chip.
As a solution, just assign the .irq_eoi conditionally if we are actually using a wakeup parent.

Quoted 8 lines

Quoted 6 lines
Cc David Heidelberg <[email protected]>
Cc Bjorn Andersson <[email protected]>
Cc Lina Iyer <[email protected]>
Cc Stephen Boyd <[email protected]>
Cc [email protected]
Fixes e35a6ae0eb3a ("pinctrl/msm: Setup GPIO chip in hierarchy")
This shouldn't go in 5.4, e35a6ae0eb3a is not there.

Ah, grep failed me, there is another reference to that commit id in a 5.4.y release.

thanks,

greg k-h