Hi Tom,
On Jun 3, 2020, at 8:03 AM, Tom Rini trini@konsulko.com wrote:
On Wed, Jun 03, 2020 at 01:05:18AM -0700, Alex Nemirovsky wrote:
From: Aaron Tseng aaron.tseng@cortina-access.com
Add Cortina Access Ethernet device driver for CAxxxx SoCs. This driver supports only the DM_ETH network model.
Signed-off-by: Aaron Tseng aaron.tseng@cortina-access.com Signed-off-by: Alex Nemirovsky alex.nemirovsky@cortina-access.com
CC: Joe Hershberger joe.hershberger@ni.com CC: Abbie Chang abbie.chang@Cortina-Access.com CC: Tom Rini trini@konsulko.com
Changes in v4: None Changes in v3:
- Changed commit comment to state that only DM model is supported
- Removed blank line at end of C file
Changes in v2:
- Remove legacy mode support
- Add support for additional SoC variants
- Remove unused variables
MAINTAINERS | 4 + drivers/net/Kconfig | 7 + drivers/net/Makefile | 1 + drivers/net/cortina_ni.c | 1909 ++++++++++++++++++++++++++++++++++++++++++++++ drivers/net/cortina_ni.h | 592 ++++++++++++++ 5 files changed, 2513 insertions(+) create mode 100644 drivers/net/cortina_ni.c create mode 100644 drivers/net/cortina_ni.h
So, is there a similar driver in upstream Linux?
We don’t have ANY Linux drivers upstream. Let me see if I can parse the comments below into action items for us. Let me know if I got it right or missed something.
This driver doesn't quite fit right.
At an "easy" level, there's still the customer macro around debug statements and not using pr_debug(),
find and convert all debug statement to using U-Boot specific pr_debug().
and contains a whole lot of debug code.
remove development debug code which is no longer used.
There's also code for a saturn platform, is that being upstreamed?
Yes, it will be upstreamed to u-boot.The approach is to first focus on getting all our u-boot drivers upstream for the presidio SoC engineering board. Most of the drivers are designed to work with other SoC platforms as those SoC reuse HW logic for our peripherals.
There's a ton of union usage which is also uncommon.
Is there an action item here?
A small item is it looks like this has its own crc function, when we have those available already.
reuse existing CRC functions from u-boot instead of providing our own.
There's also things like:
+enum ca_status_t ca_mdio_read(CA_IN unsigned int addr,
CA_IN unsigned int offset,CA_OUT unsigned short *data)+{
Where CA_IN / CA_OUT just don't fit.
What specifically is the action item here?
Which brings me back to why I asked the first question, this feels a lot like a driver for an RTOS or some other system was adapted to U-Boot, rather than writing a new driver for U-Boot based on internal knowledge of the part in question. And I think that applies to a lot of the drivers as seen in the NAND review as well. Thanks!
If I understand you correctly, you would like for the drivers to more directly REUSE native U-BOOT core functions already provided instead of providing our own which essentially duplicate the same core functions.
Did I misunderstand anything or do you have something more to add as action items for us?
-- Tom