Re: pinctrl-cherryview regression in linux-next on preproduction Braswell

[Hans de Goede <hdegoede@redhat.com>]

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

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 ?

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

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

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

Regards,

Hans

[-- Attachment #2: 0001-pinctrl-cherryview-Trigger-hwirq0-for-interrupt-line.patch --]
[-- Type: text/x-patch, Size: 1549 bytes --]

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.

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


[-- Attachment #3: 0002-pinctrl-cherryview-Allow-overwriting-the-interrupt-m.patch --]
[-- Type: text/x-patch, Size: 1578 bytes --]

From f858add52185d927db546c8066f29b545d37b3ba Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Mon, 3 Jan 2022 11:22:28 +0100
Subject: [PATCH 2/2] pinctrl: cherryview: Allow overwriting the interrupt
 mapping when it points to pin 0

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 allowing chv_gpio_irq_startup() to overwrite the interrupt-line
to pin mapping if the current mapping points to pin 0 (which used to be
interpreted as both 0 and unused before).

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

diff --git a/drivers/pinctrl/intel/pinctrl-cherryview.c b/drivers/pinctrl/intel/pinctrl-cherryview.c
index 1d5818269076..dd23c7405e37 100644
--- a/drivers/pinctrl/intel/pinctrl-cherryview.c
+++ b/drivers/pinctrl/intel/pinctrl-cherryview.c
@@ -1322,7 +1322,8 @@ static unsigned chv_gpio_irq_startup(struct irq_data *d)
 		else
 			handler = handle_edge_irq;
 
-		if (cctx->intr_lines[intsel] == CHV_INVALID_HWIRQ) {
+		if (cctx->intr_lines[intsel] == CHV_INVALID_HWIRQ ||
+		    cctx->intr_lines[intsel] == 0) {
 			irq_set_handler_locked(d, handler);
 			dev_dbg(dev, "using interrupt line %u for IRQ_TYPE_NONE on pin %u\n",
 				intsel, pin);
-- 
2.33.1