On Fri, Oct 27, 2023 at 08:37:43AM +0200, Ante Knezic wrote:
> On Tue, 24 Oct 2023 16:24:26 +0200, Oleksij Rampel wrote:
>
> > > That is correct, I guess its a matter of nomenclature, but how do you
> > > "tell" the switch whether it has REFCLKI routed externally or not if not by
> > > setting the 0xC6 bit 3? Is there another way to achieve this?
> >
> > I do not see any other way to "tell" it. The only thing to change in you
> > patches is a different way to tell it to the kernel.
> > Instead of introducing a new devicetree property, you need to reuse
> > phy-mode property.
>
> > ...
>
> > Since phy-mode for RMII was never set correctly, it will most probably
> > break every single devicetree using KSZ switches. It is the price of fixing
> > things :/
>
> To Vladimir Oltean: What are your thoughts on this?
>
Sorry for the delayed response.
It's complicated. In the Google-searchable RMII spec, the REF_CLK is an
input from the perspective of the PHY, and an input or output from the
perspective of the MAC.
Additionally, some RMII PHYs like NXP TJA1110 provide "extensions" to
the RMII spec, where the PHY itself can provide the REF_CLK based on an
internal 25 MHz crystal (see "nxp,rmii-refclk-in").
My understanding is that some other PHYs, like KSZ8061RNB, go even
further and make the REF_CLK be always an output from the PHY's
perspective (this is not configurable).
It is noteworthy that on Wikipedia, it says directly that REF_CLK can
also be driven from the PHY to the MAC, which itself represents an
extension to what the spec says.
Additionally, some MACs like the NXP SJA1105 switch ports are not as
flexible as the standard would suggest. In RMII MAC mode, this switch
always wants to drive the REF_CLK pin itself. And in RMII PHY
(self-called REV-MII) mode, the SJA1105 always wants the REF_CLK to be
driven externally (from the PHY or external oscillator). So, for example
the SJA1105 in RMII MAC mode cannot be connected to the KSZ8061RNB, as
both would attempt to drive the REF_CLK signal.
In addition to all of that, the MAC/PHY roles are not just about the
direction of the REF_CLK, but also about the /J/ /K/ codewords that are
placed by the PHY in the inter packet gap on RXD[1:0]. A MAC doesn't do
this, and if it did, the PHY wouldn't expect it, and AFAIK, would
blindly propagate those code words onto the BASE-T wire, which is
undesirable.
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.
Now as to Conor's previous question about describing the REF_CLK as a
CCF clock, which is a bit related, honestly I haven't really analyzed
the merits of doing that. We know from the RMII standard that has a
known expected frequency of 50 MHz, it's just a matter of who provides
it.
Just one small thing to note is that I have heard of RMII PHYs which,
when REF_CLK is an input from their perspective, don't even have their
registers accessible until that REF_CLK is turned on. This may be
problematic, because the attached MAC driver may also own the MDIO bus
on which said PHY is located, and it practically needs to turn on the
RMII REF_CLK before the PHY on the MDIO bus could even be probed.
Currently, IIUC, the way in which that works is that RMII MAC drivers
are simply coded up to do just that, and I guess that worked so far.
With CCF, I guess you would describe this by making the MAC be a
"clocks" provider for the PHY. But it could also be the other way
around, and in that case, the MAC would be the parent of the PHY, but
it would also depend on a component that the PHY provides. I hope
fw_devlink doesn't mind reverse dependencies like that...
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.
> 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.
Andrew
On Mon, Oct 30, 2023 at 07:42:25PM +0200, Vladimir Oltean wrote:
> On Fri, Oct 27, 2023 at 08:37:43AM +0200, Ante Knezic wrote:
> > On Tue, 24 Oct 2023 16:24:26 +0200, Oleksij Rampel wrote:
> >
> > > > That is correct, I guess its a matter of nomenclature, but how do you
> > > > "tell" the switch whether it has REFCLKI routed externally or not if not by
> > > > setting the 0xC6 bit 3? Is there another way to achieve this?
> > >
> > > I do not see any other way to "tell" it. The only thing to change in you
> > > patches is a different way to tell it to the kernel.
> > > Instead of introducing a new devicetree property, you need to reuse
> > > phy-mode property.
> >
> > > ...
> >
> > > Since phy-mode for RMII was never set correctly, it will most probably
> > > break every single devicetree using KSZ switches. It is the price of fixing
> > > things :/
> >
> > To Vladimir Oltean: What are your thoughts on this?
> >
>
> In addition to all of that, the MAC/PHY roles are not just about the
> direction of the REF_CLK, but also about the /J/ /K/ codewords that are
> placed by the PHY in the inter packet gap on RXD[1:0]. A MAC doesn't do
> this, and if it did, the PHY wouldn't expect it, and AFAIK, would
> blindly propagate those code words onto the BASE-T wire, which is
> undesirable.
Interesting detail. I didn't knew it, it would be good to document
it somewhere near to revrmii binding :)
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 |
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 |
On Tue, Oct 31, 2023 at 08:28:47AM +0100, Oleksij Rempel wrote:
> 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.
So for each driver, there are 2 components to using CCF for MAC/PHY
interface clocks, one is providing what you have, and the other is
requesting what you need.
To avoid circular dependencies, we should make the clock provider per
DSA port be independent of the DSA driver itself. That is because, as
you point out in your example, the conduit interface (stmmac) may depend
on a clock provided by the switch, but the switch also depends on the
conduit being fully probed.
Stronger separation / more granular dependencies was one reason why I
wanted for more DSA drivers to follow Colin Foster's suit, but I stopped
working on that too:
https://lore.kernel.org/lkml/20221222134844.lbzyx5hz7z5n763n@skbuf/
but in the case of interface clocks, the separation is not so clear.
I would expect that for most if not all switches, the interface clock is
implicitly provided by enabling and configuring the respective MAC, the
same MAC that is intrinsically controlled through phylink by the main
DSA (switching IP) driver. So we have 2 APIs for controlling the same
piece of hardware, I'm not sure how conflicting requests are supposed to
be resolved.
This piece of the puzzle is quite complicated to fit into the larger
architecture in a coherent way, although I'm not arguing that there will
also be benefits.