On Mon, Nov 29, 2021 at 06:53:01PM +0000, Limonciello, Mario wrote:
-----Original Message----- From: Limonciello, Mario Sent: Monday, November 29, 2021 08:48 To: Sasha Levin sashal@kernel.org; linux-kernel@vger.kernel.org; stable@vger.kernel.org Cc: Joerie de Gram j.de.gram@gmail.com; Natikar, Basavaraj Basavaraj.Natikar@amd.com; Linus Walleij linus.walleij@linaro.org; S-k, Shyam-sundar Shyam-sundar.S-k@amd.com; linux-gpio@vger.kernel.org Subject: RE: [PATCH AUTOSEL 5.10 10/28] pinctrl: amd: Fix wakeups when IRQ is shared with SCI
-----Original Message----- From: Sasha Levin sashal@kernel.org Sent: Thursday, November 25, 2021 20:33 To: linux-kernel@vger.kernel.org; stable@vger.kernel.org Cc: Limonciello, Mario Mario.Limonciello@amd.com; Joerie de Gram j.de.gram@gmail.com; Natikar, Basavaraj
Linus Walleij linus.walleij@linaro.org; Sasha Levin sashal@kernel.org;
S-
k, Shyam-sundar Shyam-sundar.S-k@amd.com; linux- gpio@vger.kernel.org Subject: [PATCH AUTOSEL 5.10 10/28] pinctrl: amd: Fix wakeups when IRQ
is
shared with SCI
From: Mario Limonciello mario.limonciello@amd.com
[ Upstream commit 2d54067fcd23aae61e23508425ae5b29e973573d ]
On some Lenovo AMD Gen2 platforms the IRQ for the SCI and pinctrl
drivers
are shared. Due to how the s2idle loop handling works, this case needs an extra explicit check whether the interrupt was caused by SCI or by the GPIO controller.
To fix this rework the existing IRQ handler function to function as a checker and an IRQ handler depending on the calling arguments.
BugLink:
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitla
b.freedesktop.org%2Fdrm%2Famd%2F-
%2Fissues%2F1738&data=04%7C01%7Cmario.limonciello%40amd.com%
7Ce14b7ddf8d1143b6649208d9b0853519%7C3dd8961fe4884e608e11a82d994
e183d%7C0%7C0%7C637734908448086367%7CUnknown%7CTWFpbGZsb3d8
eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3
D%7C3000&sdata=BHHY3gLu2pQIJ1nvSE0VNQOaioTC0QdBM44ze3HXrtk
%3D&reserved=0 Reported-by: Joerie de Gram j.de.gram@gmail.com Signed-off-by: Mario Limonciello mario.limonciello@amd.com Acked-by: Basavaraj Natikar Basavaraj.Natikar@amd.com Link:
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.
kernel.org%2Fr%2F20211101014853.6177-2-
mario.limonciello%40amd.com&data=04%7C01%7Cmario.limonciello%4
0amd.com%7Ce14b7ddf8d1143b6649208d9b0853519%7C3dd8961fe4884e608
e11a82d994e183d%7C0%7C0%7C637734908448086367%7CUnknown%7CTWF
pbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXV
CI6Mn0%3D%7C3000&sdata=zUkTF851CdUmrY1z3YZbYrnVrjHQaVfb1%
2Bg2dx28ZNA%3D&reserved=0 Signed-off-by: Linus Walleij linus.walleij@linaro.org Signed-off-by: Sasha Levin sashal@kernel.org
drivers/pinctrl/pinctrl-amd.c | 29 ++++++++++++++++++++++++++--- 1 file changed, 26 insertions(+), 3 deletions(-)
diff --git a/drivers/pinctrl/pinctrl-amd.c b/drivers/pinctrl/pinctrl-amd.c index e20bcc835d6a8..54dfa0244422c 100644 --- a/drivers/pinctrl/pinctrl-amd.c +++ b/drivers/pinctrl/pinctrl-amd.c @@ -520,14 +520,14 @@ static struct irq_chip amd_gpio_irqchip = {
#define PIN_IRQ_PENDING (BIT(INTERRUPT_STS_OFF) | BIT(WAKE_STS_OFF))
-static irqreturn_t amd_gpio_irq_handler(int irq, void *dev_id) +static bool do_amd_gpio_irq_handler(int irq, void *dev_id) { struct amd_gpio *gpio_dev = dev_id; struct gpio_chip *gc = &gpio_dev->gc;
- irqreturn_t ret = IRQ_NONE; unsigned int i, irqnr; unsigned long flags; u32 __iomem *regs;
- bool ret = false; u32 regval; u64 status, mask;
@@ -549,6 +549,14 @@ static irqreturn_t amd_gpio_irq_handler(int irq,
void
*dev_id) /* Each status bit covers four pins */ for (i = 0; i < 4; i++) { regval = readl(regs + i);
/* caused wake on resume context for shared IRQ */if (irq < 0 && (regval & BIT(WAKE_STS_OFF))) {dev_dbg(&gpio_dev->pdev->dev,"Waking due to GPIO %d: 0x%x",irqnr + i, regval);return true;}if (!(regval & PIN_IRQ_PENDING) || !(regval & BIT(INTERRUPT_MASK_OFF))) continue;@@ -574,9 +582,12 @@ static irqreturn_t amd_gpio_irq_handler(int irq,
void
*dev_id) } writel(regval, regs + i); raw_spin_unlock_irqrestore(&gpio_dev->lock, flags);
ret = IRQ_HANDLED;
ret = true;} }
/* did not cause wake on resume context for shared IRQ */
if (irq < 0)
return false;/* Signal EOI to the GPIO unit */ raw_spin_lock_irqsave(&gpio_dev->lock, flags);
@@ -588,6 +599,16 @@ static irqreturn_t amd_gpio_irq_handler(int irq,
void
*dev_id) return ret; }
+static irqreturn_t amd_gpio_irq_handler(int irq, void *dev_id) +{
- return IRQ_RETVAL(do_amd_gpio_irq_handler(irq, dev_id));
+}
+static bool __maybe_unused amd_gpio_check_wake(void *dev_id) +{
- return do_amd_gpio_irq_handler(-1, dev_id);
+}
static int amd_get_groups_count(struct pinctrl_dev *pctldev) { struct amd_gpio *gpio_dev = pinctrl_dev_get_drvdata(pctldev); @@ -958,6 +979,7 @@ static int amd_gpio_probe(struct platform_device *pdev) goto out2;
platform_set_drvdata(pdev, gpio_dev);
- acpi_register_wakeup_handler(gpio_dev->irq,
amd_gpio_check_wake, gpio_dev);
dev_dbg(&pdev->dev, "amd gpio driver loaded\n"); return ret; @@ -975,6 +997,7 @@ static int amd_gpio_remove(struct platform_device *pdev) gpio_dev = platform_get_drvdata(pdev);
gpiochip_remove(&gpio_dev->gc);
- acpi_unregister_wakeup_handler(amd_gpio_check_wake,
gpio_dev);
return 0; } -- 2.33.0
Sasha,
No concerns for me to 5.10, but would you mind also sending this to 5.14.y and 5.15.y too? I didn't see it sent up for either.
5.14 was EOL at that time.
One more thought on this - please also take this (which wasn't part of autosel) when this comes back:
e9380df85187 ACPI: Add stubs for wakeup handler functions
That prevents some compile errors in certain configurations.
I'll grab it, thanks!