On 01/19/2011 09:06 AM, Wolfgang Denk wrote:
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index d7cd09c..3abb4cb 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -554,6 +554,7 @@ Stefano Babic <sba...@denx.de>
>>      ea20            davinci
>>      polaris         xscale
>>      trizepsiv       xscale
>> +    mx35pdk         i.MX35
>>      mx51evk         i.MX51
>>      vision2         i.MX51
> 
> Please sort list.

Agree.

>> diff --git a/MAKEALL b/MAKEALL
>> index a732e6a..31dbfe1 100755
>> --- a/MAKEALL
>> +++ b/MAKEALL
>> @@ -409,6 +409,7 @@ LIST_ARM11="                     \
>>      mx31ads                 \
>>      mx31pdk                 \
>>      mx31pdk_nand            \
>> +    mx35pdk                 \
>>      qong                    \
>>      smdk6400                \
>>      tnetv107x_evm           \
> 
> NAK.  We don't add boards to MAKEALL any more. They get auto-selcted
> from their entry in boards.cfg.

I missed the point, I wil lfix it.

>> -#define X_ARM_MMU_SECTION(abase, vbase, size, cache, buff, access)  \
>> -    {                                                               \
>> -            int i; int j = abase; int k = vbase;                    \
>> -            for (i = size; i > 0 ; i--, j++, k++)                   \
>> -                    ARM_MMU_SECTION(ttb_base, j, k, cache, buff, access); \
>> -    }
> 
> Here and everywhere else: Macros with multiple statements should be
> enclosed in a do - while block.

The patch removes for mistake this file, that does not really exist in
u-boot. I have aklready fixed it.

>> +CONFIG_SYS_TEXT_BASE = 0xA0000000
> 
> NAK.  Please move CONFIG_SYS_TEXT_BASE into board config file and
> ditch config.mk

Thanks, I have understood now how it works.

> 
>> +int checkboard(void)
>> +{
>> +    u32 system_rev = get_cpu_rev();
>> +    u32 board_rev = 0;
>> +    struct ccm_regs *ccm =
>> +            (struct ccm_regs *)IMX_CCM_BASE;
>> +
>> +    puts("Board: MX35 3STACK ");
> 
> Is this the correct board name?

I will change it as MX35PDK

> 
>> +    board_rev = board_detect();
>> +
>> +    /* Print board revision */
>> +    if (board_rev)
>> +            puts("2.0");
>> +    else
>> +            puts("1.0");
> 
> Maybe board_detect() could return the board revision sirectly, so you
> can use a single printf for all this, like:
> 
>       printf("Board: mx35pdk %d.0", board_detect());
> 
> ?
> 
>> +    /* Print CPU revision */
>> +    puts(" i.MX35 ");
>> +    if (system_rev & CHIP_REV_2_0)
>> +            puts("2.0 [");
>> +    else
>> +            puts("1.0 [");
> 
> Eventually something similar could / should be done here?

Yes, this should be (get_cpu_rev() & CHIP_REV_2_0)


> ...
>> +NAND partitions can be recognized enabling in kernel 
>> CONFIG_MTD_REDBOOT_PARTS.
>> +For this board, CONFIG_MTD_REDBOOT_DIRECTORY_BLOCK should be set to 2.
>> +
>> +However, the setup in redboot is not correct and does not use the whole 
>> flash. 
>> +
>> +Better solution is to use the kernel parameter mtdparts. Here the resulting 
>> script to be defined in RedBoot with fconfig:
> 
> Lines too long.  Please fix globally (at least in text).

Ok

> 
> ...
>> +#define     CONFIG_EXTRA_ENV_SETTINGS                                       
>> \
> ...
>> +            "uboot=u-boot.bin\0"                                    \
>> +            "kernel_addr_r=0x80800000\0"                            \
>> +            "kernel=uImage\0"                                       \
> 
> Default locations are "<boardname>/u-boot.bin" resp.
> "<boardname>/uImage".
> 
>> +            "prg_uboot=tftpboot ${loadaddr} ${uboot};"              \
>> +                    "protect off ${uboot_addr} 0xa003ffff;" \
>> +                    "erase ${uboot_addr} 0xa003ffff;"               \
>> +                    "cp.b ${loadaddr} ${uboot_addr} ${filesize};"   \
>> +                    "setenv filesize;saveenv\0"
> 
> We usually split this into "load" and "update" steps, so you don;t
> automatically erase your flash even when the download failed.

I will change it.

Best regards,
Stefano Babic

-- 
=====================================================================
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: off...@denx.de
=====================================================================
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to