linux-kernel - RE: [PATCH 4/4] pinctrl: amd: Revert "pinctrl: amd: disable and mask interrupts on probe"

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <MN0PR12MB61017C1D19E64CE24BD81E8AE26B9@MN0PR12MB6101.namprd12.prod.outlook.com>
Date:   Fri, 28 Apr 2023 13:54:18 +0000
From:   "Limonciello, Mario" <Mario.Limonciello@....com>
To:     "nakato@...ato.io" <nakato@...ato.io>
CC:     "korneld@...omium.org" <korneld@...omium.org>,
        "Gong, Richard" <Richard.Gong@....com>,
        "linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linus.walleij@...aro.org" <linus.walleij@...aro.org>,
        "Natikar, Basavaraj" <Basavaraj.Natikar@....com>,
        "S-k, Shyam-sundar" <Shyam-sundar.S-k@....com>
Subject: RE: [PATCH 4/4] pinctrl: amd: Revert "pinctrl: amd: disable and mask
 interrupts on probe"

[Public]

> commit 4e5a04be88fe ("pinctrl: amd: disable and mask interrupts on probe")
> was well intentioned to mask a firmware issue on a surface laptop, but it
> has a few problems:
> 1. It had a bug in the loop handling for iteration 63 that lead to other
>    problems with GPIO0 handling.
> 2. It disables interrupts that are used internally by the SOC but masked
>    by default.
> 3. It masked a real firmware problem in some chromebooks that should have
>    been caught during development but wasn't.
> 
> There has been a lot of other development around s2idle; particularly
> around handling of the spurious wakeups.  If there is still a problem on
> the original reported surface laptop it should be avoided by adding a quirk
> to gpiolib-acpi for that system instead.
> 
> Signed-off-by: Mario Limonciello <mario.limonciello@....com>

Sachi,

Any feedback on this series, particularly on this patch?  I'd like to understand
what GPIO pins prompted the original patch and if 3/4 of this series helps the
original behavior so that we can have confirmation the revert is not going to
cause a regression for you.

Thanks,

> ---
>  drivers/pinctrl/pinctrl-amd.c | 31 -------------------------------
>  1 file changed, 31 deletions(-)
> 
> diff --git a/drivers/pinctrl/pinctrl-amd.c b/drivers/pinctrl/pinctrl-amd.c
> index 675c9826b78a..e9fef2391b38 100644
> --- a/drivers/pinctrl/pinctrl-amd.c
> +++ b/drivers/pinctrl/pinctrl-amd.c
> @@ -877,34 +877,6 @@ static const struct pinconf_ops amd_pinconf_ops = {
>  	.pin_config_group_set = amd_pinconf_group_set,
>  };
> 
> -static void amd_gpio_irq_init(struct amd_gpio *gpio_dev)
> -{
> -	struct pinctrl_desc *desc = gpio_dev->pctrl->desc;
> -	unsigned long flags;
> -	u32 pin_reg, mask;
> -	int i;
> -
> -	mask = BIT(WAKE_CNTRL_OFF_S0I3) | BIT(WAKE_CNTRL_OFF_S3) |
> -		BIT(INTERRUPT_MASK_OFF) | BIT(INTERRUPT_ENABLE_OFF)
> |
> -		BIT(WAKE_CNTRL_OFF_S4);
> -
> -	for (i = 0; i < desc->npins; i++) {
> -		int pin = desc->pins[i].number;
> -		const struct pin_desc *pd = pin_desc_get(gpio_dev->pctrl,
> pin);
> -
> -		if (!pd)
> -			continue;
> -
> -		raw_spin_lock_irqsave(&gpio_dev->lock, flags);
> -
> -		pin_reg = readl(gpio_dev->base + pin * 4);
> -		pin_reg &= ~mask;
> -		writel(pin_reg, gpio_dev->base + pin * 4);
> -
> -		raw_spin_unlock_irqrestore(&gpio_dev->lock, flags);
> -	}
> -}
> -
>  #ifdef CONFIG_PM_SLEEP
>  static bool amd_gpio_should_save(struct amd_gpio *gpio_dev, unsigned int
> pin)
>  {
> @@ -1142,9 +1114,6 @@ static int amd_gpio_probe(struct platform_device
> *pdev)
>  		return PTR_ERR(gpio_dev->pctrl);
>  	}
> 
> -	/* Disable and mask interrupts */
> -	amd_gpio_irq_init(gpio_dev);
> -
>  	girq = &gpio_dev->gc.irq;
>  	gpio_irq_chip_set_chip(girq, &amd_gpio_irqchip);
>  	/* This will let us handle the parent IRQ in the driver */
> --
> 2.34.1