[PATCH u-boot-marvell 1/3] arm: a37xx: pci: Do not try to access other buses when link is down

[PATCH u-boot-marvell 1/3] arm: a37xx: pci: Do not try to access other buses when link is down

[Pali Rohár]

If a PIO request is executed while link-down, the whole controller gets
stuck in a non-functional state, and even after link comes up again, PIO
requests won't work anymore, and a reset of the whole PCIe controller is
needed. Therefore we need to prevent sending PIO requests while the link
is down.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 drivers/pci/pci-aardvark.c | 38 +++++++++++++++++++++-----------------
 1 file changed, 21 insertions(+), 17 deletions(-)

diff --git a/drivers/pci/pci-aardvark.c b/drivers/pci/pci-aardvark.c
index 1eb257ea8b4a..ccaeecaca8e3 100644
--- a/drivers/pci/pci-aardvark.c
+++ b/drivers/pci/pci-aardvark.c
@@ -177,6 +177,23 @@ static inline uint advk_readl(struct pcie_advk *pcie, uint reg)
 	return readl(pcie->base + reg);
 }
 
+/**
+ * pcie_advk_link_up() - Check if PCIe link is up or not
+ *
+ * @pcie: The PCI device to access
+ *
+ * Return true on link up.
+ * Return false on link down.
+ */
+static bool pcie_advk_link_up(struct pcie_advk *pcie)
+{
+	u32 val, ltssm_state;
+
+	val = advk_readl(pcie, ADVK_LMI_PHY_CFG0);
+	ltssm_state = (val & ADVK_LMI_PHY_CFG0_LTSSM_MASK) >> ADVK_LMI_PHY_CFG0_LTSSM_SHIFT;
+	return ltssm_state >= ADVK_LMI_PHY_CFG0_LTSSM_L0 && ltssm_state < ADVK_LMI_PHY_CFG0_LTSSM_DISABLED;
+}
+
 /**
  * pcie_advk_addr_valid() - Check for valid bus address
  *
@@ -195,6 +212,10 @@ static bool pcie_advk_addr_valid(struct pcie_advk *pcie,
 	if (busno == 0 && (dev != 0 || func != 0))
 		return false;
 
+	/* Access to other buses is possible when link is up */
+	if (busno != 0 && !pcie_advk_link_up(pcie))
+		return false;
+
 	/*
 	 * In PCI-E only a single device (0) can exist on the secondary bus.
 	 * Beyond the secondary bus, there might be a Switch and anything is
@@ -618,23 +639,6 @@ retry:
 	return ret;
 }
 
-/**
- * pcie_advk_link_up() - Check if PCIe link is up or not
- *
- * @pcie: The PCI device to access
- *
- * Return 1 (true) on link up.
- * Return 0 (false) on link down.
- */
-static int pcie_advk_link_up(struct pcie_advk *pcie)
-{
-	u32 val, ltssm_state;
-
-	val = advk_readl(pcie, ADVK_LMI_PHY_CFG0);
-	ltssm_state = (val & ADVK_LMI_PHY_CFG0_LTSSM_MASK) >> ADVK_LMI_PHY_CFG0_LTSSM_SHIFT;
-	return ltssm_state >= ADVK_LMI_PHY_CFG0_LTSSM_L0 && ltssm_state < ADVK_LMI_PHY_CFG0_LTSSM_DISABLED;
-}
-
 /**
  * pcie_advk_wait_for_link() - Wait for link training to be accomplished
  *
-- 
2.20.1

[PATCH u-boot-marvell 2/3] arm: a37xx: pci: Register controller also when no PCIe card is connected

[Pali Rohár]

Allow access to config space of PCIe Root Port (which is always present on
the root bus) even when PCIe link is down or no card is connected.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 drivers/pci/pci-aardvark.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/pci/pci-aardvark.c b/drivers/pci/pci-aardvark.c
index ccaeecaca8e3..c795ef10b884 100644
--- a/drivers/pci/pci-aardvark.c
+++ b/drivers/pci/pci-aardvark.c
@@ -645,11 +645,8 @@ retry:
  * @pcie: The PCI device to access
  *
  * Wait up to 1 second for link training to be accomplished.
- *
- * Return 1 (true) if link training ends up with link up success.
- * Return 0 (false) if link training ends up with link up failure.
  */
-static int pcie_advk_wait_for_link(struct pcie_advk *pcie)
+static void pcie_advk_wait_for_link(struct pcie_advk *pcie)
 {
 	int retries;
 
@@ -657,15 +654,13 @@ static int pcie_advk_wait_for_link(struct pcie_advk *pcie)
 	for (retries = 0; retries < LINK_MAX_RETRIES; retries++) {
 		if (pcie_advk_link_up(pcie)) {
 			printf("PCIe: Link up\n");
-			return 0;
+			return;
 		}
 
 		udelay(LINK_WAIT_TIMEOUT);
 	}
 
 	printf("PCIe: Link down\n");
-
-	return -ETIMEDOUT;
 }
 
 /*
@@ -898,8 +893,7 @@ static int pcie_advk_setup_hw(struct pcie_advk *pcie)
 		return -EINVAL;
 
 	/* Wait for PCIe link up */
-	if (pcie_advk_wait_for_link(pcie))
-		return -ENXIO;
+	pcie_advk_wait_for_link(pcie);
 
 	return 0;
 }
-- 
2.20.1

[PATCH u-boot-marvell 3/3] arm: a37xx: pci: Update comment about Command/Direct mode

[Pali Rohár]

Code is changing PCIe controller from Command mode to Direct mode.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 drivers/pci/pci-aardvark.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/pci-aardvark.c b/drivers/pci/pci-aardvark.c
index c795ef10b884..4f7e61ecf166 100644
--- a/drivers/pci/pci-aardvark.c
+++ b/drivers/pci/pci-aardvark.c
@@ -757,7 +757,7 @@ static int pcie_advk_setup_hw(struct pcie_advk *pcie)
 	int i, wins;
 	u32 reg;
 
-	/* Set to Direct mode */
+	/* Set from Command to Direct mode */
 	reg = advk_readl(pcie, ADVK_CORE_CTRL_CONFIG);
 	reg &= ~ADVK_CORE_CTRL_CONFIG_COMMAND_MODE;
 	advk_writel(pcie, reg, ADVK_CORE_CTRL_CONFIG);
-- 
2.20.1

Re: [PATCH u-boot-marvell 1/3] arm: a37xx: pci: Do not try to access other buses when link is down

[Stefan Roese]

On 2/15/22 11:23, Pali Rohár wrote:
> If a PIO request is executed while link-down, the whole controller gets
> stuck in a non-functional state, and even after link comes up again, PIO
> requests won't work anymore, and a reset of the whole PCIe controller is
> needed. Therefore we need to prevent sending PIO requests while the link
> is down.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

> ---
>   drivers/pci/pci-aardvark.c | 38 +++++++++++++++++++++-----------------
>   1 file changed, 21 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/pci/pci-aardvark.c b/drivers/pci/pci-aardvark.c
> index 1eb257ea8b4a..ccaeecaca8e3 100644
> --- a/drivers/pci/pci-aardvark.c
> +++ b/drivers/pci/pci-aardvark.c
> @@ -177,6 +177,23 @@ static inline uint advk_readl(struct pcie_advk *pcie, uint reg)
>   	return readl(pcie->base + reg);
>   }
>   
> +/**
> + * pcie_advk_link_up() - Check if PCIe link is up or not
> + *
> + * @pcie: The PCI device to access
> + *
> + * Return true on link up.
> + * Return false on link down.
> + */
> +static bool pcie_advk_link_up(struct pcie_advk *pcie)
> +{
> +	u32 val, ltssm_state;
> +
> +	val = advk_readl(pcie, ADVK_LMI_PHY_CFG0);
> +	ltssm_state = (val & ADVK_LMI_PHY_CFG0_LTSSM_MASK) >> ADVK_LMI_PHY_CFG0_LTSSM_SHIFT;
> +	return ltssm_state >= ADVK_LMI_PHY_CFG0_LTSSM_L0 && ltssm_state < ADVK_LMI_PHY_CFG0_LTSSM_DISABLED;
> +}
> +
>   /**
>    * pcie_advk_addr_valid() - Check for valid bus address
>    *
> @@ -195,6 +212,10 @@ static bool pcie_advk_addr_valid(struct pcie_advk *pcie,
>   	if (busno == 0 && (dev != 0 || func != 0))
>   		return false;
>   
> +	/* Access to other buses is possible when link is up */
> +	if (busno != 0 && !pcie_advk_link_up(pcie))
> +		return false;
> +
>   	/*
>   	 * In PCI-E only a single device (0) can exist on the secondary bus.
>   	 * Beyond the secondary bus, there might be a Switch and anything is
> @@ -618,23 +639,6 @@ retry:
>   	return ret;
>   }
>   
> -/**
> - * pcie_advk_link_up() - Check if PCIe link is up or not
> - *
> - * @pcie: The PCI device to access
> - *
> - * Return 1 (true) on link up.
> - * Return 0 (false) on link down.
> - */
> -static int pcie_advk_link_up(struct pcie_advk *pcie)
> -{
> -	u32 val, ltssm_state;
> -
> -	val = advk_readl(pcie, ADVK_LMI_PHY_CFG0);
> -	ltssm_state = (val & ADVK_LMI_PHY_CFG0_LTSSM_MASK) >> ADVK_LMI_PHY_CFG0_LTSSM_SHIFT;
> -	return ltssm_state >= ADVK_LMI_PHY_CFG0_LTSSM_L0 && ltssm_state < ADVK_LMI_PHY_CFG0_LTSSM_DISABLED;
> -}
> -
>   /**
>    * pcie_advk_wait_for_link() - Wait for link training to be accomplished
>    *

Viele Grüße,
Stefan Roese

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de

Re: [PATCH u-boot-marvell 2/3] arm: a37xx: pci: Register controller also when no PCIe card is connected

[Stefan Roese]

On 2/15/22 11:23, Pali Rohár wrote:
> Allow access to config space of PCIe Root Port (which is always present on
> the root bus) even when PCIe link is down or no card is connected.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

> ---
>   drivers/pci/pci-aardvark.c | 12 +++---------
>   1 file changed, 3 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/pci/pci-aardvark.c b/drivers/pci/pci-aardvark.c
> index ccaeecaca8e3..c795ef10b884 100644
> --- a/drivers/pci/pci-aardvark.c
> +++ b/drivers/pci/pci-aardvark.c
> @@ -645,11 +645,8 @@ retry:
>    * @pcie: The PCI device to access
>    *
>    * Wait up to 1 second for link training to be accomplished.
> - *
> - * Return 1 (true) if link training ends up with link up success.
> - * Return 0 (false) if link training ends up with link up failure.
>    */
> -static int pcie_advk_wait_for_link(struct pcie_advk *pcie)
> +static void pcie_advk_wait_for_link(struct pcie_advk *pcie)
>   {
>   	int retries;
>   
> @@ -657,15 +654,13 @@ static int pcie_advk_wait_for_link(struct pcie_advk *pcie)
>   	for (retries = 0; retries < LINK_MAX_RETRIES; retries++) {
>   		if (pcie_advk_link_up(pcie)) {
>   			printf("PCIe: Link up\n");
> -			return 0;
> +			return;
>   		}
>   
>   		udelay(LINK_WAIT_TIMEOUT);
>   	}
>   
>   	printf("PCIe: Link down\n");
> -
> -	return -ETIMEDOUT;
>   }
>   
>   /*
> @@ -898,8 +893,7 @@ static int pcie_advk_setup_hw(struct pcie_advk *pcie)
>   		return -EINVAL;
>   
>   	/* Wait for PCIe link up */
> -	if (pcie_advk_wait_for_link(pcie))
> -		return -ENXIO;
> +	pcie_advk_wait_for_link(pcie);
>   
>   	return 0;
>   }

Viele Grüße,
Stefan Roese

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de

Re: [PATCH u-boot-marvell 3/3] arm: a37xx: pci: Update comment about Command/Direct mode

[Stefan Roese]

On 2/15/22 11:23, Pali Rohár wrote:
> Code is changing PCIe controller from Command mode to Direct mode.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

> ---
>   drivers/pci/pci-aardvark.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/pci-aardvark.c b/drivers/pci/pci-aardvark.c
> index c795ef10b884..4f7e61ecf166 100644
> --- a/drivers/pci/pci-aardvark.c
> +++ b/drivers/pci/pci-aardvark.c
> @@ -757,7 +757,7 @@ static int pcie_advk_setup_hw(struct pcie_advk *pcie)
>   	int i, wins;
>   	u32 reg;
>   
> -	/* Set to Direct mode */
> +	/* Set from Command to Direct mode */
>   	reg = advk_readl(pcie, ADVK_CORE_CTRL_CONFIG);
>   	reg &= ~ADVK_CORE_CTRL_CONFIG_COMMAND_MODE;
>   	advk_writel(pcie, reg, ADVK_CORE_CTRL_CONFIG);

Viele Grüße,
Stefan Roese

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de