On 05/26, Ezequiel Garcia wrote:
> Currently, when the rate is changed, the driver makes sure the
> PLL is enabled before doing so. This is done because the PLL
> cannot be locked while disabled. Once locked, the drivers
> returns the PLL to its previous enable/disable state.
>
> This is a bit cumbersome, and can be simplified.
>
> This commit reworks the .set_rate() functions for the integer
> and fractional PLLs. Upon rate change, the PLL is now locked
> only if it's already enabled.
>
> Also, the driver locks the PLL on .enable(). This makes sure
> the PLL is locked when enabled, and not locked when disabled.
>
> Signed-off-by: Andrew Bresticker <abrestic@xxxxxxxxxxxx>
> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@xxxxxxxxxx>
> ---
> drivers/clk/pistachio/clk-pll.c | 28 ++++++++++------------------
> 1 file changed, 10 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/clk/pistachio/clk-pll.c b/drivers/clk/pistachio/clk-pll.c
> index 9ce1be7..f12d520 100644
> --- a/drivers/clk/pistachio/clk-pll.c
> +++ b/drivers/clk/pistachio/clk-pll.c
> @@ -130,6 +130,8 @@ static int pll_gf40lp_frac_enable(struct clk_hw *hw)
> val &= ~PLL_FRAC_CTRL4_BYPASS;
> pll_writel(pll, val, PLL_CTRL4);
>
> + pll_lock(pll);
> +
> return 0;
> }
>
> @@ -155,17 +157,13 @@ static int pll_gf40lp_frac_set_rate(struct clk_hw *hw, unsigned long rate,
> {
> struct pistachio_clk_pll *pll = to_pistachio_pll(hw);
> struct pistachio_pll_rate_table *params;
> - bool was_enabled;
> + int enabled = pll_gf40lp_frac_is_enabled(hw);
Is there any sort of spinlock here so that we protect the
sleeping set_rate() path against the non-sleeping enable/disable
path? There should be a spinlock of some kind to prevent that,
or the enable/disable for the PLL should move to
prepare/unprepare so that we can't disable the PLL in the middle
of a rate switch.
This is an existing problem though, so I applied this to clk-next
anyway.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
