On Wed, 2008-06-25 at 09:08 -0600, Bjorn Helgaas wrote:> On Tuesday 24 June 2008 07:37:37 pm Zhao Yakui wrote:> > On Tue, 2008-06-24 at 13:52 +0200, Vegard Nossum wrote:> > > On 6/24/08, Ingo Molnar <mingo@elte.hu> wrote:> > > > i havent seen the warning reappear with your fix after thousands of> > > > bootups - so i guess we can consider it fixed.> > > >> > > > Len, please consider the patch below. (it's in tip/out-of-tree)> > > > > > No, please don't :-)> > > > > > It fixes your particular case (the acpi_rtc_init() hunk of the patch),> > > but the acpi_walk_namespace() part should be changed to a WARN(). But> > > that is likely to cause a lot of "spurious" reports, so the other acpi> > > drivers should be fixed as well.> > In fact this issue is related with the following factors:> > a. when acpi is disabled, OS won't initialize the ACPI mutex, which> > is accessed by many ACPI interface functions. For example:> > acpi_walk_namespace, acpi_install_fixed_event_handler.> > b. When acpi is disabled, some drivers will call the ACPI interface> > functions. For example: The acpi_walk_namespace is called in> > dock_init/bay_init.> > I think most current uses of acpi_walk_namespace() are indications> that the ACPI or PNP core is missing something.I don't think so. The acpi_walk_namespace is used to enumerate the ACPItree and execute some specific operations. For example: Add the devicenotification function for some type of device; call the INI method forall the device.> In dock_init() and bay_init(), it's used to bind a driver to a> device. I think it would be better if we could figure out how to> use the usual acpi_bus_register_driver() interface. Actually, it> looks like this is already 90% done: acpi_dock_match() does the> same thing as is_dock(), so it looks like dock_init() could easily> be converted to register as a driver for ACPI_DOCK_HID.Maybe what you said is reasonable if the dock/bay device exists and isadded to Linux ACPI device tree. But if the status of bay/dock devicedoesn't exist , it won't be added into the Linux ACPI device tree. Insuch case the dock/bay driver won't be loaded for it. So it will bereasonable to enumerate the acpi tree to install the notificationfunction for the dock device so that OS can receive the notificationevent when the dock device is hotplugged.

If acpi is disabled, it is unnecessary for OS to find the dock/baydevice again. In such case it will be reasonable to use the flag ofacpi_disabled in the function of dock_init/bay_init.

Best regards. Yakui.> bay_init() looks similar, with acpi_bay_match(), is_ejectable_bay(),> ACPI_BAY_HID, etc.> > Other users of acpi_walk_namespace() are often to install notify> handlers to deal with add/remove events. I think these are telling> us that we need to implement the "TBD: Handle device insertion/removal"> pieces in acpi_bus_check_device().> > > The acpi_install_fixed_event_handler is called in > > the acpi_rtc_init.> > Yes (via rtc_wake_setup()). I think this should be moved into the> RTC driver itself. I have some ideas on how to do this; I'll post> a patch in a few days. But for 2.6.26, I think the minimal fix of> checking acpi_disabled in acpi_rtc_init() is better.> > Bjorn> >