[U-Boot] [PATCH 6/7] phylib: Add a bunch of PHY drivers from tsec

[Detlev Zundel]

Hi Andy,

> The tsec driver had a bunch of PHY drivers already written. This
> converts them all into PHY Lib drivers, and serves as the first
> set of PHY drivers for PHY Lib.
>
> Signed-off-by: Andy Fleming <afleming at freescale.com>

[...]

> diff --git a/drivers/net/phy/atheros.c b/drivers/net/phy/atheros.c
> new file mode 100644
> index 0000000..68bb5cd
> --- /dev/null
> +++ b/drivers/net/phy/atheros.c
> @@ -0,0 +1,37 @@
> +/*
> + * Atheros PHY drivers
> + *
> + * This software may be used and distributed according to the
> + * terms of the GNU Public License, Version 2, incorporated
> + * herein by reference.

Again, new files need GPLv2+.  I know that this is split out from a
GPLv2 file, but we have our rules in the meantime.

[...]

> diff --git a/drivers/net/phy/broadcom.c b/drivers/net/phy/broadcom.c
> new file mode 100644
> index 0000000..e62a81d
> --- /dev/null
> +++ b/drivers/net/phy/broadcom.c
> @@ -0,0 +1,275 @@
> +/*
> + * Broadcom PHY drivers
> + *
> + * This software may be used and distributed according to the
> + * terms of the GNU Public License, Version 2, incorporated
> + * herein by reference.

GPLv2+


[...]

> +static int bcm54xx_parse_status(struct phy_device *phydev)
> +{
> +	unsigned int mii_reg;
> +
> +	mii_reg = phy_read(phydev, MDIO_DEVAD_NONE, MIIM_BCM54xx_AUXSTATUS);
> +
> +	switch ((mii_reg & MIIM_BCM54xx_AUXSTATUS_LINKMODE_MASK) >>
> +			MIIM_BCM54xx_AUXSTATUS_LINKMODE_SHIFT) {
> +		case 1:
> +		phydev->duplex = DUPLEX_HALF;
> +		phydev->speed = SPEED_10;
> +		break;
> +		case 2:
> +		phydev->duplex = DUPLEX_FULL;
> +		phydev->speed = SPEED_10;
> +		break;
> +		case 3:
> +		phydev->duplex = DUPLEX_HALF;
> +		phydev->speed = SPEED_100;
> +		break;
> +		case 5:
> +		phydev->duplex = DUPLEX_FULL;
> +		phydev->speed = SPEED_100;
> +		break;
> +		case 6:
> +		phydev->duplex = DUPLEX_HALF;
> +		phydev->speed = SPEED_1000;
> +		break;
> +		case 7:
> +		phydev->duplex = DUPLEX_FULL;
> +		phydev->speed = SPEED_1000;
> +		break;
> +		default:
> +		printf("Auto-neg error, defaulting to 10BT/HD\n");
> +		phydev->duplex = DUPLEX_HALF;
> +		phydev->speed = SPEED_10;
> +		break;

Very strange indentation, please fix.

[...]

> diff --git a/drivers/net/phy/davicom.c b/drivers/net/phy/davicom.c
> new file mode 100644
> index 0000000..229e1bd
> --- /dev/null
> +++ b/drivers/net/phy/davicom.c
> @@ -0,0 +1,86 @@
> +/*
> + * Davicom PHY drivers
> + *
> + * This software may be used and distributed according to the
> + * terms of the GNU Public License, Version 2, incorporated
> + * herein by reference.

GPLv2+

[...]

> diff --git a/drivers/net/phy/lxt.c b/drivers/net/phy/lxt.c
> new file mode 100644
> index 0000000..4e38bb6
> --- /dev/null
> +++ b/drivers/net/phy/lxt.c
> @@ -0,0 +1,76 @@
> +/*
> + * LXT PHY drivers
> + *
> + * This software may be used and distributed according to the
> + * terms of the GNU Public License, Version 2, incorporated
> + * herein by reference.

GPLv2+

[...]

> diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
> new file mode 100644
> index 0000000..6937c93
> --- /dev/null
> +++ b/drivers/net/phy/marvell.c
> @@ -0,0 +1,357 @@
> +/*
> + * Marvell PHY drivers
> + *
> + * This software may be used and distributed according to the
> + * terms of the GNU Public License, Version 2, incorporated
> + * herein by reference.

dito

[...]

> +/* Parse the 88E1011's status register for speed and duplex
> + * information
> + */
> +static uint m88e1011s_parse_status(struct phy_device *phydev)
> +{
> +	unsigned int speed;
> +	unsigned int mii_reg;
> +
> +	mii_reg = phy_read(phydev, MDIO_DEVAD_NONE, MIIM_88E1011_PHY_STATUS);
> +
> +	if ((mii_reg & MIIM_88E1011_PHYSTAT_LINK) &&
> +		!(mii_reg & MIIM_88E1011_PHYSTAT_SPDDONE)) {
> +		int i = 0;
> +
> +		puts("Waiting for PHY realtime link");
> +		while (!(mii_reg & MIIM_88E1011_PHYSTAT_SPDDONE)) {
> +			/* Timeout reached ? */
> +			if (i > PHY_AUTONEGOTIATE_TIMEOUT) {
> +				puts(" TIMEOUT !\n");
> +				phydev->link = 0;
> +				break;
> +			}
> +
> +			if ((i++ % 1000) == 0) {
> +				putc('.');
> +			}
> +			udelay(1000);
> +			mii_reg = phy_read(phydev, MDIO_DEVAD_NONE,
> +					MIIM_88E1011_PHY_STATUS);
> +		}
> +		puts(" done\n");
> +		udelay(500000);	/* another 500 ms (results in faster booting) */

Ah, again the "faster booting when waiting half a second" - probably
that's the origin ;)  Still a comment will be nice.

[...]

> +/* Marvell 88E1118 */
> +static int m88e1118_config(struct phy_device *phydev)
> +{
> +	/* Reset and configure the PHY */
> +	phy_write(phydev, MDIO_DEVAD_NONE, MII_BMCR, BMCR_RESET);
> +	/* Change Page Number */
> +	phy_write(phydev, MDIO_DEVAD_NONE, 0x16, 0x0002);
> +	/* Delay RGMII TX and RX */
> +	phy_write(phydev, MDIO_DEVAD_NONE, 0x15, 0x1070);
> +	/* Change Page Number */
> +	phy_write(phydev, MDIO_DEVAD_NONE, 0x16, 0x0003);
> +	/* Adjust LED control */
> +	phy_write(phydev, MDIO_DEVAD_NONE, 0x10, 0x021e);
> +	/* Change Page Number */
> +	phy_write(phydev, MDIO_DEVAD_NONE, 0x16, 0x0000);

Below there are constants for the PHY_PAGE and LED_PAGE, so why not use
them?  Also there is a constant for the LED control, this makes sense IMHO.

> +
> +	phy_write(phydev, MDIO_DEVAD_NONE, MII_BMCR, BMCR_RESET);
> +
> +	genphy_config_aneg(phydev);
> +
> +	return 0;
> +}
> +
> +static int m88e1118_startup(struct phy_device *phydev)
> +{
> +	/* Change Page Number */
> +	phy_write(phydev, MDIO_DEVAD_NONE, 0x16, 0x0000);
> +
> +	genphy_update_link(phydev);
> +	m88e1011s_parse_status(phydev);
> +
> +	return 0;
> +}
> +
> +/* Marvell 88E1121R */
> +static int m88e1121_config(struct phy_device *phydev)
> +{
> +	int pg;
> +
> +	/* Reset and configure the PHY */
> +	phy_write(phydev, MDIO_DEVAD_NONE, MII_BMCR, BMCR_RESET);
> +	genphy_config_aneg(phydev);
> +
> +	/* Switch the page to access the led register */
> +	pg = phy_read(phydev, MDIO_DEVAD_NONE, MIIM_88E1121_PHY_PAGE);
> +	phy_write(phydev, MDIO_DEVAD_NONE, MIIM_88E1121_PHY_PAGE,
> +			MIIM_88E1121_PHY_LED_PAGE);
> +	/* Configure leds */
> +	phy_write(phydev, MDIO_DEVAD_NONE, MIIM_88E1121_PHY_LED_CTRL,
> +			MIIM_88E1121_PHY_LED_DEF);
> +	/* Restore the page pointer */
> +	phy_write(phydev, MDIO_DEVAD_NONE, MIIM_88E1121_PHY_PAGE, pg);
> +
> +	/* Disable IRQs and de-assert interrupt */
> +	phy_write(phydev, MDIO_DEVAD_NONE, MIIM_88E1121_PHY_IRQ_EN, 0);
> +	phy_read(phydev, MDIO_DEVAD_NONE, MIIM_88E1121_PHY_IRQ_STATUS);

This is what I mean - please code the 88e1118 case accordingly.

> +
> +	return 0;
> +}
> +
> +/* Marvell 88E1145 */
> +static int m88e1145_config(struct phy_device *phydev)
> +{
> +	int reg;
> +
> +	/* Reset and configure the PHY */
> +	phy_write(phydev, MDIO_DEVAD_NONE, MII_BMCR, BMCR_RESET);
> +
> +	/* Errata E0, E1 */
> +	phy_write(phydev, MDIO_DEVAD_NONE, 29, 0x001b);
> +	phy_write(phydev, MDIO_DEVAD_NONE, 30, 0x418f);
> +	phy_write(phydev, MDIO_DEVAD_NONE, 29, 0x0016);
> +	phy_write(phydev, MDIO_DEVAD_NONE, 30, 0xa2da);
> +
> +	phy_write(phydev, MDIO_DEVAD_NONE, MIIM_88E1011_PHY_SCR,
> +			MIIM_88E1011_PHY_MDI_X_AUTO);

Hm, the 88e1145 uses 88e1011 constants?  Maybe we can come up with
88e1xxx constants?

> +
> +	reg = phy_read(phydev, MDIO_DEVAD_NONE, MIIM_88E1145_PHY_EXT_CR);
> +	if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID)
> +		reg |= MIIM_M88E1145_RGMII_RX_DELAY |
> +			MIIM_M88E1145_RGMII_TX_DELAY;
> +	phy_write(phydev, MDIO_DEVAD_NONE, MIIM_88E1145_PHY_EXT_CR, reg);
> +
> +	phy_write(phydev, MDIO_DEVAD_NONE, MII_BMCR, BMCR_RESET);
> +
> +	genphy_config_aneg(phydev);
> +
> +	return 0;
> +}
> +
> +static int m88e1145_startup(struct phy_device *phydev)
> +{
> +	genphy_update_link(phydev);
> +	phy_write(phydev, MDIO_DEVAD_NONE, MIIM_88E1111_PHY_LED_CONTROL,
> +			MIIM_88E1111_PHY_LED_DIRECT);

... and 88e1111 constants.  Pretty confusing.

> +	m88e1011s_parse_status(phydev);
> +
> +	return 0;
> +}
> +
> +/* Marvell 88E1149S */
> +static int m88e1149_config(struct phy_device *phydev)
> +{
> +	/* Reset and configure the PHY */
> +	phy_write(phydev, MDIO_DEVAD_NONE, MII_BMCR, BMCR_RESET);
> +
> +	phy_write(phydev, MDIO_DEVAD_NONE, 0x1d, 0x1f);
> +	phy_write(phydev, MDIO_DEVAD_NONE, 0x1e, 0x200c);
> +	phy_write(phydev, MDIO_DEVAD_NONE, 0x1d, 0x5);
> +	phy_write(phydev, MDIO_DEVAD_NONE, 0x1e, 0x0);
> +	phy_write(phydev, MDIO_DEVAD_NONE, 0x1e, 0x100);

Lots of magic numbers - defines would be nice.


[...]

> diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
> new file mode 100644
> index 0000000..4e0389a
> --- /dev/null
> +++ b/drivers/net/phy/micrel.c
> @@ -0,0 +1,29 @@
> +/*
> + * Micrel PHY drivers
> + *
> + * This software may be used and distributed according to the
> + * terms of the GNU Public License, Version 2, incorporated
> + * herein by reference.

GPLv2+

[...]

> --- /dev/null
> +++ b/drivers/net/phy/natsemi.c
> @@ -0,0 +1,85 @@
> +/*
> + * National Semiconductor PHY drivers
> + *
> + * This software may be used and distributed according to the
> + * terms of the GNU Public License, Version 2, incorporated
> + * herein by reference.

GPLv2+

[...]

> diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
> new file mode 100644
> index 0000000..8cb696f
> --- /dev/null
> +++ b/drivers/net/phy/realtek.c
> @@ -0,0 +1,120 @@
> +/*
> + * RealTek PHY drivers
> + *
> + * This software may be used and distributed according to the
> + * terms of the GNU Public License, Version 2, incorporated
> + * herein by reference.

dito.

[...]

> +static int rtl8211b_parse_status(struct phy_device *phydev)
> +{
> +	unsigned int speed;
> +	unsigned int mii_reg;
> +
> +	mii_reg = phy_read(phydev, MDIO_DEVAD_NONE, MIIM_RTL8211B_PHY_STATUS);
> +
> +	if (!(mii_reg & MIIM_RTL8211B_PHYSTAT_SPDDONE)) {
> +		int i = 0;
> +
> +		/* in case of timeout ->link is cleared */
> +		phydev->link = 1;
> +		puts("Waiting for PHY realtime link");
> +		while (!(mii_reg & MIIM_RTL8211B_PHYSTAT_SPDDONE)) {
> +			/* Timeout reached ? */
> +			if (i > PHY_AUTONEGOTIATE_TIMEOUT) {
> +				puts(" TIMEOUT !\n");
> +				phydev->link = 0;
> +				break;
> +			}
> +
> +			if ((i++ % 1000) == 0) {
> +				putc('.');
> +			}
> +			udelay(1000);	/* 1 ms */
> +			mii_reg = phy_read(phydev, MDIO_DEVAD_NONE,
> +					MIIM_RTL8211B_PHY_STATUS);
> +		}
> +		puts(" done\n");
> +		udelay(500000);	/* another 500 ms (results in faster booting) */

Once more half a second.  Maybe this is not needed at all and costs us
boot time on every single boot?  We should really check if this is
needed at all and remove it.  Somehow I doubt that we need to do this
generically.

[...]

> diff --git a/drivers/net/phy/teranetics.c b/drivers/net/phy/teranetics.c
> new file mode 100644
> index 0000000..387e044
> --- /dev/null
> +++ b/drivers/net/phy/teranetics.c
> @@ -0,0 +1,43 @@
> +/*
> + * Teranetics PHY drivers
> + *
> + * This software may be used and distributed according to the
> + * terms of the GNU Public License, Version 2, incorporated
> + * herein by reference.

GPLv2+

[...]

> diff --git a/drivers/net/phy/vitesse.c b/drivers/net/phy/vitesse.c
> new file mode 100644
> index 0000000..d34f04b
> --- /dev/null
> +++ b/drivers/net/phy/vitesse.c
> @@ -0,0 +1,330 @@
> +/*
> + * Vitesse PHY drivers
> + *
> + * This software may be used and distributed according to the
> + * terms of the GNU Public License, Version 2, incorporated
> + * herein by reference.

dito


[...]

> +static int cis8204_config(struct phy_device *phydev)
> +{
> +	/* Override PHY config settings */
> +	phy_write(phydev, MDIO_DEVAD_NONE, MIIM_CIS8201_AUX_CONSTAT,
> +			MIIM_CIS8201_AUXCONSTAT_INIT);

cis8204 usind CIS8201 constants and CIS8204 constants.  Can we generalize
some?  This continues in the 8211 cases below...  

If we cannot (easily) generalize, then maybe we should really have
correctly named defines even if they are redundand?  This would be more
consistent and better for maintenance than the current mix.


[...]

> diff --git a/include/phylib_all_drivers.h b/include/phylib_all_drivers.h
> new file mode 100644
> index 0000000..a73b3d7
> --- /dev/null
> +++ b/include/phylib_all_drivers.h
> @@ -0,0 +1,25 @@
> +/*
> + * Enable all PHYs
> + *
> + * This software may be used and distributed according to the
> + * terms of the GNU Public License, Version 2, incorporated
> + * herein by reference.

GPLv2+

Otherwise looks good, thanks!
  Detlev

-- 
Quantum physicists can either know how fast they do it, or where they
do it, but not both.
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de