All of lore.kernel.org
 help / color / mirror / Atom feed
From: Hans de Goede <hdegoede@redhat.com>
To: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Cc: linux-gpio@vger.kernel.org,
	Mika Westerberg <mika.westerberg@linux.intel.com>,
	Andy Shevchenko <andy@kernel.org>
Subject: Re: pinctrl-cherryview regression in linux-next on preproduction Braswell
Date: Mon, 3 Jan 2022 17:40:43 +0100	[thread overview]
Message-ID: <c29e98f5-c8e4-1967-a249-a461776488ad@redhat.com> (raw)
In-Reply-To: <a8b6d8f1-ad8c-23ac-a85b-2c903530735f@linux.intel.com>

[-- Attachment #1: Type: text/plain, Size: 5085 bytes --]

Hi,

On 1/3/22 13:34, Jarkko Nikula wrote:
> Hi
> 
> On 1/3/22 12:42, Hans de Goede wrote:
>> Hi Jarkko,
>>
>> On 1/3/22 10:42, Jarkko Nikula wrote:
>>> Hi
>>>
>>> We have a Braswell based preproduction HW. I noticed linux-next tag next-20211224 doesn't boot on it due to following error:
>>>
>>> [   34.674271] cherryview-pinctrl INT33FF:00: interrupt on unused interrupt line 0
>>> [   34.682476] cherryview-pinctrl INT33FF:00: interrupt on unused interrupt line 0
>>> [   34.690681] cherryview-pinctrl INT33FF:00: interrupt on unused interrupt line 0
>>> ...
>>>
>>> Linux v5.16-rc8 is ok. I found the following commit to be reason for the regression:
>>>
>>> bdfbef2d29dc ("pinctrl: cherryview: Don't use selection 0 to mark an interrupt line as unused")
>>
>> Thank you for the timely headsup about this.
>>
>> I assume that you have tried a revert (if necessary including reverting some
>> of the follow ups) and that reverting the patch you point to fixes the
>> issue, right ?
>>
> Yes since linux-next has only these three commits below to pinctrl-cherryview.c that are not in v5.16-rc8 I reverted them one by one. I often try these kind of simple steps before going to more labor git bisect :-)
> 
> db1b2a8caf5b pinctrl: cherryview: Use temporary variable for struct device
> 07199dbf8cae pinctrl: cherryview: Do not allow the same interrupt line to be used by 2 pins
> bdfbef2d29dc pinctrl: cherryview: Don't use selection 0 to mark an interrupt line as unused
> 
> I also double checked by checking out to bdfbef2d29dc and tested that commit and it reverted.
> 
>> Can you try the 2 attached patches *one at a time* ? :
>>
>> 1. Restores the old behavior of just triggering hwirq 0 of
>> the pincontroller-domain when we don't know the mapping
>>
> Patch 1 does work and here's the output from modified error print:
> 
> [   13.550781] cherryview-pinctrl INT33FF:00: interrupt on unmapped interrupt line 0
> 
> If you want to go with patch 1 you may add my
> Tested-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
> 
>> 2. Restores the old behavior which allows chv_gpio_irq_startup()
>> to overwrite the interrupt-line to pin mapping if the current
>> mapping points to pin 0
>>
> Patch 2 alone doesn't:
> 
> [   24.977116] cherryview-pinctrl INT33FF:00: interrupt on unused interrupt line 0
> [   24.985314] cherryview-pinctrl INT33FF:00: interrupt on unused interrupt line 0
> [   24.993513] cherryview-pinctrl INT33FF:00: interrupt on unused interrupt line 0
> ...

Ok, that is good news actually, I was already hoping that patch 1
would fix things.

>> Both of these restore old behavior caused by a mapping-table
>> entry containing 0 meaning both unset as well as HWIRQ0
>> before the patch in question.
>>
>> If applying them one at a time does not help, please also try with
>> both applied.
>>
>> These 2 patches should apply cleanly on top of linux-next.
>>
>> If patch 2 fixes things it would be interesting if you can grab a
>> dmesg with "pinctrl-cherryview.dyndbg" added to the command line
>> (with the patched kernel).
>>
> With both applied it does work:
> 
> # dmesg |grep cherryview-pinctrl
> [   15.465425] cherryview-pinctrl INT33FF:00: interrupt on unmapped interrupt line 0
> [   16.562282] cherryview-pinctrl INT33FF:03: using interrupt line 0 for pin 81
> [   17.824905] cherryview-pinctrl INT33FF:02: using interrupt line 0 for pin 22
> [   17.835757] cherryview-pinctrl INT33FF:03: using interrupt line 2 for pin 77
> [   17.850170] cherryview-pinctrl INT33FF:00: using interrupt line 0 for pin 76

Oh, that is actually interesting, this is a per gpio controller thing, so if we
filter on the controller then we end up with:

[   15.465425] cherryview-pinctrl INT33FF:00: interrupt on unmapped interrupt line 0
[   17.850170] cherryview-pinctrl INT33FF:00: using interrupt line 0 for pin 76

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) ?

###

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 :)

Regards,

Hans


[-- Attachment #2: 0001-pinctrl-cherryview-Hack-to-try-and-workaround-linux-.patch --]
[-- Type: text/x-patch, Size: 1188 bytes --]

From 48c739b102051b71a9d4de2d128f4f2633cd668d Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Mon, 3 Jan 2022 17:31:36 +0100
Subject: [PATCH] pinctrl: cherryview: Hack to try and workaround linux-next
 regression

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/pinctrl/intel/pinctrl-cherryview.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pinctrl/intel/pinctrl-cherryview.c b/drivers/pinctrl/intel/pinctrl-cherryview.c
index 683b95e9639a..2ee933c6304a 100644
--- a/drivers/pinctrl/intel/pinctrl-cherryview.c
+++ b/drivers/pinctrl/intel/pinctrl-cherryview.c
@@ -1558,13 +1558,13 @@ static int chv_gpio_irq_init_hw(struct gpio_chip *chip)
 	 *
 	 * See also https://bugzilla.kernel.org/show_bug.cgi?id=197953.
 	 */
-	if (!pctrl->chip.irq.init_valid_mask) {
+//	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));
-	}
+//	}
 
 	/* Clear all interrupts */
 	chv_pctrl_writel(pctrl, CHV_INTSTAT, 0xffff);
-- 
2.33.1


  parent reply	other threads:[~2022-01-03 16:40 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 [this message]
2022-01-04  9:43       ` Jarkko Nikula
2022-01-04 10:22         ` Hans de Goede
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:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

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

  git send-email \
    --in-reply-to=c29e98f5-c8e4-1967-a249-a461776488ad@redhat.com \
    --to=hdegoede@redhat.com \
    --cc=andy@kernel.org \
    --cc=jarkko.nikula@linux.intel.com \
    --cc=linux-gpio@vger.kernel.org \
    --cc=mika.westerberg@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* 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.