From mboxrd@z Thu Jan  1 00:00:00 1970
From: Stefan Roese <sr@denx.de>
Date: Thu, 30 Oct 2014 12:39:02 +0100
Subject: [U-Boot] [U-boot] [Patch] net: phy: marvell: add errata w/a for
 88E151* chips
In-Reply-To: <54520A4F.9060200@ti.com>
References: <1414607903-3079-1-git-send-email-ivan.khoronzhuk@ti.com>
	<5451E07B.50107@denx.de> <54520A4F.9060200@ti.com>
Message-ID: <54522356.8040306@denx.de>
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 30.10.2014 10:52, Ivan Khoronzhuk wrote:
>>> +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

Yes. If this errata is really only for SGMII, then putting this code 
under this if() statement makes sense.

Thanks,
Stefan