[U-Boot] [PATCH] 4xx: add support for new PMC440 revision

[Stefan Roese <sr@denx.de>]

Hi Matthias,

On 19.03.2014 10:25, Matthias Fuchs wrote:
> This patch adds support for the new PMC440 hardware revision 1.4.
> The board now uses Micrel KSZ9031 phys.
>
> Add missing i2c initialization before reading bootstrap eeprom.
>
> Signed-off-by: Matthias Fuchs <matthias.fuchs@esd.eu>
> ---
>   board/esd/pmc440/pmc440.c |   83 +++++++++++++++++++++++++++++++++------------
>   1 file changed, 62 insertions(+), 21 deletions(-)
>
> diff --git a/board/esd/pmc440/pmc440.c b/board/esd/pmc440/pmc440.c
> index e86996c..7aee8e4 100644
> --- a/board/esd/pmc440/pmc440.c
> +++ b/board/esd/pmc440/pmc440.c
> @@ -63,6 +63,8 @@ struct serial_device *default_serial_console(void)
>   		/* mark scratchreg valid */
>   		scratchreg = (scratchreg & 0xffffff00) | 0x80;
>
> +		i2c_init_all();
> +
>   		i = bootstrap_eeprom_read(CONFIG_SYS_I2C_BOOT_EEPROM_ADDR,
>   					  0x10, buf, 4);
>   		if ((i != -1) && (buf[0] == 0x19) && (buf[1] == 0x75)) {
> @@ -641,34 +643,73 @@ int is_pci_host(struct pci_controller *hose)
>   #endif /* defined(CONFIG_PCI) */
>
>   #ifdef CONFIG_RESET_PHY_R
> +int pmc440_setup_ksz9031(char *devname, int phy_addr)
> +{
> +	unsigned short id1, id2;
> +
> +	if (miiphy_read(devname, phy_addr, 2, &id1) ||
> +	    miiphy_read(devname, phy_addr, 3, &id2)) {
> +		printf("Phy%d: cannot read id\n", phy_addr);
> +		return -1;
> +	}
> +
> +	if ((id1 != 0x0022) || ((id2 & 0xfff0) != 0x1620)) {
> +		printf("Phy%d: unexpected id\n", phy_addr);
> +		return -1;
> +	}
> +
> +	/* MMD 2.08: adjust tx_clk pad skew */
> +	miiphy_write(devname, phy_addr, 0x0d, 2);
> +	miiphy_write(devname, phy_addr, 0x0e, 8);
> +	miiphy_write(devname, phy_addr, 0x0d, 0x4002);
> +	miiphy_write(devname, phy_addr, 0x0e, 0xf | (0x17 << 5));
> +
> +	return 0;
> +}
>   void reset_phy(void)
>   {
>   	char *s;
>   	unsigned short val_method, val_behavior;
>
> -	/* special LED setup for NGCC/CANDES */
> -	if ((s = getenv("bd_type")) &&
> -	    ((!strcmp(s, "ngcc")) || (!strcmp(s, "candes")))) {
> -		val_method   = 0x0e0a;
> -		val_behavior = 0x0cf2;
> -	} else {
> -		/* PMC440 standard type */
> -		val_method   = 0x0e10;
> -		val_behavior = 0x0cf0;
> -	}
> +	if (gd->board_type < 4) {
> +		/* special LED setup for NGCC/CANDES */
> +		s = getenv("bd_type");
> +		if (s && ((!strcmp(s, "ngcc")) || (!strcmp(s, "candes")))) {
> +			val_method   = 0x0e0a;
> +			val_behavior = 0x0cf2;
> +		} else {
> +			/* PMC440 standard type */
> +			val_method   = 0x0e10;
> +			val_behavior = 0x0cf0;
> +		}
>
> -	if (miiphy_write("ppc_4xx_eth0", CONFIG_PHY_ADDR, 0x1f, 0x0001) == 0) {
> -		miiphy_write("ppc_4xx_eth0", CONFIG_PHY_ADDR, 0x11, 0x0010);
> -		miiphy_write("ppc_4xx_eth0", CONFIG_PHY_ADDR, 0x11, val_behavior);
> -		miiphy_write("ppc_4xx_eth0", CONFIG_PHY_ADDR, 0x10, val_method);
> -		miiphy_write("ppc_4xx_eth0", CONFIG_PHY_ADDR, 0x1f, 0x0000);
> -	}
> +		if (miiphy_write("ppc_4xx_eth0",
> +				 CONFIG_PHY_ADDR, 0x1f, 0x0001) == 0) {
> +			miiphy_write("ppc_4xx_eth0",
> +				     CONFIG_PHY_ADDR, 0x11, 0x0010);
> +			miiphy_write("ppc_4xx_eth0",
> +				     CONFIG_PHY_ADDR, 0x11, val_behavior);
> +			miiphy_write("ppc_4xx_eth0",
> +				     CONFIG_PHY_ADDR, 0x10, val_method);
> +			miiphy_write("ppc_4xx_eth0",
> +				     CONFIG_PHY_ADDR, 0x1f, 0x0000);
> +		}
>
> -	if (miiphy_write("ppc_4xx_eth1", CONFIG_PHY1_ADDR, 0x1f, 0x0001) == 0) {
> -		miiphy_write("ppc_4xx_eth1", CONFIG_PHY1_ADDR, 0x11, 0x0010);
> -		miiphy_write("ppc_4xx_eth1", CONFIG_PHY1_ADDR, 0x11, val_behavior);
> -		miiphy_write("ppc_4xx_eth1", CONFIG_PHY1_ADDR, 0x10, val_method);
> -		miiphy_write("ppc_4xx_eth1", CONFIG_PHY1_ADDR, 0x1f, 0x0000);
> +		if (miiphy_write("ppc_4xx_eth1",
> +				 CONFIG_PHY1_ADDR, 0x1f, 0x0001) == 0) {
> +			miiphy_write("ppc_4xx_eth1",
> +				     CONFIG_PHY1_ADDR, 0x11, 0x0010);
> +			miiphy_write("ppc_4xx_eth1",
> +				     CONFIG_PHY1_ADDR, 0x11, val_behavior);
> +			miiphy_write("ppc_4xx_eth1",
> +				     CONFIG_PHY1_ADDR, 0x10, val_method);
> +			miiphy_write("ppc_4xx_eth1",
> +				     CONFIG_PHY1_ADDR, 0x1f, 0x0000);
> +		}

This if () section looks very similar to the one before in this patch. 
Only difference is the string "ppc_4xx_eth1". Can't you move this code 
into a function to reduce the code size?

I know this code duplication was not introduced with this patch. But it 
makes sense to simplify this now for my taste.

Thanks,
Stefan