diff mbox

PNPACPI: Use _CRS buffer directly as _SRS template

Message ID 20130104223810.4806.72475.stgit@bhelgaas.mtv.corp.google.com (mailing list archive)
State Accepted, archived
Headers show

Commit Message

Bjorn Helgaas Jan. 4, 2013, 10:38 p.m. UTC
Previously we went to a lot of trouble to count the supported descriptors
in _CRS, allocate a new buffer, copy the descriptor types to the new
buffer, and encode resources into the new buffer.  This will fail if _CRS
contains a descriptor type we don't support, even if we don't need to
change that descriptor.

I think it's simpler and more correct to just use the buffer returned from
_CRS directly, as suggested by the ACPI spec rev 5.0, sec 6.2.15.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pnp/pnpacpi/core.c     |   10 +++--
 drivers/pnp/pnpacpi/rsparser.c |   85 ----------------------------------------
 2 files changed, 6 insertions(+), 89 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

Rafael Wysocki Jan. 4, 2013, 11:41 p.m. UTC | #1
On Friday, January 04, 2013 03:38:10 PM Bjorn Helgaas wrote:
> Previously we went to a lot of trouble to count the supported descriptors
> in _CRS, allocate a new buffer, copy the descriptor types to the new
> buffer, and encode resources into the new buffer.  This will fail if _CRS
> contains a descriptor type we don't support, even if we don't need to
> change that descriptor.
> 
> I think it's simpler and more correct to just use the buffer returned from
> _CRS directly, as suggested by the ACPI spec rev 5.0, sec 6.2.15.

Yes, I've been thinking about a similar change.

I'll take this for v3.9.

Thanks,
Rafael


> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> ---
>  drivers/pnp/pnpacpi/core.c     |   10 +++--
>  drivers/pnp/pnpacpi/rsparser.c |   85 ----------------------------------------
>  2 files changed, 6 insertions(+), 89 deletions(-)
> 
> diff --git a/drivers/pnp/pnpacpi/core.c b/drivers/pnp/pnpacpi/core.c
> index 72e822e..701e00e 100644
> --- a/drivers/pnp/pnpacpi/core.c
> +++ b/drivers/pnp/pnpacpi/core.c
> @@ -84,7 +84,8 @@ static int pnpacpi_set_resources(struct pnp_dev *dev)
>  {
>  	struct acpi_device *acpi_dev;
>  	acpi_handle handle;
> -	struct acpi_buffer buffer;
> +	struct acpi_buffer buffer = {ACPI_ALLOCATE_BUFFER, NULL};
> +	acpi_status status;
>  	int ret;
>  
>  	pnp_dbg(&dev->dev, "set resources\n");
> @@ -98,9 +99,10 @@ static int pnpacpi_set_resources(struct pnp_dev *dev)
>  	if (WARN_ON_ONCE(acpi_dev != dev->data))
>  		dev->data = acpi_dev;
>  
> -	ret = pnpacpi_build_resource_template(dev, &buffer);
> -	if (ret)
> -		return ret;
> +	status = acpi_evaluate_object(handle, METHOD_NAME__CRS, NULL, &buffer);
> +	if (ACPI_FAILURE(status))
> +		return -EINVAL;
> +
>  	ret = pnpacpi_encode_resources(dev, &buffer);
>  	if (ret) {
>  		kfree(buffer.pointer);
> diff --git a/drivers/pnp/pnpacpi/rsparser.c b/drivers/pnp/pnpacpi/rsparser.c
> index b8f4ea7..629710a 100644
> --- a/drivers/pnp/pnpacpi/rsparser.c
> +++ b/drivers/pnp/pnpacpi/rsparser.c
> @@ -553,91 +553,6 @@ int __init pnpacpi_parse_resource_option_data(struct pnp_dev *dev)
>  	return 0;
>  }
>  
> -static int pnpacpi_supported_resource(struct acpi_resource *res)
> -{
> -	switch (res->type) {
> -	case ACPI_RESOURCE_TYPE_IRQ:
> -	case ACPI_RESOURCE_TYPE_DMA:
> -	case ACPI_RESOURCE_TYPE_IO:
> -	case ACPI_RESOURCE_TYPE_FIXED_IO:
> -	case ACPI_RESOURCE_TYPE_MEMORY24:
> -	case ACPI_RESOURCE_TYPE_MEMORY32:
> -	case ACPI_RESOURCE_TYPE_FIXED_MEMORY32:
> -	case ACPI_RESOURCE_TYPE_ADDRESS16:
> -	case ACPI_RESOURCE_TYPE_ADDRESS32:
> -	case ACPI_RESOURCE_TYPE_ADDRESS64:
> -	case ACPI_RESOURCE_TYPE_EXTENDED_ADDRESS64:
> -	case ACPI_RESOURCE_TYPE_EXTENDED_IRQ:
> -		return 1;
> -	}
> -	return 0;
> -}
> -
> -/*
> - * Set resource
> - */
> -static acpi_status pnpacpi_count_resources(struct acpi_resource *res,
> -					   void *data)
> -{
> -	int *res_cnt = data;
> -
> -	if (pnpacpi_supported_resource(res))
> -		(*res_cnt)++;
> -	return AE_OK;
> -}
> -
> -static acpi_status pnpacpi_type_resources(struct acpi_resource *res, void *data)
> -{
> -	struct acpi_resource **resource = data;
> -
> -	if (pnpacpi_supported_resource(res)) {
> -		(*resource)->type = res->type;
> -		(*resource)->length = sizeof(struct acpi_resource);
> -		if (res->type == ACPI_RESOURCE_TYPE_IRQ)
> -			(*resource)->data.irq.descriptor_length =
> -					res->data.irq.descriptor_length;
> -		(*resource)++;
> -	}
> -
> -	return AE_OK;
> -}
> -
> -int pnpacpi_build_resource_template(struct pnp_dev *dev,
> -				    struct acpi_buffer *buffer)
> -{
> -	struct acpi_device *acpi_dev = dev->data;
> -	acpi_handle handle = acpi_dev->handle;
> -	struct acpi_resource *resource;
> -	int res_cnt = 0;
> -	acpi_status status;
> -
> -	status = acpi_walk_resources(handle, METHOD_NAME__CRS,
> -				     pnpacpi_count_resources, &res_cnt);
> -	if (ACPI_FAILURE(status)) {
> -		dev_err(&dev->dev, "can't evaluate _CRS: %d\n", status);
> -		return -EINVAL;
> -	}
> -	if (!res_cnt)
> -		return -EINVAL;
> -	buffer->length = sizeof(struct acpi_resource) * (res_cnt + 1) + 1;
> -	buffer->pointer = kzalloc(buffer->length - 1, GFP_KERNEL);
> -	if (!buffer->pointer)
> -		return -ENOMEM;
> -
> -	resource = (struct acpi_resource *)buffer->pointer;
> -	status = acpi_walk_resources(handle, METHOD_NAME__CRS,
> -				     pnpacpi_type_resources, &resource);
> -	if (ACPI_FAILURE(status)) {
> -		kfree(buffer->pointer);
> -		dev_err(&dev->dev, "can't evaluate _CRS: %d\n", status);
> -		return -EINVAL;
> -	}
> -	/* resource will pointer the end resource now */
> -	resource->type = ACPI_RESOURCE_TYPE_END_TAG;
> -
> -	return 0;
> -}
> -
>  static void pnpacpi_encode_irq(struct pnp_dev *dev,
>  			       struct acpi_resource *resource,
>  			       struct resource *p)
>
diff mbox

Patch

diff --git a/drivers/pnp/pnpacpi/core.c b/drivers/pnp/pnpacpi/core.c
index 72e822e..701e00e 100644
--- a/drivers/pnp/pnpacpi/core.c
+++ b/drivers/pnp/pnpacpi/core.c
@@ -84,7 +84,8 @@  static int pnpacpi_set_resources(struct pnp_dev *dev)
 {
 	struct acpi_device *acpi_dev;
 	acpi_handle handle;
-	struct acpi_buffer buffer;
+	struct acpi_buffer buffer = {ACPI_ALLOCATE_BUFFER, NULL};
+	acpi_status status;
 	int ret;
 
 	pnp_dbg(&dev->dev, "set resources\n");
@@ -98,9 +99,10 @@  static int pnpacpi_set_resources(struct pnp_dev *dev)
 	if (WARN_ON_ONCE(acpi_dev != dev->data))
 		dev->data = acpi_dev;
 
-	ret = pnpacpi_build_resource_template(dev, &buffer);
-	if (ret)
-		return ret;
+	status = acpi_evaluate_object(handle, METHOD_NAME__CRS, NULL, &buffer);
+	if (ACPI_FAILURE(status))
+		return -EINVAL;
+
 	ret = pnpacpi_encode_resources(dev, &buffer);
 	if (ret) {
 		kfree(buffer.pointer);
diff --git a/drivers/pnp/pnpacpi/rsparser.c b/drivers/pnp/pnpacpi/rsparser.c
index b8f4ea7..629710a 100644
--- a/drivers/pnp/pnpacpi/rsparser.c
+++ b/drivers/pnp/pnpacpi/rsparser.c
@@ -553,91 +553,6 @@  int __init pnpacpi_parse_resource_option_data(struct pnp_dev *dev)
 	return 0;
 }
 
-static int pnpacpi_supported_resource(struct acpi_resource *res)
-{
-	switch (res->type) {
-	case ACPI_RESOURCE_TYPE_IRQ:
-	case ACPI_RESOURCE_TYPE_DMA:
-	case ACPI_RESOURCE_TYPE_IO:
-	case ACPI_RESOURCE_TYPE_FIXED_IO:
-	case ACPI_RESOURCE_TYPE_MEMORY24:
-	case ACPI_RESOURCE_TYPE_MEMORY32:
-	case ACPI_RESOURCE_TYPE_FIXED_MEMORY32:
-	case ACPI_RESOURCE_TYPE_ADDRESS16:
-	case ACPI_RESOURCE_TYPE_ADDRESS32:
-	case ACPI_RESOURCE_TYPE_ADDRESS64:
-	case ACPI_RESOURCE_TYPE_EXTENDED_ADDRESS64:
-	case ACPI_RESOURCE_TYPE_EXTENDED_IRQ:
-		return 1;
-	}
-	return 0;
-}
-
-/*
- * Set resource
- */
-static acpi_status pnpacpi_count_resources(struct acpi_resource *res,
-					   void *data)
-{
-	int *res_cnt = data;
-
-	if (pnpacpi_supported_resource(res))
-		(*res_cnt)++;
-	return AE_OK;
-}
-
-static acpi_status pnpacpi_type_resources(struct acpi_resource *res, void *data)
-{
-	struct acpi_resource **resource = data;
-
-	if (pnpacpi_supported_resource(res)) {
-		(*resource)->type = res->type;
-		(*resource)->length = sizeof(struct acpi_resource);
-		if (res->type == ACPI_RESOURCE_TYPE_IRQ)
-			(*resource)->data.irq.descriptor_length =
-					res->data.irq.descriptor_length;
-		(*resource)++;
-	}
-
-	return AE_OK;
-}
-
-int pnpacpi_build_resource_template(struct pnp_dev *dev,
-				    struct acpi_buffer *buffer)
-{
-	struct acpi_device *acpi_dev = dev->data;
-	acpi_handle handle = acpi_dev->handle;
-	struct acpi_resource *resource;
-	int res_cnt = 0;
-	acpi_status status;
-
-	status = acpi_walk_resources(handle, METHOD_NAME__CRS,
-				     pnpacpi_count_resources, &res_cnt);
-	if (ACPI_FAILURE(status)) {
-		dev_err(&dev->dev, "can't evaluate _CRS: %d\n", status);
-		return -EINVAL;
-	}
-	if (!res_cnt)
-		return -EINVAL;
-	buffer->length = sizeof(struct acpi_resource) * (res_cnt + 1) + 1;
-	buffer->pointer = kzalloc(buffer->length - 1, GFP_KERNEL);
-	if (!buffer->pointer)
-		return -ENOMEM;
-
-	resource = (struct acpi_resource *)buffer->pointer;
-	status = acpi_walk_resources(handle, METHOD_NAME__CRS,
-				     pnpacpi_type_resources, &resource);
-	if (ACPI_FAILURE(status)) {
-		kfree(buffer->pointer);
-		dev_err(&dev->dev, "can't evaluate _CRS: %d\n", status);
-		return -EINVAL;
-	}
-	/* resource will pointer the end resource now */
-	resource->type = ACPI_RESOURCE_TYPE_END_TAG;
-
-	return 0;
-}
-
 static void pnpacpi_encode_irq(struct pnp_dev *dev,
 			       struct acpi_resource *resource,
 			       struct resource *p)