Re: [PATCH v2 1/1] pinctlr: mcp23s08: Fix add_data and irqchip_add_nested call order

[Marco Felsch <m.felsch@xxxxxxxxxxxxxx>]

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.

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

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |