Hi Niklas, Andrew, On Thu, May 9, 2024 at 11:10 PM Niklas Söderlund <niklas.soderlund+renesas@xxxxxxxxxxxx> wrote: > Add initial support for Renesas Ethernet-TSN End-station device of R-Car > V4H. The Ethernet End-station can connect to an Ethernet network using a > 10 Mbps, 100 Mbps, or 1 Gbps full-duplex link via MII/GMII/RMII/RGMII. > Depending on the connected PHY. > > The driver supports Rx checksum and offload and hardware timestamps. > > While full power management and suspend/resume is not yet supported the > driver enables runtime PM in order to enable the module clock. While > explicit clock management using clk_enable() would suffice for the > supported SoC, the module could be reused on SoCs where the module is > part of a power domain. > > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@xxxxxxxxxxxx> > --- > * Changes since v4 > - Enable GPOUT_RDM and GPOUT_TDM delays depending on phy-mode. Thanks for the update! > +static void rtsn_set_delay_mode(struct rtsn_private *priv) > +{ > + u32 val = 0; > + > + /* The MAC is capable of applying a delay on both Rx and Tx. Each > + * delay can either be on or off, there is no way to set its length. > + * > + * The exact delay applied depends on electric characteristics of the > + * board. The datasheet describes a typical Rx delay of 1800 ps and a > + * typical Tx delay of 2000 ps. > + * > + * There are boards where the RTSN device is used together with PHYs > + * who do not support a large enough internal delays to function. These > + * boards depends on the MAC applying these inexact delays. > + */ > + > + /* If the phy-mode is rgmii or rgmii-rxid apply Rx delay on the MAC */ > + if (priv->iface == PHY_INTERFACE_MODE_RGMII_ID || > + priv->iface == PHY_INTERFACE_MODE_RGMII_RXID) > + val |= GPOUT_RDM; > + > + /* If the phy-mode is rgmii or rgmii-txid apply Tx delay on the MAC */ > + if (priv->iface == PHY_INTERFACE_MODE_RGMII_ID || > + priv->iface == PHY_INTERFACE_MODE_RGMII_TXID) > + val |= GPOUT_TDM; > + > + rtsn_write(priv, GPOUT, val); > +} > +static int rtsn_phy_init(struct rtsn_private *priv) > +{ > + struct device_node *np = priv->ndev->dev.parent->of_node; > + struct phy_device *phydev; > + struct device_node *phy; > + phy_interface_t iface; > + > + /* Delays, if any, are applied by the MAC. Mask RGMII mode passed to the > + * PHY to avoid it also adding the delay. > + */ > + switch (priv->iface) { > + case PHY_INTERFACE_MODE_RGMII: > + case PHY_INTERFACE_MODE_RGMII_ID: > + case PHY_INTERFACE_MODE_RGMII_RXID: > + case PHY_INTERFACE_MODE_RGMII_TXID: > + iface = PHY_INTERFACE_MODE_RGMII; > + break; > + default: > + iface = priv->iface; > + break; > + } This introduces the same issues (the "workaround" state below) we had with ravb before. 9b23203c32ee02cd ("ravb: Mask PHY mode to avoid inserting delays twice") was the workaround, a6f51f2efa742df0 ("ravb: Add support for explicit internal clock delay configuration") was the final fix. Do we really want to repeat that mistake? Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds