* [PATCH v3 1/8] ACPI: fix synthetic HID for \_SB_
2009-09-21 19:34 [PATCH v3 0/8] ACPI: make every acpi_device have a HID Bjorn Helgaas
@ 2009-09-21 19:35 ` Bjorn Helgaas
2009-09-23 2:21 ` ykzhao
2009-09-21 19:35 ` [PATCH v3 2/8] ACPI: use acpi_device_hid() when possible Bjorn Helgaas
` (6 subsequent siblings)
7 siblings, 1 reply; 15+ messages in thread
From: Bjorn Helgaas @ 2009-09-21 19:35 UTC (permalink / raw)
To: Len Brown
Cc: linux-acpi, Hugh Dickins, Valdis Kletnieks, Lin Ming,
Bartlomiej Zolnierkiewicz
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@hp.com>
---
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)) {
+ /* \_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) {
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH v3 1/8] ACPI: fix synthetic HID for \_SB_
2009-09-21 19:35 ` [PATCH v3 1/8] ACPI: fix synthetic HID for \_SB_ Bjorn Helgaas
@ 2009-09-23 2:21 ` ykzhao
2009-09-23 16:19 ` Bjorn Helgaas
0 siblings, 1 reply; 15+ messages in thread
From: ykzhao @ 2009-09-23 2:21 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Len Brown, linux-acpi@vger.kernel.org, Hugh Dickins,
Valdis Kletnieks, Lin, Ming M, Bartlomiej Zolnierkiewicz
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@hp.com>
> ---
> 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
Where can I find the macro definition of ACPI_IS_ROOT_DEVICE?
Thanks.
> + /* \_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@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v3 1/8] ACPI: fix synthetic HID for \_SB_
2009-09-23 2:21 ` ykzhao
@ 2009-09-23 16:19 ` Bjorn Helgaas
2009-09-24 1:44 ` ykzhao
0 siblings, 1 reply; 15+ messages in thread
From: Bjorn Helgaas @ 2009-09-23 16:19 UTC (permalink / raw)
To: ykzhao
Cc: Len Brown, linux-acpi@vger.kernel.org, Hugh Dickins,
Valdis Kletnieks, Lin, Ming M, Bartlomiej Zolnierkiewicz
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@hp.com>
> > ---
> > 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.
> 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.
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@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v3 1/8] ACPI: fix synthetic HID for \_SB_
2009-09-23 16:19 ` Bjorn Helgaas
@ 2009-09-24 1:44 ` ykzhao
2009-09-24 3:36 ` Bjorn Helgaas
0 siblings, 1 reply; 15+ messages in thread
From: ykzhao @ 2009-09-24 1:44 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Len Brown, linux-acpi@vger.kernel.org, Hugh Dickins,
Valdis Kletnieks, Lin, Ming M, Bartlomiej Zolnierkiewicz
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@hp.com>
> > > ---
> > > 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;
}
>
> > 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.
thanks.
>
> 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@vger.kernel.org
> > > 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@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v3 1/8] ACPI: fix synthetic HID for \_SB_
2009-09-24 1:44 ` ykzhao
@ 2009-09-24 3:36 ` Bjorn Helgaas
2009-09-24 5:22 ` ykzhao
0 siblings, 1 reply; 15+ messages in thread
From: Bjorn Helgaas @ 2009-09-24 3:36 UTC (permalink / raw)
To: ykzhao
Cc: Len Brown, linux-acpi@vger.kernel.org, Hugh Dickins,
Valdis Kletnieks, Lin, Ming M, Bartlomiej Zolnierkiewicz
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@hp.com>
> > > > ---
> > > > 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.
> >
> > > 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@vger.kernel.org
> > > > 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@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v3 1/8] ACPI: fix synthetic HID for \_SB_
2009-09-24 3:36 ` Bjorn Helgaas
@ 2009-09-24 5:22 ` ykzhao
2009-09-24 21:42 ` Bjorn Helgaas
0 siblings, 1 reply; 15+ messages in thread
From: ykzhao @ 2009-09-24 5:22 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Len Brown, linux-acpi@vger.kernel.org, Hugh Dickins,
Valdis Kletnieks, Lin, Ming M, Bartlomiej Zolnierkiewicz
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@hp.com>
> > > > > ---
> > > > > 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.
And you delete the ACPI_BUS_TYPE_SYSTEM type in another patch set,
right?
What is the benefit if we change the type from ACPI_BUS_TYPE_SYSTEM to
ACPI_BUS_TYPE_DEVICE for the root device?
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@vger.kernel.org
> > > > > 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@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v3 1/8] ACPI: fix synthetic HID for \_SB_
2009-09-24 5:22 ` ykzhao
@ 2009-09-24 21:42 ` Bjorn Helgaas
0 siblings, 0 replies; 15+ messages in thread
From: Bjorn Helgaas @ 2009-09-24 21:42 UTC (permalink / raw)
To: ykzhao
Cc: Len Brown, linux-acpi@vger.kernel.org, Hugh Dickins,
Valdis Kletnieks, Lin, Ming M, Bartlomiej Zolnierkiewicz
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@hp.com>
> > > > > > ---
> > > > > > 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@vger.kernel.org
> > > > > > 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@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v3 2/8] ACPI: use acpi_device_hid() when possible
2009-09-21 19:34 [PATCH v3 0/8] ACPI: make every acpi_device have a HID Bjorn Helgaas
2009-09-21 19:35 ` [PATCH v3 1/8] ACPI: fix synthetic HID for \_SB_ Bjorn Helgaas
@ 2009-09-21 19:35 ` Bjorn Helgaas
2009-09-21 19:35 ` [PATCH v3 3/8] ACPI: make sure every acpi_device has an ID Bjorn Helgaas
` (5 subsequent siblings)
7 siblings, 0 replies; 15+ messages in thread
From: Bjorn Helgaas @ 2009-09-21 19:35 UTC (permalink / raw)
To: Len Brown
Cc: linux-acpi, Hugh Dickins, Valdis Kletnieks, Lin Ming,
Bartlomiej Zolnierkiewicz
Use acpi_device_hid() rather than accessing acpi_device.pnp.hardware_id
directly.
Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
---
drivers/acpi/scan.c | 6 +++---
drivers/pnp/pnpacpi/core.c | 2 +-
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index e9227ea..269c0aa 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -185,7 +185,7 @@ static ssize_t
acpi_device_hid_show(struct device *dev, struct device_attribute *attr, char *buf) {
struct acpi_device *acpi_dev = to_acpi_device(dev);
- return sprintf(buf, "%s\n", acpi_dev->pnp.hardware_id);
+ return sprintf(buf, "%s\n", acpi_device_hid(acpi_dev));
}
static DEVICE_ATTR(hid, 0444, acpi_device_hid_show, NULL);
@@ -501,7 +501,7 @@ static int acpi_device_register(struct acpi_device *device)
* If failed, create one and link it into acpi_bus_id_list
*/
list_for_each_entry(acpi_device_bus_id, &acpi_bus_id_list, node) {
- if(!strcmp(acpi_device_bus_id->bus_id, device->flags.hardware_id? device->pnp.hardware_id : "device")) {
+ if (!strcmp(acpi_device_bus_id->bus_id, device->flags.hardware_id ? acpi_device_hid(device) : "device")) {
acpi_device_bus_id->instance_no ++;
found = 1;
kfree(new_bus_id);
@@ -510,7 +510,7 @@ static int acpi_device_register(struct acpi_device *device)
}
if (!found) {
acpi_device_bus_id = new_bus_id;
- strcpy(acpi_device_bus_id->bus_id, device->flags.hardware_id ? device->pnp.hardware_id : "device");
+ strcpy(acpi_device_bus_id->bus_id, device->flags.hardware_id ? acpi_device_hid(device) : "device");
acpi_device_bus_id->instance_no = 0;
list_add_tail(&acpi_device_bus_id->node, &acpi_bus_id_list);
}
diff --git a/drivers/pnp/pnpacpi/core.c b/drivers/pnp/pnpacpi/core.c
index c07fdb9..ff963d4 100644
--- a/drivers/pnp/pnpacpi/core.c
+++ b/drivers/pnp/pnpacpi/core.c
@@ -234,7 +234,7 @@ static int __init acpi_pnp_match(struct device *dev, void *_pnp)
/* true means it matched */
return acpi->flags.hardware_id
&& !acpi_get_physical_device(acpi->handle)
- && compare_pnp_id(pnp->id, acpi->pnp.hardware_id);
+ && compare_pnp_id(pnp->id, acpi_device_hid(acpi));
}
static int __init acpi_pnp_find_device(struct device *dev, acpi_handle * handle)
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v3 3/8] ACPI: make sure every acpi_device has an ID
2009-09-21 19:34 [PATCH v3 0/8] ACPI: make every acpi_device have a HID Bjorn Helgaas
2009-09-21 19:35 ` [PATCH v3 1/8] ACPI: fix synthetic HID for \_SB_ Bjorn Helgaas
2009-09-21 19:35 ` [PATCH v3 2/8] ACPI: use acpi_device_hid() when possible Bjorn Helgaas
@ 2009-09-21 19:35 ` Bjorn Helgaas
2009-09-21 19:35 ` [PATCH v3 4/8] ACPI: maintain a single list of _HID and _CID IDs Bjorn Helgaas
` (4 subsequent siblings)
7 siblings, 0 replies; 15+ messages in thread
From: Bjorn Helgaas @ 2009-09-21 19:35 UTC (permalink / raw)
To: Len Brown
Cc: linux-acpi, Hugh Dickins, Valdis Kletnieks, Lin Ming,
Bartlomiej Zolnierkiewicz
This makes sure every acpi_device has at least one ID. If we build an
acpi_device for a namespace node with no _HID or _CID, we sometimes
synthesize an ID like "LNXCPU" or "LNXVIDEO". If we don't even have
that, give it a default "device" ID.
Note that this means things like:
/sys/devices/LNXSYSTM:00/LNXSYBUS:00/HWP0001:00/HWP0002:04/device:00
(a PCI slot SxFy device) will have "hid" and "modprobe" entries, where
they didn't before. These aren't very useful (a HID of "device" doesn't
tell you what *kind* of device it is, so it doesn't help find a driver),
but I don't think they're harmful.
Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
---
drivers/acpi/scan.c | 10 ++++++++++
1 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 269c0aa..53b96e7 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -1155,6 +1155,16 @@ static void acpi_device_set_id(struct acpi_device *device)
break;
}
+ /*
+ * We build acpi_devices for some objects that don't have _HID or _CID,
+ * e.g., PCI bridges and slots. Drivers can't bind to these objects,
+ * but we do use them indirectly by traversing the acpi_device tree.
+ * This generic ID isn't useful for driver binding, but it provides
+ * the useful property that "every acpi_device has an ID."
+ */
+ if (!hid && !cid_list && !cid_add)
+ hid = "device";
+
if (hid) {
device->pnp.hardware_id = ACPI_ALLOCATE_ZEROED(strlen (hid) + 1);
if (device->pnp.hardware_id) {
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v3 4/8] ACPI: maintain a single list of _HID and _CID IDs
2009-09-21 19:34 [PATCH v3 0/8] ACPI: make every acpi_device have a HID Bjorn Helgaas
` (2 preceding siblings ...)
2009-09-21 19:35 ` [PATCH v3 3/8] ACPI: make sure every acpi_device has an ID Bjorn Helgaas
@ 2009-09-21 19:35 ` Bjorn Helgaas
2009-09-21 19:35 ` [PATCH v3 5/8] ACPI: remove acpi_device.flags.compatible_ids Bjorn Helgaas
` (3 subsequent siblings)
7 siblings, 0 replies; 15+ messages in thread
From: Bjorn Helgaas @ 2009-09-21 19:35 UTC (permalink / raw)
To: Len Brown
Cc: Valdis Kletnieks, Bartlomiej Zolnierkiewicz, Lin Ming, linux-acpi,
Hugh Dickins, Alex Chiang
There's no need to treat _HID and _CID differently. Keeping them in
a single list makes code that uses the IDs a little simpler because it
can just traverse the list rather than checking "do we have a HID?",
"do we have any CIDs?"
Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
Reviewed-by: Alex Chiang <achiang@hp.com>
---
drivers/acpi/scan.c | 165 ++++++++++++--------------------------------
drivers/pnp/pnpacpi/core.c | 16 ++--
include/acpi/acpi_bus.h | 10 ++-
3 files changed, 59 insertions(+), 132 deletions(-)
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 53b96e7..9c65244 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -45,6 +45,7 @@ static int create_modalias(struct acpi_device *acpi_dev, char *modalias,
{
int len;
int count;
+ struct acpi_hardware_id *id;
if (!acpi_dev->flags.hardware_id && !acpi_dev->flags.compatible_ids)
return -ENODEV;
@@ -52,33 +53,14 @@ static int create_modalias(struct acpi_device *acpi_dev, char *modalias,
len = snprintf(modalias, size, "acpi:");
size -= len;
- if (acpi_dev->flags.hardware_id) {
- count = snprintf(&modalias[len], size, "%s:",
- acpi_dev->pnp.hardware_id);
+ list_for_each_entry(id, &acpi_dev->pnp.ids, list) {
+ count = snprintf(&modalias[len], size, "%s:", id->id);
if (count < 0 || count >= size)
return -EINVAL;
len += count;
size -= count;
}
- if (acpi_dev->flags.compatible_ids) {
- struct acpica_device_id_list *cid_list;
- int i;
-
- cid_list = acpi_dev->pnp.cid_list;
- for (i = 0; i < cid_list->count; i++) {
- count = snprintf(&modalias[len], size, "%s:",
- cid_list->ids[i].string);
- if (count < 0 || count >= size) {
- printk(KERN_ERR PREFIX "%s cid[%i] exceeds event buffer size",
- acpi_dev->pnp.device_name, i);
- break;
- }
- len += count;
- size -= count;
- }
- }
-
modalias[len] = '\0';
return len;
}
@@ -273,6 +255,7 @@ int acpi_match_device_ids(struct acpi_device *device,
const struct acpi_device_id *ids)
{
const struct acpi_device_id *id;
+ struct acpi_hardware_id *hwid;
/*
* If the device is not present, it is unnecessary to load device
@@ -281,40 +264,30 @@ int acpi_match_device_ids(struct acpi_device *device,
if (!device->status.present)
return -ENODEV;
- if (device->flags.hardware_id) {
- for (id = ids; id->id[0]; id++) {
- if (!strcmp((char*)id->id, device->pnp.hardware_id))
+ for (id = ids; id->id[0]; id++)
+ list_for_each_entry(hwid, &device->pnp.ids, list)
+ if (!strcmp((char *) id->id, hwid->id))
return 0;
- }
- }
-
- if (device->flags.compatible_ids) {
- struct acpica_device_id_list *cid_list = device->pnp.cid_list;
- int i;
-
- for (id = ids; id->id[0]; id++) {
- /* compare multiple _CID entries against driver ids */
- for (i = 0; i < cid_list->count; i++) {
- if (!strcmp((char*)id->id,
- cid_list->ids[i].string))
- return 0;
- }
- }
- }
return -ENOENT;
}
EXPORT_SYMBOL(acpi_match_device_ids);
+static void acpi_free_ids(struct acpi_device *device)
+{
+ struct acpi_hardware_id *id, *tmp;
+
+ list_for_each_entry_safe(id, tmp, &device->pnp.ids, list) {
+ kfree(id->id);
+ kfree(id);
+ }
+}
+
static void acpi_device_release(struct device *dev)
{
struct acpi_device *acpi_dev = to_acpi_device(dev);
- kfree(acpi_dev->pnp.cid_list);
- if (acpi_dev->flags.hardware_id)
- kfree(acpi_dev->pnp.hardware_id);
- if (acpi_dev->flags.unique_id)
- kfree(acpi_dev->pnp.unique_id);
+ acpi_free_ids(acpi_dev);
kfree(acpi_dev);
}
@@ -1028,62 +1001,30 @@ static int acpi_dock_match(struct acpi_device *device)
return acpi_get_handle(device->handle, "_DCK", &tmp);
}
-static struct acpica_device_id_list*
-acpi_add_cid(
- struct acpi_device_info *info,
- struct acpica_device_id *new_cid)
+char *acpi_device_hid(struct acpi_device *device)
{
- struct acpica_device_id_list *cid;
- char *next_id_string;
- acpi_size cid_length;
- acpi_size new_cid_length;
- u32 i;
-
-
- /* Allocate new CID list with room for the new CID */
-
- if (!new_cid)
- new_cid_length = info->compatible_id_list.list_size;
- else if (info->compatible_id_list.list_size)
- new_cid_length = info->compatible_id_list.list_size +
- new_cid->length + sizeof(struct acpica_device_id);
- else
- new_cid_length = sizeof(struct acpica_device_id_list) + new_cid->length;
-
- cid = ACPI_ALLOCATE_ZEROED(new_cid_length);
- if (!cid) {
- return NULL;
- }
-
- cid->list_size = new_cid_length;
- cid->count = info->compatible_id_list.count;
- if (new_cid)
- cid->count++;
- next_id_string = (char *) cid->ids + (cid->count * sizeof(struct acpica_device_id));
-
- /* Copy all existing CIDs */
+ struct acpi_hardware_id *hid;
- for (i = 0; i < info->compatible_id_list.count; i++) {
- cid_length = info->compatible_id_list.ids[i].length;
- cid->ids[i].string = next_id_string;
- cid->ids[i].length = cid_length;
-
- ACPI_MEMCPY(next_id_string, info->compatible_id_list.ids[i].string,
- cid_length);
-
- next_id_string += cid_length;
- }
+ hid = list_first_entry(&device->pnp.ids, struct acpi_hardware_id, list);
+ return hid->id;
+}
- /* Append the new CID */
+static void acpi_add_id(struct acpi_device *device, const char *dev_id)
+{
+ struct acpi_hardware_id *id;
- if (new_cid) {
- cid->ids[i].string = next_id_string;
- cid->ids[i].length = new_cid->length;
+ id = kmalloc(sizeof(*id), GFP_KERNEL);
+ if (!id)
+ return;
- ACPI_MEMCPY(next_id_string, new_cid->string, new_cid->length);
+ id->id = kmalloc(strlen(dev_id) + 1, GFP_KERNEL);
+ if (!id->id) {
+ kfree(id);
+ return;
}
- return cid;
+ strcpy(id->id, dev_id);
+ list_add_tail(&id->list, &device->pnp.ids);
}
static void acpi_device_set_id(struct acpi_device *device)
@@ -1094,6 +1035,7 @@ static void acpi_device_set_id(struct acpi_device *device)
struct acpica_device_id_list *cid_list = NULL;
char *cid_add = NULL;
acpi_status status;
+ int i;
switch (device->device_type) {
case ACPI_BUS_TYPE_DEVICE:
@@ -1166,15 +1108,9 @@ static void acpi_device_set_id(struct acpi_device *device)
hid = "device";
if (hid) {
- device->pnp.hardware_id = ACPI_ALLOCATE_ZEROED(strlen (hid) + 1);
- if (device->pnp.hardware_id) {
- strcpy(device->pnp.hardware_id, hid);
- device->flags.hardware_id = 1;
- }
+ acpi_add_id(device, hid);
+ device->flags.hardware_id = 1;
}
- if (!device->flags.hardware_id)
- device->pnp.hardware_id = "";
-
if (uid) {
device->pnp.unique_id = ACPI_ALLOCATE_ZEROED(strlen (uid) + 1);
if (device->pnp.unique_id) {
@@ -1185,24 +1121,12 @@ static void acpi_device_set_id(struct acpi_device *device)
if (!device->flags.unique_id)
device->pnp.unique_id = "";
- if (cid_list || cid_add) {
- struct acpica_device_id_list *list;
-
- if (cid_add) {
- struct acpica_device_id cid;
- cid.length = strlen (cid_add) + 1;
- cid.string = cid_add;
-
- list = acpi_add_cid(info, &cid);
- } else {
- list = acpi_add_cid(info, NULL);
- }
-
- if (list) {
- device->pnp.cid_list = list;
- if (cid_add)
- device->flags.compatible_ids = 1;
- }
+ if (cid_list)
+ for (i = 0; i < cid_list->count; i++)
+ acpi_add_id(device, cid_list->ids[i].string);
+ if (cid_add) {
+ acpi_add_id(device, cid_add);
+ device->flags.compatible_ids = 1;
}
kfree(info);
@@ -1269,6 +1193,7 @@ static int acpi_add_single_object(struct acpi_device **child,
return -ENOMEM;
}
+ INIT_LIST_HEAD(&device->pnp.ids);
device->device_type = type;
device->handle = handle;
device->parent = acpi_bus_get_parent(handle);
diff --git a/drivers/pnp/pnpacpi/core.c b/drivers/pnp/pnpacpi/core.c
index ff963d4..3a4478f 100644
--- a/drivers/pnp/pnpacpi/core.c
+++ b/drivers/pnp/pnpacpi/core.c
@@ -153,6 +153,7 @@ static int __init pnpacpi_add_device(struct acpi_device *device)
acpi_handle temp = NULL;
acpi_status status;
struct pnp_dev *dev;
+ struct acpi_hardware_id *id;
/*
* If a PnPacpi device is not present , the device
@@ -193,15 +194,12 @@ static int __init pnpacpi_add_device(struct acpi_device *device)
if (dev->capabilities & PNP_CONFIGURABLE)
pnpacpi_parse_resource_option_data(dev);
- if (device->flags.compatible_ids) {
- struct acpica_device_id_list *cid_list = device->pnp.cid_list;
- int i;
-
- for (i = 0; i < cid_list->count; i++) {
- if (!ispnpidacpi(cid_list->ids[i].string))
- continue;
- pnp_add_id(dev, cid_list->ids[i].string);
- }
+ list_for_each_entry(id, &device->pnp.ids, list) {
+ if (!strcmp(id->id, acpi_device_hid(device)))
+ continue;
+ if (!ispnpidacpi(id->id))
+ continue;
+ pnp_add_id(dev, id->id);
}
/* clear out the damaged flags */
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index 13a63a6..635e305 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -171,19 +171,23 @@ typedef unsigned long acpi_bus_address;
typedef char acpi_device_name[40];
typedef char acpi_device_class[20];
+struct acpi_hardware_id {
+ struct list_head list;
+ char *id;
+};
+
struct acpi_device_pnp {
acpi_bus_id bus_id; /* Object name */
acpi_bus_address bus_address; /* _ADR */
- char *hardware_id; /* _HID */
- struct acpica_device_id_list *cid_list; /* _CIDs */
char *unique_id; /* _UID */
+ struct list_head ids; /* _HID and _CIDs */
acpi_device_name device_name; /* Driver-determined */
acpi_device_class device_class; /* " */
};
#define acpi_device_bid(d) ((d)->pnp.bus_id)
#define acpi_device_adr(d) ((d)->pnp.bus_address)
-#define acpi_device_hid(d) ((d)->pnp.hardware_id)
+char *acpi_device_hid(struct acpi_device *device);
#define acpi_device_uid(d) ((d)->pnp.unique_id)
#define acpi_device_name(d) ((d)->pnp.device_name)
#define acpi_device_class(d) ((d)->pnp.device_class)
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v3 5/8] ACPI: remove acpi_device.flags.compatible_ids
2009-09-21 19:34 [PATCH v3 0/8] ACPI: make every acpi_device have a HID Bjorn Helgaas
` (3 preceding siblings ...)
2009-09-21 19:35 ` [PATCH v3 4/8] ACPI: maintain a single list of _HID and _CID IDs Bjorn Helgaas
@ 2009-09-21 19:35 ` Bjorn Helgaas
2009-09-21 19:35 ` [PATCH v3 6/8] ACPI: remove acpi_device.flags.hardware_id Bjorn Helgaas
` (2 subsequent siblings)
7 siblings, 0 replies; 15+ messages in thread
From: Bjorn Helgaas @ 2009-09-21 19:35 UTC (permalink / raw)
To: Len Brown
Cc: linux-acpi, Hugh Dickins, Valdis Kletnieks, Lin Ming,
Bartlomiej Zolnierkiewicz
We now keep a single list of IDs that includes both the _HID and any
_CIDs. We no longer need to keep track of whether the device has a _CID.
Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
---
drivers/acpi/scan.c | 15 ++++-----------
include/acpi/acpi_bus.h | 3 +--
2 files changed, 5 insertions(+), 13 deletions(-)
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 9c65244..954cb1d 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -47,7 +47,7 @@ static int create_modalias(struct acpi_device *acpi_dev, char *modalias,
int count;
struct acpi_hardware_id *id;
- if (!acpi_dev->flags.hardware_id && !acpi_dev->flags.compatible_ids)
+ if (!acpi_dev->flags.hardware_id)
return -ENODEV;
len = snprintf(modalias, size, "acpi:");
@@ -209,7 +209,7 @@ static int acpi_device_setup_files(struct acpi_device *dev)
goto end;
}
- if (dev->flags.hardware_id || dev->flags.compatible_ids) {
+ if (dev->flags.hardware_id) {
result = device_create_file(&dev->dev, &dev_attr_modalias);
if (result)
goto end;
@@ -239,7 +239,7 @@ static void acpi_device_remove_files(struct acpi_device *dev)
if (ACPI_SUCCESS(status))
device_remove_file(&dev->dev, &dev_attr_eject);
- if (dev->flags.hardware_id || dev->flags.compatible_ids)
+ if (dev->flags.hardware_id)
device_remove_file(&dev->dev, &dev_attr_modalias);
if (dev->flags.hardware_id)
@@ -876,11 +876,6 @@ static int acpi_bus_get_flags(struct acpi_device *device)
if (ACPI_SUCCESS(status))
device->flags.dynamic_status = 1;
- /* Presence of _CID indicates 'compatible_ids' */
- status = acpi_get_handle(device->handle, "_CID", &temp);
- if (ACPI_SUCCESS(status))
- device->flags.compatible_ids = 1;
-
/* Presence of _RMV indicates 'removable' */
status = acpi_get_handle(device->handle, "_RMV", &temp);
if (ACPI_SUCCESS(status))
@@ -1124,10 +1119,8 @@ static void acpi_device_set_id(struct acpi_device *device)
if (cid_list)
for (i = 0; i < cid_list->count; i++)
acpi_add_id(device, cid_list->ids[i].string);
- if (cid_add) {
+ if (cid_add)
acpi_add_id(device, cid_add);
- device->flags.compatible_ids = 1;
- }
kfree(info);
}
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index 635e305..e422547 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -142,7 +142,6 @@ struct acpi_device_status {
struct acpi_device_flags {
u32 dynamic_status:1;
u32 hardware_id:1;
- u32 compatible_ids:1;
u32 bus_address:1;
u32 unique_id:1;
u32 removable:1;
@@ -153,7 +152,7 @@ struct acpi_device_flags {
u32 performance_manageable:1;
u32 wake_capable:1; /* Wakeup(_PRW) supported? */
u32 force_power_state:1;
- u32 reserved:19;
+ u32 reserved:20;
};
/* File System */
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v3 6/8] ACPI: remove acpi_device.flags.hardware_id
2009-09-21 19:34 [PATCH v3 0/8] ACPI: make every acpi_device have a HID Bjorn Helgaas
` (4 preceding siblings ...)
2009-09-21 19:35 ` [PATCH v3 5/8] ACPI: remove acpi_device.flags.compatible_ids Bjorn Helgaas
@ 2009-09-21 19:35 ` Bjorn Helgaas
2009-09-21 19:35 ` [PATCH v3 7/8] ACPI: remove acpi_device_uid() and related stuff Bjorn Helgaas
2009-09-21 19:35 ` [PATCH v3 8/8] ACPI: simplify building device HID/CID list Bjorn Helgaas
7 siblings, 0 replies; 15+ messages in thread
From: Bjorn Helgaas @ 2009-09-21 19:35 UTC (permalink / raw)
To: Len Brown
Cc: linux-acpi, Hugh Dickins, Valdis Kletnieks, Lin Ming,
Bartlomiej Zolnierkiewicz
Every acpi_device has at least one ID (if there's no _HID or _CID, we
give it a synthetic or default ID). So there's no longer a need to
check whether an ID exists; we can just use it.
Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
---
drivers/acpi/scan.c | 37 +++++++++++++------------------------
drivers/pnp/pnpacpi/core.c | 3 +--
include/acpi/acpi_bus.h | 3 +--
3 files changed, 15 insertions(+), 28 deletions(-)
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 954cb1d..3f9b8d0 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -47,9 +47,6 @@ static int create_modalias(struct acpi_device *acpi_dev, char *modalias,
int count;
struct acpi_hardware_id *id;
- if (!acpi_dev->flags.hardware_id)
- return -ENODEV;
-
len = snprintf(modalias, size, "acpi:");
size -= len;
@@ -203,17 +200,13 @@ static int acpi_device_setup_files(struct acpi_device *dev)
goto end;
}
- if (dev->flags.hardware_id) {
- result = device_create_file(&dev->dev, &dev_attr_hid);
- if (result)
- goto end;
- }
+ result = device_create_file(&dev->dev, &dev_attr_hid);
+ if (result)
+ goto end;
- if (dev->flags.hardware_id) {
- result = device_create_file(&dev->dev, &dev_attr_modalias);
- if (result)
- goto end;
- }
+ result = device_create_file(&dev->dev, &dev_attr_modalias);
+ if (result)
+ goto end;
/*
* If device has _EJ0, 'eject' file is created that is used to trigger
@@ -239,11 +232,8 @@ static void acpi_device_remove_files(struct acpi_device *dev)
if (ACPI_SUCCESS(status))
device_remove_file(&dev->dev, &dev_attr_eject);
- if (dev->flags.hardware_id)
- device_remove_file(&dev->dev, &dev_attr_modalias);
-
- if (dev->flags.hardware_id)
- device_remove_file(&dev->dev, &dev_attr_hid);
+ device_remove_file(&dev->dev, &dev_attr_modalias);
+ device_remove_file(&dev->dev, &dev_attr_hid);
if (dev->handle)
device_remove_file(&dev->dev, &dev_attr_path);
}
@@ -474,8 +464,9 @@ static int acpi_device_register(struct acpi_device *device)
* If failed, create one and link it into acpi_bus_id_list
*/
list_for_each_entry(acpi_device_bus_id, &acpi_bus_id_list, node) {
- if (!strcmp(acpi_device_bus_id->bus_id, device->flags.hardware_id ? acpi_device_hid(device) : "device")) {
- acpi_device_bus_id->instance_no ++;
+ if (!strcmp(acpi_device_bus_id->bus_id,
+ acpi_device_hid(device))) {
+ acpi_device_bus_id->instance_no++;
found = 1;
kfree(new_bus_id);
break;
@@ -483,7 +474,7 @@ static int acpi_device_register(struct acpi_device *device)
}
if (!found) {
acpi_device_bus_id = new_bus_id;
- strcpy(acpi_device_bus_id->bus_id, device->flags.hardware_id ? acpi_device_hid(device) : "device");
+ strcpy(acpi_device_bus_id->bus_id, acpi_device_hid(device));
acpi_device_bus_id->instance_no = 0;
list_add_tail(&acpi_device_bus_id->node, &acpi_bus_id_list);
}
@@ -1102,10 +1093,8 @@ static void acpi_device_set_id(struct acpi_device *device)
if (!hid && !cid_list && !cid_add)
hid = "device";
- if (hid) {
+ if (hid)
acpi_add_id(device, hid);
- device->flags.hardware_id = 1;
- }
if (uid) {
device->pnp.unique_id = ACPI_ALLOCATE_ZEROED(strlen (uid) + 1);
if (device->pnp.unique_id) {
diff --git a/drivers/pnp/pnpacpi/core.c b/drivers/pnp/pnpacpi/core.c
index 3a4478f..83b8b5a 100644
--- a/drivers/pnp/pnpacpi/core.c
+++ b/drivers/pnp/pnpacpi/core.c
@@ -230,8 +230,7 @@ static int __init acpi_pnp_match(struct device *dev, void *_pnp)
struct pnp_dev *pnp = _pnp;
/* true means it matched */
- return acpi->flags.hardware_id
- && !acpi_get_physical_device(acpi->handle)
+ return !acpi_get_physical_device(acpi->handle)
&& compare_pnp_id(pnp->id, acpi_device_hid(acpi));
}
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index e422547..1d205f5 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -141,7 +141,6 @@ struct acpi_device_status {
struct acpi_device_flags {
u32 dynamic_status:1;
- u32 hardware_id:1;
u32 bus_address:1;
u32 unique_id:1;
u32 removable:1;
@@ -152,7 +151,7 @@ struct acpi_device_flags {
u32 performance_manageable:1;
u32 wake_capable:1; /* Wakeup(_PRW) supported? */
u32 force_power_state:1;
- u32 reserved:20;
+ u32 reserved:21;
};
/* File System */
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v3 7/8] ACPI: remove acpi_device_uid() and related stuff
2009-09-21 19:34 [PATCH v3 0/8] ACPI: make every acpi_device have a HID Bjorn Helgaas
` (5 preceding siblings ...)
2009-09-21 19:35 ` [PATCH v3 6/8] ACPI: remove acpi_device.flags.hardware_id Bjorn Helgaas
@ 2009-09-21 19:35 ` Bjorn Helgaas
2009-09-21 19:35 ` [PATCH v3 8/8] ACPI: simplify building device HID/CID list Bjorn Helgaas
7 siblings, 0 replies; 15+ messages in thread
From: Bjorn Helgaas @ 2009-09-21 19:35 UTC (permalink / raw)
To: Len Brown
Cc: linux-acpi, Hugh Dickins, Valdis Kletnieks, Lin Ming,
Bartlomiej Zolnierkiewicz
Nobody uses acpi_device_uid(), so this patch removes it.
Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
---
drivers/acpi/scan.c | 18 ------------------
include/acpi/acpi_bus.h | 4 +---
2 files changed, 1 insertions(+), 21 deletions(-)
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 3f9b8d0..6880852 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -1017,7 +1017,6 @@ static void acpi_device_set_id(struct acpi_device *device)
{
struct acpi_device_info *info = NULL;
char *hid = NULL;
- char *uid = NULL;
struct acpica_device_id_list *cid_list = NULL;
char *cid_add = NULL;
acpi_status status;
@@ -1044,8 +1043,6 @@ static void acpi_device_set_id(struct acpi_device *device)
if (info->valid & ACPI_VALID_HID)
hid = info->hardware_id.string;
- if (info->valid & ACPI_VALID_UID)
- uid = info->unique_id.string;
if (info->valid & ACPI_VALID_CID)
cid_list = &info->compatible_id_list;
if (info->valid & ACPI_VALID_ADR) {
@@ -1095,16 +1092,6 @@ static void acpi_device_set_id(struct acpi_device *device)
if (hid)
acpi_add_id(device, hid);
- if (uid) {
- device->pnp.unique_id = ACPI_ALLOCATE_ZEROED(strlen (uid) + 1);
- if (device->pnp.unique_id) {
- strcpy(device->pnp.unique_id, uid);
- device->flags.unique_id = 1;
- }
- }
- if (!device->flags.unique_id)
- device->pnp.unique_id = "";
-
if (cid_list)
for (i = 0; i < cid_list->count; i++)
acpi_add_id(device, cid_list->ids[i].string);
@@ -1199,11 +1186,6 @@ static int acpi_add_single_object(struct acpi_device **child,
* -----------------
* TBD: Synch with Core's enumeration/initialization process.
*/
-
- /*
- * Hardware ID, Unique ID, & Bus Address
- * -------------------------------------
- */
acpi_device_set_id(device);
/*
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index 1d205f5..b8f195d 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -142,7 +142,6 @@ struct acpi_device_status {
struct acpi_device_flags {
u32 dynamic_status:1;
u32 bus_address:1;
- u32 unique_id:1;
u32 removable:1;
u32 ejectable:1;
u32 lockable:1;
@@ -151,7 +150,7 @@ struct acpi_device_flags {
u32 performance_manageable:1;
u32 wake_capable:1; /* Wakeup(_PRW) supported? */
u32 force_power_state:1;
- u32 reserved:21;
+ u32 reserved:22;
};
/* File System */
@@ -186,7 +185,6 @@ struct acpi_device_pnp {
#define acpi_device_bid(d) ((d)->pnp.bus_id)
#define acpi_device_adr(d) ((d)->pnp.bus_address)
char *acpi_device_hid(struct acpi_device *device);
-#define acpi_device_uid(d) ((d)->pnp.unique_id)
#define acpi_device_name(d) ((d)->pnp.device_name)
#define acpi_device_class(d) ((d)->pnp.device_class)
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v3 8/8] ACPI: simplify building device HID/CID list
2009-09-21 19:34 [PATCH v3 0/8] ACPI: make every acpi_device have a HID Bjorn Helgaas
` (6 preceding siblings ...)
2009-09-21 19:35 ` [PATCH v3 7/8] ACPI: remove acpi_device_uid() and related stuff Bjorn Helgaas
@ 2009-09-21 19:35 ` Bjorn Helgaas
7 siblings, 0 replies; 15+ messages in thread
From: Bjorn Helgaas @ 2009-09-21 19:35 UTC (permalink / raw)
To: Len Brown
Cc: linux-acpi, Hugh Dickins, Valdis Kletnieks, Lin Ming,
Bartlomiej Zolnierkiewicz
Minor code cleanup, no functional change. Instead of remembering
what HIDs & CIDs to add later, just add them immediately.
Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
---
drivers/acpi/scan.c | 56 +++++++++++++++++++++------------------------------
1 files changed, 23 insertions(+), 33 deletions(-)
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 6880852..1ef5b7e 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -1015,21 +1015,19 @@ static void acpi_add_id(struct acpi_device *device, const char *dev_id)
static void acpi_device_set_id(struct acpi_device *device)
{
- struct acpi_device_info *info = NULL;
- char *hid = NULL;
- struct acpica_device_id_list *cid_list = NULL;
- char *cid_add = NULL;
acpi_status status;
+ struct acpi_device_info *info;
+ struct acpica_device_id_list *cid_list;
int i;
switch (device->device_type) {
case ACPI_BUS_TYPE_DEVICE:
if (ACPI_IS_ROOT_DEVICE(device)) {
- hid = ACPI_SYSTEM_HID;
+ acpi_add_id(device, ACPI_SYSTEM_HID);
break;
} else if (ACPI_IS_ROOT_DEVICE(device->parent)) {
/* \_SB_, the only root-level namespace device */
- hid = ACPI_BUS_HID;
+ 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;
@@ -1042,41 +1040,43 @@ static void acpi_device_set_id(struct acpi_device *device)
}
if (info->valid & ACPI_VALID_HID)
- hid = info->hardware_id.string;
- if (info->valid & ACPI_VALID_CID)
+ acpi_add_id(device, info->hardware_id.string);
+ if (info->valid & ACPI_VALID_CID) {
cid_list = &info->compatible_id_list;
+ for (i = 0; i < cid_list->count; i++)
+ acpi_add_id(device, cid_list->ids[i].string);
+ }
if (info->valid & ACPI_VALID_ADR) {
device->pnp.bus_address = info->address;
device->flags.bus_address = 1;
}
- /* If we have a video/bay/dock device, add our selfdefined
- HID to the CID list. Like that the video/bay/dock drivers
- will get autoloaded and the device might still match
- against another driver.
- */
+ /*
+ * Some devices don't reliably have _HIDs & _CIDs, so add
+ * synthetic HIDs to make sure drivers can find them.
+ */
if (acpi_is_video_device(device))
- cid_add = ACPI_VIDEO_HID;
+ acpi_add_id(device, ACPI_VIDEO_HID);
else if (ACPI_SUCCESS(acpi_bay_match(device)))
- cid_add = ACPI_BAY_HID;
+ acpi_add_id(device, ACPI_BAY_HID);
else if (ACPI_SUCCESS(acpi_dock_match(device)))
- cid_add = ACPI_DOCK_HID;
+ acpi_add_id(device, ACPI_DOCK_HID);
break;
case ACPI_BUS_TYPE_POWER:
- hid = ACPI_POWER_HID;
+ acpi_add_id(device, ACPI_POWER_HID);
break;
case ACPI_BUS_TYPE_PROCESSOR:
- hid = ACPI_PROCESSOR_OBJECT_HID;
+ acpi_add_id(device, ACPI_PROCESSOR_OBJECT_HID);
break;
case ACPI_BUS_TYPE_THERMAL:
- hid = ACPI_THERMAL_HID;
+ acpi_add_id(device, ACPI_THERMAL_HID);
break;
case ACPI_BUS_TYPE_POWER_BUTTON:
- hid = ACPI_BUTTON_HID_POWERF;
+ acpi_add_id(device, ACPI_BUTTON_HID_POWERF);
break;
case ACPI_BUS_TYPE_SLEEP_BUTTON:
- hid = ACPI_BUTTON_HID_SLEEPF;
+ acpi_add_id(device, ACPI_BUTTON_HID_SLEEPF);
break;
}
@@ -1087,18 +1087,8 @@ static void acpi_device_set_id(struct acpi_device *device)
* This generic ID isn't useful for driver binding, but it provides
* the useful property that "every acpi_device has an ID."
*/
- if (!hid && !cid_list && !cid_add)
- hid = "device";
-
- if (hid)
- acpi_add_id(device, hid);
- if (cid_list)
- for (i = 0; i < cid_list->count; i++)
- acpi_add_id(device, cid_list->ids[i].string);
- if (cid_add)
- acpi_add_id(device, cid_add);
-
- kfree(info);
+ if (list_empty(&device->pnp.ids))
+ acpi_add_id(device, "device");
}
static int acpi_device_set_context(struct acpi_device *device)
^ permalink raw reply related [flat|nested] 15+ messages in thread