[U-Boot] [RFC][PATCH 2a/3] New i386 board (includes code relocation)
Wolfgang Denk
wd at denx.de
Tue Oct 28 00:37:53 CET 2008
Dear Graeme Russ,
In message <48E0D794.1020308 at gmail.com> you wrote:
> Split to meet mailing list size limit
It seems there was no need for such a split.
> index 9ccb9ac..6f65870 100755
> --- a/MAKEALL
> +++ b/MAKEALL
> @@ -645,6 +645,7 @@ LIST_I486=3D" \
> sc520_cdp \
> sc520_spunk \
> sc520_spunk_rel \
> + sc520_eNET \
Please keep lists sorted.
> diff --git a/Makefile b/Makefile
> index 7c13ce8..cdfca1c 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -2843,6 +2843,12 @@ sc520_spunk_config : unconfig
> sc520_spunk_rel_config : unconfig
> @$(MKCONFIG) $(@:_config=3D) i386 i386 sc520_spunk
> =
>
> +#########################################################################
> +## Serck eNET
> +#########################################################################
> +eNET_config : unconfig
> + @$(MKCONFIG) $(@:_config=3D) i386 i386 eNET
Ditto. And please no fancy comments for single boards.
> diff --git a/board/eNET/eNET.c b/board/eNET/eNET.c
> new file mode 100644
> index 0000000..1b4af58
> --- /dev/null
> +++ b/board/eNET/eNET.c
> @@ -0,0 +1,640 @@
> +/*
> + *
> + * (C) Copyright 2002
> + * Daniel Engström, Omicron Ceti AB <daniel at omicron.se>.
Hm... You create a new file, but don't claim any copyrights to it?
> +extern int do_autoscript (cmd_tbl_t *cmdtp, int flag, int argc, char *argv> []);
> +extern int do_bdinfo (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]);
> +extern int do_go (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]);
> +extern int do_reset (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]);
> +extern int do_bootm (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]);
> +extern int do_bootd (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]);
> +extern int do_iminfo (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]);
> +/* extern int do_imls (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])> ; */
> +extern int do_coninfo (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]);
> +extern int do_itest (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]);
> +extern int do_load_serial (cmd_tbl_t *cmdtp, int flag, int argc, char *arg> v[]);
> +extern int do_load_serial_bin (cmd_tbl_t *cmdtp, int flag, int argc, char > *argv[]);
> +extern int do_mem_md (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]);
> +extern int do_mem_mm (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]);
> +extern int do_mem_nm (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]);
> +extern int do_mem_mw (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]);
> +extern int do_mem_cp (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]);
> +extern int do_mem_cmp (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]);
> +extern int do_mem_crc (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]);
> +extern int do_mem_base (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]> );
> +extern int do_mem_loop (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]> );
> +extern int do_mem_mtest (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[> ]);
> +extern int do_sleep (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]);
> +extern int do_printenv (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]> );
> +extern int do_setenv (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]);
> +extern int do_saveenv (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]);
> +extern int do_run (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]);
> +extern int do_imgextract (cmd_tbl_t *cmdtp, int flag, int argc, char *argv> []);
> +extern int do_version (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]);
> +extern int do_echo (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]);
> +extern int do_help (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]);
No. Please NEVER do that. Use correct prototypes from the respective
header files.
> +void hw_watchdog_reset(void)
> +{
> + u16 wd_state;
> +
> + wd_state = readw(CFG_WATCHDIG_PIO_DATA);
> +
> + if (wd_state & CFG_WATCHDOG_PIO_BIT) {
> + /* Watchdog output high - lower it*/
> + writew(CFG_WATCHDOG_PIO_BIT, CFG_WATCHDIG_PIO_CLR);
> + }
> + else {
} else {
> +void init_sc520_enet (void)
> +{
> +
> + /* Set the UARTxCTL register at it's slower,
> + * baud clock giving us a 1.8432 MHz reference
> + */
> + write_mmcr_byte(SC520_UART1CTL, 7);
> +
> +
> + /* enable PCI bus arbitrer */
> +/* mov ebx, SYSARBCTL ; SC520 control bits for the CPU bus arbiter and > the PCI bus arbiter. */
Line too long.
(many more much too long lines following - please cleanup all of
these).
> + if (CFG_SC520_HIGH_SPEED) {
Please rebase your patch against latest version - "CFG_" does not exit
any more.
> +#if 0
> +/* PCI stuff */
Please do not add dead code (here and everywhere).
> + pci_hose_read_config_byte(hose, dev, PCI_INTERRUPT_PIN, &tmp_pin);
> + pin = (int)tmp_pin;
> +
> + pin-=1; /* pci config space use 1-based numbering */
Please spaces around the "-=", i. e.
pin -= 1;
(here and everywhere).
> + if (-1 == pin) {
> + return; /* device use no irq */
> + }
Please no braces for single line statements. And write:
if (pin == -1)
...
(here and everywhere).
> +u32 isa_map_rom(u32 bus_addr, int size)
> +{
> + u32 par;
> +
> + PRINTF("isa_map_rom asked to map %d bytes at %x\n",
> + size, bus_addr);
Please use standard debug() macro instead of custom code.
> +int last_stage_init(void)
> +{
> + int minor;
> + int major;
> +
> + major = minor = 0;
> +
> + printf("Serck Controls eNET\n");
> + printf("last_stage_init() at %08lx\n", (ulong)last_stage_init);
> +
> + printf("autoscript => do_autoscript() @ 0x%08lx\n", (ulong)do_au> toscript);
> + printf("bdinfo => do_bdinfo() @ 0x%08lx\n", (ulong)do_bd> info);
> + printf("go => do_go() @ 0x%08lx\n", (ulong)do_go> );
> + printf("reset => do_reset() @ 0x%08lx\n", (ulong)do_re> set);
> + printf("bootm => do_bootm() @ 0x%08lx\n", (ulong)do_bo> otm);
> + printf("boot => do_bootd() @ 0x%08lx\n", (ulong)do_bo> otd);
> + printf("bootd => do_bootd() @ 0x%08lx\n", (ulong)do_bo> otd);
> + printf("iminfo => do_iminfo() @ 0x%08lx\n", (ulong)do_im> info);
> +/* printf("imls => do_imls() @ 0x%08lx\n", (ulong)do_> imls); */
> + printf("imls => do_imls() @ <undefined>\n");
> + printf("coninfo => do_coninfo() @ 0x%08lx\n", (ulong)do_co> ninfo);
> + printf("itest => do_itest() @ 0x%08lx\n", (ulong)do_it> est);
> + printf("loads => do_load_serial() @ 0x%08lx\n", (ulong)do_lo> ad_serial);
> + printf("loadb => do_load_serial_bin() @ 0x%08lx\n", (ulong)do_lo> ad_serial_bin);
> + printf("loady => do_load_serial_bin() @ 0x%08lx\n", (ulong)do_lo> ad_serial_bin);
> + printf("md => do_mem_md() @ 0x%08lx\n", (ulong)do_me> m_md);
> + printf("mm => do_mem_mm() @ 0x%08lx\n", (ulong)do_me> m_mm);
> + printf("nm => do_mem_nm() @ 0x%08lx\n", (ulong)do_me> m_nm);
> + printf("mw => do_mem_mw() @ 0x%08lx\n", (ulong)do_me> m_mw);
> + printf("cp => do_mem_cp() @ 0x%08lx\n", (ulong)do_me> m_cp);
> + printf("cmp => do_mem_cmp() @ 0x%08lx\n", (ulong)do_me> m_cmp);
> + printf("crc32 => do_mem_crc() @ 0x%08lx\n", (ulong)do_me> m_crc);
> + printf("base => do_mem_base() @ 0x%08lx\n", (ulong)do_me> m_base);
> + printf("loop => do_mem_loop() @ 0x%08lx\n", (ulong)do_me> m_loop);
> + printf("mtest => do_mem_mtest() @ 0x%08lx\n", (ulong)do_me> m_mtest);
> + printf("sleep => do_sleep() @ 0x%08lx\n", (ulong)do_sl> eep);
> + printf("printenv => do_printenv() @ 0x%08lx\n", (ulong)do_pr> intenv);
> + printf("setenv => do_setenv() @ 0x%08lx\n", (ulong)do_se> tenv);
> + printf("saveenv => do_saveenv() @ 0x%08lx\n", (ulong)do_sa> veenv);
> + printf("run => do_run() @ 0x%08lx\n", (ulong)do_ru> n);
> + printf("imxtract => do_imgextract() @ 0x%08lx\n", (ulong)do_im> gextract);
> + printf("version => do_version() @ 0x%08lx\n", (ulong)do_ve> rsion);
> + printf("echo => do_echo() @ 0x%08lx\n", (ulong)do_ec> ho);
> + printf("help => do_help() @ 0x%08lx\n", (ulong)do_he> lp);
> + printf("? => do_help() @ 0x%08lx\n", (ulong)do_he> lp);
> +
> +
> + return 0;
> +}
This may be debug code - please mark it as such then, and make sure it
is not included in production builds.
Also, you might want to use a macro here to make code easier to
maintain.
> diff --git a/board/eNET/flash.c b/board/eNET/flash.c
> new file mode 100644
> index 0000000..168477b
> --- /dev/null
> +++ b/board/eNET/flash.c
> @@ -0,0 +1,641 @@
> +/*
> + * (C) Copyright 2002, 2003
> + * Daniel Engström, Omicron Ceti AB, daniel at omicron.se
> + *
> + * (C) Copyright 2002
> + * Sysgo Real-Time Solutions, GmbH <www.elinos.com>
> + * Alex Zuepke <azu at sysgo.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 <asm/io.h>
> +#include <pci.h>
> +#include <asm/ic/sc520.h>
> +
> +#define PROBE_BUFFER_SIZE 1024
> +static unsigned char buffer[PROBE_BUFFER_SIZE];
> +
> +#define SC520_MAX_FLASH_BANKS 3
> +#define SC520_FLASH_BANK0_BASE 0x38000000 /* BOOTCS */
> +#define SC520_FLASH_BANK1_BASE 0x10ffffff /* ROMCS0 */
> +#define SC520_FLASH_BANK2_BASE 0x11ffffff /* ROMCS1 */
> +#define SC520_FLASH_BANKSIZE 0x1000000
> +
> +#define AMD29LV016B_SIZE 0x80000
> +#define AMD29LV016B_SECTORS 32
This looks suspicuously like CFI conformant flash chips. Please use
the CFI driver instead.
Please resend after processing the comments. Thanks.
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
"It ain't so much the things we don't know that get us in trouble.
It's the things we know that ain't so." - Artemus Ward aka Charles
Farrar Brown
More information about the U-Boot
mailing list