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 for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250402090147.GF115840@e132581.arm.com>
Date: Wed, 2 Apr 2025 10:01:47 +0100
From: Leo Yan <leo.yan@....com>
To: Jie Gan <quic_jiegan@...cinc.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

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.

Thanks,
Leo

[1] Documentation/devicetree/bindings/arm/primecell.yaml

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ