lkml.org 
[lkml]   [2019]   [Feb]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [RFC] pinctrl/amd: Clear interrupt enable bits on probe
From
Date
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;
>

\
 
 \ /
  Last update: 2019-02-19 14:23    [W:1.183 / U:0.008 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site