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

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to