[PATCH 7/8] acpi: avoid using internal acpica structures

[PATCH 7/8] acpi: avoid using internal acpica structures

[Lin Ming]

Avoid using internal acpica structures acpi_namespace_node and acpi_operand_object
Call acpi_get_name or acpi_get_object_info to get node name and method arg count

Signed-off-by: Lin Ming <ming.m.lin@intel.com>
---
 drivers/acpi/ec.c          |   13 +++++++++++--
 drivers/misc/sony-laptop.c |   15 +++++++++------
 2 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
index 591b4f6..2f4828f 100644
--- a/drivers/acpi/ec.c
+++ b/drivers/acpi/ec.c
@@ -754,10 +754,19 @@ static acpi_status
 acpi_ec_register_query_methods(acpi_handle handle, u32 level,
 			       void *context, void **return_value)
 {
-	struct acpi_namespace_node *node = handle;
+	char node_name[5];
+	struct acpi_buffer buffer = { sizeof(node_name), node_name };
 	struct acpi_ec *ec = context;
 	int value = 0;
-	if (sscanf(node->name.ascii, "_Q%x", &value) == 1) {
+	acpi_status status;
+
+	status = acpi_get_name(handle, ACPI_SINGLE_NAME, &buffer);
+
+	if (ACPI_FAILURE(status)) {
+		return status;
+	}
+       
+	if (sscanf(node_name, "_Q%x", &value) == 1) {
 		acpi_ec_add_query_handler(ec, value, handle, NULL, NULL);
 	}
 	return AE_OK;
diff --git a/drivers/misc/sony-laptop.c b/drivers/misc/sony-laptop.c
index 7bcb810..dd9c16f 100644
--- a/drivers/misc/sony-laptop.c
+++ b/drivers/misc/sony-laptop.c
@@ -935,14 +935,17 @@ static void sony_acpi_notify(acpi_handle handle, u32 event, void *data)
 static acpi_status sony_walk_callback(acpi_handle handle, u32 level,
 				      void *context, void **return_value)
 {
-	struct acpi_namespace_node *node;
-	union acpi_operand_object *operand;
+	struct acpi_device_info *info;
+	struct acpi_buffer buffer = {ACPI_ALLOCATE_BUFFER, NULL};
 
-	node = (struct acpi_namespace_node *)handle;
-	operand = (union acpi_operand_object *)node->object;
+	if (ACPI_SUCCESS(acpi_get_object_info(handle, &buffer))) {
+		info = buffer.pointer;
 
-	printk(KERN_WARNING DRV_PFX "method: name: %4.4s, args %X\n", node->name.ascii,
-	       (u32) operand->method.param_count);
+		printk(KERN_WARNING DRV_PFX "method: name: %4.4s, args %X\n",
+			(char *)info->name, info->param_count);
+
+		kfree(buffer.pointer);
+	}
 
 	return AE_OK;
 }


--
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 7/8] acpi: avoid using internal acpica structures

[Alexey Starikovskiy]

At least, this patch should be split in two -- for each file it touches.
Second, please don't return failure status from this function, as all 
functions under
the EC scope should be tried, and not only ones before first failure.
Thanks,
Alex.
Lin Ming wrote:
> Avoid using internal acpica structures acpi_namespace_node and acpi_operand_object
> Call acpi_get_name or acpi_get_object_info to get node name and method arg count
>
> Signed-off-by: Lin Ming <ming.m.lin@intel.com>
> ---
>  drivers/acpi/ec.c          |   13 +++++++++++--
>  drivers/misc/sony-laptop.c |   15 +++++++++------
>  2 files changed, 20 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
> index 591b4f6..2f4828f 100644
> --- a/drivers/acpi/ec.c
> +++ b/drivers/acpi/ec.c
> @@ -754,10 +754,19 @@ static acpi_status
>  acpi_ec_register_query_methods(acpi_handle handle, u32 level,
>  			       void *context, void **return_value)
>  {
> -	struct acpi_namespace_node *node = handle;
> +	char node_name[5];
> +	struct acpi_buffer buffer = { sizeof(node_name), node_name };
>  	struct acpi_ec *ec = context;
>  	int value = 0;
> -	if (sscanf(node->name.ascii, "_Q%x", &value) == 1) {
> +	acpi_status status;
> +
> +	status = acpi_get_name(handle, ACPI_SINGLE_NAME, &buffer);
> +
> +	if (ACPI_FAILURE(status)) {
> +		return status;
> +	}
> +       
> +	if (sscanf(node_name, "_Q%x", &value) == 1) {
>  		acpi_ec_add_query_handler(ec, value, handle, NULL, NULL);
>  	}
>  	return AE_OK;
> diff --git a/drivers/misc/sony-laptop.c b/drivers/misc/sony-laptop.c
> index 7bcb810..dd9c16f 100644
> --- a/drivers/misc/sony-laptop.c
> +++ b/drivers/misc/sony-laptop.c
> @@ -935,14 +935,17 @@ static void sony_acpi_notify(acpi_handle handle, u32 event, void *data)
>  static acpi_status sony_walk_callback(acpi_handle handle, u32 level,
>  				      void *context, void **return_value)
>  {
> -	struct acpi_namespace_node *node;
> -	union acpi_operand_object *operand;
> +	struct acpi_device_info *info;
> +	struct acpi_buffer buffer = {ACPI_ALLOCATE_BUFFER, NULL};
>  
> -	node = (struct acpi_namespace_node *)handle;
> -	operand = (union acpi_operand_object *)node->object;
> +	if (ACPI_SUCCESS(acpi_get_object_info(handle, &buffer))) {
> +		info = buffer.pointer;
>  
> -	printk(KERN_WARNING DRV_PFX "method: name: %4.4s, args %X\n", node->name.ascii,
> -	       (u32) operand->method.param_count);
> +		printk(KERN_WARNING DRV_PFX "method: name: %4.4s, args %X\n",
> +			(char *)info->name, info->param_count);
> +
> +		kfree(buffer.pointer);
> +	}
>  
>  	return AE_OK;
>  }
>
>
> --
> 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

Re: [PATCH 7/8] acpi: avoid using internal acpica structures

[Lin Ming]

On Wed, 2008-11-26 at 16:12 +0800, Alexey Starikovskiy wrote:
> At least, this patch should be split in two -- for each file it touches.

OK, will split it into two.

> Second, please don't return failure status from this function, as all 
> functions under
> the EC scope should be tried, and not only ones before first failure.

How about below patch?

---
 drivers/acpi/ec.c |   14 +++++++++++---
 1 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
index 591b4f6..99bff80 100644
--- a/drivers/acpi/ec.c
+++ b/drivers/acpi/ec.c
@@ -754,12 +754,20 @@ static acpi_status
 acpi_ec_register_query_methods(acpi_handle handle, u32 level,
 			       void *context, void **return_value)
 {
-	struct acpi_namespace_node *node = handle;
+	char node_name[5];
+	struct acpi_buffer buffer = { sizeof(node_name), node_name };
 	struct acpi_ec *ec = context;
 	int value = 0;
-	if (sscanf(node->name.ascii, "_Q%x", &value) == 1) {
-		acpi_ec_add_query_handler(ec, value, handle, NULL, NULL);
+	acpi_status status;
+
+	status = acpi_get_name(handle, ACPI_SINGLE_NAME, &buffer);
+
+	if (ACPI_SUCCESS(status)) {
+		if (sscanf(node_name, "_Q%x", &value) == 1) {
+			acpi_ec_add_query_handler(ec, value, handle, NULL, NULL);
+		}
 	}
+       
 	return AE_OK;
 }
 

Thanks for review,
Lin Ming

> Thanks,
> Alex.
> Lin Ming wrote:
> > Avoid using internal acpica structures acpi_namespace_node and acpi_operand_object
> > Call acpi_get_name or acpi_get_object_info to get node name and method arg count
> >
> > Signed-off-by: Lin Ming <ming.m.lin@intel.com>
> > ---
> >  drivers/acpi/ec.c          |   13 +++++++++++--
> >  drivers/misc/sony-laptop.c |   15 +++++++++------
> >  2 files changed, 20 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
> > index 591b4f6..2f4828f 100644
> > --- a/drivers/acpi/ec.c
> > +++ b/drivers/acpi/ec.c
> > @@ -754,10 +754,19 @@ static acpi_status
> >  acpi_ec_register_query_methods(acpi_handle handle, u32 level,
> >  			       void *context, void **return_value)
> >  {
> > -	struct acpi_namespace_node *node = handle;
> > +	char node_name[5];
> > +	struct acpi_buffer buffer = { sizeof(node_name), node_name };
> >  	struct acpi_ec *ec = context;
> >  	int value = 0;
> > -	if (sscanf(node->name.ascii, "_Q%x", &value) == 1) {
> > +	acpi_status status;
> > +
> > +	status = acpi_get_name(handle, ACPI_SINGLE_NAME, &buffer);
> > +
> > +	if (ACPI_FAILURE(status)) {
> > +		return status;
> > +	}
> > +       
> > +	if (sscanf(node_name, "_Q%x", &value) == 1) {
> >  		acpi_ec_add_query_handler(ec, value, handle, NULL, NULL);
> >  	}
> >  	return AE_OK;
> > diff --git a/drivers/misc/sony-laptop.c b/drivers/misc/sony-laptop.c
> > index 7bcb810..dd9c16f 100644
> > --- a/drivers/misc/sony-laptop.c
> > +++ b/drivers/misc/sony-laptop.c
> > @@ -935,14 +935,17 @@ static void sony_acpi_notify(acpi_handle handle, u32 event, void *data)
> >  static acpi_status sony_walk_callback(acpi_handle handle, u32 level,
> >  				      void *context, void **return_value)
> >  {
> > -	struct acpi_namespace_node *node;
> > -	union acpi_operand_object *operand;
> > +	struct acpi_device_info *info;
> > +	struct acpi_buffer buffer = {ACPI_ALLOCATE_BUFFER, NULL};
> >  
> > -	node = (struct acpi_namespace_node *)handle;
> > -	operand = (union acpi_operand_object *)node->object;
> > +	if (ACPI_SUCCESS(acpi_get_object_info(handle, &buffer))) {
> > +		info = buffer.pointer;
> >  
> > -	printk(KERN_WARNING DRV_PFX "method: name: %4.4s, args %X\n", node->name.ascii,
> > -	       (u32) operand->method.param_count);
> > +		printk(KERN_WARNING DRV_PFX "method: name: %4.4s, args %X\n",
> > +			(char *)info->name, info->param_count);
> > +
> > +		kfree(buffer.pointer);
> > +	}
> >  
> >  	return AE_OK;
> >  }
> >
> >
> > --
> > 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

Re: [PATCH 7/8] acpi: avoid using internal acpica structures

[Alexey Starikovskiy]

Lin Ming wrote:
> On Wed, 2008-11-26 at 16:12 +0800, Alexey Starikovskiy wrote:
>   
>> At least, this patch should be split in two -- for each file it touches.
>>     
>
> OK, will split it into two.
>
>   
>> Second, please don't return failure status from this function, as all 
>> functions under
>> the EC scope should be tried, and not only ones before first failure.
>>     
>
> How about below patch?
>
> ---
>  drivers/acpi/ec.c |   14 +++++++++++---
>  1 files changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
> index 591b4f6..99bff80 100644
> --- a/drivers/acpi/ec.c
> +++ b/drivers/acpi/ec.c
> @@ -754,12 +754,20 @@ static acpi_status
>  acpi_ec_register_query_methods(acpi_handle handle, u32 level,
>  			       void *context, void **return_value)
>  {
> -	struct acpi_namespace_node *node = handle;
> +	char node_name[5];
> +	struct acpi_buffer buffer = { sizeof(node_name), node_name };
>  	struct acpi_ec *ec = context;
>  	int value = 0;
> -	if (sscanf(node->name.ascii, "_Q%x", &value) == 1) {
> -		acpi_ec_add_query_handler(ec, value, handle, NULL, NULL);
> +	acpi_status status;
>   
> +
> +	status = acpi_get_name(handle, ACPI_SINGLE_NAME, &buffer);
>   
Merge the above two lines?
> +
> +	if (ACPI_SUCCESS(status)) {
> +		if (sscanf(node_name, "_Q%x", &value) == 1) {
>   
single if (ACPI_SUCCESS(status) && sscanf(node_name, "_Q%x", &value) == 1) ?
> +			acpi_ec_add_query_handler(ec, value, handle, NULL, NULL);
> +		}
>  	}
> +       
>  	return AE_OK;
>  }
>  
>
> Thanks for review,
> Lin Ming
>   

Re: [PATCH 7/8] acpi: avoid using internal acpica structures

[Lin Ming]

> Merge the above two lines?
> >
> > +
> > +       if (ACPI_SUCCESS(status)) {
> > +               if (sscanf(node_name, "_Q%x", &value) == 1) {
> >
> 
> single if (ACPI_SUCCESS(status) && sscanf(node_name, "_Q%x", &value) == 1) ?

OK, that makes code compact.

Thanks,
Lin Ming