* Re: [PATCH] pnpacpi: Better format error message [not found] ` <201103011524.50340.trenn@suse.de> @ 2011-03-01 16:25 ` Bjorn Helgaas [not found] ` <201103011743.33975.trenn@suse.de> 0 siblings, 1 reply; 4+ messages in thread From: Bjorn Helgaas @ 2011-03-01 16:25 UTC (permalink / raw) To: Thomas Renninger; +Cc: lenb, robert.moore, linux-acpi On Tue, Mar 01, 2011 at 03:24:50PM +0100, Thomas Renninger wrote: > On a system with empty _CRS method I see: > pnp 00:0c: can't evaluate _CRS: 12311 > which is 0x3017, which would mean: AE_AML_INVALID_RESOURCE_TYPE > > This patch would directly show: AE_AML_INVALID_RESOURCE_TYPE The patch seems fine to me, but I question whether we need these printks at all. It seems pointless to have an empty _CRS method, but it does seem legal, or at least debatable, so I'm not sure that the message is telling us anything useful. Bjorn > Signed-off-by: Thomas Renninger <trenn@suse.de> > CC: bjorn@helgaas.com > CC: lenb@kernel.org > CC: robert.moore@intel.com > > --- > Corrected email address of Bjorn. > > drivers/pnp/pnpacpi/rsparser.c | 12 ++++++++---- > 1 file changed, 8 insertions(+), 4 deletions(-) > > Index: linux-2.6.37-master/drivers/pnp/pnpacpi/rsparser.c > =================================================================== > --- linux-2.6.37-master.orig/drivers/pnp/pnpacpi/rsparser.c > +++ linux-2.6.37-master/drivers/pnp/pnpacpi/rsparser.c > @@ -498,7 +498,8 @@ int pnpacpi_parse_allocated_resource(str > > if (ACPI_FAILURE(status)) { > if (status != AE_NOT_FOUND) > - dev_err(&dev->dev, "can't evaluate _CRS: %d", status); > + dev_err(&dev->dev, "can't evaluate _CRS: %s", > + acpi_format_exception(status)); > return -EPERM; > } > return 0; > @@ -809,7 +810,8 @@ int __init pnpacpi_parse_resource_option > > if (ACPI_FAILURE(status)) { > if (status != AE_NOT_FOUND) > - dev_err(&dev->dev, "can't evaluate _PRS: %d", status); > + dev_err(&dev->dev, "can't evaluate _PRS: %s", > + acpi_format_exception(status)); > return -EPERM; > } > return 0; > @@ -876,7 +878,8 @@ int pnpacpi_build_resource_template(stru > 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); > + dev_err(&dev->dev, "can't evaluate _CRS: %s\n", > + acpi_format_exception(status)); > return -EINVAL; > } > if (!res_cnt) > @@ -891,7 +894,8 @@ int pnpacpi_build_resource_template(stru > pnpacpi_type_resources, &resource); > if (ACPI_FAILURE(status)) { > kfree(buffer->pointer); > - dev_err(&dev->dev, "can't evaluate _CRS: %d\n", status); > + dev_err(&dev->dev, "can't evaluate _CRS: %s\n", > + acpi_format_exception(status)); > return -EINVAL; > } > /* resource will pointer the end resource now */ ^ permalink raw reply [flat|nested] 4+ messages in thread
[parent not found: <201103011743.33975.trenn@suse.de>]
* RE: [PATCH] pnpacpi: Better format error message [not found] ` <201103011743.33975.trenn@suse.de> @ 2011-03-01 17:03 ` Moore, Robert 2011-03-01 21:08 ` Thomas Renninger 0 siblings, 1 reply; 4+ messages in thread From: Moore, Robert @ 2011-03-01 17:03 UTC (permalink / raw) To: Thomas Renninger, Bjorn Helgaas Cc: lenb@kernel.org, linux-acpi@vger.kernel.org - there seem to be another problem in the acpica resource handling depths if _CRS functions do not return anything. Strictly speaking it's illegal for _CRS to not return anything. Practically, perhaps we should repair this case to a "NULL" resource descriptor which perhaps could be defined as a resource descriptor with a single End Tag descriptor. From: Thomas Renninger [mailto:trenn@suse.de] Sent: Tuesday, March 01, 2011 8:44 AM To: Bjorn Helgaas Cc: lenb@kernel.org; Moore, Robert; linux-acpi@vger.kernel.org Subject: Re: [PATCH] pnpacpi: Better format error message On Tuesday, March 01, 2011 05:25:04 PM Bjorn Helgaas wrote: > On Tue, Mar 01, 2011 at 03:24:50PM +0100, Thomas Renninger wrote: > > On a system with empty _CRS method I see: > > pnp 00:0c: can't evaluate _CRS: 12311 > > which is 0x3017, which would mean: AE_AML_INVALID_RESOURCE_TYPE > > > > This patch would directly show: AE_AML_INVALID_RESOURCE_TYPE > > The patch seems fine to me, but I question whether we need these > printks at all. It seems pointless to have an empty _CRS method, > but it does seem legal, or at least debatable, so I'm not sure > that the message is telling us anything useful. Theoretically the callback function passed to: acpi_walk_resources(handle, METHOD_NAME__CRS should get called with an empty resource string or not at all in case _CRS can return nothing. But in this case acpi_walk_resources fails which is worth reporting. I could imagine other _CRS functions are not processed anymore when acpi_walk_resources bails out with AE_AML_INVALID_RESOURCE_TYPE on a specific _CRS function, don't know. Summary: - acpi_walk_resources failures should be reported (and this patch is to do that more nicely) - there seem to be another problem in the acpica resource handling depths if _CRS functions do not return anything. Thomas > > Bjorn > > > Signed-off-by: Thomas Renninger <trenn@suse.de> > > CC: bjorn@helgaas.com > > CC: lenb@kernel.org > > CC: robert.moore@intel.com > > > > --- > > Corrected email address of Bjorn. > > > > drivers/pnp/pnpacpi/rsparser.c | 12 ++++++++---- > > 1 file changed, 8 insertions(+), 4 deletions(-) > > > > Index: linux-2.6.37-master/drivers/pnp/pnpacpi/rsparser.c > > =================================================================== > > --- linux-2.6.37-master.orig/drivers/pnp/pnpacpi/rsparser.c > > +++ linux-2.6.37-master/drivers/pnp/pnpacpi/rsparser.c > > @@ -498,7 +498,8 @@ int pnpacpi_parse_allocated_resource(str > > > > if (ACPI_FAILURE(status)) { > > if (status != AE_NOT_FOUND) > > - dev_err(&dev->dev, "can't evaluate _CRS: %d", status); > > + dev_err(&dev->dev, "can't evaluate _CRS: %s", > > + acpi_format_exception(status)); > > return -EPERM; > > } > > return 0; > > @@ -809,7 +810,8 @@ int __init pnpacpi_parse_resource_option > > > > if (ACPI_FAILURE(status)) { > > if (status != AE_NOT_FOUND) > > - dev_err(&dev->dev, "can't evaluate _PRS: %d", status); > > + dev_err(&dev->dev, "can't evaluate _PRS: %s", > > + acpi_format_exception(status)); > > return -EPERM; > > } > > return 0; > > @@ -876,7 +878,8 @@ int pnpacpi_build_resource_template(stru > > 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); > > + dev_err(&dev->dev, "can't evaluate _CRS: %s\n", > > + acpi_format_exception(status)); > > return -EINVAL; > > } > > if (!res_cnt) > > @@ -891,7 +894,8 @@ int pnpacpi_build_resource_template(stru > > pnpacpi_type_resources, &resource); > > if (ACPI_FAILURE(status)) { > > kfree(buffer->pointer); > > - dev_err(&dev->dev, "can't evaluate _CRS: %d\n", status); > > + dev_err(&dev->dev, "can't evaluate _CRS: %s\n", > > + acpi_format_exception(status)); > > return -EINVAL; > > } > > /* resource will pointer the end resource now */ > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] pnpacpi: Better format error message 2011-03-01 17:03 ` Moore, Robert @ 2011-03-01 21:08 ` Thomas Renninger 2011-03-01 22:25 ` Moore, Robert 0 siblings, 1 reply; 4+ messages in thread From: Thomas Renninger @ 2011-03-01 21:08 UTC (permalink / raw) To: Moore, Robert; +Cc: Bjorn Helgaas, lenb@kernel.org, linux-acpi@vger.kernel.org On Tuesday 01 March 2011 18:03:23 Moore, Robert wrote: > - there seem to be another problem in the acpica resource handling > depths if _CRS functions do not return anything. > > > Strictly speaking it's illegal for _CRS to not return anything. > Practically, perhaps we should repair this case to a "NULL" resource > descriptor which perhaps could be defined as a resource descriptor > with a single End Tag descriptor. Be aware that this was an early BIOS. I am fine if there is an error message if this is not allowed. It's the first time I saw such a message and an empty resource func. Thomas ^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH] pnpacpi: Better format error message 2011-03-01 21:08 ` Thomas Renninger @ 2011-03-01 22:25 ` Moore, Robert 0 siblings, 0 replies; 4+ messages in thread From: Moore, Robert @ 2011-03-01 22:25 UTC (permalink / raw) To: Thomas Renninger Cc: Bjorn Helgaas, lenb@kernel.org, linux-acpi@vger.kernel.org > -----Original Message----- > From: Thomas Renninger [mailto:trenn@suse.de] > Sent: Tuesday, March 01, 2011 1:08 PM > To: Moore, Robert > Cc: Bjorn Helgaas; lenb@kernel.org; linux-acpi@vger.kernel.org > Subject: Re: [PATCH] pnpacpi: Better format error message > > On Tuesday 01 March 2011 18:03:23 Moore, Robert wrote: > > - there seem to be another problem in the acpica resource handling > > depths if _CRS functions do not return anything. > > > > > > Strictly speaking it's illegal for _CRS to not return anything. > > Practically, perhaps we should repair this case to a "NULL" resource > > descriptor which perhaps could be defined as a resource descriptor > > with a single End Tag descriptor. > Be aware that this was an early BIOS. > I am fine if there is an error message if this is not allowed. > It's the first time I saw such a message and an empty resource func. > > Thomas The goal of the repair code is of course to simplify the drivers. What we'd like to be able to do is guarantee that if an AE_OK is returned by the evaluation of a predefined ACPI name, the returned object is of the correct type. It seems to me that a "resource descriptor" is a special type of buffer, so we can add support for that in the repair code. In the _CRS case above, we could return a resource descriptor with simply an EndTag -- the driver can of course squawk about this if it desires. On the other hand, we could just punt on the conversion and just return an OBJECT_TYPE error. ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2011-03-01 22:25 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <201103011513.40235.trenn@suse.de>
[not found] ` <201103011524.50340.trenn@suse.de>
2011-03-01 16:25 ` [PATCH] pnpacpi: Better format error message Bjorn Helgaas
[not found] ` <201103011743.33975.trenn@suse.de>
2011-03-01 17:03 ` Moore, Robert
2011-03-01 21:08 ` Thomas Renninger
2011-03-01 22:25 ` Moore, Robert
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.