linux-kernel - Re: [PATCH] clk: qcom: camcc-sc8280xp: Remove always-on GDSC hard-coding

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: <02679111-1a35-b931-fecd-01c952553652@quicinc.com>
Date: Wed, 17 Jul 2024 16:38:32 +0530
From: "Satya Priya Kakitapalli (Temp)" <quic_skakitap@...cinc.com>
To: Bryan O'Donoghue <bryan.odonoghue@...aro.org>,
        Bjorn Andersson
	<andersson@...nel.org>,
        Michael Turquette <mturquette@...libre.com>,
        Stephen
 Boyd <sboyd@...nel.org>
CC: <dmitry.baryshkov@...aro.org>, <stable@...r.kernel.org>,
        <linux-arm-msm@...r.kernel.org>, <linux-clk@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] clk: qcom: camcc-sc8280xp: Remove always-on GDSC
 hard-coding


On 7/17/2024 2:19 PM, Bryan O'Donoghue wrote:
> On 17/07/2024 07:32, Satya Priya Kakitapalli (Temp) wrote:
>>
>> On 7/15/2024 8:29 PM, Bryan O'Donoghue wrote:
>>> We have both shared_ops for the Titan Top GDSC and a hard-coded 
>>> always on
>>> whack the register and forget about it in probe().
>>>
>>> @static struct clk_branch camcc_gdsc_clk = {}
>>>
>>> Only one representation of the Top GDSC is required. Use the CCF
>>> representation not the hard-coded register write.
>>>
>>> Fixes: ff93872a9c61 ("clk: qcom: camcc-sc8280xp: Add sc8280xp CAMCC")
>>> Tested-by: Bryan O'Donoghue <bryan.odonoghue@...aro.org> # Lenovo X13s
>>> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@...aro.org>
>>> ---
>>>   drivers/clk/qcom/camcc-sc8280xp.c | 7 +------
>>>   1 file changed, 1 insertion(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/clk/qcom/camcc-sc8280xp.c 
>>> b/drivers/clk/qcom/camcc-sc8280xp.c
>>> index 479964f91608..f99cd968459c 100644
>>> --- a/drivers/clk/qcom/camcc-sc8280xp.c
>>> +++ b/drivers/clk/qcom/camcc-sc8280xp.c
>>> @@ -3031,19 +3031,14 @@ static int camcc_sc8280xp_probe(struct 
>>> platform_device *pdev)
>>>       clk_lucid_pll_configure(&camcc_pll6, regmap, &camcc_pll6_config);
>>>       clk_lucid_pll_configure(&camcc_pll7, regmap, &camcc_pll7_config);
>>> -    /* Keep some clocks always-on */
>>> -    qcom_branch_set_clk_en(regmap, 0xc1e4); /* CAMCC_GDSC_CLK */
>>
>>
>> As I mentioned on [1], this change might break the GDSC 
>> functionality. Hence this shouldn't be removed.
>
> How would it break ?
>
> We park the clock to XO it never gets turned off this way.
>

Parking the parent at XO doesn't ensure the branch clock is always on, 
it can be disabled by consumers or CCF if modelled.

If the CCF disables this clock in late init, then the clock stays in 
disabled state until it is enabled again explicitly. Hence it is 
recommended to not model such always-on clocks.


> ---
> bod