| Message ID | 20200117012047.31096-1-andre.przywara@arm.com |
|---|---|
| Headers | show |
| Series | Ethernet support for Raspberry Pi 4 | expand |
On Fri, Jan 17, 2020 at 01:20:44AM +0000, Andre Przywara wrote: > This series adds Ethernet support for the Raspberry Pi 4. The SoC > includes a "Broadcom Genet v5 MAC" IP, connected as a proper platform > device (no USB anymore!). Patch 1 provides a driver for that. There does > not seem to be publicly available documentation, so this is based on the > Linux driver, but stripped down to just provide what U-Boot needs. > Patch 2 fixes up the RPi4 memory map to accommodate the MMIO area the > MAC lives in, while patch 3 enables it in the respective defconfigs. > > This version addresses the comments by the diligent reviewers and testers, > for a changelog see below. > To see the individual changes as patches, refer to [1]. > > Please have a look and test it, I hope this helps to simplify > development, as you spare the SD card and its slot from heavy swapping. > > I dropped the Tested-by's, as there were changes in the code. Happy > to reapply them when people confirm that it still works for them. > > Cheers, > Andre. > > [1] https://github.com/apritzel/u-boot/commits/rpi4-eth-v2 > > Changelog v1 ... v2: > - use native endianess functions when accessing MMIO registers > - use dev_* DM wrappers for accessing devicetree data > - round base and length for flush_dcache_range, plus a comment > - check and round length for invalidate_cache_range > - support RGMII_RXID PHY mode, to support mainline .dtb > > Amit Singh Tomar (3): > net: Add support for Broadcom GENETv5 Ethernet controller > rpi4: Update memory map to accommodate scb devices > rpi4: Enable GENET Ethernet controller > > arch/arm/mach-bcm283x/init.c | 6 +- > configs/rpi_4_32b_defconfig | 2 + > configs/rpi_4_defconfig | 2 + > configs/rpi_arm64_defconfig | 1 + > drivers/net/Kconfig | 7 + > drivers/net/Makefile | 1 + > drivers/net/bcmgenet.c | 722 +++++++++++++++++++++++++++++++++++++++++++ > 7 files changed, 738 insertions(+), 3 deletions(-) > create mode 100644 drivers/net/bcmgenet.c > > -- > 2.14.5 > Hello I have tested it again and grabbing DHCP and doing TFTP works. But I still fail to boot any kernel. U-Boot 2020.01-00660-gec13baddca (Jan 21 2020 - 11:38:05 +0100) DRAM: 3.9 GiB RPI 4 Model B (0xc03111) MMC: emmc2 at 7e340000: 0, mmcnr at 7e300000: 1 Loading Environment from FAT... *** Warning - bad CRC, using default environment In: serial Out: serial Err: serial Net: eth0: genet at 7d580000 dhcp genet at 7d580000 Waiting for PHY auto negotiation to complete....... done BOOTP broadcast 1 BOOTP broadcast 2 BOOTP broadcast 3 DHCP client bound to address 192.168.66.27 (1255 ms) I use 0x80000 for kernel, 0x02700000 for RAMfs, 0x02400000 for DTB and booti 0x00080000 0x02700000 0x02400000 for starting kernel. Both mainline kernel and rpi kernel wont boot. But this is unrelated to your serie. Tested-by: Corentin Labbe <clabbe at baylibre.com> Thanks Regards
Hi Corentin, On 22/01/2020 11:04, LABBE Corentin wrote: > On Fri, Jan 17, 2020 at 01:20:44AM +0000, Andre Przywara wrote: >> This series adds Ethernet support for the Raspberry Pi 4. The SoC >> includes a "Broadcom Genet v5 MAC" IP, connected as a proper platform >> device (no USB anymore!). Patch 1 provides a driver for that. There does >> not seem to be publicly available documentation, so this is based on the >> Linux driver, but stripped down to just provide what U-Boot needs. >> Patch 2 fixes up the RPi4 memory map to accommodate the MMIO area the >> MAC lives in, while patch 3 enables it in the respective defconfigs. >> >> This version addresses the comments by the diligent reviewers and testers, >> for a changelog see below. >> To see the individual changes as patches, refer to [1]. >> >> Please have a look and test it, I hope this helps to simplify >> development, as you spare the SD card and its slot from heavy swapping. >> >> I dropped the Tested-by's, as there were changes in the code. Happy >> to reapply them when people confirm that it still works for them. >> >> Cheers, >> Andre. >> >> [1] https://github.com/apritzel/u-boot/commits/rpi4-eth-v2 >> >> Changelog v1 ... v2: >> - use native endianess functions when accessing MMIO registers >> - use dev_* DM wrappers for accessing devicetree data >> - round base and length for flush_dcache_range, plus a comment >> - check and round length for invalidate_cache_range >> - support RGMII_RXID PHY mode, to support mainline .dtb >> >> Amit Singh Tomar (3): >> net: Add support for Broadcom GENETv5 Ethernet controller >> rpi4: Update memory map to accommodate scb devices >> rpi4: Enable GENET Ethernet controller >> >> arch/arm/mach-bcm283x/init.c | 6 +- >> configs/rpi_4_32b_defconfig | 2 + >> configs/rpi_4_defconfig | 2 + >> configs/rpi_arm64_defconfig | 1 + >> drivers/net/Kconfig | 7 + >> drivers/net/Makefile | 1 + >> drivers/net/bcmgenet.c | 722 +++++++++++++++++++++++++++++++++++++++++++ >> 7 files changed, 738 insertions(+), 3 deletions(-) >> create mode 100644 drivers/net/bcmgenet.c >> >> -- >> 2.14.5 >> > > Hello > > I have tested it again and grabbing DHCP and doing TFTP works. > But I still fail to boot any kernel. > > U-Boot 2020.01-00660-gec13baddca (Jan 21 2020 - 11:38:05 +0100) > DRAM: 3.9 GiB > RPI 4 Model B (0xc03111) > MMC: emmc2 at 7e340000: 0, mmcnr at 7e300000: 1 > Loading Environment from FAT... *** Warning - bad CRC, using default environment > In: serial > Out: serial > Err: serial > Net: eth0: genet at 7d580000 > > dhcp > > genet at 7d580000 Waiting for PHY auto negotiation to complete....... done > BOOTP broadcast 1 > BOOTP broadcast 2 > BOOTP broadcast 3 > DHCP client bound to address 192.168.66.27 (1255 ms) > > I use 0x80000 for kernel, 0x02700000 for RAMfs, 0x02400000 for DTB and booti 0x00080000 0x02700000 0x02400000 for starting kernel. > Both mainline kernel and rpi kernel wont boot. > > But this is unrelated to your serie. Thanks for bringing this up. I only tested tftp and was able to run a grub binary. But I realized that with the patches applied, I can't boot a kernel from the SD card neither. As I have grub2 booting the kernel I wonder if you have the same problems? Regards, Matthias > Tested-by: Corentin Labbe <clabbe at baylibre.com> > > Thanks > Regards >
Hi Andre, Hi All, On 17/01/2020 02:20, Andre Przywara wrote: > This series adds Ethernet support for the Raspberry Pi 4. The SoC > includes a "Broadcom Genet v5 MAC" IP, connected as a proper platform > device (no USB anymore!). Patch 1 provides a driver for that. There does > not seem to be publicly available documentation, so this is based on the > Linux driver, but stripped down to just provide what U-Boot needs. > Patch 2 fixes up the RPi4 memory map to accommodate the MMIO area the > MAC lives in, while patch 3 enables it in the respective defconfigs. > > This version addresses the comments by the diligent reviewers and testers, > for a changelog see below. > To see the individual changes as patches, refer to [1]. > > Please have a look and test it, I hope this helps to simplify > development, as you spare the SD card and its slot from heavy swapping. > > I dropped the Tested-by's, as there were changes in the code. Happy > to reapply them when people confirm that it still works for them. > I having problems to actually boot a kernel when the genet driver is build into U-Boot. If I boot grub and linux-next from there, I get the following SError (when using earlycon): https://pastebin.com/c1sw2uZk If I skip grub and boot the kernel directly from the SD: load mmc 0:1 $kernel_addr_r Image load mmc 0:1 $fdt_addr_r bcm2711-rpi-4-b.dtb setenv bootargs "earlycon=uart8250,mmio32,0xfe215040" booti $kernel_addr_r - $fdt_addr_r Gives a similar result. Do you see similar issues? Regards, Matthias > Cheers, > Andre. > > [1] https://github.com/apritzel/u-boot/commits/rpi4-eth-v2 > > Changelog v1 ... v2: > - use native endianess functions when accessing MMIO registers > - use dev_* DM wrappers for accessing devicetree data > - round base and length for flush_dcache_range, plus a comment > - check and round length for invalidate_cache_range > - support RGMII_RXID PHY mode, to support mainline .dtb > > Amit Singh Tomar (3): > net: Add support for Broadcom GENETv5 Ethernet controller > rpi4: Update memory map to accommodate scb devices > rpi4: Enable GENET Ethernet controller > > arch/arm/mach-bcm283x/init.c | 6 +- > configs/rpi_4_32b_defconfig | 2 + > configs/rpi_4_defconfig | 2 + > configs/rpi_arm64_defconfig | 1 + > drivers/net/Kconfig | 7 + > drivers/net/Makefile | 1 + > drivers/net/bcmgenet.c | 722 +++++++++++++++++++++++++++++++++++++++++++ > 7 files changed, 738 insertions(+), 3 deletions(-) > create mode 100644 drivers/net/bcmgenet.c >
On Wed, 22 Jan 2020 18:18:39 +0100 Matthias Brugger <mbrugger at suse.com> wrote: Hi Matthias, > On 17/01/2020 02:20, Andre Przywara wrote: > > This series adds Ethernet support for the Raspberry Pi 4. The SoC > > includes a "Broadcom Genet v5 MAC" IP, connected as a proper platform > > device (no USB anymore!). Patch 1 provides a driver for that. There does > > not seem to be publicly available documentation, so this is based on the > > Linux driver, but stripped down to just provide what U-Boot needs. > > Patch 2 fixes up the RPi4 memory map to accommodate the MMIO area the > > MAC lives in, while patch 3 enables it in the respective defconfigs. > > > > This version addresses the comments by the diligent reviewers and testers, > > for a changelog see below. > > To see the individual changes as patches, refer to [1]. > > > > Please have a look and test it, I hope this helps to simplify > > development, as you spare the SD card and its slot from heavy swapping. > > > > I dropped the Tested-by's, as there were changes in the code. Happy > > to reapply them when people confirm that it still works for them. > > > > I having problems to actually boot a kernel when the genet driver is build into > U-Boot. Ah! Sorry, I misread the former reports, I thought this was about booting kernels in general, with mainline U-Boot, without this series. > If I boot grub and linux-next from there, I get the following SError (when using > earlycon): > https://pastebin.com/c1sw2uZk > > If I skip grub and boot the kernel directly from the SD: > load mmc 0:1 $kernel_addr_r Image > load mmc 0:1 $fdt_addr_r bcm2711-rpi-4-b.dtb > setenv bootargs "earlycon=uart8250,mmio32,0xfe215040" > booti $kernel_addr_r - $fdt_addr_r > > Gives a similar result. > > Do you see similar issues? I didn't manage to start some kernel even without this series, I think, but didn't investigate further. I *loaded* several kernel images via TFTP and verified them with md5sum. Some questions: - Does this happen even without touching the Ethernet in U-Boot at all (no dhcp command, no tftpboot, etc.)? (I wonder if we have still DMA going on, even after the kernel already started. But if we just call probe(), there shouldn't be much going on). - Does reverting patch 2/3 change anything? - Does TFTP load work in grub? (net_bootp efinet0; set net_default_server=<IP address>; linux (tftp)/Image-5.5-rc7 ....) I will try to debug this later tonight. Thanks! Andre. > > Regards, > Matthias > > > Cheers, > > Andre. > > > > [1] https://github.com/apritzel/u-boot/commits/rpi4-eth-v2 > > > > Changelog v1 ... v2: > > - use native endianess functions when accessing MMIO registers > > - use dev_* DM wrappers for accessing devicetree data > > - round base and length for flush_dcache_range, plus a comment > > - check and round length for invalidate_cache_range > > - support RGMII_RXID PHY mode, to support mainline .dtb > > > > Amit Singh Tomar (3): > > net: Add support for Broadcom GENETv5 Ethernet controller > > rpi4: Update memory map to accommodate scb devices > > rpi4: Enable GENET Ethernet controller > > > > arch/arm/mach-bcm283x/init.c | 6 +- > > configs/rpi_4_32b_defconfig | 2 + > > configs/rpi_4_defconfig | 2 + > > configs/rpi_arm64_defconfig | 1 + > > drivers/net/Kconfig | 7 + > > drivers/net/Makefile | 1 + > > drivers/net/bcmgenet.c | 722 +++++++++++++++++++++++++++++++++++++++++++ > > 7 files changed, 738 insertions(+), 3 deletions(-) > > create mode 100644 drivers/net/bcmgenet.c > >
On 22/01/2020 18:34, Andre Przywara wrote: > On Wed, 22 Jan 2020 18:18:39 +0100 > Matthias Brugger <mbrugger at suse.com> wrote: > > Hi Matthias, > >> On 17/01/2020 02:20, Andre Przywara wrote: >>> This series adds Ethernet support for the Raspberry Pi 4. The SoC >>> includes a "Broadcom Genet v5 MAC" IP, connected as a proper platform >>> device (no USB anymore!). Patch 1 provides a driver for that. There does >>> not seem to be publicly available documentation, so this is based on the >>> Linux driver, but stripped down to just provide what U-Boot needs. >>> Patch 2 fixes up the RPi4 memory map to accommodate the MMIO area the >>> MAC lives in, while patch 3 enables it in the respective defconfigs. >>> >>> This version addresses the comments by the diligent reviewers and testers, >>> for a changelog see below. >>> To see the individual changes as patches, refer to [1]. >>> >>> Please have a look and test it, I hope this helps to simplify >>> development, as you spare the SD card and its slot from heavy swapping. >>> >>> I dropped the Tested-by's, as there were changes in the code. Happy >>> to reapply them when people confirm that it still works for them. >>> >> >> I having problems to actually boot a kernel when the genet driver is build into >> U-Boot. > > Ah! Sorry, I misread the former reports, I thought this was about booting kernels in general, with mainline U-Boot, without this series. > >> If I boot grub and linux-next from there, I get the following SError (when using >> earlycon): >> https://pastebin.com/c1sw2uZk >> >> If I skip grub and boot the kernel directly from the SD: >> load mmc 0:1 $kernel_addr_r Image >> load mmc 0:1 $fdt_addr_r bcm2711-rpi-4-b.dtb >> setenv bootargs "earlycon=uart8250,mmio32,0xfe215040" >> booti $kernel_addr_r - $fdt_addr_r >> >> Gives a similar result. >> >> Do you see similar issues? > > I didn't manage to start some kernel even without this series, I think, but didn't investigate further. I *loaded* several kernel images via TFTP and verified them with md5sum. > I think linux-next with defconfig should work. > Some questions: > - Does this happen even without touching the Ethernet in U-Boot at all (no dhcp command, no tftpboot, etc.)? Yes, as soon as the genet is compiled into U-Boot I'm not able to boot a Linux kernel. > (I wonder if we have still DMA going on, even after the kernel already started. But if we just call probe(), there shouldn't be much going on). At least when we start grub, we are actually starting the genet. I played with the DMA shutdown in bcmgenet_gmac_eth_stop() but wasn't lucky. > - Does reverting patch 2/3 change anything? That was my first bet, but it hangs the board when it tries to initialize the network driver. > - Does TFTP load work in grub? (net_bootp efinet0; set net_default_server=<IP address>; linux (tftp)/Image-5.5-rc7 ....) Yes that works, until you boot the kernel and you end up with a SError. > > I will try to debug this later tonight. > Thanks, if I can help you in any way, let me know. Regards, Matthias > Thanks! > Andre. > >> >> Regards, >> Matthias >> >>> Cheers, >>> Andre. >>> >>> [1] https://github.com/apritzel/u-boot/commits/rpi4-eth-v2 >>> >>> Changelog v1 ... v2: >>> - use native endianess functions when accessing MMIO registers >>> - use dev_* DM wrappers for accessing devicetree data >>> - round base and length for flush_dcache_range, plus a comment >>> - check and round length for invalidate_cache_range >>> - support RGMII_RXID PHY mode, to support mainline .dtb >>> >>> Amit Singh Tomar (3): >>> net: Add support for Broadcom GENETv5 Ethernet controller >>> rpi4: Update memory map to accommodate scb devices >>> rpi4: Enable GENET Ethernet controller >>> >>> arch/arm/mach-bcm283x/init.c | 6 +- >>> configs/rpi_4_32b_defconfig | 2 + >>> configs/rpi_4_defconfig | 2 + >>> configs/rpi_arm64_defconfig | 1 + >>> drivers/net/Kconfig | 7 + >>> drivers/net/Makefile | 1 + >>> drivers/net/bcmgenet.c | 722 +++++++++++++++++++++++++++++++++++++++++++ >>> 7 files changed, 738 insertions(+), 3 deletions(-) >>> create mode 100644 drivers/net/bcmgenet.c >>> >
On Wed, 22 Jan 2020 19:05:10 +0100 Matthias Brugger <mbrugger at suse.com> wrote: Hi, Matthias, many thanks for looking at this and giving it a try! > On 22/01/2020 18:34, Andre Przywara wrote: > > On Wed, 22 Jan 2020 18:18:39 +0100 > > Matthias Brugger <mbrugger at suse.com> wrote: > > > > Hi Matthias, > > > >> On 17/01/2020 02:20, Andre Przywara wrote: > >>> This series adds Ethernet support for the Raspberry Pi 4. The SoC > >>> includes a "Broadcom Genet v5 MAC" IP, connected as a proper platform > >>> device (no USB anymore!). Patch 1 provides a driver for that. There does > >>> not seem to be publicly available documentation, so this is based on the > >>> Linux driver, but stripped down to just provide what U-Boot needs. > >>> Patch 2 fixes up the RPi4 memory map to accommodate the MMIO area the > >>> MAC lives in, while patch 3 enables it in the respective defconfigs. > >>> > >>> This version addresses the comments by the diligent reviewers and testers, > >>> for a changelog see below. > >>> To see the individual changes as patches, refer to [1]. > >>> > >>> Please have a look and test it, I hope this helps to simplify > >>> development, as you spare the SD card and its slot from heavy swapping. > >>> > >>> I dropped the Tested-by's, as there were changes in the code. Happy > >>> to reapply them when people confirm that it still works for them. > >>> > >> > >> I having problems to actually boot a kernel when the genet driver is build into > >> U-Boot. > > > > Ah! Sorry, I misread the former reports, I thought this was about booting kernels in general, with mainline U-Boot, without this series. > > > >> If I boot grub and linux-next from there, I get the following SError (when using > >> earlycon): > >> https://pastebin.com/c1sw2uZk > >> > >> If I skip grub and boot the kernel directly from the SD: > >> load mmc 0:1 $kernel_addr_r Image > >> load mmc 0:1 $fdt_addr_r bcm2711-rpi-4-b.dtb > >> setenv bootargs "earlycon=uart8250,mmio32,0xfe215040" > >> booti $kernel_addr_r - $fdt_addr_r > >> > >> Gives a similar result. > >> > >> Do you see similar issues? > > > > I didn't manage to start some kernel even without this series, I think, but didn't investigate further. I *loaded* several kernel images via TFTP and verified them with md5sum. > > > > I think linux-next with defconfig should work. Yeah, I had some success with 5.5-rc, at least till it goes into userland, which is good enough for this purpose. And indeed I could reproduce the early crash with genet compiled in vs. mainline U-Boot. > > Some questions: > > - Does this happen even without touching the Ethernet in U-Boot at all (no dhcp command, no tftpboot, etc.)? > > Yes, as soon as the genet is compiled into U-Boot I'm not able to boot a Linux > kernel. > > > (I wonder if we have still DMA going on, even after the kernel already started. But if we just call probe(), there shouldn't be much going on). > > At least when we start grub, we are actually starting the genet. I played with > the DMA shutdown in bcmgenet_gmac_eth_stop() but wasn't lucky. Stray DMA was more of a hope, as it would be easy to fix ;-) Without actively using the Ethernet in U-Boot, we never call eth_start, so don't enable DMA in the first place. Actually we don't even reset the MAC. See below. > > - Does reverting patch 2/3 change anything? > > That was my first bet, but it hangs the board when it tries to initialize the > network driver. True, it just looked promising ;-) So I did some experiments, and it seems like we only call ofdata_to_platdata() and probe() from the driver. The latter is not doing much, but it starts the whole PHY init process. I could actually avoid the crash by just *not* returning 0 in bcmgenet_phy_init(). So every hardware setup step was still performed, but U-Boot *thinks* it didn't work. Looking at this more closely I see that we don't actually reset the MAC before accessing the PHY via MDIO. This might be a lead, but I don't see immediately why this would lead to an SError interrupt later on. I don't have a working setup here at work, so if someone could try to insert: writel(0, priv->mac_reg + SYS_RBUF_FLUSH_CTRL); udelay(10); /* disable MAC while updating its registers */ writel(0, priv->mac_reg + UMAC_CMD); /* issue soft reset with (rg)mii loopback to ensure a stable rxclk */ writel(CMD_SW_RESET | CMD_LCL_LOOP_EN, priv->mac_reg + UMAC_CMD); before calling bcmgenet_mdio_init() in bcmgenet_eth_probe() and give this a try. Also it would be good to know if the PHY accesses via MDIO actually work at this point. As far as I remember the actual PHY init happens on first usage (typically the dhcp call). Another thing to check is whether we would need to put the MAC back into reset upon exiting U-Boot. I quickly wired in a debug print in a .remove routine, but that didn't show up, so not sure it gets actually called in our case? Cheers, Andre. > > - Does TFTP load work in grub? (net_bootp efinet0; set net_default_server=<IP address>; linux (tftp)/Image-5.5-rc7 ....) > > Yes that works, until you boot the kernel and you end up with a SError. > > > > > I will try to debug this later tonight. > > > > Thanks, if I can help you in any way, let me know. > > Regards, > Matthias > > > Thanks! > > Andre. > > > >> > >> Regards, > >> Matthias > >> > >>> Cheers, > >>> Andre. > >>> > >>> [1] https://github.com/apritzel/u-boot/commits/rpi4-eth-v2 > >>> > >>> Changelog v1 ... v2: > >>> - use native endianess functions when accessing MMIO registers > >>> - use dev_* DM wrappers for accessing devicetree data > >>> - round base and length for flush_dcache_range, plus a comment > >>> - check and round length for invalidate_cache_range > >>> - support RGMII_RXID PHY mode, to support mainline .dtb > >>> > >>> Amit Singh Tomar (3): > >>> net: Add support for Broadcom GENETv5 Ethernet controller > >>> rpi4: Update memory map to accommodate scb devices > >>> rpi4: Enable GENET Ethernet controller > >>> > >>> arch/arm/mach-bcm283x/init.c | 6 +- > >>> configs/rpi_4_32b_defconfig | 2 + > >>> configs/rpi_4_defconfig | 2 + > >>> configs/rpi_arm64_defconfig | 1 + > >>> drivers/net/Kconfig | 7 + > >>> drivers/net/Makefile | 1 + > >>> drivers/net/bcmgenet.c | 722 +++++++++++++++++++++++++++++++++++++++++++ > >>> 7 files changed, 738 insertions(+), 3 deletions(-) > >>> create mode 100644 drivers/net/bcmgenet.c > >>> > >
On 23/01/2020 12:29, Andre Przywara wrote: > On Wed, 22 Jan 2020 19:05:10 +0100 > Matthias Brugger <mbrugger at suse.com> wrote: > > Hi, > > Matthias, many thanks for looking at this and giving it a try! > >> On 22/01/2020 18:34, Andre Przywara wrote: >>> On Wed, 22 Jan 2020 18:18:39 +0100 >>> Matthias Brugger <mbrugger at suse.com> wrote: >>> >>> Hi Matthias, >>> >>>> On 17/01/2020 02:20, Andre Przywara wrote: >>>>> This series adds Ethernet support for the Raspberry Pi 4. The SoC >>>>> includes a "Broadcom Genet v5 MAC" IP, connected as a proper platform >>>>> device (no USB anymore!). Patch 1 provides a driver for that. There does >>>>> not seem to be publicly available documentation, so this is based on the >>>>> Linux driver, but stripped down to just provide what U-Boot needs. >>>>> Patch 2 fixes up the RPi4 memory map to accommodate the MMIO area the >>>>> MAC lives in, while patch 3 enables it in the respective defconfigs. >>>>> >>>>> This version addresses the comments by the diligent reviewers and testers, >>>>> for a changelog see below. >>>>> To see the individual changes as patches, refer to [1]. >>>>> >>>>> Please have a look and test it, I hope this helps to simplify >>>>> development, as you spare the SD card and its slot from heavy swapping. >>>>> >>>>> I dropped the Tested-by's, as there were changes in the code. Happy >>>>> to reapply them when people confirm that it still works for them. >>>>> >>>> >>>> I having problems to actually boot a kernel when the genet driver is build into >>>> U-Boot. >>> >>> Ah! Sorry, I misread the former reports, I thought this was about booting kernels in general, with mainline U-Boot, without this series. >>> >>>> If I boot grub and linux-next from there, I get the following SError (when using >>>> earlycon): >>>> https://pastebin.com/c1sw2uZk >>>> >>>> If I skip grub and boot the kernel directly from the SD: >>>> load mmc 0:1 $kernel_addr_r Image >>>> load mmc 0:1 $fdt_addr_r bcm2711-rpi-4-b.dtb >>>> setenv bootargs "earlycon=uart8250,mmio32,0xfe215040" >>>> booti $kernel_addr_r - $fdt_addr_r >>>> >>>> Gives a similar result. >>>> >>>> Do you see similar issues? >>> >>> I didn't manage to start some kernel even without this series, I think, but didn't investigate further. I *loaded* several kernel images via TFTP and verified them with md5sum. >>> >> >> I think linux-next with defconfig should work. > > Yeah, I had some success with 5.5-rc, at least till it goes into userland, which is good enough for this purpose. > And indeed I could reproduce the early crash with genet compiled in vs. mainline U-Boot. > >>> Some questions: >>> - Does this happen even without touching the Ethernet in U-Boot at all (no dhcp command, no tftpboot, etc.)? >> >> Yes, as soon as the genet is compiled into U-Boot I'm not able to boot a Linux >> kernel. >> >>> (I wonder if we have still DMA going on, even after the kernel already started. But if we just call probe(), there shouldn't be much going on). >> >> At least when we start grub, we are actually starting the genet. I played with >> the DMA shutdown in bcmgenet_gmac_eth_stop() but wasn't lucky. > > Stray DMA was more of a hope, as it would be easy to fix ;-) > Without actively using the Ethernet in U-Boot, we never call eth_start, so don't enable DMA in the first place. Actually we don't even reset the MAC. See below. > >>> - Does reverting patch 2/3 change anything? >> >> That was my first bet, but it hangs the board when it tries to initialize the >> network driver. > > True, it just looked promising ;-) > > So I did some experiments, and it seems like we only call ofdata_to_platdata() and probe() from the driver. The latter is not doing much, but it starts the whole PHY init process. > I could actually avoid the crash by just *not* returning 0 in bcmgenet_phy_init(). So every hardware setup step was still performed, but U-Boot *thinks* it didn't work. > > Looking at this more closely I see that we don't actually reset the MAC before accessing the PHY via MDIO. This might be a lead, but I don't see immediately why this would lead to an SError interrupt later on. > > I don't have a working setup here at work, so if someone could try to insert: > > writel(0, priv->mac_reg + SYS_RBUF_FLUSH_CTRL); > udelay(10); > /* disable MAC while updating its registers */ > writel(0, priv->mac_reg + UMAC_CMD); > /* issue soft reset with (rg)mii loopback to ensure a stable rxclk */ > writel(CMD_SW_RESET | CMD_LCL_LOOP_EN, priv->mac_reg + UMAC_CMD); > > before calling bcmgenet_mdio_init() in bcmgenet_eth_probe() and give this a try. > I'd say works by accident. If I use $fdtcontroladdr it works, but if I load the dtb into $fdt_addr_r the kernel crashes at boot. > Also it would be good to know if the PHY accesses via MDIO actually work at this point. As far as I remember the actual PHY init happens on first usage (typically the dhcp call). > I see bcmgenet_eth_probe() being called on U-Boot startup. After that I can read registers from the phy, e.g.: mdio read genet at 7d580000 1 Reading from bus mdio at e14 PHY at address 1: 1 - 0x7969 > Another thing to check is whether we would need to put the MAC back into reset upon exiting U-Boot. I quickly wired in a debug print in a .remove routine, but that didn't show up, so not sure it gets actually called in our case? You are right, it get's not called. I think last function called is bcmgenet_gmac_eth_stop() but that's not an analytic claim (speak, I only saw this sometimes when booting the kernel). Will keep digging tomorrow. Regards, Matthias > > Cheers, > Andre. > > >>> - Does TFTP load work in grub? (net_bootp efinet0; set net_default_server=<IP address>; linux (tftp)/Image-5.5-rc7 ....) >> >> Yes that works, until you boot the kernel and you end up with a SError. >> >>> >>> I will try to debug this later tonight. >>> >> >> Thanks, if I can help you in any way, let me know. >> >> Regards, >> Matthias >> >>> Thanks! >>> Andre. >>> >>>> >>>> Regards, >>>> Matthias >>>> >>>>> Cheers, >>>>> Andre. >>>>> >>>>> [1] https://github.com/apritzel/u-boot/commits/rpi4-eth-v2 >>>>> >>>>> Changelog v1 ... v2: >>>>> - use native endianess functions when accessing MMIO registers >>>>> - use dev_* DM wrappers for accessing devicetree data >>>>> - round base and length for flush_dcache_range, plus a comment >>>>> - check and round length for invalidate_cache_range >>>>> - support RGMII_RXID PHY mode, to support mainline .dtb >>>>> >>>>> Amit Singh Tomar (3): >>>>> net: Add support for Broadcom GENETv5 Ethernet controller >>>>> rpi4: Update memory map to accommodate scb devices >>>>> rpi4: Enable GENET Ethernet controller >>>>> >>>>> arch/arm/mach-bcm283x/init.c | 6 +- >>>>> configs/rpi_4_32b_defconfig | 2 + >>>>> configs/rpi_4_defconfig | 2 + >>>>> configs/rpi_arm64_defconfig | 1 + >>>>> drivers/net/Kconfig | 7 + >>>>> drivers/net/Makefile | 1 + >>>>> drivers/net/bcmgenet.c | 722 +++++++++++++++++++++++++++++++++++++++++++ >>>>> 7 files changed, 738 insertions(+), 3 deletions(-) >>>>> create mode 100644 drivers/net/bcmgenet.c >>>>> >>> >
On 23/01/2020 19:37, Matthias Brugger wrote: Hi, > On 23/01/2020 12:29, Andre Przywara wrote: >> On Wed, 22 Jan 2020 19:05:10 +0100 >> Matthias Brugger <mbrugger at suse.com> wrote: >> >> Hi, >> >> Matthias, many thanks for looking at this and giving it a try! >> >>> On 22/01/2020 18:34, Andre Przywara wrote: >>>> On Wed, 22 Jan 2020 18:18:39 +0100 >>>> Matthias Brugger <mbrugger at suse.com> wrote: >>>> >>>> Hi Matthias, >>>> >>>>> On 17/01/2020 02:20, Andre Przywara wrote: >>>>>> This series adds Ethernet support for the Raspberry Pi 4. The SoC >>>>>> includes a "Broadcom Genet v5 MAC" IP, connected as a proper platform >>>>>> device (no USB anymore!). Patch 1 provides a driver for that. There does >>>>>> not seem to be publicly available documentation, so this is based on the >>>>>> Linux driver, but stripped down to just provide what U-Boot needs. >>>>>> Patch 2 fixes up the RPi4 memory map to accommodate the MMIO area the >>>>>> MAC lives in, while patch 3 enables it in the respective defconfigs. >>>>>> >>>>>> This version addresses the comments by the diligent reviewers and testers, >>>>>> for a changelog see below. >>>>>> To see the individual changes as patches, refer to [1]. >>>>>> >>>>>> Please have a look and test it, I hope this helps to simplify >>>>>> development, as you spare the SD card and its slot from heavy swapping. >>>>>> >>>>>> I dropped the Tested-by's, as there were changes in the code. Happy >>>>>> to reapply them when people confirm that it still works for them. >>>>>> >>>>> >>>>> I having problems to actually boot a kernel when the genet driver is build into >>>>> U-Boot. >>>> >>>> Ah! Sorry, I misread the former reports, I thought this was about booting kernels in general, with mainline U-Boot, without this series. >>>> >>>>> If I boot grub and linux-next from there, I get the following SError (when using >>>>> earlycon): >>>>> https://pastebin.com/c1sw2uZk >>>>> >>>>> If I skip grub and boot the kernel directly from the SD: >>>>> load mmc 0:1 $kernel_addr_r Image >>>>> load mmc 0:1 $fdt_addr_r bcm2711-rpi-4-b.dtb >>>>> setenv bootargs "earlycon=uart8250,mmio32,0xfe215040" >>>>> booti $kernel_addr_r - $fdt_addr_r >>>>> >>>>> Gives a similar result. >>>>> >>>>> Do you see similar issues? >>>> >>>> I didn't manage to start some kernel even without this series, I think, but didn't investigate further. I *loaded* several kernel images via TFTP and verified them with md5sum. >>>> >>> >>> I think linux-next with defconfig should work. >> >> Yeah, I had some success with 5.5-rc, at least till it goes into userland, which is good enough for this purpose. >> And indeed I could reproduce the early crash with genet compiled in vs. mainline U-Boot. >> >>>> Some questions: >>>> - Does this happen even without touching the Ethernet in U-Boot at all (no dhcp command, no tftpboot, etc.)? >>> >>> Yes, as soon as the genet is compiled into U-Boot I'm not able to boot a Linux >>> kernel. >>> >>>> (I wonder if we have still DMA going on, even after the kernel already started. But if we just call probe(), there shouldn't be much going on). >>> >>> At least when we start grub, we are actually starting the genet. I played with >>> the DMA shutdown in bcmgenet_gmac_eth_stop() but wasn't lucky. >> >> Stray DMA was more of a hope, as it would be easy to fix ;-) >> Without actively using the Ethernet in U-Boot, we never call eth_start, so don't enable DMA in the first place. Actually we don't even reset the MAC. See below. >> >>>> - Does reverting patch 2/3 change anything? >>> >>> That was my first bet, but it hangs the board when it tries to initialize the >>> network driver. >> >> True, it just looked promising ;-) >> >> So I did some experiments, and it seems like we only call ofdata_to_platdata() and probe() from the driver. The latter is not doing much, but it starts the whole PHY init process. >> I could actually avoid the crash by just *not* returning 0 in bcmgenet_phy_init(). So every hardware setup step was still performed, but U-Boot *thinks* it didn't work. >> >> Looking at this more closely I see that we don't actually reset the MAC before accessing the PHY via MDIO. This might be a lead, but I don't see immediately why this would lead to an SError interrupt later on. >> >> I don't have a working setup here at work, so if someone could try to insert: >> >> writel(0, priv->mac_reg + SYS_RBUF_FLUSH_CTRL); >> udelay(10); >> /* disable MAC while updating its registers */ >> writel(0, priv->mac_reg + UMAC_CMD); >> /* issue soft reset with (rg)mii loopback to ensure a stable rxclk */ >> writel(CMD_SW_RESET | CMD_LCL_LOOP_EN, priv->mac_reg + UMAC_CMD); >> >> before calling bcmgenet_mdio_init() in bcmgenet_eth_probe() and give this a try. >> > > I'd say works by accident. If I use $fdtcontroladdr it works, but if I load the > dtb into $fdt_addr_r the kernel crashes at boot. Weird, thanks for giving this a try. >> Also it would be good to know if the PHY accesses via MDIO actually work at this point. As far as I remember the actual PHY init happens on first usage (typically the dhcp call). >> > > I see bcmgenet_eth_probe() being called on U-Boot startup. > > After that I can read registers from the phy, e.g.: > mdio read genet at 7d580000 1 > Reading from bus mdio at e14 > PHY at address 1: > 1 - 0x7969 So MDIO accesses seem to work fine, but actually MAC accesses seem to be a different story, see below. >> Another thing to check is whether we would need to put the MAC back into reset upon exiting U-Boot. I quickly wired in a debug print in a .remove routine, but that didn't show up, so not sure it gets actually called in our case? > > You are right, it get's not called. I think last function called is > bcmgenet_gmac_eth_stop() but that's not an analytic claim (speak, I only saw > this sometimes when booting the kernel) _stop() should only be called when _start() was called before. And this sequence happens on every network command, but not automatically without one. And I was hoping that probe() would be called on traversing the DT, fishing for devices (which it does), and remove() at the very end (EXIT_BOOTSERVICES). > Will keep digging tomorrow. Found the culprit, after following a lead started by an over-lunch discussion: Colleagues pointed out the SError (interrupts) early in the kernel could just show because they just got unmasked when dropping into EL1. And indeed in AArch64 U-Boot we keep Aborts masked - we don't clear the A bit in DAIF normally (only for Freescale). So allowing SError exceptions in U-Boot's start.S revealed that the SError interrupt was actually triggered by the writel in write_hwaddr(), I guess because the MAC wasn't reset before. And the SError condition stayed pending all the time, until the kernel announced its interest in being told about fatal errors - then it inherited U-Boot's error. So for me the issue is fixed after adding the reset routine I sketched in that thread before. But you mentioned that it still didn't work for you? Cheers, Andre
Hi, Thanks for having a look. > Yeah, I had some success with 5.5-rc, at least till it goes into userland, which is good enough for this purpose. > And indeed I could reproduce the early crash with genet compiled in vs. mainline U-Boot. Could not reproduce it with raspbian image but after using some ubuntu image definitely see this crash. > > writel(0, priv->mac_reg + SYS_RBUF_FLUSH_CTRL); > udelay(10); > /* disable MAC while updating its registers */ > writel(0, priv->mac_reg + UMAC_CMD); > /* issue soft reset with (rg)mii loopback to ensure a stable rxclk */ > writel(CMD_SW_RESET | CMD_LCL_LOOP_EN, priv->mac_reg + UMAC_CMD); > > before calling bcmgenet_mdio_init() in bcmgenet_eth_probe() and give this a try. > Indeed, it worked for me and can boot both the mainline and distro kernel with it. https://paste.ubuntu.com/p/vTY3RJF7Zy/ Also, with images loaded by TFTP https://pastebin.com/bAgfKhdq Thanks -Amit
On 24/01/2020 01:26, André Przywara wrote: > On 23/01/2020 19:37, Matthias Brugger wrote: > > Hi, > >> On 23/01/2020 12:29, Andre Przywara wrote: >>> On Wed, 22 Jan 2020 19:05:10 +0100 >>> Matthias Brugger <mbrugger at suse.com> wrote: >>> >>> Hi, >>> >>> Matthias, many thanks for looking at this and giving it a try! >>> >>>> On 22/01/2020 18:34, Andre Przywara wrote: >>>>> On Wed, 22 Jan 2020 18:18:39 +0100 >>>>> Matthias Brugger <mbrugger at suse.com> wrote: >>>>> >>>>> Hi Matthias, >>>>> >>>>>> On 17/01/2020 02:20, Andre Przywara wrote: >>>>>>> This series adds Ethernet support for the Raspberry Pi 4. The SoC >>>>>>> includes a "Broadcom Genet v5 MAC" IP, connected as a proper platform >>>>>>> device (no USB anymore!). Patch 1 provides a driver for that. There does >>>>>>> not seem to be publicly available documentation, so this is based on the >>>>>>> Linux driver, but stripped down to just provide what U-Boot needs. >>>>>>> Patch 2 fixes up the RPi4 memory map to accommodate the MMIO area the >>>>>>> MAC lives in, while patch 3 enables it in the respective defconfigs. >>>>>>> >>>>>>> This version addresses the comments by the diligent reviewers and testers, >>>>>>> for a changelog see below. >>>>>>> To see the individual changes as patches, refer to [1]. >>>>>>> >>>>>>> Please have a look and test it, I hope this helps to simplify >>>>>>> development, as you spare the SD card and its slot from heavy swapping. >>>>>>> >>>>>>> I dropped the Tested-by's, as there were changes in the code. Happy >>>>>>> to reapply them when people confirm that it still works for them. >>>>>>> >>>>>> >>>>>> I having problems to actually boot a kernel when the genet driver is build into >>>>>> U-Boot. >>>>> >>>>> Ah! Sorry, I misread the former reports, I thought this was about booting kernels in general, with mainline U-Boot, without this series. >>>>> >>>>>> If I boot grub and linux-next from there, I get the following SError (when using >>>>>> earlycon): >>>>>> https://pastebin.com/c1sw2uZk >>>>>> >>>>>> If I skip grub and boot the kernel directly from the SD: >>>>>> load mmc 0:1 $kernel_addr_r Image >>>>>> load mmc 0:1 $fdt_addr_r bcm2711-rpi-4-b.dtb >>>>>> setenv bootargs "earlycon=uart8250,mmio32,0xfe215040" >>>>>> booti $kernel_addr_r - $fdt_addr_r >>>>>> >>>>>> Gives a similar result. >>>>>> >>>>>> Do you see similar issues? >>>>> >>>>> I didn't manage to start some kernel even without this series, I think, but didn't investigate further. I *loaded* several kernel images via TFTP and verified them with md5sum. >>>>> >>>> >>>> I think linux-next with defconfig should work. >>> >>> Yeah, I had some success with 5.5-rc, at least till it goes into userland, which is good enough for this purpose. >>> And indeed I could reproduce the early crash with genet compiled in vs. mainline U-Boot. >>> >>>>> Some questions: >>>>> - Does this happen even without touching the Ethernet in U-Boot at all (no dhcp command, no tftpboot, etc.)? >>>> >>>> Yes, as soon as the genet is compiled into U-Boot I'm not able to boot a Linux >>>> kernel. >>>> >>>>> (I wonder if we have still DMA going on, even after the kernel already started. But if we just call probe(), there shouldn't be much going on). >>>> >>>> At least when we start grub, we are actually starting the genet. I played with >>>> the DMA shutdown in bcmgenet_gmac_eth_stop() but wasn't lucky. >>> >>> Stray DMA was more of a hope, as it would be easy to fix ;-) >>> Without actively using the Ethernet in U-Boot, we never call eth_start, so don't enable DMA in the first place. Actually we don't even reset the MAC. See below. >>> >>>>> - Does reverting patch 2/3 change anything? >>>> >>>> That was my first bet, but it hangs the board when it tries to initialize the >>>> network driver. >>> >>> True, it just looked promising ;-) >>> >>> So I did some experiments, and it seems like we only call ofdata_to_platdata() and probe() from the driver. The latter is not doing much, but it starts the whole PHY init process. >>> I could actually avoid the crash by just *not* returning 0 in bcmgenet_phy_init(). So every hardware setup step was still performed, but U-Boot *thinks* it didn't work. >>> >>> Looking at this more closely I see that we don't actually reset the MAC before accessing the PHY via MDIO. This might be a lead, but I don't see immediately why this would lead to an SError interrupt later on. >>> >>> I don't have a working setup here at work, so if someone could try to insert: >>> >>> writel(0, priv->mac_reg + SYS_RBUF_FLUSH_CTRL); >>> udelay(10); >>> /* disable MAC while updating its registers */ >>> writel(0, priv->mac_reg + UMAC_CMD); >>> /* issue soft reset with (rg)mii loopback to ensure a stable rxclk */ >>> writel(CMD_SW_RESET | CMD_LCL_LOOP_EN, priv->mac_reg + UMAC_CMD); >>> >>> before calling bcmgenet_mdio_init() in bcmgenet_eth_probe() and give this a try. >>> >> >> I'd say works by accident. If I use $fdtcontroladdr it works, but if I load the >> dtb into $fdt_addr_r the kernel crashes at boot. > > Weird, thanks for giving this a try. > >>> Also it would be good to know if the PHY accesses via MDIO actually work at this point. As far as I remember the actual PHY init happens on first usage (typically the dhcp call). >>> >> >> I see bcmgenet_eth_probe() being called on U-Boot startup. >> >> After that I can read registers from the phy, e.g.: >> mdio read genet at 7d580000 1 >> Reading from bus mdio at e14 >> PHY at address 1: >> 1 - 0x7969 > > So MDIO accesses seem to work fine, but actually MAC accesses seem to be > a different story, see below. > >>> Another thing to check is whether we would need to put the MAC back into reset upon exiting U-Boot. I quickly wired in a debug print in a .remove routine, but that didn't show up, so not sure it gets actually called in our case? >> >> You are right, it get's not called. I think last function called is >> bcmgenet_gmac_eth_stop() but that's not an analytic claim (speak, I only saw >> this sometimes when booting the kernel) > > _stop() should only be called when _start() was called before. And this > sequence happens on every network command, but not automatically without > one. > > And I was hoping that probe() would be called on traversing the DT, > fishing for devices (which it does), and remove() at the very end > (EXIT_BOOTSERVICES). > >> Will keep digging tomorrow. > > Found the culprit, after following a lead started by an over-lunch > discussion: Colleagues pointed out the SError (interrupts) early in the > kernel could just show because they just got unmasked when dropping into > EL1. And indeed in AArch64 U-Boot we keep Aborts masked - we don't clear > the A bit in DAIF normally (only for Freescale). > So allowing SError exceptions in U-Boot's start.S revealed that the > SError interrupt was actually triggered by the writel in write_hwaddr(), > I guess because the MAC wasn't reset before. And the SError condition > stayed pending all the time, until the kernel announced its interest in > being told about fatal errors - then it inherited U-Boot's error. > Thanks for the explanation. I think the situation leaves space for improving. Either should we warn about a pending Abort before leaving U-Boot or we should allow aborts in general. > So for me the issue is fixed after adding the reset routine I sketched > in that thread before. > > But you mentioned that it still didn't work for you? > I just double checked and everything works fine. Please feel free to send a new version :) Regards, Matthias
On 26/01/2020 02:28, Matthias Brugger wrote: > On 24/01/2020 01:26, André Przywara wrote: [ ... ] >> Found the culprit, after following a lead started by an over-lunch >> discussion: Colleagues pointed out the SError (interrupts) early in the >> kernel could just show because they just got unmasked when dropping into >> EL1. And indeed in AArch64 U-Boot we keep Aborts masked - we don't clear >> the A bit in DAIF normally (only for Freescale). >> So allowing SError exceptions in U-Boot's start.S revealed that the >> SError interrupt was actually triggered by the writel in write_hwaddr(), >> I guess because the MAC wasn't reset before. And the SError condition >> stayed pending all the time, until the kernel announced its interest in >> being told about fatal errors - then it inherited U-Boot's error. > > Thanks for the explanation. I think the situation leaves space for improving. > Either should we warn about a pending Abort before leaving U-Boot or we should > allow aborts in general. Definitively we should unmasks SErrors in U-Boot, since they point us to serious problems, with this one here actually being somewhat on the harmless side. Also U-Boot has exception handlers that dump useful information, so we should use them. But doing so would need to be done for all ARMv8 ports (in start.S), so I am a bit reluctant to post something this late in the merge window without proper testing on multiple platforms. >> So for me the issue is fixed after adding the reset routine I sketched >> in that thread before. >> >> But you mentioned that it still didn't work for you? >> > > I just double checked and everything works fine. Please feel free to send a new > version :) Great, thanks! Did just that. Cheers, Andre