Message-ID: <20231031072847.GP3803936@pengutronix.de>
Date: Tue, 31 Oct 2023 08:28:47 +0100
From: Oleksij Rempel <o.rempel@...gutronix.de>
To: Andrew Lunn <andrew@...n.ch>
Cc: Vladimir Oltean <olteanv@...il.com>,
	Ante Knezic <ante.knezic@...mholz.de>, conor+dt@...nel.org,
	UNGLinuxDriver@...rochip.com, davem@...emloft.net,
	devicetree@...r.kernel.org, edumazet@...gle.com,
	f.fainelli@...il.com, krzysztof.kozlowski+dt@...aro.org,
	kuba@...nel.org, linux-kernel@...r.kernel.org, marex@...x.de,
	netdev@...r.kernel.org, pabeni@...hat.com, robh+dt@...nel.org,
	woojung.huh@...rochip.com
Subject: Re: [PATCH net-next v4 2/2] net:dsa:microchip: add property to select

On Tue, Oct 31, 2023 at 02:00:05AM +0100, Andrew Lunn wrote:
> > So, my opinion is that although what Oleksij would like to see is
> > admirable, I don't think that the REF_CLK direction is a matter of RMII
> > MAC vs PHY role, and thus, we wouldn't need to change "rmii" to "rev-rmii"
> > and cause breakage everywhere. It's just that - a matter of REF_CLK
> > direction. It's true, though, that this is a generic problem and that
> > the generic bindings for RMII that we currently have are under-specified.
> > We could try to devise an extended RMII binding which makes it clear for
> > both the MAC and the PHY who is responsible to drive this signal. You
> > are not attempting that, you are just coming up with yet another
> > vendor-specific MAC property which solves a generic problem. I can't say
> > I am completely opposed to that, either, which is why I haven't really
> > spoken out against it. The PHY maintainers would also have to weigh in,
> > and not all of them are CCed here.
> 
> I would recommend looking around other PHYs and find a property which
> does what you want, and copy it.
> 
> We do have all sorts of properties. There are some to enable the
> REF_CLK out of the PHY. Some to disable the REF_CLK out, some to
> disable it when the link is down, some to indicate what frequency it
> should tick at, etc.
> 
> If you want to go the extra mile, maybe you can make a summary of all
> these properties, and maybe we can produce a guide line for what we
> want the properties to be called going forward.
> 
> > I am afraid that creating a CCF style binding for REF_CLK will be an
> > enormous hammer for a very small nail and will see very limited adoption
> > to other drivers, but I might as well be wrong about it. Compatibility
> > between RMII MACs and PHYs which may or may not be CCF-ready might also
> > be a concern.
> 
> I also don't think using the CCF makes too much sense, except for
> where the SoC provides the lock, and already has a CCF covering it.
> 
> I would also be hesitant to add more dependencies between the MAC and
> the PHY. The DT often has circular dependencies and we have had issues
> with probing being deferred because the core does not always
> understand these circular dependencies.

Heh, this are unsolved problems making me pain in different projects.

Here are some real life examples, which are unsolved in one or another project
and till now didn't went mainline:

1. In scenarios where PHYs require an RMII clock from the MAC, initialization
becomes complex. This is often resolved through bootloader and kernel
modifications. Right now it kind of works and postponed until it will make
real pain :)

2. Complexity increases in designs with multiple PHYs used by different MACs
but connected to one MDIO bus. Same is here, there was already some
regressions but the pain is still not enough for making things right.

3. For some MACs like STMMAC, configuration is challenging without an external
clock from the PHY. For example, VLAN configuration isn't possible with EEE
enabled unless deep power saving states are disabled during register access.
If I remember it correctly, there was floating discussions and patches trying
to address similar issues.

Transferring these issues to KSZ8863, we might face difficulties configuring
STMMAC if KSZ8863, acting as the clock provider, isn't enabled early before MAC
driver probing, a tricky scenario in the DSA framework.

Working on deep sleep states for the KSZ switch driver, I find that dynamic
clock control, potentially offered by CCF, could be quite handy.

Please do not see this answer as a request to Ante for complex rework. It's
more of a red flag notifying that the clocking issue is still unsolved, and
someone (may be me), sooner or later, will have enough motivation to jump into
this wasp nest :)

Regards,
Oleksij
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Powered by blists - more mailing lists