RE: [RFC,net-next,v1, 1/1] net: stmmac: Enable SERDES power up/down sequence

[Voon, Weifeng]

> > > > This patch is to enable Intel SERDES power up/down sequence. The
> > > > SERDES converts 8/10 bits data to SGMII signal. Below is an
> > > > example of HW configuration for SGMII mode. The SERDES is located
> > > > in the PHY IF in the diagram below.
> > > >
> > > > <-----------------GBE Controller---------->|<--External PHY
> > > > chip-->
> > > > +----------+         +----+            +---+           +----------
> +
> > > > |   EQoS   | <-GMII->| DW | < ------ > |PHY| <-SGMII-> | External
> |
> > > > |   MAC    |         |xPCS|            |IF |           | PHY
> |
> > > > +----------+         +----+            +---+           +----------
> +
> > > >        ^               ^                 ^                ^
> > > >        |               |                 |                |
> > > >        +---------------------MDIO-------------------------+
> > > >
> > > > PHY IF configuration and status registers are accessible through
> > > > mdio address 0x15 which is defined as intel_adhoc_addr. During D0,
> > > > The driver will need to power up PHY IF by changing the power
> state to P0.
> > > > Likewise, for D3, the driver sets PHY IF power state to P3.
> > >
> > > I don't think this is the right approach.
> > >
> > > You could just add a new "mdio-intel-serdes" to phy/ folder just
> > > like I did with XPCS because this is mostly related with PHY
> > > settings rather than EQoS.
> > I am taking this approach to put it in stmmac folder rather than phy
> > folder as a generic mdio-intel-serdes as this is a specific Intel
> > serdes architecture which would only pair with DW EQos and DW xPCS HW.
> > Since this serdes will not able to pair other MAC or other non-Intel
> > platform, I would like you to reconsider this approach. I am open for
> discussion.
> > Thanks Jose for the fast response.
> 
> OK, then I think we should use the BSP init/exit functions that are
> already available for platform setups (.init and .exit callback of
> plat_stmmacenet_data struct). We just need to extend this to PCI based
> setups.
> 
> You can take a look at stmmac_platform.c and check what's done.
> Basically:
> 	- Call priv->plat->init() at probe() and resume()
> 	- Call priv->plat->exit() at remove() and suspend()
> 
I have 2 concern if using the suggested BSP init/exit function.
1. Serdes is configured through MDIO bus. But the mdio bus register only happens
in stmmac_dvr_probe() in stmmac_main.c. 

2. All tx/rx packets requires serdes to be in the correct power state. If the driver 
power-down before stopping all the dma, it will cause tx queue timeout as packets 
are not able to be transmitted out. Hence, the serdes cannot be power-down before calling
the stmmac_dvr_remove(). The stmmac_dvr_remove() will unregister the mdio bus. So, the 
driver cannot powerdown the serdes after the stmmac_dvr_remove() too.    

Regards,
Weifeng

> ---
> Thanks,
> Jose Miguel Abreu