Re: [PATCH] mfd: arizona: Request parent IRQ before we request child IRQs

[Lee Jones]

On Wed, 16 Dec 2015, Charles Keepax wrote:

> Currently the driver requests the boot done and control interface IRQs
> before it has requested its own IRQ line. This can cause problems on
> edge triggered IRQ systems as if an edge occurs before the parent IRQ is
> enabled it will be missed. Whilst we are changing the error handling
> remove an unused label as well.
> 
> Signed-off-by: Charles Keepax <ckeepax@xxxxxxxxxxxxxxxxxxxxxxxxxxx>
> ---
>  drivers/mfd/arizona-irq.c | 57 ++++++++++++++++++++++-------------------------
>  1 file changed, 27 insertions(+), 30 deletions(-)

Applied, thanks.

> diff --git a/drivers/mfd/arizona-irq.c b/drivers/mfd/arizona-irq.c
> index 682bc86..5fef014 100644
> --- a/drivers/mfd/arizona-irq.c
> +++ b/drivers/mfd/arizona-irq.c
> @@ -310,7 +310,7 @@ int arizona_irq_init(struct arizona *arizona)
>  		if (ret != 0) {
>  			dev_err(arizona->dev,
>  				"Failed to add AOD IRQs: %d\n", ret);
> -			goto err_domain;
> +			goto err;
>  		}
>  	}
>  
> @@ -323,30 +323,6 @@ int arizona_irq_init(struct arizona *arizona)
>  		goto err_aod;
>  	}
>  
> -	/* Make sure the boot done IRQ is unmasked for resumes */
> -	i = arizona_map_irq(arizona, ARIZONA_IRQ_BOOT_DONE);
> -	ret = request_threaded_irq(i, NULL, arizona_boot_done, IRQF_ONESHOT,
> -				   "Boot done", arizona);
> -	if (ret != 0) {
> -		dev_err(arizona->dev, "Failed to request boot done %d: %d\n",
> -			arizona->irq, ret);
> -		goto err_boot_done;
> -	}
> -
> -	/* Handle control interface errors in the core */
> -	if (arizona->ctrlif_error) {
> -		i = arizona_map_irq(arizona, ARIZONA_IRQ_CTRLIF_ERR);
> -		ret = request_threaded_irq(i, NULL, arizona_ctrlif_err,
> -					   IRQF_ONESHOT,
> -					   "Control interface error", arizona);
> -		if (ret != 0) {
> -			dev_err(arizona->dev,
> -				"Failed to request CTRLIF_ERR %d: %d\n",
> -				arizona->irq, ret);
> -			goto err_ctrlif;
> -		}
> -	}
> -
>  	/* Used to emulate edge trigger and to work around broken pinmux */
>  	if (arizona->pdata.irq_gpio) {
>  		if (gpio_to_irq(arizona->pdata.irq_gpio) != arizona->irq) {
> @@ -376,21 +352,42 @@ int arizona_irq_init(struct arizona *arizona)
>  		goto err_main_irq;
>  	}
>  
> +	/* Make sure the boot done IRQ is unmasked for resumes */
> +	i = arizona_map_irq(arizona, ARIZONA_IRQ_BOOT_DONE);
> +	ret = request_threaded_irq(i, NULL, arizona_boot_done, IRQF_ONESHOT,
> +				   "Boot done", arizona);
> +	if (ret != 0) {
> +		dev_err(arizona->dev, "Failed to request boot done %d: %d\n",
> +			arizona->irq, ret);
> +		goto err_boot_done;
> +	}
> +
> +	/* Handle control interface errors in the core */
> +	if (arizona->ctrlif_error) {
> +		i = arizona_map_irq(arizona, ARIZONA_IRQ_CTRLIF_ERR);
> +		ret = request_threaded_irq(i, NULL, arizona_ctrlif_err,
> +					   IRQF_ONESHOT,
> +					   "Control interface error", arizona);
> +		if (ret != 0) {
> +			dev_err(arizona->dev,
> +				"Failed to request CTRLIF_ERR %d: %d\n",
> +				arizona->irq, ret);
> +			goto err_ctrlif;
> +		}
> +	}
> +
>  	return 0;
>  
> -err_main_irq:
> -	if (arizona->ctrlif_error)
> -		free_irq(arizona_map_irq(arizona, ARIZONA_IRQ_CTRLIF_ERR),
> -			 arizona);
>  err_ctrlif:
>  	free_irq(arizona_map_irq(arizona, ARIZONA_IRQ_BOOT_DONE), arizona);
>  err_boot_done:
> +	free_irq(arizona->irq, arizona);
> +err_main_irq:
>  	regmap_del_irq_chip(irq_create_mapping(arizona->virq, 1),
>  			    arizona->irq_chip);
>  err_aod:
>  	regmap_del_irq_chip(irq_create_mapping(arizona->virq, 0),
>  			    arizona->aod_irq_chip);
> -err_domain:
>  err:
>  	return ret;
>  }

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org â Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog