[U-Boot] [PATCH] powerpc/CoreNet: add tool to support pbl image build.

[Scott Wood <scottwood@freescale.com>]

On 06/05/2012 12:35 AM, Xie Shaohui-B21989 wrote:
>>> +++ b/board/freescale/corenet_ds/config.mk
>>> @@ -0,0 +1,26 @@
>>> +#
>>> +
>>> +#PBL preamble and RCW header
>>> +aa55aa55 010e0100
>>> +#64 bytes RCW data for P4080, replace it when building image #for
>>> +P3041DS or P5020DS.
>>> +4c580000 00000000 18185218 0000cccc
>>> +40464000 3c3c2000 58000000 61000000
>>> +00000000 00000000 00000000 008b6000
>>> +00000000 00000000 00000000 00000000
>>
>> Could you have the tool source this from a separate file, rather than
>> require the user to replace it manually?
>
> [Xie Shaohui] Then I have to prepare a separate file and a tool...
> It is quite simple to replace, just copy and paste, and users may
> need to modify the RCW when the default one does not fit their use
> case, they will always have to do it manually. It's simple to do it
> here.

I disagree that copy and paste is good enough, though taking an external
RCW source (such as output from Timur's tool) could be a future enhancement.

>> Talk to Timur (when he gets back from vacation in a couple weeks) about
>> his RCW tool and how best to accept the output it produces.
>>
>> Why is eSPI in here?  Isn't this supposed to just generically write an
>> image into CPC SRAM?
>
> [Xie Shaohui] No. some interfaces need to be pre-initialized before
> PBL start to load stuff from it, and default configurations for SPI
> is suitable, this tool provides a more compatible configurations.

What do you mean by a more compatible configuration?  If the default is
good enough to load the first few PBL commands, why is it not goot
enough to load the rest?

>>> diff --git a/doc/README.pblimage b/doc/README.pblimage new file mode
>>> 100644 index 0000000..73d90f1
>>> --- /dev/null
>>> +++ b/doc/README.pblimage
>>> @@ -0,0 +1,140 @@
>>> +	3). Boot from Nand
>>> +	Write u-boot.pbl to Nand from offset 0x0, Note that in case of eLBC
>> NAND
>>> +	flash, the address starts from the first good block.
>>> +	for ex in u-boot:
>>> +	=>tftp 100000 u-boot.pbl
>>> +	=>nand info
>>> +	=>nand erase 0 100000
>>> +	=>nand write 100000 0 $filesize
>>> +	Change SW1[1:5] = off on off off on
>>> +	Change SW7[1:4] = on off off on
>>
>> How do you load the environment?  We should find a way, possibly using SPL,
>> to have the environment ready early.  We do not want to wait post
>> relocation for the full NAND/SD/SPI driver to load the environment.
>
> [Xie Shaohui] This tool did not intend to provide a way to load the
> environment. The ENV thing should belong to the boot Image, this tool
> is a wrapper.

This isn't a comment on this tool specifically, but rather on this
method of booting.  This is a problem that needs to be solved.

>> Please fix whitespace.
> [Xie Shaohui] OK. Thanks. I did not find it when using checkpatch.pl

No tool can find every possible error.

>>> +	while ((getline(&line, &len, fd)) > 0) {
>>> +		lineno++;
>>> +		token = strtok_r(line, "\r\n", &saveptr1);
>>
>> Why do you need to tokenize if you're using getline()?
> [Xie Shaohui] I need to drop empty lines.

strtok is a bit heavy handed for that, but whatever...

-Scott