On Wed, 2008-01-30 at 23:36 +0100, Éric Piel wrote: > Hi, > Just a few comments for now as I don't have much time until this weekend. > 30/01/08 18:27, Thomas Renninger wrote/a écrit: > : > > On Fri, 2008-01-25 at 23:39 -0500, Len Brown wrote: > : > >> Probably it is best to forward the original patch > >> w/ proper credits, and then send updates to address 1-4. > Thomas, the patch you have included is only the "original" patch, right? > We need to address the points 1-4 (which seem very reasonable) later on > with additional patches. I think most of them are already implemented in > http://gaugusch.at/acpi-dsdt-initrd-patches/acpi-dsdt-initrd-v0.8.2-2.6.17-ssdt.patch > So that should be quite easy :-) > > > I do not know who initially came up with this, I just modified/adjusted > > the patch sometimes. I expect it's Markus, possibly Eric or best both > > who should be mentioned as author. > Yep, to be precise, Markus Gaugusch is the original author, I'm just the > current maintainer, and Thomas has been very helpful for a looong time :-) Long time is true..., very helpful, don't know, I didn't do that much... > > Jeff (Mahony) has adjusted the last bits of it when he updated it to the > > latest rcX-gitY version: > > Changes: > > We no longer call populate_rootfs() twice. If we want the custom DSDT, > > we load the rootfs before ACPI. Otherwise, it is loaded at the > > appropriate > > initcall time. -jeffm > I had left it this way because I though Linus had a good reason for > delaying populate_rootfs(). But if it works, fine! Maybe some additional > comments should be added in before the #ifdef's to clearly show that > populate_rootfs() is _always_ called, and they modify only the moment it > is called. I don't know about this, I took that change from your patches... > > > There is a new version at > > http://gaugusch.at/acpi-dsdt-initrd-patches/acpi-dsdt-initrd-v0.8.4-2.6.21.patch, > > but there doesn't seem to be any real changes other than elminating the > > file > > name array. > Yes, exactly. > > > In fact, I added the array some time ago. > > Not sure whether the latest Version of Eric/Markus can also load several > > DSDT/SSDTs? Maybe you found a more elegant way? > Nope. On the website (http://gaugusch.at/kernel.shtml) you can just find > two versions of the patch: one simple and one with your addition for > SSDTs support. At least with SSDTs support there is even an advantage > over the DSDT-in-kernel version :-) > > Just one comment about the patch for now: it misses > Documentation/dsdt-initrd.txt . Please include this file too so that the > documentation is provided at the same time (it's in the official patch). > > I'll test the patch and try to be productive this weekend. There is no real need for testing the patch. It is in OpenSUSE for quite some time. While I don't know how much it got used, it shouldn't break anything if unused. As Jeff stated (I haven't had a look at your current version) it should be nearly the same. About Len's points 1-4: 1. tainted bit needs to be set when a DSDT override is used. (this is common both to the new patch and the current override method) 2. upon setting the tainted bit, print a BIG FAT WARNING that this is not a supported configuration. 3. cmdline option needs to be available to disable an override. Otherwise a bad image in the intrd may be a serious pickle for some users. 4. get rid of alarming failure message: + printk(KERN_INFO "Looking for DSDT in initrd ..."); ... + printk(" not found!\n"); Most of this should be included in the one I sent: 1+2 should be covered by this: + printk(PREFIX "Override [%4.4s-%8.8s]" + " from initramfs -" + " tainting kernel\n", + t->signature, + t->oem_table_id); + add_taint(TAINT_NO_SUPPORT); I remember that I introduced a new TAINT flag, this got probably ripped out because I expect it needed conflict solving with new kernels all the time. Now when it goes mainline a TAINT_DSDT_OVERRIDE flag might be appropriate, so that if you see a kernel panic or whatever bug, you immediately see why the kernel is tainted. Problem from 4. should also not exist in this patch anymore. The boot parameter (3.) is still missing. Eric, would you mind bringing this all together (if Len agrees with my aspects). I'd vote for moving the current location of the DSDT file to: /dsdt/DSDT.aml This would break current userspace apps (including SuSE's mkinitrd), but would prepare the patch for further SSDT enhancements. Thomas - To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
