Re: correct use of PHY_INTERFACE_MODE_RGMII{,_TXID,_RXID,_ID}

[Helmut Grohne <helmut.grohne@intenta.de>]

Hi Ioana,

On Wed, Jun 10, 2020 at 11:10:23AM +0200, Ioana Ciornei wrote:
> > freescale/dpaa2/dpaa2-mac.c is interesting. It checks whether any rgmii mode
> > other than PHY_INTERFACE_MODE_RGMII is used and complains that delays are
> > not supported in that case. The above comment says that the MAC does not
> > support adding delays. It seems that in that case, the only working mode should
> > be PHY_INTERFACE_MODE_RGMII_ID rather than
> > PHY_INTERFACE_MODE_RGMII. Is the code mixed up or my understanding?
> 
> The snippet that you are referring to is copied below for quick reference:
> 
> /* The MAC does not have the capability to add RGMII delays so
>  * error out if the interface mode requests them and there is no PHY
>  * to act upon them
>  */
> if (of_phy_is_fixed_link(dpmac_node) &&
>     (mac->if_mode == PHY_INTERFACE_MODE_RGMII_ID ||
>      mac->if_mode == PHY_INTERFACE_MODE_RGMII_RXID ||
>      mac->if_mode == PHY_INTERFACE_MODE_RGMII_TXID)) {
> 	netdev_err(net_dev, "RGMII delay not supported\n");
> 
> The important part which you seem to be missing is that a functional RGMII link can
> have the delays inserted by the PHY, the MAC or by PCB traces (in this exact order
> of preference). Here we check for any RGMII interface mode that
> requests delays to be added when the interface is in fixed link mode
> (using of_phy_is_fixed_link()), thus there is no PHY to act upon them.
> This restriction, as the comment says, comes from the fact that the MAC
> is not able to add RGMII delays.
> 
> When we are dealing with a fixed link, the only solution for DPAA2 is to use plain
> PHY_INTERFACE_MODE_RGMII and to hope that somebody external to this Linux
> system made sure that the delays have been inserted (be those PCB delays, or
> internal delays added by the link partner).

If I am reading this correctly, you are saying that the DPAA2 driver is
operating as a PHY, not as a MAC here. Is that correct?

This distinction is a bit difficult (in particular for fixed links)
since RGMII is symmetric, but it is important for understanding the
definitions from
Documentation/devicetree/bindings/net/ethernet-controller.yaml:

|       # RX and TX delays are added by the MAC when required
|       - rgmii
| 
|       # RGMII with internal RX and TX delays provided by the PHY,
|       # the MAC should not add the RX or TX delays in this case
|       - rgmii-id
| 
|       # RGMII with internal RX delay provided by the PHY, the MAC
|       # should not add an RX delay in this case
|       - rgmii-rxid
| 
|       # RGMII with internal TX delay provided by the PHY, the MAC
|       # should not add an TX delay in this case
|       - rgmii-txid

These are turned into the matching PHY_INTERFACE_MODE_* by the OF code.

My understanding is that the delays are always described as seen by the
PHY. When it says that an "internal delay" (id) is present, the delay is
internal to the PHY, not the MAC. So unless DPAA2 is operating as a PHY,
it still seems reversed to me.

If we think of DPAA2 as a MAC (which seems more natural to me), it
should only allow rgmii-id, becaue it does not support adding delays
according to the comment.

Helmut