* [PATCH] PNPACPI: Use _CRS buffer directly as _SRS template
@ 2013-01-04 22:38 Bjorn Helgaas
2013-01-04 23:41 ` Rafael J. Wysocki
0 siblings, 1 reply; 2+ messages in thread
From: Bjorn Helgaas @ 2013-01-04 22:38 UTC (permalink / raw)
To: Rafael J. Wysocki, Len Brown; +Cc: linux-acpi
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(-)
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)
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] PNPACPI: Use _CRS buffer directly as _SRS template
2013-01-04 22:38 [PATCH] PNPACPI: Use _CRS buffer directly as _SRS template Bjorn Helgaas
@ 2013-01-04 23:41 ` Rafael J. Wysocki
0 siblings, 0 replies; 2+ messages in thread
From: Rafael J. Wysocki @ 2013-01-04 23:41 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: Len Brown, linux-acpi
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)
>
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2013-01-04 23:36 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-01-04 22:38 [PATCH] PNPACPI: Use _CRS buffer directly as _SRS template Bjorn Helgaas
2013-01-04 23:41 ` Rafael J. Wysocki
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).