I.MX6: Fix Ethernet PHY mode on Ventana boards

Gateworks Ventana boards seem to need "RGMII-ID" (internal delay) PHY mode, instead of simple "RGMII", for their Marvell 88E1510 transceiver. Otherwise, the Ethernet MAC doesn't work with Marvell PHY driver (TX doesn't seem to work correctly).

Tested on GW5400 rev. C.

This bug affects ARM Fedora 23.

Signed-off-by Krzysztof Hałasa <[email protected]>

Gateworks Ventana boards seem to need "RGMII-ID" (internal delay) PHY mode, instead of simple "RGMII", for their Marvell 88E1510 transceiver. Otherwise, the Ethernet MAC doesn't work with Marvell PHY driver (TX doesn't seem to work correctly).

Tested on GW5400 rev. C.

This bug affects ARM Fedora 23.

Signed-off-by Krzysztof Hałasa <[email protected]>

arch/arm/boot/dts/imx6q-gw5400-a.dts +1 -1

index 822ffb2..6c168dc 100644
@@ -154,7 +154,7 @@
 &fec {
        pinctrl-names = "default";
        pinctrl-0 = <&pinctrl_enet>;
-       phy-mode = "rgmii";
+       phy-mode = "rgmii-id";
        phy-reset-gpios = <&gpio1 30 GPIO_ACTIVE_HIGH>;
        status = "okay";
 };

arch/arm/boot/dts/imx6qdl-gw51xx.dtsi +1 -1

index f2867c4..90496aa 100644
@@ -94,7 +94,7 @@
 &fec {
        pinctrl-names = "default";
        pinctrl-0 = <&pinctrl_enet>;
-       phy-mode = "rgmii";
+       phy-mode = "rgmii-id";
        phy-reset-gpios = <&gpio1 30 GPIO_ACTIVE_LOW>;
        status = "okay";
 };

arch/arm/boot/dts/imx6qdl-gw52xx.dtsi +1 -1

index 4493f6e..0a6730b 100644
@@ -154,7 +154,7 @@
 &fec {
        pinctrl-names = "default";
        pinctrl-0 = <&pinctrl_enet>;
-       phy-mode = "rgmii";
+       phy-mode = "rgmii-id";
        phy-reset-gpios = <&gpio1 30 GPIO_ACTIVE_LOW>;
        status = "okay";
 };

arch/arm/boot/dts/imx6qdl-gw53xx.dtsi +1 -1

index cfad214..2c549ad 100644
@@ -174,7 +174,7 @@
 &fec {
        pinctrl-names = "default";
        pinctrl-0 = <&pinctrl_enet>;
-       phy-mode = "rgmii";
+       phy-mode = "rgmii-id";
        phy-reset-gpios = <&gpio1 30 GPIO_ACTIVE_LOW>;
        status = "okay";
 };

arch/arm/boot/dts/imx6qdl-gw54xx.dtsi +1 -2

index 535b536..b4ea087 100644
@@ -164,7 +164,7 @@
 &fec {
        pinctrl-names = "default";
        pinctrl-0 = <&pinctrl_enet>;
-       phy-mode = "rgmii";
+       phy-mode = "rgmii-id";
        phy-reset-gpios = <&gpio1 30 GPIO_ACTIVE_LOW>;
        status = "okay";
 };
 
--

Krzysztof Halasa

Industrial Research Institute for Automation and Measurements PIAP Al. Jerozolimskie 202, 02-486 Warsaw, Poland

It sounds like your saying this controls whether the phy is in charge of delay vs the MAC. I have never needed to set this and haven't found where its actually used (in at least 4.3). Is this caused by something new in the kernel I haven't seen yet or is it possible you have board that has an Ethernet issue?

Quoted 5 lines · Tim Harvey

It sounds like your saying this controls whether the phy is in charge of delay vs the MAC. I have never needed to set this and haven't found where its actually used (in at least 4.3). Is this caused by something new in the kernel I haven't seen yet or is it possible you have board that has an Ethernet issue?

I think you aren't using the Marvell PHY driver :-) The existing DTS files work fine with the "default" PHY driver. It's only the Marvell driver (drivers/net/phy/marvell.c, aka CONFIG_MARVELL_PHY) which have the issue.

The problem is caused by the following code in m88e1121_config_aneg():

        if (phy_interface_is_rgmii(phydev)) {

                mscr = phy_read(phydev, MII_88E1121_PHY_MSCR_REG) &
                        MII_88E1121_PHY_MSCR_DELAY_MASK;

                if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID)
                        mscr |= (MII_88E1121_PHY_MSCR_RX_DELAY |
                                 MII_88E1121_PHY_MSCR_TX_DELAY);
                else if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID)
                        mscr |= MII_88E1121_PHY_MSCR_RX_DELAY;
                else if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID)
                        mscr |= MII_88E1121_PHY_MSCR_TX_DELAY;

                err = phy_write(phydev, MII_88E1121_PHY_MSCR_REG, mscr);
                if (err < 0)
                        return err;
        }

mscr initially holds the correct value (one with RX_DELAY and TX_DELAY bits set). Since the _*ID are not specified, these bits are reset and (now incorrect) value is written back to the register.
OTOH the generic PHY driver knows nothing about the MSCR and thus doesn't "corrupt" it.

The whole -ID thing is about signal trace lengths (on PCB), where the non-ID setup requires special, longer traces for certain signals to compensate for propagation times etc. The -ID version does it internally (when enabled) and the PCB layout can be a bit simpler.

To be honest I only checked it on a single GW5400 rev. C, but I'd expect it to be exactly the same across all GW5[234]*. Can test on GW5[23] if there is any doubt (in few days I guess).

This may be a stupid question but what does the Marvell phy driver offer that the generic phy driver does not?

Gateworks Ventana boards seem to need "RGMII-ID" (internal delay) PHY mode, instead of simple "RGMII", for their Marvell 88E1510 transceiver. Otherwise, the Ethernet MAC doesn't work with Marvell PHY driver (TX doesn't seem to work correctly).

Tested on GW5400 rev. C.

This bug affects ARM Fedora 23.

Signed-off-by Krzysztof Hałasa <[email protected]>

Since this only touched DT files in the ARM platform code, maybe the ARM tree is the best path for this patch rather than mine?

Quoted 2 lines · David Miller

Since this only touched DT files in the ARM platform code, maybe the ARM tree is the best path for this patch rather than mine?

I think so.

Tim, would you like to handle this patch yourself, or should I send it to the ARM contacts?