linux-kernel - Re: [RFC v1 1/2] PCI: tegra: Simplify clock handling by using clk

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: <CANAwSgSZ7ANQVXyWM6wtaOG0tgAbNxoVh7Kz_AaDuxvQsA1b9g@mail.gmail.com>
Date: Thu, 18 Sep 2025 20:36:35 +0530
From: Anand Moon <linux.amoon@...il.com>
To: Jon Hunter <jonathanh@...dia.com>
Cc: Thierry Reding <thierry.reding@...il.com>, Lorenzo Pieralisi <lpieralisi@...nel.org>, 
	Krzysztof Wilczyński <kwilczynski@...nel.org>, 
	Manivannan Sadhasivam <mani@...nel.org>, Rob Herring <robh@...nel.org>, Bjorn Helgaas <bhelgaas@...gle.com>, 
	"open list:PCI DRIVER FOR NVIDIA TEGRA" <linux-tegra@...r.kernel.org>, 
	"open list:PCI DRIVER FOR NVIDIA TEGRA" <linux-pci@...r.kernel.org>, open list <linux-kernel@...r.kernel.org>
Subject: Re: [RFC v1 1/2] PCI: tegra: Simplify clock handling by using
 clk_bulk*() functions

Hi Jon,

On Thu, 18 Sept 2025 at 14:47, Jon Hunter <jonathanh@...dia.com> wrote:
>
>
> On 17/09/2025 19:26, Anand Moon wrote:
> > Hi Jon,
> >
> > On Wed, 17 Sept 2025 at 19:14, Jon Hunter <jonathanh@...dia.com> wrote:
> >>
> >>
> >> On 31/08/2025 20:00, Anand Moon wrote:
> >>> Currently, the driver acquires clocks and prepare/enable/disable/unprepare
> >>> the clocks individually thereby making the driver complex to read.
> >>>
> >>> The driver can be simplified by using the clk_bulk*() APIs.
> >>>
> >>> Use:
> >>>     - devm_clk_bulk_get_all() API to acquire all the clocks
> >>>     - clk_bulk_prepare_enable() to prepare/enable clocks
> >>>     - clk_bulk_disable_unprepare() APIs to disable/unprepare them in bulk
> >>>
> >>> As part of this cleanup, the legacy has_cml_clk flag and explicit handling
> >>> of individual clocks (pex, afi, pll_e, cml) are removed. Clock sequencing
> >>> is now implicitly determined by the order defined in the device tree,
> >>> eliminating hardcoded logic and improving maintainability.
> >>
> >> What platforms have you tested this change on?
> >>
> > I'm using a Jetson Nano 4GB model as my test platform.
>
> Thanks. One concern I have about this is that right now the DT binding
> doc for this device is still in the legacy text format and not converted
> to yaml. Therefore, there is no way to validate the device-tree bindings
> for this driver. So by making this change we are susceptible to people
> getting the device-tree incorrect and there is no way to check. Right
> now the driver will fail is a given clock is missing but after this
> change we are completely reliant that the device-tree is correct but no
> way to validate.
>
> It would be great to convert the text binding doc to yaml as part of
> this series.
>
I will convert the legacy text binding to a YAML file as part of this series.

[0] Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt

> Also if you look at the dwmac-tegra.c driver this one still populates
> the clock names when using the bulk APIs so that we know that the clocks
> that we require are present.
>
Only the Tegra20 SoC has three clocks.
    compatible = "nvidia,tegra20-pcie";
    clocks = <&tegra_car TEGRA20_CLK_PEX>,
                         <&tegra_car TEGRA20_CLK_AFI>,
                         <&tegra_car TEGRA20_CLK_PLL_E>;
                clock-names = "pex", "afi", "pll_e";

Whereas all the rest of the SoCs have 4 clocks.

  compatible = "nvidia,tegra30-pcie";
  compatible = "nvidia,tegra124-pcie";
  compatible = "nvidia,tegra210-pcie";
  compatible = "nvidia,tegra186-pcie";

  clocks = <&tegra_car TEGRA30_CLK_PCIE>,
                         <&tegra_car TEGRA30_CLK_AFI>,
                         <&tegra_car TEGRA30_CLK_PLL_E>,
                         <&tegra_car TEGRA30_CLK_CML0>;
                clock-names = "pex", "afi", "pll_e", "cml";

As suggested, I need to create two clock arrays for the clocks of the SoC.

But the code will introduce more overhead:

bulk clks -> devm_kcalloc (for clocks) -> assign id to clocks ->
devm_clk_bulk_get -> clk_bulk_prepare_enable.

I believe the use of devm_clk_bulk_get_all() is a cleaner and more modern
approach for the following reasons:
It simplifies the code by removing the need for manual memory allocation
(devm_kcalloc) and populating an array of clock specifications.
It is more efficient, as all clocks are fetched in a single API call,
reducing overhead.

Please let me know if this plan addresses your concerns.

> Jon
>
> [0] drivers/net/ethernet/stmicro/stmmac/dwmac-tegra.c
>
> --
> nvpublic
>
Thanks
-Anand