Messages in this thread | | | Date | Fri, 28 Feb 2020 17:29:42 +0100 | From | Andrew Lunn <> | Subject | Re: [PATCH net-next v2 3/3] net: phy: mscc: RGMII skew delay configuration |
| |
> +static void vsc8584_rgmii_set_skews(struct phy_device *phydev) > +{ > + u32 skew_rx, skew_tx; > + struct device *dev = &phydev->mdio.dev; > + > + /* We first set the Rx and Tx skews to their default value in h/w > + * (0.2 ns). > + */ > + skew_rx = VSC8584_RGMII_SKEW_0_2; > + skew_tx = VSC8584_RGMII_SKEW_0_2; > + > + /* Based on the interface mode, we then retrieve (if available) Rx > + * and/or Tx skews from the device tree. We do not fail if the > + * properties do not exist, the default skew configuration is a valid > + * one. > + */ > + if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID || > + phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID) > + of_property_read_u32(dev->of_node, "vsc8584,rgmii-skew-rx", > + &skew_rx); > + if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID || > + phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID) > + of_property_read_u32(dev->of_node, "vsc8584,rgmii-skew-tx", > + &skew_tx);
Hi Antoine
That is not the correct meaning of PHY_INTERFACE_MODE_RGMII_ID etc. PHY_INTERFACE_MODE_RGMII_ID means add a 2ns delay on both RX and TX. PHY_INTERFACE_MODE_RGMII_RXID means add a 2ns delay on the RX. PHY_INTERFACE_MODE_RGMII means 0 delay, with the assumption that either the MAC is adding the delay, or the PCB is designed with extra copper tracks to add the delay.
You only need "vsc8584,rgmii-skew-rx" when 2ns is not correct for you board, and you need more fine grain control.
What is not clearly defined, is how you combine PHY_INTERFACE_MODE_RGMII* with DT properties. I guess i would enforce that phydev->interface is PHY_INTERFACE_MODE_RGMII and then the delays in DT are absolute values.
There is also some discussion about what should go in DT. #defines like you have, or time in pS, and the driver converts to register values. I generally push towards pS.
Andrew
| |