On Tue, Dec 8, 2015 at 6:13 AM, Krzysztof Hałasa <khal...@piap.pl> wrote: > Tim Harvey <thar...@gateworks.com> writes: > >> 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. >
Kryzsztof, True - I have not used CONFIG_MARVELL_PHY and didn't know of its existance. > 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. right - I understand what the delay lines are, it just struck me as strange that I've never needed this. > > 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). > no doubt now. I re-created your config and see the problem as well. The marvell phy driver (drivers/net/phy/marvell.c) has a very lacking Kconfig stating its a PHY driver for the 88E1011S when in fact it has id's for 12 ID's including the 88E1510 we use on the GW51xx, GW52xx, GW53xx, GW54xx. This may be a stupid question but what does the Marvell phy driver offer that the generic phy driver does not? Thanks for finding this and submitting a patch! Acked-by: Tim Harvey <thar...@gateworks.com> Tim -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html