Hi Pali,

I've applied your patch, and ran some tests. Looks like we got to the
bind, but it was not probing (or the probe died silently). Later, I
tried pci enum and got some errors during probing.

Here is the log (I added some debug printf to drivers/pci/pci_mvebu.c
and drivers/core/device.c to see the progress).

- U-Boot boot log:

U-Boot 2022.01-tld-1-00054-g52207514ba-dirty (Nov 06 2021 - 18:11:41 -0700)
Pogoplug V4

SoC:   Kirkwood 88F6281_A1
DRAM:  128 MiB
mvebu_pcie_bind: begin
mvebu_pcie_bind: got an available node
Bound device  to pcie@82000000
mvebu_pcie_bind: bound
mvebu_pcie_bind: exit 0
Bound device pcie@82000000 to mbus@f1000000
Bound device mbus@f1000000 to root_driver
Bound device ehci@50000 to ocp@f1000000
Bound device ethernet-controller@72000 to ocp@f1000000
Bound device sata@80000 to ocp@f1000000
Bound device mvs...@90000.blk to mvsdio@90000
Bound device mvsdio@90000 to ocp@f1000000
Bound device ocp@f1000000 to root_driver
NAND:  128 MiB
MMC:   mvsdio@90000: 0
Loading Environment from NAND... *** Warning - bad CRC, using default
environment

In:    serial
Out:   serial
Err:   serial
Net:
Warning: ethernet-controller@72000 (eth0) using random MAC address -
66:43:1f:50:f0:e7
eth0: ethernet-controller@72000
Hit any key to stop autoboot:  0

Pogo_V4> pci
No such bus
Pogo_V4> pci 0
No such bus
Pogo_V4> pci 1
No such bus

Pogo_V4> pci enum
mvebu_pcie_probe:
Cannot add window '4:e8', conflicts with another window
PCIe unable to add mbus window for mem at 90000000+10000000
Cannot add window '4:e0', conflicts with another window
PCIe unable to add mbus window for IO at c0000000+00010000
mvebu_pcie_probe: exit 0
Bound device pci_0:0.0 to pcie0.0
Bound device xhci_pci to pci_0:0.0

Pogo_V4> pci regions
#   Bus start          Phys start         Size                Flags
0   0x0000000090000000 0x0000000090000000 0x0000000010000000  mem
1   0x0000000000000000 0x0000000000000000 0x0000000008000000  mem sysmem
2   0x00000000c0000000 0x00000000c0000000 0x0000000000010000  io io

- Linux dmesg (grep for pci and xhci)

[   13.163011][    T1] mvebu-pcie mbus@f1000000:pcie@82000000: host
bridge /mbus@f1000000/pcie@82000000 ranges:
[   13.163147][    T1] mvebu-pcie mbus@f1000000:pcie@82000000:
MEM 0x00f1040000..0x00f1041fff -> 0x0000040000
[   13.163217][    T1] mvebu-pcie mbus@f1000000:pcie@82000000:
MEM 0xffffffffffffffff..0x00fffffffe -> 0x0100000000
[   13.163268][    T1] mvebu-pcie mbus@f1000000:pcie@82000000:
IO 0xffffffffffffffff..0x00fffffffe -> 0x0100000000
[   13.163754][    T1] mvebu-pcie mbus@f1000000:pcie@82000000: PCI
host bridge to bus 0000:00
[   13.163791][    T1] pci_bus 0000:00: root bus resource [bus 00-ff]
[   13.163829][    T1] pci_bus 0000:00: root bus resource [mem
0xf1040000-0xf1041fff] (bus address [0x00040000-0x00041fff])
[   13.163862][    T1] pci_bus 0000:00: root bus resource [mem
0xe0000000-0xefffffff]
[   13.163892][    T1] pci_bus 0000:00: root bus resource [io  0x1000-0xeffff]
[   13.164075][    T1] pci 0000:00:01.0: [11ab:6281] type 01 class 0x060400
[   13.164133][    T1] pci 0000:00:01.0: reg 0x38: [mem
0x00000000-0x000007ff pref]
[   13.165893][    T1] PCI: bus0: Fast back to back transfers disabled
[   13.165949][    T1] pci 0000:00:01.0: bridge configuration invalid
([bus 00-00]), reconfiguring
[   13.166276][    T1] pci 0000:01:00.0: [1b73:1009] type 00 class 0x0c0330
[   13.166334][    T1] pci 0000:01:00.0: reg 0x10: [mem
0x00000000-0x0000ffff 64bit]
[   13.166381][    T1] pci 0000:01:00.0: reg 0x18: [mem
0x00000000-0x00000fff 64bit]
[   13.166424][    T1] pci 0000:01:00.0: reg 0x20: [mem
0x00000000-0x00000fff 64bit]
[   13.166561][    T1] pci 0000:01:00.0: supports D1
[   13.166588][    T1] pci 0000:01:00.0: PME# supported from D0 D1 D3hot
[   13.166685][    T1] pci 0000:01:00.0: 2.000 Gb/s available PCIe
bandwidth, limited by 2.5 GT/s PCIe x1 link at 0000:00:01.0 (capable
of 4.000 Gb/s with 5.0 GT/s PCIe x1 link)
[   13.197158][    T1] PCI: bus1: Fast back to back transfers disabled
[   13.197208][    T1] pci_bus 0000:01: busn_res: [bus 01-ff] end is
updated to 01
[   13.197538][    T1] pci 0000:00:01.0: BAR 14: assigned [mem
0xe0000000-0xe00fffff]
[   13.197581][    T1] pci 0000:00:01.0: BAR 6: assigned [mem
0xe0100000-0xe01007ff pref]
[   13.197624][    T1] pci 0000:01:00.0: BAR 0: assigned [mem
0xe0000000-0xe000ffff 64bit]
[   13.197670][    T1] pci 0000:01:00.0: BAR 2: assigned [mem
0xe0010000-0xe0010fff 64bit]
[   13.197715][    T1] pci 0000:01:00.0: BAR 4: assigned [mem
0xe0011000-0xe0011fff 64bit]
[   13.197759][    T1] pci 0000:00:01.0: PCI bridge to [bus 01]
[   13.197792][    T1] pci 0000:00:01.0:   bridge window [mem
0xe0000000-0xe00fffff]
[   13.197985][    T1] pcieport 0000:00:01.0: enabling device (0140 -> 0142)
[   13.198135][    T1] pci 0000:01:00.0: enabling device (0140 -> 0142)

[   15.619756][    T1] xhci_hcd 0000:01:00.0: xHCI Host Controller
[   15.625694][    T1] xhci_hcd 0000:01:00.0: new USB bus registered,
assigned bus number 2
[   15.635643][    T1] xhci_hcd 0000:01:00.0: hcc params 0x200073a1
hci version 0x100 quirks 0x0000000000080010
[   15.646477][    T1] xhci_hcd 0000:01:00.0: xHCI Host Controller
[   15.652432][    T1] xhci_hcd 0000:01:00.0: new USB bus registered,
assigned bus number 3
[   15.660560][    T1] xhci_hcd 0000:01:00.0: Host supports USB 3.0 SuperSpeed

Note that this board PCI and USB 3.0 have been working in the Linux
kernel for quite many years.

- These are relevant configs used in the build (not sure if I need to
use CONFIG_DM_PCI_COMPAT?)

CONFIG_DM=y
CONFIG_CMD_PCI=y
CONFIG_PCI=y
CONFIG_PCI_MVEBU=y
CONFIG_PCI_ENDPOINT=y
CONFIG_DM_PCI_COMPAT=y
CONFIG_USB_XHCI_HCD=y
CONFIG_USB_XHCI_PCI=y
CONFIG_USB_XHCI_MVEBU=y

BTW, the topic is no longer kwboot, should we move this to another new
thread? i.e. Testing PCI MVEBU with Kirkwood SoCs.

Thanks,
Tony

On Sat, Nov 6, 2021 at 4:26 PM Tony Dinh <mibo...@gmail.com> wrote:
>
> 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

Reply via email to