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.pag...@seco.com> > > Cc: sba...@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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot