[PATCH 1/1] PCI: acpi_pcihp, run _OSC on a root bridge

[PATCH 1/1] PCI: acpi_pcihp, run _OSC on a root bridge

[Jiri Slaby]

_OSC should be ran on a root bridge instead of the device itself.
Do this before touching OSHP since PCI fw specs states that _OSC
should be preferred over OSHP (however if the device has OSHP but
not _OSC -- not a root bridge -- it's not).

Signed-off-by: Jiri Slaby <jirislaby@gmail.com>
Cc: kristen.c.accardi@intel.com
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/pci/hotplug/acpi_pcihp.c |   41 ++++++++++++++++++++++++++-----------
 1 files changed, 29 insertions(+), 12 deletions(-)

diff --git a/drivers/pci/hotplug/acpi_pcihp.c b/drivers/pci/hotplug/acpi_pcihp.c
index 93e37f0..bd83197 100644
--- a/drivers/pci/hotplug/acpi_pcihp.c
+++ b/drivers/pci/hotplug/acpi_pcihp.c
@@ -382,7 +382,7 @@ EXPORT_SYMBOL_GPL(acpi_get_hp_params_from_firmware);
 int acpi_get_hp_hw_control_from_firmware(struct pci_dev *dev, u32 flags)
 {
 	acpi_status status;
-	acpi_handle chandle, handle = DEVICE_ACPI_HANDLE(&(dev->dev));
+	acpi_handle chandle, handle;
 	struct pci_dev *pdev = dev;
 	struct pci_bus *parent;
 	struct acpi_buffer string = { ACPI_ALLOCATE_BUFFER, NULL };
@@ -399,10 +399,28 @@ int acpi_get_hp_hw_control_from_firmware(struct pci_dev *dev, u32 flags)
 	 * Per PCI firmware specification, we should run the ACPI _OSC
 	 * method to get control of hotplug hardware before using it. If
 	 * an _OSC is missing, we look for an OSHP to do the same thing.
-	 * To handle different BIOS behavior, we look for _OSC and OSHP
-	 * within the scope of the hotplug controller and its parents,
+	 * To handle different BIOS behavior, we look for _OSC on a root
+	 * bridge preferentially (according to PCI fw spec). Later for
+	 * OSHP within the scope of the hotplug controller and its parents,
 	 * upto the host bridge under which this controller exists.
 	 */
+	while (pdev->bus->self)
+		pdev = pdev->bus->self;
+	handle = acpi_get_pci_rootbridge_handle(pci_domain_nr(pdev->bus),
+			pdev->bus->number);
+	if (handle) {
+		acpi_get_name(handle, ACPI_FULL_PATHNAME, &string);
+		dbg("Trying to get hotplug control for %s\n",
+				(char *)string.pointer);
+		status = pci_osc_control_set(handle, flags);
+		if (ACPI_SUCCESS(status))
+			goto got_one;
+		kfree(string.pointer);
+		string = (struct acpi_buffer){ ACPI_ALLOCATE_BUFFER, NULL };
+	}
+
+	pdev = dev;
+	handle = DEVICE_ACPI_HANDLE(&dev->dev);
 	while (!handle) {
 		/*
 		 * This hotplug controller was not listed in the ACPI name
@@ -427,15 +445,9 @@ int acpi_get_hp_hw_control_from_firmware(struct pci_dev *dev, u32 flags)
 		acpi_get_name(handle, ACPI_FULL_PATHNAME, &string);
 		dbg("Trying to get hotplug control for %s \n",
 		    (char *)string.pointer);
-		status = pci_osc_control_set(handle, flags);
-		if (status == AE_NOT_FOUND)
-			status = acpi_run_oshp(handle);
-		if (ACPI_SUCCESS(status)) {
-			dbg("Gained control for hotplug HW for pci %s (%s)\n",
-			    pci_name(dev), (char *)string.pointer);
-			kfree(string.pointer);
-			return 0;
-		}
+		status = acpi_run_oshp(handle);
+		if (ACPI_SUCCESS(status))
+			goto got_one;
 		if (acpi_root_bridge(handle))
 			break;
 		chandle = handle;
@@ -449,6 +461,11 @@ int acpi_get_hp_hw_control_from_firmware(struct pci_dev *dev, u32 flags)
 
 	kfree(string.pointer);
 	return -ENODEV;
+got_one:
+	dbg("Gained control for hotplug HW for pci %s (%s)\n", pci_name(dev),
+			(char *)string.pointer);
+	kfree(string.pointer);
+	return 0;
 }
 EXPORT_SYMBOL(acpi_get_hp_hw_control_from_firmware);
 
-- 
1.5.6.5

Re: [PATCH 1/1] PCI: acpi_pcihp, run _OSC on a root bridge

[Jesse Barnes]

[Cc'ing Alex & Kenji-san for their comments on this fix.]

On Monday, August 11, 2008 8:47 am Jiri Slaby wrote:
> _OSC should be ran on a root bridge instead of the device itself.
> Do this before touching OSHP since PCI fw specs states that _OSC
> should be preferred over OSHP (however if the device has OSHP but
> not _OSC -- not a root bridge -- it's not).
>
> Signed-off-by: Jiri Slaby <jirislaby@gmail.com>
> Cc: kristen.c.accardi@intel.com
> Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
> ---
>  drivers/pci/hotplug/acpi_pcihp.c |   41
> ++++++++++++++++++++++++++----------- 1 files changed, 29 insertions(+), 12
> deletions(-)
>
> diff --git a/drivers/pci/hotplug/acpi_pcihp.c
> b/drivers/pci/hotplug/acpi_pcihp.c index 93e37f0..bd83197 100644
> --- a/drivers/pci/hotplug/acpi_pcihp.c
> +++ b/drivers/pci/hotplug/acpi_pcihp.c
> @@ -382,7 +382,7 @@ EXPORT_SYMBOL_GPL(acpi_get_hp_params_from_firmware);
>  int acpi_get_hp_hw_control_from_firmware(struct pci_dev *dev, u32 flags)
>  {
>  	acpi_status status;
> -	acpi_handle chandle, handle = DEVICE_ACPI_HANDLE(&(dev->dev));
> +	acpi_handle chandle, handle;
>  	struct pci_dev *pdev = dev;
>  	struct pci_bus *parent;
>  	struct acpi_buffer string = { ACPI_ALLOCATE_BUFFER, NULL };
> @@ -399,10 +399,28 @@ int acpi_get_hp_hw_control_from_firmware(struct
> pci_dev *dev, u32 flags) * Per PCI firmware specification, we should run
> the ACPI _OSC
>  	 * method to get control of hotplug hardware before using it. If
>  	 * an _OSC is missing, we look for an OSHP to do the same thing.
> -	 * To handle different BIOS behavior, we look for _OSC and OSHP
> -	 * within the scope of the hotplug controller and its parents,
> +	 * To handle different BIOS behavior, we look for _OSC on a root
> +	 * bridge preferentially (according to PCI fw spec). Later for
> +	 * OSHP within the scope of the hotplug controller and its parents,
>  	 * upto the host bridge under which this controller exists.
>  	 */
> +	while (pdev->bus->self)
> +		pdev = pdev->bus->self;
> +	handle = acpi_get_pci_rootbridge_handle(pci_domain_nr(pdev->bus),
> +			pdev->bus->number);
> +	if (handle) {
> +		acpi_get_name(handle, ACPI_FULL_PATHNAME, &string);
> +		dbg("Trying to get hotplug control for %s\n",
> +				(char *)string.pointer);
> +		status = pci_osc_control_set(handle, flags);
> +		if (ACPI_SUCCESS(status))
> +			goto got_one;
> +		kfree(string.pointer);
> +		string = (struct acpi_buffer){ ACPI_ALLOCATE_BUFFER, NULL };
> +	}
> +
> +	pdev = dev;
> +	handle = DEVICE_ACPI_HANDLE(&dev->dev);
>  	while (!handle) {
>  		/*
>  		 * This hotplug controller was not listed in the ACPI name
> @@ -427,15 +445,9 @@ int acpi_get_hp_hw_control_from_firmware(struct
> pci_dev *dev, u32 flags) acpi_get_name(handle, ACPI_FULL_PATHNAME,
> &string);
>  		dbg("Trying to get hotplug control for %s \n",
>  		    (char *)string.pointer);
> -		status = pci_osc_control_set(handle, flags);
> -		if (status == AE_NOT_FOUND)
> -			status = acpi_run_oshp(handle);
> -		if (ACPI_SUCCESS(status)) {
> -			dbg("Gained control for hotplug HW for pci %s (%s)\n",
> -			    pci_name(dev), (char *)string.pointer);
> -			kfree(string.pointer);
> -			return 0;
> -		}
> +		status = acpi_run_oshp(handle);
> +		if (ACPI_SUCCESS(status))
> +			goto got_one;
>  		if (acpi_root_bridge(handle))
>  			break;
>  		chandle = handle;
> @@ -449,6 +461,11 @@ int acpi_get_hp_hw_control_from_firmware(struct
> pci_dev *dev, u32 flags)
>
>  	kfree(string.pointer);
>  	return -ENODEV;
> +got_one:
> +	dbg("Gained control for hotplug HW for pci %s (%s)\n", pci_name(dev),
> +			(char *)string.pointer);
> +	kfree(string.pointer);
> +	return 0;
>  }
>  EXPORT_SYMBOL(acpi_get_hp_hw_control_from_firmware);



-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [PATCH 1/1] PCI: acpi_pcihp, run _OSC on a root bridge

[Alex Chiang]

* Jesse Barnes <jbarnes@virtuousgeek.org>:
> [Cc'ing Alex & Kenji-san for their comments on this fix.]
> 
> On Monday, August 11, 2008 8:47 am Jiri Slaby wrote:
> > _OSC should be ran on a root bridge instead of the device itself.
> > Do this before touching OSHP since PCI fw specs states that _OSC
> > should be preferred over OSHP (however if the device has OSHP but
> > not _OSC -- not a root bridge -- it's not).

Reviewed the PCI fw spec (v3.0) and this code, and Jiri's change
seems sane to me.

One minor nit below.

> >
> > Signed-off-by: Jiri Slaby <jirislaby@gmail.com>
> > Cc: kristen.c.accardi@intel.com
> > Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
> > ---
> >  drivers/pci/hotplug/acpi_pcihp.c |   41
> > ++++++++++++++++++++++++++----------- 1 files changed, 29 insertions(+), 12
> > deletions(-)
> >
> > diff --git a/drivers/pci/hotplug/acpi_pcihp.c
> > b/drivers/pci/hotplug/acpi_pcihp.c index 93e37f0..bd83197 100644
> > --- a/drivers/pci/hotplug/acpi_pcihp.c
> > +++ b/drivers/pci/hotplug/acpi_pcihp.c
> > @@ -382,7 +382,7 @@ EXPORT_SYMBOL_GPL(acpi_get_hp_params_from_firmware);
> >  int acpi_get_hp_hw_control_from_firmware(struct pci_dev *dev, u32 flags)
> >  {
> >  	acpi_status status;
> > -	acpi_handle chandle, handle = DEVICE_ACPI_HANDLE(&(dev->dev));
> > +	acpi_handle chandle, handle;
> >  	struct pci_dev *pdev = dev;
> >  	struct pci_bus *parent;
> >  	struct acpi_buffer string = { ACPI_ALLOCATE_BUFFER, NULL };
> > @@ -399,10 +399,28 @@ int acpi_get_hp_hw_control_from_firmware(struct
> > pci_dev *dev, u32 flags) * Per PCI firmware specification, we should run
> > the ACPI _OSC
> >  	 * method to get control of hotplug hardware before using it. If
> >  	 * an _OSC is missing, we look for an OSHP to do the same thing.
> > -	 * To handle different BIOS behavior, we look for _OSC and OSHP
> > -	 * within the scope of the hotplug controller and its parents,
> > +	 * To handle different BIOS behavior, we look for _OSC on a root
> > +	 * bridge preferentially (according to PCI fw spec). Later for
> > +	 * OSHP within the scope of the hotplug controller and its parents,
> >  	 * upto the host bridge under which this controller exists.
> >  	 */
> > +	while (pdev->bus->self)
> > +		pdev = pdev->bus->self;
> > +	handle = acpi_get_pci_rootbridge_handle(pci_domain_nr(pdev->bus),
> > +			pdev->bus->number);

This looks an awful lot like the first few lines of
aer_osc_setup(). Any chance you could factor those lines out into
a common inline?

static inline acpi_handle acpi_find_root_bridge_handle(struct pci_dev *pdev)
{
	while (pdev->bus->self)
		pdev = pdev->bus->self;

	return acpi_get_pci_rootbridge_handle(pci_domain_nr(pdev->bus),
					      pdev->bus->number);
}

<linux/pci-acpi.h> might be a good place for that.

Just a thought.

But otherwise,

Acked-by: Alex Chiang <achiang@hp.com>

Thanks.

/ac

> > +	if (handle) {
> > +		acpi_get_name(handle, ACPI_FULL_PATHNAME, &string);
> > +		dbg("Trying to get hotplug control for %s\n",
> > +				(char *)string.pointer);
> > +		status = pci_osc_control_set(handle, flags);
> > +		if (ACPI_SUCCESS(status))
> > +			goto got_one;
> > +		kfree(string.pointer);
> > +		string = (struct acpi_buffer){ ACPI_ALLOCATE_BUFFER, NULL };
> > +	}
> > +
> > +	pdev = dev;
> > +	handle = DEVICE_ACPI_HANDLE(&dev->dev);
> >  	while (!handle) {
> >  		/*
> >  		 * This hotplug controller was not listed in the ACPI name
> > @@ -427,15 +445,9 @@ int acpi_get_hp_hw_control_from_firmware(struct
> > pci_dev *dev, u32 flags) acpi_get_name(handle, ACPI_FULL_PATHNAME,
> > &string);
> >  		dbg("Trying to get hotplug control for %s \n",
> >  		    (char *)string.pointer);
> > -		status = pci_osc_control_set(handle, flags);
> > -		if (status == AE_NOT_FOUND)
> > -			status = acpi_run_oshp(handle);
> > -		if (ACPI_SUCCESS(status)) {
> > -			dbg("Gained control for hotplug HW for pci %s (%s)\n",
> > -			    pci_name(dev), (char *)string.pointer);
> > -			kfree(string.pointer);
> > -			return 0;
> > -		}
> > +		status = acpi_run_oshp(handle);
> > +		if (ACPI_SUCCESS(status))
> > +			goto got_one;
> >  		if (acpi_root_bridge(handle))
> >  			break;
> >  		chandle = handle;
> > @@ -449,6 +461,11 @@ int acpi_get_hp_hw_control_from_firmware(struct
> > pci_dev *dev, u32 flags)
> >
> >  	kfree(string.pointer);
> >  	return -ENODEV;
> > +got_one:
> > +	dbg("Gained control for hotplug HW for pci %s (%s)\n", pci_name(dev),
> > +			(char *)string.pointer);
> > +	kfree(string.pointer);
> > +	return 0;
> >  }
> >  EXPORT_SYMBOL(acpi_get_hp_hw_control_from_firmware);
> 
> 
> 
> -- 
> Jesse Barnes, Intel Open Source Technology Center
> 

[PATCH 1/1] PCI: add acpi_find_root_bridge_handle

[Jiri Slaby]

Hi,

I'm sending an update proposed by Alex. Consider it and drop or merge :).
--

Consolidate finding of a root bridge and getting its handle to the
one inline function. It's cut&pasted on multiple places. Use this
new inline in those.

Signed-off-by: Jiri Slaby <jirislaby@gmail.com>
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
Cc: Alex Chiang <achiang@hp.com>
Cc: kristen.c.accardi@intel.com
Cc: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>
---
 drivers/pci/hotplug/acpi_pcihp.c   |    5 +----
 drivers/pci/pcie/aer/aerdrv_acpi.c |    7 +------
 include/linux/pci-acpi.h           |   10 ++++++++++
 3 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/drivers/pci/hotplug/acpi_pcihp.c b/drivers/pci/hotplug/acpi_pcihp.c
index bd83197..e17ef54 100644
--- a/drivers/pci/hotplug/acpi_pcihp.c
+++ b/drivers/pci/hotplug/acpi_pcihp.c
@@ -404,10 +404,7 @@ int acpi_get_hp_hw_control_from_firmware(struct pci_dev *dev, u32 flags)
 	 * OSHP within the scope of the hotplug controller and its parents,
 	 * upto the host bridge under which this controller exists.
 	 */
-	while (pdev->bus->self)
-		pdev = pdev->bus->self;
-	handle = acpi_get_pci_rootbridge_handle(pci_domain_nr(pdev->bus),
-			pdev->bus->number);
+	handle = acpi_find_root_bridge_handle(pdev);
 	if (handle) {
 		acpi_get_name(handle, ACPI_FULL_PATHNAME, &string);
 		dbg("Trying to get hotplug control for %s\n",
diff --git a/drivers/pci/pcie/aer/aerdrv_acpi.c b/drivers/pci/pcie/aer/aerdrv_acpi.c
index 30f581b..6dd7b13 100644
--- a/drivers/pci/pcie/aer/aerdrv_acpi.c
+++ b/drivers/pci/pcie/aer/aerdrv_acpi.c
@@ -36,12 +36,7 @@ int aer_osc_setup(struct pcie_device *pciedev)
 	if (acpi_pci_disabled)
 		return -1;
 
-	/* Find root host bridge */
-	while (pdev->bus->self)
-		pdev = pdev->bus->self;
-	handle = acpi_get_pci_rootbridge_handle(
-		pci_domain_nr(pdev->bus), pdev->bus->number);
-
+	handle = acpi_find_root_bridge_handle(pdev);
 	if (handle) {
 		pcie_osc_support_set(OSC_EXT_PCI_CONFIG_SUPPORT);
 		status = pci_osc_control_set(handle,
diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
index 3ba2506..807847a 100644
--- a/include/linux/pci-acpi.h
+++ b/include/linux/pci-acpi.h
@@ -57,6 +57,14 @@ static inline acpi_status pcie_osc_support_set(u32 flags)
 {
 	return __pci_osc_support_set(flags, PCI_EXPRESS_ROOT_HID_STRING);
 }
+static inline acpi_handle acpi_find_root_bridge_handle(struct pci_dev *pdev)
+{
+	while (pdev->bus->self)
+		pdev = pdev->bus->self;
+
+	return acpi_get_pci_rootbridge_handle(pci_domain_nr(pdev->bus),
+			pdev->bus->number);
+}
 #else
 #if !defined(AE_ERROR)
 typedef u32 		acpi_status;
@@ -66,6 +74,8 @@ static inline acpi_status pci_osc_control_set(acpi_handle handle, u32 flags)
 {return AE_ERROR;}
 static inline acpi_status pci_osc_support_set(u32 flags) {return AE_ERROR;} 
 static inline acpi_status pcie_osc_support_set(u32 flags) {return AE_ERROR;}
+static inline acpi_handle acpi_find_root_bridge_handle(struct pci_dev *pdev)
+{ return NULL; }
 #endif
 
 #endif	/* _PCI_ACPI_H_ */
-- 
1.6.0

Re: [PATCH 1/1] PCI: add acpi_find_root_bridge_handle

[Alex Chiang]

* Jiri Slaby <jirislaby@gmail.com>:
> Hi,
> 
> I'm sending an update proposed by Alex. Consider it and drop or merge :).
> --
> 
> Consolidate finding of a root bridge and getting its handle to the
> one inline function. It's cut&pasted on multiple places. Use this
> new inline in those.
> 
> Signed-off-by: Jiri Slaby <jirislaby@gmail.com>
> Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
> Cc: Alex Chiang <achiang@hp.com>
> Cc: kristen.c.accardi@intel.com
> Cc: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>
> ---
>  drivers/pci/hotplug/acpi_pcihp.c   |    5 +----
>  drivers/pci/pcie/aer/aerdrv_acpi.c |    7 +------
>  include/linux/pci-acpi.h           |   10 ++++++++++
>  3 files changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/pci/hotplug/acpi_pcihp.c b/drivers/pci/hotplug/acpi_pcihp.c
> index bd83197..e17ef54 100644
> --- a/drivers/pci/hotplug/acpi_pcihp.c
> +++ b/drivers/pci/hotplug/acpi_pcihp.c
> @@ -404,10 +404,7 @@ int acpi_get_hp_hw_control_from_firmware(struct pci_dev *dev, u32 flags)
>  	 * OSHP within the scope of the hotplug controller and its parents,
>  	 * upto the host bridge under which this controller exists.
>  	 */
> -	while (pdev->bus->self)
> -		pdev = pdev->bus->self;
> -	handle = acpi_get_pci_rootbridge_handle(pci_domain_nr(pdev->bus),
> -			pdev->bus->number);
> +	handle = acpi_find_root_bridge_handle(pdev);
>  	if (handle) {
>  		acpi_get_name(handle, ACPI_FULL_PATHNAME, &string);
>  		dbg("Trying to get hotplug control for %s\n",
> diff --git a/drivers/pci/pcie/aer/aerdrv_acpi.c b/drivers/pci/pcie/aer/aerdrv_acpi.c
> index 30f581b..6dd7b13 100644
> --- a/drivers/pci/pcie/aer/aerdrv_acpi.c
> +++ b/drivers/pci/pcie/aer/aerdrv_acpi.c
> @@ -36,12 +36,7 @@ int aer_osc_setup(struct pcie_device *pciedev)
>  	if (acpi_pci_disabled)
>  		return -1;
>  

The reason I even knew about aer_osc_setup() was because I found
the while (pdev->bus->self) idiom a bit confusing, so I did a git
grep to see if it was used anywhere else in the kernel. Luckily,
the below comment was there, and I found it helpful.

> -	/* Find root host bridge */

So can we move this coment ^^ into your new inline below?

Thanks!

/ac

> -	while (pdev->bus->self)
> -		pdev = pdev->bus->self;
> -	handle = acpi_get_pci_rootbridge_handle(
> -		pci_domain_nr(pdev->bus), pdev->bus->number);
> -
> +	handle = acpi_find_root_bridge_handle(pdev);
>  	if (handle) {
>  		pcie_osc_support_set(OSC_EXT_PCI_CONFIG_SUPPORT);
>  		status = pci_osc_control_set(handle,
> diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
> index 3ba2506..807847a 100644
> --- a/include/linux/pci-acpi.h
> +++ b/include/linux/pci-acpi.h
> @@ -57,6 +57,14 @@ static inline acpi_status pcie_osc_support_set(u32 flags)
>  {
>  	return __pci_osc_support_set(flags, PCI_EXPRESS_ROOT_HID_STRING);
>  }
> +static inline acpi_handle acpi_find_root_bridge_handle(struct pci_dev *pdev)
> +{
> +	while (pdev->bus->self)
> +		pdev = pdev->bus->self;
> +
> +	return acpi_get_pci_rootbridge_handle(pci_domain_nr(pdev->bus),
> +			pdev->bus->number);
> +}
>  #else
>  #if !defined(AE_ERROR)
>  typedef u32 		acpi_status;
> @@ -66,6 +74,8 @@ static inline acpi_status pci_osc_control_set(acpi_handle handle, u32 flags)
>  {return AE_ERROR;}
>  static inline acpi_status pci_osc_support_set(u32 flags) {return AE_ERROR;} 
>  static inline acpi_status pcie_osc_support_set(u32 flags) {return AE_ERROR;}
> +static inline acpi_handle acpi_find_root_bridge_handle(struct pci_dev *pdev)
> +{ return NULL; }
>  #endif
>  
>  #endif	/* _PCI_ACPI_H_ */
> -- 
> 1.6.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

[PATCH 1/1 #2] PCI: add acpi_find_root_bridge_handle

[Jiri Slaby]

Consolidate finding of a root bridge and getting its handle to the
one inline function. It's cut&pasted on multiple places. Use this
new inline in those.

Updates:
- added a comment to the acpi_find_root_bridge_handle

Signed-off-by: Jiri Slaby <jirislaby@gmail.com>
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
Cc: Alex Chiang <achiang@hp.com>
Cc: kristen.c.accardi@intel.com
Cc: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>
---
 drivers/pci/hotplug/acpi_pcihp.c   |    5 +----
 drivers/pci/pcie/aer/aerdrv_acpi.c |    7 +------
 include/linux/pci-acpi.h           |   11 +++++++++++
 3 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/drivers/pci/hotplug/acpi_pcihp.c b/drivers/pci/hotplug/acpi_pcihp.c
index bd83197..e17ef54 100644
--- a/drivers/pci/hotplug/acpi_pcihp.c
+++ b/drivers/pci/hotplug/acpi_pcihp.c
@@ -404,10 +404,7 @@ int acpi_get_hp_hw_control_from_firmware(struct pci_dev *dev, u32 flags)
 	 * OSHP within the scope of the hotplug controller and its parents,
 	 * upto the host bridge under which this controller exists.
 	 */
-	while (pdev->bus->self)
-		pdev = pdev->bus->self;
-	handle = acpi_get_pci_rootbridge_handle(pci_domain_nr(pdev->bus),
-			pdev->bus->number);
+	handle = acpi_find_root_bridge_handle(pdev);
 	if (handle) {
 		acpi_get_name(handle, ACPI_FULL_PATHNAME, &string);
 		dbg("Trying to get hotplug control for %s\n",
diff --git a/drivers/pci/pcie/aer/aerdrv_acpi.c b/drivers/pci/pcie/aer/aerdrv_acpi.c
index 30f581b..6dd7b13 100644
--- a/drivers/pci/pcie/aer/aerdrv_acpi.c
+++ b/drivers/pci/pcie/aer/aerdrv_acpi.c
@@ -36,12 +36,7 @@ int aer_osc_setup(struct pcie_device *pciedev)
 	if (acpi_pci_disabled)
 		return -1;
 
-	/* Find root host bridge */
-	while (pdev->bus->self)
-		pdev = pdev->bus->self;
-	handle = acpi_get_pci_rootbridge_handle(
-		pci_domain_nr(pdev->bus), pdev->bus->number);
-
+	handle = acpi_find_root_bridge_handle(pdev);
 	if (handle) {
 		pcie_osc_support_set(OSC_EXT_PCI_CONFIG_SUPPORT);
 		status = pci_osc_control_set(handle,
diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
index 3ba2506..8837928 100644
--- a/include/linux/pci-acpi.h
+++ b/include/linux/pci-acpi.h
@@ -57,6 +57,15 @@ static inline acpi_status pcie_osc_support_set(u32 flags)
 {
 	return __pci_osc_support_set(flags, PCI_EXPRESS_ROOT_HID_STRING);
 }
+static inline acpi_handle acpi_find_root_bridge_handle(struct pci_dev *pdev)
+{
+	/* Find root host bridge */
+	while (pdev->bus->self)
+		pdev = pdev->bus->self;
+
+	return acpi_get_pci_rootbridge_handle(pci_domain_nr(pdev->bus),
+			pdev->bus->number);
+}
 #else
 #if !defined(AE_ERROR)
 typedef u32 		acpi_status;
@@ -66,6 +75,8 @@ static inline acpi_status pci_osc_control_set(acpi_handle handle, u32 flags)
 {return AE_ERROR;}
 static inline acpi_status pci_osc_support_set(u32 flags) {return AE_ERROR;} 
 static inline acpi_status pcie_osc_support_set(u32 flags) {return AE_ERROR;}
+static inline acpi_handle acpi_find_root_bridge_handle(struct pci_dev *pdev)
+{ return NULL; }
 #endif
 
 #endif	/* _PCI_ACPI_H_ */
-- 
1.5.6.5

Re: [PATCH 1/1 #2] PCI: add acpi_find_root_bridge_handle

[Alex Chiang]

* Jiri Slaby <jirislaby@gmail.com>:
> Consolidate finding of a root bridge and getting its handle to the
> one inline function. It's cut&pasted on multiple places. Use this
> new inline in those.
> 
> Updates:
> - added a comment to the acpi_find_root_bridge_handle

Acked-by: Alex Chiang <achiang@hp.com>

Thanks.

/ac

> 
> Signed-off-by: Jiri Slaby <jirislaby@gmail.com>
> Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
> Cc: Alex Chiang <achiang@hp.com>
> Cc: kristen.c.accardi@intel.com
> Cc: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>
> ---
>  drivers/pci/hotplug/acpi_pcihp.c   |    5 +----
>  drivers/pci/pcie/aer/aerdrv_acpi.c |    7 +------
>  include/linux/pci-acpi.h           |   11 +++++++++++
>  3 files changed, 13 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/pci/hotplug/acpi_pcihp.c b/drivers/pci/hotplug/acpi_pcihp.c
> index bd83197..e17ef54 100644
> --- a/drivers/pci/hotplug/acpi_pcihp.c
> +++ b/drivers/pci/hotplug/acpi_pcihp.c
> @@ -404,10 +404,7 @@ int acpi_get_hp_hw_control_from_firmware(struct pci_dev *dev, u32 flags)
>  	 * OSHP within the scope of the hotplug controller and its parents,
>  	 * upto the host bridge under which this controller exists.
>  	 */
> -	while (pdev->bus->self)
> -		pdev = pdev->bus->self;
> -	handle = acpi_get_pci_rootbridge_handle(pci_domain_nr(pdev->bus),
> -			pdev->bus->number);
> +	handle = acpi_find_root_bridge_handle(pdev);
>  	if (handle) {
>  		acpi_get_name(handle, ACPI_FULL_PATHNAME, &string);
>  		dbg("Trying to get hotplug control for %s\n",
> diff --git a/drivers/pci/pcie/aer/aerdrv_acpi.c b/drivers/pci/pcie/aer/aerdrv_acpi.c
> index 30f581b..6dd7b13 100644
> --- a/drivers/pci/pcie/aer/aerdrv_acpi.c
> +++ b/drivers/pci/pcie/aer/aerdrv_acpi.c
> @@ -36,12 +36,7 @@ int aer_osc_setup(struct pcie_device *pciedev)
>  	if (acpi_pci_disabled)
>  		return -1;
>  
> -	/* Find root host bridge */
> -	while (pdev->bus->self)
> -		pdev = pdev->bus->self;
> -	handle = acpi_get_pci_rootbridge_handle(
> -		pci_domain_nr(pdev->bus), pdev->bus->number);
> -
> +	handle = acpi_find_root_bridge_handle(pdev);
>  	if (handle) {
>  		pcie_osc_support_set(OSC_EXT_PCI_CONFIG_SUPPORT);
>  		status = pci_osc_control_set(handle,
> diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
> index 3ba2506..8837928 100644
> --- a/include/linux/pci-acpi.h
> +++ b/include/linux/pci-acpi.h
> @@ -57,6 +57,15 @@ static inline acpi_status pcie_osc_support_set(u32 flags)
>  {
>  	return __pci_osc_support_set(flags, PCI_EXPRESS_ROOT_HID_STRING);
>  }
> +static inline acpi_handle acpi_find_root_bridge_handle(struct pci_dev *pdev)
> +{
> +	/* Find root host bridge */
> +	while (pdev->bus->self)
> +		pdev = pdev->bus->self;
> +
> +	return acpi_get_pci_rootbridge_handle(pci_domain_nr(pdev->bus),
> +			pdev->bus->number);
> +}
>  #else
>  #if !defined(AE_ERROR)
>  typedef u32 		acpi_status;
> @@ -66,6 +75,8 @@ static inline acpi_status pci_osc_control_set(acpi_handle handle, u32 flags)
>  {return AE_ERROR;}
>  static inline acpi_status pci_osc_support_set(u32 flags) {return AE_ERROR;} 
>  static inline acpi_status pcie_osc_support_set(u32 flags) {return AE_ERROR;}
> +static inline acpi_handle acpi_find_root_bridge_handle(struct pci_dev *pdev)
> +{ return NULL; }
>  #endif
>  
>  #endif	/* _PCI_ACPI_H_ */
> -- 
> 1.5.6.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 1/1] PCI: acpi_pcihp, run _OSC on a root bridge

[Jesse Barnes]

On Monday, August 11, 2008 8:47 am Jiri Slaby wrote:
> _OSC should be ran on a root bridge instead of the device itself.
> Do this before touching OSHP since PCI fw specs states that _OSC
> should be preferred over OSHP (however if the device has OSHP but
> not _OSC -- not a root bridge -- it's not).
>
> Signed-off-by: Jiri Slaby <jirislaby@gmail.com>
> Cc: kristen.c.accardi@intel.com
> Cc: Jesse Barnes <jbarnes@virtuousgeek.org>

Thanks Jiri, I applied both of these to my for-linus branch.
-- 
Jesse Barnes, Intel Open Source Technology Center