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

Eric Jarrige
2011-08-24T04:56:42Z

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@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