G'day Marco,
On 11/06/2019 23:09, Marco Felsch wrote:
Hi Linus,
On 19-06-09 12:58, Linus Walleij wrote:
Added Marco and a few other MCP23s08 people who may be more familiar
with this code. (The driver is a bit complex.)
On Thu, Jun 6, 2019 at 10:59 AM Phil Reid <preid@xxxxxxxxxxxxxxxxx> wrote:
Using kernel 5.1 I'm getting the following message:
"detected irqchip that is shared with multiple gpiochips: please fix the driver"
Which I believe should be fixed by:
19ab5ca "pinctrl: mcp23s08: Allocate irq_chip dynamic"
However mcp23s08_probe_one() ends up calling gpiochip_set_irq_hooks() twice.
It looks strange when I look at the mcp23s08_probe_one() function
because it goes like:
if (mcp->irq && mcp->irq_controller) {
ret = mcp23s08_irqchip_setup(mcp);
if (ret)
goto fail;
}
(...)
if (mcp->irq)
ret = mcp23s08_irq_setup(mcp);
That seems wrong... but overall the code in this probe_one is pretty hard to
follow and probably needs some refactoring.
I don't think that this is wrong since I splitted only the irqchip setup
and the hw irq line setup.
This comes from f259f896f234 ("pinctrl: mcp23s08: fix irq and irqchip
setup order")
by Marco, Marco can you look into this and help us figure out why this happens?
I greped the code and found the possible failure:
static int mcp23s08_probe_one()
{
...
if (mcp->irq && mcp->irq_controller) {
ret = mcp23s08_irqchip_setup(mcp);
if (ret)
goto fail;
}
ret = devm_gpiochip_add_data(dev, &mcp->chip, mcp);
if (ret < 0)
goto fail;
...
}
The mcp23s08_irqchip_setup() call hierarchy:
mcp23s08_irqchip_setup()
gpiochip_irqchip_add_nested()
gpiochip_irqchip_add_key()
gpiochip_set_irq_hooks()
The devm_gpiochip_add_data() call hierarchy:
devm_gpiochip_add_data()
gpiochip_add_data_with_key()
gpiochip_add_irqchip()
gpiochip_set_irq_hooks()
The gpiochip_add_irqchip() returns immediately if there isn't a irqchip
but we added a irqchip due to the previous mcp23s08_irqchip_setup()
call. So it calls gpiochip_set_irq_hooks() a second time. If I got this
right the proper fix is:
static int mcp23s08_probe_one()
{
...
ret = devm_gpiochip_add_data(dev, &mcp->chip, mcp);
if (ret < 0)
goto fail;
if (mcp->irq && mcp->irq_controller) {
ret = mcp23s08_irqchip_setup(mcp);
if (ret)
goto fail;
}
...
}
I checked other users of gpiochip_irqchip_add_nested() and they call
(devm_)gpiochip_add_data always infront of the
gpiochip_irqchip_add_nested() call.
I hope this helps you.
Thanks, that fixed the error message and everything else seems to be working.
I've posted a patch with what I used.
Hopefully I've attributed everything appropriately.
--
Regards
Phil Reid
