public inbox for linux-gpio@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] pinctrl: mcp23s08: Disable all pin interrupts during probe
@ 2026-03-30 16:19 Francesco Lavra
  2026-04-07  6:58 ` Linus Walleij
  0 siblings, 1 reply; 9+ messages in thread
From: Francesco Lavra @ 2026-03-30 16:19 UTC (permalink / raw)
  To: Linus Walleij, linux-gpio, linux-kernel

A chip being probed may have the interrupt-on-change feature enabled on
some of its pins, for example after a reboot. This can cause the chip to
generate interrupts for pins that don't have a registered nested handler,
which leads to a kernel crash such as below:

[    7.928897] Unable to handle kernel read from unreadable memory at virtual address 00000000000000ac
[    7.932314] Mem abort info:
[    7.935081]   ESR = 0x0000000096000004
[    7.938808]   EC = 0x25: DABT (current EL), IL = 32 bits
[    7.944094]   SET = 0, FnV = 0
[    7.947127]   EA = 0, S1PTW = 0
[    7.950247]   FSC = 0x04: level 0 translation fault
[    7.955101] Data abort info:
[    7.957961]   ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000
[    7.963421]   CM = 0, WnR = 0, TnD = 0, TagAccess = 0
[    7.968447]   GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
[    7.973734] user pgtable: 4k pages, 48-bit VAs, pgdp=00000000089b7000
[    7.980148] [00000000000000ac] pgd=0000000000000000, p4d=0000000000000000
[    7.986913] Internal error: Oops: 0000000096000004 [#1]  SMP
[    7.992545] Modules linked in:
[    8.073678] CPU: 0 UID: 0 PID: 81 Comm: irq/18-4-0025 Not tainted 7.0.0-rc6-gd2b5a1f931c8-dirty #199
[    8.073689] Hardware name: Khadas VIM3 (DT)
[    8.073692] pstate: 604000c5 (nZCv daIF +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[    8.094639] pc : _raw_spin_lock_irq+0x40/0x80
[    8.098970] lr : handle_nested_irq+0x2c/0x168
[    8.098979] sp : ffff800082b2bd20
[    8.106599] x29: ffff800082b2bd20 x28: ffff800080107920 x27: ffff800080104d88
[    8.106611] x26: ffff000003298080 x25: 0000000000000001 x24: 000000000000ff00
[    8.113707] x23: 0000000000000001 x22: 0000000000000000 x21: 000000000000000e
[    8.120850] x20: 0000000000000000 x19: 00000000000000ac x18: 0000000000000000
[    8.135046] x17: 0000000000000000 x16: 0000000000000000 x15: 0000000000000000
[    8.135062] x14: ffff800081567ea8 x13: ffffffffffffffff x12: 0000000000000000
[    8.135070] x11: 00000000000000c0 x10: 0000000000000b60 x9 : ffff800080109e0c
[    8.135078] x8 : 1fffe0000069dbc1 x7 : 0000000000000001 x6 : ffff0000034ede00
[    8.135086] x5 : 0000000000000000 x4 : ffff0000034ede08 x3 : 0000000000000001
[    8.163460] x2 : 0000000000000000 x1 : 0000000000000001 x0 : 00000000000000ac
[    8.170560] Call trace:
[    8.180094]  _raw_spin_lock_irq+0x40/0x80 (P)
[    8.184443]  mcp23s08_irq+0x248/0x358
[    8.184462]  irq_thread_fn+0x34/0xb8
[    8.184470]  irq_thread+0x1a4/0x310
[    8.195093]  kthread+0x13c/0x150
[    8.198309]  ret_from_fork+0x10/0x20
[    8.201850] Code: d65f03c0 d2800002 52800023 f9800011 (885ffc01)
[    8.207931] ---[ end trace 0000000000000000 ]---

This issue has always been present, but has been latent until commit
"f9f4fda15e72" ("pinctrl: mcp23s08: init reg_defaults from HW at probe and
switch cache type"), which correctly removed reg_defaults from the regmap
and as a side effect changed the behavior of the interrupt handler so that
the real value of the MCP_GPINTEN register is now being read from the chip
instead of using a bogus 0 default value; a non-zero value for this
register can trigger the invocation of a nested handler which may not exist
(yet).
Fix this issue by disabling all pin interrupts during initialization.

Fixes: "f9f4fda15e72" ("pinctrl: mcp23s08: init reg_defaults from HW at probe and switch cache type")
Signed-off-by: Francesco Lavra <flavra@baylibre.com>
---
 drivers/pinctrl/pinctrl-mcp23s08.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/pinctrl/pinctrl-mcp23s08.c b/drivers/pinctrl/pinctrl-mcp23s08.c
index 586f2f67c617..b89b3169e8be 100644
--- a/drivers/pinctrl/pinctrl-mcp23s08.c
+++ b/drivers/pinctrl/pinctrl-mcp23s08.c
@@ -664,6 +664,15 @@ int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev,
 	if (mcp->irq && mcp->irq_controller) {
 		struct gpio_irq_chip *girq = &mcp->chip.irq;
 
+		/*
+		 * Disable all pin interrupts, to prevent the interrupt handler from
+		 * calling nested handlers for any currently-enabled interrupts that
+		 * do not (yet) have an actual handler.
+		 */
+		ret = mcp_write(mcp, MCP_GPINTEN, 0);
+		if (ret < 0)
+			return dev_err_probe(dev, ret, "can't disable interrupts\n");
+
 		gpio_irq_chip_set_chip(girq, &mcp23s08_irq_chip);
 		/* This will let us handle the parent IRQ in the driver */
 		girq->parent_handler = NULL;
-- 
2.39.5


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] pinctrl: mcp23s08: Disable all pin interrupts during probe
  2026-03-30 16:19 [PATCH] pinctrl: mcp23s08: Disable all pin interrupts during probe Francesco Lavra
@ 2026-04-07  6:58 ` Linus Walleij
  2026-04-07  7:50   ` Francesco Lavra
                     ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Linus Walleij @ 2026-04-07  6:58 UTC (permalink / raw)
  To: Francesco Lavra, Maksim Kiselev, Sander Vanheule, Andy Shevchenko,
	Mike Looijmans, Dmitry Mastykin, Evgenii Shatokhin,
	Arturas Moskvinas, Uwe Kleine-König, Andreas Kaessens,
	Radim Pavlik, Thomas Preston
  Cc: linux-gpio, linux-kernel

On Mon, Mar 30, 2026 at 6:19 PM Francesco Lavra <flavra@baylibre.com> wrote:

> A chip being probed may have the interrupt-on-change feature enabled on
> some of its pins, for example after a reboot. This can cause the chip to
> generate interrupts for pins that don't have a registered nested handler,
> which leads to a kernel crash such as below:
>
> [    7.928897] Unable to handle kernel read from unreadable memory at virtual address 00000000000000ac
> [    7.932314] Mem abort info:
> [    7.935081]   ESR = 0x0000000096000004
> [    7.938808]   EC = 0x25: DABT (current EL), IL = 32 bits
> [    7.944094]   SET = 0, FnV = 0
> [    7.947127]   EA = 0, S1PTW = 0
> [    7.950247]   FSC = 0x04: level 0 translation fault
> [    7.955101] Data abort info:
> [    7.957961]   ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000
> [    7.963421]   CM = 0, WnR = 0, TnD = 0, TagAccess = 0
> [    7.968447]   GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
> [    7.973734] user pgtable: 4k pages, 48-bit VAs, pgdp=00000000089b7000
> [    7.980148] [00000000000000ac] pgd=0000000000000000, p4d=0000000000000000
> [    7.986913] Internal error: Oops: 0000000096000004 [#1]  SMP
> [    7.992545] Modules linked in:
> [    8.073678] CPU: 0 UID: 0 PID: 81 Comm: irq/18-4-0025 Not tainted 7.0.0-rc6-gd2b5a1f931c8-dirty #199
> [    8.073689] Hardware name: Khadas VIM3 (DT)
> [    8.073692] pstate: 604000c5 (nZCv daIF +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> [    8.094639] pc : _raw_spin_lock_irq+0x40/0x80
> [    8.098970] lr : handle_nested_irq+0x2c/0x168
> [    8.098979] sp : ffff800082b2bd20
> [    8.106599] x29: ffff800082b2bd20 x28: ffff800080107920 x27: ffff800080104d88
> [    8.106611] x26: ffff000003298080 x25: 0000000000000001 x24: 000000000000ff00
> [    8.113707] x23: 0000000000000001 x22: 0000000000000000 x21: 000000000000000e
> [    8.120850] x20: 0000000000000000 x19: 00000000000000ac x18: 0000000000000000
> [    8.135046] x17: 0000000000000000 x16: 0000000000000000 x15: 0000000000000000
> [    8.135062] x14: ffff800081567ea8 x13: ffffffffffffffff x12: 0000000000000000
> [    8.135070] x11: 00000000000000c0 x10: 0000000000000b60 x9 : ffff800080109e0c
> [    8.135078] x8 : 1fffe0000069dbc1 x7 : 0000000000000001 x6 : ffff0000034ede00
> [    8.135086] x5 : 0000000000000000 x4 : ffff0000034ede08 x3 : 0000000000000001
> [    8.163460] x2 : 0000000000000000 x1 : 0000000000000001 x0 : 00000000000000ac
> [    8.170560] Call trace:
> [    8.180094]  _raw_spin_lock_irq+0x40/0x80 (P)
> [    8.184443]  mcp23s08_irq+0x248/0x358
> [    8.184462]  irq_thread_fn+0x34/0xb8
> [    8.184470]  irq_thread+0x1a4/0x310
> [    8.195093]  kthread+0x13c/0x150
> [    8.198309]  ret_from_fork+0x10/0x20
> [    8.201850] Code: d65f03c0 d2800002 52800023 f9800011 (885ffc01)
> [    8.207931] ---[ end trace 0000000000000000 ]---
>
> This issue has always been present, but has been latent until commit
> "f9f4fda15e72" ("pinctrl: mcp23s08: init reg_defaults from HW at probe and
> switch cache type"), which correctly removed reg_defaults from the regmap
> and as a side effect changed the behavior of the interrupt handler so that
> the real value of the MCP_GPINTEN register is now being read from the chip
> instead of using a bogus 0 default value; a non-zero value for this
> register can trigger the invocation of a nested handler which may not exist
> (yet).
> Fix this issue by disabling all pin interrupts during initialization.
>
> Fixes: "f9f4fda15e72" ("pinctrl: mcp23s08: init reg_defaults from HW at probe and switch cache type")
> Signed-off-by: Francesco Lavra <flavra@baylibre.com>

Patch applied for fixes since it's pretty urgent, and it also looks
right to me.

However added some people using this to the CC so they
get a chance to react before it goes upstream.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] pinctrl: mcp23s08: Disable all pin interrupts during probe
  2026-04-07  6:58 ` Linus Walleij
@ 2026-04-07  7:50   ` Francesco Lavra
  2026-04-07  9:30     ` Linus Walleij
  2026-04-07 13:47     ` Andy Shevchenko
  2026-04-07 10:17   ` Mike Looijmans
  2026-04-07 22:21   ` Uwe Kleine-König
  2 siblings, 2 replies; 9+ messages in thread
From: Francesco Lavra @ 2026-04-07  7:50 UTC (permalink / raw)
  To: Linus Walleij, Maksim Kiselev, Sander Vanheule, Andy Shevchenko,
	Mike Looijmans, Dmitry Mastykin, Evgenii Shatokhin,
	Arturas Moskvinas, Uwe Kleine-König, Andreas Kaessens,
	Radim Pavlik, Thomas Preston
  Cc: linux-gpio, linux-kernel

On Tue, 2026-04-07 at 08:58 +0200, Linus Walleij wrote:
> On Mon, Mar 30, 2026 at 6:19 PM Francesco Lavra <flavra@baylibre.com>
> wrote:
> 

....

> > This issue has always been present, but has been latent until commit
> > "f9f4fda15e72" ("pinctrl: mcp23s08: init reg_defaults from HW at probe
> > and
> > switch cache type"), which correctly removed reg_defaults from the
> > regmap
> > and as a side effect changed the behavior of the interrupt handler so
> > that
> > the real value of the MCP_GPINTEN register is now being read from the
> > chip
> > instead of using a bogus 0 default value; a non-zero value for this
> > register can trigger the invocation of a nested handler which may not
> > exist
> > (yet).
> > Fix this issue by disabling all pin interrupts during initialization.
> > 
> > Fixes: "f9f4fda15e72" ("pinctrl: mcp23s08: init reg_defaults from HW at
> > probe and switch cache type")

I realized I put stray double quotes around the commit hash.

> > Signed-off-by: Francesco Lavra <flavra@baylibre.com>
> 
> Patch applied for fixes since it's pretty urgent, and it also looks
> right to me.
> 
> However added some people using this to the CC so they
> get a chance to react before it goes upstream.

Thanks!

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] pinctrl: mcp23s08: Disable all pin interrupts during probe
  2026-04-07  7:50   ` Francesco Lavra
@ 2026-04-07  9:30     ` Linus Walleij
  2026-04-07 13:47     ` Andy Shevchenko
  1 sibling, 0 replies; 9+ messages in thread
From: Linus Walleij @ 2026-04-07  9:30 UTC (permalink / raw)
  To: Francesco Lavra
  Cc: Maksim Kiselev, Sander Vanheule, Andy Shevchenko, Mike Looijmans,
	Dmitry Mastykin, Evgenii Shatokhin, Arturas Moskvinas,
	Uwe Kleine-König, Andreas Kaessens, Radim Pavlik,
	Thomas Preston, linux-gpio, linux-kernel

On Tue, Apr 7, 2026 at 9:50 AM Francesco Lavra <flavra@baylibre.com> wrote:
> On Tue, 2026-04-07 at 08:58 +0200, Linus Walleij wrote:
> > On Mon, Mar 30, 2026 at 6:19 PM Francesco Lavra <flavra@baylibre.com>
> > wrote:
> >
>
> ....
>
> > > This issue has always been present, but has been latent until commit
> > > "f9f4fda15e72" ("pinctrl: mcp23s08: init reg_defaults from HW at probe
> > > and
> > > switch cache type"), which correctly removed reg_defaults from the
> > > regmap
> > > and as a side effect changed the behavior of the interrupt handler so
> > > that
> > > the real value of the MCP_GPINTEN register is now being read from the
> > > chip
> > > instead of using a bogus 0 default value; a non-zero value for this
> > > register can trigger the invocation of a nested handler which may not
> > > exist
> > > (yet).
> > > Fix this issue by disabling all pin interrupts during initialization.
> > >
> > > Fixes: "f9f4fda15e72" ("pinctrl: mcp23s08: init reg_defaults from HW at
> > > probe and switch cache type")
>
> I realized I put stray double quotes around the commit hash.

I fixed it up, no problems!

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] pinctrl: mcp23s08: Disable all pin interrupts during probe
  2026-04-07  6:58 ` Linus Walleij
  2026-04-07  7:50   ` Francesco Lavra
@ 2026-04-07 10:17   ` Mike Looijmans
  2026-04-07 12:59     ` Francesco Lavra
  2026-04-07 22:21   ` Uwe Kleine-König
  2 siblings, 1 reply; 9+ messages in thread
From: Mike Looijmans @ 2026-04-07 10:17 UTC (permalink / raw)
  To: Linus Walleij, Francesco Lavra, Maksim Kiselev, Sander Vanheule,
	Andy Shevchenko, Dmitry Mastykin, Evgenii Shatokhin,
	Arturas Moskvinas, Uwe Kleine-König, Andreas Kaessens,
	Radim Pavlik, Thomas Preston
  Cc: linux-gpio, linux-kernel

On 07-04-2026 08:58, Linus Walleij wrote:
> On Mon, Mar 30, 2026 at 6:19 PM Francesco Lavra <flavra@baylibre.com> wrote:
>
>> A chip being probed may have the interrupt-on-change feature enabled on
>> some of its pins, for example after a reboot. This can cause the chip to
>> generate interrupts for pins that don't have a registered nested handler,
>> which leads to a kernel crash such as below:
>>
>> [    7.928897] Unable to handle kernel read from unreadable memory at virtual address 00000000000000ac
>> [    7.932314] Mem abort info:
>> [    7.935081]   ESR = 0x0000000096000004
>> [    7.938808]   EC = 0x25: DABT (current EL), IL = 32 bits
>> [    7.944094]   SET = 0, FnV = 0
>> [    7.947127]   EA = 0, S1PTW = 0
>> [    7.950247]   FSC = 0x04: level 0 translation fault
>> [    7.955101] Data abort info:
>> [    7.957961]   ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000
>> [    7.963421]   CM = 0, WnR = 0, TnD = 0, TagAccess = 0
>> [    7.968447]   GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
>> [    7.973734] user pgtable: 4k pages, 48-bit VAs, pgdp=00000000089b7000
>> [    7.980148] [00000000000000ac] pgd=0000000000000000, p4d=0000000000000000
>> [    7.986913] Internal error: Oops: 0000000096000004 [#1]  SMP
>> [    7.992545] Modules linked in:
>> [    8.073678] CPU: 0 UID: 0 PID: 81 Comm: irq/18-4-0025 Not tainted 7.0.0-rc6-gd2b5a1f931c8-dirty #199
>> [    8.073689] Hardware name: Khadas VIM3 (DT)
>> [    8.073692] pstate: 604000c5 (nZCv daIF +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
>> [    8.094639] pc : _raw_spin_lock_irq+0x40/0x80
>> [    8.098970] lr : handle_nested_irq+0x2c/0x168
>> [    8.098979] sp : ffff800082b2bd20
>> [    8.106599] x29: ffff800082b2bd20 x28: ffff800080107920 x27: ffff800080104d88
>> [    8.106611] x26: ffff000003298080 x25: 0000000000000001 x24: 000000000000ff00
>> [    8.113707] x23: 0000000000000001 x22: 0000000000000000 x21: 000000000000000e
>> [    8.120850] x20: 0000000000000000 x19: 00000000000000ac x18: 0000000000000000
>> [    8.135046] x17: 0000000000000000 x16: 0000000000000000 x15: 0000000000000000
>> [    8.135062] x14: ffff800081567ea8 x13: ffffffffffffffff x12: 0000000000000000
>> [    8.135070] x11: 00000000000000c0 x10: 0000000000000b60 x9 : ffff800080109e0c
>> [    8.135078] x8 : 1fffe0000069dbc1 x7 : 0000000000000001 x6 : ffff0000034ede00
>> [    8.135086] x5 : 0000000000000000 x4 : ffff0000034ede08 x3 : 0000000000000001
>> [    8.163460] x2 : 0000000000000000 x1 : 0000000000000001 x0 : 00000000000000ac
>> [    8.170560] Call trace:
>> [    8.180094]  _raw_spin_lock_irq+0x40/0x80 (P)
>> [    8.184443]  mcp23s08_irq+0x248/0x358
>> [    8.184462]  irq_thread_fn+0x34/0xb8
>> [    8.184470]  irq_thread+0x1a4/0x310
>> [    8.195093]  kthread+0x13c/0x150
>> [    8.198309]  ret_from_fork+0x10/0x20
>> [    8.201850] Code: d65f03c0 d2800002 52800023 f9800011 (885ffc01)
>> [    8.207931] ---[ end trace 0000000000000000 ]---
>>
>> This issue has always been present, but has been latent until commit
>> "f9f4fda15e72" ("pinctrl: mcp23s08: init reg_defaults from HW at probe and
>> switch cache type"), which correctly removed reg_defaults from the regmap
>> and as a side effect changed the behavior of the interrupt handler so that
>> the real value of the MCP_GPINTEN register is now being read from the chip
>> instead of using a bogus 0 default value; a non-zero value for this
>> register can trigger the invocation of a nested handler which may not exist
>> (yet).
>> Fix this issue by disabling all pin interrupts during initialization.
>>
>> Fixes: "f9f4fda15e72" ("pinctrl: mcp23s08: init reg_defaults from HW at probe and switch cache type")
>> Signed-off-by: Francesco Lavra <flavra@baylibre.com>
> Patch applied for fixes since it's pretty urgent, and it also looks
> right to me.
>
> However added some people using this to the CC so they
> get a chance to react before it goes upstream.
Looks okay to me too.

Maybe it'd be better to unconditionally write "0" to this register? No 
need to exercise the interrupt logic and pins when no-one is listening...

I was going to say "if the device doesn't have a reset GPIO", but 
looking at the code, the reset GPIO is never asserted by this driver.


-- 
Mike Looijmans
System Expert

TOPIC Embedded Products B.V.
Materiaalweg 4, 5681 RJ Best
The Netherlands

T: +31 (0) 499 33 69 69
E: mike.looijmans@topic.nl
W: www.topic.nl




^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] pinctrl: mcp23s08: Disable all pin interrupts during probe
  2026-04-07 10:17   ` Mike Looijmans
@ 2026-04-07 12:59     ` Francesco Lavra
  2026-04-07 13:44       ` Mike Looijmans
  0 siblings, 1 reply; 9+ messages in thread
From: Francesco Lavra @ 2026-04-07 12:59 UTC (permalink / raw)
  To: Mike Looijmans, Linus Walleij, Maksim Kiselev, Sander Vanheule,
	Andy Shevchenko, Dmitry Mastykin, Evgenii Shatokhin,
	Arturas Moskvinas, Uwe Kleine-König, Andreas Kaessens,
	Radim Pavlik, Thomas Preston
  Cc: linux-gpio, linux-kernel

On Tue, 2026-04-07 at 12:17 +0200, Mike Looijmans wrote:
> On 07-04-2026 08:58, Linus Walleij wrote:
> > On Mon, Mar 30, 2026 at 6:19 PM Francesco Lavra <flavra@baylibre.com>
> > wrote:
> > 
> > > A chip being probed may have the interrupt-on-change feature enabled
> > > on
> > > some of its pins, for example after a reboot. This can cause the chip
> > > to
> > > generate interrupts for pins that don't have a registered nested
> > > handler,
> > > which leads to a kernel crash such as below:
> > > 
> > > [    7.928897] Unable to handle kernel read from unreadable memory at
> > > virtual address 00000000000000ac
> > > [    7.932314] Mem abort info:
> > > [    7.935081]   ESR = 0x0000000096000004
> > > [    7.938808]   EC = 0x25: DABT (current EL), IL = 32 bits
> > > [    7.944094]   SET = 0, FnV = 0
> > > [    7.947127]   EA = 0, S1PTW = 0
> > > [    7.950247]   FSC = 0x04: level 0 translation fault
> > > [    7.955101] Data abort info:
> > > [    7.957961]   ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000
> > > [    7.963421]   CM = 0, WnR = 0, TnD = 0, TagAccess = 0
> > > [    7.968447]   GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
> > > [    7.973734] user pgtable: 4k pages, 48-bit VAs,
> > > pgdp=00000000089b7000
> > > [    7.980148] [00000000000000ac] pgd=0000000000000000,
> > > p4d=0000000000000000
> > > [    7.986913] Internal error: Oops: 0000000096000004 [#1]  SMP
> > > [    7.992545] Modules linked in:
> > > [    8.073678] CPU: 0 UID: 0 PID: 81 Comm: irq/18-4-0025 Not tainted
> > > 7.0.0-rc6-gd2b5a1f931c8-dirty #199
> > > [    8.073689] Hardware name: Khadas VIM3 (DT)
> > > [    8.073692] pstate: 604000c5 (nZCv daIF +PAN -UAO -TCO -DIT -SSBS
> > > BTYPE=--)
> > > [    8.094639] pc : _raw_spin_lock_irq+0x40/0x80
> > > [    8.098970] lr : handle_nested_irq+0x2c/0x168
> > > [    8.098979] sp : ffff800082b2bd20
> > > [    8.106599] x29: ffff800082b2bd20 x28: ffff800080107920 x27:
> > > ffff800080104d88
> > > [    8.106611] x26: ffff000003298080 x25: 0000000000000001 x24:
> > > 000000000000ff00
> > > [    8.113707] x23: 0000000000000001 x22: 0000000000000000 x21:
> > > 000000000000000e
> > > [    8.120850] x20: 0000000000000000 x19: 00000000000000ac x18:
> > > 0000000000000000
> > > [    8.135046] x17: 0000000000000000 x16: 0000000000000000 x15:
> > > 0000000000000000
> > > [    8.135062] x14: ffff800081567ea8 x13: ffffffffffffffff x12:
> > > 0000000000000000
> > > [    8.135070] x11: 00000000000000c0 x10: 0000000000000b60 x9 :
> > > ffff800080109e0c
> > > [    8.135078] x8 : 1fffe0000069dbc1 x7 : 0000000000000001 x6 :
> > > ffff0000034ede00
> > > [    8.135086] x5 : 0000000000000000 x4 : ffff0000034ede08 x3 :
> > > 0000000000000001
> > > [    8.163460] x2 : 0000000000000000 x1 : 0000000000000001 x0 :
> > > 00000000000000ac
> > > [    8.170560] Call trace:
> > > [    8.180094]  _raw_spin_lock_irq+0x40/0x80 (P)
> > > [    8.184443]  mcp23s08_irq+0x248/0x358
> > > [    8.184462]  irq_thread_fn+0x34/0xb8
> > > [    8.184470]  irq_thread+0x1a4/0x310
> > > [    8.195093]  kthread+0x13c/0x150
> > > [    8.198309]  ret_from_fork+0x10/0x20
> > > [    8.201850] Code: d65f03c0 d2800002 52800023 f9800011 (885ffc01)
> > > [    8.207931] ---[ end trace 0000000000000000 ]---
> > > 
> > > This issue has always been present, but has been latent until commit
> > > "f9f4fda15e72" ("pinctrl: mcp23s08: init reg_defaults from HW at
> > > probe and
> > > switch cache type"), which correctly removed reg_defaults from the
> > > regmap
> > > and as a side effect changed the behavior of the interrupt handler so
> > > that
> > > the real value of the MCP_GPINTEN register is now being read from the
> > > chip
> > > instead of using a bogus 0 default value; a non-zero value for this
> > > register can trigger the invocation of a nested handler which may not
> > > exist
> > > (yet).
> > > Fix this issue by disabling all pin interrupts during initialization.
> > > 
> > > Fixes: "f9f4fda15e72" ("pinctrl: mcp23s08: init reg_defaults from HW
> > > at probe and switch cache type")
> > > Signed-off-by: Francesco Lavra <flavra@baylibre.com>
> > Patch applied for fixes since it's pretty urgent, and it also looks
> > right to me.
> > 
> > However added some people using this to the CC so they
> > get a chance to react before it goes upstream.
> Looks okay to me too.
> 
> Maybe it'd be better to unconditionally write "0" to this register? No 
> need to exercise the interrupt logic and pins when no-one is listening...

Not sure I understand, unconditionally writing "0" to this register is
exactly what this patch does.

> I was going to say "if the device doesn't have a reset GPIO", but 
> looking at the code, the reset GPIO is never asserted by this driver.

Right. In any case, the reset GPIO is optional, so we would still need to
initialize the register manually if there is no reset GPIO.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] pinctrl: mcp23s08: Disable all pin interrupts during probe
  2026-04-07 12:59     ` Francesco Lavra
@ 2026-04-07 13:44       ` Mike Looijmans
  0 siblings, 0 replies; 9+ messages in thread
From: Mike Looijmans @ 2026-04-07 13:44 UTC (permalink / raw)
  To: Francesco Lavra, Linus Walleij, Maksim Kiselev, Sander Vanheule,
	Andy Shevchenko, Dmitry Mastykin, Evgenii Shatokhin,
	Arturas Moskvinas, Uwe Kleine-König, Andreas Kaessens,
	Radim Pavlik, Thomas Preston
  Cc: linux-gpio, linux-kernel

On 07-04-2026 14:59, Francesco Lavra wrote:
> On Tue, 2026-04-07 at 12:17 +0200, Mike Looijmans wrote:
>> On 07-04-2026 08:58, Linus Walleij wrote:
>>> On Mon, Mar 30, 2026 at 6:19 PM Francesco Lavra <flavra@baylibre.com>
>>> wrote:
>>>
>>>> A chip being probed may have the interrupt-on-change feature enabled
>>>> on
>>>> some of its pins, for example after a reboot. This can cause the chip
>>>> to
>>>> generate interrupts for pins that don't have a registered nested
>>>> handler,
>>>> which leads to a kernel crash such as below:
>>>>
>>>> [    7.928897] Unable to handle kernel read from unreadable memory at
>>>> virtual address 00000000000000ac
>>>> [    7.932314] Mem abort info:
>>>> [    7.935081]   ESR = 0x0000000096000004
>>>> [    7.938808]   EC = 0x25: DABT (current EL), IL = 32 bits
>>>> [    7.944094]   SET = 0, FnV = 0
>>>> [    7.947127]   EA = 0, S1PTW = 0
>>>> [    7.950247]   FSC = 0x04: level 0 translation fault
>>>> [    7.955101] Data abort info:
>>>> [    7.957961]   ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000
>>>> [    7.963421]   CM = 0, WnR = 0, TnD = 0, TagAccess = 0
>>>> [    7.968447]   GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
>>>> [    7.973734] user pgtable: 4k pages, 48-bit VAs,
>>>> pgdp=00000000089b7000
>>>> [    7.980148] [00000000000000ac] pgd=0000000000000000,
>>>> p4d=0000000000000000
>>>> [    7.986913] Internal error: Oops: 0000000096000004 [#1]  SMP
>>>> [    7.992545] Modules linked in:
>>>> [    8.073678] CPU: 0 UID: 0 PID: 81 Comm: irq/18-4-0025 Not tainted
>>>> 7.0.0-rc6-gd2b5a1f931c8-dirty #199
>>>> [    8.073689] Hardware name: Khadas VIM3 (DT)
>>>> [    8.073692] pstate: 604000c5 (nZCv daIF +PAN -UAO -TCO -DIT -SSBS
>>>> BTYPE=--)
>>>> [    8.094639] pc : _raw_spin_lock_irq+0x40/0x80
>>>> [    8.098970] lr : handle_nested_irq+0x2c/0x168
>>>> [    8.098979] sp : ffff800082b2bd20
>>>> [    8.106599] x29: ffff800082b2bd20 x28: ffff800080107920 x27:
>>>> ffff800080104d88
>>>> [    8.106611] x26: ffff000003298080 x25: 0000000000000001 x24:
>>>> 000000000000ff00
>>>> [    8.113707] x23: 0000000000000001 x22: 0000000000000000 x21:
>>>> 000000000000000e
>>>> [    8.120850] x20: 0000000000000000 x19: 00000000000000ac x18:
>>>> 0000000000000000
>>>> [    8.135046] x17: 0000000000000000 x16: 0000000000000000 x15:
>>>> 0000000000000000
>>>> [    8.135062] x14: ffff800081567ea8 x13: ffffffffffffffff x12:
>>>> 0000000000000000
>>>> [    8.135070] x11: 00000000000000c0 x10: 0000000000000b60 x9 :
>>>> ffff800080109e0c
>>>> [    8.135078] x8 : 1fffe0000069dbc1 x7 : 0000000000000001 x6 :
>>>> ffff0000034ede00
>>>> [    8.135086] x5 : 0000000000000000 x4 : ffff0000034ede08 x3 :
>>>> 0000000000000001
>>>> [    8.163460] x2 : 0000000000000000 x1 : 0000000000000001 x0 :
>>>> 00000000000000ac
>>>> [    8.170560] Call trace:
>>>> [    8.180094]  _raw_spin_lock_irq+0x40/0x80 (P)
>>>> [    8.184443]  mcp23s08_irq+0x248/0x358
>>>> [    8.184462]  irq_thread_fn+0x34/0xb8
>>>> [    8.184470]  irq_thread+0x1a4/0x310
>>>> [    8.195093]  kthread+0x13c/0x150
>>>> [    8.198309]  ret_from_fork+0x10/0x20
>>>> [    8.201850] Code: d65f03c0 d2800002 52800023 f9800011 (885ffc01)
>>>> [    8.207931] ---[ end trace 0000000000000000 ]---
>>>>
>>>> This issue has always been present, but has been latent until commit
>>>> "f9f4fda15e72" ("pinctrl: mcp23s08: init reg_defaults from HW at
>>>> probe and
>>>> switch cache type"), which correctly removed reg_defaults from the
>>>> regmap
>>>> and as a side effect changed the behavior of the interrupt handler so
>>>> that
>>>> the real value of the MCP_GPINTEN register is now being read from the
>>>> chip
>>>> instead of using a bogus 0 default value; a non-zero value for this
>>>> register can trigger the invocation of a nested handler which may not
>>>> exist
>>>> (yet).
>>>> Fix this issue by disabling all pin interrupts during initialization.
>>>>
>>>> Fixes: "f9f4fda15e72" ("pinctrl: mcp23s08: init reg_defaults from HW
>>>> at probe and switch cache type")
>>>> Signed-off-by: Francesco Lavra <flavra@baylibre.com>
>>> Patch applied for fixes since it's pretty urgent, and it also looks
>>> right to me.
>>>
>>> However added some people using this to the CC so they
>>> get a chance to react before it goes upstream.
>> Looks okay to me too.
>>
>> Maybe it'd be better to unconditionally write "0" to this register? No
>> need to exercise the interrupt logic and pins when no-one is listening...
> Not sure I understand, unconditionally writing "0" to this register is
> exactly what this patch does.

It appears to be inside a "if (mcp->irq && mcp->irq_controller)" block.

Which is fine, of course. If there's no handler, the register has no effect.

I just thought it'd be cleaner to set it to 0 as part of the global 
initialization at the start of probe.



>> I was going to say "if the device doesn't have a reset GPIO", but
>> looking at the code, the reset GPIO is never asserted by this driver.
> Right. In any case, the reset GPIO is optional, so we would still need to
> initialize the register manually if there is no reset GPIO.

I suspect it's actually a bug in the driver that it doesn't assert the 
reset (before de-asserting). But that's for another patch...


-- 
Mike Looijmans
System Expert

TOPIC Embedded Products B.V.
Materiaalweg 4, 5681 RJ Best
The Netherlands

T: +31 (0) 499 33 69 69
E: mike.looijmans@topic.nl
W: www.topic.nl




^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] pinctrl: mcp23s08: Disable all pin interrupts during probe
  2026-04-07  7:50   ` Francesco Lavra
  2026-04-07  9:30     ` Linus Walleij
@ 2026-04-07 13:47     ` Andy Shevchenko
  1 sibling, 0 replies; 9+ messages in thread
From: Andy Shevchenko @ 2026-04-07 13:47 UTC (permalink / raw)
  To: Francesco Lavra
  Cc: Linus Walleij, Maksim Kiselev, Sander Vanheule, Mike Looijmans,
	Dmitry Mastykin, Evgenii Shatokhin, Arturas Moskvinas,
	Uwe Kleine-König, Andreas Kaessens, Radim Pavlik,
	Thomas Preston, linux-gpio, linux-kernel

On Tue, Apr 07, 2026 at 09:50:39AM +0200, Francesco Lavra wrote:
> On Tue, 2026-04-07 at 08:58 +0200, Linus Walleij wrote:
> > On Mon, Mar 30, 2026 at 6:19 PM Francesco Lavra <flavra@baylibre.com>
> > wrote:

...

> > > This issue has always been present, but has been latent until commit
> > > "f9f4fda15e72" ("pinctrl: mcp23s08: init reg_defaults from HW at probe
> > > and
> > > switch cache type"), which correctly removed reg_defaults from the
> > > regmap
> > > and as a side effect changed the behavior of the interrupt handler so
> > > that
> > > the real value of the MCP_GPINTEN register is now being read from the
> > > chip
> > > instead of using a bogus 0 default value; a non-zero value for this
> > > register can trigger the invocation of a nested handler which may not
> > > exist
> > > (yet).
> > > Fix this issue by disabling all pin interrupts during initialization.
> > > 
> > > Fixes: "f9f4fda15e72" ("pinctrl: mcp23s08: init reg_defaults from HW at
> > > probe and switch cache type")
> 
> I realized I put stray double quotes around the commit hash.

Also you put way too many lines of backtrace. Submitting Patches recommends to
leave ~3-5 lines only (the one that bring value).

> > > Signed-off-by: Francesco Lavra <flavra@baylibre.com>

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] pinctrl: mcp23s08: Disable all pin interrupts during probe
  2026-04-07  6:58 ` Linus Walleij
  2026-04-07  7:50   ` Francesco Lavra
  2026-04-07 10:17   ` Mike Looijmans
@ 2026-04-07 22:21   ` Uwe Kleine-König
  2 siblings, 0 replies; 9+ messages in thread
From: Uwe Kleine-König @ 2026-04-07 22:21 UTC (permalink / raw)
  To: Francesco Lavra, Linus Walleij
  Cc: Maksim Kiselev, Sander Vanheule, Andy Shevchenko, Mike Looijmans,
	Dmitry Mastykin, Evgenii Shatokhin, Arturas Moskvinas,
	Andreas Kaessens, Radim Pavlik, Thomas Preston, linux-gpio,
	linux-kernel

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

Hello Linus, hello Francesco,

On Tue, Apr 07, 2026 at 08:58:09AM +0200, Linus Walleij wrote:
> On Mon, Mar 30, 2026 at 6:19 PM Francesco Lavra <flavra@baylibre.com> wrote:
> 
> > A chip being probed may have the interrupt-on-change feature enabled on
> > some of its pins, for example after a reboot. This can cause the chip to
> > generate interrupts for pins that don't have a registered nested handler,
> > which leads to a kernel crash such as below:
> >
> > [    7.928897] Unable to handle kernel read from unreadable memory at virtual address 00000000000000ac
> > [    7.932314] Mem abort info:
> > [    7.935081]   ESR = 0x0000000096000004
> > [    7.938808]   EC = 0x25: DABT (current EL), IL = 32 bits
> > [    7.944094]   SET = 0, FnV = 0
> > [    7.947127]   EA = 0, S1PTW = 0
> > [    7.950247]   FSC = 0x04: level 0 translation fault
> > [    7.955101] Data abort info:
> > [    7.957961]   ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000
> > [    7.963421]   CM = 0, WnR = 0, TnD = 0, TagAccess = 0
> > [    7.968447]   GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
> > [    7.973734] user pgtable: 4k pages, 48-bit VAs, pgdp=00000000089b7000
> > [    7.980148] [00000000000000ac] pgd=0000000000000000, p4d=0000000000000000
> > [    7.986913] Internal error: Oops: 0000000096000004 [#1]  SMP
> > [    7.992545] Modules linked in:
> > [    8.073678] CPU: 0 UID: 0 PID: 81 Comm: irq/18-4-0025 Not tainted 7.0.0-rc6-gd2b5a1f931c8-dirty #199
> > [    8.073689] Hardware name: Khadas VIM3 (DT)
> > [    8.073692] pstate: 604000c5 (nZCv daIF +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> > [    8.094639] pc : _raw_spin_lock_irq+0x40/0x80
> > [    8.098970] lr : handle_nested_irq+0x2c/0x168
> > [    8.098979] sp : ffff800082b2bd20
> > [    8.106599] x29: ffff800082b2bd20 x28: ffff800080107920 x27: ffff800080104d88
> > [    8.106611] x26: ffff000003298080 x25: 0000000000000001 x24: 000000000000ff00
> > [    8.113707] x23: 0000000000000001 x22: 0000000000000000 x21: 000000000000000e
> > [    8.120850] x20: 0000000000000000 x19: 00000000000000ac x18: 0000000000000000
> > [    8.135046] x17: 0000000000000000 x16: 0000000000000000 x15: 0000000000000000
> > [    8.135062] x14: ffff800081567ea8 x13: ffffffffffffffff x12: 0000000000000000
> > [    8.135070] x11: 00000000000000c0 x10: 0000000000000b60 x9 : ffff800080109e0c
> > [    8.135078] x8 : 1fffe0000069dbc1 x7 : 0000000000000001 x6 : ffff0000034ede00
> > [    8.135086] x5 : 0000000000000000 x4 : ffff0000034ede08 x3 : 0000000000000001
> > [    8.163460] x2 : 0000000000000000 x1 : 0000000000000001 x0 : 00000000000000ac
> > [    8.170560] Call trace:
> > [    8.180094]  _raw_spin_lock_irq+0x40/0x80 (P)
> > [    8.184443]  mcp23s08_irq+0x248/0x358
> > [    8.184462]  irq_thread_fn+0x34/0xb8
> > [    8.184470]  irq_thread+0x1a4/0x310
> > [    8.195093]  kthread+0x13c/0x150
> > [    8.198309]  ret_from_fork+0x10/0x20
> > [    8.201850] Code: d65f03c0 d2800002 52800023 f9800011 (885ffc01)
> > [    8.207931] ---[ end trace 0000000000000000 ]---

I haven't looked in detail but wonder about the the noisy failure which
makes me wonder if there is an(other) issue that is just papered over
here.

Apart from that, it makes totally sense to disable interrupts during
probe.

Best regards
Uwe

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2026-04-07 22:21 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-30 16:19 [PATCH] pinctrl: mcp23s08: Disable all pin interrupts during probe Francesco Lavra
2026-04-07  6:58 ` Linus Walleij
2026-04-07  7:50   ` Francesco Lavra
2026-04-07  9:30     ` Linus Walleij
2026-04-07 13:47     ` Andy Shevchenko
2026-04-07 10:17   ` Mike Looijmans
2026-04-07 12:59     ` Francesco Lavra
2026-04-07 13:44       ` Mike Looijmans
2026-04-07 22:21   ` Uwe Kleine-König

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox