[PATCH] - Increase PNP_MAX_PORT. ACPI devices can have a lot IO resource declarations

[PATCH] - Increase PNP_MAX_PORT. ACPI devices can have a lot IO resource declarations

[Thomas Renninger]

PNP0C02 devices normally have a lot more IO port declarations than
currently defined in PNP_MAX_PORT

For checking, have a look at your disassembled DSDT, and look out for
the _CRS function (and/or a ResourceTemplate often called CRS).

Here an  example:
IO (Decode16,0x0010,             // Range Minimum
             0x0010,             // Range Maximum
             0x00,               // Alignment
             0x10,               // Length
)
IO (Decode16,
             0x0022,             // Range Minimum
             0x0022,             // Range Maximum
             0x00,               // Alignment
             0x1E,               // Length
)
...
IO (Decode16,
             0x0000,             // Range Minimum
             0x0000,             // Range Maximum
             0x00,               // Alignment
             0x00,               // Length
_Y06)
IO (Decode16,
             0x0000,             // Range Minimum
             0x0000,             // Range Maximum
             0x00,               // Alignment
             0x00,               // Length
_Y07)

The latter zeroed (_Y07) declarations are filled with dynamic values in
_CRS and also need an PNP IO port reserved.

On this machine (x86_64 workstation) more then 20 IO resource
declarations exist for one PNP0C02 device.
It could be that the suggested 32 is still not big enough.

Most of the ports are below 0x100, which get ignored by the pnp layer
anyways (cmp. with drivers/pnp/system.c:reserve_resources_of_dev() ).
These could already be ignore in
drivers/pnp/pnpacpi/rsparser.c:pnpacpi_parse_allocated_ioresource()
not filling up PNP_MAX_PORTs, but this is not that nice and probably
will get reverted at some later point when standard PC hardware (below
0x100, pics, kbd, timer, dma, ...) also gets requested by ACPI
sublayers.


I also wonder whether other limits like:
 #define PNP_MAX_MEM        4
 #define PNP_MAX_IRQ        2
 #define PNP_MAX_DMA        2
could get exceeded with pnpacpi?

I don't know what the theoretical max for ports on one device is, but
IMO the static array should get something dynamically allocated?

Something like:

+++ linux-2.6.22.1/include/linux/pnp.h
@@ -14,7 +14,7 @@
-#define PNP_MAX_PORT           8
+#define PNP_MAX_PORT           256

 struct pnp_resource_table {
-       struct resource port_resource[PNP_MAX_PORT];
+       struct resource *port_resource[PNP_MAX_PORT];
        struct resource mem_resource[PNP_MAX_MEM];

or
-       struct resource port_resource[PNP_MAX_PORT];
+       struct resource **port_resource;

and then k(z)alloc the resource structs...

BTW: This was the reason why:
  50e0-50ef : amd756_smbus
got not reserved on the patch:
"Identify native drivers and ACPI subsystem accessing the same
resources"
I sent some days ago. In 2.6.18 it seems this got handled (correctly) by
/drivers/acpi/motherboard which got replaced with pnpacpi system driver.

This would be the less intrusive short term fix:

------------------

Increase PNP_MAX_PORT. ACPI devices can have a lot IO resource declarations

Also print a message if PNP_DEBUG is set if we run out of PNP_MAX_PORTS.

Signed-off-by: Thomas Renninger <trenn@suse.de>

---
 drivers/pnp/pnpacpi/rsparser.c |    3 ++-
 include/linux/pnp.h            |    2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

Index: linux-2.6.22.1/include/linux/pnp.h
===================================================================
--- linux-2.6.22.1.orig/include/linux/pnp.h
+++ linux-2.6.22.1/include/linux/pnp.h
@@ -14,7 +14,7 @@
 #include <linux/errno.h>
 #include <linux/mod_devicetable.h>
 
-#define PNP_MAX_PORT		8
+#define PNP_MAX_PORT		32
 #define PNP_MAX_MEM		4
 #define PNP_MAX_IRQ		2
 #define PNP_MAX_DMA		2
Index: linux-2.6.22.1/drivers/pnp/pnpacpi/rsparser.c
===================================================================
--- linux-2.6.22.1.orig/drivers/pnp/pnpacpi/rsparser.c
+++ linux-2.6.22.1/drivers/pnp/pnpacpi/rsparser.c
@@ -185,7 +185,8 @@ pnpacpi_parse_allocated_ioresource(struc
 		}
 		res->port_resource[i].start = io;
 		res->port_resource[i].end = io + len - 1;
-	}
+	} else
+		pnp_dbg("Run out of pnp ports - MAX_PNP_PORT must be increased");
 }
 
 static void

Re: [PATCH] - Increase PNP_MAX_PORT. ACPI devices can have a lot IO resource declarations

[Jean Delvare]

Hi Thomas,

On Mon, 16 Jul 2007 16:21:07 +0200, Thomas Renninger wrote:
> PNP0C02 devices normally have a lot more IO port declarations than
> currently defined in PNP_MAX_PORT
> 
> For checking, have a look at your disassembled DSDT, and look out for
> the _CRS function (and/or a ResourceTemplate often called CRS).
> 
> Here an  example:
> IO (Decode16,0x0010,             // Range Minimum
>              0x0010,             // Range Maximum
>              0x00,               // Alignment
>              0x10,               // Length
> )
> IO (Decode16,
>              0x0022,             // Range Minimum
>              0x0022,             // Range Maximum
>              0x00,               // Alignment
>              0x1E,               // Length
> )
> ...
> IO (Decode16,
>              0x0000,             // Range Minimum
>              0x0000,             // Range Maximum
>              0x00,               // Alignment
>              0x00,               // Length
> _Y06)
> IO (Decode16,
>              0x0000,             // Range Minimum
>              0x0000,             // Range Maximum
>              0x00,               // Alignment
>              0x00,               // Length
> _Y07)
> 
> The latter zeroed (_Y07) declarations are filled with dynamic values in
> _CRS and also need an PNP IO port reserved.
> 
> On this machine (x86_64 workstation) more then 20 IO resource
> declarations exist for one PNP0C02 device.
> It could be that the suggested 32 is still not big enough.

I just tested on my own workstation, and 8 wasn't enough for me either:
there are 12 resources listed for the PNP0C02 device on my workstation.

After applying your patch, I get the following differences in /proc/ioports:

--- ioports.before      2007-07-16 18:29:30.000000000 +0200
+++ ioports.after       2007-07-16 18:39:05.000000000 +0200
@@ -13,7 +13,8 @@
 01f0-01f7 : 0000:00:0f.0
   01f0-01f7 : ide0
 0290-0297 : f71805f
-  0295-0296 : f71805f
+  0290-0297 : pnp 00:03
+    0295-0296 : f71805f
 0376-0376 : 0000:00:0f.0
   0376-0376 : ide1
 0378-037a : parport0
@@ -21,6 +22,8 @@
 03f6-03f6 : 0000:00:0f.0
   03f6-03f6 : ide0
 03f8-03ff : serial
+04d0-04d1 : pnp 00:03
+0800-0805 : pnp 00:03
 0cf8-0cff : PCI conf1
 4000-407f : pnp 00:02
   4000-4003 : ACPI PM1a_EVT_BLK

> (...)
> ------------------
> 
> Increase PNP_MAX_PORT. ACPI devices can have a lot IO resource declarations
> 
> Also print a message if PNP_DEBUG is set if we run out of PNP_MAX_PORTS.
> 
> Signed-off-by: Thomas Renninger <trenn@suse.de>
> 
> ---
>  drivers/pnp/pnpacpi/rsparser.c |    3 ++-
>  include/linux/pnp.h            |    2 +-
>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> Index: linux-2.6.22.1/include/linux/pnp.h
> ===================================================================
> --- linux-2.6.22.1.orig/include/linux/pnp.h
> +++ linux-2.6.22.1/include/linux/pnp.h
> @@ -14,7 +14,7 @@
>  #include <linux/errno.h>
>  #include <linux/mod_devicetable.h>
>  
> -#define PNP_MAX_PORT		8
> +#define PNP_MAX_PORT		32
>  #define PNP_MAX_MEM		4
>  #define PNP_MAX_IRQ		2
>  #define PNP_MAX_DMA		2
> Index: linux-2.6.22.1/drivers/pnp/pnpacpi/rsparser.c
> ===================================================================
> --- linux-2.6.22.1.orig/drivers/pnp/pnpacpi/rsparser.c
> +++ linux-2.6.22.1/drivers/pnp/pnpacpi/rsparser.c
> @@ -185,7 +185,8 @@ pnpacpi_parse_allocated_ioresource(struc
>  		}
>  		res->port_resource[i].start = io;
>  		res->port_resource[i].end = io + len - 1;
> -	}
> +	} else
> +		pnp_dbg("Run out of pnp ports - MAX_PNP_PORT must be increased");

PNP_MAX_PORT, not MAX_PNP_PORT.

>  }
>  
>  static void

I like this patch and I'd like to see it upstream soon. I think it can
even be improved:

* If we run out of pnp ports, we should print an error message rather
than a debug message. If it takes CONFIG_PNP_DEBUG=y to get reports
that the max is too low, we won't have many reports.

* Can we have similar messages for PNP_MAX_MEM, PNP_MAX_IRQ and
PNP_MAX_DMA? I think that we really want to know when we hit limits for
these too.

Thanks,
-- 
Jean Delvare

Re: [PATCH] - Increase PNP_MAX_PORT. ACPI devices can have a lot IO resource declarations

[Bjorn Helgaas]

On Monday 16 July 2007 08:21:07 am Thomas Renninger wrote:
> PNP0C02 devices normally have a lot more IO port declarations than
> currently defined in PNP_MAX_PORT

Yes.

> I also wonder whether other limits like:
>  #define PNP_MAX_MEM        4
>  #define PNP_MAX_IRQ        2
>  #define PNP_MAX_DMA        2
> could get exceeded with pnpacpi?

Definitely.  I think the current limits come from the PNP ISA spec
(sec 4.6).  I don't see similar limits in the PNPBIOS or ACPI specs,
so ideally I think they should be dynamically allocated as you suggest.

HPET devices often use more than two IRQs, so I worked up the following
patch a while ago.  I didn't post it because Adam had plans for a better
fix.  But I suspect he's otherwise occupied now :-)

It doesn't change PNP_MAX_PORT, so that would have to be updated.




PNP: increase number of resources devices can use

PNPACPI and PNPBIOS devices can use more than 2 IRQs, so bump PNP_MAX_IRQ
from 2 to 4.  For example, HPET devices can have up to 32 timers, each with
an IRQ.  (Current HPETs typically have 3 timers.)

Add ISAPNP_MAX_PORT, ISAPNP_MAX_MEM, ISAPNP_MAX_IRQ, and ISAPNP_MAX_DMA
because ISAPNP does limit the number of resources a device can use.

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>

Index: work-mm1/drivers/pnp/isapnp/core.c
===================================================================
--- work-mm1.orig/drivers/pnp/isapnp/core.c	2005-08-09 10:07:45.000000000 -0600
+++ work-mm1/drivers/pnp/isapnp/core.c	2005-08-09 14:03:03.000000000 -0600
@@ -950,28 +950,28 @@
 
 	dev->active = isapnp_read_byte(ISAPNP_CFG_ACTIVATE);
 	if (dev->active) {
-		for (tmp = 0; tmp < PNP_MAX_PORT; tmp++) {
+		for (tmp = 0; tmp < ISAPNP_MAX_PORT; tmp++) {
 			ret = isapnp_read_word(ISAPNP_CFG_PORT + (tmp << 1));
 			if (!ret)
 				continue;
 			res->port_resource[tmp].start = ret;
 			res->port_resource[tmp].flags = IORESOURCE_IO;
 		}
-		for (tmp = 0; tmp < PNP_MAX_MEM; tmp++) {
+		for (tmp = 0; tmp < ISAPNP_MAX_MEM; tmp++) {
 			ret = isapnp_read_word(ISAPNP_CFG_MEM + (tmp << 3)) << 8;
 			if (!ret)
 				continue;
 			res->mem_resource[tmp].start = ret;
 			res->mem_resource[tmp].flags = IORESOURCE_MEM;
 		}
-		for (tmp = 0; tmp < PNP_MAX_IRQ; tmp++) {
+		for (tmp = 0; tmp < ISAPNP_MAX_IRQ; tmp++) {
 			ret = (isapnp_read_word(ISAPNP_CFG_IRQ + (tmp << 1)) >> 8);
 			if (!ret)
 				continue;
 			res->irq_resource[tmp].start = res->irq_resource[tmp].end = ret;
 			res->irq_resource[tmp].flags = IORESOURCE_IRQ;
 		}
-		for (tmp = 0; tmp < PNP_MAX_DMA; tmp++) {
+		for (tmp = 0; tmp < ISAPNP_MAX_DMA; tmp++) {
 			ret = isapnp_read_byte(ISAPNP_CFG_DMA + tmp);
 			if (ret == 4)
 				continue;
@@ -998,17 +998,17 @@
 
 	isapnp_cfg_begin(dev->card->number, dev->number);
 	dev->active = 1;
-	for (tmp = 0; tmp < PNP_MAX_PORT && (res->port_resource[tmp].flags & (IORESOURCE_IO | IORESOURCE_UNSET)) == IORESOURCE_IO; tmp++)
+	for (tmp = 0; tmp < ISAPNP_MAX_PORT && (res->port_resource[tmp].flags & (IORESOURCE_IO | IORESOURCE_UNSET)) == IORESOURCE_IO; tmp++)
 		isapnp_write_word(ISAPNP_CFG_PORT+(tmp<<1), res->port_resource[tmp].start);
-	for (tmp = 0; tmp < PNP_MAX_IRQ && (res->irq_resource[tmp].flags & (IORESOURCE_IRQ | IORESOURCE_UNSET)) == IORESOURCE_IRQ; tmp++) {
+	for (tmp = 0; tmp < ISAPNP_MAX_IRQ && (res->irq_resource[tmp].flags & (IORESOURCE_IRQ | IORESOURCE_UNSET)) == IORESOURCE_IRQ; tmp++) {
 		int irq = res->irq_resource[tmp].start;
 		if (irq == 2)
 			irq = 9;
 		isapnp_write_byte(ISAPNP_CFG_IRQ+(tmp<<1), irq);
 	}
-	for (tmp = 0; tmp < PNP_MAX_DMA && (res->dma_resource[tmp].flags & (IORESOURCE_DMA | IORESOURCE_UNSET)) == IORESOURCE_DMA; tmp++)
+	for (tmp = 0; tmp < ISAPNP_MAX_DMA && (res->dma_resource[tmp].flags & (IORESOURCE_DMA | IORESOURCE_UNSET)) == IORESOURCE_DMA; tmp++)
 		isapnp_write_byte(ISAPNP_CFG_DMA+tmp, res->dma_resource[tmp].start);
-	for (tmp = 0; tmp < PNP_MAX_MEM && (res->mem_resource[tmp].flags & (IORESOURCE_MEM | IORESOURCE_UNSET)) == IORESOURCE_MEM; tmp++)
+	for (tmp = 0; tmp < ISAPNP_MAX_MEM && (res->mem_resource[tmp].flags & (IORESOURCE_MEM | IORESOURCE_UNSET)) == IORESOURCE_MEM; tmp++)
 		isapnp_write_word(ISAPNP_CFG_MEM+(tmp<<3), (res->mem_resource[tmp].start >> 8) & 0xffff);
 	/* FIXME: We aren't handling 32bit mems properly here */
 	isapnp_activate(dev->number);
Index: work-mm1/include/linux/isapnp.h
===================================================================
--- work-mm1.orig/include/linux/isapnp.h	2005-06-17 13:48:29.000000000 -0600
+++ work-mm1/include/linux/isapnp.h	2005-08-09 14:35:31.000000000 -0600
@@ -24,6 +24,7 @@
 
 #include <linux/config.h>
 #include <linux/errno.h>
+#include <linux/kernel.h>
 #include <linux/pnp.h>
 
 /*
@@ -36,6 +37,11 @@
 #define ISAPNP_CFG_IRQ			0x70	/* 2 * word */
 #define ISAPNP_CFG_DMA			0x74	/* 2 * byte */
 
+#define ISAPNP_MAX_PORT			min(8, PNP_MAX_PORT)
+#define ISAPNP_MAX_MEM			min(4, PNP_MAX_MEM)
+#define ISAPNP_MAX_IRQ			min(2, PNP_MAX_IRQ)
+#define ISAPNP_MAX_DMA			min(2, PNP_MAX_DMA)
+
 /*
  *
  */
Index: work-mm1/include/linux/pnp.h
===================================================================
--- work-mm1.orig/include/linux/pnp.h	2005-08-09 10:07:46.000000000 -0600
+++ work-mm1/include/linux/pnp.h	2005-08-09 14:01:08.000000000 -0600
@@ -16,7 +16,7 @@
 
 #define PNP_MAX_PORT		8
 #define PNP_MAX_MEM		4
-#define PNP_MAX_IRQ		2
+#define PNP_MAX_IRQ		4
 #define PNP_MAX_DMA		2
 #define PNP_NAME_LEN		50
 
Index: work-mm1/drivers/pnp/pnpacpi/rsparser.c
===================================================================
--- work-mm1.orig/drivers/pnp/pnpacpi/rsparser.c	2005-08-09 10:07:45.000000000 -0600
+++ work-mm1/drivers/pnp/pnpacpi/rsparser.c	2005-08-09 14:29:59.000000000 -0600
@@ -72,6 +72,10 @@
 	}
 }
 
+/*
+ * ACPI doesn't limit the number of resources a device can use, so
+ * we can use the generic PNP limits (PNP_MAX_IRQ, etc).
+ */
 static void
 pnpacpi_parse_allocated_irqresource(struct pnp_resource_table * res, u32 gsi,
 	int edge_level, int active_high_low)
Index: work-mm1/drivers/pnp/pnpbios/rsparser.c
===================================================================
--- work-mm1.orig/drivers/pnp/pnpbios/rsparser.c	2005-08-09 10:07:45.000000000 -0600
+++ work-mm1/drivers/pnp/pnpbios/rsparser.c	2005-08-09 14:29:37.000000000 -0600
@@ -51,6 +51,10 @@
  * Allocated Resources
  */
 
+/*
+ * PNPBIOS doesn't limit the number of resources a device can use, so
+ * we can use the generic PNP limits (PNP_MAX_IRQ, etc).
+ */
 static void
 pnpbios_parse_allocated_irqresource(struct pnp_resource_table * res, int irq)
 {

Re: [PATCH] - Increase PNP_MAX_PORT. ACPI devices can have a lot IO resource declarations

[Thomas Renninger]

On Tue, 2007-07-17 at 09:49 -0600, Bjorn Helgaas wrote:
> On Monday 16 July 2007 08:21:07 am Thomas Renninger wrote:
> > PNP0C02 devices normally have a lot more IO port declarations than
> > currently defined in PNP_MAX_PORT
> 
> Yes.
> 
> > I also wonder whether other limits like:
> >  #define PNP_MAX_MEM        4
> >  #define PNP_MAX_IRQ        2
> >  #define PNP_MAX_DMA        2
> > could get exceeded with pnpacpi?
> 
> Definitely.  I think the current limits come from the PNP ISA spec
> (sec 4.6).  I don't see similar limits in the PNPBIOS or ACPI specs,
> so ideally I think they should be dynamically allocated as you suggest.
> 
I wanted to implement the dynamic approach and used a dynamically
allocated array, filled up from beginning. While this is close to the
current implementation I thought this is the easiest sufficient way...
(I also only did this for io ports where most mem is wasted).
Now I am thinking about hotplug (e.g. if a SSDT with resources gets
hot-added, removed)... If a device can vanish, the array must get
reordered, not a really well fitting structure, a list (a pnp specific
set up, or from include/linux/list.h?) should be better?

I only have a half baken (even not compiling, I already saw several bugs
myself in this one while flying over...) patch. It shows what I wanted
to do.  The important part is in include/linux/pnp.h. After sleeping
over it, I fear I am doing this work for nothing...

As this is touching nearly every file in drivers/pnp I'd like to have
some advice/discussion, before I start (over and over) again...

If someone screams who knows that part well and wanted to add this
anyway..., I am busy enough :) (,but I can do it if not...)

Thanks,

   Thomas

---
 drivers/pnp/interface.c        |   16 ++++++++++------
 drivers/pnp/isapnp/core.c      |    9 +++++----
 drivers/pnp/manager.c          |   13 ++++++++-----
 drivers/pnp/pnpacpi/rsparser.c |   17 +++++++++--------
 drivers/pnp/pnpbios/rsparser.c |   14 ++++++++------
 drivers/pnp/resource.c         |   16 ++++++++--------
 drivers/pnp/system.c           |    2 +-
 include/linux/pnp.h            |   12 +++++++-----
 8 files changed, 56 insertions(+), 43 deletions(-)

Index: linux-2.6.22.1/drivers/pnp/interface.c
===================================================================
--- linux-2.6.22.1.orig/drivers/pnp/interface.c
+++ linux-2.6.22.1/drivers/pnp/interface.c
@@ -258,7 +258,7 @@ static ssize_t pnp_show_current_resource
 	else
 		pnp_printf(buffer,"disabled\n");
 
-	for (i = 0; i < PNP_MAX_PORT; i++) {
+	for (i = 0; pnp_port_res_pointer(dev, i); i++) {
 		if (pnp_port_valid(dev, i)) {
 			pnp_printf(buffer,"io");
 			if (pnp_port_flags(dev, i) & IORESOURCE_DISABLED)
@@ -370,19 +370,23 @@ pnp_set_current_resources(struct device 
 				buf += 2;
 				while (isspace(*buf))
 					++buf;
-				dev->res.port_resource[nport].start = simple_strtoul(buf,&buf,0);
+				pnp_port_start(dev,nport) =
+					simple_strtoul(buf,&buf,0);
 				while (isspace(*buf))
 					++buf;
 				if(*buf == '-') {
 					buf += 1;
 					while (isspace(*buf))
 						++buf;
-					dev->res.port_resource[nport].end = simple_strtoul(buf,&buf,0);
+					pnp_port_end(dev,nport) =
+						simple_strtoul(buf,&buf,0);
 				} else
-					dev->res.port_resource[nport].end = dev->res.port_resource[nport].start;
-				dev->res.port_resource[nport].flags = IORESOURCE_IO;
+					pnp_port_end(dev,nport) =
+						pnp_port_start(dev,nport);
+				pnp_port_flags(dev,nport) = IORESOURCE_IO;
 				nport++;
-				if (nport >= PNP_MAX_PORT)
+				if (!pnp_port_res_pointer(dev,nport) ||
+				    nport >= PNP_MAX_PORT)
 					break;
 				continue;
 			}
Index: linux-2.6.22.1/drivers/pnp/manager.c
===================================================================
--- linux-2.6.22.1.orig/drivers/pnp/manager.c
+++ linux-2.6.22.1/drivers/pnp/manager.c
@@ -26,18 +26,21 @@ static int pnp_assign_port(struct pnp_de
 		return -EINVAL;
 
 	if (idx >= PNP_MAX_PORT) {
-		pnp_err("More than 4 ports is incompatible with pnp specifications.");
+		pnp_err("Run out of pnp ports\n");
 		/* pretend we were successful so at least the manager won't try again */
 		return 1;
 	}
 
 	/* check if this resource has been manually set, if so skip */
-	if (!(dev->res.port_resource[idx].flags & IORESOURCE_AUTO))
+	if (pnp_port_res_pointer(dev, idx) && !(dev->res.port_resource[idx].flags & IORESOURCE_AUTO))
 		return 1;
 
-	start = &dev->res.port_resource[idx].start;
-	end = &dev->res.port_resource[idx].end;
-	flags = &dev->res.port_resource[idx].flags;
+	if (!pnp_port_res_pointer(dev, idx))
+		pnp_port_res_pointer(dev, idx) = kzalloc(sizeof(struct resource), GFP_KERNEL);
+
+	start = &pnp_port_res_pointer(dev, idx)->start;
+	end =   &pnp_port_res_pointer(dev, idx)->end;
+	flags = &pnp_port_res_pointer(dev, idx)->flags;
 
 	/* set the initial values */
 	*flags |= rule->flags | IORESOURCE_IO;
Index: linux-2.6.22.1/drivers/pnp/resource.c
===================================================================
--- linux-2.6.22.1.orig/drivers/pnp/resource.c
+++ linux-2.6.22.1/drivers/pnp/resource.c
@@ -241,11 +241,11 @@ int pnp_check_port(struct pnp_dev * dev,
 	int tmp;
 	struct pnp_dev *tdev;
 	resource_size_t *port, *end, *tport, *tend;
-	port = &dev->res.port_resource[idx].start;
-	end = &dev->res.port_resource[idx].end;
+	port = &(pnp_port_start(dev,idx));
+	end = &(pnp_port_end(dev,idx));
 
 	/* if the resource doesn't exist, don't complain about it */
-	if (cannot_compare(dev->res.port_resource[idx].flags))
+	if (cannot_compare(pnp_port_flags(dev,idx)))
 		return 1;
 
 	/* check if the resource is already in use, skip if the
@@ -264,10 +264,10 @@ int pnp_check_port(struct pnp_dev * dev,
 	}
 
 	/* check for internal conflicts */
-	for (tmp = 0; tmp < PNP_MAX_PORT && tmp != idx; tmp++) {
-		if (dev->res.port_resource[tmp].flags & IORESOURCE_IO) {
-			tport = &dev->res.port_resource[tmp].start;
-			tend = &dev->res.port_resource[tmp].end;
+	for (tmp = 0; pnp_port_res_pointer(dev, tmp) && tmp != idx; tmp++) {
+		if (pnp_port_flags(dev,tmp) & IORESOURCE_IO) {
+			tport = &(pnp_port_start(dev,tmp));
+			tend = &(pnp_port_end(dev,tmp));
 			if (ranged_conflict(port,end,tport,tend))
 				return 0;
 		}
@@ -277,7 +277,7 @@ int pnp_check_port(struct pnp_dev * dev,
 	pnp_for_each_dev(tdev) {
 		if (tdev == dev)
 			continue;
-		for (tmp = 0; tmp < PNP_MAX_PORT; tmp++) {
+		for (tmp = 0;  pnp_port_res_pointer(dev, tmp); tmp++) {
 			if (tdev->res.port_resource[tmp].flags & IORESOURCE_IO) {
 				if (cannot_compare(tdev->res.port_resource[tmp].flags))
 					continue;
Index: linux-2.6.22.1/drivers/pnp/system.c
===================================================================
--- linux-2.6.22.1.orig/drivers/pnp/system.c
+++ linux-2.6.22.1/drivers/pnp/system.c
@@ -55,7 +55,7 @@ static void reserve_resources_of_dev(con
 {
 	int i;
 
-	for (i = 0; i < PNP_MAX_PORT; i++) {
+	for (i = 0; pnp_port_res_pointer(dev, i); i++) {
 		if (!pnp_port_valid(dev, i))
 			continue;
 		if (pnp_port_start(dev, i) == 0)
Index: linux-2.6.22.1/include/linux/pnp.h
===================================================================
--- linux-2.6.22.1.orig/include/linux/pnp.h
+++ linux-2.6.22.1/include/linux/pnp.h
@@ -14,7 +14,7 @@
 #include <linux/errno.h>
 #include <linux/mod_devicetable.h>
 
-#define PNP_MAX_PORT		8
+#define PNP_MAX_PORT		256
 #define PNP_MAX_MEM		4
 #define PNP_MAX_IRQ		2
 #define PNP_MAX_DMA		2
@@ -29,9 +29,11 @@ struct pnp_dev;
  */
 
 /* Use these instead of directly reading pnp_dev to get resource information */
-#define pnp_port_start(dev,bar)   ((dev)->res.port_resource[(bar)].start)
-#define pnp_port_end(dev,bar)     ((dev)->res.port_resource[(bar)].end)
-#define pnp_port_flags(dev,bar)   ((dev)->res.port_resource[(bar)].flags)
+#define pnp_port_res_pointer(dev,bar)   (bar >= PNP_MAX_PORT ? NULL : \
+					 (dev)->res.port_resource[(bar)])
+#define pnp_port_start(dev,bar)   ((dev)->res.port_resource[(bar)]->start)
+#define pnp_port_end(dev,bar)     ((dev)->res.port_resource[(bar)]->end)
+#define pnp_port_flags(dev,bar)   ((dev)->res.port_resource[(bar)]->flags)
 #define pnp_port_valid(dev,bar) \
 	((pnp_port_flags((dev),(bar)) & (IORESOURCE_IO | IORESOURCE_UNSET)) \
 		== IORESOURCE_IO)
@@ -121,7 +123,7 @@ struct pnp_option {
 };
 
 struct pnp_resource_table {
-	struct resource port_resource[PNP_MAX_PORT];
+	struct resource *port_resource[PNP_MAX_PORT];
 	struct resource mem_resource[PNP_MAX_MEM];
 	struct resource dma_resource[PNP_MAX_DMA];
 	struct resource irq_resource[PNP_MAX_IRQ];
Index: linux-2.6.22.1/drivers/pnp/isapnp/core.c
===================================================================
--- linux-2.6.22.1.orig/drivers/pnp/isapnp/core.c
+++ linux-2.6.22.1/drivers/pnp/isapnp/core.c
@@ -954,12 +954,13 @@ static int isapnp_read_resources(struct 
 
 	dev->active = isapnp_read_byte(ISAPNP_CFG_ACTIVATE);
 	if (dev->active) {
-		for (tmp = 0; tmp < PNP_MAX_PORT; tmp++) {
+		for (tmp = 0; res->port_resource[tmp] && tmp < PNP_MAX_PORT;
+		     tmp++) {
 			ret = isapnp_read_word(ISAPNP_CFG_PORT + (tmp << 1));
 			if (!ret)
 				continue;
-			res->port_resource[tmp].start = ret;
-			res->port_resource[tmp].flags = IORESOURCE_IO;
+			res->port_resource[tmp]->start = ret;
+			res->port_resource[tmp]->flags = IORESOURCE_IO;
 		}
 		for (tmp = 0; tmp < PNP_MAX_MEM; tmp++) {
 			ret = isapnp_read_word(ISAPNP_CFG_MEM + (tmp << 3)) << 8;
@@ -1002,7 +1003,7 @@ static int isapnp_set_resources(struct p
 
 	isapnp_cfg_begin(dev->card->number, dev->number);
 	dev->active = 1;
-	for (tmp = 0; tmp < PNP_MAX_PORT && (res->port_resource[tmp].flags & (IORESOURCE_IO | IORESOURCE_UNSET)) == IORESOURCE_IO; tmp++)
+	for (tmp = 0; res->port_resource[tmp] && tmp < PNP_MAX_PORT && (res->port_resource[tmp].flags & (IORESOURCE_IO | IORESOURCE_UNSET)) == IORESOURCE_IO; tmp++)
 		isapnp_write_word(ISAPNP_CFG_PORT+(tmp<<1), res->port_resource[tmp].start);
 	for (tmp = 0; tmp < PNP_MAX_IRQ && (res->irq_resource[tmp].flags & (IORESOURCE_IRQ | IORESOURCE_UNSET)) == IORESOURCE_IRQ; tmp++) {
 		int irq = res->irq_resource[tmp].start;
Index: linux-2.6.22.1/drivers/pnp/pnpacpi/rsparser.c
===================================================================
--- linux-2.6.22.1.orig/drivers/pnp/pnpacpi/rsparser.c
+++ linux-2.6.22.1/drivers/pnp/pnpacpi/rsparser.c
@@ -172,19 +172,20 @@ pnpacpi_parse_allocated_ioresource(struc
 	u64 io, u64 len, int io_decode)
 {
 	int i = 0;
-	while (!(res->port_resource[i].flags & IORESOURCE_UNSET) &&
-			i < PNP_MAX_PORT)
+	while (res->port_resource[i] &&
+	       !(res->port_resource[i]->flags & IORESOURCE_UNSET) &&
+	       i < PNP_MAX_PORT)
 		i++;
-	if (i < PNP_MAX_PORT) {
-		res->port_resource[i].flags = IORESOURCE_IO;  // Also clears _UNSET flag
+	if (res->port_resource[i] && i < PNP_MAX_PORT) {
+		res->port_resource[i]->flags = IORESOURCE_IO;  // Also clears _UNSET flag
 		if (io_decode == ACPI_DECODE_16)
-			res->port_resource[i].flags |= PNP_PORT_FLAG_16BITADDR;
+			res->port_resource[i]->flags |= PNP_PORT_FLAG_16BITADDR;
 		if (len <= 0 || (io + len -1) >= 0x10003) {
-			res->port_resource[i].flags |= IORESOURCE_DISABLED;
+			res->port_resource[i]->flags |= IORESOURCE_DISABLED;
 			return;
 		}
-		res->port_resource[i].start = io;
-		res->port_resource[i].end = io + len - 1;
+		res->port_resource[i]->start = io;
+		res->port_resource[i]->end = io + len - 1;
 	}
 }
 
Index: linux-2.6.22.1/drivers/pnp/pnpbios/rsparser.c
===================================================================
--- linux-2.6.22.1.orig/drivers/pnp/pnpbios/rsparser.c
+++ linux-2.6.22.1/drivers/pnp/pnpbios/rsparser.c
@@ -91,15 +91,17 @@ static void
 pnpbios_parse_allocated_ioresource(struct pnp_resource_table * res, int io, int len)
 {
 	int i = 0;
-	while (!(res->port_resource[i].flags & IORESOURCE_UNSET) && i < PNP_MAX_PORT) i++;
-	if (i < PNP_MAX_PORT) {
-		res->port_resource[i].flags = IORESOURCE_IO;  // Also clears _UNSET flag
+	while (res->port_resource[i] &&
+	       !(res->port_resource[i]->flags & IORESOURCE_UNSET) &&
+	       i < PNP_MAX_PORT) i++;
+	if (res->port_resource[i] && i < PNP_MAX_PORT) {
+		res->port_resource[i]->flags = IORESOURCE_IO;  // Also clears _UNSET flag
 		if (len <= 0 || (io + len -1) >= 0x10003) {
-			res->port_resource[i].flags |= IORESOURCE_DISABLED;
+			res->port_resource[i]->flags |= IORESOURCE_DISABLED;
 			return;
 		}
-		res->port_resource[i].start = (unsigned long) io;
-		res->port_resource[i].end = (unsigned long)(io + len - 1);
+		res->port_resource[i]->start = (unsigned long) io;
+		res->port_resource[i]->end = (unsigned long)(io + len - 1);
 	}
 }
 

Re: [PATCH] - Increase PNP_MAX_PORT. ACPI devices can have a lot IO resource declarations

[Bjorn Helgaas]

On Wednesday 18 July 2007 02:21:14 am Thomas Renninger wrote:
> On Tue, 2007-07-17 at 09:49 -0600, Bjorn Helgaas wrote:
> > On Monday 16 July 2007 08:21:07 am Thomas Renninger wrote:
> > > PNP0C02 devices normally have a lot more IO port declarations than
> > > currently defined in PNP_MAX_PORT
> > 
> > Yes.
> > 
> > > I also wonder whether other limits like:
> > >  #define PNP_MAX_MEM        4
> > >  #define PNP_MAX_IRQ        2
> > >  #define PNP_MAX_DMA        2
> > > could get exceeded with pnpacpi?
> > 
> > Definitely.  I think the current limits come from the PNP ISA spec
> > (sec 4.6).  I don't see similar limits in the PNPBIOS or ACPI specs,
> > so ideally I think they should be dynamically allocated as you suggest.
> > 
> I wanted to implement the dynamic approach and used a dynamically
> allocated array, filled up from beginning. While this is close to the
> current implementation I thought this is the easiest sufficient way...
> (I also only did this for io ports where most mem is wasted).
> Now I am thinking about hotplug (e.g. if a SSDT with resources gets
> hot-added, removed)... If a device can vanish, the array must get
> reordered, not a really well fitting structure, a list (a pnp specific
> set up, or from include/linux/list.h?) should be better?

I don't understand the array reordering problem.  Either a device exists
or it doesn't.  Loading or unloading an SSDT should not change the number
of resources for devices (except that it might add or remove an entire
device).

I think for now, it would be sufficient to increase PNP_MAX_IRQ to 8
and PNP_MAX_PORT to 32 and be done with it.  I don't think it's worth
getting more complicated unless we dynamically allocate everything.

Bjorn

Re: [PATCH] - Increase PNP_MAX_PORT. ACPI devices can have a lot IO resource declarations

[Thomas Renninger]

On Wed, 2007-07-18 at 15:33 -0600, Bjorn Helgaas wrote:
> On Wednesday 18 July 2007 02:21:14 am Thomas Renninger wrote:
> > On Tue, 2007-07-17 at 09:49 -0600, Bjorn Helgaas wrote:
> > > On Monday 16 July 2007 08:21:07 am Thomas Renninger wrote:
> > > > PNP0C02 devices normally have a lot more IO port declarations than
> > > > currently defined in PNP_MAX_PORT
> > > 
> > > Yes.
> > > 
> > > > I also wonder whether other limits like:
> > > >  #define PNP_MAX_MEM        4
> > > >  #define PNP_MAX_IRQ        2
> > > >  #define PNP_MAX_DMA        2
> > > > could get exceeded with pnpacpi?
> > > 
> > > Definitely.  I think the current limits come from the PNP ISA spec
> > > (sec 4.6).  I don't see similar limits in the PNPBIOS or ACPI specs,
> > > so ideally I think they should be dynamically allocated as you suggest.
> > > 
> > I wanted to implement the dynamic approach and used a dynamically
> > allocated array, filled up from beginning. While this is close to the
> > current implementation I thought this is the easiest sufficient way...
> > (I also only did this for io ports where most mem is wasted).
> > Now I am thinking about hotplug (e.g. if a SSDT with resources gets
> > hot-added, removed)... If a device can vanish, the array must get
> > reordered, not a really well fitting structure, a list (a pnp specific
> > set up, or from include/linux/list.h?) should be better?
> 
> I don't understand the array reordering problem.  Either a device exists
> or it doesn't.  Loading or unloading an SSDT should not change the number
> of resources for devices (except that it might add or remove an entire
> device).
Yes, you are right. I feared there could be cases were single resources
should be able to be removed.
> 
> I think for now, it would be sufficient to increase PNP_MAX_IRQ to 8
> and PNP_MAX_PORT to 32 and be done with it.  I don't think it's worth
> getting more complicated unless we dynamically allocate everything.
Ok.

Thanks,

   Thomas