Re: [PATCH v2 2/2] net: phy: realtek: Change TX-delay setting for RGMII modes only

[Serge Semin]

Hello folks

On Tue, Apr 30, 2019 at 10:44:00PM +0200, Martin Blumenstingl wrote:
> Hello Vladimir,
> 
> On Tue, Apr 30, 2019 at 12:37 AM Vladimir Oltean <olteanv@xxxxxxxxx> wrote:
> [...]
> > Moreover, RGMII *always* needs clock skew. As a fact, all delays
> > applied on RX and RX, by the PHY, MAC or traces, should always amount
> > to a logical "rgmii-id". There's nothing that needs to be described
> > about that. Everybody knows it.
> thank you for mentioning this - I didn't know about it. I thought that
> the delays have to be added in "some cases" only (without knowing the
> definition of "some cases").
> 
> > What Linux gets told through the phy-mode property for RGMII is where
> > there's extra stuff to do, and where there's nothing to do. There are
> > also unwritten rules about whose job it is to apply the clock skew
> > (MAC or PHY).That is 100% configuration and 0% description.
> the phy-mode property is documented here [0] and the rgmii modes have
> a short explanation about the delays.
> that said: the documentation currently ignores the fact that a PCB
> designer might have added a delay
> 
> > > Then in accordance with the phy-mode property value MAC and PHY drivers
> > > determine which way the MAC and PHY are connected to each other and how
> > > their settings are supposed to be customized to comply with it. This
> > > interpretation perfectly fits with the "DT is the hardware description"
> > > rule.
> > >
> >
> > Most of the phy-mode properties really mean nothing. I changed the
> > phy-mode from "sgmii" to "rgmii" on a PHY binding I had at hand and
> > nothing happened (traffic still runs normally). I think this behavior
> > is 100% within expectation.
> the PHY drivers I know of don't complain if the phy-mode is not supported.
> however, there are MAC drivers which complain in this case, see [1]
> for one example
> 
> 
> Martin
> 
> 
> [0] https://github.com/torvalds/linux/blob/bf3bd966dfd7d9582f50e9bd08b15922197cd277/Documentation/devicetree/bindings/net/ethernet.txt#L17
> [1] https://github.com/torvalds/linux/blob/bf3bd966dfd7d9582f50e9bd08b15922197cd277/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c#L187

So as far as I can conclude from the whole discussion, we don't really
want the realtek driver to return an error in case if any unsupported
mode is passed to the config_init method including the RGMII_RXID one,
which in some cases could be enabled via RXDLY pin strapping (like on
rtl8211F). The reason is quite justified: even though there is no
known way to alter the RX-delay on the rtl8211F PHY side, the delay
can be enabled via external pin. In this case rgmii-rxid mode can be
enabled in dts and should be considered a correct error-less PHY node
property.

In order to have this all summed up, I'll shortly send a v3 patchset with
alterations based on the discussion results. Maintainers then will be able
to post their comments in reply to it.

Cheers.
-Sergey