linux-kernel - Re: [PATCH] pinctrl: cherryview: Do not mask all 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: <20160818121328.GE30827@lahna.fi.intel.com>
Date:	Thu, 18 Aug 2016 15:13:28 +0300
From:	Mika Westerberg <mika.westerberg@...ux.intel.com>
To:	Anisse Astier <anisse@...ier.eu>
Cc:	Linus Walleij <linus.walleij@...aro.org>,
	Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
	Yu C Chen <yu.c.chen@...el.com>,
	"linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] pinctrl: cherryview: Do not mask all interrupts on probe

On Wed, Aug 17, 2016 at 03:42:58PM +0200, Anisse Astier wrote:
> On Wed, Aug 17, 2016 at 10:13 AM, Mika Westerberg
> <mika.westerberg@...ux.intel.com> wrote:
> > On Tue, Aug 16, 2016 at 06:12:40PM +0200, Anisse Astier wrote:
> >> Hi Mika,
> >>
> >> Did you find a way to fix this issue ? I'm seeing a similar problem on a
> >> laptop where this masks the interrupt used for ACPI events (brightness,
> >> lid, battery).
> >
> > I seem to have forgotten this completely :-/
> >
> > Can you send me output of /sys/kernel/debug/pinctrl/INT33FF:*/pins for
> > that particular EC pin?
> >
> > In addition if you apply this patch do you see that ACPI events start
> > working?
> 
> >From what I've seen it's in the north range, I don't know which pin in
> particular it is yet.
> 
> If the interrupts aren't masked for the north community, ACPI events
> start working.
> 
> 
> # cat /sys/kernel/debug/pinctrl/INT33FF\:01/pins
> registered pins: 59
> pin 0 (GPIO_DFX_0) GPIO ctrl0 0x00118102 ctrl1 0x05c00000
> pin 1 (GPIO_DFX_3) GPIO ctrl0 0x2c018100 ctrl1 0x05c00000
> pin 2 (GPIO_DFX_7) GPIO ctrl0 0x00918102 ctrl1 0x05c00000
> pin 3 (GPIO_DFX_1) GPIO ctrl0 0x18118100 ctrl1 0x05c00000
> pin 4 (GPIO_DFX_5) GPIO ctrl0 0x00918102 ctrl1 0x05c00000
> pin 5 (GPIO_DFX_4) GPIO ctrl0 0x00118102 ctrl1 0x05c00000
> pin 6 (GPIO_DFX_8) GPIO ctrl0 0x00918102 ctrl1 0x05c00000
> pin 7 (GPIO_DFX_2) GPIO ctrl0 0x00118100 ctrl1 0x05c00000
> pin 8 (GPIO_DFX_6) GPIO ctrl0 0x00918102 ctrl1 0x05c00000
> pin 15 (GPIO_SUS0) GPIO ctrl0 0x3c018201 ctrl1 0x05c00001
> pin 16 (SEC_GPIO_SUS10) GPIO ctrl0 0x00118100 ctrl1 0x05c00000
> pin 17 (GPIO_SUS3) GPIO ctrl0 0x4c118100 ctrl1 0x05c00000
> pin 18 (GPIO_SUS7) GPIO ctrl0 0xfc918201 ctrl1 0x05c00001
> pin 19 (GPIO_SUS1) mode 6 ctrl0 0x00160301 ctrl1 0x05c00000
> pin 20 (GPIO_SUS5) mode 1 ctrl0 0x00910200 ctrl1 0x05c00000
> pin 21 (SEC_GPIO_SUS11) GPIO ctrl0 0x5c118100 ctrl1 0x05c00000
> pin 22 (GPIO_SUS4) mode 6 ctrl0 0x00960301 ctrl1 0x05c00000
> pin 23 (SEC_GPIO_SUS8) mode 1 ctrl0 0x00910300 ctrl1 0x05c00000
> pin 24 (GPIO_SUS2) GPIO ctrl0 0x00918102 ctrl1 0x05c00000
> pin 25 (GPIO_SUS6) GPIO ctrl0 0xec918201 ctrl1 0x05c00001

It is this one (GPIO_SUS6).

I wonder if we can relax the driver so that it only masks pins which are
not configured to generate interrupts by the BIOS. I quickly tried
following on one Braswell machine and it did not generate spurious
interrupts.

Can you check if this works for you?

diff --git a/drivers/pinctrl/intel/pinctrl-cherryview.c b/drivers/pinctrl/intel/pinctrl-cherryview.c
index 5749a4eee746..579e0e48bdee 100644
--- a/drivers/pinctrl/intel/pinctrl-cherryview.c
+++ b/drivers/pinctrl/intel/pinctrl-cherryview.c
@@ -1513,6 +1513,7 @@ static int chv_gpio_probe(struct chv_pinctrl *pctrl, int irq)
 	const struct chv_gpio_pinrange *range;
 	struct gpio_chip *chip = &pctrl->chip;
 	int ret, i, offset;
+	u32 intmask = 0;
 
 	*chip = chv_gpio_chip;
 
@@ -1539,8 +1540,27 @@ static int chv_gpio_probe(struct chv_pinctrl *pctrl, int irq)
 		offset += range->npins;
 	}
 
-	/* Mask and clear all interrupts */
-	chv_writel(0, pctrl->regs + CHV_INTMASK);
+	/*
+	 * Mask all interrupts except those which BIOS has configured to
+	 * actually generate interrupts in their padctrl registers.
+	 */
+	for (i = 0; i < pctrl->community->npins; i++) {
+		unsigned pin = pctrl->community->pins[i].number;
+		u32 intsel, ctrl1;
+
+		intsel = readl(chv_padreg(pctrl, pin, CHV_PADCTRL0));
+		intsel &= CHV_PADCTRL0_INTSEL_MASK;
+		intsel >>= CHV_PADCTRL0_INTSEL_SHIFT;
+		ctrl1 = readl(chv_padreg(pctrl, pin, CHV_PADCTRL1));
+
+		if (intsel && (ctrl1 & CHV_PADCTRL1_INTWAKECFG_MASK))
+			intmask |= BIT(intsel);
+	}
+
+	intmask &= readl(pctrl->regs + CHV_INTMASK);
+	writel(intmask, pctrl->regs + CHV_INTMASK);
+
+	/* Clear all interrupts */
 	chv_writel(0xffff, pctrl->regs + CHV_INTSTAT);
 
 	ret = gpiochip_irqchip_add(chip, &chv_gpio_irqchip, 0,