Re: [PATCH] pinctrl: mcp23s08: Allocate irq_chip dynamic

From: "Jan Kundrát" <jan.kundrat@cesnet.cz>
To: Linus Walleij <linus.walleij@linaro.org>
Cc: Lars Poeschel <poeschel@lemonage.de>,
	"open list:PIN CONTROL SUBSYSTEM" <linux-gpio@vger.kernel.org>,
	open list <linux-kernel@vger.kernel.org>,
	Hans Verkuil <hverkuil@xs4all.nl>,
	Heikki Krogerus <heikki.krogerus@linux.intel.com>
Subject: Re: [PATCH] pinctrl: mcp23s08: Allocate irq_chip dynamic
Date: Fri, 08 Mar 2019 13:30:19 +0100	[thread overview]
Message-ID: <a79af7cc-20e7-4a52-b77f-24f40ddca23d@cesnet.cz> (raw)
In-Reply-To: <CACRpkdYJHyj+TMOde=veQsj8m+xFf0XGEnT0DS0qgMXCSA5iOA@mail.gmail.com>

On pátek 8. března 2019 10:15:50 CET, Linus Walleij wrote:
> On Tue, Mar 5, 2019 at 4:19 PM Jan Kundrát <jan.kundrat@cesnet.cz> wrote:
>
>> This commit should probably go to 5.0-stable and 4.20-stable as well
>
> I doubt that this commit is fixing a regression, but who knows...
>> If I cherry-pick the following two commits to my 5.0, my board boots once
>> again:
>> 
>> 16f4372fd7a5 pinctrl: mcp23s08: use struct_size() in devm_kzalloc()
>> 19ab5ca9b77d pinctrl: mcp23s08: Allocate irq_chip dynamic
>
> Hm weird.

My working theory is that this is because I have two physical chips on the 
same SPI CS. If I am correct, it works like this between 4.20 and current 
gpio-next:

- GPIO expander #1 is initialized, including the irqchip
- GPIO expander #2 is initialized, and the irqchip initialization fails
- an interrupt on a shared IRQ line which is common to GPIO expanders #1 
and #2 and one other unrelated device starts to be processed
- GPIO expander #1 says "not mine interrupt"
- GPIO expander #2 attempts to dereference a null pointer becase since 
171948ea33e14, the gpiochip->irq.irq_enable/disable are not initialized

>> As on "why am I hitting this while nobody else did", my board has a shared
>> IRQ line which is active during bootup, perhaps that might have something
>> to do with this -- I don't know.
>
> Sounds like the code in mcp23s08 .probe() should write to all
> registers clearing and disabling interrupts before proceeding to
> request interrupts or register the irqchip, else it will fire immediately
> and cause oopses.

I do not think that this would help me. My IRQ line is shared with another 
chip, so it can be physically asserted even if this particular GPIO 
expander doesn't generate an interrupt.

I suspect that this configuration (multiple MCP23S17 on one SPI CS, and an 
IRQ line shared with something else) is quite rare...

With kind regards,
Jan