[U-Boot-Users] [PATCH 3/9] Add Ethernet 1000BASE-X support for PPC4xx

[Stefan Roese]

On Saturday 22 December 2007, Larry Johnson wrote:
> This patch adds a new switch: "CONFIG_PHY_DYNAMIC_ANEG".  When this symbol
> is defined, the PHY will advertise it's capabilities for autonegotiation
> based on the capabilities shown in the PHY's status registers, including
> 1000BASE-X.  When "CONFIG_PHY_DYNAMIC_ANEG" is not defined, the PHY will
> advertise hard-coded capabilities, as before.

Sorry for the late review, but I have still some remarks. Please see below.

> Signed-off-by: Larry Johnson <lrj at acm.org>
> ---
>  cpu/ppc4xx/miiphy.c |  143
> +++++++++++++++++++++++++++++++++++--------------- 1 files changed, 100
> insertions(+), 43 deletions(-)
>
> diff --git a/cpu/ppc4xx/miiphy.c b/cpu/ppc4xx/miiphy.c
> index 98ba0a7..97ec023 100644
> --- a/cpu/ppc4xx/miiphy.c
> +++ b/cpu/ppc4xx/miiphy.c
> @@ -27,19 +27,6 @@
>
>    |  Author:	 Mark Wisner
>
> -  |  Change Activity-
> -  |
> -  |  Date	 Description of Change					     BY
> -  |  ---------	 ---------------------					     ---
> -  |  05-May-99	 Created						     MKW
> -  |  01-Jul-99	 Changed clock setting of sta_reg from 66Mhz to 50Mhz to
> -  |		 better match OPB speed. Also modified delay times.	     JWB
> -  |  29-Jul-99	 Added Full duplex support				     MKW
> -  |  24-Aug-99	 Removed printf from dp83843_duplex()			     JWB
> -  |  19-Jul-00	 Ported to esd cpci405					     sr
> -  |  23-Dec-03	 Ported from miiphy.c to 440GX Travis Sawyer		     TBS
> -  |		 <travis.sawyer at sandburst.com>
> -  |
>   
> +--------------------------------------------------------------------------
>---*/
>
>  #include <common.h>
> @@ -61,7 +48,6 @@ void miiphy_dump (char *devname, unsigned char addr)
>  	unsigned long i;
>  	unsigned short data;
>
> -
>  	for (i = 0; i < 0x1A; i++) {
>  		if (miiphy_read (devname, addr, i, &data)) {
>  			printf ("read error for reg %lx\n", i);
> @@ -76,15 +62,87 @@ void miiphy_dump (char *devname, unsigned char addr)
>  	}			/* end for loop */
>  }				/* end dump */
>
> -
>  /***********************************************************/
>  /* (Re)start autonegotiation				   */
>  /***********************************************************/
>  int phy_setup_aneg (char *devname, unsigned char addr)
>  {
> -	unsigned short ctl, adv;
> +	u16 bmcr;
> +
> +#if defined(CONFIG_PHY_DYNAMIC_ANEG)
> +	/*
> +	 * Set up advertisement based on capablilities reported by the PHY.
> +	 * This should work for both copper and fiber.
> +	 */
> +	u16 bmsr;
> +#if defined(CONFIG_PHY_GIGE)
> +	u16 exsr = 0x0000;
> +#endif
> +
> +	miiphy_read (devname, addr, PHY_BMSR, &bmsr);
> +
> +#if defined(CONFIG_PHY_GIGE)
> +	if (bmsr & PHY_BMSR_EXT_STAT) {
> +		miiphy_read (devname, addr, PHY_EXSR, &exsr);
> +	}

No parantheses (braces ?) for one line statements please.

> +	if (exsr & (PHY_EXSR_1000XF | PHY_EXSR_1000XH)) {
> +		/* 1000BASE-X */
> +		u16 anar = 0x0000;
> +
> +		if (exsr & PHY_EXSR_1000XF) {
> +			anar |= PHY_X_ANLPAR_FD;
> +		}
> +		if (exsr & PHY_EXSR_1000XH) {
> +			anar |= PHY_X_ANLPAR_HD;
> +		}

Again, please check the complete source for such on lined statements and 
remove those braces.

Thanks.

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-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================