[PATCH 1/2] PCI: rockchip: improve the deassert sequence of four reset pins

[PATCH 1/2] PCI: rockchip: improve the deassert sequence of four reset pins

[Shawn Lin]

Per TRM, we need to deassert the four reset pins simultaneously.
Currently the reset framework doesn't support that so we did it
one by one. It seems no side effect found but it does impact the
state machine of controller, so sometimes the change speed bit is
not setted when sending training sequence from recover state.
After the silicon RTL review from Soc guys, we don't need to do
the sequence recommended by TRM, and could just move the deassert
of mgmt_sticky_rst to the first place.

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

 drivers/pci/host/pcie-rockchip.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/host/pcie-rockchip.c b/drivers/pci/host/pcie-rockchip.c
index c3593e6..5e51121 100644
--- a/drivers/pci/host/pcie-rockchip.c
+++ b/drivers/pci/host/pcie-rockchip.c
@@ -433,21 +433,25 @@ static int rockchip_pcie_init_port(struct rockchip_pcie *rockchip)
 		return err;
 	}
 
-	err = reset_control_deassert(rockchip->core_rst);
+	/*
+	 * Please don't reorder the deassert sequence of the following
+	 * four reset pins.
+	 */
+	err = reset_control_deassert(rockchip->mgmt_sticky_rst);
 	if (err) {
-		dev_err(dev, "deassert core_rst err %d\n", err);
+		dev_err(dev, "deassert mgmt_sticky_rst err %d\n", err);
 		return err;
 	}
 
-	err = reset_control_deassert(rockchip->mgmt_rst);
+	err = reset_control_deassert(rockchip->core_rst);
 	if (err) {
-		dev_err(dev, "deassert mgmt_rst err %d\n", err);
+		dev_err(dev, "deassert core_rst err %d\n", err);
 		return err;
 	}
 
-	err = reset_control_deassert(rockchip->mgmt_sticky_rst);
+	err = reset_control_deassert(rockchip->mgmt_rst);
 	if (err) {
-		dev_err(dev, "deassert mgmt_sticky_rst err %d\n", err);
+		dev_err(dev, "deassert mgmt_rst err %d\n", err);
 		return err;
 	}
 
-- 
2.3.7

[PATCH 2/2] PCI: rockchip: fix wrong transmitted FTS count

[Shawn Lin]

If the expected numbers of FTS aren't recevied by RC when
exiting from L0s, the LTSSM will fall into recover state.
So it will need to send TS for retraining which makes the
latency of exiting from L0s a little longer than expected.
This issue is caused by the wrong reset value of FTS count
on PLC1 register(offset 0x4). The expected value for Gen1/2
should be more than 240 and we may leave a little margin here.
Let's fix this before starting Gen1 traning which will makes
TS1 contains the correct FTS count.

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

 drivers/pci/host/pcie-rockchip.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/pci/host/pcie-rockchip.c b/drivers/pci/host/pcie-rockchip.c
index 5e51121..6a7a9df 100644
--- a/drivers/pci/host/pcie-rockchip.c
+++ b/drivers/pci/host/pcie-rockchip.c
@@ -95,6 +95,10 @@
 #define   PCIE_CORE_PL_CONF_SPEED_MASK		0x00000018
 #define   PCIE_CORE_PL_CONF_LANE_MASK		0x00000006
 #define   PCIE_CORE_PL_CONF_LANE_SHIFT		1
+#define PCIE_CORE_CTRL_PLC1		(PCIE_CORE_CTRL_MGMT_BASE + 0x004)
+#define   PCIE_CORE_CTRL_PLC1_FTS_MASK		GENMASK(23, 8)
+#define   PCIE_CORE_CTRL_PLC1_FTS_SHIFT	8
+#define   PCIE_CORE_CTRL_PLC1_FTS_CNT		0xffff
 #define PCIE_CORE_INT_STATUS		(PCIE_CORE_CTRL_MGMT_BASE + 0x20c)
 #define   PCIE_CORE_INT_PRFPE			BIT(0)
 #define   PCIE_CORE_INT_CRFPE			BIT(1)
@@ -470,6 +474,12 @@ static int rockchip_pcie_init_port(struct rockchip_pcie *rockchip)
 	status = rockchip_pcie_read(rockchip, PCIE_RC_CONFIG_L1_SUBSTATE_CTRL2);
 	rockchip_pcie_write(rockchip, status, PCIE_RC_CONFIG_L1_SUBSTATE_CTRL2);
 
+	/* Fix the transmitted FTS count desired to exit from L0s. */
+	status = rockchip_pcie_read(rockchip, PCIE_CORE_CTRL_PLC1);
+	status = (status & PCIE_CORE_CTRL_PLC1_FTS_MASK) |
+		 (PCIE_CORE_CTRL_PLC1_FTS_CNT << PCIE_CORE_CTRL_PLC1_FTS_SHIFT);
+	rockchip_pcie_write(rockchip, status, PCIE_CORE_CTRL_PLC1);
+
 	/* Enable Gen1 training */
 	rockchip_pcie_write(rockchip, PCIE_CLIENT_LINK_TRAIN_ENABLE,
 			    PCIE_CLIENT_CONFIG);
-- 
2.3.7

Re: [PATCH 1/2] PCI: rockchip: improve the deassert sequence of four reset pins

[Bjorn Helgaas]

On Fri, Sep 23, 2016 at 10:05:59AM +0800, Shawn Lin wrote:
> Per TRM, we need to deassert the four reset pins simultaneously.
> Currently the reset framework doesn't support that so we did it
> one by one. It seems no side effect found but it does impact the
> state machine of controller, so sometimes the change speed bit is
> not setted when sending training sequence from recover state.
> After the silicon RTL review from Soc guys, we don't need to do
> the sequence recommended by TRM, and could just move the deassert
> of mgmt_sticky_rst to the first place.
> 
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>

Applied both to pci/host-rockchip for v4.9, thanks!

> ---
> 
>  drivers/pci/host/pcie-rockchip.c | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/pci/host/pcie-rockchip.c b/drivers/pci/host/pcie-rockchip.c
> index c3593e6..5e51121 100644
> --- a/drivers/pci/host/pcie-rockchip.c
> +++ b/drivers/pci/host/pcie-rockchip.c
> @@ -433,21 +433,25 @@ static int rockchip_pcie_init_port(struct rockchip_pcie *rockchip)
>  		return err;
>  	}
>  
> -	err = reset_control_deassert(rockchip->core_rst);
> +	/*
> +	 * Please don't reorder the deassert sequence of the following
> +	 * four reset pins.
> +	 */
> +	err = reset_control_deassert(rockchip->mgmt_sticky_rst);
>  	if (err) {
> -		dev_err(dev, "deassert core_rst err %d\n", err);
> +		dev_err(dev, "deassert mgmt_sticky_rst err %d\n", err);
>  		return err;
>  	}
>  
> -	err = reset_control_deassert(rockchip->mgmt_rst);
> +	err = reset_control_deassert(rockchip->core_rst);
>  	if (err) {
> -		dev_err(dev, "deassert mgmt_rst err %d\n", err);
> +		dev_err(dev, "deassert core_rst err %d\n", err);
>  		return err;
>  	}
>  
> -	err = reset_control_deassert(rockchip->mgmt_sticky_rst);
> +	err = reset_control_deassert(rockchip->mgmt_rst);
>  	if (err) {
> -		dev_err(dev, "deassert mgmt_sticky_rst err %d\n", err);
> +		dev_err(dev, "deassert mgmt_rst err %d\n", err);
>  		return err;
>  	}
>  
> -- 
> 2.3.7
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] PCI: rockchip: fix wrong transmitted FTS count

[Brian Norris]

On Fri, Sep 23, 2016 at 10:06:00AM +0800, Shawn Lin wrote:
> If the expected numbers of FTS aren't recevied by RC when
> exiting from L0s, the LTSSM will fall into recover state.
> So it will need to send TS for retraining which makes the
> latency of exiting from L0s a little longer than expected.
> This issue is caused by the wrong reset value of FTS count
> on PLC1 register(offset 0x4). The expected value for Gen1/2
> should be more than 240 and we may leave a little margin here.
> Let's fix this before starting Gen1 traning which will makes
> TS1 contains the correct FTS count.
> 
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> ---
> 
>  drivers/pci/host/pcie-rockchip.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/pci/host/pcie-rockchip.c b/drivers/pci/host/pcie-rockchip.c
> index 5e51121..6a7a9df 100644
> --- a/drivers/pci/host/pcie-rockchip.c
> +++ b/drivers/pci/host/pcie-rockchip.c
> @@ -95,6 +95,10 @@
>  #define   PCIE_CORE_PL_CONF_SPEED_MASK		0x00000018
>  #define   PCIE_CORE_PL_CONF_LANE_MASK		0x00000006
>  #define   PCIE_CORE_PL_CONF_LANE_SHIFT		1
> +#define PCIE_CORE_CTRL_PLC1		(PCIE_CORE_CTRL_MGMT_BASE + 0x004)
> +#define   PCIE_CORE_CTRL_PLC1_FTS_MASK		GENMASK(23, 8)

^^^ That's 0x00ffff00, isn't it?

> +#define   PCIE_CORE_CTRL_PLC1_FTS_SHIFT	8
> +#define   PCIE_CORE_CTRL_PLC1_FTS_CNT		0xffff
>  #define PCIE_CORE_INT_STATUS		(PCIE_CORE_CTRL_MGMT_BASE + 0x20c)
>  #define   PCIE_CORE_INT_PRFPE			BIT(0)
>  #define   PCIE_CORE_INT_CRFPE			BIT(1)
> @@ -470,6 +474,12 @@ static int rockchip_pcie_init_port(struct rockchip_pcie *rockchip)
>  	status = rockchip_pcie_read(rockchip, PCIE_RC_CONFIG_L1_SUBSTATE_CTRL2);
>  	rockchip_pcie_write(rockchip, status, PCIE_RC_CONFIG_L1_SUBSTATE_CTRL2);
>  
> +	/* Fix the transmitted FTS count desired to exit from L0s. */
> +	status = rockchip_pcie_read(rockchip, PCIE_CORE_CTRL_PLC1);
> +	status = (status & PCIE_CORE_CTRL_PLC1_FTS_MASK) |

^^^ That's not the way to use this then, is it? We want

	(status & ~PCIE_CORE_CTRL_PLC1_FTS_MASK) | ...

Fortunately the lowest 8 bits default to 0, and the highest 8 bits are
for 8GT/s operation (not used).

This is already upstream, so I guess we need a follow-up.

Brian

> +		 (PCIE_CORE_CTRL_PLC1_FTS_CNT << PCIE_CORE_CTRL_PLC1_FTS_SHIFT);
> +	rockchip_pcie_write(rockchip, status, PCIE_CORE_CTRL_PLC1);
> +
>  	/* Enable Gen1 training */
>  	rockchip_pcie_write(rockchip, PCIE_CLIENT_LINK_TRAIN_ENABLE,
>  			    PCIE_CLIENT_CONFIG);
> -- 
> 2.3.7
> 
>