[U-Boot-Users] [PATCH for STLS]: Add flash.c, ide.c and pci.c in board/st/stls/ directory!

[Wolfgang Denk]

In message <008701c8f1ec$e023d640$30065e0a at SHZ.ST.COM> you wrote:
> commit 7f818bb236de596c5bbcf21919625db9912b2c43
> Parent: 699f05125509249072a0b865c8d35520d97cd501
> Author: Ryan Chen <ryan.chen at st.com>
> Date:   Wed Jul 30 06:26:36 2008 -0400
> 
>     Signed-off-by: Ryan Chen <ryan.chen at st.com>
>     
>     	new file:   board/st/stls/flash.c
>     	new file:   board/st/stls/ide.c
>     	new file:   board/st/stls/pci.c

Please use git-format-patch to prepare the patch.

See also hints aboyut missing patch sequence / threading in previous
mail.

> diff --git a/board/st/stls/flash.c b/board/st/stls/flash.c
> new file mode 100755
> index 0000000..1afee80
> --- /dev/null
> +++ b/board/st/stls/flash.c

I reject this patch. Please use the CFI driver instead of adding
custom code.

> diff --git a/board/st/stls/ide.c b/board/st/stls/ide.c
> new file mode 100755
> index 0000000..0954b3f
> --- /dev/null
> +++ b/board/st/stls/ide.c
> @@ -0,0 +1,110 @@
> +/* 
> + * Prototypes, etc. for the STMicroelectronics STLS

This comment seems to be wrong ?

> + * 2008 (c) STMicroelectronics, Inc.
> + * Author: Ryan Chen <Ryan.Chen at st.com>
> + *
> + * 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>
> +#if defined (CONFIG_CMD_IDE) && defined(CONFIG_MULTI_IDE_CONTROLLER)
> +#if defined(CONFIG_VT82C686A) || defined(CONFIG_VT82C686B)
> +#include <vt82c686.h>
> +#endif/*defined(CONFIG_VT82C686A) || defined(CONFIG_VT82C686B)*/
> +#ifdef CFG_SATA_VIA
> +#include <sata_via.h>
> +#endif/* CFG_SATA_VIA */

There are no such #defines as CONFIG_VT82C686A or CONFIG_VT82C686B or
CFG_SATA_VIA in U-Boot.

CONFIG_VT* versus CFG_SATA_* is inconsistent.

There are no such header files as vt82c686.h or sata_via.h in U-Boot.

This seems to need some major cleanup ??

> +#ifdef DEBUG_IDE
> +#define PRINTF(fmt,args...)	printf (fmt ,##args)
> +#else
> +#define PRINTF(fmt,args...)
> +#endif

Please use debug() for debug messages.

> diff --git a/board/st/stls/pci.c b/board/st/stls/pci.c
> new file mode 100755
> index 0000000..072887d
> --- /dev/null
> +++ b/board/st/stls/pci.c
...
> +#include <common.h>
> +#ifdef CONFIG_PCI
> +#include <pci.h>
> +#ifdef CONFIG_STLS
> +#include <asm/stls/pci.h>
> +#endif
> +#if 0

Please do not add dead code.


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"Faith: not *wanting* to know what is true."    - Friedrich Nietzsche