* [PATCH AUTOSEL 5.10 10/28] pinctrl: amd: Fix wakeups when IRQ is shared with SCI [not found] <20211126023343.442045-1-sashal@kernel.org> @ 2021-11-26 2:33 ` Sasha Levin 2021-11-29 14:47 ` Limonciello, Mario 2021-11-26 2:33 ` [PATCH AUTOSEL 5.10 11/28] pinctrl: qcom: fix unmet dependencies on GPIOLIB for GPIOLIB_IRQCHIP Sasha Levin 1 sibling, 1 reply; 5+ messages in thread From: Sasha Levin @ 2021-11-26 2:33 UTC (permalink / raw) To: linux-kernel, stable Cc: Mario Limonciello, Joerie de Gram, Basavaraj Natikar, Linus Walleij, Sasha Levin, Shyam-sundar.S-k, linux-gpio 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://gitlab.freedesktop.org/drm/amd/-/issues/1738 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://lore.kernel.org/r/20211101014853.6177-2-mario.limonciello@amd.com 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 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* RE: [PATCH AUTOSEL 5.10 10/28] pinctrl: amd: Fix wakeups when IRQ is shared with SCI 2021-11-26 2:33 ` [PATCH AUTOSEL 5.10 10/28] pinctrl: amd: Fix wakeups when IRQ is shared with SCI Sasha Levin @ 2021-11-29 14:47 ` Limonciello, Mario 2021-11-29 18:53 ` Limonciello, Mario 0 siblings, 1 reply; 5+ messages in thread From: Limonciello, Mario @ 2021-11-29 14:47 UTC (permalink / raw) To: Sasha Levin, linux-kernel@vger.kernel.org, stable@vger.kernel.org Cc: Joerie de Gram, Natikar, Basavaraj, Linus Walleij, S-k, Shyam-sundar, linux-gpio@vger.kernel.org > -----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 <Basavaraj.Natikar@amd.com>; > 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. Thanks, ^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH AUTOSEL 5.10 10/28] pinctrl: amd: Fix wakeups when IRQ is shared with SCI 2021-11-29 14:47 ` Limonciello, Mario @ 2021-11-29 18:53 ` Limonciello, Mario 2021-12-03 18:25 ` Sasha Levin 0 siblings, 1 reply; 5+ messages in thread From: Limonciello, Mario @ 2021-11-29 18:53 UTC (permalink / raw) To: Sasha Levin, linux-kernel@vger.kernel.org, stable@vger.kernel.org Cc: Joerie de Gram, Natikar, Basavaraj, Linus Walleij, S-k, Shyam-sundar, linux-gpio@vger.kernel.org > -----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 > <Basavaraj.Natikar@amd.com>; > > 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. 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, ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH AUTOSEL 5.10 10/28] pinctrl: amd: Fix wakeups when IRQ is shared with SCI 2021-11-29 18:53 ` Limonciello, Mario @ 2021-12-03 18:25 ` Sasha Levin 0 siblings, 0 replies; 5+ messages in thread From: Sasha Levin @ 2021-12-03 18:25 UTC (permalink / raw) To: Limonciello, Mario Cc: linux-kernel@vger.kernel.org, stable@vger.kernel.org, Joerie de Gram, Natikar, Basavaraj, Linus Walleij, S-k, Shyam-sundar, linux-gpio@vger.kernel.org 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 >> <Basavaraj.Natikar@amd.com>; >> > 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! -- Thanks, Sasha ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH AUTOSEL 5.10 11/28] pinctrl: qcom: fix unmet dependencies on GPIOLIB for GPIOLIB_IRQCHIP [not found] <20211126023343.442045-1-sashal@kernel.org> 2021-11-26 2:33 ` [PATCH AUTOSEL 5.10 10/28] pinctrl: amd: Fix wakeups when IRQ is shared with SCI Sasha Levin @ 2021-11-26 2:33 ` Sasha Levin 1 sibling, 0 replies; 5+ messages in thread From: Sasha Levin @ 2021-11-26 2:33 UTC (permalink / raw) To: linux-kernel, stable Cc: Julian Braha, Linus Walleij, Sasha Levin, agross, bjorn.andersson, linux-arm-msm, linux-gpio From: Julian Braha <julianbraha@gmail.com> [ Upstream commit 60430d4c4eddcdf8eac2bdbec9704f84a436eedf ] When PINCTRL_QCOM_SPMI_PMIC or PINCTRL_QCOM_SSBI_PMIC is selected, and GPIOLIB is not selected, Kbuild gives the following warnings: WARNING: unmet direct dependencies detected for GPIOLIB_IRQCHIP Depends on [n]: GPIOLIB [=n] Selected by [y]: - PINCTRL_QCOM_SPMI_PMIC [=y] && PINCTRL [=y] && (ARCH_QCOM [=n] || COMPILE_TEST [=y]) && OF [=y] && SPMI [=y] WARNING: unmet direct dependencies detected for GPIOLIB_IRQCHIP Depends on [n]: GPIOLIB [=n] Selected by [y]: - PINCTRL_QCOM_SSBI_PMIC [=y] && PINCTRL [=y] && (ARCH_QCOM [=n] || COMPILE_TEST [=y]) && OF [=y] This is because these config options enable GPIOLIB_IRQCHIP without selecting or depending on GPIOLIB, despite GPIOLIB_IRQCHIP depending on GPIOLIB. These unmet dependency bugs were detected by Kismet, a static analysis tool for Kconfig. Please advise if this is not the appropriate solution. Signed-off-by: Julian Braha <julianbraha@gmail.com> Link: https://lore.kernel.org/r/20211029004610.35131-1-julianbraha@gmail.com Signed-off-by: Linus Walleij <linus.walleij@linaro.org> Signed-off-by: Sasha Levin <sashal@kernel.org> --- drivers/pinctrl/qcom/Kconfig | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/pinctrl/qcom/Kconfig b/drivers/pinctrl/qcom/Kconfig index 5fe7b8aaf69d8..a209eb1f189ab 100644 --- a/drivers/pinctrl/qcom/Kconfig +++ b/drivers/pinctrl/qcom/Kconfig @@ -169,6 +169,7 @@ config PINCTRL_QCOM_SPMI_PMIC select PINMUX select PINCONF select GENERIC_PINCONF + select GPIOLIB select GPIOLIB_IRQCHIP select IRQ_DOMAIN_HIERARCHY help @@ -183,6 +184,7 @@ config PINCTRL_QCOM_SSBI_PMIC select PINMUX select PINCONF select GENERIC_PINCONF + select GPIOLIB select GPIOLIB_IRQCHIP select IRQ_DOMAIN_HIERARCHY help -- 2.33.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-12-03 18:25 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20211126023343.442045-1-sashal@kernel.org>
2021-11-26 2:33 ` [PATCH AUTOSEL 5.10 10/28] pinctrl: amd: Fix wakeups when IRQ is shared with SCI Sasha Levin
2021-11-29 14:47 ` Limonciello, Mario
2021-11-29 18:53 ` Limonciello, Mario
2021-12-03 18:25 ` Sasha Levin
2021-11-26 2:33 ` [PATCH AUTOSEL 5.10 11/28] pinctrl: qcom: fix unmet dependencies on GPIOLIB for GPIOLIB_IRQCHIP Sasha Levin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).