All of lore.kernel.org
 help / color / mirror / Atom feed
From: Hans de Goede <hdegoede@redhat.com>
To: Mika Westerberg <mika.westerberg@linux.intel.com>
Cc: Jarkko Nikula <jarkko.nikula@linux.intel.com>,
	linux-gpio@vger.kernel.org, Andy Shevchenko <andy@kernel.org>
Subject: Re: pinctrl-cherryview regression in linux-next on preproduction Braswell
Date: Tue, 4 Jan 2022 17:37:39 +0100	[thread overview]
Message-ID: <1d094a1d-61f3-ec0f-c072-243839ecb55b@redhat.com> (raw)
In-Reply-To: <YdRnExZuPuvdfXgT@lahna>

Hi,

On 1/4/22 16:26, Mika Westerberg wrote:
> Hi,
> 
> On Tue, Jan 04, 2022 at 11:22:53AM +0100, Hans de Goede wrote:
>> 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 https://bugzilla.kernel.org/show_bug.cgi?id=197953.
>> +	 * 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);
>>
>> ?
> 
> IIRC masking them all broke some systems at the time. Unfortunately I
> don't remember the details anymore :(

Ok, so since this may hit other devices to I think we should go with one
of my first fix attempts for this then:

From 46aba0f423b890a8ee21c76b5d460d8ba5c205f8 Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Mon, 3 Jan 2022 11:16:00 +0100
Subject: [PATCH 1/2] pinctrl: cherryview: Trigger hwirq0 for interrupt-lines
 without a mapping

Commit bdfbef2d29dc ("pinctrl: cherryview: Don't use selection 0 to mark
an interrupt line as unused") made the code properly differentiate
between unset vs (hwirq) 0 entries in the GPIO-controller interrupt-line
to GPIO pinnumber/hwirq mapping.

This is causing some boards to not boot. This commit restores the old
behavior of triggering hwirq 0 when receiving an interrupt on an
interrupt-line for which there is no mapping.

Reported-and-tested-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/pinctrl/intel/pinctrl-cherryview.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/pinctrl/intel/pinctrl-cherryview.c b/drivers/pinctrl/intel/pinctrl-cherryview.c
index abffda1fd51e..1d5818269076 100644
--- a/drivers/pinctrl/intel/pinctrl-cherryview.c
+++ b/drivers/pinctrl/intel/pinctrl-cherryview.c
@@ -1471,8 +1471,9 @@ static void chv_gpio_irq_handler(struct irq_desc *desc)
 
 		offset = cctx->intr_lines[intr_line];
 		if (offset == CHV_INVALID_HWIRQ) {
-			dev_err(dev, "interrupt on unused interrupt line %u\n", intr_line);
-			continue;
+			dev_warn_once(dev, "interrupt on unmapped interrupt line %u\n", intr_line);
+			/* Some boards expect hwirq 0 to trigger in this case */
+			offset = 0;
 		}
 
 		generic_handle_domain_irq(gc->irq.domain, offset);
-- 
2.33.1


Which works around this because calling generic_handle_domain_irq(gc->irq.domain, 0)
somehow shuts up the IRQ until it gets assigned.  I guess it ends up getting masked
by the low-level handler because there is no action assigned to it.

But I cannot find the code-path doing that masking for an irq_desc
with its handler set to handle_bad_irq, which is what the handler
for offset 0 should be at this point in time AFAICT...  Anyways this
fix has been tested and works (it basically restores the old behavior
for unmapped interrupt-lines, with a dev_want_once added).

Given that the hardware which Jarkko is using is pre-production hw
I believe that there is no need for a DMI quirk for that special hw then,
as this fix is sufficient to fix things there.

I'll submit this patch upstream in the usual manner right away.

Regards,

Hans


      reply	other threads:[~2022-01-04 16:37 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
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 [this message]

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=1d094a1d-61f3-ec0f-c072-243839ecb55b@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.