Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1605608 > unrolled thread

[PATCH v7 3/4] PCI: imx6: Do not wait for speed change on i.MX7

Started byAndrey Smirnov <andrew.smirnov@gmail.com>
First post2017-03-21 14:50 +0100
Last post2017-03-21 15:00 +0100
Articles 2 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  [PATCH v7 3/4] PCI: imx6: Do not wait for speed change on i.MX7 Andrey Smirnov <andrew.smirnov@gmail.com> - 2017-03-21 14:50 +0100
    Re: [PATCH v7 3/4] PCI: imx6: Do not wait for speed change on i.MX7 Lucas Stach <l.stach@pengutronix.de> - 2017-03-21 15:00 +0100

#1605608 — [PATCH v7 3/4] PCI: imx6: Do not wait for speed change on i.MX7

FromAndrey Smirnov <andrew.smirnov@gmail.com>
Date2017-03-21 14:50 +0100
Subject[PATCH v7 3/4] PCI: imx6: Do not wait for speed change on i.MX7
Message-ID<tnskG-3ZE-19@gated-at.bofh.it>
As can be seen from [1]:

"...the different behavior between iMX6Q PCIe and iMX7D PCIe maybe
caused by the different controller version.

Regarding to the DOC description, the DIRECT_SPEED_CHANGE should be
cleared after the speed change from GEN1 to GEN2. Unfortunately, when
GEN1 device is used, the behavior is not documented.

So, IC design guys run the simulation and
find out the following behaviors:

     1. DIRECT_SPEED_CHANGE will be cleared in 7D after speed change
     	from GEN1 to GEN2. This matches doc’s description

     2. set MAX link speed(PCIE_CAP_TARGET_LINK_SPEED=0x01) as GEN1 and
     	re-run the simulation, DIRECT_SPEED_CHANGE will not be cleared;
     	remain as 1, this matches your result, but function test is
     	passed, so this bit should not affect the normal PCIe function.
..."

imx6_pcie_wait_for_speed_change will report false failures for Gen1 ->
Gen1 speed transition, so avoid doing that check and just rely on
imx6_pcie_wait_for_link only.

[1] https://community.nxp.com/message/867943

Cc: yurovsky@gmail.com
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
Cc: Dong Aisheng <dongas86@gmail.com>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 drivers/pci/dwc/pci-imx6.c | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/dwc/pci-imx6.c b/drivers/pci/dwc/pci-imx6.c
index 64de9bb..c731e41 100644
--- a/drivers/pci/dwc/pci-imx6.c
+++ b/drivers/pci/dwc/pci-imx6.c
@@ -545,10 +545,21 @@ static int imx6_pcie_establish_link(struct imx6_pcie *imx6_pcie)
 	tmp |= PORT_LOGIC_SPEED_CHANGE;
 	dw_pcie_writel_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL, tmp);
 
-	ret = imx6_pcie_wait_for_speed_change(imx6_pcie);
-	if (ret) {
-		dev_err(dev, "Failed to bring link up!\n");
-		goto err_reset_phy;
+	if (imx6_pcie->variant != IMX7D) {
+		/*
+		 * On i.MX7, DIRECT_SPEED_CHANGE behaves differently
+		 * from i.MX6 family when no link speed transition
+		 * occurs and we go Gen1 -> yep, Gen1. The difference
+		 * is that, in such case, it will not be cleared by HW
+		 * which will cause the following code to report false
+		 * failure.
+		 */
+
+		ret = imx6_pcie_wait_for_speed_change(imx6_pcie);
+		if (ret) {
+			dev_err(dev, "Failed to bring link up!\n");
+			goto err_reset_phy;
+		}
 	}
 
 	/* Make sure link training is finished as well! */
-- 
2.9.3

[toc] | [next] | [standalone]


#1605621

FromLucas Stach <l.stach@pengutronix.de>
Date2017-03-21 15:00 +0100
Message-ID<tnsum-43V-19@gated-at.bofh.it>
In reply to#1605608
Am Dienstag, den 21.03.2017, 06:42 -0700 schrieb Andrey Smirnov:
> As can be seen from [1]:
> 
> "...the different behavior between iMX6Q PCIe and iMX7D PCIe maybe
> caused by the different controller version.
> 
> Regarding to the DOC description, the DIRECT_SPEED_CHANGE should be
> cleared after the speed change from GEN1 to GEN2. Unfortunately, when
> GEN1 device is used, the behavior is not documented.
> 
> So, IC design guys run the simulation and
> find out the following behaviors:
> 
>      1. DIRECT_SPEED_CHANGE will be cleared in 7D after speed change
>      	from GEN1 to GEN2. This matches doc’s description
> 
>      2. set MAX link speed(PCIE_CAP_TARGET_LINK_SPEED=0x01) as GEN1 and
>      	re-run the simulation, DIRECT_SPEED_CHANGE will not be cleared;
>      	remain as 1, this matches your result, but function test is
>      	passed, so this bit should not affect the normal PCIe function.
> ..."
> 
> imx6_pcie_wait_for_speed_change will report false failures for Gen1 ->
> Gen1 speed transition, so avoid doing that check and just rely on
> imx6_pcie_wait_for_link only.
> 
> [1] https://community.nxp.com/message/867943
> 
> Cc: yurovsky@gmail.com
> Cc: Lucas Stach <l.stach@pengutronix.de>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Fabio Estevam <fabio.estevam@nxp.com>
> Cc: Dong Aisheng <dongas86@gmail.com>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>

Reviewed-by: Lucas Stach <l.stach@pengutronix.de>

> ---
>  drivers/pci/dwc/pci-imx6.c | 19 +++++++++++++++----
>  1 file changed, 15 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/pci/dwc/pci-imx6.c b/drivers/pci/dwc/pci-imx6.c
> index 64de9bb..c731e41 100644
> --- a/drivers/pci/dwc/pci-imx6.c
> +++ b/drivers/pci/dwc/pci-imx6.c
> @@ -545,10 +545,21 @@ static int imx6_pcie_establish_link(struct imx6_pcie *imx6_pcie)
>  	tmp |= PORT_LOGIC_SPEED_CHANGE;
>  	dw_pcie_writel_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL, tmp);
>  
> -	ret = imx6_pcie_wait_for_speed_change(imx6_pcie);
> -	if (ret) {
> -		dev_err(dev, "Failed to bring link up!\n");
> -		goto err_reset_phy;
> +	if (imx6_pcie->variant != IMX7D) {
> +		/*
> +		 * On i.MX7, DIRECT_SPEED_CHANGE behaves differently
> +		 * from i.MX6 family when no link speed transition
> +		 * occurs and we go Gen1 -> yep, Gen1. The difference
> +		 * is that, in such case, it will not be cleared by HW
> +		 * which will cause the following code to report false
> +		 * failure.
> +		 */
> +
> +		ret = imx6_pcie_wait_for_speed_change(imx6_pcie);
> +		if (ret) {
> +			dev_err(dev, "Failed to bring link up!\n");
> +			goto err_reset_phy;
> +		}
>  	}
>  
>  	/* Make sure link training is finished as well! */

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web