[U-Boot] [PATCH] arm: at91: support for the Calao USB-A9263 board (based on AT91SAM9263)

[Mateusz Kulikowski]

Dear Andreas Bießmann,


On 29.10.2013 10:12, Andreas Bießmann wrote:
>>> +
>>> +/*define the area offsets*/
>>> +dataflash_protect_t area_list[NB_DATAFLASH_AREA] = {
>>> +    {0x00000000, 0x00001FFF, FLAG_PROTECT_SET, 0, "Bootstrap"},
>>> +    {0x00002000, 0x00003FFF, FLAG_PROTECT_CLEAR, 0, "Environment"},
>>> +    {0x00004000, 0xFFFFFFFF, FLAG_PROTECT_SET, 0, "U-Boot"},
>>> +};
>
> I would really like to see that removed. The dataflash thing is another
> part on my todo list. The common mtd code can handle these devices too
> (see for example at91sam9n12ek, at91sam9x5ek, ethernut5 sam5d3xek and
> top9000). I did some short testing on that for at91sam9263ek (at home,
> never sent to the list) and it worked basically. Will see if I find it
> this evening, clean up and post it this evening CET.

What kind of Dataflash have you used?

Dataflash on Calao (at45db021b) does not have read-id command.
It also has different command set than more recent version (*021d).
It may be tricky to probe and operate it without modifying SPI drivers.

For now I will move partition code to "main" board file;
If I figure out how to handle that Dataflash - I'll update the code.
Unless I'm missing something?

>>> +
>>> +int board_eth_init(bd_t *bis)
>>> +{
>>> +    int rc = 0;
>>> +
>>> +    (void)bis;
>>
>> No need this.
>
> A good way to avoid 'unused parameter' warnings is to just leave the
> parameter name out (i.e. use 'int board_eth_init(bd_t *)').

AFAIK this doesn't go well with C, and will not work on default compile flags set by u-boot.

>>> +#define CONFIG_SYS_TEXT_BASE 0x23f00000
>>
>> This address should be considered as u-boot is top down map, so if your
>> system only 64MiB, there is only 1MiB left.
>
> As mentioned in another mail. Just one point more. It is up to you to
> set a correct value here. It may be ok for your setup to just leave 1MiB
> to top of RAM, but I doubt it really works in any case cause of the
> relocation. Please hack the code and print out the '__image_copy_start'
> and '__image_copy_end' symbol position before and after relocate_code().
> Beware, arm moved to generic relocation, you have to look up
> common/board_[fr].c.
> Also it may happen that the relocation clobbers some stuff in rel_dyn or
> bss sections.
> If you can prove it will not collide at that position I'm fine to take
> it as is.

Thanks for the tip about board_[fr] - I was not aware of it and used old boot mechanism.
I'll update config file to use generic board code.

As for the memory concerns, I did little test:
- enabled debugs in board_[fr].c (they print nicely memory layout),
- Filled u-boot binary with commands/garbage to reach max size supported by AT91bootstrap (200704)
- Flashed, booted the board. Below - log (parts):
<log>
U-Boot 2013.10-00122-g509dca7-dirty (Nov 01 2013 - 20:09:46)

U-Boot code: 23F00000 -> 23F2C570  BSS: -> 23F31498
CPU: AT91SAM9263
Crystal frequency:       12 MHz
CPU clock        :      180 MHz
Master clock     :       90 MHz
DRAM:  Monitor len: 00031498
Ram size: 04000000
Ram top: 24000000
TLB table from 23ff0000 to 23ff4000
Reserving 197k for U-Boot at: 23fbe000
Reserving 152k for malloc() at: 23f98000
Reserving 84 Bytes for Board Info at: 23f97fac
Reserving 176 Bytes for Global Data at: 23f97efc

RAM Configuration:
Bank #0: 20000000
DRAM:  64 MiB
New Stack Pointer is: 23f97ed0
Relocation Offset is: 000be000
Relocating to 23fbe000, new gd at 23f97efc, sp at 23f97ed0
image_copy: 23f00000 23f2c570 <- @setup_reloc()
image_copy: 23fbe000 23fea570 <- @board_init_r()
WARNING: Caches not enabled
Now running in RAM - U-Boot at: 23fbe000
</log>

If I understand correctly, this means that we have plenty of space between start of stack, and end of non-relocated U-Boot (.

>>> (CONFIG_SYS_DATAFLASH_LOGIC_ADDR_CS0 + 0x4000)
>>> +#define CONFIG_ENV_OFFSET        0x2000
>>> +#define CONFIG_ENV_ADDR        (CONFIG_SYS_DATAFLASH_LOGIC_ADDR_CS0 + \
>>> +                 CONFIG_ENV_OFFSET)
>>> +#define CONFIG_ENV_SIZE        0x2000
>>> +#define CONFIG_BOOTCOMMAND    "nboot 21000000 0"
>>> +#define CONFIG_BOOTARGS        "console=ttyS0,115200 " \
>>> +    "root=/dev/mtdblock1 " \
>>> +    "mtdparts=atmel_nand:16m(kernel)ro,120m(root1),-(root2)"\
>
> Just a pointer ... you could use the CONFIG_CMD_MTDPARTS define. The
> same mtdparts provided to the kernel could be used in u-boot -> 'nboot
> kernel'. Some newer at91 boards use that, I personally find it quite useful.

I'll do it after I have SPL working - it will not fit now.

>>> +
>>> +/*
>>> + * Size of malloc() pool
>>> + */
>>> +#define CONFIG_SYS_MALLOC_LEN    ROUND(3 * CONFIG_ENV_SIZE +
>>> 128*1024, 0x1000)
>
> Malloc is 152KiB, gd_t is smaller than 1KiB, your u-boot is smaller than
> 196KiB (as you mention in some comment) ... It may fit into the 1MiB
> gap, if there is no other runtime defined parameters changing the
> relocation address ...
> But this is only true if you do not add more features, for example
> ubi(fs) which has some more runtime code and requires more than 512KiB
> malloc arena, about 4MiB recommended. Yust my 2¢, you should really have
> a look for ubi(fs) when using NAND devices ...
It will not be possible to add more features until I get rid of AT91bootstrap.
I prefer to stick to config that may used on "stock" boards (kernel is located on separate partition, so no FS driver is needed)

>
> Apart from that your patch looks good. Please address my concern about
> dataflash driver and Bo's comments and send a v2 for inclusion.

I will post v2 soon.

Best Regards,
Mateusz