| 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
| ||
|
Message-ID: <20250826162522.GA838733@bhelgaas>
Date: Tue, 26 Aug 2025 11:25:22 -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
In subject, remove "dwc: " to follow historical convention. (See
"git log --oneline")
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.
> Signed-off-by: Anand Moon <linux.amoon@...il.com>
> ---
> drivers/pci/controller/dwc/pcie-histb.c | 70 ++++---------------------
> 1 file changed, 11 insertions(+), 59 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-histb.c b/drivers/pci/controller/dwc/pcie-histb.c
> index a52071589377..4022349e85d2 100644
> --- a/drivers/pci/controller/dwc/pcie-histb.c
> +++ b/drivers/pci/controller/dwc/pcie-histb.c
> @@ -51,10 +51,8 @@
>
> struct histb_pcie {
> struct dw_pcie *pci;
> - struct clk *aux_clk;
> - struct clk *pipe_clk;
> - struct clk *sys_clk;
> - struct clk *bus_clk;
> + struct clk_bulk_data *clks;
> + int num_clks;
> struct phy *phy;
> struct reset_control *soft_reset;
> struct reset_control *sys_reset;
> @@ -204,10 +202,7 @@ static void histb_pcie_host_disable(struct histb_pcie *hipcie)
> reset_control_assert(hipcie->sys_reset);
> reset_control_assert(hipcie->bus_reset);
>
> - clk_disable_unprepare(hipcie->aux_clk);
> - clk_disable_unprepare(hipcie->pipe_clk);
> - clk_disable_unprepare(hipcie->sys_clk);
> - clk_disable_unprepare(hipcie->bus_clk);
> + clk_bulk_disable_unprepare(hipcie->num_clks, hipcie->clks);
>
> if (hipcie->reset_gpio)
> gpiod_set_value_cansleep(hipcie->reset_gpio, 1);
> @@ -235,28 +230,10 @@ static int histb_pcie_host_enable(struct dw_pcie_rp *pp)
> if (hipcie->reset_gpio)
> gpiod_set_value_cansleep(hipcie->reset_gpio, 0);
>
> - ret = clk_prepare_enable(hipcie->bus_clk);
> + ret = clk_bulk_prepare_enable(hipcie->num_clks, hipcie->clks);
> if (ret) {
> - dev_err(dev, "cannot prepare/enable bus clk\n");
> - goto err_bus_clk;
> - }
> -
> - ret = clk_prepare_enable(hipcie->sys_clk);
> - if (ret) {
> - dev_err(dev, "cannot prepare/enable sys clk\n");
> - goto err_sys_clk;
> - }
> -
> - ret = clk_prepare_enable(hipcie->pipe_clk);
> - if (ret) {
> - dev_err(dev, "cannot prepare/enable pipe clk\n");
> - goto err_pipe_clk;
> - }
> -
> - ret = clk_prepare_enable(hipcie->aux_clk);
> - if (ret) {
> - dev_err(dev, "cannot prepare/enable aux clk\n");
> - goto err_aux_clk;
> + ret = dev_err_probe(dev, ret, "failed to enable clocks\n");
> + goto reg_dis;
> }
>
> reset_control_assert(hipcie->soft_reset);
> @@ -270,13 +247,7 @@ static int histb_pcie_host_enable(struct dw_pcie_rp *pp)
>
> return 0;
>
> -err_aux_clk:
> - clk_disable_unprepare(hipcie->pipe_clk);
> -err_pipe_clk:
> - clk_disable_unprepare(hipcie->sys_clk);
> -err_sys_clk:
> - clk_disable_unprepare(hipcie->bus_clk);
> -err_bus_clk:
> +reg_dis:
> if (hipcie->vpcie)
> regulator_disable(hipcie->vpcie);
>
> @@ -345,29 +316,10 @@ static int histb_pcie_probe(struct platform_device *pdev)
> return ret;
> }
>
> - hipcie->aux_clk = devm_clk_get(dev, "aux");
> - if (IS_ERR(hipcie->aux_clk)) {
> - dev_err(dev, "Failed to get PCIe aux clk\n");
> - return PTR_ERR(hipcie->aux_clk);
> - }
> -
> - hipcie->pipe_clk = devm_clk_get(dev, "pipe");
> - if (IS_ERR(hipcie->pipe_clk)) {
> - dev_err(dev, "Failed to get PCIe pipe clk\n");
> - return PTR_ERR(hipcie->pipe_clk);
> - }
> -
> - hipcie->sys_clk = devm_clk_get(dev, "sys");
> - if (IS_ERR(hipcie->sys_clk)) {
> - dev_err(dev, "Failed to get PCIEe sys clk\n");
> - return PTR_ERR(hipcie->sys_clk);
> - }
> -
> - hipcie->bus_clk = devm_clk_get(dev, "bus");
> - if (IS_ERR(hipcie->bus_clk)) {
> - dev_err(dev, "Failed to get PCIe bus clk\n");
> - return PTR_ERR(hipcie->bus_clk);
> - }
> + hipcie->num_clks = devm_clk_bulk_get_all(dev, &hipcie->clks);
> + if (hipcie->num_clks < 0)
> + return dev_err_probe(dev, hipcie->num_clks,
> + "failed to get clocks\n");
>
> hipcie->soft_reset = devm_reset_control_get(dev, "soft");
> if (IS_ERR(hipcie->soft_reset)) {
> --
> 2.50.1
>
Powered by blists - more mailing lists