Messages in this thread | | | Subject | Re: [RFC] pinctrl/amd: Clear interrupt enable bits on probe | From | Hans de Goede <> | Date | Tue, 19 Feb 2019 14:23:03 +0100 |
| |
Hi,
On 19-02-19 11:59, Leonard Crestez wrote: > My Acer Nitro 5 AN515-42 laptop with a Ryzen 2700U hangs on boot because > of spurious interrupts from pinctrl-amd. > > This seems to happen because the touchpad interrupt is enabled on boot > in "level" mode and there is no way to clear it until a touchpad driver > probes. > > Fix by disabling all gpio interrupts at probe time until they are > explicitly requested by drivers. > > Signed-off-by: Leonard Crestez <cdleonard@gmail.com> > > --- > > It's strange that nobody else has run into this problem, AMD hardware is > relatively common. Maybe firmware generally disables GPIO interrupts > itself?
Others have run into the problem, but no-one so far has had both time and access to the hardware to actually get to the bottom of this, see: https://bugzilla.kernel.org/show_bug.cgi?id=201817#c4
Your patch looks like it is exactly what is needed to fix this, thank you very much for the patch!
I will attach a copy of your patch to that bug and ask people who are having similar problems to test it.
Regards,
Hans
> > This patch fixes boot but this same laptop has other issues: > > * Suspend is broken > * Ethernet is broken (only sometimes) > * The CPU freq gets stuck at 400 Mhz (sometimes) > > Those issues happen on maybe 80% of boots without a clear pattern. It > seems that inserting/removing the ethernet jack during boot helps > cpufreq? It's possible that these problems are also caused by pin > misconfiguration so this fix might be incomplete. > > When the cpufreq issue happens `rdmsr 0xc0010061 -a` shows 0x22 for all > cpus; maybe this is some broken thermal throttling? > > Also, perhaps amd_gpio_irq_disable_all should enumerate in a nicer less > verbose way? > > Previously: https://lore.kernel.org/patchwork/patch/1028047/ > > diff --git a/drivers/pinctrl/pinctrl-amd.c b/drivers/pinctrl/pinctrl-amd.c > index 2a7d638978d8..3cb7ea46f32c 100644 > --- a/drivers/pinctrl/pinctrl-amd.c > +++ b/drivers/pinctrl/pinctrl-amd.c > @@ -592,10 +592,53 @@ static irqreturn_t amd_gpio_irq_handler(int irq, void *dev_id) > raw_spin_unlock_irqrestore(&gpio_dev->lock, flags); > > return ret; > } > > +static void amd_gpio_irq_disable_all(struct amd_gpio *gpio_dev) > +{ > + unsigned int bank, i, pin_num; > + u32 regval; > + > + for (bank = 0; bank < gpio_dev->hwbank_num; bank++) { > + switch (bank) { > + case 0: > + i = 0; > + pin_num = AMD_GPIO_PINS_BANK0; > + break; > + case 1: > + i = 64; > + pin_num = AMD_GPIO_PINS_BANK1 + i; > + break; > + case 2: > + i = 128; > + pin_num = AMD_GPIO_PINS_BANK2 + i; > + break; > + case 3: > + i = 192; > + pin_num = AMD_GPIO_PINS_BANK3 + i; > + break; > + default: > + /* Illegal bank number, ignore */ > + continue; > + } > + > + for (; i < pin_num; i++) { > + unsigned long flags; > + raw_spin_lock_irqsave(&gpio_dev->lock, flags); > + regval = readl(gpio_dev->base + i * 4); > + if (regval & BIT(INTERRUPT_ENABLE_OFF)) { > + dev_info(&gpio_dev->pdev->dev, > + "Pin %d interrupt enabled on boot: disable\n", i); > + regval &= ~BIT(INTERRUPT_ENABLE_OFF); > + writel(regval, gpio_dev->base + i * 4); > + } > + raw_spin_unlock_irqrestore(&gpio_dev->lock, flags); > + } > + } > +} > + > static int amd_get_groups_count(struct pinctrl_dev *pctldev) > { > struct amd_gpio *gpio_dev = pinctrl_dev_get_drvdata(pctldev); > > return gpio_dev->ngroups; > @@ -910,10 +953,12 @@ static int amd_gpio_probe(struct platform_device *pdev) > > ret = gpiochip_add_data(&gpio_dev->gc, gpio_dev); > if (ret) > return ret; > > + amd_gpio_irq_disable_all(gpio_dev); > + > ret = gpiochip_add_pin_range(&gpio_dev->gc, dev_name(&pdev->dev), > 0, 0, gpio_dev->gc.ngpio); > if (ret) { > dev_err(&pdev->dev, "Failed to add pin range\n"); > goto out2; >
| |