Re: Could the DTS experts look at this?
On Tue, 2008-02-12 at 13:10 -0600, Scott Wood wrote: 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). -- dwmw2 ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: Could the DTS experts look at this?
On Feb 12, 2008 11:52 AM, Scott Wood [EMAIL PROTECTED] wrote: On Mon, Feb 11, 2008 at 07:41:07PM -0500, Sean MacLennan wrote: David Gibson wrote: 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. 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. -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: Could the DTS experts look at this?
On Feb 12, 2008 8:44 AM, Timur Tabi [EMAIL PROTECTED] wrote: Arnd Bergmann wrote: On Tuesday 12 February 2008, David Gibson wrote: 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. 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. 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. -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: Could the DTS experts look at this?
On Feb 12, 2008 12:45 PM, Timur Tabi [EMAIL PROTECTED] wrote: Grant Likely wrote: 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. 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. 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. 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. 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 ... 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. 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. 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 socchip@addr to [EMAIL PROTECTED] 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. 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
Re: Could the DTS experts look at this?
David Gibson wrote: 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. 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. -- Timur Tabi Linux kernel developer at Freescale ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: Could the DTS experts look at this?
David Gibson wrote: 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. 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. -- Timur Tabi Linux kernel developer at Freescale ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: Could the DTS experts look at this?
On Tue, Feb 12, 2008 at 05:41:12PM -0600, Timur Tabi wrote: David Gibson wrote: 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. -- 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 ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: Could the DTS experts look at this?
David Gibson wrote: 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. -- Timur Tabi Linux kernel developer at Freescale ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: Could the DTS experts look at this?
On Tue, Feb 12, 2008 at 01:45:39PM -0600, Timur Tabi wrote: Grant Likely wrote: [snip] That's not a dtb version issue. That is a dtb content issue. Technically, that's true, but ... 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. 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. 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? You don't. If your agent takes a dtb, dtb layout and agent must match. 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. -- 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 ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: Could the DTS experts look at this?
On Tue, Feb 12, 2008 at 05:47:23PM -0600, Timur Tabi wrote: David Gibson wrote: 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. 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. 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). -- 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 ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: Could the DTS experts look at this?
On Tue, Feb 12, 2008 at 12:51:06PM -0600, Scott Wood wrote: On Tue, Feb 12, 2008 at 11:36:33AM +1100, David Gibson wrote: On Tue, Feb 12, 2008 at 01:21:44AM +0100, Arnd Bergmann wrote: On Tuesday 12 February 2008, David Gibson wrote: 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. 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... 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. -- 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 ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: Could the DTS experts look at this?
On Feb 12, 2008 4:47 PM, Timur Tabi [EMAIL PROTECTED] wrote: David Gibson wrote: 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. -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: Could the DTS experts look at this?
On Feb 12, 2008 4:35 PM, David Gibson [EMAIL PROTECTED] wrote: 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. -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: Could the DTS experts look at this?
On Tue, Feb 12, 2008 at 09:44:39AM -0600, Timur Tabi wrote: Arnd Bergmann wrote: On Tuesday 12 February 2008, David Gibson wrote: 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. 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. 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. 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. -- 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 ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: Could the DTS experts look at this?
Grant Likely wrote: On Feb 12, 2008 11:52 AM, Scott Wood [EMAIL PROTECTED] wrote: 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. 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 ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: Could the DTS experts look at this?
Grant Likely wrote: On Feb 12, 2008 8:44 AM, Timur Tabi [EMAIL PROTECTED] wrote: Arnd Bergmann wrote: On Tuesday 12 February 2008, David Gibson wrote: 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. I agree with that. 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. 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. 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 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. 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. 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. 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. -- Timur Tabi Linux kernel developer at Freescale ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: Could the DTS experts look at this?
On Feb 12, 2008 12:08 PM, Timur Tabi [EMAIL PROTECTED] wrote: Grant Likely wrote: 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. 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. 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. 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. 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. 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. -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: Could the DTS experts look at this?
On Tue, Feb 12, 2008 at 11:36:33AM +1100, David Gibson wrote: On Tue, Feb 12, 2008 at 01:21:44AM +0100, Arnd Bergmann wrote: On Tuesday 12 February 2008, David Gibson wrote: 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. 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... 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 ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: Could the DTS experts look at this?
[snip] On Tue, Feb 12, 2008 at 01:08:24PM -0600, Timur Tabi wrote: Grant Likely wrote: On Feb 12, 2008 8:44 AM, Timur Tabi [EMAIL PROTECTED] wrote: 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. 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. 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. 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. 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. -- 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 ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: Could the DTS experts look at this?
Grant Likely wrote: 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 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. 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? 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. 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. 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 ... 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. 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? 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? -- Timur Tabi Linux kernel developer at Freescale ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: Could the DTS experts look at this?
Arnd Bergmann wrote: On Tuesday 12 February 2008, David Gibson wrote: 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? 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. 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. -- Timur Tabi Linux kernel developer at Freescale ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: Could the DTS experts look at this?
On Mon, Feb 11, 2008 at 07:41:07PM -0500, Sean MacLennan wrote: David Gibson wrote: 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 ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: Could the DTS experts look at this?
Arnd Bergmann wrote: 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. -- Timur Tabi Linux kernel developer at Freescale ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: Could the DTS experts look at this?
On Mon, Feb 11, 2008 at 07:41:07PM -0500, Sean MacLennan wrote: David Gibson wrote: 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. -- 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 ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: Could the DTS experts look at this?
On Tue, Feb 12, 2008 at 01:21:44AM +0100, Arnd Bergmann wrote: On Tuesday 12 February 2008, David Gibson wrote: 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. 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... -- 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 ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: Could the DTS experts look at this?
On Mon, Feb 11, 2008 at 08:07:14PM -0500, Sean MacLennan wrote: David Gibson wrote: 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. 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... 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.. I now have a Rev B board in my hands and tested that it works for both Rev A and Rev B. -- 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 ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: Could the DTS experts look at this?
David Gibson wrote: 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. 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 ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: Could the DTS experts look at this?
On Mon, Feb 11, 2008 at 11:57:10AM -0600, Timur Tabi wrote: Arnd Bergmann wrote: 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. In the binary tree representation itself? No way. -- 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 ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: Could the DTS experts look at this?
David Gibson wrote: 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 ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: Could the DTS experts look at this?
On Sun, Feb 10, 2008 at 10:49:55PM -0500, Sean MacLennan wrote: David Gibson wrote: On Sun, Feb 10, 2008 at 09:40:19PM -0500, Sean MacLennan wrote: David Gibson wrote: 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? 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. 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? 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? 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. -- 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 ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: Could the DTS experts look at this?
On Tue, Feb 12, 2008 at 10:54:09AM +1100, David Gibson wrote: On Mon, Feb 11, 2008 at 11:57:10AM -0600, Timur Tabi wrote: Arnd Bergmann wrote: 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. 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. -- 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 ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: Could the DTS experts look at this?
On Tuesday 12 February 2008, David Gibson wrote: 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. 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. Arnd ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: Could the DTS experts look at this?
On Fri, Feb 08, 2008 at 06:30:56PM -0500, Sean MacLennan wrote:
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:
[EMAIL PROTECTED],0 {
compatible = amd,s29gl512n, cfi-flash;
bank-width = 2;
reg = 0 0 400;
#address-cells = 1;
#size-cells = 1;
[EMAIL PROTECTED] {
label = fpga;
reg = 30 4;
};
[EMAIL PROTECTED] {
label = env;
reg = 34 4;
};
[EMAIL PROTECTED] {
label = u-boot;
reg = 38 8;
};
};
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/[EMAIL PROTECTED],0/[EMAIL PROTECTED],
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 = 6600;
ibm440ep_fixup_clocks(sysclk, 11059200, 5000);
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 == 0x400) {
warp_fixup_one_nor(0x30, 0x3f0);
warp_fixup_one_nor(0x34, 0x3f4);
warp_fixup_one_nor(0x38, 0x3f8);
}
}
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.
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.
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.
--
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
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: Could the DTS experts look at this?
David Gibson wrote: 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. 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. 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 ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: Could the DTS experts look at this?
David Gibson wrote: On Sun, Feb 10, 2008 at 09:40:19PM -0500, Sean MacLennan wrote: David Gibson wrote: 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? 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 ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: Could the DTS experts look at this?
On Sun, Feb 10, 2008 at 09:40:19PM -0500, Sean MacLennan wrote: David Gibson wrote: 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? -- 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 ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: Could the DTS experts look at this?
On Saturday 09 February 2008, Sean MacLennan wrote: 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 ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: Could the DTS experts look at this?
Arnd Bergmann wrote: 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. 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. 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 ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
