Hi Tom, > On Jun 3, 2020, at 8:03 AM, Tom Rini <tr...@konsulko.com> wrote: > > On Wed, Jun 03, 2020 at 01:05:18AM -0700, Alex Nemirovsky wrote: >> From: Aaron Tseng <aaron.ts...@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.ts...@cortina-access.com> >> Signed-off-by: Alex Nemirovsky <alex.nemirov...@cortina-access.com> >> >> CC: Joe Hershberger <joe.hershber...@ni.com> >> CC: Abbie Chang <abbie.ch...@cortina-access.com> >> CC: Tom Rini <tr...@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