[PATCH] PNPACPI: handle address descriptors in _PRS

[PATCH] PNPACPI: handle address descriptors in _PRS

[Bjorn Helgaas]

PNPACPI complained about and ignored devices with ADDRESS16, ADDRESS32,
or ADDRESS64 descriptors in _PRS.  Those descriptors seem legitimate,
and HP firmware uses them for built-in serial ports, so this patch adds
support for them.

But I notice the mention of them under "#if 0" -- did somebody explicitly
decide not to handle them?  If so, what was the reason?

Signed-off-by: Bjorn Helgaas <bjorn.helgaas-VXdhtT5mjnY@public.gmane.org>

Index: work-vga2/drivers/pnp/pnpacpi/rsparser.c
===================================================================
--- work-vga2.orig/drivers/pnp/pnpacpi/rsparser.c	2005-09-14 09:29:06.000000000 -0600
+++ work-vga2/drivers/pnp/pnpacpi/rsparser.c	2005-09-20 10:44:58.000000000 -0600
@@ -453,6 +453,36 @@
 	return;
 }
 
+static void
+pnpacpi_parse_address_option(struct pnp_option *option,
+	struct acpi_resource_address64 *p)
+{
+	struct pnp_mem * mem;
+	struct pnp_port * port;
+
+	if (p->address_length == 0)
+		return;
+
+	if (p->resource_type == ACPI_MEMORY_RANGE) {
+		mem = kcalloc(1, sizeof(struct pnp_mem), GFP_KERNEL);
+		if (!mem)
+			return;
+		mem->min = mem->max = p->min_address_range;
+		mem->size = p->address_length;
+		mem->align = 0;
+		pnp_register_mem_resource(option,mem);
+	} else if (p->resource_type == ACPI_IO_RANGE) {
+		port = kcalloc(1, sizeof(struct pnp_port), GFP_KERNEL);
+		if (!port)
+			return;
+		port->min = port->max = p->min_address_range;
+		port->size = p->address_length;
+		port->align = 0;
+		port->flags = PNP_PORT_FLAG_FIXED;
+		pnp_register_port_resource(option,port);
+	}
+}
+
 struct acpipnp_parse_option_s {
 	struct pnp_option *option;
 	struct pnp_option *option_independent;
@@ -466,6 +496,8 @@
 	struct acpipnp_parse_option_s *parse_data = (struct acpipnp_parse_option_s *)data;
 	struct pnp_dev *dev = parse_data->dev;
 	struct pnp_option *option = parse_data->option;
+	struct acpi_resource_address64 addr;
+	acpi_status status;
 
 	switch (res->id) {
 		case ACPI_RSTYPE_IRQ:
@@ -495,6 +527,17 @@
 			pnpacpi_parse_fixed_mem32_option(option,
 				&res->data.fixed_memory32);
 			break;
+		case ACPI_RSTYPE_ADDRESS16:
+		case ACPI_RSTYPE_ADDRESS32:
+		case ACPI_RSTYPE_ADDRESS64:
+			status = acpi_resource_to_address64(res, &addr);
+			if (!ACPI_SUCCESS(status)) {
+				pnp_warn("PnPACPI: failed to convert resource "
+					"type %d", res->id);
+				return AE_ERROR;
+			}
+			pnpacpi_parse_address_option(option, &addr);
+			break;
 		case ACPI_RSTYPE_START_DPF:
 			switch (res->data.start_dpf.compatibility_priority) {
 				case ACPI_GOOD_CONFIGURATION:
@@ -568,11 +611,9 @@
 	case ACPI_RSTYPE_MEM24:
 	case ACPI_RSTYPE_MEM32:
 	case ACPI_RSTYPE_FIXED_MEM32:
-#if 0
 	case ACPI_RSTYPE_ADDRESS16:
 	case ACPI_RSTYPE_ADDRESS32:
 	case ACPI_RSTYPE_ADDRESS64:
-#endif
 		(*res_cnt) ++;
 	default:
 		return AE_OK;
@@ -593,11 +634,9 @@
 	case ACPI_RSTYPE_MEM24:
 	case ACPI_RSTYPE_MEM32:
 	case ACPI_RSTYPE_FIXED_MEM32:
-#if 0
 	case ACPI_RSTYPE_ADDRESS16:
 	case ACPI_RSTYPE_ADDRESS32:
 	case ACPI_RSTYPE_ADDRESS64:
-#endif
 		(*resource)->id = res->id;
 		(*resource)++;
 	default:


-------------------------------------------------------
SF.Net email is sponsored by:
Tame your development challenges with Apache's Geronimo App Server. 
Download it for free - -and be entered to win a 42" plasma tv or your very
own Sony(tm)PSP.  Click here to play: http://sourceforge.net/geronimo.php

Re: [PATCH] PNPACPI: handle address descriptors in _PRS

[Shaohua Li]

Hi,
On Tue, 2005-09-20 at 16:26 -0600, Bjorn Helgaas wrote:
> PNPACPI complained about and ignored devices with ADDRESS16, ADDRESS32,
> or ADDRESS64 descriptors in _PRS.  Those descriptors seem legitimate,
> and HP firmware uses them for built-in serial ports, so this patch adds
> support for them.
> 
> But I notice the mention of them under "#if 0" -- did somebody explicitly
> decide not to handle them?  If so, what was the reason?
IIRC, the reason is just we did find any pnp device uses it. If hp
firmware uses it, we should add the support.
You might have missed the similar change in pnpacpi_encode_resources in
your patch. It's used when we assign new resources (_SRS method) to pnp
devices.

Thanks,
Shaohua



-------------------------------------------------------
SF.Net email is sponsored by:
Tame your development challenges with Apache's Geronimo App Server. 
Download it for free - -and be entered to win a 42" plasma tv or your very
own Sony(tm)PSP.  Click here to play: http://sourceforge.net/geronimo.php

Re: Re: [PATCH] PNPACPI: handle address descriptors in _PRS

[Bjorn Helgaas]

On Tuesday 20 September 2005 7:17 pm, Shaohua Li wrote:
> On Tue, 2005-09-20 at 16:26 -0600, Bjorn Helgaas wrote:
> > PNPACPI complained about and ignored devices with ADDRESS16, ADDRESS32,
> > or ADDRESS64 descriptors in _PRS.  Those descriptors seem legitimate,
> > and HP firmware uses them for built-in serial ports, so this patch adds
> > support for them.

> You might have missed the similar change in pnpacpi_encode_resources in
> your patch. It's used when we assign new resources (_SRS method) to pnp
> devices.

You're right, I did miss the corresponding encoding change.  I'll
add that.  I feel a bit uneasy about it, though, because it will be
a fair amount of code, and I don't know how to test it.  Do we ever
really use _SRS today?  And there's information lost when decoding
the ACPI resources, such as the _TRA.


-------------------------------------------------------
SF.Net email is sponsored by:
Tame your development challenges with Apache's Geronimo App Server. 
Download it for free - -and be entered to win a 42" plasma tv or your very
own Sony(tm)PSP.  Click here to play: http://sourceforge.net/geronimo.php

Re: Re: [PATCH] PNPACPI: handle address descriptors in _PRS

[Shaohua Li]

On Wed, 2005-09-21 at 23:27 +0800, Bjorn Helgaas wrote:
> On Tuesday 20 September 2005 7:17 pm, Shaohua Li wrote: 
> > On Tue, 2005-09-20 at 16:26 -0600, Bjorn Helgaas wrote: 
> > > PNPACPI complained about and ignored devices with ADDRESS16,
> ADDRESS32, 
> > > or ADDRESS64 descriptors in _PRS.  Those descriptors seem
> legitimate, 
> > > and HP firmware uses them for built-in serial ports, so this patch
> adds 
> > > support for them.
> 
> > You might have missed the similar change in pnpacpi_encode_resources
> in 
> > your patch. It's used when we assign new resources (_SRS method) to
> pnp 
> > devices.
> 
> You're right, I did miss the corresponding encoding change.  I'll 
> add that.  I feel a bit uneasy about it, though, because it will be 
> a fair amount of code, and I don't know how to test it.  Do we ever 
> really use _SRS today?  And there's information lost when decoding 
> the ACPI resources, such as the _TRA.
We did use _SRS, at least under x86. This only is done when some PNP
devices havn't resources, so only very small number of systems need it.
Ignoring it might be ok currently, if HP firmware always assigns
resources to PNP devices. Yes, we did lose the info of _TRA. I didn't
find any system uses it. Maybe some big IA64 systems use.

Thanks,
Shaohua



-------------------------------------------------------
SF.Net email is sponsored by:
Tame your development challenges with Apache's Geronimo App Server. 
Download it for free - -and be entered to win a 42" plasma tv or your very
own Sony(tm)PSP.  Click here to play: http://sourceforge.net/geronimo.php

Re: Re: [PATCH] PNPACPI: handle address descriptors in _PRS

[Bjorn Helgaas]

On Wednesday 21 September 2005 6:49 pm, Shaohua Li wrote:
> On Wed, 2005-09-21 at 23:27 +0800, Bjorn Helgaas wrote:
> > On Tuesday 20 September 2005 7:17 pm, Shaohua Li wrote: 
> > > On Tue, 2005-09-20 at 16:26 -0600, Bjorn Helgaas wrote: 
> > > > PNPACPI complained about and ignored devices with ADDRESS16,
> > ADDRESS32, 
> > > > or ADDRESS64 descriptors in _PRS.  Those descriptors seem
> > legitimate, 
> > > > and HP firmware uses them for built-in serial ports, so this patch
> > adds 
> > > > support for them.
> > 
> > > You might have missed the similar change in pnpacpi_encode_resources
> > in 
> > > your patch. It's used when we assign new resources (_SRS method) to
> > pnp 
> > > devices.
> > 
> > You're right, I did miss the corresponding encoding change.  I'll 
> > add that.  I feel a bit uneasy about it, though, because it will be 
> > a fair amount of code, and I don't know how to test it.  Do we ever 
> > really use _SRS today?  And there's information lost when decoding 
> > the ACPI resources, such as the _TRA.
> We did use _SRS, at least under x86. This only is done when some PNP
> devices havn't resources, so only very small number of systems need it.
> Ignoring it might be ok currently, if HP firmware always assigns
> resources to PNP devices. Yes, we did lose the info of _TRA. I didn't
> find any system uses it. Maybe some big IA64 systems use.

HP ia64 systems definitely use _TRA.  I would expect most systems
with multiple PCI root bridges would use _TRA, because if you
don't, the amount of 32-bit PCI bus address space you can use
is severely limited.

HP firmware does assign resources to the PNP devices (typically
they're things hardwired onto the motherboard at fixed addresses).
So we don't actually need the encoding changes at the moment.

I'm inclined to just leave the encoding out for now.  That
way, we don't carry around the untested code, and if somebody
eventually tries to use it, the attempt to set resources will
fail, and we can write the code and test it at that time.

What do you think?

Bjorn


-------------------------------------------------------
SF.Net email is sponsored by:
Tame your development challenges with Apache's Geronimo App Server. 
Download it for free - -and be entered to win a 42" plasma tv or your very
own Sony(tm)PSP.  Click here to play: http://sourceforge.net/geronimo.php

Re: Re: [PATCH] PNPACPI: handle address descriptors in _PRS

[Shaohua Li]

On Thu, 2005-09-22 at 17:48 -0600, Bjorn Helgaas wrote:
> On Wednesday 21 September 2005 6:49 pm, Shaohua Li wrote:
> > On Wed, 2005-09-21 at 23:27 +0800, Bjorn Helgaas wrote:
> > > On Tuesday 20 September 2005 7:17 pm, Shaohua Li wrote: 
> > > > On Tue, 2005-09-20 at 16:26 -0600, Bjorn Helgaas wrote: 
> > > > > PNPACPI complained about and ignored devices with ADDRESS16,
> > > ADDRESS32, 
> > > > > or ADDRESS64 descriptors in _PRS.  Those descriptors seem
> > > legitimate, 
> > > > > and HP firmware uses them for built-in serial ports, so this patch
> > > adds 
> > > > > support for them.
> > > 
> > > > You might have missed the similar change in pnpacpi_encode_resources
> > > in 
> > > > your patch. It's used when we assign new resources (_SRS method) to
> > > pnp 
> > > > devices.
> > > 
> > > You're right, I did miss the corresponding encoding change.  I'll 
> > > add that.  I feel a bit uneasy about it, though, because it will be 
> > > a fair amount of code, and I don't know how to test it.  Do we ever 
> > > really use _SRS today?  And there's information lost when decoding 
> > > the ACPI resources, such as the _TRA.
> > We did use _SRS, at least under x86. This only is done when some PNP
> > devices havn't resources, so only very small number of systems need it.
> > Ignoring it might be ok currently, if HP firmware always assigns
> > resources to PNP devices. Yes, we did lose the info of _TRA. I didn't
> > find any system uses it. Maybe some big IA64 systems use.
> 
> HP ia64 systems definitely use _TRA.  I would expect most systems
> with multiple PCI root bridges would use _TRA, because if you
> don't, the amount of 32-bit PCI bus address space you can use
> is severely limited.
> 
> HP firmware does assign resources to the PNP devices (typically
> they're things hardwired onto the motherboard at fixed addresses).
> So we don't actually need the encoding changes at the moment.
> 
> I'm inclined to just leave the encoding out for now.  That
> way, we don't carry around the untested code, and if somebody
> eventually tries to use it, the attempt to set resources will
> fail, and we can write the code and test it at that time.
Ok, lets wait till it actually is used.

Thanks,
Shaohua



-------------------------------------------------------
SF.Net email is sponsored by:
Tame your development challenges with Apache's Geronimo App Server. 
Download it for free - -and be entered to win a 42" plasma tv or your very
own Sony(tm)PSP.  Click here to play: http://sourceforge.net/geronimo.php

[PATCH] PNPACPI: handle address descriptors in _PRS

[Bjorn Helgaas]

PNPACPI complained about and ignored devices with ADDRESS16, ADDRESS32, or
ADDRESS64 descriptors in _PRS.  HP firmware uses them for built-in serial
ports, so this patch adds support for parsing these descriptors from _PRS.

Note that this does not add the corresponding support for encoding them
in preparation for _SRS, because I don't have any machine that supports
_SRS on these descriptors, so I couldn't test that support.  Attempts to
encode them will cause a warning and an -EINVAL return.

Previous discussion:
    http://sourceforge.net/mailarchive/forum.php?thread_id=8250154&forum_id=6102

Signed-off-by: Bjorn Helgaas <bjorn.helgaas-VXdhtT5mjnY@public.gmane.org>

Index: work-vga2/drivers/pnp/pnpacpi/rsparser.c
===================================================================
--- work-vga2.orig/drivers/pnp/pnpacpi/rsparser.c	2005-09-20 16:12:47.000000000 -0600
+++ work-vga2/drivers/pnp/pnpacpi/rsparser.c	2005-09-23 09:45:37.000000000 -0600
@@ -453,6 +453,45 @@
 	return;
 }
 
+static void
+pnpacpi_parse_address_option(struct pnp_option *option, struct acpi_resource *r)
+{
+	struct acpi_resource_address64 addr, *p = &addr;
+	acpi_status status;
+	struct pnp_mem * mem;
+	struct pnp_port * port;
+
+	status = acpi_resource_to_address64(r, p);
+	if (!ACPI_SUCCESS(status)) {
+		pnp_warn("PnPACPI: failed to convert resource type %d", r->id);
+		return;
+	}
+
+	if (p->address_length == 0)
+		return;
+
+	if (p->resource_type == ACPI_MEMORY_RANGE) {
+		mem = kcalloc(1, sizeof(struct pnp_mem), GFP_KERNEL);
+		if (!mem)
+			return;
+		mem->min = mem->max = p->min_address_range;
+		mem->size = p->address_length;
+		mem->align = 0;
+		mem->flags = (p->attribute.memory.read_write_attribute ==
+		    ACPI_READ_WRITE_MEMORY) ? IORESOURCE_MEM_WRITEABLE : 0;
+		pnp_register_mem_resource(option,mem);
+	} else if (p->resource_type == ACPI_IO_RANGE) {
+		port = kcalloc(1, sizeof(struct pnp_port), GFP_KERNEL);
+		if (!port)
+			return;
+		port->min = port->max = p->min_address_range;
+		port->size = p->address_length;
+		port->align = 0;
+		port->flags = PNP_PORT_FLAG_FIXED;
+		pnp_register_port_resource(option,port);
+	}
+}
+
 struct acpipnp_parse_option_s {
 	struct pnp_option *option;
 	struct pnp_option *option_independent;
@@ -495,6 +534,11 @@
 			pnpacpi_parse_fixed_mem32_option(option,
 				&res->data.fixed_memory32);
 			break;
+		case ACPI_RSTYPE_ADDRESS16:
+		case ACPI_RSTYPE_ADDRESS32:
+		case ACPI_RSTYPE_ADDRESS64:
+			pnpacpi_parse_address_option(option, res);
+			break;
 		case ACPI_RSTYPE_START_DPF:
 			switch (res->data.start_dpf.compatibility_priority) {
 				case ACPI_GOOD_CONFIGURATION:
@@ -568,11 +612,9 @@
 	case ACPI_RSTYPE_MEM24:
 	case ACPI_RSTYPE_MEM32:
 	case ACPI_RSTYPE_FIXED_MEM32:
-#if 0
 	case ACPI_RSTYPE_ADDRESS16:
 	case ACPI_RSTYPE_ADDRESS32:
 	case ACPI_RSTYPE_ADDRESS64:
-#endif
 		(*res_cnt) ++;
 	default:
 		return AE_OK;
@@ -593,11 +635,9 @@
 	case ACPI_RSTYPE_MEM24:
 	case ACPI_RSTYPE_MEM32:
 	case ACPI_RSTYPE_FIXED_MEM32:
-#if 0
 	case ACPI_RSTYPE_ADDRESS16:
 	case ACPI_RSTYPE_ADDRESS32:
 	case ACPI_RSTYPE_ADDRESS64:
-#endif
 		(*resource)->id = res->id;
 		(*resource)++;
 	default:


-------------------------------------------------------
SF.Net email is sponsored by:
Tame your development challenges with Apache's Geronimo App Server. 
Download it for free - -and be entered to win a 42" plasma tv or your very
own Sony(tm)PSP.  Click here to play: http://sourceforge.net/geronimo.php