2016-11-30 0:00 GMT+01:00 Joe Hershberger <joe.hershber...@gmail.com>:
> 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.
Problem is that the initialization sequence from the Marvell Release
Notes is writing undocumented values to undocumented registers. It
should be considered a binary blob to get this chip up and running.
All the information that is available is added as comments.
>
>> + 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.
Mario, would you take care of this in V2?
Cheers
Dirk
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
