Bug 15605 - \_SB_ is not the only root-level namespace device - introduced by commit 78b8e141
Summary: \_SB_ is not the only root-level namespace device - introduced by commit 78b8...
Status: CLOSED PATCH_ALREADY_AVAILABLE
Alias: None
Product: ACPI
Classification: Unclassified
Component: BIOS (show other bugs)
Hardware: All Linux
: P1 high
Assignee: Bjorn Helgaas
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-03-22 05:54 UTC by Zhang Rui
Modified: 2010-04-16 19:58 UTC (History)
3 users (show)

See Also:
Kernel Version: 2.6.32-rc1 and later
Subsystem:
Regression: Yes
Bisected commit-id:


Attachments
DSDT that has device AMW0 as a root-level namespace device (306.65 KB, text/plain)
2010-03-22 05:54 UTC, Zhang Rui
Details
patch for testing (use _HID when present) (2.11 KB, patch)
2010-03-22 20:25 UTC, Bjorn Helgaas
Details | Diff
patch: set ACPI_BUS_HID to \_SB only (1.20 KB, patch)
2010-03-23 07:02 UTC, Zhang Rui
Details | Diff
patch from comment #2 as applied to 2.6.34-rc3 (2.33 KB, patch)
2010-04-04 03:34 UTC, Len Brown
Details | Diff

Description Zhang Rui 2010-03-22 05:54:58 UTC
Created attachment 25636 [details]
DSDT that has device AMW0  as a root-level namespace device

In commit 78b8e141, we assume all the root-level namespace devices are \_SB devices. But this is not true.

Here is part of the output of "grep . /sys/bus/acpi/devices/*/path" in 2.6.33
/sys/bus/acpi/devices/LNXSYSTM:00/LNXSYBUS:00/path:\_SB_
/sys/bus/acpi/devices/LNXSYSTM:00/LNXSYBUS:01/path:\AMW0

and here is the output of "grep . /sys/bus/acpi/drivers/wmi/*/path" in 2.6.31
/sys/bus/acpi/drivers/wmi/PNP0C14:00/path:\AMW0

device \AMW0 is renamed and the ACPI wmi driver is not bind to it any more.
Comment 1 Bjorn Helgaas 2010-03-22 20:25:25 UTC
Created attachment 25644 [details]
patch for testing (use _HID when present)

Thanks for debugging and reporting this!

The assumption that all root-level devices are \_SB devices was there long before 78b8e141, but the implementation was broken (it compared an acpi_device pointer with a handle, which was never true), so we never saw any LNXSYBUS devices at all.

After I fixed the implementation in 78b8e141, we gave LNXSYBUS HIDs to *all* root-level devices, which isn't quite right either.

Can you test this patch, please?
Comment 2 Yong Wang 2010-03-23 01:57:49 UTC
I just tested this patch and it works for me. Thanks for the patch, Helgaas!

Rui, could you please asn lenb to queue up this patch?
Comment 3 Zhang Rui 2010-03-23 02:26:15 UTC
with this patch applied, LNXSYBUS is assigned to \AMW0 as a cid.
I don't think this is what we want.

perhaps something like this?
-                       acpi_add_id(device, ACPI_BUS_HID);
-                       strcpy(device->pnp.device_name, ACPI_BUS_DEVICE_NAME);
-                       strcpy(device->pnp.device_class, ACPI_BUS_CLASS);
-                       break;
+                       if (strncmp(device->pnp.bus_id, "SB", 3) &&
+                           strncmp(device->pnp.bus_id, "SB_", 4) &&
+                           strncmp(device->pnp.bus_id, "_SB", 4) &&
+                           strncmp(device->pnp.bus_id, "_SB_", 4)) {
+                               acpi_add_id(device, ACPI_BUS_HID);
+                               strcpy(device->pnp.device_name, ACPI_BUS_DEVICE_NAME);
+                               strcpy(device->pnp.device_class, ACPI_BUS_CLASS);
+                               break;
+                       }
Comment 4 Zhang Rui 2010-03-23 07:02:06 UTC
Created attachment 25650 [details]
patch: set ACPI_BUS_HID to \_SB only

Hi, Bjorn, how about this one?

Yong,
will you please apply this patch and see if it helps?
Comment 5 Yong Wang 2010-03-23 07:23:01 UTC
Rui, I just tried your patch and it works too.
Comment 6 Bjorn Helgaas 2010-03-23 15:08:30 UTC
Did you verify that LNXSYBUS is assigned to \AMW0 as a CID?

Just from reading the code in attachment 25644 [details], I don't see how that would happen.  \AMW0 has a valid _HID, so we'll assign PNP0C14 first, and then acpi_device_hid() will return a valid pointer, so we shouldn't add ACPI_BUS_HID.
Comment 7 Zhang Rui 2010-03-24 06:02:26 UTC
(In reply to comment #6)
> Did you verify that LNXSYBUS is assigned to \AMW0 as a CID?
> 
> Just from reading the code in attachment 25644 [details], I don't see how
> that would
> happen.  \AMW0 has a valid _HID, so we'll assign PNP0C14 first, and then
> acpi_device_hid() will return a valid pointer, so we shouldn't add
> ACPI_BUS_HID.

that's right. sorry for the mistake.

Acked-by: Zhang Rui <rui.zhang@intel.com>
Comment 8 Len Brown 2010-04-04 03:34:38 UTC
Created attachment 25843 [details]
patch from comment #2 as applied to 2.6.34-rc3
Comment 9 Len Brown 2010-04-16 19:58:43 UTC
commit b7b30de53aef6ce773d34837ba7d8422bd3baeec
Author: Bjorn Helgaas <bjorn.helgaas@hp.com>
Date:   Wed Mar 24 10:44:33 2010 -0600

    ACPI: use _HID when supplied by root-level devices


shipped in linux-2.6.34-rc4
closed

Note You need to log in before you can comment on or make changes to this bug.