Re: [PATCH v4 01/15] PCI: imx6: Simplify clock handling by using bulk_clk_*() function
From: Manivannan Sadhasivam <hidden>
Date: 2023-12-17 17:07:07
Also in:
imx, linux-devicetree, linux-pci, lkml
On Sun, Dec 17, 2023 at 12:11:56AM -0500, Frank Li wrote:
Refactors the clock handling logic in the imx6 PCI driver by adding clk_names[] define in drvdata . Simplifies the code and makes it more maintainable, as future additions of SOC support will only require straightforward changes.
Commit description should be in imperative mood as per Documentation/process/submitting-patches.rst: "Describe your changes in imperative mood, e.g. "make xyzzy do frotz" instead of "[This patch] makes xyzzy do frotz" or "[I] changed xyzzy to do frotz", as if you are giving orders to the codebase to change its behaviour."
quoted hunk ↗ jump to hunk
Signed-off-by: Frank Li <Frank.Li@nxp.com> --- Notes: Change from v3 to v4 - using clk_bulk_*() API Change from v1 to v3 - none drivers/pci/controller/dwc/pci-imx6.c | 128 ++++++++------------------ 1 file changed, 38 insertions(+), 90 deletions(-)diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c index 74703362aeec7..2086214345e9a 100644 --- a/drivers/pci/controller/dwc/pci-imx6.c +++ b/drivers/pci/controller/dwc/pci-imx6.c
[...]
quoted hunk ↗ jump to hunk
static void imx6_pcie_assert_core_reset(struct imx6_pcie *imx6_pcie)@@ -1305,32 +1265,19 @@ static int imx6_pcie_probe(struct platform_device *pdev) return imx6_pcie->reset_gpio; } - /* Fetch clocks */ - imx6_pcie->pcie_bus = devm_clk_get(dev, "pcie_bus"); - if (IS_ERR(imx6_pcie->pcie_bus)) - return dev_err_probe(dev, PTR_ERR(imx6_pcie->pcie_bus), - "pcie_bus clock source missing or invalid\n"); + while (imx6_pcie->drvdata->clk_names[imx6_pcie->clks_cnt]) { + int i = imx6_pcie->clks_cnt; - imx6_pcie->pcie = devm_clk_get(dev, "pcie"); - if (IS_ERR(imx6_pcie->pcie)) - return dev_err_probe(dev, PTR_ERR(imx6_pcie->pcie), - "pcie clock source missing or invalid\n"); + imx6_pcie->clks[i].id = imx6_pcie->drvdata->clk_names[i]; + imx6_pcie->clks_cnt++;
You can just initialize clks_cnt in drv_data with sizeof() of clk_names.
quoted hunk ↗ jump to hunk
+ } + + /* Fetch clocks */ + ret = devm_clk_bulk_get(dev, imx6_pcie->clks_cnt, imx6_pcie->clks); + if (ret) + return ret; switch (imx6_pcie->drvdata->variant) { - case IMX6SX: - imx6_pcie->pcie_inbound_axi = devm_clk_get(dev, - "pcie_inbound_axi"); - if (IS_ERR(imx6_pcie->pcie_inbound_axi)) - return dev_err_probe(dev, PTR_ERR(imx6_pcie->pcie_inbound_axi), - "pcie_inbound_axi clock missing or invalid\n"); - break; - case IMX8MQ: - case IMX8MQ_EP: - imx6_pcie->pcie_aux = devm_clk_get(dev, "pcie_aux"); - if (IS_ERR(imx6_pcie->pcie_aux)) - return dev_err_probe(dev, PTR_ERR(imx6_pcie->pcie_aux), - "pcie_aux clock source missing or invalid\n"); - fallthrough; case IMX7D: if (dbi_base->start == IMX8MQ_PCIE2_BASE_ADDR) imx6_pcie->controller_id = 1;@@ -1353,10 +1300,6 @@ static int imx6_pcie_probe(struct platform_device *pdev) case IMX8MM_EP: case IMX8MP: case IMX8MP_EP: - imx6_pcie->pcie_aux = devm_clk_get(dev, "pcie_aux"); - if (IS_ERR(imx6_pcie->pcie_aux)) - return dev_err_probe(dev, PTR_ERR(imx6_pcie->pcie_aux), - "pcie_aux clock source missing or invalid\n"); imx6_pcie->apps_reset = devm_reset_control_get_exclusive(dev, "apps"); if (IS_ERR(imx6_pcie->apps_reset))@@ -1372,14 +1315,6 @@ static int imx6_pcie_probe(struct platform_device *pdev) default: break; } - /* Don't fetch the pcie_phy clock, if it has abstract PHY driver */ - if (imx6_pcie->phy == NULL) { - imx6_pcie->pcie_phy = devm_clk_get(dev, "pcie_phy"); - if (IS_ERR(imx6_pcie->pcie_phy)) - return dev_err_probe(dev, PTR_ERR(imx6_pcie->pcie_phy), - "pcie_phy clock source missing or invalid\n"); - } - /* Grab turnoff reset */ imx6_pcie->turnoff_reset = devm_reset_control_get_optional_exclusive(dev, "turnoff");@@ -1470,6 +1405,9 @@ static void imx6_pcie_shutdown(struct platform_device *pdev) imx6_pcie_assert_core_reset(imx6_pcie); } +#define IMX6_CLKS_COMMON "pcie_bus", "pcie" +#define IMX6_CLKS_NO_PHYDRV IMX6_CLKS_COMMON, "pcie_phy" +
Just use the clock names directly instead of definitions. It makes the code more readable. Rest LGTM! - Mani -- மணிவண்ணன் சதாசிவம் _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel