PCI: rockchip: Simplify clock handling by using clk_bulk*() function
PATCH v2 1/3 PCI: rockchip: Simplify clock handling by using clk_bulk*() function
Refactors the clock handling in the Rockchip PCIe driver, introducing a more robust and efficient method for enabling and disabling clocks using clk_bulk*() API. Using the clk_bulk APIs, the clock handling for the core clocks becomes much simpler.
---
Fix compilation error reported by Intel test robot
--- drivers/pci/controller/pcie-rockchip.c | 64 ++++---------------------- drivers/pci/controller/pcie-rockchip.h | 15 ++++-- 2 files changed, 21 insertions(+), 58 deletions(-)
drivers/pci/controller/pcie-rockchip.c +10 -54
@@ -30,7 +30,7 @@ int rockchip_pcie_parse_dt(struct rockchip_pcie *rockchip)struct platform_device *pdev =to_platform_device (dev);struct device_node *node = dev->of_node;struct resource *regs; -int err; +int err, i;if (rockchip->is_rc) { regs = platform_get_resource_byname(pdev,
@@ -127,28 +127,13 @@ int rockchip_pcie_parse_dt(struct rockchip_pcie *rockchip) "failed to get ep GPIO\n"); } - rockchip->aclk_pcie =devm_clk_get (dev,"aclk" ); -if (IS_ERR (rockchip->aclk_pcie)) { -dev_err (dev,"aclk clock not found\n" ); -return PTR_ERR (rockchip->aclk_pcie); - } - - rockchip->aclk_perf_pcie =devm_clk_get (dev,"aclk-perf" ); -if (IS_ERR (rockchip->aclk_perf_pcie)) { -dev_err (dev,"aclk_perf clock not found\n" ); -return PTR_ERR (rockchip->aclk_perf_pcie); - } - - rockchip->hclk_pcie =devm_clk_get (dev,"hclk" ); -if (IS_ERR (rockchip->hclk_pcie)) { -dev_err (dev,"hclk clock not found\n" ); -return PTR_ERR (rockchip->hclk_pcie); - } +for (i =0 ; i <ROCKCHIP_NUM_CLKS ; i++) + rockchip->clks[i].id = rockchip_pci_clks[i]; - rockchip->clk_pcie_pm =devm_clk_get (dev,"pm" ); -if (IS_ERR (rockchip->clk_pcie_pm)) { -dev_err (dev,"pm clock not found\n" ); -return PTR_ERR (rockchip->clk_pcie_pm); + err =devm_clk_bulk_get (dev,ROCKCHIP_NUM_CLKS , rockchip->clks); +if (err) { +dev_err (dev,"rockchip clk bulk get failed\n" ); +return err; }return 0 ;
@@ -372,39 +357,13 @@ int rockchip_pcie_enable_clocks(struct rockchip_pcie *rockchip)struct device *dev = rockchip->dev;int err; - err =clk_prepare_enable (rockchip->aclk_pcie); + err =clk_bulk_prepare_enable (ROCKCHIP_NUM_CLKS , rockchip->clks);if (err) { -dev_err (dev,"unable to enable aclk_pcie clock\n" ); +dev_err (dev,"rockchip clk bulk prepare enable failed\n" );return err; } - err =clk_prepare_enable (rockchip->aclk_perf_pcie); -if (err) { -dev_err (dev,"unable to enable aclk_perf_pcie clock\n" ); - goto err_aclk_perf_pcie; - } - - err =clk_prepare_enable (rockchip->hclk_pcie); -if (err) { -dev_err (dev,"unable to enable hclk_pcie clock\n" ); - goto err_hclk_pcie; - } - - err =clk_prepare_enable (rockchip->clk_pcie_pm); -if (err) { -dev_err (dev,"unable to enable clk_pcie_pm clock\n" ); - goto err_clk_pcie_pm; - } -return 0 ; - -err_clk_pcie_pm: -clk_disable_unprepare (rockchip->hclk_pcie); -err_hclk_pcie: -clk_disable_unprepare (rockchip->aclk_perf_pcie); -err_aclk_perf_pcie: -clk_disable_unprepare (rockchip->aclk_pcie); -return err; }EXPORT_SYMBOL_GPL (rockchip_pcie_enable_clocks);
@@ -412,10 +371,7 @@ void rockchip_pcie_disable_clocks(void *data) {struct rockchip_pcie *rockchip = data; -clk_disable_unprepare (rockchip->clk_pcie_pm); -clk_disable_unprepare (rockchip->hclk_pcie); -clk_disable_unprepare (rockchip->aclk_perf_pcie); -clk_disable_unprepare (rockchip->aclk_pcie); +clk_bulk_disable_unprepare (ROCKCHIP_NUM_CLKS , rockchip->clks); }EXPORT_SYMBOL_GPL (rockchip_pcie_disable_clocks);
drivers/pci/controller/pcie-rockchip.h +11 -4
@@ -11,6 +11,7 @@#ifndef _PCIE_ROCKCHIP_H#define _PCIE_ROCKCHIP_H +#include <linux/clk.h> #include <linux/kernel.h> #include <linux/pci.h> #include <linux/pci-ecam.h>
@@ -287,6 +288,15 @@ (((c) << ((b) *8 +5 )) & \ ROCKCHIP_PCIE_CORE_EP_FUNC_BAR_CFG_BAR_CTRL_MASK(b)) +#define ROCKCHIP_NUM_CLKS ARRAY_SIZE(rockchip_pci_clks) + +static const char *const rockchip_pci_clks[] = { +"aclk" , +"aclk-perf" , +"hclk" , +"pm" , +}; +struct rockchip_pcie {void __iomem *reg_base;/* DT axi-base */ void __iomem *apb_base;/* DT apb-base */
@@ -299,10 +309,7 @@ struct rockchip_pcie {struct reset_control *pm_rst;struct reset_control *aclk_rst;struct reset_control *pclk_rst; -struct clk *aclk_pcie; -struct clk *aclk_perf_pcie; -struct clk *hclk_pcie; -struct clk *clk_pcie_pm; +struct clk_bulk_data clks[ROCKCHIP_NUM_CLKS ];struct regulator *vpcie12v;/* 12V power supply */ struct regulator *vpcie3v3;/* 3.3V power supply */ struct regulator *vpcie1v8;/* 1.8V power supply */
base-commit: 50736169ecc8387247fe6a00932852ce7b057083
Signature · 1 line
2.44.0
PATCH v2 2/3 PCI: rockchip: Simplify reset control handling by using reset_control_bulk*() function
Refactors the reset control clock handling in the Rockchip PCIe driver, introducing a more robust and efficient method for assert and deassert reset controller using reset_control_bulk*() API. Using the reset_control_bulk APIs, the reset handling for the core clocks reset unit becomes much simpler.
As per rockchip rk3399 TRM SOFTRST_CON8 soft reset controller have clock reset unit value set to 0x1 for example "pcie_pipe", "pcie_mgmt_sticky", "pcie_mgmt" and "pci_core", hence group then under one reset bulk controller.
Where as "pcie_pm", "presetn_pcie", "aresetn_pcie" have reset value set to 0x0 ,hence group them under reset control bulk controller.
---
Fix compilation error reported by Intel test robot Fix : checkpatch warning
--- drivers/pci/controller/pcie-rockchip.c | 141 +++++-------------------- drivers/pci/controller/pcie-rockchip.h | 25 +++-- 2 files changed, 47 insertions(+), 119 deletions(-)
drivers/pci/controller/pcie-rockchip.c +29 -112
@@ -69,54 +69,26 @@ int rockchip_pcie_parse_dt(struct rockchip_pcie *rockchip)if (rockchip->link_gen <0 || rockchip->link_gen >2 ) rockchip->link_gen =2 ; - rockchip->core_rst =devm_reset_control_get_exclusive (dev,"core" ); -if (IS_ERR (rockchip->core_rst)) { -if (PTR_ERR (rockchip->core_rst) != -EPROBE_DEFER ) -dev_err (dev,"missing core reset property in node\n" ); -return PTR_ERR (rockchip->core_rst); - } - - rockchip->mgmt_rst =devm_reset_control_get_exclusive (dev,"mgmt" ); -if (IS_ERR (rockchip->mgmt_rst)) { -if (PTR_ERR (rockchip->mgmt_rst) != -EPROBE_DEFER ) -dev_err (dev,"missing mgmt reset property in node\n" ); -return PTR_ERR (rockchip->mgmt_rst); - } - - rockchip->mgmt_sticky_rst =devm_reset_control_get_exclusive (dev, -"mgmt-sticky" ); -if (IS_ERR (rockchip->mgmt_sticky_rst)) { -if (PTR_ERR (rockchip->mgmt_sticky_rst) != -EPROBE_DEFER ) -dev_err (dev,"missing mgmt-sticky reset property in node\n" ); -return PTR_ERR (rockchip->mgmt_sticky_rst); - } - - rockchip->pipe_rst =devm_reset_control_get_exclusive (dev,"pipe" ); -if (IS_ERR (rockchip->pipe_rst)) { -if (PTR_ERR (rockchip->pipe_rst) != -EPROBE_DEFER ) -dev_err (dev,"missing pipe reset property in node\n" ); -return PTR_ERR (rockchip->pipe_rst); - } +for (i =0 ; i <ROCKCHIP_NUM_PM_RSTS ; i++) + rockchip->pm_rsts[i].id = rockchip_pci_pm_rsts[i]; - rockchip->pm_rst =devm_reset_control_get_exclusive (dev,"pm" ); -if (IS_ERR (rockchip->pm_rst)) { -if (PTR_ERR (rockchip->pm_rst) != -EPROBE_DEFER ) -dev_err (dev,"missing pm reset property in node\n" ); -return PTR_ERR (rockchip->pm_rst); + err =devm_reset_control_bulk_get_optional_exclusive (dev, +ROCKCHIP_NUM_PM_RSTS , + rockchip->pm_rsts); +if (err) { +dev_err (dev,"cannot get the devm_reset_control err %d\n" , err); +return err; } - rockchip->pclk_rst =devm_reset_control_get_exclusive (dev,"pclk" ); -if (IS_ERR (rockchip->pclk_rst)) { -if (PTR_ERR (rockchip->pclk_rst) != -EPROBE_DEFER ) -dev_err (dev,"missing pclk reset property in node\n" ); -return PTR_ERR (rockchip->pclk_rst); - } +for (i =0 ; i <ROCKCHIP_NUM_CORE_RSTS ; i++) + rockchip->core_rsts[i].id = rockchip_pci_core_rsts[i]; - rockchip->aclk_rst =devm_reset_control_get_exclusive (dev,"aclk" ); -if (IS_ERR (rockchip->aclk_rst)) { -if (PTR_ERR (rockchip->aclk_rst) != -EPROBE_DEFER ) -dev_err (dev,"missing aclk reset property in node\n" ); -return PTR_ERR (rockchip->aclk_rst); + err =devm_reset_control_bulk_get_optional_exclusive (dev, +ROCKCHIP_NUM_CORE_RSTS , + rockchip->core_rsts); +if (err) { +dev_err (dev,"cannot get the devm_reset_control err %d\n" , err); +return err; }if (rockchip->is_rc) {
@@ -152,21 +124,10 @@ int rockchip_pcie_init_port(struct rockchip_pcie *rockchip)int err, i;u32 regs; - err =reset_control_assert (rockchip->aclk_rst); + err =reset_control_bulk_assert (ROCKCHIP_NUM_PM_RSTS , + rockchip->pm_rsts);if (err) { -dev_err (dev,"assert aclk_rst err %d\n" , err); -return err; - } - - err =reset_control_assert (rockchip->pclk_rst); -if (err) { -dev_err (dev,"assert pclk_rst err %d\n" , err); -return err; - } - - err =reset_control_assert (rockchip->pm_rst); -if (err) { -dev_err (dev,"assert pm_rst err %d\n" , err); +dev_err (dev,"reset bulk assert pm_rsts err %d\n" , err);return err; }
@@ -178,47 +139,19 @@ int rockchip_pcie_init_port(struct rockchip_pcie *rockchip) } } - err =reset_control_assert (rockchip->core_rst); + err =reset_control_bulk_assert (ROCKCHIP_NUM_CORE_RSTS , + rockchip->core_rsts);if (err) { -dev_err (dev,"assert core_rst err %d\n" , err); - goto err_exit_phy; - } - - err =reset_control_assert (rockchip->mgmt_rst); -if (err) { -dev_err (dev,"assert mgmt_rst err %d\n" , err); - goto err_exit_phy; - } - - err =reset_control_assert (rockchip->mgmt_sticky_rst); -if (err) { -dev_err (dev,"assert mgmt_sticky_rst err %d\n" , err); - goto err_exit_phy; - } - - err =reset_control_assert (rockchip->pipe_rst); -if (err) { -dev_err (dev,"assert pipe_rst err %d\n" , err); +dev_err (dev,"reset bulk assert core_rsts err %d\n" , err); goto err_exit_phy; }udelay (10 ); - err =reset_control_deassert (rockchip->pm_rst); -if (err) { -dev_err (dev,"deassert pm_rst err %d\n" , err); - goto err_exit_phy; - } - - err =reset_control_deassert (rockchip->aclk_rst); -if (err) { -dev_err (dev,"deassert aclk_rst err %d\n" , err); - goto err_exit_phy; - } - - err =reset_control_deassert (rockchip->pclk_rst); + err =reset_control_bulk_deassert (ROCKCHIP_NUM_PM_RSTS , + rockchip->pm_rsts);if (err) { -dev_err (dev,"deassert pclk_rst err %d\n" , err); +dev_err (dev,"reset bulk deassert pm_rsts err %d\n" , err); goto err_exit_phy; }
@@ -261,31 +194,15 @@ int rockchip_pcie_init_port(struct rockchip_pcie *rockchip)* Please don't reorder the deassert sequence of the following * four reset pins. */ - err =reset_control_deassert (rockchip->mgmt_sticky_rst); + err =reset_control_bulk_deassert (ROCKCHIP_NUM_CORE_RSTS , + rockchip->core_rsts);if (err) { -dev_err (dev,"deassert mgmt_sticky_rst err %d\n" , err); - goto err_power_off_phy; - } - - err =reset_control_deassert (rockchip->core_rst); -if (err) { -dev_err (dev,"deassert core_rst err %d\n" , err); - goto err_power_off_phy; - } - - err =reset_control_deassert (rockchip->mgmt_rst); -if (err) { -dev_err (dev,"deassert mgmt_rst err %d\n" , err); - goto err_power_off_phy; - } - - err =reset_control_deassert (rockchip->pipe_rst); -if (err) { -dev_err (dev,"deassert pipe_rst err %d\n" , err); +dev_err (dev,"reset bulk deassert core_rsts err %d\n" , err); goto err_power_off_phy; }return 0 ; + err_power_off_phy:while (i--)phy_power_off (rockchip->phys[i]);
drivers/pci/controller/pcie-rockchip.h +18 -7
@@ -15,6 +15,7 @@#include <linux/kernel.h> #include <linux/pci.h> #include <linux/pci-ecam.h> +#include <linux/reset.h> /* * The upper 16 bits of PCIE_CLIENT_CONFIG are a write mask for the lower 16
@@ -289,6 +290,8 @@ ROCKCHIP_PCIE_CORE_EP_FUNC_BAR_CFG_BAR_CTRL_MASK(b))#define ROCKCHIP_NUM_CLKS ARRAY_SIZE(rockchip_pci_clks) +#define ROCKCHIP_NUM_PM_RSTS ARRAY_SIZE(rockchip_pci_pm_rsts) +#define ROCKCHIP_NUM_CORE_RSTS ARRAY_SIZE(rockchip_pci_core_rsts)static const char *const rockchip_pci_clks[] = {"aclk" ,
@@ -297,18 +300,26 @@ static const char * const rockchip_pci_clks[] = { "pm", }; +static const char *const rockchip_pci_pm_rsts[] = { +"pm" , +"pclk" , +"aclk" , +}; + +static const char *const rockchip_pci_core_rsts[] = { +"core" , +"mgmt" , +"mgmt-sticky" , +"pipe" , +}; +struct rockchip_pcie {void __iomem *reg_base;/* DT axi-base */ void __iomem *apb_base;/* DT apb-base */ bool legacy_phy;struct phy *phys[MAX_LANE_NUM ]; -struct reset_control *core_rst; -struct reset_control *mgmt_rst; -struct reset_control *mgmt_sticky_rst; -struct reset_control *pipe_rst; -struct reset_control *pm_rst; -struct reset_control *aclk_rst; -struct reset_control *pclk_rst; +struct reset_control_bulk_data pm_rsts[ROCKCHIP_NUM_PM_RSTS ]; +struct reset_control_bulk_data core_rsts[ROCKCHIP_NUM_CORE_RSTS ];struct clk_bulk_data clks[ROCKCHIP_NUM_CLKS ];struct regulator *vpcie12v;/* 12V power supply */ struct regulator *vpcie3v3;/* 3.3V power supply */
Signature · 1 line
2.44.0
PATCH v2 3/3 PCI: rockchip: Refactor rockchip_pcie_disable_clocks function signature
In reply to Anand MoonUpdated rockchip_pcie_disable_clocks function to accept a struct rockchip pointer instead of a void pointer.
--- drivers/pci/controller/pcie-rockchip.c | 4 +--- drivers/pci/controller/pcie-rockchip.h | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-)
drivers/pci/controller/pcie-rockchip.c +1 -3
@@ -284,10 +284,8 @@ int rockchip_pcie_enable_clocks(struct rockchip_pcie *rockchip) }EXPORT_SYMBOL_GPL (rockchip_pcie_enable_clocks); -void rockchip_pcie_disable_clocks (void *data) +void rockchip_pcie_disable_clocks (struct rockchip_pcie *rockchip) { -struct rockchip_pcie *rockchip = data; -clk_bulk_disable_unprepare (ROCKCHIP_NUM_CLKS , rockchip->clks); }EXPORT_SYMBOL_GPL (rockchip_pcie_disable_clocks);
drivers/pci/controller/pcie-rockchip.h +1 -1
@@ -354,7 +354,7 @@ int rockchip_pcie_init_port(struct rockchip_pcie *rockchip);int rockchip_pcie_get_phys (struct rockchip_pcie *rockchip);void rockchip_pcie_deinit_phys (struct rockchip_pcie *rockchip);int rockchip_pcie_enable_clocks (struct rockchip_pcie *rockchip); -void rockchip_pcie_disable_clocks (void *data); +void rockchip_pcie_disable_clocks (struct rockchip_pcie *rockchip);void rockchip_pcie_cfg_configuration_accesses (struct rockchip_pcie *rockchip,u32 type);
Signature · 1 line
2.44.0
Quoted 7 lines · Anand Moon
Refactors the clock handling in the Rockchip PCIe driver, introducing a more robust and efficient method for enabling and disabling clocks using clk_bulk*() API. Using the
clk_bulkAPIs, the clock handling for the core clocks becomes much simpler.
Drop these two lines, as suggested in the test robot report:
If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags ...
This is a new version of the same patch, so it doesn't need those tags.
The problem you're solving with this patch is that the clock handling is too complicated. The test robot didn't report *that* problem.
Since you'll repost for this, also s/Refactors/Refactor/ in the commit log so this is in imperative mood:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id=v6.9#n94
https://chris.beams.io/posts/git-commit/
Quoted 3 lines
drivers/pci/controller/pcie-rockchip.c | 64 ++++---------------------- drivers/pci/controller/pcie-rockchip.h | 15 ++++-- 2 files changed, 21 insertions(+), 58 deletions(-)
Nice reduction in lines!
Hi Bjorn
Quoted 10 lines · Bjorn Helgaas
Quoted 7 lines · Anand Moon
Refactors the clock handling in the Rockchip PCIe driver, introducing a more robust and efficient method for enabling and disabling clocks using clk_bulk*() API. Using the
clk_bulkAPIs, the clock handling for the core clocks becomes much simpler.
Quoted 8 lines
Drop these two lines, as suggested in the test robot report:
If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags ...
This is a new version of the same patch, so it doesn't need those tags.
Ok.
Quoted 8 lines
The problem you're solving with this patch is that the clock handling is too complicated. The test robot didn't report *that* problem.
Since you'll repost for this, also s/Refactors/Refactor/ in the commit log so this is in imperative mood:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id=v6.9#n94
https://chris.beams.io/posts/git-commit/
Ok, I will follow up on this process in the future.
Quoted 5 lines
Quoted 3 lines
drivers/pci/controller/pcie-rockchip.c | 64 ++++---------------------- drivers/pci/controller/pcie-rockchip.h | 15 ++++-- 2 files changed, 21 insertions(+), 58 deletions(-)Nice reduction in lines!
Thanks
-Anand