Hi Pali, On Sat, Nov 6, 2021 at 3:57 AM Pali Rohár <p...@kernel.org> wrote: > > Hello! > > On Friday 05 November 2021 20:50:18 Tony Dinh wrote: > > Hi Pali, > > > > On Fri, Nov 5, 2021 at 5:10 PM Pali Rohár <p...@kernel.org> wrote: > > > > > > On Friday 05 November 2021 16:36:47 Tony Dinh wrote: > > > > Hi Pali, > > > > > > > > On Fri, Nov 5, 2021 at 3:15 PM Pali Rohár <p...@kernel.org> wrote: > > > > > > > > > > On Friday 05 November 2021 15:07:17 Tony Dinh wrote: > > > > > > > > Also, I have several Kirkwood boards (with various old BootROM > > > > > > > > versions) that I can run the kwboot tests on. Will keep you > > > > > > > > posted. > > > > > > > > > > > > > > Ok! Do you have some Kirkwood board with PCIe slot? If yes, I > > > > > > > would like > > > > > > > to see dumps from config space of Kirkwood PCIe Root Port, see: > > > > > > > https://lore.kernel.org/u-boot/20211104154921.b6zxjpczj7t6qlct@pali/ > > > > > > > > > > > > I have these Kirwood boards with PCI: > > > > > > - No slot (host bus for USB 3.0): Pogoplug V4 (6192), Zyxel NSA325v2 > > > > > > (6282). These 2 boards can be kwboot. > > > > > > - Iomega iConnect (6281), with PCIe slot for Wifi card. This board > > > > > > does not have kwboot booting support. > > > > > > > > > > What do you mean that it 'does not have kwboot booting support'? > > > > > 88F6281 is also Kirkwood and UART booting with kwboot should work. > > > > > > > > Most of the Kirkwood boards do have UART booting support. However, in > > > > my past experience, some Kirkwood boxes did not work with kwboot > > > > booting. It was observed experimentally that certain BootROM versions > > > > (depending on the time of manufacturing) on the 88F6281 SoC have > > > > problems with UART booting. But we have not proven this to be the real > > > > reason. These boards failed UART booting (the behavior is like the > > > > UART magic string handshake never occur): > > > > > > > > Seagate Dockstar (all), Iomega iConnect (all), Sheevaplug (some models > > > > probably do work), Seagate GoFlex Net (most boxes work, but a few > > > > models don't, and they have a different BootROM version from ones that > > > > do work). > > > > > > Hmm... ok. Maybe some bootrom versions have broken UART booting. > > > > > > During experiment with A385 I observed that it is needed to send > > > continuous stream of boot pattern without delay, so bootrom can properly > > > detect it and enter UART booting. But after bootrom is in UART booting > > > mode, it responds only when do not transmitting anything for some few > > > milliseconds. > > > So it is needed to solve two timing issues. First with upper bound (you > > > cannot use large delay as bootrom does not detect boot pattern) and > > > second with lower bound (you cannot use small delay as bootrom does not > > > answer). Plus another issue that linux kernel does provide asynchronous > > > tty API which could tell when output buffer was transmitted via UART. > > > > That's exactly what I've found trying to boot the Thecus N2350 (Armada > > 385). I've tried various -q -s parameters but could not find the right > > combination! OTOH, the Zyxel NAS326 (Armada 380) is OK with just the > > default timing (still more work on my part in the u-boot image, but > > kwboot started OK). > > > > > > > > If some bootrom versions are too much timing sensitive and you do not > > > know exact characteristic of it (and also of UART HW on the host) then > > > it could be hard or impossible to enter that UART boot mode. > > > > I've always suspected the box UART HW is the reason for failure to > > handshake, not the BootROM. But I'll try testing the old Kirkwood > > boxes again with the new kwboot to be sure. > > > > > I'm planning to rewrite kwboot code which is sending boot pattern to be > > > more precise on timings... So if you are interested in testing it, I > > > could do it in a way with more configurable delays... once I would have > > > some time for it. > > > > I'll be glad to test any new kwboot code you will have. My main > > interest is the Armada 38x and all Kirkwood boards. > > > > > > > > You could try to use tools/mrvl_uart.sh instead of kwboot. It implements > > > also code for sending boot pattern. But it requires valid image with > > > UART signature, it does not support on-the-fly patching like kwboot. > > > > That's what I did to boot the stock Thecus N2350 u-boot UART version. > > An old version of this mrvl_uart.sh script has been on the net for > > quite some time. But kwboot is more robust in the timing setup and > > allows us to boot the final version that will be flashed. > > What could be interested is to try to use tools/mrvl_uart.sh script for > booting those Kirkwood boards which was said that have broken UART > booting. If tools/mrvl_uart.sh is able to trigger bootrom to switch to > UART or not. Last version of tools/mrvl_uart.sh is still in U-Boot > repository.
Good point. I will try that script to boot the iConnect and other boards. > > But as you said, kwboot is more robust and once kwboot would be to > transfer all images which tools/mrvl_uart.sh is able then I send request > to removal of tools/mrvl_uart.sh from U-Boot. As in U-Boot there is no > need to have two different tools which implements same functionality. > > > > > > > I'll take a look at your link above and get back to you about the > > > > > > config space dumps. > > > > > > > > > > > > By the way, I'm starting to look at the driver/pci/pci_mvebu.c to > > > > > > see > > > > > > if it can be made to work with Kirkwood SoCs. I think there are many > > > > > > differences in the addresses and memory space. I would appreciate it > > > > > > if you have a general assessment whether I can use that driver for > > > > > > Kirkwood. > > > > > > > > > > pci_mvebu.c should work with Kirkwood SoCs and also with all these > > > > > 32-bit Marvell SoCs: Orion, Discovery, Kirkwood, Dove, A370, AXP, > > > > > A375, > > > > > A38x and A39x. According to Functional Specifications all these SoCs > > > > > have common PCIe register set. > > > > > > > > That's great to hear! > > > > > > > > > > > > > > If there is any issue with it, I could try to look at it. > > > > > > > > At the moment, pci_mvebu.c is not included in the build for Kirkwood > > > > boards because ./drivers/pci/Kconfig excludes it: > > > > > > > > config PCI_MVEBU > > > > bool "Enable Armada XP/38x PCIe driver" > > > > depends on ARCH_MVEBU > > > > > > > > When I removed the above dependency, the build had errors. Because > > > > different soc.h and cpu.h are brought into pci_mvebu.c when > > > > ARCH_KIRWOOD is enabled and ARCH_MVEBU is disabled. > > > > > > > > #include <asm/arch/cpu.h> > > > > #include <asm/arch/soc.h> > > > > > > So, it it needed to do some adjustment of SoC related code and defines. > > > I think the relevant parts are mapping of mbus windows. > > > > I did look a bit at the mbus windows. There are some differences from > > MVEBU, such as in arch/arm/dts/kirkwood.dtsi > > > > pcie-mem-aperture = <0xe0000000 0x10000000>; /* 256 MiB memory space */ > > pcie-io-aperture = <0xf2000000 0x100000>; /* 1 MiB I/O space */ > > > > But my knowledge in PCI drivers is practically nothing, just hacking > > it :) So if you plan to make this driver work for Kirkwood, I'd be > > happy to volunteer to be a tester. > > I do not have Kirkwood hardware for testing. But it looks like that > mach-kirkwood has just different names for mbus window constants. > > Here is simple (untested) patch which allows me to compile pci_mvebu.c > for ARCH_KIRKWOOD: > > diff --git a/arch/arm/mach-kirkwood/cpu.c b/arch/arm/mach-kirkwood/cpu.c > index e9571298a824..80f893ab369a 100644 > --- a/arch/arm/mach-kirkwood/cpu.c > +++ b/arch/arm/mach-kirkwood/cpu.c > @@ -54,11 +54,11 @@ unsigned int kw_winctrl_calcsize(unsigned int sizeval) > > static struct mbus_win windows[] = { > /* Window 0: PCIE MEM address space */ > - { KW_DEFADR_PCI_MEM, 1024 * 1024 * 256, > + { KW_DEFADR_PCI_MEM, KW_DEFADR_PCI_MEM_SIZE, > KWCPU_TARGET_PCIE, KWCPU_ATTR_PCIE_MEM }, > > /* Window 1: PCIE IO address space */ > - { KW_DEFADR_PCI_IO, 1024 * 64, > + { KW_DEFADR_PCI_IO, KW_DEFADR_PCI_IO_SIZE, > KWCPU_TARGET_PCIE, KWCPU_ATTR_PCIE_IO }, > > /* Window 2: NAND Flash address space */ > diff --git a/arch/arm/mach-kirkwood/include/mach/cpu.h > b/arch/arm/mach-kirkwood/include/mach/cpu.h > index ea42182cf9c6..71c546f9acf6 100644 > --- a/arch/arm/mach-kirkwood/include/mach/cpu.h > +++ b/arch/arm/mach-kirkwood/include/mach/cpu.h > @@ -68,6 +68,9 @@ enum kwcpu_attrib { > #define KW_DEFADR_SPIF 0xE8000000 > #define KW_DEFADR_BOOTROM 0xF8000000 > > +#define KW_DEFADR_PCI_MEM_SIZE (1024 * 1024 * 256) > +#define KW_DEFADR_PCI_IO_SIZE (1024 * 64) > + > struct mbus_win { > u32 base; > u32 size; > diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig > index cc139af6cb57..71fac12257ad 100644 > --- a/drivers/pci/Kconfig > +++ b/drivers/pci/Kconfig > @@ -256,12 +256,12 @@ config PCIE_IPROC > Say Y here if you want to enable Broadcom iProc PCIe controller, > > config PCI_MVEBU > - bool "Enable Armada XP/38x PCIe driver" > - depends on ARCH_MVEBU > + bool "Enable Kirkwood / Armada 370/XP/375/38x PCIe driver" > + depends on (ARCH_KIRKWOOD || ARCH_MVEBU) > select MISC > help > Say Y here if you want to enable PCIe controller support on > - Armada XP/38x SoCs. > + Kirkwood and Armada 370/XP/375/38x SoCs. > > config PCIE_DW_COMMON > bool > diff --git a/drivers/pci/pci_mvebu.c b/drivers/pci/pci_mvebu.c > index c575e9412b2a..4cc8d2014052 100644 > --- a/drivers/pci/pci_mvebu.c > +++ b/drivers/pci/pci_mvebu.c > @@ -26,6 +26,14 @@ > #include <linux/ioport.h> > #include <linux/mbus.h> > > +#ifdef CONFIG_ARCH_KIRKWOOD > +#define SOC_REGS_PHY_BASE KW_REGS_PHY_BASE > +#define MBUS_PCI_MEM_BASE KW_DEFADR_PCI_MEM > +#define MBUS_PCI_IO_BASE KW_DEFADR_PCI_IO > +#define MBUS_PCI_MEM_SIZE KW_DEFADR_PCI_MEM_SIZE > +#define MBUS_PCI_IO_SIZE KW_DEFADR_PCI_IO_SIZE > +#endif > + > DECLARE_GLOBAL_DATA_PTR; > > /* PCIe unit register offsets */ > @@ -97,7 +105,6 @@ struct mvebu_pcie { > * and 64K of I/O space when registered. > */ > static void __iomem *mvebu_pcie_membase = (void __iomem *)MBUS_PCI_MEM_BASE; > -#define PCIE_MEM_SIZE (128 << 20) > static void __iomem *mvebu_pcie_iobase = (void __iomem *)MBUS_PCI_IO_BASE; > > static inline bool mvebu_pcie_link_up(struct mvebu_pcie *pcie) > @@ -433,14 +440,14 @@ static int mvebu_pcie_probe(struct udevice *dev) > mvebu_pcie_set_local_dev_nr(pcie, 1); > > pcie->mem.start = (u32)mvebu_pcie_membase; > - pcie->mem.end = pcie->mem.start + PCIE_MEM_SIZE - 1; > - mvebu_pcie_membase += PCIE_MEM_SIZE; > + pcie->mem.end = pcie->mem.start + MBUS_PCI_MEM_SIZE - 1; > + mvebu_pcie_membase += MBUS_PCI_MEM_SIZE; > > if (mvebu_mbus_add_window_by_id(pcie->mem_target, pcie->mem_attr, > (phys_addr_t)pcie->mem.start, > - PCIE_MEM_SIZE)) { > + MBUS_PCI_MEM_SIZE)) { > printf("PCIe unable to add mbus window for mem at > %08x+%08x\n", > - (u32)pcie->mem.start, PCIE_MEM_SIZE); > + (u32)pcie->mem.start, MBUS_PCI_MEM_SIZE); > } > > pcie->io.start = (u32)mvebu_pcie_iobase; > @@ -459,7 +466,7 @@ static int mvebu_pcie_probe(struct udevice *dev) > > /* PCI memory space */ > pci_set_region(hose->regions + 0, pcie->mem.start, > - pcie->mem.start, PCIE_MEM_SIZE, PCI_REGION_MEM); > + pcie->mem.start, MBUS_PCI_MEM_SIZE, PCI_REGION_MEM); > pci_set_region(hose->regions + 1, > 0, 0, > gd->ram_size, > @@ -659,6 +666,7 @@ static int mvebu_pcie_bind(struct udevice *parent) > static const struct udevice_id mvebu_pcie_ids[] = { > { .compatible = "marvell,armada-xp-pcie" }, > { .compatible = "marvell,armada-370-pcie" }, > + { .compatible = "marvell,kirkwood-pcie" }, > { } > }; > Cool! I will give it a try and let you know. Thanks, Tony