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 <mbrug...@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 <mbrug...@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@7d580000 1 > Reading from bus mdio@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