[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <20200610081236.GA31659@laureti-dev>
Date: Wed, 10 Jun 2020 10:12:37 +0200
From: Helmut Grohne <helmut.grohne@...enta.de>
To: <netdev@...r.kernel.org>
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?
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
Powered by blists - more mailing lists