RE: [PATCH 4/7] net: stmmac: introducing support for DWC xPCS logics

[Jose Abreu]

From: Voon, Weifeng <weifeng.voon@xxxxxxxxx>
Date: Thu, Apr 25, 2019 at 08:06:43

> > From: Ong Boon Leong <boon.leong.ong@xxxxxxxxx>
> > 
> > xPCS is DWC Ethernet Physical Coding Sublayer that may be integrated into a
> > GbE controller that uses DWC EQoS MAC controller. An example of HW
> > configuration is shown below:-
> > 
> >   <-----------------GBE Controller---------->|<--External PHY chip-->
> > 
> >   +----------+         +----+    +---+               +--------------+
> >   |   EQoS   | <-GMII->|xPCS|<-->|L1 | <-- SGMII --> | External GbE |
> >   |   MAC    |         |    |    |PHY|               | PHY Chip     |
> >   +----------+         +----+    +---+               +--------------+
> >          ^               ^                                  ^
> >          |               |                                  |
> >          +---------------------MDIO-------------------------+
> > 
> > xPCS is a Clause-45 MDIO Manageable Device (MMD) and we need a way to
> > differentiate it from external PHY chip that is discovered over MDIO.
> > Therefore, xpcs_phy_addr is introduced in stmmac platform data
> > (plat_stmmacenet_data) for differentiating xPCS from 'phy_addr' that
> > belongs to external PHY.
> > 
> > Basic functionalities for initializing xPCS and configuring auto negotiation (AN),
> > loopback, link status, AN advertisement and Link Partner ability are
> > implemented.
> > 
> > xPCS interrupt handling for C37 AN complete is also implemented.
> > 
> > Tested-by: Kweh Hock Leong <hock.leong.kweh@xxxxxxxxx>
> > Reviewed-by: Chuah Kim Tatt <kim.tatt.chuah@xxxxxxxxx>
> > Reviewed-by: Voon Weifeng <weifeng.voon@xxxxxxxxx>
> > Reviewed-by: Kweh Hock Leong <hock.leong.kweh@xxxxxxxxx>
> > Reviewed-by: Baoli Zhang <baoli.zhang@xxxxxxxxx>
> > Signed-off-by: Ong Boon Leong <boon.leong.ong@xxxxxxxxx>
> > ---
> >  drivers/net/ethernet/stmicro/stmmac/dw_xpcs.h | 288
> > ++++++++++++++++++++++++++
> >  drivers/net/ethernet/stmicro/stmmac/hwif.h    |  17 ++
> >  include/linux/stmmac.h                        |   1 +
> >  3 files changed, 306 insertions(+)
> >  create mode 100644 drivers/net/ethernet/stmicro/stmmac/dw_xpcs.h
> > 
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/dw_xpcs.h
> > b/drivers/net/ethernet/stmicro/stmmac/dw_xpcs.h
> > new file mode 100644
> > index 0000000..446b714
> > --- /dev/null
> > +++ b/drivers/net/ethernet/stmicro/stmmac/dw_xpcs.h

I would rather prefer see this as a .c file and then just export a new 
structure for HWIF because this does not belong to the MAC. Is there any 
specific reason why you added this as a .h file besides the reuse of 
callbacks across cores ?

And having inline functions everywhere seems overkill also.

Thanks,
Jose Miguel Abreu