linux-kernel - Re: [PATCH v1 1/2] PCI: dwc: histb: 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: <20250826190255.GA851588@bhelgaas>
Date: Tue, 26 Aug 2025 14:02:55 -0500
From: Bjorn Helgaas <helgaas@...nel.org>
To: Anand Moon <linux.amoon@...il.com>
Cc: Shawn Guo <shawn.guo@...aro.org>,
	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>,
	Philipp Zabel <p.zabel@...gutronix.de>,
	"open list:PCIE DRIVER FOR HISILICON STB" <linux-pci@...r.kernel.org>,
	open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v1 1/2] PCI: dwc: histb: Simplify clock handling by using
 clk_bulk*() functions

On Tue, Aug 26, 2025 at 11:32:34PM +0530, Anand Moon wrote:
> Hi Bjorn,
> 
> Thanks for your review comments.
> On Tue, 26 Aug 2025 at 21:55, Bjorn Helgaas <helgaas@...nel.org> wrote:
> >
> > In subject, remove "dwc: " to follow historical convention.  (See
> > "git log --oneline")
> >
> Ok I will keep it as per the git history.
> 
> > On Tue, Aug 26, 2025 at 05:12:40PM +0530, 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
> >
> > I assume this means the order in which we prepare/enable and
> > disable/unprepare will now depend on the order the clocks appear in
> > the device tree instead of the order in the code?  If so, please
> > mention that here and verify that all upstream device trees have the
> > correct order.
> >
> Following is the order in the device tree
> 
>        clocks = <&crg HISTB_PCIE_AUX_CLK>,
>                                  <&crg HISTB_PCIE_PIPE_CLK>,
>                                  <&crg HISTB_PCIE_SYS_CLK>,
>                                  <&crg HISTB_PCIE_BUS_CLK>;
>                         clock-names = "aux", "pipe", "sys", "bus";

The current order in the code is:

  histb_pcie_host_enable
    clk_prepare_enable(hipcie->bus_clk);
    clk_prepare_enable(hipcie->sys_clk);
    clk_prepare_enable(hipcie->pipe_clk);
    clk_prepare_enable(hipcie->aux_clk);

  histb_pcie_host_disable
    clk_disable_unprepare(hipcie->aux_clk);
    clk_disable_unprepare(hipcie->pipe_clk);
    clk_disable_unprepare(hipcie->sys_clk);
    clk_disable_unprepare(hipcie->bus_clk);

After this patch, it looks like we'll have:

  histb_pcie_host_enable
    clk_bulk_prepare_enable
      aux
      pipe
      sys
      bus

  histb_pcie_host_disable
    clk_bulk_disable_unprepare
      bus
      sys
      pipe
      aux

So it looks like this patch will reverse the ordering both for
enabling and disabling, right?  I'm totally fine with this as long as
it works.

Bjorn