Re: [PATCH RESEND] mfd: arizona: Fix initialisation of the PM runtime

[Lee Jones <lee.jones@linaro.org>]

On Tue, 11 Aug 2015, Charles Keepax wrote:

> The PM runtime core by default assumes a chip is suspended when runtime
> PM is enabled. Currently the arizona driver enables runtime PM when the
> chip is fully active and then disables the DCVDD regulator at the end of
> arizona_dev_init. This however has several problems, firstly the if we
> reach the end of arizona_dev_init, we did not properly follow all the
> proceedures for shutting down the chip, and most notably we never marked
> the chip as cache only so any writes occuring between then and the next
> PM runtime resume will be lost. Secondly, if we are already resumed when
> we reach the end of dev_init, then at best we get unbalanced regulator
> enable/disables at work we lose DCVDD whilst we need it.
> 
> Additionally, since the commit 4f0216409f7c ("mfd: arizona: Add better
> support for system suspend"), the PM runtime operations may
> disable/enable the IRQ, so the IRQs must now be enabled before we call
> any PM operations.
> 
> This patch adds a call to pm_runtime_set_active to inform the PM core
> that the device is starting up active and moves the PM enabling to
> around the IRQ initialisation to avoid any PM callbacks happening until
> the IRQs are initialised.
> 
> Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
> ---
> 
> This patch was marked as applied on the 24th of June but
> doesn't seem to have shown up in your tree yet. Just
> resending just in case it got missed as it is a fairly
> important bug fix.
> 
> Thanks,
> Charles
> 
>  drivers/mfd/arizona-core.c |   14 ++++++--------
>  1 files changed, 6 insertions(+), 8 deletions(-)

Why are you re-sending this?

> diff --git a/drivers/mfd/arizona-core.c b/drivers/mfd/arizona-core.c
> index bc814b0..375b954 100644
> --- a/drivers/mfd/arizona-core.c
> +++ b/drivers/mfd/arizona-core.c
> @@ -1201,10 +1201,6 @@ int arizona_dev_init(struct arizona *arizona)
>  			     arizona->pdata.gpio_defaults[i]);
>  	}
>  
> -	pm_runtime_set_autosuspend_delay(arizona->dev, 100);
> -	pm_runtime_use_autosuspend(arizona->dev);
> -	pm_runtime_enable(arizona->dev);
> -
>  	/* Chip default */
>  	if (!arizona->pdata.clk32k_src)
>  		arizona->pdata.clk32k_src = ARIZONA_32KZ_MCLK2;
> @@ -1329,11 +1325,17 @@ int arizona_dev_init(struct arizona *arizona)
>  					   arizona->pdata.spk_fmt[i]);
>  	}
>  
> +	pm_runtime_set_active(arizona->dev);
> +	pm_runtime_enable(arizona->dev);
> +
>  	/* Set up for interrupts */
>  	ret = arizona_irq_init(arizona);
>  	if (ret != 0)
>  		goto err_reset;
>  
> +	pm_runtime_set_autosuspend_delay(arizona->dev, 100);
> +	pm_runtime_use_autosuspend(arizona->dev);
> +
>  	arizona_request_irq(arizona, ARIZONA_IRQ_CLKGEN_ERR, "CLKGEN error",
>  			    arizona_clkgen_err, arizona);
>  	arizona_request_irq(arizona, ARIZONA_IRQ_OVERCLOCKED, "Overclocked",
> @@ -1367,10 +1369,6 @@ int arizona_dev_init(struct arizona *arizona)
>  		goto err_irq;
>  	}
>  
> -#ifdef CONFIG_PM
> -	regulator_disable(arizona->dcvdd);
> -#endif
> -
>  	return 0;
>  
>  err_irq:

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