linux-kernel - Re: [PATCH v1 9/9] coresight: Consolidate clock enabling

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: <1a496341-cb74-4636-a3dd-9644dee4fc36@quicinc.com>
Date: Thu, 3 Apr 2025 08:29:52 +0800
From: Jie Gan <quic_jiegan@...cinc.com>
To: Leo Yan <leo.yan@....com>
CC: James Clark <james.clark@...aro.org>,
        Suzuki K Poulose
	<suzuki.poulose@....com>,
        Mike Leach <mike.leach@...aro.org>,
        Anshuman
 Khandual <anshuman.khandual@....com>,
        Alexander Shishkin
	<alexander.shishkin@...ux.intel.com>,
        Maxime Coquelin
	<mcoquelin.stm32@...il.com>,
        Alexandre Torgue <alexandre.torgue@...s.st.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        <coresight@...ts.linaro.org>, <linux-arm-kernel@...ts.infradead.org>,
        <linux-kernel@...r.kernel.org>,
        <linux-stm32@...md-mailman.stormreply.com>,
        Rob Herring <robh@...nel.org>
Subject: Re: [PATCH v1 9/9] coresight: Consolidate clock enabling



On 4/2/2025 5:01 PM, Leo Yan wrote:
> Hi Jie,
> 
> [ + Rob ]
> 
> On Wed, Apr 02, 2025 at 08:55:51AM +0800, Jie Gan wrote:
> 
> [...]
> 
>>>>    {
>>>> -    struct clk *pclk = NULL;
>>>> +    WARN_ON(!pclk);
>>>>        if (!dev_is_amba(dev)) {
>>>> -        pclk = devm_clk_get_enabled(dev, "apb_pclk");
>>>> -        if (IS_ERR(pclk))
>>>> -            pclk = devm_clk_get_enabled(dev, "apb");
>>>> +        *pclk = devm_clk_get_enabled(dev, "apb_pclk");
>>>> +        if (IS_ERR(*pclk))
>>>> +            *pclk = devm_clk_get_enabled(dev, "apb");
>>>> +        if (IS_ERR(*pclk))
>>>> +            return PTR_ERR(*pclk);
>>>> +    } else {
>>>> +        /* Don't enable pclk for an AMBA device */
>>>> +        *pclk = NULL;
>>>
>>> Now the "apb" clock won't be enabled for amba devices. I'm assuming
>>> that's fine if the clock was always called "apb_pclk" for them, but the
>>> commit that added the new clock name didn't specify any special casing
>>> either.
>>>
>>> Can we have a comment that says it's deliberate? But the more I think
>>> about it the more I'm confused why CTCU needed a different clock name to
>>> be defined, when all the other Coresight devices use "apb_pclk".
>>
>> Hi James,
>>
>> The original clock-name for CTCU is apb_pclk, but the dt-binding maintainer
>> request me to change it to apb, that's why the clock name is different from
>> others.
>>
>> I am not why we need apb instead of apb_pclk in dt-binding. Maybe some rules
>> have changed for dt-binding requirement.
> 
> My conclusion is that if a device is an Arm Primecell peripheral, it
> should use the clock name "apb_pclk" (See the DT binding doc [1]).
> 
> CTCU is not an Arm Primecell peripheral, so it does not need to strictly
> follow up the clock naming for Primecell peripheral.
> 
> In Arm CoreSight framework, for code consistency, I would suggest
> using the clock naming "apb_pclk" for the APB clock for a newly added
> device that even it is not a Primecell peripheral.
> 
> (We don't need to make any change to the CTCU driver, as we need to
> remain compatible with existed DTB blobs).
> 
> Cc'ing Rob in case he has any suggestions.

Hi Leo,

Thanks for the explanation. I agree with you, we should use the 
"apb_pclk" for the APB clock for a newly added device.

Thanks,
Jie

> 
> Thanks,
> Leo
> 
> [1] Documentation/devicetree/bindings/arm/primecell.yaml