On Wednesday 23 September 2009 11:22:28 pm ykzhao wrote:
> On Thu, 2009-09-24 at 11:36 +0800, Bjorn Helgaas wrote:
> > On Thu, 2009-09-24 at 09:44 +0800, ykzhao wrote:
> > > On Thu, 2009-09-24 at 00:19 +0800, Bjorn Helgaas wrote:
> > > > On Tuesday 22 September 2009 08:21:46 pm ykzhao wrote:
> > > > > On Tue, 2009-09-22 at 03:35 +0800, Bjorn Helgaas wrote:
> > > > > > This makes \_SB_ show up as /sys/devices/LNXSYSTM:00/LNXSYBUS:00
> > > > > > rather than "device:00". This has been broken for a loooong time
> > > > > > (at least since 2.6.13) because device->parent is an acpi_device
> > > > > > pointer, not a handle.
> > > > >
> > > > > > Signed-off-by: Bjorn Helgaas <bjorn.helgaas@xxxxxx>
> > > > > > ---
> > > > > > drivers/acpi/scan.c | 18 ++++++------------
> > > > > > 1 files changed, 6 insertions(+), 12 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> > > > > > index 2c4cac5..e9227ea 100644
> > > > > > --- a/drivers/acpi/scan.c
> > > > > > +++ b/drivers/acpi/scan.c
> > > > > > @@ -1100,6 +1100,12 @@ static void acpi_device_set_id(struct acpi_device *device)
> > > > > > if (ACPI_IS_ROOT_DEVICE(device)) {
> > > > > > hid = ACPI_SYSTEM_HID;
> > > > > > break;
> > > > > > + } else if (ACPI_IS_ROOT_DEVICE(device->parent)) {
> > > >
> > > > > Can we still add the judgement about the device type?
> > > > > device->type == ACPI_BUS_TYPE_DEVICE
> > > >
> > > > We are already checking this because this test is inside the
> > > > ACPI_BUS_TYPE_DEVICE case of a switch statement.
> > > If this is checed inside the ACPI_BUS_TYPE_DEVICE case, can we delete
> > > the following check as the type of root device is ACPI_BUS_TYPE_SYSTEM?
> > > If (ACPI_IS_ROOT_DEVICE(device)) {
> > > hid = ACPI_SYSTEM_HID;
> > > break;
> > > }
> >
> > What? Are you saying that we shouldn't add a HID of "LNXSYSTM" to the
> > root object? I'm really not sure what you're proposing.
> Maybe I mix the two patch sets. One has 8 patches. And another has 17
> patches. In fact there exists the dependency between the two patch set.
Yes, the second series ("make every acpi_device have a HID") does
depend on the first series ("cleanups for hotplug"). I forgot to
mention that in the initial posting, sorry.
> And you delete the ACPI_BUS_TYPE_SYSTEM type in another patch set,
> right?
Yes, I removed ACPI_BUS_TYPE_SYSTEM in patch [13/17] of the "ACPI:
cleanups for hotplug" series.
> What is the benefit if we change the type from ACPI_BUS_TYPE_SYSTEM to
> ACPI_BUS_TYPE_DEVICE for the root device?
The benefit is removing special cases from the code. If we don't
need to keep track of the difference between ACPI_BUS_TYPE_SYSTEM
and ACPI_BUS_TYPE_DEVICE, we shouldn't do it.
Bjorn
> Thanks.
> >
> > > >
> > > > > Where can I find the macro definition of ACPI_IS_ROOT_DEVICE?
> > > >
> > > > It's patch 13/17 in the previous "ACPI: cleanups for hotplug" series.
> > > > I should have mentioned that this series depends on that one.
> > > Yes.
> > > I also find that it is defined in patch 13/17.
> > > It had better be defined as early as possible. Otherwise we may fail in
> > > git-bisect.
> >
> > Absolutely. I'm pretty sure I defined ACPI_IS_ROOT_DEVICE before any
> > uses of it, but please point it out if I'm mistaken.
> >
> > Bjorn
> >
> > > > > > + /* \_SB_, the only root-level namespace device */
> > > > > > + hid = ACPI_BUS_HID;
> > > > > > + strcpy(device->pnp.device_name, ACPI_BUS_DEVICE_NAME);
> > > > > > + strcpy(device->pnp.device_class, ACPI_BUS_CLASS);
> > > > > > + break;
> > > > > > }
> > > > > >
> > > > > > status = acpi_get_object_info(device->handle, &info);
> > > > > > @@ -1149,18 +1155,6 @@ static void acpi_device_set_id(struct acpi_device *device)
> > > > > > break;
> > > > > > }
> > > > > >
> > > > > > - /*
> > > > > > - * \_SB
> > > > > > - * ----
> > > > > > - * Fix for the system root bus device -- the only root-level device.
> > > > > > - */
> > > > > > - if (((acpi_handle)device->parent == ACPI_ROOT_OBJECT) &&
> > > > > > - (device->device_type == ACPI_BUS_TYPE_DEVICE)) {
> > > > > > - hid = ACPI_BUS_HID;
> > > > > > - strcpy(device->pnp.device_name, ACPI_BUS_DEVICE_NAME);
> > > > > > - strcpy(device->pnp.device_class, ACPI_BUS_CLASS);
> > > > > > - }
> > > > > > -
> > > > > > if (hid) {
> > > > > > device->pnp.hardware_id = ACPI_ALLOCATE_ZEROED(strlen (hid) + 1);
> > > > > > if (device->pnp.hardware_id) {
> > > > > >
> > > > > > --
> > > > > > 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
> > > > >
> > > > >
> > > >
> > > >
> > >
> >
>
>
--
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
