Could the DTS experts look at this?
The Rev B warp is moving to a 4M NOR / 256M NAND flash setup from the current 64M NOR / 64M NAND. I would like to keep support for the 64M NOR so I modified the boot code to be a bit more dynamic. Here is the new NOR parition layout in the DTS:
nor_flash@0,0 {
compatible = "amd,s29gl512n", "cfi-flash";
bank-width = <2>;
reg = <0 0 4000000>;
#address-cells = <1>;
#size-cells = <1>;
partition@300000 {
label = "fpga";
reg = <300000 40000>;
};
partition@340000 {
label = "env";
reg = <340000 40000>;
};
partition@380000 {
label = "u-boot";
reg = <380000 80000>;
};
};Yes, the top of the NOR will be empty. Here is the code from cuboot-warp.c to handle fixups for the 64M flash:
static void warp_fixup_one_nor(u32 from, u32 to)
{
void *devp;
char name[40];
u32 v[2];
sprintf(name, "/plb/opb/ebc/nor_flash@0,0/partition@%x", from);
devp = finddevice(name);
if (!devp) return;
if (getprop(devp, "reg", v, sizeof(v)) == sizeof(v)) {
v[0] = to;
setprop(devp, "reg", v, sizeof(v));
printf("NOR 64M fixup %x -> %x\n", from, to);
}
}static void warp_fixups(void)
{
unsigned long sysclk = 66000000;
ibm440ep_fixup_clocks(sysclk, 11059200, 50000000);
ibm4xx_sdram_fixup_memsize();
ibm4xx_fixup_ebc_ranges("/plb/opb/ebc");
dt_fixup_mac_addresses(&bd.bi_enetaddr);
/* Fixup for 64M flash on Rev A boards. */
if(bd.bi_flashsize == 0x4000000) {
warp_fixup_one_nor(0x300000, 0x3f00000);
warp_fixup_one_nor(0x340000, 0x3f40000);
warp_fixup_one_nor(0x380000, 0x3f80000);
}
}I have tested this with the 64M NOR, and it seems to work. However, are there going to be problems with the partition name not matching the reg address entry?
If anybody has suggestions on better ways to do this, fire away.
And looking at this code, and other board ports, why is sysclk a local variable and all the other numbers hardcoded in the args? I left it the same way as the others but it does look a bit strange.
Cheers,
Sean
Quoted 1 line · Sean MacLennan
If anybody has suggestions on better ways to do this, fire away.
I guess the cleanest solution would be to include two complete device trees for this platform, and then choose the correct one in cuboot-warp.c based on the board revision.
The obvious disadvantage of this is that you'd get two device trees that you need to keep in sync with every change, so it might not be very practical.
Maybe we can introduce a more generic way of having conditional device nodes in the tree that get knocked out in the boot wrapper.
Arnd <><
Quoted 9 lines · Arnd Bergmann
I guess the cleanest solution would be to include two complete device trees for this platform, and then choose the correct one in
cuboot-warp.cbased on the board revision.The obvious disadvantage of this is that you'd get two device trees that you need to keep in sync with every change, so it might not be very practical.
Keeping two device trees would be awkward. My final solution will probably be to just have
the 4M flash partition layout. Since we really only use the partition information to write to the partitions from a user mode program, I can live with having to flash the images from u-boot.
But if we where planning on moving forward with two configurations, which we where planning to do, this would not be an option. We would need to auto configure *and* possibly add partitions based on size.
Quoted 3 lines
Maybe we can introduce a more generic way of having conditional device nodes in the tree that get knocked out in the boot wrapper.
Of even a way to have ifdefs in the dts. In this case, we have to build a specific version of u-boot for each nor size (WARNING: This may not be *technically* true, but this is how we are handling it). Having to set a config and build a specific kernel, while not ideal, would also be a reasonable solution.
Just throwing out ideas. In the short term this is not a problem, but in the long term?
Cheers,
Sean
Quoted 2 lines · Arnd Bergmann
Maybe we can introduce a more generic way of having conditional device nodes in the tree that get knocked out in the boot wrapper.
I've been thinking about doing just this for quite some time now. I've had a few informal discussions without people about.
One idea is to allow attaching simple conditional expressions (like X is <, =, or > than Y) to a node. It is the responsibility of the code that parses the device tree to assign values to X and Y. For instance, they could be the names of U-Boot environment variables. If the expression is false, then the node is removed (or ignored) from the device tree. If it's true, then it's kept in.
Signature · 2 lines
Timur Tabi Linux kernel developer at Freescale
Quoted 8 lines · Timur Tabi
Quoted 2 lines · Arnd Bergmann
Maybe we can introduce a more generic way of having conditional device nodes in the tree that get knocked out in the boot wrapper.
I've been thinking about doing just this for quite some time now. I've had a few informal discussions without people about.
Quoted 6 lines
One idea is to allow attaching simple conditional expressions (like X is <, =, or > than Y) to a node. It is the responsibility of the code that parses the device tree to assign values to X and Y. For instance, they could be the names of U-Boot environment variables. If the expression is false, then the node is removed (or ignored) from the device tree. If it's true, then it's kept in.
In the binary tree representation itself? No way.
Signature · 4 lines
David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
Quoted 9 lines · David Gibson
Quoted 8 lines · Timur Tabi
Quoted 2 lines · Arnd Bergmann
Maybe we can introduce a more generic way of having conditional device nodes in the tree that get knocked out in the boot wrapper.
I've been thinking about doing just this for quite some time now. I've had a few informal discussions without people about.
Quoted 8 lines
Quoted 6 lines
One idea is to allow attaching simple conditional expressions (like X is <, =, or > than Y) to a node. It is the responsibility of the code that parses the device tree to assign values to X and Y. For instance, they could be the names of U-Boot environment variables. If the expression is false, then the node is removed (or ignored) from the device tree. If it's true, then it's kept in.
In the binary tree representation itself? No way.
Or to expand. It's relatively easy now to just include multiple nodes in the tree and either delete or nop some of them out conditionally using libfdt. But the conditional logic should be in the manipulating agent (u-boot or bootwrapper or whatever), there's no way we're going to require a conditional expression parser to interpret the device tree blob itself.
Signature · 4 lines
David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
Quoted 6 lines · David Gibson
Or to expand. =A0It's relatively easy now to just include multiple nodes in the tree and either delete or nop some of them out conditionally using libfdt. =A0But the conditional logic should be in the manipulating agent (u-boot or bootwrapper or whatever), there's no way we're going to require a conditional expression parser to interpret the device tree blob itself.
How about making the logic to nop out nodes a little more generic without changes to the binary format?
E.g. you could have a "linux,conditional-node" property in the device tree whose value is compared to a HW configuration specific string. In Sean's example, you can have linux,conditional-node=3D"Rev.A" in some nodes and linux,conditional-node=3D"Rev.B" in others, then knock out all devices that have a non-matching linux,conditional-node property, and finally remove the properties themselves before starting the kernel.
Arnd <><
Quoted 8 lines · Arnd Bergmann
Quoted 6 lines · David Gibson
Or to expand. It's relatively easy now to just include multiple nodes in the tree and either delete or nop some of them out conditionally using libfdt. But the conditional logic should be in the manipulating agent (u-boot or bootwrapper or whatever), there's no way we're going to require a conditional expression parser to interpret the device tree blob itself.
Quoted 9 lines
How about making the logic to nop out nodes a little more generic without changes to the binary format?
E.g. you could have a "linux,conditional-node" property in the device tree whose value is compared to a HW configuration specific string. In Sean's example, you can have linux,conditional-node="Rev.A" in some nodes and linux,conditional-node="Rev.B" in others, then knock out all devices that have a non-matching linux,conditional-node property, and finally remove the properties themselves before starting the kernel.
Well, that's basically a u-boot issue. If they want to do their input trees that way, and have helper functions that deal with it...
Signature · 4 lines
David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
Quoted 19 lines · David Gibson
Quoted 8 lines · Arnd Bergmann
Quoted 6 lines · David Gibson
Or to expand. It's relatively easy now to just include multiple nodes in the tree and either delete or nop some of them out conditionally using libfdt. But the conditional logic should be in the manipulating agent (u-boot or bootwrapper or whatever), there's no way we're going to require a conditional expression parser to interpret the device tree blob itself.
Quoted 9 lines
How about making the logic to nop out nodes a little more generic without changes to the binary format?
E.g. you could have a "linux,conditional-node" property in the device tree whose value is compared to a HW configuration specific string. In Sean's example, you can have linux,conditional-node="Rev.A" in some nodes and linux,conditional-node="Rev.B" in others, then knock out all devices that have a non-matching linux,conditional-node property, and finally remove the properties themselves before starting the kernel.
Quoted 2 lines
Well, that's basically a u-boot issue. If they want to do their input trees that way, and have helper functions that deal with it...
The actual mechanism that we originially discussed, which Timur later morphed into conditions-on-nodes, was to have a separate hwoptions node, under which would be described various hwoptions (jumpers and such) whose state could be either detected by u-boot or set by environment variable. Each hwoption setting would contain a device tree fragment to be merged into the main device tree.
-Scott
Quoted 23 lines · Scott Wood
Quoted 19 lines · David Gibson
Quoted 8 lines · Arnd Bergmann
Quoted 6 lines · David Gibson
Or to expand. It's relatively easy now to just include multiple nodes in the tree and either delete or nop some of them out conditionally using libfdt. But the conditional logic should be in the manipulating agent (u-boot or bootwrapper or whatever), there's no way we're going to require a conditional expression parser to interpret the device tree blob itself.
Quoted 9 lines
How about making the logic to nop out nodes a little more generic without changes to the binary format?
E.g. you could have a "linux,conditional-node" property in the device tree whose value is compared to a HW configuration specific string. In Sean's example, you can have linux,conditional-node="Rev.A" in some nodes and linux,conditional-node="Rev.B" in others, then knock out all devices that have a non-matching linux,conditional-node property, and finally remove the properties themselves before starting the kernel.Quoted 2 lines
Well, that's basically a u-boot issue. If they want to do their input trees that way, and have helper functions that deal with it...
Quoted 6 lines
The actual mechanism that we originially discussed, which Timur later morphed into conditions-on-nodes, was to have a separate hwoptions node, under which would be described various hwoptions (jumpers and such) whose state could be either detected by u-boot or set by environment variable. Each hwoption setting would contain a device tree fragment to be merged into the main device tree.
I'm not sure I'm entirely happy about storing the fragments under a special node - but certainly u-boot could do that if it wants. What would certainly be ok is to store various fragments as separate blobs and fold them together as necessary. Which reminds me, I meant to implement a "graft" function in libfdt.
Signature · 4 lines
David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
Quoted 5 lines · David Gibson
I'm not sure I'm entirely happy about storing the fragments under a special node - but certainly u-boot could do that if it wants. What would certainly be ok is to store various fragments as separate blobs and fold them together as necessary. Which reminds me, I meant to implement a "graft" function in libfdt.
Most likely, U-Boot would strip out the special node after processing it. The idea is for the boot loader to customize the device tree based before sending it to the kernel.
Signature · 2 lines
Timur Tabi Linux kernel developer at Freescale
Quoted 11 lines · Timur Tabi
Quoted 5 lines · David Gibson
I'm not sure I'm entirely happy about storing the fragments under a special node - but certainly u-boot could do that if it wants. What would certainly be ok is to store various fragments as separate blobs and fold them together as necessary. Which reminds me, I meant to implement a "graft" function in libfdt.
Most likely, U-Boot would strip out the special node after processing it. The idea is for the boot loader to customize the device tree based before sending it to the kernel.
Of course. U-boot can use whatever representation it likes internally, as long as it's all fixed up by the time it reaches the kernel. I just think using sepa`rate "device tree fragment" blobs might be a better way of doing it.
Signature · 4 lines
David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
Quoted 4 lines · Arnd Bergmann
Quoted 3 lines · David Gibson
Or to expand. It's relatively easy now to just include multiple nodes in the tree and either delete or nop some of them out conditionally using libfdt.
Yes, but what better place to store the conditions than in the device tree itself? How would libfdt know where the conditions are? Do you want to have two binary blobs?
Quoted 4 lines
Quoted 4 lines
But the conditional logic should be in the manipulating agent (u-boot or bootwrapper or whatever), there's no way we're going to require a conditional expression parser to interpret the device tree blob itself.
I think it's a great feature that solves a lot of problems, and it does so in an elegant and efficient manner. I look forward to trying to change your mind when I get around to implementing it.
Quoted 4 lines
How about making the logic to nop out nodes a little more generic without changes to the binary format?
E.g. you could have a "linux,conditional-node" property in the device tree whose value is compared to a HW configuration specific string.
The problem with this is that if you use a version of libfdt that does not understand "linux,conditional-node", then your device tree will be wrong, because it could contain nodes that don't belong. We would need a new, incompatible version number for the device tree to make sure that this doesn't happen, even though nothing has changed in the binary layout of the tree.
Signature · 2 lines
Timur Tabi Linux kernel developer at Freescale
Quoted 10 lines · Timur Tabi
Quoted 4 lines · Arnd Bergmann
Quoted 3 lines · David Gibson
Or to expand. It's relatively easy now to just include multiple nodes in the tree and either delete or nop some of them out conditionally using libfdt.
Yes, but what better place to store the conditions than in the device tree itself? How would libfdt know where the conditions are? Do you want to have two binary blobs?
The transient state of the dts before it is handed to the kernel proper is almost irrelevant. It is totally reasonable to add in whatever properties/nodes that are needed to *eventually* describe the hardware correctly. Heck, we already do this will all dts files that go through u-boot is a simple sense. We put placeholder properties for mac addresses and bus frequencies, but u-boot fills them in.
However, if a designer does write a device tree containing more nodes than is needed, then it is also the responsibility of that designer to make sure the boot loader can use that tree to generate a real description of hardware. This requires coordination and documentation, but id does not requires special formatting or versioning of the device tree blob.
The dtb is a data structure, not a programming language. I think it is a slippery slope to try and encode conditionals into the structure; but it is entirely reasonable to encode *data* into the dt that helps make those conditional decisions.
Quoted 8 lines
Quoted 4 lines
Quoted 4 lines
But the conditional logic should be in the manipulating agent (u-boot or bootwrapper or whatever), there's no way we're going to require a conditional expression parser to interpret the device tree blob itself.
I think it's a great feature that solves a lot of problems, and it does so in an elegant and efficient manner. I look forward to trying to change your mind when I get around to implementing it.
I agree with David here; logic belongs in the agent code, not the data structure.
Quoted 10 lines
Quoted 4 lines
How about making the logic to nop out nodes a little more generic without changes to the binary format?
E.g. you could have a "linux,conditional-node" property in the device tree whose value is compared to a HW configuration specific string.The problem with this is that if you use a version of libfdt that does not understand "linux,conditional-node", then your device tree will be wrong, because it could contain nodes that don't belong. We would need a new, incompatible version number for the device tree to make sure that this doesn't happen, even though nothing has changed in the binary layout of the tree.
We've already got that issue. If you pass the device tree for the wrong board it will still validate correctly, but the board is not going to boot. The device tree must match what the bootloader expects. Changing the version number will do nothing to help this. The version number ensures that the tree is parsable. It does not ensure that it is correct.
Cheers,
g.
Signature · 2 lines
Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd.
Quoted 11 lines · Grant Likely
Quoted 9 lines · Timur Tabi
Quoted 4 lines · Arnd Bergmann
Quoted 3 lines · David Gibson
Or to expand. It's relatively easy now to just include multiple nodes in the tree and either delete or nop some of them out conditionally using libfdt.
Yes, but what better place to store the conditions than in the device tree itself? How would libfdt know where the conditions are? Do you want to have two binary blobs?
Quoted 6 lines
The transient state of the dts before it is handed to the kernel proper is almost irrelevant. It is totally reasonable to add in whatever properties/nodes that are needed to *eventually* describe the hardware correctly. Heck, we already do this will all dts files that go through u-boot is a simple sense. We put placeholder properties for mac addresses and bus frequencies, but u-boot fills them in.
I agree with that.
Quoted 4 lines
However, if a designer does write a device tree containing more nodes than is needed, then it is also the responsibility of that designer to make sure the boot loader can use that tree to generate a real description of hardware.
No problem here.
Quoted 3 lines
This requires coordination and
documentation, but id does not requires special formatting or versioning of the device tree blob.
Unless the mechanism by which the designer ensures that the boot loader presents a proper device tree to the kernel requires special versioning.
Quoted 1 line
The dtb is a data structure, not a programming language.
But we have a problem with the current device tree definition that makes it difficult to use in real-world situations. The current "solution" is to have multiple DTBs, each one covering a different variant of the hardware. My proposal is to expand the definition of the DTB to allow the boot loader to modify it in a standard manner. I believe that such a change would be both useful and not problematic.
Quoted 2 lines
I think it
is a slippery slope to try and encode conditionals into the structure;
Perhaps, which is why I said it should be simple conditionals - two values and a comparison.
Quoted 2 lines
but it is entirely reasonable to encode *data* into the dt that helps make those conditional decisions.
That's okay too, except that if we just add additional nodes that describe conditions, then we need to make sure that whatever parses that DTB must also parse those additional nodes. The only way to do that is create a new version number, like 18, which is marked as being incompatible with the current version (17). Otherwise, a person could pass that DTB to an old version of U-Boot, and U-boot will just pass it on to the kernel as-is.
Quoted 3 lines
We've already got that issue. If you pass the device tree for the wrong board it will still validate correctly, but the board is not going to boot.
There's nothing stopping U-Boot today from scanning the device tree and making sure the SOC's compatible node is correct. That's not currently done, but it could be.
Quoted 4 lines
The device tree must match what the bootloader
expects. Changing the version number will do nothing to help this. The version number ensures that the tree is parsable. It does not ensure that it is correct.
I think you're missing the point. If we add conditional expressions to the device tree (whether attached to a node or as part of a separate node or whatever), we must also add a mechanism to ensure that these conditions are parsed by the boot loader. As far as I know, the only mechanism that can do this is the version identifier.
Signature · 2 lines
Timur Tabi Linux kernel developer at Freescale
Quoted 8 lines · Timur Tabi
Quoted 4 lines · Grant Likely
On Feb 12, 2008 8:44 AM, Timur Tabi <[email protected]> wrote:
This requires coordination and
documentation, but id does not requires special formatting or versioning of the device tree blob.Unless the mechanism by which the designer ensures that the boot loader presents a proper device tree to the kernel requires special versioning.
Then use a local version in the data; don't overload the purpose of the dtb version.
Quoted 8 lines
Quoted 1 line
The dtb is a data structure, not a programming language.
But we have a problem with the current device tree definition that makes it difficult to use in real-world situations. The current "solution" is to have multiple DTBs, each one covering a different variant of the hardware. My proposal is to expand the definition of the DTB to allow the boot loader to modify it in a standard manner. I believe that such a change would be both useful and not problematic.
I don't think it is yet possible to define a reasonable 'standard manner' for massaging device trees. There are going to be a lot of experiments and false starts before we come to consensus on common manipulations which everyone will want to have access too. Therefore for the time being dtb fixups and manipulations will remain a board specific thing.
Plus, even when we do have a common set of manipulations, they can be implemented with property values. It is not something that needs to change the dtb version.
Quoted 5 lines
Quoted 2 lines
I think it
is a slippery slope to try and encode conditionals into the structure;Perhaps, which is why I said it should be simple conditionals - two values and a comparison.
And when something comes along that doesn't fit into that model? Add more constructs? I say better to handle that within the existing data format. If we get it wrong, then we just change the affected device trees and boot loaders. We don't have to upgrade every platform that uses dt blobs.
Quoted 9 lines
Quoted 2 lines
but it is entirely reasonable to encode *data* into the dt that helps make those conditional decisions.
That's okay too, except that if we just add additional nodes that describe conditions, then we need to make sure that whatever parses that DTB must also parse those additional nodes. The only way to do that is create a new version number, like 18, which is marked as being incompatible with the current version (17). Otherwise, a person could pass that DTB to an old version of U-Boot, and U-boot will just pass it on to the kernel as-is.
That's not a dtb version issue. That is a dtb content issue. It does not warrant changing the dtb version number.
Quoted 7 lines
Quoted 3 lines
We've already got that issue. If you pass the device tree for the wrong board it will still validate correctly, but the board is not going to boot.
There's nothing stopping U-Boot today from scanning the device tree and making sure the SOC's compatible node is correct. That's not currently done, but it could be.
Fair enough, and it is also reasonable for the boot loader to look for a specific property name to decide how to massage the data structure. This alone does not require a dtb version change.
Quoted 10 lines
Quoted 4 lines
The device tree must match what the bootloader
expects. Changing the version number will do nothing to help this. The version number ensures that the tree is parsable. It does not ensure that it is correct.I think you're missing the point. If we add conditional expressions to the device tree (whether attached to a node or as part of a separate node or whatever), we must also add a mechanism to ensure that these conditions are parsed by the boot loader. As far as I know, the only mechanism that can do this is the version identifier.
I'm not missing the point because I disagree entirely with the addition of conditional expressions to the device tree. Instead, I think properties can be added to the device tree that a bootloader can look for and decide to apply conditions against them which means the conditions are encoded in the boot loader, not the device tree. (the device tree simply contains data which supports the boot loaders decision; a rather different thing).
Finally, it is *always* required that the user (or installer) know enough to pass the correct device tree to the correct board. No amount of versioning at the dtb level is going to change this situation.
Cheers,
g.
Signature · 2 lines
Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd.
Quoted 2 lines · Grant Likely
Then use a local version in the data; don't overload the purpose of the dtb version.
I don't know what you mean by that.
Quoted 6 lines
I don't think it is yet possible to define a reasonable 'standard manner' for massaging device trees. There are going to be a lot of experiments and false starts before we come to consensus on common manipulations which everyone will want to have access too. Therefore for the time being dtb fixups and manipulations will remain a board specific thing.
That's okay with me. I'm just proposing one method that I like, and Scott proposed another.
Quoted 2 lines
And when something comes along that doesn't fit into that model? Add more constructs?
If necessary, yes. What's wrong with expanding the power of the device tree format when it solves more problems?
Quoted 2 lines
I say better to handle that within the existing data
format.
And the point I've been trying to make is that we have real problems today that cannot be solved elegantly with the current device tree problem. Having board-specific code in U-Boot that is hard-coded to look for specific nodes in the device tree, and making hard-coded edits on that tree, is *not* elegant.
Quoted 3 lines
If we get it wrong, then we just change the affected device
trees and boot loaders. We don't have to upgrade every platform that uses dt blobs.
Only the platforms that need to take advantage of conditional nodes need to be upgraded in the first place. Most platforms are happy with just one device tree.
Quoted 8 lines
Quoted 6 lines
That's okay too, except that if we just add additional nodes that describe conditions, then we need to make sure that whatever parses that DTB must also parse those additional nodes. The only way to do that is create a new version number, like 18, which is marked as being incompatible with the current version (17). Otherwise, a person could pass that DTB to an old version of U-Boot, and U-boot will just pass it on to the kernel as-is.
That's not a dtb version issue. That is a dtb content issue.
Technically, that's true, but ...
Quoted 2 lines
It does
not warrant changing the dtb version number.
Then how do you solve the problem of passing a device tree to a boot loader that does not know how to parse it properly? A device tree with these additional nodes *must* be parsed by a boot loader that is aware of them. Otherwise, it will pass the device tree as-is to the kernel without warning. This is a bad thing, and steps should be taken to prevent that. If you can think of a way to make this happen without changing the version number, I'd love to hear. All I'm hearing from you now is denial that this is a problem.
Quoted 10 lines
Quoted 6 lines
Quoted 3 lines
We've already got that issue. If you pass the device tree for the wrong board it will still validate correctly, but the board is not going to boot.
There's nothing stopping U-Boot today from scanning the device tree and making sure the SOC's compatible node is correct. That's not currently done, but it could be.
Fair enough, and it is also reasonable for the boot loader to look for a specific property name to decide how to massage the data structure. This alone does not require a dtb version change.
Current versions of U-Boot do not know how to do this. So again, I'm asking you: how do you solve the problem of passing a device tree with additional nodes to a boot loader that does not know how to parse them properly? How do you prevent that old U-Boot from ignoring those nodes?
Quoted 7 lines
I'm not missing the point because I disagree entirely with the addition of conditional expressions to the device tree. Instead, I think properties can be added to the device tree that a bootloader can look for and decide to apply conditions against them which means the conditions are encoded in the boot loader, not the device tree. (the device tree simply contains data which supports the boot loaders decision; a rather different thing).
Then why bother passing a DTB to the boot loader at all? Why not just have the boot loader create the device tree from scratch?
Signature · 2 lines
Timur Tabi Linux kernel developer at Freescale
Quoted 6 lines · Timur Tabi
Quoted 2 lines · Grant Likely
Then use a local version in the data; don't overload the purpose of the dtb version.
I don't know what you mean by that.
I'm saying that the dtb version is to describe the binary format of the dtb; not the content. Using it to say "this dtb contains data that needs to be massaged in a particular way" is using (overloading) the dtb version for a different and orthogonal purpose.
Quoted 5 lines
Quoted 2 lines
And when something comes along that doesn't fit into that model? Add more constructs?
If necessary, yes. What's wrong with expanding the power of the device tree format when it solves more problems?
Nothing at all; but the flipside of that is it should only be done when it is the appropriate place to do so.
Quoted 29 lines
Quoted 2 lines
I say better to handle that within the existing data
format.And the point I've been trying to make is that we have real problems today that cannot be solved elegantly with the current device tree problem. Having board-specific code in U-Boot that is hard-coded to look for specific nodes in the device tree, and making hard-coded edits on that tree, is *not* elegant.
Quoted 3 lines
If we get it wrong, then we just change the affected device
trees and boot loaders. We don't have to upgrade every platform that uses dt blobs.Only the platforms that need to take advantage of conditional nodes need to be upgraded in the first place. Most platforms are happy with just one device tree.
Quoted 8 lines
Quoted 6 lines
That's okay too, except that if we just add additional nodes that describe conditions, then we need to make sure that whatever parses that DTB must also parse those additional nodes. The only way to do that is create a new version number, like 18, which is marked as being incompatible with the current version (17). Otherwise, a person could pass that DTB to an old version of U-Boot, and U-boot will just pass it on to the kernel as-is.
That's not a dtb version issue. That is a dtb content issue.
Technically, that's true, but ...
Quoted 2 lines
It does
not warrant changing the dtb version number.
Quoted 7 lines
Then how do you solve the problem of passing a device tree to a boot loader that does not know how to parse it properly? A device tree with these additional nodes *must* be parsed by a boot loader that is aware of them. Otherwise, it will pass the device tree as-is to the kernel without warning. This is a bad thing, and steps should be taken to prevent that. If you can think of a way to make this happen without changing the version number, I'd love to hear. All I'm hearing from you now is denial that this is a problem.
I do not deny that it is a problem. I assert that the proposed solution does not at all solve the greater problem. Changing the DTB version solves the specific problem of a dtb that needs to be modified being passed into to an older version of u-boot. This is a very narrow use case and it is not the greater problem. The greater problem is passing the wrong device tree either because of a u-boot/dtb mismatch or the dtb is for an entirely different board. Changing the dtb version is just a bandaid fix that makes things more complex and only provides the illusion of a solution.
Your suggestion of checking for specific property values is a far more reliable and workable solution.
Quoted 8 lines
Quoted 3 lines
Fair enough, and it is also reasonable for the boot loader to look for a specific property name to decide how to massage the data structure. This alone does not require a dtb version change.
Current versions of U-Boot do not know how to do this. So again, I'm asking
you: how do you solve the problem of passing a device tree with additional nodes to a boot loader that does not know how to parse them properly? How do you prevent that old U-Boot from ignoring those nodes?
We don't. We've got no sane way to do so (except perhaps going ahead and renaming the soc nodes from soc<chip>@addr to soc@addr. That will break almost all existing ports. :-). I do not agree that changing the dtb version is a sane change.
For older u-boots, this is a configuration management problem. Just like for current platforms we need to make sure that the kernel is compiled for the correct platform before trying to boot. There are no protections in u-boot to make sure the kernel image matches the arch yet it all works just fine.
Quoted 10 lines
Quoted 7 lines
I'm not missing the point because I disagree entirely with the addition of conditional expressions to the device tree. Instead, I think properties can be added to the device tree that a bootloader can look for and decide to apply conditions against them which means the conditions are encoded in the boot loader, not the device tree. (the device tree simply contains data which supports the boot loaders decision; a rather different thing).
Then why bother passing a DTB to the boot loader at all? Why not just have the boot loader create the device tree from scratch?
There is absolutely no reason for the boot loader not to create the device tree from scratch. dtbs are just more convenient to get up and running with. OpenFirmware has been doing that from day one and it works quite well.
g.
Signature · 2 lines
Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd.
Quoted 1 line · Timur Tabi
Grant Likely wrote:
[snip]
Quoted 11 lines
Quoted 1 line
That's not a dtb version issue. That is a dtb content issue.
Technically, that's true, but ...
Quoted 2 lines
It does
not warrant changing the dtb version number.Then how do you solve the problem of passing a device tree to a boot loader that does not know how to parse it properly? A device tree with these additional nodes *must* be parsed by a boot loader that is aware of them.
Correct. Just as you must give a dtb with the information to the correct board to a bootloader or things won't work. Changing this is not within the reasonable scope of what dtbs will do.
Quoted 17 lines
Otherwise, it will pass the device tree as-is to
the kernel without warning. This is a bad thing, and steps should be taken to prevent that. If you can think of a way to make this happen without changing the version number, I'd love to hear. All I'm hearing from you now is denial that this is a problem.Quoted 10 lines
Quoted 6 lines
Quoted 3 lines
We've already got that issue. If you pass the device tree for the wrong board it will still validate correctly, but the board is not going to boot.
There's nothing stopping U-Boot today from scanning the device tree and making sure the SOC's compatible node is correct. That's not currently done, but it could be.
Fair enough, and it is also reasonable for the boot loader to look for a specific property name to decide how to massage the data structure. This alone does not require a dtb version change.
Quoted 5 lines
Current versions of U-Boot do not know how to do this. So again, I'm asking you: how do you solve the problem of passing a device tree with additional nodes to a boot loader that does not know how to parse them properly? How do you prevent that old U-Boot from ignoring those nodes?
You don't. If your agent takes a dtb, dtb layout and agent must match.
Quoted 10 lines
Quoted 7 lines
I'm not missing the point because I disagree entirely with the addition of conditional expressions to the device tree. Instead, I think properties can be added to the device tree that a bootloader can look for and decide to apply conditions against them which means the conditions are encoded in the boot loader, not the device tree. (the device tree simply contains data which supports the boot loaders decision; a rather different thing).
Then why bother passing a DTB to the boot loader at all? Why not just have the boot loader create the device tree from scratch?
That's a perfectly acceptable option - and it's what I'd expect if a real OF decided to add support for flattened device trees (which might happen with ePAPR). libfdt's serial-write functions are designed for exactly this use case.
In fact, in one way of looking at it that's always what happens: the dtb format is defined for passing hardware information from the bootloader to the kernel; nothing else. Passing a dtb *into* the bootloader is just a bootloader implementation convenience, because the possible variations on an output tree are small, so it's useful to have a skeleton tree built-in. But in order for the bootloader to process those variations correctly, the skeleton *must* be in the right format. dtb input to a bootloader must match the bootloaders expectations. This has always been true, and will continue to be true.
Signature · 4 lines
David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
Quoted 2 lines · David Gibson
You don't. If your agent takes a dtb, dtb layout and agent must match.
So what I would like to see is a way for the agent to validate the dtb. U-Boot could currently validate the SOC's compatible field. However, if we add a special node that contains rules for modifying the rest of the tree, the only possible way to block older, incompatible U-Boots from accepting the tree is to bump the version number. Since that is not the right thing to do, the best approach is to define a new node type that has conditional expression attached to it. Then we can bump the version.
Quoted 10 lines
In fact, in one way of looking at it that's always what happens: the dtb format is defined for passing hardware information from the bootloader to the kernel; nothing else. Passing a dtb *into* the bootloader is just a bootloader implementation convenience, because the possible variations on an output tree are small, so it's useful to have a skeleton tree built-in. But in order for the bootloader to process those variations correctly, the skeleton *must* be in the right format. dtb input to a bootloader must match the bootloaders expectations. This has always been true, and will continue to be true.
The problem with this approach is that you're replacing data with code, and that always makes things more difficult.
Signature · 2 lines
Timur Tabi Linux kernel developer at Freescale
Quoted 10 lines · David Gibson
In fact, in one way of looking at it that's always what happens: the dtb format is defined for passing hardware information from the bootloader to the kernel; nothing else. Passing a dtb *into* the bootloader is just a bootloader implementation convenience, because the possible variations on an output tree are small, so it's useful to have a skeleton tree built-in. But in order for the bootloader to process those variations correctly, the skeleton *must* be in the right format. dtb input to a bootloader must match the bootloaders expectations. This has always been true, and will continue to be true.
Well said.
g.
Signature · 2 lines
Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd.
[snip]
Quoted 8 lines · Timur Tabi
Quoted 4 lines · Grant Likely
Quoted 1 line · Timur Tabi
Arnd Bergmann wrote:
I think it
is a slippery slope to try and encode conditionals into the structure;Perhaps, which is why I said it should be simple conditionals - two values and a comparison.
I can pretty much guarantee you that someone will find that insufficient and want to expand the conditional representation. This way madness lies.
Quoted 10 lines
Quoted 2 lines
but it is entirely reasonable to encode *data* into the dt that helps make those conditional decisions.
That's okay too, except that if we just add additional nodes that describe conditions, then we need to make sure that whatever parses that DTB must also parse those additional nodes. The only way to do that is create a new version number, like 18, which is marked as being incompatible with the current version (17). Otherwise, a person could pass that DTB to an old version of U-Boot, and U-boot will just pass it on to the kernel as-is.
No. As Grant says, that's not what the version number is for. It represents the version of the encoding, not the content. If you must version the content (which you should try really hard to avoid) the correct way is to add versioning properties to the root node.
Quoted 13 lines
Quoted 3 lines
We've already got that issue. If you pass the device tree for the wrong board it will still validate correctly, but the board is not going to boot.
There's nothing stopping U-Boot today from scanning the device tree and making sure the SOC's compatible node is correct. That's not currently done, but it could be.
Quoted 4 lines
The device tree must match what the bootloader
expects. Changing the version number will do nothing to help this. The version number ensures that the tree is parsable. It does not ensure that it is correct.
Quoted 5 lines
I think you're missing the point. If we add conditional expressions to the device tree (whether attached to a node or as part of a separate node or whatever), we must also add a mechanism to ensure that these conditions are parsed by the boot loader. As far as I know, the only mechanism that can do this is the version identifier.
No. a) the version identifier is not a mechanism for doing that and b) the conditional mechanism is inherently agent-specific, therefore in any case you *must* match an input incomplete tree (by which I mean *anything* that requires processing before it's ready for consumption by the kernel) to the specific bootloader or agent that will process it and pass to the kernel.
Signature · 4 lines
David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
Quoted 3 lines · David Gibson
I can pretty much guarantee you that someone will find that insufficient and want to expand the conditional representation. This way madness lies.
Then let them. We can have version numbers associated with the conditional expressions. If they want to make more complex condition expressions, they can bump the version number and define a spec and write code to parse it.
Quoted 4 lines
No. As Grant says, that's not what the version number is for. It represents the version of the encoding, not the content. If you must version the content (which you should try really hard to avoid) the correct way is to add versioning properties to the root node.
And that's why I prefer updating the DTB format to allow attaching conditional expressions to nodes. This would then necessitate bumping the version number. Older U-Boots will reject this new DTB. We can also modify DTC to support conditional nodes, so that if a customer has an older U-Boot he can't update, he can use DTC to generate a V17 DTB that has the conditionals already processed.
Signature · 2 lines
Timur Tabi Linux kernel developer at Freescale
Quoted 15 lines · Timur Tabi
Quoted 3 lines · David Gibson
I can pretty much guarantee you that someone will find that insufficient and want to expand the conditional representation. This way madness lies.
Then let them. We can have version numbers associated with the conditional expressions. If they want to make more complex condition expressions, they can bump the version number and define a spec and write code to parse it.
Quoted 4 lines
No. As Grant says, that's not what the version number is for. It represents the version of the encoding, not the content. If you must version the content (which you should try really hard to avoid) the correct way is to add versioning properties to the root node.
Quoted 6 lines
And that's why I prefer updating the DTB format to allow attaching conditional expressions to nodes. This would then necessitate bumping the version number. Older U-Boots will reject this new DTB. We can also modify DTC to support conditional nodes, so that if a customer has an older U-Boot he can't update, he can use DTC to generate a V17 DTB that has the conditionals already processed.
Well, yes, folding conditionals into the format would mean a version bump. But I am *not* going to put conditionals into a dtb version, and I'm pretty sure BenH and Paulus would also reject this notion. It's simply not what the format is for. dtbs are for parsing by the kernel, giving a complete device representation. If bootloaders and other things also find them a useful input format, that's great, but I'm not going to significantly extend the semantics just to support other uses.
Now, if you want to define a new binary-level "meta-dtb" format, designed for representing various conditionals or whatnot, which can be processed into a final true dtb, go for it. But a) if you base it on the dtb format, make sure you use a different magic number so as not to interfere with the actual dtbs version progression and b) I think you'll find it's more trouble than it's worth, at least at this stage (feel free to try to prove me wrong on this point, of course).
At present, I think the "meta-dtb" format which makes the most sense, based on a balance between simplicity and flexibility is C, with one or more dtb fragments embedded. This can be done now (though if there are libfdt extensions which would make this usage easier, please suggest them - fdt_graft() is an obvious one).
Signature · 4 lines
David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
Quoted 9 lines · Timur Tabi
Quoted 3 lines · David Gibson
I can pretty much guarantee you that someone will find that insufficient and want to expand the conditional representation. This way madness lies.
Then let them. We can have version numbers associated with the conditional expressions. If they want to make more complex condition expressions, they can bump the version number and define a spec and write code to parse it.
You say that as if bumping the version number is cheap to do.
g.
Signature · 2 lines
Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd.
Quoted 10 lines · Timur Tabi
Quoted 4 lines · Arnd Bergmann
Quoted 3 lines · David Gibson
Or to expand. It's relatively easy now to just include multiple nodes in the tree and either delete or nop some of them out conditionally using libfdt.
Yes, but what better place to store the conditions than in the device tree itself? How would libfdt know where the conditions are? Do you want to have two binary blobs?
libfdt wouldn't. The conditional logic must be in the agent using libfdt.
Quoted 14 lines
Quoted 4 lines
Quoted 4 lines
But the conditional logic should be in the manipulating agent (u-boot or bootwrapper or whatever), there's no way we're going to require a conditional expression parser to interpret the device tree blob itself.
I think it's a great feature that solves a lot of problems, and it does so in an elegant and efficient manner. I look forward to trying to change your mind when I get around to implementing it.
Quoted 4 lines
How about making the logic to nop out nodes a little more generic without changes to the binary format?
E.g. you could have a "linux,conditional-node" property in the device tree whose value is compared to a HW configuration specific string.
Quoted 6 lines
The problem with this is that if you use a version of libfdt that does not understand "linux,conditional-node", then your device tree will be wrong, because it could contain nodes that don't belong. We would need a new, incompatible version number for the device tree to make sure that this doesn't happen, even though nothing has changed in the binary layout of the tree.
Passing an incomplete device tree to an agent that doesn't expect it is always going to cause trouble. This is nothing new. And as you've said the interpretation of these variables in the conditionals is already agent specific, so you'd still have to pass these conditionalised trees to the correct agent in order for them to be correctly interpreted.
No, this has to be agent-local logic. If you want to annotate your agent's input device trees with information that will help it do this, go for it, but don't expect it to be in any way a standardized aspect of the device tree format.
Signature · 4 lines
David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
Quoted 49 lines · Sean MacLennan
The Rev B warp is moving to a 4M NOR / 256M NAND flash setup from the current 64M NOR / 64M NAND. I would like to keep support for the 64M NOR so I modified the boot code to be a bit more dynamic. Here is the new NOR parition layout in the DTS:
nor_flash@0,0 { compatible = "amd,s29gl512n", "cfi-flash"; bank-width = <2>; reg = <0 0 4000000>; #address-cells = <1>; #size-cells = <1>; partition@300000 { label = "fpga"; reg = <300000 40000>; }; partition@340000 { label = "env"; reg = <340000 40000>; }; partition@380000 { label = "u-boot"; reg = <380000 80000>; }; };Yes, the top of the NOR will be empty. Here is the code from
cuboot-warp.cto handle fixups for the 64M flash:static void
warp_fixup_one_nor(u32 from, u32 to){ void *devp; char name[40]; u32 v[2]; sprintf(name, "/plb/opb/ebc/nor_flash@0,0/partition@%x", from); devp = finddevice(name); if (!devp) return; if (getprop(devp, "reg", v, sizeof(v)) == sizeof(v)) { v[0] = to; setprop(devp, "reg", v, sizeof(v)); printf("NOR 64M fixup %x -> %x\n", from, to); } }static void
warp_fixups(void)
Quoted 15 lines
{
unsigned long sysclk = 66000000;
ibm440ep_fixup_clocks(sysclk, 11059200, 50000000);
ibm4xx_sdram_fixup_memsize();
ibm4xx_fixup_ebc_ranges("/plb/opb/ebc");
dt_fixup_mac_addresses(&bd.bi_enetaddr);
/* Fixup for 64M flash on Rev A boards. */
if(bd.bi_flashsize == 0x4000000) {
warp_fixup_one_nor(0x300000, 0x3f00000);
warp_fixup_one_nor(0x340000, 0x3f40000);
warp_fixup_one_nor(0x380000, 0x3f80000);
}
}This doesn't seem right. warp_fixup_one_nor() changes only the partition's offset, so you're not changing the size of any partitions. If you're not going to actually use any of the extra flash space with 64M, I can't see why you'd bother moving around the partitions you have.
Quoted 3 lines
I have tested this with the 64M NOR, and it seems to work. However, are there going to be problems with the partition name not matching the reg address entry?
In practice, probably not. We already do a similar fixup on Ebony for different flash layouts that won't leave the unit names correct. We really should get this right - and the fdt_set_name() function that's now in libfdt should make that possible, it just needs some wiring up to use in the bootwrapper. That can come later, though, for now I think applying your fixups without correcting the unit addresses is adequate.
Quoted 5 lines
If anybody has suggestions on better ways to do this, fire away.
And looking at this code, and other board ports, why is sysclk a local variable and all the other numbers hardcoded in the args? I left it the same way as the others but it does look a bit strange.
I think this also came from Ebony. IIRC, the sysclk isn't strictly speaking fixed, although it almost always has initialized value. The point of the local variable was that I planned to replace the static initialization with some sort of probing once I figured out the details.
Signature · 4 lines
David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
Quoted 6 lines · David Gibson
This doesn't seem right.
warp_fixup_one_nor()changes only the partition's offset, so you're not changing the size of any partitions. If you're not going to actually use any of the extra flash space with 64M, I can't see why you'd bother moving around the partitions you have.
u-boot must be at the bottom of the flash. Also, for the 64M NOR flash you can put everything in the NOR flash, I just don't show the partitions. Booting from NOR is *much* faster than booting from NAND.
Quoted 10 lines
In practice, probably not. We already do a similar fixup on Ebony for different flash layouts that won't leave the unit names correct. We really should get this right - and the
fdt_set_name()function that's now in libfdt should make that possible, it just needs some wiring up to use in the bootwrapper. That can come later, though, for now I think applying your fixups without correcting the unit addresses is adequate.
Ok.
Quoted 11 lines
Quoted 4 lines
If anybody has suggestions on better ways to do this, fire away.
And looking at this code, and other board ports, why is sysclk a local variable and all the other numbers hardcoded in the args? I left it the same way as the others but it does look a bit strange.
I think this also came from Ebony. IIRC, the sysclk isn't strictly speaking fixed, although it almost always has initialized value. The point of the local variable was that I planned to replace the static initialization with some sort of probing once I figured out the details.
That makes sense. I don't think you can probe for the sysclk on the taco, so I may just put it as a constant to the function.
Cheers,
Sean
Quoted 11 lines · Sean MacLennan
Quoted 6 lines · David Gibson
This doesn't seem right.
warp_fixup_one_nor()changes only the partition's offset, so you're not changing the size of any partitions. If you're not going to actually use any of the extra flash space with 64M, I can't see why you'd bother moving around the partitions you have.u-boot must be at the bottom of the flash. Also, for the 64M NOR flash you can put everything in the NOR flash, I just don't show the partitions. Booting from NOR is *much* faster than booting from NAND.
Sorry, still not really following what's going on. Without worrying about the dts formatting or fixup code, can you summarise what the two flash maps look like?
Signature · 4 lines
David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
Quoted 11 lines · David Gibson
Quoted 9 lines · Sean MacLennan
Quoted 7 lines · David Gibson
This doesn't seem right.
warp_fixup_one_nor()changes only the partition's offset, so you're not changing the size of any partitions. If you're not going to actually use any of the extra flash space with 64M, I can't see why you'd bother moving around the partitions you have.
Quoted 9 lines
Quoted 3 lines
u-boot must be at the bottom of the flash. Also, for the 64M NOR flash you can put everything in the NOR flash, I just don't show the partitions. Booting from NOR is *much* faster than booting from NAND.
Sorry, still not really following what's going on. Without worrying about the dts formatting or fixup code, can you summarise what the two flash maps look like?
I guess what is confusing is that I am actually working with 3 flash maps right now, although there will only be one map in the final version.
Map1:
NOR:
Kernel @ 0
Ramdisk
User
FPGA
Env
U-boot @ 63.5M
Map 2:
NOR:
FPGA
Env
U-boot @ 63.5M
NAND:
Kernel @ 0
Ramdisk
User
Map 3:
Same as Map 2 only 4M NOR rather than 64M, so u-boot @ 3.5M.
The u-boot, env, and FPGA are anchored at the bottom of the flash. Kernel is anchored at the top. Everything else goes in the middle.
The FPGA partition contains the FPGA image. The user partition contains a persistent JFFS2 file system. I don't use the user partition, so it doesn't show up in the map I sent.
So map 1 was used until we got the NAND working. Map 2 is an interim solution until we get the 4M flash. Map 3 is the final version.
Cheers,
Sean
Quoted 47 lines · Sean MacLennan
Quoted 17 lines · David Gibson
Quoted 9 lines · Sean MacLennan
Quoted 7 lines · David Gibson
This doesn't seem right.
warp_fixup_one_nor()changes only the partition's offset, so you're not changing the size of any partitions. If you're not going to actually use any of the extra flash space with 64M, I can't see why you'd bother moving around the partitions you have.Quoted 5 lines
u-boot must be at the bottom of the flash. Also, for the 64M NOR flash you can put everything in the NOR flash, I just don't show the partitions. Booting from NOR is *much* faster than booting from NAND.
Quoted 5 lines
Sorry, still not really following what's going on. Without worrying about the dts formatting or fixup code, can you summarise what the two flash maps look like?
I guess what is confusing is that I am actually working with 3 flash maps right now, although there will only be one map in the final version.
Map1:
NOR:
Kernel @ 0
Ramdisk
User
FPGA
Env
U-boot @ 63.5MMap 2:
NOR:
FPGA
Env
U-boot @ 63.5M
NAND:
Kernel @ 0
Ramdisk
User
Quoted 2 lines
Map 3:
Same as Map 2 only 4M NOR rather than 64M, so u-boot @ 3.5M.
But the partitions are all the same size, so in Map 2 there's a great big gap between Env and U-boot? Or there's a great big gap before FPGA?
Quoted 2 lines
The u-boot, env, and FPGA are anchored at the bottom of the flash. Kernel is anchored at the top. Everything else goes in the middle.
Um.. so "bottom" actually means "high addresses" in the above?
Quoted 6 lines
The FPGA partition contains the FPGA image. The user partition contains a persistent JFFS2 file system. I don't use the user partition, so it doesn't show up in the map I sent.
So map 1 was used until we got the NAND working. Map 2 is an interim solution until we get the 4M flash. Map 3 is the final version.
Signature · 4 lines
David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
Quoted 4 lines · David Gibson
But the partitions are all the same size, so in Map 2 there's a great big gap between Env and U-boot? Or there's a great big gap before FPGA?
There's a great big gap before the FPGA, 63M worth. Before we got the NAND working, we stored the kernel, the ramdisk image, and a persistent store there. But we moved everything to the NAND and are now going to a much smaller NOR chip.
Quoted 6 lines
Quoted 3 lines
The u-boot, env, and FPGA are anchored at the bottom of the flash. Kernel is anchored at the top. Everything else goes in the middle.
Um.. so "bottom" actually means "high addresses" in the above?
Yes. Top = offset 0, bottom = size-of-flash.
And there is a bug in the initial code, I have to update the reg length field. I now have a Rev B board in my hands and tested that it works for both Rev A and Rev B.
Cheers,
Sean
Quoted 9 lines · Sean MacLennan
Quoted 4 lines · David Gibson
But the partitions are all the same size, so in Map 2 there's a great big gap between Env and U-boot? Or there's a great big gap before FPGA?
There's a great big gap before the FPGA, 63M worth. Before we got the NAND working, we stored the kernel, the ramdisk image, and a persistent store there. But we moved everything to the NAND and are now going to a much smaller NOR chip.
Ok.
Quoted 7 lines
Quoted 6 lines
Quoted 3 lines
The u-boot, env, and FPGA are anchored at the bottom of the flash. Kernel is anchored at the top. Everything else goes in the middle.
Um.. so "bottom" actually means "high addresses" in the above?
Yes. Top = offset 0, bottom = size-of-flash.
Um.. ok. Not the way around I'd use, certainly...
Quoted 2 lines
And there is a bug in the initial code, I have to update the reg length field.
Err.. now I'm doubly confused. Initially I thought you'd need to change the size part of reg somewhere, but your description above just convinced me you didn't (because you were essentially just shifting a 4M map up into the high rather than low 4M of the 64M space). Now you're saying you do..
Quoted 2 lines
I now have a Rev B board in my hands and tested that it works for both Rev A and Rev B.
Signature · 4 lines
David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
Quoted 6 lines · David Gibson
Err.. now I'm doubly confused. Initially I thought you'd need to change the size part of reg somewhere, but your description above just convinced me you didn't (because you were essentially just shifting a 4M map up into the high rather than low 4M of the 64M space). Now you're saying you do..
If you tell the mtd driver that the flash is 64M, when it is really 4M, it goes oops. So you do have to get the size right in the reg field.
Cheers,
Sean
Quoted 9 lines · Sean MacLennan
Quoted 6 lines · David Gibson
Err.. now I'm doubly confused. Initially I thought you'd need to change the size part of reg somewhere, but your description above just convinced me you didn't (because you were essentially just shifting a 4M map up into the high rather than low 4M of the 64M space). Now you're saying you do..
If you tell the mtd driver that the flash is 64M, when it is really 4M, it goes oops. So you do have to get the size right in the reg field.
Ah! The top-level reg field, not the partitions. Now I understand.
Except, since the 4M layout is the expected future norm, I would have thought that's what you'd encode into the dts, and mangle it into the 64M version, rather than the other way around.
Signature · 4 lines
David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
Quoted 9 lines · Sean MacLennan
Quoted 6 lines · David Gibson
Err.. now I'm doubly confused. Initially I thought you'd need to change the size part of reg somewhere, but your description above just convinced me you didn't (because you were essentially just shifting a 4M map up into the high rather than low 4M of the 64M space). Now you're saying you do..
If you tell the mtd driver that the flash is 64M, when it is really 4M, it goes oops. So you do have to get the size right in the reg field.
It'd be nice if we could pass in a flag to tell it not to try to find additional consecutive chips in the mapping... It's a shame to have probable chips, and still have to know how big they are anyway.
-Scott
Quoted 8 lines · Scott Wood
Quoted 7 lines · Sean MacLennan
Quoted 6 lines · David Gibson
Err.. now I'm doubly confused. Initially I thought you'd need to change the size part of reg somewhere, but your description above just convinced me you didn't (because you were essentially just shifting a 4M map up into the high rather than low 4M of the 64M space). Now you're saying you do..
Quoted 6 lines
Quoted 2 lines
If you tell the mtd driver that the flash is 64M, when it is really 4M, it goes oops. So you do have to get the size right in the reg field.
It'd be nice if we could pass in a flag to tell it not to try to find additional consecutive chips in the mapping... It's a shame to have probable chips, and still have to know how big they are anyway.
That is the job of the boot loader or wrapper. The whole concept of the device tree is that by the time it gets to the kernel it is an accurate representation of the hardware; not a list of things which might or might not be present.
I see two choices here;
1. have a different .dts variant for each board config (or a .dts with macros that can generate different .dtb variants)
2. make the boot code massage the tree so it is accurate before it gets to the kernel.
Either way, the mtd driver must be able to trust that the dt is correct and complete.
g.
Signature · 2 lines
Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd.
Quoted 6 lines · Grant Likely
Quoted 3 lines · Scott Wood
It'd be nice if we could pass in a flag to tell it not to try to find additional consecutive chips in the mapping... It's a shame to have probable chips, and still have to know how big they are anyway.
That is the job of the boot loader or wrapper.
Hmm? All I meant was that it'd be nice if there were an option in the Linux mtd code to disable the "look for another chip and cause a machine check if it isn't there" functionality. It was an aside from the dts-variant issue.
Quoted 4 lines
The whole concept of
the device tree is that by the time it gets to the kernel it is an accurate representation of the hardware; not a list of things which might or might not be present.
But we don't generally include things which can be probed in a standard manner... which includes flash size.
-Scott
Quoted 4 lines · Scott Wood
Hmm? All I meant was that it'd be nice if there were an option in the Linux mtd code to disable the "look for another chip and cause a machine check if it isn't there" functionality. It was an aside from the dts-variant issue.
Yeah, maybe -- although the probe code is hairy enough already; I'm not massively keen on adding to it. Or even looking at it hard, if the truth be told.
Alternatively, perhaps we should catch the machine check and recover (like we do for failing I/O accesses on ppc32).
Signature · 1 line
dwmw2