All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] Add new resets for pcie-rockchip
@ 2016-10-21  2:43 Shawn Lin
       [not found] ` <1477017836-19317-1-git-send-email-shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Shawn Lin @ 2016-10-21  2:43 UTC (permalink / raw)
  To: Heiko Stuebner, Bjorn Helgaas
  Cc: Rob Herring, devicetree, linux-rockchip, Brian Norris, linux-pci,
	Shawn Lin


Hi Bjorn and Heiko,

Sorry for updating pcie-rockchip so frequently under this development
cycle. This patch is going to fix a urgent issue of missing control for
pm_rst, aclk_rst and pclk_rst. These three resets was controlled by rom code
but now the driver will take over it in order to solve some weird issues
found by MP test. Thanks to that it is still under MP test internally, so
the backward compatibility of dtb won't be a big deal.

And there are still some on-going patches for pcie-rockchip on linux-pci
wasn't merged. All of them was also rebased on your next branch, so hope there
is no conflict if you wish to pick them(including this patch 1), otherwise I could
rebase all of them again. :)



Shawn Lin (2):
  PCI: rockchip: Add three new resets as required properties
  arm64: dts: rockchip: add three new resets for rk3399 PCIe controller

 .../devicetree/bindings/pci/rockchip-pcie.txt      | 11 ++--
 arch/arm64/boot/dts/rockchip/rk3399.dtsi           |  7 ++-
 drivers/pci/host/pcie-rockchip.c                   | 62 ++++++++++++++++++++++
 3 files changed, 75 insertions(+), 5 deletions(-)

-- 
2.3.7

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/2] PCI: rockchip: Add three new resets as required properties
  2016-10-21  2:43 [PATCH 0/2] Add new resets for pcie-rockchip Shawn Lin
@ 2016-10-21  2:43     ` Shawn Lin
  2016-10-21  2:43 ` [PATCH 2/2] arm64: dts: rockchip: add three new resets for rk3399 PCIe controller Shawn Lin
  2016-11-09  0:05 ` [PATCH 0/2] Add new resets for pcie-rockchip Heiko Stuebner
  2 siblings, 0 replies; 11+ messages in thread
From: Shawn Lin @ 2016-10-21  2:43 UTC (permalink / raw)
  To: Heiko Stuebner, Bjorn Helgaas
  Cc: Rob Herring, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Brian Norris,
	linux-pci-u79uwXL29TY76Z2rM5mHXA, Shawn Lin

pm_rst, aclk_rst, pclk_rst was controlled by rom code so the
software wasn't needed to control it again in theory. But it
didn't work properly, so we do need to do it again and add a
enough delay between the assert of pm_rst and the deassert of
pm_rst. The Soc intergrated with this controller, rk3399 is still
under MP test internally, so the backward compatibility won't be
a big deal.

Signed-off-by: Shawn Lin <shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>

---

 .../devicetree/bindings/pci/rockchip-pcie.txt      | 11 ++--
 drivers/pci/host/pcie-rockchip.c                   | 62 ++++++++++++++++++++++
 2 files changed, 70 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/rockchip-pcie.txt b/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
index ba67b39..71aeda1 100644
--- a/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
+++ b/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
@@ -26,13 +26,16 @@ Required properties:
 	- "sys"
 	- "legacy"
 	- "client"
-- resets: Must contain five entries for each entry in reset-names.
+- resets: Must contain seven entries for each entry in reset-names.
 	   See ../reset/reset.txt for details.
 - reset-names: Must include the following names
 	- "core"
 	- "mgmt"
 	- "mgmt-sticky"
 	- "pipe"
+	- "pm"
+	- "aclk"
+	- "pclk"
 - pinctrl-names : The pin control state names
 - pinctrl-0: The "default" pinctrl state
 - #interrupt-cells: specifies the number of cells needed to encode an
@@ -86,8 +89,10 @@ pcie0: pcie@f8000000 {
 	reg = <0x0 0xf8000000 0x0 0x2000000>, <0x0 0xfd000000 0x0 0x1000000>;
 	reg-names = "axi-base", "apb-base";
 	resets = <&cru SRST_PCIE_CORE>, <&cru SRST_PCIE_MGMT>,
-		 <&cru SRST_PCIE_MGMT_STICKY>, <&cru SRST_PCIE_PIPE>;
-	reset-names = "core", "mgmt", "mgmt-sticky", "pipe";
+		 <&cru SRST_PCIE_MGMT_STICKY>, <&cru SRST_PCIE_PIPE> ,
+		 <&cru SRST_PCIE_PM>, <&cru SRST_P_PCIE>, <&cru SRST_A_PCIE>;
+	reset-names = "core", "mgmt", "mgmt-sticky", "pipe",
+		      "pm", "pclk", "aclk";
 	phys = <&pcie_phy>;
 	phy-names = "pcie-phy";
 	pinctrl-names = "default";
diff --git a/drivers/pci/host/pcie-rockchip.c b/drivers/pci/host/pcie-rockchip.c
index e0b22da..e04f69b 100644
--- a/drivers/pci/host/pcie-rockchip.c
+++ b/drivers/pci/host/pcie-rockchip.c
@@ -190,6 +190,9 @@ struct rockchip_pcie {
 	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	clk *aclk_pcie;
 	struct	clk *aclk_perf_pcie;
 	struct	clk *hclk_pcie;
@@ -408,6 +411,44 @@ static int rockchip_pcie_init_port(struct rockchip_pcie *rockchip)
 
 	gpiod_set_value(rockchip->ep_gpio, 0);
 
+	err = reset_control_assert(rockchip->aclk_rst);
+	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);
+		return err;
+	}
+
+	udelay(10);
+
+	err = reset_control_deassert(rockchip->pm_rst);
+	if (err) {
+		dev_err(dev, "deassert pm_rst err %d\n", err);
+		return err;
+	}
+
+	err = reset_control_deassert(rockchip->aclk_rst);
+	if (err) {
+		dev_err(dev, "deassert mgmt_sticky_rst err %d\n", err);
+		return err;
+	}
+
+	err = reset_control_deassert(rockchip->pclk_rst);
+	if (err) {
+		dev_err(dev, "deassert mgmt_sticky_rst err %d\n", err);
+		return err;
+	}
+
 	err = phy_init(rockchip->phy);
 	if (err < 0) {
 		dev_err(dev, "fail to init phy, err %d\n", err);
@@ -781,6 +822,27 @@ static int rockchip_pcie_parse_dt(struct rockchip_pcie *rockchip)
 		return PTR_ERR(rockchip->pipe_rst);
 	}
 
+	rockchip->pm_rst = devm_reset_control_get(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);
+	}
+
+	rockchip->pclk_rst = devm_reset_control_get(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);
+	}
+
+	rockchip->aclk_rst = devm_reset_control_get(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);
+	}
+
 	rockchip->ep_gpio = devm_gpiod_get(dev, "ep", GPIOD_OUT_HIGH);
 	if (IS_ERR(rockchip->ep_gpio)) {
 		dev_err(dev, "missing ep-gpios property in node\n");
-- 
2.3.7


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 1/2] PCI: rockchip: Add three new resets as required properties
@ 2016-10-21  2:43     ` Shawn Lin
  0 siblings, 0 replies; 11+ messages in thread
From: Shawn Lin @ 2016-10-21  2:43 UTC (permalink / raw)
  To: Heiko Stuebner, Bjorn Helgaas
  Cc: Rob Herring, devicetree, linux-rockchip, Brian Norris, linux-pci,
	Shawn Lin

pm_rst, aclk_rst, pclk_rst was controlled by rom code so the
software wasn't needed to control it again in theory. But it
didn't work properly, so we do need to do it again and add a
enough delay between the assert of pm_rst and the deassert of
pm_rst. The Soc intergrated with this controller, rk3399 is still
under MP test internally, so the backward compatibility won't be
a big deal.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>

---

 .../devicetree/bindings/pci/rockchip-pcie.txt      | 11 ++--
 drivers/pci/host/pcie-rockchip.c                   | 62 ++++++++++++++++++++++
 2 files changed, 70 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/rockchip-pcie.txt b/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
index ba67b39..71aeda1 100644
--- a/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
+++ b/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
@@ -26,13 +26,16 @@ Required properties:
 	- "sys"
 	- "legacy"
 	- "client"
-- resets: Must contain five entries for each entry in reset-names.
+- resets: Must contain seven entries for each entry in reset-names.
 	   See ../reset/reset.txt for details.
 - reset-names: Must include the following names
 	- "core"
 	- "mgmt"
 	- "mgmt-sticky"
 	- "pipe"
+	- "pm"
+	- "aclk"
+	- "pclk"
 - pinctrl-names : The pin control state names
 - pinctrl-0: The "default" pinctrl state
 - #interrupt-cells: specifies the number of cells needed to encode an
@@ -86,8 +89,10 @@ pcie0: pcie@f8000000 {
 	reg = <0x0 0xf8000000 0x0 0x2000000>, <0x0 0xfd000000 0x0 0x1000000>;
 	reg-names = "axi-base", "apb-base";
 	resets = <&cru SRST_PCIE_CORE>, <&cru SRST_PCIE_MGMT>,
-		 <&cru SRST_PCIE_MGMT_STICKY>, <&cru SRST_PCIE_PIPE>;
-	reset-names = "core", "mgmt", "mgmt-sticky", "pipe";
+		 <&cru SRST_PCIE_MGMT_STICKY>, <&cru SRST_PCIE_PIPE> ,
+		 <&cru SRST_PCIE_PM>, <&cru SRST_P_PCIE>, <&cru SRST_A_PCIE>;
+	reset-names = "core", "mgmt", "mgmt-sticky", "pipe",
+		      "pm", "pclk", "aclk";
 	phys = <&pcie_phy>;
 	phy-names = "pcie-phy";
 	pinctrl-names = "default";
diff --git a/drivers/pci/host/pcie-rockchip.c b/drivers/pci/host/pcie-rockchip.c
index e0b22da..e04f69b 100644
--- a/drivers/pci/host/pcie-rockchip.c
+++ b/drivers/pci/host/pcie-rockchip.c
@@ -190,6 +190,9 @@ struct rockchip_pcie {
 	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	clk *aclk_pcie;
 	struct	clk *aclk_perf_pcie;
 	struct	clk *hclk_pcie;
@@ -408,6 +411,44 @@ static int rockchip_pcie_init_port(struct rockchip_pcie *rockchip)
 
 	gpiod_set_value(rockchip->ep_gpio, 0);
 
+	err = reset_control_assert(rockchip->aclk_rst);
+	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);
+		return err;
+	}
+
+	udelay(10);
+
+	err = reset_control_deassert(rockchip->pm_rst);
+	if (err) {
+		dev_err(dev, "deassert pm_rst err %d\n", err);
+		return err;
+	}
+
+	err = reset_control_deassert(rockchip->aclk_rst);
+	if (err) {
+		dev_err(dev, "deassert mgmt_sticky_rst err %d\n", err);
+		return err;
+	}
+
+	err = reset_control_deassert(rockchip->pclk_rst);
+	if (err) {
+		dev_err(dev, "deassert mgmt_sticky_rst err %d\n", err);
+		return err;
+	}
+
 	err = phy_init(rockchip->phy);
 	if (err < 0) {
 		dev_err(dev, "fail to init phy, err %d\n", err);
@@ -781,6 +822,27 @@ static int rockchip_pcie_parse_dt(struct rockchip_pcie *rockchip)
 		return PTR_ERR(rockchip->pipe_rst);
 	}
 
+	rockchip->pm_rst = devm_reset_control_get(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);
+	}
+
+	rockchip->pclk_rst = devm_reset_control_get(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);
+	}
+
+	rockchip->aclk_rst = devm_reset_control_get(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);
+	}
+
 	rockchip->ep_gpio = devm_gpiod_get(dev, "ep", GPIOD_OUT_HIGH);
 	if (IS_ERR(rockchip->ep_gpio)) {
 		dev_err(dev, "missing ep-gpios property in node\n");
-- 
2.3.7



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/2] arm64: dts: rockchip: add three new resets for rk3399 PCIe controller
  2016-10-21  2:43 [PATCH 0/2] Add new resets for pcie-rockchip Shawn Lin
       [not found] ` <1477017836-19317-1-git-send-email-shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
@ 2016-10-21  2:43 ` Shawn Lin
       [not found]   ` <1477017836-19317-3-git-send-email-shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
  2016-11-09  0:05 ` [PATCH 0/2] Add new resets for pcie-rockchip Heiko Stuebner
  2 siblings, 1 reply; 11+ messages in thread
From: Shawn Lin @ 2016-10-21  2:43 UTC (permalink / raw)
  To: Heiko Stuebner, Bjorn Helgaas
  Cc: Rob Herring, devicetree, linux-rockchip, Brian Norris, linux-pci,
	Shawn Lin

pm_rst, aclk_rst and pclk_rst should be controlled by driver, so we
need to add these three resets for PCIe controller.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

 arch/arm64/boot/dts/rockchip/rk3399.dtsi | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index 76b2864..5191cfd 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -300,8 +300,11 @@
 		ranges = <0x83000000 0x0 0xfa000000 0x0 0xfa000000 0x0 0x600000
 			  0x81000000 0x0 0xfa600000 0x0 0xfa600000 0x0 0x100000>;
 		resets = <&cru SRST_PCIE_CORE>, <&cru SRST_PCIE_MGMT>,
-			 <&cru SRST_PCIE_MGMT_STICKY>, <&cru SRST_PCIE_PIPE>;
-		reset-names = "core", "mgmt", "mgmt-sticky", "pipe";
+			 <&cru SRST_PCIE_MGMT_STICKY>, <&cru SRST_PCIE_PIPE>,
+			 <&cru SRST_PCIE_PM>, <&cru SRST_P_PCIE>,
+			 <&cru SRST_A_PCIE>;
+		reset-names = "core", "mgmt", "mgmt-sticky", "pipe",
+			      "pm", "pclk", "aclk";
 		status = "disabled";
 
 		pcie0_intc: interrupt-controller {
-- 
2.3.7

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] PCI: rockchip: Add three new resets as required properties
  2016-10-21  2:43     ` Shawn Lin
@ 2016-10-21  7:56         ` Heiko Stuebner
  -1 siblings, 0 replies; 11+ messages in thread
From: Heiko Stuebner @ 2016-10-21  7:56 UTC (permalink / raw)
  To: Shawn Lin
  Cc: Bjorn Helgaas, Rob Herring, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Brian Norris,
	linux-pci-u79uwXL29TY76Z2rM5mHXA

Am Freitag, 21. Oktober 2016, 10:43:55 CEST schrieb Shawn Lin:
> pm_rst, aclk_rst, pclk_rst was controlled by rom code so the
> software wasn't needed to control it again in theory. But it
> didn't work properly, so we do need to do it again and add a
> enough delay between the assert of pm_rst and the deassert of
> pm_rst. The Soc intergrated with this controller, rk3399 is still
> under MP test internally, so the backward compatibility won't be
> a big deal.
> 
> Signed-off-by: Shawn Lin <shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>

looks good from my side.
Reviewed-by: Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>

As Shawn wrote the soc in question (first one at all with a pcie controller) 
has not yet found its way into any device, so no existing one can abviously be 
affected by this change. So doing it this way will hopefully be ok.

Obviously it would probably be nice, to have this in 4.9-rc to keep the gap 
small.


Heiko

>  .../devicetree/bindings/pci/rockchip-pcie.txt      | 11 ++--
>  drivers/pci/host/pcie-rockchip.c                   | 62
> ++++++++++++++++++++++ 2 files changed, 70 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
> b/Documentation/devicetree/bindings/pci/rockchip-pcie.txt index
> ba67b39..71aeda1 100644
> --- a/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
> @@ -26,13 +26,16 @@ Required properties:
>  	- "sys"
>  	- "legacy"
>  	- "client"
> -- resets: Must contain five entries for each entry in reset-names.
> +- resets: Must contain seven entries for each entry in reset-names.
>  	   See ../reset/reset.txt for details.
>  - reset-names: Must include the following names
>  	- "core"
>  	- "mgmt"
>  	- "mgmt-sticky"
>  	- "pipe"
> +	- "pm"
> +	- "aclk"
> +	- "pclk"
>  - pinctrl-names : The pin control state names
>  - pinctrl-0: The "default" pinctrl state
>  - #interrupt-cells: specifies the number of cells needed to encode an
> @@ -86,8 +89,10 @@ pcie0: pcie@f8000000 {
>  	reg = <0x0 0xf8000000 0x0 0x2000000>, <0x0 0xfd000000 0x0 0x1000000>;
>  	reg-names = "axi-base", "apb-base";
>  	resets = <&cru SRST_PCIE_CORE>, <&cru SRST_PCIE_MGMT>,
> -		 <&cru SRST_PCIE_MGMT_STICKY>, <&cru SRST_PCIE_PIPE>;
> -	reset-names = "core", "mgmt", "mgmt-sticky", "pipe";
> +		 <&cru SRST_PCIE_MGMT_STICKY>, <&cru SRST_PCIE_PIPE> ,
> +		 <&cru SRST_PCIE_PM>, <&cru SRST_P_PCIE>, <&cru SRST_A_PCIE>;
> +	reset-names = "core", "mgmt", "mgmt-sticky", "pipe",
> +		      "pm", "pclk", "aclk";
>  	phys = <&pcie_phy>;
>  	phy-names = "pcie-phy";
>  	pinctrl-names = "default";
> diff --git a/drivers/pci/host/pcie-rockchip.c
> b/drivers/pci/host/pcie-rockchip.c index e0b22da..e04f69b 100644
> --- a/drivers/pci/host/pcie-rockchip.c
> +++ b/drivers/pci/host/pcie-rockchip.c
> @@ -190,6 +190,9 @@ struct rockchip_pcie {
>  	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	clk *aclk_pcie;
>  	struct	clk *aclk_perf_pcie;
>  	struct	clk *hclk_pcie;
> @@ -408,6 +411,44 @@ static int rockchip_pcie_init_port(struct rockchip_pcie
> *rockchip)
> 
>  	gpiod_set_value(rockchip->ep_gpio, 0);
> 
> +	err = reset_control_assert(rockchip->aclk_rst);
> +	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);
> +		return err;
> +	}
> +
> +	udelay(10);
> +
> +	err = reset_control_deassert(rockchip->pm_rst);
> +	if (err) {
> +		dev_err(dev, "deassert pm_rst err %d\n", err);
> +		return err;
> +	}
> +
> +	err = reset_control_deassert(rockchip->aclk_rst);
> +	if (err) {
> +		dev_err(dev, "deassert mgmt_sticky_rst err %d\n", err);
> +		return err;
> +	}
> +
> +	err = reset_control_deassert(rockchip->pclk_rst);
> +	if (err) {
> +		dev_err(dev, "deassert mgmt_sticky_rst err %d\n", err);
> +		return err;
> +	}
> +
>  	err = phy_init(rockchip->phy);
>  	if (err < 0) {
>  		dev_err(dev, "fail to init phy, err %d\n", err);
> @@ -781,6 +822,27 @@ static int rockchip_pcie_parse_dt(struct rockchip_pcie
> *rockchip) return PTR_ERR(rockchip->pipe_rst);
>  	}
> 
> +	rockchip->pm_rst = devm_reset_control_get(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);
> +	}
> +
> +	rockchip->pclk_rst = devm_reset_control_get(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);
> +	}
> +
> +	rockchip->aclk_rst = devm_reset_control_get(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);
> +	}
> +
>  	rockchip->ep_gpio = devm_gpiod_get(dev, "ep", GPIOD_OUT_HIGH);
>  	if (IS_ERR(rockchip->ep_gpio)) {
>  		dev_err(dev, "missing ep-gpios property in node\n");


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] PCI: rockchip: Add three new resets as required properties
@ 2016-10-21  7:56         ` Heiko Stuebner
  0 siblings, 0 replies; 11+ messages in thread
From: Heiko Stuebner @ 2016-10-21  7:56 UTC (permalink / raw)
  To: Shawn Lin
  Cc: Bjorn Helgaas, Rob Herring, devicetree, linux-rockchip,
	Brian Norris, linux-pci

Am Freitag, 21. Oktober 2016, 10:43:55 CEST schrieb Shawn Lin:
> pm_rst, aclk_rst, pclk_rst was controlled by rom code so the
> software wasn't needed to control it again in theory. But it
> didn't work properly, so we do need to do it again and add a
> enough delay between the assert of pm_rst and the deassert of
> pm_rst. The Soc intergrated with this controller, rk3399 is still
> under MP test internally, so the backward compatibility won't be
> a big deal.
> 
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>

looks good from my side.
Reviewed-by: Heiko Stuebner <heiko@sntech.de>

As Shawn wrote the soc in question (first one at all with a pcie controller) 
has not yet found its way into any device, so no existing one can abviously be 
affected by this change. So doing it this way will hopefully be ok.

Obviously it would probably be nice, to have this in 4.9-rc to keep the gap 
small.


Heiko

>  .../devicetree/bindings/pci/rockchip-pcie.txt      | 11 ++--
>  drivers/pci/host/pcie-rockchip.c                   | 62
> ++++++++++++++++++++++ 2 files changed, 70 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
> b/Documentation/devicetree/bindings/pci/rockchip-pcie.txt index
> ba67b39..71aeda1 100644
> --- a/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
> @@ -26,13 +26,16 @@ Required properties:
>  	- "sys"
>  	- "legacy"
>  	- "client"
> -- resets: Must contain five entries for each entry in reset-names.
> +- resets: Must contain seven entries for each entry in reset-names.
>  	   See ../reset/reset.txt for details.
>  - reset-names: Must include the following names
>  	- "core"
>  	- "mgmt"
>  	- "mgmt-sticky"
>  	- "pipe"
> +	- "pm"
> +	- "aclk"
> +	- "pclk"
>  - pinctrl-names : The pin control state names
>  - pinctrl-0: The "default" pinctrl state
>  - #interrupt-cells: specifies the number of cells needed to encode an
> @@ -86,8 +89,10 @@ pcie0: pcie@f8000000 {
>  	reg = <0x0 0xf8000000 0x0 0x2000000>, <0x0 0xfd000000 0x0 0x1000000>;
>  	reg-names = "axi-base", "apb-base";
>  	resets = <&cru SRST_PCIE_CORE>, <&cru SRST_PCIE_MGMT>,
> -		 <&cru SRST_PCIE_MGMT_STICKY>, <&cru SRST_PCIE_PIPE>;
> -	reset-names = "core", "mgmt", "mgmt-sticky", "pipe";
> +		 <&cru SRST_PCIE_MGMT_STICKY>, <&cru SRST_PCIE_PIPE> ,
> +		 <&cru SRST_PCIE_PM>, <&cru SRST_P_PCIE>, <&cru SRST_A_PCIE>;
> +	reset-names = "core", "mgmt", "mgmt-sticky", "pipe",
> +		      "pm", "pclk", "aclk";
>  	phys = <&pcie_phy>;
>  	phy-names = "pcie-phy";
>  	pinctrl-names = "default";
> diff --git a/drivers/pci/host/pcie-rockchip.c
> b/drivers/pci/host/pcie-rockchip.c index e0b22da..e04f69b 100644
> --- a/drivers/pci/host/pcie-rockchip.c
> +++ b/drivers/pci/host/pcie-rockchip.c
> @@ -190,6 +190,9 @@ struct rockchip_pcie {
>  	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	clk *aclk_pcie;
>  	struct	clk *aclk_perf_pcie;
>  	struct	clk *hclk_pcie;
> @@ -408,6 +411,44 @@ static int rockchip_pcie_init_port(struct rockchip_pcie
> *rockchip)
> 
>  	gpiod_set_value(rockchip->ep_gpio, 0);
> 
> +	err = reset_control_assert(rockchip->aclk_rst);
> +	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);
> +		return err;
> +	}
> +
> +	udelay(10);
> +
> +	err = reset_control_deassert(rockchip->pm_rst);
> +	if (err) {
> +		dev_err(dev, "deassert pm_rst err %d\n", err);
> +		return err;
> +	}
> +
> +	err = reset_control_deassert(rockchip->aclk_rst);
> +	if (err) {
> +		dev_err(dev, "deassert mgmt_sticky_rst err %d\n", err);
> +		return err;
> +	}
> +
> +	err = reset_control_deassert(rockchip->pclk_rst);
> +	if (err) {
> +		dev_err(dev, "deassert mgmt_sticky_rst err %d\n", err);
> +		return err;
> +	}
> +
>  	err = phy_init(rockchip->phy);
>  	if (err < 0) {
>  		dev_err(dev, "fail to init phy, err %d\n", err);
> @@ -781,6 +822,27 @@ static int rockchip_pcie_parse_dt(struct rockchip_pcie
> *rockchip) return PTR_ERR(rockchip->pipe_rst);
>  	}
> 
> +	rockchip->pm_rst = devm_reset_control_get(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);
> +	}
> +
> +	rockchip->pclk_rst = devm_reset_control_get(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);
> +	}
> +
> +	rockchip->aclk_rst = devm_reset_control_get(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);
> +	}
> +
>  	rockchip->ep_gpio = devm_gpiod_get(dev, "ep", GPIOD_OUT_HIGH);
>  	if (IS_ERR(rockchip->ep_gpio)) {
>  		dev_err(dev, "missing ep-gpios property in node\n");



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/2] arm64: dts: rockchip: add three new resets for rk3399 PCIe controller
  2016-10-21  2:43 ` [PATCH 2/2] arm64: dts: rockchip: add three new resets for rk3399 PCIe controller Shawn Lin
@ 2016-10-21  7:58       ` Heiko Stuebner
  0 siblings, 0 replies; 11+ messages in thread
From: Heiko Stuebner @ 2016-10-21  7:58 UTC (permalink / raw)
  To: Shawn Lin
  Cc: Bjorn Helgaas, Rob Herring, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Brian Norris,
	linux-pci-u79uwXL29TY76Z2rM5mHXA

Hi Bjorn,

Am Freitag, 21. Oktober 2016, 10:43:56 CEST schrieb Shawn Lin:
> pm_rst, aclk_rst and pclk_rst should be controlled by driver, so we
> need to add these three resets for PCIe controller.
> 
> Signed-off-by: Shawn Lin <shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>

If patch1 is ok and you want to take both patches at once,
that is fine by me, so
Acked-by: Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>

Otherwise I can also pick this one up after (and if) your ok with patch1.


Heiko

> ---
> 
>  arch/arm64/boot/dts/rockchip/rk3399.dtsi | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> b/arch/arm64/boot/dts/rockchip/rk3399.dtsi index 76b2864..5191cfd 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> @@ -300,8 +300,11 @@
>  		ranges = <0x83000000 0x0 0xfa000000 0x0 0xfa000000 0x0 0x600000
>  			  0x81000000 0x0 0xfa600000 0x0 0xfa600000 0x0 0x100000>;
>  		resets = <&cru SRST_PCIE_CORE>, <&cru SRST_PCIE_MGMT>,
> -			 <&cru SRST_PCIE_MGMT_STICKY>, <&cru SRST_PCIE_PIPE>;
> -		reset-names = "core", "mgmt", "mgmt-sticky", "pipe";
> +			 <&cru SRST_PCIE_MGMT_STICKY>, <&cru SRST_PCIE_PIPE>,
> +			 <&cru SRST_PCIE_PM>, <&cru SRST_P_PCIE>,
> +			 <&cru SRST_A_PCIE>;
> +		reset-names = "core", "mgmt", "mgmt-sticky", "pipe",
> +			      "pm", "pclk", "aclk";
>  		status = "disabled";
> 
>  		pcie0_intc: interrupt-controller {


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/2] arm64: dts: rockchip: add three new resets for rk3399 PCIe controller
@ 2016-10-21  7:58       ` Heiko Stuebner
  0 siblings, 0 replies; 11+ messages in thread
From: Heiko Stuebner @ 2016-10-21  7:58 UTC (permalink / raw)
  To: Shawn Lin
  Cc: Bjorn Helgaas, Rob Herring, devicetree, linux-rockchip,
	Brian Norris, linux-pci

Hi Bjorn,

Am Freitag, 21. Oktober 2016, 10:43:56 CEST schrieb Shawn Lin:
> pm_rst, aclk_rst and pclk_rst should be controlled by driver, so we
> need to add these three resets for PCIe controller.
> 
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>

If patch1 is ok and you want to take both patches at once,
that is fine by me, so
Acked-by: Heiko Stuebner <heiko@sntech.de>

Otherwise I can also pick this one up after (and if) your ok with patch1.


Heiko

> ---
> 
>  arch/arm64/boot/dts/rockchip/rk3399.dtsi | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> b/arch/arm64/boot/dts/rockchip/rk3399.dtsi index 76b2864..5191cfd 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> @@ -300,8 +300,11 @@
>  		ranges = <0x83000000 0x0 0xfa000000 0x0 0xfa000000 0x0 0x600000
>  			  0x81000000 0x0 0xfa600000 0x0 0xfa600000 0x0 0x100000>;
>  		resets = <&cru SRST_PCIE_CORE>, <&cru SRST_PCIE_MGMT>,
> -			 <&cru SRST_PCIE_MGMT_STICKY>, <&cru SRST_PCIE_PIPE>;
> -		reset-names = "core", "mgmt", "mgmt-sticky", "pipe";
> +			 <&cru SRST_PCIE_MGMT_STICKY>, <&cru SRST_PCIE_PIPE>,
> +			 <&cru SRST_PCIE_PM>, <&cru SRST_P_PCIE>,
> +			 <&cru SRST_A_PCIE>;
> +		reset-names = "core", "mgmt", "mgmt-sticky", "pipe",
> +			      "pm", "pclk", "aclk";
>  		status = "disabled";
> 
>  		pcie0_intc: interrupt-controller {



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] PCI: rockchip: Add three new resets as required properties
  2016-10-21  2:43     ` Shawn Lin
@ 2016-10-27 14:54         ` Rob Herring
  -1 siblings, 0 replies; 11+ messages in thread
From: Rob Herring @ 2016-10-27 14:54 UTC (permalink / raw)
  To: Shawn Lin
  Cc: Heiko Stuebner, Bjorn Helgaas, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Brian Norris,
	linux-pci-u79uwXL29TY76Z2rM5mHXA

On Fri, Oct 21, 2016 at 10:43:55AM +0800, Shawn Lin wrote:
> pm_rst, aclk_rst, pclk_rst was controlled by rom code so the
> software wasn't needed to control it again in theory. But it
> didn't work properly, so we do need to do it again and add a
> enough delay between the assert of pm_rst and the deassert of
> pm_rst. The Soc intergrated with this controller, rk3399 is still
> under MP test internally, so the backward compatibility won't be
> a big deal.
> 
> Signed-off-by: Shawn Lin <shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> 
> ---
> 
>  .../devicetree/bindings/pci/rockchip-pcie.txt      | 11 ++--

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

>  drivers/pci/host/pcie-rockchip.c                   | 62 ++++++++++++++++++++++
>  2 files changed, 70 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/rockchip-pcie.txt b/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
> index ba67b39..71aeda1 100644
> --- a/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
> @@ -26,13 +26,16 @@ Required properties:
>  	- "sys"
>  	- "legacy"
>  	- "client"
> -- resets: Must contain five entries for each entry in reset-names.
> +- resets: Must contain seven entries for each entry in reset-names.
>  	   See ../reset/reset.txt for details.
>  - reset-names: Must include the following names
>  	- "core"
>  	- "mgmt"
>  	- "mgmt-sticky"
>  	- "pipe"
> +	- "pm"
> +	- "aclk"
> +	- "pclk"
>  - pinctrl-names : The pin control state names
>  - pinctrl-0: The "default" pinctrl state
>  - #interrupt-cells: specifies the number of cells needed to encode an
> @@ -86,8 +89,10 @@ pcie0: pcie@f8000000 {
>  	reg = <0x0 0xf8000000 0x0 0x2000000>, <0x0 0xfd000000 0x0 0x1000000>;
>  	reg-names = "axi-base", "apb-base";
>  	resets = <&cru SRST_PCIE_CORE>, <&cru SRST_PCIE_MGMT>,
> -		 <&cru SRST_PCIE_MGMT_STICKY>, <&cru SRST_PCIE_PIPE>;
> -	reset-names = "core", "mgmt", "mgmt-sticky", "pipe";
> +		 <&cru SRST_PCIE_MGMT_STICKY>, <&cru SRST_PCIE_PIPE> ,
> +		 <&cru SRST_PCIE_PM>, <&cru SRST_P_PCIE>, <&cru SRST_A_PCIE>;
> +	reset-names = "core", "mgmt", "mgmt-sticky", "pipe",
> +		      "pm", "pclk", "aclk";
>  	phys = <&pcie_phy>;
>  	phy-names = "pcie-phy";
>  	pinctrl-names = "default";
> diff --git a/drivers/pci/host/pcie-rockchip.c b/drivers/pci/host/pcie-rockchip.c
> index e0b22da..e04f69b 100644
> --- a/drivers/pci/host/pcie-rockchip.c
> +++ b/drivers/pci/host/pcie-rockchip.c
> @@ -190,6 +190,9 @@ struct rockchip_pcie {
>  	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	clk *aclk_pcie;
>  	struct	clk *aclk_perf_pcie;
>  	struct	clk *hclk_pcie;
> @@ -408,6 +411,44 @@ static int rockchip_pcie_init_port(struct rockchip_pcie *rockchip)
>  
>  	gpiod_set_value(rockchip->ep_gpio, 0);
>  
> +	err = reset_control_assert(rockchip->aclk_rst);
> +	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);
> +		return err;
> +	}
> +
> +	udelay(10);
> +
> +	err = reset_control_deassert(rockchip->pm_rst);
> +	if (err) {
> +		dev_err(dev, "deassert pm_rst err %d\n", err);
> +		return err;
> +	}
> +
> +	err = reset_control_deassert(rockchip->aclk_rst);
> +	if (err) {
> +		dev_err(dev, "deassert mgmt_sticky_rst err %d\n", err);
> +		return err;
> +	}
> +
> +	err = reset_control_deassert(rockchip->pclk_rst);
> +	if (err) {
> +		dev_err(dev, "deassert mgmt_sticky_rst err %d\n", err);
> +		return err;
> +	}
> +
>  	err = phy_init(rockchip->phy);
>  	if (err < 0) {
>  		dev_err(dev, "fail to init phy, err %d\n", err);
> @@ -781,6 +822,27 @@ static int rockchip_pcie_parse_dt(struct rockchip_pcie *rockchip)
>  		return PTR_ERR(rockchip->pipe_rst);
>  	}
>  
> +	rockchip->pm_rst = devm_reset_control_get(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);
> +	}
> +
> +	rockchip->pclk_rst = devm_reset_control_get(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);
> +	}
> +
> +	rockchip->aclk_rst = devm_reset_control_get(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);
> +	}
> +
>  	rockchip->ep_gpio = devm_gpiod_get(dev, "ep", GPIOD_OUT_HIGH);
>  	if (IS_ERR(rockchip->ep_gpio)) {
>  		dev_err(dev, "missing ep-gpios property in node\n");
> -- 
> 2.3.7
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] PCI: rockchip: Add three new resets as required properties
@ 2016-10-27 14:54         ` Rob Herring
  0 siblings, 0 replies; 11+ messages in thread
From: Rob Herring @ 2016-10-27 14:54 UTC (permalink / raw)
  To: Shawn Lin
  Cc: Heiko Stuebner, Bjorn Helgaas, devicetree, linux-rockchip,
	Brian Norris, linux-pci

On Fri, Oct 21, 2016 at 10:43:55AM +0800, Shawn Lin wrote:
> pm_rst, aclk_rst, pclk_rst was controlled by rom code so the
> software wasn't needed to control it again in theory. But it
> didn't work properly, so we do need to do it again and add a
> enough delay between the assert of pm_rst and the deassert of
> pm_rst. The Soc intergrated with this controller, rk3399 is still
> under MP test internally, so the backward compatibility won't be
> a big deal.
> 
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> 
> ---
> 
>  .../devicetree/bindings/pci/rockchip-pcie.txt      | 11 ++--

Acked-by: Rob Herring <robh@kernel.org>

>  drivers/pci/host/pcie-rockchip.c                   | 62 ++++++++++++++++++++++
>  2 files changed, 70 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/rockchip-pcie.txt b/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
> index ba67b39..71aeda1 100644
> --- a/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
> @@ -26,13 +26,16 @@ Required properties:
>  	- "sys"
>  	- "legacy"
>  	- "client"
> -- resets: Must contain five entries for each entry in reset-names.
> +- resets: Must contain seven entries for each entry in reset-names.
>  	   See ../reset/reset.txt for details.
>  - reset-names: Must include the following names
>  	- "core"
>  	- "mgmt"
>  	- "mgmt-sticky"
>  	- "pipe"
> +	- "pm"
> +	- "aclk"
> +	- "pclk"
>  - pinctrl-names : The pin control state names
>  - pinctrl-0: The "default" pinctrl state
>  - #interrupt-cells: specifies the number of cells needed to encode an
> @@ -86,8 +89,10 @@ pcie0: pcie@f8000000 {
>  	reg = <0x0 0xf8000000 0x0 0x2000000>, <0x0 0xfd000000 0x0 0x1000000>;
>  	reg-names = "axi-base", "apb-base";
>  	resets = <&cru SRST_PCIE_CORE>, <&cru SRST_PCIE_MGMT>,
> -		 <&cru SRST_PCIE_MGMT_STICKY>, <&cru SRST_PCIE_PIPE>;
> -	reset-names = "core", "mgmt", "mgmt-sticky", "pipe";
> +		 <&cru SRST_PCIE_MGMT_STICKY>, <&cru SRST_PCIE_PIPE> ,
> +		 <&cru SRST_PCIE_PM>, <&cru SRST_P_PCIE>, <&cru SRST_A_PCIE>;
> +	reset-names = "core", "mgmt", "mgmt-sticky", "pipe",
> +		      "pm", "pclk", "aclk";
>  	phys = <&pcie_phy>;
>  	phy-names = "pcie-phy";
>  	pinctrl-names = "default";
> diff --git a/drivers/pci/host/pcie-rockchip.c b/drivers/pci/host/pcie-rockchip.c
> index e0b22da..e04f69b 100644
> --- a/drivers/pci/host/pcie-rockchip.c
> +++ b/drivers/pci/host/pcie-rockchip.c
> @@ -190,6 +190,9 @@ struct rockchip_pcie {
>  	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	clk *aclk_pcie;
>  	struct	clk *aclk_perf_pcie;
>  	struct	clk *hclk_pcie;
> @@ -408,6 +411,44 @@ static int rockchip_pcie_init_port(struct rockchip_pcie *rockchip)
>  
>  	gpiod_set_value(rockchip->ep_gpio, 0);
>  
> +	err = reset_control_assert(rockchip->aclk_rst);
> +	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);
> +		return err;
> +	}
> +
> +	udelay(10);
> +
> +	err = reset_control_deassert(rockchip->pm_rst);
> +	if (err) {
> +		dev_err(dev, "deassert pm_rst err %d\n", err);
> +		return err;
> +	}
> +
> +	err = reset_control_deassert(rockchip->aclk_rst);
> +	if (err) {
> +		dev_err(dev, "deassert mgmt_sticky_rst err %d\n", err);
> +		return err;
> +	}
> +
> +	err = reset_control_deassert(rockchip->pclk_rst);
> +	if (err) {
> +		dev_err(dev, "deassert mgmt_sticky_rst err %d\n", err);
> +		return err;
> +	}
> +
>  	err = phy_init(rockchip->phy);
>  	if (err < 0) {
>  		dev_err(dev, "fail to init phy, err %d\n", err);
> @@ -781,6 +822,27 @@ static int rockchip_pcie_parse_dt(struct rockchip_pcie *rockchip)
>  		return PTR_ERR(rockchip->pipe_rst);
>  	}
>  
> +	rockchip->pm_rst = devm_reset_control_get(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);
> +	}
> +
> +	rockchip->pclk_rst = devm_reset_control_get(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);
> +	}
> +
> +	rockchip->aclk_rst = devm_reset_control_get(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);
> +	}
> +
>  	rockchip->ep_gpio = devm_gpiod_get(dev, "ep", GPIOD_OUT_HIGH);
>  	if (IS_ERR(rockchip->ep_gpio)) {
>  		dev_err(dev, "missing ep-gpios property in node\n");
> -- 
> 2.3.7
> 
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/2] Add new resets for pcie-rockchip
  2016-10-21  2:43 [PATCH 0/2] Add new resets for pcie-rockchip Shawn Lin
       [not found] ` <1477017836-19317-1-git-send-email-shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
  2016-10-21  2:43 ` [PATCH 2/2] arm64: dts: rockchip: add three new resets for rk3399 PCIe controller Shawn Lin
@ 2016-11-09  0:05 ` Heiko Stuebner
  2 siblings, 0 replies; 11+ messages in thread
From: Heiko Stuebner @ 2016-11-09  0:05 UTC (permalink / raw)
  To: Shawn Lin, Bjorn Helgaas
  Cc: Rob Herring, devicetree, linux-rockchip, Brian Norris, linux-pci

Hi Björn,

Am Freitag, 21. Oktober 2016, 10:43:54 CET schrieb Shawn Lin:
> Hi Bjorn and Heiko,
> 
> Sorry for updating pcie-rockchip so frequently under this development
> cycle. This patch is going to fix a urgent issue of missing control for
> pm_rst, aclk_rst and pclk_rst. These three resets was controlled by rom code
> but now the driver will take over it in order to solve some weird issues
> found by MP test. Thanks to that it is still under MP test internally, so
> the backward compatibility of dtb won't be a big deal.

could you take a look at these patches and maybe think about including them 
still for 4.9?


Thanks
Heiko

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2016-11-09  0:05 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-21  2:43 [PATCH 0/2] Add new resets for pcie-rockchip Shawn Lin
     [not found] ` <1477017836-19317-1-git-send-email-shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
2016-10-21  2:43   ` [PATCH 1/2] PCI: rockchip: Add three new resets as required properties Shawn Lin
2016-10-21  2:43     ` Shawn Lin
     [not found]     ` <1477017836-19317-2-git-send-email-shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
2016-10-21  7:56       ` Heiko Stuebner
2016-10-21  7:56         ` Heiko Stuebner
2016-10-27 14:54       ` Rob Herring
2016-10-27 14:54         ` Rob Herring
2016-10-21  2:43 ` [PATCH 2/2] arm64: dts: rockchip: add three new resets for rk3399 PCIe controller Shawn Lin
     [not found]   ` <1477017836-19317-3-git-send-email-shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
2016-10-21  7:58     ` Heiko Stuebner
2016-10-21  7:58       ` Heiko Stuebner
2016-11-09  0:05 ` [PATCH 0/2] Add new resets for pcie-rockchip Heiko Stuebner

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.