[PATCH v2 0/3] Ethernet support for Raspberry Pi 4

[Matthias Brugger]

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
>>>>>     
>>>   
>