diff mbox

[v2] pinctrl: cherryview: Do not mask all interrupts in probe

Message ID 1471866172-64154-1-git-send-email-mika.westerberg@linux.intel.com
State New
Headers show

Commit Message

Mika Westerberg Aug. 22, 2016, 11:42 a.m. UTC
The Cherryview GPIO controller has 8 or 16 wires connected to the I/O-APIC
which can be used directly by the platform/BIOS or drivers. One such wire
is used as SCI (System Control Interrupt) which ACPI depends on to be able
to trigger GPEs (General Purpose Events).

The pinctrl driver itself uses another IRQ resource which is wire OR of all
the 8 (or 16) wires and follows what BIOS has programmed to the IntSel
register of each pin.

Currently the driver masks all interrupts at probe time and this prevents
these direct interrupts from working as expected. The reason for this is
that some early stage prototypes had some pins misconfigured causing lots
of spurious interrupts.

We fix this by leaving the interrupt mask untouched. This allows SCI and
other direct interrupts work properly. What comes to the possible spurious
interrupts we switch the default handler to be handle_bad_irq() instead of
handle_simple_irq() (which was not correct anyway).

Reported-by: Yu C Chen <yu.c.chen@intel.com>
Reported-by: Anisse Astier <anisse@astier.eu>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
Changes from the previous version [1]:

  - Update changelog
  - Use handle_bad_irq() instead of handle_simple_irq()

[1] https://lkml.org/lkml/2015/5/22/111

Anisse,

This is slightly different what I asked you test last time. I wonder if you
could give this a try and see if you still get ACPI GPEs.

 drivers/pinctrl/intel/pinctrl-cherryview.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

Comments

Linus Walleij Aug. 22, 2016, 1:57 p.m. UTC | #1
On Mon, Aug 22, 2016 at 1:42 PM, Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:

> The Cherryview GPIO controller has 8 or 16 wires connected to the I/O-APIC
> which can be used directly by the platform/BIOS or drivers. One such wire
> is used as SCI (System Control Interrupt) which ACPI depends on to be able
> to trigger GPEs (General Purpose Events).
>
> The pinctrl driver itself uses another IRQ resource which is wire OR of all
> the 8 (or 16) wires and follows what BIOS has programmed to the IntSel
> register of each pin.
>
> Currently the driver masks all interrupts at probe time and this prevents
> these direct interrupts from working as expected. The reason for this is
> that some early stage prototypes had some pins misconfigured causing lots
> of spurious interrupts.
>
> We fix this by leaving the interrupt mask untouched. This allows SCI and
> other direct interrupts work properly. What comes to the possible spurious
> interrupts we switch the default handler to be handle_bad_irq() instead of
> handle_simple_irq() (which was not correct anyway).
>
> Reported-by: Yu C Chen <yu.c.chen@intel.com>
> Reported-by: Anisse Astier <anisse@astier.eu>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Patch applied for fixes.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Anisse Astier Aug. 22, 2016, 3:56 p.m. UTC | #2
On Mon, Aug 22, 2016 at 1:42 PM, Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:
> The Cherryview GPIO controller has 8 or 16 wires connected to the I/O-APIC
> which can be used directly by the platform/BIOS or drivers. One such wire
> is used as SCI (System Control Interrupt) which ACPI depends on to be able
> to trigger GPEs (General Purpose Events).
>
> The pinctrl driver itself uses another IRQ resource which is wire OR of all
> the 8 (or 16) wires and follows what BIOS has programmed to the IntSel
> register of each pin.
>
> Currently the driver masks all interrupts at probe time and this prevents
> these direct interrupts from working as expected. The reason for this is
> that some early stage prototypes had some pins misconfigured causing lots
> of spurious interrupts.
>
> We fix this by leaving the interrupt mask untouched. This allows SCI and
> other direct interrupts work properly. What comes to the possible spurious
> interrupts we switch the default handler to be handle_bad_irq() instead of
> handle_simple_irq() (which was not correct anyway).
>
> Reported-by: Yu C Chen <yu.c.chen@intel.com>
> Reported-by: Anisse Astier <anisse@astier.eu>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Tested-by: Anisse Astier <anisse@astier.eu>


I confirm this is working and I still get the ACPI events

Regards,

Anisse
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/pinctrl/intel/pinctrl-cherryview.c b/drivers/pinctrl/intel/pinctrl-cherryview.c
index 5749a4eee746..0fe8fad25e4d 100644
--- a/drivers/pinctrl/intel/pinctrl-cherryview.c
+++ b/drivers/pinctrl/intel/pinctrl-cherryview.c
@@ -1539,12 +1539,11 @@  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);
+	/* Clear all interrupts */
 	chv_writel(0xffff, pctrl->regs + CHV_INTSTAT);
 
 	ret = gpiochip_irqchip_add(chip, &chv_gpio_irqchip, 0,
-				   handle_simple_irq, IRQ_TYPE_NONE);
+				   handle_bad_irq, IRQ_TYPE_NONE);
 	if (ret) {
 		dev_err(pctrl->dev, "failed to add IRQ chip\n");
 		goto fail;