[U-Boot-Users] Ideas on U-Boot configuration with FDT

Hello,
here are my ideas on what is basically necessary to use FDT for U-Boot configuration. I tried to keep it simple:
- If CFG_FDT_ADDR is defined in the boards configuration file, the global variable "fdt", already used by the FDT command, will be set at compile time to that address:
#ifdef CFG_FDT_ADDR struct fdt_header *fdt = CFG_FDT_ADDR; else struct fdt_header *fdt = NULL; #endif
- In the early boot phase, before relocation, the FDT is checked and an error message printed in case it's not found or invalid:
CPU: MPC5200 v1.0, Core v1.1 at 396 MHz Bus 132 MHz, IPB 66 MHz, PCI 33 MHz FDT: FDT_ERR_BADMAGIC (or OK, or version number?)
At this level also the compatibility of the FDT tree could be checked using fdt_boardcheck().
- After relocation, the blob is copied into memory via fdt_open_into() to a defined place and "fdt" is updated accordingly. Not sure, what location in RAM to use, but it should fit into the existing scheme. Any recommendations?
- When U-Boot is up, "fdt" points to a valid FDT, e.g. "fdt addr" is not necessary.
- I think the basic mechanism should also work _without_ FDT commands (cmd_fdt.c, getting ride of quit some code).
- bootm should then automatically use the address of "fdt" to boot the kernel.
Some more comments. I realized rather heavy consistency checks on almost any public FDT function via fdt_check_header. I actually would prefer to have one extended check function at the beginning, which also ensures the compatibility with the board and reduce the other checks to just verify the magic number.
Other related issues? Do we also need to maintain a checksum of the blob? At least it's on Jerry's to-do list.
What do you think? I'm going to show a first patch soon.
Wolfgang.

Wolfgang Grandegger wrote:
CPU: MPC5200 v1.0, Core v1.1 at 396 MHz Bus 132 MHz, IPB 66 MHz, PCI 33 MHz FDT: FDT_ERR_BADMAGIC (or OK, or version number?) At this level also the compatibility of the FDT tree could be checked using fdt_boardcheck().
I think fdt_checkboard() (or boardcheck) should be run from inside the fdt_open_into() command. This takes advantage of the existing mechanism of fdt_open_into() to return an error. It also allows for a device tree to be opened after U-Boot has booted.
- After relocation, the blob is copied into memory via fdt_open_into() to a defined place and "fdt" is updated accordingly.
So we'll need to have CFG_FDT_ADDR_FLASH and CFG_FDT_ADDR_RAM.
Not sure, what location in RAM to use, but it should fit into the existing scheme. Any recommendations?
- When U-Boot is up, "fdt" points to a valid FDT, e.g. "fdt addr" is not necessary.
I think this feature needs to be optional. That is, if CFG_FDT_ADDR_xxx are defined, then we can enable this feature. Otherwise, fdt_addr will still be necessary. Remember, we'll still need to support architectures that don't have device trees.

Timur Tabi wrote:
Wolfgang Grandegger wrote:
CPU: MPC5200 v1.0, Core v1.1 at 396 MHz Bus 132 MHz, IPB 66 MHz, PCI 33 MHz FDT: FDT_ERR_BADMAGIC (or OK, or version number?) At this level also the compatibility of the FDT tree could be checked using fdt_boardcheck().
I think fdt_checkboard() (or boardcheck) should be run from inside the fdt_open_into() command. This takes advantage of the existing mechanism of fdt_open_into() to return an error. It also allows for a device tree to be opened after U-Boot has booted.
But it's too late for initial initialization (before RAM is available). Using the FDT directly from ROM is not recommended because access might be slow, but we likely need it for early initialization.
- After relocation, the blob is copied into memory via fdt_open_into() to a defined place and "fdt" is updated accordingly.
So we'll need to have CFG_FDT_ADDR_FLASH and CFG_FDT_ADDR_RAM.
OK.
Not sure, what location in RAM to use, but it should fit into the existing scheme. Any recommendations?
- When U-Boot is up, "fdt" points to a valid FDT, e.g. "fdt addr" is not necessary.
I think this feature needs to be optional. That is, if CFG_FDT_ADDR_xxx are defined, then we can enable this feature. Otherwise, fdt_addr will still be necessary. Remember, we'll still need to support architectures that don't have device trees.
Exactly, that's also what I meant.
Wolfgang.

Wolfgang Grandegger wrote:
I think fdt_checkboard() (or boardcheck) should be run from inside the fdt_open_into() command. This takes advantage of the existing mechanism of fdt_open_into() to return an error. It also allows for a device tree to be opened after U-Boot has booted.
But it's too late for initial initialization (before RAM is available).
Just make it so that fdt_open_into() is called early:
checkboard, INIT_FUNC_WATCHDOG_INIT #if defined(CFG_FDT_ADDR_FLASH) init_fdt, #endif #if defined(CONFIG_MISC_INIT_F) misc_init_f, #endif INIT_FUNC_WATCHDOG_RESET #if defined(CONFIG_HARD_I2C) || defined(CONFIG_SOFT_I2C) init_func_i2c, #endif
and init_fdt() calls fdt_open_into().
Using the FDT directly from ROM is not recommended because access might be slow, but we likely need it for early initialization.
I don't access speed is important for the reading the FDT, especially flash vs. RAM. It's not that much slower.

Timur Tabi wrote:
Wolfgang Grandegger wrote:
I think fdt_checkboard() (or boardcheck) should be run from inside the fdt_open_into() command. This takes advantage of the existing mechanism of fdt_open_into() to return an error. It also allows for a device tree to be opened after U-Boot has booted.
But it's too late for initial initialization (before RAM is available).
Just make it so that fdt_open_into() is called early:
checkboard, INIT_FUNC_WATCHDOG_INIT
#if defined(CFG_FDT_ADDR_FLASH) init_fdt, #endif #if defined(CONFIG_MISC_INIT_F) misc_init_f, #endif INIT_FUNC_WATCHDOG_RESET #if defined(CONFIG_HARD_I2C) || defined(CONFIG_SOFT_I2C) init_func_i2c, #endif
and init_fdt() calls fdt_open_into().
RAM is _not_ available at that stage.
Using the FDT directly from ROM is not recommended because access might be slow, but we likely need it for early initialization.
I don't access speed is important for the reading the FDT, especially flash vs. RAM. It's not that much slower.
Depends, you might be right for FLASH on a 32-bit bus. On slow ROM devices and slow processors, it matters.
Wolfgang.

Wolfgang Grandegger wrote:
RAM is _not_ available at that stage.
Do we really need RAM to parse the device tree and configure U-Boot? I think not.
The device tree has a memory section:
memory { device_type = "memory"; reg = <00000000 10000000>; // 256MB };
So the question is: do we want U-Boot to use this node to determine how much RAM there is, or do we want U-Boot to determine how much RAM there is and update this node?
Depends, you might be right for FLASH on a 32-bit bus. On slow ROM devices and slow processors, it matters.
I still don't see how it's important. If the device tree is on slow memory, so what? You have to read it sooner or later. If your board can't handle this feature, then don't enable it, and configure U-Boot the "old fashioned way".

Timur Tabi wrote:
Wolfgang Grandegger wrote:
RAM is _not_ available at that stage.
Do we really need RAM to parse the device tree and configure U-Boot? I think not.
But you need it anyhow in RAM for booting Linux. Why not doing it early, e.g. after relocation when memory is available.
The device tree has a memory section:
memory { device_type = "memory"; reg = <00000000 10000000>; // 256MB };
So the question is: do we want U-Boot to use this node to determine how much RAM there is, or do we want U-Boot to determine how much RAM there is and update this node?
Both makes sense, I think. How we finally use FDT it U-Boot is not yet clear, but exhaustive usage like in the kernel seems not attractive to me. Currently I just need it do enable some devices dynamically, like PCMCIA, RTC and CAN and configure the LCD controller for various panels.
Depends, you might be right for FLASH on a 32-bit bus. On slow ROM devices and slow processors, it matters.
I still don't see how it's important. If the device tree is on slow memory, so what? You have to read it sooner or later. If your board can't handle this feature, then don't enable it, and configure U-Boot the "old fashioned way".
Don't like the "don't care" argument. If you can make your code faster, just do it.
Wolfgang.

Wolfgang Grandegger wrote:
But you need it anyhow in RAM for booting Linux. Why not doing it early, e.g. after relocation when memory is available.
Ok.
So the question is: do we want U-Boot to use this node to determine how much RAM there is, or do we want U-Boot to determine how much RAM there is and update this node?
Both makes sense, I think.
Well, we can't do both. We have to do one or the other. I think we should have U-Boot determine the amount of RAM and update the device tree accordingly.
How we finally use FDT it U-Boot is not yet clear, but exhaustive usage like in the kernel seems not attractive to me. Currently I just need it do enable some devices dynamically, like PCMCIA, RTC and CAN and configure the LCD controller for various panels.
Once we let the cat out of the bag, there's no telling what people will do. My guess is that we will probably never reach the kernel's level of usage, but we'll just keep getting closer and closer.
Don't like the "don't care" argument. If you can make your code faster, just do it.
I'm okay with waiting until RAM is enabled and then copying the device tree into RAM before using it. But again, that assumes that CFG_FDT_ADDR_RAM is defined.

In message 464DBFC8.4050708@freescale.com you wrote:
So the question is: do we want U-Boot to use this node to determine how much RAM there is, or do we want U-Boot to determine how much RAM there is and update this node?
Tradition is that U-Boot will determine how much RAM there is, and I definitely want to keep it that way. Among other things, this is a cheap and yet still pretty efficient protection against most types of RAM errors.
I still don't see how it's important. If the device tree is on slow memory, so what? You
Timu, there is lots of code in U-Boot that you haven't seen yet...
have to read it sooner or later. If your board can't handle this feature, then don't enable it, and configure U-Boot the "old fashioned way".
Having to read it later (after relocation into RAM) may be *significantly* faster. We can then use buffering, while we may have to use repeated small reads (repeated for each element we're looking up) while running from ROM. Remember that there is more things than just NOR flash. Data may be stored in slow serial devices like EEPROM on I2C, or serial flash on SPI, or NAND flash, etc.
Best regards,
Wolfgang Denk

Timur Tabi wrote:
Wolfgang Grandegger wrote:
In the early boot phase, before relocation, the FDT is checked and an error message printed in case it's not found or invalid:
CPU: MPC5200 v1.0, Core v1.1 at 396 MHz Bus 132 MHz, IPB 66 MHz, PCI 33 MHz FDT: FDT_ERR_BADMAGIC (or OK, or version number?)
At this level also the compatibility of the FDT tree could be checked using fdt_boardcheck().
I think fdt_checkboard() (or boardcheck) should be run from inside the fdt_open_into() command. This takes advantage of the existing mechanism of fdt_open_into() to return an error. It also allows for a device tree to be opened after U-Boot has booted.
NO. fdt_open_into() is part of libfdt, which is a _generic_ fdt library. fdt_check_board() would be board specific and any call-out to it from the generic libfdt would be a mistake because it would make libfdt u-boot specific.
If we implement a fdt_checkboard(), I agree with Wolfgang that it would be called early in the board-specific start up code.
If we implement a fdt_checkboard(), I would create a new subcommand for it. While it could be added to the "fdt addr" command to verify the user pointed to a valid fdt, I would strongly resist that. "UNIX was not designed to stop its users from doing stupid things, as that would also stop them from doing clever things." – Doug Gwyn http://en.wikipedia.org/wiki/Unix_philosophy#Quotes
The above quote is another reason for my shouting "NO" above.
- After relocation, the blob is copied into memory via fdt_open_into() to a defined place and "fdt" is updated accordingly.
So we'll need to have CFG_FDT_ADDR_FLASH and CFG_FDT_ADDR_RAM.
Not sure, what location in RAM to use, but it should fit into the existing scheme. Any recommendations?
- When U-Boot is up, "fdt" points to a valid FDT, e.g. "fdt addr" is not necessary.
I think this feature needs to be optional. That is, if CFG_FDT_ADDR_xxx are defined, then we can enable this feature. Otherwise, fdt_addr will still be necessary. Remember, we'll still need to support architectures that don't have device trees.
"fdt addr" (no underscore) is the subcommand that allows the user to change the fdt address (implementation: it sets the C variable "fdt"). I would be reluctant to remove the "addr" subcommand because the user/scripts may want to change which fdt is used. Given Wolfgang's thoughts/proposals above, it will not be used very often, but it would still be useful for developers.
"fdt addr" would also be more useful if "fdt addr" printed out the current value of the variable "fdt" (I don't believe it is implemented this way currently).
Best regards, gvb

Jerry Van Baren wrote:
NO. fdt_open_into() is part of libfdt, which is a _generic_ fdt library. fdt_check_board() would be board specific and any call-out to it from the generic libfdt would be a mistake because it would make libfdt u-boot specific.
I can understand that, but then we have a problem. We don't want to be able to use a device tree without checking it. We have to implement some kind of mechanism to ensure that fdt_checkboard() is called before the kernel is booted.
If we implement a fdt_checkboard(), I agree with Wolfgang that it would be called early in the board-specific start up code.
But *after* fdt_open_into() is called. Perhaps we need a higher-level version of fdt_open_into()? And perhaps "fdt addr" should call this function instead?
If we implement a fdt_checkboard(), I would create a new subcommand for it. While it could be added to the "fdt addr" command to verify the user pointed to a valid fdt, I would strongly resist that.
Ok.
"UNIX was not designed to stop its users from doing stupid things, as that would also stop them from doing clever things." – Doug Gwyn
U-Boot isn't Unix.
"fdt addr" (no underscore) is the subcommand that allows the user to change the fdt address
*change* the fdt address? I'm not sure that's the best way of handling it. The fdt library should simply be told where the FDT is located. Moving the FDT around in memory is probably beyond its scope.

Timur Tabi wrote:
Jerry Van Baren wrote:
[snip]
"UNIX was not designed to stop its users from doing stupid things, as that would also stop them from doing clever things." – Doug Gwyn
U-Boot isn't Unix.
Indeed. However, the above quote is one of Wolfgang Denk's favorites. Don't diss it, Wolfgang will get p1ssed. ;-)
"fdt addr" (no underscore) is the subcommand that allows the user to change the fdt address
*change* the fdt address? I'm not sure that's the best way of handling it. The fdt library should simply be told where the FDT is located. Moving the FDT around in memory is probably beyond its scope.
Yes, that is what "fdt addr" does today, it tells the u-boot innards where the blob is (sets/changes the blob location). The user can download a new blob and switch to using it via the "fdt addr" command.
Your statment "The fdt library should simply be told where the FDT is located." doesn't have meaning in this context: 1) We are talking about the user command "fdt", not the C code that implements the use and manipulation of flattened device trees 2) All of the libfdt functions take the blob address as the first parameter so your statement is accurate already.
The user can use the "fdt addr" command to set/change the location of the blob. Moving blobs is not out of scope, it is what "fdt move" does. It's all in there and useful.
I think you are confusing the "fdt" command (with lots of subcommands) with the C code implementation.
Best regards, gvb

Jerry Van Baren wrote:
Yes, that is what "fdt addr" does today, it tells the u-boot innards where the blob is (sets/changes the blob location). The user can download a new blob and switch to using it via the "fdt addr" command.
By "change", I thought you meant that "fdt addr" would actually move the device tree, since that's technically changing the address. Perhaps you meant "set the address"?
The user can use the "fdt addr" command to set/change the location of the blob. Moving blobs is not out of scope, it is what "fdt move" does.
How is "fdt move" different than cp.b followed by another "fdt addr"?

Timur Tabi wrote:
Jerry Van Baren wrote:
Yes, that is what "fdt addr" does today, it tells the u-boot innards where the blob is (sets/changes the blob location). The user can download a new blob and switch to using it via the "fdt addr" command.
By "change", I thought you meant that "fdt addr" would actually move the device tree, since that's technically changing the address. Perhaps you meant "set the address"?
Yes.
The user can use the "fdt addr" command to set/change the location of the blob. Moving blobs is not out of scope, it is what "fdt move" does.
How is "fdt move" different than cp.b followed by another "fdt addr"?
Conceptually the same thing but easier: you don't have to know (guess) the blob size because fdt_open_into() gets the size from the source blob.
gvb

Jerry Van Baren wrote:
Timur Tabi wrote:
Jerry Van Baren wrote:
Yes, that is what "fdt addr" does today, it tells the u-boot innards where the blob is (sets/changes the blob location). The user can download a new blob and switch to using it via the "fdt addr" command.
By "change", I thought you meant that "fdt addr" would actually move the device tree, since that's technically changing the address. Perhaps you meant "set the address"?
Yes.
The user can use the "fdt addr" command to set/change the location of the blob. Moving blobs is not out of scope, it is what "fdt move" does.
How is "fdt move" different than cp.b followed by another "fdt addr"?
Conceptually the same thing but easier: you don't have to know (guess) the blob size because fdt_open_into() gets the size from the source blob.
Summing up, brainstorming a bit more, here are my revised ideas:
The following definitions control the FDT usage in U-Boot:
CFG_FDT_ADDR_FLASH: If defined, "fdt" is set to that address at compile time. The FDT can be used from the early beginning of the boot.
CFG_FDT_ADDR_BY_ENV: If defined, the env variable "fdtaddr" is looked up early in the boot and "fdt" is set accordingly. This allows to hold more than one blob in FLASH and select one via env setting. This would allow for _one_ combination of images of U-Boot + Linux + Blobs for a _set_ of boards.
CFG_FDT_ADDR_RAM: If defined, the blob is moved to RAM after relocation for further preparation or for performance reasons. "fdt" is re-set accordingly. The blob is then ready and in place for booting Linux. If CFG_FDT_ADDR_RAM is set to 0, the blob will be copied to a default location, e.g. before the initrd location.
A board-specific checkboad function is called as early as possible to verify the FDT.
This should also make Timur happy, as he has the choice, e.g. read the FDT solely from FLASH. With this scheme, I also do not see the real need for fdt commands to manipulate the blob in RAM. Read-only support would be a nice to have for debugging, though.
Please comment.
Wolfgang.

In message 46500875.6060706@grandegger.com you wrote:
CFG_FDT_ADDR_FLASH: If defined, "fdt" is set to that address at compile time. The FDT can be used from the early beginning of the boot.
Just nitpicking: maybe we should call this CFG_FDT_ADDR_STATIC or CFG_FDT_ADDR_DEFAULT or CFG_FDT_ADDR_INIT or something like that. Rationale: flash is just one of the possible storage devies - it could be ROM instead, or even RAM in certain configurations.
Also, while we are defining this, please keep in mind that sooner or later someone will come up with the idea of storing the FDT in EEPROM, so we might want to reserve such identifiers.
CFG_FDT_ADDR_BY_ENV: If defined, the env variable "fdtaddr" is looked up early in the boot and "fdt" is set accordingly. This allows to hold more than one blob in FLASH and select one via env setting. This would allow for _one_ combination of images of U-Boot + Linux + Blobs for a _set_ of boards.
Traditionally, that should be named CFG_FDT_ADDR_IN_ENV
CFG_FDT_ADDR_RAM: If defined, the blob is moved to RAM after relocation for further preparation or for performance reasons. "fdt" is re-set accordingly. The blob is then ready and in place for booting Linux. If CFG_FDT_ADDR_RAM is set to 0, the blob will be copied to a default location, e.g. before the initrd location.
I'm not sure I understand this - will CFG_FDT_ADDR_RAM hold the *address* in RAM where the FDT gets copied to? That sounds pretty static to me.
A board-specific checkboad function is called as early as possible to verify the FDT.
"checkboard()" is a name that can mean anything. If the function is to check or to verify the FDT, the function name should represent this, i. e. please callit checkfdt() or verifyfdt() of probably fdtcheck() / fdtverify() or fdt_check() / fdt_verify().
Best regards,
Wolfgang Denk

Wolfgang Denk wrote:
In message 46500875.6060706@grandegger.com you wrote:
CFG_FDT_ADDR_FLASH: If defined, "fdt" is set to that address at compile time. The FDT can be used from the early beginning of the boot.
Just nitpicking: maybe we should call this CFG_FDT_ADDR_STATIC or CFG_FDT_ADDR_DEFAULT or CFG_FDT_ADDR_INIT or something like that. Rationale: flash is just one of the possible storage devies - it could be ROM instead, or even RAM in certain configurations.
Also, while we are defining this, please keep in mind that sooner or later someone will come up with the idea of storing the FDT in EEPROM, so we might want to reserve such identifiers.
Good points, I agree with them. One possibility would be to name it CFG_FDT_ADDR_INIT (my 2c for name choice) and #define it to a static address for the typical flash storage case and #define it to be a function that returns an address for the read-from-EEPROM case.
CFG_FDT_ADDR_BY_ENV: If defined, the env variable "fdtaddr" is looked up early in the boot and "fdt" is set accordingly. This allows to hold more than one blob in FLASH and select one via env setting. This would allow for _one_ combination of images of U-Boot + Linux + Blobs for a _set_ of boards.
Traditionally, that should be named CFG_FDT_ADDR_IN_ENV
Ditto.
CFG_FDT_ADDR_RAM: If defined, the blob is moved to RAM after relocation for further preparation or for performance reasons. "fdt" is re-set accordingly. The blob is then ready and in place for booting Linux. If CFG_FDT_ADDR_RAM is set to 0, the blob will be copied to a default location, e.g. before the initrd location.
I'm not sure I understand this - will CFG_FDT_ADDR_RAM hold the *address* in RAM where the FDT gets copied to? That sounds pretty static to me.
This would be used if the blob has to be copied out of flash into RAM (or could be part of the read-from-EEPROM usage).
I'm not sure this is necessary (echoing wd's "pretty static" reaction). The board-specific code in the start up should know where a good place to place the RAM copy of the blob is, for instance, right after the RAM copy of u-boot. Is there a case where it _must_ be a fixed address? I cannot think of any.
If there is a case where it _must_ be fixed, it seems to me it would be so unusual a case that I would leave it up to the board supporter to hardcode it in the board-specific start up.
My gut feel is to not document/define a CFG_FDT_ADDR_RAM until we get use cases that show that it is a Good Thing[tm].
A board-specific checkboad function is called as early as possible to verify the FDT.
"checkboard()" is a name that can mean anything. If the function is to check or to verify the FDT, the function name should represent this, i. e. please callit checkfdt() or verifyfdt() of probably fdtcheck() / fdtverify() or fdt_check() / fdt_verify().
I'm somewhat skeptical about the fdt_checkboard() concept, how well it would work in practice. If the blob is for the wrong board, will the serial work? Sometimes yes, sometimes no. For the times the answer is "yes", the ability is probably worth considering, at least.
I would like to see the verification/validation that we currently do on the env data to be done on the blob as well. This is heading towards my assertion that the blob can take over most, perhaps all, of the duties of the current "env" storage. Not all of the thoughts below are directly on topic since I'm expanding beyond just u-boot config. * A CRC * Backup copy (if selected in the board config) * Default compiled in copy (?) * Likely not useful since, by definition, some parts of the fdt would be board specific and not detectable by the board start up code. * Hmm, if we had a minimal default compiled in copy, we could compare its values to a user-loadable version as (part of) a fdt_checkboard() function.
Best regards, Wolfgang Denk
Ditto, gvb

Wolfgang Denk wrote:
In message 46500875.6060706@grandegger.com you wrote:
CFG_FDT_ADDR_FLASH: If defined, "fdt" is set to that address at compile time. The FDT can be used from the early beginning of the boot.
Just nitpicking: maybe we should call this CFG_FDT_ADDR_STATIC or CFG_FDT_ADDR_DEFAULT or CFG_FDT_ADDR_INIT or something like that. Rationale: flash is just one of the possible storage devies - it could be ROM instead, or even RAM in certain configurations.
Also, while we are defining this, please keep in mind that sooner or later someone will come up with the idea of storing the FDT in EEPROM, so we might want to reserve such identifiers.
Then something similar to the ENV could be chosen:
CFG_FDT_ADDR CFG_FDT_IS_IN_FLASH CFG_FDT_IS_IN_xxx ...
CFG_FDT_ADDR_BY_ENV: If defined, the env variable "fdtaddr" is looked up early in the boot and "fdt" is set accordingly. This allows to hold more than one blob in FLASH and select one via env setting. This would allow for _one_ combination of images of U-Boot + Linux + Blobs for a _set_ of boards.
Traditionally, that should be named CFG_FDT_ADDR_IN_ENV
OK for me.
CFG_FDT_ADDR_RAM: If defined, the blob is moved to RAM after relocation for further preparation or for performance reasons. "fdt" is re-set accordingly. The blob is then ready and in place for booting Linux. If CFG_FDT_ADDR_RAM is set to 0, the blob will be copied to a default location, e.g. before the initrd location.
I'm not sure I understand this - will CFG_FDT_ADDR_RAM hold the *address* in RAM where the FDT gets copied to? That sounds pretty static to me.
Well, yes, this address must be suitable for Linux and should be treated similar to the initial ramdisk. Therefore a proper default address seems sufficient.
A board-specific checkboad function is called as early as possible to verify the FDT.
"checkboard()" is a name that can mean anything. If the function is to check or to verify the FDT, the function name should represent this, i. e. please callit checkfdt() or verifyfdt() of probably fdtcheck() / fdtverify() or fdt_check() / fdt_verify().
Then checkfdt(). In principle, this could be done in the already existing board-specific checkboard function, but a dedicated function for verifying the FDT makes sense, I think.
Wolfgang.

Wolfgang Grandegger wrote:
Then something similar to the ENV could be chosen:
CFG_FDT_ADDR CFG_FDT_IS_IN_FLASH CFG_FDT_IS_IN_xxx
We need two addresses:
1) The address where the FDT is stored when the system is powered up 2) The address in RAM where the FDT should be placed before Linux is booted.
Therefore, we can't have just CFG_FDT_ADDR. We need CFG_FDT_ADDR_xxxx
My vote is to have xxxx be the type of memory. So if CFG_FDT_ADDR_FLASH is defined to value X, that means that the FDT is stored in flash at address X. If CFG_FDT_ADDR_EEPROM is defined instead, then it means that FDT is located in EEPROM at address X.
This would eliminate the "CFG_FDT_IS_IN_xxx"-type macros, which I think are redundant.
CFG_FDT_ADDR_BY_ENV: If defined, the env variable "fdtaddr" is looked up early in the boot and "fdt" is set accordingly. This allows to hold more than
I would rather that the FDT subsystem *set* the fdtaddr variable, instead of the other way around.
CFG_FDT_ADDR_RAM: If defined, the blob is moved to RAM after relocation for further preparation or for performance reasons. "fdt" is re-set accordingly. The blob is then ready and in place for booting Linux. If CFG_FDT_ADDR_RAM is set to 0, the blob will be copied to a default location, e.g. before the initrd location.
I think we're going to have to always relocate the FDT to RAM. Considering the level of functionality that libfdt provides, and considering how much of that functionality depends on being able to write to the FDT, it makes sense to require it to be in RAM.
"checkboard()" is a name that can mean anything. If the function is
The purpose of the function is to provide board-specific code that validates the FDT. The amount of checking performed is at the discretion of the function.
For example, checkboard() *should* compare the values in the board header file with the FDT to verify that the memory mappings map, for instance.

Timur Tabi wrote:
Wolfgang Grandegger wrote:
Then something similar to the ENV could be chosen:
CFG_FDT_ADDR CFG_FDT_IS_IN_FLASH CFG_FDT_IS_IN_xxx
We need two addresses:
- The address where the FDT is stored when the system is powered up
OK.
- The address in RAM where the FDT should be placed before Linux is
booted.
This should be some kind of default address. Also be aware, that the blob can be within a uImage created with mkimage. Then the load address defined in the uImage should be used.
Therefore, we can't have just CFG_FDT_ADDR. We need CFG_FDT_ADDR_xxxx
My vote is to have xxxx be the type of memory. So if CFG_FDT_ADDR_FLASH is defined to value X, that means that the FDT is stored in flash at address X. If CFG_FDT_ADDR_EEPROM is defined instead, then it means that FDT is located in EEPROM at address X.
This would eliminate the "CFG_FDT_IS_IN_xxx"-type macros, which I think are redundant.
You might be right. The _IS_IN_ is used for the ENV, I have to check what the rational behind it is (if there is one at all).
CFG_FDT_ADDR_BY_ENV: If defined, the env variable "fdtaddr" is looked up early in the boot and "fdt" is set accordingly. This allows to hold more than
I would rather that the FDT subsystem *set* the fdtaddr variable, instead of the other way around.
I'm not sure if you understand the intended purpose. The address of the _initial_ blob could be stored in the env variable "fdtaddr" to select _one_ blob out of many in the FLASH.
CFG_FDT_ADDR_RAM: If defined, the blob is moved to RAM after relocation for further preparation or for performance reasons. "fdt" is re-set accordingly. The blob is then ready and in place for booting Linux. If CFG_FDT_ADDR_RAM is set to 0, the blob will be copied to a default location, e.g. before the initrd location.
I think we're going to have to always relocate the FDT to RAM. Considering the level of functionality that libfdt provides, and considering how much of that functionality depends on being able to write to the FDT, it makes sense to require it to be in RAM.
Good, and for booting the FDT must be in RAM anyhow.
"checkboard()" is a name that can mean anything. If the function is
The purpose of the function is to provide board-specific code that validates the FDT. The amount of checking performed is at the discretion of the function.
For example, checkboard() *should* compare the values in the board header file with the FDT to verify that the memory mappings map, for instance.
I tend to leave it up to the board specific code where and how to verify the FDT. There are already various entry points like checkboard() or misc_init_r().
Wolfgang.

Wolfgang Grandegger wrote:
Timur Tabi wrote:
Wolfgang Grandegger wrote:
Then something similar to the ENV could be chosen:
CFG_FDT_ADDR CFG_FDT_IS_IN_FLASH CFG_FDT_IS_IN_xxx
We need two addresses:
- The address where the FDT is stored when the system is powered up
OK.
- The address in RAM where the FDT should be placed before Linux is
booted.
This should be some kind of default address.
Any default address is most likely board-specific. It would be nice if U-Boot could determine the best address automatically when possible.
Also be aware, that the blob can be within a uImage created with mkimage. Then the load address defined in the uImage should be used.
Yes.
You might be right. The _IS_IN_ is used for the ENV, I have to check what the rational behind it is (if there is one at all).
It's handy if you want to use the same macro to represent different types of addresses. Personally, I don't see much value in that. I would rather that a particular macro be used to represent only one kind of value. When I see CFG_FDT_ADDR_RAM, I know that the value is a virtual address that the CPU can dereference at any time.
I'm not sure if you understand the intended purpose. The address of the _initial_ blob could be stored in the env variable "fdtaddr" to select _one_ blob out of many in the FLASH.
Hmm....
I can see value in that, but then that variable can contain completely different addresses depending on the type of storage, which I don't like. Plus, I would rather that we use macro commands to set the FDT address, rather than an environment variable. This gives us more flexibility.
I tend to leave it up to the board specific code where and how to verify the FDT.
fdt_checkboard() *is* board-specific code.
There are already various entry points like checkboard() or
misc_init_r().
But this functions don't normally know where the FDT is. fdt_checkboard() would be designed specifically to validate an FDT from a board-specific point-of-view. It's good to have that code isolated from other board-specific code.

Timur Tabi wrote:
Wolfgang Grandegger wrote:
Timur Tabi wrote:
Wolfgang Grandegger wrote:
Then something similar to the ENV could be chosen:
CFG_FDT_ADDR CFG_FDT_IS_IN_FLASH CFG_FDT_IS_IN_xxx
We need two addresses:
- The address where the FDT is stored when the system is powered up
OK.
- The address in RAM where the FDT should be placed before Linux is
booted.
This should be some kind of default address.
Any default address is most likely board-specific. It would be nice if U-Boot could determine the best address automatically when possible.
Also be aware, that the blob can be within a uImage created with mkimage. Then the load address defined in the uImage should be used.
Yes.
You might be right. The _IS_IN_ is used for the ENV, I have to check what the rational behind it is (if there is one at all).
It's handy if you want to use the same macro to represent different types of addresses. Personally, I don't see much value in that. I would rather that a particular macro be used to represent only one kind of value. When I see CFG_FDT_ADDR_RAM, I know that the value is a virtual address that the CPU can dereference at any time.
I'm not sure if you understand the intended purpose. The address of the _initial_ blob could be stored in the env variable "fdtaddr" to select _one_ blob out of many in the FLASH.
Hmm....
I can see value in that, but then that variable can contain completely different addresses depending on the type of storage, which I don't like. Plus, I would rather that we use macro commands to set the FDT address, rather than an environment variable. This gives us more flexibility.
Hi Timur,
You totally lost me on that assertion. What do you mean when you say "macro commands?"
If you are talking preprocessor #define macros, you are comparing a hardcoded address (after compilation) to a runtime modifiable address in an env variable.
If you mean a hush script when you say "macro commands", that would be a way of setting an env variable, so how is that more flexible? We already have the "fdt addr" command at the hush level that can be used by hand or in scripts to set the fdt blob address, but that is _way_ later in the boot cycle than what Wolfgang will be using.
[snip]
Best regards, gvb

Jerry Van Baren wrote:
You totally lost me on that assertion. What do you mean when you say "macro commands?"
It's a sequence of commands stored in an environment variable, executed via the "run" command.
I guess they're called hush scripts.
If you mean a hush script when you say "macro commands", that would be a way of setting an env variable, so how is that more flexible? We already have the "fdt addr" command at the hush level that can be used by hand or in scripts to set the fdt blob address, but that is _way_ later in the boot cycle than what Wolfgang will be using.
I mean more flexible in that you can use a hush script to set that variable and do other things at boot time.

In message 4651E579.40905@freescale.com you wrote:
You totally lost me on that assertion. What do you mean when you say "macro commands?"
It's a sequence of commands stored in an environment variable, executed via the "run" command.
That does not win you anything over just setting an environment variable - you can maniopulate this later as you like anyway.
I guess they're called hush scripts.
No. Hush (shell) scripts are executed by the hush shell; simple command / variable substitution is also available with the simple command line parser.
I mean more flexible in that you can use a hush script to set that variable and do other things at boot time.
This is most probably much too late for the things needed - you can do such things only after relocation to RAM, while WolfgangG. needs access very early.
Best regards,
Wolfgang Denk

Timur Tabi wrote:
Wolfgang Grandegger wrote:
Timur Tabi wrote:
Wolfgang Grandegger wrote:
Then something similar to the ENV could be chosen:
CFG_FDT_ADDR CFG_FDT_IS_IN_FLASH CFG_FDT_IS_IN_xxx
We need two addresses:
- The address where the FDT is stored when the system is powered up
OK.
- The address in RAM where the FDT should be placed before Linux is
booted.
This should be some kind of default address.
Any default address is most likely board-specific. It would be nice if U-Boot could determine the best address automatically when possible.
Also be aware, that the blob can be within a uImage created with mkimage. Then the load address defined in the uImage should be used.
Yes.
You might be right. The _IS_IN_ is used for the ENV, I have to check what the rational behind it is (if there is one at all).
It's handy if you want to use the same macro to represent different types of addresses. Personally, I don't see much value in that. I would rather that a particular macro be used to represent only one kind of value. When I see CFG_FDT_ADDR_RAM, I know that the value is a virtual address that the CPU can dereference at any time.
I'm not sure if you understand the intended purpose. The address of the _initial_ blob could be stored in the env variable "fdtaddr" to select _one_ blob out of many in the FLASH.
Hmm....
I can see value in that, but then that variable can contain completely different addresses depending on the type of storage, which I don't like. Plus, I would rather that we use macro commands to set the FDT address, rather than an environment variable. This gives us more flexibility.
What do you mean with "macro commands"? Simply macro definitions in the board's config file? Such an address is fixed at compile time pointing to one FDT blob. The purpose of the env variable "fdtaddr" is to select that address at run time. Again, this allows to hold more than one blob in FLASH and select one via env setting. This would allow for _one_ combination of images of U-Boot + Linux + Blobs for a _set_ of boards which might reduce the effort to bring up board. Nevertheless, regard it as useful add-on.
I tend to leave it up to the board specific code where and how to verify the FDT.
fdt_checkboard() *is* board-specific code.
There are already various entry points like checkboard() or
misc_init_r().
But this functions don't normally know where the FDT is.
They know. The global variable "fdt" will point to the blob and a very early common check has then already be done. Note that the FDT commands also uses the global "fdt" variable.
fdt_checkboard() would be designed specifically to validate an FDT from a board-specific point-of-view. It's good to have that code isolated from other board-specific code.
Why more entry points if there are already suitable ones. I'm still not sure if we need an extra function.
Wolfgang.

In message 4651D0B9.9080000@freescale.com you wrote:
depending on the type of storage, which I don't like. Plus, I would rather that we use macro commands to set the FDT address, rather than an environment variable. This gives us more flexibility.
Please explain. I don't understand what you mean or which restrictions you see.
Best regards,
Wolfgang Denk

Wolfgang Denk wrote:
In message 4651D0B9.9080000@freescale.com you wrote:
depending on the type of storage, which I don't like. Plus, I would rather that we use macro commands to set the FDT address, rather than an environment variable. This gives us more flexibility.
Please explain. I don't understand what you mean or which restrictions you see.
By "macro command", I meant "hush script". And by more flexibility, I meant that instead of using one environment variable to specify which FDT to use (a situation that really shouldn't occur, and will probably never occur once we we have dynamic FDTs in place), just have the user run a different hush script to set the fdt address.
However, now that I think about it, I'm not sure what my point is any more.

Wolfgang Grandegger wrote:
Jerry Van Baren wrote:
Timur Tabi wrote:
Jerry Van Baren wrote:
Yes, that is what "fdt addr" does today, it tells the u-boot innards where the blob is (sets/changes the blob location). The user can download a new blob and switch to using it via the "fdt addr" command.
By "change", I thought you meant that "fdt addr" would actually move the device tree, since that's technically changing the address. Perhaps you meant "set the address"?
Yes.
The user can use the "fdt addr" command to set/change the location of the blob. Moving blobs is not out of scope, it is what "fdt move" does.
How is "fdt move" different than cp.b followed by another "fdt addr"?
Conceptually the same thing but easier: you don't have to know (guess) the blob size because fdt_open_into() gets the size from the source blob.
Summing up, brainstorming a bit more, here are my revised ideas:
The following definitions control the FDT usage in U-Boot:
CFG_FDT_ADDR_FLASH: If defined, "fdt" is set to that address at compile time. The FDT can be used from the early beginning of the boot.
CFG_FDT_ADDR_BY_ENV: If defined, the env variable "fdtaddr" is looked up early in the boot and "fdt" is set accordingly. This allows to hold more than one blob in FLASH and select one via env setting. This would allow for _one_ combination of images of U-Boot + Linux + Blobs for a _set_ of boards.
If CFG_FDT_ADDR_BY_ENV is *not* defined, should the FDT code then set that variable?
CFG_FDT_ADDR_RAM: If defined, the blob is moved to RAM after relocation for further preparation or for performance reasons. "fdt" is re-set accordingly. The blob is then ready and in place for booting Linux. If CFG_FDT_ADDR_RAM is set to 0, the blob will be copied to a default location, e.g. before the initrd location.
I think the FDT blob should *always* be copied to RAM.
A board-specific checkboad function is called as early as possible to verify the FDT.
This should also make Timur happy, as he has the choice, e.g. read the FDT solely from FLASH.
I think I may have changed my mind a bit. I'm not so much concerned about read-only FDT as I want to automate the process of copying it to RAM.

Timur Tabi wrote:
Wolfgang Grandegger wrote:
Jerry Van Baren wrote:
Timur Tabi wrote:
Jerry Van Baren wrote:
Yes, that is what "fdt addr" does today, it tells the u-boot innards where the blob is (sets/changes the blob location). The user can download a new blob and switch to using it via the "fdt addr" command.
By "change", I thought you meant that "fdt addr" would actually move the device tree, since that's technically changing the address. Perhaps you meant "set the address"?
Yes.
The user can use the "fdt addr" command to set/change the location of the blob. Moving blobs is not out of scope, it is what "fdt move" does.
How is "fdt move" different than cp.b followed by another "fdt addr"?
Conceptually the same thing but easier: you don't have to know (guess) the blob size because fdt_open_into() gets the size from the source blob.
Summing up, brainstorming a bit more, here are my revised ideas:
The following definitions control the FDT usage in U-Boot:
CFG_FDT_ADDR_FLASH: If defined, "fdt" is set to that address at compile time. The FDT can be used from the early beginning of the boot.
CFG_FDT_ADDR_BY_ENV: If defined, the env variable "fdtaddr" is looked up early in the boot and "fdt" is set accordingly. This allows to hold more than one blob in FLASH and select one via env setting. This would allow for _one_ combination of images of U-Boot + Linux + Blobs for a _set_ of boards.
If CFG_FDT_ADDR_BY_ENV is *not* defined, should the FDT code then set that variable?
Yes, then the address is set to CFG_FDT_ADDR_XXX at compile time.
CFG_FDT_ADDR_RAM: If defined, the blob is moved to RAM after relocation for further preparation or for performance reasons. "fdt" is re-set accordingly. The blob is then ready and in place for booting Linux. If CFG_FDT_ADDR_RAM is set to 0, the blob will be copied to a default location, e.g. before the initrd location.
I think the FDT blob should *always* be copied to RAM.
A board-specific checkboad function is called as early as possible to verify the FDT.
This should also make Timur happy, as he has the choice, e.g. read the FDT solely from FLASH.
I think I may have changed my mind a bit. I'm not so much concerned about read-only FDT as I want to automate the process of copying it to RAM.
As a result of discussion, in the meantime I have now also a better idea on how to implement FDT for U-Boot configuration. Going to make a demo for my Icecube board a.s.a.p.
Wolfgang.

In message 464DCEAE.3030201@smiths-aerospace.com you wrote:
Indeed. However, the above quote is one of Wolfgang Denk's favorites.
Indeed it is.
Don't diss it, Wolfgang will get p1ssed. ;-)
Not really. Everybody has the right to have his own opinion. As long as they not try to code such opinions into U-Boot, that is ;-)
Best regards,
Wolfgang Denk

On Fri, 2007-05-18 at 14:16, Wolfgang Denk wrote:
Don't diss it, Wolfgang will get p1ssed. ;-)
Not really. Everybody has the right to have his own opinion. As long as they not try to code such opinions into U-Boot, that is ;-)
Or, to paraphrase Wolfgang here:
Everyone else is entitled to their own wrong opinion.
:-)
jdl

In message 464DC7D8.2050703@freescale.com you wrote:
"UNIX was not designed to stop its users from doing stupid things, as that would also stop them from doing clever things." Doug Gwyn
U-Boot isn't Unix.
But is very closely follows the Unix design philosophy. If you haven't understood this by now, you should try to forget what you know about U-Boot, and restart from scratch.
Best regards,
Wolfgang Denk
participants (6)
-
Jerry Van Baren
-
Jerry Van Baren
-
Jon Loeliger
-
Timur Tabi
-
Wolfgang Denk
-
Wolfgang Grandegger