RE: [PATCH net-next v5 1/2] net/ethernet/realtek: Add Realtek automotive PCIe driver code

From: Justin Lai
Date: Mon Aug 21 2023 - 09:39:15 EST


> Fri, Aug 18, 2023 at 01:55:00PM CEST, justinlai0215@xxxxxxxxxxx wrote:
> V>This patch is to add the ethernet device driver for the PCIe interface
> V>of Realtek Automotive Ethernet Switch,
> >applicable to RTL9054, RTL9068, RTL9072, RTL9075, RTL9068, RTL9071.
> >
> >Below is a simplified block diagram of the chip and its relevant interfaces.
> >
> > *************************
> > * *
> > * CPU network device *
> > * ____________ *
> > * | | *
> > * | PCIE Host | *
> > *************************
> > ||
> > PCIE
> > ||
> > ****************************************
> > * | PCIE Endpoint | *
> > * |---------------| *
> > * | GMAC | *
> > * |------| Realtek *
> > * || RTL90xx Series *
> > * || *
> > * _____________||______________ *
> > * | |MAC| | *
> > * | |---| | *
> > * | | *
> > * | Ethernet Switch Core | *
> > * | | *
> > * | ----- ----- | *
> > * | |MAC| ............|MAC| | *
> > * |__|___|_____________|___|____| *
> > * |PHY| ............|PHY| *
> > * ----- ----- *
> > *********||****************||***********
> >
> >The block of the Realtek RTL90xx series is our entire chip
> >architecture, the GMAC is connected to the switch core, and there is no
> >PHY in between. In addition, this driver is mainly used to control GMAC, but
> does not control the switch core, so it is not the same as DSA.
>
> [..]
>
>
> >+
> >+#define NETIF_F_ALL_CSUM NETIF_F_CSUM_MASK
> >+
> >+#define NETIF_F_HW_VLAN_RX NETIF_F_HW_VLAN_CTAG_RX #define
> >+NETIF_F_HW_VLAN_TX NETIF_F_HW_VLAN_CTAG_TX
>
> [..]
>
> I see 3 essentials wrong from 10sec review:
> 1) You don't cc people who commented your previous versions
> 2) You don't respect 72cols for patch description text (checkpatch did
> warn you, didn't it?)
> 3) You are defining very odd macros like these 3
>
> No need to read any longer... Really, can't you please ask someone who knows
> to help you with the submission preparation? You are vasting time of people :/
>

Thank you for your reply, I will re-check this driver and the content you mentioned above.