Page MenuHomeFreeBSD

Add genet driver for Raspberry Pi 4B Ethernet
ClosedPublic

Authored by karels on Apr 16 2020, 12:49 AM.
Tags
None
Referenced Files
Unknown Object (File)
Feb 23 2024, 8:22 AM
Unknown Object (File)
Feb 13 2024, 12:46 AM
Unknown Object (File)
Dec 10 2023, 7:23 PM
Unknown Object (File)
Nov 24 2023, 2:11 AM
Unknown Object (File)
Nov 23 2023, 8:27 AM
Unknown Object (File)
Nov 23 2023, 8:02 AM
Unknown Object (File)
Nov 22 2023, 5:28 PM
Unknown Object (File)
Nov 22 2023, 8:52 AM

Details

Summary

Add driver for Broadcom "GENET" version 5, as found in BCM-2711 on
Raspberry Pi 4B. The driver is derived in part from the bcmgenet.c
driver in NetBSD, along with bcmgenetreg.h.

Test Plan

Tested by several alpha testers

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 30621
Build 28361: arc lint + arc unit

Event Timeline

sys/arm64/broadcom/genet/if_genet.c
314

You need to release the resource before returning here.

351

We have mii_fdt_get_contype in sys/dev/mii/mii_fdt.c for this.

sys/conf/files.arm64
182

optional SOC_BRCM_BCM2838 fdt genet

I see what you mean now about gen_setup_extres() having been useless for your purposes -- the problem is that our RPi stuff doesn't use the extres clk framework like it should, so these clk/hwreset may be found in the FDT but they aren't locally registered like the framework's expecting. This scared me at first, and it still kinda sucks, but that's baggage that predates your work on this.

would be nice to split the FDT attachment into a separate if_genet_fdt file already, since we know if_genet_acpi will eventually come (here's the acpi table)

skibo added inline comments.
sys/arm64/broadcom/genet/if_genet.c
1181–1184

Weird indentation. Is this leftover debug code?

@greg: I'm not quite sure how the FDT/ACPI split should be done; I think that will have to wait for another time.

sys/arm64/broadcom/genet/if_genet.c
314

Which resource?

351

Thanks, fixed in next iteration.

1181–1184

Yes, thanks Removed in next iteration.

Updates based on review comments, including teardown after failed attach.

In D24436#538192, @greg_unrelenting.technology wrote:

would be nice to split the FDT attachment into a separate if_genet_fdt file already, since we know if_genet_acpi will eventually come (here's the acpi table)

Agreed, even if this could be done later it would be better to split now.

sys/arm64/broadcom/genet/if_genet.c
333

Why not make this function a complete detach ?
This would also help for dev/test by only compiling this driver as a module.

376

It always returns 0 even if we cannot parse the phy mode, please return 1 here in case of an error.

518

This function return void but can fail, if bus_dma* calls fails we will still attach but the interface wouldn't be usable. Better return the error code and handle that in attach().

582

bus_dmamap_destroy can fails, I think that there is nothing that we can do if it fails but a warning printf would be nice.

586

Same for bus_dma_tag_destroy

1655

that doesn't seem right

Is there an existing example of FDT / ACPI split to use as a model?

About doing a full detach: then code is needed to undo the init routine, and there are far more failure modes. I'll look at it though.

Is there an existing example of FDT / ACPI split to use as a model?

You could have a look at sys/dev/usb/controller/generic_ehci*

Address more review comments; fix bug with bpf and txcsum.

Notes:

I started a version with full detach; it is not working correctly yet.

I looked at the FDT/ACPI versions of the generic ehci; is there any
documentation of how inheritance works? I think moving to that framework
would take me a while, so I'm favoring lazy evaluation at the moment.

Address more review comments; fix bug with bpf and txcsum.

Notes:

I started a version with full detach; it is not working correctly yet.

What is the problem exactly ?

I looked at the FDT/ACPI versions of the generic ehci; is there any
documentation of how inheritance works?

I'm not sure ...

I think moving to that framework would take me a while, so I'm favoring lazy evaluation at the moment.

I'm not even sure that we could boot RPI4 with ACPI right now so I guess it's ok to leave this for now.

This looks good for me now, I've not tested the driver as I don't have the hardware but all my comments are now resolved.
Thanks for doing this port.

This revision is now accepted and ready to land.Apr 20 2020, 9:44 PM
In D24436#539201, @manu wrote:

Address more review comments; fix bug with bpf and txcsum.

Notes:

I started a version with full detach; it is not working correctly yet.

What is the problem exactly ?

I started with code to "stop" on ifconfig down, then re-init on "up". The plan was to use "stop" to undo all the running state, like flushing rings, when doing detach. It was no longer crashing, but not working after re-init.

I looked at the FDT/ACPI versions of the generic ehci; is there any
documentation of how inheritance works?

I'm not sure ...

I think moving to that framework would take me a while, so I'm favoring lazy evaluation at the moment.

I'm not even sure that we could boot RPI4 with ACPI right now so I guess it's ok to leave this for now.

Thanks, that seems best to me too.

I will wait until tomorrow for any other comments, then commit if there is nothing significant/unfixed.

This revision was automatically updated to reflect the committed changes.