netdev - RE: correct use of PHY_INTERFACE_MODE_RGMII{,_TXID,_RXID,

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 10 Jun 2020 09:10:23 +0000
From:   Ioana Ciornei <ioana.ciornei@....com>
To:     Helmut Grohne <helmut.grohne@...enta.de>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: RE: correct use of PHY_INTERFACE_MODE_RGMII{,_TXID,_RXID,_ID}


Hi Helmut,

> Subject: correct use of PHY_INTERFACE_MODE_RGMII{,_TXID,_RXID,_ID}
> 
> Hi,
> 
> I've been trying to write a dt for a board and got quite confused about the
> RGMII delays. That's why I looked into it and got even more confused by what I
> found. Different drivers handle this quite differently. Let me summarize.
> 
> Some drivers handle the RGMII modes individually. This is how I expected it to
> be. Examples:
> * renesas/ravb_main.c
> * stmicro/stmmac/dwmac-rk.c
> 
> A number of drivers handle all RGMII modes in uniformly. They don't actually
> configure any dealys. Is that supposed to work?  Examples:
> * apm/xgene/xgene_enet_main.c
> * aurora/nb8800.c
> * cadence/macb_main.c
> * freescale/fman/fman_memac.c
> * freescale/ucc_geth.c
> * ibm/emac/rgmii.c
> * renesas/sh_eth.c
> * socionext/sni_ave.c
> * stmicro/stmmac/dwmac-stm32.c
> 
> 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).

Ioana

> Another interesting one is cadence/macb_main.c. While it handles all the RGMII
> modes uniformly, the Zynq GEM hardware (supported by the driver) does not
> actually support adding any delays. The driver happily accepts these modes
> without telling the user that it really is using PHY_INTERFACE_MODE_RGMII_ID.
> Should the driver warn about or reject the other modes? Rejecting could break
> existing users. Some feedback (failure or warning) would be very useful
> however.
> 
> stmicro/stmmac/dwmac-sti.c has a #define IS_PHY_IF_MODE_RGMII, which
> seems to be a duplicate of phy_interface_mode_is_rgmii from <linux/phy.h>.
> Should that or phy_interface_is_rgmii be used instead?
> 
> Helmut