Add 64-bit address support to Cadence CGEM Ethernet driver for use in
other SoCs such as the Zynq UltraScale+ and SiFive Highfive Unleashed.
Details
Details
- Reviewers
manu philip - Group Reviewers
manpages - Commits
- rGfacdd1cd2045: cgem: add 64-bit support
Testing on Ultra96 board with ethernet mezzanine board. I have tweaked
the Ultra96's DTS so that physical memory is split across low-memory
addresses and the 32G boundary and have verified that high memory
addresses are appearing in transmit and receive descriptors.
Diff Detail
Diff Detail
- Repository
- rS FreeBSD src repository - subversion
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 30650 Build 28386: arc lint + arc unit
Event Timeline
Comment Actions
Tweak cgem description in config files. Correct man page with regards to which
devices the rxhangwar is enabled.
Comment Actions
I am a little dubious about the many instances of #if INTPTR_MAX == INT64_MAX compile-time gymnastics.
| share/man/man4/cgem.4 | ||
|---|---|---|
| 48 | "among many others"? It's a pretty common IP block. | |
| sys/dev/cadence/if_cgem.c | ||
| 418 | Isn't it clearer to simply always restrict it to 1ULL << 32? | |
| sys/dev/cadence/if_cgem_hw.h | ||
| 40 | Put the URL on a line by itself so it doesn't scroll off the screen quite so far. ;-) | |
Comment Actions
The device tree on my FU540 lists the interface as a 'cdns,macb', which isn't listed in the compat_data any more. It's probably worth keeping an entry for it.
With both HWTYPE_GENERIC_GEM and HWTYPE_SIFIVE_FU540 it work (well, pings do, I've not tested more than that).
Comment Actions
I'm open to suggestions for something cleaner. In most of those instances it's because the compiler balks when I shift an address 32 bits in armv7.
| share/man/man4/cgem.4 | ||
|---|---|---|
| 48 | Is it? The only other place I could find any reference to it was in the Atmel AT91 boards and support for those was pulled out a while ago. Further, the "macb" device is a different enough beast than cgem that the driver in (that other operating system) has separate register definitions and separate routines for initialization and filling and retrieving descriptors. I don't think this driver would work for the macb and that's why I removed "cdns,macb" from the compatibility list. | |
| sys/dev/cadence/if_cgem.c | ||
| 418 | The compiler balks at this in 32-bit mode. | |
Comment Actions
Simplify support for 64-bit systems.
This diff got ugly because I was trying to support the case of a 32-bit
device in a 64-bit kernel. But, I don't think that would work anyway and
I think it's safe to assume that any 64-bit system (arm64 or riscv64) will
have a 64-bit capable gem core. Also, I have added support for using the
clock infrastructure for changing the transmit reference clock which I need
for my zynqmp implementation.
Comment Actions
Hi @skibo , is there any remaining work to be done on this patch? It appears to be ready to go, other than one small nitpick. It would be nice to see it committed before stable/13 branches later this month.
| share/man/man4/Makefile | ||
|---|---|---|
| 885 | Generally ${MACHINE_CPUARCH} is preferred since it captures all variants of an architecture. Described further in arch(7). | |
Comment Actions
I'd love to see this go into 13.0. I want to sanity check the driver again and then I'll make your suggested change and update, hopefully by tomorrow.
Comment Actions
I am not a committer but I bumped the date. Earlier today, I sanity checked the driver on both Zynq board and a Zynq UltraScale so I think it's ready.
Comment Actions
Ahh, my mistake. Thanks for taking the time to verify it. I can commit it some time this weekend if nobody beats me to it.