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;}
[...]