From mboxrd@z Thu Jan  1 00:00:00 1970
From: Giuseppe Pagano <giuseppe.pagano@seco.com>
Date: Thu, 07 Nov 2013 13:37:29 +0100
Subject: [U-Boot] [PATCH 1/4] udoo: Add Ethernet support.
In-Reply-To: <527B5174.8040605@denx.de>
References: <1383769986.3741.15.camel@localhost> <527B5174.8040605@denx.de>
Message-ID: <1383827849.3371.90.camel@localhost>
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

Hi Stefano,

On Thu, 2013-11-07 at 09:38 +0100, Stefano Babic wrote:
> Hi Giuseppe,
> 
> On 06/11/2013 21:33, Giuseppe Pagano wrote:
> > Add Ethernet and networking support on uDoo board (FEC + phy Micrel)
> > 
> > 
> > Signed-off-by: Giuseppe Pagano <giuseppe.pagano@seco.com>
> > Cc: sbabic at denx.de
> > 

> > +int mx6_rgmii_rework(struct phy_device *phydev)
> > +{
> > +	/* To advertise only 10 Mbs */
> > +	phy_write(phydev, MDIO_DEVAD_NONE, 0x4, 0x61);
> > +	phy_write(phydev, MDIO_DEVAD_NONE, 0x9, 0x0c00);
> > +
> 
> Why only 10 Mb/s ? I think the Micrel 9031 allows 1Gb/s.

I will check again, I remember this solves an issue present also in
sabrelite board; but not sure. Maybe we can remove.

> Generally, use defines instead of hard coded values.

Ok


> > +	/* min rx data delay */
> > +	ksz9021_phy_extended_write(phydev,
> > +				   MII_KSZ9021_EXT_RGMII_RX_DATA_SKEW, 0x0);
> 
> Is is 9021 or 9031 ?

uDoo adopt a Micrel KSZ9031 phy. 
Most of the register address are common to ksz9021 and ksz9031, and have
the same value. Maybe we should rename some variable to MII_KSZ90XX_...


> > +
> > +	gpio_set_value(IMX_GPIO_NR(3, 23), 1); /* SABRE Lite PHY rst */
> 
> SABRE as comment is maybe wrong
> 
> > +#define CONFIG_PHY_MICREL_KSZ9021
> 
> Ok, it is 9021 - please be consistent with the comments avoiding mixing
> 9031 and 9021.

No, it is 9031. I will create a new define.

> 
> Best regards,
> Stefano Babic

Best regards,
Giuseppe Pagano