On 12/06/2019 16:32, Marco Felsch wrote:
Hi Phil,
thanks for the patch. Can you check that the error which should be fixed
by commit 02e389e6 ("pinctrl: mcp23s08: fix irq setup order") do not
appear. If so we should also add a Fixes line.
G'day Marco,
I remember that one know.
I'm also using the mcp with gpio-keys driver.
I don't think I saw the same behaviour with my setup then.
I'm using the (spi) mcp23s16 (with gpio-keys), and Dmitry was using mcp23008 (i2c).
I noted at the time the difference in when
i2c_set_clientdata & spi_set_drvdata are called in the spi / i2c probe paths.
It seems wrong to call i2c_set_clientdata after devm_pinctrl_register is called.
But I'm by no means an expert.
I do have a system with an i2c variant now, but it doesn't use the gpio-keys driver.
Anyways I'm still not seeing any adverse behaviour with the patch so far.
Regards,
Marco
On 19-06-12 10:24, Phil Reid wrote:
Currently probing of the mcp23s08 results in an error message
"detected irqchip that is shared with multiple gpiochips:
please fix the driver"
This is due to the following:
Call to mcp23s08_irqchip_setup() with call hierarchy:
mcp23s08_irqchip_setup()
gpiochip_irqchip_add_nested()
gpiochip_irqchip_add_key()
gpiochip_set_irq_hooks()
Call to devm_gpiochip_add_data() with 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.
Fix this by moving the call to devm_gpiochip_add_data before
the call to mcp23s08_irqchip_setup
Suggested-by: Marco Felsch <m.felsch@xxxxxxxxxxxxxx>
Signed-off-by: Phil Reid <preid@xxxxxxxxxxxxxxxxx>
---
Notes:
v2:
- remove unrelated whitespace changes
drivers/pinctrl/pinctrl-mcp23s08.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/pinctrl/pinctrl-mcp23s08.c b/drivers/pinctrl/pinctrl-mcp23s08.c
index 5d7a851..b727de56 100644
--- a/drivers/pinctrl/pinctrl-mcp23s08.c
+++ b/drivers/pinctrl/pinctrl-mcp23s08.c
@@ -881,6 +881,10 @@ static int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev,
if (ret < 0)
goto fail;
+ ret = devm_gpiochip_add_data(dev, &mcp->chip, mcp);
+ if (ret < 0)
+ goto fail;
+
mcp->irq_controller =
device_property_read_bool(dev, "interrupt-controller");
if (mcp->irq && mcp->irq_controller) {
@@ -922,10 +926,6 @@ static int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev,
goto fail;
}
- ret = devm_gpiochip_add_data(dev, &mcp->chip, mcp);
- if (ret < 0)
- goto fail;
-
if (one_regmap_config) {
mcp->pinctrl_desc.name = devm_kasprintf(dev, GFP_KERNEL,
"mcp23xxx-pinctrl.%d", raw_chip_address);
--
1.8.3.1
--
Regards
Phil Reid
ElectroMagnetic Imaging Technology Pty Ltd
Development of Geophysical Instrumentation & Software
www.electromag.com.au
3 The Avenue, Midland WA 6056, AUSTRALIA
Ph: +61 8 9250 8100
Fax: +61 8 9250 7100
Email: preid@xxxxxxxxxxxxxxxxx
