All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/8] pci: pci_mvebu: Fix access to config space and PCIe Root Port
@ 2021-10-22 14:22 Pali Rohár
  2021-10-22 14:22 ` [PATCH 1/8] pci: pci_mvebu: Fix write_config() with PCI_SIZE_8 or PCI_SIZE_16 Pali Rohár
                   ` (8 more replies)
  0 siblings, 9 replies; 18+ messages in thread
From: Pali Rohár @ 2021-10-22 14:22 UTC (permalink / raw)
  To: Stefan Roese; +Cc: Marek Behún, u-boot

This patch series fixes access to config space and fixes issues with the
mysterious "Memory controller" PCI device (which is PCIe Root Port).

Tested on Armada 385 Turris Omnia device which has 3 mPCIe slots.
PCIe Root Port device is available for each slot on separate bus and has
PCI Bridge class code as required by PCIe base specifications.

=> pci 0
Scanning PCI devices on bus 0
BusDevFun  VendorId   DeviceId   Device Class       Sub-Class
_____________________________________________________________
00.00.00   0x11ab     0x6820     Bridge device           0x04
=> pci 1
Scanning PCI devices on bus 1
BusDevFun  VendorId   DeviceId   Device Class       Sub-Class
_____________________________________________________________
01.00.00   0x168c     0x003c     Network controller      0x80
=> pci 2
Scanning PCI devices on bus 2
BusDevFun  VendorId   DeviceId   Device Class       Sub-Class
_____________________________________________________________
02.00.00   0x11ab     0x6820     Bridge device           0x04
=> pci 3
Scanning PCI devices on bus 3
BusDevFun  VendorId   DeviceId   Device Class       Sub-Class
_____________________________________________________________
03.00.00   0x168c     0x003c     Network controller      0x80
=> pci 4
Scanning PCI devices on bus 4
BusDevFun  VendorId   DeviceId   Device Class       Sub-Class
_____________________________________________________________
04.00.00   0x11ab     0x6820     Bridge device           0x04
=> pci 5
Scanning PCI devices on bus 5
BusDevFun  VendorId   DeviceId   Device Class       Sub-Class
_____________________________________________________________
05.00.00   0x168c     0x002e     Network controller      0x80
=> pci 6
No such bus
=>

U-Boot command "pci display.b 0.0.0 0 200" will display config space of
the first PCIe Root Port device with the Type 1 PCI header as required by
PCIe base specification.

=> pci display.b 0.0.0 0 200
00000000: ab 11 20 68 07 00 10 00 04 00 04 06 00 00 01 00
00000010: 00 00 00 00 00 00 00 00 00 01 01 00 01 f1 00 00
00000020: 00 c0 20 c0 01 10 01 00 00 00 00 00 00 00 00 00
00000030: 10 f1 0f f1 40 00 00 00 00 00 00 00 00 01 00 00
00000040: 01 50 03 06 00 00 00 00 00 00 00 00 00 00 00 00
00000050: 05 60 80 00 00 00 00 00 00 00 00 00 00 00 00 00
00000060: 10 00 42 00 80 80 00 00 00 20 00 00 12 ac 07 00
00000070: 00 00 11 10 00 00 00 00 00 00 40 00 00 00 00 00
00000080: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00000090: 02 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
000000a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
000000b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
000000c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
000000d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
000000e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
000000f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00000100: 01 00 01 00 00 00 00 00 00 00 00 00 10 00 06 00
00000110: 00 00 00 00 00 20 00 00 00 00 00 00 01 00 00 4a
00000120: 04 00 00 01 00 01 08 01 02 80 00 00 00 00 00 00
00000130: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00000140: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00000150: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00000160: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00000170: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00000180: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00000190: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
000001a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
000001b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
000001c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
000001d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
000001e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
000001f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00

Pali Rohár (8):
  pci: pci_mvebu: Fix write_config() with PCI_SIZE_8 or PCI_SIZE_16
  pci: pci_mvebu: Fix read_config() with PCI_SIZE_8 or PCI_SIZE_16
  pci: pci_mvebu: Properly configure and use PCI Bridge (PCIe Root Port)
  pci: pci_mvebu: Remove unused functions
  pci: pci_mvebu: Fix place of link up detection
  pci: pci_mvebu: Do not automatically enable bus mastering on PCI
    Bridge
  pci: pci_mvebu: Setup PCI controller to Root Complex mode
  pci: pci_mvebu: Fix comment about driver class name

 drivers/pci/pci_mvebu.c | 275 +++++++++++++++++++++++++++++-----------
 1 file changed, 202 insertions(+), 73 deletions(-)

-- 
2.20.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 1/8] pci: pci_mvebu: Fix write_config() with PCI_SIZE_8 or PCI_SIZE_16
  2021-10-22 14:22 [PATCH 0/8] pci: pci_mvebu: Fix access to config space and PCIe Root Port Pali Rohár
@ 2021-10-22 14:22 ` Pali Rohár
  2021-11-02 10:44   ` Stefan Roese
  2021-10-22 14:22 ` [PATCH 2/8] pci: pci_mvebu: Fix read_config() " Pali Rohár
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 18+ messages in thread
From: Pali Rohár @ 2021-10-22 14:22 UTC (permalink / raw)
  To: Stefan Roese; +Cc: Marek Behún, u-boot

Current implementation of write_config() is broken for PCI_SIZE_8 or
PCI_SIZE_16 as it always uses writel(), which means that write operation
is always 32-bit, so upper 24 bits for PCI_SIZE_8 and upper 16 bits for
PCI_SIZE_16 are cleared.

Fix this by using writeb() and writew(), respectively.

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

diff --git a/drivers/pci/pci_mvebu.c b/drivers/pci/pci_mvebu.c
index 0c1d7cd770f1..8175511514ab 100644
--- a/drivers/pci/pci_mvebu.c
+++ b/drivers/pci/pci_mvebu.c
@@ -211,8 +211,19 @@ static int mvebu_pcie_write_config(struct udevice *bus, pci_dev_t bdf,
 	writel(PCIE_CONF_ADDR(bdf, offset), pcie->base + PCIE_CONF_ADDR_OFF);
 
 	/* write data */
-	data = pci_conv_size_to_32(0, value, offset, size);
-	writel(data, pcie->base + PCIE_CONF_DATA_OFF);
+	switch (size) {
+	case PCI_SIZE_8:
+		writeb(value, pcie->base + PCIE_CONF_DATA_OFF + (offset & 3));
+		break;
+	case PCI_SIZE_16:
+		writew(value, pcie->base + PCIE_CONF_DATA_OFF + (offset & 2));
+		break;
+	case PCI_SIZE_32:
+		writel(value, pcie->base + PCIE_CONF_DATA_OFF);
+		break;
+	default:
+		return -EINVAL;
+	}
 
 	return 0;
 }
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 2/8] pci: pci_mvebu: Fix read_config() with PCI_SIZE_8 or PCI_SIZE_16
  2021-10-22 14:22 [PATCH 0/8] pci: pci_mvebu: Fix access to config space and PCIe Root Port Pali Rohár
  2021-10-22 14:22 ` [PATCH 1/8] pci: pci_mvebu: Fix write_config() with PCI_SIZE_8 or PCI_SIZE_16 Pali Rohár
@ 2021-10-22 14:22 ` Pali Rohár
  2021-11-02 10:45   ` Stefan Roese
  2021-10-22 14:22 ` [PATCH 3/8] pci: pci_mvebu: Properly configure and use PCI Bridge (PCIe Root Port) Pali Rohár
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 18+ messages in thread
From: Pali Rohár @ 2021-10-22 14:22 UTC (permalink / raw)
  To: Stefan Roese; +Cc: Marek Behún, u-boot

When reading 8 or 16 bits from config space, use appropriate readb() or
readw() calls. This ensures that PCIe controller does not read more bits
from endpoint card as asked by read_config() function.

Technically there should not be an issue with reading data from config
space which are not later used as there are no clear-by-read registers.
But it is better to use correct read operation based on requested size.

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

diff --git a/drivers/pci/pci_mvebu.c b/drivers/pci/pci_mvebu.c
index 8175511514ab..3991086e0d29 100644
--- a/drivers/pci/pci_mvebu.c
+++ b/drivers/pci/pci_mvebu.c
@@ -184,9 +184,22 @@ static int mvebu_pcie_read_config(const struct udevice *bus, pci_dev_t bdf,
 	writel(PCIE_CONF_ADDR(bdf, offset), pcie->base + PCIE_CONF_ADDR_OFF);
 
 	/* read data */
-	data = readl(pcie->base + PCIE_CONF_DATA_OFF);
+	switch (size) {
+	case PCI_SIZE_8:
+		data = readb(pcie->base + PCIE_CONF_DATA_OFF + (offset & 3));
+		break;
+	case PCI_SIZE_16:
+		data = readw(pcie->base + PCIE_CONF_DATA_OFF + (offset & 2));
+		break;
+	case PCI_SIZE_32:
+		data = readl(pcie->base + PCIE_CONF_DATA_OFF);
+		break;
+	default:
+		return -EINVAL;
+	}
+
 	debug("(addr,size,val)=(0x%04x, %d, 0x%08x)\n", offset, size, data);
-	*valuep = pci_conv_32_to_size(data, offset, size);
+	*valuep = data;
 
 	return 0;
 }
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 3/8] pci: pci_mvebu: Properly configure and use PCI Bridge (PCIe Root Port)
  2021-10-22 14:22 [PATCH 0/8] pci: pci_mvebu: Fix access to config space and PCIe Root Port Pali Rohár
  2021-10-22 14:22 ` [PATCH 1/8] pci: pci_mvebu: Fix write_config() with PCI_SIZE_8 or PCI_SIZE_16 Pali Rohár
  2021-10-22 14:22 ` [PATCH 2/8] pci: pci_mvebu: Fix read_config() " Pali Rohár
@ 2021-10-22 14:22 ` Pali Rohár
  2021-11-02 10:47   ` Stefan Roese
  2021-10-22 14:22 ` [PATCH 4/8] pci: pci_mvebu: Remove unused functions Pali Rohár
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 18+ messages in thread
From: Pali Rohár @ 2021-10-22 14:22 UTC (permalink / raw)
  To: Stefan Roese; +Cc: Marek Behún, u-boot

The mysterious "Memory controller" PCI device which is present in PCI
config space is improperly configured and crippled PCI Bridge which acts
as PCIe Root Port for endpoint PCIe card.

This PCI Bridge reports in PCI config space incorrect Class Code (Memory
Controller) and incorrect Header Type (Type 0). It looks like HW bug in
mvebu PCIe controller but apparently it can be changed via mvebu registers
to correct values.

The worst thing is that this PCI Bridge is crippled and its PCI config
registers in range 0x10-0x34 alias access to internal mvebu registers which
have different functionality as PCI Bridge registers. Moreover,
configuration of PCI primary and secondary bus numbers (registers 0x18
and 0x19) is done via totally different mvebu registers via totally strange
method and cannot be done via PCI Bridge config space.

Due to above fact about PCI config range 0x10-0x34, allocate a private
cfgcache[] buffer in the driver, to which PCI config access requests to
the 0x10-0x34 space will be redirected in mvebu_pcie_read_config() and
mvebu_pcie_write_config() functions. Function mvebu_pcie_write_config()
will also catch writes to PCI_PRIMARY_BUS (0x18) and PCI_SECONDARY_BUS
(0x19) registers and set PCI Bridge primary and secondary bus numbers via
mvebu's own method.

Also, Expansion ROM Base Address register (0x38) is available, but at
different offset 0x30. So recalculate register offset before accessing PCI
config space.

After these steps U-Boot sees working PCI Bridge and CONFIG_PCI_PNP code
can finally start enumerating all PCIe devices correctly, even with more
complicated PCI topology. So update also mvebu_pcie_valid_addr() function
to reflect state of the real device topology.

Each PCIe port is de-facto isolated and every PCI Bridge which is part of
PCIe Root Complex is also isolated, so put them on separate PCI buses as
(local) device 0.

U-Boot already supports enumerating separate PCI buses, real (HW) bus
number can be retrieved by "PCI_BUS(bdf) - dev_seq(bus)" code, so update
config read/write functions to properly handle more complicated tree
topologies (e.g. when a PCIe switch with multiple PCI buses is connected
to the PCIe port).

Local bus number and local device number on mvebu are used for determining
which config request type is used (Type 0 vs Type 1). On normal non-broken
PCIe hardware it is done by primary and secondary bus numbers. So correctly
translate settings between these numbers to ensure that correct config
requests are sent over the PCIe bus.

As bus numbers are correctly re-configured, it does not make sense to print
some initial bogus configuration during probe, so remove this debug code.

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

diff --git a/drivers/pci/pci_mvebu.c b/drivers/pci/pci_mvebu.c
index 3991086e0d29..244dcc8fb050 100644
--- a/drivers/pci/pci_mvebu.c
+++ b/drivers/pci/pci_mvebu.c
@@ -36,6 +36,7 @@ DECLARE_GLOBAL_DATA_PTR;
 #define PCIE_DEV_REV_OFF		0x0008
 #define  PCIE_BAR_LO_OFF(n)		(0x0010 + ((n) << 3))
 #define  PCIE_BAR_HI_OFF(n)		(0x0014 + ((n) << 3))
+#define PCIE_EXP_ROM_BAR_OFF		0x0030
 #define PCIE_CAPAB_OFF			0x0060
 #define PCIE_CTRL_STAT_OFF		0x0068
 #define PCIE_HEADER_LOG_4_OFF		0x0128
@@ -52,9 +53,9 @@ DECLARE_GLOBAL_DATA_PTR;
 #define  PCIE_CONF_BUS(b)		(((b) & 0xff) << 16)
 #define  PCIE_CONF_DEV(d)		(((d) & 0x1f) << 11)
 #define  PCIE_CONF_FUNC(f)		(((f) & 0x7) << 8)
-#define  PCIE_CONF_ADDR(dev, reg) \
-	(PCIE_CONF_BUS(PCI_BUS(dev)) | PCIE_CONF_DEV(PCI_DEV(dev))    | \
-	 PCIE_CONF_FUNC(PCI_FUNC(dev)) | PCIE_CONF_REG(reg) | \
+#define  PCIE_CONF_ADDR(b, d, f, reg) \
+	(PCIE_CONF_BUS(b) | PCIE_CONF_DEV(d)    | \
+	 PCIE_CONF_FUNC(f) | PCIE_CONF_REG(reg) | \
 	 PCIE_CONF_ADDR_EN)
 #define PCIE_CONF_DATA_OFF		0x18fc
 #define PCIE_MASK_OFF			0x1910
@@ -80,12 +81,13 @@ struct mvebu_pcie {
 	int devfn;
 	u32 lane_mask;
 	int first_busno;
-	int local_dev;
+	int sec_busno;
 	char name[16];
 	unsigned int mem_target;
 	unsigned int mem_attr;
 	unsigned int io_target;
 	unsigned int io_attr;
+	u32 cfgcache[0x34 - 0x10];
 };
 
 /*
@@ -145,23 +147,18 @@ static inline struct mvebu_pcie *hose_to_pcie(struct pci_controller *hose)
 	return container_of(hose, struct mvebu_pcie, hose);
 }
 
-static int mvebu_pcie_valid_addr(struct mvebu_pcie *pcie, pci_dev_t bdf)
+static bool mvebu_pcie_valid_addr(struct mvebu_pcie *pcie,
+				  int busno, int dev, int func)
 {
-	/*
-	 * There are two devices visible on local bus:
-	 *   * on slot configured by function mvebu_pcie_set_local_dev_nr()
-	 *     (by default this register is set to 0) there is a
-	 *     "Marvell Memory controller", which isn't useful in root complex
-	 *     mode,
-	 *   * on all other slots the real PCIe card connected to the PCIe slot.
-	 *
-	 * We therefore allow access only to the real PCIe card.
-	 */
-	if (PCI_BUS(bdf) == pcie->first_busno &&
-	    PCI_DEV(bdf) != !pcie->local_dev)
-		return 0;
+	/* On primary bus is only one PCI Bridge */
+	if (busno == pcie->first_busno && (dev != 0 || func != 0))
+		return false;
+
+	/* On secondary bus can be only one PCIe device */
+	if (busno == pcie->sec_busno && dev != 0)
+		return false;
 
-	return 1;
+	return true;
 }
 
 static int mvebu_pcie_read_config(const struct udevice *bus, pci_dev_t bdf,
@@ -169,19 +166,46 @@ static int mvebu_pcie_read_config(const struct udevice *bus, pci_dev_t bdf,
 				  enum pci_size_t size)
 {
 	struct mvebu_pcie *pcie = dev_get_plat(bus);
-	u32 data;
+	int busno = PCI_BUS(bdf) - dev_seq(bus);
+	u32 addr, data;
 
 	debug("PCIE CFG read: (b,d,f)=(%2d,%2d,%2d) ",
 	      PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf));
 
-	if (!mvebu_pcie_valid_addr(pcie, bdf)) {
+	if (!mvebu_pcie_valid_addr(pcie, busno, PCI_DEV(bdf), PCI_FUNC(bdf))) {
 		debug("- out of range\n");
 		*valuep = pci_get_ff(size);
 		return 0;
 	}
 
+	/*
+	 * mvebu has different internal registers mapped into PCI config space
+	 * in range 0x10-0x34 for PCI bridge, so do not access PCI config space
+	 * for this range and instead read content from driver virtual cfgcache
+	 */
+	if (busno == pcie->first_busno && offset >= 0x10 && offset < 0x34) {
+		data = pcie->cfgcache[(offset - 0x10) / 4];
+		debug("(addr,size,val)=(0x%04x, %d, 0x%08x) from cfgcache\n",
+		      offset, size, data);
+		*valuep = pci_conv_32_to_size(data, offset, size);
+		return 0;
+	} else if (busno == pcie->first_busno &&
+		   (offset & ~3) == PCI_ROM_ADDRESS1) {
+		/* mvebu has Expansion ROM Base Address (0x38) at offset 0x30 */
+		offset -= PCI_ROM_ADDRESS1 - PCIE_EXP_ROM_BAR_OFF;
+	}
+
+	/*
+	 * PCI bridge is device 0 at primary bus but mvebu has it mapped on
+	 * secondary bus with device number 1.
+	 */
+	if (busno == pcie->first_busno)
+		addr = PCIE_CONF_ADDR(pcie->sec_busno, 1, 0, offset);
+	else
+		addr = PCIE_CONF_ADDR(busno, PCI_DEV(bdf), PCI_FUNC(bdf), offset);
+
 	/* write address */
-	writel(PCIE_CONF_ADDR(bdf, offset), pcie->base + PCIE_CONF_ADDR_OFF);
+	writel(addr, pcie->base + PCIE_CONF_ADDR_OFF);
 
 	/* read data */
 	switch (size) {
@@ -198,6 +222,19 @@ static int mvebu_pcie_read_config(const struct udevice *bus, pci_dev_t bdf,
 		return -EINVAL;
 	}
 
+	if (busno == pcie->first_busno &&
+	    (offset & ~3) == (PCI_HEADER_TYPE & ~3)) {
+		/*
+		 * Change Header Type of PCI Bridge device to Type 1
+		 * (0x01, used by PCI Bridges) because mvebu reports
+		 * Type 0 (0x00, used by Upstream and Endpoint devices).
+		 */
+		data = pci_conv_size_to_32(data, 0, offset, size);
+		data &= ~0x007f0000;
+		data |= PCI_HEADER_TYPE_BRIDGE << 16;
+		data = pci_conv_32_to_size(data, offset, size);
+	}
+
 	debug("(addr,size,val)=(0x%04x, %d, 0x%08x)\n", offset, size, data);
 	*valuep = data;
 
@@ -209,19 +246,64 @@ static int mvebu_pcie_write_config(struct udevice *bus, pci_dev_t bdf,
 				   enum pci_size_t size)
 {
 	struct mvebu_pcie *pcie = dev_get_plat(bus);
-	u32 data;
+	int busno = PCI_BUS(bdf) - dev_seq(bus);
+	u32 addr, data;
 
 	debug("PCIE CFG write: (b,d,f)=(%2d,%2d,%2d) ",
 	      PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf));
 	debug("(addr,size,val)=(0x%04x, %d, 0x%08lx)\n", offset, size, value);
 
-	if (!mvebu_pcie_valid_addr(pcie, bdf)) {
+	if (!mvebu_pcie_valid_addr(pcie, busno, PCI_DEV(bdf), PCI_FUNC(bdf))) {
 		debug("- out of range\n");
 		return 0;
 	}
 
+	/*
+	 * mvebu has different internal registers mapped into PCI config space
+	 * in range 0x10-0x34 for PCI bridge, so do not access PCI config space
+	 * for this range and instead write content to driver virtual cfgcache
+	 */
+	if (busno == pcie->first_busno && offset >= 0x10 && offset < 0x34) {
+		debug("Writing to cfgcache only\n");
+		data = pcie->cfgcache[(offset - 0x10) / 4];
+		data = pci_conv_size_to_32(data, value, offset, size);
+		/* mvebu PCI bridge does not have configurable bars */
+		if ((offset & ~3) == PCI_BASE_ADDRESS_0 ||
+		    (offset & ~3) == PCI_BASE_ADDRESS_1)
+			data = 0x0;
+		pcie->cfgcache[(offset - 0x10) / 4] = data;
+		/* mvebu has its own way how to set PCI primary bus number */
+		if (offset == PCI_PRIMARY_BUS) {
+			pcie->first_busno = data & 0xff;
+			debug("Primary bus number was changed to %d\n",
+			      pcie->first_busno);
+		}
+		/* mvebu has its own way how to set PCI secondary bus number */
+		if (offset == PCI_SECONDARY_BUS ||
+		    (offset == PCI_PRIMARY_BUS && size != PCI_SIZE_8)) {
+			pcie->sec_busno = (data >> 8) & 0xff;
+			mvebu_pcie_set_local_bus_nr(pcie, pcie->sec_busno);
+			debug("Secondary bus number was changed to %d\n",
+			      pcie->sec_busno);
+		}
+		return 0;
+	} else if (busno == pcie->first_busno &&
+		   (offset & ~3) == PCI_ROM_ADDRESS1) {
+		/* mvebu has Expansion ROM Base Address (0x38) at offset 0x30 */
+		offset -= PCI_ROM_ADDRESS1 - PCIE_EXP_ROM_BAR_OFF;
+	}
+
+	/*
+	 * PCI bridge is device 0 at primary bus but mvebu has it mapped on
+	 * secondary bus with device number 1.
+	 */
+	if (busno == pcie->first_busno)
+		addr = PCIE_CONF_ADDR(pcie->sec_busno, 1, 0, offset);
+	else
+		addr = PCIE_CONF_ADDR(busno, PCI_DEV(bdf), PCI_FUNC(bdf), offset);
+
 	/* write address */
-	writel(PCIE_CONF_ADDR(bdf, offset), pcie->base + PCIE_CONF_ADDR_OFF);
+	writel(addr, pcie->base + PCIE_CONF_ADDR_OFF);
 
 	/* write data */
 	switch (size) {
@@ -301,22 +383,63 @@ static int mvebu_pcie_probe(struct udevice *dev)
 	struct mvebu_pcie *pcie = dev_get_plat(dev);
 	struct udevice *ctlr = pci_get_controller(dev);
 	struct pci_controller *hose = dev_get_uclass_priv(ctlr);
-	int bus = dev_seq(dev);
 	u32 reg;
 
 	debug("%s: PCIe %d.%d - up, base %08x\n", __func__,
 	      pcie->port, pcie->lane, (u32)pcie->base);
 
-	/* Read Id info and local bus/dev */
-	debug("direct conf read %08x, local bus %d, local dev %d\n",
-	      readl(pcie->base), mvebu_pcie_get_local_bus_nr(pcie),
-	      mvebu_pcie_get_local_dev_nr(pcie));
-
-	pcie->first_busno = bus;
-	pcie->local_dev = 1;
+	/*
+	 * Change Class Code of PCI Bridge device to PCI Bridge (0x600400)
+	 * because default value is Memory controller (0x508000) which
+	 * U-Boot cannot recognize as P2P Bridge.
+	 *
+	 * Note that this mvebu PCI Bridge does not have compliant Type 1
+	 * Configuration Space. Header Type is reported as Type 0 and in
+	 * range 0x10-0x34 it has aliased internal mvebu registers 0x10-0x34
+	 * (e.g. PCIE_BAR_LO_OFF) and register 0x38 is reserved.
+	 *
+	 * Driver for this range redirects access to virtual cfgcache[] buffer
+	 * which avoids changing internal mvebu registers. And changes Header
+	 * Type response value to Type 1.
+	 */
+	reg = readl(pcie->base + PCIE_DEV_REV_OFF);
+	reg &= ~0xffffff00;
+	reg |= (PCI_CLASS_BRIDGE_PCI << 8) << 8;
+	writel(reg, pcie->base + PCIE_DEV_REV_OFF);
 
-	mvebu_pcie_set_local_bus_nr(pcie, bus);
-	mvebu_pcie_set_local_dev_nr(pcie, pcie->local_dev);
+	/*
+	 * mvebu uses local bus number and local device number to determinate
+	 * type of config request. Type 0 is used if target bus number equals
+	 * local bus number and target device number differs from local device
+	 * number. Type 1 is used if target bus number differs from local bus
+	 * number. And when target bus number equals local bus number and
+	 * target device equals local device number then request is routed to
+	 * PCI Bridge which represent local PCIe Root Port.
+	 *
+	 * It means that PCI primary and secondary buses shares one bus number
+	 * which is configured via local bus number. Determination if config
+	 * request should go to primary or secondary bus is done based on local
+	 * device number.
+	 *
+	 * PCIe is point-to-point bus, so at secondary bus is always exactly one
+	 * device with number 0. So set local device number to 1, it would not
+	 * conflict with any device on secondary bus number and will ensure that
+	 * accessing secondary bus and all buses behind secondary would work
+	 * automatically and correctly. Therefore this configuration of local
+	 * device number implies that setting of local bus number configures
+	 * secondary bus number. Set it to 0 as U-Boot CONFIG_PCI_PNP code will
+	 * later configure it via config write requests to the correct value.
+	 * mvebu_pcie_write_config() catches config write requests which tries
+	 * to change primary/secondary bus number and correctly updates local
+	 * bus number based on new secondary bus number.
+	 *
+	 * With this configuration is PCI Bridge available at secondary bus as
+	 * device number 1. But it must be available at primary bus as device
+	 * number 0. So in mvebu_pcie_read_config() and mvebu_pcie_write_config()
+	 * functions rewrite address to the real one when accessing primary bus.
+	 */
+	mvebu_pcie_set_local_bus_nr(pcie, 0);
+	mvebu_pcie_set_local_dev_nr(pcie, 1);
 
 	pcie->mem.start = (u32)mvebu_pcie_membase;
 	pcie->mem.end = pcie->mem.start + PCIE_MEM_SIZE - 1;
@@ -366,6 +489,12 @@ static int mvebu_pcie_probe(struct udevice *dev)
 	writel(SOC_REGS_PHY_BASE, pcie->base + PCIE_BAR_LO_OFF(0));
 	writel(0, pcie->base + PCIE_BAR_HI_OFF(0));
 
+	/* 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 0;
 }
 
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 4/8] pci: pci_mvebu: Remove unused functions
  2021-10-22 14:22 [PATCH 0/8] pci: pci_mvebu: Fix access to config space and PCIe Root Port Pali Rohár
                   ` (2 preceding siblings ...)
  2021-10-22 14:22 ` [PATCH 3/8] pci: pci_mvebu: Properly configure and use PCI Bridge (PCIe Root Port) Pali Rohár
@ 2021-10-22 14:22 ` Pali Rohár
  2021-11-02 10:47   ` Stefan Roese
  2021-10-22 14:22 ` [PATCH 5/8] pci: pci_mvebu: Fix place of link up detection Pali Rohár
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 18+ messages in thread
From: Pali Rohár @ 2021-10-22 14:22 UTC (permalink / raw)
  To: Stefan Roese; +Cc: Marek Behún, u-boot

Functions mvebu_pcie_get_local_bus_nr() and mvebu_pcie_get_local_dev_nr()
are not used, so remove them.

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

diff --git a/drivers/pci/pci_mvebu.c b/drivers/pci/pci_mvebu.c
index 244dcc8fb050..4b8e56f22dfa 100644
--- a/drivers/pci/pci_mvebu.c
+++ b/drivers/pci/pci_mvebu.c
@@ -126,22 +126,6 @@ static void mvebu_pcie_set_local_dev_nr(struct mvebu_pcie *pcie, int devno)
 	writel(stat, pcie->base + PCIE_STAT_OFF);
 }
 
-static int mvebu_pcie_get_local_bus_nr(struct mvebu_pcie *pcie)
-{
-	u32 stat;
-
-	stat = readl(pcie->base + PCIE_STAT_OFF);
-	return (stat & PCIE_STAT_BUS) >> 8;
-}
-
-static int mvebu_pcie_get_local_dev_nr(struct mvebu_pcie *pcie)
-{
-	u32 stat;
-
-	stat = readl(pcie->base + PCIE_STAT_OFF);
-	return (stat & PCIE_STAT_DEV) >> 16;
-}
-
 static inline struct mvebu_pcie *hose_to_pcie(struct pci_controller *hose)
 {
 	return container_of(hose, struct mvebu_pcie, hose);
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 5/8] pci: pci_mvebu: Fix place of link up detection
  2021-10-22 14:22 [PATCH 0/8] pci: pci_mvebu: Fix access to config space and PCIe Root Port Pali Rohár
                   ` (3 preceding siblings ...)
  2021-10-22 14:22 ` [PATCH 4/8] pci: pci_mvebu: Remove unused functions Pali Rohár
@ 2021-10-22 14:22 ` Pali Rohár
  2021-11-02 10:50   ` Stefan Roese
  2021-10-22 14:22 ` [PATCH 6/8] pci: pci_mvebu: Do not automatically enable bus mastering on PCI Bridge Pali Rohár
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 18+ messages in thread
From: Pali Rohár @ 2021-10-22 14:22 UTC (permalink / raw)
  To: Stefan Roese; +Cc: Marek Behún, u-boot

PCI Bridge is always accessible also when link is down. So move detection
of link up from mvebu_pcie_of_to_plat() function to mvebu_pcie_valid_addr()
function which is used when accessing PCI config space.

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

diff --git a/drivers/pci/pci_mvebu.c b/drivers/pci/pci_mvebu.c
index 4b8e56f22dfa..40b8a57bbe1e 100644
--- a/drivers/pci/pci_mvebu.c
+++ b/drivers/pci/pci_mvebu.c
@@ -138,6 +138,10 @@ static bool mvebu_pcie_valid_addr(struct mvebu_pcie *pcie,
 	if (busno == pcie->first_busno && (dev != 0 || func != 0))
 		return false;
 
+	/* Access to other buses is possible when link is up */
+	if (busno != pcie->first_busno && !mvebu_pcie_link_up(pcie))
+		return false;
+
 	/* On secondary bus can be only one PCIe device */
 	if (busno == pcie->sec_busno && dev != 0)
 		return false;
@@ -369,9 +373,6 @@ static int mvebu_pcie_probe(struct udevice *dev)
 	struct pci_controller *hose = dev_get_uclass_priv(ctlr);
 	u32 reg;
 
-	debug("%s: PCIe %d.%d - up, base %08x\n", __func__,
-	      pcie->port, pcie->lane, (u32)pcie->base);
-
 	/*
 	 * Change Class Code of PCI Bridge device to PCI Bridge (0x600400)
 	 * because default value is Memory controller (0x508000) which
@@ -603,13 +604,6 @@ static int mvebu_pcie_of_to_plat(struct udevice *dev)
 	if (ret < 0)
 		goto err;
 
-	/* Check link and skip ports that have no link */
-	if (!mvebu_pcie_link_up(pcie)) {
-		debug("%s: %s - down\n", __func__, pcie->name);
-		ret = -ENODEV;
-		goto err;
-	}
-
 	return 0;
 
 err:
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 6/8] pci: pci_mvebu: Do not automatically enable bus mastering on PCI Bridge
  2021-10-22 14:22 [PATCH 0/8] pci: pci_mvebu: Fix access to config space and PCIe Root Port Pali Rohár
                   ` (4 preceding siblings ...)
  2021-10-22 14:22 ` [PATCH 5/8] pci: pci_mvebu: Fix place of link up detection Pali Rohár
@ 2021-10-22 14:22 ` Pali Rohár
  2021-11-02 10:50   ` Stefan Roese
  2021-10-22 14:22 ` [PATCH 7/8] pci: pci_mvebu: Setup PCI controller to Root Complex mode Pali Rohár
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 18+ messages in thread
From: Pali Rohár @ 2021-10-22 14:22 UTC (permalink / raw)
  To: Stefan Roese; +Cc: Marek Behún, u-boot

Now that PCI Bridge is working, U-Boot's CONFIG_PCI_PNP code automatically
enables memory access and bus mastering when it is needed. So do not
prematurely enable memory access and bus mastering.

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

diff --git a/drivers/pci/pci_mvebu.c b/drivers/pci/pci_mvebu.c
index 40b8a57bbe1e..e43fa12d3819 100644
--- a/drivers/pci/pci_mvebu.c
+++ b/drivers/pci/pci_mvebu.c
@@ -451,14 +451,6 @@ static int mvebu_pcie_probe(struct udevice *dev)
 	/* Setup windows and configure host bridge */
 	mvebu_pcie_setup_wins(pcie);
 
-	/* Master + slave enable. */
-	reg = readl(pcie->base + PCIE_CMD_OFF);
-	reg |= PCI_COMMAND_MEMORY;
-	reg |= PCI_COMMAND_IO;
-	reg |= PCI_COMMAND_MASTER;
-	reg |= BIT(10);		/* disable interrupts */
-	writel(reg, pcie->base + PCIE_CMD_OFF);
-
 	/* PCI memory space */
 	pci_set_region(hose->regions + 0, pcie->mem.start,
 		       pcie->mem.start, PCIE_MEM_SIZE, PCI_REGION_MEM);
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 7/8] pci: pci_mvebu: Setup PCI controller to Root Complex mode
  2021-10-22 14:22 [PATCH 0/8] pci: pci_mvebu: Fix access to config space and PCIe Root Port Pali Rohár
                   ` (5 preceding siblings ...)
  2021-10-22 14:22 ` [PATCH 6/8] pci: pci_mvebu: Do not automatically enable bus mastering on PCI Bridge Pali Rohár
@ 2021-10-22 14:22 ` Pali Rohár
  2021-11-02 10:50   ` Stefan Roese
  2021-10-22 14:22 ` [PATCH 8/8] pci: pci_mvebu: Fix comment about driver class name Pali Rohár
  2021-11-03  7:46 ` [PATCH 0/8] pci: pci_mvebu: Fix access to config space and PCIe Root Port Stefan Roese
  8 siblings, 1 reply; 18+ messages in thread
From: Pali Rohár @ 2021-10-22 14:22 UTC (permalink / raw)
  To: Stefan Roese; +Cc: Marek Behún, u-boot

Root Complex should be the default mode, let's set it explicitly.

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

diff --git a/drivers/pci/pci_mvebu.c b/drivers/pci/pci_mvebu.c
index e43fa12d3819..b0c673d8c472 100644
--- a/drivers/pci/pci_mvebu.c
+++ b/drivers/pci/pci_mvebu.c
@@ -62,6 +62,7 @@ DECLARE_GLOBAL_DATA_PTR;
 #define  PCIE_MASK_ENABLE_INTS          (0xf << 24)
 #define PCIE_CTRL_OFF			0x1a00
 #define  PCIE_CTRL_X1_MODE		BIT(0)
+#define  PCIE_CTRL_RC_MODE		BIT(1)
 #define PCIE_STAT_OFF			0x1a04
 #define  PCIE_STAT_BUS                  (0xff << 8)
 #define  PCIE_STAT_DEV                  (0x1f << 16)
@@ -373,6 +374,11 @@ static int mvebu_pcie_probe(struct udevice *dev)
 	struct pci_controller *hose = dev_get_uclass_priv(ctlr);
 	u32 reg;
 
+	/* Setup PCIe controller to Root Complex mode */
+	reg = readl(pcie->base + PCIE_CTRL_OFF);
+	reg |= PCIE_CTRL_RC_MODE;
+	writel(reg, pcie->base + PCIE_CTRL_OFF);
+
 	/*
 	 * Change Class Code of PCI Bridge device to PCI Bridge (0x600400)
 	 * because default value is Memory controller (0x508000) which
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 8/8] pci: pci_mvebu: Fix comment about driver class name
  2021-10-22 14:22 [PATCH 0/8] pci: pci_mvebu: Fix access to config space and PCIe Root Port Pali Rohár
                   ` (6 preceding siblings ...)
  2021-10-22 14:22 ` [PATCH 7/8] pci: pci_mvebu: Setup PCI controller to Root Complex mode Pali Rohár
@ 2021-10-22 14:22 ` Pali Rohár
  2021-11-02 10:50   ` Stefan Roese
  2021-11-03  7:46 ` [PATCH 0/8] pci: pci_mvebu: Fix access to config space and PCIe Root Port Stefan Roese
  8 siblings, 1 reply; 18+ messages in thread
From: Pali Rohár @ 2021-10-22 14:22 UTC (permalink / raw)
  To: Stefan Roese; +Cc: Marek Behún, u-boot

This is a pci driver, not an eth driver.

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

diff --git a/drivers/pci/pci_mvebu.c b/drivers/pci/pci_mvebu.c
index b0c673d8c472..ed151a05a4bf 100644
--- a/drivers/pci/pci_mvebu.c
+++ b/drivers/pci/pci_mvebu.c
@@ -633,7 +633,7 @@ static int mvebu_pcie_bind(struct udevice *parent)
 	struct udevice *dev;
 	ofnode subnode;
 
-	/* Lookup eth driver */
+	/* Lookup pci driver */
 	drv = lists_uclass_lookup(UCLASS_PCI);
 	if (!drv) {
 		puts("Cannot find PCI driver\n");
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/8] pci: pci_mvebu: Fix write_config() with PCI_SIZE_8 or PCI_SIZE_16
  2021-10-22 14:22 ` [PATCH 1/8] pci: pci_mvebu: Fix write_config() with PCI_SIZE_8 or PCI_SIZE_16 Pali Rohár
@ 2021-11-02 10:44   ` Stefan Roese
  0 siblings, 0 replies; 18+ messages in thread
From: Stefan Roese @ 2021-11-02 10:44 UTC (permalink / raw)
  To: Pali Rohár; +Cc: Marek Behún, u-boot

On 22.10.21 16:22, Pali Rohár wrote:
> Current implementation of write_config() is broken for PCI_SIZE_8 or
> PCI_SIZE_16 as it always uses writel(), which means that write operation
> is always 32-bit, so upper 24 bits for PCI_SIZE_8 and upper 16 bits for
> PCI_SIZE_16 are cleared.
> 
> Fix this by using writeb() and writew(), respectively.
> 
> 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_mvebu.c | 15 +++++++++++++--
>   1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/pci_mvebu.c b/drivers/pci/pci_mvebu.c
> index 0c1d7cd770f1..8175511514ab 100644
> --- a/drivers/pci/pci_mvebu.c
> +++ b/drivers/pci/pci_mvebu.c
> @@ -211,8 +211,19 @@ static int mvebu_pcie_write_config(struct udevice *bus, pci_dev_t bdf,
>   	writel(PCIE_CONF_ADDR(bdf, offset), pcie->base + PCIE_CONF_ADDR_OFF);
>   
>   	/* write data */
> -	data = pci_conv_size_to_32(0, value, offset, size);
> -	writel(data, pcie->base + PCIE_CONF_DATA_OFF);
> +	switch (size) {
> +	case PCI_SIZE_8:
> +		writeb(value, pcie->base + PCIE_CONF_DATA_OFF + (offset & 3));
> +		break;
> +	case PCI_SIZE_16:
> +		writew(value, pcie->base + PCIE_CONF_DATA_OFF + (offset & 2));
> +		break;
> +	case PCI_SIZE_32:
> +		writel(value, pcie->base + PCIE_CONF_DATA_OFF);
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
>   
>   	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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/8] pci: pci_mvebu: Fix read_config() with PCI_SIZE_8 or PCI_SIZE_16
  2021-10-22 14:22 ` [PATCH 2/8] pci: pci_mvebu: Fix read_config() " Pali Rohár
@ 2021-11-02 10:45   ` Stefan Roese
  0 siblings, 0 replies; 18+ messages in thread
From: Stefan Roese @ 2021-11-02 10:45 UTC (permalink / raw)
  To: Pali Rohár; +Cc: Marek Behún, u-boot

On 22.10.21 16:22, Pali Rohár wrote:
> When reading 8 or 16 bits from config space, use appropriate readb() or
> readw() calls. This ensures that PCIe controller does not read more bits
> from endpoint card as asked by read_config() function.
> 
> Technically there should not be an issue with reading data from config
> space which are not later used as there are no clear-by-read registers.
> But it is better to use correct read operation based on requested size.
> 
> 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_mvebu.c | 17 +++++++++++++++--
>   1 file changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/pci_mvebu.c b/drivers/pci/pci_mvebu.c
> index 8175511514ab..3991086e0d29 100644
> --- a/drivers/pci/pci_mvebu.c
> +++ b/drivers/pci/pci_mvebu.c
> @@ -184,9 +184,22 @@ static int mvebu_pcie_read_config(const struct udevice *bus, pci_dev_t bdf,
>   	writel(PCIE_CONF_ADDR(bdf, offset), pcie->base + PCIE_CONF_ADDR_OFF);
>   
>   	/* read data */
> -	data = readl(pcie->base + PCIE_CONF_DATA_OFF);
> +	switch (size) {
> +	case PCI_SIZE_8:
> +		data = readb(pcie->base + PCIE_CONF_DATA_OFF + (offset & 3));
> +		break;
> +	case PCI_SIZE_16:
> +		data = readw(pcie->base + PCIE_CONF_DATA_OFF + (offset & 2));
> +		break;
> +	case PCI_SIZE_32:
> +		data = readl(pcie->base + PCIE_CONF_DATA_OFF);
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
>   	debug("(addr,size,val)=(0x%04x, %d, 0x%08x)\n", offset, size, data);
> -	*valuep = pci_conv_32_to_size(data, offset, size);
> +	*valuep = data;
>   
>   	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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/8] pci: pci_mvebu: Properly configure and use PCI Bridge (PCIe Root Port)
  2021-10-22 14:22 ` [PATCH 3/8] pci: pci_mvebu: Properly configure and use PCI Bridge (PCIe Root Port) Pali Rohár
@ 2021-11-02 10:47   ` Stefan Roese
  0 siblings, 0 replies; 18+ messages in thread
From: Stefan Roese @ 2021-11-02 10:47 UTC (permalink / raw)
  To: Pali Rohár; +Cc: Marek Behún, u-boot

On 22.10.21 16:22, Pali Rohár wrote:
> The mysterious "Memory controller" PCI device which is present in PCI
> config space is improperly configured and crippled PCI Bridge which acts
> as PCIe Root Port for endpoint PCIe card.
> 
> This PCI Bridge reports in PCI config space incorrect Class Code (Memory
> Controller) and incorrect Header Type (Type 0). It looks like HW bug in
> mvebu PCIe controller but apparently it can be changed via mvebu registers
> to correct values.
> 
> The worst thing is that this PCI Bridge is crippled and its PCI config
> registers in range 0x10-0x34 alias access to internal mvebu registers which
> have different functionality as PCI Bridge registers. Moreover,
> configuration of PCI primary and secondary bus numbers (registers 0x18
> and 0x19) is done via totally different mvebu registers via totally strange
> method and cannot be done via PCI Bridge config space.
> 
> Due to above fact about PCI config range 0x10-0x34, allocate a private
> cfgcache[] buffer in the driver, to which PCI config access requests to
> the 0x10-0x34 space will be redirected in mvebu_pcie_read_config() and
> mvebu_pcie_write_config() functions. Function mvebu_pcie_write_config()
> will also catch writes to PCI_PRIMARY_BUS (0x18) and PCI_SECONDARY_BUS
> (0x19) registers and set PCI Bridge primary and secondary bus numbers via
> mvebu's own method.
> 
> Also, Expansion ROM Base Address register (0x38) is available, but at
> different offset 0x30. So recalculate register offset before accessing PCI
> config space.
> 
> After these steps U-Boot sees working PCI Bridge and CONFIG_PCI_PNP code
> can finally start enumerating all PCIe devices correctly, even with more
> complicated PCI topology. So update also mvebu_pcie_valid_addr() function
> to reflect state of the real device topology.
> 
> Each PCIe port is de-facto isolated and every PCI Bridge which is part of
> PCIe Root Complex is also isolated, so put them on separate PCI buses as
> (local) device 0.
> 
> U-Boot already supports enumerating separate PCI buses, real (HW) bus
> number can be retrieved by "PCI_BUS(bdf) - dev_seq(bus)" code, so update
> config read/write functions to properly handle more complicated tree
> topologies (e.g. when a PCIe switch with multiple PCI buses is connected
> to the PCIe port).
> 
> Local bus number and local device number on mvebu are used for determining
> which config request type is used (Type 0 vs Type 1). On normal non-broken
> PCIe hardware it is done by primary and secondary bus numbers. So correctly
> translate settings between these numbers to ensure that correct config
> requests are sent over the PCIe bus.
> 
> As bus numbers are correctly re-configured, it does not make sense to print
> some initial bogus configuration during probe, so remove this debug code.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>
> Reviewed-by: Marek Behún <marek.behun@nic.cz>

Many thanks for this huge work here. Especially with the extensive
documentation.

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

Thanks,
Stefan

> ---
>   drivers/pci/pci_mvebu.c | 199 +++++++++++++++++++++++++++++++++-------
>   1 file changed, 164 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/pci/pci_mvebu.c b/drivers/pci/pci_mvebu.c
> index 3991086e0d29..244dcc8fb050 100644
> --- a/drivers/pci/pci_mvebu.c
> +++ b/drivers/pci/pci_mvebu.c
> @@ -36,6 +36,7 @@ DECLARE_GLOBAL_DATA_PTR;
>   #define PCIE_DEV_REV_OFF		0x0008
>   #define  PCIE_BAR_LO_OFF(n)		(0x0010 + ((n) << 3))
>   #define  PCIE_BAR_HI_OFF(n)		(0x0014 + ((n) << 3))
> +#define PCIE_EXP_ROM_BAR_OFF		0x0030
>   #define PCIE_CAPAB_OFF			0x0060
>   #define PCIE_CTRL_STAT_OFF		0x0068
>   #define PCIE_HEADER_LOG_4_OFF		0x0128
> @@ -52,9 +53,9 @@ DECLARE_GLOBAL_DATA_PTR;
>   #define  PCIE_CONF_BUS(b)		(((b) & 0xff) << 16)
>   #define  PCIE_CONF_DEV(d)		(((d) & 0x1f) << 11)
>   #define  PCIE_CONF_FUNC(f)		(((f) & 0x7) << 8)
> -#define  PCIE_CONF_ADDR(dev, reg) \
> -	(PCIE_CONF_BUS(PCI_BUS(dev)) | PCIE_CONF_DEV(PCI_DEV(dev))    | \
> -	 PCIE_CONF_FUNC(PCI_FUNC(dev)) | PCIE_CONF_REG(reg) | \
> +#define  PCIE_CONF_ADDR(b, d, f, reg) \
> +	(PCIE_CONF_BUS(b) | PCIE_CONF_DEV(d)    | \
> +	 PCIE_CONF_FUNC(f) | PCIE_CONF_REG(reg) | \
>   	 PCIE_CONF_ADDR_EN)
>   #define PCIE_CONF_DATA_OFF		0x18fc
>   #define PCIE_MASK_OFF			0x1910
> @@ -80,12 +81,13 @@ struct mvebu_pcie {
>   	int devfn;
>   	u32 lane_mask;
>   	int first_busno;
> -	int local_dev;
> +	int sec_busno;
>   	char name[16];
>   	unsigned int mem_target;
>   	unsigned int mem_attr;
>   	unsigned int io_target;
>   	unsigned int io_attr;
> +	u32 cfgcache[0x34 - 0x10];
>   };
>   
>   /*
> @@ -145,23 +147,18 @@ static inline struct mvebu_pcie *hose_to_pcie(struct pci_controller *hose)
>   	return container_of(hose, struct mvebu_pcie, hose);
>   }
>   
> -static int mvebu_pcie_valid_addr(struct mvebu_pcie *pcie, pci_dev_t bdf)
> +static bool mvebu_pcie_valid_addr(struct mvebu_pcie *pcie,
> +				  int busno, int dev, int func)
>   {
> -	/*
> -	 * There are two devices visible on local bus:
> -	 *   * on slot configured by function mvebu_pcie_set_local_dev_nr()
> -	 *     (by default this register is set to 0) there is a
> -	 *     "Marvell Memory controller", which isn't useful in root complex
> -	 *     mode,
> -	 *   * on all other slots the real PCIe card connected to the PCIe slot.
> -	 *
> -	 * We therefore allow access only to the real PCIe card.
> -	 */
> -	if (PCI_BUS(bdf) == pcie->first_busno &&
> -	    PCI_DEV(bdf) != !pcie->local_dev)
> -		return 0;
> +	/* On primary bus is only one PCI Bridge */
> +	if (busno == pcie->first_busno && (dev != 0 || func != 0))
> +		return false;
> +
> +	/* On secondary bus can be only one PCIe device */
> +	if (busno == pcie->sec_busno && dev != 0)
> +		return false;
>   
> -	return 1;
> +	return true;
>   }
>   
>   static int mvebu_pcie_read_config(const struct udevice *bus, pci_dev_t bdf,
> @@ -169,19 +166,46 @@ static int mvebu_pcie_read_config(const struct udevice *bus, pci_dev_t bdf,
>   				  enum pci_size_t size)
>   {
>   	struct mvebu_pcie *pcie = dev_get_plat(bus);
> -	u32 data;
> +	int busno = PCI_BUS(bdf) - dev_seq(bus);
> +	u32 addr, data;
>   
>   	debug("PCIE CFG read: (b,d,f)=(%2d,%2d,%2d) ",
>   	      PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf));
>   
> -	if (!mvebu_pcie_valid_addr(pcie, bdf)) {
> +	if (!mvebu_pcie_valid_addr(pcie, busno, PCI_DEV(bdf), PCI_FUNC(bdf))) {
>   		debug("- out of range\n");
>   		*valuep = pci_get_ff(size);
>   		return 0;
>   	}
>   
> +	/*
> +	 * mvebu has different internal registers mapped into PCI config space
> +	 * in range 0x10-0x34 for PCI bridge, so do not access PCI config space
> +	 * for this range and instead read content from driver virtual cfgcache
> +	 */
> +	if (busno == pcie->first_busno && offset >= 0x10 && offset < 0x34) {
> +		data = pcie->cfgcache[(offset - 0x10) / 4];
> +		debug("(addr,size,val)=(0x%04x, %d, 0x%08x) from cfgcache\n",
> +		      offset, size, data);
> +		*valuep = pci_conv_32_to_size(data, offset, size);
> +		return 0;
> +	} else if (busno == pcie->first_busno &&
> +		   (offset & ~3) == PCI_ROM_ADDRESS1) {
> +		/* mvebu has Expansion ROM Base Address (0x38) at offset 0x30 */
> +		offset -= PCI_ROM_ADDRESS1 - PCIE_EXP_ROM_BAR_OFF;
> +	}
> +
> +	/*
> +	 * PCI bridge is device 0 at primary bus but mvebu has it mapped on
> +	 * secondary bus with device number 1.
> +	 */
> +	if (busno == pcie->first_busno)
> +		addr = PCIE_CONF_ADDR(pcie->sec_busno, 1, 0, offset);
> +	else
> +		addr = PCIE_CONF_ADDR(busno, PCI_DEV(bdf), PCI_FUNC(bdf), offset);
> +
>   	/* write address */
> -	writel(PCIE_CONF_ADDR(bdf, offset), pcie->base + PCIE_CONF_ADDR_OFF);
> +	writel(addr, pcie->base + PCIE_CONF_ADDR_OFF);
>   
>   	/* read data */
>   	switch (size) {
> @@ -198,6 +222,19 @@ static int mvebu_pcie_read_config(const struct udevice *bus, pci_dev_t bdf,
>   		return -EINVAL;
>   	}
>   
> +	if (busno == pcie->first_busno &&
> +	    (offset & ~3) == (PCI_HEADER_TYPE & ~3)) {
> +		/*
> +		 * Change Header Type of PCI Bridge device to Type 1
> +		 * (0x01, used by PCI Bridges) because mvebu reports
> +		 * Type 0 (0x00, used by Upstream and Endpoint devices).
> +		 */
> +		data = pci_conv_size_to_32(data, 0, offset, size);
> +		data &= ~0x007f0000;
> +		data |= PCI_HEADER_TYPE_BRIDGE << 16;
> +		data = pci_conv_32_to_size(data, offset, size);
> +	}
> +
>   	debug("(addr,size,val)=(0x%04x, %d, 0x%08x)\n", offset, size, data);
>   	*valuep = data;
>   
> @@ -209,19 +246,64 @@ static int mvebu_pcie_write_config(struct udevice *bus, pci_dev_t bdf,
>   				   enum pci_size_t size)
>   {
>   	struct mvebu_pcie *pcie = dev_get_plat(bus);
> -	u32 data;
> +	int busno = PCI_BUS(bdf) - dev_seq(bus);
> +	u32 addr, data;
>   
>   	debug("PCIE CFG write: (b,d,f)=(%2d,%2d,%2d) ",
>   	      PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf));
>   	debug("(addr,size,val)=(0x%04x, %d, 0x%08lx)\n", offset, size, value);
>   
> -	if (!mvebu_pcie_valid_addr(pcie, bdf)) {
> +	if (!mvebu_pcie_valid_addr(pcie, busno, PCI_DEV(bdf), PCI_FUNC(bdf))) {
>   		debug("- out of range\n");
>   		return 0;
>   	}
>   
> +	/*
> +	 * mvebu has different internal registers mapped into PCI config space
> +	 * in range 0x10-0x34 for PCI bridge, so do not access PCI config space
> +	 * for this range and instead write content to driver virtual cfgcache
> +	 */
> +	if (busno == pcie->first_busno && offset >= 0x10 && offset < 0x34) {
> +		debug("Writing to cfgcache only\n");
> +		data = pcie->cfgcache[(offset - 0x10) / 4];
> +		data = pci_conv_size_to_32(data, value, offset, size);
> +		/* mvebu PCI bridge does not have configurable bars */
> +		if ((offset & ~3) == PCI_BASE_ADDRESS_0 ||
> +		    (offset & ~3) == PCI_BASE_ADDRESS_1)
> +			data = 0x0;
> +		pcie->cfgcache[(offset - 0x10) / 4] = data;
> +		/* mvebu has its own way how to set PCI primary bus number */
> +		if (offset == PCI_PRIMARY_BUS) {
> +			pcie->first_busno = data & 0xff;
> +			debug("Primary bus number was changed to %d\n",
> +			      pcie->first_busno);
> +		}
> +		/* mvebu has its own way how to set PCI secondary bus number */
> +		if (offset == PCI_SECONDARY_BUS ||
> +		    (offset == PCI_PRIMARY_BUS && size != PCI_SIZE_8)) {
> +			pcie->sec_busno = (data >> 8) & 0xff;
> +			mvebu_pcie_set_local_bus_nr(pcie, pcie->sec_busno);
> +			debug("Secondary bus number was changed to %d\n",
> +			      pcie->sec_busno);
> +		}
> +		return 0;
> +	} else if (busno == pcie->first_busno &&
> +		   (offset & ~3) == PCI_ROM_ADDRESS1) {
> +		/* mvebu has Expansion ROM Base Address (0x38) at offset 0x30 */
> +		offset -= PCI_ROM_ADDRESS1 - PCIE_EXP_ROM_BAR_OFF;
> +	}
> +
> +	/*
> +	 * PCI bridge is device 0 at primary bus but mvebu has it mapped on
> +	 * secondary bus with device number 1.
> +	 */
> +	if (busno == pcie->first_busno)
> +		addr = PCIE_CONF_ADDR(pcie->sec_busno, 1, 0, offset);
> +	else
> +		addr = PCIE_CONF_ADDR(busno, PCI_DEV(bdf), PCI_FUNC(bdf), offset);
> +
>   	/* write address */
> -	writel(PCIE_CONF_ADDR(bdf, offset), pcie->base + PCIE_CONF_ADDR_OFF);
> +	writel(addr, pcie->base + PCIE_CONF_ADDR_OFF);
>   
>   	/* write data */
>   	switch (size) {
> @@ -301,22 +383,63 @@ static int mvebu_pcie_probe(struct udevice *dev)
>   	struct mvebu_pcie *pcie = dev_get_plat(dev);
>   	struct udevice *ctlr = pci_get_controller(dev);
>   	struct pci_controller *hose = dev_get_uclass_priv(ctlr);
> -	int bus = dev_seq(dev);
>   	u32 reg;
>   
>   	debug("%s: PCIe %d.%d - up, base %08x\n", __func__,
>   	      pcie->port, pcie->lane, (u32)pcie->base);
>   
> -	/* Read Id info and local bus/dev */
> -	debug("direct conf read %08x, local bus %d, local dev %d\n",
> -	      readl(pcie->base), mvebu_pcie_get_local_bus_nr(pcie),
> -	      mvebu_pcie_get_local_dev_nr(pcie));
> -
> -	pcie->first_busno = bus;
> -	pcie->local_dev = 1;
> +	/*
> +	 * Change Class Code of PCI Bridge device to PCI Bridge (0x600400)
> +	 * because default value is Memory controller (0x508000) which
> +	 * U-Boot cannot recognize as P2P Bridge.
> +	 *
> +	 * Note that this mvebu PCI Bridge does not have compliant Type 1
> +	 * Configuration Space. Header Type is reported as Type 0 and in
> +	 * range 0x10-0x34 it has aliased internal mvebu registers 0x10-0x34
> +	 * (e.g. PCIE_BAR_LO_OFF) and register 0x38 is reserved.
> +	 *
> +	 * Driver for this range redirects access to virtual cfgcache[] buffer
> +	 * which avoids changing internal mvebu registers. And changes Header
> +	 * Type response value to Type 1.
> +	 */
> +	reg = readl(pcie->base + PCIE_DEV_REV_OFF);
> +	reg &= ~0xffffff00;
> +	reg |= (PCI_CLASS_BRIDGE_PCI << 8) << 8;
> +	writel(reg, pcie->base + PCIE_DEV_REV_OFF);
>   
> -	mvebu_pcie_set_local_bus_nr(pcie, bus);
> -	mvebu_pcie_set_local_dev_nr(pcie, pcie->local_dev);
> +	/*
> +	 * mvebu uses local bus number and local device number to determinate
> +	 * type of config request. Type 0 is used if target bus number equals
> +	 * local bus number and target device number differs from local device
> +	 * number. Type 1 is used if target bus number differs from local bus
> +	 * number. And when target bus number equals local bus number and
> +	 * target device equals local device number then request is routed to
> +	 * PCI Bridge which represent local PCIe Root Port.
> +	 *
> +	 * It means that PCI primary and secondary buses shares one bus number
> +	 * which is configured via local bus number. Determination if config
> +	 * request should go to primary or secondary bus is done based on local
> +	 * device number.
> +	 *
> +	 * PCIe is point-to-point bus, so at secondary bus is always exactly one
> +	 * device with number 0. So set local device number to 1, it would not
> +	 * conflict with any device on secondary bus number and will ensure that
> +	 * accessing secondary bus and all buses behind secondary would work
> +	 * automatically and correctly. Therefore this configuration of local
> +	 * device number implies that setting of local bus number configures
> +	 * secondary bus number. Set it to 0 as U-Boot CONFIG_PCI_PNP code will
> +	 * later configure it via config write requests to the correct value.
> +	 * mvebu_pcie_write_config() catches config write requests which tries
> +	 * to change primary/secondary bus number and correctly updates local
> +	 * bus number based on new secondary bus number.
> +	 *
> +	 * With this configuration is PCI Bridge available at secondary bus as
> +	 * device number 1. But it must be available at primary bus as device
> +	 * number 0. So in mvebu_pcie_read_config() and mvebu_pcie_write_config()
> +	 * functions rewrite address to the real one when accessing primary bus.
> +	 */
> +	mvebu_pcie_set_local_bus_nr(pcie, 0);
> +	mvebu_pcie_set_local_dev_nr(pcie, 1);
>   
>   	pcie->mem.start = (u32)mvebu_pcie_membase;
>   	pcie->mem.end = pcie->mem.start + PCIE_MEM_SIZE - 1;
> @@ -366,6 +489,12 @@ static int mvebu_pcie_probe(struct udevice *dev)
>   	writel(SOC_REGS_PHY_BASE, pcie->base + PCIE_BAR_LO_OFF(0));
>   	writel(0, pcie->base + PCIE_BAR_HI_OFF(0));
>   
> +	/* 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 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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 4/8] pci: pci_mvebu: Remove unused functions
  2021-10-22 14:22 ` [PATCH 4/8] pci: pci_mvebu: Remove unused functions Pali Rohár
@ 2021-11-02 10:47   ` Stefan Roese
  0 siblings, 0 replies; 18+ messages in thread
From: Stefan Roese @ 2021-11-02 10:47 UTC (permalink / raw)
  To: Pali Rohár; +Cc: Marek Behún, u-boot

On 22.10.21 16:22, Pali Rohár wrote:
> Functions mvebu_pcie_get_local_bus_nr() and mvebu_pcie_get_local_dev_nr()
> are not used, so remove them.
> 
> 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_mvebu.c | 16 ----------------
>   1 file changed, 16 deletions(-)
> 
> diff --git a/drivers/pci/pci_mvebu.c b/drivers/pci/pci_mvebu.c
> index 244dcc8fb050..4b8e56f22dfa 100644
> --- a/drivers/pci/pci_mvebu.c
> +++ b/drivers/pci/pci_mvebu.c
> @@ -126,22 +126,6 @@ static void mvebu_pcie_set_local_dev_nr(struct mvebu_pcie *pcie, int devno)
>   	writel(stat, pcie->base + PCIE_STAT_OFF);
>   }
>   
> -static int mvebu_pcie_get_local_bus_nr(struct mvebu_pcie *pcie)
> -{
> -	u32 stat;
> -
> -	stat = readl(pcie->base + PCIE_STAT_OFF);
> -	return (stat & PCIE_STAT_BUS) >> 8;
> -}
> -
> -static int mvebu_pcie_get_local_dev_nr(struct mvebu_pcie *pcie)
> -{
> -	u32 stat;
> -
> -	stat = readl(pcie->base + PCIE_STAT_OFF);
> -	return (stat & PCIE_STAT_DEV) >> 16;
> -}
> -
>   static inline struct mvebu_pcie *hose_to_pcie(struct pci_controller *hose)
>   {
>   	return container_of(hose, struct mvebu_pcie, hose);
> 


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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 5/8] pci: pci_mvebu: Fix place of link up detection
  2021-10-22 14:22 ` [PATCH 5/8] pci: pci_mvebu: Fix place of link up detection Pali Rohár
@ 2021-11-02 10:50   ` Stefan Roese
  0 siblings, 0 replies; 18+ messages in thread
From: Stefan Roese @ 2021-11-02 10:50 UTC (permalink / raw)
  To: Pali Rohár; +Cc: Marek Behún, u-boot

On 22.10.21 16:22, Pali Rohár wrote:
> PCI Bridge is always accessible also when link is down. So move detection
> of link up from mvebu_pcie_of_to_plat() function to mvebu_pcie_valid_addr()
> function which is used when accessing PCI config space.
> 
> 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_mvebu.c | 14 ++++----------
>   1 file changed, 4 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/pci/pci_mvebu.c b/drivers/pci/pci_mvebu.c
> index 4b8e56f22dfa..40b8a57bbe1e 100644
> --- a/drivers/pci/pci_mvebu.c
> +++ b/drivers/pci/pci_mvebu.c
> @@ -138,6 +138,10 @@ static bool mvebu_pcie_valid_addr(struct mvebu_pcie *pcie,
>   	if (busno == pcie->first_busno && (dev != 0 || func != 0))
>   		return false;
>   
> +	/* Access to other buses is possible when link is up */
> +	if (busno != pcie->first_busno && !mvebu_pcie_link_up(pcie))
> +		return false;
> +
>   	/* On secondary bus can be only one PCIe device */
>   	if (busno == pcie->sec_busno && dev != 0)
>   		return false;
> @@ -369,9 +373,6 @@ static int mvebu_pcie_probe(struct udevice *dev)
>   	struct pci_controller *hose = dev_get_uclass_priv(ctlr);
>   	u32 reg;
>   
> -	debug("%s: PCIe %d.%d - up, base %08x\n", __func__,
> -	      pcie->port, pcie->lane, (u32)pcie->base);
> -
>   	/*
>   	 * Change Class Code of PCI Bridge device to PCI Bridge (0x600400)
>   	 * because default value is Memory controller (0x508000) which
> @@ -603,13 +604,6 @@ static int mvebu_pcie_of_to_plat(struct udevice *dev)
>   	if (ret < 0)
>   		goto err;
>   
> -	/* Check link and skip ports that have no link */
> -	if (!mvebu_pcie_link_up(pcie)) {
> -		debug("%s: %s - down\n", __func__, pcie->name);
> -		ret = -ENODEV;
> -		goto err;
> -	}
> -
>   	return 0;
>   
>   err:
> 


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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 6/8] pci: pci_mvebu: Do not automatically enable bus mastering on PCI Bridge
  2021-10-22 14:22 ` [PATCH 6/8] pci: pci_mvebu: Do not automatically enable bus mastering on PCI Bridge Pali Rohár
@ 2021-11-02 10:50   ` Stefan Roese
  0 siblings, 0 replies; 18+ messages in thread
From: Stefan Roese @ 2021-11-02 10:50 UTC (permalink / raw)
  To: Pali Rohár; +Cc: Marek Behún, u-boot

On 22.10.21 16:22, Pali Rohár wrote:
> Now that PCI Bridge is working, U-Boot's CONFIG_PCI_PNP code automatically
> enables memory access and bus mastering when it is needed. So do not
> prematurely enable memory access and bus mastering.
> 
> 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_mvebu.c | 8 --------
>   1 file changed, 8 deletions(-)
> 
> diff --git a/drivers/pci/pci_mvebu.c b/drivers/pci/pci_mvebu.c
> index 40b8a57bbe1e..e43fa12d3819 100644
> --- a/drivers/pci/pci_mvebu.c
> +++ b/drivers/pci/pci_mvebu.c
> @@ -451,14 +451,6 @@ static int mvebu_pcie_probe(struct udevice *dev)
>   	/* Setup windows and configure host bridge */
>   	mvebu_pcie_setup_wins(pcie);
>   
> -	/* Master + slave enable. */
> -	reg = readl(pcie->base + PCIE_CMD_OFF);
> -	reg |= PCI_COMMAND_MEMORY;
> -	reg |= PCI_COMMAND_IO;
> -	reg |= PCI_COMMAND_MASTER;
> -	reg |= BIT(10);		/* disable interrupts */
> -	writel(reg, pcie->base + PCIE_CMD_OFF);
> -
>   	/* PCI memory space */
>   	pci_set_region(hose->regions + 0, pcie->mem.start,
>   		       pcie->mem.start, PCIE_MEM_SIZE, PCI_REGION_MEM);
> 


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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 7/8] pci: pci_mvebu: Setup PCI controller to Root Complex mode
  2021-10-22 14:22 ` [PATCH 7/8] pci: pci_mvebu: Setup PCI controller to Root Complex mode Pali Rohár
@ 2021-11-02 10:50   ` Stefan Roese
  0 siblings, 0 replies; 18+ messages in thread
From: Stefan Roese @ 2021-11-02 10:50 UTC (permalink / raw)
  To: Pali Rohár; +Cc: Marek Behún, u-boot

On 22.10.21 16:22, Pali Rohár wrote:
> Root Complex should be the default mode, let's set it explicitly.
> 
> 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_mvebu.c | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/pci/pci_mvebu.c b/drivers/pci/pci_mvebu.c
> index e43fa12d3819..b0c673d8c472 100644
> --- a/drivers/pci/pci_mvebu.c
> +++ b/drivers/pci/pci_mvebu.c
> @@ -62,6 +62,7 @@ DECLARE_GLOBAL_DATA_PTR;
>   #define  PCIE_MASK_ENABLE_INTS          (0xf << 24)
>   #define PCIE_CTRL_OFF			0x1a00
>   #define  PCIE_CTRL_X1_MODE		BIT(0)
> +#define  PCIE_CTRL_RC_MODE		BIT(1)
>   #define PCIE_STAT_OFF			0x1a04
>   #define  PCIE_STAT_BUS                  (0xff << 8)
>   #define  PCIE_STAT_DEV                  (0x1f << 16)
> @@ -373,6 +374,11 @@ static int mvebu_pcie_probe(struct udevice *dev)
>   	struct pci_controller *hose = dev_get_uclass_priv(ctlr);
>   	u32 reg;
>   
> +	/* Setup PCIe controller to Root Complex mode */
> +	reg = readl(pcie->base + PCIE_CTRL_OFF);
> +	reg |= PCIE_CTRL_RC_MODE;
> +	writel(reg, pcie->base + PCIE_CTRL_OFF);
> +
>   	/*
>   	 * Change Class Code of PCI Bridge device to PCI Bridge (0x600400)
>   	 * because default value is Memory controller (0x508000) which
> 


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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 8/8] pci: pci_mvebu: Fix comment about driver class name
  2021-10-22 14:22 ` [PATCH 8/8] pci: pci_mvebu: Fix comment about driver class name Pali Rohár
@ 2021-11-02 10:50   ` Stefan Roese
  0 siblings, 0 replies; 18+ messages in thread
From: Stefan Roese @ 2021-11-02 10:50 UTC (permalink / raw)
  To: Pali Rohár; +Cc: Marek Behún, u-boot

On 22.10.21 16:22, Pali Rohár wrote:
> This is a pci driver, not an eth driver.
> 
> 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_mvebu.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/pci_mvebu.c b/drivers/pci/pci_mvebu.c
> index b0c673d8c472..ed151a05a4bf 100644
> --- a/drivers/pci/pci_mvebu.c
> +++ b/drivers/pci/pci_mvebu.c
> @@ -633,7 +633,7 @@ static int mvebu_pcie_bind(struct udevice *parent)
>   	struct udevice *dev;
>   	ofnode subnode;
>   
> -	/* Lookup eth driver */
> +	/* Lookup pci driver */
>   	drv = lists_uclass_lookup(UCLASS_PCI);
>   	if (!drv) {
>   		puts("Cannot find PCI driver\n");
> 


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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/8] pci: pci_mvebu: Fix access to config space and PCIe Root Port
  2021-10-22 14:22 [PATCH 0/8] pci: pci_mvebu: Fix access to config space and PCIe Root Port Pali Rohár
                   ` (7 preceding siblings ...)
  2021-10-22 14:22 ` [PATCH 8/8] pci: pci_mvebu: Fix comment about driver class name Pali Rohár
@ 2021-11-03  7:46 ` Stefan Roese
  8 siblings, 0 replies; 18+ messages in thread
From: Stefan Roese @ 2021-11-03  7:46 UTC (permalink / raw)
  To: Pali Rohár; +Cc: Marek Behún, u-boot

On 22.10.21 16:22, Pali Rohár wrote:
> This patch series fixes access to config space and fixes issues with the
> mysterious "Memory controller" PCI device (which is PCIe Root Port).
> 
> Tested on Armada 385 Turris Omnia device which has 3 mPCIe slots.
> PCIe Root Port device is available for each slot on separate bus and has
> PCI Bridge class code as required by PCIe base specifications.
> 
> => pci 0
> Scanning PCI devices on bus 0
> BusDevFun  VendorId   DeviceId   Device Class       Sub-Class
> _____________________________________________________________
> 00.00.00   0x11ab     0x6820     Bridge device           0x04
> => pci 1
> Scanning PCI devices on bus 1
> BusDevFun  VendorId   DeviceId   Device Class       Sub-Class
> _____________________________________________________________
> 01.00.00   0x168c     0x003c     Network controller      0x80
> => pci 2
> Scanning PCI devices on bus 2
> BusDevFun  VendorId   DeviceId   Device Class       Sub-Class
> _____________________________________________________________
> 02.00.00   0x11ab     0x6820     Bridge device           0x04
> => pci 3
> Scanning PCI devices on bus 3
> BusDevFun  VendorId   DeviceId   Device Class       Sub-Class
> _____________________________________________________________
> 03.00.00   0x168c     0x003c     Network controller      0x80
> => pci 4
> Scanning PCI devices on bus 4
> BusDevFun  VendorId   DeviceId   Device Class       Sub-Class
> _____________________________________________________________
> 04.00.00   0x11ab     0x6820     Bridge device           0x04
> => pci 5
> Scanning PCI devices on bus 5
> BusDevFun  VendorId   DeviceId   Device Class       Sub-Class
> _____________________________________________________________
> 05.00.00   0x168c     0x002e     Network controller      0x80
> => pci 6
> No such bus
> =>
> 
> U-Boot command "pci display.b 0.0.0 0 200" will display config space of
> the first PCIe Root Port device with the Type 1 PCI header as required by
> PCIe base specification.
> 
> => pci display.b 0.0.0 0 200
> 00000000: ab 11 20 68 07 00 10 00 04 00 04 06 00 00 01 00
> 00000010: 00 00 00 00 00 00 00 00 00 01 01 00 01 f1 00 00
> 00000020: 00 c0 20 c0 01 10 01 00 00 00 00 00 00 00 00 00
> 00000030: 10 f1 0f f1 40 00 00 00 00 00 00 00 00 01 00 00
> 00000040: 01 50 03 06 00 00 00 00 00 00 00 00 00 00 00 00
> 00000050: 05 60 80 00 00 00 00 00 00 00 00 00 00 00 00 00
> 00000060: 10 00 42 00 80 80 00 00 00 20 00 00 12 ac 07 00
> 00000070: 00 00 11 10 00 00 00 00 00 00 40 00 00 00 00 00
> 00000080: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 00000090: 02 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 000000a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 000000b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 000000c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 000000d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 000000e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 000000f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 00000100: 01 00 01 00 00 00 00 00 00 00 00 00 10 00 06 00
> 00000110: 00 00 00 00 00 20 00 00 00 00 00 00 01 00 00 4a
> 00000120: 04 00 00 01 00 01 08 01 02 80 00 00 00 00 00 00
> 00000130: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 00000140: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 00000150: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 00000160: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 00000170: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 00000180: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 00000190: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 000001a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 000001b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 000001c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 000001d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 000001e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 000001f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 
> Pali Rohár (8):
>    pci: pci_mvebu: Fix write_config() with PCI_SIZE_8 or PCI_SIZE_16
>    pci: pci_mvebu: Fix read_config() with PCI_SIZE_8 or PCI_SIZE_16
>    pci: pci_mvebu: Properly configure and use PCI Bridge (PCIe Root Port)
>    pci: pci_mvebu: Remove unused functions
>    pci: pci_mvebu: Fix place of link up detection
>    pci: pci_mvebu: Do not automatically enable bus mastering on PCI
>      Bridge
>    pci: pci_mvebu: Setup PCI controller to Root Complex mode
>    pci: pci_mvebu: Fix comment about driver class name
> 
>   drivers/pci/pci_mvebu.c | 275 +++++++++++++++++++++++++++++-----------
>   1 file changed, 202 insertions(+), 73 deletions(-)
> 

Applied to u-boot-marvell/master

Thanks,
Stefan

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2021-11-03  7:46 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-22 14:22 [PATCH 0/8] pci: pci_mvebu: Fix access to config space and PCIe Root Port Pali Rohár
2021-10-22 14:22 ` [PATCH 1/8] pci: pci_mvebu: Fix write_config() with PCI_SIZE_8 or PCI_SIZE_16 Pali Rohár
2021-11-02 10:44   ` Stefan Roese
2021-10-22 14:22 ` [PATCH 2/8] pci: pci_mvebu: Fix read_config() " Pali Rohár
2021-11-02 10:45   ` Stefan Roese
2021-10-22 14:22 ` [PATCH 3/8] pci: pci_mvebu: Properly configure and use PCI Bridge (PCIe Root Port) Pali Rohár
2021-11-02 10:47   ` Stefan Roese
2021-10-22 14:22 ` [PATCH 4/8] pci: pci_mvebu: Remove unused functions Pali Rohár
2021-11-02 10:47   ` Stefan Roese
2021-10-22 14:22 ` [PATCH 5/8] pci: pci_mvebu: Fix place of link up detection Pali Rohár
2021-11-02 10:50   ` Stefan Roese
2021-10-22 14:22 ` [PATCH 6/8] pci: pci_mvebu: Do not automatically enable bus mastering on PCI Bridge Pali Rohár
2021-11-02 10:50   ` Stefan Roese
2021-10-22 14:22 ` [PATCH 7/8] pci: pci_mvebu: Setup PCI controller to Root Complex mode Pali Rohár
2021-11-02 10:50   ` Stefan Roese
2021-10-22 14:22 ` [PATCH 8/8] pci: pci_mvebu: Fix comment about driver class name Pali Rohár
2021-11-02 10:50   ` Stefan Roese
2021-11-03  7:46 ` [PATCH 0/8] pci: pci_mvebu: Fix access to config space and PCIe Root Port Stefan Roese

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.