> -----Original Message----- > From: Limonciello, Mario > Sent: Monday, November 29, 2021 08:48 > To: Sasha Levin <sashal@xxxxxxxxxx>; linux-kernel@xxxxxxxxxxxxxxx; > stable@xxxxxxxxxxxxxxx > Cc: Joerie de Gram <j.de.gram@xxxxxxxxx>; Natikar, Basavaraj > <Basavaraj.Natikar@xxxxxxx>; Linus Walleij <linus.walleij@xxxxxxxxxx>; S-k, > Shyam-sundar <Shyam-sundar.S-k@xxxxxxx>; linux-gpio@xxxxxxxxxxxxxxx > Subject: RE: [PATCH AUTOSEL 5.10 10/28] pinctrl: amd: Fix wakeups when IRQ > is shared with SCI > > > > > -----Original Message----- > > From: Sasha Levin <sashal@xxxxxxxxxx> > > Sent: Thursday, November 25, 2021 20:33 > > To: linux-kernel@xxxxxxxxxxxxxxx; stable@xxxxxxxxxxxxxxx > > Cc: Limonciello, Mario <Mario.Limonciello@xxxxxxx>; Joerie de Gram > > <j.de.gram@xxxxxxxxx>; Natikar, Basavaraj > <Basavaraj.Natikar@xxxxxxx>; > > Linus Walleij <linus.walleij@xxxxxxxxxx>; Sasha Levin <sashal@xxxxxxxxxx>; > S- > > k, Shyam-sundar <Shyam-sundar.S-k@xxxxxxx>; linux- > > gpio@xxxxxxxxxxxxxxx > > Subject: [PATCH AUTOSEL 5.10 10/28] pinctrl: amd: Fix wakeups when IRQ > is > > shared with SCI > > > > From: Mario Limonciello <mario.limonciello@xxxxxxx> > > > > [ 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@xxxxxxxxx> > > Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx> > > Acked-by: Basavaraj Natikar <Basavaraj.Natikar@xxxxxxx> > > 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@xxxxxxxxxx> > > Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx> > > --- > > 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. 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. > > Thanks,
