Messages in this thread | | | From | "Voon, Weifeng" <> | Subject | RE: [RFC,net-next,v1, 1/1] net: stmmac: Enable SERDES power up/down sequence | Date | Wed, 1 Apr 2020 14:23:09 +0000 |
| |
> > > > > 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.
I went through the code again, I understand that your intention is to keep the platform specific setup in its own file and keep the main dwmac logic clean. But, I did not see any way to separate this SERDES configuration from the stmmac_main logic cleanly. Hope to get more ideas and discussion. Thanks.
Weifeng
> > Regards, > Weifeng > > > --- > > Thanks, > > Jose Miguel Abreu
| |