[U-Boot] [PATCH 5/9] apf9328: add default board configuration file

[Eric Jarrige]

Dear Wolfgang, Dear Stefano,

On 23 août 2011, at 13:26, Wolfgang Denk wrote:

> Dear Eric Jarrige,
> 
> can you please mind your line length?  It is strongly recommended that
> text lines should not exceed 70 characters or so.  Thanks.

I will take care of that. I do apologize for the inconvenient.

> 
> In message <1FAD4112-2A57-415D-9CDF-8FC7A9EDB626 at armadeus.org> you wrote:
> 
>>>> +#define CONFIG_EXTRA_ENV_SETTINGS \
>>>> +	"env_version="		CONFIG_ENV_VERSION		"\0"	\
>>>> +	"fileaddr="		MK_STR(CONFIG_SYS_LOAD_ADDR)	"\0"	\
>>>> +	"filesize="		MK_STR(CONFIG_SYS_MONITOR_LEN)	"\0"	\
>>> 
>>> filesize is dynamically computed, you should not add it
>> 
>> We need this variable already initialized at boot time to support the specific imx boot mode from serial port when the user lost the full content of the flash.
> 
> Stefano is right.  "filesize" and "fileaddr" are dynamic variables,
> thet get created and updated on the fly. It makes no sense to
> pre-define them to any specific value. [Actually I have some changes
> in mind that avoid to save such variables with the environment at
> all.]

I understand - It make sense for me to wait for the new interface. 

> 
>> In such case (that is a stressing situation for the user) he can push U-Boot through the serial port and use directly the script "flash_uboot" to recover the original content of the flash. In such a use case this variable is not dynamically computed.
> 
> What do you mean by "push U-Boot through the serial port"?  Any of the
> respective commands in U-Boot ("loads", "loadb", "loady") will
> automatically update the respective variables.

I meant  to boot using the iMX internal bootstrap loader as U-Boot is not
yet running. Alternatively I can download U-Boot a second time to have
the environment variables initialized. 
It is just a longer process than the original one.

> 
>>>> +#define CONFIG_NETMASK		255.255.255.0
>>>> +#define CONFIG_IPADDR		192.168.000.10
>>> 
>>> Please drop fix ip address. They should not be part of mainline,
>> 
>> Here, I have a problem as our documentation on the wiki armadeus.org is based on this default IP addresses.
> 
> Then fix the documentation?

That is something we will do in any case.

> 
> It is a somewhat bad idea to publish documentation to end users before
> the code has passed even the inital code review...

Thanks for the recommendation.

> 
>> We really need a set of default IP addresses for private network to simplify as much as possible the life of the armadeus project developers. 
> 
> And what makes you think that 192.168.0.10 might be a free IP address
> in my network?  Or assume that 255.255.255.0 is the right netmask in
> this network?

http://tools.ietf.org/html/rfc1918#page-4

> 
> These addresses may work for you, but they will not work for others,
> and thus it makes no sense to use them as defaults.

The armadeus BSP provide a user interface to customize the network
parameters according to end user network environment.

> 
> If you need a reasonable default, then configure the board to use
> DHCP.
> 
>> I never seen such a restriction in U-Boot doc and moreover there is a set CONFIG_XXIP documented in U-BOOT that confusing me,
> 
> There are situations where this cannot be avoided - like when U-Boot
> is installed in a ROM, where the environment cannot be changed at all,
> and where for some reason DHCP or similar is not possible.  But that
> does not mean that this makes sense for the generic case.
> 
>> Is there another solution for a complete usable default configuration?
> 
> Use DHCP?

Sure, DHCP is the best *technical* solution.
I meant for static IP address?

> 
>>> Why do we need such a stuff when your configuration is fixed at compile
>>> time with CONFIG_SYS_SDRAM_MBYTE_SYZE 16 ?
>> 
>> 16MiB is the regular configuration but there are many configurations of boards with different size of memory.
>> This set of parameters enables to support every boards at compilation by just changing the value of CONFIG_SYS_SDRAM_MBYTE_SYZE.
>> So that the binary generated is fully optimized for each board.
> 
> That's a maintenance nightmage for everybody and not needed at all.
> 
> Please use get_ram_size() to have U-Boot auto-adjust to the actual RAM
> size of your board and forget about all such static "optimizations".

Thanks for the update. 

> 
>>>> +#define CONFIG_SYS_INIT_SP_ADDR	(CONFIG_SYS_SDRAM_BASE	\
>>>> +		+ CONFIG_SYS_SDRAM_1_SIZE - 0x0100000)
>>> 
>>> You do not use U_Boot macro here (GENERATED_GBL_DATA_SIZE), as it is
>>> usual. Stack pointer is already in RAM before relocation. Is it correct ?
>> 
>> Correct. I do have to optimize the location of the initial Stack pointer  for my board so I put the init SP at any safe place in RAM.
> 
> I find your answers not really helpful and constructive.  You write
> you "have to optimize the location", but you fail to explain why you
> think so.  I consider it likely that you are wrong about the optimize
> part.  And you are definitely wrong about the rest, as you obviously
> did not understand Stefano's comment at all - he was referring to the
> magic constant 0x0100000 in above macro.

Thanks for the clarification - by pointing the constant 0x0100000 
I understand the message - So forget my previous answer.

For the rest, we will look for the best compromised solution  for everyone.

Best regards,
Eric