[PATCH 1/2] net: ravb: Add tx/rx delay flag checks and support for rgmii-rxid
Some boards like the Beacon RZ/G2 SOM use either flags for tx-internal-delay-ps, rx-internal-delay-ps or rgmii-rxid.
In Linux the APSR_RDM flag is set when either rx-internal-delay-ps is set or the mode is rgmii-rxid, and the APSR_TDM is set when tx-internal-delay-ps is found or rgmii-txid is set, and both are set if rgmii-id is set.
The ravb driver in U-Boot driver was missing rgmii-rxid support, so add that support in a similar fashion to what is done in Linux.
Signed-off-by: Adam Ford aford173@gmail.com
diff --git a/drivers/net/ravb.c b/drivers/net/ravb.c index 1d1118c341..9e09e998e3 100644 --- a/drivers/net/ravb.c +++ b/drivers/net/ravb.c @@ -52,6 +52,7 @@ #define CSR_OPS 0x0000000F #define CSR_OPS_CONFIG BIT(1)
+#define APSR_RDM BIT(13) #define APSR_TDM BIT(14)
#define TCCR_TSRQ0 BIT(0) @@ -376,6 +377,8 @@ static int ravb_dmac_init(struct udevice *dev) struct ravb_priv *eth = dev_get_priv(dev); struct eth_pdata *pdata = dev_get_plat(dev); int ret = 0; + int mode = 0; + unsigned int delay;
/* Set CONFIG mode */ ret = ravb_reset(dev); @@ -402,9 +405,25 @@ static int ravb_dmac_init(struct udevice *dev) (rmobile_get_cpu_type() == RMOBILE_CPU_TYPE_R8A77995)) return 0;
- if ((pdata->phy_interface == PHY_INTERFACE_MODE_RGMII_ID) || - (pdata->phy_interface == PHY_INTERFACE_MODE_RGMII_TXID)) - writel(APSR_TDM, eth->iobase + RAVB_REG_APSR); + if (!dev_read_u32(dev, "tx-internal-delay-ps", &delay)) { + if (delay) + mode |= APSR_TDM; + } + + if (!dev_read_u32(dev, "rx-internal-delay-ps", &delay)) { + if (delay) + mode |= APSR_RDM; + } + + if (pdata->phy_interface == PHY_INTERFACE_MODE_RGMII_ID || + pdata->phy_interface == PHY_INTERFACE_MODE_RGMII_RXID) + mode |= APSR_RDM; + + if (pdata->phy_interface == PHY_INTERFACE_MODE_RGMII_ID || + pdata->phy_interface == PHY_INTERFACE_MODE_RGMII_TXID) + mode |= APSR_TDM; + + writel(mode, eth->iobase + RAVB_REG_APSR);
return 0; }
The wrong phy was being enabled, because it worked and the proper PHY did not. After the Renesas maintainer made some adjustments to the device tree, Linux was able to use the proper driver, and when that device tree was ported to Linux, the ethernet stopped working due to the lack of rgmii-rxid support. Now that rgmii-rxid is supported, enable the proper driver to restore ethernet function.
Fixes: 1eaf61c84db6 ("arm: dts: beacon-rzg2: Resync device trees with Linux 5.16-rc3") Signed-off-by: Adam Ford aford173@gmail.com
diff --git a/configs/rzg2_beacon_defconfig b/configs/rzg2_beacon_defconfig index e6a0d68962..91b3fa2948 100644 --- a/configs/rzg2_beacon_defconfig +++ b/configs/rzg2_beacon_defconfig @@ -62,7 +62,7 @@ CONFIG_DM_MTD=y CONFIG_DM_SPI_FLASH=y CONFIG_SPI_FLASH_WINBOND=y CONFIG_BITBANGMII=y -CONFIG_PHY_REALTEK=y +CONFIG_PHY_ATHEROS=y CONFIG_DM_ETH=y CONFIG_RENESAS_RAVB=y CONFIG_DM_REGULATOR=y
On 2/20/22 22:45, Adam Ford wrote:
Hi,
[...]
@@ -376,6 +377,8 @@ static int ravb_dmac_init(struct udevice *dev) struct ravb_priv *eth = dev_get_priv(dev); struct eth_pdata *pdata = dev_get_plat(dev); int ret = 0;
int mode = 0;
unsigned int delay;
/* Set CONFIG mode */ ret = ravb_reset(dev);
@@ -402,9 +405,25 @@ static int ravb_dmac_init(struct udevice *dev) (rmobile_get_cpu_type() == RMOBILE_CPU_TYPE_R8A77995)) return 0;
- if ((pdata->phy_interface == PHY_INTERFACE_MODE_RGMII_ID) ||
(pdata->phy_interface == PHY_INTERFACE_MODE_RGMII_TXID))writel(APSR_TDM, eth->iobase + RAVB_REG_APSR);
- if (!dev_read_u32(dev, "tx-internal-delay-ps", &delay)) {
if (delay)mode |= APSR_TDM;- }
- if (!dev_read_u32(dev, "rx-internal-delay-ps", &delay)) {
if (delay)mode |= APSR_RDM;- }
Are these two conditionals above really needed ?
Isn't it enough to set the PHY mode to rgmii-NNN like it is checked below ?
if (pdata->phy_interface == PHY_INTERFACE_MODE_RGMII_ID ||
pdata->phy_interface == PHY_INTERFACE_MODE_RGMII_RXID)mode |= APSR_RDM;if (pdata->phy_interface == PHY_INTERFACE_MODE_RGMII_ID ||
pdata->phy_interface == PHY_INTERFACE_MODE_RGMII_TXID)mode |= APSR_TDM;writel(mode, eth->iobase + RAVB_REG_APSR);
return 0; }
[...]
On Sun, Feb 20, 2022 at 5:58 PM Marek Vasut marex@denx.de wrote:
On 2/20/22 22:45, Adam Ford wrote:
Hi,
[...]
@@ -376,6 +377,8 @@ static int ravb_dmac_init(struct udevice *dev) struct ravb_priv *eth = dev_get_priv(dev); struct eth_pdata *pdata = dev_get_plat(dev); int ret = 0;
int mode = 0;unsigned int delay; /* Set CONFIG mode */ ret = ravb_reset(dev);@@ -402,9 +405,25 @@ static int ravb_dmac_init(struct udevice *dev) (rmobile_get_cpu_type() == RMOBILE_CPU_TYPE_R8A77995)) return 0;
if ((pdata->phy_interface == PHY_INTERFACE_MODE_RGMII_ID) ||(pdata->phy_interface == PHY_INTERFACE_MODE_RGMII_TXID))writel(APSR_TDM, eth->iobase + RAVB_REG_APSR);
if (!dev_read_u32(dev, "tx-internal-delay-ps", &delay)) {if (delay)mode |= APSR_TDM;}if (!dev_read_u32(dev, "rx-internal-delay-ps", &delay)) {if (delay)mode |= APSR_RDM;}Are these two conditionals above really needed ?
These two conditionals are present in the Linux kernel version [1], so I assumed there is a reason and copied that.
The way Geert reconfigured the ethernet for the Beacon board [2][3], he changed the mode rgmii-rxid and it already had both tx and rx delays. The rgmii-rxid flag by itself implies the APSR_RDM flag is set, but adding the tx-internal-delay-ps appears to add the APSR_TDM. I am not that familiar with phys to know the main differences between this configuration and the PHY_INTERFACE_MODE_RGMII_ID which appears to imply both. If you want, I could merge the checks so the delays are combined with the phy_interface type that follows.
[1] - https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/driv... [2] - https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/ar... [3] - https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/ar...
adam
Isn't it enough to set the PHY mode to rgmii-NNN like it is checked below ?
if (pdata->phy_interface == PHY_INTERFACE_MODE_RGMII_ID ||pdata->phy_interface == PHY_INTERFACE_MODE_RGMII_RXID)mode |= APSR_RDM;if (pdata->phy_interface == PHY_INTERFACE_MODE_RGMII_ID ||pdata->phy_interface == PHY_INTERFACE_MODE_RGMII_TXID)mode |= APSR_TDM;writel(mode, eth->iobase + RAVB_REG_APSR); return 0;}
[...]
On 2/21/22 02:45, Adam Ford wrote:
On Sun, Feb 20, 2022 at 5:58 PM Marek Vasut marex@denx.de wrote:
On 2/20/22 22:45, Adam Ford wrote:
Hi,
[...]
@@ -376,6 +377,8 @@ static int ravb_dmac_init(struct udevice *dev) struct ravb_priv *eth = dev_get_priv(dev); struct eth_pdata *pdata = dev_get_plat(dev); int ret = 0;
int mode = 0;unsigned int delay; /* Set CONFIG mode */ ret = ravb_reset(dev);@@ -402,9 +405,25 @@ static int ravb_dmac_init(struct udevice *dev) (rmobile_get_cpu_type() == RMOBILE_CPU_TYPE_R8A77995)) return 0;
if ((pdata->phy_interface == PHY_INTERFACE_MODE_RGMII_ID) ||(pdata->phy_interface == PHY_INTERFACE_MODE_RGMII_TXID))writel(APSR_TDM, eth->iobase + RAVB_REG_APSR);
if (!dev_read_u32(dev, "tx-internal-delay-ps", &delay)) {if (delay)mode |= APSR_TDM;}if (!dev_read_u32(dev, "rx-internal-delay-ps", &delay)) {if (delay)mode |= APSR_RDM;}Are these two conditionals above really needed ?
These two conditionals are present in the Linux kernel version [1], so I assumed there is a reason and copied that.
The way Geert reconfigured the ethernet for the Beacon board [2][3], he changed the mode rgmii-rxid and it already had both tx and rx delays. The rgmii-rxid flag by itself implies the APSR_RDM flag is set, but adding the tx-internal-delay-ps appears to add the APSR_TDM. I am not that familiar with phys to know the main differences between this configuration and the PHY_INTERFACE_MODE_RGMII_ID which appears to imply both. If you want, I could merge the checks so the delays are combined with the phy_interface type that follows.
The kernel driver has comments there which clarify this:
drivers/net/ethernet/renesas/ravb_main.c ravb_parse_delay_mode()
if (!of_property_read_u32(np, "rx-internal-delay-ps", &delay)) { /* Valid values are 0 and 1800, according to DT bindings */
if (!of_property_read_u32(np, "tx-internal-delay-ps", &delay)) { /* Valid values are 0 and 2000, according to DT bindings */
So these internal-delay-ps have only two states, 0 or some-delay. Retain the comments, otherwise it is real inobvious why there are two identical ways to set the delays.
btw. the kernel driver also has this $explicit_delay check and if the internal-delay-ps is present , it skips the phy-mode PH_INTERFACE_MODE_ check, that's missing in this patch.
participants (2)
-
Adam Ford -
Marek Vasut