linux-kernel - Re: [PATCH v3 06/11] clk: qcom: Fix .set_rate to handle alpha PLLs w/wo dynamic update

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]

Date:   Thu, 03 Nov 2016 13:58:07 +0530
From:   Rajendra Nayak <rnayak@...eaurora.org>
To:     Stephen Boyd <sboyd@...eaurora.org>
CC:     mturquette@...libre.com, linux-clk@...r.kernel.org,
        linux-arm-msm@...r.kernel.org, linux-kernel@...r.kernel.org,
        tdas@...eaurora.org
Subject: Re: [PATCH v3 06/11] clk: qcom: Fix .set_rate to handle alpha PLLs
 w/wo dynamic update



On 11/03/2016 03:24 AM, Stephen Boyd wrote:
> On 09/29, Rajendra Nayak wrote:
>> Alpha PLLs which do not support dynamic update feature
>> need to be explicitly disabled before a rate change.
>> The ones which do support dynamic update do so within a
>> single vco range, so add a min/max freq check for such
>> PLLs so they fall in the vco range.
>>
>> Signed-off-by: Taniya Das <tdas@...eaurora.org>
> 
> Is Taniya the author?

ah, yes, I seem to have messed up the authorship in v3, will fix.

> 
>> Signed-off-by: Rajendra Nayak <rnayak@...eaurora.org>
>> ---
>>  drivers/clk/qcom/clk-alpha-pll.c | 49 +++++++++++++++++++++++++++++++++-------
>>  drivers/clk/qcom/clk-alpha-pll.h |  3 +++
>>  2 files changed, 44 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/clk/qcom/clk-alpha-pll.c b/drivers/clk/qcom/clk-alpha-pll.c
>> index 89c7fdb..6f90a86 100644
>> --- a/drivers/clk/qcom/clk-alpha-pll.c
>> +++ b/drivers/clk/qcom/clk-alpha-pll.c
>> @@ -382,16 +382,41 @@ clk_alpha_pll_recalc_rate(struct clk_hw *hw, unsigned long parent_rate)
>>  static int clk_alpha_pll_set_rate(struct clk_hw *hw, unsigned long rate,
>>  				  unsigned long prate)
>>  {
>> +	bool enabled;
>>  	struct clk_alpha_pll *pll = to_clk_alpha_pll(hw);
>>  	const struct pll_vco *vco;
>>  	u32 l, off = pll->offset;
>>  	u64 a;
>>  
>>  	rate = alpha_pll_round_rate(rate, prate, &l, &a);
>> -	vco = alpha_pll_find_vco(pll, rate);
>> -	if (!vco) {
>> -		pr_err("alpha pll not in a valid vco range\n");
>> -		return -EINVAL;
>> +	enabled = clk_hw_is_enabled(hw);
>> +
>> +	if (pll->flags & SUPPORTS_DYNAMIC_UPDATE) {
>> +		/*
>> +		 * PLLs which support dynamic updates support one single
>> +		 * vco range, between min_rate and max_rate supported
>> +		 */
>> +		if (rate < pll->min_rate || rate > pll->max_rate) {
>> +			pr_err("alpha pll rate outside supported min/max range\n");
>> +			return -EINVAL;
>> +		}
>> +	} else {
>> +		/*
>> +		 * All alpha PLLs which do not support dynamic update,
>> +		 * should be disabled before a vco update.
>> +		 */
>> +		if (enabled)
>> +			hw->init->ops->disable(hw);
> 
> Please just call the function directly instead of going through
> the init structure.

But we now have 2 different versions of disable based on clk_ops,
clk_alpha_pll_disable and clk_alpha_pll_hwfsm_disable.

> 
>> +
>> +		vco = alpha_pll_find_vco(pll, rate);
>> +		if (!vco) {
>> +			pr_err("alpha pll not in a valid vco range\n");
>> +			return -EINVAL;
>> +		}
>> +
>> +		regmap_update_bits(pll->clkr.regmap, off + PLL_USER_CTL,
>> +				   PLL_VCO_MASK << PLL_VCO_SHIFT,
>> +				   vco->val << PLL_VCO_SHIFT);
>>  	}
>>  
>>  	regmap_write(pll->clkr.regmap, off + PLL_L_VAL, l);
>> diff --git a/drivers/clk/qcom/clk-alpha-pll.h b/drivers/clk/qcom/clk-alpha-pll.h
>> index d6e1ee2..e43a9c0 100644
>> --- a/drivers/clk/qcom/clk-alpha-pll.h
>> +++ b/drivers/clk/qcom/clk-alpha-pll.h
>> @@ -37,8 +37,11 @@ struct clk_alpha_pll {
>>  #define SUPPORTS_OFFLINE_REQ	BIT(0)
>>  #define SUPPORTS_16BIT_ALPHA	BIT(1)
>>  #define SUPPORTS_FSM_MODE	BIT(2)
>> +#define SUPPORTS_DYNAMIC_UPDATE	BIT(3)
>>  	u8 flags;
>>  
>> +	unsigned long min_rate;
>> +	unsigned long max_rate;
> 
> Document these?

will do, thanks.

> 
>>  	struct clk_regmap clkr;
>>  };
>>  
> 

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation