diff mbox

[v3,1/8] ACPI: fix synthetic HID for \_SB_

Message ID 20090921193504.21656.68847.stgit@bob.kio (mailing list archive)
State Accepted
Headers show

Commit Message

Bjorn Helgaas Sept. 21, 2009, 7:35 p.m. UTC 
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(-)


--
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

Comments

Zhao, Yakui Sept. 23, 2009, 2:21 a.m. UTC | #1 
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

--
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
Bjorn Helgaas Sept. 23, 2009, 4:19 p.m. UTC | #2 
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
> 
> 


--
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
Zhao, Yakui Sept. 24, 2009, 1:44 a.m. UTC | #3 
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
Bjorn Helgaas Sept. 24, 2009, 3:36 a.m. UTC | #4 
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
Zhao, Yakui Sept. 24, 2009, 5:22 a.m. UTC | #5 
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
Bjorn Helgaas Sept. 24, 2009, 9:42 p.m. UTC | #6 
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
diff mbox

Patch

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) {