linux-kernel - Re: [PATCH] clk: qcom: gcc: Update the SDCC clock to use shared_floor

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: <5ba5fb11-96ed-41bd-ba21-f30476cdd570@oss.qualcomm.com>
Date: Wed, 8 Oct 2025 14:26:48 +0200
From: Konrad Dybcio <konrad.dybcio@....qualcomm.com>
To: Taniya Das <taniya.das@....qualcomm.com>,
        Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
Cc: Bjorn Andersson <andersson@...nel.org>,
        Michael Turquette <mturquette@...libre.com>,
        Stephen Boyd
 <sboyd@...nel.org>, Dmitry Baryshkov <lumag@...nel.org>,
        Taniya Das <quic_tdas@...cinc.com>,
        Ajit Pandey <quic_ajipan@...cinc.com>,
        Imran Shaik <quic_imrashai@...cinc.com>,
        Jagadeesh Kona <quic_jkona@...cinc.com>, linux-arm-msm@...r.kernel.org,
        linux-clk@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] clk: qcom: gcc: Update the SDCC clock to use
 shared_floor_ops

On 9/26/25 11:41 AM, Taniya Das wrote:
> 
> 
> On 8/8/2025 5:48 PM, Dmitry Baryshkov wrote:
>> On Fri, Aug 08, 2025 at 02:51:50PM +0530, Taniya Das wrote:
>>>
>>>
>>> On 8/7/2025 10:32 PM, Konrad Dybcio wrote:
>>>> On 8/6/25 11:39 AM, Taniya Das wrote:
>>>>>
>>>>>
>>>>> On 8/6/2025 3:00 PM, Konrad Dybcio wrote:
>>>>>> On 8/6/25 11:27 AM, Taniya Das wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 8/5/2025 10:52 AM, Dmitry Baryshkov wrote:
>>>>>>>> On Mon, Aug 04, 2025 at 11:59:21PM +0530, Taniya Das wrote:
>>>>>>>>> gcc_sdcc2_apps_clk_src: rcg didn't update its configuration" during
>>>>>>>>> boot. This happens due to the floor_ops tries to update the rcg
>>>>>>>>> configuration even if the clock is not enabled.
>>>>>>>>
>>>>>>>> This has been working for other platforms (I see Milos, SAR2130P,
>>>>>>>> SM6375, SC8280XP, SM8550, SM8650 using shared ops, all other platforms
>>>>>>>> seem to use non-shared ops). What's the difference? Should we switch all
>>>>>>>> platforms? Is it related to the hypervisor?
>>>>>>>>
>>>>>>>
>>>>>>> If a set rate is called on a clock before clock enable, the
>>>>>>
>>>>>> Is this something we should just fix up the drivers not to do?
>>>>>>
>>>>>
>>>>> I do not think CCF has any such limitation where the clock should be
>>>>> enabled and then a clock rate should be invoked. We should handle it
>>>>> gracefully and that is what we have now when the caching capabilities
>>>>> were added in the code. This has been already in our downstream drivers.
>>>>
>>>> Should we do CFG caching on *all* RCGs to avoid having to scratch our
>>>> heads over which ops to use with each clock individually?
>>>>
>>>
>>> Yes, Konrad, that’s definitely the cleanest approach. If you're okay
>>> with it, we can proceed with the current change first and then follow up
>>> with a broader cleanup of the rcg2 ops. As part of that, we can also
>>> transition the relevant SDCC clock targets to use floor_ops. This way,
>>> we can avoid the rcg configuration failure logs in the boot sequence on
>>> QCS615.
>>
>> the rcg2_shared_ops have one main usecase - parking of the clock to the
>> safe source. If it is not required for the SDCC clock, then it is
>> incorrect to land this patch.
> 
> Along with the floor functionality we require parking of the clock to
> safe source for SDCC clock. I am reusing the shared_floor_ops introduced
> recently for SAR2130P explicitly for SDCC clocks.
> 
>>
>> If you are saying that we should be caching CFG value for all clock
>> controllers, then we should change instead the clk_rcg2_ops.
>>
> 
> That is not required for all clock controllers and which ever clock
> controller's clock requires it we use rcg2_shared_ops which was updated
> to park the cfg.

I think Dmitry just wanted you to confirm that what you're doing in this
patch is guided by the necessity of safe parking and not only to enable
rcg caching

Konrad