Re: [U-Boot] [PATCH] add support for Toradex Colibri PXA300 module

[Jean-Christophe PLAGNIOL-VILLARD]

On 18:42 Sat 29 Nov     , Daniel Mack wrote:
> Hi,
> 
> this patch adds support for Toradex' "Colibri PXA300" module. I had to
> modify the AX88796 driver a bit to make it work, but that goes in a
> different patch.
> 
some general comment
please becarefull on the 80 chars limit
please check all the whitespace
please replace all hardcoded value with macro
> Also, the board code does not support NAND yet. I'll implement the code
> from Oliver Ford soon.
> 
> Signed-off-by: Daniel Mack <[EMAIL PROTECTED]>
> 
> 
> diff --git a/MAKEALL b/MAKEALL
> index dbed268..868205f 100755
> --- a/MAKEALL
> +++ b/MAKEALL
> @@ -554,6 +554,7 @@ LIST_at91="               \
>  
>  LIST_pxa="           \
>       cerf250         \
> +     colibri_pxa300  \
>       cradle          \
>       csb226          \
>       delta           \
> diff --git a/Makefile b/Makefile
> index befb608..ee94d17 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -2804,6 +2804,9 @@ actux4_config   :       unconfig
>  cerf250_config :     unconfig
>       @$(MKCONFIG) $(@:_config=) arm pxa cerf250
>  
> +colibri_pxa300_config:       unconfig
> +     @$(MKCONFIG) $(@:_config=) arm pxa colibri_pxa300
> +
>  cradle_config        :       unconfig
>       @$(MKCONFIG) $(@:_config=) arm pxa cradle
>  
 +DECLARE_GLOBAL_DATA_PTR;
> +
> +/*
> + * Miscelaneous platform dependent initialisations
> + */
> +
> +int board_init (void)
> +{
> +     /* arch number for linux kernel */
> +     gd->bd->bi_arch_number = MACH_TYPE_COLIBRI300;
> +
> +     /* adress of boot parameters */
> +     gd->bd->bi_boot_params = 0xa0000100;
> +
> +     return 0;
> +}
> +
> +int board_late_init(void)
> +{
> +     setenv("stdout", "serial");
> +     setenv("stderr", "serial");
why do want to force it?
> +     return 0;
> +}
> +
> +int dram_init (void)
> +{
> +     gd->bd->bi_dram[0].start = PHYS_SDRAM_1;
> +     gd->bd->bi_dram[0].size = PHYS_SDRAM_1_SIZE;
> +     gd->bd->bi_dram[1].start = PHYS_SDRAM_2;
> +     gd->bd->bi_dram[1].size = PHYS_SDRAM_2_SIZE;
> +     gd->bd->bi_dram[2].start = PHYS_SDRAM_3;
> +     gd->bd->bi_dram[2].size = PHYS_SDRAM_3_SIZE;
> +     gd->bd->bi_dram[3].start = PHYS_SDRAM_4;
> +     gd->bd->bi_dram[3].size = PHYS_SDRAM_4_SIZE;
> +     return 0;
is there anyway to detect it?
> +}
> diff --git a/board/colibri_pxa300/config.mk b/board/colibri_pxa300/config.mk
> new file mode 100644
> index 0000000..350ff3d
> --- /dev/null
> +++ b/board/colibri_pxa300/config.mk
> @@ -0,0 +1,2 @@
> +TEXT_BASE = 0x81000000
> +
> diff --git a/board/colibri_pxa300/lowlevel_init.S 
> b/board/colibri_pxa300/lowlevel_init.S
> new file mode 100644
> index 0000000..23155ab
> --- /dev/null
> +++ b/board/colibri_pxa300/lowlevel_init.S
> @@ -0,0 +1,97 @@
> +/*
> + * 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 <config.h>
> +#include <version.h>
> +#include <asm/arch/pxa-regs.h>
> +
for all please replace hardcode value with MACRO
> +/* There is no memory init here since U-Boot was booted from DRAM
> +   already. You need to implement a first-stage bootloader to setup
> +   the RAM controller and load contents from NAND if needed. */
please use this style of comment
/*
 *
 */
> +
> +.globl lowlevel_init
> +lowlevel_init:
> +
> +     /*  Configure GPIO pins for UART1 / altFn 1 */
           ^
white please remove
an so on
> +
> +/*
> + * select serial console configuration
> + */
> +#define CONFIG_FFUART               1
> +
> +/* allow to overwrite serial and ethaddr */
> +#define CONFIG_ENV_OVERWRITE
> +#define CONFIG_BAUDRATE              115200
> +
> +/*
> + * BOOTP options
> + */
> +#define CONFIG_BOOTP_BOOTFILESIZE
> +#define CONFIG_BOOTP_BOOTPATH
> +#define CONFIG_BOOTP_GATEWAY
> +#define CONFIG_BOOTP_HOSTNAME
> +
> +/*
> + * Command line configuration.
> + */
> +#include <config_cmd_default.h>
> +
> +#define CONFIG_CMD_NET
> +#define CONFIG_CMD_PING
> +#define CONFIG_CMD_ENV
> +#undef CONFIG_CMD_IMLS
> +
> +#define CONFIG_BOOTDELAY     -1
> +#define CONFIG_NETMASK               255.255.255.0
> +#define CONFIG_IPADDR                192.168.1.52
> +#define CONFIG_SERVERIP              192.168.1.51
please remove network config
> +#define CONFIG_BOOTCOMMAND   "bootm 80000"
> +#define CONFIG_BOOTARGS              "console=ttyS0,115200"
> +#define CONFIG_TIMESTAMP
> +#define CONFIG_EXTRA_ENV_SETTINGS "ethaddr=08:00:3e:26:0a:5b"
> +
Best Regards,
J.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot