From mboxrd@z Thu Jan  1 00:00:00 1970
From: Stefan Roese <sr@denx.de>
Date: Thu, 20 Mar 2014 12:23:20 +0100
Subject: [U-Boot] [PATCH] 4xx: cleanup ethernet phy initialization on
 PMC440 boards
In-Reply-To: <1395261039-19640-1-git-send-email-matthias.fuchs@esd.eu>
References: <1395261039-19640-1-git-send-email-matthias.fuchs@esd.eu>
Message-ID: <532ACFA8.90009@denx.de>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Hi Matthias,

On 19.03.2014 21:30, Matthias Fuchs wrote:
> This patch moves phy initialization for VSC8601 ethernet
> phys that are used on early board revisions into a separate
> setup function.

Thanks for working on this. Some additional comments though.

> Signed-off-by: Matthias Fuchs <matthias.fuchs@esd.eu>
> ---
>   board/esd/pmc440/pmc440.c |   45 ++++++++++++++++++++++-----------------------
>   1 files changed, 22 insertions(+), 23 deletions(-)
>
> diff --git a/board/esd/pmc440/pmc440.c b/board/esd/pmc440/pmc440.c
> index 7aee8e4..3165486 100644
> --- a/board/esd/pmc440/pmc440.c
> +++ b/board/esd/pmc440/pmc440.c
> @@ -643,6 +643,23 @@ int is_pci_host(struct pci_controller *hose)
>   #endif /* defined(CONFIG_PCI) */
>
>   #ifdef CONFIG_RESET_PHY_R
> +int pmc440_setup_vsc8601(char *devname, int phy_addr,
> +			 unsigned short behavior, unsigned short method)

static int pmc440_...

> +{
> +	/* adjust LED behavior */
> +	if (miiphy_write(devname, phy_addr, 0x1f, 0x0001) != 0) {
> +		printf("Phy%d: register write access failed\n", phy_addr);
> +		return -1;
> +	}
> +
> +	miiphy_write(devname, phy_addr, 0x11, 0x0010);
> +	miiphy_write(devname, phy_addr, 0x11, behavior);
> +	miiphy_write(devname, phy_addr, 0x10, method);
> +	miiphy_write(devname, phy_addr, 0x1f, 0x0000);
> +
> +	return 0;
> +}

Look much nicer with less indentation. :)

>   int pmc440_setup_ksz9031(char *devname, int phy_addr)

And now I notice that this should also be:

static int pmc440_...

A new patch-set would be best to handle this. Or a new patch with both 
changes squashed together would be fine as well. Whatever you prefer.

Thanks,
Stefan