All of
 help / color / mirror / Atom feed
From: Hans de Goede <>
To: Jarkko Nikula <>
	Mika Westerberg <>,
	Andy Shevchenko <>
Subject: Re: pinctrl-cherryview regression in linux-next on preproduction Braswell
Date: Tue, 4 Jan 2022 11:22:53 +0100	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>


On 1/4/22 10:43, Jarkko Nikula wrote:
> Hi
> On 1/3/22 18:40, Hans de Goede wrote:
>> So we do eventually get an IRQ request for a pin using the GPIO controller
>> internal interrupt-line 0. But the IRQ triggers at least once before then and
>> even though we haven't set a handler yet, calling generic_handle_irq for the
>> GPIO-chips irqdomain, offset 0 IRQ does manage to silence the interrupt.
>> I've been tracing this through the kernel code and AFAICT we end up in:
>> arch/x86/kernel/irq.c: ack_bad_irq() in this case:
>> Which means that this message should show up in dmesg:
>>          if (printk_ratelimit())
>>                  pr_err("unexpected IRQ trap at vector %02x\n", irq);
>> Can you confirm this? Also can you share the full dmesg output of this
>> device (with both patches, with dyndbg option) ?
> Hmm.. don't see it. Attached dmesg_20220103 is with both yesterday's patches.

I guess I must be misreading the code somehow then, the pinctrl-cherryview code
sets the default low-level IRQ handler to handle_bad_irq and it does not
get changed to handle_level_irq / handle_edge_irq until we hit a code
path which also sets intr_lines[intsel] to the pin for which the IRQ is
being activated.

And handle_bad_irq() is:

void handle_bad_irq(struct irq_desc *desc)
        unsigned int irq = irq_desc_get_irq(desc);

        print_irq_desc(irq, desc);

So we should end up in arch/x86/kernel/irq.c: ack_bad_irq() AFAICT, but
I guess I'm missing something somewhere.

Anyways since my hack to enable the spurious IRQ workaround works and
a spurious IRQ is the root-cause here lets focus on that.

>> Note what we are seeing here is basically a spurious IRQ. Except on a few
>> known broken devices the cherryview pinctrl code relies on the BIOS having
>> configured things so that there are no spurious IRQs. I've attached a
>> quick hack which activates the workaround for known broken devices
>> unconditionally. This replace my previous 2 patches. I expect this to
>> fix things too.
>> If you can make some time to give this one a test too that would be
>> great, then we have some options on how to fix this :)
> Hack patch booted too. Attached dmesg_20220104-hack is from this test.


Andy, Mika, why don't we just mask out all IRQs at boot unconditionally:

diff --git a/drivers/pinctrl/intel/pinctrl-cherryview.c b/drivers/pinctrl/intel/pinctrl-cherryview.c
index 683b95e9639a..8981755a5d83 100644
--- a/drivers/pinctrl/intel/pinctrl-cherryview.c
+++ b/drivers/pinctrl/intel/pinctrl-cherryview.c
@@ -1552,19 +1552,10 @@ static int chv_gpio_irq_init_hw(struct gpio_chip *chip)
 	const struct intel_community *community = &pctrl->communities[0];
-	 * The same set of machines in chv_no_valid_mask[] have incorrectly
-	 * configured GPIOs that generate spurious interrupts so we use
-	 * this same list to apply another quirk for them.
-	 *
-	 * See also
+	 * Start with all normal interrupts in the community masked,
+	 * but leave the ones that can only generate GPEs unmasked.
-	if (!pctrl->chip.irq.init_valid_mask) {
-		/*
-		 * Mask all interrupts the community is able to generate
-		 * but leave the ones that can only generate GPEs unmasked.
-		 */
-		chv_pctrl_writel(pctrl, CHV_INTMASK, GENMASK(31, community->nirqs));
-	}
+	chv_pctrl_writel(pctrl, CHV_INTMASK, GENMASK(31, community->nirqs));
 	/* Clear all interrupts */
 	chv_pctrl_writel(pctrl, CHV_INTSTAT, 0xffff);


I never really understood why the code only did this on models which are
known to generate spurious IRQs, leaving the IRQs unmasked before any driver
has requested them seems not useful? And as this shows can actually be
harmful at times ?



  reply	other threads:[~2022-01-04 10:22 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-03  9:42 pinctrl-cherryview regression in linux-next on preproduction Braswell Jarkko Nikula
2022-01-03 10:42 ` Hans de Goede
2022-01-03 12:34   ` Jarkko Nikula
2022-01-03 16:06     ` Hans de Goede
2022-01-03 16:40     ` Hans de Goede
2022-01-04  9:43       ` Jarkko Nikula
2022-01-04 10:22         ` Hans de Goede [this message]
2022-01-04 10:48           ` Hans de Goede
2022-01-04 14:38             ` Jarkko Nikula
2022-01-04 14:47               ` Hans de Goede
2022-01-05 14:23                 ` Jarkko Nikula
2022-01-10 15:44                   ` Hans de Goede
2022-01-04 15:26           ` Mika Westerberg
2022-01-04 16:37             ` Hans de Goede

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \ \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.