From mboxrd@z Thu Jan  1 00:00:00 1970
From: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
Date: Thu, 30 Oct 2014 11:52:15 +0200
Subject: [U-Boot] [U-boot] [Patch] net: phy: marvell: add errata w/a for
 88E151* chips
In-Reply-To: <5451E07B.50107@denx.de>
References: <1414607903-3079-1-git-send-email-ivan.khoronzhuk@ti.com>
	<5451E07B.50107@denx.de>
Message-ID: <54520A4F.9060200@ti.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

On 10/30/2014 08:53 AM, Stefan Roese wrote:
> On 29.10.2014 19:38, Ivan Khoronzhuk wrote:
>> From: Hao Zhang <hzhang@ti.com>
>>
>> As per Marvell Release Notes - Alaska 88E1510/88E1518/88E1512/88E1514
>> Rev A0, Errata Section 3.1 Marvell PHY has an errata which requires
>> that certain registers get written in order to restart
>> autonegotiation.
>>
>> Signed-off-by: Hao Zhang <hzhang@ti.com>
>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
>> ---
>>   drivers/net/phy/marvell.c | 51 
>> ++++++++++++++++++++++++++++++++++++++++++++++-
>>   1 file changed, 50 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
>> index d2ecadc..425db94 100644
>> --- a/drivers/net/phy/marvell.c
>> +++ b/drivers/net/phy/marvell.c
>> @@ -276,6 +276,55 @@ static int m88e1111s_config(struct phy_device 
>> *phydev)
>>       return 0;
>>   }
>>
>> +/**
>> + * m88e1518_phy_writebits - write bits to a register
>> + */
>> +void m88e1518_phy_writebits(struct phy_device *phydev,
>> +           u8 reg_num, u16 offset, u16 len, u16 data)
>> +{
>> +    u16 reg, mask;
>> +
>> +    if ((len + offset) >= 16)
>> +        mask = 0 - (1 << offset);
>> +    else
>> +        mask = (1 << (len + offset)) - (1 << offset);
>> +
>> +    reg = phy_read(phydev, MDIO_DEVAD_NONE, reg_num);
>> +
>> +    reg &= ~mask;
>> +    reg |= data << offset;
>> +
>> +    phy_write(phydev, MDIO_DEVAD_NONE, reg_num, reg);
>> +}
>> +
>> +static int m88e1518_config(struct phy_device *phydev)
>> +{
>> +    /*
>> +     * As per Marvell Release Notes - Alaska 
>> 88E1510/88E1518/88E1512/88E1514
>> +     * Rev A0, Errata Section 3.1
>> +     */
>> +    phy_write(phydev, MDIO_DEVAD_NONE, 22, 0x00ff);    /* reg page 
>> 0xff */
>> +    phy_write(phydev, MDIO_DEVAD_NONE, 17, 0x214B);
>> +    phy_write(phydev, MDIO_DEVAD_NONE, 16, 0x2144);
>> +    phy_write(phydev, MDIO_DEVAD_NONE, 17, 0x0C28);
>> +    phy_write(phydev, MDIO_DEVAD_NONE, 16, 0x2146);
>> +    phy_write(phydev, MDIO_DEVAD_NONE, 17, 0xB233);
>> +    phy_write(phydev, MDIO_DEVAD_NONE, 16, 0x214D);
>> +    phy_write(phydev, MDIO_DEVAD_NONE, 17, 0xCC0C);
>> +    phy_write(phydev, MDIO_DEVAD_NONE, 16, 0x2159);
>> +    phy_write(phydev, MDIO_DEVAD_NONE, 22, 0x0000);    /* reg page 0 */
>> +    phy_write(phydev, MDIO_DEVAD_NONE, 22, 18);    /* reg page 18 */
>> +    /* Write HWCFG_MODE = SGMII to Copper */
>> +    m88e1518_phy_writebits(phydev, 20, 0, 3, 1);
>
> Won't this set the mode to SGMII for all users of this code? I know of 
> at least one board that uses this driver and uses RGMII. So you 
> shouldn't set this mode here to SGMII unconditionally.
>
> Thanks,
> Stefan
>

Yes.
I will put whole errata w/o under:
if (phydev->interface == PHY_INTERFACE_MODE_SGMII) {

}

as I can face it only for SGMII

Thanks!

-- 
Regards,
Ivan Khoronzhuk