[patch] pci-acpi: handle multiple _OSC

[patch] pci-acpi: handle multiple _OSC

[Shaohua Li]

There is an IA64 system here which have two pci root bridges with _OSC.
One _OSC disables SHPC control bit but the other not. Below patch makes
_OSC data per-device instead of one global, otherwise linux takes both
root bridges don't support SHPC

Signed-off-by: Shaohua Li <shaohua.li@intel.com>

diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
index 72f7476..8f17064 100644
--- a/drivers/pci/pci-acpi.c
+++ b/drivers/pci/pci-acpi.c
@@ -19,8 +19,29 @@
 #include <linux/pci-acpi.h>
 #include "pci.h"
 
-static u32 ctrlset_buf[3] = {0, 0, 0};
-static u32 global_ctrlsets = 0;
+#define MAX_ACPI_OSC 30 /* Should be enough */
+struct acpi_osc_data {
+	acpi_handle handle;
+	u32 ctrlset_buf[3];
+	u32 global_ctrlsets;
+} acpi_osc_data_array[MAX_ACPI_OSC];
+
+struct acpi_osc_data *acpi_get_osc_data(acpi_handle handle)
+{
+	int i;
+
+	for (i = 0; i < MAX_ACPI_OSC; i++) {
+		if (acpi_osc_data_array[i].handle == handle)
+			return &acpi_osc_data_array[i];
+		if (acpi_osc_data_array[i].handle == NULL)
+			break;
+	}
+	if (i >= MAX_ACPI_OSC)
+		return NULL;
+	acpi_osc_data_array[i].handle = handle;
+	return &acpi_osc_data_array[i];
+}
+
 static u8 OSC_UUID[16] = {0x5B, 0x4D, 0xDB, 0x33, 0xF7, 0x1F, 0x1C, 0x40, 0x96, 0x57, 0x74, 0x41, 0xC0, 0x3D, 0xD7, 0x66};
 
 static acpi_status  
@@ -37,8 +58,21 @@ acpi_query_osc (
 	union acpi_object 	*out_obj;
 	u32			osc_dw0;
 	acpi_status *ret_status = (acpi_status *)retval;
+	struct acpi_osc_data *osc_data = acpi_get_osc_data(handle);
+	u32 flags = (unsigned long)context, temp;
+
+	if (!osc_data) {
+		printk(KERN_ERR "acpi osc data array is full\n");
+		return AE_ERROR;
+	}
+
+	osc_data->ctrlset_buf[OSC_SUPPORT_TYPE] |= (flags & OSC_SUPPORT_MASKS);
+
+	/* do _OSC query for all possible controls */
+	temp = osc_data->ctrlset_buf[OSC_CONTROL_TYPE];
+	osc_data->ctrlset_buf[OSC_QUERY_TYPE] = OSC_QUERY_ENABLE;
+	osc_data->ctrlset_buf[OSC_CONTROL_TYPE] = OSC_CONTROL_MASKS;
 
-	
 	/* Setting up input parameters */
 	input.count = 4;
 	input.pointer = in_params;
@@ -51,13 +85,11 @@ acpi_query_osc (
 	in_params[2].integer.value	= 3;
 	in_params[3].type		= ACPI_TYPE_BUFFER;
 	in_params[3].buffer.length 	= 12;
-	in_params[3].buffer.pointer 	= (u8 *)context;
+	in_params[3].buffer.pointer 	= (u8 *)osc_data->ctrlset_buf;
 
 	status = acpi_evaluate_object(handle, "_OSC", &input, &output);
-	if (ACPI_FAILURE (status)) {
-		*ret_status = status;
-		return status;
-	}
+	if (ACPI_FAILURE (status))
+		goto out_nofree;
 	out_obj = output.pointer;
 
 	if (out_obj->type != ACPI_TYPE_BUFFER) {
@@ -76,7 +108,7 @@ acpi_query_osc (
 			printk(KERN_DEBUG "_OSC invalid revision\n"); 
 		if (osc_dw0 & OSC_CAPABILITIES_MASK_ERROR) {
 			/* Update Global Control Set */
-			global_ctrlsets = *((u32 *)(out_obj->buffer.pointer+8));
+			osc_data->global_ctrlsets = *((u32 *)(out_obj->buffer.pointer+8));
 			status = AE_OK;
 			goto query_osc_out;
 		}
@@ -85,12 +117,21 @@ acpi_query_osc (
 	}
 
 	/* Update Global Control Set */
-	global_ctrlsets = *((u32 *)(out_obj->buffer.pointer + 8));
+	osc_data->global_ctrlsets = *((u32 *)(out_obj->buffer.pointer + 8));
 	status = AE_OK;
 
 query_osc_out:
 	kfree(output.pointer);
+out_nofree:
 	*ret_status = status;
+
+	osc_data->ctrlset_buf[OSC_QUERY_TYPE] = !OSC_QUERY_ENABLE;
+	osc_data->ctrlset_buf[OSC_CONTROL_TYPE] = temp;
+	if (ACPI_FAILURE(status)) {
+		/* no osc support at all */
+		osc_data->ctrlset_buf[OSC_SUPPORT_TYPE] = 0;
+	}
+
 	return status;
 }
 
@@ -165,28 +206,15 @@ run_osc_out:
  **/
 acpi_status __pci_osc_support_set(u32 flags, const char *hid)
 {
-	u32 temp;
 	acpi_status retval;
 
 	if (!(flags & OSC_SUPPORT_MASKS)) {
 		return AE_TYPE;
 	}
-	ctrlset_buf[OSC_SUPPORT_TYPE] |= (flags & OSC_SUPPORT_MASKS);
-
-	/* do _OSC query for all possible controls */
-	temp = ctrlset_buf[OSC_CONTROL_TYPE];
-	ctrlset_buf[OSC_QUERY_TYPE] = OSC_QUERY_ENABLE;
-	ctrlset_buf[OSC_CONTROL_TYPE] = OSC_CONTROL_MASKS;
 	acpi_get_devices(hid,
 			acpi_query_osc,
-			ctrlset_buf,
+			(void *)(unsigned long)flags,
 			(void **) &retval );
-	ctrlset_buf[OSC_QUERY_TYPE] = !OSC_QUERY_ENABLE;
-	ctrlset_buf[OSC_CONTROL_TYPE] = temp;
-	if (ACPI_FAILURE(retval)) {
-		/* no osc support at all */
-		ctrlset_buf[OSC_SUPPORT_TYPE] = 0;
-	}
 	return AE_OK;
 }
 
@@ -201,19 +229,25 @@ acpi_status pci_osc_control_set(acpi_handle handle, u32 flags)
 {
 	acpi_status	status;
 	u32		ctrlset;
+	struct acpi_osc_data *osc_data = acpi_get_osc_data(handle);
+
+	if (!osc_data) {
+		printk(KERN_ERR "acpi osc data array is full\n");
+		return AE_ERROR;
+	}
 
 	ctrlset = (flags & OSC_CONTROL_MASKS);
 	if (!ctrlset) {
 		return AE_TYPE;
 	}
-	if (ctrlset_buf[OSC_SUPPORT_TYPE] && 
-	 	((global_ctrlsets & ctrlset) != ctrlset)) {
+	if (osc_data->ctrlset_buf[OSC_SUPPORT_TYPE] && 
+	 	((osc_data->global_ctrlsets & ctrlset) != ctrlset)) {
 		return AE_SUPPORT;
 	}
-	ctrlset_buf[OSC_CONTROL_TYPE] |= ctrlset;
-	status = acpi_run_osc(handle, ctrlset_buf);
+	osc_data->ctrlset_buf[OSC_CONTROL_TYPE] |= ctrlset;
+	status = acpi_run_osc(handle, osc_data->ctrlset_buf);
 	if (ACPI_FAILURE (status)) {
-		ctrlset_buf[OSC_CONTROL_TYPE] &= ~ctrlset;
+		osc_data->ctrlset_buf[OSC_CONTROL_TYPE] &= ~ctrlset;
 	}
 	
 	return status;

Re: [patch] pci-acpi: handle multiple _OSC

[Kenji Kaneshige]

Shaohua Li wrote:
> There is an IA64 system here which have two pci root bridges with _OSC.
> One _OSC disables SHPC control bit but the other not. Below patch makes
> _OSC data per-device instead of one global, otherwise linux takes both
> root bridges don't support SHPC
> 
> Signed-off-by: Shaohua Li <shaohua.li@intel.com>
> 
> diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
> index 72f7476..8f17064 100644
> --- a/drivers/pci/pci-acpi.c
> +++ b/drivers/pci/pci-acpi.c
> @@ -19,8 +19,29 @@
>  #include <linux/pci-acpi.h>
>  #include "pci.h"
>  
> -static u32 ctrlset_buf[3] = {0, 0, 0};
> -static u32 global_ctrlsets = 0;
> +#define MAX_ACPI_OSC 30 /* Should be enough */
> +struct acpi_osc_data {
> +	acpi_handle handle;
> +	u32 ctrlset_buf[3];
> +	u32 global_ctrlsets;
> +} acpi_osc_data_array[MAX_ACPI_OSC];
> +
> +struct acpi_osc_data *acpi_get_osc_data(acpi_handle handle)
> +{

One nit pick.
I think this should be static.

Thanks,
Kenji Kaneshige

Re: [patch] pci-acpi: handle multiple _OSC

[Shaohua Li]

On Thu, 2008-05-08 at 21:54 +0900, Kenji Kaneshige wrote:
> Shaohua Li wrote:
> > There is an IA64 system here which have two pci root bridges with _OSC.
> > One _OSC disables SHPC control bit but the other not. Below patch makes
> > _OSC data per-device instead of one global, otherwise linux takes both
> > root bridges don't support SHPC
> > 
> > Signed-off-by: Shaohua Li <shaohua.li@intel.com>
> > 
> > diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
> > index 72f7476..8f17064 100644
> > --- a/drivers/pci/pci-acpi.c
> > +++ b/drivers/pci/pci-acpi.c
> > @@ -19,8 +19,29 @@
> >  #include <linux/pci-acpi.h>
> >  #include "pci.h"
> >  
> > -static u32 ctrlset_buf[3] = {0, 0, 0};
> > -static u32 global_ctrlsets = 0;
> > +#define MAX_ACPI_OSC 30 /* Should be enough */
> > +struct acpi_osc_data {
> > +	acpi_handle handle;
> > +	u32 ctrlset_buf[3];
> > +	u32 global_ctrlsets;
> > +} acpi_osc_data_array[MAX_ACPI_OSC];
> > +
> > +struct acpi_osc_data *acpi_get_osc_data(acpi_handle handle)
> > +{
> 
> One nit pick.
> I think this should be static.
Thanks, fixed.

There is an IA64 system here which have two pci root bridges with _OSC.
One _OSC disables SHPC control bit but the other not. Below patch makes
_OSC data per-device instead of one global, otherwise linux takes both
root bridges don't support SHPC.

Signed-off-by: Shaohua Li <shaohua.li@intel.com>

diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
index 72f7476..3fe0ccb 100644
--- a/drivers/pci/pci-acpi.c
+++ b/drivers/pci/pci-acpi.c
@@ -19,8 +19,29 @@
 #include <linux/pci-acpi.h>
 #include "pci.h"
 
-static u32 ctrlset_buf[3] = {0, 0, 0};
-static u32 global_ctrlsets = 0;
+#define MAX_ACPI_OSC 30 /* Should be enough */
+static struct acpi_osc_data {
+	acpi_handle handle;
+	u32 ctrlset_buf[3];
+	u32 global_ctrlsets;
+} acpi_osc_data_array[MAX_ACPI_OSC];
+
+static struct acpi_osc_data *acpi_get_osc_data(acpi_handle handle)
+{
+	int i;
+
+	for (i = 0; i < MAX_ACPI_OSC; i++) {
+		if (acpi_osc_data_array[i].handle == handle)
+			return &acpi_osc_data_array[i];
+		if (acpi_osc_data_array[i].handle == NULL)
+			break;
+	}
+	if (i >= MAX_ACPI_OSC)
+		return NULL;
+	acpi_osc_data_array[i].handle = handle;
+	return &acpi_osc_data_array[i];
+}
+
 static u8 OSC_UUID[16] = {0x5B, 0x4D, 0xDB, 0x33, 0xF7, 0x1F, 0x1C, 0x40, 0x96, 0x57, 0x74, 0x41, 0xC0, 0x3D, 0xD7, 0x66};
 
 static acpi_status  
@@ -37,8 +58,21 @@ acpi_query_osc (
 	union acpi_object 	*out_obj;
 	u32			osc_dw0;
 	acpi_status *ret_status = (acpi_status *)retval;
+	struct acpi_osc_data *osc_data = acpi_get_osc_data(handle);
+	u32 flags = (unsigned long)context, temp;
+
+	if (!osc_data) {
+		printk(KERN_ERR "acpi osc data array is full\n");
+		return AE_ERROR;
+	}
+
+	osc_data->ctrlset_buf[OSC_SUPPORT_TYPE] |= (flags & OSC_SUPPORT_MASKS);
+
+	/* do _OSC query for all possible controls */
+	temp = osc_data->ctrlset_buf[OSC_CONTROL_TYPE];
+	osc_data->ctrlset_buf[OSC_QUERY_TYPE] = OSC_QUERY_ENABLE;
+	osc_data->ctrlset_buf[OSC_CONTROL_TYPE] = OSC_CONTROL_MASKS;
 
-	
 	/* Setting up input parameters */
 	input.count = 4;
 	input.pointer = in_params;
@@ -51,13 +85,11 @@ acpi_query_osc (
 	in_params[2].integer.value	= 3;
 	in_params[3].type		= ACPI_TYPE_BUFFER;
 	in_params[3].buffer.length 	= 12;
-	in_params[3].buffer.pointer 	= (u8 *)context;
+	in_params[3].buffer.pointer 	= (u8 *)osc_data->ctrlset_buf;
 
 	status = acpi_evaluate_object(handle, "_OSC", &input, &output);
-	if (ACPI_FAILURE (status)) {
-		*ret_status = status;
-		return status;
-	}
+	if (ACPI_FAILURE (status))
+		goto out_nofree;
 	out_obj = output.pointer;
 
 	if (out_obj->type != ACPI_TYPE_BUFFER) {
@@ -76,7 +108,7 @@ acpi_query_osc (
 			printk(KERN_DEBUG "_OSC invalid revision\n"); 
 		if (osc_dw0 & OSC_CAPABILITIES_MASK_ERROR) {
 			/* Update Global Control Set */
-			global_ctrlsets = *((u32 *)(out_obj->buffer.pointer+8));
+			osc_data->global_ctrlsets = *((u32 *)(out_obj->buffer.pointer+8));
 			status = AE_OK;
 			goto query_osc_out;
 		}
@@ -85,12 +117,21 @@ acpi_query_osc (
 	}
 
 	/* Update Global Control Set */
-	global_ctrlsets = *((u32 *)(out_obj->buffer.pointer + 8));
+	osc_data->global_ctrlsets = *((u32 *)(out_obj->buffer.pointer + 8));
 	status = AE_OK;
 
 query_osc_out:
 	kfree(output.pointer);
+out_nofree:
 	*ret_status = status;
+
+	osc_data->ctrlset_buf[OSC_QUERY_TYPE] = !OSC_QUERY_ENABLE;
+	osc_data->ctrlset_buf[OSC_CONTROL_TYPE] = temp;
+	if (ACPI_FAILURE(status)) {
+		/* no osc support at all */
+		osc_data->ctrlset_buf[OSC_SUPPORT_TYPE] = 0;
+	}
+
 	return status;
 }
 
@@ -165,28 +206,15 @@ run_osc_out:
  **/
 acpi_status __pci_osc_support_set(u32 flags, const char *hid)
 {
-	u32 temp;
 	acpi_status retval;
 
 	if (!(flags & OSC_SUPPORT_MASKS)) {
 		return AE_TYPE;
 	}
-	ctrlset_buf[OSC_SUPPORT_TYPE] |= (flags & OSC_SUPPORT_MASKS);
-
-	/* do _OSC query for all possible controls */
-	temp = ctrlset_buf[OSC_CONTROL_TYPE];
-	ctrlset_buf[OSC_QUERY_TYPE] = OSC_QUERY_ENABLE;
-	ctrlset_buf[OSC_CONTROL_TYPE] = OSC_CONTROL_MASKS;
 	acpi_get_devices(hid,
 			acpi_query_osc,
-			ctrlset_buf,
+			(void *)(unsigned long)flags,
 			(void **) &retval );
-	ctrlset_buf[OSC_QUERY_TYPE] = !OSC_QUERY_ENABLE;
-	ctrlset_buf[OSC_CONTROL_TYPE] = temp;
-	if (ACPI_FAILURE(retval)) {
-		/* no osc support at all */
-		ctrlset_buf[OSC_SUPPORT_TYPE] = 0;
-	}
 	return AE_OK;
 }
 
@@ -201,19 +229,25 @@ acpi_status pci_osc_control_set(acpi_handle handle, u32 flags)
 {
 	acpi_status	status;
 	u32		ctrlset;
+	struct acpi_osc_data *osc_data = acpi_get_osc_data(handle);
+
+	if (!osc_data) {
+		printk(KERN_ERR "acpi osc data array is full\n");
+		return AE_ERROR;
+	}
 
 	ctrlset = (flags & OSC_CONTROL_MASKS);
 	if (!ctrlset) {
 		return AE_TYPE;
 	}
-	if (ctrlset_buf[OSC_SUPPORT_TYPE] && 
-	 	((global_ctrlsets & ctrlset) != ctrlset)) {
+	if (osc_data->ctrlset_buf[OSC_SUPPORT_TYPE] && 
+	 	((osc_data->global_ctrlsets & ctrlset) != ctrlset)) {
 		return AE_SUPPORT;
 	}
-	ctrlset_buf[OSC_CONTROL_TYPE] |= ctrlset;
-	status = acpi_run_osc(handle, ctrlset_buf);
+	osc_data->ctrlset_buf[OSC_CONTROL_TYPE] |= ctrlset;
+	status = acpi_run_osc(handle, osc_data->ctrlset_buf);
 	if (ACPI_FAILURE (status)) {
-		ctrlset_buf[OSC_CONTROL_TYPE] &= ~ctrlset;
+		osc_data->ctrlset_buf[OSC_CONTROL_TYPE] &= ~ctrlset;
 	}
 	
 	return status;



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

Re: [patch] pci-acpi: handle multiple _OSC

[Kenji Kaneshige]

Hi,

One more thing, sorry.

+	if (osc_data->ctrlset_buf[OSC_SUPPORT_TYPE] && 

Trailing white space here.

Your patch is conflicting with the patch "PCI ACPI: fix uninitialized variable
in __pci_osc_support_set" that is currently in Jesse's tree. I'm attaching the
fixed one that removes the conflict. It also removes the trailing white space.
Please use it if necessary.

Thanks,
Kenji Kaneshige


There is an IA64 system here which have two pci root bridges with _OSC.
One _OSC disables SHPC control bit but the other not. Below patch makes
_OSC data per-device instead of one global, otherwise linux takes both
root bridges don't support SHPC.

Signed-off-by: Shaohua Li <shaohua.li@intel.com>

---
 drivers/pci/pci-acpi.c |   92 +++++++++++++++++++++++++++++++++----------------
 1 file changed, 63 insertions(+), 29 deletions(-)

Index: linux-2.6.26-rc1/drivers/pci/pci-acpi.c
===================================================================
--- linux-2.6.26-rc1.orig/drivers/pci/pci-acpi.c
+++ linux-2.6.26-rc1/drivers/pci/pci-acpi.c
@@ -19,8 +19,29 @@
 #include <linux/pci-acpi.h>
 #include "pci.h"
 
-static u32 ctrlset_buf[3] = {0, 0, 0};
-static u32 global_ctrlsets = 0;
+#define MAX_ACPI_OSC 30 /* Should be enough */
+static struct acpi_osc_data {
+	acpi_handle handle;
+	u32 ctrlset_buf[3];
+	u32 global_ctrlsets;
+} acpi_osc_data_array[MAX_ACPI_OSC];
+
+static struct acpi_osc_data *acpi_get_osc_data(acpi_handle handle)
+{
+	int i;
+
+	for (i = 0; i < MAX_ACPI_OSC; i++) {
+		if (acpi_osc_data_array[i].handle == handle)
+			return &acpi_osc_data_array[i];
+		if (acpi_osc_data_array[i].handle == NULL)
+			break;
+	}
+	if (i >= MAX_ACPI_OSC)
+		return NULL;
+	acpi_osc_data_array[i].handle = handle;
+	return &acpi_osc_data_array[i];
+}
+
 static u8 OSC_UUID[16] = {0x5B, 0x4D, 0xDB, 0x33, 0xF7, 0x1F, 0x1C, 0x40, 0x96, 0x57, 0x74, 0x41, 0xC0, 0x3D, 0xD7, 0x66};
 
 static acpi_status  
@@ -37,8 +58,21 @@ acpi_query_osc (
 	union acpi_object 	*out_obj;
 	u32			osc_dw0;
 	acpi_status *ret_status = (acpi_status *)retval;
+	struct acpi_osc_data *osc_data = acpi_get_osc_data(handle);
+	u32 flags = (unsigned long)context, temp;
+
+	if (!osc_data) {
+		printk(KERN_ERR "acpi osc data array is full\n");
+		return AE_ERROR;
+	}
+
+	osc_data->ctrlset_buf[OSC_SUPPORT_TYPE] |= (flags & OSC_SUPPORT_MASKS);
+
+	/* do _OSC query for all possible controls */
+	temp = osc_data->ctrlset_buf[OSC_CONTROL_TYPE];
+	osc_data->ctrlset_buf[OSC_QUERY_TYPE] = OSC_QUERY_ENABLE;
+	osc_data->ctrlset_buf[OSC_CONTROL_TYPE] = OSC_CONTROL_MASKS;
 
-	
 	/* Setting up input parameters */
 	input.count = 4;
 	input.pointer = in_params;
@@ -51,13 +85,11 @@ acpi_query_osc (
 	in_params[2].integer.value	= 3;
 	in_params[3].type		= ACPI_TYPE_BUFFER;
 	in_params[3].buffer.length 	= 12;
-	in_params[3].buffer.pointer 	= (u8 *)context;
+	in_params[3].buffer.pointer 	= (u8 *)osc_data->ctrlset_buf;
 
 	status = acpi_evaluate_object(handle, "_OSC", &input, &output);
-	if (ACPI_FAILURE (status)) {
-		*ret_status = status;
-		return status;
-	}
+	if (ACPI_FAILURE (status))
+		goto out_nofree;
 	out_obj = output.pointer;
 
 	if (out_obj->type != ACPI_TYPE_BUFFER) {
@@ -76,7 +108,7 @@ acpi_query_osc (
 			printk(KERN_DEBUG "_OSC invalid revision\n"); 
 		if (osc_dw0 & OSC_CAPABILITIES_MASK_ERROR) {
 			/* Update Global Control Set */
-			global_ctrlsets = *((u32 *)(out_obj->buffer.pointer+8));
+			osc_data->global_ctrlsets = *((u32 *)(out_obj->buffer.pointer+8));
 			status = AE_OK;
 			goto query_osc_out;
 		}
@@ -85,12 +117,21 @@ acpi_query_osc (
 	}
 
 	/* Update Global Control Set */
-	global_ctrlsets = *((u32 *)(out_obj->buffer.pointer + 8));
+	osc_data->global_ctrlsets = *((u32 *)(out_obj->buffer.pointer + 8));
 	status = AE_OK;
 
 query_osc_out:
 	kfree(output.pointer);
+out_nofree:
 	*ret_status = status;
+
+	osc_data->ctrlset_buf[OSC_QUERY_TYPE] = !OSC_QUERY_ENABLE;
+	osc_data->ctrlset_buf[OSC_CONTROL_TYPE] = temp;
+	if (ACPI_FAILURE(status)) {
+		/* no osc support at all */
+		osc_data->ctrlset_buf[OSC_SUPPORT_TYPE] = 0;
+	}
+
 	return status;
 }
 
@@ -165,28 +206,15 @@ run_osc_out:
  **/
 acpi_status __pci_osc_support_set(u32 flags, const char *hid)
 {
-	u32 temp;
 	acpi_status retval = AE_NOT_FOUND;
 
 	if (!(flags & OSC_SUPPORT_MASKS)) {
 		return AE_TYPE;
 	}
-	ctrlset_buf[OSC_SUPPORT_TYPE] |= (flags & OSC_SUPPORT_MASKS);
-
-	/* do _OSC query for all possible controls */
-	temp = ctrlset_buf[OSC_CONTROL_TYPE];
-	ctrlset_buf[OSC_QUERY_TYPE] = OSC_QUERY_ENABLE;
-	ctrlset_buf[OSC_CONTROL_TYPE] = OSC_CONTROL_MASKS;
 	acpi_get_devices(hid,
 			acpi_query_osc,
-			ctrlset_buf,
+			(void *)(unsigned long)flags,
 			(void **) &retval );
-	ctrlset_buf[OSC_QUERY_TYPE] = !OSC_QUERY_ENABLE;
-	ctrlset_buf[OSC_CONTROL_TYPE] = temp;
-	if (ACPI_FAILURE(retval)) {
-		/* no osc support at all */
-		ctrlset_buf[OSC_SUPPORT_TYPE] = 0;
-	}
 	return AE_OK;
 }
 
@@ -201,19 +229,25 @@ acpi_status pci_osc_control_set(acpi_han
 {
 	acpi_status	status;
 	u32		ctrlset;
+	struct acpi_osc_data *osc_data = acpi_get_osc_data(handle);
+
+	if (!osc_data) {
+		printk(KERN_ERR "acpi osc data array is full\n");
+		return AE_ERROR;
+	}
 
 	ctrlset = (flags & OSC_CONTROL_MASKS);
 	if (!ctrlset) {
 		return AE_TYPE;
 	}
-	if (ctrlset_buf[OSC_SUPPORT_TYPE] && 
-	 	((global_ctrlsets & ctrlset) != ctrlset)) {
+	if (osc_data->ctrlset_buf[OSC_SUPPORT_TYPE] &&
+	 	((osc_data->global_ctrlsets & ctrlset) != ctrlset)) {
 		return AE_SUPPORT;
 	}
-	ctrlset_buf[OSC_CONTROL_TYPE] |= ctrlset;
-	status = acpi_run_osc(handle, ctrlset_buf);
+	osc_data->ctrlset_buf[OSC_CONTROL_TYPE] |= ctrlset;
+	status = acpi_run_osc(handle, osc_data->ctrlset_buf);
 	if (ACPI_FAILURE (status)) {
-		ctrlset_buf[OSC_CONTROL_TYPE] &= ~ctrlset;
+		osc_data->ctrlset_buf[OSC_CONTROL_TYPE] &= ~ctrlset;
 	}
 	
 	return status;

Re: [patch] pci-acpi: handle multiple _OSC

[Jesse Barnes]

On Friday, May 09, 2008 12:43 am Kenji Kaneshige wrote:
> -static u32 ctrlset_buf[3] = {0, 0, 0};
> -static u32 global_ctrlsets = 0;
> +#define MAX_ACPI_OSC 30 /* Should be enough */
> +static struct acpi_osc_data {
> +	acpi_handle handle;
> +	u32 ctrlset_buf[3];
> +	u32 global_ctrlsets;
> +} acpi_osc_data_array[MAX_ACPI_OSC];

Could this just be a linked list of OSC objects instead?

Jesse

Re: [patch] pci-acpi: handle multiple _OSC

[Shaohua Li]

On Fri, 2008-05-09 at 10:40 -0700, Jesse Barnes wrote:
> On Friday, May 09, 2008 12:43 am Kenji Kaneshige wrote:
> > -static u32 ctrlset_buf[3] = {0, 0, 0};
> > -static u32 global_ctrlsets = 0;
> > +#define MAX_ACPI_OSC 30 /* Should be enough */
> > +static struct acpi_osc_data {
> > +	acpi_handle handle;
> > +	u32 ctrlset_buf[3];
> > +	u32 global_ctrlsets;
> > +} acpi_osc_data_array[MAX_ACPI_OSC];
> 
> Could this just be a linked list of OSC objects instead?
fixed. patch is against Kenji's "PCI ACPI: fix uninitialized variable
in __pci_osc_support_set" patch.

There is an IA64 system here which have two pci root bridges with _OSC.
One _OSC disables SHPC control bit but the other not. Below patch makes
_OSC data per-device instead of one global, otherwise linux takes both
root bridges don't support SHPC.

Signed-off-by: Shaohua Li <shaohua.li@intel.com>
---
 drivers/pci/pci-acpi.c |   95 ++++++++++++++++++++++++++++++++++---------------
 1 file changed, 66 insertions(+), 29 deletions(-)

Index: linux/drivers/pci/pci-acpi.c
===================================================================
--- linux.orig/drivers/pci/pci-acpi.c	2008-05-12 10:16:36.000000000 +0800
+++ linux/drivers/pci/pci-acpi.c	2008-05-12 10:43:46.000000000 +0800
@@ -19,8 +19,31 @@
 #include <linux/pci-acpi.h>
 #include "pci.h"
 
-static u32 ctrlset_buf[3] = {0, 0, 0};
-static u32 global_ctrlsets = 0;
+struct acpi_osc_data {
+	acpi_handle handle;
+	u32 ctrlset_buf[3];
+	u32 global_ctrlsets;
+	struct list_head sibiling;
+};
+static LIST_HEAD(acpi_osc_data_list);
+
+static struct acpi_osc_data *acpi_get_osc_data(acpi_handle handle)
+{
+	struct acpi_osc_data *data;
+
+	list_for_each_entry(data, &acpi_osc_data_list, sibiling) {
+		if (data->handle == handle)
+			return data;
+	}
+	data = kzalloc(sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return NULL;
+	INIT_LIST_HEAD(&data->sibiling);
+	data->handle = handle;
+	list_add_tail(&data->sibiling, &acpi_osc_data_list);
+	return data;
+}
+
 static u8 OSC_UUID[16] = {0x5B, 0x4D, 0xDB, 0x33, 0xF7, 0x1F, 0x1C, 0x40, 0x96, 0x57, 0x74, 0x41, 0xC0, 0x3D, 0xD7, 0x66};
 
 static acpi_status  
@@ -37,8 +60,21 @@ acpi_query_osc (
 	union acpi_object 	*out_obj;
 	u32			osc_dw0;
 	acpi_status *ret_status = (acpi_status *)retval;
+	struct acpi_osc_data *osc_data = acpi_get_osc_data(handle);
+	u32 flags = (unsigned long)context, temp;
+
+	if (!osc_data) {
+		printk(KERN_ERR "acpi osc data array is full\n");
+		return AE_ERROR;
+	}
+
+	osc_data->ctrlset_buf[OSC_SUPPORT_TYPE] |= (flags & OSC_SUPPORT_MASKS);
+
+	/* do _OSC query for all possible controls */
+	temp = osc_data->ctrlset_buf[OSC_CONTROL_TYPE];
+	osc_data->ctrlset_buf[OSC_QUERY_TYPE] = OSC_QUERY_ENABLE;
+	osc_data->ctrlset_buf[OSC_CONTROL_TYPE] = OSC_CONTROL_MASKS;
 
-	
 	/* Setting up input parameters */
 	input.count = 4;
 	input.pointer = in_params;
@@ -51,13 +87,11 @@ acpi_query_osc (
 	in_params[2].integer.value	= 3;
 	in_params[3].type		= ACPI_TYPE_BUFFER;
 	in_params[3].buffer.length 	= 12;
-	in_params[3].buffer.pointer 	= (u8 *)context;
+	in_params[3].buffer.pointer 	= (u8 *)osc_data->ctrlset_buf;
 
 	status = acpi_evaluate_object(handle, "_OSC", &input, &output);
-	if (ACPI_FAILURE (status)) {
-		*ret_status = status;
-		return status;
-	}
+	if (ACPI_FAILURE(status))
+		goto out_nofree;
 	out_obj = output.pointer;
 
 	if (out_obj->type != ACPI_TYPE_BUFFER) {
@@ -76,7 +110,8 @@ acpi_query_osc (
 			printk(KERN_DEBUG "_OSC invalid revision\n"); 
 		if (osc_dw0 & OSC_CAPABILITIES_MASK_ERROR) {
 			/* Update Global Control Set */
-			global_ctrlsets = *((u32 *)(out_obj->buffer.pointer+8));
+			osc_data->global_ctrlsets =
+				*((u32 *)(out_obj->buffer.pointer + 8));
 			status = AE_OK;
 			goto query_osc_out;
 		}
@@ -85,12 +120,21 @@ acpi_query_osc (
 	}
 
 	/* Update Global Control Set */
-	global_ctrlsets = *((u32 *)(out_obj->buffer.pointer + 8));
+	osc_data->global_ctrlsets = *((u32 *)(out_obj->buffer.pointer + 8));
 	status = AE_OK;
 
 query_osc_out:
 	kfree(output.pointer);
+out_nofree:
 	*ret_status = status;
+
+	osc_data->ctrlset_buf[OSC_QUERY_TYPE] = !OSC_QUERY_ENABLE;
+	osc_data->ctrlset_buf[OSC_CONTROL_TYPE] = temp;
+	if (ACPI_FAILURE(status)) {
+		/* no osc support at all */
+		osc_data->ctrlset_buf[OSC_SUPPORT_TYPE] = 0;
+	}
+
 	return status;
 }
 
@@ -165,28 +209,15 @@ run_osc_out:
  **/
 acpi_status __pci_osc_support_set(u32 flags, const char *hid)
 {
-	u32 temp;
 	acpi_status retval = AE_NOT_FOUND;
 
 	if (!(flags & OSC_SUPPORT_MASKS)) {
 		return AE_TYPE;
 	}
-	ctrlset_buf[OSC_SUPPORT_TYPE] |= (flags & OSC_SUPPORT_MASKS);
-
-	/* do _OSC query for all possible controls */
-	temp = ctrlset_buf[OSC_CONTROL_TYPE];
-	ctrlset_buf[OSC_QUERY_TYPE] = OSC_QUERY_ENABLE;
-	ctrlset_buf[OSC_CONTROL_TYPE] = OSC_CONTROL_MASKS;
 	acpi_get_devices(hid,
 			acpi_query_osc,
-			ctrlset_buf,
+			(void *)(unsigned long)flags,
 			(void **) &retval );
-	ctrlset_buf[OSC_QUERY_TYPE] = !OSC_QUERY_ENABLE;
-	ctrlset_buf[OSC_CONTROL_TYPE] = temp;
-	if (ACPI_FAILURE(retval)) {
-		/* no osc support at all */
-		ctrlset_buf[OSC_SUPPORT_TYPE] = 0;
-	}
 	return AE_OK;
 }
 
@@ -201,19 +232,25 @@ acpi_status pci_osc_control_set(acpi_han
 {
 	acpi_status	status;
 	u32		ctrlset;
+	struct acpi_osc_data *osc_data = acpi_get_osc_data(handle);
+
+	if (!osc_data) {
+		printk(KERN_ERR "acpi osc data array is full\n");
+		return AE_ERROR;
+	}
 
 	ctrlset = (flags & OSC_CONTROL_MASKS);
 	if (!ctrlset) {
 		return AE_TYPE;
 	}
-	if (ctrlset_buf[OSC_SUPPORT_TYPE] && 
-	 	((global_ctrlsets & ctrlset) != ctrlset)) {
+	if (osc_data->ctrlset_buf[OSC_SUPPORT_TYPE] &&
+		((osc_data->global_ctrlsets & ctrlset) != ctrlset)) {
 		return AE_SUPPORT;
 	}
-	ctrlset_buf[OSC_CONTROL_TYPE] |= ctrlset;
-	status = acpi_run_osc(handle, ctrlset_buf);
+	osc_data->ctrlset_buf[OSC_CONTROL_TYPE] |= ctrlset;
+	status = acpi_run_osc(handle, osc_data->ctrlset_buf);
 	if (ACPI_FAILURE (status)) {
-		ctrlset_buf[OSC_CONTROL_TYPE] &= ~ctrlset;
+		osc_data->ctrlset_buf[OSC_CONTROL_TYPE] &= ~ctrlset;
 	}
 	
 	return status;


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

Re: [patch] pci-acpi: handle multiple _OSC

[Kenji Kaneshige]

Shaohua Li wrote:
> On Fri, 2008-05-09 at 10:40 -0700, Jesse Barnes wrote:
>> On Friday, May 09, 2008 12:43 am Kenji Kaneshige wrote:
>>> -static u32 ctrlset_buf[3] = {0, 0, 0};
>>> -static u32 global_ctrlsets = 0;
>>> +#define MAX_ACPI_OSC 30 /* Should be enough */
>>> +static struct acpi_osc_data {
>>> +	acpi_handle handle;
>>> +	u32 ctrlset_buf[3];
>>> +	u32 global_ctrlsets;
>>> +} acpi_osc_data_array[MAX_ACPI_OSC];
>> Could this just be a linked list of OSC objects instead?
> fixed. patch is against Kenji's ?"PCI ACPI: fix uninitialized variable
> in __pci_osc_support_set" patch.
> 

I found a problem.

> @@ -201,19 +232,25 @@ acpi_status pci_osc_control_set(acpi_han
>  {
>  	acpi_status	status;
>  	u32		ctrlset;
> +	struct acpi_osc_data *osc_data = acpi_get_osc_data(handle);
> +
> +	if (!osc_data) {
> +		printk(KERN_ERR "acpi osc data array is full\n");
> +		return AE_ERROR;
> +	}

The pci_osc_control_set() function can be called for the ACPI object
that doesn't have _OSC method. In this case, acpi_get_osc_data() would
allocate a useless memory region. To avoid this, we need to check the
existence of _OSC before calling acpi_get_osc_data(). Here is a patch
to fix this problem. It is against your patch.

---
 drivers/pci/pci-acpi.c |    8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Index: linux-2.6.26-rc2/drivers/pci/pci-acpi.c
===================================================================
--- linux-2.6.26-rc2.orig/drivers/pci/pci-acpi.c
+++ linux-2.6.26-rc2/drivers/pci/pci-acpi.c
@@ -232,8 +232,14 @@ acpi_status pci_osc_control_set(acpi_han
 {
 	acpi_status	status;
 	u32		ctrlset;
-	struct acpi_osc_data *osc_data = acpi_get_osc_data(handle);
+	acpi_handle tmp;
+	struct acpi_osc_data *osc_data;
+
+	status = acpi_get_handle(handle, "_OSC", &tmp);
+	if (ACPI_FAILURE(status))
+		return status;
 
+	osc_data = acpi_get_osc_data(handle);
 	if (!osc_data) {
 		printk(KERN_ERR "acpi osc data array is full\n");
 		return AE_ERROR;

Re: [patch] pci-acpi: handle multiple _OSC

[Jesse Barnes]

On Monday, May 12, 2008 6:55 am Kenji Kaneshige wrote:
> Shaohua Li wrote:
> > On Fri, 2008-05-09 at 10:40 -0700, Jesse Barnes wrote:
> >> On Friday, May 09, 2008 12:43 am Kenji Kaneshige wrote:
> >>> -static u32 ctrlset_buf[3] = {0, 0, 0};
> >>> -static u32 global_ctrlsets = 0;
> >>> +#define MAX_ACPI_OSC 30 /* Should be enough */
> >>> +static struct acpi_osc_data {
> >>> +	acpi_handle handle;
> >>> +	u32 ctrlset_buf[3];
> >>> +	u32 global_ctrlsets;
> >>> +} acpi_osc_data_array[MAX_ACPI_OSC];
> >>
> >> Could this just be a linked list of OSC objects instead?
> >
> > fixed. patch is against Kenji's ?"PCI ACPI: fix uninitialized variable
> > in __pci_osc_support_set" patch.
>
> I found a problem.
>
> > @@ -201,19 +232,25 @@ acpi_status pci_osc_control_set(acpi_han
> >  {
> >  	acpi_status	status;
> >  	u32		ctrlset;
> > +	struct acpi_osc_data *osc_data = acpi_get_osc_data(handle);
> > +
> > +	if (!osc_data) {
> > +		printk(KERN_ERR "acpi osc data array is full\n");
> > +		return AE_ERROR;
> > +	}
>
> The pci_osc_control_set() function can be called for the ACPI object
> that doesn't have _OSC method. In this case, acpi_get_osc_data() would
> allocate a useless memory region. To avoid this, we need to check the
> existence of _OSC before calling acpi_get_osc_data(). Here is a patch
> to fix this problem. It is against your patch.
>
> ---
>  drivers/pci/pci-acpi.c |    8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)

Thanks Shaohua & Kenji, I applied both fixes to the 'for-linus' tree.  Please 
test it out soon, I'd like to send Linus a pull request shortly.

Thanks,
Jesse

Re: [patch] pci-acpi: handle multiple _OSC

[Kenji Kaneshige]

Jesse Barnes wrote:
> On Monday, May 12, 2008 6:55 am Kenji Kaneshige wrote:
>> Shaohua Li wrote:
>>> On Fri, 2008-05-09 at 10:40 -0700, Jesse Barnes wrote:
>>>> On Friday, May 09, 2008 12:43 am Kenji Kaneshige wrote:
>>>>> -static u32 ctrlset_buf[3] = {0, 0, 0};
>>>>> -static u32 global_ctrlsets = 0;
>>>>> +#define MAX_ACPI_OSC 30 /* Should be enough */
>>>>> +static struct acpi_osc_data {
>>>>> +	acpi_handle handle;
>>>>> +	u32 ctrlset_buf[3];
>>>>> +	u32 global_ctrlsets;
>>>>> +} acpi_osc_data_array[MAX_ACPI_OSC];
>>>> Could this just be a linked list of OSC objects instead?
>>> fixed. patch is against Kenji's ?"PCI ACPI: fix uninitialized variable
>>> in __pci_osc_support_set" patch.
>> I found a problem.
>>
>>> @@ -201,19 +232,25 @@ acpi_status pci_osc_control_set(acpi_han
>>>  {
>>>  	acpi_status	status;
>>>  	u32		ctrlset;
>>> +	struct acpi_osc_data *osc_data = acpi_get_osc_data(handle);
>>> +
>>> +	if (!osc_data) {
>>> +		printk(KERN_ERR "acpi osc data array is full\n");
>>> +		return AE_ERROR;
>>> +	}
>> The pci_osc_control_set() function can be called for the ACPI object
>> that doesn't have _OSC method. In this case, acpi_get_osc_data() would
>> allocate a useless memory region. To avoid this, we need to check the
>> existence of _OSC before calling acpi_get_osc_data(). Here is a patch
>> to fix this problem. It is against your patch.
>>
>> ---
>>  drivers/pci/pci-acpi.c |    8 +++++++-
>>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> Thanks Shaohua & Kenji, I applied both fixes to the 'for-linus' tree.  Please 
> test it out soon, I'd like to send Linus a pull request shortly.
> 

I noticed the similar fix is also needed for acpi_query_osc().

Thanks,
Kenji Kaneshige


The acpi_query_osc() function can be called for the ACPI object that
doesn't have _OSC method. In this case, acpi_get_osc_data() would
allocate a useless memory region. To avoid this, we need to check the
existence of _OSC before calling acpi_get_osc_data().

Signed-off-by: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>

---
 drivers/pci/pci-acpi.c |    8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Index: linux-2.6.26-rc2/drivers/pci/pci-acpi.c
===================================================================
--- linux-2.6.26-rc2.orig/drivers/pci/pci-acpi.c
+++ linux-2.6.26-rc2/drivers/pci/pci-acpi.c
@@ -60,9 +60,15 @@ acpi_query_osc (
 	union acpi_object 	*out_obj;
 	u32			osc_dw0;
 	acpi_status *ret_status = (acpi_status *)retval;
-	struct acpi_osc_data *osc_data = acpi_get_osc_data(handle);
+	struct acpi_osc_data *osc_data;
 	u32 flags = (unsigned long)context, temp;
+	acpi_handle tmp;
 
+	status = acpi_get_handle(handle, "_OSC", &tmp);
+	if (ACPI_FAILURE(status))
+		return status;
+
+	osc_data = acpi_get_osc_data(handle);
 	if (!osc_data) {
 		printk(KERN_ERR "acpi osc data array is full\n");
 		return AE_ERROR;

Re: [patch] pci-acpi: handle multiple _OSC

[Jesse Barnes]

On Tuesday, May 13, 2008 12:48 am Kenji Kaneshige wrote:
> The acpi_query_osc() function can be called for the ACPI object that
> doesn't have _OSC method. In this case, acpi_get_osc_data() would
> allocate a useless memory region. To avoid this, we need to check the
> existence of _OSC before calling acpi_get_osc_data().
>
> Signed-off-by: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>

Thanks Kenji, applied.

Jesse