Hi Tom, Thanks for your feedback.
> On Jun 3, 2020, at 3:03 PM, Tom Rini <tr...@konsulko.com> wrote: > > 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 <tr...@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 <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. >>> >>> 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