[U-Boot] [Patch v2] PHY: micrel.c: add support for KSZ9031

[Stefan Roese <sr@denx.de>]

Hi David,

On 06.02.2013 22:18, David Andrey wrote:
> Add support for Micrel PHY KSZ9031 in phylib,
> including small rework for KSZ9021 to avoid
> code duplication
> 
> Signed-off-by: David Andrey <david.andrey@netmodule.com>
> Cc: Troy Kisky <troy.kisky@boundarydevices.com>
> Cc: Joe Herschberger <joe.hershberger@gmail.com>
> Cc: Andy Fleming <afleming@freescale.com>
> 
> ---
> 
> Changes for v2:
> 	- Same startup function for KSZ9021 and 9031
> ---
>  drivers/net/phy/micrel.c |   79 ++++++++++++++++++++++++++++++----------------
>  1 files changed, 52 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
> index 30f3264..2a8b6cb 100644
> --- a/drivers/net/phy/micrel.c
> +++ b/drivers/net/phy/micrel.c
> @@ -18,6 +18,7 @@
>   *
>   * Copyright 2010-2011 Freescale Semiconductor, Inc.
>   * author Andy Fleming
> + * (C) 2012 NetModule AG, David Andrey, added KSZ9031
>   *
>   */
>  #include <config.h>
> @@ -52,16 +53,46 @@ static struct phy_driver KS8721_driver = {
>  };
>  #endif
>  
> +
> +/**
> + * KSZ9021 - KSZ9031 common
> + */
> +
> +#define MII_KSZ90xx_PHY_CTL		0x1f
> +#define MIIM_KSZ90xx_PHYCTL_1000	(1 << 6)
> +#define MIIM_KSZ90xx_PHYCTL_100		(1 << 5)
> +#define MIIM_KSZ90xx_PHYCTL_10		(1 << 4)
> +#define MIIM_KSZ90xx_PHYCTL_DUPLEX	(1 << 3)
> +
> +static int ksz90xx_startup(struct phy_device *phydev)
> +{
> +	unsigned phy_ctl;
> +	genphy_update_link(phydev);
> +	phy_ctl = phy_read(phydev, MDIO_DEVAD_NONE, MII_KSZ90xx_PHY_CTL);
> +
> +	if (phy_ctl & MIIM_KSZ90xx_PHYCTL_DUPLEX)
> +		phydev->duplex = DUPLEX_FULL;
> +	else
> +		phydev->duplex = DUPLEX_HALF;
> +
> +	if (phy_ctl & MIIM_KSZ90xx_PHYCTL_1000)
> +		phydev->speed = SPEED_1000;
> +	else if (phy_ctl & MIIM_KSZ90xx_PHYCTL_100)
> +		phydev->speed = SPEED_100;
> +	else if (phy_ctl & MIIM_KSZ90xx_PHYCTL_10)
> +		phydev->speed = SPEED_10;
> +	return 0;
> +}
>  #ifdef CONFIG_PHY_MICREL_KSZ9021
> -/* ksz9021 PHY Registers */
> +
> +/*
> + * KSZ9021
> + */
> +
> +/* PHY Registers */
>  #define MII_KSZ9021_EXTENDED_CTRL	0x0b
>  #define MII_KSZ9021_EXTENDED_DATAW	0x0c
>  #define MII_KSZ9021_EXTENDED_DATAR	0x0d
> -#define MII_KSZ9021_PHY_CTL		0x1f
> -#define MIIM_KSZ9021_PHYCTL_1000	(1 << 6)
> -#define MIIM_KSZ9021_PHYCTL_100		(1 << 5)
> -#define MIIM_KSZ9021_PHYCTL_10		(1 << 4)
> -#define MIIM_KSZ9021_PHYCTL_DUPLEX	(1 << 3)
>  
>  #define CTRL1000_PREFER_MASTER		(1 << 10)
>  #define CTRL1000_CONFIG_MASTER		(1 << 11)
> @@ -106,37 +137,30 @@ static int ksz9021_config(struct phy_device *phydev)
>  	return 0;
>  }
>  
> -static int ksz9021_startup(struct phy_device *phydev)
> -{
> -	unsigned phy_ctl;
> -	genphy_update_link(phydev);
> -	phy_ctl = phy_read(phydev, MDIO_DEVAD_NONE, MII_KSZ9021_PHY_CTL);
> -
> -	if (phy_ctl & MIIM_KSZ9021_PHYCTL_DUPLEX)
> -		phydev->duplex = DUPLEX_FULL;
> -	else
> -		phydev->duplex = DUPLEX_HALF;
> -
> -	if (phy_ctl & MIIM_KSZ9021_PHYCTL_1000)
> -		phydev->speed = SPEED_1000;
> -	else if (phy_ctl & MIIM_KSZ9021_PHYCTL_100)
> -		phydev->speed = SPEED_100;
> -	else if (phy_ctl & MIIM_KSZ9021_PHYCTL_10)
> -		phydev->speed = SPEED_10;
> -	return 0;
> -}
> -
>  static struct phy_driver ksz9021_driver = {
>  	.name = "Micrel ksz9021",
>  	.uid  = 0x221610,
>  	.mask = 0xfffff0,

Why don't you just change the mask to support both PHY's with one struct
here? Something like this:

 static struct phy_driver ksz9021_driver = {
-	.name = "Micrel ksz9021",
+	.name = "Micrel ksz90x1",
 	.uid  = 0x221610,
-	.mask = 0xfffff0,
+	.mask = 0xffffc0,

This would be much "simpler". Does it work for you?

Best regards,
Stefan

--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: office at denx.de