Re: [PATCH 2/8] pseries: phyp dump: reserve-release proof-of-concept
On Tue, 2008-02-12 at 01:08 -0600, Manish Ahuja wrote: Initial patch for reserving memory in early boot, and freeing it later. If the previous boot had ended with a crash, the reserved memory would contain a copy of the crashed kernel data. Signed-off-by: Manish Ahuja [EMAIL PROTECTED] Signed-off-by: Linas Vepstas [EMAIL PROTECTED] arch/powerpc/kernel/prom.c | 50 arch/powerpc/kernel/rtas.c | 32 + arch/powerpc/platforms/pseries/Makefile|1 arch/powerpc/platforms/pseries/phyp_dump.c | 71 + include/asm-powerpc/phyp_dump.h| 38 +++ include/asm/rtas.h |3 + ^^ asm/rtas.h doesn't exist. You need to clean your tree, and patch asm-powerpc/rtas.h instead. cheers -- Michael Ellerman OzLabs, IBM Australia Development Lab wwweb: http://michael.ellerman.id.au phone: +61 2 6212 1183 (tie line 70 21183) We do not inherit the earth from our ancestors, we borrow it from our children. - S.M.A.R.T Person signature.asc Description: This is a digitally signed message part ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 6/8] pseries: phyp dump: Invalidate and print dump areas.
Hi Manish, On Tue, 12 Feb 2008 01:18:22 -0600 Manish Ahuja [EMAIL PROTECTED] wrote: -static void -release_memory_range(unsigned long start_pfn, unsigned long nr_pages) +static +void release_memory_range(unsigned long start_pfn, unsigned long nr_pages) Cosmetic changes like this should normally be put in separate patch as they just distract from a real review (which this isn't :-)) -- Cheers, Stephen Rothwell[EMAIL PROTECTED] http://www.canb.auug.org.au/~sfr/ pgp0nUI4CWqt4.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 3/8] pseries: phyp dump: use sysfs to release reserved mem
Hi Manish, Just a small comment. On Tue, 12 Feb 2008 01:11:58 -0600 Manish Ahuja [EMAIL PROTECTED] wrote: + /* Is there dump data waiting for us? */ + rtas = of_find_node_by_path(/rtas); + dump_header = of_get_property(rtas, ibm,kernel-dump, header_len); You need an of_node_put(rtas) here. + if (dump_header == NULL) { + release_all(); + return 0; + } -- Cheers, Stephen Rothwell[EMAIL PROTECTED] http://www.canb.auug.org.au/~sfr/ pgpaXTk0b83pb.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 4/8] pseries: phyp dump: register dump area.
Hi Manish, - /* Is there dump data waiting for us? */ + /* Is there dump data waiting for us? If there isn't, + * then register a new dump area, and release all of + * the rest of the reserved ram. + * + * The /rtas/ibm,kernel-dump rtas node is present only + * if there is dump data waiting for us. + */ rtas = of_find_node_by_path(/rtas); dump_header = of_get_property(rtas, ibm,kernel-dump, header_len); + of_node_put(rtas); Oh, here is the of_node_put() - you should move that to patch 3. -- Cheers, Stephen Rothwell[EMAIL PROTECTED] http://www.canb.auug.org.au/~sfr/ pgpzgB3ajSftO.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: DTS question - MPC5200b
On Tuesday 12 February 2008 19:37:07 Nick wrote: How do I specify the timer based on the cell-index? I don't know if that is possible to do in a one call, but maybe using the approach from mpc52xx_uart might help? --clip-- for_each_node_by_type(np, serial) { if (!of_match_node(mpc52xx_uart_of_match, np)) continue; /* Is a particular device number requested? */ devno = of_get_property(np, port-number, NULL); mpc52xx_uart_of_assign(of_node_get(np), devno ? *devno : -1); } --clip-- And change serial-gpt, port-number to cell-index and add some logic to select the devices you want. Or if you wan't to do it a bit differently you could add a pseudo device outside the main tree like mydev { gpt-dev = the_gpt_dev: }; And get it that way. However I don't know if this is recommended approach, but I've used it for some simple stuff like binding gpt in PWM mode to framebuffer backlight, along with power-pin. Please correct any mistakes you who know better. - Jarno ___ 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
[PATCH 2/8] pseries: phyp dump: reserve-release proof-of-concept
Michael, Fixed. -Manish -- Initial patch for reserving memory in early boot, and freeing it later. If the previous boot had ended with a crash, the reserved memory would contain a copy of the crashed kernel data. Signed-off-by: Manish Ahuja [EMAIL PROTECTED] Signed-off-by: Linas Vepstas [EMAIL PROTECTED] arch/powerpc/kernel/prom.c | 50 arch/powerpc/kernel/rtas.c | 32 + arch/powerpc/platforms/pseries/Makefile|1 arch/powerpc/platforms/pseries/phyp_dump.c | 71 + include/asm-powerpc/phyp_dump.h| 38 +++ include/asm-powerpc/rtas.h |3 + 6 files changed, 195 insertions(+) Index: 2.6.24-rc5/include/asm-powerpc/phyp_dump.h === --- /dev/null 1970-01-01 00:00:00.0 + +++ 2.6.24-rc5/include/asm-powerpc/phyp_dump.h 2008-02-12 16:12:45.0 -0600 @@ -0,0 +1,38 @@ +/* + * Hypervisor-assisted dump + * + * Linas Vepstas, Manish Ahuja 2007 + * Copyright (c) 2007 IBM Corp. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version + * 2 of the License, or (at your option) any later version. + */ + +#ifndef _PPC64_PHYP_DUMP_H +#define _PPC64_PHYP_DUMP_H + +#ifdef CONFIG_PHYP_DUMP + +/* The RMR region will be saved for later dumping + * whenever the kernel crashes. Set this to 256MB. */ +#define PHYP_DUMP_RMR_START 0x0 +#define PHYP_DUMP_RMR_END (1UL28) + +struct phyp_dump { + /* Memory that is reserved during very early boot. */ + unsigned long init_reserve_start; + unsigned long init_reserve_size; + /* Check status during boot if dump supported, active present*/ + unsigned long phyp_dump_configured; + unsigned long phyp_dump_is_active; + /* store cpu hpte size */ + unsigned long cpu_state_size; + unsigned long hpte_region_size; +}; + +extern struct phyp_dump *phyp_dump_info; + +#endif /* CONFIG_PHYP_DUMP */ +#endif /* _PPC64_PHYP_DUMP_H */ Index: 2.6.24-rc5/arch/powerpc/platforms/pseries/phyp_dump.c === --- /dev/null 1970-01-01 00:00:00.0 + +++ 2.6.24-rc5/arch/powerpc/platforms/pseries/phyp_dump.c 2008-02-12 16:12:45.0 -0600 @@ -0,0 +1,71 @@ +/* + * Hypervisor-assisted dump + * + * Linas Vepstas, Manish Ahuja 2007 + * Copyrhgit (c) 2007 IBM Corp. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version + * 2 of the License, or (at your option) any later version. + * + */ + +#include linux/init.h +#include linux/mm.h +#include linux/pfn.h +#include linux/swap.h + +#include asm/page.h +#include asm/phyp_dump.h + +/* Global, used to communicate data between early boot and late boot */ +static struct phyp_dump phyp_dump_global; +struct phyp_dump *phyp_dump_info = phyp_dump_global; + +/** + * release_memory_range -- release memory previously lmb_reserved + * @start_pfn: starting physical frame number + * @nr_pages: number of pages to free. + * + * This routine will release memory that had been previously + * lmb_reserved in early boot. The released memory becomes + * available for genreal use. + */ +static void +release_memory_range(unsigned long start_pfn, unsigned long nr_pages) +{ + struct page *rpage; + unsigned long end_pfn; + long i; + + end_pfn = start_pfn + nr_pages; + + for (i=start_pfn; i = end_pfn; i++) { + rpage = pfn_to_page(i); + if (PageReserved(rpage)) { + ClearPageReserved(rpage); + init_page_count(rpage); + __free_page(rpage); + totalram_pages++; + } + } +} + +static int __init phyp_dump_setup(void) +{ + unsigned long start_pfn, nr_pages; + + /* If no memory was reserved in early boot, there is nothing to do */ + if (phyp_dump_info-init_reserve_size == 0) + return 0; + + /* Release memory that was reserved in early boot */ + start_pfn = PFN_DOWN(phyp_dump_info-init_reserve_start); + nr_pages = PFN_DOWN(phyp_dump_info-init_reserve_size); + release_memory_range(start_pfn, nr_pages); + + return 0; +} + +subsys_initcall(phyp_dump_setup); Index: 2.6.24-rc5/arch/powerpc/platforms/pseries/Makefile === --- 2.6.24-rc5.orig/arch/powerpc/platforms/pseries/Makefile 2008-02-12 16:11:44.0 -0600 +++ 2.6.24-rc5/arch/powerpc/platforms/pseries/Makefile 2008-02-12 16:12:45.0 -0600 @@ -18,3 +18,4
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: [PATCH] FIx compile of swim3 as module
On Wed, 2008-02-13 at 13:40 +1100, Tony Breeds wrote: The current pmac32_defconfig fails to build with the following error: Building modules, stage 2. ERROR: check_media_bay [drivers/block/swim3.ko] undefined! WARNING: modpost: Found 23 section mismatch(es). To see full details build your kernel with: 'make CONFIG_DEBUG_SECTION_MISMATCH=y' make[2]: *** [__modpost] Error 1 Bart, I told you I didn't want those ifdef's in mediabay ... they are just cluttering things and causing trouble. This patch fixes that. Signed-off-by: Tony Breeds [EMAIL PROTECTED] Acked-by: Benjamin Herrenschmidt [EMAIL PROTECTED] --- drivers/block/swim3.c|4 drivers/macintosh/mediabay.c |2 -- 2 files changed, 0 insertions(+), 6 deletions(-) diff --git a/drivers/block/swim3.c b/drivers/block/swim3.c index b4e462f..730ccea 100644 --- a/drivers/block/swim3.c +++ b/drivers/block/swim3.c @@ -251,10 +251,6 @@ static int floppy_release(struct inode *inode, struct file *filp); static int floppy_check_change(struct gendisk *disk); static int floppy_revalidate(struct gendisk *disk); -#ifndef CONFIG_PMAC_MEDIABAY -#define check_media_bay(which, what) 1 -#endif - static void swim3_select(struct floppy_state *fs, int sel) { struct swim3 __iomem *sw = fs-swim3; diff --git a/drivers/macintosh/mediabay.c b/drivers/macintosh/mediabay.c index 9367882..51a1128 100644 --- a/drivers/macintosh/mediabay.c +++ b/drivers/macintosh/mediabay.c @@ -416,7 +416,6 @@ static void poll_media_bay(struct media_bay_info* bay) } } -#ifdef CONFIG_MAC_FLOPPY int check_media_bay(struct device_node *which_bay, int what) { int i; @@ -431,7 +430,6 @@ int check_media_bay(struct device_node *which_bay, int what) return -ENODEV; } EXPORT_SYMBOL(check_media_bay); -#endif /* CONFIG_MAC_FLOPPY */ #ifdef CONFIG_BLK_DEV_IDE_PMAC int check_media_bay_by_base(unsigned long base, int what) Yours Tony linux.conf.auhttp://linux.conf.au/ || http://lca2008.linux.org.au/ Jan 28 - Feb 02 2008 The Australian Linux Technical Conference! ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] FIx compile of swim3 as module
On Tue, Feb 12, 2008 at 08:48:02PM -0600, Josh Boyer wrote: Kyle posted a slightly different patch that seemed to still keep the original intention of ifdefery in tact. I've no idea which is better, but the three of you might want to compare notes. /me checks Hmm Kyle's seem the leave the: #ifndef CONFIG_PMAC_MEDIABAY #define check_media_bay(which, what)1 #endif in swim3.c, which doesn't seem right. But I'm easy as long as it gets fixed. :) Yours Tony linux.conf.auhttp://linux.conf.au/ || http://lca2008.linux.org.au/ Jan 28 - Feb 02 2008 The Australian Linux Technical Conference! ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] FIx compile of swim3 as module
On Wed, 13 Feb 2008 13:40:20 +1100 [EMAIL PROTECTED] (Tony Breeds) wrote: The current pmac32_defconfig fails to build with the following error: Building modules, stage 2. ERROR: check_media_bay [drivers/block/swim3.ko] undefined! WARNING: modpost: Found 23 section mismatch(es). To see full details build your kernel with: 'make CONFIG_DEBUG_SECTION_MISMATCH=y' make[2]: *** [__modpost] Error 1 This patch fixes that. Kyle posted a slightly different patch that seemed to still keep the original intention of ifdefery in tact. I've no idea which is better, but the three of you might want to compare notes. josh ___ 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
[PATCH rev2] powerpc: Marvell 64x60 EDAC platform devices setup
Creating platform devices (memory controller, sram error registers, cpu error registers, PCI error registers) for Error Detection and Correction (EDAC) driver. The platform devices allow the mv64x60 EDAC driver to detect errors from the memory controller (ECC erorrs), SRAM controller, CPU data path error registers, and PCI error registers. The errors are reported to syslog. Software ECC scrubbing is provided. These replace the mv64x60 error handlers in the ppc branch. They are being moved to EDAC subsystem in order to centralize error reporting. The error reporting can be triggered via interrupts from the mv64x60 bridge chip or via polling mechanism provided by the EDAC core code. Signed-off-by: Dave Jiang [EMAIL PROTECTED] Acked-by: Dale Farnsworth [EMAIL PROTECTED] --- Updated with changes from Stephen Rothwell mv64x60_dev.c | 88 ++ 1 file changed, 88 insertions(+) diff --git a/arch/powerpc/sysdev/mv64x60_dev.c b/arch/powerpc/sysdev/mv64x60_dev.c index efda002..21bb791 100644 --- a/arch/powerpc/sysdev/mv64x60_dev.c +++ b/arch/powerpc/sysdev/mv64x60_dev.c @@ -17,6 +17,7 @@ #include linux/platform_device.h #include asm/prom.h +#include asm/io.h /* * These functions provide the necessary setup for the mv64x60 drivers. @@ -439,6 +440,63 @@ error: return err; } +static int __init mv64x60_edac_pdev_init(struct device_node *np, + int id, + int num_addr, + const char *pdev_name) +{ + struct resource *r; + struct platform_device *pdev; + int i, ret; + + r = kzalloc(num_addr * sizeof(*r) + sizeof(*r), GFP_KERNEL); + if (!r) + return -ENOMEM; + + for (i = 0; i num_addr; i++) { + ret = of_address_to_resource(np, i, r[i]); + if (ret) { + kfree(r); + return ret; + } + } + + of_irq_to_resource(np, 0, r[i]); + + pdev = platform_device_register_simple(pdev_name, id, r, num_addr + 1); + + kfree(r); + + if (IS_ERR(pdev)) + return PTR_ERR(pdev); + + return 0; +} + +#ifdef CONFIG_PCI +/* + * Bit 0 of MV64x60_PCIx_ERR_MASK does not exist on the 64360 and because of + * errata FEr-#11 and FEr-##16 for the 64460, it should be 0 on that chip as + * well. IOW, don't set bit 0. + */ +#define MV64X60_PCIx_ERR_MASK_VAL 0x00a50c24 + +/* Erratum FEr PCI-#16: clear bit 0 of PCI SERRn Mask reg. */ +static int __init mv64x60_pci_fixup(struct device_node *np) +{ + void __iomem *pci_serr; + + pci_serr = of_iomap(np, 1); + if (!pci_serr) + return -ENOMEM; + + out_le32(pci_serr, in_le32(pci_serr) ~0x1); + iounmap(pci_serr); + + return 0; +} +#endif /* CONFIG_PCI */ + static int __init mv64x60_device_setup(void) { struct device_node *np = NULL; @@ -460,6 +518,36 @@ static int __init mv64x60_device_setup(void) if ((err = mv64x60_i2c_device_setup(np, id++))) goto error; + id = 0; + for_each_compatible_node(np, NULL, marvell,mv64x60-mem-ctrl) + if ((err = mv64x60_edac_pdev_init(np, id++, 1, + mv64x60_mc_err))) + goto error; + + id = 0; + for_each_compatible_node(np, NULL, marvell,mv64x60-cpu-error) + if ((err = mv64x60_edac_pdev_init(np, id++, 2, + mv64x60_cpu_err))) + goto error; + + id = 0; + for_each_compatible_node(np, NULL, marvell,mv64x60-sram-ctrl) + if ((err = mv64x60_edac_pdev_init(np, id++, 1, + mv64x60_sram_err))) + goto error; + +#ifdef CONFIG_PCI + id = 0; + for_each_compatible_node(np, NULL, marvell,mv64x60-pci-error) { + if ((err = mv64x60_pci_fixup(np))) + goto error; + + if ((err = mv64x60_edac_pdev_init(np, id++, 1, + mv64x60_pci_err))) + goto error; + } +#endif + /* support up to one watchdog timer */ np = of_find_compatible_node(np, NULL, marvell,mv64x60-wdt); if (np) { ___ 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: DTS question - MPC5200b
On Tue, Feb 12, 2008 at 12:37:07PM -0500, Nick wrote: Hi, I need some help. I am trying to access timer 7 on the MPC5200B processor. I have the DTS file setup like this Others have addressed the most salient points, but some general corrections for your device tree.. [EMAIL PROTECTED] {// General Purpose Timer device_type = gpt; device_type shouldn't be here. compatible = fsl,mpc5200b-gpt,fsl,mpc5200-gpt; cell-index = 7; You probably don't want cell-index. cell-index should *only* be used when there is some global register somewhere that's indexed by the cell number. reg = 670 10; interrupts = 1 10 0; interrupt-parent = mpc5200_pic; }; I have timers 0 to 6 defined the same way except the cell-index reflects the timer number. Presumably 'reg' is different for each, as well. In my platform file where I am doing my board setup, I tried the following. timer7 = mpc52xx_find_and_map (mpc5200b-gpt); How do I specify the timer based on the cell-index? You don't. As Grant explains, use reg instead. -- 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: [PATCH 00/18] ide: warm-plug support for IDE devices and other goodies
On Wed, 13 Feb 2008 08:04:07 +1100 Benjamin Herrenschmidt [EMAIL PROTECTED] wrote: On Fri, 2008-02-08 at 19:40 +1100, Benjamin Herrenschmidt wrote: On Fri, 2008-02-08 at 01:44 +0100, Bartlomiej Zolnierkiewicz wrote: - couple of fixes and preparatory patches - rework of PowerMac media-bay support ([un]register IDE devices instead of [un]registering IDE interface) [ it is the main reason for spamming PPC ML ] Interesting... I was thinking about doing a full remove of the device at a higher level instead but I suppose what you propose is easier. I'll have a look test next week hopefully. Also, the above would have the advantage of not relying on drivers/ide infrastructure, and thus working with libata (once somebody has ported pmac ide to libata). Unfortunately you need a degree in dentistry to open a Macintosh up and fix it otherwise we would have support by now. Alan ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 00/18] ide: warm-plug support for IDE devices and other goodies
On Tuesday 12 February 2008, Benjamin Herrenschmidt wrote: On Fri, 2008-02-08 at 19:40 +1100, Benjamin Herrenschmidt wrote: On Fri, 2008-02-08 at 01:44 +0100, Bartlomiej Zolnierkiewicz wrote: - couple of fixes and preparatory patches - rework of PowerMac media-bay support ([un]register IDE devices instead of [un]registering IDE interface) [ it is the main reason for spamming PPC ML ] Interesting... I was thinking about doing a full remove of the device at a higher level instead but I suppose what you propose is easier. I'll have a look test next week hopefully. Also, the above would have the advantage of not relying on drivers/ide infrastructure, and thus working with libata (once somebody has ported pmac ide to libata). Neither of these things exist at the moment so lets stick to the existing code which is scheduled for 2.6.26 and which finally allows removal of ppc specific ide hacks from arch/{powerpc,ppc} (500 LOC gone, patches to be posted this week), not to mention other nice changes in the future... Thanks, Bart ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCH] [RFC] Xilinx: Add generic configuration option to enable all xilinx drivers.
In the future, this will be used to provide similar configuration for PowerPC and Microblaze. It may also be convenient for those using Xilinx cores as peripherals for external processors, rather than explicitly having a dependance on the processor architecture. Signed-off-by: Stephen Neuendorffer [EMAIL PROTECTED] --- Grant, This is the patch, updated for all of the drivers that I think are in the tree. I think the problematic parts may be the ppc part, which is required for backward compatibility. If this has to wait until ppc dies, then that's fine with me, I guess. It may also be better to clean up the Kconfig lines for Sysace and framebuffer drivers by having PPC32 or PPC4xx select XILINX_DRIVERS. My understanding is that those config options are there because of people using external PPCs with those devices in the FPGA. Steve --- arch/powerpc/platforms/40x/Kconfig |1 + arch/ppc/platforms/4xx/Kconfig |1 + drivers/block/Kconfig |2 +- drivers/char/Kconfig |2 +- drivers/misc/Kconfig | 10 ++ drivers/serial/Kconfig |2 +- drivers/spi/Kconfig|2 +- drivers/video/Kconfig |2 +- 8 files changed, 17 insertions(+), 5 deletions(-) diff --git a/arch/powerpc/platforms/40x/Kconfig b/arch/powerpc/platforms/40x/Kconfig index 8f6699f..03051bc 100644 --- a/arch/powerpc/platforms/40x/Kconfig +++ b/arch/powerpc/platforms/40x/Kconfig @@ -110,6 +110,7 @@ config 405GPR config XILINX_VIRTEX bool + select XILINX_DRIVERS config XILINX_VIRTEX_II_PRO bool diff --git a/arch/ppc/platforms/4xx/Kconfig b/arch/ppc/platforms/4xx/Kconfig index 76551b6..d7db7e4 100644 --- a/arch/ppc/platforms/4xx/Kconfig +++ b/arch/ppc/platforms/4xx/Kconfig @@ -228,6 +228,7 @@ config XILINX_VIRTEX_4_FX config XILINX_VIRTEX bool + select XILINX_DRIVERS config STB03xxx bool diff --git a/drivers/block/Kconfig b/drivers/block/Kconfig index 4d0119e..0166560 100644 --- a/drivers/block/Kconfig +++ b/drivers/block/Kconfig @@ -412,7 +412,7 @@ source drivers/s390/block/Kconfig config XILINX_SYSACE tristate Xilinx SystemACE support - depends on 4xx + depends on 4xx || XILINX_DRIVERS help Include support for the Xilinx SystemACE CompactFlash interface diff --git a/drivers/char/Kconfig b/drivers/char/Kconfig index 157ae2a..8230ad1 100644 --- a/drivers/char/Kconfig +++ b/drivers/char/Kconfig @@ -833,7 +833,7 @@ config DTLK config XILINX_HWICAP tristate Xilinx HWICAP Support - depends on XILINX_VIRTEX + depends on XILINX_DRIVERS help This option enables support for Xilinx Internal Configuration Access Port (ICAP) driver. diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig index b5e67c0..e7b0bed 100644 --- a/drivers/misc/Kconfig +++ b/drivers/misc/Kconfig @@ -233,3 +233,13 @@ config ATMEL_SSC If unsure, say N. endif # MISC_DEVICES +endmenu + + +# +# Xilinx devices and common device driver infrastructure +# + +config XILINX_DRIVERS + bool + diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig index d7e1996..f922ec6 100644 --- a/drivers/serial/Kconfig +++ b/drivers/serial/Kconfig @@ -757,7 +757,7 @@ config SERIAL_IMX_CONSOLE config SERIAL_UARTLITE tristate Xilinx uartlite serial port support - depends on PPC32 + depends on PPC32 || XILINX_DRIVERS select SERIAL_CORE help Say Y here if you want to use the Xilinx uartlite serial controller. diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig index abf0504..c66838f 100644 --- a/drivers/spi/Kconfig +++ b/drivers/spi/Kconfig @@ -183,7 +183,7 @@ config SPI_TXX9 config SPI_XILINX tristate Xilinx SPI controller - depends on SPI_MASTER XILINX_VIRTEX EXPERIMENTAL + depends on SPI_MASTER XILINX_DRIVERS EXPERIMENTAL select SPI_BITBANG help This exposes the SPI controller IP from the Xilinx EDK. diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig index 5b3dbcf..a66ff4b 100644 --- a/drivers/video/Kconfig +++ b/drivers/video/Kconfig @@ -1871,7 +1871,7 @@ config FB_PS3_DEFAULT_SIZE_M config FB_XILINX tristate Xilinx frame buffer support - depends on FB XILINX_VIRTEX + depends on FB XILINX_DRIVERS select FB_CFB_FILLRECT select FB_CFB_COPYAREA select FB_CFB_IMAGEBLIT -- 1.5.3.4-dirty ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 00/18] ide: warm-plug support for IDE devices and other goodies
On Tue, 2008-02-12 at 21:41 +, Alan Cox wrote: On Wed, 13 Feb 2008 08:04:07 +1100 Benjamin Herrenschmidt [EMAIL PROTECTED] wrote: On Fri, 2008-02-08 at 19:40 +1100, Benjamin Herrenschmidt wrote: On Fri, 2008-02-08 at 01:44 +0100, Bartlomiej Zolnierkiewicz wrote: - couple of fixes and preparatory patches - rework of PowerMac media-bay support ([un]register IDE devices instead of [un]registering IDE interface) [ it is the main reason for spamming PPC ML ] Interesting... I was thinking about doing a full remove of the device at a higher level instead but I suppose what you propose is easier. I'll have a look test next week hopefully. Also, the above would have the advantage of not relying on drivers/ide infrastructure, and thus working with libata (once somebody has ported pmac ide to libata). Unfortunately you need a degree in dentistry to open a Macintosh up and fix it otherwise we would have support by now. Heh :-) Recent powermacs are trivial to open ! But yeah, I do need to produce a driver for libata one of these days. On my todo list ... Cheers, Ben. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 00/18] ide: warm-plug support for IDE devices and other goodies
On Fri, 2008-02-08 at 19:40 +1100, Benjamin Herrenschmidt wrote: On Fri, 2008-02-08 at 01:44 +0100, Bartlomiej Zolnierkiewicz wrote: - couple of fixes and preparatory patches - rework of PowerMac media-bay support ([un]register IDE devices instead of [un]registering IDE interface) [ it is the main reason for spamming PPC ML ] Interesting... I was thinking about doing a full remove of the device at a higher level instead but I suppose what you propose is easier. I'll have a look test next week hopefully. Also, the above would have the advantage of not relying on drivers/ide infrastructure, and thus working with libata (once somebody has ported pmac ide to libata). Ben. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: DTS question - MPC5200b
On Feb 12, 2008 11:07 AM, Jarno Manninen [EMAIL PROTECTED] wrote: On Tuesday 12 February 2008 19:37:07 Nick wrote: How do I specify the timer based on the cell-index? I don't know if that is possible to do in a one call, but maybe using the approach from mpc52xx_uart might help? --clip-- for_each_node_by_type(np, serial) { if (!of_match_node(mpc52xx_uart_of_match, np)) continue; /* Is a particular device number requested? */ devno = of_get_property(np, port-number, NULL); mpc52xx_uart_of_assign(of_node_get(np), devno ? *devno : -1); } --clip-- This code has actually changed in 2.6.25-rc1. It is now for_each_matching_node() and the call to of_match_node is no longer necessary. And change serial-gpt, port-number to cell-index and add some logic to select the devices you want. use 'reg' instead. cell-index (and port-number for that matter) will probably be going away in the near future. Or if you wan't to do it a bit differently you could add a pseudo device outside the main tree like mydev { gpt-dev = the_gpt_dev: }; And get it that way. However I don't know if this is recommended approach, but I've used it for some simple stuff like binding gpt in PWM mode to framebuffer backlight, along with power-pin. Yes, this is a good approach too. 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?
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: [PATCH 3/8] pseries: phyp dump: use sysfs to release reserved mem
As noted, its fixed in patch 4. If its okay for this time, I will prefer to leave it there. -Manish Stephen Rothwell wrote: Hi Manish, Just a small comment. On Tue, 12 Feb 2008 01:11:58 -0600 Manish Ahuja [EMAIL PROTECTED] wrote: +/* Is there dump data waiting for us? */ +rtas = of_find_node_by_path(/rtas); +dump_header = of_get_property(rtas, ibm,kernel-dump, header_len); You need an of_node_put(rtas) here. +if (dump_header == NULL) { +release_all(); +return 0; +} ___ 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: DTS question - MPC5200b
On Feb 12, 2008 10:37 AM, Nick [EMAIL PROTECTED] wrote: Hi, I need some help. I am trying to access timer 7 on the MPC5200B processor. I have the DTS file setup like this [EMAIL PROTECTED] {// General Purpose Timer device_type = gpt; compatible = fsl,mpc5200b-gpt,fsl,mpc5200-gpt; cell-index = 7; reg = 670 10; interrupts = 1 10 0; interrupt-parent = mpc5200_pic; }; I have timers 0 to 6 defined the same way except the cell-index reflects the timer number. In my platform file where I am doing my board setup, I tried the following. timer7 = mpc52xx_find_and_map (mpc5200b-gpt); Don't use find_and_map; it was a stupid API that I never should have written. Instead, use for_each_compatible_node() or for_each_matching_node() to iterate over them until you find the one with the correct reg address. Then you can use of_iomap() to map the device registers. How do I specify the timer based on the cell-index? Match on the reg property instead of cell-index; I'll probably be dropping the cell-index property from future 5200 device trees because it just ends up duplicating information already provided by reg. 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
[PATCH] FIx compile of swim3 as module
The current pmac32_defconfig fails to build with the following error: Building modules, stage 2. ERROR: check_media_bay [drivers/block/swim3.ko] undefined! WARNING: modpost: Found 23 section mismatch(es). To see full details build your kernel with: 'make CONFIG_DEBUG_SECTION_MISMATCH=y' make[2]: *** [__modpost] Error 1 This patch fixes that. Signed-off-by: Tony Breeds [EMAIL PROTECTED] --- drivers/block/swim3.c|4 drivers/macintosh/mediabay.c |2 -- 2 files changed, 0 insertions(+), 6 deletions(-) diff --git a/drivers/block/swim3.c b/drivers/block/swim3.c index b4e462f..730ccea 100644 --- a/drivers/block/swim3.c +++ b/drivers/block/swim3.c @@ -251,10 +251,6 @@ static int floppy_release(struct inode *inode, struct file *filp); static int floppy_check_change(struct gendisk *disk); static int floppy_revalidate(struct gendisk *disk); -#ifndef CONFIG_PMAC_MEDIABAY -#define check_media_bay(which, what) 1 -#endif - static void swim3_select(struct floppy_state *fs, int sel) { struct swim3 __iomem *sw = fs-swim3; diff --git a/drivers/macintosh/mediabay.c b/drivers/macintosh/mediabay.c index 9367882..51a1128 100644 --- a/drivers/macintosh/mediabay.c +++ b/drivers/macintosh/mediabay.c @@ -416,7 +416,6 @@ static void poll_media_bay(struct media_bay_info* bay) } } -#ifdef CONFIG_MAC_FLOPPY int check_media_bay(struct device_node *which_bay, int what) { int i; @@ -431,7 +430,6 @@ int check_media_bay(struct device_node *which_bay, int what) return -ENODEV; } EXPORT_SYMBOL(check_media_bay); -#endif /* CONFIG_MAC_FLOPPY */ #ifdef CONFIG_BLK_DEV_IDE_PMAC int check_media_bay_by_base(unsigned long base, int what) Yours Tony linux.conf.auhttp://linux.conf.au/ || http://lca2008.linux.org.au/ Jan 28 - Feb 02 2008 The Australian Linux Technical Conference! ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] drivers/base: export gpl (un)register_memory_notifier
On Mon, 2008-02-11 at 17:24 +0100, Jan-Bernd Themann wrote: Drivers like eHEA need memory notifiers in order to update their internal DMA memory map when memory is added to or removed from the system. Patch for eHEA memory hotplug support that uses these functions: http://www.spinics.net/lists/netdev/msg54484.html This driver is broken pretty horribly. It won't even compile for a plain ppc64 kernel: http://sr71.net/~dave/linux/ehea-is-broken.config I know it's used for very specific hardware, but this is the symptom of it not using the proper abstracted interfaces to the VM. In file included from /home/dave/work/linux/2.6/23/linux-2.6.git/drivers/net/ehea/ehea_main.c:42: /home/dave/work/linux/2.6/23/linux-2.6.git/drivers/net/ehea/ehea_qmr.h:44:14: warning: SECTION_SIZE_BITS is not defined /home/dave/work/linux/2.6/23/linux-2.6.git/drivers/net/ehea/ehea_qmr.h:45:2: error: #error eHEA module cannot work if kernel sectionsize ehea sectionsize CC drivers/net/mii.o make[4]: *** [drivers/net/ehea/ehea_main.o] Error 1 make[4]: *** Waiting for unfinished jobs CC drivers/net/ixgb/ixgb_param.o In file included from /home/dave/work/linux/2.6/23/linux-2.6.git/drivers/net/ehea/ehea_qmr.c:32: /home/dave/work/linux/2.6/23/linux-2.6.git/drivers/net/ehea/ehea_qmr.h:44:14: warning: SECTION_SIZE_BITS is not defined /home/dave/work/linux/2.6/23/linux-2.6.git/drivers/net/ehea/ehea_qmr.h:45:2: error: #error eHEA module cannot work if kernel sectionsize ehea sectionsize /home/dave/work/linux/2.6/23/linux-2.6.git/drivers/net/ehea/ehea_qmr.c: In function 'ehea_create_busmap': /home/dave/work/linux/2.6/23/linux-2.6.git/drivers/net/ehea/ehea_qmr.c:574: error: 'NR_MEM_SECTIONS' undeclared (first use in this function) /home/dave/work/linux/2.6/23/linux-2.6.git/drivers/net/ehea/ehea_qmr.c:574: error: (Each undeclared identifier is reported only once /home/dave/work/linux/2.6/23/linux-2.6.git/drivers/net/ehea/ehea_qmr.c:574: error: for each function it appears in.) /home/dave/work/linux/2.6/23/linux-2.6.git/drivers/net/ehea/ehea_qmr.c:575: error: implicit declaration of function 'valid_section_nr' /home/dave/work/linux/2.6/23/linux-2.6.git/drivers/net/ehea/ehea_qmr.c: In function 'ehea_map_vaddr': /home/dave/work/linux/2.6/23/linux-2.6.git/drivers/net/ehea/ehea_qmr.c:606: error: 'SECTION_SIZE_BITS' undeclared (first use in this function) /home/dave/work/linux/2.6/23/linux-2.6.git/drivers/net/ehea/ehea_qmr.c: In function 'ehea_reg_kernel_mr': /home/dave/work/linux/2.6/23/linux-2.6.git/drivers/net/ehea/ehea_qmr.c:655: error: 'SECTION_SIZE_BITS' undeclared (first use in this function) -- Dave ___ 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
DTS question - MPC5200b
Hi, I need some help. I am trying to access timer 7 on the MPC5200B processor. I have the DTS file setup like this [EMAIL PROTECTED] {// General Purpose Timer device_type = gpt; compatible = fsl,mpc5200b-gpt,fsl,mpc5200-gpt; cell-index = 7; reg = 670 10; interrupts = 1 10 0; interrupt-parent = mpc5200_pic; }; I have timers 0 to 6 defined the same way except the cell-index reflects the timer number. In my platform file where I am doing my board setup, I tried the following. timer7 = mpc52xx_find_and_map (mpc5200b-gpt); How do I specify the timer based on the cell-index? Thanks Nick ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 00/18] ide: warm-plug support for IDE devices and other goodies
On Tue, 2008-02-12 at 12:49 +0100, Gabriel Paubert wrote: On Fri, Feb 08, 2008 at 07:40:43PM +1100, Benjamin Herrenschmidt wrote: On Fri, 2008-02-08 at 01:44 +0100, Bartlomiej Zolnierkiewicz wrote: - couple of fixes and preparatory patches - rework of PowerMac media-bay support ([un]register IDE devices instead of [un]registering IDE interface) [ it is the main reason for spamming PPC ML ] Interesting... I was thinking about doing a full remove of the device at a higher level instead but I suppose what you propose is easier. Well, I have serious problem on a Pegasos which appeared some time between 2.6.24 and 2.6.25-rc1: at boot I get an infinite string of hdb: empty DMA table? I'm trying to bisect it right now. Pegasos doesn't use the pmac ide which is what we were discussing. It uses a VIA IDE which uses a normal PRD table. So something else must have broken... Ben. ___ 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: [PATCH] FIx compile of swim3 as module
On Tue, 2008-02-12 at 20:48 -0600, Josh Boyer wrote: The current pmac32_defconfig fails to build with the following error: Building modules, stage 2. ERROR: check_media_bay [drivers/block/swim3.ko] undefined! WARNING: modpost: Found 23 section mismatch(es). To see full details build your kernel with: 'make CONFIG_DEBUG_SECTION_MISMATCH=y' make[2]: *** [__modpost] Error 1 This patch fixes that. Kyle posted a slightly different patch that seemed to still keep the original intention of ifdefery in tact. I've no idea which is better, but the three of you might want to compare notes. Just remove the bloody ifdef's, they are just a useless pain. Ben. ___ 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: libfdt: Add and use a node iteration helper function.
So, like, the other day David Gibson mumbled: This patch adds an fdt_next_node() function which can be used to iterate through nodes of the tree while keeping track of depth. This function is used to simplify the iteration code in a lot of other functions, and is also exported for use by library users. Signed-off-by: David Gibson [EMAIL PROTECTED] --- I think we're ready to go with this one. I'm still thinking about suitable for_each_* macros, but in the mean time I'm happy with the exported interface here, and it's a code-reducing patch. Applied. Thanks, jdl ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 00/18] ide: warm-plug support for IDE devices and other goodies
On Fri, Feb 08, 2008 at 07:40:43PM +1100, Benjamin Herrenschmidt wrote: On Fri, 2008-02-08 at 01:44 +0100, Bartlomiej Zolnierkiewicz wrote: - couple of fixes and preparatory patches - rework of PowerMac media-bay support ([un]register IDE devices instead of [un]registering IDE interface) [ it is the main reason for spamming PPC ML ] Interesting... I was thinking about doing a full remove of the device at a higher level instead but I suppose what you propose is easier. Well, I have serious problem on a Pegasos which appeared some time between 2.6.24 and 2.6.25-rc1: at boot I get an infinite string of hdb: empty DMA table? I'm trying to bisect it right now. Gabriel I'll have a look test next week hopefully. Ben. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 00/18] ide: warm-plug support for IDE devices and other goodies
On Tue, Feb 12, 2008 at 12:49:05PM +0100, Gabriel Paubert wrote: On Fri, Feb 08, 2008 at 07:40:43PM +1100, Benjamin Herrenschmidt wrote: On Fri, 2008-02-08 at 01:44 +0100, Bartlomiej Zolnierkiewicz wrote: - couple of fixes and preparatory patches - rework of PowerMac media-bay support ([un]register IDE devices instead of [un]registering IDE interface) [ it is the main reason for spamming PPC ML ] Interesting... I was thinking about doing a full remove of the device at a higher level instead but I suppose what you propose is easier. Well, I have serious problem on a Pegasos which appeared some time between 2.6.24 and 2.6.25-rc1: at boot I get an infinite string of hdb: empty DMA table? I'm trying to bisect it right now. Argh, the first bisect point ended up with timeouts on hdb... Flagged as bad, to try to see when the problems started, but I suspect that there are several. Gabriel ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 4/8] pseries: phyp dump: register dump area.
For now, if we can leave this patch as is, that will be great. That move requires me to work all remaining patches as they apply uncleanly after that. I will bunch those two together functionally next time onwards. Thanks, Manish Stephen Rothwell wrote: Hi Manish, -/* Is there dump data waiting for us? */ +/* Is there dump data waiting for us? If there isn't, + * then register a new dump area, and release all of + * the rest of the reserved ram. + * + * The /rtas/ibm,kernel-dump rtas node is present only + * if there is dump data waiting for us. + */ rtas = of_find_node_by_path(/rtas); dump_header = of_get_property(rtas, ibm,kernel-dump, header_len); +of_node_put(rtas); Oh, here is the of_node_put() - you should move that to patch 3. ___ 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: [PATCH 09/18] ide: rework PowerMac media-bay support
On Fri, 2008-02-08 at 01:45 +0100, Bartlomiej Zolnierkiewicz wrote: Rework PowerMac media-bay support in such way that instead of un/registering the IDE interface we un/register IDE devices: * Add ide_port_scan() helper for probing+registerering devices on a port. * Rename ide_port_unregister_devices() to __ide_port_unregister_devices(). * Add ide_port_unregister_devices() helper for unregistering devices on a port. * Add 'ide_hwif_t *cd_port' to 'struct media_bay_info', pass 'hwif' instead of hwif-index to media_bay_set_ide_infos() and use it to setup 'cd_port'. * Use ide_port_unregister_devices() instead of ide_unregister() and ide_port_scan() instead of ide_register_hw() in media_bay_step(). * Unexport ide_register_hw() and make it static. Signed-off-by: Bartlomiej Zolnierkiewicz [EMAIL PROTECTED] --- drivers/ide/ide-probe.c| 18 ++ drivers/ide/ide.c | 22 -- drivers/ide/ppc/pmac.c |3 ++- drivers/macintosh/mediabay.c | 17 +++-- include/asm-powerpc/mediabay.h |4 +++- include/linux/ide.h|6 ++ 6 files changed, 48 insertions(+), 22 deletions(-) Index: b/include/asm-powerpc/mediabay.h === --- a/include/asm-powerpc/mediabay.h +++ b/include/asm-powerpc/mediabay.h @@ -22,10 +22,12 @@ int check_media_bay(struct device_node * /* Number of bays in the machine or 0 */ extern int media_bay_count; +#ifdef CONFIG_BLK_DEV_IDE_PMAC int check_media_bay_by_base(unsigned long base, int what); /* called by IDE PMAC host driver to register IDE controller for media bay */ int media_bay_set_ide_infos(struct device_node *which_bay, unsigned long base, - int irq, int index); + int irq, ide_hwif_t *hwif); +#endif #endif /* __KERNEL__ */ #endif /* _PPC_MEDIABAY_H */ This hunk breaks pmac32_defconfig for me. include2/asm/mediabay.h:29: error: syntax error before 'ide_hwif_t' make[3]: *** [arch/powerpc/platforms/powermac/setup.o] Error 1 make[2]: *** [arch/powerpc/platforms/powermac] Error 2 make[1]: *** [arch/powerpc/platforms] Error 2 make: *** [sub-make] Error 2 cheers -- Michael Ellerman OzLabs, IBM Australia Development Lab wwweb: http://michael.ellerman.id.au phone: +61 2 6212 1183 (tie line 70 21183) We do not inherit the earth from our ancestors, we borrow it from our children. - S.M.A.R.T Person signature.asc Description: This is a digitally signed message part ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev