From: Guillaume Gardet <guillaume.gardet@free.fr>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] ARM: rpi: Add CONFIG_ENV_VARS_UBOOT_RUNTIME_CONFIG support
Date: Fri, 21 Aug 2015 11:47:31 +0200 [thread overview]
Message-ID: <55D6F3B3.2000009@free.fr> (raw)
In-Reply-To: <55D3F480.5080809@wwwdotorg.org>
Le 19/08/2015 05:14, Stephen Warren a ?crit :
> On 08/18/2015 08:03 AM, Guillaume GARDET wrote:
>> Add CONFIG_ENV_VARS_UBOOT_RUNTIME_CONFIG support to set 'board_rev' and
>> 'board_name' envs.
> That states what the patch does rather than why its useful to do it. Can
> you expand on why it's useful to set these variables?
Using boot scripts you may need to get the board version / revision infos, for example to select the right DTB since u-boot DTB names and kernel DTB files do not match.
>
>> diff --git a/board/raspberrypi/rpi/rpi.c b/board/raspberrypi/rpi/rpi.c
>> @@ -240,6 +240,12 @@ int misc_init_r(void)
>> {
>> set_fdtfile();
>> set_usbethaddr();
>> +#ifdef CONFIG_ENV_VARS_UBOOT_RUNTIME_CONFIG
>> + char str_rev[11];
>> + sprintf(str_rev, "0x%X", rpi_board_rev);
>> + setenv("board_rev", str_rev);
>> + setenv("board_name", models[rpi_board_rev].name);
>> +#endif
>> return 0;
>> }
> That adds a variable declaration in the middle of code. I'd suggest
> moving the new code into set_board_info() (a name that some other
> boards/SoCs that honor CONFIG_ENV_VARS_UBOOT_RUNTIME_CONFIG use), and
> calling that function inside the ifdef in misc_init_r(). You can always
> make the function static and implement it before misc_init_r() so that
> the compiler is likely to inline it.
Ok, will do that.
>
> I'm not sure that models[rpi_board_rev].name contains values that are
> useful to place into an environment variable. It depends what you expect
> to do with that variable. Note that the values are not unique, and
> contain spaces, which might make the value hard to use and/or not
> reliable to differentiate between all the different types of boards.
>
> Conceptually I have no general objection to this patch, although I am a
> little worried about turning the board_name variable into some kind of ABI.
>
board_name variable should be considered to get a pretty board info print, not any kind of ABI. If people need to differentiate boards revs, just use board_rev variable.
Guillaume
next prev parent reply other threads:[~2015-08-21 9:47 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-08-18 14:03 [U-Boot] [PATCH] ARM: rpi: Add CONFIG_ENV_VARS_UBOOT_RUNTIME_CONFIG support Guillaume GARDET
2015-08-19 3:14 ` Stephen Warren
2015-08-21 9:47 ` Guillaume Gardet [this message]
2015-08-26 1:34 ` Stephen Warren
2015-08-26 1:46 ` Ian Lepore
2015-08-26 1:59 ` Stephen Warren
2015-08-25 13:10 ` [U-Boot] [PATCH V2] " Guillaume GARDET
2015-08-26 1:40 ` Stephen Warren
2015-10-24 21:15 ` [U-Boot] [U-Boot, " Tom Rini
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=55D6F3B3.2000009@free.fr \
--to=guillaume.gardet@free.fr \
--cc=u-boot@lists.denx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.