[PATCH v4 1/2] net: cortina_ni: Add eth support for Cortina Access CAxxxx SoCs

[Tom Rini]

On Wed, Jun 03, 2020 at 09:10:46PM +0000, Alex Nemirovsky wrote:
> Hi Tom
> 
> > On Jun 3, 2020, at 1:59 PM, Tom Rini <trini at konsulko.com> wrote:
> > 
> > On Wed, Jun 03, 2020 at 07:03:09PM +0000, Alex Nemirovsky wrote:
> >> Hi Tom,
> >> 
> >>> On Jun 3, 2020, at 8:03 AM, Tom Rini <trini at konsulko.com> wrote:
> >>> 
> >>> On Wed, Jun 03, 2020 at 01:05:18AM -0700, Alex Nemirovsky wrote:
> >>>> From: Aaron Tseng <aaron.tseng at 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 at cortina-access.com>
> >>>> Signed-off-by: Alex Nemirovsky <alex.nemirovsky at cortina-access.com>
> >>>> 
> >>>> CC: Joe Hershberger <joe.hershberger at ni.com>
> >>>> CC: Abbie Chang <abbie.chang at Cortina-Access.com>
> >>>> CC: Tom Rini <trini at 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. 
> > 
> > Ah right.
> > 
> >> 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().
> > 
> > pr_debug and other functions, and a handful of other print functions are
> > in <linux/printk.h> and will both mean you don't need to wrap debug
> > statements but also leverage the logging functionality correctly so that
> > we can also discard messages to save space but also still easily bring
> > in useful information when a developer has a problem they're tracing.
> > 
> >>> and contains a whole
> >>> lot of debug code.
> >> 
> >> remove development debug code which is no longer used.
> > 
> > Yes, and probably then do some clean-up of the functions once that's
> > removed, if that then makes sense.
> > 
> >>> 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.  
> > 
> > OK, so lets leave the stuff for other platforms out for now.  If there's
> > abstractions that are needed for them, you can note that's why there's a
> > hook when there is a hook.  But that's probably still better handled
> > when introducing / posting the platform that needs them for further
> > context.
> > 
> >>> There's a ton of union usage which is also uncommon.
> >> Is there an action item here?
> > 
> > Well, why is it written the way it's written?  
> 
> The SW team didn’t create these structures by hand. They are autogenerated 
> my our HW design tools and adopted by the SW team.  
> To be clear, I am not trying to defend the use of this auto generated structures, just explaining its origin.
> Obviously it will take significant effort for us to recreate these structures in the desired form.
> 
> Therefore, it must be asked.  Is it required to be recreated in the structure form that you request and C code modified 
> accordingly  to be accepted into the u-boot upstream tree going forward?

Yes, and work with the HW design tool folks to generate output that's
more acceptable to various open source projects.  You aren't the first
team to have to push for this, I can tell you from experience.

But since it's generated output, it can be transformed too via sed, etc.
Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200603/39613609/attachment.sig>