* [patch 0/8] PNP: convert resource table to dynamic list, v1 @ 2008-05-05 22:36 ` Bjorn Helgaas 0 siblings, 0 replies; 27+ messages in thread From: Bjorn Helgaas @ 2008-05-05 22:36 UTC (permalink / raw) To: Len Brown Cc: linux-acpi, linux-kernel, Adam Belay, Adam M Belay, Li Shaohua, Matthieu Castet, Thomas Renninger, Rene Herman, Jaroslav Kysela, Andrew Morton This series replaces the fixed-size pnp_resource_table with a linked list of resources. The first patch is in -mm already and should appear in Linus' tree soon, so just skip that patch if necessary. As a reminder, if anybody tests this and sees problems, the dmesg log with CONFIG_PNP_DEBUG enabled would be a great help to me. Bjorn -- ^ permalink raw reply [flat|nested] 27+ messages in thread
* [patch 0/8] PNP: convert resource table to dynamic list, v1 @ 2008-05-05 22:36 ` Bjorn Helgaas 0 siblings, 0 replies; 27+ messages in thread From: Bjorn Helgaas @ 2008-05-05 22:36 UTC (permalink / raw) To: Len Brown Cc: linux-acpi, linux-kernel, Adam Belay, Adam M Belay, Li Shaohua, Matthieu Castet, Thomas Renninger, Rene Herman, Jaroslav Kysela, Andrew Morton This series replaces the fixed-size pnp_resource_table with a linked list of resources. The first patch is in -mm already and should appear in Linus' tree soon, so just skip that patch if necessary. As a reminder, if anybody tests this and sees problems, the dmesg log with CONFIG_PNP_DEBUG enabled would be a great help to me. Bjorn -- ^ permalink raw reply [flat|nested] 27+ messages in thread
* [patch 1/8] PNP: set IRQ index in sysfs "set irq" interface 2008-05-05 22:36 ` Bjorn Helgaas @ 2008-05-05 22:36 ` Bjorn Helgaas -1 siblings, 0 replies; 27+ messages in thread From: Bjorn Helgaas @ 2008-05-05 22:36 UTC (permalink / raw) To: Len Brown Cc: linux-acpi, linux-kernel, Adam Belay, Adam M Belay, Li Shaohua, Matthieu Castet, Thomas Renninger, Rene Herman, Jaroslav Kysela, Andrew Morton [-- Attachment #1: pnp-interface-fixup --] [-- Type: text/plain, Size: 737 bytes --] We have to set the ISAPNP register index when setting an IRQ via the sysfs interface. We already do it for IO, MEM, and DMA resources; I just missed the IRQ one. This should be in 2.6.26. Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com> Index: work10/drivers/pnp/interface.c =================================================================== --- work10.orig/drivers/pnp/interface.c 2008-05-01 15:26:28.000000000 -0600 +++ work10/drivers/pnp/interface.c 2008-05-01 15:26:39.000000000 -0600 @@ -424,7 +424,7 @@ start = simple_strtoul(buf, &buf, 0); pnp_res = pnp_add_irq_resource(dev, start, 0); if (pnp_res) - nirq++; + pnp_res->index = nirq++; continue; } if (!strnicmp(buf, "dma", 3)) { -- ^ permalink raw reply [flat|nested] 27+ messages in thread
* [patch 1/8] PNP: set IRQ index in sysfs "set irq" interface @ 2008-05-05 22:36 ` Bjorn Helgaas 0 siblings, 0 replies; 27+ messages in thread From: Bjorn Helgaas @ 2008-05-05 22:36 UTC (permalink / raw) To: Len Brown Cc: linux-acpi, linux-kernel, Adam Belay, Adam M Belay, Li Shaohua, Matthieu Castet, Thomas Renninger, Rene Herman, Jaroslav Kysela, Andrew Morton [-- Attachment #1: pnp-interface-fixup --] [-- Type: text/plain, Size: 737 bytes --] We have to set the ISAPNP register index when setting an IRQ via the sysfs interface. We already do it for IO, MEM, and DMA resources; I just missed the IRQ one. This should be in 2.6.26. Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com> Index: work10/drivers/pnp/interface.c =================================================================== --- work10.orig/drivers/pnp/interface.c 2008-05-01 15:26:28.000000000 -0600 +++ work10/drivers/pnp/interface.c 2008-05-01 15:26:39.000000000 -0600 @@ -424,7 +424,7 @@ start = simple_strtoul(buf, &buf, 0); pnp_res = pnp_add_irq_resource(dev, start, 0); if (pnp_res) - nirq++; + pnp_res->index = nirq++; continue; } if (!strnicmp(buf, "dma", 3)) { -- ^ permalink raw reply [flat|nested] 27+ messages in thread
* [patch 2/8] PNP: add detail to debug resource dump 2008-05-05 22:36 ` Bjorn Helgaas @ 2008-05-05 22:36 ` Bjorn Helgaas -1 siblings, 0 replies; 27+ messages in thread From: Bjorn Helgaas @ 2008-05-05 22:36 UTC (permalink / raw) To: Len Brown Cc: linux-acpi, linux-kernel, Adam Belay, Adam M Belay, Li Shaohua, Matthieu Castet, Thomas Renninger, Rene Herman, Jaroslav Kysela, Andrew Morton [-- Attachment #1: pnp-better-resource-dump --] [-- Type: text/plain, Size: 2480 bytes --] In the debug resource dump, decode the flags and indicate when a resource is disabled or has been automatically assigned. Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com> Index: work10/drivers/pnp/support.c =================================================================== --- work10.orig/drivers/pnp/support.c 2008-05-02 11:18:50.000000000 -0600 +++ work10/drivers/pnp/support.c 2008-05-05 09:39:03.000000000 -0600 @@ -63,28 +63,46 @@ void dbg_pnp_show_resources(struct pnp_d for (i = 0; i < PNP_MAX_IRQ; i++) { res = pnp_get_resource(dev, IORESOURCE_IRQ, i); if (res && !(res->flags & IORESOURCE_UNSET)) - dev_dbg(&dev->dev, " irq %lld flags %#lx\n", - (unsigned long long) res->start, res->flags); + dev_dbg(&dev->dev, " irq %lld flags %#lx%s%s\n", + (unsigned long long) res->start, res->flags, + res->flags & IORESOURCE_DISABLED ? + " DISABLED" : "", + res->flags & IORESOURCE_AUTO ? + " AUTO" : ""); } for (i = 0; i < PNP_MAX_DMA; i++) { res = pnp_get_resource(dev, IORESOURCE_DMA, i); if (res && !(res->flags & IORESOURCE_UNSET)) - dev_dbg(&dev->dev, " dma %lld flags %#lx\n", - (unsigned long long) res->start, res->flags); + dev_dbg(&dev->dev, " dma %lld flags %#lx%s%s\n", + (unsigned long long) res->start, res->flags, + res->flags & IORESOURCE_DISABLED ? + " DISABLED" : "", + res->flags & IORESOURCE_AUTO ? + " AUTO" : ""); } for (i = 0; i < PNP_MAX_PORT; i++) { res = pnp_get_resource(dev, IORESOURCE_IO, i); if (res && !(res->flags & IORESOURCE_UNSET)) - dev_dbg(&dev->dev, " io %#llx-%#llx flags %#lx\n", + dev_dbg(&dev->dev, " io %#llx-%#llx flags %#lx" + "%s%s\n", (unsigned long long) res->start, - (unsigned long long) res->end, res->flags); + (unsigned long long) res->end, res->flags, + res->flags & IORESOURCE_DISABLED ? + " DISABLED" : "", + res->flags & IORESOURCE_AUTO ? + " AUTO" : ""); } for (i = 0; i < PNP_MAX_MEM; i++) { res = pnp_get_resource(dev, IORESOURCE_MEM, i); if (res && !(res->flags & IORESOURCE_UNSET)) - dev_dbg(&dev->dev, " mem %#llx-%#llx flags %#lx\n", + dev_dbg(&dev->dev, " mem %#llx-%#llx flags %#lx" + "%s%s\n", (unsigned long long) res->start, - (unsigned long long) res->end, res->flags); + (unsigned long long) res->end, res->flags, + res->flags & IORESOURCE_DISABLED ? + " DISABLED" : "", + res->flags & IORESOURCE_AUTO ? + " AUTO" : ""); } #endif } -- ^ permalink raw reply [flat|nested] 27+ messages in thread
* [patch 2/8] PNP: add detail to debug resource dump @ 2008-05-05 22:36 ` Bjorn Helgaas 0 siblings, 0 replies; 27+ messages in thread From: Bjorn Helgaas @ 2008-05-05 22:36 UTC (permalink / raw) To: Len Brown Cc: linux-acpi, linux-kernel, Adam Belay, Adam M Belay, Li Shaohua, Matthieu Castet, Thomas Renninger, Rene Herman, Jaroslav Kysela, Andrew Morton [-- Attachment #1: pnp-better-resource-dump --] [-- Type: text/plain, Size: 2480 bytes --] In the debug resource dump, decode the flags and indicate when a resource is disabled or has been automatically assigned. Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com> Index: work10/drivers/pnp/support.c =================================================================== --- work10.orig/drivers/pnp/support.c 2008-05-02 11:18:50.000000000 -0600 +++ work10/drivers/pnp/support.c 2008-05-05 09:39:03.000000000 -0600 @@ -63,28 +63,46 @@ void dbg_pnp_show_resources(struct pnp_d for (i = 0; i < PNP_MAX_IRQ; i++) { res = pnp_get_resource(dev, IORESOURCE_IRQ, i); if (res && !(res->flags & IORESOURCE_UNSET)) - dev_dbg(&dev->dev, " irq %lld flags %#lx\n", - (unsigned long long) res->start, res->flags); + dev_dbg(&dev->dev, " irq %lld flags %#lx%s%s\n", + (unsigned long long) res->start, res->flags, + res->flags & IORESOURCE_DISABLED ? + " DISABLED" : "", + res->flags & IORESOURCE_AUTO ? + " AUTO" : ""); } for (i = 0; i < PNP_MAX_DMA; i++) { res = pnp_get_resource(dev, IORESOURCE_DMA, i); if (res && !(res->flags & IORESOURCE_UNSET)) - dev_dbg(&dev->dev, " dma %lld flags %#lx\n", - (unsigned long long) res->start, res->flags); + dev_dbg(&dev->dev, " dma %lld flags %#lx%s%s\n", + (unsigned long long) res->start, res->flags, + res->flags & IORESOURCE_DISABLED ? + " DISABLED" : "", + res->flags & IORESOURCE_AUTO ? + " AUTO" : ""); } for (i = 0; i < PNP_MAX_PORT; i++) { res = pnp_get_resource(dev, IORESOURCE_IO, i); if (res && !(res->flags & IORESOURCE_UNSET)) - dev_dbg(&dev->dev, " io %#llx-%#llx flags %#lx\n", + dev_dbg(&dev->dev, " io %#llx-%#llx flags %#lx" + "%s%s\n", (unsigned long long) res->start, - (unsigned long long) res->end, res->flags); + (unsigned long long) res->end, res->flags, + res->flags & IORESOURCE_DISABLED ? + " DISABLED" : "", + res->flags & IORESOURCE_AUTO ? + " AUTO" : ""); } for (i = 0; i < PNP_MAX_MEM; i++) { res = pnp_get_resource(dev, IORESOURCE_MEM, i); if (res && !(res->flags & IORESOURCE_UNSET)) - dev_dbg(&dev->dev, " mem %#llx-%#llx flags %#lx\n", + dev_dbg(&dev->dev, " mem %#llx-%#llx flags %#lx" + "%s%s\n", (unsigned long long) res->start, - (unsigned long long) res->end, res->flags); + (unsigned long long) res->end, res->flags, + res->flags & IORESOURCE_DISABLED ? + " DISABLED" : "", + res->flags & IORESOURCE_AUTO ? + " AUTO" : ""); } #endif } -- ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [patch 2/8] PNP: add detail to debug resource dump 2008-05-05 22:36 ` Bjorn Helgaas (?) @ 2008-05-19 21:47 ` Rene Herman -1 siblings, 0 replies; 27+ messages in thread From: Rene Herman @ 2008-05-19 21:47 UTC (permalink / raw) To: Bjorn Helgaas Cc: Len Brown, linux-acpi, linux-kernel, Adam Belay, Adam M Belay, Li Shaohua, Matthieu Castet, Thomas Renninger, Jaroslav Kysela, Andrew Morton On 06-05-08 00:36, Bjorn Helgaas wrote: > In the debug resource dump, decode the flags and indicate when > a resource is disabled or has been automatically assigned. > > Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com> Acked-by: Rene Herman <rene.herman@gmail.com> Rene. ^ permalink raw reply [flat|nested] 27+ messages in thread
* [patch 3/8] PNP: remove pnp_resource.index 2008-05-05 22:36 ` Bjorn Helgaas @ 2008-05-05 22:36 ` Bjorn Helgaas -1 siblings, 0 replies; 27+ messages in thread From: Bjorn Helgaas @ 2008-05-05 22:36 UTC (permalink / raw) To: Len Brown Cc: linux-acpi, linux-kernel, Adam Belay, Adam M Belay, Li Shaohua, Matthieu Castet, Thomas Renninger, Rene Herman, Jaroslav Kysela, Andrew Morton [-- Attachment #1: pnp-remove-pnp_resource-index --] [-- Type: text/plain, Size: 9050 bytes --] We used pnp_resource.index to keep track of which ISAPNP configuration register a resource should be written to. We needed this only to handle the case where a register is disabled but a subsequent register in the same set is enabled. Rather than explicitly maintaining the pnp_resource.index, this patch adds a resource every time we read an ISAPNP configuration register and marks the resource as IORESOURCE_DISABLED when appropriate. This makes the position in the pnp_resource_table always correspond to the config register index. Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com> --- drivers/pnp/base.h | 1 drivers/pnp/interface.c | 20 ++--------- drivers/pnp/isapnp/core.c | 80 ++++++++++++++-------------------------------- drivers/pnp/manager.c | 4 -- 4 files changed, 29 insertions(+), 76 deletions(-) Index: work10/drivers/pnp/base.h =================================================================== --- work10.orig/drivers/pnp/base.h 2008-05-05 15:58:47.000000000 -0600 +++ work10/drivers/pnp/base.h 2008-05-05 15:59:00.000000000 -0600 @@ -56,7 +56,6 @@ struct pnp_resource *pnp_get_pnp_resourc struct pnp_resource { struct resource res; - unsigned int index; /* ISAPNP config register index */ }; struct pnp_resource_table { Index: work10/drivers/pnp/interface.c =================================================================== --- work10.orig/drivers/pnp/interface.c 2008-05-05 15:58:49.000000000 -0600 +++ work10/drivers/pnp/interface.c 2008-05-05 15:59:00.000000000 -0600 @@ -320,7 +320,6 @@ pnp_set_current_resources(struct device const char *ubuf, size_t count) { struct pnp_dev *dev = to_pnp_dev(dmdev); - struct pnp_resource *pnp_res; char *buf = (void *)ubuf; int retval = 0; resource_size_t start, end; @@ -368,7 +367,6 @@ pnp_set_current_resources(struct device goto done; } if (!strnicmp(buf, "set", 3)) { - int nport = 0, nmem = 0, nirq = 0, ndma = 0; if (dev->active) goto done; buf += 3; @@ -391,10 +389,7 @@ pnp_set_current_resources(struct device end = simple_strtoul(buf, &buf, 0); } else end = start; - pnp_res = pnp_add_io_resource(dev, start, end, - 0); - if (pnp_res) - pnp_res->index = nport++; + pnp_add_io_resource(dev, start, end, 0); continue; } if (!strnicmp(buf, "mem", 3)) { @@ -411,10 +406,7 @@ pnp_set_current_resources(struct device end = simple_strtoul(buf, &buf, 0); } else end = start; - pnp_res = pnp_add_mem_resource(dev, start, end, - 0); - if (pnp_res) - pnp_res->index = nmem++; + pnp_add_mem_resource(dev, start, end, 0); continue; } if (!strnicmp(buf, "irq", 3)) { @@ -422,9 +414,7 @@ pnp_set_current_resources(struct device while (isspace(*buf)) ++buf; start = simple_strtoul(buf, &buf, 0); - pnp_res = pnp_add_irq_resource(dev, start, 0); - if (pnp_res) - pnp_res->index = nirq++; + pnp_add_irq_resource(dev, start, 0); continue; } if (!strnicmp(buf, "dma", 3)) { @@ -432,9 +422,7 @@ pnp_set_current_resources(struct device while (isspace(*buf)) ++buf; start = simple_strtoul(buf, &buf, 0); - pnp_res = pnp_add_dma_resource(dev, start, 0); - if (pnp_res) - pnp_res->index = ndma++; + pnp_add_dma_resource(dev, start, 0); continue; } break; Index: work10/drivers/pnp/isapnp/core.c =================================================================== --- work10.orig/drivers/pnp/isapnp/core.c 2008-05-05 15:58:47.000000000 -0600 +++ work10/drivers/pnp/isapnp/core.c 2008-05-05 16:09:19.000000000 -0600 @@ -928,7 +928,6 @@ EXPORT_SYMBOL(isapnp_write_byte); static int isapnp_get_resources(struct pnp_dev *dev) { - struct pnp_resource *pnp_res; int i, ret; dev_dbg(&dev->dev, "get resources\n"); @@ -940,35 +939,23 @@ static int isapnp_get_resources(struct p for (i = 0; i < ISAPNP_MAX_PORT; i++) { ret = isapnp_read_word(ISAPNP_CFG_PORT + (i << 1)); - if (ret) { - pnp_res = pnp_add_io_resource(dev, ret, ret, 0); - if (pnp_res) - pnp_res->index = i; - } + pnp_add_io_resource(dev, ret, ret, + ret == 0 ? IORESOURCE_DISABLED : 0); } for (i = 0; i < ISAPNP_MAX_MEM; i++) { ret = isapnp_read_word(ISAPNP_CFG_MEM + (i << 3)) << 8; - if (ret) { - pnp_res = pnp_add_mem_resource(dev, ret, ret, 0); - if (pnp_res) - pnp_res->index = i; - } + pnp_add_mem_resource(dev, ret, ret, + ret == 0 ? IORESOURCE_DISABLED : 0); } for (i = 0; i < ISAPNP_MAX_IRQ; i++) { ret = isapnp_read_word(ISAPNP_CFG_IRQ + (i << 1)) >> 8; - if (ret) { - pnp_res = pnp_add_irq_resource(dev, ret, 0); - if (pnp_res) - pnp_res->index = i; - } + pnp_add_irq_resource(dev, ret, + ret == 0 ? IORESOURCE_DISABLED : 0); } for (i = 0; i < ISAPNP_MAX_DMA; i++) { ret = isapnp_read_byte(ISAPNP_CFG_DMA + i); - if (ret != 4) { - pnp_res = pnp_add_dma_resource(dev, ret, 0); - if (pnp_res) - pnp_res->index = i; - } + pnp_add_dma_resource(dev, ret, + ret == 4 ? IORESOURCE_DISABLED : 0); } __end: @@ -978,62 +965,49 @@ __end: static int isapnp_set_resources(struct pnp_dev *dev) { - struct pnp_resource *pnp_res; struct resource *res; - int tmp, index; + int tmp; dev_dbg(&dev->dev, "set resources\n"); isapnp_cfg_begin(dev->card->number, dev->number); dev->active = 1; for (tmp = 0; tmp < ISAPNP_MAX_PORT; tmp++) { - pnp_res = pnp_get_pnp_resource(dev, IORESOURCE_IO, tmp); - if (!pnp_res) - continue; - res = &pnp_res->res; - if (pnp_resource_valid(res)) { - index = pnp_res->index; + res = pnp_get_resource(dev, IORESOURCE_IO, tmp); + if (res && pnp_resource_valid(res) && + !(res->flags & IORESOURCE_DISABLED)) { dev_dbg(&dev->dev, " set io %d to %#llx\n", - index, (unsigned long long) res->start); - isapnp_write_word(ISAPNP_CFG_PORT + (index << 1), + tmp, (unsigned long long) res->start); + isapnp_write_word(ISAPNP_CFG_PORT + (tmp << 1), res->start); } } for (tmp = 0; tmp < ISAPNP_MAX_IRQ; tmp++) { - pnp_res = pnp_get_pnp_resource(dev, IORESOURCE_IRQ, tmp); - if (!pnp_res) - continue; - res = &pnp_res->res; - if (pnp_resource_valid(res)) { + res = pnp_get_resource(dev, IORESOURCE_IRQ, tmp); + if (res && pnp_resource_valid(res) && + !(res->flags & IORESOURCE_DISABLED)) { int irq = res->start; if (irq == 2) irq = 9; - index = pnp_res->index; - dev_dbg(&dev->dev, " set irq %d to %d\n", index, irq); - isapnp_write_byte(ISAPNP_CFG_IRQ + (index << 1), irq); + dev_dbg(&dev->dev, " set irq %d to %d\n", tmp, irq); + isapnp_write_byte(ISAPNP_CFG_IRQ + (tmp << 1), irq); } } for (tmp = 0; tmp < ISAPNP_MAX_DMA; tmp++) { - pnp_res = pnp_get_pnp_resource(dev, IORESOURCE_DMA, tmp); - if (!pnp_res) - continue; - res = &pnp_res->res; - if (pnp_resource_valid(res)) { - index = pnp_res->index; + res = pnp_get_resource(dev, IORESOURCE_DMA, tmp); + if (res && pnp_resource_valid(res) && + !(res->flags & IORESOURCE_DISABLED)) { dev_dbg(&dev->dev, " set dma %d to %lld\n", - index, (unsigned long long) res->start); - isapnp_write_byte(ISAPNP_CFG_DMA + index, res->start); + tmp, (unsigned long long) res->start); + isapnp_write_byte(ISAPNP_CFG_DMA + tmp, res->start); } } for (tmp = 0; tmp < ISAPNP_MAX_MEM; tmp++) { - pnp_res = pnp_get_pnp_resource(dev, IORESOURCE_MEM, tmp); - if (!pnp_res) - continue; - res = &pnp_res->res; - if (pnp_resource_valid(res)) { - index = pnp_res->index; + res = pnp_get_resource(dev, IORESOURCE_MEM, tmp); + if (res && pnp_resource_valid(res) && + !(res->flags & IORESOURCE_DISABLED)) { dev_dbg(&dev->dev, " set mem %d to %#llx\n", - index, (unsigned long long) res->start); - isapnp_write_word(ISAPNP_CFG_MEM + (index << 3), + tmp, (unsigned long long) res->start); + isapnp_write_word(ISAPNP_CFG_MEM + (tmp << 3), (res->start >> 8) & 0xffff); } } Index: work10/drivers/pnp/manager.c =================================================================== --- work10.orig/drivers/pnp/manager.c 2008-05-05 15:58:47.000000000 -0600 +++ work10/drivers/pnp/manager.c 2008-05-05 15:59:00.000000000 -0600 @@ -40,7 +40,6 @@ static int pnp_assign_port(struct pnp_de } /* set the initial values */ - pnp_res->index = idx; res->flags |= rule->flags | IORESOURCE_IO; res->flags &= ~IORESOURCE_UNSET; @@ -90,7 +89,6 @@ static int pnp_assign_mem(struct pnp_dev } /* set the initial values */ - pnp_res->index = idx; res->flags |= rule->flags | IORESOURCE_MEM; res->flags &= ~IORESOURCE_UNSET; @@ -155,7 +153,6 @@ static int pnp_assign_irq(struct pnp_dev } /* set the initial values */ - pnp_res->index = idx; res->flags |= rule->flags | IORESOURCE_IRQ; res->flags &= ~IORESOURCE_UNSET; @@ -214,7 +211,6 @@ static void pnp_assign_dma(struct pnp_de } /* set the initial values */ - pnp_res->index = idx; res->flags |= rule->flags | IORESOURCE_DMA; res->flags &= ~IORESOURCE_UNSET; -- ^ permalink raw reply [flat|nested] 27+ messages in thread
* [patch 3/8] PNP: remove pnp_resource.index @ 2008-05-05 22:36 ` Bjorn Helgaas 0 siblings, 0 replies; 27+ messages in thread From: Bjorn Helgaas @ 2008-05-05 22:36 UTC (permalink / raw) To: Len Brown Cc: linux-acpi, linux-kernel, Adam Belay, Adam M Belay, Li Shaohua, Matthieu Castet, Thomas Renninger, Rene Herman, Jaroslav Kysela, Andrew Morton [-- Attachment #1: pnp-remove-pnp_resource-index --] [-- Type: text/plain, Size: 9050 bytes --] We used pnp_resource.index to keep track of which ISAPNP configuration register a resource should be written to. We needed this only to handle the case where a register is disabled but a subsequent register in the same set is enabled. Rather than explicitly maintaining the pnp_resource.index, this patch adds a resource every time we read an ISAPNP configuration register and marks the resource as IORESOURCE_DISABLED when appropriate. This makes the position in the pnp_resource_table always correspond to the config register index. Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com> --- drivers/pnp/base.h | 1 drivers/pnp/interface.c | 20 ++--------- drivers/pnp/isapnp/core.c | 80 ++++++++++++++-------------------------------- drivers/pnp/manager.c | 4 -- 4 files changed, 29 insertions(+), 76 deletions(-) Index: work10/drivers/pnp/base.h =================================================================== --- work10.orig/drivers/pnp/base.h 2008-05-05 15:58:47.000000000 -0600 +++ work10/drivers/pnp/base.h 2008-05-05 15:59:00.000000000 -0600 @@ -56,7 +56,6 @@ struct pnp_resource *pnp_get_pnp_resourc struct pnp_resource { struct resource res; - unsigned int index; /* ISAPNP config register index */ }; struct pnp_resource_table { Index: work10/drivers/pnp/interface.c =================================================================== --- work10.orig/drivers/pnp/interface.c 2008-05-05 15:58:49.000000000 -0600 +++ work10/drivers/pnp/interface.c 2008-05-05 15:59:00.000000000 -0600 @@ -320,7 +320,6 @@ pnp_set_current_resources(struct device const char *ubuf, size_t count) { struct pnp_dev *dev = to_pnp_dev(dmdev); - struct pnp_resource *pnp_res; char *buf = (void *)ubuf; int retval = 0; resource_size_t start, end; @@ -368,7 +367,6 @@ pnp_set_current_resources(struct device goto done; } if (!strnicmp(buf, "set", 3)) { - int nport = 0, nmem = 0, nirq = 0, ndma = 0; if (dev->active) goto done; buf += 3; @@ -391,10 +389,7 @@ pnp_set_current_resources(struct device end = simple_strtoul(buf, &buf, 0); } else end = start; - pnp_res = pnp_add_io_resource(dev, start, end, - 0); - if (pnp_res) - pnp_res->index = nport++; + pnp_add_io_resource(dev, start, end, 0); continue; } if (!strnicmp(buf, "mem", 3)) { @@ -411,10 +406,7 @@ pnp_set_current_resources(struct device end = simple_strtoul(buf, &buf, 0); } else end = start; - pnp_res = pnp_add_mem_resource(dev, start, end, - 0); - if (pnp_res) - pnp_res->index = nmem++; + pnp_add_mem_resource(dev, start, end, 0); continue; } if (!strnicmp(buf, "irq", 3)) { @@ -422,9 +414,7 @@ pnp_set_current_resources(struct device while (isspace(*buf)) ++buf; start = simple_strtoul(buf, &buf, 0); - pnp_res = pnp_add_irq_resource(dev, start, 0); - if (pnp_res) - pnp_res->index = nirq++; + pnp_add_irq_resource(dev, start, 0); continue; } if (!strnicmp(buf, "dma", 3)) { @@ -432,9 +422,7 @@ pnp_set_current_resources(struct device while (isspace(*buf)) ++buf; start = simple_strtoul(buf, &buf, 0); - pnp_res = pnp_add_dma_resource(dev, start, 0); - if (pnp_res) - pnp_res->index = ndma++; + pnp_add_dma_resource(dev, start, 0); continue; } break; Index: work10/drivers/pnp/isapnp/core.c =================================================================== --- work10.orig/drivers/pnp/isapnp/core.c 2008-05-05 15:58:47.000000000 -0600 +++ work10/drivers/pnp/isapnp/core.c 2008-05-05 16:09:19.000000000 -0600 @@ -928,7 +928,6 @@ EXPORT_SYMBOL(isapnp_write_byte); static int isapnp_get_resources(struct pnp_dev *dev) { - struct pnp_resource *pnp_res; int i, ret; dev_dbg(&dev->dev, "get resources\n"); @@ -940,35 +939,23 @@ static int isapnp_get_resources(struct p for (i = 0; i < ISAPNP_MAX_PORT; i++) { ret = isapnp_read_word(ISAPNP_CFG_PORT + (i << 1)); - if (ret) { - pnp_res = pnp_add_io_resource(dev, ret, ret, 0); - if (pnp_res) - pnp_res->index = i; - } + pnp_add_io_resource(dev, ret, ret, + ret == 0 ? IORESOURCE_DISABLED : 0); } for (i = 0; i < ISAPNP_MAX_MEM; i++) { ret = isapnp_read_word(ISAPNP_CFG_MEM + (i << 3)) << 8; - if (ret) { - pnp_res = pnp_add_mem_resource(dev, ret, ret, 0); - if (pnp_res) - pnp_res->index = i; - } + pnp_add_mem_resource(dev, ret, ret, + ret == 0 ? IORESOURCE_DISABLED : 0); } for (i = 0; i < ISAPNP_MAX_IRQ; i++) { ret = isapnp_read_word(ISAPNP_CFG_IRQ + (i << 1)) >> 8; - if (ret) { - pnp_res = pnp_add_irq_resource(dev, ret, 0); - if (pnp_res) - pnp_res->index = i; - } + pnp_add_irq_resource(dev, ret, + ret == 0 ? IORESOURCE_DISABLED : 0); } for (i = 0; i < ISAPNP_MAX_DMA; i++) { ret = isapnp_read_byte(ISAPNP_CFG_DMA + i); - if (ret != 4) { - pnp_res = pnp_add_dma_resource(dev, ret, 0); - if (pnp_res) - pnp_res->index = i; - } + pnp_add_dma_resource(dev, ret, + ret == 4 ? IORESOURCE_DISABLED : 0); } __end: @@ -978,62 +965,49 @@ __end: static int isapnp_set_resources(struct pnp_dev *dev) { - struct pnp_resource *pnp_res; struct resource *res; - int tmp, index; + int tmp; dev_dbg(&dev->dev, "set resources\n"); isapnp_cfg_begin(dev->card->number, dev->number); dev->active = 1; for (tmp = 0; tmp < ISAPNP_MAX_PORT; tmp++) { - pnp_res = pnp_get_pnp_resource(dev, IORESOURCE_IO, tmp); - if (!pnp_res) - continue; - res = &pnp_res->res; - if (pnp_resource_valid(res)) { - index = pnp_res->index; + res = pnp_get_resource(dev, IORESOURCE_IO, tmp); + if (res && pnp_resource_valid(res) && + !(res->flags & IORESOURCE_DISABLED)) { dev_dbg(&dev->dev, " set io %d to %#llx\n", - index, (unsigned long long) res->start); - isapnp_write_word(ISAPNP_CFG_PORT + (index << 1), + tmp, (unsigned long long) res->start); + isapnp_write_word(ISAPNP_CFG_PORT + (tmp << 1), res->start); } } for (tmp = 0; tmp < ISAPNP_MAX_IRQ; tmp++) { - pnp_res = pnp_get_pnp_resource(dev, IORESOURCE_IRQ, tmp); - if (!pnp_res) - continue; - res = &pnp_res->res; - if (pnp_resource_valid(res)) { + res = pnp_get_resource(dev, IORESOURCE_IRQ, tmp); + if (res && pnp_resource_valid(res) && + !(res->flags & IORESOURCE_DISABLED)) { int irq = res->start; if (irq == 2) irq = 9; - index = pnp_res->index; - dev_dbg(&dev->dev, " set irq %d to %d\n", index, irq); - isapnp_write_byte(ISAPNP_CFG_IRQ + (index << 1), irq); + dev_dbg(&dev->dev, " set irq %d to %d\n", tmp, irq); + isapnp_write_byte(ISAPNP_CFG_IRQ + (tmp << 1), irq); } } for (tmp = 0; tmp < ISAPNP_MAX_DMA; tmp++) { - pnp_res = pnp_get_pnp_resource(dev, IORESOURCE_DMA, tmp); - if (!pnp_res) - continue; - res = &pnp_res->res; - if (pnp_resource_valid(res)) { - index = pnp_res->index; + res = pnp_get_resource(dev, IORESOURCE_DMA, tmp); + if (res && pnp_resource_valid(res) && + !(res->flags & IORESOURCE_DISABLED)) { dev_dbg(&dev->dev, " set dma %d to %lld\n", - index, (unsigned long long) res->start); - isapnp_write_byte(ISAPNP_CFG_DMA + index, res->start); + tmp, (unsigned long long) res->start); + isapnp_write_byte(ISAPNP_CFG_DMA + tmp, res->start); } } for (tmp = 0; tmp < ISAPNP_MAX_MEM; tmp++) { - pnp_res = pnp_get_pnp_resource(dev, IORESOURCE_MEM, tmp); - if (!pnp_res) - continue; - res = &pnp_res->res; - if (pnp_resource_valid(res)) { - index = pnp_res->index; + res = pnp_get_resource(dev, IORESOURCE_MEM, tmp); + if (res && pnp_resource_valid(res) && + !(res->flags & IORESOURCE_DISABLED)) { dev_dbg(&dev->dev, " set mem %d to %#llx\n", - index, (unsigned long long) res->start); - isapnp_write_word(ISAPNP_CFG_MEM + (index << 3), + tmp, (unsigned long long) res->start); + isapnp_write_word(ISAPNP_CFG_MEM + (tmp << 3), (res->start >> 8) & 0xffff); } } Index: work10/drivers/pnp/manager.c =================================================================== --- work10.orig/drivers/pnp/manager.c 2008-05-05 15:58:47.000000000 -0600 +++ work10/drivers/pnp/manager.c 2008-05-05 15:59:00.000000000 -0600 @@ -40,7 +40,6 @@ static int pnp_assign_port(struct pnp_de } /* set the initial values */ - pnp_res->index = idx; res->flags |= rule->flags | IORESOURCE_IO; res->flags &= ~IORESOURCE_UNSET; @@ -90,7 +89,6 @@ static int pnp_assign_mem(struct pnp_dev } /* set the initial values */ - pnp_res->index = idx; res->flags |= rule->flags | IORESOURCE_MEM; res->flags &= ~IORESOURCE_UNSET; @@ -155,7 +153,6 @@ static int pnp_assign_irq(struct pnp_dev } /* set the initial values */ - pnp_res->index = idx; res->flags |= rule->flags | IORESOURCE_IRQ; res->flags &= ~IORESOURCE_UNSET; @@ -214,7 +211,6 @@ static void pnp_assign_dma(struct pnp_de } /* set the initial values */ - pnp_res->index = idx; res->flags |= rule->flags | IORESOURCE_DMA; res->flags &= ~IORESOURCE_UNSET; -- ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [patch 3/8] PNP: remove pnp_resource.index 2008-05-05 22:36 ` Bjorn Helgaas (?) @ 2008-05-19 22:01 ` Rene Herman 2008-05-19 23:27 ` Bjorn Helgaas -1 siblings, 1 reply; 27+ messages in thread From: Rene Herman @ 2008-05-19 22:01 UTC (permalink / raw) To: Bjorn Helgaas Cc: Len Brown, linux-acpi, linux-kernel, Adam Belay, Adam M Belay, Li Shaohua, Matthieu Castet, Thomas Renninger, Jaroslav Kysela, Andrew Morton On 06-05-08 00:36, Bjorn Helgaas wrote: > We used pnp_resource.index to keep track of which ISAPNP configuration > register a resource should be written to. We needed this only to > handle the case where a register is disabled but a subsequent register > in the same set is enabled. > > Rather than explicitly maintaining the pnp_resource.index, this patch > adds a resource every time we read an ISAPNP configuration register > and marks the resource as IORESOURCE_DISABLED when appropriate. This > makes the position in the pnp_resource_table always correspond to the > config register index. > > Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com> > - pnp_res = pnp_add_io_resource(dev, start, end, > - 0); > - if (pnp_res) > - pnp_res->index = nport++; > + pnp_add_io_resource(dev, start, end, 0); In the tree after your v2 series, pnp_add_foo_resource() are called as void functions yet still return a struct pnp_resource *. You might have other plans but if not, I guess they can _be_ void functions? Otherwise: Acked-by: Rene Herman <rene.herman@gmail.com> Rene. ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [patch 3/8] PNP: remove pnp_resource.index 2008-05-19 22:01 ` Rene Herman @ 2008-05-19 23:27 ` Bjorn Helgaas 0 siblings, 0 replies; 27+ messages in thread From: Bjorn Helgaas @ 2008-05-19 23:27 UTC (permalink / raw) To: Rene Herman Cc: Len Brown, linux-acpi, linux-kernel, Adam Belay, Adam M Belay, Li Shaohua, Matthieu Castet, Thomas Renninger, Jaroslav Kysela, Andrew Morton On Monday 19 May 2008 04:01:32 pm Rene Herman wrote: > On 06-05-08 00:36, Bjorn Helgaas wrote: > > > We used pnp_resource.index to keep track of which ISAPNP configuration > > register a resource should be written to. We needed this only to > > handle the case where a register is disabled but a subsequent register > > in the same set is enabled. > > > > Rather than explicitly maintaining the pnp_resource.index, this patch > > adds a resource every time we read an ISAPNP configuration register > > and marks the resource as IORESOURCE_DISABLED when appropriate. This > > makes the position in the pnp_resource_table always correspond to the > > config register index. > > > > Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com> > > > - pnp_res = pnp_add_io_resource(dev, start, end, > > - 0); > > - if (pnp_res) > > - pnp_res->index = nport++; > > + pnp_add_io_resource(dev, start, end, 0); > > In the tree after your v2 series, pnp_add_foo_resource() are called as > void functions yet still return a struct pnp_resource *. You might have > other plans but if not, I guess they can _be_ void functions? You're right, I still don't do anything with the return value. It could be used to check for success/failure, but we currently don't do that. Possibly a future cleanup since there's no functional problem here. Bjorn ^ permalink raw reply [flat|nested] 27+ messages in thread
* [patch 4/8] PNP: add pnp_resource_type() internal interface 2008-05-05 22:36 ` Bjorn Helgaas @ 2008-05-05 22:36 ` Bjorn Helgaas -1 siblings, 0 replies; 27+ messages in thread From: Bjorn Helgaas @ 2008-05-05 22:36 UTC (permalink / raw) To: Len Brown Cc: linux-acpi, linux-kernel, Adam Belay, Adam M Belay, Li Shaohua, Matthieu Castet, Thomas Renninger, Rene Herman, Jaroslav Kysela, Andrew Morton [-- Attachment #1: pnp-add-pnp_resource_type --] [-- Type: text/plain, Size: 1284 bytes --] Given a struct resource, this returns the type (IO, MEM, IRQ, DMA). Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com> Index: work10/drivers/pnp/base.h =================================================================== --- work10.orig/drivers/pnp/base.h 2008-05-02 14:23:13.000000000 -0600 +++ work10/drivers/pnp/base.h 2008-05-05 09:39:03.000000000 -0600 @@ -45,6 +45,7 @@ int pnp_check_dma(struct pnp_dev *dev, s void dbg_pnp_show_resources(struct pnp_dev *dev, char *desc); void pnp_init_resource(struct resource *res); +int pnp_resource_type(struct resource *res); struct pnp_resource *pnp_get_pnp_resource(struct pnp_dev *dev, unsigned int type, unsigned int num); Index: work10/drivers/pnp/resource.c =================================================================== --- work10.orig/drivers/pnp/resource.c 2008-05-02 14:23:13.000000000 -0600 +++ work10/drivers/pnp/resource.c 2008-05-05 09:38:57.000000000 -0600 @@ -499,6 +499,12 @@ int pnp_check_dma(struct pnp_dev *dev, s #endif } +int pnp_resource_type(struct resource *res) +{ + return res->flags & (IORESOURCE_IO | IORESOURCE_MEM | + IORESOURCE_IRQ | IORESOURCE_DMA); +} + struct pnp_resource *pnp_get_pnp_resource(struct pnp_dev *dev, unsigned int type, unsigned int num) { -- ^ permalink raw reply [flat|nested] 27+ messages in thread
* [patch 4/8] PNP: add pnp_resource_type() internal interface @ 2008-05-05 22:36 ` Bjorn Helgaas 0 siblings, 0 replies; 27+ messages in thread From: Bjorn Helgaas @ 2008-05-05 22:36 UTC (permalink / raw) To: Len Brown Cc: linux-acpi, linux-kernel, Adam Belay, Adam M Belay, Li Shaohua, Matthieu Castet, Thomas Renninger, Rene Herman, Jaroslav Kysela, Andrew Morton [-- Attachment #1: pnp-add-pnp_resource_type --] [-- Type: text/plain, Size: 1284 bytes --] Given a struct resource, this returns the type (IO, MEM, IRQ, DMA). Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com> Index: work10/drivers/pnp/base.h =================================================================== --- work10.orig/drivers/pnp/base.h 2008-05-02 14:23:13.000000000 -0600 +++ work10/drivers/pnp/base.h 2008-05-05 09:39:03.000000000 -0600 @@ -45,6 +45,7 @@ int pnp_check_dma(struct pnp_dev *dev, s void dbg_pnp_show_resources(struct pnp_dev *dev, char *desc); void pnp_init_resource(struct resource *res); +int pnp_resource_type(struct resource *res); struct pnp_resource *pnp_get_pnp_resource(struct pnp_dev *dev, unsigned int type, unsigned int num); Index: work10/drivers/pnp/resource.c =================================================================== --- work10.orig/drivers/pnp/resource.c 2008-05-02 14:23:13.000000000 -0600 +++ work10/drivers/pnp/resource.c 2008-05-05 09:38:57.000000000 -0600 @@ -499,6 +499,12 @@ int pnp_check_dma(struct pnp_dev *dev, s #endif } +int pnp_resource_type(struct resource *res) +{ + return res->flags & (IORESOURCE_IO | IORESOURCE_MEM | + IORESOURCE_IRQ | IORESOURCE_DMA); +} + struct pnp_resource *pnp_get_pnp_resource(struct pnp_dev *dev, unsigned int type, unsigned int num) { -- ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [patch 4/8] PNP: add pnp_resource_type() internal interface 2008-05-05 22:36 ` Bjorn Helgaas (?) @ 2008-05-19 22:02 ` Rene Herman -1 siblings, 0 replies; 27+ messages in thread From: Rene Herman @ 2008-05-19 22:02 UTC (permalink / raw) To: Bjorn Helgaas Cc: Len Brown, linux-acpi, linux-kernel, Adam Belay, Adam M Belay, Li Shaohua, Matthieu Castet, Thomas Renninger, Jaroslav Kysela, Andrew Morton On 06-05-08 00:36, Bjorn Helgaas wrote: > Given a struct resource, this returns the type (IO, MEM, IRQ, DMA). > > Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com> Acked-by: Rene Herman <rene.herman@gmail.com> Rene. ^ permalink raw reply [flat|nested] 27+ messages in thread
* [patch 5/8] PNP: add pnp_resource_type_name() helper function 2008-05-05 22:36 ` Bjorn Helgaas @ 2008-05-05 22:36 ` Bjorn Helgaas -1 siblings, 0 replies; 27+ messages in thread From: Bjorn Helgaas @ 2008-05-05 22:36 UTC (permalink / raw) To: Len Brown Cc: linux-acpi, linux-kernel, Adam Belay, Adam M Belay, Li Shaohua, Matthieu Castet, Thomas Renninger, Rene Herman, Jaroslav Kysela, Andrew Morton [-- Attachment #1: pnp-add-resource-type-name --] [-- Type: text/plain, Size: 1527 bytes --] This patch adds a "pnp_resource_type_name(struct resource *)" that returns the string resource type. This will be used by the sysfs "show resources" function and the debug resource dump function. Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com> Index: work10/drivers/pnp/base.h =================================================================== --- work10.orig/drivers/pnp/base.h 2008-05-02 14:28:40.000000000 -0600 +++ work10/drivers/pnp/base.h 2008-05-05 09:38:56.000000000 -0600 @@ -42,6 +42,7 @@ int pnp_check_mem(struct pnp_dev *dev, s int pnp_check_irq(struct pnp_dev *dev, struct resource *res); int pnp_check_dma(struct pnp_dev *dev, struct resource *res); +char *pnp_resource_type_name(struct resource *res); void dbg_pnp_show_resources(struct pnp_dev *dev, char *desc); void pnp_init_resource(struct resource *res); Index: work10/drivers/pnp/support.c =================================================================== --- work10.orig/drivers/pnp/support.c 2008-05-02 11:18:54.000000000 -0600 +++ work10/drivers/pnp/support.c 2008-05-05 09:38:57.000000000 -0600 @@ -52,6 +52,21 @@ void pnp_eisa_id_to_string(u32 id, char str[7] = '\0'; } +char *pnp_resource_type_name(struct resource *res) +{ + switch (pnp_resource_type(res)) { + case IORESOURCE_IO: + return "io"; + case IORESOURCE_MEM: + return "mem"; + case IORESOURCE_IRQ: + return "irq"; + case IORESOURCE_DMA: + return "dma"; + } + return NULL; +} + void dbg_pnp_show_resources(struct pnp_dev *dev, char *desc) { #ifdef DEBUG -- ^ permalink raw reply [flat|nested] 27+ messages in thread
* [patch 5/8] PNP: add pnp_resource_type_name() helper function @ 2008-05-05 22:36 ` Bjorn Helgaas 0 siblings, 0 replies; 27+ messages in thread From: Bjorn Helgaas @ 2008-05-05 22:36 UTC (permalink / raw) To: Len Brown Cc: linux-acpi, linux-kernel, Adam Belay, Adam M Belay, Li Shaohua, Matthieu Castet, Thomas Renninger, Rene Herman, Jaroslav Kysela, Andrew Morton [-- Attachment #1: pnp-add-resource-type-name --] [-- Type: text/plain, Size: 1527 bytes --] This patch adds a "pnp_resource_type_name(struct resource *)" that returns the string resource type. This will be used by the sysfs "show resources" function and the debug resource dump function. Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com> Index: work10/drivers/pnp/base.h =================================================================== --- work10.orig/drivers/pnp/base.h 2008-05-02 14:28:40.000000000 -0600 +++ work10/drivers/pnp/base.h 2008-05-05 09:38:56.000000000 -0600 @@ -42,6 +42,7 @@ int pnp_check_mem(struct pnp_dev *dev, s int pnp_check_irq(struct pnp_dev *dev, struct resource *res); int pnp_check_dma(struct pnp_dev *dev, struct resource *res); +char *pnp_resource_type_name(struct resource *res); void dbg_pnp_show_resources(struct pnp_dev *dev, char *desc); void pnp_init_resource(struct resource *res); Index: work10/drivers/pnp/support.c =================================================================== --- work10.orig/drivers/pnp/support.c 2008-05-02 11:18:54.000000000 -0600 +++ work10/drivers/pnp/support.c 2008-05-05 09:38:57.000000000 -0600 @@ -52,6 +52,21 @@ void pnp_eisa_id_to_string(u32 id, char str[7] = '\0'; } +char *pnp_resource_type_name(struct resource *res) +{ + switch (pnp_resource_type(res)) { + case IORESOURCE_IO: + return "io"; + case IORESOURCE_MEM: + return "mem"; + case IORESOURCE_IRQ: + return "irq"; + case IORESOURCE_DMA: + return "dma"; + } + return NULL; +} + void dbg_pnp_show_resources(struct pnp_dev *dev, char *desc) { #ifdef DEBUG -- ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [patch 5/8] PNP: add pnp_resource_type_name() helper function 2008-05-05 22:36 ` Bjorn Helgaas (?) @ 2008-05-19 22:03 ` Rene Herman -1 siblings, 0 replies; 27+ messages in thread From: Rene Herman @ 2008-05-19 22:03 UTC (permalink / raw) To: Bjorn Helgaas Cc: Len Brown, linux-acpi, linux-kernel, Adam Belay, Adam M Belay, Li Shaohua, Matthieu Castet, Thomas Renninger, Jaroslav Kysela, Andrew Morton On 06-05-08 00:36, Bjorn Helgaas wrote: > This patch adds a "pnp_resource_type_name(struct resource *)" that > returns the string resource type. This will be used by the sysfs > "show resources" function and the debug resource dump function. > > Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com> Acked-by: Rene Herman <rene.herman@gmail.com> Rene. ^ permalink raw reply [flat|nested] 27+ messages in thread
* [patch 6/8] PNP: replace pnp_resource_table with dynamically allocated resources 2008-05-05 22:36 ` Bjorn Helgaas @ 2008-05-05 22:36 ` Bjorn Helgaas -1 siblings, 0 replies; 27+ messages in thread From: Bjorn Helgaas @ 2008-05-05 22:36 UTC (permalink / raw) To: Len Brown Cc: linux-acpi, linux-kernel, Adam Belay, Adam M Belay, Li Shaohua, Matthieu Castet, Thomas Renninger, Rene Herman, Jaroslav Kysela, Andrew Morton [-- Attachment #1: pnp-convert-to-lists --] [-- Type: text/plain, Size: 30529 bytes --] PNP used to have a fixed-size pnp_resource_table for tracking the resources used by a device. This table often overflowed, so we've had to increase the table size, which wastes memory because most devices have very few resources. This patch replaces the table with a linked list of resources where the entries are allocated on demand. This removes messages like these: pnpacpi: exceeded the max number of IO resources 00:01: too many I/O port resources References: http://bugzilla.kernel.org/show_bug.cgi?id=9535 http://bugzilla.kernel.org/show_bug.cgi?id=9740 http://lkml.org/lkml/2007/11/30/110 This patch also changes the way PNP uses the IORESOURCE_UNSET, IORESOURCE_AUTO, and IORESOURCE_DISABLED flags. Prior to this patch, the pnp_resource_table entries used the flags like this: IORESOURCE_UNSET This table entry is unused and available for use. When this flag is set, we shouldn't look at anything else in the resource structure. This flag is set when a resource table entry is initialized. IORESOURCE_AUTO This resource was assigned automatically by pnp_assign_{io,mem,etc}(). This flag is set when a resource table entry is initialized and cleared whenever we discover a resource setting by reading an ISAPNP config register, parsing a PNPBIOS resource data stream, parsing an ACPI _CRS list, or interpreting a sysfs "set" command. Resources marked IORESOURCE_AUTO are reinitialized and marked as IORESOURCE_UNSET by pnp_clean_resource_table() in these cases: - before we attempt to assign resources automatically, - if we fail to assign resources automatically, - after disabling a device IORESOURCE_DISABLED Set by pnp_assign_{io,mem,etc}() when automatic assignment fails. Also set by PNPBIOS and PNPACPI for: - invalid IRQs or GSI registration failures - invalid DMA channels - I/O ports above 0x10000 - mem ranges with negative length After this patch, there is no pnp_resource_table, and the resource list entries use the flags like this: IORESOURCE_UNSET This flag is no longer used in PNP. Instead of keeping IORESOURCE_UNSET entries in the resource list, we remove entries from the list and free them. IORESOURCE_AUTO No change in meaning: it still means the resource was assigned automatically by pnp_assign_{port,mem,etc}(), but these functions now set the bit explicitly. We still "clean" a device's resource list in the same places, but rather than reinitializing IORESOURCE_AUTO entries, we just remove them from the list. Note that IORESOURCE_AUTO entries are always at the end of the list, so removing them doesn't reorder other list entries. This is because non-IORESOURCE_AUTO entries are added by the ISAPNP, PNPBIOS, or PNPACPI "get resources" methods and by the sysfs "set" command. In each of these cases, we completely free the resource list first. IORESOURCE_DISABLED In addition to the cases where we used to set this flag, ISAPNP now adds an IORESOURCE_DISABLED resource when it reads a configuration register with a "disabled" value. [Rene, I made isapnp_set_resources() skip IORESOURCE_DISABLED resources, because that's effectively what we used to do. I think it *ought* to work to write zero (or 4 for DMA) back to a configuration register to disable it, and that might even be useful as a way to manually disable a single resource, but for now it seems safer to preserve the previous behavior.] Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com> --- drivers/pnp/base.h | 18 --- drivers/pnp/core.c | 25 +++-- drivers/pnp/interface.c | 60 +++++------- drivers/pnp/isapnp/core.c | 8 - drivers/pnp/manager.c | 218 +++++++++++----------------------------------- drivers/pnp/quirks.c | 3 drivers/pnp/resource.c | 86 +++--------------- drivers/pnp/support.c | 68 +++++--------- drivers/pnp/system.c | 8 - include/linux/pnp.h | 5 - 10 files changed, 150 insertions(+), 349 deletions(-) Index: work10/drivers/pnp/base.h =================================================================== --- work10.orig/drivers/pnp/base.h 2008-05-05 16:09:47.000000000 -0600 +++ work10/drivers/pnp/base.h 2008-05-05 16:09:51.000000000 -0600 @@ -45,27 +45,15 @@ int pnp_check_dma(struct pnp_dev *dev, s char *pnp_resource_type_name(struct resource *res); void dbg_pnp_show_resources(struct pnp_dev *dev, char *desc); -void pnp_init_resource(struct resource *res); +void pnp_free_resources(struct pnp_dev *dev); int pnp_resource_type(struct resource *res); -struct pnp_resource *pnp_get_pnp_resource(struct pnp_dev *dev, - unsigned int type, unsigned int num); - -#define PNP_MAX_PORT 40 -#define PNP_MAX_MEM 24 -#define PNP_MAX_IRQ 2 -#define PNP_MAX_DMA 2 - struct pnp_resource { + struct list_head list; struct resource res; }; -struct pnp_resource_table { - struct pnp_resource port[PNP_MAX_PORT]; - struct pnp_resource mem[PNP_MAX_MEM]; - struct pnp_resource dma[PNP_MAX_DMA]; - struct pnp_resource irq[PNP_MAX_IRQ]; -}; +void pnp_free_resource(struct pnp_resource *pnp_res); struct pnp_resource *pnp_add_irq_resource(struct pnp_dev *dev, int irq, int flags); Index: work10/drivers/pnp/core.c =================================================================== --- work10.orig/drivers/pnp/core.c 2008-05-05 15:58:46.000000000 -0600 +++ work10/drivers/pnp/core.c 2008-05-05 16:09:51.000000000 -0600 @@ -99,6 +99,21 @@ static void pnp_free_ids(struct pnp_dev } } +void pnp_free_resource(struct pnp_resource *pnp_res) +{ + list_del(&pnp_res->list); + kfree(pnp_res); +} + +void pnp_free_resources(struct pnp_dev *dev) +{ + struct pnp_resource *pnp_res, *tmp; + + list_for_each_entry_safe(pnp_res, tmp, &dev->resources, list) { + pnp_free_resource(pnp_res); + } +} + static void pnp_release_device(struct device *dmdev) { struct pnp_dev *dev = to_pnp_dev(dmdev); @@ -106,7 +121,7 @@ static void pnp_release_device(struct de pnp_free_option(dev->independent); pnp_free_option(dev->dependent); pnp_free_ids(dev); - kfree(dev->res); + pnp_free_resources(dev); kfree(dev); } @@ -119,12 +134,7 @@ struct pnp_dev *pnp_alloc_dev(struct pnp if (!dev) return NULL; - dev->res = kzalloc(sizeof(struct pnp_resource_table), GFP_KERNEL); - if (!dev->res) { - kfree(dev); - return NULL; - } - + INIT_LIST_HEAD(&dev->resources); dev->protocol = protocol; dev->number = id; dev->dma_mask = DMA_24BIT_MASK; @@ -140,7 +150,6 @@ struct pnp_dev *pnp_alloc_dev(struct pnp dev_id = pnp_add_id(dev, pnpid); if (!dev_id) { - kfree(dev->res); kfree(dev); return NULL; } Index: work10/drivers/pnp/interface.c =================================================================== --- work10.orig/drivers/pnp/interface.c 2008-05-05 15:59:00.000000000 -0600 +++ work10/drivers/pnp/interface.c 2008-05-05 16:09:51.000000000 -0600 @@ -269,46 +269,38 @@ static ssize_t pnp_show_current_resource pnp_printf(buffer, "disabled\n"); for (i = 0; (res = pnp_get_resource(dev, IORESOURCE_IO, i)); i++) { - if (pnp_resource_valid(res)) { - pnp_printf(buffer, "io"); - if (res->flags & IORESOURCE_DISABLED) - pnp_printf(buffer, " disabled\n"); - else - pnp_printf(buffer, " 0x%llx-0x%llx\n", - (unsigned long long) res->start, - (unsigned long long) res->end); - } + pnp_printf(buffer, "io"); + if (res->flags & IORESOURCE_DISABLED) + pnp_printf(buffer, " disabled\n"); + else + pnp_printf(buffer, " 0x%llx-0x%llx\n", + (unsigned long long) res->start, + (unsigned long long) res->end); } for (i = 0; (res = pnp_get_resource(dev, IORESOURCE_MEM, i)); i++) { - if (pnp_resource_valid(res)) { - pnp_printf(buffer, "mem"); - if (res->flags & IORESOURCE_DISABLED) - pnp_printf(buffer, " disabled\n"); - else - pnp_printf(buffer, " 0x%llx-0x%llx\n", - (unsigned long long) res->start, - (unsigned long long) res->end); - } + pnp_printf(buffer, "mem"); + if (res->flags & IORESOURCE_DISABLED) + pnp_printf(buffer, " disabled\n"); + else + pnp_printf(buffer, " 0x%llx-0x%llx\n", + (unsigned long long) res->start, + (unsigned long long) res->end); } for (i = 0; (res = pnp_get_resource(dev, IORESOURCE_IRQ, i)); i++) { - if (pnp_resource_valid(res)) { - pnp_printf(buffer, "irq"); - if (res->flags & IORESOURCE_DISABLED) - pnp_printf(buffer, " disabled\n"); - else - pnp_printf(buffer, " %lld\n", - (unsigned long long) res->start); - } + pnp_printf(buffer, "irq"); + if (res->flags & IORESOURCE_DISABLED) + pnp_printf(buffer, " disabled\n"); + else + pnp_printf(buffer, " %lld\n", + (unsigned long long) res->start); } for (i = 0; (res = pnp_get_resource(dev, IORESOURCE_DMA, i)); i++) { - if (pnp_resource_valid(res)) { - pnp_printf(buffer, "dma"); - if (res->flags & IORESOURCE_DISABLED) - pnp_printf(buffer, " disabled\n"); - else - pnp_printf(buffer, " %lld\n", - (unsigned long long) res->start); - } + pnp_printf(buffer, "dma"); + if (res->flags & IORESOURCE_DISABLED) + pnp_printf(buffer, " disabled\n"); + else + pnp_printf(buffer, " %lld\n", + (unsigned long long) res->start); } ret = (buffer->curr - buf); kfree(buffer); Index: work10/drivers/pnp/isapnp/core.c =================================================================== --- work10.orig/drivers/pnp/isapnp/core.c 2008-05-05 16:09:19.000000000 -0600 +++ work10/drivers/pnp/isapnp/core.c 2008-05-05 16:10:24.000000000 -0600 @@ -973,8 +973,7 @@ static int isapnp_set_resources(struct p dev->active = 1; for (tmp = 0; tmp < ISAPNP_MAX_PORT; tmp++) { res = pnp_get_resource(dev, IORESOURCE_IO, tmp); - if (res && pnp_resource_valid(res) && - !(res->flags & IORESOURCE_DISABLED)) { + if (res && !(res->flags & IORESOURCE_DISABLED)) { dev_dbg(&dev->dev, " set io %d to %#llx\n", tmp, (unsigned long long) res->start); isapnp_write_word(ISAPNP_CFG_PORT + (tmp << 1), @@ -983,8 +982,7 @@ static int isapnp_set_resources(struct p } for (tmp = 0; tmp < ISAPNP_MAX_IRQ; tmp++) { res = pnp_get_resource(dev, IORESOURCE_IRQ, tmp); - if (res && pnp_resource_valid(res) && - !(res->flags & IORESOURCE_DISABLED)) { + if (res && !(res->flags & IORESOURCE_DISABLED)) { int irq = res->start; if (irq == 2) irq = 9; @@ -994,8 +992,7 @@ static int isapnp_set_resources(struct p } for (tmp = 0; tmp < ISAPNP_MAX_DMA; tmp++) { res = pnp_get_resource(dev, IORESOURCE_DMA, tmp); - if (res && pnp_resource_valid(res) && - !(res->flags & IORESOURCE_DISABLED)) { + if (res && !(res->flags & IORESOURCE_DISABLED)) { dev_dbg(&dev->dev, " set dma %d to %lld\n", tmp, (unsigned long long) res->start); isapnp_write_byte(ISAPNP_CFG_DMA + tmp, res->start); @@ -1003,8 +1000,7 @@ static int isapnp_set_resources(struct p } for (tmp = 0; tmp < ISAPNP_MAX_MEM; tmp++) { res = pnp_get_resource(dev, IORESOURCE_MEM, tmp); - if (res && pnp_resource_valid(res) && - !(res->flags & IORESOURCE_DISABLED)) { + if (res && !(res->flags & IORESOURCE_DISABLED)) { dev_dbg(&dev->dev, " set mem %d to %#llx\n", tmp, (unsigned long long) res->start); isapnp_write_word(ISAPNP_CFG_MEM + (tmp << 3), Index: work10/drivers/pnp/manager.c =================================================================== --- work10.orig/drivers/pnp/manager.c 2008-05-05 15:59:00.000000000 -0600 +++ work10/drivers/pnp/manager.c 2008-05-05 16:09:51.000000000 -0600 @@ -19,40 +19,30 @@ DEFINE_MUTEX(pnp_res_mutex); static int pnp_assign_port(struct pnp_dev *dev, struct pnp_port *rule, int idx) { - struct pnp_resource *pnp_res; - struct resource *res; + struct resource *res, local_res; - pnp_res = pnp_get_pnp_resource(dev, IORESOURCE_IO, idx); - if (!pnp_res) { - dev_err(&dev->dev, "too many I/O port resources\n"); - /* pretend we were successful so at least the manager won't try again */ - return 1; - } - - res = &pnp_res->res; - - /* check if this resource has been manually set, if so skip */ - if (!(res->flags & IORESOURCE_AUTO)) { + res = pnp_get_resource(dev, IORESOURCE_IO, idx); + if (res) { dev_dbg(&dev->dev, " io %d already set to %#llx-%#llx " "flags %#lx\n", idx, (unsigned long long) res->start, (unsigned long long) res->end, res->flags); return 1; } - /* set the initial values */ - res->flags |= rule->flags | IORESOURCE_IO; - res->flags &= ~IORESOURCE_UNSET; + res = &local_res; + res->flags = rule->flags | IORESOURCE_AUTO; + res->start = 0; + res->end = 0; if (!rule->size) { res->flags |= IORESOURCE_DISABLED; dev_dbg(&dev->dev, " io %d disabled\n", idx); - return 1; /* skip disabled resource requests */ + goto __add; } res->start = rule->min; res->end = res->start + rule->size - 1; - /* run through until pnp_check_port is happy */ while (!pnp_check_port(dev, res)) { res->start += rule->align; res->end = res->start + rule->size - 1; @@ -61,38 +51,29 @@ static int pnp_assign_port(struct pnp_de return 0; } } - dev_dbg(&dev->dev, " assign io %d %#llx-%#llx\n", idx, - (unsigned long long) res->start, (unsigned long long) res->end); + +__add: + pnp_add_io_resource(dev, res->start, res->end, res->flags); return 1; } static int pnp_assign_mem(struct pnp_dev *dev, struct pnp_mem *rule, int idx) { - struct pnp_resource *pnp_res; - struct resource *res; - - pnp_res = pnp_get_pnp_resource(dev, IORESOURCE_MEM, idx); - if (!pnp_res) { - dev_err(&dev->dev, "too many memory resources\n"); - /* pretend we were successful so at least the manager won't try again */ - return 1; - } - - res = &pnp_res->res; + struct resource *res, local_res; - /* check if this resource has been manually set, if so skip */ - if (!(res->flags & IORESOURCE_AUTO)) { + res = pnp_get_resource(dev, IORESOURCE_MEM, idx); + if (res) { dev_dbg(&dev->dev, " mem %d already set to %#llx-%#llx " "flags %#lx\n", idx, (unsigned long long) res->start, (unsigned long long) res->end, res->flags); return 1; } - /* set the initial values */ - res->flags |= rule->flags | IORESOURCE_MEM; - res->flags &= ~IORESOURCE_UNSET; + res = &local_res; + res->flags = rule->flags | IORESOURCE_AUTO; + res->start = 0; + res->end = 0; - /* convert pnp flags to standard Linux flags */ if (!(rule->flags & IORESOURCE_MEM_WRITEABLE)) res->flags |= IORESOURCE_READONLY; if (rule->flags & IORESOURCE_MEM_CACHEABLE) @@ -105,13 +86,12 @@ static int pnp_assign_mem(struct pnp_dev if (!rule->size) { res->flags |= IORESOURCE_DISABLED; dev_dbg(&dev->dev, " mem %d disabled\n", idx); - return 1; /* skip disabled resource requests */ + goto __add; } res->start = rule->min; res->end = res->start + rule->size - 1; - /* run through until pnp_check_mem is happy */ while (!pnp_check_mem(dev, res)) { res->start += rule->align; res->end = res->start + rule->size - 1; @@ -120,15 +100,15 @@ static int pnp_assign_mem(struct pnp_dev return 0; } } - dev_dbg(&dev->dev, " assign mem %d %#llx-%#llx\n", idx, - (unsigned long long) res->start, (unsigned long long) res->end); + +__add: + pnp_add_mem_resource(dev, res->start, res->end, res->flags); return 1; } static int pnp_assign_irq(struct pnp_dev *dev, struct pnp_irq *rule, int idx) { - struct pnp_resource *pnp_res; - struct resource *res; + struct resource *res, local_res; int i; /* IRQ priority: this table is good for i386 */ @@ -136,58 +116,48 @@ static int pnp_assign_irq(struct pnp_dev 5, 10, 11, 12, 9, 14, 15, 7, 3, 4, 13, 0, 1, 6, 8, 2 }; - pnp_res = pnp_get_pnp_resource(dev, IORESOURCE_IRQ, idx); - if (!pnp_res) { - dev_err(&dev->dev, "too many IRQ resources\n"); - /* pretend we were successful so at least the manager won't try again */ - return 1; - } - - res = &pnp_res->res; - - /* check if this resource has been manually set, if so skip */ - if (!(res->flags & IORESOURCE_AUTO)) { + res = pnp_get_resource(dev, IORESOURCE_IRQ, idx); + if (res) { dev_dbg(&dev->dev, " irq %d already set to %d flags %#lx\n", idx, (int) res->start, res->flags); return 1; } - /* set the initial values */ - res->flags |= rule->flags | IORESOURCE_IRQ; - res->flags &= ~IORESOURCE_UNSET; + res = &local_res; + res->flags = rule->flags | IORESOURCE_AUTO; + res->start = -1; + res->end = -1; if (bitmap_empty(rule->map, PNP_IRQ_NR)) { res->flags |= IORESOURCE_DISABLED; dev_dbg(&dev->dev, " irq %d disabled\n", idx); - return 1; /* skip disabled resource requests */ + goto __add; } /* TBD: need check for >16 IRQ */ res->start = find_next_bit(rule->map, PNP_IRQ_NR, 16); if (res->start < PNP_IRQ_NR) { res->end = res->start; - dev_dbg(&dev->dev, " assign irq %d %d\n", idx, - (int) res->start); - return 1; + goto __add; } for (i = 0; i < 16; i++) { if (test_bit(xtab[i], rule->map)) { res->start = res->end = xtab[i]; - if (pnp_check_irq(dev, res)) { - dev_dbg(&dev->dev, " assign irq %d %d\n", idx, - (int) res->start); - return 1; - } + if (pnp_check_irq(dev, res)) + goto __add; } } dev_dbg(&dev->dev, " couldn't assign irq %d\n", idx); return 0; + +__add: + pnp_add_irq_resource(dev, res->start, res->flags); + return 1; } static void pnp_assign_dma(struct pnp_dev *dev, struct pnp_dma *rule, int idx) { - struct pnp_resource *pnp_res; - struct resource *res; + struct resource *res, local_res; int i; /* DMA priority: this table is good for i386 */ @@ -195,127 +165,47 @@ static void pnp_assign_dma(struct pnp_de 1, 3, 5, 6, 7, 0, 2, 4 }; - pnp_res = pnp_get_pnp_resource(dev, IORESOURCE_DMA, idx); - if (!pnp_res) { - dev_err(&dev->dev, "too many DMA resources\n"); - return; - } - - res = &pnp_res->res; - - /* check if this resource has been manually set, if so skip */ - if (!(res->flags & IORESOURCE_AUTO)) { + res = pnp_get_resource(dev, IORESOURCE_DMA, idx); + if (res) { dev_dbg(&dev->dev, " dma %d already set to %d flags %#lx\n", idx, (int) res->start, res->flags); return; } - /* set the initial values */ - res->flags |= rule->flags | IORESOURCE_DMA; - res->flags &= ~IORESOURCE_UNSET; + res = &local_res; + res->flags = rule->flags | IORESOURCE_AUTO; + res->start = -1; + res->end = -1; for (i = 0; i < 8; i++) { if (rule->map & (1 << xtab[i])) { res->start = res->end = xtab[i]; - if (pnp_check_dma(dev, res)) { - dev_dbg(&dev->dev, " assign dma %d %d\n", idx, - (int) res->start); - return; - } + if (pnp_check_dma(dev, res)) + goto __add; } } #ifdef MAX_DMA_CHANNELS res->start = res->end = MAX_DMA_CHANNELS; #endif - res->flags |= IORESOURCE_UNSET | IORESOURCE_DISABLED; + res->flags |= IORESOURCE_DISABLED; dev_dbg(&dev->dev, " disable dma %d\n", idx); -} -void pnp_init_resource(struct resource *res) -{ - unsigned long type; - - type = res->flags & (IORESOURCE_IO | IORESOURCE_MEM | - IORESOURCE_IRQ | IORESOURCE_DMA); - - res->name = NULL; - res->flags = type | IORESOURCE_AUTO | IORESOURCE_UNSET; - if (type == IORESOURCE_IRQ || type == IORESOURCE_DMA) { - res->start = -1; - res->end = -1; - } else { - res->start = 0; - res->end = 0; - } +__add: + pnp_add_dma_resource(dev, res->start, res->flags); } -/** - * pnp_init_resources - Resets a resource table to default values. - * @table: pointer to the desired resource table - */ void pnp_init_resources(struct pnp_dev *dev) { - struct resource *res; - int idx; - - for (idx = 0; idx < PNP_MAX_IRQ; idx++) { - res = &dev->res->irq[idx].res; - res->flags = IORESOURCE_IRQ; - pnp_init_resource(res); - } - for (idx = 0; idx < PNP_MAX_DMA; idx++) { - res = &dev->res->dma[idx].res; - res->flags = IORESOURCE_DMA; - pnp_init_resource(res); - } - for (idx = 0; idx < PNP_MAX_PORT; idx++) { - res = &dev->res->port[idx].res; - res->flags = IORESOURCE_IO; - pnp_init_resource(res); - } - for (idx = 0; idx < PNP_MAX_MEM; idx++) { - res = &dev->res->mem[idx].res; - res->flags = IORESOURCE_MEM; - pnp_init_resource(res); - } + pnp_free_resources(dev); } -/** - * pnp_clean_resources - clears resources that were not manually set - * @res: the resources to clean - */ static void pnp_clean_resource_table(struct pnp_dev *dev) { - struct resource *res; - int idx; + struct pnp_resource *pnp_res, *tmp; - for (idx = 0; idx < PNP_MAX_IRQ; idx++) { - res = &dev->res->irq[idx].res; - if (res->flags & IORESOURCE_AUTO) { - res->flags = IORESOURCE_IRQ; - pnp_init_resource(res); - } - } - for (idx = 0; idx < PNP_MAX_DMA; idx++) { - res = &dev->res->dma[idx].res; - if (res->flags & IORESOURCE_AUTO) { - res->flags = IORESOURCE_DMA; - pnp_init_resource(res); - } - } - for (idx = 0; idx < PNP_MAX_PORT; idx++) { - res = &dev->res->port[idx].res; - if (res->flags & IORESOURCE_AUTO) { - res->flags = IORESOURCE_IO; - pnp_init_resource(res); - } - } - for (idx = 0; idx < PNP_MAX_MEM; idx++) { - res = &dev->res->mem[idx].res; - if (res->flags & IORESOURCE_AUTO) { - res->flags = IORESOURCE_MEM; - pnp_init_resource(res); - } + list_for_each_entry_safe(pnp_res, tmp, &dev->resources, list) { + if (pnp_res->res.flags & IORESOURCE_AUTO) + pnp_free_resource(pnp_res); } } Index: work10/drivers/pnp/quirks.c =================================================================== --- work10.orig/drivers/pnp/quirks.c 2008-05-05 15:58:46.000000000 -0600 +++ work10/drivers/pnp/quirks.c 2008-05-05 16:09:51.000000000 -0600 @@ -141,8 +141,7 @@ static void quirk_system_pci_resources(s for (j = 0; (res = pnp_get_resource(dev, IORESOURCE_MEM, j)); j++) { - if (res->flags & IORESOURCE_UNSET || - (res->start == 0 && res->end == 0)) + if (res->start == 0 && res->end == 0) continue; pnp_start = res->start; Index: work10/drivers/pnp/resource.c =================================================================== --- work10.orig/drivers/pnp/resource.c 2008-05-05 16:09:46.000000000 -0600 +++ work10/drivers/pnp/resource.c 2008-05-05 16:09:51.000000000 -0600 @@ -237,7 +237,7 @@ void pnp_free_option(struct pnp_option * !((*(enda) < *(startb)) || (*(endb) < *(starta))) #define cannot_compare(flags) \ -((flags) & (IORESOURCE_UNSET | IORESOURCE_DISABLED)) +((flags) & IORESOURCE_DISABLED) int pnp_check_port(struct pnp_dev *dev, struct resource *res) { @@ -505,81 +505,31 @@ int pnp_resource_type(struct resource *r IORESOURCE_IRQ | IORESOURCE_DMA); } -struct pnp_resource *pnp_get_pnp_resource(struct pnp_dev *dev, - unsigned int type, unsigned int num) -{ - struct pnp_resource_table *res = dev->res; - - switch (type) { - case IORESOURCE_IO: - if (num >= PNP_MAX_PORT) - return NULL; - return &res->port[num]; - case IORESOURCE_MEM: - if (num >= PNP_MAX_MEM) - return NULL; - return &res->mem[num]; - case IORESOURCE_IRQ: - if (num >= PNP_MAX_IRQ) - return NULL; - return &res->irq[num]; - case IORESOURCE_DMA: - if (num >= PNP_MAX_DMA) - return NULL; - return &res->dma[num]; - } - return NULL; -} - struct resource *pnp_get_resource(struct pnp_dev *dev, unsigned int type, unsigned int num) { struct pnp_resource *pnp_res; + struct resource *res; - pnp_res = pnp_get_pnp_resource(dev, type, num); - if (pnp_res) - return &pnp_res->res; - + list_for_each_entry(pnp_res, &dev->resources, list) { + res = &pnp_res->res; + if (pnp_resource_type(res) == type && num-- == 0) + return res; + } return NULL; } EXPORT_SYMBOL(pnp_get_resource); -static struct pnp_resource *pnp_new_resource(struct pnp_dev *dev, int type) +static struct pnp_resource *pnp_new_resource(struct pnp_dev *dev) { struct pnp_resource *pnp_res; - int i; - switch (type) { - case IORESOURCE_IO: - for (i = 0; i < PNP_MAX_PORT; i++) { - pnp_res = pnp_get_pnp_resource(dev, IORESOURCE_IO, i); - if (pnp_res && !pnp_resource_valid(&pnp_res->res)) - return pnp_res; - } - break; - case IORESOURCE_MEM: - for (i = 0; i < PNP_MAX_MEM; i++) { - pnp_res = pnp_get_pnp_resource(dev, IORESOURCE_MEM, i); - if (pnp_res && !pnp_resource_valid(&pnp_res->res)) - return pnp_res; - } - break; - case IORESOURCE_IRQ: - for (i = 0; i < PNP_MAX_IRQ; i++) { - pnp_res = pnp_get_pnp_resource(dev, IORESOURCE_IRQ, i); - if (pnp_res && !pnp_resource_valid(&pnp_res->res)) - return pnp_res; - } - break; - case IORESOURCE_DMA: - for (i = 0; i < PNP_MAX_DMA; i++) { - pnp_res = pnp_get_pnp_resource(dev, IORESOURCE_DMA, i); - if (pnp_res && !pnp_resource_valid(&pnp_res->res)) - return pnp_res; - } - break; - } - return NULL; + pnp_res = kzalloc(sizeof(struct pnp_resource), GFP_KERNEL); + if (!pnp_res) + return NULL; + + list_add_tail(&pnp_res->list, &dev->resources); + return pnp_res; } struct pnp_resource *pnp_add_irq_resource(struct pnp_dev *dev, int irq, @@ -589,7 +539,7 @@ struct pnp_resource *pnp_add_irq_resourc struct resource *res; static unsigned char warned; - pnp_res = pnp_new_resource(dev, IORESOURCE_IRQ); + pnp_res = pnp_new_resource(dev); if (!pnp_res) { if (!warned) { dev_err(&dev->dev, "can't add resource for IRQ %d\n", @@ -615,7 +565,7 @@ struct pnp_resource *pnp_add_dma_resourc struct resource *res; static unsigned char warned; - pnp_res = pnp_new_resource(dev, IORESOURCE_DMA); + pnp_res = pnp_new_resource(dev); if (!pnp_res) { if (!warned) { dev_err(&dev->dev, "can't add resource for DMA %d\n", @@ -642,7 +592,7 @@ struct pnp_resource *pnp_add_io_resource struct resource *res; static unsigned char warned; - pnp_res = pnp_new_resource(dev, IORESOURCE_IO); + pnp_res = pnp_new_resource(dev); if (!pnp_res) { if (!warned) { dev_err(&dev->dev, "can't add resource for IO " @@ -671,7 +621,7 @@ struct pnp_resource *pnp_add_mem_resourc struct resource *res; static unsigned char warned; - pnp_res = pnp_new_resource(dev, IORESOURCE_MEM); + pnp_res = pnp_new_resource(dev); if (!pnp_res) { if (!warned) { dev_err(&dev->dev, "can't add resource for MEM " Index: work10/drivers/pnp/support.c =================================================================== --- work10.orig/drivers/pnp/support.c 2008-05-05 16:09:47.000000000 -0600 +++ work10/drivers/pnp/support.c 2008-05-05 16:09:51.000000000 -0600 @@ -70,54 +70,34 @@ char *pnp_resource_type_name(struct reso void dbg_pnp_show_resources(struct pnp_dev *dev, char *desc) { #ifdef DEBUG + struct pnp_resource *pnp_res; struct resource *res; - int i; dev_dbg(&dev->dev, "current resources: %s\n", desc); + list_for_each_entry(pnp_res, &dev->resources, list) { + res = &pnp_res->res; - for (i = 0; i < PNP_MAX_IRQ; i++) { - res = pnp_get_resource(dev, IORESOURCE_IRQ, i); - if (res && !(res->flags & IORESOURCE_UNSET)) - dev_dbg(&dev->dev, " irq %lld flags %#lx%s%s\n", - (unsigned long long) res->start, res->flags, - res->flags & IORESOURCE_DISABLED ? - " DISABLED" : "", - res->flags & IORESOURCE_AUTO ? - " AUTO" : ""); - } - for (i = 0; i < PNP_MAX_DMA; i++) { - res = pnp_get_resource(dev, IORESOURCE_DMA, i); - if (res && !(res->flags & IORESOURCE_UNSET)) - dev_dbg(&dev->dev, " dma %lld flags %#lx%s%s\n", - (unsigned long long) res->start, res->flags, - res->flags & IORESOURCE_DISABLED ? - " DISABLED" : "", - res->flags & IORESOURCE_AUTO ? - " AUTO" : ""); - } - for (i = 0; i < PNP_MAX_PORT; i++) { - res = pnp_get_resource(dev, IORESOURCE_IO, i); - if (res && !(res->flags & IORESOURCE_UNSET)) - dev_dbg(&dev->dev, " io %#llx-%#llx flags %#lx" - "%s%s\n", - (unsigned long long) res->start, - (unsigned long long) res->end, res->flags, - res->flags & IORESOURCE_DISABLED ? - " DISABLED" : "", - res->flags & IORESOURCE_AUTO ? - " AUTO" : ""); - } - for (i = 0; i < PNP_MAX_MEM; i++) { - res = pnp_get_resource(dev, IORESOURCE_MEM, i); - if (res && !(res->flags & IORESOURCE_UNSET)) - dev_dbg(&dev->dev, " mem %#llx-%#llx flags %#lx" - "%s%s\n", - (unsigned long long) res->start, - (unsigned long long) res->end, res->flags, - res->flags & IORESOURCE_DISABLED ? - " DISABLED" : "", - res->flags & IORESOURCE_AUTO ? - " AUTO" : ""); + dev_dbg(&dev->dev, " %-3s ", pnp_resource_type_name(res)); + + if (res->flags & IORESOURCE_DISABLED) { + printk("disabled\n"); + continue; + } + + switch (pnp_resource_type(res)) { + case IORESOURCE_IO: + case IORESOURCE_MEM: + printk("%#llx-%#llx flags %#lx", + (unsigned long long) res->start, + (unsigned long long) res->end, res->flags); + break; + case IORESOURCE_IRQ: + case IORESOURCE_DMA: + printk("%lld flags %#lx", + (unsigned long long) res->start, res->flags); + break; + } + printk("\n"); } #endif } Index: work10/drivers/pnp/system.c =================================================================== --- work10.orig/drivers/pnp/system.c 2008-05-05 15:58:46.000000000 -0600 +++ work10/drivers/pnp/system.c 2008-05-05 16:09:51.000000000 -0600 @@ -60,8 +60,6 @@ static void reserve_resources_of_dev(str int i; for (i = 0; (res = pnp_get_resource(dev, IORESOURCE_IO, i)); i++) { - if (res->flags & IORESOURCE_UNSET) - continue; if (res->start == 0) continue; /* disabled */ if (res->start < 0x100) @@ -80,12 +78,8 @@ static void reserve_resources_of_dev(str reserve_range(dev, res->start, res->end, 1); } - for (i = 0; (res = pnp_get_resource(dev, IORESOURCE_MEM, i)); i++) { - if (res->flags & IORESOURCE_UNSET) - continue; - + for (i = 0; (res = pnp_get_resource(dev, IORESOURCE_MEM, i)); i++) reserve_range(dev, res->start, res->end, 0); - } } static int system_pnp_probe(struct pnp_dev *dev, Index: work10/include/linux/pnp.h =================================================================== --- work10.orig/include/linux/pnp.h 2008-05-05 15:58:46.000000000 -0600 +++ work10/include/linux/pnp.h 2008-05-05 16:09:51.000000000 -0600 @@ -15,7 +15,6 @@ struct pnp_protocol; struct pnp_dev; -struct pnp_resource_table; /* * Resource Management @@ -24,7 +23,7 @@ struct resource *pnp_get_resource(struct static inline int pnp_resource_valid(struct resource *res) { - if (res && !(res->flags & IORESOURCE_UNSET)) + if (res) return 1; return 0; } @@ -248,7 +247,7 @@ struct pnp_dev { int capabilities; struct pnp_option *independent; struct pnp_option *dependent; - struct pnp_resource_table *res; + struct list_head resources; char name[PNP_NAME_LEN]; /* contains a human-readable name */ int flags; /* used by protocols */ -- ^ permalink raw reply [flat|nested] 27+ messages in thread
* [patch 6/8] PNP: replace pnp_resource_table with dynamically allocated resources @ 2008-05-05 22:36 ` Bjorn Helgaas 0 siblings, 0 replies; 27+ messages in thread From: Bjorn Helgaas @ 2008-05-05 22:36 UTC (permalink / raw) To: Len Brown Cc: linux-acpi, linux-kernel, Adam Belay, Adam M Belay, Li Shaohua, Matthieu Castet, Thomas Renninger, Rene Herman, Jaroslav Kysela, Andrew Morton [-- Attachment #1: pnp-convert-to-lists --] [-- Type: text/plain, Size: 30529 bytes --] PNP used to have a fixed-size pnp_resource_table for tracking the resources used by a device. This table often overflowed, so we've had to increase the table size, which wastes memory because most devices have very few resources. This patch replaces the table with a linked list of resources where the entries are allocated on demand. This removes messages like these: pnpacpi: exceeded the max number of IO resources 00:01: too many I/O port resources References: http://bugzilla.kernel.org/show_bug.cgi?id=9535 http://bugzilla.kernel.org/show_bug.cgi?id=9740 http://lkml.org/lkml/2007/11/30/110 This patch also changes the way PNP uses the IORESOURCE_UNSET, IORESOURCE_AUTO, and IORESOURCE_DISABLED flags. Prior to this patch, the pnp_resource_table entries used the flags like this: IORESOURCE_UNSET This table entry is unused and available for use. When this flag is set, we shouldn't look at anything else in the resource structure. This flag is set when a resource table entry is initialized. IORESOURCE_AUTO This resource was assigned automatically by pnp_assign_{io,mem,etc}(). This flag is set when a resource table entry is initialized and cleared whenever we discover a resource setting by reading an ISAPNP config register, parsing a PNPBIOS resource data stream, parsing an ACPI _CRS list, or interpreting a sysfs "set" command. Resources marked IORESOURCE_AUTO are reinitialized and marked as IORESOURCE_UNSET by pnp_clean_resource_table() in these cases: - before we attempt to assign resources automatically, - if we fail to assign resources automatically, - after disabling a device IORESOURCE_DISABLED Set by pnp_assign_{io,mem,etc}() when automatic assignment fails. Also set by PNPBIOS and PNPACPI for: - invalid IRQs or GSI registration failures - invalid DMA channels - I/O ports above 0x10000 - mem ranges with negative length After this patch, there is no pnp_resource_table, and the resource list entries use the flags like this: IORESOURCE_UNSET This flag is no longer used in PNP. Instead of keeping IORESOURCE_UNSET entries in the resource list, we remove entries from the list and free them. IORESOURCE_AUTO No change in meaning: it still means the resource was assigned automatically by pnp_assign_{port,mem,etc}(), but these functions now set the bit explicitly. We still "clean" a device's resource list in the same places, but rather than reinitializing IORESOURCE_AUTO entries, we just remove them from the list. Note that IORESOURCE_AUTO entries are always at the end of the list, so removing them doesn't reorder other list entries. This is because non-IORESOURCE_AUTO entries are added by the ISAPNP, PNPBIOS, or PNPACPI "get resources" methods and by the sysfs "set" command. In each of these cases, we completely free the resource list first. IORESOURCE_DISABLED In addition to the cases where we used to set this flag, ISAPNP now adds an IORESOURCE_DISABLED resource when it reads a configuration register with a "disabled" value. [Rene, I made isapnp_set_resources() skip IORESOURCE_DISABLED resources, because that's effectively what we used to do. I think it *ought* to work to write zero (or 4 for DMA) back to a configuration register to disable it, and that might even be useful as a way to manually disable a single resource, but for now it seems safer to preserve the previous behavior.] Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com> --- drivers/pnp/base.h | 18 --- drivers/pnp/core.c | 25 +++-- drivers/pnp/interface.c | 60 +++++------- drivers/pnp/isapnp/core.c | 8 - drivers/pnp/manager.c | 218 +++++++++++----------------------------------- drivers/pnp/quirks.c | 3 drivers/pnp/resource.c | 86 +++--------------- drivers/pnp/support.c | 68 +++++--------- drivers/pnp/system.c | 8 - include/linux/pnp.h | 5 - 10 files changed, 150 insertions(+), 349 deletions(-) Index: work10/drivers/pnp/base.h =================================================================== --- work10.orig/drivers/pnp/base.h 2008-05-05 16:09:47.000000000 -0600 +++ work10/drivers/pnp/base.h 2008-05-05 16:09:51.000000000 -0600 @@ -45,27 +45,15 @@ int pnp_check_dma(struct pnp_dev *dev, s char *pnp_resource_type_name(struct resource *res); void dbg_pnp_show_resources(struct pnp_dev *dev, char *desc); -void pnp_init_resource(struct resource *res); +void pnp_free_resources(struct pnp_dev *dev); int pnp_resource_type(struct resource *res); -struct pnp_resource *pnp_get_pnp_resource(struct pnp_dev *dev, - unsigned int type, unsigned int num); - -#define PNP_MAX_PORT 40 -#define PNP_MAX_MEM 24 -#define PNP_MAX_IRQ 2 -#define PNP_MAX_DMA 2 - struct pnp_resource { + struct list_head list; struct resource res; }; -struct pnp_resource_table { - struct pnp_resource port[PNP_MAX_PORT]; - struct pnp_resource mem[PNP_MAX_MEM]; - struct pnp_resource dma[PNP_MAX_DMA]; - struct pnp_resource irq[PNP_MAX_IRQ]; -}; +void pnp_free_resource(struct pnp_resource *pnp_res); struct pnp_resource *pnp_add_irq_resource(struct pnp_dev *dev, int irq, int flags); Index: work10/drivers/pnp/core.c =================================================================== --- work10.orig/drivers/pnp/core.c 2008-05-05 15:58:46.000000000 -0600 +++ work10/drivers/pnp/core.c 2008-05-05 16:09:51.000000000 -0600 @@ -99,6 +99,21 @@ static void pnp_free_ids(struct pnp_dev } } +void pnp_free_resource(struct pnp_resource *pnp_res) +{ + list_del(&pnp_res->list); + kfree(pnp_res); +} + +void pnp_free_resources(struct pnp_dev *dev) +{ + struct pnp_resource *pnp_res, *tmp; + + list_for_each_entry_safe(pnp_res, tmp, &dev->resources, list) { + pnp_free_resource(pnp_res); + } +} + static void pnp_release_device(struct device *dmdev) { struct pnp_dev *dev = to_pnp_dev(dmdev); @@ -106,7 +121,7 @@ static void pnp_release_device(struct de pnp_free_option(dev->independent); pnp_free_option(dev->dependent); pnp_free_ids(dev); - kfree(dev->res); + pnp_free_resources(dev); kfree(dev); } @@ -119,12 +134,7 @@ struct pnp_dev *pnp_alloc_dev(struct pnp if (!dev) return NULL; - dev->res = kzalloc(sizeof(struct pnp_resource_table), GFP_KERNEL); - if (!dev->res) { - kfree(dev); - return NULL; - } - + INIT_LIST_HEAD(&dev->resources); dev->protocol = protocol; dev->number = id; dev->dma_mask = DMA_24BIT_MASK; @@ -140,7 +150,6 @@ struct pnp_dev *pnp_alloc_dev(struct pnp dev_id = pnp_add_id(dev, pnpid); if (!dev_id) { - kfree(dev->res); kfree(dev); return NULL; } Index: work10/drivers/pnp/interface.c =================================================================== --- work10.orig/drivers/pnp/interface.c 2008-05-05 15:59:00.000000000 -0600 +++ work10/drivers/pnp/interface.c 2008-05-05 16:09:51.000000000 -0600 @@ -269,46 +269,38 @@ static ssize_t pnp_show_current_resource pnp_printf(buffer, "disabled\n"); for (i = 0; (res = pnp_get_resource(dev, IORESOURCE_IO, i)); i++) { - if (pnp_resource_valid(res)) { - pnp_printf(buffer, "io"); - if (res->flags & IORESOURCE_DISABLED) - pnp_printf(buffer, " disabled\n"); - else - pnp_printf(buffer, " 0x%llx-0x%llx\n", - (unsigned long long) res->start, - (unsigned long long) res->end); - } + pnp_printf(buffer, "io"); + if (res->flags & IORESOURCE_DISABLED) + pnp_printf(buffer, " disabled\n"); + else + pnp_printf(buffer, " 0x%llx-0x%llx\n", + (unsigned long long) res->start, + (unsigned long long) res->end); } for (i = 0; (res = pnp_get_resource(dev, IORESOURCE_MEM, i)); i++) { - if (pnp_resource_valid(res)) { - pnp_printf(buffer, "mem"); - if (res->flags & IORESOURCE_DISABLED) - pnp_printf(buffer, " disabled\n"); - else - pnp_printf(buffer, " 0x%llx-0x%llx\n", - (unsigned long long) res->start, - (unsigned long long) res->end); - } + pnp_printf(buffer, "mem"); + if (res->flags & IORESOURCE_DISABLED) + pnp_printf(buffer, " disabled\n"); + else + pnp_printf(buffer, " 0x%llx-0x%llx\n", + (unsigned long long) res->start, + (unsigned long long) res->end); } for (i = 0; (res = pnp_get_resource(dev, IORESOURCE_IRQ, i)); i++) { - if (pnp_resource_valid(res)) { - pnp_printf(buffer, "irq"); - if (res->flags & IORESOURCE_DISABLED) - pnp_printf(buffer, " disabled\n"); - else - pnp_printf(buffer, " %lld\n", - (unsigned long long) res->start); - } + pnp_printf(buffer, "irq"); + if (res->flags & IORESOURCE_DISABLED) + pnp_printf(buffer, " disabled\n"); + else + pnp_printf(buffer, " %lld\n", + (unsigned long long) res->start); } for (i = 0; (res = pnp_get_resource(dev, IORESOURCE_DMA, i)); i++) { - if (pnp_resource_valid(res)) { - pnp_printf(buffer, "dma"); - if (res->flags & IORESOURCE_DISABLED) - pnp_printf(buffer, " disabled\n"); - else - pnp_printf(buffer, " %lld\n", - (unsigned long long) res->start); - } + pnp_printf(buffer, "dma"); + if (res->flags & IORESOURCE_DISABLED) + pnp_printf(buffer, " disabled\n"); + else + pnp_printf(buffer, " %lld\n", + (unsigned long long) res->start); } ret = (buffer->curr - buf); kfree(buffer); Index: work10/drivers/pnp/isapnp/core.c =================================================================== --- work10.orig/drivers/pnp/isapnp/core.c 2008-05-05 16:09:19.000000000 -0600 +++ work10/drivers/pnp/isapnp/core.c 2008-05-05 16:10:24.000000000 -0600 @@ -973,8 +973,7 @@ static int isapnp_set_resources(struct p dev->active = 1; for (tmp = 0; tmp < ISAPNP_MAX_PORT; tmp++) { res = pnp_get_resource(dev, IORESOURCE_IO, tmp); - if (res && pnp_resource_valid(res) && - !(res->flags & IORESOURCE_DISABLED)) { + if (res && !(res->flags & IORESOURCE_DISABLED)) { dev_dbg(&dev->dev, " set io %d to %#llx\n", tmp, (unsigned long long) res->start); isapnp_write_word(ISAPNP_CFG_PORT + (tmp << 1), @@ -983,8 +982,7 @@ static int isapnp_set_resources(struct p } for (tmp = 0; tmp < ISAPNP_MAX_IRQ; tmp++) { res = pnp_get_resource(dev, IORESOURCE_IRQ, tmp); - if (res && pnp_resource_valid(res) && - !(res->flags & IORESOURCE_DISABLED)) { + if (res && !(res->flags & IORESOURCE_DISABLED)) { int irq = res->start; if (irq == 2) irq = 9; @@ -994,8 +992,7 @@ static int isapnp_set_resources(struct p } for (tmp = 0; tmp < ISAPNP_MAX_DMA; tmp++) { res = pnp_get_resource(dev, IORESOURCE_DMA, tmp); - if (res && pnp_resource_valid(res) && - !(res->flags & IORESOURCE_DISABLED)) { + if (res && !(res->flags & IORESOURCE_DISABLED)) { dev_dbg(&dev->dev, " set dma %d to %lld\n", tmp, (unsigned long long) res->start); isapnp_write_byte(ISAPNP_CFG_DMA + tmp, res->start); @@ -1003,8 +1000,7 @@ static int isapnp_set_resources(struct p } for (tmp = 0; tmp < ISAPNP_MAX_MEM; tmp++) { res = pnp_get_resource(dev, IORESOURCE_MEM, tmp); - if (res && pnp_resource_valid(res) && - !(res->flags & IORESOURCE_DISABLED)) { + if (res && !(res->flags & IORESOURCE_DISABLED)) { dev_dbg(&dev->dev, " set mem %d to %#llx\n", tmp, (unsigned long long) res->start); isapnp_write_word(ISAPNP_CFG_MEM + (tmp << 3), Index: work10/drivers/pnp/manager.c =================================================================== --- work10.orig/drivers/pnp/manager.c 2008-05-05 15:59:00.000000000 -0600 +++ work10/drivers/pnp/manager.c 2008-05-05 16:09:51.000000000 -0600 @@ -19,40 +19,30 @@ DEFINE_MUTEX(pnp_res_mutex); static int pnp_assign_port(struct pnp_dev *dev, struct pnp_port *rule, int idx) { - struct pnp_resource *pnp_res; - struct resource *res; + struct resource *res, local_res; - pnp_res = pnp_get_pnp_resource(dev, IORESOURCE_IO, idx); - if (!pnp_res) { - dev_err(&dev->dev, "too many I/O port resources\n"); - /* pretend we were successful so at least the manager won't try again */ - return 1; - } - - res = &pnp_res->res; - - /* check if this resource has been manually set, if so skip */ - if (!(res->flags & IORESOURCE_AUTO)) { + res = pnp_get_resource(dev, IORESOURCE_IO, idx); + if (res) { dev_dbg(&dev->dev, " io %d already set to %#llx-%#llx " "flags %#lx\n", idx, (unsigned long long) res->start, (unsigned long long) res->end, res->flags); return 1; } - /* set the initial values */ - res->flags |= rule->flags | IORESOURCE_IO; - res->flags &= ~IORESOURCE_UNSET; + res = &local_res; + res->flags = rule->flags | IORESOURCE_AUTO; + res->start = 0; + res->end = 0; if (!rule->size) { res->flags |= IORESOURCE_DISABLED; dev_dbg(&dev->dev, " io %d disabled\n", idx); - return 1; /* skip disabled resource requests */ + goto __add; } res->start = rule->min; res->end = res->start + rule->size - 1; - /* run through until pnp_check_port is happy */ while (!pnp_check_port(dev, res)) { res->start += rule->align; res->end = res->start + rule->size - 1; @@ -61,38 +51,29 @@ static int pnp_assign_port(struct pnp_de return 0; } } - dev_dbg(&dev->dev, " assign io %d %#llx-%#llx\n", idx, - (unsigned long long) res->start, (unsigned long long) res->end); + +__add: + pnp_add_io_resource(dev, res->start, res->end, res->flags); return 1; } static int pnp_assign_mem(struct pnp_dev *dev, struct pnp_mem *rule, int idx) { - struct pnp_resource *pnp_res; - struct resource *res; - - pnp_res = pnp_get_pnp_resource(dev, IORESOURCE_MEM, idx); - if (!pnp_res) { - dev_err(&dev->dev, "too many memory resources\n"); - /* pretend we were successful so at least the manager won't try again */ - return 1; - } - - res = &pnp_res->res; + struct resource *res, local_res; - /* check if this resource has been manually set, if so skip */ - if (!(res->flags & IORESOURCE_AUTO)) { + res = pnp_get_resource(dev, IORESOURCE_MEM, idx); + if (res) { dev_dbg(&dev->dev, " mem %d already set to %#llx-%#llx " "flags %#lx\n", idx, (unsigned long long) res->start, (unsigned long long) res->end, res->flags); return 1; } - /* set the initial values */ - res->flags |= rule->flags | IORESOURCE_MEM; - res->flags &= ~IORESOURCE_UNSET; + res = &local_res; + res->flags = rule->flags | IORESOURCE_AUTO; + res->start = 0; + res->end = 0; - /* convert pnp flags to standard Linux flags */ if (!(rule->flags & IORESOURCE_MEM_WRITEABLE)) res->flags |= IORESOURCE_READONLY; if (rule->flags & IORESOURCE_MEM_CACHEABLE) @@ -105,13 +86,12 @@ static int pnp_assign_mem(struct pnp_dev if (!rule->size) { res->flags |= IORESOURCE_DISABLED; dev_dbg(&dev->dev, " mem %d disabled\n", idx); - return 1; /* skip disabled resource requests */ + goto __add; } res->start = rule->min; res->end = res->start + rule->size - 1; - /* run through until pnp_check_mem is happy */ while (!pnp_check_mem(dev, res)) { res->start += rule->align; res->end = res->start + rule->size - 1; @@ -120,15 +100,15 @@ static int pnp_assign_mem(struct pnp_dev return 0; } } - dev_dbg(&dev->dev, " assign mem %d %#llx-%#llx\n", idx, - (unsigned long long) res->start, (unsigned long long) res->end); + +__add: + pnp_add_mem_resource(dev, res->start, res->end, res->flags); return 1; } static int pnp_assign_irq(struct pnp_dev *dev, struct pnp_irq *rule, int idx) { - struct pnp_resource *pnp_res; - struct resource *res; + struct resource *res, local_res; int i; /* IRQ priority: this table is good for i386 */ @@ -136,58 +116,48 @@ static int pnp_assign_irq(struct pnp_dev 5, 10, 11, 12, 9, 14, 15, 7, 3, 4, 13, 0, 1, 6, 8, 2 }; - pnp_res = pnp_get_pnp_resource(dev, IORESOURCE_IRQ, idx); - if (!pnp_res) { - dev_err(&dev->dev, "too many IRQ resources\n"); - /* pretend we were successful so at least the manager won't try again */ - return 1; - } - - res = &pnp_res->res; - - /* check if this resource has been manually set, if so skip */ - if (!(res->flags & IORESOURCE_AUTO)) { + res = pnp_get_resource(dev, IORESOURCE_IRQ, idx); + if (res) { dev_dbg(&dev->dev, " irq %d already set to %d flags %#lx\n", idx, (int) res->start, res->flags); return 1; } - /* set the initial values */ - res->flags |= rule->flags | IORESOURCE_IRQ; - res->flags &= ~IORESOURCE_UNSET; + res = &local_res; + res->flags = rule->flags | IORESOURCE_AUTO; + res->start = -1; + res->end = -1; if (bitmap_empty(rule->map, PNP_IRQ_NR)) { res->flags |= IORESOURCE_DISABLED; dev_dbg(&dev->dev, " irq %d disabled\n", idx); - return 1; /* skip disabled resource requests */ + goto __add; } /* TBD: need check for >16 IRQ */ res->start = find_next_bit(rule->map, PNP_IRQ_NR, 16); if (res->start < PNP_IRQ_NR) { res->end = res->start; - dev_dbg(&dev->dev, " assign irq %d %d\n", idx, - (int) res->start); - return 1; + goto __add; } for (i = 0; i < 16; i++) { if (test_bit(xtab[i], rule->map)) { res->start = res->end = xtab[i]; - if (pnp_check_irq(dev, res)) { - dev_dbg(&dev->dev, " assign irq %d %d\n", idx, - (int) res->start); - return 1; - } + if (pnp_check_irq(dev, res)) + goto __add; } } dev_dbg(&dev->dev, " couldn't assign irq %d\n", idx); return 0; + +__add: + pnp_add_irq_resource(dev, res->start, res->flags); + return 1; } static void pnp_assign_dma(struct pnp_dev *dev, struct pnp_dma *rule, int idx) { - struct pnp_resource *pnp_res; - struct resource *res; + struct resource *res, local_res; int i; /* DMA priority: this table is good for i386 */ @@ -195,127 +165,47 @@ static void pnp_assign_dma(struct pnp_de 1, 3, 5, 6, 7, 0, 2, 4 }; - pnp_res = pnp_get_pnp_resource(dev, IORESOURCE_DMA, idx); - if (!pnp_res) { - dev_err(&dev->dev, "too many DMA resources\n"); - return; - } - - res = &pnp_res->res; - - /* check if this resource has been manually set, if so skip */ - if (!(res->flags & IORESOURCE_AUTO)) { + res = pnp_get_resource(dev, IORESOURCE_DMA, idx); + if (res) { dev_dbg(&dev->dev, " dma %d already set to %d flags %#lx\n", idx, (int) res->start, res->flags); return; } - /* set the initial values */ - res->flags |= rule->flags | IORESOURCE_DMA; - res->flags &= ~IORESOURCE_UNSET; + res = &local_res; + res->flags = rule->flags | IORESOURCE_AUTO; + res->start = -1; + res->end = -1; for (i = 0; i < 8; i++) { if (rule->map & (1 << xtab[i])) { res->start = res->end = xtab[i]; - if (pnp_check_dma(dev, res)) { - dev_dbg(&dev->dev, " assign dma %d %d\n", idx, - (int) res->start); - return; - } + if (pnp_check_dma(dev, res)) + goto __add; } } #ifdef MAX_DMA_CHANNELS res->start = res->end = MAX_DMA_CHANNELS; #endif - res->flags |= IORESOURCE_UNSET | IORESOURCE_DISABLED; + res->flags |= IORESOURCE_DISABLED; dev_dbg(&dev->dev, " disable dma %d\n", idx); -} -void pnp_init_resource(struct resource *res) -{ - unsigned long type; - - type = res->flags & (IORESOURCE_IO | IORESOURCE_MEM | - IORESOURCE_IRQ | IORESOURCE_DMA); - - res->name = NULL; - res->flags = type | IORESOURCE_AUTO | IORESOURCE_UNSET; - if (type == IORESOURCE_IRQ || type == IORESOURCE_DMA) { - res->start = -1; - res->end = -1; - } else { - res->start = 0; - res->end = 0; - } +__add: + pnp_add_dma_resource(dev, res->start, res->flags); } -/** - * pnp_init_resources - Resets a resource table to default values. - * @table: pointer to the desired resource table - */ void pnp_init_resources(struct pnp_dev *dev) { - struct resource *res; - int idx; - - for (idx = 0; idx < PNP_MAX_IRQ; idx++) { - res = &dev->res->irq[idx].res; - res->flags = IORESOURCE_IRQ; - pnp_init_resource(res); - } - for (idx = 0; idx < PNP_MAX_DMA; idx++) { - res = &dev->res->dma[idx].res; - res->flags = IORESOURCE_DMA; - pnp_init_resource(res); - } - for (idx = 0; idx < PNP_MAX_PORT; idx++) { - res = &dev->res->port[idx].res; - res->flags = IORESOURCE_IO; - pnp_init_resource(res); - } - for (idx = 0; idx < PNP_MAX_MEM; idx++) { - res = &dev->res->mem[idx].res; - res->flags = IORESOURCE_MEM; - pnp_init_resource(res); - } + pnp_free_resources(dev); } -/** - * pnp_clean_resources - clears resources that were not manually set - * @res: the resources to clean - */ static void pnp_clean_resource_table(struct pnp_dev *dev) { - struct resource *res; - int idx; + struct pnp_resource *pnp_res, *tmp; - for (idx = 0; idx < PNP_MAX_IRQ; idx++) { - res = &dev->res->irq[idx].res; - if (res->flags & IORESOURCE_AUTO) { - res->flags = IORESOURCE_IRQ; - pnp_init_resource(res); - } - } - for (idx = 0; idx < PNP_MAX_DMA; idx++) { - res = &dev->res->dma[idx].res; - if (res->flags & IORESOURCE_AUTO) { - res->flags = IORESOURCE_DMA; - pnp_init_resource(res); - } - } - for (idx = 0; idx < PNP_MAX_PORT; idx++) { - res = &dev->res->port[idx].res; - if (res->flags & IORESOURCE_AUTO) { - res->flags = IORESOURCE_IO; - pnp_init_resource(res); - } - } - for (idx = 0; idx < PNP_MAX_MEM; idx++) { - res = &dev->res->mem[idx].res; - if (res->flags & IORESOURCE_AUTO) { - res->flags = IORESOURCE_MEM; - pnp_init_resource(res); - } + list_for_each_entry_safe(pnp_res, tmp, &dev->resources, list) { + if (pnp_res->res.flags & IORESOURCE_AUTO) + pnp_free_resource(pnp_res); } } Index: work10/drivers/pnp/quirks.c =================================================================== --- work10.orig/drivers/pnp/quirks.c 2008-05-05 15:58:46.000000000 -0600 +++ work10/drivers/pnp/quirks.c 2008-05-05 16:09:51.000000000 -0600 @@ -141,8 +141,7 @@ static void quirk_system_pci_resources(s for (j = 0; (res = pnp_get_resource(dev, IORESOURCE_MEM, j)); j++) { - if (res->flags & IORESOURCE_UNSET || - (res->start == 0 && res->end == 0)) + if (res->start == 0 && res->end == 0) continue; pnp_start = res->start; Index: work10/drivers/pnp/resource.c =================================================================== --- work10.orig/drivers/pnp/resource.c 2008-05-05 16:09:46.000000000 -0600 +++ work10/drivers/pnp/resource.c 2008-05-05 16:09:51.000000000 -0600 @@ -237,7 +237,7 @@ void pnp_free_option(struct pnp_option * !((*(enda) < *(startb)) || (*(endb) < *(starta))) #define cannot_compare(flags) \ -((flags) & (IORESOURCE_UNSET | IORESOURCE_DISABLED)) +((flags) & IORESOURCE_DISABLED) int pnp_check_port(struct pnp_dev *dev, struct resource *res) { @@ -505,81 +505,31 @@ int pnp_resource_type(struct resource *r IORESOURCE_IRQ | IORESOURCE_DMA); } -struct pnp_resource *pnp_get_pnp_resource(struct pnp_dev *dev, - unsigned int type, unsigned int num) -{ - struct pnp_resource_table *res = dev->res; - - switch (type) { - case IORESOURCE_IO: - if (num >= PNP_MAX_PORT) - return NULL; - return &res->port[num]; - case IORESOURCE_MEM: - if (num >= PNP_MAX_MEM) - return NULL; - return &res->mem[num]; - case IORESOURCE_IRQ: - if (num >= PNP_MAX_IRQ) - return NULL; - return &res->irq[num]; - case IORESOURCE_DMA: - if (num >= PNP_MAX_DMA) - return NULL; - return &res->dma[num]; - } - return NULL; -} - struct resource *pnp_get_resource(struct pnp_dev *dev, unsigned int type, unsigned int num) { struct pnp_resource *pnp_res; + struct resource *res; - pnp_res = pnp_get_pnp_resource(dev, type, num); - if (pnp_res) - return &pnp_res->res; - + list_for_each_entry(pnp_res, &dev->resources, list) { + res = &pnp_res->res; + if (pnp_resource_type(res) == type && num-- == 0) + return res; + } return NULL; } EXPORT_SYMBOL(pnp_get_resource); -static struct pnp_resource *pnp_new_resource(struct pnp_dev *dev, int type) +static struct pnp_resource *pnp_new_resource(struct pnp_dev *dev) { struct pnp_resource *pnp_res; - int i; - switch (type) { - case IORESOURCE_IO: - for (i = 0; i < PNP_MAX_PORT; i++) { - pnp_res = pnp_get_pnp_resource(dev, IORESOURCE_IO, i); - if (pnp_res && !pnp_resource_valid(&pnp_res->res)) - return pnp_res; - } - break; - case IORESOURCE_MEM: - for (i = 0; i < PNP_MAX_MEM; i++) { - pnp_res = pnp_get_pnp_resource(dev, IORESOURCE_MEM, i); - if (pnp_res && !pnp_resource_valid(&pnp_res->res)) - return pnp_res; - } - break; - case IORESOURCE_IRQ: - for (i = 0; i < PNP_MAX_IRQ; i++) { - pnp_res = pnp_get_pnp_resource(dev, IORESOURCE_IRQ, i); - if (pnp_res && !pnp_resource_valid(&pnp_res->res)) - return pnp_res; - } - break; - case IORESOURCE_DMA: - for (i = 0; i < PNP_MAX_DMA; i++) { - pnp_res = pnp_get_pnp_resource(dev, IORESOURCE_DMA, i); - if (pnp_res && !pnp_resource_valid(&pnp_res->res)) - return pnp_res; - } - break; - } - return NULL; + pnp_res = kzalloc(sizeof(struct pnp_resource), GFP_KERNEL); + if (!pnp_res) + return NULL; + + list_add_tail(&pnp_res->list, &dev->resources); + return pnp_res; } struct pnp_resource *pnp_add_irq_resource(struct pnp_dev *dev, int irq, @@ -589,7 +539,7 @@ struct pnp_resource *pnp_add_irq_resourc struct resource *res; static unsigned char warned; - pnp_res = pnp_new_resource(dev, IORESOURCE_IRQ); + pnp_res = pnp_new_resource(dev); if (!pnp_res) { if (!warned) { dev_err(&dev->dev, "can't add resource for IRQ %d\n", @@ -615,7 +565,7 @@ struct pnp_resource *pnp_add_dma_resourc struct resource *res; static unsigned char warned; - pnp_res = pnp_new_resource(dev, IORESOURCE_DMA); + pnp_res = pnp_new_resource(dev); if (!pnp_res) { if (!warned) { dev_err(&dev->dev, "can't add resource for DMA %d\n", @@ -642,7 +592,7 @@ struct pnp_resource *pnp_add_io_resource struct resource *res; static unsigned char warned; - pnp_res = pnp_new_resource(dev, IORESOURCE_IO); + pnp_res = pnp_new_resource(dev); if (!pnp_res) { if (!warned) { dev_err(&dev->dev, "can't add resource for IO " @@ -671,7 +621,7 @@ struct pnp_resource *pnp_add_mem_resourc struct resource *res; static unsigned char warned; - pnp_res = pnp_new_resource(dev, IORESOURCE_MEM); + pnp_res = pnp_new_resource(dev); if (!pnp_res) { if (!warned) { dev_err(&dev->dev, "can't add resource for MEM " Index: work10/drivers/pnp/support.c =================================================================== --- work10.orig/drivers/pnp/support.c 2008-05-05 16:09:47.000000000 -0600 +++ work10/drivers/pnp/support.c 2008-05-05 16:09:51.000000000 -0600 @@ -70,54 +70,34 @@ char *pnp_resource_type_name(struct reso void dbg_pnp_show_resources(struct pnp_dev *dev, char *desc) { #ifdef DEBUG + struct pnp_resource *pnp_res; struct resource *res; - int i; dev_dbg(&dev->dev, "current resources: %s\n", desc); + list_for_each_entry(pnp_res, &dev->resources, list) { + res = &pnp_res->res; - for (i = 0; i < PNP_MAX_IRQ; i++) { - res = pnp_get_resource(dev, IORESOURCE_IRQ, i); - if (res && !(res->flags & IORESOURCE_UNSET)) - dev_dbg(&dev->dev, " irq %lld flags %#lx%s%s\n", - (unsigned long long) res->start, res->flags, - res->flags & IORESOURCE_DISABLED ? - " DISABLED" : "", - res->flags & IORESOURCE_AUTO ? - " AUTO" : ""); - } - for (i = 0; i < PNP_MAX_DMA; i++) { - res = pnp_get_resource(dev, IORESOURCE_DMA, i); - if (res && !(res->flags & IORESOURCE_UNSET)) - dev_dbg(&dev->dev, " dma %lld flags %#lx%s%s\n", - (unsigned long long) res->start, res->flags, - res->flags & IORESOURCE_DISABLED ? - " DISABLED" : "", - res->flags & IORESOURCE_AUTO ? - " AUTO" : ""); - } - for (i = 0; i < PNP_MAX_PORT; i++) { - res = pnp_get_resource(dev, IORESOURCE_IO, i); - if (res && !(res->flags & IORESOURCE_UNSET)) - dev_dbg(&dev->dev, " io %#llx-%#llx flags %#lx" - "%s%s\n", - (unsigned long long) res->start, - (unsigned long long) res->end, res->flags, - res->flags & IORESOURCE_DISABLED ? - " DISABLED" : "", - res->flags & IORESOURCE_AUTO ? - " AUTO" : ""); - } - for (i = 0; i < PNP_MAX_MEM; i++) { - res = pnp_get_resource(dev, IORESOURCE_MEM, i); - if (res && !(res->flags & IORESOURCE_UNSET)) - dev_dbg(&dev->dev, " mem %#llx-%#llx flags %#lx" - "%s%s\n", - (unsigned long long) res->start, - (unsigned long long) res->end, res->flags, - res->flags & IORESOURCE_DISABLED ? - " DISABLED" : "", - res->flags & IORESOURCE_AUTO ? - " AUTO" : ""); + dev_dbg(&dev->dev, " %-3s ", pnp_resource_type_name(res)); + + if (res->flags & IORESOURCE_DISABLED) { + printk("disabled\n"); + continue; + } + + switch (pnp_resource_type(res)) { + case IORESOURCE_IO: + case IORESOURCE_MEM: + printk("%#llx-%#llx flags %#lx", + (unsigned long long) res->start, + (unsigned long long) res->end, res->flags); + break; + case IORESOURCE_IRQ: + case IORESOURCE_DMA: + printk("%lld flags %#lx", + (unsigned long long) res->start, res->flags); + break; + } + printk("\n"); } #endif } Index: work10/drivers/pnp/system.c =================================================================== --- work10.orig/drivers/pnp/system.c 2008-05-05 15:58:46.000000000 -0600 +++ work10/drivers/pnp/system.c 2008-05-05 16:09:51.000000000 -0600 @@ -60,8 +60,6 @@ static void reserve_resources_of_dev(str int i; for (i = 0; (res = pnp_get_resource(dev, IORESOURCE_IO, i)); i++) { - if (res->flags & IORESOURCE_UNSET) - continue; if (res->start == 0) continue; /* disabled */ if (res->start < 0x100) @@ -80,12 +78,8 @@ static void reserve_resources_of_dev(str reserve_range(dev, res->start, res->end, 1); } - for (i = 0; (res = pnp_get_resource(dev, IORESOURCE_MEM, i)); i++) { - if (res->flags & IORESOURCE_UNSET) - continue; - + for (i = 0; (res = pnp_get_resource(dev, IORESOURCE_MEM, i)); i++) reserve_range(dev, res->start, res->end, 0); - } } static int system_pnp_probe(struct pnp_dev *dev, Index: work10/include/linux/pnp.h =================================================================== --- work10.orig/include/linux/pnp.h 2008-05-05 15:58:46.000000000 -0600 +++ work10/include/linux/pnp.h 2008-05-05 16:09:51.000000000 -0600 @@ -15,7 +15,6 @@ struct pnp_protocol; struct pnp_dev; -struct pnp_resource_table; /* * Resource Management @@ -24,7 +23,7 @@ struct resource *pnp_get_resource(struct static inline int pnp_resource_valid(struct resource *res) { - if (res && !(res->flags & IORESOURCE_UNSET)) + if (res) return 1; return 0; } @@ -248,7 +247,7 @@ struct pnp_dev { int capabilities; struct pnp_option *independent; struct pnp_option *dependent; - struct pnp_resource_table *res; + struct list_head resources; char name[PNP_NAME_LEN]; /* contains a human-readable name */ int flags; /* used by protocols */ -- ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [patch 6/8] PNP: replace pnp_resource_table with dynamically allocated resources 2008-05-05 22:36 ` Bjorn Helgaas (?) @ 2008-05-14 2:42 ` Andrew Morton 2008-05-14 23:18 ` Bjorn Helgaas -1 siblings, 1 reply; 27+ messages in thread From: Andrew Morton @ 2008-05-14 2:42 UTC (permalink / raw) To: Bjorn Helgaas Cc: Len Brown, linux-acpi, linux-kernel, Adam Belay, Adam M Belay, Li Shaohua, Matthieu Castet, Thomas Renninger, Rene Herman, Jaroslav Kysela On Mon, 05 May 2008 16:36:36 -0600 Bjorn Helgaas <bjorn.helgaas@hp.com> wrote: > PNP used to have a fixed-size pnp_resource_table for tracking the > resources used by a device. This table often overflowed, so we've > had to increase the table size, which wastes memory because most > devices have very few resources. > > This patch replaces the table with a linked list of resources where > the entries are allocated on demand. > > This removes messages like these: > > pnpacpi: exceeded the max number of IO resources > 00:01: too many I/O port resources > > References: > > http://bugzilla.kernel.org/show_bug.cgi?id=9535 > http://bugzilla.kernel.org/show_bug.cgi?id=9740 > http://lkml.org/lkml/2007/11/30/110 > > This patch also changes the way PNP uses the IORESOURCE_UNSET, > IORESOURCE_AUTO, and IORESOURCE_DISABLED flags. This patch kills my prehistoric dual PIII. http://userweb.kernel.org/~akpm/p5135912.jpg http://userweb.kernel.org/~akpm/config-vmm.txt http://userweb.kernel.org/~akpm/dmesg-vmm.txt ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [patch 6/8] PNP: replace pnp_resource_table with dynamically allocated resources 2008-05-14 2:42 ` Andrew Morton @ 2008-05-14 23:18 ` Bjorn Helgaas 2008-05-15 7:54 ` Rene Herman 0 siblings, 1 reply; 27+ messages in thread From: Bjorn Helgaas @ 2008-05-14 23:18 UTC (permalink / raw) To: Andrew Morton Cc: Len Brown, linux-acpi, linux-kernel, Adam Belay, Adam M Belay, Li Shaohua, Matthieu Castet, Thomas Renninger, Rene Herman, Jaroslav Kysela On Tuesday 13 May 2008 08:42:41 pm Andrew Morton wrote: > On Mon, 05 May 2008 16:36:36 -0600 Bjorn Helgaas <bjorn.helgaas@hp.com> wrote: > > > PNP used to have a fixed-size pnp_resource_table for tracking the > > resources used by a device. This table often overflowed, so we've > > had to increase the table size, which wastes memory because most > > devices have very few resources. > > > > This patch replaces the table with a linked list of resources where > > the entries are allocated on demand. > > > > This removes messages like these: > > > > pnpacpi: exceeded the max number of IO resources > > 00:01: too many I/O port resources > > > > References: > > > > http://bugzilla.kernel.org/show_bug.cgi?id=9535 > > http://bugzilla.kernel.org/show_bug.cgi?id=9740 > > http://lkml.org/lkml/2007/11/30/110 > > > > This patch also changes the way PNP uses the IORESOURCE_UNSET, > > IORESOURCE_AUTO, and IORESOURCE_DISABLED flags. > > This patch kills my prehistoric dual PIII. > > http://userweb.kernel.org/~akpm/p5135912.jpg > http://userweb.kernel.org/~akpm/config-vmm.txt > http://userweb.kernel.org/~akpm/dmesg-vmm.txt Thanks for the report. I think I've fixed the problem. You had to remove the following three patches: pnp-replace-pnp_resource_table-with-dynamically-allocated-resources.patch pnp-remove-ratelimit-on-add-resource-failures.patch pnp-dont-sort-by-type-in-sys-resources.patch I'll post a new set of four patches (I added one patch to fix this problem) to replace those. They should go at the same point in the series as the three you removed. Bjorn ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [patch 6/8] PNP: replace pnp_resource_table with dynamically allocated resources 2008-05-14 23:18 ` Bjorn Helgaas @ 2008-05-15 7:54 ` Rene Herman 2008-05-16 17:17 ` Bjorn Helgaas 0 siblings, 1 reply; 27+ messages in thread From: Rene Herman @ 2008-05-15 7:54 UTC (permalink / raw) To: Bjorn Helgaas Cc: Andrew Morton, Len Brown, linux-acpi, linux-kernel, Adam Belay, Adam M Belay, Li Shaohua, Matthieu Castet, Thomas Renninger, Jaroslav Kysela On 15-05-08 01:18, Bjorn Helgaas wrote: > Thanks for the report. I think I've fixed the problem. I saw it was PnPBIOS by the way. PnPBIOS appears to be obsolete these days; do you have machines to test it on yourself? I do (plenty), so although I'm not being fast with testing anyway, would you want me to switch those from PnPACPI to PnPBIOS? Rene. ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [patch 6/8] PNP: replace pnp_resource_table with dynamically allocated resources 2008-05-15 7:54 ` Rene Herman @ 2008-05-16 17:17 ` Bjorn Helgaas 0 siblings, 0 replies; 27+ messages in thread From: Bjorn Helgaas @ 2008-05-16 17:17 UTC (permalink / raw) To: Rene Herman Cc: Andrew Morton, Len Brown, linux-acpi, linux-kernel, Adam Belay, Adam M Belay, Li Shaohua, Matthieu Castet, Thomas Renninger, Jaroslav Kysela On Thursday 15 May 2008 01:54:42 am Rene Herman wrote: > On 15-05-08 01:18, Bjorn Helgaas wrote: > > > Thanks for the report. I think I've fixed the problem. > > I saw it was PnPBIOS by the way. PnPBIOS appears to be obsolete these > days; do you have machines to test it on yourself? I do (plenty), so > although I'm not being fast with testing anyway, would you want me to > switch those from PnPACPI to PnPBIOS? All of my machines have ACPI, but in this case, I was able to reproduce the null pointer dereference Andrew by booting with "pnpacpi=off". PNPBIOS testing is pretty scarce, though, so anything you do along those lines would be much appreciated. Bjorn ^ permalink raw reply [flat|nested] 27+ messages in thread
* [patch 7/8] PNP: remove ratelimit on add resource failures 2008-05-05 22:36 ` Bjorn Helgaas @ 2008-05-05 22:36 ` Bjorn Helgaas -1 siblings, 0 replies; 27+ messages in thread From: Bjorn Helgaas @ 2008-05-05 22:36 UTC (permalink / raw) To: Len Brown Cc: linux-acpi, linux-kernel, Adam Belay, Adam M Belay, Li Shaohua, Matthieu Castet, Thomas Renninger, Rene Herman, Jaroslav Kysela, Andrew Morton [-- Attachment #1: pnp-remove-warning-ratelimit --] [-- Type: text/plain, Size: 2527 bytes --] We used to have a fixed-size resource table. If a device had twenty resources when the table only had space for ten, we didn't need ten warnings, so we added the ratelimit. Now that we can dynamically allocate new resources, we should only get failures if the allocation fails. That should be rare enough that we don't need to ratelimit the messages. Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com> Index: work10/drivers/pnp/resource.c =================================================================== --- work10.orig/drivers/pnp/resource.c 2008-05-05 11:38:16.000000000 -0600 +++ work10/drivers/pnp/resource.c 2008-05-05 11:49:05.000000000 -0600 @@ -537,15 +537,10 @@ struct pnp_resource *pnp_add_irq_resourc { struct pnp_resource *pnp_res; struct resource *res; - static unsigned char warned; pnp_res = pnp_new_resource(dev); if (!pnp_res) { - if (!warned) { - dev_err(&dev->dev, "can't add resource for IRQ %d\n", - irq); - warned = 1; - } + dev_err(&dev->dev, "can't add resource for IRQ %d\n", irq); return NULL; } @@ -563,15 +558,10 @@ struct pnp_resource *pnp_add_dma_resourc { struct pnp_resource *pnp_res; struct resource *res; - static unsigned char warned; pnp_res = pnp_new_resource(dev); if (!pnp_res) { - if (!warned) { - dev_err(&dev->dev, "can't add resource for DMA %d\n", - dma); - warned = 1; - } + dev_err(&dev->dev, "can't add resource for DMA %d\n", dma); return NULL; } @@ -590,16 +580,12 @@ struct pnp_resource *pnp_add_io_resource { struct pnp_resource *pnp_res; struct resource *res; - static unsigned char warned; pnp_res = pnp_new_resource(dev); if (!pnp_res) { - if (!warned) { - dev_err(&dev->dev, "can't add resource for IO " - "%#llx-%#llx\n",(unsigned long long) start, - (unsigned long long) end); - warned = 1; - } + dev_err(&dev->dev, "can't add resource for IO %#llx-%#llx\n", + (unsigned long long) start, + (unsigned long long) end); return NULL; } @@ -619,16 +605,12 @@ struct pnp_resource *pnp_add_mem_resourc { struct pnp_resource *pnp_res; struct resource *res; - static unsigned char warned; pnp_res = pnp_new_resource(dev); if (!pnp_res) { - if (!warned) { - dev_err(&dev->dev, "can't add resource for MEM " - "%#llx-%#llx\n",(unsigned long long) start, - (unsigned long long) end); - warned = 1; - } + dev_err(&dev->dev, "can't add resource for MEM %#llx-%#llx\n", + (unsigned long long) start, + (unsigned long long) end); return NULL; } -- ^ permalink raw reply [flat|nested] 27+ messages in thread
* [patch 7/8] PNP: remove ratelimit on add resource failures @ 2008-05-05 22:36 ` Bjorn Helgaas 0 siblings, 0 replies; 27+ messages in thread From: Bjorn Helgaas @ 2008-05-05 22:36 UTC (permalink / raw) To: Len Brown Cc: linux-acpi, linux-kernel, Adam Belay, Adam M Belay, Li Shaohua, Matthieu Castet, Thomas Renninger, Rene Herman, Jaroslav Kysela, Andrew Morton [-- Attachment #1: pnp-remove-warning-ratelimit --] [-- Type: text/plain, Size: 2527 bytes --] We used to have a fixed-size resource table. If a device had twenty resources when the table only had space for ten, we didn't need ten warnings, so we added the ratelimit. Now that we can dynamically allocate new resources, we should only get failures if the allocation fails. That should be rare enough that we don't need to ratelimit the messages. Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com> Index: work10/drivers/pnp/resource.c =================================================================== --- work10.orig/drivers/pnp/resource.c 2008-05-05 11:38:16.000000000 -0600 +++ work10/drivers/pnp/resource.c 2008-05-05 11:49:05.000000000 -0600 @@ -537,15 +537,10 @@ struct pnp_resource *pnp_add_irq_resourc { struct pnp_resource *pnp_res; struct resource *res; - static unsigned char warned; pnp_res = pnp_new_resource(dev); if (!pnp_res) { - if (!warned) { - dev_err(&dev->dev, "can't add resource for IRQ %d\n", - irq); - warned = 1; - } + dev_err(&dev->dev, "can't add resource for IRQ %d\n", irq); return NULL; } @@ -563,15 +558,10 @@ struct pnp_resource *pnp_add_dma_resourc { struct pnp_resource *pnp_res; struct resource *res; - static unsigned char warned; pnp_res = pnp_new_resource(dev); if (!pnp_res) { - if (!warned) { - dev_err(&dev->dev, "can't add resource for DMA %d\n", - dma); - warned = 1; - } + dev_err(&dev->dev, "can't add resource for DMA %d\n", dma); return NULL; } @@ -590,16 +580,12 @@ struct pnp_resource *pnp_add_io_resource { struct pnp_resource *pnp_res; struct resource *res; - static unsigned char warned; pnp_res = pnp_new_resource(dev); if (!pnp_res) { - if (!warned) { - dev_err(&dev->dev, "can't add resource for IO " - "%#llx-%#llx\n",(unsigned long long) start, - (unsigned long long) end); - warned = 1; - } + dev_err(&dev->dev, "can't add resource for IO %#llx-%#llx\n", + (unsigned long long) start, + (unsigned long long) end); return NULL; } @@ -619,16 +605,12 @@ struct pnp_resource *pnp_add_mem_resourc { struct pnp_resource *pnp_res; struct resource *res; - static unsigned char warned; pnp_res = pnp_new_resource(dev); if (!pnp_res) { - if (!warned) { - dev_err(&dev->dev, "can't add resource for MEM " - "%#llx-%#llx\n",(unsigned long long) start, - (unsigned long long) end); - warned = 1; - } + dev_err(&dev->dev, "can't add resource for MEM %#llx-%#llx\n", + (unsigned long long) start, + (unsigned long long) end); return NULL; } -- ^ permalink raw reply [flat|nested] 27+ messages in thread
* [patch 8/8] PNP: dont sort by type in /sys/.../resources 2008-05-05 22:36 ` Bjorn Helgaas @ 2008-05-05 22:36 ` Bjorn Helgaas -1 siblings, 0 replies; 27+ messages in thread From: Bjorn Helgaas @ 2008-05-05 22:36 UTC (permalink / raw) To: Len Brown Cc: linux-acpi, linux-kernel, Adam Belay, Adam M Belay, Li Shaohua, Matthieu Castet, Thomas Renninger, Rene Herman, Jaroslav Kysela, Andrew Morton [-- Attachment #1: pnp-unsort-sysfs-resources --] [-- Type: text/plain, Size: 3075 bytes --] Rather than stepping through all IO resources, then stepping through all MMIO resources, etc., we can just iterate over the resource list once directly. This can change the order in /sys, e.g., # cat /sys/devices/pnp0/00:07/resources # OLD state = active io 0x3f8-0x3ff irq 4 # cat /sys/devices/pnp0/00:07/resources # NEW state = active irq 4 io 0x3f8-0x3ff The old code artificially sorted resources by type; the new code just lists them in the order we read them from the ISAPNP hardware or the BIOS. Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com> Index: work10/drivers/pnp/interface.c =================================================================== --- work10.orig/drivers/pnp/interface.c 2008-05-05 11:54:26.000000000 -0600 +++ work10/drivers/pnp/interface.c 2008-05-05 11:59:53.000000000 -0600 @@ -248,8 +248,9 @@ static ssize_t pnp_show_current_resource char *buf) { struct pnp_dev *dev = to_pnp_dev(dmdev); + struct pnp_resource *pnp_res; struct resource *res; - int i, ret; + int ret; pnp_info_buffer_t *buffer; if (!dev) @@ -262,46 +263,33 @@ static ssize_t pnp_show_current_resource buffer->buffer = buf; buffer->curr = buffer->buffer; - pnp_printf(buffer, "state = "); - if (dev->active) - pnp_printf(buffer, "active\n"); - else - pnp_printf(buffer, "disabled\n"); - - for (i = 0; (res = pnp_get_resource(dev, IORESOURCE_IO, i)); i++) { - pnp_printf(buffer, "io"); - if (res->flags & IORESOURCE_DISABLED) - pnp_printf(buffer, " disabled\n"); - else - pnp_printf(buffer, " 0x%llx-0x%llx\n", - (unsigned long long) res->start, - (unsigned long long) res->end); - } - for (i = 0; (res = pnp_get_resource(dev, IORESOURCE_MEM, i)); i++) { - pnp_printf(buffer, "mem"); - if (res->flags & IORESOURCE_DISABLED) + pnp_printf(buffer, "state = %s\n", dev->active ? "active" : "disabled"); + + list_for_each_entry(pnp_res, &dev->resources, list) { + res = &pnp_res->res; + + pnp_printf(buffer, pnp_resource_type_name(res)); + + if (res->flags & IORESOURCE_DISABLED) { pnp_printf(buffer, " disabled\n"); - else - pnp_printf(buffer, " 0x%llx-0x%llx\n", + continue; + } + + switch (pnp_resource_type(res)) { + case IORESOURCE_IO: + case IORESOURCE_MEM: + pnp_printf(buffer, " %#llx-%#llx\n", (unsigned long long) res->start, (unsigned long long) res->end); - } - for (i = 0; (res = pnp_get_resource(dev, IORESOURCE_IRQ, i)); i++) { - pnp_printf(buffer, "irq"); - if (res->flags & IORESOURCE_DISABLED) - pnp_printf(buffer, " disabled\n"); - else - pnp_printf(buffer, " %lld\n", - (unsigned long long) res->start); - } - for (i = 0; (res = pnp_get_resource(dev, IORESOURCE_DMA, i)); i++) { - pnp_printf(buffer, "dma"); - if (res->flags & IORESOURCE_DISABLED) - pnp_printf(buffer, " disabled\n"); - else + break; + case IORESOURCE_IRQ: + case IORESOURCE_DMA: pnp_printf(buffer, " %lld\n", (unsigned long long) res->start); + break; + } } + ret = (buffer->curr - buf); kfree(buffer); return ret; -- ^ permalink raw reply [flat|nested] 27+ messages in thread
* [patch 8/8] PNP: dont sort by type in /sys/.../resources @ 2008-05-05 22:36 ` Bjorn Helgaas 0 siblings, 0 replies; 27+ messages in thread From: Bjorn Helgaas @ 2008-05-05 22:36 UTC (permalink / raw) To: Len Brown Cc: linux-acpi, linux-kernel, Adam Belay, Adam M Belay, Li Shaohua, Matthieu Castet, Thomas Renninger, Rene Herman, Jaroslav Kysela, Andrew Morton [-- Attachment #1: pnp-unsort-sysfs-resources --] [-- Type: text/plain, Size: 3075 bytes --] Rather than stepping through all IO resources, then stepping through all MMIO resources, etc., we can just iterate over the resource list once directly. This can change the order in /sys, e.g., # cat /sys/devices/pnp0/00:07/resources # OLD state = active io 0x3f8-0x3ff irq 4 # cat /sys/devices/pnp0/00:07/resources # NEW state = active irq 4 io 0x3f8-0x3ff The old code artificially sorted resources by type; the new code just lists them in the order we read them from the ISAPNP hardware or the BIOS. Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com> Index: work10/drivers/pnp/interface.c =================================================================== --- work10.orig/drivers/pnp/interface.c 2008-05-05 11:54:26.000000000 -0600 +++ work10/drivers/pnp/interface.c 2008-05-05 11:59:53.000000000 -0600 @@ -248,8 +248,9 @@ static ssize_t pnp_show_current_resource char *buf) { struct pnp_dev *dev = to_pnp_dev(dmdev); + struct pnp_resource *pnp_res; struct resource *res; - int i, ret; + int ret; pnp_info_buffer_t *buffer; if (!dev) @@ -262,46 +263,33 @@ static ssize_t pnp_show_current_resource buffer->buffer = buf; buffer->curr = buffer->buffer; - pnp_printf(buffer, "state = "); - if (dev->active) - pnp_printf(buffer, "active\n"); - else - pnp_printf(buffer, "disabled\n"); - - for (i = 0; (res = pnp_get_resource(dev, IORESOURCE_IO, i)); i++) { - pnp_printf(buffer, "io"); - if (res->flags & IORESOURCE_DISABLED) - pnp_printf(buffer, " disabled\n"); - else - pnp_printf(buffer, " 0x%llx-0x%llx\n", - (unsigned long long) res->start, - (unsigned long long) res->end); - } - for (i = 0; (res = pnp_get_resource(dev, IORESOURCE_MEM, i)); i++) { - pnp_printf(buffer, "mem"); - if (res->flags & IORESOURCE_DISABLED) + pnp_printf(buffer, "state = %s\n", dev->active ? "active" : "disabled"); + + list_for_each_entry(pnp_res, &dev->resources, list) { + res = &pnp_res->res; + + pnp_printf(buffer, pnp_resource_type_name(res)); + + if (res->flags & IORESOURCE_DISABLED) { pnp_printf(buffer, " disabled\n"); - else - pnp_printf(buffer, " 0x%llx-0x%llx\n", + continue; + } + + switch (pnp_resource_type(res)) { + case IORESOURCE_IO: + case IORESOURCE_MEM: + pnp_printf(buffer, " %#llx-%#llx\n", (unsigned long long) res->start, (unsigned long long) res->end); - } - for (i = 0; (res = pnp_get_resource(dev, IORESOURCE_IRQ, i)); i++) { - pnp_printf(buffer, "irq"); - if (res->flags & IORESOURCE_DISABLED) - pnp_printf(buffer, " disabled\n"); - else - pnp_printf(buffer, " %lld\n", - (unsigned long long) res->start); - } - for (i = 0; (res = pnp_get_resource(dev, IORESOURCE_DMA, i)); i++) { - pnp_printf(buffer, "dma"); - if (res->flags & IORESOURCE_DISABLED) - pnp_printf(buffer, " disabled\n"); - else + break; + case IORESOURCE_IRQ: + case IORESOURCE_DMA: pnp_printf(buffer, " %lld\n", (unsigned long long) res->start); + break; + } } + ret = (buffer->curr - buf); kfree(buffer); return ret; -- ^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2008-05-19 23:27 UTC | newest] Thread overview: 27+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2008-05-05 22:36 [patch 0/8] PNP: convert resource table to dynamic list, v1 Bjorn Helgaas 2008-05-05 22:36 ` Bjorn Helgaas 2008-05-05 22:36 ` [patch 1/8] PNP: set IRQ index in sysfs "set irq" interface Bjorn Helgaas 2008-05-05 22:36 ` Bjorn Helgaas 2008-05-05 22:36 ` [patch 2/8] PNP: add detail to debug resource dump Bjorn Helgaas 2008-05-05 22:36 ` Bjorn Helgaas 2008-05-19 21:47 ` Rene Herman 2008-05-05 22:36 ` [patch 3/8] PNP: remove pnp_resource.index Bjorn Helgaas 2008-05-05 22:36 ` Bjorn Helgaas 2008-05-19 22:01 ` Rene Herman 2008-05-19 23:27 ` Bjorn Helgaas 2008-05-05 22:36 ` [patch 4/8] PNP: add pnp_resource_type() internal interface Bjorn Helgaas 2008-05-05 22:36 ` Bjorn Helgaas 2008-05-19 22:02 ` Rene Herman 2008-05-05 22:36 ` [patch 5/8] PNP: add pnp_resource_type_name() helper function Bjorn Helgaas 2008-05-05 22:36 ` Bjorn Helgaas 2008-05-19 22:03 ` Rene Herman 2008-05-05 22:36 ` [patch 6/8] PNP: replace pnp_resource_table with dynamically allocated resources Bjorn Helgaas 2008-05-05 22:36 ` Bjorn Helgaas 2008-05-14 2:42 ` Andrew Morton 2008-05-14 23:18 ` Bjorn Helgaas 2008-05-15 7:54 ` Rene Herman 2008-05-16 17:17 ` Bjorn Helgaas 2008-05-05 22:36 ` [patch 7/8] PNP: remove ratelimit on add resource failures Bjorn Helgaas 2008-05-05 22:36 ` Bjorn Helgaas 2008-05-05 22:36 ` [patch 8/8] PNP: dont sort by type in /sys/.../resources Bjorn Helgaas 2008-05-05 22:36 ` Bjorn Helgaas
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.