From mboxrd@z Thu Jan  1 00:00:00 1970
From: Stefan Roese <sr@denx.de>
Date: Wed, 13 Oct 2010 13:16:26 +0200
Subject: [U-Boot] [PATCH v2] ppc4xx: Add CATCenter Io 405EP board support
In-Reply-To: <1286965218-12023-1-git-send-email-eibach@gdsys.de>
References: <1286965218-12023-1-git-send-email-eibach@gdsys.de>
Message-ID: <201010131316.26358.sr@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 Dirk,

On Wednesday 13 October 2010 12:20:18 Dirk Eibach wrote:
> Board support for the Guntermann & Drunck CATCenter Io.
> 
> Signed-off-by: Dirk Eibach <eibach@gdsys.de>
> ---
> Changes since v1:
> - added MAINTAINERS entry
> - sort MAKEALL
> - whitespace/codestyle cleanup
> - use boards.cfg instead of Makefile
> - added some comments for clarification
> - use CONFIG_BITBANGMII_MULTI

Now that was quick. :) But I still have some improvement suggestions below.
 
> +++ b/board/gdsys/io/io_miiphybb.c
> @@ -0,0 +1,102 @@
> +/*
> + * (C) Copyright 2010
> + * Dirk Eibach,  Guntermann & Drunck GmbH, eibach at gdsys.de
> + *
> + * See file CREDITS for list of people who contributed to this
> + * project.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
> + * MA 02111-1307 USA
> + */
> +
> +#include <common.h>
> +#include <miiphy.h>
> +
> +#include <asm/io.h>
> +
> +static int io_bb_mii_init(struct bb_miiphy_bus *bus)
> +{
> +	return 0;
> +}
> +
> +static int io_bb_mdio_active(struct bb_miiphy_bus *bus)
> +{
> +	out_be32((void *)GPIO0_TCR,
> +		in_be32((void *)GPIO0_TCR) | CONFIG_SYS_MDIO_PIN);

Why not use the already available GPIO functions instead (gpio_write_bit() 
etc)? This will make it easier to move this to the common (Linux) GPIO API at 
some time later.

Cheers,
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