On Tue, Aug 03, 2021 at 10:24:27PM +0530, Prasanna Vengateshan wrote:
> Thanks Vladimir & Andrew for the right pointers and info. The thread talks about
> "rgmii-*" are going to be applied by the PHY only as per the doc. For fixed-
> link, MAC needs to add the delay. This fixed-link can be No-PHY or MAC-MAC or
> MAC to in-accessible PHY. In such case, i am not convinced in using rgmii-tx-
> delay-ps & rgmii-rx-delay-ps on the MAC side and apply delay. I still think
> proposed code in earlier mail thread should still be okay.
Why? I genuinely do not understand your reasoning
- I read a thread that brings some arguments for which MACs should not
add delays based on the delay type in the "rgmii-*" phy-mode string
[ but based on explicit rgmii-tx-delay-ps and rgmii-rx-delay-ps
properties under the MAC OF node; this is written in the same
message as the quote that you chose ]
- I acknowledge that in certain configurations I need the MAC to apply
internal delays.
=> I disagree that I should parse the rgmii-tx-delay-ps and
rgmii-rx-delay-ps OF properties of the MAC, just apply RGMII delays
based on the "rgmii-*" phy-mode string value, when I am a DSA CPU
port and in no other circumstance
?!
I mean, feel free to feel convinced or not, but you have not actually
brought any argument to the table here, or I'm not seeing it.
Anyway, I don't believe that whatever you decide to do with the RGMII
delays is likely to be a decisive factor in whether the patches are
accepted or not, considering the fact that traditionally, everyone did
what suited their board best and that's about it; I will stop pushing back.
I have a theory that all the RGMII setups driven by the Linux PHY
library cannot all work at the same time, with the same code base.
Someone will sooner or later come and change a driver to make it do what
they need, which will break what the original author intended, which
will then be again patched, which will again break ..., which ....
If a perpetual motion device will ever be built by mankind, I am sure it
will be powered by RGMII delays.
On Wed, Aug 04, 2021 at 02:54:01AM +0300, Vladimir Oltean wrote:
> On Tue, Aug 03, 2021 at 10:24:27PM +0530, Prasanna Vengateshan wrote:
> > Thanks Vladimir & Andrew for the right pointers and info. The thread talks about
> > "rgmii-*" are going to be applied by the PHY only as per the doc. For fixed-
> > link, MAC needs to add the delay. This fixed-link can be No-PHY or MAC-MAC or
> > MAC to in-accessible PHY. In such case, i am not convinced in using rgmii-tx-
> > delay-ps & rgmii-rx-delay-ps on the MAC side and apply delay. I still think
> > proposed code in earlier mail thread should still be okay.
>
> Why? I genuinely do not understand your reasoning
>
> - I read a thread that brings some arguments for which MACs should not
> add delays based on the delay type in the "rgmii-*" phy-mode string
> [ but based on explicit rgmii-tx-delay-ps and rgmii-rx-delay-ps
> properties under the MAC OF node; this is written in the same
> message as the quote that you chose ]
>
> - I acknowledge that in certain configurations I need the MAC to apply
> internal delays.
>
> => I disagree that I should parse the rgmii-tx-delay-ps and
> rgmii-rx-delay-ps OF properties of the MAC, just apply RGMII delays
> based on the "rgmii-*" phy-mode string value, when I am a DSA CPU
> port and in no other circumstance
>
> ?!
>
> I mean, feel free to feel convinced or not, but you have not actually
> brought any argument to the table here, or I'm not seeing it.
>
> Anyway, I don't believe that whatever you decide to do with the RGMII
> delays is likely to be a decisive factor in whether the patches are
> accepted or not, considering the fact that traditionally, everyone did
> what suited their board best and that's about it; I will stop pushing back.
>
> I have a theory that all the RGMII setups driven by the Linux PHY
> library cannot all work at the same time, with the same code base.
> Someone will sooner or later come and change a driver to make it do what
> they need, which will break what the original author intended, which
> will then be again patched, which will again break ..., which ....
This is why we need to have a clear definition of what the various
RGMII interface types are, how and where they are applied, and make
sure everyone sticks to that. We have this documented in
Documentation/networking/phy.rst.
The RGMII interface modes _only_ determine how the PHY should be
configured - they do not determine how the MAC should be configured
(with /maybe/ the exception of PHY_INTERFACE_MODE_RGMII allowing the
MAC to insert "default delays".)
In the case of a fixed link, there is no "PHY" as such, but the PHY
interface mode describes the properties of the link from the MAC
perspective, since it is specified in the MAC node. So, if we have
e.g. PHY_INTERFACE_MODE_RGMII_TXID, then from the MAC perspective,
we expect the device on the other end of the fixed link to be adding
the transmit data line delay. Since we have a fixed link though, we
have no way to communicate that to the other side - but the delays do
need to be configured to conform with RGMII.
An interesting point here, however, is the mv88e6xxx DSA driver - it
appears to set the RGMII delays for _all_ ports based on what is in
the DT node for the port. So, even if you have a port operating in
RGMII with an external PHY, specifying a phy-mode of rgmii-txid results
in that being configured at the DSA end of the RGMII link. Andrew - we
may need to look at that since it doesn't conform to what we have in
the documentation...
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
On Wed, Aug 04, 2021 at 10:59:54AM +0100, Russell King (Oracle) wrote:
> This is why we need to have a clear definition of what the various
> RGMII interface types are, how and where they are applied, and make
> sure everyone sticks to that. We have this documented in
> Documentation/networking/phy.rst.
The problem is that I have no clear migration path for the drivers I
maintain, like sja1105, and I suspect that others might be in the exact
same situation.
Currently, if the sja1105 needs to add internal delays in a MAC-to-MAC
(fixed-link) setup, it does that based on the phy-mode string. So
"rgmii-id" + "fixed-link" means for sja1105 "add RX and TX RGMII
internal delays", even though the documentation now says "the MAC should
not add the RX or TX delays in this case".
There are 2 cases to think about, old driver with new DT blob and new
driver with old DT blob. If breakage is involved, I am not actually very
interested in doing the migration, because even though the interpretation
of the phy-mode string is inconsistent between the phy-handle and fixed-link
case (which was deliberate), at least it currently does all that I need it to.
I am not even clear what is the expected canonical behavior for a MAC
driver. It parses rx-internal-delay-ps and tx-internal-delay-ps, and
then what? It treats all "rgmii*" phy-mode strings identically? Or is it
an error to have "rgmii-rxid" for phy-mode and non-zero rx-internal-delay-ps?
If it is an error, should all MAC drivers check for it? And if it is an
error, does it not make migration even more difficult (adding an
rx-internal-delay-ps property to a MAC OF node which already uses
"rgmii-id" would be preferable to also having to change the "rgmii-id"
to "rgmii", because an old kernel might also need to work with that DT
blob, and that will ignore the new rx-internal-delay-ps property).
Does qca8k_setup_of_rgmii_delay(), a very recent function, even do the
right thing with rx-internal-delay-ps, or is it doing the exact opposite
of the right thing (it applies rx-internal-delay-ps when in rgmii-id or
rgmii-rxid mode).
> I am not even clear what is the expected canonical behavior for a MAC
> driver. It parses rx-internal-delay-ps and tx-internal-delay-ps, and
> then what?
So best practices are based around a MAC-PHY link. phy-mode is passed
to the PHY, and the MAC does not act upon it. MAC rx-internal-delay-ps
and tx-internal-delay-ps can be used to fine tune the link. You can
use them to add and sometimes subtract small amounts of delay.
> It treats all "rgmii*" phy-mode strings identically? Or is it an
> error to have "rgmii-rxid" for phy-mode and non-zero
> rx-internal-delay-ps?
I would say the first is correct, the second statement is false. You
should always be able to fine tune the link, independent of the PHY
mode.
We also have to consider the case when the PHY is not actually able to
implement the delay. It hopefully returns -EOPNOTSUPP for anything
other than "rgmii". You can then put the full 2ns delay into
tx-internal-delay-ps nd rx-internal-delay-ps.
And lastly there is one MAC driver which mostly ignores these best
practices because the vendor crap tree always did the delay in the
MAC. It correctly masks the phy-mode, so the PHY does not add delays.
For MAC-MAC and fixed link best practices are very fuzzily defined.
It is not something we have much of in the kernel. We might also want
to narrow the discussion down to MACs within a switch. MACs within in
NIC should probably follow the best practices for a MAC-PHY link, even
if it is actually a switch on the other end.
I also agree with Russell that mv88e6xxx is probably broken for a
MAC-PHY link. It is known to work for a Marvell DSA in MAC-MAC link,
we have boards doing that.
It seems like a switch MAC should parse rx-internal-delay-ps and
tx-internal-delay-ps and apply them independent of the phy-mode. That
keeps it consistent with MAC-PHY. And if there is a PHY connected,
pass the phy-mode on unmasked.
So that we don't break mv88e6xxx, for a CPU or DSA port, we probably
should continue to locally implement the delay, with the assumption
there is no PHY, it is a MAC-MAC link. We probably want to patch
mv88e6xxx to do nothing for user ports.
I suspect it is a 50/50 roll of a dice what rx and tx actually
mean. Is it from the perspective of the MAC or the PHY? Luckily,
rgmii-rxid and rgmii-txid don't appear in DT very often.
Andrew
On Sat, Aug 07, 2021 at 05:40:35PM +0200, Andrew Lunn wrote:
> I suspect it is a 50/50 roll of a dice what rx and tx actually
> mean. Is it from the perspective of the MAC or the PHY? Luckily,
> rgmii-rxid and rgmii-txid don't appear in DT very often.
I checked an NXP board schematic which has both, and:
When you connect a MAC to a PHY using RGMII, the RXD[3:0] pins of the
MAC connect to the RXD[3:0] pins of the PHY, and the TXD[3:0] goes to
TXD[3:0]. So it is neither the perspective of the MAC nor of the PHY.
When you connect a MAC to a MAC using RGMII, the RXD[3:0] of one MAC
goes to the TXD[3:0] of the other, and vice versa.
Nonetheless, a phy-mode of "rgmii-rxid" always means to the local MAC
that "the RX delays have been dealt with" - either by PCB traces, or a
remote MAC applying TX delay, or the PHY applying RX delay.