[U-Boot] [PATCH v2 1/3] pci: Don't use pci_indirect when DM is active

[U-Boot] [PATCH v2 1/3] pci: Don't use pci_indirect when DM is active

[Mario Six]

Declaration of indirect PCI bridges is not compatible with DM: Both
define PCI operations, but in different ways. Hence, don't use indirect
bridges if DM is active.

Signed-off-by: Mario Six <mario.six@gdsys.cc>
Reviewed-by: Simon Glass <sjg@chromium.org>
---

v1 -> v2:
No changes

---
 drivers/pci/pci_indirect.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/pci_indirect.c b/drivers/pci/pci_indirect.c
index 4a36749aa4..2db73f92de 100644
--- a/drivers/pci/pci_indirect.c
+++ b/drivers/pci/pci_indirect.c
@@ -8,7 +8,7 @@
 
 #include <common.h>
 
-#if !defined(__I386__)
+#if !defined(__I386__) && !defined(CONFIG_DM_PCI)
 
 #include <asm/processor.h>
 #include <asm/io.h>
-- 
2.16.1

[U-Boot] [PATCH v2 2/3] mpc83xx/pci: Register IMMR region

[Mario Six]

Register the IMMR region as a PCI region when PCI is used on MPC83xx.

Signed-off-by: Mario Six <mario.six@gdsys.cc>
---

v1 -> v2:
No changes

---
 drivers/pci/pci-uclass.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c
index a2e829608a..37ca09d76b 100644
--- a/drivers/pci/pci-uclass.c
+++ b/drivers/pci/pci-uclass.c
@@ -902,6 +902,11 @@ static int decode_regions(struct pci_controller *hose, ofnode parent_node,
 			base, size, PCI_REGION_MEM | PCI_REGION_SYS_MEMORY);
 #endif
 
+#if defined(MPC83xx) && defined(CONFIG_SYS_IMMR)
+	pci_set_region(hose->regions + hose->region_count++,
+		       CONFIG_SYS_IMMR, CONFIG_SYS_IMMR, 0x100000,
+		       PCI_REGION_MEM | PCI_REGION_SYS_MEMORY);
+#endif
 	return 0;
 }
 
-- 
2.16.1

[U-Boot] [PATCH v2 3/3] pci: Add MPC83xx PCIe driver

[Mario Six]

Add a PCIe driver for the MPC83xx architecture.

Signed-off-by: Mario Six <mario.six@gdsys.cc>
---

v1 -> v2:
No changes

---
 arch/powerpc/cpu/mpc83xx/pcie.c |   4 +
 drivers/pci/Kconfig             |  16 ++
 drivers/pci/Makefile            |   1 +
 drivers/pci/pcie_mpc83xx.c      | 442 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 463 insertions(+)
 create mode 100644 drivers/pci/pcie_mpc83xx.c

diff --git a/arch/powerpc/cpu/mpc83xx/pcie.c b/arch/powerpc/cpu/mpc83xx/pcie.c
index 28c25e5feb..ed9c3d5d9d 100644
--- a/arch/powerpc/cpu/mpc83xx/pcie.c
+++ b/arch/powerpc/cpu/mpc83xx/pcie.c
@@ -8,6 +8,8 @@
  * SPDX-License-Identifier:	GPL-2.0+
  */
 
+#ifndef CONFIG_PCIE_MPC83XX
+
 #include <common.h>
 #include <pci.h>
 #include <mpc83xx.h>
@@ -333,3 +335,5 @@ void mpc83xx_pcie_init(int num_buses, struct pci_region **reg)
 	for (i = 0; i < num_buses; i++)
 		mpc83xx_pcie_init_bus(i, reg[i]);
 }
+
+#endif /* !CONFIG_PCIE_MPC83XX */
diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
index c20a0cc060..dcea7ffdb3 100644
--- a/drivers/pci/Kconfig
+++ b/drivers/pci/Kconfig
@@ -69,6 +69,22 @@ config PCI_RCAR_GEN2
 	  Renesas RCar Gen2 SoCs. The PCIe controller on RCar Gen2 is
 	  also used to access EHCI USB controller on the SoC.
 
+config PCIE_MPC83XX
+	bool "MPC83xx PCIe driver support"
+	depends on DM_PCI
+	help
+	  Say Y here if you want to enable support for the PCIe controller on
+	  MPC83xx SoCs.
+
+if PCIE_MPC83XX
+
+config 83XX_GENERIC_PCIE_REGISTER_HOSES
+	bool "Register generic hoses"
+	help
+	  Register generic PCIe hoses when probing the PCIe device.
+
+endif
+
 config PCI_SANDBOX
 	bool "Sandbox PCI support"
 	depends on SANDBOX && DM_PCI
diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile
index 40ebc06f6d..ae7ef1d811 100644
--- a/drivers/pci/Makefile
+++ b/drivers/pci/Makefile
@@ -34,4 +34,5 @@ obj-$(CONFIG_PCI_AARDVARK) += pci-aardvark.o
 obj-$(CONFIG_PCIE_DW_MVEBU) += pcie_dw_mvebu.o
 obj-$(CONFIG_PCIE_LAYERSCAPE) += pcie_layerscape.o
 obj-$(CONFIG_PCIE_LAYERSCAPE) += pcie_layerscape_fixup.o
+obj-$(CONFIG_PCIE_MPC83XX) += pcie_mpc83xx.o
 obj-$(CONFIG_PCI_XILINX) += pcie_xilinx.o
diff --git a/drivers/pci/pcie_mpc83xx.c b/drivers/pci/pcie_mpc83xx.c
new file mode 100644
index 0000000000..19abf225b0
--- /dev/null
+++ b/drivers/pci/pcie_mpc83xx.c
@@ -0,0 +1,442 @@
+/*
+ * (C) Copyright 2018
+ * Mario Six, Guntermann & Drunck GmbH, mario.six@gdsys.cc
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <pci.h>
+#include <clk.h>
+#include <mapmem.h>
+#include <asm/io.h>
+#include <asm-generic/gpio.h>
+
+struct pex_inbound_window {
+	u32 ar;
+	u32 tar;
+	u32 barl;
+	u32 barh;
+};
+
+struct pex_outbound_window {
+	u32 ar;
+	u32 bar;
+	u32 tarl;
+	u32 tarh;
+};
+
+struct pex_csb_bridge {
+	u32 pex_csb_ver;
+	u32 pex_csb_cab;
+	u32 pex_csb_ctrl;
+	u8 res0[8];
+	u32 pex_dms_dstmr;
+	u8 res1[4];
+	u32 pex_cbs_stat;
+	u8 res2[0x20];
+	u32 pex_csb_obctrl;
+	u32 pex_csb_obstat;
+	u8 res3[0x98];
+	u32 pex_csb_ibctrl;
+	u32 pex_csb_ibstat;
+	u8 res4[0xb8];
+	u32 pex_wdma_ctrl;
+	u32 pex_wdma_addr;
+	u32 pex_wdma_stat;
+	u8 res5[0x94];
+	u32 pex_rdma_ctrl;
+	u32 pex_rdma_addr;
+	u32 pex_rdma_stat;
+	u8 res6[0xd4];
+	u32 pex_ombcr;
+	u32 pex_ombdr;
+	u8 res7[0x38];
+	u32 pex_imbcr;
+	u32 pex_imbdr;
+	u8 res8[0x38];
+	u32 pex_int_enb;
+	u32 pex_int_stat;
+	u32 pex_int_apio_vec1;
+	u32 pex_int_apio_vec2;
+	u8 res9[0x10];
+	u32 pex_int_ppio_vec1;
+	u32 pex_int_ppio_vec2;
+	u32 pex_int_wdma_vec1;
+	u32 pex_int_wdma_vec2;
+	u32 pex_int_rdma_vec1;
+	u32 pex_int_rdma_vec2;
+	u32 pex_int_misc_vec;
+	u8 res10[4];
+	u32 pex_int_axi_pio_enb;
+	u32 pex_int_axi_wdma_enb;
+	u32 pex_int_axi_rdma_enb;
+	u32 pex_int_axi_misc_enb;
+	u32 pex_int_axi_pio_stat;
+	u32 pex_int_axi_wdma_stat;
+	u32 pex_int_axi_rdma_stat;
+	u32 pex_int_axi_misc_stat;
+	u8 res11[0xa0];
+	struct pex_outbound_window pex_outbound_win[4];
+	u8 res12[0x100];
+	u32 pex_epiwtar0;
+	u32 pex_epiwtar1;
+	u32 pex_epiwtar2;
+	u32 pex_epiwtar3;
+	u8 res13[0x70];
+	struct pex_inbound_window pex_inbound_win[4];
+};
+
+struct pcie_mpc83xx_regs {
+	u8 pex_cfg_header[0x404];
+	u32 pex_ltssm_stat;
+	u8 res0[0x30];
+	u32 pex_ack_replay_timeout;
+	u8 res1[4];
+	u32 pex_gclk_ratio;
+	u8 res2[0xc];
+	u32 pex_pm_timer;
+	u32 pex_pme_timeout;
+	u8 res3[4];
+	u32 pex_aspm_req_timer;
+	u8 res4[0x18];
+	u32 pex_ssvid_update;
+	u8 res5[0x34];
+	u32 pex_cfg_ready;
+	u8 res6[0x24];
+	u32 pex_bar_sizel;
+	u8 res7[4];
+	u32 pex_bar_sel;
+	u8 res8[0x20];
+	u32 pex_bar_pf;
+	u8 res9[0x88];
+	u32 pex_pme_to_ack_tor;
+	u8 res10[0xc];
+	u32 pex_ss_intr_mask;
+	u8 res11[0x25c];
+	struct pex_csb_bridge bridge;
+	u8 res12[0x160];
+};
+
+struct pcie_mpc83xx {
+	struct pcie_mpc83xx_regs *regs;
+	struct pci_region region[2];
+	struct {
+		u32 base;
+		u32 size;
+	} cfg_space;
+};
+
+#if CONFIG_83XX_GENERIC_PCIE_REGISTER_HOSES
+
+static int pcie_mpc83xx_remap_cfg(struct udevice *bus, pci_dev_t bdf)
+{
+	struct udevice *ctlr = pci_get_controller(bus);
+	struct pci_controller *hose = dev_get_uclass_priv(ctlr);
+	struct pcie_mpc83xx *priv = dev_get_priv(bus);
+	struct pcie_mpc83xx_regs *pex = priv->regs;
+	struct pex_outbound_window *out_win = &pex->bridge.pex_outbound_win[0];
+
+	int busn = PCI_BUS(bdf) - hose->first_busno;
+	u8 devfn = PCI_DEV(bdf) << 3 | PCI_FUNC(bdf);
+	u32 dev_base = busn << 24 | devfn << 16;
+
+	if (hose->indirect_type == INDIRECT_TYPE_NO_PCIE_LINK)
+		return 0;
+	/*
+	 * Workaround for the HW bug: for Type 0 configure transactions the
+	 * PCI-E controller does not check the device number bits and just
+	 * assumes that the device number bits are 0.
+	 */
+	if (devfn & 0xf8)
+		return -1;
+
+	out_le32(&out_win->tarl, dev_base);
+	return 0;
+}
+
+static void mpc83xx_pcie_register_hose(struct udevice *bus, struct pci_region *reg,
+				       u8 link)
+{
+	struct udevice *ctlr = pci_get_controller(bus);
+	struct pci_controller *hose = dev_get_uclass_priv(ctlr);
+
+	/*
+	 * There are no spare BATs to remap all PCI-E windows for U-Boot, so
+	 * disable translations. In general, this is not great solution, and
+	 * that's why we don't register PCI-E hoses by default.
+	 */
+	disable_addr_trans();
+
+	if (!link)
+		hose->indirect_type = INDIRECT_TYPE_NO_PCIE_LINK;
+}
+#else
+
+static void mpc83xx_pcie_register_hose(struct udevice *bus, struct pci_region *reg,
+				       u8 link) {}
+
+#endif /* CONFIG_83XX_GENERIC_PCIE_REGISTER_HOSES */
+
+static int pcie_mpc83xx_read_config(struct udevice *bus, pci_dev_t bdf,
+				    uint offset, ulong *valuep,
+				    enum pci_size_t size)
+{
+#ifdef CONFIG_83XX_GENERIC_PCIE_REGISTER_HOSES
+	struct udevice *ctlr = pci_get_controller(bus);
+	struct pci_controller *hose = dev_get_uclass_priv(ctlr);
+
+	int ret;
+
+	ret = pcie_mpc83xx_remap_cfg(bus, bdf);
+	if (ret) {
+		/* TODO: Old code sets the value to -1 here... */
+		return ret;
+	}
+
+	switch (size) {
+	case PCI_SIZE_8:
+		*valuep = in_8((u8 *)(hose->cfg_addr + offset));
+		break;
+	case PCI_SIZE_16:
+		*valuep = in_le16((u16 *)(hose->cfg_addr + offset));
+		break;
+	case PCI_SIZE_32:
+		*valuep = in_le32((u32 *)(hose->cfg_addr + offset));
+		break;
+	}
+#endif
+	return 0;
+}
+
+static int pcie_mpc83xx_write_config(struct udevice *bus, pci_dev_t bdf,
+				     uint offset, ulong value,
+				     enum pci_size_t size)
+{
+#ifdef CONFIG_83XX_GENERIC_PCIE_REGISTER_HOSES
+	struct udevice *ctlr = pci_get_controller(bus);
+	struct pci_controller *hose = dev_get_uclass_priv(ctlr);
+
+	int ret;
+
+	ret = pcie_mpc83xx_remap_cfg(bus, bdf);
+	if (ret)
+		return ret;
+
+	switch (size) {
+	case PCI_SIZE_8:
+		out_8((u8 *)(hose->cfg_addr + offset), value);
+		break;
+	case PCI_SIZE_16:
+		out_le16((u16 *)(hose->cfg_addr + offset), value);
+		break;
+	case PCI_SIZE_32:
+		out_le32((u32 *)(hose->cfg_addr + offset), value);
+		break;
+	}
+#endif
+	return 0;
+}
+
+void configure_pcie_law(pci_addr_t addr)
+{
+	immap_t *immr = (immap_t *)CONFIG_SYS_IMMR;
+	sysconf83xx_t *sysconf = &immr->sysconf;
+	law83xx_t *pcie_law = sysconf->pcielaw;
+
+	/* Deassert the resets in the control register */
+	out_be32(&sysconf->pecr1, 0xE0008000);
+	udelay(2000);
+
+	/* Configure PCI Express Local Access Windows */
+	out_be32(&pcie_law[0].bar, addr & LAWBAR_BAR);
+	out_be32(&pcie_law[0].ar, LBLAWAR_EN | LBLAWAR_512MB);
+}
+
+static int pcie_mpc83xx_probe(struct udevice *bus)
+{
+	//struct udevice *ctlr = pci_get_controller(bus);
+	//struct pci_controller *hose = dev_get_uclass_priv(ctlr);
+	struct pcie_mpc83xx *priv = dev_get_priv(bus);
+	struct pcie_mpc83xx_regs *pex;
+	struct pex_outbound_window *out_win;
+	struct pex_inbound_window *in_win;
+	u8 *hose_cfg_base;
+	unsigned int ram_sz;
+	unsigned int barl;
+	unsigned int tar;
+	u16 ltssm;
+	bool have_link;
+	int i;
+	struct clk clock;
+	struct pci_region *mem;
+	struct pci_region *io;
+	struct pci_region *pref;
+
+	pci_get_regions(bus, &io, &mem, &pref);
+	configure_pcie_law(mem->bus_start);
+
+	priv->regs = map_sysmem(dev_read_addr(bus),
+				sizeof(struct pcie_mpc83xx));
+	pex = priv->regs;
+
+	/*
+	 * Release PCI RST Output signal.
+	 * Power on to RST high must be at least 100 ms as per PCI spec.
+	 * On warm boots only 1 ms is required, but we play it safe.
+	 */
+	mdelay(100);
+
+	/* Enable pex csb bridge inbound & outbound transactions */
+	setbits_le32(&pex->bridge.pex_csb_ctrl, PEX_CSB_CTRL_OBPIOE |
+						PEX_CSB_CTRL_IBPIOE);
+
+	/* Enable bridge outbound */
+	out_le32(&pex->bridge.pex_csb_obctrl, PEX_CSB_OBCTRL_PIOE |
+					      PEX_CSB_OBCTRL_MEMWE |
+					      PEX_CSB_OBCTRL_IOWE |
+					      PEX_CSB_OBCTRL_CFGWE);
+
+	out_win = &pex->bridge.pex_outbound_win[0];
+	out_le32(&out_win->ar, PEX_OWAR_EN | PEX_OWAR_TYPE_CFG |
+			priv->cfg_space.size);
+	out_le32(&out_win->bar, priv->cfg_space.base);
+	out_le32(&out_win->tarl, 0);
+	out_le32(&out_win->tarh, 0);
+
+	for (i = 0; i < 2; i++) {
+		u32 ar;
+
+		if (priv->region[i].size == 0)
+			break;
+
+		out_win = &pex->bridge.pex_outbound_win[i + 1];
+		out_le32(&out_win->bar, priv->region[i].phys_start);
+		out_le32(&out_win->tarl, priv->region[i].bus_start);
+		out_le32(&out_win->tarh, 0);
+		ar = PEX_OWAR_EN | (priv->region[i].size & PEX_OWAR_SIZE);
+		if (priv->region[i].flags & PCI_REGION_IO)
+			ar |= PEX_OWAR_TYPE_IO;
+		else
+			ar |= PEX_OWAR_TYPE_MEM;
+		out_le32(&out_win->ar, ar);
+	}
+
+	out_le32(&pex->bridge.pex_csb_ibctrl, PEX_CSB_IBCTRL_PIOE);
+
+	ram_sz = gd->ram_size;
+	barl = 0;
+	tar = 0;
+	i = 0;
+	while (ram_sz > 0) {
+		in_win = &pex->bridge.pex_inbound_win[i];
+		out_le32(&in_win->barl, barl);
+		out_le32(&in_win->barh, 0x0);
+		out_le32(&in_win->tar, tar);
+		if (ram_sz >= 0x10000000) {
+			/* The maximum windows size is 256M */
+			out_le32(&in_win->ar, PEX_IWAR_EN | PEX_IWAR_NSOV |
+				PEX_IWAR_TYPE_PF | 0x0FFFF000);
+			barl += 0x10000000;
+			tar += 0x10000000;
+			ram_sz -= 0x10000000;
+		} else {
+			/* The UM is not clear here.
+			 * So, round up to even Mb boundary
+			 */
+
+			ram_sz = ram_sz >> (20 +
+					((ram_sz & 0xFFFFF) ? 1 : 0));
+			if (!(ram_sz % 2))
+				ram_sz -= 1;
+			out_le32(&in_win->ar, PEX_IWAR_EN | PEX_IWAR_NSOV |
+				PEX_IWAR_TYPE_PF | (ram_sz << 20) | 0xFF000);
+			ram_sz = 0;
+		}
+		i++;
+	}
+
+	in_win = &pex->bridge.pex_inbound_win[i];
+	out_le32(&in_win->barl, CONFIG_SYS_IMMR);
+	out_le32(&in_win->barh, 0);
+	out_le32(&in_win->tar, CONFIG_SYS_IMMR);
+	out_le32(&in_win->ar, PEX_IWAR_EN |
+		PEX_IWAR_TYPE_NO_PF | PEX_IWAR_SIZE_1M);
+
+	/* Enable the host virtual INTX interrupts */
+	out_le32(&pex->bridge.pex_int_axi_misc_enb,
+		 in_le32(&pex->bridge.pex_int_axi_misc_enb) | 0x1E0);
+
+	/* Hose configure header is memory-mapped */
+	hose_cfg_base = (u8 *)pex;
+
+	/* Configure the PCIE controller core clock ratio */
+	clk_get_by_index(bus, 0, &clock);
+	out_le32((u32 *)(hose_cfg_base + PEX_GCLK_RATIO),
+		 ((clk_get_rate(&clock) / 1000000) * 16) / 333);
+	mdelay(1000);
+
+	/* Do Type 1 bridge configuration */
+	out_8(hose_cfg_base + PCI_PRIMARY_BUS, 0);
+	out_8(hose_cfg_base + PCI_SECONDARY_BUS, 1);
+	out_8(hose_cfg_base + PCI_SUBORDINATE_BUS, 255);
+
+	/*
+	 * Write to Command register
+	 */
+	setbits_le16((u16 *)(hose_cfg_base + PCI_COMMAND), PCI_COMMAND_MASTER |
+							   PCI_COMMAND_MEMORY |
+							   PCI_COMMAND_IO |
+							   PCI_COMMAND_SERR |
+							   PCI_COMMAND_PARITY);
+	/*
+	 * Clear non-reserved bits in status register.
+	 */
+	out_le16((u16 *)(hose_cfg_base + PCI_STATUS), 0xffff);
+	out_8(hose_cfg_base + PCI_LATENCY_TIMER, 0x80);
+	out_8(hose_cfg_base + PCI_CACHE_LINE_SIZE, 0x08);
+
+	printf("PCIE(%s): ", bus->name);
+
+#define PCI_LTSSM	0x404 /* PCIe Link Training, Status State Machine */
+#define PCI_LTSSM_L0	0x16 /* L0 state */
+	ltssm = in_le16((u16 *)(hose_cfg_base + PCI_LTSSM));
+	have_link = (ltssm >= PCI_LTSSM_L0);
+	if (have_link)
+		printf("link\n");
+	else
+		printf("No link\n");
+
+	mpc83xx_pcie_register_hose(bus, priv->region, have_link);
+
+	return 0;
+}
+
+static int pcie_mpc83xx_ofdata_to_platdata(struct udevice *bus)
+{
+	/* TODO: Read region data from DT */
+	/* TODO: Read cfg_space data from DT */
+	return 0;
+}
+
+static const struct dm_pci_ops pcie_mpc83xx_ops = {
+	.read_config	= pcie_mpc83xx_read_config,
+	.write_config	= pcie_mpc83xx_write_config,
+};
+
+static const struct udevice_id pcie_mpc83xx_ids[] = {
+	{ .compatible = "fsl,mpc8308-pcie" },
+	{ .compatible = "fsl,mpc8314-pcie" },
+	{ }
+};
+
+U_BOOT_DRIVER(pcie_mpc83xx) = {
+	.name			= "pcie_mpc83xx",
+	.id			= UCLASS_PCI,
+	.of_match		= pcie_mpc83xx_ids,
+	.ops			= &pcie_mpc83xx_ops,
+	.ofdata_to_platdata	= pcie_mpc83xx_ofdata_to_platdata,
+	.probe			= pcie_mpc83xx_probe,
+	.priv_auto_alloc_size	= sizeof(struct pcie_mpc83xx),
+};
-- 
2.16.1

[U-Boot] [PATCH v2 2/3] mpc83xx/pci: Register IMMR region

[Simon Glass]

Hi Mario,

On 27 April 2018 at 06:53, Mario Six <mario.six@gdsys.cc> wrote:
> Register the IMMR region as a PCI region when PCI is used on MPC83xx.
>
> Signed-off-by: Mario Six <mario.six@gdsys.cc>
> ---
>
> v1 -> v2:
> No changes
>
> ---
>  drivers/pci/pci-uclass.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c
> index a2e829608a..37ca09d76b 100644
> --- a/drivers/pci/pci-uclass.c
> +++ b/drivers/pci/pci-uclass.c
> @@ -902,6 +902,11 @@ static int decode_regions(struct pci_controller *hose, ofnode parent_node,
>                         base, size, PCI_REGION_MEM | PCI_REGION_SYS_MEMORY);
>  #endif
>
> +#if defined(MPC83xx) && defined(CONFIG_SYS_IMMR)
> +       pci_set_region(hose->regions + hose->region_count++,
> +                      CONFIG_SYS_IMMR, CONFIG_SYS_IMMR, 0x100000,
> +                      PCI_REGION_MEM | PCI_REGION_SYS_MEMORY);
> +#endif

Please don't add device-specific code in this file. This could go in
the device tree, perhaps. If not, we perhaps need to do it in your PCI
driver?

>         return 0;
>  }
>
> --
> 2.16.1
>

Regards,
Simon

[U-Boot] [PATCH v2 2/3] mpc83xx/pci: Register IMMR region

[Mario Six]

Hi Simon,

On Thu, May 3, 2018 at 9:02 PM, Simon Glass <sjg@chromium.org> wrote:
> Hi Mario,
>
> On 27 April 2018 at 06:53, Mario Six <mario.six@gdsys.cc> wrote:
>> Register the IMMR region as a PCI region when PCI is used on MPC83xx.
>>
>> Signed-off-by: Mario Six <mario.six@gdsys.cc>
>> ---
>>
>> v1 -> v2:
>> No changes
>>
>> ---
>>  drivers/pci/pci-uclass.c | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c
>> index a2e829608a..37ca09d76b 100644
>> --- a/drivers/pci/pci-uclass.c
>> +++ b/drivers/pci/pci-uclass.c
>> @@ -902,6 +902,11 @@ static int decode_regions(struct pci_controller *hose, ofnode parent_node,
>>                         base, size, PCI_REGION_MEM | PCI_REGION_SYS_MEMORY);
>>  #endif
>>
>> +#if defined(MPC83xx) && defined(CONFIG_SYS_IMMR)
>> +       pci_set_region(hose->regions + hose->region_count++,
>> +                      CONFIG_SYS_IMMR, CONFIG_SYS_IMMR, 0x100000,
>> +                      PCI_REGION_MEM | PCI_REGION_SYS_MEMORY);
>> +#endif
>
> Please don't add device-specific code in this file. This could go in
> the device tree, perhaps. If not, we perhaps need to do it in your PCI
> driver?
>

OK, I'll try to put it into the DT (I have absolutely no expertise with PCI, so
I'm still learning with this one).

>>         return 0;
>>  }
>>
>> --
>> 2.16.1
>>
>
> Regards,
> Simon
>

Best regards,
Mario

[U-Boot] [PATCH v2 2/3] mpc83xx/pci: Register IMMR region

[Simon Glass]

Hi Mario,

On 4 May 2018 at 02:15, Mario Six <mario.six@gdsys.cc> wrote:
> Hi Simon,
>
> On Thu, May 3, 2018 at 9:02 PM, Simon Glass <sjg@chromium.org> wrote:
>> Hi Mario,
>>
>> On 27 April 2018 at 06:53, Mario Six <mario.six@gdsys.cc> wrote:
>>> Register the IMMR region as a PCI region when PCI is used on MPC83xx.
>>>
>>> Signed-off-by: Mario Six <mario.six@gdsys.cc>
>>> ---
>>>
>>> v1 -> v2:
>>> No changes
>>>
>>> ---
>>>  drivers/pci/pci-uclass.c | 5 +++++
>>>  1 file changed, 5 insertions(+)
>>>
>>> diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c
>>> index a2e829608a..37ca09d76b 100644
>>> --- a/drivers/pci/pci-uclass.c
>>> +++ b/drivers/pci/pci-uclass.c
>>> @@ -902,6 +902,11 @@ static int decode_regions(struct pci_controller
*hose, ofnode parent_node,
>>>                         base, size, PCI_REGION_MEM |
PCI_REGION_SYS_MEMORY);
>>>  #endif
>>>
>>> +#if defined(MPC83xx) && defined(CONFIG_SYS_IMMR)
>>> +       pci_set_region(hose->regions + hose->region_count++,
>>> +                      CONFIG_SYS_IMMR, CONFIG_SYS_IMMR, 0x100000,
>>> +                      PCI_REGION_MEM | PCI_REGION_SYS_MEMORY);
>>> +#endif
>>
>> Please don't add device-specific code in this file. This could go in
>> the device tree, perhaps. If not, we perhaps need to do it in your PCI
>> driver?
>>
>
> OK, I'll try to put it into the DT (I have absolutely no expertise with
PCI, so
> I'm still learning with this one).

Here's a possible idea:

decode_regions() is called from the PCI uclass' pre_probe() method. This
means that by the time your PCI controller enters the probe() method, all
the regions should be set up. So you should be able to add the above code
into the probe() method of your PCI controller driver. Perhaps can check a
compatible string etc. to decide whether to add this region, if it is no
described in the DT itself.

Regards,
Simon

[U-Boot] [U-Boot, v2, 1/3] pci: Don't use pci_indirect when DM is active

[Tom Rini]

On Fri, Apr 27, 2018 at 02:53:37PM +0200, Mario Six wrote:

> Declaration of indirect PCI bridges is not compatible with DM: Both
> define PCI operations, but in different ways. Hence, don't use indirect
> bridges if DM is active.
> 
> Signed-off-by: Mario Six <mario.six@gdsys.cc>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180508/2ce9c926/attachment.sig>

[U-Boot] [PATCH v2 2/3] mpc83xx/pci: Register IMMR region

[Mario Six]

Hi Simon,

On Fri, May 4, 2018 at 11:37 PM, Simon Glass <sjg@chromium.org> wrote:
> Hi Mario,
>
> On 4 May 2018 at 02:15, Mario Six <mario.six@gdsys.cc> wrote:
>> Hi Simon,
>>
>> On Thu, May 3, 2018 at 9:02 PM, Simon Glass <sjg@chromium.org> wrote:
>>> Hi Mario,
>>>
>>> On 27 April 2018 at 06:53, Mario Six <mario.six@gdsys.cc> wrote:
>>>> Register the IMMR region as a PCI region when PCI is used on MPC83xx.
>>>>
>>>> Signed-off-by: Mario Six <mario.six@gdsys.cc>
>>>> ---
>>>>
>>>> v1 -> v2:
>>>> No changes
>>>>
>>>> ---
>>>>  drivers/pci/pci-uclass.c | 5 +++++
>>>>  1 file changed, 5 insertions(+)
>>>>
>>>> diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c
>>>> index a2e829608a..37ca09d76b 100644
>>>> --- a/drivers/pci/pci-uclass.c
>>>> +++ b/drivers/pci/pci-uclass.c
>>>> @@ -902,6 +902,11 @@ static int decode_regions(struct pci_controller
> *hose, ofnode parent_node,
>>>>                         base, size, PCI_REGION_MEM |
> PCI_REGION_SYS_MEMORY);
>>>>  #endif
>>>>
>>>> +#if defined(MPC83xx) && defined(CONFIG_SYS_IMMR)
>>>> +       pci_set_region(hose->regions + hose->region_count++,
>>>> +                      CONFIG_SYS_IMMR, CONFIG_SYS_IMMR, 0x100000,
>>>> +                      PCI_REGION_MEM | PCI_REGION_SYS_MEMORY);
>>>> +#endif
>>>
>>> Please don't add device-specific code in this file. This could go in
>>> the device tree, perhaps. If not, we perhaps need to do it in your PCI
>>> driver?
>>>
>>
>> OK, I'll try to put it into the DT (I have absolutely no expertise with
> PCI, so
>> I'm still learning with this one).
>
> Here's a possible idea:
>
> decode_regions() is called from the PCI uclass' pre_probe() method. This
> means that by the time your PCI controller enters the probe() method, all
> the regions should be set up. So you should be able to add the above code
> into the probe() method of your PCI controller driver. Perhaps can check a
> compatible string etc. to decide whether to add this region, if it is no
> described in the DT itself.
>

OK, I'll do that for v3, thanks for the explanation.

> Regards,
> Simon
>

Best regards,
Mario