linux-kernel - Re: [PATCH v2] clk: qcom: clk-smd-rpm: Fix the reported rate of branches

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: <08a12dcd-bd96-cd81-66e1-17502cb15cee@linaro.org>
Date:   Fri, 14 Jul 2017 16:01:33 +0300
From:   Georgi Djakov <georgi.djakov@...aro.org>
To:     Stephen Boyd <sboyd@...eaurora.org>
Cc:     mturquette@...libre.com, andy.gross@...aro.org,
        architt@...eaurora.org, hverkuil@...all.nl,
        bjorn.andersson@...aro.org, linux-arm-msm@...r.kernel.org,
        linux-clk@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] clk: qcom: clk-smd-rpm: Fix the reported rate of
 branches

On 07/14/2017 12:56 AM, Stephen Boyd wrote:
> On 07/13, Georgi Djakov wrote:
>> As there is no way to actually query the hardware for the current clock
>> rate, now racalc_rate() just returns the last rate that was previously
>> set. But if the rate was not set yet, we return the bogus rate of 1000Hz.
>>
>> The branch clocks actually have the same rate as their parent (xo_board),
>> so just return this rate.
>>
>> Reported-by: Archit Taneja <architt@...eaurora.org>
>> Fixes: 00f64b58874e ("clk: qcom: Add support for SMD-RPM Clocks")
>> Signed-off-by: Georgi Djakov <georgi.djakov@...aro.org>
>> ---
>>  drivers/clk/qcom/clk-smd-rpm.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/clk/qcom/clk-smd-rpm.c b/drivers/clk/qcom/clk-smd-rpm.c
>> index d990fe44aef3..b45782657ca9 100644
>> --- a/drivers/clk/qcom/clk-smd-rpm.c
>> +++ b/drivers/clk/qcom/clk-smd-rpm.c
>> @@ -364,6 +364,10 @@ static unsigned long clk_smd_rpm_recalc_rate(struct clk_hw *hw,
>>  {
>>  	struct clk_smd_rpm *r = to_clk_smd_rpm(hw);
>>  
>> +	/* Return the parent rate for branches */
>> +	if (r->branch)
>> +		return parent_rate;
>> +
> 
> What's parent_rate here though? 0? I don't see where we parent
> the branch clks to anything.

Its 19.2MHz as xo_board is a fixed-clock in DT. We use the
__DEFINE_CLK_SMD_RPM_BRANCH macro to define all the branch clks
and this macro sets their parent name to "xo_board". Sorry for
not explaining this earlier.

> And we should really just remove the recalc_rate() op for
> branches entirely so that we don't have to call down into the
> driver to find out something we could have known in the core.

Yes this would be better. I agree. Will remove recalc/round_rate
ops for the branch clks.

Thanks,
Georgi