linux-kernel - Re: [PATCH net-next v2 3/3] net: phy: mscc: RGMII skew delay configuration

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200228162942.GF29979@lunn.ch>
Date:   Fri, 28 Feb 2020 17:29:42 +0100
From:   Andrew Lunn <andrew@...n.ch>
To:     Antoine Tenart <antoine.tenart@...tlin.com>
Cc:     davem@...emloft.net, f.fainelli@...il.com, hkallweit1@...il.com,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        foss@...il.net
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