netdev - Re: [PATCH v4 2/8] clk: qcom: ipq5332: enable few nssnoc clocks in driver probe

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: <11fda059-3d8d-4030-922a-8fef16349a65@quicinc.com>
Date: Wed, 14 Feb 2024 14:49:41 +0530
From: Kathiravan Thirumoorthy <quic_kathirav@...cinc.com>
To: Andrew Lunn <andrew@...n.ch>
CC: Bjorn Andersson <andersson@...nel.org>,
        Konrad Dybcio
	<konrad.dybcio@...aro.org>,
        Michael Turquette <mturquette@...libre.com>,
        Stephen Boyd <sboyd@...nel.org>, Rob Herring <robh+dt@...nel.org>,
        Krzysztof
 Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Conor Dooley
	<conor+dt@...nel.org>,
        Richard Cochran <richardcochran@...il.com>,
        Catalin
 Marinas <catalin.marinas@....com>,
        Will Deacon <will@...nel.org>, <linux-arm-msm@...r.kernel.org>,
        <linux-clk@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <devicetree@...r.kernel.org>, <netdev@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v4 2/8] clk: qcom: ipq5332: enable few nssnoc clocks in
 driver probe

On 1/26/2024 1:35 AM, Andrew Lunn wrote:
> On Mon, Jan 22, 2024 at 11:26:58AM +0530, Kathiravan Thirumoorthy wrote:
>> gcc_snoc_nssnoc_clk, gcc_snoc_nssnoc_1_clk, gcc_nssnoc_nsscc_clk are
>> enabled by default and it's RCG is properly configured by bootloader.
> 
> Which bootloader? Mainline barebox?

Thanks for taking time to review the patches. I couldn't get time to 
respond back, sorry for the delay.

I was referring to the U-boot which is delivered as part of the QSDK. I 
will call it out explicitly in the next patch.

> 
>> Some of the NSS clocks needs these clocks to be enabled. To avoid
>> these clocks being disabled by clock framework, drop these entries
>> from the clock table and enable it in the driver probe itself.
> 
> If they are critical clocks, i would expect a device to reference
> them. The CCF only disabled unused clocks in late_initcall_sync(),
> which means all drivers should of probed and taken a reference on any
> clocks they require.

Some of the NSSCC clocks are enabled by bootloaders and CCF disables the 
same (because currently there are no consumers for these clocks 
available in the tree. These clocks are consumed by the Networking 
drivers which are being upstreamed). To access the NSSCC clocks, 
gcc_snoc_nssnoc_clk, gcc_snoc_nssnoc_1_clk, gcc_nssnoc_nsscc_clk clocks 
needs to be enabled, else system is going to reboot. To prevent this, I 
enabled it in probe.

However looking back, gcc_snoc_nssnoc_clk, gcc_snoc_nssnoc_1_clk, 
gcc_nssnoc_nsscc_clk are consumed by the networking drivers only. So is 
it okay to drop these clocks from the GCC driver and add it back once 
the actual consumer needs it? So that we don't have to enable it in probe.

Please let me know your thoughts.

> 
> Please correctly describe the clock tree in device tree, not hide
> clocks because your DT description is not complete.
> 
>      Andrew
> 
> ---
> pw-bot: cr