RE: [PATCH V1 net-next 0/3] net: stmmac: implement clocks

From: Joakim Zhang <qiangqing.zhang@nxp.com>
To: Andrew Lunn <andrew@lunn.ch>, Florian Fainelli <f.fainelli@gmail.com>
Cc: netdev <netdev@vger.kernel.org>, dl-linux-imx <linux-imx@nxp.com>
Subject: RE: [PATCH V1 net-next 0/3] net: stmmac: implement clocks
Date: Thu, 25 Feb 2021 11:48:05 +0000	[thread overview]
Message-ID: <DB8PR04MB679510F099C88F6B4BA10970E69E9@DB8PR04MB6795.eurprd04.prod.outlook.com> (raw)
In-Reply-To: <YDcMgr2rvcFvs746@lunn.ch>

> -----Original Message-----
> From: Andrew Lunn <andrew@lunn.ch>
> Sent: 2021年2月25日 10:34
> To: Joakim Zhang <qiangqing.zhang@nxp.com>
> Cc: netdev <netdev@vger.kernel.org>
> Subject: Re: [PATCH V1 net-next 0/3] net: stmmac: implement clocks
> 
> > Hi Andrew,
> >
> 
> > I don't have experience with Ethernet switch, according to your
> > points, you mean we can connect STMMAC to an Ethernet switch, and then
> > Ethernet switch managed STMMAC by the MDIO bus but without checking
> > whether STMMAC interface is opened or not, so STMMAC needs clocks for
> > MDIO even interface is closed, right?
> 
> Correct. The MDIO bus has a different life cycle to the MAC. If any of
> stmmac_xgmac2_mdio_read(), stmmac_xgmac2_mdio_write(),
> stmmac_mdio_read(), and stmmac_mdio_write() need clocks ticking, you need
> to ensure the clock is ticking, because these functions can be called while the
> interface is not opened.

Hi Andrew,

Thanks for you explanation, I still don't quite understand what the use case it is, could you give me more details, thanks a lot!
AFAIK now, there are two connections methods, we can abstract the layer:
	MAC <-> MAC, there is no PHY attached. It seems to know as Fixed link, right?
	MAC+PHY <-> PHY+MAC
From your expression, you should use an external Ethernet switch, if yes, why Ethernet switch needs to use MDIO bus to access another MAC's(STMMAC) PHY?

> > > You said you copied the FEC driver. Take a look at that, it was
> > > initially broken in this way, and i needed to extend it when i got a
> > > board with an Ethernet switch attached to the FEC.
> >
> 
> > Could you point me how to implement clocks management to cover above
> > Ethernet switch case? Or can we upstream this first and then fix it
> > later for such case?
> 
> I actually got is wrong on the first attempt. So you need to look at:
> 
> 42ea4457ae net: fec: normalize return value of pm_runtime_get_sync() in
> MDIO write
> 14d2b7c1a9 net: fec: fix initial runtime PM refcount 8fff755e9f net: fec: Ensure
> clocks are enabled while using mdio bus
> 
> And no, you cannot fix it later, because your patches potentially break existing
> systems using an Ethernet switch. See:
> 
> ommit da29f2d84bd10234df570b7f07cbd0166e738230
> Author: Jose Abreu <Jose.Abreu@synopsys.com>
> Date:   Tue Jan 7 13:35:42 2020 +0100
> 
>     net: stmmac: Fixed link does not need MDIO Bus
> 
>     When using fixed link we don't need the MDIO bus support.
> 
> ...
>     Tested-by: Florian Fainelli <f.fainelli@gmail> # Lamobo R1 (fixed-link +
> MDIO sub node for roboswitch).
> 
> So there are boards which make use of a switch and MDIO. Florian might
> however be able to run tests for you, if you ask him.

Hi Florian,

I am curious about " fixed-link + MDIO sub node for roboswitch ", how does this implement?

Florian, Andrew, I will send a V2, it could still has defects. Welcome to review the patch, I don't expect it to break existing systems. Thanks.

Best Regards,
Joakim Zhang
> 
>    Andrew