[PATCH u-boot-marvell v2 0/6] A3720 PCIe enhancements

[PATCH u-boot-marvell v2 0/6] A3720 PCIe enhancements

[Marek Behún]

From: Marek Behún <marek.behun@nic.cz>

Hello Stefan,

Pali has worked on some more enhancements for Armada 3720 PCIe driver
(Aardvark).

The main change is adding support for accessing configuration space of
the PCI Bridge on the PCIe Root Port. Linux does something similar with
pci-bridge-emul.

Marek

Changes since v1:
- fix mistake in the documentation change in the last patch

Marek Behún (2):
  arm: a37xx: pci: Cosmetic change
  arm: a37xx: pci: Update private structure documentation

Pali Rohár (4):
  arm: a37xx: pci: Fix pcie_advk_link_up()
  arm: a37xx: pci: Add support for accessing PCI Bridge on root bus
  arm: a37xx: pci: Do not automatically enable bus mastering on PCI
    Bridge
  arm: a37xx: pci: Handle propagation of CRSSVE bit from PCIe Root Port

 drivers/pci/pci-aardvark.c | 199 +++++++++++++++++++++++++++++--------
 include/pci.h              |   4 +
 2 files changed, 163 insertions(+), 40 deletions(-)

-- 
2.32.0

[PATCH u-boot-marvell v2 1/6] arm: a37xx: pci: Fix pcie_advk_link_up()

[Marek Behún]

From: Pali Rohár <pali@kernel.org>

Aardvark reports Disabled and Hot Reset LTSSM states as values >= 0x20.
Link is not up in these states, so fix pcie_advk_link_up() function.

Signed-off-by: Pali Rohár <pali@kernel.org>
Reviewed-by: Marek Behún <marek.behun@nic.cz>
---
 drivers/pci/pci-aardvark.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/pci-aardvark.c b/drivers/pci/pci-aardvark.c
index cf6e30f936..741e0431e1 100644
--- a/drivers/pci/pci-aardvark.c
+++ b/drivers/pci/pci-aardvark.c
@@ -145,6 +145,7 @@
 #define     LTSSM_SHIFT				24
 #define     LTSSM_MASK				0x3f
 #define     LTSSM_L0				0x10
+#define     LTSSM_DISABLED			0x20
 #define VENDOR_ID_REG				(LMI_BASE_ADDR + 0x44)
 
 /* PCIe core controller registers */
@@ -569,7 +570,7 @@ static int pcie_advk_link_up(struct pcie_advk *pcie)
 
 	val = advk_readl(pcie, CFG_REG);
 	ltssm_state = (val >> LTSSM_SHIFT) & LTSSM_MASK;
-	return ltssm_state >= LTSSM_L0;
+	return ltssm_state >= LTSSM_L0 && ltssm_state < LTSSM_DISABLED;
 }
 
 /**
-- 
2.32.0

Re: [PATCH u-boot-marvell v2 1/6] arm: a37xx: pci: Fix pcie_advk_link_up()

[Stefan Roese]

On 26.09.21 00:54, Marek Behún wrote:
> From: Pali Rohár <pali@kernel.org>
> 
> Aardvark reports Disabled and Hot Reset LTSSM states as values >= 0x20.
> Link is not up in these states, so fix pcie_advk_link_up() function.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>
> Reviewed-by: Marek Behún <marek.behun@nic.cz>

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

Thanks,
Stefan

> ---
>   drivers/pci/pci-aardvark.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/pci-aardvark.c b/drivers/pci/pci-aardvark.c
> index cf6e30f936..741e0431e1 100644
> --- a/drivers/pci/pci-aardvark.c
> +++ b/drivers/pci/pci-aardvark.c
> @@ -145,6 +145,7 @@
>   #define     LTSSM_SHIFT				24
>   #define     LTSSM_MASK				0x3f
>   #define     LTSSM_L0				0x10
> +#define     LTSSM_DISABLED			0x20
>   #define VENDOR_ID_REG				(LMI_BASE_ADDR + 0x44)
>   
>   /* PCIe core controller registers */
> @@ -569,7 +570,7 @@ static int pcie_advk_link_up(struct pcie_advk *pcie)
>   
>   	val = advk_readl(pcie, CFG_REG);
>   	ltssm_state = (val >> LTSSM_SHIFT) & LTSSM_MASK;
> -	return ltssm_state >= LTSSM_L0;
> +	return ltssm_state >= LTSSM_L0 && ltssm_state < LTSSM_DISABLED;
>   }
>   
>   /**
> 


Viele Grüße,
Stefan

-- 
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

[PATCH u-boot-marvell v2 2/6] arm: a37xx: pci: Add support for accessing PCI Bridge on root bus

[Marek Behún]

From: Pali Rohár <pali@kernel.org>

Aardvark does not have a real PCIe Root Port device on the root bus.
Instead it has PCIe registers of PCIe Root Port device mapped in
internal Aardvark memory space starting at offset 0xc0.

The PCIe Root Port itself is normally available as a PCI Bridge device
on the root bus with bus number zero. Aardvark instead has the
configuration registers of this PCI Bridge at offset 0x00 of Aardvark's
memory space, but the class code of this device is Mass Storage
Controller (0x010400), instead of PCI Bridge (0x600400), which causes
U-Boot to fail to recognize it as a P2P Bridge

Add a hook into the pcie_advk_read_config() / pcie_advk_write_config()
functions to redirect access for root bus from PIO transfer to this
internal Aardvark memory space. This will allow U-Boot to access
configuration space of this PCI Bridge which represents PCIe Root Port.

Redirect access to PCI Bridge registers in range 0x10 - 0x34 to driver's
internal buffer (cfgcache[]). This is because at those addresses
Aardvark has different registers, incompatible with config space of a
PCI Bridge.

Redirect access to PCI Bridge register PCI_ROM_ADDRESS1 (0x38) to
Aardvark internal address for that register (0x30).

When reading PCI Bridge register PCI_HEADER_TYPE, set it explicitly to
value Type 1 (PCI_HEADER_TYPE_BRIDGE) as PCI Bridge must be of Type 1.

When writing to PCI_PRIMARY_BUS or PCI_SECONDARY_BUS registers on this
PCI Bridge, correctly update driver's first_busno and sec_busno
variables, so that pcie_advk_addr_valid() function can check if address
of any device behind the root bus is valid and that PIO transfers are
started with correct config type (1 vs 0), which is required for
accessing devices behind some PCI bridge after the root bus.

U-Boot's PCI_PNP code sets primary and secondary bus numbers as relative
to the configured bus number of the root bus. This is done so that
U-Boot can support multiple PCIe host bridges or multiple root port
buses, when internal bus numbers are different.

Now that root bus is available, update code in pcie_advk_read_config()
and pcie_advk_write_config() functions to correctly calculate real
Aardvark bus number of the target device from U-Boot's bus number as:
  busno = PCI_BUS(bdf) - dev_seq(bus)

Signed-off-by: Pali Rohár <pali@kernel.org>
Reviewed-by: Marek Behún <marek.behun@nic.cz>
---
 drivers/pci/pci-aardvark.c | 143 ++++++++++++++++++++++++++++++++-----
 1 file changed, 124 insertions(+), 19 deletions(-)

diff --git a/drivers/pci/pci-aardvark.c b/drivers/pci/pci-aardvark.c
index 741e0431e1..692210ded9 100644
--- a/drivers/pci/pci-aardvark.c
+++ b/drivers/pci/pci-aardvark.c
@@ -39,6 +39,8 @@
 #define     PCIE_CORE_CMD_IO_ACCESS_EN				BIT(0)
 #define     PCIE_CORE_CMD_MEM_ACCESS_EN				BIT(1)
 #define     PCIE_CORE_CMD_MEM_IO_REQ_EN				BIT(2)
+#define PCIE_CORE_DEV_REV_REG					0x8
+#define PCIE_CORE_EXP_ROM_BAR_REG				0x30
 #define PCIE_CORE_DEV_CTRL_STATS_REG				0xc8
 #define     PCIE_CORE_DEV_CTRL_STATS_RELAX_ORDER_DISABLE	(0 << 4)
 #define     PCIE_CORE_DEV_CTRL_STATS_SNOOP_DISABLE		(0 << 11)
@@ -163,7 +165,7 @@
 #define PCIE_CONFIG_WR_TYPE1			0xb
 
 /* PCI_BDF shifts 8bit, so we need extra 4bit shift */
-#define PCIE_BDF(dev)				(dev << 4)
+#define PCIE_BDF(b, d, f)			(PCI_BDF(b, d, f) << 4)
 #define PCIE_CONF_BUS(bus)			(((bus) & 0xff) << 20)
 #define PCIE_CONF_DEV(dev)			(((dev) & 0x1f) << 15)
 #define PCIE_CONF_FUNC(fun)			(((fun) & 0x7)	<< 12)
@@ -188,13 +190,17 @@
  *               first_busno stores the bus number of the PCIe root-port
  *               number which may vary depending on the PCIe setup
  *               (PEX switches etc).
+ * @sec_busno:   sec_busno stores the bus number for the device behind
+ *               the PCIe root-port
  * @device:      The pointer to PCI uclass device.
  */
 struct pcie_advk {
 	void           *base;
 	int            first_busno;
+	int            sec_busno;
 	struct udevice *dev;
 	struct gpio_desc reset_gpio;
+	u32            cfgcache[0x34 - 0x10];
 };
 
 static inline void advk_writel(struct pcie_advk *pcie, uint val, uint reg)
@@ -210,22 +216,30 @@ static inline uint advk_readl(struct pcie_advk *pcie, uint reg)
 /**
  * pcie_advk_addr_valid() - Check for valid bus address
  *
+ * @pcie: Pointer to the PCI bus
+ * @busno: Bus number of PCI device
+ * @dev: Device number of PCI device
+ * @func: Function number of PCI device
  * @bdf: The PCI device to access
- * @first_busno: Bus number of the PCIe controller root complex
  *
- * Return: 1 on valid, 0 on invalid
+ * Return: true on valid, false on invalid
  */
-static int pcie_advk_addr_valid(pci_dev_t bdf, int first_busno)
+static bool pcie_advk_addr_valid(struct pcie_advk *pcie,
+				 int busno, u8 dev, u8 func)
 {
+	/* On the primary (local) bus there is only one PCI Bridge */
+	if (busno == pcie->first_busno && (dev != 0 || func != 0))
+		return false;
+
 	/*
-	 * In PCIE-E only a single device (0) can exist
-	 * on the local bus. Beyound the local bus, there might be
-	 * a Switch and everything is possible.
+	 * 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
+	 * possible.
 	 */
-	if ((PCI_BUS(bdf) == first_busno) && (PCI_DEV(bdf) > 0))
-		return 0;
+	if (busno == pcie->sec_busno && dev != 0)
+		return false;
 
-	return 1;
+	return true;
 }
 
 /**
@@ -355,20 +369,55 @@ static int pcie_advk_read_config(const struct udevice *bus, pci_dev_t bdf,
 				 enum pci_size_t size)
 {
 	struct pcie_advk *pcie = dev_get_priv(bus);
+	int busno = PCI_BUS(bdf) - dev_seq(bus);
 	int retry_count;
 	bool allow_crs;
+	ulong data;
 	uint reg;
 	int ret;
 
 	dev_dbg(pcie->dev, "PCIE CFG read:  (b,d,f)=(%2d,%2d,%2d) ",
 		PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf));
 
-	if (!pcie_advk_addr_valid(bdf, pcie->first_busno)) {
+	if (!pcie_advk_addr_valid(pcie, busno, PCI_DEV(bdf), PCI_FUNC(bdf))) {
 		dev_dbg(pcie->dev, "- out of range\n");
 		*valuep = pci_get_ff(size);
 		return 0;
 	}
 
+	/*
+	 * The configuration space of the PCI Bridge on primary (local) bus is
+	 * not accessible via PIO transfers like all other PCIe devices. PCI
+	 * Bridge config registers are available directly in Aardvark memory
+	 * space starting at offset zero. Moreover PCI Bridge registers in the
+	 * range 0x10 - 0x34 are not available and register 0x38 (Expansion ROM
+	 * Base Address) is at offset 0x30.
+	 * We therefore read configuration space content of the primary PCI
+	 * Bridge from our virtual cache.
+	 */
+	if (busno == pcie->first_busno) {
+		if (offset >= 0x10 && offset < 0x34)
+			data = pcie->cfgcache[(offset - 0x10) / 4];
+		else if ((offset & ~3) == PCI_ROM_ADDRESS1)
+			data = advk_readl(pcie, PCIE_CORE_EXP_ROM_BAR_REG);
+		else
+			data = advk_readl(pcie, offset & ~3);
+
+		if ((offset & ~3) == (PCI_HEADER_TYPE & ~3)) {
+			/*
+			 * Change Header Type of PCI Bridge device to Type 1
+			 * (0x01, used by PCI Bridges) because hardwired value
+			 * is Type 0 (0x00, used by Endpoint devices).
+			 */
+			data &= ~0x00ff0000;
+			data |= PCI_HEADER_TYPE_BRIDGE << 16;
+		}
+
+		*valuep = pci_conv_32_to_size(data, offset, size);
+
+		return 0;
+	}
+
 	/*
 	 * Returning fabricated CRS value (0xFFFF0001) by PCIe Root Complex to
 	 * OS is allowed only for 4-byte PCI_VENDOR_ID config read request and
@@ -396,14 +445,14 @@ static int pcie_advk_read_config(const struct udevice *bus, pci_dev_t bdf,
 	/* Program the control register */
 	reg = advk_readl(pcie, PIO_CTRL);
 	reg &= ~PIO_CTRL_TYPE_MASK;
-	if (PCI_BUS(bdf) == pcie->first_busno)
+	if (busno == pcie->sec_busno)
 		reg |= PCIE_CONFIG_RD_TYPE0;
 	else
 		reg |= PCIE_CONFIG_RD_TYPE1;
 	advk_writel(pcie, reg, PIO_CTRL);
 
 	/* Program the address registers */
-	reg = PCIE_BDF(bdf) | PCIE_CONF_REG(offset);
+	reg = PCIE_BDF(busno, PCI_DEV(bdf), PCI_FUNC(bdf)) | PCIE_CONF_REG(offset);
 	advk_writel(pcie, reg, PIO_ADDR_LS);
 	advk_writel(pcie, 0, PIO_ADDR_MS);
 
@@ -491,7 +540,9 @@ static int pcie_advk_write_config(struct udevice *bus, pci_dev_t bdf,
 				  enum pci_size_t size)
 {
 	struct pcie_advk *pcie = dev_get_priv(bus);
+	int busno = PCI_BUS(bdf) - dev_seq(bus);
 	int retry_count;
+	ulong data;
 	uint reg;
 	int ret;
 
@@ -500,11 +551,41 @@ static int pcie_advk_write_config(struct udevice *bus, pci_dev_t bdf,
 	dev_dbg(pcie->dev, "(addr,size,val)=(0x%04x, %d, 0x%08lx)\n",
 		offset, size, value);
 
-	if (!pcie_advk_addr_valid(bdf, pcie->first_busno)) {
+	if (!pcie_advk_addr_valid(pcie, busno, PCI_DEV(bdf), PCI_FUNC(bdf))) {
 		dev_dbg(pcie->dev, "- out of range\n");
 		return 0;
 	}
 
+	/*
+	 * As explained in pcie_advk_read_config(), for the configuration
+	 * space of the primary PCI Bridge, we write the content into virtual
+	 * cache.
+	 */
+	if (busno == pcie->first_busno) {
+		if (offset >= 0x10 && offset < 0x34) {
+			data = pcie->cfgcache[(offset - 0x10) / 4];
+			data = pci_conv_size_to_32(data, value, offset, size);
+			pcie->cfgcache[(offset - 0x10) / 4] = data;
+		} else if ((offset & ~3) == PCI_ROM_ADDRESS1) {
+			data = advk_readl(pcie, PCIE_CORE_EXP_ROM_BAR_REG);
+			data = pci_conv_size_to_32(data, value, offset, size);
+			advk_writel(pcie, data, PCIE_CORE_EXP_ROM_BAR_REG);
+		} else {
+			data = advk_readl(pcie, offset & ~3);
+			data = pci_conv_size_to_32(data, value, offset, size);
+			advk_writel(pcie, data, offset & ~3);
+		}
+
+		if (offset == PCI_PRIMARY_BUS)
+			pcie->first_busno = data & 0xff;
+
+		if (offset == PCI_SECONDARY_BUS ||
+		    (offset == PCI_PRIMARY_BUS && size != PCI_SIZE_8))
+			pcie->sec_busno = (data >> 8) & 0xff;
+
+		return 0;
+	}
+
 	if (advk_readl(pcie, PIO_START)) {
 		dev_err(pcie->dev,
 			"Previous PIO read/write transfer is still running\n");
@@ -514,14 +595,14 @@ static int pcie_advk_write_config(struct udevice *bus, pci_dev_t bdf,
 	/* Program the control register */
 	reg = advk_readl(pcie, PIO_CTRL);
 	reg &= ~PIO_CTRL_TYPE_MASK;
-	if (PCI_BUS(bdf) == pcie->first_busno)
+	if (busno == pcie->sec_busno)
 		reg |= PCIE_CONFIG_WR_TYPE0;
 	else
 		reg |= PCIE_CONFIG_WR_TYPE1;
 	advk_writel(pcie, reg, PIO_CTRL);
 
 	/* Program the address registers */
-	reg = PCIE_BDF(bdf) | PCIE_CONF_REG(offset);
+	reg = PCIE_BDF(busno, PCI_DEV(bdf), PCI_FUNC(bdf)) | PCIE_CONF_REG(offset);
 	advk_writel(pcie, reg, PIO_ADDR_LS);
 	advk_writel(pcie, 0, PIO_ADDR_MS);
 	dev_dbg(pcie->dev, "\tPIO req. - addr = 0x%08x\n", reg);
@@ -590,14 +671,14 @@ static int pcie_advk_wait_for_link(struct pcie_advk *pcie)
 	/* check if the link is up or not */
 	for (retries = 0; retries < LINK_MAX_RETRIES; retries++) {
 		if (pcie_advk_link_up(pcie)) {
-			printf("PCIE-%d: Link up\n", pcie->first_busno);
+			printf("PCIe: Link up\n");
 			return 0;
 		}
 
 		udelay(LINK_WAIT_TIMEOUT);
 	}
 
-	printf("PCIE-%d: Link down\n", pcie->first_busno);
+	printf("PCIe: Link down\n");
 
 	return -ETIMEDOUT;
 }
@@ -716,6 +797,25 @@ static int pcie_advk_setup_hw(struct pcie_advk *pcie)
 	 */
 	advk_writel(pcie, 0x11ab11ab, VENDOR_ID_REG);
 
+	/*
+	 * Change Class Code of PCI Bridge device to PCI Bridge (0x600400),
+	 * because default value is Mass Storage Controller (0x010400), causing
+	 * U-Boot to fail to recognize it as P2P Bridge.
+	 *
+	 * Note that this Aardvark PCI Bridge does not have a compliant Type 1
+	 * Configuration Space and it even cannot be accessed via Aardvark's
+	 * PCI config space access method. Something like config space is
+	 * available in internal Aardvark registers starting at offset 0x0
+	 * and is reported as Type 0. In range 0x10 - 0x34 it has totally
+	 * different registers. So our driver reports Header Type as Type 1 and
+	 * for the above mentioned range redirects access to the virtual
+	 * cfgcache[] buffer, which avoids changing internal Aardvark registers.
+	 */
+	reg = advk_readl(pcie, PCIE_CORE_DEV_REV_REG);
+	reg &= ~0xffffff00;
+	reg |= (PCI_CLASS_BRIDGE_PCI << 8) << 8;
+	advk_writel(pcie, reg, PCIE_CORE_DEV_REV_REG);
+
 	/* Set Advanced Error Capabilities and Control PF0 register */
 	reg = PCIE_CORE_ERR_CAPCTL_ECRC_CHK_TX |
 		PCIE_CORE_ERR_CAPCTL_ECRC_CHK_TX_EN |
@@ -857,9 +957,14 @@ static int pcie_advk_probe(struct udevice *dev)
 		dev_warn(dev, "PCIE Reset on GPIO support is missing\n");
 	}
 
-	pcie->first_busno = dev_seq(dev);
 	pcie->dev = pci_get_controller(dev);
 
+	/* PCI Bridge support 32-bit I/O and 64-bit prefetch mem addressing */
+	pcie->cfgcache[(PCI_IO_BASE - 0x10) / 4] =
+		PCI_IO_RANGE_TYPE_32 | (PCI_IO_RANGE_TYPE_32 << 8);
+	pcie->cfgcache[(PCI_PREF_MEMORY_BASE - 0x10) / 4] =
+		PCI_PREF_RANGE_TYPE_64 | (PCI_PREF_RANGE_TYPE_64 << 16);
+
 	return pcie_advk_setup_hw(pcie);
 }
 
-- 
2.32.0

Re: [PATCH u-boot-marvell v2 2/6] arm: a37xx: pci: Add support for accessing PCI Bridge on root bus

[Pali Rohár]

On Sunday 26 September 2021 00:54:42 Marek Behún wrote:
> @@ -355,20 +369,55 @@ static int pcie_advk_read_config(const struct udevice *bus, pci_dev_t bdf,
>  				 enum pci_size_t size)
>  {
>  	struct pcie_advk *pcie = dev_get_priv(bus);
> +	int busno = PCI_BUS(bdf) - dev_seq(bus);
>  	int retry_count;
>  	bool allow_crs;
> +	ulong data;
>  	uint reg;
>  	int ret;
>  
>  	dev_dbg(pcie->dev, "PCIE CFG read:  (b,d,f)=(%2d,%2d,%2d) ",
>  		PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf));
>  
> -	if (!pcie_advk_addr_valid(bdf, pcie->first_busno)) {
> +	if (!pcie_advk_addr_valid(pcie, busno, PCI_DEV(bdf), PCI_FUNC(bdf))) {
>  		dev_dbg(pcie->dev, "- out of range\n");
>  		*valuep = pci_get_ff(size);
>  		return 0;
>  	}
>  
> +	/*
> +	 * The configuration space of the PCI Bridge on primary (local) bus is
> +	 * not accessible via PIO transfers like all other PCIe devices. PCI
> +	 * Bridge config registers are available directly in Aardvark memory
> +	 * space starting at offset zero. Moreover PCI Bridge registers in the
> +	 * range 0x10 - 0x34 are not available and register 0x38 (Expansion ROM
> +	 * Base Address) is at offset 0x30.
> +	 * We therefore read configuration space content of the primary PCI
> +	 * Bridge from our virtual cache.
> +	 */
> +	if (busno == pcie->first_busno) {
> +		if (offset >= 0x10 && offset < 0x34)
> +			data = pcie->cfgcache[(offset - 0x10) / 4];
> +		else if ((offset & ~3) == PCI_ROM_ADDRESS1)
> +			data = advk_readl(pcie, PCIE_CORE_EXP_ROM_BAR_REG);
> +		else
> +			data = advk_readl(pcie, offset & ~3);
> +
> +		if ((offset & ~3) == (PCI_HEADER_TYPE & ~3)) {
> +			/*
> +			 * Change Header Type of PCI Bridge device to Type 1
> +			 * (0x01, used by PCI Bridges) because hardwired value
> +			 * is Type 0 (0x00, used by Endpoint devices).
> +			 */
> +			data &= ~0x00ff0000;
                                     ^^
There is a small mistake. Header Type is 7-bit value. Upper 8th bit
indicates if device is multifunction. So correct mask should be:

			data &= ~0x007f0000;

> +			data |= PCI_HEADER_TYPE_BRIDGE << 16;
> +		}
> +
> +		*valuep = pci_conv_32_to_size(data, offset, size);
> +
> +		return 0;
> +	}
> +
>  	/*
>  	 * Returning fabricated CRS value (0xFFFF0001) by PCIe Root Complex to
>  	 * OS is allowed only for 4-byte PCI_VENDOR_ID config read request and

Re: [PATCH u-boot-marvell v2 2/6] arm: a37xx: pci: Add support for accessing PCI Bridge on root bus

[Stefan Roese]

On 05.10.21 23:55, Pali Rohár wrote:
> On Sunday 26 September 2021 00:54:42 Marek Behún wrote:
>> @@ -355,20 +369,55 @@ static int pcie_advk_read_config(const struct udevice *bus, pci_dev_t bdf,
>>   				 enum pci_size_t size)
>>   {
>>   	struct pcie_advk *pcie = dev_get_priv(bus);
>> +	int busno = PCI_BUS(bdf) - dev_seq(bus);
>>   	int retry_count;
>>   	bool allow_crs;
>> +	ulong data;
>>   	uint reg;
>>   	int ret;
>>   
>>   	dev_dbg(pcie->dev, "PCIE CFG read:  (b,d,f)=(%2d,%2d,%2d) ",
>>   		PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf));
>>   
>> -	if (!pcie_advk_addr_valid(bdf, pcie->first_busno)) {
>> +	if (!pcie_advk_addr_valid(pcie, busno, PCI_DEV(bdf), PCI_FUNC(bdf))) {
>>   		dev_dbg(pcie->dev, "- out of range\n");
>>   		*valuep = pci_get_ff(size);
>>   		return 0;
>>   	}
>>   
>> +	/*
>> +	 * The configuration space of the PCI Bridge on primary (local) bus is
>> +	 * not accessible via PIO transfers like all other PCIe devices. PCI
>> +	 * Bridge config registers are available directly in Aardvark memory
>> +	 * space starting at offset zero. Moreover PCI Bridge registers in the
>> +	 * range 0x10 - 0x34 are not available and register 0x38 (Expansion ROM
>> +	 * Base Address) is at offset 0x30.
>> +	 * We therefore read configuration space content of the primary PCI
>> +	 * Bridge from our virtual cache.
>> +	 */
>> +	if (busno == pcie->first_busno) {
>> +		if (offset >= 0x10 && offset < 0x34)
>> +			data = pcie->cfgcache[(offset - 0x10) / 4];
>> +		else if ((offset & ~3) == PCI_ROM_ADDRESS1)
>> +			data = advk_readl(pcie, PCIE_CORE_EXP_ROM_BAR_REG);
>> +		else
>> +			data = advk_readl(pcie, offset & ~3);
>> +
>> +		if ((offset & ~3) == (PCI_HEADER_TYPE & ~3)) {
>> +			/*
>> +			 * Change Header Type of PCI Bridge device to Type 1
>> +			 * (0x01, used by PCI Bridges) because hardwired value
>> +			 * is Type 0 (0x00, used by Endpoint devices).
>> +			 */
>> +			data &= ~0x00ff0000;
>                                       ^^
> There is a small mistake. Header Type is 7-bit value. Upper 8th bit
> indicates if device is multifunction. So correct mask should be:
> 
> 			data &= ~0x007f0000;

I'll try to get to these (and other) patches today and will fix this
while applying.

Thanks,
Stefan

>> +			data |= PCI_HEADER_TYPE_BRIDGE << 16;
>> +		}
>> +
>> +		*valuep = pci_conv_32_to_size(data, offset, size);
>> +
>> +		return 0;
>> +	}
>> +
>>   	/*
>>   	 * Returning fabricated CRS value (0xFFFF0001) by PCIe Root Complex to
>>   	 * OS is allowed only for 4-byte PCI_VENDOR_ID config read request and


Viele Grüße,
Stefan

-- 
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 v2 2/6] arm: a37xx: pci: Add support for accessing PCI Bridge on root bus

[Stefan Roese]

On 26.09.21 00:54, Marek Behún wrote:
> From: Pali Rohár <pali@kernel.org>
> 
> Aardvark does not have a real PCIe Root Port device on the root bus.
> Instead it has PCIe registers of PCIe Root Port device mapped in
> internal Aardvark memory space starting at offset 0xc0.
> 
> The PCIe Root Port itself is normally available as a PCI Bridge device
> on the root bus with bus number zero. Aardvark instead has the
> configuration registers of this PCI Bridge at offset 0x00 of Aardvark's
> memory space, but the class code of this device is Mass Storage
> Controller (0x010400), instead of PCI Bridge (0x600400), which causes
> U-Boot to fail to recognize it as a P2P Bridge
> 
> Add a hook into the pcie_advk_read_config() / pcie_advk_write_config()
> functions to redirect access for root bus from PIO transfer to this
> internal Aardvark memory space. This will allow U-Boot to access
> configuration space of this PCI Bridge which represents PCIe Root Port.
> 
> Redirect access to PCI Bridge registers in range 0x10 - 0x34 to driver's
> internal buffer (cfgcache[]). This is because at those addresses
> Aardvark has different registers, incompatible with config space of a
> PCI Bridge.
> 
> Redirect access to PCI Bridge register PCI_ROM_ADDRESS1 (0x38) to
> Aardvark internal address for that register (0x30).
> 
> When reading PCI Bridge register PCI_HEADER_TYPE, set it explicitly to
> value Type 1 (PCI_HEADER_TYPE_BRIDGE) as PCI Bridge must be of Type 1.
> 
> When writing to PCI_PRIMARY_BUS or PCI_SECONDARY_BUS registers on this
> PCI Bridge, correctly update driver's first_busno and sec_busno
> variables, so that pcie_advk_addr_valid() function can check if address
> of any device behind the root bus is valid and that PIO transfers are
> started with correct config type (1 vs 0), which is required for
> accessing devices behind some PCI bridge after the root bus.
> 
> U-Boot's PCI_PNP code sets primary and secondary bus numbers as relative
> to the configured bus number of the root bus. This is done so that
> U-Boot can support multiple PCIe host bridges or multiple root port
> buses, when internal bus numbers are different.
> 
> Now that root bus is available, update code in pcie_advk_read_config()
> and pcie_advk_write_config() functions to correctly calculate real
> Aardvark bus number of the target device from U-Boot's bus number as:
>    busno = PCI_BUS(bdf) - dev_seq(bus)
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>
> Reviewed-by: Marek Behún <marek.behun@nic.cz>

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

Thanks,
Stefan

> ---
>   drivers/pci/pci-aardvark.c | 143 ++++++++++++++++++++++++++++++++-----
>   1 file changed, 124 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/pci/pci-aardvark.c b/drivers/pci/pci-aardvark.c
> index 741e0431e1..692210ded9 100644
> --- a/drivers/pci/pci-aardvark.c
> +++ b/drivers/pci/pci-aardvark.c
> @@ -39,6 +39,8 @@
>   #define     PCIE_CORE_CMD_IO_ACCESS_EN				BIT(0)
>   #define     PCIE_CORE_CMD_MEM_ACCESS_EN				BIT(1)
>   #define     PCIE_CORE_CMD_MEM_IO_REQ_EN				BIT(2)
> +#define PCIE_CORE_DEV_REV_REG					0x8
> +#define PCIE_CORE_EXP_ROM_BAR_REG				0x30
>   #define PCIE_CORE_DEV_CTRL_STATS_REG				0xc8
>   #define     PCIE_CORE_DEV_CTRL_STATS_RELAX_ORDER_DISABLE	(0 << 4)
>   #define     PCIE_CORE_DEV_CTRL_STATS_SNOOP_DISABLE		(0 << 11)
> @@ -163,7 +165,7 @@
>   #define PCIE_CONFIG_WR_TYPE1			0xb
>   
>   /* PCI_BDF shifts 8bit, so we need extra 4bit shift */
> -#define PCIE_BDF(dev)				(dev << 4)
> +#define PCIE_BDF(b, d, f)			(PCI_BDF(b, d, f) << 4)
>   #define PCIE_CONF_BUS(bus)			(((bus) & 0xff) << 20)
>   #define PCIE_CONF_DEV(dev)			(((dev) & 0x1f) << 15)
>   #define PCIE_CONF_FUNC(fun)			(((fun) & 0x7)	<< 12)
> @@ -188,13 +190,17 @@
>    *               first_busno stores the bus number of the PCIe root-port
>    *               number which may vary depending on the PCIe setup
>    *               (PEX switches etc).
> + * @sec_busno:   sec_busno stores the bus number for the device behind
> + *               the PCIe root-port
>    * @device:      The pointer to PCI uclass device.
>    */
>   struct pcie_advk {
>   	void           *base;
>   	int            first_busno;
> +	int            sec_busno;
>   	struct udevice *dev;
>   	struct gpio_desc reset_gpio;
> +	u32            cfgcache[0x34 - 0x10];
>   };
>   
>   static inline void advk_writel(struct pcie_advk *pcie, uint val, uint reg)
> @@ -210,22 +216,30 @@ static inline uint advk_readl(struct pcie_advk *pcie, uint reg)
>   /**
>    * pcie_advk_addr_valid() - Check for valid bus address
>    *
> + * @pcie: Pointer to the PCI bus
> + * @busno: Bus number of PCI device
> + * @dev: Device number of PCI device
> + * @func: Function number of PCI device
>    * @bdf: The PCI device to access
> - * @first_busno: Bus number of the PCIe controller root complex
>    *
> - * Return: 1 on valid, 0 on invalid
> + * Return: true on valid, false on invalid
>    */
> -static int pcie_advk_addr_valid(pci_dev_t bdf, int first_busno)
> +static bool pcie_advk_addr_valid(struct pcie_advk *pcie,
> +				 int busno, u8 dev, u8 func)
>   {
> +	/* On the primary (local) bus there is only one PCI Bridge */
> +	if (busno == pcie->first_busno && (dev != 0 || func != 0))
> +		return false;
> +
>   	/*
> -	 * In PCIE-E only a single device (0) can exist
> -	 * on the local bus. Beyound the local bus, there might be
> -	 * a Switch and everything is possible.
> +	 * 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
> +	 * possible.
>   	 */
> -	if ((PCI_BUS(bdf) == first_busno) && (PCI_DEV(bdf) > 0))
> -		return 0;
> +	if (busno == pcie->sec_busno && dev != 0)
> +		return false;
>   
> -	return 1;
> +	return true;
>   }
>   
>   /**
> @@ -355,20 +369,55 @@ static int pcie_advk_read_config(const struct udevice *bus, pci_dev_t bdf,
>   				 enum pci_size_t size)
>   {
>   	struct pcie_advk *pcie = dev_get_priv(bus);
> +	int busno = PCI_BUS(bdf) - dev_seq(bus);
>   	int retry_count;
>   	bool allow_crs;
> +	ulong data;
>   	uint reg;
>   	int ret;
>   
>   	dev_dbg(pcie->dev, "PCIE CFG read:  (b,d,f)=(%2d,%2d,%2d) ",
>   		PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf));
>   
> -	if (!pcie_advk_addr_valid(bdf, pcie->first_busno)) {
> +	if (!pcie_advk_addr_valid(pcie, busno, PCI_DEV(bdf), PCI_FUNC(bdf))) {
>   		dev_dbg(pcie->dev, "- out of range\n");
>   		*valuep = pci_get_ff(size);
>   		return 0;
>   	}
>   
> +	/*
> +	 * The configuration space of the PCI Bridge on primary (local) bus is
> +	 * not accessible via PIO transfers like all other PCIe devices. PCI
> +	 * Bridge config registers are available directly in Aardvark memory
> +	 * space starting at offset zero. Moreover PCI Bridge registers in the
> +	 * range 0x10 - 0x34 are not available and register 0x38 (Expansion ROM
> +	 * Base Address) is at offset 0x30.
> +	 * We therefore read configuration space content of the primary PCI
> +	 * Bridge from our virtual cache.
> +	 */
> +	if (busno == pcie->first_busno) {
> +		if (offset >= 0x10 && offset < 0x34)
> +			data = pcie->cfgcache[(offset - 0x10) / 4];
> +		else if ((offset & ~3) == PCI_ROM_ADDRESS1)
> +			data = advk_readl(pcie, PCIE_CORE_EXP_ROM_BAR_REG);
> +		else
> +			data = advk_readl(pcie, offset & ~3);
> +
> +		if ((offset & ~3) == (PCI_HEADER_TYPE & ~3)) {
> +			/*
> +			 * Change Header Type of PCI Bridge device to Type 1
> +			 * (0x01, used by PCI Bridges) because hardwired value
> +			 * is Type 0 (0x00, used by Endpoint devices).
> +			 */
> +			data &= ~0x00ff0000;
> +			data |= PCI_HEADER_TYPE_BRIDGE << 16;
> +		}
> +
> +		*valuep = pci_conv_32_to_size(data, offset, size);
> +
> +		return 0;
> +	}
> +
>   	/*
>   	 * Returning fabricated CRS value (0xFFFF0001) by PCIe Root Complex to
>   	 * OS is allowed only for 4-byte PCI_VENDOR_ID config read request and
> @@ -396,14 +445,14 @@ static int pcie_advk_read_config(const struct udevice *bus, pci_dev_t bdf,
>   	/* Program the control register */
>   	reg = advk_readl(pcie, PIO_CTRL);
>   	reg &= ~PIO_CTRL_TYPE_MASK;
> -	if (PCI_BUS(bdf) == pcie->first_busno)
> +	if (busno == pcie->sec_busno)
>   		reg |= PCIE_CONFIG_RD_TYPE0;
>   	else
>   		reg |= PCIE_CONFIG_RD_TYPE1;
>   	advk_writel(pcie, reg, PIO_CTRL);
>   
>   	/* Program the address registers */
> -	reg = PCIE_BDF(bdf) | PCIE_CONF_REG(offset);
> +	reg = PCIE_BDF(busno, PCI_DEV(bdf), PCI_FUNC(bdf)) | PCIE_CONF_REG(offset);
>   	advk_writel(pcie, reg, PIO_ADDR_LS);
>   	advk_writel(pcie, 0, PIO_ADDR_MS);
>   
> @@ -491,7 +540,9 @@ static int pcie_advk_write_config(struct udevice *bus, pci_dev_t bdf,
>   				  enum pci_size_t size)
>   {
>   	struct pcie_advk *pcie = dev_get_priv(bus);
> +	int busno = PCI_BUS(bdf) - dev_seq(bus);
>   	int retry_count;
> +	ulong data;
>   	uint reg;
>   	int ret;
>   
> @@ -500,11 +551,41 @@ static int pcie_advk_write_config(struct udevice *bus, pci_dev_t bdf,
>   	dev_dbg(pcie->dev, "(addr,size,val)=(0x%04x, %d, 0x%08lx)\n",
>   		offset, size, value);
>   
> -	if (!pcie_advk_addr_valid(bdf, pcie->first_busno)) {
> +	if (!pcie_advk_addr_valid(pcie, busno, PCI_DEV(bdf), PCI_FUNC(bdf))) {
>   		dev_dbg(pcie->dev, "- out of range\n");
>   		return 0;
>   	}
>   
> +	/*
> +	 * As explained in pcie_advk_read_config(), for the configuration
> +	 * space of the primary PCI Bridge, we write the content into virtual
> +	 * cache.
> +	 */
> +	if (busno == pcie->first_busno) {
> +		if (offset >= 0x10 && offset < 0x34) {
> +			data = pcie->cfgcache[(offset - 0x10) / 4];
> +			data = pci_conv_size_to_32(data, value, offset, size);
> +			pcie->cfgcache[(offset - 0x10) / 4] = data;
> +		} else if ((offset & ~3) == PCI_ROM_ADDRESS1) {
> +			data = advk_readl(pcie, PCIE_CORE_EXP_ROM_BAR_REG);
> +			data = pci_conv_size_to_32(data, value, offset, size);
> +			advk_writel(pcie, data, PCIE_CORE_EXP_ROM_BAR_REG);
> +		} else {
> +			data = advk_readl(pcie, offset & ~3);
> +			data = pci_conv_size_to_32(data, value, offset, size);
> +			advk_writel(pcie, data, offset & ~3);
> +		}
> +
> +		if (offset == PCI_PRIMARY_BUS)
> +			pcie->first_busno = data & 0xff;
> +
> +		if (offset == PCI_SECONDARY_BUS ||
> +		    (offset == PCI_PRIMARY_BUS && size != PCI_SIZE_8))
> +			pcie->sec_busno = (data >> 8) & 0xff;
> +
> +		return 0;
> +	}
> +
>   	if (advk_readl(pcie, PIO_START)) {
>   		dev_err(pcie->dev,
>   			"Previous PIO read/write transfer is still running\n");
> @@ -514,14 +595,14 @@ static int pcie_advk_write_config(struct udevice *bus, pci_dev_t bdf,
>   	/* Program the control register */
>   	reg = advk_readl(pcie, PIO_CTRL);
>   	reg &= ~PIO_CTRL_TYPE_MASK;
> -	if (PCI_BUS(bdf) == pcie->first_busno)
> +	if (busno == pcie->sec_busno)
>   		reg |= PCIE_CONFIG_WR_TYPE0;
>   	else
>   		reg |= PCIE_CONFIG_WR_TYPE1;
>   	advk_writel(pcie, reg, PIO_CTRL);
>   
>   	/* Program the address registers */
> -	reg = PCIE_BDF(bdf) | PCIE_CONF_REG(offset);
> +	reg = PCIE_BDF(busno, PCI_DEV(bdf), PCI_FUNC(bdf)) | PCIE_CONF_REG(offset);
>   	advk_writel(pcie, reg, PIO_ADDR_LS);
>   	advk_writel(pcie, 0, PIO_ADDR_MS);
>   	dev_dbg(pcie->dev, "\tPIO req. - addr = 0x%08x\n", reg);
> @@ -590,14 +671,14 @@ static int pcie_advk_wait_for_link(struct pcie_advk *pcie)
>   	/* check if the link is up or not */
>   	for (retries = 0; retries < LINK_MAX_RETRIES; retries++) {
>   		if (pcie_advk_link_up(pcie)) {
> -			printf("PCIE-%d: Link up\n", pcie->first_busno);
> +			printf("PCIe: Link up\n");
>   			return 0;
>   		}
>   
>   		udelay(LINK_WAIT_TIMEOUT);
>   	}
>   
> -	printf("PCIE-%d: Link down\n", pcie->first_busno);
> +	printf("PCIe: Link down\n");
>   
>   	return -ETIMEDOUT;
>   }
> @@ -716,6 +797,25 @@ static int pcie_advk_setup_hw(struct pcie_advk *pcie)
>   	 */
>   	advk_writel(pcie, 0x11ab11ab, VENDOR_ID_REG);
>   
> +	/*
> +	 * Change Class Code of PCI Bridge device to PCI Bridge (0x600400),
> +	 * because default value is Mass Storage Controller (0x010400), causing
> +	 * U-Boot to fail to recognize it as P2P Bridge.
> +	 *
> +	 * Note that this Aardvark PCI Bridge does not have a compliant Type 1
> +	 * Configuration Space and it even cannot be accessed via Aardvark's
> +	 * PCI config space access method. Something like config space is
> +	 * available in internal Aardvark registers starting at offset 0x0
> +	 * and is reported as Type 0. In range 0x10 - 0x34 it has totally
> +	 * different registers. So our driver reports Header Type as Type 1 and
> +	 * for the above mentioned range redirects access to the virtual
> +	 * cfgcache[] buffer, which avoids changing internal Aardvark registers.
> +	 */
> +	reg = advk_readl(pcie, PCIE_CORE_DEV_REV_REG);
> +	reg &= ~0xffffff00;
> +	reg |= (PCI_CLASS_BRIDGE_PCI << 8) << 8;
> +	advk_writel(pcie, reg, PCIE_CORE_DEV_REV_REG);
> +
>   	/* Set Advanced Error Capabilities and Control PF0 register */
>   	reg = PCIE_CORE_ERR_CAPCTL_ECRC_CHK_TX |
>   		PCIE_CORE_ERR_CAPCTL_ECRC_CHK_TX_EN |
> @@ -857,9 +957,14 @@ static int pcie_advk_probe(struct udevice *dev)
>   		dev_warn(dev, "PCIE Reset on GPIO support is missing\n");
>   	}
>   
> -	pcie->first_busno = dev_seq(dev);
>   	pcie->dev = pci_get_controller(dev);
>   
> +	/* PCI Bridge support 32-bit I/O and 64-bit prefetch mem addressing */
> +	pcie->cfgcache[(PCI_IO_BASE - 0x10) / 4] =
> +		PCI_IO_RANGE_TYPE_32 | (PCI_IO_RANGE_TYPE_32 << 8);
> +	pcie->cfgcache[(PCI_PREF_MEMORY_BASE - 0x10) / 4] =
> +		PCI_PREF_RANGE_TYPE_64 | (PCI_PREF_RANGE_TYPE_64 << 16);
> +
>   	return pcie_advk_setup_hw(pcie);
>   }
>   
> 


Viele Grüße,
Stefan

-- 
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

[PATCH u-boot-marvell v2 3/6] arm: a37xx: pci: Do not automatically enable bus mastering on PCI Bridge

[Marek Behún]

From: Pali Rohár <pali@kernel.org>

Now that PCI Bridge is working for the PCIe Root Port, U-Boot's PCI_PNP
code automatically enables memory access and bus mastering when needed.

We do not need to enable it when setting the HW up.

Signed-off-by: Pali Rohár <pali@kernel.org>
Reviewed-by: Marek Behún <marek.behun@nic.cz>
---
 drivers/pci/pci-aardvark.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/pci/pci-aardvark.c b/drivers/pci/pci-aardvark.c
index 692210ded9..8c025dc45d 100644
--- a/drivers/pci/pci-aardvark.c
+++ b/drivers/pci/pci-aardvark.c
@@ -910,12 +910,6 @@ static int pcie_advk_setup_hw(struct pcie_advk *pcie)
 	if (pcie_advk_wait_for_link(pcie))
 		return -ENXIO;
 
-	reg = advk_readl(pcie, PCIE_CORE_CMD_STATUS_REG);
-	reg |= PCIE_CORE_CMD_MEM_ACCESS_EN |
-		PCIE_CORE_CMD_IO_ACCESS_EN |
-		PCIE_CORE_CMD_MEM_IO_REQ_EN;
-	advk_writel(pcie, reg, PCIE_CORE_CMD_STATUS_REG);
-
 	return 0;
 }
 
-- 
2.32.0

Re: [PATCH u-boot-marvell v2 3/6] arm: a37xx: pci: Do not automatically enable bus mastering on PCI Bridge

[Stefan Roese]

On 26.09.21 00:54, Marek Behún wrote:
> From: Pali Rohár <pali@kernel.org>
> 
> Now that PCI Bridge is working for the PCIe Root Port, U-Boot's PCI_PNP
> code automatically enables memory access and bus mastering when needed.
> 
> We do not need to enable it when setting the HW up.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>
> Reviewed-by: Marek Behún <marek.behun@nic.cz>

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

Thanks,
Stefan

> ---
>   drivers/pci/pci-aardvark.c | 6 ------
>   1 file changed, 6 deletions(-)
> 
> diff --git a/drivers/pci/pci-aardvark.c b/drivers/pci/pci-aardvark.c
> index 692210ded9..8c025dc45d 100644
> --- a/drivers/pci/pci-aardvark.c
> +++ b/drivers/pci/pci-aardvark.c
> @@ -910,12 +910,6 @@ static int pcie_advk_setup_hw(struct pcie_advk *pcie)
>   	if (pcie_advk_wait_for_link(pcie))
>   		return -ENXIO;
>   
> -	reg = advk_readl(pcie, PCIE_CORE_CMD_STATUS_REG);
> -	reg |= PCIE_CORE_CMD_MEM_ACCESS_EN |
> -		PCIE_CORE_CMD_IO_ACCESS_EN |
> -		PCIE_CORE_CMD_MEM_IO_REQ_EN;
> -	advk_writel(pcie, reg, PCIE_CORE_CMD_STATUS_REG);
> -
>   	return 0;
>   }
>   
> 


Viele Grüße,
Stefan

-- 
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

[PATCH u-boot-marvell v2 4/6] arm: a37xx: pci: Handle propagation of CRSSVE bit from PCIe Root Port

[Marek Behún]

From: Pali Rohár <pali@kernel.org>

Now that PCI Bridge (PCIe Root Port) for Aardvark is emulated in U-Boot,
add support for handling and propagation of CRSSVE bit.

When CRSSVE bit is unset (default), driver has to reissue config
read/write request on CRS response.

CRSSVE bit is supported only when CRSVIS bit is provided in read-only
Root Capabilities register. So manually inject this CRSVIS bit into read
response for that register.

Signed-off-by: Pali Rohár <pali@kernel.org>
Reviewed-by: Marek Behún <marek.behun@nic.cz>
---
 drivers/pci/pci-aardvark.c | 26 ++++++++++++++++++++++----
 include/pci.h              |  4 ++++
 2 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/pci-aardvark.c b/drivers/pci/pci-aardvark.c
index 8c025dc45d..53e9e23d4a 100644
--- a/drivers/pci/pci-aardvark.c
+++ b/drivers/pci/pci-aardvark.c
@@ -41,6 +41,7 @@
 #define     PCIE_CORE_CMD_MEM_IO_REQ_EN				BIT(2)
 #define PCIE_CORE_DEV_REV_REG					0x8
 #define PCIE_CORE_EXP_ROM_BAR_REG				0x30
+#define PCIE_CORE_PCIEXP_CAP_OFF				0xc0
 #define PCIE_CORE_DEV_CTRL_STATS_REG				0xc8
 #define     PCIE_CORE_DEV_CTRL_STATS_RELAX_ORDER_DISABLE	(0 << 4)
 #define     PCIE_CORE_DEV_CTRL_STATS_SNOOP_DISABLE		(0 << 11)
@@ -201,6 +202,7 @@ struct pcie_advk {
 	struct udevice *dev;
 	struct gpio_desc reset_gpio;
 	u32            cfgcache[0x34 - 0x10];
+	bool           cfgcrssve;
 };
 
 static inline void advk_writel(struct pcie_advk *pcie, uint val, uint reg)
@@ -413,6 +415,18 @@ static int pcie_advk_read_config(const struct udevice *bus, pci_dev_t bdf,
 			data |= PCI_HEADER_TYPE_BRIDGE << 16;
 		}
 
+		if ((offset & ~3) == PCIE_CORE_PCIEXP_CAP_OFF + PCI_EXP_RTCTL) {
+			/* CRSSVE bit is stored only in cache */
+			if (pcie->cfgcrssve)
+				data |= PCI_EXP_RTCTL_CRSSVE;
+		}
+
+		if ((offset & ~3) == PCIE_CORE_PCIEXP_CAP_OFF +
+				     (PCI_EXP_RTCAP & ~3)) {
+			/* CRS is emulated below, so set CRSVIS capability */
+			data |= PCI_EXP_RTCAP_CRSVIS << 16;
+		}
+
 		*valuep = pci_conv_32_to_size(data, offset, size);
 
 		return 0;
@@ -423,13 +437,14 @@ static int pcie_advk_read_config(const struct udevice *bus, pci_dev_t bdf,
 	 * OS is allowed only for 4-byte PCI_VENDOR_ID config read request and
 	 * only when CRSSVE bit in Root Port PCIe device is enabled. In all
 	 * other error PCIe Root Complex must return all-ones.
-	 * Aardvark HW does not have Root Port PCIe device and U-Boot does not
-	 * implement emulation of this device.
+	 *
 	 * U-Boot currently does not support handling of CRS return value for
 	 * PCI_VENDOR_ID config read request and also does not set CRSSVE bit.
-	 * Therefore disable returning CRS response for now.
+	 * So it means that pcie->cfgcrssve is false. But the code is prepared
+	 * for returning CRS, so that if U-Boot does support CRS in the future,
+	 * it will work for Aardvark.
 	 */
-	allow_crs = false;
+	allow_crs = pcie->cfgcrssve;
 
 	if (advk_readl(pcie, PIO_START)) {
 		dev_err(pcie->dev,
@@ -583,6 +598,9 @@ static int pcie_advk_write_config(struct udevice *bus, pci_dev_t bdf,
 		    (offset == PCI_PRIMARY_BUS && size != PCI_SIZE_8))
 			pcie->sec_busno = (data >> 8) & 0xff;
 
+		if ((offset & ~3) == PCIE_CORE_PCIEXP_CAP_OFF + PCI_EXP_RTCTL)
+			pcie->cfgcrssve = data & PCI_EXP_RTCTL_CRSSVE;
+
 		return 0;
 	}
 
diff --git a/include/pci.h b/include/pci.h
index 0fc22adffd..69eafeb4b9 100644
--- a/include/pci.h
+++ b/include/pci.h
@@ -495,6 +495,10 @@
 #define  PCI_EXP_LNKSTA_DLLLA	0x2000	/* Data Link Layer Link Active */
 #define PCI_EXP_SLTCAP		20	/* Slot Capabilities */
 #define  PCI_EXP_SLTCAP_PSN	0xfff80000 /* Physical Slot Number */
+#define PCI_EXP_RTCTL		28	/* Root Control */
+#define  PCI_EXP_RTCTL_CRSSVE	0x0010	/* CRS Software Visibility Enable */
+#define PCI_EXP_RTCAP		30	/* Root Capabilities */
+#define  PCI_EXP_RTCAP_CRSVIS	0x0001	/* CRS Software Visibility capability */
 #define PCI_EXP_DEVCAP2		36	/* Device Capabilities 2 */
 #define  PCI_EXP_DEVCAP2_ARI	0x00000020 /* ARI Forwarding Supported */
 #define PCI_EXP_DEVCTL2		40	/* Device Control 2 */
-- 
2.32.0

Re: [PATCH u-boot-marvell v2 4/6] arm: a37xx: pci: Handle propagation of CRSSVE bit from PCIe Root Port

[Stefan Roese]

On 26.09.21 00:54, Marek Behún wrote:
> From: Pali Rohár <pali@kernel.org>
> 
> Now that PCI Bridge (PCIe Root Port) for Aardvark is emulated in U-Boot,
> add support for handling and propagation of CRSSVE bit.
> 
> When CRSSVE bit is unset (default), driver has to reissue config
> read/write request on CRS response.
> 
> CRSSVE bit is supported only when CRSVIS bit is provided in read-only
> Root Capabilities register. So manually inject this CRSVIS bit into read
> response for that register.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>
> Reviewed-by: Marek Behún <marek.behun@nic.cz>

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

Thanks,
Stefan

> ---
>   drivers/pci/pci-aardvark.c | 26 ++++++++++++++++++++++----
>   include/pci.h              |  4 ++++
>   2 files changed, 26 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/pci/pci-aardvark.c b/drivers/pci/pci-aardvark.c
> index 8c025dc45d..53e9e23d4a 100644
> --- a/drivers/pci/pci-aardvark.c
> +++ b/drivers/pci/pci-aardvark.c
> @@ -41,6 +41,7 @@
>   #define     PCIE_CORE_CMD_MEM_IO_REQ_EN				BIT(2)
>   #define PCIE_CORE_DEV_REV_REG					0x8
>   #define PCIE_CORE_EXP_ROM_BAR_REG				0x30
> +#define PCIE_CORE_PCIEXP_CAP_OFF				0xc0
>   #define PCIE_CORE_DEV_CTRL_STATS_REG				0xc8
>   #define     PCIE_CORE_DEV_CTRL_STATS_RELAX_ORDER_DISABLE	(0 << 4)
>   #define     PCIE_CORE_DEV_CTRL_STATS_SNOOP_DISABLE		(0 << 11)
> @@ -201,6 +202,7 @@ struct pcie_advk {
>   	struct udevice *dev;
>   	struct gpio_desc reset_gpio;
>   	u32            cfgcache[0x34 - 0x10];
> +	bool           cfgcrssve;
>   };
>   
>   static inline void advk_writel(struct pcie_advk *pcie, uint val, uint reg)
> @@ -413,6 +415,18 @@ static int pcie_advk_read_config(const struct udevice *bus, pci_dev_t bdf,
>   			data |= PCI_HEADER_TYPE_BRIDGE << 16;
>   		}
>   
> +		if ((offset & ~3) == PCIE_CORE_PCIEXP_CAP_OFF + PCI_EXP_RTCTL) {
> +			/* CRSSVE bit is stored only in cache */
> +			if (pcie->cfgcrssve)
> +				data |= PCI_EXP_RTCTL_CRSSVE;
> +		}
> +
> +		if ((offset & ~3) == PCIE_CORE_PCIEXP_CAP_OFF +
> +				     (PCI_EXP_RTCAP & ~3)) {
> +			/* CRS is emulated below, so set CRSVIS capability */
> +			data |= PCI_EXP_RTCAP_CRSVIS << 16;
> +		}
> +
>   		*valuep = pci_conv_32_to_size(data, offset, size);
>   
>   		return 0;
> @@ -423,13 +437,14 @@ static int pcie_advk_read_config(const struct udevice *bus, pci_dev_t bdf,
>   	 * OS is allowed only for 4-byte PCI_VENDOR_ID config read request and
>   	 * only when CRSSVE bit in Root Port PCIe device is enabled. In all
>   	 * other error PCIe Root Complex must return all-ones.
> -	 * Aardvark HW does not have Root Port PCIe device and U-Boot does not
> -	 * implement emulation of this device.
> +	 *
>   	 * U-Boot currently does not support handling of CRS return value for
>   	 * PCI_VENDOR_ID config read request and also does not set CRSSVE bit.
> -	 * Therefore disable returning CRS response for now.
> +	 * So it means that pcie->cfgcrssve is false. But the code is prepared
> +	 * for returning CRS, so that if U-Boot does support CRS in the future,
> +	 * it will work for Aardvark.
>   	 */
> -	allow_crs = false;
> +	allow_crs = pcie->cfgcrssve;
>   
>   	if (advk_readl(pcie, PIO_START)) {
>   		dev_err(pcie->dev,
> @@ -583,6 +598,9 @@ static int pcie_advk_write_config(struct udevice *bus, pci_dev_t bdf,
>   		    (offset == PCI_PRIMARY_BUS && size != PCI_SIZE_8))
>   			pcie->sec_busno = (data >> 8) & 0xff;
>   
> +		if ((offset & ~3) == PCIE_CORE_PCIEXP_CAP_OFF + PCI_EXP_RTCTL)
> +			pcie->cfgcrssve = data & PCI_EXP_RTCTL_CRSSVE;
> +
>   		return 0;
>   	}
>   
> diff --git a/include/pci.h b/include/pci.h
> index 0fc22adffd..69eafeb4b9 100644
> --- a/include/pci.h
> +++ b/include/pci.h
> @@ -495,6 +495,10 @@
>   #define  PCI_EXP_LNKSTA_DLLLA	0x2000	/* Data Link Layer Link Active */
>   #define PCI_EXP_SLTCAP		20	/* Slot Capabilities */
>   #define  PCI_EXP_SLTCAP_PSN	0xfff80000 /* Physical Slot Number */
> +#define PCI_EXP_RTCTL		28	/* Root Control */
> +#define  PCI_EXP_RTCTL_CRSSVE	0x0010	/* CRS Software Visibility Enable */
> +#define PCI_EXP_RTCAP		30	/* Root Capabilities */
> +#define  PCI_EXP_RTCAP_CRSVIS	0x0001	/* CRS Software Visibility capability */
>   #define PCI_EXP_DEVCAP2		36	/* Device Capabilities 2 */
>   #define  PCI_EXP_DEVCAP2_ARI	0x00000020 /* ARI Forwarding Supported */
>   #define PCI_EXP_DEVCTL2		40	/* Device Control 2 */
> 


Viele Grüße,
Stefan

-- 
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

[PATCH u-boot-marvell v2 5/6] arm: a37xx: pci: Cosmetic change

[Marek Behún]

From: Marek Behún <marek.behun@nic.cz>

Update indentation in driver's private structure.

Signed-off-by: Marek Behún <marek.behun@nic.cz>
---
 drivers/pci/pci-aardvark.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/pci/pci-aardvark.c b/drivers/pci/pci-aardvark.c
index 53e9e23d4a..b4f350ca2c 100644
--- a/drivers/pci/pci-aardvark.c
+++ b/drivers/pci/pci-aardvark.c
@@ -196,13 +196,13 @@
  * @device:      The pointer to PCI uclass device.
  */
 struct pcie_advk {
-	void           *base;
-	int            first_busno;
-	int            sec_busno;
-	struct udevice *dev;
-	struct gpio_desc reset_gpio;
-	u32            cfgcache[0x34 - 0x10];
-	bool           cfgcrssve;
+	void			*base;
+	int			first_busno;
+	int			sec_busno;
+	struct udevice		*dev;
+	struct gpio_desc	reset_gpio;
+	u32			cfgcache[0x34 - 0x10];
+	bool			cfgcrssve;
 };
 
 static inline void advk_writel(struct pcie_advk *pcie, uint val, uint reg)
-- 
2.32.0

Re: [PATCH u-boot-marvell v2 5/6] arm: a37xx: pci: Cosmetic change

[Stefan Roese]

On 26.09.21 00:54, Marek Behún wrote:
> From: Marek Behún <marek.behun@nic.cz>
> 
> Update indentation in driver's private structure.
> 
> Signed-off-by: Marek Behún <marek.behun@nic.cz>

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

Thanks,
Stefan

> ---
>   drivers/pci/pci-aardvark.c | 14 +++++++-------
>   1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/pci/pci-aardvark.c b/drivers/pci/pci-aardvark.c
> index 53e9e23d4a..b4f350ca2c 100644
> --- a/drivers/pci/pci-aardvark.c
> +++ b/drivers/pci/pci-aardvark.c
> @@ -196,13 +196,13 @@
>    * @device:      The pointer to PCI uclass device.
>    */
>   struct pcie_advk {
> -	void           *base;
> -	int            first_busno;
> -	int            sec_busno;
> -	struct udevice *dev;
> -	struct gpio_desc reset_gpio;
> -	u32            cfgcache[0x34 - 0x10];
> -	bool           cfgcrssve;
> +	void			*base;
> +	int			first_busno;
> +	int			sec_busno;
> +	struct udevice		*dev;
> +	struct gpio_desc	reset_gpio;
> +	u32			cfgcache[0x34 - 0x10];
> +	bool			cfgcrssve;
>   };
>   
>   static inline void advk_writel(struct pcie_advk *pcie, uint val, uint reg)
> 


Viele Grüße,
Stefan

-- 
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

[PATCH u-boot-marvell v2 6/6] arm: a37xx: pci: Update private structure documentation

[Marek Behún]

From: Marek Behún <marek.behun@nic.cz>

There were several changes for this structure but the documentation was
not changed at the time. Fix this.

Signed-off-by: Marek Behún <marek.behun@nic.cz>
---
 drivers/pci/pci-aardvark.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/pci/pci-aardvark.c b/drivers/pci/pci-aardvark.c
index b4f350ca2c..414872ed35 100644
--- a/drivers/pci/pci-aardvark.c
+++ b/drivers/pci/pci-aardvark.c
@@ -186,14 +186,15 @@
 /**
  * struct pcie_advk - Advk PCIe controller state
  *
- * @reg_base:    The base address of the register space.
- * @first_busno: This driver supports multiple PCIe controllers.
- *               first_busno stores the bus number of the PCIe root-port
- *               number which may vary depending on the PCIe setup
- *               (PEX switches etc).
- * @sec_busno:   sec_busno stores the bus number for the device behind
- *               the PCIe root-port
- * @device:      The pointer to PCI uclass device.
+ * @base:        The base address of the register space.
+ * @first_busno: Bus number of the PCIe root-port.
+ *               This may vary depending on the PCIe setup.
+ * @sec_busno:   Bus number for the device behind the PCIe root-port.
+ * @dev:         The pointer to PCI uclass device.
+ * @reset_gpio:  GPIO descriptor for PERST.
+ * @cfgcache:    Buffer for emulation of PCIe Root Port's PCI Bridge registers
+ *               that are not available on Aardvark.
+ * @cfgcrssve:   For CRSSVE emulation.
  */
 struct pcie_advk {
 	void			*base;
-- 
2.32.0

Re: [PATCH u-boot-marvell v2 6/6] arm: a37xx: pci: Update private structure documentation

[Stefan Roese]

On 26.09.21 00:54, Marek Behún wrote:
> From: Marek Behún <marek.behun@nic.cz>
> 
> There were several changes for this structure but the documentation was
> not changed at the time. Fix this.
> 
> Signed-off-by: Marek Behún <marek.behun@nic.cz>

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

Thanks,
Stefan

> ---
>   drivers/pci/pci-aardvark.c | 17 +++++++++--------
>   1 file changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/pci/pci-aardvark.c b/drivers/pci/pci-aardvark.c
> index b4f350ca2c..414872ed35 100644
> --- a/drivers/pci/pci-aardvark.c
> +++ b/drivers/pci/pci-aardvark.c
> @@ -186,14 +186,15 @@
>   /**
>    * struct pcie_advk - Advk PCIe controller state
>    *
> - * @reg_base:    The base address of the register space.
> - * @first_busno: This driver supports multiple PCIe controllers.
> - *               first_busno stores the bus number of the PCIe root-port
> - *               number which may vary depending on the PCIe setup
> - *               (PEX switches etc).
> - * @sec_busno:   sec_busno stores the bus number for the device behind
> - *               the PCIe root-port
> - * @device:      The pointer to PCI uclass device.
> + * @base:        The base address of the register space.
> + * @first_busno: Bus number of the PCIe root-port.
> + *               This may vary depending on the PCIe setup.
> + * @sec_busno:   Bus number for the device behind the PCIe root-port.
> + * @dev:         The pointer to PCI uclass device.
> + * @reset_gpio:  GPIO descriptor for PERST.
> + * @cfgcache:    Buffer for emulation of PCIe Root Port's PCI Bridge registers
> + *               that are not available on Aardvark.
> + * @cfgcrssve:   For CRSSVE emulation.
>    */
>   struct pcie_advk {
>   	void			*base;
> 


Viele Grüße,
Stefan

-- 
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 v2 0/6] A3720 PCIe enhancements

[Stefan Roese]

On 26.09.21 00:54, Marek Behún wrote:
> From: Marek Behún <marek.behun@nic.cz>
> 
> Hello Stefan,
> 
> Pali has worked on some more enhancements for Armada 3720 PCIe driver
> (Aardvark).
> 
> The main change is adding support for accessing configuration space of
> the PCI Bridge on the PCIe Root Port. Linux does something similar with
> pci-bridge-emul.
> 
> Marek
> 
> Changes since v1:
> - fix mistake in the documentation change in the last patch
> 
> Marek Behún (2):
>    arm: a37xx: pci: Cosmetic change
>    arm: a37xx: pci: Update private structure documentation
> 
> Pali Rohár (4):
>    arm: a37xx: pci: Fix pcie_advk_link_up()
>    arm: a37xx: pci: Add support for accessing PCI Bridge on root bus
>    arm: a37xx: pci: Do not automatically enable bus mastering on PCI
>      Bridge
>    arm: a37xx: pci: Handle propagation of CRSSVE bit from PCIe Root Port
> 
>   drivers/pci/pci-aardvark.c | 199 +++++++++++++++++++++++++++++--------
>   include/pci.h              |   4 +
>   2 files changed, 163 insertions(+), 40 deletions(-)
> 

Applied to u-boot-marvell/master

Thanks,
Stefan