[PATCH 1/5] PCI: imx6: Rename imx6_pcie_start_link() to imx6_pcie_establish_link()

[PATCH 1/5] PCI: imx6: Rename imx6_pcie_start_link() to imx6_pcie_establish_link()

[Bjorn Helgaas]

Rename imx6_pcie_start_link() to imx6_pcie_establish_link() to follow the
convention of other DesignWare-based host drivers.  No functional change.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/host/pci-imx6.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
index fdb9536..8cc0123 100644
--- a/drivers/pci/host/pci-imx6.c
+++ b/drivers/pci/host/pci-imx6.c
@@ -359,7 +359,7 @@ static irqreturn_t imx6_pcie_msi_handler(int irq, void *arg)
 	return dw_handle_msi_irq(pp);
 }
 
-static int imx6_pcie_start_link(struct pcie_port *pp)
+static int imx6_pcie_establish_link(struct pcie_port *pp)
 {
 	struct imx6_pcie *imx6_pcie = to_imx6_pcie(pp);
 	uint32_t tmp;
@@ -432,7 +432,7 @@ static void imx6_pcie_host_init(struct pcie_port *pp)
 
 	dw_pcie_setup_rc(pp);
 
-	imx6_pcie_start_link(pp);
+	imx6_pcie_establish_link(pp);
 
 	if (IS_ENABLED(CONFIG_PCI_MSI))
 		dw_pcie_msi_init(pp);

[PATCH 2/5] PCI: dra7xx: Use dw_pcie_link_up() consistently

[Bjorn Helgaas]

We already use dw_pcie_link_up() once in dra7xx_pcie_establish_link(), but
we duplicate its code later.  Use dw_pcie_link_up() for consistency.  No
functional change.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/host/pci-dra7xx.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/pci/host/pci-dra7xx.c b/drivers/pci/host/pci-dra7xx.c
index 2d57e19..e3d15d7 100644
--- a/drivers/pci/host/pci-dra7xx.c
+++ b/drivers/pci/host/pci-dra7xx.c
@@ -107,8 +107,7 @@ static int dra7xx_pcie_establish_link(struct pcie_port *pp)
 	dra7xx_pcie_writel(dra7xx, PCIECTRL_DRA7XX_CONF_DEVICE_CMD, reg);
 
 	while (retries--) {
-		reg = dra7xx_pcie_readl(dra7xx,	PCIECTRL_DRA7XX_CONF_PHY_CS);
-		if (reg & LINK_UP)
+		if (dw_pcie_link_up(pp))
 			break;
 		usleep_range(10, 20);
 	}

[PATCH 3/5] PCI: layerscape: Use dw_pcie_link_up() consistently

[Bjorn Helgaas]

All the other DesignWare-based drivers use dw_pcie_link_up(), so use it in
this driver, too, for consistency.  No functional change.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/host/pci-layerscape.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/host/pci-layerscape.c b/drivers/pci/host/pci-layerscape.c
index 4a6e62f..bb5894f 100644
--- a/drivers/pci/host/pci-layerscape.c
+++ b/drivers/pci/host/pci-layerscape.c
@@ -70,7 +70,7 @@ static void ls_pcie_host_init(struct pcie_port *pp)
 
 	dw_pcie_setup_rc(pp);
 
-	while (!ls_pcie_link_up(pp)) {
+	while (!dw_pcie_link_up(pp)) {
 		usleep_range(100, 1000);
 		count++;
 		if (count >= 200) {

[PATCH 4/5] PCI: layerscape: Factor out ls_pcie_establish_link()

[Bjorn Helgaas]

All other DesignWare-based drivers have a *_establish_link() function.

This functionality is trivial for Layerscape, but factor out a
ls_pcie_establish_link() for consistency with the other drivers.  No
functional change.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/host/pci-layerscape.c |   19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/host/pci-layerscape.c b/drivers/pci/host/pci-layerscape.c
index bb5894f..434b116 100644
--- a/drivers/pci/host/pci-layerscape.c
+++ b/drivers/pci/host/pci-layerscape.c
@@ -62,23 +62,30 @@ static int ls_pcie_link_up(struct pcie_port *pp)
 	return 1;
 }
 
-static void ls_pcie_host_init(struct pcie_port *pp)
+static int ls_pcie_establish_link(struct pcie_port *pp)
 {
-	struct ls_pcie *pcie = to_ls_pcie(pp);
 	int count = 0;
-	u32 val;
-
-	dw_pcie_setup_rc(pp);
 
 	while (!dw_pcie_link_up(pp)) {
 		usleep_range(100, 1000);
 		count++;
 		if (count >= 200) {
 			dev_err(pp->dev, "phy link never came up\n");
-			return;
+			return -EINVAL;
 		}
 	}
 
+	return 0;
+}
+
+static void ls_pcie_host_init(struct pcie_port *pp)
+{
+	struct ls_pcie *pcie = to_ls_pcie(pp);
+	u32 val;
+
+	dw_pcie_setup_rc(pp);
+	ls_pcie_establish_link(pp);
+
 	/*
 	 * LS1021A Workaround for internal TKT228622
 	 * to fix the INTx hang issue

[PATCH 5/5] PCI: designware: Wait for link to come up with consistent style

[Bjorn Helgaas]

All the DesignWare-based host drivers loop waiting for the link to come up,
but they do it several ways that are needlessly different.

Wait for the link to come up in a consistent style across all the
DesignWare drivers.  No functional change.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/host/pci-dra7xx.c     |   16 ++++++----------
 drivers/pci/host/pci-exynos.c     |   34 +++++++++++++++-------------------
 drivers/pci/host/pci-imx6.c       |   20 +++++++++-----------
 drivers/pci/host/pci-keystone.c   |   16 +++++++---------
 drivers/pci/host/pci-layerscape.c |   14 ++++++--------
 drivers/pci/host/pcie-spear13xx.c |   17 ++++++++---------
 6 files changed, 51 insertions(+), 66 deletions(-)

diff --git a/drivers/pci/host/pci-dra7xx.c b/drivers/pci/host/pci-dra7xx.c
index e3d15d7..80db09e 100644
--- a/drivers/pci/host/pci-dra7xx.c
+++ b/drivers/pci/host/pci-dra7xx.c
@@ -93,9 +93,9 @@ static int dra7xx_pcie_link_up(struct pcie_port *pp)
 
 static int dra7xx_pcie_establish_link(struct pcie_port *pp)
 {
-	u32 reg;
-	unsigned int retries = 1000;
 	struct dra7xx_pcie *dra7xx = to_dra7xx_pcie(pp);
+	u32 reg;
+	unsigned int retries;
 
 	if (dw_pcie_link_up(pp)) {
 		dev_err(pp->dev, "link is already up\n");
@@ -106,18 +106,14 @@ static int dra7xx_pcie_establish_link(struct pcie_port *pp)
 	reg |= LTSSM_EN;
 	dra7xx_pcie_writel(dra7xx, PCIECTRL_DRA7XX_CONF_DEVICE_CMD, reg);
 
-	while (retries--) {
+	for (retries = 0; retries < 1000; retries++) {
 		if (dw_pcie_link_up(pp))
-			break;
+			return 0;
 		usleep_range(10, 20);
 	}
 
-	if (retries == 0) {
-		dev_err(pp->dev, "link is not up\n");
-		return -ETIMEDOUT;
-	}
-
-	return 0;
+	dev_err(pp->dev, "link is not up\n");
+	return -EINVAL;
 }
 
 static void dra7xx_pcie_enable_interrupts(struct pcie_port *pp)
diff --git a/drivers/pci/host/pci-exynos.c b/drivers/pci/host/pci-exynos.c
index c139237..f9f468d 100644
--- a/drivers/pci/host/pci-exynos.c
+++ b/drivers/pci/host/pci-exynos.c
@@ -316,9 +316,9 @@ static void exynos_pcie_assert_reset(struct pcie_port *pp)
 
 static int exynos_pcie_establish_link(struct pcie_port *pp)
 {
-	u32 val;
-	int count = 0;
 	struct exynos_pcie *exynos_pcie = to_exynos_pcie(pp);
+	u32 val;
+	unsigned int retries;
 
 	if (dw_pcie_link_up(pp)) {
 		dev_err(pp->dev, "Link already up\n");
@@ -357,27 +357,23 @@ static int exynos_pcie_establish_link(struct pcie_port *pp)
 			  PCIE_APP_LTSSM_ENABLE);
 
 	/* check if the link is up or not */
-	while (!dw_pcie_link_up(pp)) {
-		mdelay(100);
-		count++;
-		if (count == 10) {
-			while (exynos_phy_readl(exynos_pcie,
-						PCIE_PHY_PLL_LOCKED) == 0) {
-				val = exynos_blk_readl(exynos_pcie,
-						       PCIE_PHY_PLL_LOCKED);
-				dev_info(pp->dev, "PLL Locked: 0x%x\n", val);
-			}
-			/* power off phy */
-			exynos_pcie_power_off_phy(pp);
-
-			dev_err(pp->dev, "PCIe Link Fail\n");
-			return -EINVAL;
+	for (retries = 0; retries < 10; retries++) {
+		if (dw_pcie_link_up(pp)) {
+			dev_info(pp->dev, "Link up\n");
+			return 0;
 		}
+		mdelay(100);
 	}
 
-	dev_info(pp->dev, "Link up\n");
+	while (exynos_phy_readl(exynos_pcie, PCIE_PHY_PLL_LOCKED) == 0) {
+		val = exynos_blk_readl(exynos_pcie, PCIE_PHY_PLL_LOCKED);
+		dev_info(pp->dev, "PLL Locked: 0x%x\n", val);
+	}
+	/* power off phy */
+	exynos_pcie_power_off_phy(pp);
 
-	return 0;
+	dev_err(pp->dev, "PCIe Link Fail\n");
+	return -EINVAL;
 }
 
 static void exynos_pcie_clear_irq_pulse(struct pcie_port *pp)
diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
index 8cc0123..af7da8a 100644
--- a/drivers/pci/host/pci-imx6.c
+++ b/drivers/pci/host/pci-imx6.c
@@ -335,21 +335,19 @@ static void imx6_pcie_init_phy(struct pcie_port *pp)
 
 static int imx6_pcie_wait_for_link(struct pcie_port *pp)
 {
-	int count = 200;
+	unsigned int retries;
 
-	while (!dw_pcie_link_up(pp)) {
+	for (retries = 0; retries < 200; retries++) {
+		if (dw_pcie_link_up(pp))
+			return 0;
 		usleep_range(100, 1000);
-		if (--count)
-			continue;
-
-		dev_err(pp->dev, "phy link never came up\n");
-		dev_dbg(pp->dev, "DEBUG_R0: 0x%08x, DEBUG_R1: 0x%08x\n",
-			readl(pp->dbi_base + PCIE_PHY_DEBUG_R0),
-			readl(pp->dbi_base + PCIE_PHY_DEBUG_R1));
-		return -EINVAL;
 	}
 
-	return 0;
+	dev_err(pp->dev, "phy link never came up\n");
+	dev_dbg(pp->dev, "DEBUG_R0: 0x%08x, DEBUG_R1: 0x%08x\n",
+		readl(pp->dbi_base + PCIE_PHY_DEBUG_R0),
+		readl(pp->dbi_base + PCIE_PHY_DEBUG_R1));
+	return -EINVAL;
 }
 
 static irqreturn_t imx6_pcie_msi_handler(int irq, void *arg)
diff --git a/drivers/pci/host/pci-keystone.c b/drivers/pci/host/pci-keystone.c
index 75333b0..b75d684 100644
--- a/drivers/pci/host/pci-keystone.c
+++ b/drivers/pci/host/pci-keystone.c
@@ -88,7 +88,7 @@ DECLARE_PCI_FIXUP_ENABLE(PCI_ANY_ID, PCI_ANY_ID, quirk_limit_mrrs);
 static int ks_pcie_establish_link(struct keystone_pcie *ks_pcie)
 {
 	struct pcie_port *pp = &ks_pcie->pp;
-	int count = 200;
+	unsigned int retries;
 
 	dw_pcie_setup_rc(pp);
 
@@ -99,17 +99,15 @@ static int ks_pcie_establish_link(struct keystone_pcie *ks_pcie)
 
 	ks_dw_pcie_initiate_link_train(ks_pcie);
 	/* check if the link is up or not */
-	while (!dw_pcie_link_up(pp)) {
+	for (retries = 0; retries < 200; retries++) {
+		if (dw_pcie_link_up(pp))
+			return 0;
 		usleep_range(100, 1000);
-		if (--count) {
-			ks_dw_pcie_initiate_link_train(ks_pcie);
-			continue;
-		}
-		dev_err(pp->dev, "phy link never came up\n");
-		return -EINVAL;
+		ks_dw_pcie_initiate_link_train(ks_pcie);
 	}
 
-	return 0;
+	dev_err(pp->dev, "phy link never came up\n");
+	return -EINVAL;
 }
 
 static void ks_pcie_msi_irq_handler(unsigned int irq, struct irq_desc *desc)
diff --git a/drivers/pci/host/pci-layerscape.c b/drivers/pci/host/pci-layerscape.c
index 434b116..b2328ea1 100644
--- a/drivers/pci/host/pci-layerscape.c
+++ b/drivers/pci/host/pci-layerscape.c
@@ -64,18 +64,16 @@ static int ls_pcie_link_up(struct pcie_port *pp)
 
 static int ls_pcie_establish_link(struct pcie_port *pp)
 {
-	int count = 0;
+	unsigned int retries;
 
-	while (!dw_pcie_link_up(pp)) {
+	for (retries = 0; retries < 200; retries++) {
+		if (dw_pcie_link_up(pp))
+			return 0;
 		usleep_range(100, 1000);
-		count++;
-		if (count >= 200) {
-			dev_err(pp->dev, "phy link never came up\n");
-			return -EINVAL;
-		}
 	}
 
-	return 0;
+	dev_err(pp->dev, "phy link never came up\n");
+	return -EINVAL;
 }
 
 static void ls_pcie_host_init(struct pcie_port *pp)
diff --git a/drivers/pci/host/pcie-spear13xx.c b/drivers/pci/host/pcie-spear13xx.c
index 020d788..dfec428 100644
--- a/drivers/pci/host/pcie-spear13xx.c
+++ b/drivers/pci/host/pcie-spear13xx.c
@@ -146,10 +146,10 @@ struct pcie_app_reg {
 static int spear13xx_pcie_establish_link(struct pcie_port *pp)
 {
 	u32 val;
-	int count = 0;
 	struct spear13xx_pcie *spear13xx_pcie = to_spear13xx_pcie(pp);
 	struct pcie_app_reg *app_reg = spear13xx_pcie->app_base;
 	u32 exp_cap_off = EXP_CAP_ID_OFFSET;
+	unsigned int retries;
 
 	if (dw_pcie_link_up(pp)) {
 		dev_err(pp->dev, "link already up\n");
@@ -201,17 +201,16 @@ static int spear13xx_pcie_establish_link(struct pcie_port *pp)
 			&app_reg->app_ctrl_0);
 
 	/* check if the link is up or not */
-	while (!dw_pcie_link_up(pp)) {
-		mdelay(100);
-		count++;
-		if (count == 10) {
-			dev_err(pp->dev, "link Fail\n");
-			return -EINVAL;
+	for (retries = 0; retries < 10; retries++) {
+		if (dw_pcie_link_up(pp)) {
+			dev_info(pp->dev, "link up\n");
+			return 0;
 		}
+		mdelay(100);
 	}
-	dev_info(pp->dev, "link up\n");
 
-	return 0;
+	dev_err(pp->dev, "link Fail\n");
+	return -EINVAL;
 }
 
 static irqreturn_t spear13xx_pcie_irq_handler(int irq, void *arg)

Re: [PATCH 1/5] PCI: imx6: Rename imx6_pcie_start_link() to imx6_pcie_establish_link()

[Bjorn Helgaas]

[oops, sorry, I used Pratyush's old email address.  I just forwarded
them to his updated address, but if you respond to any of these
patches, please update his address to "Pratyush Anand
<pratyush.anand@gmail.com>"]

On Thu, Jun 4, 2015 at 5:01 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> Rename imx6_pcie_start_link() to imx6_pcie_establish_link() to follow the
> convention of other DesignWare-based host drivers.  No functional change.
>
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> ---
>  drivers/pci/host/pci-imx6.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
> index fdb9536..8cc0123 100644
> --- a/drivers/pci/host/pci-imx6.c
> +++ b/drivers/pci/host/pci-imx6.c
> @@ -359,7 +359,7 @@ static irqreturn_t imx6_pcie_msi_handler(int irq, void *arg)
>         return dw_handle_msi_irq(pp);
>  }
>
> -static int imx6_pcie_start_link(struct pcie_port *pp)
> +static int imx6_pcie_establish_link(struct pcie_port *pp)
>  {
>         struct imx6_pcie *imx6_pcie = to_imx6_pcie(pp);
>         uint32_t tmp;
> @@ -432,7 +432,7 @@ static void imx6_pcie_host_init(struct pcie_port *pp)
>
>         dw_pcie_setup_rc(pp);
>
> -       imx6_pcie_start_link(pp);
> +       imx6_pcie_establish_link(pp);
>
>         if (IS_ENABLED(CONFIG_PCI_MSI))
>                 dw_pcie_msi_init(pp);
>

Re: [PATCH 1/5] PCI: imx6: Rename imx6_pcie_start_link() to imx6_pcie_establish_link()

[Pratyush Anand]

On Fri, Jun 5, 2015 at 3:36 AM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> [oops, sorry, I used Pratyush's old email address.  I just forwarded
> them to his updated address, but if you respond to any of these
> patches, please update his address to "Pratyush Anand
> <pratyush.anand@gmail.com>"]
>

All modifications are nice.

For complete series:
Acked-by: Pratyush Anand <pratyush.anand@gmail.com>

Re: [PATCH 2/5] PCI: dra7xx: Use dw_pcie_link_up() consistently

[Kishon Vijay Abraham I]

On Friday 05 June 2015 03:31 AM, Bjorn Helgaas wrote:
> We already use dw_pcie_link_up() once in dra7xx_pcie_establish_link(), but
> we duplicate its code later.  Use dw_pcie_link_up() for consistency.  No
> functional change.
>
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>

Acked-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
>   drivers/pci/host/pci-dra7xx.c |    3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/pci/host/pci-dra7xx.c b/drivers/pci/host/pci-dra7xx.c
> index 2d57e19..e3d15d7 100644
> --- a/drivers/pci/host/pci-dra7xx.c
> +++ b/drivers/pci/host/pci-dra7xx.c
> @@ -107,8 +107,7 @@ static int dra7xx_pcie_establish_link(struct pcie_port *pp)
>   	dra7xx_pcie_writel(dra7xx, PCIECTRL_DRA7XX_CONF_DEVICE_CMD, reg);
>
>   	while (retries--) {
> -		reg = dra7xx_pcie_readl(dra7xx,	PCIECTRL_DRA7XX_CONF_PHY_CS);
> -		if (reg & LINK_UP)
> +		if (dw_pcie_link_up(pp))
>   			break;
>   		usleep_range(10, 20);
>   	}
>