kwboot: Testing latest kwboot with Kirkwood SoC boards

Pali Rohár pali at kernel.org
Sat Nov 6 11:57:01 CET 2021


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 <pali at 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 <pali at 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.

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" },
 	{ }
 };
 


More information about the U-Boot mailing list