On Wed, Nov 23, 2016 at 9:12 AM, Mario Six <mario....@gdsys.cc> wrote: > From: Dirk Eibach <dirk.eib...@gdsys.cc> > > Add support for Marvell 88E1680 Integrated Octal > 10/100/1000 Mbps Energy Efficient Ethernet Transceiver. > > Signed-off-by: Dirk Eibach <dirk.eib...@gdsys.cc> > --- > drivers/net/phy/marvell.c | 51 > +++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 51 insertions(+) > > diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c > index 4eeb0f6..72f3f92 100644 > --- a/drivers/net/phy/marvell.c > +++ b/drivers/net/phy/marvell.c > @@ -480,6 +480,46 @@ static int m88e1310_config(struct phy_device *phydev) > return genphy_config_aneg(phydev); > } > > +static int m88e1680_config(struct phy_device *phydev) > +{ > + /* > + * As per Marvell Release Notes - Alaska V 88E1680 Rev A2 > + * Errata Section 4.1 > + */ > + u16 reg; > + > + /* Matrix LED mode (not neede if single LED mode is used */ > + phy_write(phydev, MDIO_DEVAD_NONE, 22, 0x0004);
I realize the 88e151* driver went in without my ack (35fa0dda: "net: phy: marvell: add errata w/a for 88E151* chips"), and is loaded with magic numbers, but let's not proliferate the problem. Please define register offsets or use already-defined register offsets. If reasonable, use defined field values to build values from defines and something like bitfield_replace() from bitfield.h or clrsetbits_le32() from asm/io.h. When it is a constant that represents an encoded physical value that will never be used elsewhere, it's ok to just keep the hard-coded number in the write, but it should be preceeded with a comment that describes the actual meaning in engineering units and prefereably the equation used to come up with the constant. If you have the information to improve the 151* implementation as well, that would be very welcome. > + reg = phy_read(phydev, MDIO_DEVAD_NONE, 27); > + reg |= (1 << 5); > + phy_write(phydev, MDIO_DEVAD_NONE, 27, reg); > + > + /* QSGMII TX amplitude change */ > + phy_write(phydev, MDIO_DEVAD_NONE, 22, 0x00fd); > + phy_write(phydev, MDIO_DEVAD_NONE, 8, 0x0b53); > + phy_write(phydev, MDIO_DEVAD_NONE, 7, 0x200d); > + phy_write(phydev, MDIO_DEVAD_NONE, 22, 0x0000); > + > + /* EEE initialization */ > + phy_write(phydev, MDIO_DEVAD_NONE, 22, 0x00ff); > + phy_write(phydev, MDIO_DEVAD_NONE, 17, 0xb030); > + phy_write(phydev, MDIO_DEVAD_NONE, 16, 0x215c); > + phy_write(phydev, MDIO_DEVAD_NONE, 22, 0x00fc); > + phy_write(phydev, MDIO_DEVAD_NONE, 24, 0x888c); > + phy_write(phydev, MDIO_DEVAD_NONE, 25, 0x888c); > + phy_write(phydev, MDIO_DEVAD_NONE, 22, 0x0000); > + phy_write(phydev, MDIO_DEVAD_NONE, 0, 0x9140); > + > + genphy_config_aneg(phydev); This should check the return code and return it if negative. > + > + /* soft reset */ > + reg = phy_read(phydev, MDIO_DEVAD_NONE, MII_BMCR); > + reg |= BMCR_RESET; > + phy_write(phydev, MDIO_DEVAD_NONE, MII_BMCR, reg); > + > + return 0; > +} Thanks, -Joe _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot