All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH] core: ofnode: Add ofnode_pci_get_devfn()
@ 2019-01-18 11:46 Stefan Roese
  2019-01-18 11:46 ` [U-Boot] [PATCH] pci: Add PCI_SLOT macro to include/pci.h Stefan Roese
                   ` (4 more replies)
  0 siblings, 5 replies; 17+ messages in thread
From: Stefan Roese @ 2019-01-18 11:46 UTC (permalink / raw)
  To: u-boot

This function will be used by the Marvell Armada XP/38x PCIe driver,
which is moved to DM right now. It's mostly copied from the Linux
version.

Signed-off-by: Stefan Roese <sr@denx.de>
Cc: Simon Glass <sjg@chromium.org>
---
 drivers/core/ofnode.c | 12 ++++++++++++
 include/dm/ofnode.h   | 11 +++++++++++
 2 files changed, 23 insertions(+)

diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c
index 0e584c12dc..b74b95ff61 100644
--- a/drivers/core/ofnode.c
+++ b/drivers/core/ofnode.c
@@ -666,6 +666,18 @@ int ofnode_read_pci_vendev(ofnode node, u16 *vendor, u16 *device)
 	return -ENOENT;
 }
 
+int ofnode_pci_get_devfn(ofnode node)
+{
+	u32 reg[5];
+	int error;
+
+	error = ofnode_read_u32_array(node, "reg", reg, ARRAY_SIZE(reg));
+	if (error)
+		return error;
+
+	return (reg[0] >> 8) & 0xff;
+}
+
 int ofnode_read_addr_cells(ofnode node)
 {
 	if (ofnode_is_np(node))
diff --git a/include/dm/ofnode.h b/include/dm/ofnode.h
index d206ee2caa..66d7f8d055 100644
--- a/include/dm/ofnode.h
+++ b/include/dm/ofnode.h
@@ -608,6 +608,17 @@ int ofnode_read_pci_addr(ofnode node, enum fdt_pci_space type,
  */
 int ofnode_read_pci_vendev(ofnode node, u16 *vendor, u16 *device);
 
+/**
+ * ofnode_pci_get_devfn() - Get device and function numbers for a device node
+ * @ofnode: node to examine
+ *
+ * Parses a standard 5-cell PCI resource and returns an 8-bit value that can
+ * be passed to the PCI_SLOT() and PCI_FUNC() macros to extract the device
+ * and function numbers respectively. On error a negative error code is
+ * returned.
+ */
+int ofnode_pci_get_devfn(ofnode node);
+
 /**
  * ofnode_read_addr_cells() - Get the number of address cells for a node
  *
-- 
2.20.1

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

* [U-Boot] [PATCH] pci: Add PCI_SLOT macro to include/pci.h
  2019-01-18 11:46 [U-Boot] [PATCH] core: ofnode: Add ofnode_pci_get_devfn() Stefan Roese
@ 2019-01-18 11:46 ` Stefan Roese
  2019-01-22  0:32   ` Simon Glass
  2019-01-18 11:46 ` [U-Boot] [PATCH 1/3 v2] pci: pci_mvebu: Add DM_PCI support and move CONFIG_PCI_MVEBU to defconfig Stefan Roese
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 17+ messages in thread
From: Stefan Roese @ 2019-01-18 11:46 UTC (permalink / raw)
  To: u-boot

This macro will be used the by the Marvell Armada XP/38x PCIe driver,
which is moved to DM right now.

Signed-off-by: Stefan Roese <sr@denx.de>
Cc: Bin Meng <bmeng.cn@gmail.com>
Cc: Simon Glass <sjg@chromium.org>
---
 include/pci.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/pci.h b/include/pci.h
index 785d7d28b7..f4a9e025b3 100644
--- a/include/pci.h
+++ b/include/pci.h
@@ -501,6 +501,7 @@ typedef int pci_dev_t;
 #define PCI_BUS(d)		(((d) >> 16) & 0xff)
 #define PCI_DEV(d)		(((d) >> 11) & 0x1f)
 #define PCI_FUNC(d)		(((d) >> 8) & 0x7)
+#define PCI_SLOT(d)		(((d) >> 3) & 0x1f)
 #define PCI_DEVFN(d, f)		((d) << 11 | (f) << 8)
 #define PCI_MASK_BUS(bdf)	((bdf) & 0xffff)
 #define PCI_ADD_BUS(bus, devfn)	(((bus) << 16) | (devfn))
-- 
2.20.1

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

* [U-Boot] [PATCH 1/3 v2] pci: pci_mvebu: Add DM_PCI support and move CONFIG_PCI_MVEBU to defconfig
  2019-01-18 11:46 [U-Boot] [PATCH] core: ofnode: Add ofnode_pci_get_devfn() Stefan Roese
  2019-01-18 11:46 ` [U-Boot] [PATCH] pci: Add PCI_SLOT macro to include/pci.h Stefan Roese
@ 2019-01-18 11:46 ` Stefan Roese
  2019-01-22  7:35   ` Chris Packham
  2019-01-18 11:46 ` [U-Boot] [PATCH 2/3 v2] arm: mvebu: armada-xp/37x.dtsi: Sync PCIe DT nodes with Linux v4.20 Stefan Roese
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 17+ messages in thread
From: Stefan Roese @ 2019-01-18 11:46 UTC (permalink / raw)
  To: u-boot

This patch adds DM_PCI support to the MVEBU PCIe driver. This is
necessary, since all PCI drivers have to be moved to DM (driver model)
until the v2019.07 release.

To not break git bisect'ablility, this patch also moves CONFIG_PCI_MVEBU
from config headers to the defconfig files.

Signed-off-by: Stefan Roese <sr@denx.de>
Cc: Dirk Eibach <dirk.eibach@gdsys.cc>
Cc: Mario Six <mario.six@gdsys.cc>
Cc: Chris Packham <chris.packham@alliedtelesis.co.nz>
Cc: Phil Sutter <phil@nwl.cc>
Cc: Marek Behún <marek.behun@nic.cz>
Cc: VlaoMao <vlaomao@gmail.com>
---
v2:
- Completely configure the controller based on DT properties. Now
  port and lane are read from the DT nodes and also the tgt and
  attr values are generated using the DT similar to how this is
  done in the Linux driver version. All A38x / XP specific defines
  can now be removed from this driver because of this.
- Please note that now the board specific dts file needs to enable
  the used PCIe ports, as this is also done in Linux.

 configs/clearfog_defconfig          |   1 +
 configs/controlcenterdc_defconfig   |   3 +
 configs/db-88f6820-amc_defconfig    |   1 +
 configs/db-88f6820-gp_defconfig     |   1 +
 configs/db-mv784mp-gp_defconfig     |   1 +
 configs/ds414_defconfig             |   1 +
 configs/theadorable_debug_defconfig |   2 +
 configs/turris_omnia_defconfig      |   3 +-
 drivers/pci/Kconfig                 |   9 +
 drivers/pci/pci_mvebu.c             | 457 ++++++++++++++++------------
 include/configs/clearfog.h          |   1 -
 include/configs/controlcenterdc.h   |   3 -
 include/configs/db-88f6820-amc.h    |   1 -
 include/configs/db-88f6820-gp.h     |   1 -
 include/configs/db-mv784mp-gp.h     |   1 -
 include/configs/ds414.h             |   1 -
 include/configs/theadorable.h       |   7 -
 include/configs/turris_omnia.h      |   1 -
 scripts/config_whitelist.txt        |   1 -
 19 files changed, 287 insertions(+), 209 deletions(-)

diff --git a/configs/clearfog_defconfig b/configs/clearfog_defconfig
index e1c5a1fa13..0f69ff606d 100644
--- a/configs/clearfog_defconfig
+++ b/configs/clearfog_defconfig
@@ -56,6 +56,7 @@ CONFIG_PHY_GIGE=y
 CONFIG_MVNETA=y
 CONFIG_MII=y
 CONFIG_PCI=y
+CONFIG_PCI_MVEBU=y
 CONFIG_SCSI=y
 CONFIG_DEBUG_UART_SHIFT=2
 CONFIG_SYS_NS16550=y
diff --git a/configs/controlcenterdc_defconfig b/configs/controlcenterdc_defconfig
index 0c762ad77c..f22a3ab819 100644
--- a/configs/controlcenterdc_defconfig
+++ b/configs/controlcenterdc_defconfig
@@ -58,6 +58,9 @@ CONFIG_PHY_MARVELL=y
 CONFIG_PHY_GIGE=y
 CONFIG_MVNETA=y
 CONFIG_MII=y
+CONFIG_PCI=y
+CONFIG_DM_PCI_COMPAT=y
+CONFIG_PCI_MVEBU=y
 CONFIG_SCSI=y
 CONFIG_DEBUG_UART_SHIFT=2
 CONFIG_SYS_NS16550=y
diff --git a/configs/db-88f6820-amc_defconfig b/configs/db-88f6820-amc_defconfig
index 9068a58406..03bf2bcafc 100644
--- a/configs/db-88f6820-amc_defconfig
+++ b/configs/db-88f6820-amc_defconfig
@@ -60,6 +60,7 @@ CONFIG_PHY_GIGE=y
 CONFIG_MVNETA=y
 CONFIG_MII=y
 CONFIG_PCI=y
+CONFIG_PCI_MVEBU=y
 CONFIG_DEBUG_UART_SHIFT=2
 CONFIG_SYS_NS16550=y
 CONFIG_KIRKWOOD_SPI=y
diff --git a/configs/db-88f6820-gp_defconfig b/configs/db-88f6820-gp_defconfig
index 413010c4ef..b36b7c3b2c 100644
--- a/configs/db-88f6820-gp_defconfig
+++ b/configs/db-88f6820-gp_defconfig
@@ -56,6 +56,7 @@ CONFIG_PHY_GIGE=y
 CONFIG_MVNETA=y
 CONFIG_MII=y
 CONFIG_PCI=y
+CONFIG_PCI_MVEBU=y
 CONFIG_SCSI=y
 CONFIG_DEBUG_UART_SHIFT=2
 CONFIG_SYS_NS16550=y
diff --git a/configs/db-mv784mp-gp_defconfig b/configs/db-mv784mp-gp_defconfig
index 3f140986b6..b6c61c3a48 100644
--- a/configs/db-mv784mp-gp_defconfig
+++ b/configs/db-mv784mp-gp_defconfig
@@ -55,6 +55,7 @@ CONFIG_PHY_GIGE=y
 CONFIG_MVNETA=y
 CONFIG_MII=y
 CONFIG_PCI=y
+CONFIG_PCI_MVEBU=y
 CONFIG_DEBUG_UART_SHIFT=2
 CONFIG_SYS_NS16550=y
 CONFIG_KIRKWOOD_SPI=y
diff --git a/configs/ds414_defconfig b/configs/ds414_defconfig
index 5325bd9968..32f37440c3 100644
--- a/configs/ds414_defconfig
+++ b/configs/ds414_defconfig
@@ -52,6 +52,7 @@ CONFIG_PHY_GIGE=y
 CONFIG_MVNETA=y
 CONFIG_MII=y
 CONFIG_PCI=y
+CONFIG_PCI_MVEBU=y
 CONFIG_DEBUG_UART_SHIFT=2
 CONFIG_SYS_NS16550=y
 CONFIG_KIRKWOOD_SPI=y
diff --git a/configs/theadorable_debug_defconfig b/configs/theadorable_debug_defconfig
index 9e99618998..ac6dfd6844 100644
--- a/configs/theadorable_debug_defconfig
+++ b/configs/theadorable_debug_defconfig
@@ -62,6 +62,8 @@ CONFIG_PHY_GIGE=y
 CONFIG_MVNETA=y
 CONFIG_MII=y
 CONFIG_PCI=y
+CONFIG_DM_PCI_COMPAT=y
+CONFIG_PCI_MVEBU=y
 CONFIG_DEBUG_UART_SHIFT=2
 CONFIG_SYS_NS16550=y
 CONFIG_KIRKWOOD_SPI=y
diff --git a/configs/turris_omnia_defconfig b/configs/turris_omnia_defconfig
index 4a1e23c86c..5cafe2ee34 100644
--- a/configs/turris_omnia_defconfig
+++ b/configs/turris_omnia_defconfig
@@ -37,7 +37,6 @@ CONFIG_DEFAULT_DEVICE_TREE="armada-385-turris-omnia"
 CONFIG_ENV_IS_IN_SPI_FLASH=y
 CONFIG_SPL_OF_TRANSLATE=y
 CONFIG_SCSI_AHCI=y
-CONFIG_MISC=y
 CONFIG_ATSHA204A=y
 CONFIG_MMC_SDHCI=y
 CONFIG_MMC_SDHCI_MV=y
@@ -45,6 +44,8 @@ CONFIG_PHY_MARVELL=y
 CONFIG_PHY_GIGE=y
 CONFIG_MVNETA=y
 CONFIG_MII=y
+CONFIG_PCI=y
+CONFIG_PCI_MVEBU=y
 CONFIG_DEBUG_UART_SHIFT=2
 CONFIG_SYS_NS16550=y
 CONFIG_KIRKWOOD_SPI=y
diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
index f59803dbd6..1521885bde 100644
--- a/drivers/pci/Kconfig
+++ b/drivers/pci/Kconfig
@@ -112,4 +112,13 @@ config PCIE_INTEL_FPGA
 	  Say Y here if you want to enable PCIe controller support on Intel
 	  FPGA, example Stratix 10.
 
+config PCI_MVEBU
+	bool "Enable Armada XP/38x PCIe driver"
+	depends on ARCH_MVEBU
+	select DM_PCI
+	select MISC
+	help
+	  Say Y here if you want to enable PCIe controller support on
+	  Armada XP/38x SoCs.
+
 endif
diff --git a/drivers/pci/pci_mvebu.c b/drivers/pci/pci_mvebu.c
index affe9f98ad..a03b979692 100644
--- a/drivers/pci/pci_mvebu.c
+++ b/drivers/pci/pci_mvebu.c
@@ -10,11 +10,16 @@
  */
 
 #include <common.h>
+#include <dm.h>
+#include <dm/device-internal.h>
+#include <dm/lists.h>
+#include <dm/of_access.h>
 #include <pci.h>
-#include <linux/errno.h>
 #include <asm/io.h>
 #include <asm/arch/cpu.h>
 #include <asm/arch/soc.h>
+#include <linux/errno.h>
+#include <linux/ioport.h>
 #include <linux/mbus.h>
 
 DECLARE_GLOBAL_DATA_PTR;
@@ -59,26 +64,22 @@ DECLARE_GLOBAL_DATA_PTR;
 #define PCIE_DEBUG_CTRL			0x1a60
 #define  PCIE_DEBUG_SOFT_RESET		BIT(20)
 
-struct resource {
-	u32 start;
-	u32 end;
-};
-
 struct mvebu_pcie {
 	struct pci_controller hose;
-	char *name;
 	void __iomem *base;
 	void __iomem *membase;
 	struct resource mem;
 	void __iomem *iobase;
 	u32 port;
 	u32 lane;
+	int devfn;
 	u32 lane_mask;
 	pci_dev_t dev;
+	char name[16];
+	unsigned int mem_target;
+	unsigned int mem_attr;
 };
 
-#define to_pcie(_hc)	container_of(_hc, struct mvebu_pcie, pci)
-
 /*
  * MVEBU PCIe controller needs MEMORY and I/O BARs to be mapped
  * into SoCs address space. Each controller will map 128M of MEM
@@ -87,82 +88,6 @@ struct mvebu_pcie {
 static void __iomem *mvebu_pcie_membase = (void __iomem *)MBUS_PCI_MEM_BASE;
 #define PCIE_MEM_SIZE	(128 << 20)
 
-#if defined(CONFIG_ARMADA_38X)
-#define PCIE_BASE(if)					\
-	((if) == 0 ?					\
-	 MVEBU_REG_PCIE0_BASE :				\
-	 (MVEBU_REG_PCIE_BASE + 0x4000 * (if - 1)))
-
-/*
- * On A38x MV6820 these PEX ports are supported:
- *  0 - Port 0.0
- *  1 - Port 1.0
- *  2 - Port 2.0
- *  3 - Port 3.0
- */
-#define MAX_PEX 4
-static struct mvebu_pcie pcie_bus[MAX_PEX];
-
-static void mvebu_get_port_lane(struct mvebu_pcie *pcie, int pex_idx,
-				int *mem_target, int *mem_attr)
-{
-	u8 port[] = { 0, 1, 2, 3 };
-	u8 lane[] = { 0, 0, 0, 0 };
-	u8 target[] = { 8, 4, 4, 4 };
-	u8 attr[] = { 0xe8, 0xe8, 0xd8, 0xb8 };
-
-	pcie->port = port[pex_idx];
-	pcie->lane = lane[pex_idx];
-	*mem_target = target[pex_idx];
-	*mem_attr = attr[pex_idx];
-}
-#else
-#define PCIE_BASE(if)							\
-	((if) < 8 ?							\
-	 (MVEBU_REG_PCIE_BASE + ((if) / 4) * 0x40000 + ((if) % 4) * 0x4000) : \
-	 (MVEBU_REG_PCIE_BASE + 0x2000 + ((if) % 8) * 0x40000))
-
-/*
- * On AXP MV78460 these PEX ports are supported:
- *  0 - Port 0.0
- *  1 - Port 0.1
- *  2 - Port 0.2
- *  3 - Port 0.3
- *  4 - Port 1.0
- *  5 - Port 1.1
- *  6 - Port 1.2
- *  7 - Port 1.3
- *  8 - Port 2.0
- *  9 - Port 3.0
- */
-#define MAX_PEX 10
-static struct mvebu_pcie pcie_bus[MAX_PEX];
-
-static void mvebu_get_port_lane(struct mvebu_pcie *pcie, int pex_idx,
-				int *mem_target, int *mem_attr)
-{
-	u8 port[] = { 0, 0, 0, 0, 1, 1, 1, 1, 2, 3 };
-	u8 lane[] = { 0, 1, 2, 3, 0, 1, 2, 3, 0, 0 };
-	u8 target[] = { 4, 4, 4, 4, 8, 8, 8, 8, 4, 8 };
-	u8 attr[] = { 0xe8, 0xd8, 0xb8, 0x78,
-		      0xe8, 0xd8, 0xb8, 0x78,
-		      0xf8, 0xf8 };
-
-	pcie->port = port[pex_idx];
-	pcie->lane = lane[pex_idx];
-	*mem_target = target[pex_idx];
-	*mem_attr = attr[pex_idx];
-}
-#endif
-
-static int mvebu_pex_unit_is_x4(int pex_idx)
-{
-	int pex_unit = pex_idx < 9 ? pex_idx >> 2 : 3;
-	u32 mask = (0x0f << (pex_unit * 8));
-
-	return (readl(COMPHY_REFCLK_ALIGNMENT) & mask) == mask;
-}
-
 static inline bool mvebu_pcie_link_up(struct mvebu_pcie *pcie)
 {
 	u32 val;
@@ -211,67 +136,83 @@ static inline struct mvebu_pcie *hose_to_pcie(struct pci_controller *hose)
 	return container_of(hose, struct mvebu_pcie, hose);
 }
 
-static int mvebu_pcie_read_config_dword(struct pci_controller *hose,
-		pci_dev_t dev, int offset, u32 *val)
+static int mvebu_pcie_read_config(struct udevice *bus, pci_dev_t bdf,
+				  uint offset, ulong *valuep,
+				  enum pci_size_t size)
 {
-	struct mvebu_pcie *pcie = hose_to_pcie(hose);
+	struct mvebu_pcie *pcie = dev_get_platdata(bus);
 	int local_bus = PCI_BUS(pcie->dev);
 	int local_dev = PCI_DEV(pcie->dev);
 	u32 reg;
+	u32 data;
+
+	debug("PCIE CFG read:  (b,d,f)=(%2d,%2d,%2d) ",
+	      PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf));
 
 	/* Only allow one other device besides the local one on the local bus */
-	if (PCI_BUS(dev) == local_bus && PCI_DEV(dev) != local_dev) {
-		if (local_dev == 0 && PCI_DEV(dev) != 1) {
+	if (PCI_BUS(bdf) == local_bus && PCI_DEV(bdf) != local_dev) {
+		if (local_dev == 0 && PCI_DEV(bdf) != 1) {
+			debug("- out of range\n");
 			/*
 			 * If local dev is 0, the first other dev can
 			 * only be 1
 			 */
-			*val = 0xffffffff;
-			return 1;
-		} else if (local_dev != 0 && PCI_DEV(dev) != 0) {
+			*valuep = pci_get_ff(size);
+			return 0;
+		} else if (local_dev != 0 && PCI_DEV(bdf) != 0) {
+			debug("- out of range\n");
 			/*
 			 * If local dev is not 0, the first other dev can
 			 * only be 0
 			 */
-			*val = 0xffffffff;
-			return 1;
+			*valuep = pci_get_ff(size);
+			return 0;
 		}
 	}
 
 	/* write address */
-	reg = PCIE_CONF_ADDR(dev, offset);
+	reg = PCIE_CONF_ADDR(bdf, offset);
 	writel(reg, pcie->base + PCIE_CONF_ADDR_OFF);
-	*val = readl(pcie->base + PCIE_CONF_DATA_OFF);
+	data = readl(pcie->base + PCIE_CONF_DATA_OFF);
+	debug("(addr,val)=(0x%04x, 0x%08x)\n", offset, data);
+	*valuep = pci_conv_32_to_size(data, offset, size);
 
 	return 0;
 }
 
-static int mvebu_pcie_write_config_dword(struct pci_controller *hose,
-		pci_dev_t dev, int offset, u32 val)
+static int mvebu_pcie_write_config(struct udevice *bus, pci_dev_t bdf,
+				   uint offset, ulong value,
+				   enum pci_size_t size)
 {
-	struct mvebu_pcie *pcie = hose_to_pcie(hose);
+	struct mvebu_pcie *pcie = dev_get_platdata(bus);
 	int local_bus = PCI_BUS(pcie->dev);
 	int local_dev = PCI_DEV(pcie->dev);
+	u32 data;
+
+	debug("PCIE CFG write: (b,d,f)=(%2d,%2d,%2d) ",
+	      PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf));
+	debug("(addr,val)=(0x%04x, 0x%08lx)\n", offset, value);
 
 	/* Only allow one other device besides the local one on the local bus */
-	if (PCI_BUS(dev) == local_bus && PCI_DEV(dev) != local_dev) {
-		if (local_dev == 0 && PCI_DEV(dev) != 1) {
+	if (PCI_BUS(bdf) == local_bus && PCI_DEV(bdf) != local_dev) {
+		if (local_dev == 0 && PCI_DEV(bdf) != 1) {
 			/*
 			 * If local dev is 0, the first other dev can
 			 * only be 1
 			 */
-			return 1;
-		} else if (local_dev != 0 && PCI_DEV(dev) != 0) {
+			return 0;
+		} else if (local_dev != 0 && PCI_DEV(bdf) != 0) {
 			/*
 			 * If local dev is not 0, the first other dev can
 			 * only be 0
 			 */
-			return 1;
+			return 0;
 		}
 	}
 
-	writel(PCIE_CONF_ADDR(dev, offset), pcie->base + PCIE_CONF_ADDR_OFF);
-	writel(val, pcie->base + PCIE_CONF_DATA_OFF);
+	writel(PCIE_CONF_ADDR(bdf, offset), pcie->base + PCIE_CONF_ADDR_OFF);
+	data = pci_conv_size_to_32(0, value, offset, size);
+	writel(data, pcie->base + PCIE_CONF_DATA_OFF);
 
 	return 0;
 }
@@ -331,107 +272,241 @@ static void mvebu_pcie_setup_wins(struct mvebu_pcie *pcie)
 	       pcie->base + PCIE_BAR_CTRL_OFF(1));
 }
 
-void pci_init_board(void)
+/**
+ * pcie_dw_mvebu_probe() - Probe the PCIe bus for active link
+ *
+ * @dev: A pointer to the device being operated on
+ *
+ * Probe for an active link on the PCIe bus and configure the controller
+ * to enable this port.
+ *
+ * Return: 0 on success, else -ENODEV
+ */
+static int mvebu_pcie_probe(struct udevice *dev)
 {
-	int mem_target, mem_attr, i;
-	int bus = 0;
+	struct mvebu_pcie *pcie = dev_get_platdata(dev);
+	struct udevice *ctlr = pci_get_controller(dev);
+	struct pci_controller *hose = dev_get_uclass_priv(ctlr);
+	static int bus;
 	u32 reg;
-	u32 soc_ctrl = readl(MVEBU_SYSTEM_REG_BASE + 0x4);
 
-	/* Check SoC Control Power State */
-	debug("%s: SoC Control %08x, 0en %01lx, 1en %01lx, 2en %01lx\n",
-	      __func__, soc_ctrl, SELECT(soc_ctrl, 0), SELECT(soc_ctrl, 1),
-	      SELECT(soc_ctrl, 2));
+	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));
+
+	mvebu_pcie_set_local_bus_nr(pcie, bus);
+	mvebu_pcie_set_local_dev_nr(pcie, 0);
+	pcie->dev = PCI_BDF(bus, 0, 0);
+
+	pcie->mem.start = (u32)mvebu_pcie_membase;
+	pcie->mem.end = pcie->mem.start + PCIE_MEM_SIZE - 1;
+	mvebu_pcie_membase += PCIE_MEM_SIZE;
+
+	if (mvebu_mbus_add_window_by_id(pcie->mem_target, pcie->mem_attr,
+					(phys_addr_t)pcie->mem.start,
+					PCIE_MEM_SIZE)) {
+		printf("PCIe unable to add mbus window for mem at %08x+%08x\n",
+		       (u32)pcie->mem.start, PCIE_MEM_SIZE);
+	}
+
+	/* 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_MASTER;
+	reg |= BIT(10);		/* disable interrupts */
+	writel(reg, pcie->base + PCIE_CMD_OFF);
+
+	/* Set BAR0 to internal registers */
+	writel(SOC_REGS_PHY_BASE, pcie->base + PCIE_BAR_LO_OFF(0));
+	writel(0, pcie->base + PCIE_BAR_HI_OFF(0));
 
-	for (i = 0; i < MAX_PEX; i++) {
-		struct mvebu_pcie *pcie = &pcie_bus[i];
-		struct pci_controller *hose = &pcie->hose;
+	/* PCI memory space */
+	pci_set_region(hose->regions + 0, pcie->mem.start,
+		       pcie->mem.start, PCIE_MEM_SIZE, PCI_REGION_MEM);
+	pci_set_region(hose->regions + 1,
+		       0, 0,
+		       gd->ram_size,
+		       PCI_REGION_MEM | PCI_REGION_SYS_MEMORY);
+	hose->region_count = 2;
+
+	bus++;
+
+	return 0;
+}
+
+static const struct dm_pci_ops mvebu_pcie_ops = {
+	.read_config	= mvebu_pcie_read_config,
+	.write_config	= mvebu_pcie_write_config,
+};
+
+static struct driver pcie_mvebu_drv = {
+	.name			= "pcie_mvebu",
+	.id			= UCLASS_PCI,
+	.ops			= &mvebu_pcie_ops,
+	.probe			= mvebu_pcie_probe,
+	.platdata_auto_alloc_size = sizeof(struct mvebu_pcie),
+};
+
+static int mvebu_pcie_port_parse_dt(ofnode node, struct mvebu_pcie *pcie)
+{
+	const u32 *addr;
+	int len;
+
+	addr = ofnode_get_property(node, "assigned-addresses", &len);
+	if (!addr) {
+		pr_err("property \"assigned-addresses\" not found");
+		return -FDT_ERR_NOTFOUND;
+	}
+
+	pcie->base = (void *)(fdt32_to_cpu(addr[2]) + SOC_REGS_PHY_BASE);
+
+	return 0;
+}
+
+#define DT_FLAGS_TO_TYPE(flags)       (((flags) >> 24) & 0x03)
+#define    DT_TYPE_IO                 0x1
+#define    DT_TYPE_MEM32              0x2
+#define DT_CPUADDR_TO_TARGET(cpuaddr) (((cpuaddr) >> 56) & 0xFF)
+#define DT_CPUADDR_TO_ATTR(cpuaddr)   (((cpuaddr) >> 48) & 0xFF)
+
+static int mvebu_get_tgt_attr(ofnode node, int devfn,
+			      unsigned long type,
+			      unsigned int *tgt,
+			      unsigned int *attr)
+{
+	const int na = 3, ns = 2;
+	const __be32 *range;
+	int rlen, nranges, rangesz, pna, i;
+
+	*tgt = -1;
+	*attr = -1;
+
+	range = ofnode_get_property(node, "ranges", &rlen);
+	if (!range)
+		return -EINVAL;
+
+	pna = 2; /* hardcoded for now because of lack of of_n_addr_cells() */
+	rangesz = pna + na + ns;
+	nranges = rlen / sizeof(__be32) / rangesz;
+
+	for (i = 0; i < nranges; i++, range += rangesz) {
+		u32 flags = of_read_number(range, 1);
+		u32 slot = of_read_number(range + 1, 1);
+		u64 cpuaddr = of_read_number(range + na, pna);
+		unsigned long rtype;
+
+		if (DT_FLAGS_TO_TYPE(flags) == DT_TYPE_IO)
+			rtype = IORESOURCE_IO;
+		else if (DT_FLAGS_TO_TYPE(flags) == DT_TYPE_MEM32)
+			rtype = IORESOURCE_MEM;
+		else
+			continue;
+
+		if (slot == PCI_SLOT(devfn) && type == rtype) {
+			*tgt = DT_CPUADDR_TO_TARGET(cpuaddr);
+			*attr = DT_CPUADDR_TO_ATTR(cpuaddr);
+			return 0;
+		}
+	}
+
+	return -ENOENT;
+}
+
+/*
+ * Use a MISC device to bind the n instances (child nodes) of the
+ * PCIe base controller in UCLASS_PCI.
+ */
+static int mvebu_pcie_bind(struct udevice *parent)
+{
+	struct mvebu_pcie *pcie;
+	struct uclass_driver *drv;
+	struct udevice *dev;
+	ofnode subnode;
+	int ret;
+
+	/* Lookup eth driver */
+	drv = lists_uclass_lookup(UCLASS_PCI);
+	if (!drv) {
+		puts("Cannot find PCI driver\n");
+		return -ENOENT;
+	}
+
+	ofnode_for_each_subnode(subnode, dev_ofnode(parent)) {
+		if (!ofnode_is_available(subnode))
+			continue;
+
+		pcie = calloc(1, sizeof(*pcie));
+		if (!pcie)
+			return -ENOMEM;
 
 		/* Get port number, lane number and memory target / attr */
-		mvebu_get_port_lane(pcie, i, &mem_target, &mem_attr);
+		if (ofnode_read_u32(subnode, "marvell,pcie-port",
+				    &pcie->port)) {
+			free(pcie);
+			continue;
+		}
+
+		if (ofnode_read_u32(subnode, "marvell,pcie-lane", &pcie->lane))
+			pcie->lane = 0;
 
-		/* Don't read at all from pci registers if port power is down */
-		if (SELECT(soc_ctrl, pcie->port) == 0) {
-			if (pcie->lane == 0)
-				debug("%s: skipping port %d\n", __func__, pcie->port);
+		sprintf(pcie->name, "pcie%d.%d", pcie->port, pcie->lane);
+
+		pcie->devfn = ofnode_pci_get_devfn(subnode);
+		if (pcie->devfn < 0) {
+			free(pcie);
 			continue;
 		}
 
-		pcie->base = (void __iomem *)PCIE_BASE(i);
+		ret = mvebu_get_tgt_attr(dev_ofnode(parent), pcie->devfn,
+					 IORESOURCE_MEM,
+					 &pcie->mem_target, &pcie->mem_attr);
+		if (ret < 0) {
+			printf("%s: cannot get tgt/attr for mem window\n",
+			       pcie->name);
+			free(pcie);
+			continue;
+		}
+
+		/* Parse PCIe controller register base from DT */
+		ret = mvebu_pcie_port_parse_dt(subnode, pcie);
+		if (ret < 0) {
+			free(pcie);
+			continue;
+		}
 
 		/* Check link and skip ports that have no link */
 		if (!mvebu_pcie_link_up(pcie)) {
 			debug("%s: PCIe %d.%d - down\n", __func__,
 			      pcie->port, pcie->lane);
+
+			free(pcie);
 			continue;
 		}
-		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));
-
-		mvebu_pcie_set_local_bus_nr(pcie, bus);
-		mvebu_pcie_set_local_dev_nr(pcie, 0);
-		pcie->dev = PCI_BDF(bus, 0, 0);
-
-		pcie->mem.start = (u32)mvebu_pcie_membase;
-		pcie->mem.end = pcie->mem.start + PCIE_MEM_SIZE - 1;
-		mvebu_pcie_membase += PCIE_MEM_SIZE;
-
-		if (mvebu_mbus_add_window_by_id(mem_target, mem_attr,
-						(phys_addr_t)pcie->mem.start,
-						PCIE_MEM_SIZE)) {
-			printf("PCIe unable to add mbus window for mem at %08x+%08x\n",
-			       (u32)pcie->mem.start, PCIE_MEM_SIZE);
-		}
 
-		/* 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_MASTER;
-		reg |= BIT(10);		/* disable interrupts */
-		writel(reg, pcie->base + PCIE_CMD_OFF);
-
-		/* Setup U-Boot PCI Controller */
-		hose->first_busno = 0;
-		hose->current_busno = bus;
-
-		/* PCI memory space */
-		pci_set_region(hose->regions + 0, pcie->mem.start,
-			       pcie->mem.start, PCIE_MEM_SIZE, PCI_REGION_MEM);
-		pci_set_region(hose->regions + 1,
-			       0, 0,
-			       gd->ram_size,
-			       PCI_REGION_MEM | PCI_REGION_SYS_MEMORY);
-		hose->region_count = 2;
-
-		pci_set_ops(hose,
-			    pci_hose_read_config_byte_via_dword,
-			    pci_hose_read_config_word_via_dword,
-			    mvebu_pcie_read_config_dword,
-			    pci_hose_write_config_byte_via_dword,
-			    pci_hose_write_config_word_via_dword,
-			    mvebu_pcie_write_config_dword);
-		pci_register_hose(hose);
-
-		hose->last_busno = pci_hose_scan(hose);
-
-		/* Set BAR0 to internal registers */
-		writel(SOC_REGS_PHY_BASE, pcie->base + PCIE_BAR_LO_OFF(0));
-		writel(0, pcie->base + PCIE_BAR_HI_OFF(0));
-
-		bus = hose->last_busno + 1;
-
-		/* need to skip more for X4 links, otherwise scan will hang */
-		if (mvebu_soc_family() == MVEBU_SOC_AXP) {
-			if (mvebu_pex_unit_is_x4(i))
-				i += 3;
-		}
+		/* Create child device UCLASS_PCI and bind it */
+		device_bind_ofnode(parent, &pcie_mvebu_drv, pcie->name, pcie,
+				   subnode, &dev);
 	}
+
+	return 0;
 }
+
+static const struct udevice_id mvebu_pcie_ids[] = {
+	{ .compatible = "marvell,armada-xp-pcie" },
+	{ .compatible = "marvell,armada-370-pcie" },
+	{ }
+};
+
+U_BOOT_DRIVER(pcie_mvebu_base) = {
+	.name			= "pcie_mvebu_base",
+	.id			= UCLASS_MISC,
+	.of_match		= mvebu_pcie_ids,
+	.bind			= mvebu_pcie_bind,
+};
diff --git a/include/configs/clearfog.h b/include/configs/clearfog.h
index 77ab6caf52..f9510826d7 100644
--- a/include/configs/clearfog.h
+++ b/include/configs/clearfog.h
@@ -55,7 +55,6 @@
 
 /* PCIe support */
 #ifndef CONFIG_SPL_BUILD
-#define CONFIG_PCI_MVEBU
 #define CONFIG_PCI_SCAN_SHOW
 #endif
 
diff --git a/include/configs/controlcenterdc.h b/include/configs/controlcenterdc.h
index b38cab1164..06c93c3e66 100644
--- a/include/configs/controlcenterdc.h
+++ b/include/configs/controlcenterdc.h
@@ -63,9 +63,6 @@
 
 /* PCIe support */
 #ifndef CONFIG_SPL_BUILD
-#define CONFIG_PCI
-#define CONFIG_PCI_MVEBU
-#define CONFIG_PCI_PNP
 #define CONFIG_PCI_SCAN_SHOW
 #endif
 
diff --git a/include/configs/db-88f6820-amc.h b/include/configs/db-88f6820-amc.h
index e68246cc0f..626a406cff 100644
--- a/include/configs/db-88f6820-amc.h
+++ b/include/configs/db-88f6820-amc.h
@@ -34,7 +34,6 @@
 
 /* PCIe support */
 #ifndef CONFIG_SPL_BUILD
-#define CONFIG_PCI_MVEBU
 #define CONFIG_PCI_SCAN_SHOW
 #endif
 
diff --git a/include/configs/db-88f6820-gp.h b/include/configs/db-88f6820-gp.h
index 3900cbed2d..1f328e97c5 100644
--- a/include/configs/db-88f6820-gp.h
+++ b/include/configs/db-88f6820-gp.h
@@ -59,7 +59,6 @@
 
 /* PCIe support */
 #ifndef CONFIG_SPL_BUILD
-#define CONFIG_PCI_MVEBU
 #define CONFIG_PCI_SCAN_SHOW
 #endif
 
diff --git a/include/configs/db-mv784mp-gp.h b/include/configs/db-mv784mp-gp.h
index 8ad007cc49..6cba326927 100644
--- a/include/configs/db-mv784mp-gp.h
+++ b/include/configs/db-mv784mp-gp.h
@@ -46,7 +46,6 @@
 
 /* PCIe support */
 #ifndef CONFIG_SPL_BUILD
-#define CONFIG_PCI_MVEBU
 #define CONFIG_PCI_SCAN_SHOW
 #endif
 
diff --git a/include/configs/ds414.h b/include/configs/ds414.h
index b9b708ad41..4ba050553a 100644
--- a/include/configs/ds414.h
+++ b/include/configs/ds414.h
@@ -41,7 +41,6 @@
 
 /* PCIe support */
 #ifndef CONFIG_SPL_BUILD
-#define CONFIG_PCI_MVEBU
 #define CONFIG_PCI_SCAN_SHOW
 #endif
 
diff --git a/include/configs/theadorable.h b/include/configs/theadorable.h
index de31681f8c..6837d55507 100644
--- a/include/configs/theadorable.h
+++ b/include/configs/theadorable.h
@@ -62,13 +62,6 @@
 #define CONFIG_SYS_SATA_MAX_DEVICE	1
 #define CONFIG_LBA48
 
-/* PCIe support */
-#ifdef CONFIG_CMD_PCI
-#ifndef CONFIG_SPL_BUILD
-#define CONFIG_PCI_MVEBU
-#endif
-#endif
-
 /* Enable LCD and reserve 512KB from top of memory*/
 #define CONFIG_SYS_MEM_TOP_HIDE		0x80000
 
diff --git a/include/configs/turris_omnia.h b/include/configs/turris_omnia.h
index 598674c96e..710b8991a4 100644
--- a/include/configs/turris_omnia.h
+++ b/include/configs/turris_omnia.h
@@ -66,7 +66,6 @@
 
 /* PCIe support */
 #ifndef CONFIG_SPL_BUILD
-#define CONFIG_PCI_MVEBU
 #define CONFIG_PCI_SCAN_SHOW
 #endif
 
diff --git a/scripts/config_whitelist.txt b/scripts/config_whitelist.txt
index 527050de97..da9809dcf3 100644
--- a/scripts/config_whitelist.txt
+++ b/scripts/config_whitelist.txt
@@ -1449,7 +1449,6 @@ CONFIG_PCI_MEM_BUS
 CONFIG_PCI_MEM_PHYS
 CONFIG_PCI_MEM_SIZE
 CONFIG_PCI_MSC01
-CONFIG_PCI_MVEBU
 CONFIG_PCI_NOSCAN
 CONFIG_PCI_OHCI
 CONFIG_PCI_OHCI_DEVNO
-- 
2.20.1

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

* [U-Boot] [PATCH 2/3 v2] arm: mvebu: armada-xp/37x.dtsi: Sync PCIe DT nodes with Linux v4.20
  2019-01-18 11:46 [U-Boot] [PATCH] core: ofnode: Add ofnode_pci_get_devfn() Stefan Roese
  2019-01-18 11:46 ` [U-Boot] [PATCH] pci: Add PCI_SLOT macro to include/pci.h Stefan Roese
  2019-01-18 11:46 ` [U-Boot] [PATCH 1/3 v2] pci: pci_mvebu: Add DM_PCI support and move CONFIG_PCI_MVEBU to defconfig Stefan Roese
@ 2019-01-18 11:46 ` Stefan Roese
  2019-01-18 11:46 ` [U-Boot] [PATCH 3/3 v2] arm: mvebu: armada-xp-theadorable.dts: Enable PCIe DT nodes Stefan Roese
  2019-01-21 18:15 ` [U-Boot] [PATCH] core: ofnode: Add ofnode_pci_get_devfn() Simon Glass
  4 siblings, 0 replies; 17+ messages in thread
From: Stefan Roese @ 2019-01-18 11:46 UTC (permalink / raw)
  To: u-boot

This patch sync's the PCIe DT nodes with the recent Linux v4.20 version.
This change makes it easier to reference specific PCIe nodes in the
board dts files to e.g. enable a PCIe port as this is now necessary with
the new DM PCI driver for these platforms.

Signed-off-by: Stefan Roese <sr@denx.de>
Cc: Dirk Eibach <dirk.eibach@gdsys.cc>
Cc: Mario Six <mario.six@gdsys.cc>
Cc: Chris Packham <chris.packham@alliedtelesis.co.nz>
Cc: Phil Sutter <phil@nwl.cc>
Cc: Marek Behún <marek.behun@nic.cz>
Cc: VlaoMao <vlaomao@gmail.com>
Cc: Tom Rini <trini@konsulko.com>
---
v2:
- New file

 arch/arm/dts/armada-375.dtsi        |  8 +++++---
 arch/arm/dts/armada-xp-mv78230.dtsi | 17 +++++++++------
 arch/arm/dts/armada-xp-mv78260.dtsi | 29 +++++++++++++++++---------
 arch/arm/dts/armada-xp-mv78460.dtsi | 32 +++++++++++++++++++----------
 4 files changed, 56 insertions(+), 30 deletions(-)

diff --git a/arch/arm/dts/armada-375.dtsi b/arch/arm/dts/armada-375.dtsi
index 249c41c757..62a548a55f 100644
--- a/arch/arm/dts/armada-375.dtsi
+++ b/arch/arm/dts/armada-375.dtsi
@@ -582,7 +582,7 @@
 			};
 		};
 
-		pcie-controller {
+		pciec: pcie at 82000000 {
 			compatible = "marvell,armada-370-pcie";
 			status = "disabled";
 			device_type = "pci";
@@ -601,7 +601,7 @@
 				0x82000000 0x2 0       MBUS_ID(0x04, 0xd8) 0 1 0 /* Port 1 MEM */
 				0x81000000 0x2 0       MBUS_ID(0x04, 0xd0) 0 1 0 /* Port 1 IO  */>;
 
-			pcie at 1,0 {
+			pcie0: pcie at 1,0 {
 				device_type = "pci";
 				assigned-addresses = <0x82000800 0 0x40000 0 0x2000>;
 				reg = <0x0800 0 0 0 0>;
@@ -610,6 +610,7 @@
 				#interrupt-cells = <1>;
 				ranges = <0x82000000 0 0 0x82000000 0x1 0 1 0
 					  0x81000000 0 0 0x81000000 0x1 0 1 0>;
+				bus-range = <0x00 0xff>;
 				interrupt-map-mask = <0 0 0 0>;
 				interrupt-map = <0 0 0 0 &gic GIC_SPI 29 IRQ_TYPE_LEVEL_HIGH>;
 				marvell,pcie-port = <0>;
@@ -618,7 +619,7 @@
 				status = "disabled";
 			};
 
-			pcie at 2,0 {
+			pcie1: pcie at 2,0 {
 				device_type = "pci";
 				assigned-addresses = <0x82000800 0 0x44000 0 0x2000>;
 				reg = <0x1000 0 0 0 0>;
@@ -627,6 +628,7 @@
 				#interrupt-cells = <1>;
 				ranges = <0x82000000 0 0 0x82000000 0x2 0 1 0
 					  0x81000000 0 0 0x81000000 0x2 0 1 0>;
+				bus-range = <0x00 0xff>;
 				interrupt-map-mask = <0 0 0 0>;
 				interrupt-map = <0 0 0 0 &gic GIC_SPI 33 IRQ_TYPE_LEVEL_HIGH>;
 				marvell,pcie-port = <0>;
diff --git a/arch/arm/dts/armada-xp-mv78230.dtsi b/arch/arm/dts/armada-xp-mv78230.dtsi
index 6e6d0f04bf..f6bab9fb20 100644
--- a/arch/arm/dts/armada-xp-mv78230.dtsi
+++ b/arch/arm/dts/armada-xp-mv78230.dtsi
@@ -86,7 +86,7 @@
 		 * configured as x4 or quad x1 lanes. One unit is
 		 * x1 only.
 		 */
-		pcie-controller {
+		pciec: pcie at 82000000 {
 			compatible = "marvell,armada-xp-pcie";
 			status = "disabled";
 			device_type = "pci";
@@ -114,7 +114,7 @@
 				0x82000000 0x5 0       MBUS_ID(0x08, 0xe8) 0 1 0 /* Port 1.0 MEM */
 				0x81000000 0x5 0       MBUS_ID(0x08, 0xe0) 0 1 0 /* Port 1.0 IO  */>;
 
-			pcie at 1,0 {
+			pcie1: pcie at 1,0 {
 				device_type = "pci";
 				assigned-addresses = <0x82000800 0 0x40000 0 0x2000>;
 				reg = <0x0800 0 0 0 0>;
@@ -123,6 +123,7 @@
 				#interrupt-cells = <1>;
 				ranges = <0x82000000 0 0 0x82000000 0x1 0 1 0
 					  0x81000000 0 0 0x81000000 0x1 0 1 0>;
+				bus-range = <0x00 0xff>;
 				interrupt-map-mask = <0 0 0 0>;
 				interrupt-map = <0 0 0 0 &mpic 58>;
 				marvell,pcie-port = <0>;
@@ -131,7 +132,7 @@
 				status = "disabled";
 			};
 
-			pcie at 2,0 {
+			pcie2: pcie at 2,0 {
 				device_type = "pci";
 				assigned-addresses = <0x82000800 0 0x44000 0 0x2000>;
 				reg = <0x1000 0 0 0 0>;
@@ -140,6 +141,7 @@
 				#interrupt-cells = <1>;
 				ranges = <0x82000000 0 0 0x82000000 0x2 0 1 0
 					  0x81000000 0 0 0x81000000 0x2 0 1 0>;
+				bus-range = <0x00 0xff>;
 				interrupt-map-mask = <0 0 0 0>;
 				interrupt-map = <0 0 0 0 &mpic 59>;
 				marvell,pcie-port = <0>;
@@ -148,7 +150,7 @@
 				status = "disabled";
 			};
 
-			pcie at 3,0 {
+			pcie3: pcie at 3,0 {
 				device_type = "pci";
 				assigned-addresses = <0x82000800 0 0x48000 0 0x2000>;
 				reg = <0x1800 0 0 0 0>;
@@ -157,6 +159,7 @@
 				#interrupt-cells = <1>;
 				ranges = <0x82000000 0 0 0x82000000 0x3 0 1 0
 					  0x81000000 0 0 0x81000000 0x3 0 1 0>;
+				bus-range = <0x00 0xff>;
 				interrupt-map-mask = <0 0 0 0>;
 				interrupt-map = <0 0 0 0 &mpic 60>;
 				marvell,pcie-port = <0>;
@@ -165,7 +168,7 @@
 				status = "disabled";
 			};
 
-			pcie at 4,0 {
+			pcie4: pcie at 4,0 {
 				device_type = "pci";
 				assigned-addresses = <0x82000800 0 0x4c000 0 0x2000>;
 				reg = <0x2000 0 0 0 0>;
@@ -174,6 +177,7 @@
 				#interrupt-cells = <1>;
 				ranges = <0x82000000 0 0 0x82000000 0x4 0 1 0
 					  0x81000000 0 0 0x81000000 0x4 0 1 0>;
+				bus-range = <0x00 0xff>;
 				interrupt-map-mask = <0 0 0 0>;
 				interrupt-map = <0 0 0 0 &mpic 61>;
 				marvell,pcie-port = <0>;
@@ -182,7 +186,7 @@
 				status = "disabled";
 			};
 
-			pcie at 5,0 {
+			pcie5: pcie at 5,0 {
 				device_type = "pci";
 				assigned-addresses = <0x82000800 0 0x80000 0 0x2000>;
 				reg = <0x2800 0 0 0 0>;
@@ -191,6 +195,7 @@
 				#interrupt-cells = <1>;
 				ranges = <0x82000000 0 0 0x82000000 0x5 0 1 0
 					  0x81000000 0 0 0x81000000 0x5 0 1 0>;
+				bus-range = <0x00 0xff>;
 				interrupt-map-mask = <0 0 0 0>;
 				interrupt-map = <0 0 0 0 &mpic 62>;
 				marvell,pcie-port = <1>;
diff --git a/arch/arm/dts/armada-xp-mv78260.dtsi b/arch/arm/dts/armada-xp-mv78260.dtsi
index c5fdc99f0d..d39231f69d 100644
--- a/arch/arm/dts/armada-xp-mv78260.dtsi
+++ b/arch/arm/dts/armada-xp-mv78260.dtsi
@@ -87,7 +87,7 @@
 		 * configured as x4 or quad x1 lanes. One unit is
 		 * x4 only.
 		 */
-		pcie-controller {
+		pciec: pcie at 82000000 {
 			compatible = "marvell,armada-xp-pcie";
 			status = "disabled";
 			device_type = "pci";
@@ -129,7 +129,7 @@
 				0x82000000 0x9 0     MBUS_ID(0x04, 0xf8) 0 1 0 /* Port 2.0 MEM */
 				0x81000000 0x9 0     MBUS_ID(0x04, 0xf0) 0 1 0 /* Port 2.0 IO  */>;
 
-			pcie at 1,0 {
+			pcie1: pcie at 1,0 {
 				device_type = "pci";
 				assigned-addresses = <0x82000800 0 0x40000 0 0x2000>;
 				reg = <0x0800 0 0 0 0>;
@@ -138,6 +138,7 @@
 				#interrupt-cells = <1>;
 				ranges = <0x82000000 0 0 0x82000000 0x1 0 1 0
 					  0x81000000 0 0 0x81000000 0x1 0 1 0>;
+				bus-range = <0x00 0xff>;
 				interrupt-map-mask = <0 0 0 0>;
 				interrupt-map = <0 0 0 0 &mpic 58>;
 				marvell,pcie-port = <0>;
@@ -146,7 +147,7 @@
 				status = "disabled";
 			};
 
-			pcie at 2,0 {
+			pcie2: pcie at 2,0 {
 				device_type = "pci";
 				assigned-addresses = <0x82000800 0 0x44000 0 0x2000>;
 				reg = <0x1000 0 0 0 0>;
@@ -155,6 +156,7 @@
 				#interrupt-cells = <1>;
 				ranges = <0x82000000 0 0 0x82000000 0x2 0 1 0
 					  0x81000000 0 0 0x81000000 0x2 0 1 0>;
+				bus-range = <0x00 0xff>;
 				interrupt-map-mask = <0 0 0 0>;
 				interrupt-map = <0 0 0 0 &mpic 59>;
 				marvell,pcie-port = <0>;
@@ -163,7 +165,7 @@
 				status = "disabled";
 			};
 
-			pcie at 3,0 {
+			pcie3: pcie at 3,0 {
 				device_type = "pci";
 				assigned-addresses = <0x82000800 0 0x48000 0 0x2000>;
 				reg = <0x1800 0 0 0 0>;
@@ -172,6 +174,7 @@
 				#interrupt-cells = <1>;
 				ranges = <0x82000000 0 0 0x82000000 0x3 0 1 0
 					  0x81000000 0 0 0x81000000 0x3 0 1 0>;
+				bus-range = <0x00 0xff>;
 				interrupt-map-mask = <0 0 0 0>;
 				interrupt-map = <0 0 0 0 &mpic 60>;
 				marvell,pcie-port = <0>;
@@ -180,7 +183,7 @@
 				status = "disabled";
 			};
 
-			pcie at 4,0 {
+			pcie4: pcie at 4,0 {
 				device_type = "pci";
 				assigned-addresses = <0x82000800 0 0x4c000 0 0x2000>;
 				reg = <0x2000 0 0 0 0>;
@@ -189,6 +192,7 @@
 				#interrupt-cells = <1>;
 				ranges = <0x82000000 0 0 0x82000000 0x4 0 1 0
 					  0x81000000 0 0 0x81000000 0x4 0 1 0>;
+				bus-range = <0x00 0xff>;
 				interrupt-map-mask = <0 0 0 0>;
 				interrupt-map = <0 0 0 0 &mpic 61>;
 				marvell,pcie-port = <0>;
@@ -197,7 +201,7 @@
 				status = "disabled";
 			};
 
-			pcie at 5,0 {
+			pcie5: pcie at 5,0 {
 				device_type = "pci";
 				assigned-addresses = <0x82000800 0 0x80000 0 0x2000>;
 				reg = <0x2800 0 0 0 0>;
@@ -206,6 +210,7 @@
 				#interrupt-cells = <1>;
 				ranges = <0x82000000 0 0 0x82000000 0x5 0 1 0
 					  0x81000000 0 0 0x81000000 0x5 0 1 0>;
+				bus-range = <0x00 0xff>;
 				interrupt-map-mask = <0 0 0 0>;
 				interrupt-map = <0 0 0 0 &mpic 62>;
 				marvell,pcie-port = <1>;
@@ -214,7 +219,7 @@
 				status = "disabled";
 			};
 
-			pcie at 6,0 {
+			pcie6: pcie at 6,0 {
 				device_type = "pci";
 				assigned-addresses = <0x82000800 0 0x84000 0 0x2000>;
 				reg = <0x3000 0 0 0 0>;
@@ -223,6 +228,7 @@
 				#interrupt-cells = <1>;
 				ranges = <0x82000000 0 0 0x82000000 0x6 0 1 0
 					  0x81000000 0 0 0x81000000 0x6 0 1 0>;
+				bus-range = <0x00 0xff>;
 				interrupt-map-mask = <0 0 0 0>;
 				interrupt-map = <0 0 0 0 &mpic 63>;
 				marvell,pcie-port = <1>;
@@ -231,7 +237,7 @@
 				status = "disabled";
 			};
 
-			pcie at 7,0 {
+			pcie7: pcie at 7,0 {
 				device_type = "pci";
 				assigned-addresses = <0x82000800 0 0x88000 0 0x2000>;
 				reg = <0x3800 0 0 0 0>;
@@ -240,6 +246,7 @@
 				#interrupt-cells = <1>;
 				ranges = <0x82000000 0 0 0x82000000 0x7 0 1 0
 					  0x81000000 0 0 0x81000000 0x7 0 1 0>;
+				bus-range = <0x00 0xff>;
 				interrupt-map-mask = <0 0 0 0>;
 				interrupt-map = <0 0 0 0 &mpic 64>;
 				marvell,pcie-port = <1>;
@@ -248,7 +255,7 @@
 				status = "disabled";
 			};
 
-			pcie at 8,0 {
+			pcie8: pcie at 8,0 {
 				device_type = "pci";
 				assigned-addresses = <0x82000800 0 0x8c000 0 0x2000>;
 				reg = <0x4000 0 0 0 0>;
@@ -257,6 +264,7 @@
 				#interrupt-cells = <1>;
 				ranges = <0x82000000 0 0 0x82000000 0x8 0 1 0
 					  0x81000000 0 0 0x81000000 0x8 0 1 0>;
+				bus-range = <0x00 0xff>;
 				interrupt-map-mask = <0 0 0 0>;
 				interrupt-map = <0 0 0 0 &mpic 65>;
 				marvell,pcie-port = <1>;
@@ -265,7 +273,7 @@
 				status = "disabled";
 			};
 
-			pcie at 9,0 {
+			pcie9: pcie at 9,0 {
 				device_type = "pci";
 				assigned-addresses = <0x82000800 0 0x42000 0 0x2000>;
 				reg = <0x4800 0 0 0 0>;
@@ -274,6 +282,7 @@
 				#interrupt-cells = <1>;
 				ranges = <0x82000000 0 0 0x82000000 0x9 0 1 0
 					  0x81000000 0 0 0x81000000 0x9 0 1 0>;
+				bus-range = <0x00 0xff>;
 				interrupt-map-mask = <0 0 0 0>;
 				interrupt-map = <0 0 0 0 &mpic 99>;
 				marvell,pcie-port = <2>;
diff --git a/arch/arm/dts/armada-xp-mv78460.dtsi b/arch/arm/dts/armada-xp-mv78460.dtsi
index 0e24f1a385..c642565d1b 100644
--- a/arch/arm/dts/armada-xp-mv78460.dtsi
+++ b/arch/arm/dts/armada-xp-mv78460.dtsi
@@ -104,7 +104,7 @@
 		 * configured as x4 or quad x1 lanes. Two units are
 		 * x4/x1.
 		 */
-		pcie-controller {
+		pciec: pcie at 82000000 {
 			compatible = "marvell,armada-xp-pcie";
 			status = "disabled";
 			device_type = "pci";
@@ -150,7 +150,7 @@
 				0x82000000 0xa 0     MBUS_ID(0x08, 0xf8) 0 1 0 /* Port 3.0 MEM */
 				0x81000000 0xa 0     MBUS_ID(0x08, 0xf0) 0 1 0 /* Port 3.0 IO  */>;
 
-			pcie at 1,0 {
+			pcie1: pcie at 1,0 {
 				device_type = "pci";
 				assigned-addresses = <0x82000800 0 0x40000 0 0x2000>;
 				reg = <0x0800 0 0 0 0>;
@@ -159,6 +159,7 @@
 				#interrupt-cells = <1>;
 				ranges = <0x82000000 0 0 0x82000000 0x1 0 1 0
 					  0x81000000 0 0 0x81000000 0x1 0 1 0>;
+				bus-range = <0x00 0xff>;
 				interrupt-map-mask = <0 0 0 0>;
 				interrupt-map = <0 0 0 0 &mpic 58>;
 				marvell,pcie-port = <0>;
@@ -167,7 +168,7 @@
 				status = "disabled";
 			};
 
-			pcie at 2,0 {
+			pcie2: pcie at 2,0 {
 				device_type = "pci";
 				assigned-addresses = <0x82001000 0 0x44000 0 0x2000>;
 				reg = <0x1000 0 0 0 0>;
@@ -176,6 +177,7 @@
 				#interrupt-cells = <1>;
 				ranges = <0x82000000 0 0 0x82000000 0x2 0 1 0
 					  0x81000000 0 0 0x81000000 0x2 0 1 0>;
+				bus-range = <0x00 0xff>;
 				interrupt-map-mask = <0 0 0 0>;
 				interrupt-map = <0 0 0 0 &mpic 59>;
 				marvell,pcie-port = <0>;
@@ -184,7 +186,7 @@
 				status = "disabled";
 			};
 
-			pcie at 3,0 {
+			pcie3: pcie at 3,0 {
 				device_type = "pci";
 				assigned-addresses = <0x82001800 0 0x48000 0 0x2000>;
 				reg = <0x1800 0 0 0 0>;
@@ -193,6 +195,7 @@
 				#interrupt-cells = <1>;
 				ranges = <0x82000000 0 0 0x82000000 0x3 0 1 0
 					  0x81000000 0 0 0x81000000 0x3 0 1 0>;
+				bus-range = <0x00 0xff>;
 				interrupt-map-mask = <0 0 0 0>;
 				interrupt-map = <0 0 0 0 &mpic 60>;
 				marvell,pcie-port = <0>;
@@ -201,7 +204,7 @@
 				status = "disabled";
 			};
 
-			pcie at 4,0 {
+			pcie4: pcie at 4,0 {
 				device_type = "pci";
 				assigned-addresses = <0x82002000 0 0x4c000 0 0x2000>;
 				reg = <0x2000 0 0 0 0>;
@@ -210,6 +213,7 @@
 				#interrupt-cells = <1>;
 				ranges = <0x82000000 0 0 0x82000000 0x4 0 1 0
 					  0x81000000 0 0 0x81000000 0x4 0 1 0>;
+				bus-range = <0x00 0xff>;
 				interrupt-map-mask = <0 0 0 0>;
 				interrupt-map = <0 0 0 0 &mpic 61>;
 				marvell,pcie-port = <0>;
@@ -218,7 +222,7 @@
 				status = "disabled";
 			};
 
-			pcie at 5,0 {
+			pcie5: pcie at 5,0 {
 				device_type = "pci";
 				assigned-addresses = <0x82002800 0 0x80000 0 0x2000>;
 				reg = <0x2800 0 0 0 0>;
@@ -227,6 +231,7 @@
 				#interrupt-cells = <1>;
 				ranges = <0x82000000 0 0 0x82000000 0x5 0 1 0
 					  0x81000000 0 0 0x81000000 0x5 0 1 0>;
+				bus-range = <0x00 0xff>;
 				interrupt-map-mask = <0 0 0 0>;
 				interrupt-map = <0 0 0 0 &mpic 62>;
 				marvell,pcie-port = <1>;
@@ -235,7 +240,7 @@
 				status = "disabled";
 			};
 
-			pcie at 6,0 {
+			pcie6: pcie at 6,0 {
 				device_type = "pci";
 				assigned-addresses = <0x82003000 0 0x84000 0 0x2000>;
 				reg = <0x3000 0 0 0 0>;
@@ -244,6 +249,7 @@
 				#interrupt-cells = <1>;
 				ranges = <0x82000000 0 0 0x82000000 0x6 0 1 0
 					  0x81000000 0 0 0x81000000 0x6 0 1 0>;
+				bus-range = <0x00 0xff>;
 				interrupt-map-mask = <0 0 0 0>;
 				interrupt-map = <0 0 0 0 &mpic 63>;
 				marvell,pcie-port = <1>;
@@ -252,7 +258,7 @@
 				status = "disabled";
 			};
 
-			pcie at 7,0 {
+			pcie7: pcie at 7,0 {
 				device_type = "pci";
 				assigned-addresses = <0x82003800 0 0x88000 0 0x2000>;
 				reg = <0x3800 0 0 0 0>;
@@ -261,6 +267,7 @@
 				#interrupt-cells = <1>;
 				ranges = <0x82000000 0 0 0x82000000 0x7 0 1 0
 					  0x81000000 0 0 0x81000000 0x7 0 1 0>;
+				bus-range = <0x00 0xff>;
 				interrupt-map-mask = <0 0 0 0>;
 				interrupt-map = <0 0 0 0 &mpic 64>;
 				marvell,pcie-port = <1>;
@@ -269,7 +276,7 @@
 				status = "disabled";
 			};
 
-			pcie at 8,0 {
+			pcie8: pcie at 8,0 {
 				device_type = "pci";
 				assigned-addresses = <0x82004000 0 0x8c000 0 0x2000>;
 				reg = <0x4000 0 0 0 0>;
@@ -278,6 +285,7 @@
 				#interrupt-cells = <1>;
 				ranges = <0x82000000 0 0 0x82000000 0x8 0 1 0
 					  0x81000000 0 0 0x81000000 0x8 0 1 0>;
+				bus-range = <0x00 0xff>;
 				interrupt-map-mask = <0 0 0 0>;
 				interrupt-map = <0 0 0 0 &mpic 65>;
 				marvell,pcie-port = <1>;
@@ -286,7 +294,7 @@
 				status = "disabled";
 			};
 
-			pcie at 9,0 {
+			pcie9: pcie at 9,0 {
 				device_type = "pci";
 				assigned-addresses = <0x82004800 0 0x42000 0 0x2000>;
 				reg = <0x4800 0 0 0 0>;
@@ -295,6 +303,7 @@
 				#interrupt-cells = <1>;
 				ranges = <0x82000000 0 0 0x82000000 0x9 0 1 0
 					  0x81000000 0 0 0x81000000 0x9 0 1 0>;
+				bus-range = <0x00 0xff>;
 				interrupt-map-mask = <0 0 0 0>;
 				interrupt-map = <0 0 0 0 &mpic 99>;
 				marvell,pcie-port = <2>;
@@ -303,7 +312,7 @@
 				status = "disabled";
 			};
 
-			pcie at 10,0 {
+			pcie10: pcie at a,0 {
 				device_type = "pci";
 				assigned-addresses = <0x82005000 0 0x82000 0 0x2000>;
 				reg = <0x5000 0 0 0 0>;
@@ -312,6 +321,7 @@
 				#interrupt-cells = <1>;
 				ranges = <0x82000000 0 0 0x82000000 0xa 0 1 0
 					  0x81000000 0 0 0x81000000 0xa 0 1 0>;
+				bus-range = <0x00 0xff>;
 				interrupt-map-mask = <0 0 0 0>;
 				interrupt-map = <0 0 0 0 &mpic 103>;
 				marvell,pcie-port = <3>;
-- 
2.20.1

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

* [U-Boot] [PATCH 3/3 v2] arm: mvebu: armada-xp-theadorable.dts: Enable PCIe DT nodes
  2019-01-18 11:46 [U-Boot] [PATCH] core: ofnode: Add ofnode_pci_get_devfn() Stefan Roese
                   ` (2 preceding siblings ...)
  2019-01-18 11:46 ` [U-Boot] [PATCH 2/3 v2] arm: mvebu: armada-xp/37x.dtsi: Sync PCIe DT nodes with Linux v4.20 Stefan Roese
@ 2019-01-18 11:46 ` Stefan Roese
  2019-01-21 18:15 ` [U-Boot] [PATCH] core: ofnode: Add ofnode_pci_get_devfn() Simon Glass
  4 siblings, 0 replies; 17+ messages in thread
From: Stefan Roese @ 2019-01-18 11:46 UTC (permalink / raw)
  To: u-boot

Now that the PCIe driver supports DM and DT parsing, enable the PCIe DT
nodes that are used by this board.

Signed-off-by: Stefan Roese <sr@denx.de>
Cc: Dirk Eibach <dirk.eibach@gdsys.cc>
Cc: Mario Six <mario.six@gdsys.cc>
Cc: Chris Packham <chris.packham@alliedtelesis.co.nz>
Cc: Phil Sutter <phil@nwl.cc>
Cc: Marek Behún <marek.behun@nic.cz>
Cc: VlaoMao <vlaomao@gmail.com>
---
v2:
- New file

 arch/arm/dts/armada-xp-theadorable.dts | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/arch/arm/dts/armada-xp-theadorable.dts b/arch/arm/dts/armada-xp-theadorable.dts
index 965c38426c..9b66ec678d 100644
--- a/arch/arm/dts/armada-xp-theadorable.dts
+++ b/arch/arm/dts/armada-xp-theadorable.dts
@@ -162,3 +162,17 @@
 		};
 	};
 };
+
+&pciec {
+	status = "okay";
+
+	pcie at 1,0 {
+		/* Port 0, Lane 0 */
+		status = "okay";
+	};
+
+	pcie at 9,0 {
+		/* Port 2, Lane 0 */
+		status = "okay";
+	};
+};
-- 
2.20.1

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

* [U-Boot] [PATCH] core: ofnode: Add ofnode_pci_get_devfn()
  2019-01-18 11:46 [U-Boot] [PATCH] core: ofnode: Add ofnode_pci_get_devfn() Stefan Roese
                   ` (3 preceding siblings ...)
  2019-01-18 11:46 ` [U-Boot] [PATCH 3/3 v2] arm: mvebu: armada-xp-theadorable.dts: Enable PCIe DT nodes Stefan Roese
@ 2019-01-21 18:15 ` Simon Glass
  2019-01-22  9:36   ` Stefan Roese
  4 siblings, 1 reply; 17+ messages in thread
From: Simon Glass @ 2019-01-21 18:15 UTC (permalink / raw)
  To: u-boot

Hi Stefan,

On Sat, 19 Jan 2019 at 00:46, Stefan Roese <sr@denx.de> wrote:
>
> This function will be used by the Marvell Armada XP/38x PCIe driver,
> which is moved to DM right now. It's mostly copied from the Linux
> version.
>
> Signed-off-by: Stefan Roese <sr@denx.de>
> Cc: Simon Glass <sjg@chromium.org>
> ---
>  drivers/core/ofnode.c | 12 ++++++++++++
>  include/dm/ofnode.h   | 11 +++++++++++
>  2 files changed, 23 insertions(+)

The code to do this right now is in pci_uclass_child_post_bind(). Do
you think you could break that out into a pci_... function that you
can call from your new function?

Also I had a look at the code you wrote that calls this. Ideally we
would have the PCIe nodes have their own driver so that driver model
will automatically bind them, but I am not sure that is feasible.

Regards,
Simon


>
> diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c
> index 0e584c12dc..b74b95ff61 100644
> --- a/drivers/core/ofnode.c
> +++ b/drivers/core/ofnode.c
> @@ -666,6 +666,18 @@ int ofnode_read_pci_vendev(ofnode node, u16 *vendor, u16 *device)
>         return -ENOENT;
>  }
>
> +int ofnode_pci_get_devfn(ofnode node)
> +{
> +       u32 reg[5];
> +       int error;
> +
> +       error = ofnode_read_u32_array(node, "reg", reg, ARRAY_SIZE(reg));
> +       if (error)
> +               return error;
> +
> +       return (reg[0] >> 8) & 0xff;
> +}
> +
>  int ofnode_read_addr_cells(ofnode node)
>  {
>         if (ofnode_is_np(node))
> diff --git a/include/dm/ofnode.h b/include/dm/ofnode.h
> index d206ee2caa..66d7f8d055 100644
> --- a/include/dm/ofnode.h
> +++ b/include/dm/ofnode.h
> @@ -608,6 +608,17 @@ int ofnode_read_pci_addr(ofnode node, enum fdt_pci_space type,
>   */
>  int ofnode_read_pci_vendev(ofnode node, u16 *vendor, u16 *device);
>
> +/**
> + * ofnode_pci_get_devfn() - Get device and function numbers for a device node
> + * @ofnode: node to examine
> + *
> + * Parses a standard 5-cell PCI resource and returns an 8-bit value that can
> + * be passed to the PCI_SLOT() and PCI_FUNC() macros to extract the device
> + * and function numbers respectively. On error a negative error code is
> + * returned.
> + */
> +int ofnode_pci_get_devfn(ofnode node);
> +
>  /**
>   * ofnode_read_addr_cells() - Get the number of address cells for a node
>   *
> --
> 2.20.1
>

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

* [U-Boot] [PATCH] pci: Add PCI_SLOT macro to include/pci.h
  2019-01-18 11:46 ` [U-Boot] [PATCH] pci: Add PCI_SLOT macro to include/pci.h Stefan Roese
@ 2019-01-22  0:32   ` Simon Glass
  2019-01-22  2:11     ` Bin Meng
  2019-01-22  9:53     ` Stefan Roese
  0 siblings, 2 replies; 17+ messages in thread
From: Simon Glass @ 2019-01-22  0:32 UTC (permalink / raw)
  To: u-boot

Hi Stefan,

On Sat, 19 Jan 2019 at 00:46, Stefan Roese <sr@denx.de> wrote:
>
> This macro will be used the by the Marvell Armada XP/38x PCIe driver,
> which is moved to DM right now.
>
> Signed-off-by: Stefan Roese <sr@denx.de>
> Cc: Bin Meng <bmeng.cn@gmail.com>
> Cc: Simon Glass <sjg@chromium.org>
> ---
>  include/pci.h | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/include/pci.h b/include/pci.h
> index 785d7d28b7..f4a9e025b3 100644
> --- a/include/pci.h
> +++ b/include/pci.h
> @@ -501,6 +501,7 @@ typedef int pci_dev_t;
>  #define PCI_BUS(d)             (((d) >> 16) & 0xff)
>  #define PCI_DEV(d)             (((d) >> 11) & 0x1f)
>  #define PCI_FUNC(d)            (((d) >> 8) & 0x7)
> +#define PCI_SLOT(d)            (((d) >> 3) & 0x1f)

This seems unrelated to the other macros, since is shifts left only 3
positions. Can you perhaps move it to the end and add a comment as to
what the input is and what it returns? It seems different to the
others.

>  #define PCI_DEVFN(d, f)                ((d) << 11 | (f) << 8)
>  #define PCI_MASK_BUS(bdf)      ((bdf) & 0xffff)
>  #define PCI_ADD_BUS(bus, devfn)        (((bus) << 16) | (devfn))
> --
> 2.20.1
>

Regards,
Simon

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

* [U-Boot] [PATCH] pci: Add PCI_SLOT macro to include/pci.h
  2019-01-22  0:32   ` Simon Glass
@ 2019-01-22  2:11     ` Bin Meng
  2019-01-22  9:54       ` Stefan Roese
  2019-01-22  9:53     ` Stefan Roese
  1 sibling, 1 reply; 17+ messages in thread
From: Bin Meng @ 2019-01-22  2:11 UTC (permalink / raw)
  To: u-boot

Hi Stefan,

On Tue, Jan 22, 2019 at 8:32 AM Simon Glass <sjg@chromium.org> wrote:
>
> Hi Stefan,
>
> On Sat, 19 Jan 2019 at 00:46, Stefan Roese <sr@denx.de> wrote:
> >
> > This macro will be used the by the Marvell Armada XP/38x PCIe driver,
> > which is moved to DM right now.
> >
> > Signed-off-by: Stefan Roese <sr@denx.de>
> > Cc: Bin Meng <bmeng.cn@gmail.com>
> > Cc: Simon Glass <sjg@chromium.org>
> > ---

It's weird I did not receive this email in my inbox.

> >  include/pci.h | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/include/pci.h b/include/pci.h
> > index 785d7d28b7..f4a9e025b3 100644
> > --- a/include/pci.h
> > +++ b/include/pci.h
> > @@ -501,6 +501,7 @@ typedef int pci_dev_t;
> >  #define PCI_BUS(d)             (((d) >> 16) & 0xff)
> >  #define PCI_DEV(d)             (((d) >> 11) & 0x1f)
> >  #define PCI_FUNC(d)            (((d) >> 8) & 0x7)
> > +#define PCI_SLOT(d)            (((d) >> 3) & 0x1f)
>
> This seems unrelated to the other macros, since is shifts left only 3
> positions. Can you perhaps move it to the end and add a comment as to
> what the input is and what it returns? It seems different to the
> others.

Agreed with Simon. Do you have any follow-up patches that will use
this macro for better understanding?

>
> >  #define PCI_DEVFN(d, f)                ((d) << 11 | (f) << 8)
> >  #define PCI_MASK_BUS(bdf)      ((bdf) & 0xffff)
> >  #define PCI_ADD_BUS(bus, devfn)        (((bus) << 16) | (devfn))
> > --

Regards,
Bin

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

* [U-Boot] [PATCH 1/3 v2] pci: pci_mvebu: Add DM_PCI support and move CONFIG_PCI_MVEBU to defconfig
  2019-01-18 11:46 ` [U-Boot] [PATCH 1/3 v2] pci: pci_mvebu: Add DM_PCI support and move CONFIG_PCI_MVEBU to defconfig Stefan Roese
@ 2019-01-22  7:35   ` Chris Packham
  2019-01-22 10:06     ` Stefan Roese
  0 siblings, 1 reply; 17+ messages in thread
From: Chris Packham @ 2019-01-22  7:35 UTC (permalink / raw)
  To: u-boot

Hi Stefan,
On Sat, Jan 19, 2019 at 12:51 AM Stefan Roese <sr@denx.de> wrote:
>
> This patch adds DM_PCI support to the MVEBU PCIe driver. This is
> necessary, since all PCI drivers have to be moved to DM (driver model)
> until the v2019.07 release.
>
> To not break git bisect'ablility, this patch also moves CONFIG_PCI_MVEBU
> from config headers to the defconfig files.
>
> Signed-off-by: Stefan Roese <sr@denx.de>
> Cc: Dirk Eibach <dirk.eibach@gdsys.cc>
> Cc: Mario Six <mario.six@gdsys.cc>
> Cc: Chris Packham <chris.packham@alliedtelesis.co.nz>
> Cc: Phil Sutter <phil@nwl.cc>
> Cc: Marek Behún <marek.behun@nic.cz>
> Cc: VlaoMao <vlaomao@gmail.com>

A couple of minor comments below. With them taken care of

Reviewed-by: Chris Packham <judge.packham@gmail.com>
Tested-by: Chris Packham <judge.packham@gmail.com>

> ---
> v2:
> - Completely configure the controller based on DT properties. Now
>   port and lane are read from the DT nodes and also the tgt and
>   attr values are generated using the DT similar to how this is
>   done in the Linux driver version. All A38x / XP specific defines
>   can now be removed from this driver because of this.
> - Please note that now the board specific dts file needs to enable
>   the used PCIe ports, as this is also done in Linux.
>
>  configs/clearfog_defconfig          |   1 +
>  configs/controlcenterdc_defconfig   |   3 +
>  configs/db-88f6820-amc_defconfig    |   1 +
>  configs/db-88f6820-gp_defconfig     |   1 +
>  configs/db-mv784mp-gp_defconfig     |   1 +
>  configs/ds414_defconfig             |   1 +
>  configs/theadorable_debug_defconfig |   2 +
>  configs/turris_omnia_defconfig      |   3 +-
>  drivers/pci/Kconfig                 |   9 +
>  drivers/pci/pci_mvebu.c             | 457 ++++++++++++++++------------
>  include/configs/clearfog.h          |   1 -
>  include/configs/controlcenterdc.h   |   3 -
>  include/configs/db-88f6820-amc.h    |   1 -
>  include/configs/db-88f6820-gp.h     |   1 -
>  include/configs/db-mv784mp-gp.h     |   1 -
>  include/configs/ds414.h             |   1 -
>  include/configs/theadorable.h       |   7 -
>  include/configs/turris_omnia.h      |   1 -
>  scripts/config_whitelist.txt        |   1 -
>  19 files changed, 287 insertions(+), 209 deletions(-)
>
> diff --git a/configs/clearfog_defconfig b/configs/clearfog_defconfig
> index e1c5a1fa13..0f69ff606d 100644
> --- a/configs/clearfog_defconfig
> +++ b/configs/clearfog_defconfig
> @@ -56,6 +56,7 @@ CONFIG_PHY_GIGE=y
>  CONFIG_MVNETA=y
>  CONFIG_MII=y
>  CONFIG_PCI=y
> +CONFIG_PCI_MVEBU=y
>  CONFIG_SCSI=y
>  CONFIG_DEBUG_UART_SHIFT=2
>  CONFIG_SYS_NS16550=y
> diff --git a/configs/controlcenterdc_defconfig b/configs/controlcenterdc_defconfig
> index 0c762ad77c..f22a3ab819 100644
> --- a/configs/controlcenterdc_defconfig
> +++ b/configs/controlcenterdc_defconfig
> @@ -58,6 +58,9 @@ CONFIG_PHY_MARVELL=y
>  CONFIG_PHY_GIGE=y
>  CONFIG_MVNETA=y
>  CONFIG_MII=y
> +CONFIG_PCI=y
> +CONFIG_DM_PCI_COMPAT=y
> +CONFIG_PCI_MVEBU=y
>  CONFIG_SCSI=y
>  CONFIG_DEBUG_UART_SHIFT=2
>  CONFIG_SYS_NS16550=y
> diff --git a/configs/db-88f6820-amc_defconfig b/configs/db-88f6820-amc_defconfig
> index 9068a58406..03bf2bcafc 100644
> --- a/configs/db-88f6820-amc_defconfig
> +++ b/configs/db-88f6820-amc_defconfig
> @@ -60,6 +60,7 @@ CONFIG_PHY_GIGE=y
>  CONFIG_MVNETA=y
>  CONFIG_MII=y
>  CONFIG_PCI=y
> +CONFIG_PCI_MVEBU=y
>  CONFIG_DEBUG_UART_SHIFT=2
>  CONFIG_SYS_NS16550=y
>  CONFIG_KIRKWOOD_SPI=y
> diff --git a/configs/db-88f6820-gp_defconfig b/configs/db-88f6820-gp_defconfig
> index 413010c4ef..b36b7c3b2c 100644
> --- a/configs/db-88f6820-gp_defconfig
> +++ b/configs/db-88f6820-gp_defconfig
> @@ -56,6 +56,7 @@ CONFIG_PHY_GIGE=y
>  CONFIG_MVNETA=y
>  CONFIG_MII=y
>  CONFIG_PCI=y
> +CONFIG_PCI_MVEBU=y
>  CONFIG_SCSI=y
>  CONFIG_DEBUG_UART_SHIFT=2
>  CONFIG_SYS_NS16550=y
> diff --git a/configs/db-mv784mp-gp_defconfig b/configs/db-mv784mp-gp_defconfig
> index 3f140986b6..b6c61c3a48 100644
> --- a/configs/db-mv784mp-gp_defconfig
> +++ b/configs/db-mv784mp-gp_defconfig
> @@ -55,6 +55,7 @@ CONFIG_PHY_GIGE=y
>  CONFIG_MVNETA=y
>  CONFIG_MII=y
>  CONFIG_PCI=y
> +CONFIG_PCI_MVEBU=y
>  CONFIG_DEBUG_UART_SHIFT=2
>  CONFIG_SYS_NS16550=y
>  CONFIG_KIRKWOOD_SPI=y
> diff --git a/configs/ds414_defconfig b/configs/ds414_defconfig
> index 5325bd9968..32f37440c3 100644
> --- a/configs/ds414_defconfig
> +++ b/configs/ds414_defconfig
> @@ -52,6 +52,7 @@ CONFIG_PHY_GIGE=y
>  CONFIG_MVNETA=y
>  CONFIG_MII=y
>  CONFIG_PCI=y
> +CONFIG_PCI_MVEBU=y
>  CONFIG_DEBUG_UART_SHIFT=2
>  CONFIG_SYS_NS16550=y
>  CONFIG_KIRKWOOD_SPI=y
> diff --git a/configs/theadorable_debug_defconfig b/configs/theadorable_debug_defconfig
> index 9e99618998..ac6dfd6844 100644
> --- a/configs/theadorable_debug_defconfig
> +++ b/configs/theadorable_debug_defconfig
> @@ -62,6 +62,8 @@ CONFIG_PHY_GIGE=y
>  CONFIG_MVNETA=y
>  CONFIG_MII=y
>  CONFIG_PCI=y
> +CONFIG_DM_PCI_COMPAT=y
> +CONFIG_PCI_MVEBU=y
>  CONFIG_DEBUG_UART_SHIFT=2
>  CONFIG_SYS_NS16550=y
>  CONFIG_KIRKWOOD_SPI=y
> diff --git a/configs/turris_omnia_defconfig b/configs/turris_omnia_defconfig
> index 4a1e23c86c..5cafe2ee34 100644
> --- a/configs/turris_omnia_defconfig
> +++ b/configs/turris_omnia_defconfig
> @@ -37,7 +37,6 @@ CONFIG_DEFAULT_DEVICE_TREE="armada-385-turris-omnia"
>  CONFIG_ENV_IS_IN_SPI_FLASH=y
>  CONFIG_SPL_OF_TRANSLATE=y
>  CONFIG_SCSI_AHCI=y
> -CONFIG_MISC=y
>  CONFIG_ATSHA204A=y
>  CONFIG_MMC_SDHCI=y
>  CONFIG_MMC_SDHCI_MV=y
> @@ -45,6 +44,8 @@ CONFIG_PHY_MARVELL=y
>  CONFIG_PHY_GIGE=y
>  CONFIG_MVNETA=y
>  CONFIG_MII=y
> +CONFIG_PCI=y
> +CONFIG_PCI_MVEBU=y
>  CONFIG_DEBUG_UART_SHIFT=2
>  CONFIG_SYS_NS16550=y
>  CONFIG_KIRKWOOD_SPI=y

Now that it's been merged you'll need the following for the x530_defconfig

---8<---
Subject: [PATCH] fixup! pci: pci_mvebu: Add DM_PCI support and move
 CONFIG_PCI_MVEBU to defconfig

---
 configs/x530_defconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/configs/x530_defconfig b/configs/x530_defconfig
index 22482f8d39be..6014aaa32134 100644
--- a/configs/x530_defconfig
+++ b/configs/x530_defconfig
@@ -57,6 +57,8 @@ CONFIG_SPI_FLASH_STMICRO=y
 CONFIG_SPI_FLASH_SST=y
 CONFIG_MTD_UBI=y
 CONFIG_PCI=y
+CONFIG_DM_PCI=y
+CONFIG_DM_PCI_COMPAT=y
 CONFIG_DM_RTC=y
 CONFIG_RTC_DS1307=y
 CONFIG_DEBUG_UART_SHIFT=2
-- 
---8<---

> diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
> index f59803dbd6..1521885bde 100644
> --- a/drivers/pci/Kconfig
> +++ b/drivers/pci/Kconfig
> @@ -112,4 +112,13 @@ config PCIE_INTEL_FPGA
>           Say Y here if you want to enable PCIe controller support on Intel
>           FPGA, example Stratix 10.
>
> +config PCI_MVEBU
> +       bool "Enable Armada XP/38x PCIe driver"
> +       depends on ARCH_MVEBU
> +       select DM_PCI
> +       select MISC
> +       help
> +         Say Y here if you want to enable PCIe controller support on
> +         Armada XP/38x SoCs.
> +
>  endif
> diff --git a/drivers/pci/pci_mvebu.c b/drivers/pci/pci_mvebu.c
> index affe9f98ad..a03b979692 100644
> --- a/drivers/pci/pci_mvebu.c
> +++ b/drivers/pci/pci_mvebu.c
> @@ -10,11 +10,16 @@
>   */
>
>  #include <common.h>
> +#include <dm.h>
> +#include <dm/device-internal.h>
> +#include <dm/lists.h>
> +#include <dm/of_access.h>
>  #include <pci.h>
> -#include <linux/errno.h>
>  #include <asm/io.h>
>  #include <asm/arch/cpu.h>
>  #include <asm/arch/soc.h>
> +#include <linux/errno.h>
> +#include <linux/ioport.h>
>  #include <linux/mbus.h>
>
>  DECLARE_GLOBAL_DATA_PTR;
> @@ -59,26 +64,22 @@ DECLARE_GLOBAL_DATA_PTR;
>  #define PCIE_DEBUG_CTRL                        0x1a60
>  #define  PCIE_DEBUG_SOFT_RESET         BIT(20)
>
> -struct resource {
> -       u32 start;
> -       u32 end;
> -};
> -
>  struct mvebu_pcie {
>         struct pci_controller hose;
> -       char *name;
>         void __iomem *base;
>         void __iomem *membase;
>         struct resource mem;
>         void __iomem *iobase;
>         u32 port;
>         u32 lane;
> +       int devfn;
>         u32 lane_mask;
>         pci_dev_t dev;
> +       char name[16];
> +       unsigned int mem_target;
> +       unsigned int mem_attr;
>  };
>
> -#define to_pcie(_hc)   container_of(_hc, struct mvebu_pcie, pci)
> -
>  /*
>   * MVEBU PCIe controller needs MEMORY and I/O BARs to be mapped
>   * into SoCs address space. Each controller will map 128M of MEM
> @@ -87,82 +88,6 @@ struct mvebu_pcie {
>  static void __iomem *mvebu_pcie_membase = (void __iomem *)MBUS_PCI_MEM_BASE;
>  #define PCIE_MEM_SIZE  (128 << 20)
>
> -#if defined(CONFIG_ARMADA_38X)
> -#define PCIE_BASE(if)                                  \
> -       ((if) == 0 ?                                    \
> -        MVEBU_REG_PCIE0_BASE :                         \
> -        (MVEBU_REG_PCIE_BASE + 0x4000 * (if - 1)))
> -
> -/*
> - * On A38x MV6820 these PEX ports are supported:
> - *  0 - Port 0.0
> - *  1 - Port 1.0
> - *  2 - Port 2.0
> - *  3 - Port 3.0
> - */
> -#define MAX_PEX 4
> -static struct mvebu_pcie pcie_bus[MAX_PEX];
> -
> -static void mvebu_get_port_lane(struct mvebu_pcie *pcie, int pex_idx,
> -                               int *mem_target, int *mem_attr)
> -{
> -       u8 port[] = { 0, 1, 2, 3 };
> -       u8 lane[] = { 0, 0, 0, 0 };
> -       u8 target[] = { 8, 4, 4, 4 };
> -       u8 attr[] = { 0xe8, 0xe8, 0xd8, 0xb8 };
> -
> -       pcie->port = port[pex_idx];
> -       pcie->lane = lane[pex_idx];
> -       *mem_target = target[pex_idx];
> -       *mem_attr = attr[pex_idx];
> -}
> -#else
> -#define PCIE_BASE(if)                                                  \
> -       ((if) < 8 ?                                                     \
> -        (MVEBU_REG_PCIE_BASE + ((if) / 4) * 0x40000 + ((if) % 4) * 0x4000) : \
> -        (MVEBU_REG_PCIE_BASE + 0x2000 + ((if) % 8) * 0x40000))
> -
> -/*
> - * On AXP MV78460 these PEX ports are supported:
> - *  0 - Port 0.0
> - *  1 - Port 0.1
> - *  2 - Port 0.2
> - *  3 - Port 0.3
> - *  4 - Port 1.0
> - *  5 - Port 1.1
> - *  6 - Port 1.2
> - *  7 - Port 1.3
> - *  8 - Port 2.0
> - *  9 - Port 3.0
> - */
> -#define MAX_PEX 10
> -static struct mvebu_pcie pcie_bus[MAX_PEX];
> -
> -static void mvebu_get_port_lane(struct mvebu_pcie *pcie, int pex_idx,
> -                               int *mem_target, int *mem_attr)
> -{
> -       u8 port[] = { 0, 0, 0, 0, 1, 1, 1, 1, 2, 3 };
> -       u8 lane[] = { 0, 1, 2, 3, 0, 1, 2, 3, 0, 0 };
> -       u8 target[] = { 4, 4, 4, 4, 8, 8, 8, 8, 4, 8 };
> -       u8 attr[] = { 0xe8, 0xd8, 0xb8, 0x78,
> -                     0xe8, 0xd8, 0xb8, 0x78,
> -                     0xf8, 0xf8 };
> -
> -       pcie->port = port[pex_idx];
> -       pcie->lane = lane[pex_idx];
> -       *mem_target = target[pex_idx];
> -       *mem_attr = attr[pex_idx];
> -}
> -#endif
> -
> -static int mvebu_pex_unit_is_x4(int pex_idx)
> -{
> -       int pex_unit = pex_idx < 9 ? pex_idx >> 2 : 3;
> -       u32 mask = (0x0f << (pex_unit * 8));
> -
> -       return (readl(COMPHY_REFCLK_ALIGNMENT) & mask) == mask;
> -}
> -
>  static inline bool mvebu_pcie_link_up(struct mvebu_pcie *pcie)
>  {
>         u32 val;
> @@ -211,67 +136,83 @@ static inline struct mvebu_pcie *hose_to_pcie(struct pci_controller *hose)
>         return container_of(hose, struct mvebu_pcie, hose);
>  }
>
> -static int mvebu_pcie_read_config_dword(struct pci_controller *hose,
> -               pci_dev_t dev, int offset, u32 *val)
> +static int mvebu_pcie_read_config(struct udevice *bus, pci_dev_t bdf,
> +                                 uint offset, ulong *valuep,
> +                                 enum pci_size_t size)
>  {
> -       struct mvebu_pcie *pcie = hose_to_pcie(hose);
> +       struct mvebu_pcie *pcie = dev_get_platdata(bus);
>         int local_bus = PCI_BUS(pcie->dev);
>         int local_dev = PCI_DEV(pcie->dev);
>         u32 reg;
> +       u32 data;
> +
> +       debug("PCIE CFG read:  (b,d,f)=(%2d,%2d,%2d) ",
> +             PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf));
>
>         /* Only allow one other device besides the local one on the local bus */
> -       if (PCI_BUS(dev) == local_bus && PCI_DEV(dev) != local_dev) {
> -               if (local_dev == 0 && PCI_DEV(dev) != 1) {
> +       if (PCI_BUS(bdf) == local_bus && PCI_DEV(bdf) != local_dev) {
> +               if (local_dev == 0 && PCI_DEV(bdf) != 1) {
> +                       debug("- out of range\n");
>                         /*
>                          * If local dev is 0, the first other dev can
>                          * only be 1
>                          */
> -                       *val = 0xffffffff;
> -                       return 1;
> -               } else if (local_dev != 0 && PCI_DEV(dev) != 0) {
> +                       *valuep = pci_get_ff(size);
> +                       return 0;
> +               } else if (local_dev != 0 && PCI_DEV(bdf) != 0) {
> +                       debug("- out of range\n");
>                         /*
>                          * If local dev is not 0, the first other dev can
>                          * only be 0
>                          */
> -                       *val = 0xffffffff;
> -                       return 1;
> +                       *valuep = pci_get_ff(size);
> +                       return 0;
>                 }
>         }
>
>         /* write address */
> -       reg = PCIE_CONF_ADDR(dev, offset);
> +       reg = PCIE_CONF_ADDR(bdf, offset);
>         writel(reg, pcie->base + PCIE_CONF_ADDR_OFF);
> -       *val = readl(pcie->base + PCIE_CONF_DATA_OFF);
> +       data = readl(pcie->base + PCIE_CONF_DATA_OFF);
> +       debug("(addr,val)=(0x%04x, 0x%08x)\n", offset, data);
> +       *valuep = pci_conv_32_to_size(data, offset, size);
>
>         return 0;
>  }
>
> -static int mvebu_pcie_write_config_dword(struct pci_controller *hose,
> -               pci_dev_t dev, int offset, u32 val)
> +static int mvebu_pcie_write_config(struct udevice *bus, pci_dev_t bdf,
> +                                  uint offset, ulong value,
> +                                  enum pci_size_t size)
>  {
> -       struct mvebu_pcie *pcie = hose_to_pcie(hose);
> +       struct mvebu_pcie *pcie = dev_get_platdata(bus);
>         int local_bus = PCI_BUS(pcie->dev);
>         int local_dev = PCI_DEV(pcie->dev);
> +       u32 data;
> +
> +       debug("PCIE CFG write: (b,d,f)=(%2d,%2d,%2d) ",
> +             PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf));
> +       debug("(addr,val)=(0x%04x, 0x%08lx)\n", offset, value);
>
>         /* Only allow one other device besides the local one on the local bus */
> -       if (PCI_BUS(dev) == local_bus && PCI_DEV(dev) != local_dev) {
> -               if (local_dev == 0 && PCI_DEV(dev) != 1) {
> +       if (PCI_BUS(bdf) == local_bus && PCI_DEV(bdf) != local_dev) {
> +               if (local_dev == 0 && PCI_DEV(bdf) != 1) {
>                         /*
>                          * If local dev is 0, the first other dev can
>                          * only be 1
>                          */
> -                       return 1;
> -               } else if (local_dev != 0 && PCI_DEV(dev) != 0) {
> +                       return 0;
> +               } else if (local_dev != 0 && PCI_DEV(bdf) != 0) {
>                         /*
>                          * If local dev is not 0, the first other dev can
>                          * only be 0
>                          */
> -                       return 1;
> +                       return 0;
>                 }
>         }
>
> -       writel(PCIE_CONF_ADDR(dev, offset), pcie->base + PCIE_CONF_ADDR_OFF);
> -       writel(val, pcie->base + PCIE_CONF_DATA_OFF);
> +       writel(PCIE_CONF_ADDR(bdf, offset), pcie->base + PCIE_CONF_ADDR_OFF);
> +       data = pci_conv_size_to_32(0, value, offset, size);
> +       writel(data, pcie->base + PCIE_CONF_DATA_OFF);
>
>         return 0;
>  }
> @@ -331,107 +272,241 @@ static void mvebu_pcie_setup_wins(struct mvebu_pcie *pcie)
>                pcie->base + PCIE_BAR_CTRL_OFF(1));
>  }
>
> -void pci_init_board(void)
> +/**
> + * pcie_dw_mvebu_probe() - Probe the PCIe bus for active link

Typo? Should this be be mvebu_pcie_probe? Or nothing since the
function name is just below.

> + *
> + * @dev: A pointer to the device being operated on
> + *
> + * Probe for an active link on the PCIe bus and configure the controller
> + * to enable this port.
> + *
> + * Return: 0 on success, else -ENODEV
> + */
> +static int mvebu_pcie_probe(struct udevice *dev)
>  {
> -       int mem_target, mem_attr, i;
> -       int bus = 0;
> +       struct mvebu_pcie *pcie = dev_get_platdata(dev);
> +       struct udevice *ctlr = pci_get_controller(dev);
> +       struct pci_controller *hose = dev_get_uclass_priv(ctlr);
> +       static int bus;
>         u32 reg;
> -       u32 soc_ctrl = readl(MVEBU_SYSTEM_REG_BASE + 0x4);
>
> -       /* Check SoC Control Power State */
> -       debug("%s: SoC Control %08x, 0en %01lx, 1en %01lx, 2en %01lx\n",
> -             __func__, soc_ctrl, SELECT(soc_ctrl, 0), SELECT(soc_ctrl, 1),
> -             SELECT(soc_ctrl, 2));
> +       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));
> +
> +       mvebu_pcie_set_local_bus_nr(pcie, bus);
> +       mvebu_pcie_set_local_dev_nr(pcie, 0);
> +       pcie->dev = PCI_BDF(bus, 0, 0);
> +
> +       pcie->mem.start = (u32)mvebu_pcie_membase;
> +       pcie->mem.end = pcie->mem.start + PCIE_MEM_SIZE - 1;
> +       mvebu_pcie_membase += PCIE_MEM_SIZE;
> +
> +       if (mvebu_mbus_add_window_by_id(pcie->mem_target, pcie->mem_attr,
> +                                       (phys_addr_t)pcie->mem.start,
> +                                       PCIE_MEM_SIZE)) {
> +               printf("PCIe unable to add mbus window for mem at %08x+%08x\n",
> +                      (u32)pcie->mem.start, PCIE_MEM_SIZE);
> +       }
> +
> +       /* 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_MASTER;
> +       reg |= BIT(10);         /* disable interrupts */
> +       writel(reg, pcie->base + PCIE_CMD_OFF);
> +
> +       /* Set BAR0 to internal registers */
> +       writel(SOC_REGS_PHY_BASE, pcie->base + PCIE_BAR_LO_OFF(0));
> +       writel(0, pcie->base + PCIE_BAR_HI_OFF(0));
>
> -       for (i = 0; i < MAX_PEX; i++) {
> -               struct mvebu_pcie *pcie = &pcie_bus[i];
> -               struct pci_controller *hose = &pcie->hose;
> +       /* PCI memory space */
> +       pci_set_region(hose->regions + 0, pcie->mem.start,
> +                      pcie->mem.start, PCIE_MEM_SIZE, PCI_REGION_MEM);
> +       pci_set_region(hose->regions + 1,
> +                      0, 0,
> +                      gd->ram_size,
> +                      PCI_REGION_MEM | PCI_REGION_SYS_MEMORY);
> +       hose->region_count = 2;
> +
> +       bus++;
> +
> +       return 0;
> +}
> +
> +static const struct dm_pci_ops mvebu_pcie_ops = {
> +       .read_config    = mvebu_pcie_read_config,
> +       .write_config   = mvebu_pcie_write_config,
> +};
> +
> +static struct driver pcie_mvebu_drv = {
> +       .name                   = "pcie_mvebu",
> +       .id                     = UCLASS_PCI,
> +       .ops                    = &mvebu_pcie_ops,
> +       .probe                  = mvebu_pcie_probe,
> +       .platdata_auto_alloc_size = sizeof(struct mvebu_pcie),
> +};
> +
> +static int mvebu_pcie_port_parse_dt(ofnode node, struct mvebu_pcie *pcie)
> +{
> +       const u32 *addr;
> +       int len;
> +
> +       addr = ofnode_get_property(node, "assigned-addresses", &len);
> +       if (!addr) {
> +               pr_err("property \"assigned-addresses\" not found");
> +               return -FDT_ERR_NOTFOUND;
> +       }
> +
> +       pcie->base = (void *)(fdt32_to_cpu(addr[2]) + SOC_REGS_PHY_BASE);
> +
> +       return 0;
> +}
> +
> +#define DT_FLAGS_TO_TYPE(flags)       (((flags) >> 24) & 0x03)
> +#define    DT_TYPE_IO                 0x1
> +#define    DT_TYPE_MEM32              0x2
> +#define DT_CPUADDR_TO_TARGET(cpuaddr) (((cpuaddr) >> 56) & 0xFF)
> +#define DT_CPUADDR_TO_ATTR(cpuaddr)   (((cpuaddr) >> 48) & 0xFF)
> +
> +static int mvebu_get_tgt_attr(ofnode node, int devfn,
> +                             unsigned long type,
> +                             unsigned int *tgt,
> +                             unsigned int *attr)
> +{
> +       const int na = 3, ns = 2;
> +       const __be32 *range;
> +       int rlen, nranges, rangesz, pna, i;
> +
> +       *tgt = -1;
> +       *attr = -1;
> +
> +       range = ofnode_get_property(node, "ranges", &rlen);
> +       if (!range)
> +               return -EINVAL;
> +
> +       pna = 2; /* hardcoded for now because of lack of of_n_addr_cells() */
> +       rangesz = pna + na + ns;
> +       nranges = rlen / sizeof(__be32) / rangesz;
> +
> +       for (i = 0; i < nranges; i++, range += rangesz) {
> +               u32 flags = of_read_number(range, 1);
> +               u32 slot = of_read_number(range + 1, 1);
> +               u64 cpuaddr = of_read_number(range + na, pna);
> +               unsigned long rtype;
> +
> +               if (DT_FLAGS_TO_TYPE(flags) == DT_TYPE_IO)
> +                       rtype = IORESOURCE_IO;
> +               else if (DT_FLAGS_TO_TYPE(flags) == DT_TYPE_MEM32)
> +                       rtype = IORESOURCE_MEM;
> +               else
> +                       continue;
> +
> +               if (slot == PCI_SLOT(devfn) && type == rtype) {
> +                       *tgt = DT_CPUADDR_TO_TARGET(cpuaddr);
> +                       *attr = DT_CPUADDR_TO_ATTR(cpuaddr);
> +                       return 0;
> +               }
> +       }
> +
> +       return -ENOENT;
> +}
> +
> +/*
> + * Use a MISC device to bind the n instances (child nodes) of the
> + * PCIe base controller in UCLASS_PCI.
> + */
> +static int mvebu_pcie_bind(struct udevice *parent)
> +{
> +       struct mvebu_pcie *pcie;
> +       struct uclass_driver *drv;
> +       struct udevice *dev;
> +       ofnode subnode;
> +       int ret;
> +
> +       /* Lookup eth driver */
> +       drv = lists_uclass_lookup(UCLASS_PCI);
> +       if (!drv) {
> +               puts("Cannot find PCI driver\n");
> +               return -ENOENT;
> +       }
> +
> +       ofnode_for_each_subnode(subnode, dev_ofnode(parent)) {
> +               if (!ofnode_is_available(subnode))
> +                       continue;
> +
> +               pcie = calloc(1, sizeof(*pcie));
> +               if (!pcie)
> +                       return -ENOMEM;
>
>                 /* Get port number, lane number and memory target / attr */
> -               mvebu_get_port_lane(pcie, i, &mem_target, &mem_attr);
> +               if (ofnode_read_u32(subnode, "marvell,pcie-port",
> +                                   &pcie->port)) {
> +                       free(pcie);
> +                       continue;
> +               }
> +
> +               if (ofnode_read_u32(subnode, "marvell,pcie-lane", &pcie->lane))
> +                       pcie->lane = 0;
>
> -               /* Don't read at all from pci registers if port power is down */
> -               if (SELECT(soc_ctrl, pcie->port) == 0) {
> -                       if (pcie->lane == 0)
> -                               debug("%s: skipping port %d\n", __func__, pcie->port);
> +               sprintf(pcie->name, "pcie%d.%d", pcie->port, pcie->lane);
> +
> +               pcie->devfn = ofnode_pci_get_devfn(subnode);
> +               if (pcie->devfn < 0) {
> +                       free(pcie);
>                         continue;
>                 }
>
> -               pcie->base = (void __iomem *)PCIE_BASE(i);
> +               ret = mvebu_get_tgt_attr(dev_ofnode(parent), pcie->devfn,
> +                                        IORESOURCE_MEM,
> +                                        &pcie->mem_target, &pcie->mem_attr);
> +               if (ret < 0) {
> +                       printf("%s: cannot get tgt/attr for mem window\n",
> +                              pcie->name);
> +                       free(pcie);
> +                       continue;
> +               }
> +
> +               /* Parse PCIe controller register base from DT */
> +               ret = mvebu_pcie_port_parse_dt(subnode, pcie);
> +               if (ret < 0) {
> +                       free(pcie);
> +                       continue;
> +               }
>
>                 /* Check link and skip ports that have no link */
>                 if (!mvebu_pcie_link_up(pcie)) {
>                         debug("%s: PCIe %d.%d - down\n", __func__,
>                               pcie->port, pcie->lane);
> +
> +                       free(pcie);
>                         continue;
>                 }
> -               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));
> -
> -               mvebu_pcie_set_local_bus_nr(pcie, bus);
> -               mvebu_pcie_set_local_dev_nr(pcie, 0);
> -               pcie->dev = PCI_BDF(bus, 0, 0);
> -
> -               pcie->mem.start = (u32)mvebu_pcie_membase;
> -               pcie->mem.end = pcie->mem.start + PCIE_MEM_SIZE - 1;
> -               mvebu_pcie_membase += PCIE_MEM_SIZE;
> -
> -               if (mvebu_mbus_add_window_by_id(mem_target, mem_attr,
> -                                               (phys_addr_t)pcie->mem.start,
> -                                               PCIE_MEM_SIZE)) {
> -                       printf("PCIe unable to add mbus window for mem at %08x+%08x\n",
> -                              (u32)pcie->mem.start, PCIE_MEM_SIZE);
> -               }
>
> -               /* 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_MASTER;
> -               reg |= BIT(10);         /* disable interrupts */
> -               writel(reg, pcie->base + PCIE_CMD_OFF);
> -
> -               /* Setup U-Boot PCI Controller */
> -               hose->first_busno = 0;
> -               hose->current_busno = bus;
> -
> -               /* PCI memory space */
> -               pci_set_region(hose->regions + 0, pcie->mem.start,
> -                              pcie->mem.start, PCIE_MEM_SIZE, PCI_REGION_MEM);
> -               pci_set_region(hose->regions + 1,
> -                              0, 0,
> -                              gd->ram_size,
> -                              PCI_REGION_MEM | PCI_REGION_SYS_MEMORY);
> -               hose->region_count = 2;
> -
> -               pci_set_ops(hose,
> -                           pci_hose_read_config_byte_via_dword,
> -                           pci_hose_read_config_word_via_dword,
> -                           mvebu_pcie_read_config_dword,
> -                           pci_hose_write_config_byte_via_dword,
> -                           pci_hose_write_config_word_via_dword,
> -                           mvebu_pcie_write_config_dword);
> -               pci_register_hose(hose);
> -
> -               hose->last_busno = pci_hose_scan(hose);
> -
> -               /* Set BAR0 to internal registers */
> -               writel(SOC_REGS_PHY_BASE, pcie->base + PCIE_BAR_LO_OFF(0));
> -               writel(0, pcie->base + PCIE_BAR_HI_OFF(0));
> -
> -               bus = hose->last_busno + 1;
> -
> -               /* need to skip more for X4 links, otherwise scan will hang */
> -               if (mvebu_soc_family() == MVEBU_SOC_AXP) {
> -                       if (mvebu_pex_unit_is_x4(i))
> -                               i += 3;
> -               }
> +               /* Create child device UCLASS_PCI and bind it */
> +               device_bind_ofnode(parent, &pcie_mvebu_drv, pcie->name, pcie,
> +                                  subnode, &dev);
>         }
> +
> +       return 0;
>  }
> +
> +static const struct udevice_id mvebu_pcie_ids[] = {
> +       { .compatible = "marvell,armada-xp-pcie" },
> +       { .compatible = "marvell,armada-370-pcie" },
> +       { }
> +};
> +
> +U_BOOT_DRIVER(pcie_mvebu_base) = {
> +       .name                   = "pcie_mvebu_base",
> +       .id                     = UCLASS_MISC,
> +       .of_match               = mvebu_pcie_ids,
> +       .bind                   = mvebu_pcie_bind,
> +};
> diff --git a/include/configs/clearfog.h b/include/configs/clearfog.h
> index 77ab6caf52..f9510826d7 100644
> --- a/include/configs/clearfog.h
> +++ b/include/configs/clearfog.h
> @@ -55,7 +55,6 @@
>
>  /* PCIe support */
>  #ifndef CONFIG_SPL_BUILD
> -#define CONFIG_PCI_MVEBU
>  #define CONFIG_PCI_SCAN_SHOW
>  #endif
>
> diff --git a/include/configs/controlcenterdc.h b/include/configs/controlcenterdc.h
> index b38cab1164..06c93c3e66 100644
> --- a/include/configs/controlcenterdc.h
> +++ b/include/configs/controlcenterdc.h
> @@ -63,9 +63,6 @@
>
>  /* PCIe support */
>  #ifndef CONFIG_SPL_BUILD
> -#define CONFIG_PCI
> -#define CONFIG_PCI_MVEBU
> -#define CONFIG_PCI_PNP
>  #define CONFIG_PCI_SCAN_SHOW
>  #endif
>
> diff --git a/include/configs/db-88f6820-amc.h b/include/configs/db-88f6820-amc.h
> index e68246cc0f..626a406cff 100644
> --- a/include/configs/db-88f6820-amc.h
> +++ b/include/configs/db-88f6820-amc.h
> @@ -34,7 +34,6 @@
>
>  /* PCIe support */
>  #ifndef CONFIG_SPL_BUILD
> -#define CONFIG_PCI_MVEBU
>  #define CONFIG_PCI_SCAN_SHOW
>  #endif
>
> diff --git a/include/configs/db-88f6820-gp.h b/include/configs/db-88f6820-gp.h
> index 3900cbed2d..1f328e97c5 100644
> --- a/include/configs/db-88f6820-gp.h
> +++ b/include/configs/db-88f6820-gp.h
> @@ -59,7 +59,6 @@
>
>  /* PCIe support */
>  #ifndef CONFIG_SPL_BUILD
> -#define CONFIG_PCI_MVEBU
>  #define CONFIG_PCI_SCAN_SHOW
>  #endif
>
> diff --git a/include/configs/db-mv784mp-gp.h b/include/configs/db-mv784mp-gp.h
> index 8ad007cc49..6cba326927 100644
> --- a/include/configs/db-mv784mp-gp.h
> +++ b/include/configs/db-mv784mp-gp.h
> @@ -46,7 +46,6 @@
>
>  /* PCIe support */
>  #ifndef CONFIG_SPL_BUILD
> -#define CONFIG_PCI_MVEBU
>  #define CONFIG_PCI_SCAN_SHOW
>  #endif
>
> diff --git a/include/configs/ds414.h b/include/configs/ds414.h
> index b9b708ad41..4ba050553a 100644
> --- a/include/configs/ds414.h
> +++ b/include/configs/ds414.h
> @@ -41,7 +41,6 @@
>
>  /* PCIe support */
>  #ifndef CONFIG_SPL_BUILD
> -#define CONFIG_PCI_MVEBU
>  #define CONFIG_PCI_SCAN_SHOW
>  #endif
>
> diff --git a/include/configs/theadorable.h b/include/configs/theadorable.h
> index de31681f8c..6837d55507 100644
> --- a/include/configs/theadorable.h
> +++ b/include/configs/theadorable.h
> @@ -62,13 +62,6 @@
>  #define CONFIG_SYS_SATA_MAX_DEVICE     1
>  #define CONFIG_LBA48
>
> -/* PCIe support */
> -#ifdef CONFIG_CMD_PCI
> -#ifndef CONFIG_SPL_BUILD
> -#define CONFIG_PCI_MVEBU
> -#endif
> -#endif
> -
>  /* Enable LCD and reserve 512KB from top of memory*/
>  #define CONFIG_SYS_MEM_TOP_HIDE                0x80000
>
> diff --git a/include/configs/turris_omnia.h b/include/configs/turris_omnia.h
> index 598674c96e..710b8991a4 100644
> --- a/include/configs/turris_omnia.h
> +++ b/include/configs/turris_omnia.h
> @@ -66,7 +66,6 @@
>
>  /* PCIe support */
>  #ifndef CONFIG_SPL_BUILD
> -#define CONFIG_PCI_MVEBU
>  #define CONFIG_PCI_SCAN_SHOW
>  #endif
>
> diff --git a/scripts/config_whitelist.txt b/scripts/config_whitelist.txt
> index 527050de97..da9809dcf3 100644
> --- a/scripts/config_whitelist.txt
> +++ b/scripts/config_whitelist.txt
> @@ -1449,7 +1449,6 @@ CONFIG_PCI_MEM_BUS
>  CONFIG_PCI_MEM_PHYS
>  CONFIG_PCI_MEM_SIZE
>  CONFIG_PCI_MSC01
> -CONFIG_PCI_MVEBU
>  CONFIG_PCI_NOSCAN
>  CONFIG_PCI_OHCI
>  CONFIG_PCI_OHCI_DEVNO
> --
> 2.20.1
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot

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

* [U-Boot] [PATCH] core: ofnode: Add ofnode_pci_get_devfn()
  2019-01-21 18:15 ` [U-Boot] [PATCH] core: ofnode: Add ofnode_pci_get_devfn() Simon Glass
@ 2019-01-22  9:36   ` Stefan Roese
  2019-01-31 10:04     ` Simon Glass
  0 siblings, 1 reply; 17+ messages in thread
From: Stefan Roese @ 2019-01-22  9:36 UTC (permalink / raw)
  To: u-boot

Hi Simon,

(added Bin, whom I forgot in this PCI patches)

On 21.01.19 19:15, Simon Glass wrote:
> On Sat, 19 Jan 2019 at 00:46, Stefan Roese <sr@denx.de> wrote:
>>
>> This function will be used by the Marvell Armada XP/38x PCIe driver,
>> which is moved to DM right now. It's mostly copied from the Linux
>> version.
>>
>> Signed-off-by: Stefan Roese <sr@denx.de>
>> Cc: Simon Glass <sjg@chromium.org>
>> ---
>>   drivers/core/ofnode.c | 12 ++++++++++++
>>   include/dm/ofnode.h   | 11 +++++++++++
>>   2 files changed, 23 insertions(+)
> 
> The code to do this right now is in pci_uclass_child_post_bind(). Do
> you think you could break that out into a pci_... function that you
> can call from your new function?

Sure, I'll give it a try. While working on it, I noticed this difference
in the current DEVFN usage in this pci_uclass_child_post_bind()
implementation:

		pplat->devfn = addr.phys_hi & 0xff00;

So, pplat->devfn uses bits 15-8 for DEVFN. Linux uses this definition
instead:

include/uapi/linux/pci.h:
/*
  * The PCI interface treats multi-function devices as independent
  * devices.  The slot/function address of each device is encoded
  * in a single byte as follows:
  *
  *	7:3 = slot
  *	2:0 = function
  */
#define PCI_DEVFN(slot, func)	((((slot) & 0x1f) << 3) | ((func) & 0x07))
#define PCI_SLOT(devfn)		(((devfn) >> 3) & 0x1f)
#define PCI_FUNC(devfn)		((devfn) & 0x07)

So here devfn uses bits 7-0 instead, which is what the MVEBU PCIe
driver also expects. Do you know why there is this different
implementation for devfn here in pci_uclass_child_post_bind()?
Is this a bug which I should fix by shifting the bits correspondingly?

> Also I had a look at the code you wrote that calls this. Ideally we
> would have the PCIe nodes have their own driver so that driver model
> will automatically bind them, but I am not sure that is feasible.

IIRC, this approach of the MISC bind function creating the PCI device
instances for the child nodes was suggested by you when I wrote the
Marvell MVPP2 ethernet driver (drivers/net/mvpp2.c) a few years ago.

Thanks,
Stefan

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

* [U-Boot] [PATCH] pci: Add PCI_SLOT macro to include/pci.h
  2019-01-22  0:32   ` Simon Glass
  2019-01-22  2:11     ` Bin Meng
@ 2019-01-22  9:53     ` Stefan Roese
  1 sibling, 0 replies; 17+ messages in thread
From: Stefan Roese @ 2019-01-22  9:53 UTC (permalink / raw)
  To: u-boot

Hi Simon,

On 22.01.19 01:32, Simon Glass wrote:
> On Sat, 19 Jan 2019 at 00:46, Stefan Roese <sr@denx.de> wrote:
>>
>> This macro will be used the by the Marvell Armada XP/38x PCIe driver,
>> which is moved to DM right now.
>>
>> Signed-off-by: Stefan Roese <sr@denx.de>
>> Cc: Bin Meng <bmeng.cn@gmail.com>
>> Cc: Simon Glass <sjg@chromium.org>
>> ---
>>   include/pci.h | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/include/pci.h b/include/pci.h
>> index 785d7d28b7..f4a9e025b3 100644
>> --- a/include/pci.h
>> +++ b/include/pci.h
>> @@ -501,6 +501,7 @@ typedef int pci_dev_t;
>>   #define PCI_BUS(d)             (((d) >> 16) & 0xff)
>>   #define PCI_DEV(d)             (((d) >> 11) & 0x1f)
>>   #define PCI_FUNC(d)            (((d) >> 8) & 0x7)
>> +#define PCI_SLOT(d)            (((d) >> 3) & 0x1f)
> 
> This seems unrelated to the other macros, since is shifts left only 3
> positions. Can you perhaps move it to the end and add a comment as to
> what the input is and what it returns? It seems different to the
> others.

We seem to have the same different interpretation (U-Boot vs Linux)
as remarked in my last mail. It seems that U-Boot expects devfn
to reside in bits 15-8.

Regarding the input and what it returns: My current understanding is,
that PCI_DEV in U-Boot is identical to PCI_SLOT in Linux (PCI_DEV is
not used there at all). The input here is devfn as well.

I can adapt my driver to use PCI_DEV instead. But frankly, these
different macro implementations (U-Boot vs Linux) are confusing.

Thanks,
Stefan

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

* [U-Boot] [PATCH] pci: Add PCI_SLOT macro to include/pci.h
  2019-01-22  2:11     ` Bin Meng
@ 2019-01-22  9:54       ` Stefan Roese
  0 siblings, 0 replies; 17+ messages in thread
From: Stefan Roese @ 2019-01-22  9:54 UTC (permalink / raw)
  To: u-boot

Hi Bin,

On 22.01.19 03:11, Bin Meng wrote:
> On Tue, Jan 22, 2019 at 8:32 AM Simon Glass <sjg@chromium.org> wrote:
>>
>> Hi Stefan,
>>
>> On Sat, 19 Jan 2019 at 00:46, Stefan Roese <sr@denx.de> wrote:
>>>
>>> This macro will be used the by the Marvell Armada XP/38x PCIe driver,
>>> which is moved to DM right now.
>>>
>>> Signed-off-by: Stefan Roese <sr@denx.de>
>>> Cc: Bin Meng <bmeng.cn@gmail.com>
>>> Cc: Simon Glass <sjg@chromium.org>
>>> ---
> 
> It's weird I did not receive this email in my inbox.

Hmm, strange.
  
>>>   include/pci.h | 1 +
>>>   1 file changed, 1 insertion(+)
>>>
>>> diff --git a/include/pci.h b/include/pci.h
>>> index 785d7d28b7..f4a9e025b3 100644
>>> --- a/include/pci.h
>>> +++ b/include/pci.h
>>> @@ -501,6 +501,7 @@ typedef int pci_dev_t;
>>>   #define PCI_BUS(d)             (((d) >> 16) & 0xff)
>>>   #define PCI_DEV(d)             (((d) >> 11) & 0x1f)
>>>   #define PCI_FUNC(d)            (((d) >> 8) & 0x7)
>>> +#define PCI_SLOT(d)            (((d) >> 3) & 0x1f)
>>
>> This seems unrelated to the other macros, since is shifts left only 3
>> positions. Can you perhaps move it to the end and add a comment as to
>> what the input is and what it returns? It seems different to the
>> others.
> 
> Agreed with Simon. Do you have any follow-up patches that will use
> this macro for better understanding?

Please see my comments on the last 2 mails and this PCI driver move
to DM:

http://patchwork.ozlabs.org/patch/1027268/

Thanks,
Stefan

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

* [U-Boot] [PATCH 1/3 v2] pci: pci_mvebu: Add DM_PCI support and move CONFIG_PCI_MVEBU to defconfig
  2019-01-22  7:35   ` Chris Packham
@ 2019-01-22 10:06     ` Stefan Roese
  0 siblings, 0 replies; 17+ messages in thread
From: Stefan Roese @ 2019-01-22 10:06 UTC (permalink / raw)
  To: u-boot

Hi Chris,

On 22.01.19 08:35, Chris Packham wrote:
> Hi Stefan,
> On Sat, Jan 19, 2019 at 12:51 AM Stefan Roese <sr@denx.de> wrote:
>>
>> This patch adds DM_PCI support to the MVEBU PCIe driver. This is
>> necessary, since all PCI drivers have to be moved to DM (driver model)
>> until the v2019.07 release.
>>
>> To not break git bisect'ablility, this patch also moves CONFIG_PCI_MVEBU
>> from config headers to the defconfig files.
>>
>> Signed-off-by: Stefan Roese <sr@denx.de>
>> Cc: Dirk Eibach <dirk.eibach@gdsys.cc>
>> Cc: Mario Six <mario.six@gdsys.cc>
>> Cc: Chris Packham <chris.packham@alliedtelesis.co.nz>
>> Cc: Phil Sutter <phil@nwl.cc>
>> Cc: Marek Behún <marek.behun@nic.cz>
>> Cc: VlaoMao <vlaomao@gmail.com>
> 
> A couple of minor comments below. With them taken care of
> 
> Reviewed-by: Chris Packham <judge.packham@gmail.com>
> Tested-by: Chris Packham <judge.packham@gmail.com>

Thanks for the review. I'll make those changes in v3.

Thanks,
Stefan

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

* [U-Boot] [PATCH] core: ofnode: Add ofnode_pci_get_devfn()
  2019-01-22  9:36   ` Stefan Roese
@ 2019-01-31 10:04     ` Simon Glass
  2019-01-31 10:45       ` Stefan Roese
  0 siblings, 1 reply; 17+ messages in thread
From: Simon Glass @ 2019-01-31 10:04 UTC (permalink / raw)
  To: u-boot

Hi Stefan,

On Tue, 22 Jan 2019 at 02:36, Stefan Roese <sr@denx.de> wrote:
>
> Hi Simon,
>
> (added Bin, whom I forgot in this PCI patches)
>
> On 21.01.19 19:15, Simon Glass wrote:
> > On Sat, 19 Jan 2019 at 00:46, Stefan Roese <sr@denx.de> wrote:
> >>
> >> This function will be used by the Marvell Armada XP/38x PCIe driver,
> >> which is moved to DM right now. It's mostly copied from the Linux
> >> version.
> >>
> >> Signed-off-by: Stefan Roese <sr@denx.de>
> >> Cc: Simon Glass <sjg@chromium.org>
> >> ---
> >>   drivers/core/ofnode.c | 12 ++++++++++++
> >>   include/dm/ofnode.h   | 11 +++++++++++
> >>   2 files changed, 23 insertions(+)
> >
> > The code to do this right now is in pci_uclass_child_post_bind(). Do
> > you think you could break that out into a pci_... function that you
> > can call from your new function?
>
> Sure, I'll give it a try. While working on it, I noticed this difference
> in the current DEVFN usage in this pci_uclass_child_post_bind()
> implementation:
>
>                 pplat->devfn = addr.phys_hi & 0xff00;
>
> So, pplat->devfn uses bits 15-8 for DEVFN. Linux uses this definition
> instead:
>
> include/uapi/linux/pci.h:
> /*
>   * The PCI interface treats multi-function devices as independent
>   * devices.  The slot/function address of each device is encoded
>   * in a single byte as follows:
>   *
>   *     7:3 = slot
>   *     2:0 = function
>   */
> #define PCI_DEVFN(slot, func)   ((((slot) & 0x1f) << 3) | ((func) & 0x07))
> #define PCI_SLOT(devfn)         (((devfn) >> 3) & 0x1f)
> #define PCI_FUNC(devfn)         ((devfn) & 0x07)
>
> So here devfn uses bits 7-0 instead, which is what the MVEBU PCIe
> driver also expects. Do you know why there is this different
> implementation for devfn here in pci_uclass_child_post_bind()?
> Is this a bug which I should fix by shifting the bits correspondingly?

Yes I think it should be consistent. I hope this is a simple fix and
does not affect the drivers much.

>
> > Also I had a look at the code you wrote that calls this. Ideally we
> > would have the PCIe nodes have their own driver so that driver model
> > will automatically bind them, but I am not sure that is feasible.
>
> IIRC, this approach of the MISC bind function creating the PCI device
> instances for the child nodes was suggested by you when I wrote the
> Marvell MVPP2 ethernet driver (drivers/net/mvpp2.c) a few years ago.

OK. Can these be added to the device tree, or are they bound dynamically?

Regards,
Simon

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

* [U-Boot] [PATCH] core: ofnode: Add ofnode_pci_get_devfn()
  2019-01-31 10:04     ` Simon Glass
@ 2019-01-31 10:45       ` Stefan Roese
  2019-02-02  6:05         ` Simon Glass
  0 siblings, 1 reply; 17+ messages in thread
From: Stefan Roese @ 2019-01-31 10:45 UTC (permalink / raw)
  To: u-boot

Hi Simon,

On 31.01.19 11:04, Simon Glass wrote:
> Hi Stefan,
> 
> On Tue, 22 Jan 2019 at 02:36, Stefan Roese <sr@denx.de> wrote:
>>
>> Hi Simon,
>>
>> (added Bin, whom I forgot in this PCI patches)
>>
>> On 21.01.19 19:15, Simon Glass wrote:
>>> On Sat, 19 Jan 2019 at 00:46, Stefan Roese <sr@denx.de> wrote:
>>>>
>>>> This function will be used by the Marvell Armada XP/38x PCIe driver,
>>>> which is moved to DM right now. It's mostly copied from the Linux
>>>> version.
>>>>
>>>> Signed-off-by: Stefan Roese <sr@denx.de>
>>>> Cc: Simon Glass <sjg@chromium.org>
>>>> ---
>>>>    drivers/core/ofnode.c | 12 ++++++++++++
>>>>    include/dm/ofnode.h   | 11 +++++++++++
>>>>    2 files changed, 23 insertions(+)
>>>
>>> The code to do this right now is in pci_uclass_child_post_bind(). Do
>>> you think you could break that out into a pci_... function that you
>>> can call from your new function?
>>
>> Sure, I'll give it a try. While working on it, I noticed this difference
>> in the current DEVFN usage in this pci_uclass_child_post_bind()
>> implementation:
>>
>>                  pplat->devfn = addr.phys_hi & 0xff00;
>>
>> So, pplat->devfn uses bits 15-8 for DEVFN. Linux uses this definition
>> instead:
>>
>> include/uapi/linux/pci.h:
>> /*
>>    * The PCI interface treats multi-function devices as independent
>>    * devices.  The slot/function address of each device is encoded
>>    * in a single byte as follows:
>>    *
>>    *     7:3 = slot
>>    *     2:0 = function
>>    */
>> #define PCI_DEVFN(slot, func)   ((((slot) & 0x1f) << 3) | ((func) & 0x07))
>> #define PCI_SLOT(devfn)         (((devfn) >> 3) & 0x1f)
>> #define PCI_FUNC(devfn)         ((devfn) & 0x07)
>>
>> So here devfn uses bits 7-0 instead, which is what the MVEBU PCIe
>> driver also expects. Do you know why there is this different
>> implementation for devfn here in pci_uclass_child_post_bind()?
>> Is this a bug which I should fix by shifting the bits correspondingly?
> 
> Yes I think it should be consistent. I hope this is a simple fix and
> does not affect the drivers much.

As you might have spotted in my later patch version (e.g. v3), I've
moved my patch to use the U-Boot "devfn" usage in bits 15-8. I've
commented this in the driver (ported from Linux) and also added a
comment about this in the function header of pci_get_devfn():

+/**
+ * pci_get_devfn() - Extract the devfn from fdt_pci_addr of the device
+ *
+ * Get devfn from fdt_pci_addr of the specifified device
+ *
+ * @dev:	PCI device
+ * @return devfn in bits 15...8 if found, -ENODEV if not found

This seemed to be the least intrusive option. I hesitate to completely
move to the Linux "devfn" usage in bits 7-0, as this might have serious
problems with the current U-Boot implementation in its drivers and
interfaces.

One thing we might do though, is to add a comment about this difference
in the U-Boot PCI_DEVFN macro definition. Should I generate a patch for
this?
  
>>
>>> Also I had a look at the code you wrote that calls this. Ideally we
>>> would have the PCIe nodes have their own driver so that driver model
>>> will automatically bind them, but I am not sure that is feasible.
>>
>> IIRC, this approach of the MISC bind function creating the PCI device
>> instances for the child nodes was suggested by you when I wrote the
>> Marvell MVPP2 ethernet driver (drivers/net/mvpp2.c) a few years ago.
> 
> OK. Can these be added to the device tree, or are they bound dynamically?

The child nodes are already in the device tree. Each child represents
a PCIe root port with its own register base. But the compatible
property resides in the parent node. Please note that I don't want to
make any changes to the DT, as this is the original Linux version.

Thanks,
Stefan

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

* [U-Boot] [PATCH] core: ofnode: Add ofnode_pci_get_devfn()
  2019-01-31 10:45       ` Stefan Roese
@ 2019-02-02  6:05         ` Simon Glass
  2019-02-05  9:34           ` Stefan Roese
  0 siblings, 1 reply; 17+ messages in thread
From: Simon Glass @ 2019-02-02  6:05 UTC (permalink / raw)
  To: u-boot

Hi Stefan,

On Thu, 31 Jan 2019 at 03:45, Stefan Roese <sr@denx.de> wrote:
>
> Hi Simon,
>
> On 31.01.19 11:04, Simon Glass wrote:
> > Hi Stefan,
> >
> > On Tue, 22 Jan 2019 at 02:36, Stefan Roese <sr@denx.de> wrote:
> >>
> >> Hi Simon,
> >>
> >> (added Bin, whom I forgot in this PCI patches)
> >>
> >> On 21.01.19 19:15, Simon Glass wrote:
> >>> On Sat, 19 Jan 2019 at 00:46, Stefan Roese <sr@denx.de> wrote:
> >>>>
> >>>> This function will be used by the Marvell Armada XP/38x PCIe driver,
> >>>> which is moved to DM right now. It's mostly copied from the Linux
> >>>> version.
> >>>>
> >>>> Signed-off-by: Stefan Roese <sr@denx.de>
> >>>> Cc: Simon Glass <sjg@chromium.org>
> >>>> ---
> >>>>    drivers/core/ofnode.c | 12 ++++++++++++
> >>>>    include/dm/ofnode.h   | 11 +++++++++++
> >>>>    2 files changed, 23 insertions(+)
> >>>
> >>> The code to do this right now is in pci_uclass_child_post_bind(). Do
> >>> you think you could break that out into a pci_... function that you
> >>> can call from your new function?
> >>
> >> Sure, I'll give it a try. While working on it, I noticed this
difference
> >> in the current DEVFN usage in this pci_uclass_child_post_bind()
> >> implementation:
> >>
> >>                  pplat->devfn = addr.phys_hi & 0xff00;
> >>
> >> So, pplat->devfn uses bits 15-8 for DEVFN. Linux uses this definition
> >> instead:
> >>
> >> include/uapi/linux/pci.h:
> >> /*
> >>    * The PCI interface treats multi-function devices as independent
> >>    * devices.  The slot/function address of each device is encoded
> >>    * in a single byte as follows:
> >>    *
> >>    *     7:3 = slot
> >>    *     2:0 = function
> >>    */
> >> #define PCI_DEVFN(slot, func)   ((((slot) & 0x1f) << 3) | ((func) &
0x07))
> >> #define PCI_SLOT(devfn)         (((devfn) >> 3) & 0x1f)
> >> #define PCI_FUNC(devfn)         ((devfn) & 0x07)
> >>
> >> So here devfn uses bits 7-0 instead, which is what the MVEBU PCIe
> >> driver also expects. Do you know why there is this different
> >> implementation for devfn here in pci_uclass_child_post_bind()?
> >> Is this a bug which I should fix by shifting the bits correspondingly?
> >
> > Yes I think it should be consistent. I hope this is a simple fix and
> > does not affect the drivers much.
>
> As you might have spotted in my later patch version (e.g. v3), I've
> moved my patch to use the U-Boot "devfn" usage in bits 15-8. I've
> commented this in the driver (ported from Linux) and also added a
> comment about this in the function header of pci_get_devfn():

OK.

>
> +/**
> + * pci_get_devfn() - Extract the devfn from fdt_pci_addr of the device
> + *
> + * Get devfn from fdt_pci_addr of the specifified device
> + *
> + * @dev:       PCI device
> + * @return devfn in bits 15...8 if found, -ENODEV if not found
>
> This seemed to be the least intrusive option. I hesitate to completely
> move to the Linux "devfn" usage in bits 7-0, as this might have serious
> problems with the current U-Boot implementation in its drivers and
> interfaces.

Yes that sounds best..

>
> One thing we might do though, is to add a comment about this difference
> in the U-Boot PCI_DEVFN macro definition. Should I generate a patch for
> this?

Yes that's a good idea.

>
> >>
> >>> Also I had a look at the code you wrote that calls this. Ideally we
> >>> would have the PCIe nodes have their own driver so that driver model
> >>> will automatically bind them, but I am not sure that is feasible.
> >>
> >> IIRC, this approach of the MISC bind function creating the PCI device
> >> instances for the child nodes was suggested by you when I wrote the
> >> Marvell MVPP2 ethernet driver (drivers/net/mvpp2.c) a few years ago.
> >
> > OK. Can these be added to the device tree, or are they bound
dynamically?
>
> The child nodes are already in the device tree. Each child represents
> a PCIe root port with its own register base. But the compatible
> property resides in the parent node. Please note that I don't want to
> make any changes to the DT, as this is the original Linux version.

OK, fair enough.

Regards,
Simon

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

* [U-Boot] [PATCH] core: ofnode: Add ofnode_pci_get_devfn()
  2019-02-02  6:05         ` Simon Glass
@ 2019-02-05  9:34           ` Stefan Roese
  0 siblings, 0 replies; 17+ messages in thread
From: Stefan Roese @ 2019-02-05  9:34 UTC (permalink / raw)
  To: u-boot

Hi Simon,

On 02.02.19 07:05, Simon Glass wrote:
> On Thu, 31 Jan 2019 at 03:45, Stefan Roese <sr at denx.de <mailto:sr@denx.de>> wrote:
>  >
>  > Hi Simon,
>  >
>  > On 31.01.19 11:04, Simon Glass wrote:
>  > > Hi Stefan,
>  > >
>  > > On Tue, 22 Jan 2019 at 02:36, Stefan Roese <sr at denx.de <mailto:sr@denx.de>> wrote:
>  > >>
>  > >> Hi Simon,
>  > >>
>  > >> (added Bin, whom I forgot in this PCI patches)
>  > >>
>  > >> On 21.01.19 19:15, Simon Glass wrote:
>  > >>> On Sat, 19 Jan 2019 at 00:46, Stefan Roese <sr at denx.de <mailto:sr@denx.de>> wrote:
>  > >>>>
>  > >>>> This function will be used by the Marvell Armada XP/38x PCIe driver,
>  > >>>> which is moved to DM right now. It's mostly copied from the Linux
>  > >>>> version.
>  > >>>>
>  > >>>> Signed-off-by: Stefan Roese <sr at denx.de <mailto:sr@denx.de>>
>  > >>>> Cc: Simon Glass <sjg at chromium.org <mailto:sjg@chromium.org>>
>  > >>>> ---
>  > >>>>    drivers/core/ofnode.c | 12 ++++++++++++
>  > >>>>    include/dm/ofnode.h   | 11 +++++++++++
>  > >>>>    2 files changed, 23 insertions(+)
>  > >>>
>  > >>> The code to do this right now is in pci_uclass_child_post_bind(). Do
>  > >>> you think you could break that out into a pci_... function that you
>  > >>> can call from your new function?
>  > >>
>  > >> Sure, I'll give it a try. While working on it, I noticed this difference
>  > >> in the current DEVFN usage in this pci_uclass_child_post_bind()
>  > >> implementation:
>  > >>
>  > >>                  pplat->devfn = addr.phys_hi & 0xff00;
>  > >>
>  > >> So, pplat->devfn uses bits 15-8 for DEVFN. Linux uses this definition
>  > >> instead:
>  > >>
>  > >> include/uapi/linux/pci.h:
>  > >> /*
>  > >>    * The PCI interface treats multi-function devices as independent
>  > >>    * devices.  The slot/function address of each device is encoded
>  > >>    * in a single byte as follows:
>  > >>    *
>  > >>    *     7:3 = slot
>  > >>    *     2:0 = function
>  > >>    */
>  > >> #define PCI_DEVFN(slot, func)   ((((slot) & 0x1f) << 3) | ((func) & 0x07))
>  > >> #define PCI_SLOT(devfn)         (((devfn) >> 3) & 0x1f)
>  > >> #define PCI_FUNC(devfn)         ((devfn) & 0x07)
>  > >>
>  > >> So here devfn uses bits 7-0 instead, which is what the MVEBU PCIe
>  > >> driver also expects. Do you know why there is this different
>  > >> implementation for devfn here in pci_uclass_child_post_bind()?
>  > >> Is this a bug which I should fix by shifting the bits correspondingly?
>  > >
>  > > Yes I think it should be consistent. I hope this is a simple fix and
>  > > does not affect the drivers much.
>  >
>  > As you might have spotted in my later patch version (e.g. v3), I've
>  > moved my patch to use the U-Boot "devfn" usage in bits 15-8. I've
>  > commented this in the driver (ported from Linux) and also added a
>  > comment about this in the function header of pci_get_devfn():
> 
> OK.
> 
>  >
>  > +/**
>  > + * pci_get_devfn() - Extract the devfn from fdt_pci_addr of the device
>  > + *
>  > + * Get devfn from fdt_pci_addr of the specifified device
>  > + *
>  > + * @dev:       PCI device
>  > + * @return devfn in bits 15...8 if found, -ENODEV if not found
>  >
>  > This seemed to be the least intrusive option. I hesitate to completely
>  > move to the Linux "devfn" usage in bits 7-0, as this might have serious
>  > problems with the current U-Boot implementation in its drivers and
>  > interfaces.
> 
> Yes that sounds best..
> 
>  >
>  > One thing we might do though, is to add a comment about this difference
>  > in the U-Boot PCI_DEVFN macro definition. Should I generate a patch for
>  > this?
> 
> Yes that's a good idea.

I'll do that with a follow-up patch, to not delay this series any longer
in this merge window.

Thanks,
Stefan

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

end of thread, other threads:[~2019-02-05  9:34 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-18 11:46 [U-Boot] [PATCH] core: ofnode: Add ofnode_pci_get_devfn() Stefan Roese
2019-01-18 11:46 ` [U-Boot] [PATCH] pci: Add PCI_SLOT macro to include/pci.h Stefan Roese
2019-01-22  0:32   ` Simon Glass
2019-01-22  2:11     ` Bin Meng
2019-01-22  9:54       ` Stefan Roese
2019-01-22  9:53     ` Stefan Roese
2019-01-18 11:46 ` [U-Boot] [PATCH 1/3 v2] pci: pci_mvebu: Add DM_PCI support and move CONFIG_PCI_MVEBU to defconfig Stefan Roese
2019-01-22  7:35   ` Chris Packham
2019-01-22 10:06     ` Stefan Roese
2019-01-18 11:46 ` [U-Boot] [PATCH 2/3 v2] arm: mvebu: armada-xp/37x.dtsi: Sync PCIe DT nodes with Linux v4.20 Stefan Roese
2019-01-18 11:46 ` [U-Boot] [PATCH 3/3 v2] arm: mvebu: armada-xp-theadorable.dts: Enable PCIe DT nodes Stefan Roese
2019-01-21 18:15 ` [U-Boot] [PATCH] core: ofnode: Add ofnode_pci_get_devfn() Simon Glass
2019-01-22  9:36   ` Stefan Roese
2019-01-31 10:04     ` Simon Glass
2019-01-31 10:45       ` Stefan Roese
2019-02-02  6:05         ` Simon Glass
2019-02-05  9:34           ` 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.