[U-Boot] [PATCH 00/23] MIPS Boston fixes, ethernet support & cleanup
This series fixes a few things with the primary aim of enabling use of the ethernet controller found in the Intel EG20T Platform Controller Hub that is connected to one of the Xilinx AXI to PCIe controllers in the system. Thus the series makes fixes across the PCIe driver & PCI subsystem, the ethernet driver & the board code.
With this series applied it is possible to load a Linux kernel image over ethernet on a MIPS Boston board & successfully boot it to a state where it can access the PCI devices in the system for itself.
Paul Burton (23): image: Use ram_top, not bi_memsize, in getenv_bootm_size MIPS: Use ram_top, not bi_memsize, in arch_lmb_reserve MIPS: Fix map_physmem for cached mappings pci: xilinx: Initialise the root bridge during probe pci: xilinx: Avoid writing memory base or limit registers pci: Set of_offset for devices not probed via DT compatible strings pci: Handle MIPS systems with virtual CONFIG_SYS_SDRAM_BASE pci: Make PCI bridge memory alignment configurable boston: Disable PCI bridge memory space alignment net: pch_gbe: Reset during probe net: pch_gbe: Fix rx descriptor buffer addresses net: pch_gbe: CPU accessible addresses are virtual net: pch_gbe: Add cache maintenance gpio: Provide dummy get/request & is_valid functions gpio: eg20t: Add driver for Intel EG20T GPIO controllers net: pch_gbe: Support PHY reset GPIOs MIPS: Make CM GCR base configurable boston: Move CM GCRs away from flash boston: Setup memory ranges in FDT provided to Linux boston: Bump CONFIG_SYS_BOOTM_LEN to 64MiB boston: Enable Realtek ethernet PHY support boston: Probe ethernet controller during boot boston: Enable CONFIG_DISTRO_DEFAULTS in defconfigs
arch/mips/Kconfig | 5 +- arch/mips/dts/img,boston.dts | 6 +- arch/mips/include/asm/io.h | 2 +- arch/mips/lib/bootm.c | 2 +- board/imgtec/boston/Makefile | 1 + board/imgtec/boston/dt.c | 27 +++++++++ common/image.c | 2 +- configs/boston32r2_defconfig | 10 ++-- configs/boston32r2el_defconfig | 10 ++-- configs/boston64r2_defconfig | 10 ++-- configs/boston64r2el_defconfig | 10 ++-- drivers/gpio/Kconfig | 8 +++ drivers/gpio/Makefile | 1 + drivers/gpio/eg20t-gpio.c | 133 +++++++++++++++++++++++++++++++++++++++++ drivers/net/pch_gbe.c | 71 +++++++++++++++++----- drivers/net/pch_gbe.h | 1 + drivers/pci/Kconfig | 7 +++ drivers/pci/pci-uclass.c | 36 ++++++++++- drivers/pci/pci_auto.c | 24 +++++--- drivers/pci/pcie_xilinx.c | 45 +++++++++++++- include/asm-generic/gpio.h | 38 ++++++++++++ include/configs/boston.h | 6 ++ 22 files changed, 399 insertions(+), 56 deletions(-) create mode 100644 board/imgtec/boston/dt.c create mode 100644 drivers/gpio/eg20t-gpio.c
When determining the region of memory to allow for use by bootm, using bi_memstart & adding bi_memsize can cause problems if that leads to an integer overflow. For example on some MIPS systems bi_memstart would be 0xffffffff80000000 (ie. the start of the MIPS ckseg0 region) and if the system has 2GB of memory then the addition would wrap around to 0.
The maximum amount of memory to be used by U-Boot is already accounted for by the ram_top field of struct global_data, so make use of that for the calculation instead.
Signed-off-by: Paul Burton paul.burton@imgtec.com ---
common/image.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/common/image.c b/common/image.c index a5d19ab..30537cd 100644 --- a/common/image.c +++ b/common/image.c @@ -489,7 +489,7 @@ phys_size_t getenv_bootm_size(void) size = gd->bd->bi_dram[0].size; #else start = gd->bd->bi_memstart; - size = gd->bd->bi_memsize; + size = gd->ram_top - start; #endif
s = getenv("bootm_low");
+cc Simon and Tom
2016-09-26 20:28 GMT+02:00 Paul Burton paul.burton@imgtec.com:
When determining the region of memory to allow for use by bootm, using bi_memstart & adding bi_memsize can cause problems if that leads to an integer overflow. For example on some MIPS systems bi_memstart would be 0xffffffff80000000 (ie. the start of the MIPS ckseg0 region) and if the system has 2GB of memory then the addition would wrap around to 0.
The maximum amount of memory to be used by U-Boot is already accounted for by the ram_top field of struct global_data, so make use of that for the calculation instead.
Signed-off-by: Paul Burton paul.burton@imgtec.com
common/image.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/common/image.c b/common/image.c index a5d19ab..30537cd 100644 --- a/common/image.c +++ b/common/image.c @@ -489,7 +489,7 @@ phys_size_t getenv_bootm_size(void) size = gd->bd->bi_dram[0].size; #else start = gd->bd->bi_memstart;
size = gd->bd->bi_memsize;
size = gd->ram_top - start;#endif
s = getenv("bootm_low");-- 2.10.0
I think the code in the #if branch above should updated too to keep a consistent behaviour
On Tue, Sep 27, 2016 at 01:03:47PM +0200, Daniel Schwierzeck wrote:
+cc Simon and Tom
2016-09-26 20:28 GMT+02:00 Paul Burton paul.burton@imgtec.com:
When determining the region of memory to allow for use by bootm, using bi_memstart & adding bi_memsize can cause problems if that leads to an integer overflow. For example on some MIPS systems bi_memstart would be 0xffffffff80000000 (ie. the start of the MIPS ckseg0 region) and if the system has 2GB of memory then the addition would wrap around to 0.
The maximum amount of memory to be used by U-Boot is already accounted for by the ram_top field of struct global_data, so make use of that for the calculation instead.
Signed-off-by: Paul Burton paul.burton@imgtec.com
common/image.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/common/image.c b/common/image.c index a5d19ab..30537cd 100644 --- a/common/image.c +++ b/common/image.c @@ -489,7 +489,7 @@ phys_size_t getenv_bootm_size(void) size = gd->bd->bi_dram[0].size; #else start = gd->bd->bi_memstart;
size = gd->bd->bi_memsize;
size = gd->ram_top - start;#endif
s = getenv("bootm_low");I think the code in the #if branch above should updated too to keep a consistent behaviour
The top half of the else is for ARM, and the bottom is everyone else. So this needs testing and acks outside of MIPS. But yes, I think we should use size = gd->ram_top - start for everyone.
When calculating the region to reserve for the stack in arch_lmb_reserve, make use of ram_top instead of adding bi_memsize to CONFIG_SYS_SDRAM_BASE. This avoids overflow if the system has enough memory to reach the end of the address space.
Signed-off-by: Paul Burton paul.burton@imgtec.com ---
arch/mips/lib/bootm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/mips/lib/bootm.c b/arch/mips/lib/bootm.c index 0c6a4ab..9fec4ad 100644 --- a/arch/mips/lib/bootm.c +++ b/arch/mips/lib/bootm.c @@ -42,7 +42,7 @@ void arch_lmb_reserve(struct lmb *lmb)
/* adjust sp by 4K to be safe */ sp -= 4096; - lmb_reserve(lmb, sp, CONFIG_SYS_SDRAM_BASE + gd->ram_size - sp); + lmb_reserve(lmb, sp, gd->ram_top - sp); }
static void linux_cmdline_init(void)
Am 26.09.2016 um 20:28 schrieb Paul Burton:
When calculating the region to reserve for the stack in arch_lmb_reserve, make use of ram_top instead of adding bi_memsize to CONFIG_SYS_SDRAM_BASE. This avoids overflow if the system has enough memory to reach the end of the address space.
Signed-off-by: Paul Burton paul.burton@imgtec.com
arch/mips/lib/bootm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
applied to u-boot-mips, thanks!
map_physmem should return a pointer that can be used by the CPU to access the given memory - on MIPS simply returning the physical address as it does prior to this patch doesn't achieve that. Instead return a pointer to the memory within (c)kseg0, which matches up consistently with the (c)kseg1 pointer that uncached mappings return via ioremap.
Signed-off-by: Paul Burton paul.burton@imgtec.com ---
arch/mips/include/asm/io.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/mips/include/asm/io.h b/arch/mips/include/asm/io.h index 5b86386..ee7a592 100644 --- a/arch/mips/include/asm/io.h +++ b/arch/mips/include/asm/io.h @@ -501,7 +501,7 @@ map_physmem(phys_addr_t paddr, unsigned long len, unsigned long flags) if (flags == MAP_NOCACHE) return ioremap(paddr, len);
- return (void *)paddr; + return (void *)CKSEG0ADDR(paddr); }
/*
Am 26.09.2016 um 20:28 schrieb Paul Burton:
map_physmem should return a pointer that can be used by the CPU to access the given memory - on MIPS simply returning the physical address as it does prior to this patch doesn't achieve that. Instead return a pointer to the memory within (c)kseg0, which matches up consistently with the (c)kseg1 pointer that uncached mappings return via ioremap.
Signed-off-by: Paul Burton paul.burton@imgtec.com
applied to u-boot-mips, thanks!
Whilst the pcie_xilinx driver was sufficient to run under QEMU, it was failing on FPGA because it wasn't configuring the root bridge, and access from the PCI auto-configuration code to subordinate busses would lead to data bus errors. Fix this by configuring the root bridge to allow access to all possible subordinate busses based upon the size of the ECAM region, and disable interrupts since U-Boot isn't using them.
Signed-off-by: Paul Burton paul.burton@imgtec.com ---
drivers/pci/pcie_xilinx.c | 36 ++++++++++++++++++++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/pcie_xilinx.c b/drivers/pci/pcie_xilinx.c index 5216001..9059c41 100644 --- a/drivers/pci/pcie_xilinx.c +++ b/drivers/pci/pcie_xilinx.c @@ -23,8 +23,14 @@ struct xilinx_pcie { };
/* Register definitions */ -#define XILINX_PCIE_REG_PSCR 0x144 -#define XILINX_PCIE_REG_PSCR_LNKUP BIT(11) +#define XILINX_PCIE_REG_BRIDGE_INFO 0x130 +#define XILINX_PCIE_REG_BRIDGE_INFO_ECAMSZ_SHIFT 16 +#define XILINX_PCIE_REG_BRIDGE_INFO_ECAMSZ_MASK (0x7 << 16) +#define XILINX_PCIE_REG_INT_MASK 0x13c +#define XILINX_PCIE_REG_PSCR 0x144 +#define XILINX_PCIE_REG_PSCR_LNKUP BIT(11) +#define XILINX_PCIE_REG_RPSC 0x148 +#define XILINX_PCIE_REG_RPSC_BRIDGEEN BIT(0)
/** * pcie_xilinx_link_up() - Check whether the PCIe link is up @@ -200,6 +206,31 @@ static int pcie_xilinx_ofdata_to_platdata(struct udevice *dev) return 0; }
+static int pcie_xilinx_probe(struct udevice *dev) +{ + struct xilinx_pcie *pcie = dev_get_priv(dev); + u32 bridge_info, ecam_sz, rpsc; + + /* Disable all interrupts */ + writel(0, pcie->cfg_base + XILINX_PCIE_REG_INT_MASK); + + /* Enable the bridge */ + rpsc = readl(pcie->cfg_base + XILINX_PCIE_REG_RPSC); + rpsc |= XILINX_PCIE_REG_RPSC_BRIDGEEN; + writel(rpsc, pcie->cfg_base + XILINX_PCIE_REG_RPSC); + + /* Discover the size of the ECAM region */ + bridge_info = readl(pcie->cfg_base + XILINX_PCIE_REG_BRIDGE_INFO); + ecam_sz = bridge_info & XILINX_PCIE_REG_BRIDGE_INFO_ECAMSZ_MASK; + ecam_sz >>= XILINX_PCIE_REG_BRIDGE_INFO_ECAMSZ_SHIFT; + + /* Enable access to all possible subordinate buses */ + writel((0 << 0) | (1 << 8) | (GENMASK(ecam_sz - 1, 0) << 16), + pcie->cfg_base + PCI_PRIMARY_BUS); + + return 0; +} + static const struct dm_pci_ops pcie_xilinx_ops = { .read_config = pcie_xilinx_read_config, .write_config = pcie_xilinx_write_config, @@ -216,5 +247,6 @@ U_BOOT_DRIVER(pcie_xilinx) = { .of_match = pcie_xilinx_ids, .ops = &pcie_xilinx_ops, .ofdata_to_platdata = pcie_xilinx_ofdata_to_platdata, + .probe = pcie_xilinx_probe, .priv_auto_alloc_size = sizeof(struct xilinx_pcie), };
Writing the PCI memory base & limit registers leads to the root bridge reporting a PCI_MEMORY_BASE value of 0 & a PCI_MEMORY_LIMIT value of 0x1600. If we then boot Linux, it sees that the bridge device needs 0x16000000 bytes of memory space & fails to assign it.
It's unclear to me why this happens, and poking values from the shell doesn't seem to make anything clearer, but this workaround allows a MIPS Boston board to boot Linux & let Linux successfully probe the PCIe bus & all devices connected to it.
Signed-off-by: Paul Burton paul.burton@imgtec.com ---
drivers/pci/pcie_xilinx.c | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/drivers/pci/pcie_xilinx.c b/drivers/pci/pcie_xilinx.c index 9059c41..0237bec 100644 --- a/drivers/pci/pcie_xilinx.c +++ b/drivers/pci/pcie_xilinx.c @@ -160,6 +160,15 @@ static int pcie_xilinx_write_config(struct udevice *bus, pci_dev_t bdf, if (err < 0) return 0;
+ if (bdf == PCI_BDF(bus->seq, 0, 0)) { + switch (offset) { + case PCI_MEMORY_BASE: + case PCI_MEMORY_LIMIT: + /* Writing the memory base or limit causes problems */ + return 0; + } + } + switch (size) { case PCI_SIZE_8: __raw_writeb(value, address);
A PCI device may be probed through standard PCI config space probing but still be represented in a device tree. However U-Boot would not previously set the of_offset field of the struct udevice for such PCI devices. Fix this by searching for a DT node based upon its "reg" property after binding a PCI device that wasn't already seen in the DT.
Signed-off-by: Paul Burton paul.burton@imgtec.com
---
drivers/pci/pci-uclass.c | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+)
diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c index 3b00e6a..415c632 100644 --- a/drivers/pci/pci-uclass.c +++ b/drivers/pci/pci-uclass.c @@ -670,6 +670,33 @@ error: return ret; }
+static int find_pci_of_offset(struct udevice *bus, struct udevice *dev, + pci_dev_t bdf) +{ + struct fdt_pci_addr addr; + int offset, err; + + if (bus->of_offset == -1) + return -ENOENT; + + fdt_for_each_subnode(gd->fdt_blob, offset, bus->of_offset) { + err = fdtdec_get_pci_addr(gd->fdt_blob, offset, + FDT_PCI_SPACE_CONFIG, "reg", &addr); + if (err == -ENOENT) + continue; + if (err) + return err; + + if (bdf != (addr.phys_hi & 0xffff00)) + continue; + + dev->of_offset = offset; + return 0; + } + + return -ENOENT; +} + int pci_bind_bus_devices(struct udevice *bus) { ulong vendor, device; @@ -731,6 +758,11 @@ int pci_bind_bus_devices(struct udevice *bus) } ret = pci_find_and_bind_driver(bus, &find_id, bdf, &dev); + if (!ret) { + ret = find_pci_of_offset(bus, dev, bdf); + if (ret == -ENOENT) + ret = 0; + } } if (ret == -EPERM) continue;
Hi Paul,
On 26 September 2016 at 12:29, Paul Burton paul.burton@imgtec.com wrote:
A PCI device may be probed through standard PCI config space probing but still be represented in a device tree. However U-Boot would not previously set the of_offset field of the struct udevice for such PCI devices. Fix this by searching for a DT node based upon its "reg" property after binding a PCI device that wasn't already seen in the DT.
Signed-off-by: Paul Burton paul.burton@imgtec.com
drivers/pci/pci-uclass.c | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+)
I can't see how this can happen. The PCI binding is supposed to operate using the device tree:
/* Find this device in the device tree */ ret = pci_bus_find_devfn(bus, PCI_MASK_BUS(bdf), &dev);
Do you know what is going wrong?
Regards, Simon
The decode_regions() function in the PCI code presumes that CONFIG_SYS_SDRAM_BASE is a physical address, which seems reasonable given that README states that it should be.
However there is also common code which expects CONFIG_SYS_SDRAM_BASE to be an address accessible by the CPU, ie. a valid virtual address - notably gd->ram_top is set to it & various pieces of data are located relative to that, and getenv_bootm_low() defaults to CONFIG_SYS_SDRAM_BASE as the lower bound on addresses to load into. Thus on MIPS CONFIG_SYS_SDRAM_BASE is a virtual address.
This patch takes the simple approach to fixing this & converts CONFIG_SYS_SDRAM_BASE to a physical address for use by the PCI code when built for MIPS.
Signed-off-by: Paul Burton paul.burton@imgtec.com
---
drivers/pci/pci-uclass.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c index 415c632..26fe0f4 100644 --- a/drivers/pci/pci-uclass.c +++ b/drivers/pci/pci-uclass.c @@ -848,7 +848,9 @@ static int decode_regions(struct pci_controller *hose, const void *blob,
/* Add a region for our local memory */ size = gd->ram_size; -#ifdef CONFIG_SYS_SDRAM_BASE +#if defined(CONFIG_MIPS) + base = virt_to_phys((void *)CONFIG_SYS_SDRAM_BASE); +#elif defined(CONFIG_SYS_SDRAM_BASE) base = CONFIG_SYS_SDRAM_BASE; #endif if (gd->pci_ram_top && gd->pci_ram_top < base + size)
Hi Paul,
On 26 September 2016 at 12:29, Paul Burton paul.burton@imgtec.com wrote:
The decode_regions() function in the PCI code presumes that CONFIG_SYS_SDRAM_BASE is a physical address, which seems reasonable given that README states that it should be.
However there is also common code which expects CONFIG_SYS_SDRAM_BASE to be an address accessible by the CPU, ie. a valid virtual address - notably gd->ram_top is set to it & various pieces of data are located relative to that, and getenv_bootm_low() defaults to CONFIG_SYS_SDRAM_BASE as the lower bound on addresses to load into. Thus on MIPS CONFIG_SYS_SDRAM_BASE is a virtual address.
This patch takes the simple approach to fixing this & converts CONFIG_SYS_SDRAM_BASE to a physical address for use by the PCI code when built for MIPS.
Signed-off-by: Paul Burton paul.burton@imgtec.com
drivers/pci/pci-uclass.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c index 415c632..26fe0f4 100644 --- a/drivers/pci/pci-uclass.c +++ b/drivers/pci/pci-uclass.c @@ -848,7 +848,9 @@ static int decode_regions(struct pci_controller *hose, const void *blob,
/* Add a region for our local memory */ size = gd->ram_size;-#ifdef CONFIG_SYS_SDRAM_BASE +#if defined(CONFIG_MIPS)
base = virt_to_phys((void *)CONFIG_SYS_SDRAM_BASE);+#elif defined(CONFIG_SYS_SDRAM_BASE) base = CONFIG_SYS_SDRAM_BASE; #endif if (gd->pci_ram_top && gd->pci_ram_top < base + size) -- 2.10.0
Can you find another way? We don't want arch- or board-specific #ifdefs in uclasses or driver code.
Regards, Simon
2016-09-27 2:35 GMT+02:00 Simon Glass sjg@chromium.org:
Hi Paul,
On 26 September 2016 at 12:29, Paul Burton paul.burton@imgtec.com wrote:
The decode_regions() function in the PCI code presumes that CONFIG_SYS_SDRAM_BASE is a physical address, which seems reasonable given that README states that it should be.
However there is also common code which expects CONFIG_SYS_SDRAM_BASE to be an address accessible by the CPU, ie. a valid virtual address - notably gd->ram_top is set to it & various pieces of data are located relative to that, and getenv_bootm_low() defaults to CONFIG_SYS_SDRAM_BASE as the lower bound on addresses to load into. Thus on MIPS CONFIG_SYS_SDRAM_BASE is a virtual address.
This patch takes the simple approach to fixing this & converts CONFIG_SYS_SDRAM_BASE to a physical address for use by the PCI code when built for MIPS.
Signed-off-by: Paul Burton paul.burton@imgtec.com
drivers/pci/pci-uclass.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c index 415c632..26fe0f4 100644 --- a/drivers/pci/pci-uclass.c +++ b/drivers/pci/pci-uclass.c @@ -848,7 +848,9 @@ static int decode_regions(struct pci_controller *hose, const void *blob,
/* Add a region for our local memory */ size = gd->ram_size;-#ifdef CONFIG_SYS_SDRAM_BASE +#if defined(CONFIG_MIPS)
base = virt_to_phys((void *)CONFIG_SYS_SDRAM_BASE);+#elif defined(CONFIG_SYS_SDRAM_BASE) base = CONFIG_SYS_SDRAM_BASE; #endif if (gd->pci_ram_top && gd->pci_ram_top < base + size) -- 2.10.0
Can you find another way? We don't want arch- or board-specific #ifdefs in uclasses or driver code.
I suggest that we fix all MIPS boards to configure CONFIG_SYS_SDRAM_BASE as physical address. I guess that would be simply 0 for all current supported MIPS boards. In "arch/mips/" there are only two users ("arch/mips/cpu/start.S" and "arch/mips/lib/bootm.c") which could be easily fixed to map CONFIG_SYS_SDRAM_BASE to KSEG0.
On Tuesday, 27 September 2016 12:01:32 BST Daniel Schwierzeck wrote:
2016-09-27 2:35 GMT+02:00 Simon Glass sjg@chromium.org:
Hi Paul,
On 26 September 2016 at 12:29, Paul Burton paul.burton@imgtec.com wrote:
The decode_regions() function in the PCI code presumes that CONFIG_SYS_SDRAM_BASE is a physical address, which seems reasonable given that README states that it should be.
However there is also common code which expects CONFIG_SYS_SDRAM_BASE to be an address accessible by the CPU, ie. a valid virtual address - notably gd->ram_top is set to it & various pieces of data are located relative to that, and getenv_bootm_low() defaults to CONFIG_SYS_SDRAM_BASE as the lower bound on addresses to load into. Thus on MIPS CONFIG_SYS_SDRAM_BASE is a virtual address.
This patch takes the simple approach to fixing this & converts CONFIG_SYS_SDRAM_BASE to a physical address for use by the PCI code when built for MIPS.
Signed-off-by: Paul Burton paul.burton@imgtec.com
drivers/pci/pci-uclass.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c index 415c632..26fe0f4 100644 --- a/drivers/pci/pci-uclass.c +++ b/drivers/pci/pci-uclass.c @@ -848,7 +848,9 @@ static int decode_regions(struct pci_controller *hose, const void *blob,>> /* Add a region for our local memory */ size = gd->ram_size;
-#ifdef CONFIG_SYS_SDRAM_BASE +#if defined(CONFIG_MIPS)
base = virt_to_phys((void *)CONFIG_SYS_SDRAM_BASE);+#elif defined(CONFIG_SYS_SDRAM_BASE)
base = CONFIG_SYS_SDRAM_BASE;#endif
if (gd->pci_ram_top && gd->pci_ram_top < base + size)-- 2.10.0
Can you find another way? We don't want arch- or board-specific #ifdefs in uclasses or driver code.
I suggest that we fix all MIPS boards to configure CONFIG_SYS_SDRAM_BASE as physical address. I guess that would be simply 0 for all current supported MIPS boards. In "arch/mips/" there are only two users ("arch/mips/cpu/start.S" and "arch/mips/lib/bootm.c") which could be easily fixed to map CONFIG_SYS_SDRAM_BASE to KSEG0.
Hi Daniel & Simon,
So doing that led to the 27 patch series I just sent out. It's never simple :) It's not so much the 2 users in arch/mips/ as the generic uses which cause the trouble - and avoiding them needs something like phys_to_virt() so the bulk of that series is about providing that for all architectures.
FYI CONFIG_SYS_SDRAM_BASE is zero for all MIPS boards except the PIC32 one. I've tested the series on a 32 bit Malta & a 64 bit boston, and build tested a bunch of other boards. The only thing to really take note of is that for MIPS64 systems using a physical CONFIG_SYS_SDRAM_BASE and phys_to_virt we end up accessing memory through xkphys instead of ckseg0. Nothing seems to have exploded too badly from that (just the boston board_get_usable_ram_top() needed adjusting) but it's probably worth pointing out.
Thanks, Paul
On some systems aligning PCI memory to a 1MB boundary every time a bridge is encountered may lead to exhausting the available memory space before all devices have been assigned addresses.
For example on the MIPS Boston development board we have an Intel EG20T Platform Controller Hub connected to a Xilinx AXI to PCIe root port which is only assigned a 1MB memory region. The Intel EG20T contains a bridge device beneath which all of its peripheral devices can be found, and that bridge device contains a ROM. If we align to 1MB when we encounter each bridge device we therefore do something like this:
- Start with bus_lower at 0x16000000.
- Find the Xilinx root bridge, which has no visible BARs so we do very little to it.
- Probe the bus beneath the Xilinx bridge device, aligning bus_lower to a 1MB boundary first. That leaves it still at 0x16000000.
- Find the EG20T bridge device, which we find has a 64KiB ROM. We assign it the address range 0x16000000-0x1600ffff which leaves bus_lower at 0x16010000.
- Probe the bus beneath the EG20T bridge device, aligning bus_lower to a 1MB boundary first. This leaves bus_lower at 0x16100000, which is the end of the available memory space.
- Find the various peripheral devices the EG20T contains, but fail to assign any memory space to them since bus_lower is at the end of the memory space available to the PCI bus.
This patch allows that 1MB alignment to be changed or disabled via Kconfig, so that systems such as this can select an alignment which works for them.
Signed-off-by: Paul Burton paul.burton@imgtec.com
---
drivers/pci/Kconfig | 7 +++++++ drivers/pci/pci_auto.c | 24 ++++++++++++++++-------- 2 files changed, 23 insertions(+), 8 deletions(-)
diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig index 9a7c187..35bf8fe 100644 --- a/drivers/pci/Kconfig +++ b/drivers/pci/Kconfig @@ -18,6 +18,13 @@ config DM_PCI_COMPAT measure when porting a board to use driver model for PCI. Once the board is fully supported, this option should be disabled.
+config DM_PCI_BRIDGE_MEM_ALIGN + hex "PCI bridge memory alignment" + default 0x100000 + help + PCI memory space will be aligned to be a multiple of this value + whenever a PCI-to-PCI bridge is encountered. + config PCI_SANDBOX bool "Sandbox PCI support" depends on SANDBOX && DM_PCI diff --git a/drivers/pci/pci_auto.c b/drivers/pci/pci_auto.c index ee9a854..0326316 100644 --- a/drivers/pci/pci_auto.c +++ b/drivers/pci/pci_auto.c @@ -186,8 +186,10 @@ void dm_pciauto_prescan_setup_bridge(struct udevice *dev, int sub_bus) dm_pci_write_config8(dev, PCI_SUBORDINATE_BUS, 0xff);
if (pci_mem) { - /* Round memory allocator to 1MB boundary */ - pciauto_region_align(pci_mem, 0x100000); + /* Round memory allocator to a suitable boundary */ + if (CONFIG_DM_PCI_BRIDGE_MEM_ALIGN) + pciauto_region_align(pci_mem, + CONFIG_DM_PCI_BRIDGE_MEM_ALIGN);
/* * Set up memory and I/O filter limits, assume 32-bit @@ -200,8 +202,10 @@ void dm_pciauto_prescan_setup_bridge(struct udevice *dev, int sub_bus) }
if (pci_prefetch) { - /* Round memory allocator to 1MB boundary */ - pciauto_region_align(pci_prefetch, 0x100000); + /* Round memory allocator to a suitable boundary */ + if (CONFIG_DM_PCI_BRIDGE_MEM_ALIGN) + pciauto_region_align(pci_prefetch, + CONFIG_DM_PCI_BRIDGE_MEM_ALIGN);
/* * Set up memory and I/O filter limits, assume 32-bit @@ -260,8 +264,10 @@ void dm_pciauto_postscan_setup_bridge(struct udevice *dev, int sub_bus) dm_pci_write_config8(dev, PCI_SUBORDINATE_BUS, sub_bus);
if (pci_mem) { - /* Round memory allocator to 1MB boundary */ - pciauto_region_align(pci_mem, 0x100000); + /* Round memory allocator to a suitable boundary */ + if (CONFIG_DM_PCI_BRIDGE_MEM_ALIGN) + pciauto_region_align(pci_mem, + CONFIG_DM_PCI_BRIDGE_MEM_ALIGN);
dm_pci_write_config16(dev, PCI_MEMORY_LIMIT, (pci_mem->bus_lower - 1) >> 16); @@ -274,8 +280,10 @@ void dm_pciauto_postscan_setup_bridge(struct udevice *dev, int sub_bus) &prefechable_64); prefechable_64 &= PCI_PREF_RANGE_TYPE_MASK;
- /* Round memory allocator to 1MB boundary */ - pciauto_region_align(pci_prefetch, 0x100000); + /* Round memory allocator to a suitable boundary */ + if (CONFIG_DM_PCI_BRIDGE_MEM_ALIGN) + pciauto_region_align(pci_prefetch, + CONFIG_DM_PCI_BRIDGE_MEM_ALIGN);
dm_pci_write_config16(dev, PCI_PREF_MEMORY_LIMIT, (pci_prefetch->bus_lower - 1) >> 16);
Hi Paul,
On 26 September 2016 at 12:29, Paul Burton paul.burton@imgtec.com wrote:
On some systems aligning PCI memory to a 1MB boundary every time a bridge is encountered may lead to exhausting the available memory space before all devices have been assigned addresses.
For example on the MIPS Boston development board we have an Intel EG20T Platform Controller Hub connected to a Xilinx AXI to PCIe root port which is only assigned a 1MB memory region. The Intel EG20T contains a bridge device beneath which all of its peripheral devices can be found, and that bridge device contains a ROM. If we align to 1MB when we encounter each bridge device we therefore do something like this:
Start with bus_lower at 0x16000000.
Find the Xilinx root bridge, which has no visible BARs so we do very little to it.
Probe the bus beneath the Xilinx bridge device, aligning bus_lower to a 1MB boundary first. That leaves it still at 0x16000000.
Find the EG20T bridge device, which we find has a 64KiB ROM. We assign it the address range 0x16000000-0x1600ffff which leaves bus_lower at 0x16010000.
Probe the bus beneath the EG20T bridge device, aligning bus_lower to a 1MB boundary first. This leaves bus_lower at 0x16100000, which is the end of the available memory space.
Find the various peripheral devices the EG20T contains, but fail to assign any memory space to them since bus_lower is at the end of the memory space available to the PCI bus.
This patch allows that 1MB alignment to be changed or disabled via Kconfig, so that systems such as this can select an alignment which works for them.
Signed-off-by: Paul Burton paul.burton@imgtec.com
drivers/pci/Kconfig | 7 +++++++ drivers/pci/pci_auto.c | 24 ++++++++++++++++-------- 2 files changed, 23 insertions(+), 8 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
Please see below
diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig index 9a7c187..35bf8fe 100644 --- a/drivers/pci/Kconfig +++ b/drivers/pci/Kconfig @@ -18,6 +18,13 @@ config DM_PCI_COMPAT measure when porting a board to use driver model for PCI. Once the board is fully supported, this option should be disabled.
+config DM_PCI_BRIDGE_MEM_ALIGN
hex "PCI bridge memory alignment"default 0x100000helpPCI memory space will be aligned to be a multiple of this valuewhenever a PCI-to-PCI bridge is encountered.
Can you also add why this is useful, and why 1MB is a suitable default?
config PCI_SANDBOX bool "Sandbox PCI support" depends on SANDBOX && DM_PCI diff --git a/drivers/pci/pci_auto.c b/drivers/pci/pci_auto.c index ee9a854..0326316 100644 --- a/drivers/pci/pci_auto.c +++ b/drivers/pci/pci_auto.c @@ -186,8 +186,10 @@ void dm_pciauto_prescan_setup_bridge(struct udevice *dev, int sub_bus) dm_pci_write_config8(dev, PCI_SUBORDINATE_BUS, 0xff);
if (pci_mem) {
/* Round memory allocator to 1MB boundary */pciauto_region_align(pci_mem, 0x100000);
/* Round memory allocator to a suitable boundary */if (CONFIG_DM_PCI_BRIDGE_MEM_ALIGN)
Can you update the pciauto_region_align() so you can omit this if () ?
pciauto_region_align(pci_mem,CONFIG_DM_PCI_BRIDGE_MEM_ALIGN); /* * Set up memory and I/O filter limits, assume 32-bit@@ -200,8 +202,10 @@ void dm_pciauto_prescan_setup_bridge(struct udevice *dev, int sub_bus) }
if (pci_prefetch) {
/* Round memory allocator to 1MB boundary */pciauto_region_align(pci_prefetch, 0x100000);
/* Round memory allocator to a suitable boundary */if (CONFIG_DM_PCI_BRIDGE_MEM_ALIGN)pciauto_region_align(pci_prefetch,CONFIG_DM_PCI_BRIDGE_MEM_ALIGN); /* * Set up memory and I/O filter limits, assume 32-bit@@ -260,8 +264,10 @@ void dm_pciauto_postscan_setup_bridge(struct udevice *dev, int sub_bus) dm_pci_write_config8(dev, PCI_SUBORDINATE_BUS, sub_bus);
if (pci_mem) {
/* Round memory allocator to 1MB boundary */pciauto_region_align(pci_mem, 0x100000);
/* Round memory allocator to a suitable boundary */if (CONFIG_DM_PCI_BRIDGE_MEM_ALIGN)pciauto_region_align(pci_mem,CONFIG_DM_PCI_BRIDGE_MEM_ALIGN); dm_pci_write_config16(dev, PCI_MEMORY_LIMIT, (pci_mem->bus_lower - 1) >> 16);@@ -274,8 +280,10 @@ void dm_pciauto_postscan_setup_bridge(struct udevice *dev, int sub_bus) &prefechable_64); prefechable_64 &= PCI_PREF_RANGE_TYPE_MASK;
/* Round memory allocator to 1MB boundary */pciauto_region_align(pci_prefetch, 0x100000);
/* Round memory allocator to a suitable boundary */if (CONFIG_DM_PCI_BRIDGE_MEM_ALIGN)pciauto_region_align(pci_prefetch,CONFIG_DM_PCI_BRIDGE_MEM_ALIGN); dm_pci_write_config16(dev, PCI_PREF_MEMORY_LIMIT, (pci_prefetch->bus_lower - 1) >> 16);-- 2.10.0
Regards, Simon
On the MIPS Boston development board we have an Intel EG20T Platform Controller Hub connected to a Xilinx AXI to PCIe root port which is only assigned a 1MB memory region. The Intel EG20T contains a bridge device beneath which all of its peripheral devices can be found, and that bridge device contains a ROM. If we align to 1MB when we encounter each bridge device we therefore do something like this:
- Start with bus_lower at 0x16000000.
- Find the Xilinx root bridge, which has no visible BARs so we do very little to it.
- Probe the bus beneath the Xilinx bridge device, aligning bus_lower to a 1MB boundary first. That leaves it still at 0x16000000.
- Find the EG20T bridge device, which we find has a 64KiB ROM. We assign it the address range 0x16000000-0x1600ffff which leaves bus_lower at 0x16010000.
- Probe the bus beneath the EG20T bridge device, aligning bus_lower to a 1MB boundary first. This leaves bus_lower at 0x16100000, which is the end of the available memory space.
- Find the various peripheral devices the EG20T contains, but fail to assign any memory space to them since bus_lower is at the end of the memory space available to the PCI bus.
Fix this by disabling that 1MB alignment, which allows all of the EG20T peripheral devices to be assigned memory space within the 1MB region available.
Signed-off-by: Paul Burton paul.burton@imgtec.com
---
configs/boston32r2_defconfig | 1 + configs/boston32r2el_defconfig | 1 + configs/boston64r2_defconfig | 1 + configs/boston64r2el_defconfig | 1 + 4 files changed, 4 insertions(+)
diff --git a/configs/boston32r2_defconfig b/configs/boston32r2_defconfig index ca66248..e5f61b8 100644 --- a/configs/boston32r2_defconfig +++ b/configs/boston32r2_defconfig @@ -36,6 +36,7 @@ CONFIG_CFI_FLASH=y CONFIG_DM_ETH=y CONFIG_PCH_GBE=y CONFIG_DM_PCI=y +CONFIG_DM_PCI_BRIDGE_MEM_ALIGN=0x0 CONFIG_PCI_XILINX=y CONFIG_SYS_NS16550=y CONFIG_LZ4=y diff --git a/configs/boston32r2el_defconfig b/configs/boston32r2el_defconfig index 67f54bf..e9a23b8 100644 --- a/configs/boston32r2el_defconfig +++ b/configs/boston32r2el_defconfig @@ -37,6 +37,7 @@ CONFIG_CFI_FLASH=y CONFIG_DM_ETH=y CONFIG_PCH_GBE=y CONFIG_DM_PCI=y +CONFIG_DM_PCI_BRIDGE_MEM_ALIGN=0x0 CONFIG_PCI_XILINX=y CONFIG_SYS_NS16550=y CONFIG_LZ4=y diff --git a/configs/boston64r2_defconfig b/configs/boston64r2_defconfig index 1245d1b..55943c5 100644 --- a/configs/boston64r2_defconfig +++ b/configs/boston64r2_defconfig @@ -36,6 +36,7 @@ CONFIG_CFI_FLASH=y CONFIG_DM_ETH=y CONFIG_PCH_GBE=y CONFIG_DM_PCI=y +CONFIG_DM_PCI_BRIDGE_MEM_ALIGN=0x0 CONFIG_PCI_XILINX=y CONFIG_SYS_NS16550=y CONFIG_LZ4=y diff --git a/configs/boston64r2el_defconfig b/configs/boston64r2el_defconfig index 9b5fa5a..865177d 100644 --- a/configs/boston64r2el_defconfig +++ b/configs/boston64r2el_defconfig @@ -37,6 +37,7 @@ CONFIG_CFI_FLASH=y CONFIG_DM_ETH=y CONFIG_PCH_GBE=y CONFIG_DM_PCI=y +CONFIG_DM_PCI_BRIDGE_MEM_ALIGN=0x0 CONFIG_PCI_XILINX=y CONFIG_SYS_NS16550=y CONFIG_LZ4=y
On 26 September 2016 at 12:29, Paul Burton paul.burton@imgtec.com wrote:
On the MIPS Boston development board we have an Intel EG20T Platform Controller Hub connected to a Xilinx AXI to PCIe root port which is only assigned a 1MB memory region. The Intel EG20T contains a bridge device beneath which all of its peripheral devices can be found, and that bridge device contains a ROM. If we align to 1MB when we encounter each bridge device we therefore do something like this:
Start with bus_lower at 0x16000000.
Find the Xilinx root bridge, which has no visible BARs so we do very little to it.
Probe the bus beneath the Xilinx bridge device, aligning bus_lower to a 1MB boundary first. That leaves it still at 0x16000000.
Find the EG20T bridge device, which we find has a 64KiB ROM. We assign it the address range 0x16000000-0x1600ffff which leaves bus_lower at 0x16010000.
Probe the bus beneath the EG20T bridge device, aligning bus_lower to a 1MB boundary first. This leaves bus_lower at 0x16100000, which is the end of the available memory space.
Find the various peripheral devices the EG20T contains, but fail to assign any memory space to them since bus_lower is at the end of the memory space available to the PCI bus.
Fix this by disabling that 1MB alignment, which allows all of the EG20T peripheral devices to be assigned memory space within the 1MB region available.
Signed-off-by: Paul Burton paul.burton@imgtec.com
configs/boston32r2_defconfig | 1 + configs/boston32r2el_defconfig | 1 + configs/boston64r2_defconfig | 1 + configs/boston64r2el_defconfig | 1 + 4 files changed, 4 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
On Tue, Sep 27, 2016 at 2:29 AM, Paul Burton paul.burton@imgtec.com wrote:
On the MIPS Boston development board we have an Intel EG20T Platform Controller Hub connected to a Xilinx AXI to PCIe root port which is only assigned a 1MB memory region. The Intel EG20T contains a bridge device beneath which all of its peripheral devices can be found, and that bridge device contains a ROM. If we align to 1MB when we encounter each bridge device we therefore do something like this:
Start with bus_lower at 0x16000000.
Find the Xilinx root bridge, which has no visible BARs so we do very little to it.
Probe the bus beneath the Xilinx bridge device, aligning bus_lower to a 1MB boundary first. That leaves it still at 0x16000000.
Find the EG20T bridge device, which we find has a 64KiB ROM. We assign it the address range 0x16000000-0x1600ffff which leaves bus_lower at 0x16010000.
Probe the bus beneath the EG20T bridge device, aligning bus_lower to a 1MB boundary first. This leaves bus_lower at 0x16100000, which is the end of the available memory space.
Find the various peripheral devices the EG20T contains, but fail to assign any memory space to them since bus_lower is at the end of the memory space available to the PCI bus.
Fix this by disabling that 1MB alignment, which allows all of the EG20T peripheral devices to be assigned memory space within the 1MB region available.
Signed-off-by: Paul Burton paul.burton@imgtec.com
configs/boston32r2_defconfig | 1 + configs/boston32r2el_defconfig | 1 + configs/boston64r2_defconfig | 1 + configs/boston64r2el_defconfig | 1 + 4 files changed, 4 insertions(+)
Reviewed-by: Bin Meng bmeng.cn@gmail.com
Using the EG20T gigabit ethernet controller on the MIPS Boston board, we find that we have to reset the controller in order for the RGMII link to the PHY to become functional. Without doing so we constantly time out in pch_gbe_mdio_ready.
Signed-off-by: Paul Burton paul.burton@imgtec.com ---
drivers/net/pch_gbe.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/drivers/net/pch_gbe.c b/drivers/net/pch_gbe.c index d40fff0..4aac0f6 100644 --- a/drivers/net/pch_gbe.c +++ b/drivers/net/pch_gbe.c @@ -422,6 +422,7 @@ int pch_gbe_probe(struct udevice *dev) struct pch_gbe_priv *priv; struct eth_pdata *plat = dev_get_platdata(dev); void *iobase; + int err;
/* * The priv structure contains the descriptors and frame buffers which @@ -444,6 +445,10 @@ int pch_gbe_probe(struct udevice *dev) pch_gbe_mdio_init(dev->name, priv->mac_regs); priv->bus = miiphy_get_dev_by_name(dev->name);
+ err = pch_gbe_reset(dev); + if (err) + return err; + return pch_gbe_phy_init(dev); }
On Tue, Sep 27, 2016 at 2:29 AM, Paul Burton paul.burton@imgtec.com wrote:
Using the EG20T gigabit ethernet controller on the MIPS Boston board, we find that we have to reset the controller in order for the RGMII link to the PHY to become functional. Without doing so we constantly time out in pch_gbe_mdio_ready.
Signed-off-by: Paul Burton paul.burton@imgtec.com
drivers/net/pch_gbe.c | 5 +++++ 1 file changed, 5 insertions(+)
Reviewed-by: Bin Meng bmeng.cn@gmail.com
Tested on Crown Bay Tested-by: Bin Meng bmeng.cn@gmail.com
The loop to set up buffer addresses in rx descriptors always operated on descriptor 0, rather than on each descriptor sequentially. Fix this in order to setup correct buffer addresses for each descriptor.
Signed-off-by: Paul Burton paul.burton@imgtec.com ---
drivers/net/pch_gbe.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/pch_gbe.c b/drivers/net/pch_gbe.c index 4aac0f6..e41d737 100644 --- a/drivers/net/pch_gbe.c +++ b/drivers/net/pch_gbe.c @@ -117,8 +117,8 @@ static void pch_gbe_rx_descs_init(struct udevice *dev)
memset(rx_desc, 0, sizeof(struct pch_gbe_rx_desc) * PCH_GBE_DESC_NUM); for (i = 0; i < PCH_GBE_DESC_NUM; i++) - rx_desc->buffer_addr = dm_pci_phys_to_mem(priv->dev, - (ulong)(priv->rx_buff[i])); + rx_desc[i].buffer_addr = dm_pci_virt_to_mem(priv->dev, + priv->rx_buff[i]);
writel(dm_pci_phys_to_mem(priv->dev, (ulong)rx_desc), &mac_regs->rx_dsc_base);
Hi Paul,
On Tue, Sep 27, 2016 at 2:29 AM, Paul Burton paul.burton@imgtec.com wrote:
The loop to set up buffer addresses in rx descriptors always operated on descriptor 0, rather than on each descriptor sequentially. Fix this in order to setup correct buffer addresses for each descriptor.
Signed-off-by: Paul Burton paul.burton@imgtec.com
drivers/net/pch_gbe.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/pch_gbe.c b/drivers/net/pch_gbe.c index 4aac0f6..e41d737 100644 --- a/drivers/net/pch_gbe.c +++ b/drivers/net/pch_gbe.c @@ -117,8 +117,8 @@ static void pch_gbe_rx_descs_init(struct udevice *dev)
memset(rx_desc, 0, sizeof(struct pch_gbe_rx_desc) * PCH_GBE_DESC_NUM); for (i = 0; i < PCH_GBE_DESC_NUM; i++)
rx_desc->buffer_addr = dm_pci_phys_to_mem(priv->dev,(ulong)(priv->rx_buff[i]));
rx_desc[i].buffer_addr = dm_pci_virt_to_mem(priv->dev,
The change from phys_to_men to virt_to_mem should be put in the patch#12 of this series.
priv->rx_buff[i]); writel(dm_pci_phys_to_mem(priv->dev, (ulong)rx_desc), &mac_regs->rx_dsc_base);--
Other than that,
Reviewed-by: Bin Meng bmeng.cn@gmail.com
Tested on Crown Bay Tested-by: Bin Meng bmeng.cn@gmail.com
Regards, Bin
Use the virt_to_bus & bus_to_virt functions rather than phys_to_bus & bus_to_phys, since the addresses accessed by the CPU will be virtual rather than physical. On MIPS physical & virtual addresses differ as we use virtual addresses in kseg0, and attempting to use physical addresses directly caused problems as they're in the user segment which would be mapped via the uninitialised TLB.
Signed-off-by: Paul Burton paul.burton@imgtec.com ---
drivers/net/pch_gbe.c | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/drivers/net/pch_gbe.c b/drivers/net/pch_gbe.c index e41d737..1432351 100644 --- a/drivers/net/pch_gbe.c +++ b/drivers/net/pch_gbe.c @@ -120,12 +120,12 @@ static void pch_gbe_rx_descs_init(struct udevice *dev) rx_desc[i].buffer_addr = dm_pci_virt_to_mem(priv->dev, priv->rx_buff[i]);
- writel(dm_pci_phys_to_mem(priv->dev, (ulong)rx_desc), + writel(dm_pci_virt_to_mem(priv->dev, rx_desc), &mac_regs->rx_dsc_base); writel(sizeof(struct pch_gbe_rx_desc) * (PCH_GBE_DESC_NUM - 1), &mac_regs->rx_dsc_size);
- writel(dm_pci_phys_to_mem(priv->dev, (ulong)(rx_desc + 1)), + writel(dm_pci_virt_to_mem(priv->dev, rx_desc + 1), &mac_regs->rx_dsc_sw_p); }
@@ -137,11 +137,11 @@ static void pch_gbe_tx_descs_init(struct udevice *dev)
memset(tx_desc, 0, sizeof(struct pch_gbe_tx_desc) * PCH_GBE_DESC_NUM);
- writel(dm_pci_phys_to_mem(priv->dev, (ulong)tx_desc), + writel(dm_pci_virt_to_mem(priv->dev, tx_desc), &mac_regs->tx_dsc_base); writel(sizeof(struct pch_gbe_tx_desc) * (PCH_GBE_DESC_NUM - 1), &mac_regs->tx_dsc_size); - writel(dm_pci_phys_to_mem(priv->dev, (ulong)(tx_desc + 1)), + writel(dm_pci_virt_to_mem(priv->dev, tx_desc + 1), &mac_regs->tx_dsc_sw_p); }
@@ -251,7 +251,7 @@ static int pch_gbe_send(struct udevice *dev, void *packet, int length) if (length < 64) frame_ctrl |= PCH_GBE_TXD_CTRL_APAD;
- tx_desc->buffer_addr = dm_pci_phys_to_mem(priv->dev, (ulong)packet); + tx_desc->buffer_addr = dm_pci_virt_to_mem(priv->dev, packet); tx_desc->length = length; tx_desc->tx_words_eob = length + 3; tx_desc->tx_frame_ctrl = frame_ctrl; @@ -262,7 +262,7 @@ static int pch_gbe_send(struct udevice *dev, void *packet, int length) if (++priv->tx_idx >= PCH_GBE_DESC_NUM) priv->tx_idx = 0;
- writel(dm_pci_phys_to_mem(priv->dev, (ulong)(tx_head + priv->tx_idx)), + writel(dm_pci_virt_to_mem(priv->dev, tx_head + priv->tx_idx), &mac_regs->tx_dsc_sw_p);
start = get_timer(0); @@ -283,7 +283,8 @@ static int pch_gbe_recv(struct udevice *dev, int flags, uchar **packetp) struct pch_gbe_priv *priv = dev_get_priv(dev); struct pch_gbe_regs *mac_regs = priv->mac_regs; struct pch_gbe_rx_desc *rx_desc; - ulong hw_desc, buffer_addr, length; + ulong hw_desc, length; + void *buffer;
rx_desc = &priv->rx_desc[priv->rx_idx];
@@ -291,12 +292,12 @@ static int pch_gbe_recv(struct udevice *dev, int flags, uchar **packetp) hw_desc = readl(&mac_regs->rx_dsc_hw_p_hld);
/* Just return if not receiving any packet */ - if ((ulong)rx_desc == hw_desc) + if (virt_to_phys(rx_desc) == hw_desc) return -EAGAIN;
- buffer_addr = dm_pci_mem_to_phys(priv->dev, rx_desc->buffer_addr); - *packetp = (uchar *)buffer_addr; length = rx_desc->rx_words_eob - 3 - ETH_FCS_LEN; + buffer = dm_pci_mem_to_virt(priv->dev, rx_desc->buffer_addr, length, 0); + *packetp = (uchar *)buffer;
return length; } @@ -315,7 +316,7 @@ static int pch_gbe_free_pkt(struct udevice *dev, uchar *packet, int length) if (++rx_swp >= PCH_GBE_DESC_NUM) rx_swp = 0;
- writel(dm_pci_phys_to_mem(priv->dev, (ulong)(rx_head + rx_swp)), + writel(dm_pci_virt_to_mem(priv->dev, rx_head + rx_swp), &mac_regs->rx_dsc_sw_p);
return 0;
On Tue, Sep 27, 2016 at 2:29 AM, Paul Burton paul.burton@imgtec.com wrote:
Use the virt_to_bus & bus_to_virt functions rather than phys_to_bus & bus_to_phys, since the addresses accessed by the CPU will be virtual rather than physical. On MIPS physical & virtual addresses differ as we use virtual addresses in kseg0, and attempting to use physical addresses directly caused problems as they're in the user segment which would be mapped via the uninitialised TLB.
Signed-off-by: Paul Burton paul.burton@imgtec.com
drivers/net/pch_gbe.c | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com
Tested on Crown Bay Tested-by: Bin Meng bmeng.cn@gmail.com
On MIPS systems DMA isn't coherent with the CPU caches unless an IOCU is present. When there is no IOCU we need to writeback or invalidate the data caches at appropriate points. Perform this cache maintenance in the pch_gbe driver which is used on the MIPS Boston development board.
Signed-off-by: Paul Burton paul.burton@imgtec.com ---
drivers/net/pch_gbe.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
diff --git a/drivers/net/pch_gbe.c b/drivers/net/pch_gbe.c index 1432351..8866f66 100644 --- a/drivers/net/pch_gbe.c +++ b/drivers/net/pch_gbe.c @@ -120,6 +120,8 @@ static void pch_gbe_rx_descs_init(struct udevice *dev) rx_desc[i].buffer_addr = dm_pci_virt_to_mem(priv->dev, priv->rx_buff[i]);
+ flush_dcache_range((ulong)rx_desc, (ulong)&rx_desc[PCH_GBE_DESC_NUM]); + writel(dm_pci_virt_to_mem(priv->dev, rx_desc), &mac_regs->rx_dsc_base); writel(sizeof(struct pch_gbe_rx_desc) * (PCH_GBE_DESC_NUM - 1), @@ -137,6 +139,8 @@ static void pch_gbe_tx_descs_init(struct udevice *dev)
memset(tx_desc, 0, sizeof(struct pch_gbe_tx_desc) * PCH_GBE_DESC_NUM);
+ flush_dcache_range((ulong)tx_desc, (ulong)&tx_desc[PCH_GBE_DESC_NUM]); + writel(dm_pci_virt_to_mem(priv->dev, tx_desc), &mac_regs->tx_dsc_base); writel(sizeof(struct pch_gbe_tx_desc) * (PCH_GBE_DESC_NUM - 1), @@ -245,6 +249,8 @@ static int pch_gbe_send(struct udevice *dev, void *packet, int length) u32 int_st; ulong start;
+ flush_dcache_range((ulong)packet, (ulong)packet + length); + tx_head = &priv->tx_desc[0]; tx_desc = &priv->tx_desc[priv->tx_idx];
@@ -258,6 +264,8 @@ static int pch_gbe_send(struct udevice *dev, void *packet, int length) tx_desc->dma_status = 0; tx_desc->gbec_status = 0;
+ flush_dcache_range((ulong)tx_desc, (ulong)&tx_desc[1]); + /* Test the wrap-around condition */ if (++priv->tx_idx >= PCH_GBE_DESC_NUM) priv->tx_idx = 0; @@ -295,8 +303,12 @@ static int pch_gbe_recv(struct udevice *dev, int flags, uchar **packetp) if (virt_to_phys(rx_desc) == hw_desc) return -EAGAIN;
+ /* Invalidate the descriptor */ + invalidate_dcache_range((ulong)rx_desc, (ulong)&rx_desc[1]); + length = rx_desc->rx_words_eob - 3 - ETH_FCS_LEN; buffer = dm_pci_mem_to_virt(priv->dev, rx_desc->buffer_addr, length, 0); + invalidate_dcache_range((ulong)buffer, (ulong)buffer + length); *packetp = (uchar *)buffer;
return length;
On Tue, Sep 27, 2016 at 2:29 AM, Paul Burton paul.burton@imgtec.com wrote:
On MIPS systems DMA isn't coherent with the CPU caches unless an IOCU is present. When there is no IOCU we need to writeback or invalidate the data caches at appropriate points. Perform this cache maintenance in the pch_gbe driver which is used on the MIPS Boston development board.
Signed-off-by: Paul Burton paul.burton@imgtec.com
drivers/net/pch_gbe.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
Reviewed-by: Bin Meng bmeng.cn@gmail.com
Tested on Crown Bay Tested-by: Bin Meng bmeng.cn@gmail.com
Allow for drivers to make use of driver model GPIOs when they're enabled & available without needing to #ifdef on CONFIG_DM_GPIO by providing dummy functions covering GPIO requests. Each will simply return -ENODEV or -EINVAL, depending upon which the real implementation returns when a GPIO isn't found. Only the driver model versions of the GPIO request functions are covered & dm_gpio_request is excluded since it's documented as only being of use for debugging, so drivers shouldn't be calling it anyway.
Also provide a dummy dm_gpio_is_valid, with the idea that all other GPIO functions called would be within an if (dm_gpio_is_valid(...)) statement and have been optimised out in cases where that returns a compile-time constant false.
This parallels the clock API, keeping the #ifdefs & checks in a single location allowing drivers or other code to use GPIOs without needing to perform such checks themselves.
Signed-off-by: Paul Burton paul.burton@imgtec.com
---
include/asm-generic/gpio.h | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+)
diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h index 4aa0004..f6593a7 100644 --- a/include/asm-generic/gpio.h +++ b/include/asm-generic/gpio.h @@ -135,6 +135,9 @@ struct gpio_desc { */ static inline bool dm_gpio_is_valid(const struct gpio_desc *desc) { + if (!IS_ENABLED(CONFIG_DM_GPIO)) + return false; + return desc->dev != NULL; }
@@ -343,7 +346,14 @@ const char *gpio_get_bank_info(struct udevice *dev, int *offset_count); * @desc: Returns description, on success * @return 0 if OK, -ve on error */ +#ifdef CONFIG_DM_GPIO int dm_gpio_lookup_name(const char *name, struct gpio_desc *desc); +#else +static inline int dm_gpio_lookup_name(const char *name, struct gpio_desc *desc) +{ + return -EINVAL; +} +#endif
/** * gpio_lookup_name - Look up a GPIO name and return its details @@ -356,8 +366,17 @@ int dm_gpio_lookup_name(const char *name, struct gpio_desc *desc); * @offsetp: Returns the offset number within this device * @gpiop: Returns the absolute GPIO number, numbered from 0 */ +#ifdef CONFIG_DM_GPIO int gpio_lookup_name(const char *name, struct udevice **devp, unsigned int *offsetp, unsigned int *gpiop); +#else +static inline int +gpio_lookup_name(const char *name, struct udevice **devp, + unsigned int *offsetp, unsigned int *gpiop) +{ + return -EINVAL; +} +#endif
/** * gpio_get_values_as_int() - Turn the values of a list of GPIOs into an int @@ -428,8 +447,17 @@ int gpio_claim_vector(const int *gpio_num_array, const char *fmt); * something wrong with the list, or other -ve for another error (e.g. * -EBUSY if a GPIO was already requested) */ +#ifdef CONFIG_DM_GPIO int gpio_request_by_name(struct udevice *dev, const char *list_name, int index, struct gpio_desc *desc, int flags); +#else +static inline int +gpio_request_by_name(struct udevice *dev, const char *list_name, + int index, struct gpio_desc *desc, int flags) +{ + return -ENOENT; +} +#endif
/** * gpio_request_list_by_name() - Request a list of GPIOs @@ -452,9 +480,19 @@ int gpio_request_by_name(struct udevice *dev, const char *list_name, * @flags: Indicates the GPIO input/output settings (GPIOD_...) * @return number of GPIOs requested, or -ve on error */ +#ifdef CONFIG_DM_GPIO int gpio_request_list_by_name(struct udevice *dev, const char *list_name, struct gpio_desc *desc_list, int max_count, int flags); +#else +static inline int +gpio_request_list_by_name(struct udevice *dev, const char *list_name, + struct gpio_desc *desc_list, int max_count, + int flags) +{ + return -ENOENT; +} +#endif
/** * dm_gpio_request() - manually request a GPIO
Hi Paul,
On 26 September 2016 at 12:29, Paul Burton paul.burton@imgtec.com wrote:
Allow for drivers to make use of driver model GPIOs when they're enabled & available without needing to #ifdef on CONFIG_DM_GPIO by providing dummy functions covering GPIO requests. Each will simply return -ENODEV or -EINVAL, depending upon which the real implementation returns when a GPIO isn't found. Only the driver model versions of the GPIO request functions are covered & dm_gpio_request is excluded since it's documented as only being of use for debugging, so drivers shouldn't be calling it anyway.
Also provide a dummy dm_gpio_is_valid, with the idea that all other GPIO functions called would be within an if (dm_gpio_is_valid(...)) statement and have been optimised out in cases where that returns a compile-time constant false.
This parallels the clock API, keeping the #ifdefs & checks in a single location allowing drivers or other code to use GPIOs without needing to perform such checks themselves.
Signed-off-by: Paul Burton paul.burton@imgtec.com
include/asm-generic/gpio.h | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+)
Ick - why not just define DM_GPIO? I don't think we want to provide special support for legacy things. It is just confusing.
- Simon
On Monday, 26 September 2016 18:35:25 BST Simon Glass wrote:
Hi Paul,
On 26 September 2016 at 12:29, Paul Burton paul.burton@imgtec.com wrote:
Allow for drivers to make use of driver model GPIOs when they're enabled & available without needing to #ifdef on CONFIG_DM_GPIO by providing dummy functions covering GPIO requests. Each will simply return -ENODEV or -EINVAL, depending upon which the real implementation returns when a GPIO isn't found. Only the driver model versions of the GPIO request functions are covered & dm_gpio_request is excluded since it's documented as only being of use for debugging, so drivers shouldn't be calling it anyway.
Also provide a dummy dm_gpio_is_valid, with the idea that all other GPIO functions called would be within an if (dm_gpio_is_valid(...)) statement and have been optimised out in cases where that returns a compile-time constant false.
This parallels the clock API, keeping the #ifdefs & checks in a single location allowing drivers or other code to use GPIOs without needing to perform such checks themselves.
Signed-off-by: Paul Burton paul.burton@imgtec.com
include/asm-generic/gpio.h | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+)
Ick - why not just define DM_GPIO? I don't think we want to provide special support for legacy things. It is just confusing.
- Simon
Hi Simon,
For the MIPS Boston system I already enable DM_GPIO, I went with this approach so that it wouldn't necessarily need to be enabled for the Crown Bay x86 system which is the other user of the pch_gbe driver.
Would having pch_gbe depend on CONFIG_DM_GPIO be ok with you Bin? I see crownbay_defconfig enables CONFIG_CMD_GPIO but couldn't find any GPIO-using code at a glance. So hopefully it would just work without needing to change any existing crownbay code?
Thanks, Paul
Hi Paul,
On 30 September 2016 at 12:12, Paul Burton paul.burton@imgtec.com wrote:
On Monday, 26 September 2016 18:35:25 BST Simon Glass wrote:
Hi Paul,
On 26 September 2016 at 12:29, Paul Burton paul.burton@imgtec.com wrote:
Allow for drivers to make use of driver model GPIOs when they're enabled
& available without needing to #ifdef on CONFIG_DM_GPIO by providing
dummy functions covering GPIO requests. Each will simply return -ENODEV
or -EINVAL, depending upon which the real implementation returns when a
GPIO isn't found. Only the driver model versions of the GPIO request
functions are covered & dm_gpio_request is excluded since it's
documented as only being of use for debugging, so drivers shouldn't be
calling it anyway.
Also provide a dummy dm_gpio_is_valid, with the idea that all other GPIO
functions called would be within an if (dm_gpio_is_valid(...)) statement
and have been optimised out in cases where that returns a compile-time
constant false.
This parallels the clock API, keeping the #ifdefs & checks in a single
location allowing drivers or other code to use GPIOs without needing to
perform such checks themselves.
Signed-off-by: Paul Burton paul.burton@imgtec.com
include/asm-generic/gpio.h | 38 ++++++++++++++++++++++++++++++++++++++
1 file changed, 38 insertions(+)
Ick - why not just define DM_GPIO? I don't think we want to provide
special support for legacy things. It is just confusing.
- Simon
Hi Simon,
For the MIPS Boston system I already enable DM_GPIO, I went with this approach so that it wouldn't necessarily need to be enabled for the Crown Bay x86 system which is the other user of the pch_gbe driver.
Would having pch_gbe depend on CONFIG_DM_GPIO be ok with you Bin? I see crownbay_defconfig enables CONFIG_CMD_GPIO but couldn't find any GPIO-using code at a glance. So hopefully it would just work without needing to change any existing crownbay code?
It seems OK to me, but in any case, DM_GPIO should be enabled for x86.
Regards, Simon
Add a driver for the GPIO controller found in the Intel EG20T Platform Controller Hub. This is used on the MIPS Boston development board to provide GPIOs including ethernet PHY reset.
Signed-off-by: Paul Burton paul.burton@imgtec.com
---
drivers/gpio/Kconfig | 8 +++ drivers/gpio/Makefile | 1 + drivers/gpio/eg20t-gpio.c | 133 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 142 insertions(+) create mode 100644 drivers/gpio/eg20t-gpio.c
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index 8d9ab52..4a6a22f 100644 --- a/drivers/gpio/Kconfig +++ b/drivers/gpio/Kconfig @@ -221,4 +221,12 @@ config MPC85XX_GPIO
The driver has been tested on MPC85XX, but it is likely that other PowerQUICC III devices will work as well. + +config EG20T_GPIO + bool "Intel EG20T GPIO driver" + depends on DM_GPIO && DM_PCI + help + Enable this to support the GPIO controller found in the Intel EG20T + Platform Controller Hub. + endmenu diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile index 8939226..a94aeb1 100644 --- a/drivers/gpio/Makefile +++ b/drivers/gpio/Makefile @@ -58,3 +58,4 @@ obj-$(CONFIG_MVEBU_GPIO) += mvebu_gpio.o obj-$(CONFIG_MSM_GPIO) += msm_gpio.o obj-$(CONFIG_$(SPL_)PCF8575_GPIO) += pcf8575_gpio.o obj-$(CONFIG_PM8916_GPIO) += pm8916_gpio.o +obj-$(CONFIG_EG20T_GPIO) += eg20t-gpio.o diff --git a/drivers/gpio/eg20t-gpio.c b/drivers/gpio/eg20t-gpio.c new file mode 100644 index 0000000..05db771 --- /dev/null +++ b/drivers/gpio/eg20t-gpio.c @@ -0,0 +1,133 @@ +/* + * Copyright (C) 2016 Imagination Technologies + * + * SPDX-License-Identifier: GPL-2.0 + */ + +#include <common.h> +#include <dm.h> +#include <errno.h> +#include <pci.h> +#include <asm/io.h> +#include <asm/gpio.h> + +enum { + REG_IEN = 0x00, + REG_ISTATUS = 0x04, + REG_IDISP = 0x08, + REG_ICLR = 0x0c, + REG_IMASK = 0x10, + REG_IMASKCLR = 0x14, + REG_PO = 0x18, + REG_PI = 0x1c, + REG_PM = 0x20, +}; + +struct eg20t_gpio_priv { + void *base; +}; + +static int eg20t_gpio_get_value(struct udevice *dev, unsigned int offset) +{ + struct eg20t_gpio_priv *priv = dev_get_priv(dev); + uint32_t pm, pval; + + pm = readl(priv->base + REG_PM); + if ((pm >> offset) & 0x1) + pval = readl(priv->base + REG_PO); + else + pval = readl(priv->base + REG_PI); + + return (pval >> offset) & 0x1; +} + +static int eg20t_gpio_set_value(struct udevice *dev, unsigned int offset, + int value) +{ + struct eg20t_gpio_priv *priv = dev_get_priv(dev); + uint32_t po; + + po = readl(priv->base + REG_PO); + if (value) + po |= 1 << offset; + else + po &= ~(1 << offset); + writel(po, priv->base + REG_PO); + return 0; +} + +static int eg20t_gpio_direction_input(struct udevice *dev, unsigned int offset) +{ + struct eg20t_gpio_priv *priv = dev_get_priv(dev); + uint32_t pm; + + pm = readl(priv->base + REG_PM); + pm &= ~(1 << offset); + writel(pm, priv->base + REG_PM); + return 0; +} + +static int eg20t_gpio_direction_output(struct udevice *dev, unsigned int offset, + int value) +{ + struct eg20t_gpio_priv *priv = dev_get_priv(dev); + uint32_t pm; + + pm = readl(priv->base + REG_PM); + pm |= 1 << offset; + writel(pm, priv->base + REG_PM); + + return eg20t_gpio_set_value(dev, offset, value); +} + +static int eg20t_gpio_get_function(struct udevice *dev, unsigned int offset) +{ + struct eg20t_gpio_priv *priv = dev_get_priv(dev); + uint32_t pm; + + pm = readl(priv->base + REG_PM); + + if ((pm >> offset) & 0x1) + return GPIOF_OUTPUT; + + return GPIOF_INPUT; +} + +static const struct dm_gpio_ops eg20t_gpio_ops = { + .direction_input = eg20t_gpio_direction_input, + .direction_output = eg20t_gpio_direction_output, + .get_value = eg20t_gpio_get_value, + .set_value = eg20t_gpio_set_value, + .get_function = eg20t_gpio_get_function, +}; + +static int eg20t_gpio_probe(struct udevice *dev) +{ + struct eg20t_gpio_priv *priv = dev_get_priv(dev); + struct gpio_dev_priv *uc_priv = dev->uclass_priv; + + priv->base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_1, PCI_REGION_MEM); + if (!priv->base) { + debug("failed to map GPIO registers\n"); + return -EINVAL; + } + + uc_priv->gpio_count = 12; + uc_priv->bank_name = "eg20t"; + return 0; +} + +U_BOOT_DRIVER(eg20t_gpio) = { + .name = "eg20t-gpio", + .id = UCLASS_GPIO, + .probe = eg20t_gpio_probe, + .priv_auto_alloc_size = sizeof(struct eg20t_gpio_priv), + .ops = &eg20t_gpio_ops, +}; + +static struct pci_device_id eg20t_gpio_supported[] = { + { PCI_VENDOR_ID_INTEL, 0x8803 }, + { }, +}; + +U_BOOT_PCI_DEVICE(eg20t_gpio, eg20t_gpio_supported);
Hi Paul,
On 26 September 2016 at 12:29, Paul Burton paul.burton@imgtec.com wrote:
Add a driver for the GPIO controller found in the Intel EG20T Platform Controller Hub. This is used on the MIPS Boston development board to provide GPIOs including ethernet PHY reset.
Signed-off-by: Paul Burton paul.burton@imgtec.com
drivers/gpio/Kconfig | 8 +++ drivers/gpio/Makefile | 1 + drivers/gpio/eg20t-gpio.c | 133 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 142 insertions(+) create mode 100644 drivers/gpio/eg20t-gpio.c
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index 8d9ab52..4a6a22f 100644 --- a/drivers/gpio/Kconfig +++ b/drivers/gpio/Kconfig @@ -221,4 +221,12 @@ config MPC85XX_GPIO
The driver has been tested on MPC85XX, but it is likely that other PowerQUICC III devices will work as well.
+config EG20T_GPIO
bool "Intel EG20T GPIO driver"depends on DM_GPIO && DM_PCIhelpEnable this to support the GPIO controller found in the Intel EG20TPlatform Controller Hub.
Can you add a few more details? How many GPIOs? Bank organisation? Pull-up/down support?
endmenu diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile index 8939226..a94aeb1 100644 --- a/drivers/gpio/Makefile +++ b/drivers/gpio/Makefile @@ -58,3 +58,4 @@ obj-$(CONFIG_MVEBU_GPIO) += mvebu_gpio.o obj-$(CONFIG_MSM_GPIO) += msm_gpio.o obj-$(CONFIG_$(SPL_)PCF8575_GPIO) += pcf8575_gpio.o obj-$(CONFIG_PM8916_GPIO) += pm8916_gpio.o +obj-$(CONFIG_EG20T_GPIO) += eg20t-gpio.o diff --git a/drivers/gpio/eg20t-gpio.c b/drivers/gpio/eg20t-gpio.c new file mode 100644 index 0000000..05db771 --- /dev/null +++ b/drivers/gpio/eg20t-gpio.c @@ -0,0 +1,133 @@ +/*
- Copyright (C) 2016 Imagination Technologies
- SPDX-License-Identifier: GPL-2.0
- */
+#include <common.h> +#include <dm.h> +#include <errno.h> +#include <pci.h> +#include <asm/io.h> +#include <asm/gpio.h>
+enum {
REG_IEN = 0x00,REG_ISTATUS = 0x04,REG_IDISP = 0x08,REG_ICLR = 0x0c,REG_IMASK = 0x10,REG_IMASKCLR = 0x14,REG_PO = 0x18,REG_PI = 0x1c,REG_PM = 0x20,
How about using a struct?
+};
+struct eg20t_gpio_priv {
void *base;+};
+static int eg20t_gpio_get_value(struct udevice *dev, unsigned int offset) +{
struct eg20t_gpio_priv *priv = dev_get_priv(dev);uint32_t pm, pval;pm = readl(priv->base + REG_PM);if ((pm >> offset) & 0x1)pval = readl(priv->base + REG_PO);elsepval = readl(priv->base + REG_PI);return (pval >> offset) & 0x1;+}
+static int eg20t_gpio_set_value(struct udevice *dev, unsigned int offset,
int value)+{
struct eg20t_gpio_priv *priv = dev_get_priv(dev);uint32_t po;po = readl(priv->base + REG_PO);if (value)po |= 1 << offset;elsepo &= ~(1 << offset);writel(po, priv->base + REG_PO);return 0;+}
+static int eg20t_gpio_direction_input(struct udevice *dev, unsigned int offset) +{
struct eg20t_gpio_priv *priv = dev_get_priv(dev);uint32_t pm;pm = readl(priv->base + REG_PM);pm &= ~(1 << offset);writel(pm, priv->base + REG_PM);
clrbits_le32(ADDR, 1 << OFFSET);
return 0;+}
+static int eg20t_gpio_direction_output(struct udevice *dev, unsigned int offset,
int value)+{
struct eg20t_gpio_priv *priv = dev_get_priv(dev);uint32_t pm;pm = readl(priv->base + REG_PM);pm |= 1 << offset;writel(pm, priv->base + REG_PM);
setbits_le32()
return eg20t_gpio_set_value(dev, offset, value);+}
+static int eg20t_gpio_get_function(struct udevice *dev, unsigned int offset) +{
struct eg20t_gpio_priv *priv = dev_get_priv(dev);uint32_t pm;pm = readl(priv->base + REG_PM);if ((pm >> offset) & 0x1)return GPIOF_OUTPUT;return GPIOF_INPUT;
Can it have pins which are GPIOF_FUNC?
+}
+static const struct dm_gpio_ops eg20t_gpio_ops = {
.direction_input = eg20t_gpio_direction_input,.direction_output = eg20t_gpio_direction_output,.get_value = eg20t_gpio_get_value,.set_value = eg20t_gpio_set_value,.get_function = eg20t_gpio_get_function,+};
+static int eg20t_gpio_probe(struct udevice *dev) +{
struct eg20t_gpio_priv *priv = dev_get_priv(dev);struct gpio_dev_priv *uc_priv = dev->uclass_priv;priv->base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_1, PCI_REGION_MEM);if (!priv->base) {debug("failed to map GPIO registers\n");return -EINVAL;
How about -ENOMEM or -ENXIO
}uc_priv->gpio_count = 12;uc_priv->bank_name = "eg20t";return 0;+}
+U_BOOT_DRIVER(eg20t_gpio) = {
.name = "eg20t-gpio",.id = UCLASS_GPIO,.probe = eg20t_gpio_probe,.priv_auto_alloc_size = sizeof(struct eg20t_gpio_priv),.ops = &eg20t_gpio_ops,+};
+static struct pci_device_id eg20t_gpio_supported[] = {
{ PCI_VENDOR_ID_INTEL, 0x8803 },{ },+};
+U_BOOT_PCI_DEVICE(eg20t_gpio, eg20t_gpio_supported);
2.10.0
Regards, Simon
On Monday, 26 September 2016 18:35:32 BST Simon Glass wrote:
+enum {
REG_IEN = 0x00,REG_ISTATUS = 0x04,REG_IDISP = 0x08,REG_ICLR = 0x0c,REG_IMASK = 0x10,REG_IMASKCLR = 0x14,REG_PO = 0x18,REG_PI = 0x1c,REG_PM = 0x20,How about using a struct?
Hi Simon,
Interesting - I was told over here[1] that using structs for register access was "deprecated".
[1] https://www.mail-archive.com/u-boot@lists.denx.de/msg220500.html%5B1]
...so I actually converted the driver away from that style before submitting. Yet both yourself & Bin have suggested it - shall I take it that it's not deprecated after all?
Thanks, Paul
-------- [1] https://www.mail-archive.com/u-boot@lists.denx.de/msg220500.html
Hi Paul,
On 27 September 2016 at 02:21, Paul Burton paul.burton@imgtec.com wrote:
On Monday, 26 September 2016 18:35:32 BST Simon Glass wrote:
+enum {
- REG_IEN = 0x00,
- REG_ISTATUS = 0x04,
- REG_IDISP = 0x08,
- REG_ICLR = 0x0c,
- REG_IMASK = 0x10,
- REG_IMASKCLR = 0x14,
- REG_PO = 0x18,
- REG_PI = 0x1c,
- REG_PM = 0x20,
How about using a struct?
Hi Simon,
Interesting - I was told over here[1] that using structs for register access was "deprecated".
[1] https://www.mail-archive.com/u-boot@lists.denx.de/msg220500.html
...so I actually converted the driver away from that style before submitting. Yet both yourself & Bin have suggested it - shall I take it that it's not deprecated after all?
Well I try to use it when possible, as does Bin. It reads better and we may as well use the features of C. It can't be used if register offsets differ (e.g. a driver needs to deal with registers being 4 bytes apart on one machine and 8 on another). But in general I think it is preferable to offsets.
Regards, Simon
Hi Paul,
On Tue, Sep 27, 2016 at 2:29 AM, Paul Burton paul.burton@imgtec.com wrote:
Add a driver for the GPIO controller found in the Intel EG20T Platform Controller Hub. This is used on the MIPS Boston development board to provide GPIOs including ethernet PHY reset.
Signed-off-by: Paul Burton paul.burton@imgtec.com
drivers/gpio/Kconfig | 8 +++ drivers/gpio/Makefile | 1 + drivers/gpio/eg20t-gpio.c | 133 ++++++++++++++++++++++++++++++++++++++++++++++
Please use eg20t_gpio.c as the filename, like others do.
3 files changed, 142 insertions(+) create mode 100644 drivers/gpio/eg20t-gpio.c
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index 8d9ab52..4a6a22f 100644 --- a/drivers/gpio/Kconfig +++ b/drivers/gpio/Kconfig @@ -221,4 +221,12 @@ config MPC85XX_GPIO
The driver has been tested on MPC85XX, but it is likely that other PowerQUICC III devices will work as well.
+config EG20T_GPIO
bool "Intel EG20T GPIO driver"depends on DM_GPIO && DM_PCIhelpEnable this to support the GPIO controller found in the Intel EG20TPlatform Controller Hub.endmenu diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile index 8939226..a94aeb1 100644 --- a/drivers/gpio/Makefile +++ b/drivers/gpio/Makefile @@ -58,3 +58,4 @@ obj-$(CONFIG_MVEBU_GPIO) += mvebu_gpio.o obj-$(CONFIG_MSM_GPIO) += msm_gpio.o obj-$(CONFIG_$(SPL_)PCF8575_GPIO) += pcf8575_gpio.o obj-$(CONFIG_PM8916_GPIO) += pm8916_gpio.o +obj-$(CONFIG_EG20T_GPIO) += eg20t-gpio.o diff --git a/drivers/gpio/eg20t-gpio.c b/drivers/gpio/eg20t-gpio.c new file mode 100644 index 0000000..05db771 --- /dev/null +++ b/drivers/gpio/eg20t-gpio.c @@ -0,0 +1,133 @@ +/*
- Copyright (C) 2016 Imagination Technologies
- SPDX-License-Identifier: GPL-2.0
- */
+#include <common.h> +#include <dm.h> +#include <errno.h> +#include <pci.h> +#include <asm/io.h> +#include <asm/gpio.h>
+enum {
REG_IEN = 0x00,REG_ISTATUS = 0x04,REG_IDISP = 0x08,REG_ICLR = 0x0c,REG_IMASK = 0x10,REG_IMASKCLR = 0x14,REG_PO = 0x18,REG_PI = 0x1c,REG_PM = 0x20,+};
Please use a struct.
+struct eg20t_gpio_priv {
void *base;+};
+static int eg20t_gpio_get_value(struct udevice *dev, unsigned int offset) +{
struct eg20t_gpio_priv *priv = dev_get_priv(dev);uint32_t pm, pval;pm = readl(priv->base + REG_PM);if ((pm >> offset) & 0x1)pval = readl(priv->base + REG_PO);elsepval = readl(priv->base + REG_PI);return (pval >> offset) & 0x1;+}
+static int eg20t_gpio_set_value(struct udevice *dev, unsigned int offset,
int value)+{
struct eg20t_gpio_priv *priv = dev_get_priv(dev);uint32_t po;po = readl(priv->base + REG_PO);if (value)po |= 1 << offset;elsepo &= ~(1 << offset);writel(po, priv->base + REG_PO);
nits: need a blank line.
return 0;+}
+static int eg20t_gpio_direction_input(struct udevice *dev, unsigned int offset) +{
struct eg20t_gpio_priv *priv = dev_get_priv(dev);uint32_t pm;pm = readl(priv->base + REG_PM);pm &= ~(1 << offset);writel(pm, priv->base + REG_PM);
nits: need a blank line.
return 0;+}
+static int eg20t_gpio_direction_output(struct udevice *dev, unsigned int offset,
int value)+{
struct eg20t_gpio_priv *priv = dev_get_priv(dev);uint32_t pm;pm = readl(priv->base + REG_PM);pm |= 1 << offset;writel(pm, priv->base + REG_PM);return eg20t_gpio_set_value(dev, offset, value);+}
+static int eg20t_gpio_get_function(struct udevice *dev, unsigned int offset) +{
struct eg20t_gpio_priv *priv = dev_get_priv(dev);uint32_t pm;pm = readl(priv->base + REG_PM);if ((pm >> offset) & 0x1)return GPIOF_OUTPUT;return GPIOF_INPUT;+}
+static const struct dm_gpio_ops eg20t_gpio_ops = {
.direction_input = eg20t_gpio_direction_input,.direction_output = eg20t_gpio_direction_output,.get_value = eg20t_gpio_get_value,.set_value = eg20t_gpio_set_value,.get_function = eg20t_gpio_get_function,+};
+static int eg20t_gpio_probe(struct udevice *dev) +{
struct eg20t_gpio_priv *priv = dev_get_priv(dev);struct gpio_dev_priv *uc_priv = dev->uclass_priv;priv->base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_1, PCI_REGION_MEM);if (!priv->base) {debug("failed to map GPIO registers\n");return -EINVAL;}uc_priv->gpio_count = 12;uc_priv->bank_name = "eg20t";
nits: need a blank line.
return 0;+}
+U_BOOT_DRIVER(eg20t_gpio) = {
.name = "eg20t-gpio",.id = UCLASS_GPIO,.probe = eg20t_gpio_probe,.priv_auto_alloc_size = sizeof(struct eg20t_gpio_priv),.ops = &eg20t_gpio_ops,+};
+static struct pci_device_id eg20t_gpio_supported[] = {
{ PCI_VENDOR_ID_INTEL, 0x8803 },{ },+};
+U_BOOT_PCI_DEVICE(eg20t_gpio, eg20t_gpio_supported);
Regards, Bin
On Tue, Sep 27, 2016 at 2:22 PM, Bin Meng bmeng.cn@gmail.com wrote:
Hi Paul,
On Tue, Sep 27, 2016 at 2:29 AM, Paul Burton paul.burton@imgtec.com wrote:
Add a driver for the GPIO controller found in the Intel EG20T Platform Controller Hub. This is used on the MIPS Boston development board to provide GPIOs including ethernet PHY reset.
Signed-off-by: Paul Burton paul.burton@imgtec.com
drivers/gpio/Kconfig | 8 +++ drivers/gpio/Makefile | 1 + drivers/gpio/eg20t-gpio.c | 133 ++++++++++++++++++++++++++++++++++++++++++++++
Please use eg20t_gpio.c as the filename, like others do.
3 files changed, 142 insertions(+) create mode 100644 drivers/gpio/eg20t-gpio.c
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index 8d9ab52..4a6a22f 100644 --- a/drivers/gpio/Kconfig +++ b/drivers/gpio/Kconfig @@ -221,4 +221,12 @@ config MPC85XX_GPIO
The driver has been tested on MPC85XX, but it is likely that other PowerQUICC III devices will work as well.
+config EG20T_GPIO
bool "Intel EG20T GPIO driver"depends on DM_GPIO && DM_PCIhelpEnable this to support the GPIO controller found in the Intel EG20TPlatform Controller Hub.endmenu diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile index 8939226..a94aeb1 100644 --- a/drivers/gpio/Makefile +++ b/drivers/gpio/Makefile @@ -58,3 +58,4 @@ obj-$(CONFIG_MVEBU_GPIO) += mvebu_gpio.o obj-$(CONFIG_MSM_GPIO) += msm_gpio.o obj-$(CONFIG_$(SPL_)PCF8575_GPIO) += pcf8575_gpio.o obj-$(CONFIG_PM8916_GPIO) += pm8916_gpio.o +obj-$(CONFIG_EG20T_GPIO) += eg20t-gpio.o diff --git a/drivers/gpio/eg20t-gpio.c b/drivers/gpio/eg20t-gpio.c new file mode 100644 index 0000000..05db771 --- /dev/null +++ b/drivers/gpio/eg20t-gpio.c @@ -0,0 +1,133 @@ +/*
- Copyright (C) 2016 Imagination Technologies
- SPDX-License-Identifier: GPL-2.0
- */
+#include <common.h> +#include <dm.h> +#include <errno.h> +#include <pci.h> +#include <asm/io.h> +#include <asm/gpio.h>
+enum {
REG_IEN = 0x00,REG_ISTATUS = 0x04,REG_IDISP = 0x08,REG_ICLR = 0x0c,REG_IMASK = 0x10,REG_IMASKCLR = 0x14,REG_PO = 0x18,REG_PI = 0x1c,REG_PM = 0x20,+};
Please use a struct.
+struct eg20t_gpio_priv {
void *base;+};
+static int eg20t_gpio_get_value(struct udevice *dev, unsigned int offset) +{
struct eg20t_gpio_priv *priv = dev_get_priv(dev);uint32_t pm, pval;pm = readl(priv->base + REG_PM);if ((pm >> offset) & 0x1)pval = readl(priv->base + REG_PO);elsepval = readl(priv->base + REG_PI);return (pval >> offset) & 0x1;+}
+static int eg20t_gpio_set_value(struct udevice *dev, unsigned int offset,
int value)+{
struct eg20t_gpio_priv *priv = dev_get_priv(dev);uint32_t po;po = readl(priv->base + REG_PO);if (value)po |= 1 << offset;elsepo &= ~(1 << offset);writel(po, priv->base + REG_PO);nits: need a blank line.
return 0;+}
+static int eg20t_gpio_direction_input(struct udevice *dev, unsigned int offset) +{
struct eg20t_gpio_priv *priv = dev_get_priv(dev);uint32_t pm;pm = readl(priv->base + REG_PM);pm &= ~(1 << offset);writel(pm, priv->base + REG_PM);nits: need a blank line.
return 0;+}
+static int eg20t_gpio_direction_output(struct udevice *dev, unsigned int offset,
int value)+{
struct eg20t_gpio_priv *priv = dev_get_priv(dev);uint32_t pm;pm = readl(priv->base + REG_PM);pm |= 1 << offset;writel(pm, priv->base + REG_PM);return eg20t_gpio_set_value(dev, offset, value);+}
+static int eg20t_gpio_get_function(struct udevice *dev, unsigned int offset) +{
struct eg20t_gpio_priv *priv = dev_get_priv(dev);uint32_t pm;pm = readl(priv->base + REG_PM);if ((pm >> offset) & 0x1)return GPIOF_OUTPUT;return GPIOF_INPUT;+}
+static const struct dm_gpio_ops eg20t_gpio_ops = {
.direction_input = eg20t_gpio_direction_input,.direction_output = eg20t_gpio_direction_output,.get_value = eg20t_gpio_get_value,.set_value = eg20t_gpio_set_value,.get_function = eg20t_gpio_get_function,+};
+static int eg20t_gpio_probe(struct udevice *dev) +{
struct eg20t_gpio_priv *priv = dev_get_priv(dev);struct gpio_dev_priv *uc_priv = dev->uclass_priv;priv->base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_1, PCI_REGION_MEM);if (!priv->base) {debug("failed to map GPIO registers\n");return -EINVAL;}uc_priv->gpio_count = 12;uc_priv->bank_name = "eg20t";nits: need a blank line.
return 0;+}
+U_BOOT_DRIVER(eg20t_gpio) = {
.name = "eg20t-gpio",.id = UCLASS_GPIO,.probe = eg20t_gpio_probe,.priv_auto_alloc_size = sizeof(struct eg20t_gpio_priv),.ops = &eg20t_gpio_ops,+};
+static struct pci_device_id eg20t_gpio_supported[] = {
{ PCI_VENDOR_ID_INTEL, 0x8803 },{ },+};
+U_BOOT_PCI_DEVICE(eg20t_gpio, eg20t_gpio_supported);
Other than the above comments,
Reviewed-by: Bin Meng bmeng.cn@gmail.com
Tested on Crown Bay Tested-by: Bin Meng bmeng.cn@gmail.com
Add support to the pch_gbe driver for resetting the PHY using a GPIO specified in the device tree. This matches the support already in Linux.
Signed-off-by: Paul Burton paul.burton@imgtec.com
---
drivers/net/pch_gbe.c | 29 +++++++++++++++++++++++++++-- drivers/net/pch_gbe.h | 1 + 2 files changed, 28 insertions(+), 2 deletions(-)
diff --git a/drivers/net/pch_gbe.c b/drivers/net/pch_gbe.c index 8866f66..cc3ca8b 100644 --- a/drivers/net/pch_gbe.c +++ b/drivers/net/pch_gbe.c @@ -12,6 +12,7 @@ #include <asm/io.h> #include <pci.h> #include <miiphy.h> +#include <asm/gpio.h> #include "pch_gbe.h"
#if !defined(CONFIG_PHYLIB) @@ -72,6 +73,14 @@ static int pch_gbe_reset(struct udevice *dev) priv->rx_idx = 0; priv->tx_idx = 0;
+ if (dm_gpio_is_valid(&priv->gpio_phy_reset)) { + /* Reset the PHY */ + dm_gpio_set_value(&priv->gpio_phy_reset, 1); + udelay(15000); + dm_gpio_set_value(&priv->gpio_phy_reset, 0); + udelay(5000); + } + writel(PCH_GBE_ALL_RST, &mac_regs->reset);
/* @@ -451,6 +460,11 @@ int pch_gbe_probe(struct udevice *dev) plat->iobase = (ulong)iobase; priv->mac_regs = (struct pch_gbe_regs *)iobase;
+ err = gpio_request_by_name(dev, "phy-reset-gpios", 0, + &priv->gpio_phy_reset, GPIOD_IS_OUT); + if (err && (err != -ENOENT)) + return err; + /* Read MAC address from SROM and initialize dev->enetaddr with it */ pch_gbe_mac_read(priv->mac_regs, plat->enetaddr);
@@ -460,9 +474,17 @@ int pch_gbe_probe(struct udevice *dev)
err = pch_gbe_reset(dev); if (err) - return err; + goto out_err;
- return pch_gbe_phy_init(dev); + err = pch_gbe_phy_init(dev); + if (err) + goto out_err; + + return 0; +out_err: + if (dm_gpio_is_valid(&priv->gpio_phy_reset)) + dm_gpio_free(dev, &priv->gpio_phy_reset); + return err; }
int pch_gbe_remove(struct udevice *dev) @@ -473,6 +495,9 @@ int pch_gbe_remove(struct udevice *dev) mdio_unregister(priv->bus); mdio_free(priv->bus);
+ if (dm_gpio_is_valid(&priv->gpio_phy_reset)) + dm_gpio_free(dev, &priv->gpio_phy_reset); + return 0; }
diff --git a/drivers/net/pch_gbe.h b/drivers/net/pch_gbe.h index 0ea0c73..1d13380 100644 --- a/drivers/net/pch_gbe.h +++ b/drivers/net/pch_gbe.h @@ -293,6 +293,7 @@ struct pch_gbe_priv { struct udevice *dev; int rx_idx; int tx_idx; + struct gpio_desc gpio_phy_reset; };
#endif /* _PCH_GBE_H_ */
Hi Paul,
On 26 September 2016 at 12:29, Paul Burton paul.burton@imgtec.com wrote:
Add support to the pch_gbe driver for resetting the PHY using a GPIO specified in the device tree. This matches the support already in Linux.
Signed-off-by: Paul Burton paul.burton@imgtec.com
drivers/net/pch_gbe.c | 29 +++++++++++++++++++++++++++-- drivers/net/pch_gbe.h | 1 + 2 files changed, 28 insertions(+), 2 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
diff --git a/drivers/net/pch_gbe.c b/drivers/net/pch_gbe.c index 8866f66..cc3ca8b 100644 --- a/drivers/net/pch_gbe.c +++ b/drivers/net/pch_gbe.c @@ -12,6 +12,7 @@ #include <asm/io.h> #include <pci.h> #include <miiphy.h> +#include <asm/gpio.h> #include "pch_gbe.h"
#if !defined(CONFIG_PHYLIB) @@ -72,6 +73,14 @@ static int pch_gbe_reset(struct udevice *dev) priv->rx_idx = 0; priv->tx_idx = 0;
if (dm_gpio_is_valid(&priv->gpio_phy_reset)) {/* Reset the PHY */dm_gpio_set_value(&priv->gpio_phy_reset, 1);
I'm assuming you don't check the return value because this GPIO might not be valid / used?
Regards. Simon
On Tue, Sep 27, 2016 at 2:29 AM, Paul Burton paul.burton@imgtec.com wrote:
Add support to the pch_gbe driver for resetting the PHY using a GPIO specified in the device tree. This matches the support already in Linux.
Signed-off-by: Paul Burton paul.burton@imgtec.com
drivers/net/pch_gbe.c | 29 +++++++++++++++++++++++++++-- drivers/net/pch_gbe.h | 1 + 2 files changed, 28 insertions(+), 2 deletions(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com
Tested on Crown Bay Tested-by: Bin Meng bmeng.cn@gmail.com
Without adding a prompt for CONFIG_MIPS_CM_BASE, Kconfig doesn't allow defconfigs to set it. Provide the prompt in order to allow for that.
Signed-off-by: Paul Burton paul.burton@imgtec.com ---
arch/mips/Kconfig | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig index 097ad58..b425414 100644 --- a/arch/mips/Kconfig +++ b/arch/mips/Kconfig @@ -352,7 +352,8 @@ config MIPS_CM information such as cache configuration.
config MIPS_CM_BASE - hex + hex "MIPS CM GCR Base Address" + depends on MIPS_CM default 0x1fbf8000 help The physical base address at which to map the MIPS Coherence Manager
2016-09-26 20:29 GMT+02:00 Paul Burton paul.burton@imgtec.com:
Without adding a prompt for CONFIG_MIPS_CM_BASE, Kconfig doesn't allow defconfigs to set it. Provide the prompt in order to allow for that.
Signed-off-by: Paul Burton paul.burton@imgtec.com
arch/mips/Kconfig | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig index 097ad58..b425414 100644 --- a/arch/mips/Kconfig +++ b/arch/mips/Kconfig @@ -352,7 +352,8 @@ config MIPS_CM information such as cache configuration.
config MIPS_CM_BASE
hex
hex "MIPS CM GCR Base Address"depends on MIPS_CM default 0x1fbf8000 help The physical base address at which to map the MIPS Coherence Manager-- 2.10.0
I guess that is board-specific and shouldn't be modified by a user. How about this:
config MIPS_CM_BASE hex depends on MIPS_CM default 0x16100000 if TARGET_BOSTON default 0x1fbf8000 help The physical base address at which to map the MIPS Coherence Manager
Then you don't have to add this option to your board's defconfig files. This would also help if we want to support builds with "make randconfig" in the future.
Move the MIPS Coherence Manager (CM) Global Configuration Registers (GCRs) away from the region of the physical address space which the Boston board's parallel flash is found in, such that we can access all of flash without clobbering GCRs.
Signed-off-by: Paul Burton paul.burton@imgtec.com ---
configs/boston32r2_defconfig | 1 + configs/boston32r2el_defconfig | 1 + configs/boston64r2_defconfig | 1 + configs/boston64r2el_defconfig | 1 + 4 files changed, 4 insertions(+)
diff --git a/configs/boston32r2_defconfig b/configs/boston32r2_defconfig index e5f61b8..6b38da9 100644 --- a/configs/boston32r2_defconfig +++ b/configs/boston32r2_defconfig @@ -4,6 +4,7 @@ CONFIG_SYS_TEXT_BASE=0x9fc00000 # CONFIG_MIPS_BOOT_CMDLINE_LEGACY is not set # CONFIG_MIPS_BOOT_ENV_LEGACY is not set CONFIG_MIPS_BOOT_FDT=y +CONFIG_MIPS_CM_BASE=0x16100000 CONFIG_DEFAULT_DEVICE_TREE="img,boston" CONFIG_FIT=y CONFIG_FIT_VERBOSE=y diff --git a/configs/boston32r2el_defconfig b/configs/boston32r2el_defconfig index e9a23b8..d56c405 100644 --- a/configs/boston32r2el_defconfig +++ b/configs/boston32r2el_defconfig @@ -5,6 +5,7 @@ CONFIG_SYS_LITTLE_ENDIAN=y # CONFIG_MIPS_BOOT_CMDLINE_LEGACY is not set # CONFIG_MIPS_BOOT_ENV_LEGACY is not set CONFIG_MIPS_BOOT_FDT=y +CONFIG_MIPS_CM_BASE=0x16100000 CONFIG_DEFAULT_DEVICE_TREE="img,boston" CONFIG_FIT=y CONFIG_FIT_VERBOSE=y diff --git a/configs/boston64r2_defconfig b/configs/boston64r2_defconfig index 55943c5..2c0ac4d 100644 --- a/configs/boston64r2_defconfig +++ b/configs/boston64r2_defconfig @@ -4,6 +4,7 @@ CONFIG_CPU_MIPS64_R2=y # CONFIG_MIPS_BOOT_CMDLINE_LEGACY is not set # CONFIG_MIPS_BOOT_ENV_LEGACY is not set CONFIG_MIPS_BOOT_FDT=y +CONFIG_MIPS_CM_BASE=0x16100000 CONFIG_DEFAULT_DEVICE_TREE="img,boston" CONFIG_FIT=y CONFIG_FIT_VERBOSE=y diff --git a/configs/boston64r2el_defconfig b/configs/boston64r2el_defconfig index 865177d..e784d68 100644 --- a/configs/boston64r2el_defconfig +++ b/configs/boston64r2el_defconfig @@ -5,6 +5,7 @@ CONFIG_CPU_MIPS64_R2=y # CONFIG_MIPS_BOOT_CMDLINE_LEGACY is not set # CONFIG_MIPS_BOOT_ENV_LEGACY is not set CONFIG_MIPS_BOOT_FDT=y +CONFIG_MIPS_CM_BASE=0x16100000 CONFIG_DEFAULT_DEVICE_TREE="img,boston" CONFIG_FIT=y CONFIG_FIT_VERBOSE=y
The boston memory map isn't suited to the simple "all memory starting from 0" approach that the MIPS arch_fixup_fdt() implementation takes. Instead we need to indicate the first 256MiB of DDR from 0 and the rest from 0x90000000. Implement ft_board_setup to do that.
Signed-off-by: Paul Burton paul.burton@imgtec.com ---
arch/mips/Kconfig | 1 + board/imgtec/boston/Makefile | 1 + board/imgtec/boston/dt.c | 27 +++++++++++++++++++++++++++ 3 files changed, 29 insertions(+) create mode 100644 board/imgtec/boston/dt.c
diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig index b425414..9fa41e9 100644 --- a/arch/mips/Kconfig +++ b/arch/mips/Kconfig @@ -83,6 +83,7 @@ config TARGET_BOSTON select MIPS_CM select MIPS_L1_CACHE_SHIFT_6 select MIPS_L2_CACHE + select OF_BOARD_SETUP select SUPPORTS_BIG_ENDIAN select SUPPORTS_LITTLE_ENDIAN select SUPPORTS_CPU_MIPS32_R1 diff --git a/board/imgtec/boston/Makefile b/board/imgtec/boston/Makefile index deda457..d3fd49d 100644 --- a/board/imgtec/boston/Makefile +++ b/board/imgtec/boston/Makefile @@ -6,4 +6,5 @@
obj-y += checkboard.o obj-y += ddr.o +obj-y += dt.o obj-y += lowlevel_init.o diff --git a/board/imgtec/boston/dt.c b/board/imgtec/boston/dt.c new file mode 100644 index 0000000..b34f9bc --- /dev/null +++ b/board/imgtec/boston/dt.c @@ -0,0 +1,27 @@ +/* + * Copyright (C) 2016 Imagination Technologies + * + * SPDX-License-Identifier: GPL-2.0 + */ + +#include <common.h> +#include <fdt_support.h> + +int ft_board_setup(void *blob, bd_t *bd) +{ + DECLARE_GLOBAL_DATA_PTR; + u64 mem_start[2], mem_size[2]; + int mem_regions; + + mem_start[0] = 0; + mem_size[0] = min_t(u64, 256llu << 20, gd->ram_size); + mem_regions = 1; + + if (gd->ram_size > mem_size[0]) { + mem_start[1] = 0x80000000 + mem_size[0]; + mem_size[1] = gd->ram_size - mem_size[0]; + mem_regions++; + } + + return fdt_fixup_memory_banks(blob, mem_start, mem_size, mem_regions); +}
2016-09-26 20:29 GMT+02:00 Paul Burton paul.burton@imgtec.com:
The boston memory map isn't suited to the simple "all memory starting from 0" approach that the MIPS arch_fixup_fdt() implementation takes. Instead we need to indicate the first 256MiB of DDR from 0 and the rest from 0x90000000. Implement ft_board_setup to do that.
Signed-off-by: Paul Burton paul.burton@imgtec.com
arch/mips/Kconfig | 1 + board/imgtec/boston/Makefile | 1 + board/imgtec/boston/dt.c | 27 +++++++++++++++++++++++++++ 3 files changed, 29 insertions(+) create mode 100644 board/imgtec/boston/dt.c
diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig index b425414..9fa41e9 100644 --- a/arch/mips/Kconfig +++ b/arch/mips/Kconfig @@ -83,6 +83,7 @@ config TARGET_BOSTON select MIPS_CM select MIPS_L1_CACHE_SHIFT_6 select MIPS_L2_CACHE
select OF_BOARD_SETUP select SUPPORTS_BIG_ENDIAN select SUPPORTS_LITTLE_ENDIAN select SUPPORTS_CPU_MIPS32_R1diff --git a/board/imgtec/boston/Makefile b/board/imgtec/boston/Makefile index deda457..d3fd49d 100644 --- a/board/imgtec/boston/Makefile +++ b/board/imgtec/boston/Makefile @@ -6,4 +6,5 @@
obj-y += checkboard.o obj-y += ddr.o +obj-y += dt.o obj-y += lowlevel_init.o diff --git a/board/imgtec/boston/dt.c b/board/imgtec/boston/dt.c new file mode 100644 index 0000000..b34f9bc --- /dev/null +++ b/board/imgtec/boston/dt.c @@ -0,0 +1,27 @@ +/*
- Copyright (C) 2016 Imagination Technologies
- SPDX-License-Identifier: GPL-2.0
- */
+#include <common.h> +#include <fdt_support.h>
+int ft_board_setup(void *blob, bd_t *bd) +{
DECLARE_GLOBAL_DATA_PTR;u64 mem_start[2], mem_size[2];int mem_regions;mem_start[0] = 0;mem_size[0] = min_t(u64, 256llu << 20, gd->ram_size);mem_regions = 1;if (gd->ram_size > mem_size[0]) {mem_start[1] = 0x80000000 + mem_size[0];mem_size[1] = gd->ram_size - mem_size[0];mem_regions++;}return fdt_fixup_memory_banks(blob, mem_start, mem_size, mem_regions);+}
2.10.0
would it be better to convert Boston to use CONFIG_NR_DRAM_BANKS and to implement the weak dram_init_banksize(). Then U-Boot could also know the memory mapping and this code could be made generic for all MIPS platforms who wants to use CONFIG_NR_DRAM_BANKS. But I'm afraid that common/board_f.c:setup_board_part1() and cmd/bdinfo.c need some fixes to support this.
The default value of CONFIG_SYS_BOOTM_LEN is too small for typical boston Linux kernels. Increase the limit to 64MB, which covers current kernels with plenty of breathing room.
Signed-off-by: Paul Burton paul.burton@imgtec.com ---
include/configs/boston.h | 1 + 1 file changed, 1 insertion(+)
diff --git a/include/configs/boston.h b/include/configs/boston.h index e958054..7c6210e 100644 --- a/include/configs/boston.h +++ b/include/configs/boston.h @@ -11,6 +11,7 @@ * General board configuration */ #define CONFIG_DISPLAY_BOARDINFO +#define CONFIG_SYS_BOOTM_LEN (64 * 1024 * 1024)
/* * CPU
The ethernet PHY used on the MIPS Boston development board is a Realtek RTL8211E. Enable support for it.
Signed-off-by: Paul Burton paul.burton@imgtec.com ---
include/configs/boston.h | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/include/configs/boston.h b/include/configs/boston.h index 7c6210e..9fa2714 100644 --- a/include/configs/boston.h +++ b/include/configs/boston.h @@ -79,4 +79,9 @@ (0xb8000000 + (128 << 20) - CONFIG_ENV_SIZE) #endif
+/* + * Ethernet + */ +#define CONFIG_PHY_REALTEK + #endif /* __CONFIGS_BOSTON_H__ */
Add compatible strings for the PCIe bridges & EG20T ethernet controller such that the devices are probed during boot, without the user needing to manually cause that to happen by running "pci enum" after boot. This allows for use of the ethernet controller without the manual PCI enumeration step. Enable the GPIO driver to provide the PHY reset GPIO.
Signed-off-by: Paul Burton paul.burton@imgtec.com ---
arch/mips/Kconfig | 1 + arch/mips/dts/img,boston.dts | 6 +++--- configs/boston32r2_defconfig | 1 + configs/boston32r2el_defconfig | 1 + configs/boston64r2_defconfig | 1 + configs/boston64r2el_defconfig | 1 + 6 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig index 9fa41e9..407dad8 100644 --- a/arch/mips/Kconfig +++ b/arch/mips/Kconfig @@ -78,6 +78,7 @@ config MACH_PIC32 config TARGET_BOSTON bool "Support Boston" select DM + select DM_GPIO select DM_SERIAL select OF_CONTROL select MIPS_CM diff --git a/arch/mips/dts/img,boston.dts b/arch/mips/dts/img,boston.dts index 1d4eeda..1623a25 100644 --- a/arch/mips/dts/img,boston.dts +++ b/arch/mips/dts/img,boston.dts @@ -130,7 +130,7 @@ };
pci2_root@0,0,0 { - compatible = "pci10ee,7021"; + compatible = "pci10ee,7021", "pci-bridge"; reg = <0x00000000 0 0 0 0>;
#address-cells = <3>; @@ -138,7 +138,7 @@ #interrupt-cells = <1>;
eg20t_bridge@1,0,0 { - compatible = "pci8086,8800"; + compatible = "pci8086,8800", "pci-bridge"; reg = <0x00010000 0 0 0 0>;
#address-cells = <3>; @@ -146,7 +146,7 @@ #interrupt-cells = <1>;
eg20t_mac@2,0,1 { - compatible = "pci8086,8802"; + compatible = "pci8086,8802", "intel,pch-gbe"; reg = <0x00020100 0 0 0 0>; phy-reset-gpios = <&eg20t_gpio 6 GPIO_ACTIVE_LOW>; }; diff --git a/configs/boston32r2_defconfig b/configs/boston32r2_defconfig index 6b38da9..61360c5 100644 --- a/configs/boston32r2_defconfig +++ b/configs/boston32r2_defconfig @@ -32,6 +32,7 @@ CONFIG_CMD_FS_GENERIC=y CONFIG_OF_EMBED=y CONFIG_NET_RANDOM_ETHADDR=y CONFIG_CLK=y +CONFIG_EG20T_GPIO=y CONFIG_MTD=y CONFIG_CFI_FLASH=y CONFIG_DM_ETH=y diff --git a/configs/boston32r2el_defconfig b/configs/boston32r2el_defconfig index d56c405..74d16ea 100644 --- a/configs/boston32r2el_defconfig +++ b/configs/boston32r2el_defconfig @@ -33,6 +33,7 @@ CONFIG_CMD_FS_GENERIC=y CONFIG_OF_EMBED=y CONFIG_NET_RANDOM_ETHADDR=y CONFIG_CLK=y +CONFIG_EG20T_GPIO=y CONFIG_MTD=y CONFIG_CFI_FLASH=y CONFIG_DM_ETH=y diff --git a/configs/boston64r2_defconfig b/configs/boston64r2_defconfig index 2c0ac4d..4d67804 100644 --- a/configs/boston64r2_defconfig +++ b/configs/boston64r2_defconfig @@ -32,6 +32,7 @@ CONFIG_CMD_FS_GENERIC=y CONFIG_OF_EMBED=y CONFIG_NET_RANDOM_ETHADDR=y CONFIG_CLK=y +CONFIG_EG20T_GPIO=y CONFIG_MTD=y CONFIG_CFI_FLASH=y CONFIG_DM_ETH=y diff --git a/configs/boston64r2el_defconfig b/configs/boston64r2el_defconfig index e784d68..e0d514f 100644 --- a/configs/boston64r2el_defconfig +++ b/configs/boston64r2el_defconfig @@ -33,6 +33,7 @@ CONFIG_CMD_FS_GENERIC=y CONFIG_OF_EMBED=y CONFIG_NET_RANDOM_ETHADDR=y CONFIG_CLK=y +CONFIG_EG20T_GPIO=y CONFIG_MTD=y CONFIG_CFI_FLASH=y CONFIG_DM_ETH=y
CONFIG_DISTRO_DEFAULTS selects a number of things we want for Boston defconfigs & generally describes what we want - to be able to boot an arbitrary Linux distribution. Enable it in order to shorten the defconfigs & to automatically keep up with any changes in the choice of Kconfig symbols selected.
Signed-off-by: Paul Burton paul.burton@imgtec.com
---
configs/boston32r2_defconfig | 7 +------ configs/boston32r2el_defconfig | 7 +------ configs/boston64r2_defconfig | 7 +------ configs/boston64r2el_defconfig | 7 +------ 4 files changed, 4 insertions(+), 24 deletions(-)
diff --git a/configs/boston32r2_defconfig b/configs/boston32r2_defconfig index 61360c5..3843411 100644 --- a/configs/boston32r2_defconfig +++ b/configs/boston32r2_defconfig @@ -6,11 +6,11 @@ CONFIG_SYS_TEXT_BASE=0x9fc00000 CONFIG_MIPS_BOOT_FDT=y CONFIG_MIPS_CM_BASE=0x16100000 CONFIG_DEFAULT_DEVICE_TREE="img,boston" +CONFIG_DISTRO_DEFAULTS=y CONFIG_FIT=y CONFIG_FIT_VERBOSE=y CONFIG_FIT_BEST_MATCH=y CONFIG_OF_STDOUT_VIA_ALIAS=y -CONFIG_HUSH_PARSER=y CONFIG_SYS_PROMPT="boston # " # CONFIG_CMD_ELF is not set # CONFIG_CMD_IMLS is not set @@ -19,16 +19,11 @@ CONFIG_CMD_MEMTEST=y # CONFIG_CMD_LOADB is not set # CONFIG_CMD_LOADS is not set # CONFIG_CMD_FPGA is not set -CONFIG_CMD_DHCP=y -CONFIG_CMD_PING=y CONFIG_CMD_SNTP=y CONFIG_CMD_DNS=y CONFIG_CMD_LINK_LOCAL=y CONFIG_CMD_TIME=y -CONFIG_CMD_EXT4=y CONFIG_CMD_EXT4_WRITE=y -CONFIG_CMD_FAT=y -CONFIG_CMD_FS_GENERIC=y CONFIG_OF_EMBED=y CONFIG_NET_RANDOM_ETHADDR=y CONFIG_CLK=y diff --git a/configs/boston32r2el_defconfig b/configs/boston32r2el_defconfig index 74d16ea..d9436b8 100644 --- a/configs/boston32r2el_defconfig +++ b/configs/boston32r2el_defconfig @@ -7,11 +7,11 @@ CONFIG_SYS_LITTLE_ENDIAN=y CONFIG_MIPS_BOOT_FDT=y CONFIG_MIPS_CM_BASE=0x16100000 CONFIG_DEFAULT_DEVICE_TREE="img,boston" +CONFIG_DISTRO_DEFAULTS=y CONFIG_FIT=y CONFIG_FIT_VERBOSE=y CONFIG_FIT_BEST_MATCH=y CONFIG_OF_STDOUT_VIA_ALIAS=y -CONFIG_HUSH_PARSER=y CONFIG_SYS_PROMPT="boston # " # CONFIG_CMD_ELF is not set # CONFIG_CMD_IMLS is not set @@ -20,16 +20,11 @@ CONFIG_CMD_MEMTEST=y # CONFIG_CMD_LOADB is not set # CONFIG_CMD_LOADS is not set # CONFIG_CMD_FPGA is not set -CONFIG_CMD_DHCP=y -CONFIG_CMD_PING=y CONFIG_CMD_SNTP=y CONFIG_CMD_DNS=y CONFIG_CMD_LINK_LOCAL=y CONFIG_CMD_TIME=y -CONFIG_CMD_EXT4=y CONFIG_CMD_EXT4_WRITE=y -CONFIG_CMD_FAT=y -CONFIG_CMD_FS_GENERIC=y CONFIG_OF_EMBED=y CONFIG_NET_RANDOM_ETHADDR=y CONFIG_CLK=y diff --git a/configs/boston64r2_defconfig b/configs/boston64r2_defconfig index 4d67804..fb0a99c 100644 --- a/configs/boston64r2_defconfig +++ b/configs/boston64r2_defconfig @@ -6,11 +6,11 @@ CONFIG_CPU_MIPS64_R2=y CONFIG_MIPS_BOOT_FDT=y CONFIG_MIPS_CM_BASE=0x16100000 CONFIG_DEFAULT_DEVICE_TREE="img,boston" +CONFIG_DISTRO_DEFAULTS=y CONFIG_FIT=y CONFIG_FIT_VERBOSE=y CONFIG_FIT_BEST_MATCH=y CONFIG_OF_STDOUT_VIA_ALIAS=y -CONFIG_HUSH_PARSER=y CONFIG_SYS_PROMPT="boston # " # CONFIG_CMD_ELF is not set # CONFIG_CMD_IMLS is not set @@ -19,16 +19,11 @@ CONFIG_CMD_MEMTEST=y # CONFIG_CMD_LOADB is not set # CONFIG_CMD_LOADS is not set # CONFIG_CMD_FPGA is not set -CONFIG_CMD_DHCP=y -CONFIG_CMD_PING=y CONFIG_CMD_SNTP=y CONFIG_CMD_DNS=y CONFIG_CMD_LINK_LOCAL=y CONFIG_CMD_TIME=y -CONFIG_CMD_EXT4=y CONFIG_CMD_EXT4_WRITE=y -CONFIG_CMD_FAT=y -CONFIG_CMD_FS_GENERIC=y CONFIG_OF_EMBED=y CONFIG_NET_RANDOM_ETHADDR=y CONFIG_CLK=y diff --git a/configs/boston64r2el_defconfig b/configs/boston64r2el_defconfig index e0d514f..f1d752f 100644 --- a/configs/boston64r2el_defconfig +++ b/configs/boston64r2el_defconfig @@ -7,11 +7,11 @@ CONFIG_CPU_MIPS64_R2=y CONFIG_MIPS_BOOT_FDT=y CONFIG_MIPS_CM_BASE=0x16100000 CONFIG_DEFAULT_DEVICE_TREE="img,boston" +CONFIG_DISTRO_DEFAULTS=y CONFIG_FIT=y CONFIG_FIT_VERBOSE=y CONFIG_FIT_BEST_MATCH=y CONFIG_OF_STDOUT_VIA_ALIAS=y -CONFIG_HUSH_PARSER=y CONFIG_SYS_PROMPT="boston # " # CONFIG_CMD_ELF is not set # CONFIG_CMD_IMLS is not set @@ -20,16 +20,11 @@ CONFIG_CMD_MEMTEST=y # CONFIG_CMD_LOADB is not set # CONFIG_CMD_LOADS is not set # CONFIG_CMD_FPGA is not set -CONFIG_CMD_DHCP=y -CONFIG_CMD_PING=y CONFIG_CMD_SNTP=y CONFIG_CMD_DNS=y CONFIG_CMD_LINK_LOCAL=y CONFIG_CMD_TIME=y -CONFIG_CMD_EXT4=y CONFIG_CMD_EXT4_WRITE=y -CONFIG_CMD_FAT=y -CONFIG_CMD_FS_GENERIC=y CONFIG_OF_EMBED=y CONFIG_NET_RANDOM_ETHADDR=y CONFIG_CLK=y
participants (5)
-
Bin Meng -
Daniel Schwierzeck -
Paul Burton -
Simon Glass -
Tom Rini