linux-kernel - Re: [PATCH 2/3] clk: pistachio: Lock the PLL when enabled upon rate change

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20150604194736.GG676@codeaurora.org>
Date:	Thu, 4 Jun 2015 12:47:36 -0700
From:	Stephen Boyd <sboyd@...eaurora.org>
To:	Ezequiel Garcia <ezequiel.garcia@...tec.com>
Cc:	linux-mips@...ux-mips.org, linux-kernel@...r.kernel.org,
	Mike Turquette <mturquette@...aro.org>,
	Andrew Bresticker <abrestic@...omium.org>,
	James Hartley <james.hartley@...tec.com>,
	James Hogan <james.hogan@...tec.com>, cernekee@...omium.org,
	Govindraj.Raja@...tec.com, Damien.Horsley@...tec.com
Subject: Re: [PATCH 2/3] clk: pistachio: Lock the PLL when enabled upon rate
 change

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@...omium.org>
> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@...tec.com>
> ---
>  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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/