* [patch 0/4] PNP: convert resource table to dynamic list, v2 @ 2008-05-15 22:07 ` Bjorn Helgaas 0 siblings, 0 replies; 18+ messages in thread From: Bjorn Helgaas @ 2008-05-15 22:07 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, Takashi Iwai This series replaces the fixed-size pnp_resource_table with a linked list of resources. This iteration fixes a null pointer dereference Andrew found on a PNPBIOS machine. Because of that problem, Andrew dropped the following patches from the -mm tree (these were patches 6-8 of the v1 series): 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 This posting doesn't repeat patches 1-5 from the original v1 series; the first is already in Linus's tree, and the rest are still in -mm. Bjorn -- ^ permalink raw reply [flat|nested] 18+ messages in thread
* [patch 0/4] PNP: convert resource table to dynamic list, v2 @ 2008-05-15 22:07 ` Bjorn Helgaas 0 siblings, 0 replies; 18+ messages in thread From: Bjorn Helgaas @ 2008-05-15 22:07 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, Takashi Iwai This series replaces the fixed-size pnp_resource_table with a linked list of resources. This iteration fixes a null pointer dereference Andrew found on a PNPBIOS machine. Because of that problem, Andrew dropped the following patches from the -mm tree (these were patches 6-8 of the v1 series): 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 This posting doesn't repeat patches 1-5 from the original v1 series; the first is already in Linus's tree, and the rest are still in -mm. Bjorn -- ^ permalink raw reply [flat|nested] 18+ messages in thread
* [patch 1/4] PNP: make pnp_{port,mem,etc}_start(), et al work for invalid resources 2008-05-15 22:07 ` Bjorn Helgaas @ 2008-05-15 22:07 ` Bjorn Helgaas -1 siblings, 0 replies; 18+ messages in thread From: Bjorn Helgaas @ 2008-05-15 22:07 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, Takashi Iwai [-- Attachment #1: pnp-make-accessors-work-for-invalid-bars --] [-- Type: text/plain, Size: 5068 bytes --] Some callers use pnp_port_start() and similar functions without making sure the resource is valid. This patch makes us fall back to returning the initial values if the resource is not valid or not even present. This mostly preserves the previous behavior, where we would just return the initial values set by pnp_init_resource_table(). The original 2.6.25 code didn't range-check the "bar", so it would return garbage if the bar exceeded the table size. This code returns sensible values instead. Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com> Index: work10/include/linux/pnp.h =================================================================== --- work10.orig/include/linux/pnp.h 2008-05-14 16:57:23.000000000 -0600 +++ work10/include/linux/pnp.h 2008-05-14 17:04:30.000000000 -0600 @@ -40,19 +40,31 @@ static inline resource_size_t pnp_resour static inline resource_size_t pnp_port_start(struct pnp_dev *dev, unsigned int bar) { - return pnp_get_resource(dev, IORESOURCE_IO, bar)->start; + struct resource *res = pnp_get_resource(dev, IORESOURCE_IO, bar); + + if (pnp_resource_valid(res)) + return res->start; + return 0; } static inline resource_size_t pnp_port_end(struct pnp_dev *dev, unsigned int bar) { - return pnp_get_resource(dev, IORESOURCE_IO, bar)->end; + struct resource *res = pnp_get_resource(dev, IORESOURCE_IO, bar); + + if (pnp_resource_valid(res)) + return res->end; + return 0; } static inline unsigned long pnp_port_flags(struct pnp_dev *dev, unsigned int bar) { - return pnp_get_resource(dev, IORESOURCE_IO, bar)->flags; + struct resource *res = pnp_get_resource(dev, IORESOURCE_IO, bar); + + if (pnp_resource_valid(res)) + return res->flags; + return IORESOURCE_IO | IORESOURCE_AUTO | IORESOURCE_UNSET; } static inline int pnp_port_valid(struct pnp_dev *dev, unsigned int bar) @@ -63,25 +75,41 @@ static inline int pnp_port_valid(struct static inline resource_size_t pnp_port_len(struct pnp_dev *dev, unsigned int bar) { - return pnp_resource_len(pnp_get_resource(dev, IORESOURCE_IO, bar)); + struct resource *res = pnp_get_resource(dev, IORESOURCE_IO, bar); + + if (pnp_resource_valid(res)) + return pnp_resource_len(res); + return 0; } static inline resource_size_t pnp_mem_start(struct pnp_dev *dev, unsigned int bar) { - return pnp_get_resource(dev, IORESOURCE_MEM, bar)->start; + struct resource *res = pnp_get_resource(dev, IORESOURCE_MEM, bar); + + if (pnp_resource_valid(res)) + return res->start; + return 0; } static inline resource_size_t pnp_mem_end(struct pnp_dev *dev, unsigned int bar) { - return pnp_get_resource(dev, IORESOURCE_MEM, bar)->end; + struct resource *res = pnp_get_resource(dev, IORESOURCE_MEM, bar); + + if (pnp_resource_valid(res)) + return res->end; + return 0; } static inline unsigned long pnp_mem_flags(struct pnp_dev *dev, unsigned int bar) { - return pnp_get_resource(dev, IORESOURCE_MEM, bar)->flags; + struct resource *res = pnp_get_resource(dev, IORESOURCE_MEM, bar); + + if (pnp_resource_valid(res)) + return res->flags; + return IORESOURCE_MEM | IORESOURCE_AUTO | IORESOURCE_UNSET; } static inline int pnp_mem_valid(struct pnp_dev *dev, unsigned int bar) @@ -92,18 +120,30 @@ static inline int pnp_mem_valid(struct p static inline resource_size_t pnp_mem_len(struct pnp_dev *dev, unsigned int bar) { - return pnp_resource_len(pnp_get_resource(dev, IORESOURCE_MEM, bar)); + struct resource *res = pnp_get_resource(dev, IORESOURCE_MEM, bar); + + if (pnp_resource_valid(res)) + return pnp_resource_len(res); + return 0; } static inline resource_size_t pnp_irq(struct pnp_dev *dev, unsigned int bar) { - return pnp_get_resource(dev, IORESOURCE_IRQ, bar)->start; + struct resource *res = pnp_get_resource(dev, IORESOURCE_IRQ, bar); + + if (pnp_resource_valid(res)) + return res->start; + return -1; } static inline unsigned long pnp_irq_flags(struct pnp_dev *dev, unsigned int bar) { - return pnp_get_resource(dev, IORESOURCE_IRQ, bar)->flags; + struct resource *res = pnp_get_resource(dev, IORESOURCE_IRQ, bar); + + if (pnp_resource_valid(res)) + return res->flags; + return IORESOURCE_IRQ | IORESOURCE_AUTO | IORESOURCE_UNSET; } static inline int pnp_irq_valid(struct pnp_dev *dev, unsigned int bar) @@ -114,12 +154,20 @@ static inline int pnp_irq_valid(struct p static inline resource_size_t pnp_dma(struct pnp_dev *dev, unsigned int bar) { - return pnp_get_resource(dev, IORESOURCE_DMA, bar)->start; + struct resource *res = pnp_get_resource(dev, IORESOURCE_DMA, bar); + + if (pnp_resource_valid(res)) + return res->start; + return -1; } static inline unsigned long pnp_dma_flags(struct pnp_dev *dev, unsigned int bar) { - return pnp_get_resource(dev, IORESOURCE_DMA, bar)->flags; + struct resource *res = pnp_get_resource(dev, IORESOURCE_DMA, bar); + + if (pnp_resource_valid(res)) + return res->flags; + return IORESOURCE_DMA | IORESOURCE_AUTO | IORESOURCE_UNSET; } static inline int pnp_dma_valid(struct pnp_dev *dev, unsigned int bar) -- ^ permalink raw reply [flat|nested] 18+ messages in thread
* [patch 1/4] PNP: make pnp_{port,mem,etc}_start(), et al work for invalid resources @ 2008-05-15 22:07 ` Bjorn Helgaas 0 siblings, 0 replies; 18+ messages in thread From: Bjorn Helgaas @ 2008-05-15 22:07 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, Takashi Iwai [-- Attachment #1: pnp-make-accessors-work-for-invalid-bars --] [-- Type: text/plain, Size: 5068 bytes --] Some callers use pnp_port_start() and similar functions without making sure the resource is valid. This patch makes us fall back to returning the initial values if the resource is not valid or not even present. This mostly preserves the previous behavior, where we would just return the initial values set by pnp_init_resource_table(). The original 2.6.25 code didn't range-check the "bar", so it would return garbage if the bar exceeded the table size. This code returns sensible values instead. Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com> Index: work10/include/linux/pnp.h =================================================================== --- work10.orig/include/linux/pnp.h 2008-05-14 16:57:23.000000000 -0600 +++ work10/include/linux/pnp.h 2008-05-14 17:04:30.000000000 -0600 @@ -40,19 +40,31 @@ static inline resource_size_t pnp_resour static inline resource_size_t pnp_port_start(struct pnp_dev *dev, unsigned int bar) { - return pnp_get_resource(dev, IORESOURCE_IO, bar)->start; + struct resource *res = pnp_get_resource(dev, IORESOURCE_IO, bar); + + if (pnp_resource_valid(res)) + return res->start; + return 0; } static inline resource_size_t pnp_port_end(struct pnp_dev *dev, unsigned int bar) { - return pnp_get_resource(dev, IORESOURCE_IO, bar)->end; + struct resource *res = pnp_get_resource(dev, IORESOURCE_IO, bar); + + if (pnp_resource_valid(res)) + return res->end; + return 0; } static inline unsigned long pnp_port_flags(struct pnp_dev *dev, unsigned int bar) { - return pnp_get_resource(dev, IORESOURCE_IO, bar)->flags; + struct resource *res = pnp_get_resource(dev, IORESOURCE_IO, bar); + + if (pnp_resource_valid(res)) + return res->flags; + return IORESOURCE_IO | IORESOURCE_AUTO | IORESOURCE_UNSET; } static inline int pnp_port_valid(struct pnp_dev *dev, unsigned int bar) @@ -63,25 +75,41 @@ static inline int pnp_port_valid(struct static inline resource_size_t pnp_port_len(struct pnp_dev *dev, unsigned int bar) { - return pnp_resource_len(pnp_get_resource(dev, IORESOURCE_IO, bar)); + struct resource *res = pnp_get_resource(dev, IORESOURCE_IO, bar); + + if (pnp_resource_valid(res)) + return pnp_resource_len(res); + return 0; } static inline resource_size_t pnp_mem_start(struct pnp_dev *dev, unsigned int bar) { - return pnp_get_resource(dev, IORESOURCE_MEM, bar)->start; + struct resource *res = pnp_get_resource(dev, IORESOURCE_MEM, bar); + + if (pnp_resource_valid(res)) + return res->start; + return 0; } static inline resource_size_t pnp_mem_end(struct pnp_dev *dev, unsigned int bar) { - return pnp_get_resource(dev, IORESOURCE_MEM, bar)->end; + struct resource *res = pnp_get_resource(dev, IORESOURCE_MEM, bar); + + if (pnp_resource_valid(res)) + return res->end; + return 0; } static inline unsigned long pnp_mem_flags(struct pnp_dev *dev, unsigned int bar) { - return pnp_get_resource(dev, IORESOURCE_MEM, bar)->flags; + struct resource *res = pnp_get_resource(dev, IORESOURCE_MEM, bar); + + if (pnp_resource_valid(res)) + return res->flags; + return IORESOURCE_MEM | IORESOURCE_AUTO | IORESOURCE_UNSET; } static inline int pnp_mem_valid(struct pnp_dev *dev, unsigned int bar) @@ -92,18 +120,30 @@ static inline int pnp_mem_valid(struct p static inline resource_size_t pnp_mem_len(struct pnp_dev *dev, unsigned int bar) { - return pnp_resource_len(pnp_get_resource(dev, IORESOURCE_MEM, bar)); + struct resource *res = pnp_get_resource(dev, IORESOURCE_MEM, bar); + + if (pnp_resource_valid(res)) + return pnp_resource_len(res); + return 0; } static inline resource_size_t pnp_irq(struct pnp_dev *dev, unsigned int bar) { - return pnp_get_resource(dev, IORESOURCE_IRQ, bar)->start; + struct resource *res = pnp_get_resource(dev, IORESOURCE_IRQ, bar); + + if (pnp_resource_valid(res)) + return res->start; + return -1; } static inline unsigned long pnp_irq_flags(struct pnp_dev *dev, unsigned int bar) { - return pnp_get_resource(dev, IORESOURCE_IRQ, bar)->flags; + struct resource *res = pnp_get_resource(dev, IORESOURCE_IRQ, bar); + + if (pnp_resource_valid(res)) + return res->flags; + return IORESOURCE_IRQ | IORESOURCE_AUTO | IORESOURCE_UNSET; } static inline int pnp_irq_valid(struct pnp_dev *dev, unsigned int bar) @@ -114,12 +154,20 @@ static inline int pnp_irq_valid(struct p static inline resource_size_t pnp_dma(struct pnp_dev *dev, unsigned int bar) { - return pnp_get_resource(dev, IORESOURCE_DMA, bar)->start; + struct resource *res = pnp_get_resource(dev, IORESOURCE_DMA, bar); + + if (pnp_resource_valid(res)) + return res->start; + return -1; } static inline unsigned long pnp_dma_flags(struct pnp_dev *dev, unsigned int bar) { - return pnp_get_resource(dev, IORESOURCE_DMA, bar)->flags; + struct resource *res = pnp_get_resource(dev, IORESOURCE_DMA, bar); + + if (pnp_resource_valid(res)) + return res->flags; + return IORESOURCE_DMA | IORESOURCE_AUTO | IORESOURCE_UNSET; } static inline int pnp_dma_valid(struct pnp_dev *dev, unsigned int bar) -- ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [patch 1/4] PNP: make pnp_{port,mem,etc}_start(), et al work for invalid resources 2008-05-15 22:07 ` Bjorn Helgaas (?) @ 2008-05-19 22:23 ` Rene Herman -1 siblings, 0 replies; 18+ messages in thread From: Rene Herman @ 2008-05-19 22:23 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, Takashi Iwai On 16-05-08 00:07, Bjorn Helgaas wrote: > Some callers use pnp_port_start() and similar functions without > making sure the resource is valid. This patch makes us fall > back to returning the initial values if the resource is not > valid or not even present. > > This mostly preserves the previous behavior, where we would just > return the initial values set by pnp_init_resource_table(). The > original 2.6.25 code didn't range-check the "bar", so it would > return garbage if the bar exceeded the table size. This code > returns sensible values instead. > > Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com> Acked-by: Rene Herman <rene.herman@gmail.com> Rene. ^ permalink raw reply [flat|nested] 18+ messages in thread
* [patch 2/4] PNP: replace pnp_resource_table with dynamically allocated resources 2008-05-15 22:07 ` Bjorn Helgaas @ 2008-05-15 22:07 ` Bjorn Helgaas -1 siblings, 0 replies; 18+ messages in thread From: Bjorn Helgaas @ 2008-05-15 22:07 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, Takashi Iwai [-- Attachment #1: pnp-convert-to-lists --] [-- Type: text/plain, Size: 31715 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. 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 | 12 -- drivers/pnp/manager.c | 218 +++++++++++----------------------------------- drivers/pnp/quirks.c | 3 drivers/pnp/resource.c | 86 +++--------------- drivers/pnp/support.c | 72 +++++---------- drivers/pnp/system.c | 8 - include/linux/pnp.h | 13 +- 10 files changed, 158 insertions(+), 357 deletions(-) Index: work10/drivers/pnp/base.h =================================================================== --- work10.orig/drivers/pnp/base.h 2008-05-15 15:23:29.000000000 -0600 +++ work10/drivers/pnp/base.h 2008-05-15 15:23:32.000000000 -0600 @@ -46,27 +46,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-15 15:21:52.000000000 -0600 +++ work10/drivers/pnp/core.c 2008-05-15 15:23:32.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-15 15:23:24.000000000 -0600 +++ work10/drivers/pnp/interface.c 2008-05-15 15:23:32.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-15 15:23:24.000000000 -0600 +++ work10/drivers/pnp/isapnp/core.c 2008-05-15 15:23:32.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-15 15:23:24.000000000 -0600 +++ work10/drivers/pnp/manager.c 2008-05-15 15:23:32.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-15 15:22:36.000000000 -0600 +++ work10/drivers/pnp/quirks.c 2008-05-15 15:23:32.000000000 -0600 @@ -248,8 +248,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-15 15:23:27.000000000 -0600 +++ work10/drivers/pnp/resource.c 2008-05-15 15:23:32.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-15 15:23:29.000000000 -0600 +++ work10/drivers/pnp/support.c 2008-05-15 15:23:32.000000000 -0600 @@ -16,6 +16,10 @@ */ int pnp_is_active(struct pnp_dev *dev) { + /* + * I don't think this is very reliable because pnp_disable_dev() + * only clears out auto-assigned resources. + */ if (!pnp_port_start(dev, 0) && pnp_port_len(dev, 0) <= 1 && !pnp_mem_start(dev, 0) && pnp_mem_len(dev, 0) <= 1 && pnp_irq(dev, 0) == -1 && pnp_dma(dev, 0) == -1) @@ -70,54 +74,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-15 15:21:52.000000000 -0600 +++ work10/drivers/pnp/system.c 2008-05-15 15:23:32.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-15 15:23:31.000000000 -0600 +++ work10/include/linux/pnp.h 2008-05-15 15:23:32.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; } @@ -64,7 +63,7 @@ static inline unsigned long pnp_port_fla if (pnp_resource_valid(res)) return res->flags; - return IORESOURCE_IO | IORESOURCE_AUTO | IORESOURCE_UNSET; + return IORESOURCE_IO | IORESOURCE_AUTO; } static inline int pnp_port_valid(struct pnp_dev *dev, unsigned int bar) @@ -109,7 +108,7 @@ static inline unsigned long pnp_mem_flag if (pnp_resource_valid(res)) return res->flags; - return IORESOURCE_MEM | IORESOURCE_AUTO | IORESOURCE_UNSET; + return IORESOURCE_MEM | IORESOURCE_AUTO; } static inline int pnp_mem_valid(struct pnp_dev *dev, unsigned int bar) @@ -143,7 +142,7 @@ static inline unsigned long pnp_irq_flag if (pnp_resource_valid(res)) return res->flags; - return IORESOURCE_IRQ | IORESOURCE_AUTO | IORESOURCE_UNSET; + return IORESOURCE_IRQ | IORESOURCE_AUTO; } static inline int pnp_irq_valid(struct pnp_dev *dev, unsigned int bar) @@ -167,7 +166,7 @@ static inline unsigned long pnp_dma_flag if (pnp_resource_valid(res)) return res->flags; - return IORESOURCE_DMA | IORESOURCE_AUTO | IORESOURCE_UNSET; + return IORESOURCE_DMA | IORESOURCE_AUTO; } static inline int pnp_dma_valid(struct pnp_dev *dev, unsigned int bar) @@ -296,7 +295,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] 18+ messages in thread
* [patch 2/4] PNP: replace pnp_resource_table with dynamically allocated resources @ 2008-05-15 22:07 ` Bjorn Helgaas 0 siblings, 0 replies; 18+ messages in thread From: Bjorn Helgaas @ 2008-05-15 22:07 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, Takashi Iwai [-- Attachment #1: pnp-convert-to-lists --] [-- Type: text/plain, Size: 31715 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. 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 | 12 -- drivers/pnp/manager.c | 218 +++++++++++----------------------------------- drivers/pnp/quirks.c | 3 drivers/pnp/resource.c | 86 +++--------------- drivers/pnp/support.c | 72 +++++---------- drivers/pnp/system.c | 8 - include/linux/pnp.h | 13 +- 10 files changed, 158 insertions(+), 357 deletions(-) Index: work10/drivers/pnp/base.h =================================================================== --- work10.orig/drivers/pnp/base.h 2008-05-15 15:23:29.000000000 -0600 +++ work10/drivers/pnp/base.h 2008-05-15 15:23:32.000000000 -0600 @@ -46,27 +46,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-15 15:21:52.000000000 -0600 +++ work10/drivers/pnp/core.c 2008-05-15 15:23:32.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-15 15:23:24.000000000 -0600 +++ work10/drivers/pnp/interface.c 2008-05-15 15:23:32.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-15 15:23:24.000000000 -0600 +++ work10/drivers/pnp/isapnp/core.c 2008-05-15 15:23:32.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-15 15:23:24.000000000 -0600 +++ work10/drivers/pnp/manager.c 2008-05-15 15:23:32.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-15 15:22:36.000000000 -0600 +++ work10/drivers/pnp/quirks.c 2008-05-15 15:23:32.000000000 -0600 @@ -248,8 +248,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-15 15:23:27.000000000 -0600 +++ work10/drivers/pnp/resource.c 2008-05-15 15:23:32.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-15 15:23:29.000000000 -0600 +++ work10/drivers/pnp/support.c 2008-05-15 15:23:32.000000000 -0600 @@ -16,6 +16,10 @@ */ int pnp_is_active(struct pnp_dev *dev) { + /* + * I don't think this is very reliable because pnp_disable_dev() + * only clears out auto-assigned resources. + */ if (!pnp_port_start(dev, 0) && pnp_port_len(dev, 0) <= 1 && !pnp_mem_start(dev, 0) && pnp_mem_len(dev, 0) <= 1 && pnp_irq(dev, 0) == -1 && pnp_dma(dev, 0) == -1) @@ -70,54 +74,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-15 15:21:52.000000000 -0600 +++ work10/drivers/pnp/system.c 2008-05-15 15:23:32.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-15 15:23:31.000000000 -0600 +++ work10/include/linux/pnp.h 2008-05-15 15:23:32.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; } @@ -64,7 +63,7 @@ static inline unsigned long pnp_port_fla if (pnp_resource_valid(res)) return res->flags; - return IORESOURCE_IO | IORESOURCE_AUTO | IORESOURCE_UNSET; + return IORESOURCE_IO | IORESOURCE_AUTO; } static inline int pnp_port_valid(struct pnp_dev *dev, unsigned int bar) @@ -109,7 +108,7 @@ static inline unsigned long pnp_mem_flag if (pnp_resource_valid(res)) return res->flags; - return IORESOURCE_MEM | IORESOURCE_AUTO | IORESOURCE_UNSET; + return IORESOURCE_MEM | IORESOURCE_AUTO; } static inline int pnp_mem_valid(struct pnp_dev *dev, unsigned int bar) @@ -143,7 +142,7 @@ static inline unsigned long pnp_irq_flag if (pnp_resource_valid(res)) return res->flags; - return IORESOURCE_IRQ | IORESOURCE_AUTO | IORESOURCE_UNSET; + return IORESOURCE_IRQ | IORESOURCE_AUTO; } static inline int pnp_irq_valid(struct pnp_dev *dev, unsigned int bar) @@ -167,7 +166,7 @@ static inline unsigned long pnp_dma_flag if (pnp_resource_valid(res)) return res->flags; - return IORESOURCE_DMA | IORESOURCE_AUTO | IORESOURCE_UNSET; + return IORESOURCE_DMA | IORESOURCE_AUTO; } static inline int pnp_dma_valid(struct pnp_dev *dev, unsigned int bar) @@ -296,7 +295,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] 18+ messages in thread
* Re: [patch 2/4] PNP: replace pnp_resource_table with dynamically allocated resources 2008-05-15 22:07 ` Bjorn Helgaas (?) @ 2008-05-19 23:14 ` Rene Herman 2008-05-19 23:41 ` Bjorn Helgaas -1 siblings, 1 reply; 18+ messages in thread From: Rene Herman @ 2008-05-19 23:14 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, Takashi Iwai On 16-05-08 00:07, Bjorn Helgaas 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. [ ... ] > 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); > } > } Do correct me if I'm wrong but I don't believe this will do. The index for the resources is preserved simply due to the list position after the DISABLED thing but here the list is reshuffled. So say I have an ISAPnP device with 2 port resources the second of which I force to manual setting through sysfs (ie, AUTO is cleared). This API would then delete the first port resource after which the second port resource is the first entry in the list which would make for example isapnp_set_resource() program it into hardware index 1, no? This function would appear to be unimplementable now? It's called from pnp_assign_resources() and pnp_disable_dev(). (stopped review there, since it seems this needs to either change or you need to slap some sense into me first) Rene. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [patch 2/4] PNP: replace pnp_resource_table with dynamically allocated resources 2008-05-19 23:14 ` Rene Herman @ 2008-05-19 23:41 ` Bjorn Helgaas 2008-05-22 21:18 ` Rene Herman 0 siblings, 1 reply; 18+ messages in thread From: Bjorn Helgaas @ 2008-05-19 23:41 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, Takashi Iwai On Monday 19 May 2008 05:14:07 pm Rene Herman wrote: > On 16-05-08 00:07, Bjorn Helgaas 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. > > [ ... ] > > > 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); > > } > > } > > > Do correct me if I'm wrong but I don't believe this will do. The index > for the resources is preserved simply due to the list position after the > DISABLED thing but here the list is reshuffled. > > So say I have an ISAPnP device with 2 port resources the second of which > I force to manual setting through sysfs (ie, AUTO is cleared). This API > would then delete the first port resource after which the second port > resource is the first entry in the list which would make for example > isapnp_set_resource() program it into hardware index 1, no? My reasoning was that all AUTO entries should be at the end of the list, so deleting them should not change the order of other entries. I don't think there's a way to create a non-AUTO entry that follows an AUTO one. We add non-AUTO resources in: - isapnp_get_resources() - pnpbios_parse_allocated_resource_data() - pnpacpi_parse_allocated_resource() - pnp_set_current_resources() (sysfs interface) and in all those cases, we use pnp_init_resources() first, which clears out all existing resources. We add AUTO resources in pnp_assign_{port,mem,etc}(), and those bail out early if there's already a resource at the specified index. All this is certainly not obvious from the code and it took me a long time to convince myself of it, so I could easily be mistaken. Bjorn ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [patch 2/4] PNP: replace pnp_resource_table with dynamically allocated resources 2008-05-19 23:41 ` Bjorn Helgaas @ 2008-05-22 21:18 ` Rene Herman 0 siblings, 0 replies; 18+ messages in thread From: Rene Herman @ 2008-05-22 21:18 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, Takashi Iwai On 20-05-08 01:41, Bjorn Helgaas wrote: > On Monday 19 May 2008 05:14:07 pm Rene Herman wrote: >>> static void pnp_clean_resource_table(struct pnp_dev *dev) [ ... ] >> Do correct me if I'm wrong but I don't believe this will do. The index >> for the resources is preserved simply due to the list position after the >> DISABLED thing but here the list is reshuffled. >> >> So say I have an ISAPnP device with 2 port resources the second of which >> I force to manual setting through sysfs (ie, AUTO is cleared). This API >> would then delete the first port resource after which the second port >> resource is the first entry in the list which would make for example >> isapnp_set_resource() program it into hardware index 1, no? > > My reasoning was that all AUTO entries should be at the end of the > list, so deleting them should not change the order of other entries. Yes, so it seems. You even said that in the changelog. Missed it, sorry. Acked-by: Rene Herman <rene.herman@gmail.com> I have had your v2 series running on two machines (using PnPBIOS and ISAPnP) for a few days now and I haven't seen anything wrong. Looking good therefore. Good stuff! Rene. ^ permalink raw reply [flat|nested] 18+ messages in thread
* [patch 3/4] PNP: remove ratelimit on add resource failures 2008-05-15 22:07 ` Bjorn Helgaas @ 2008-05-15 22:07 ` Bjorn Helgaas -1 siblings, 0 replies; 18+ messages in thread From: Bjorn Helgaas @ 2008-05-15 22:07 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, Takashi Iwai [-- 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] 18+ messages in thread
* [patch 3/4] PNP: remove ratelimit on add resource failures @ 2008-05-15 22:07 ` Bjorn Helgaas 0 siblings, 0 replies; 18+ messages in thread From: Bjorn Helgaas @ 2008-05-15 22:07 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, Takashi Iwai [-- 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] 18+ messages in thread
* Re: [patch 3/4] PNP: remove ratelimit on add resource failures 2008-05-15 22:07 ` Bjorn Helgaas (?) @ 2008-05-19 23:15 ` Rene Herman -1 siblings, 0 replies; 18+ messages in thread From: Rene Herman @ 2008-05-19 23:15 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, Takashi Iwai On 16-05-08 00:07, Bjorn Helgaas wrote: > 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> Acked-by: Rene Herman <rene.herman@gmail.com> Rene ^ permalink raw reply [flat|nested] 18+ messages in thread
* [patch 4/4] PNP: dont sort by type in /sys/.../resources 2008-05-15 22:07 ` Bjorn Helgaas @ 2008-05-15 22:07 ` Bjorn Helgaas -1 siblings, 0 replies; 18+ messages in thread From: Bjorn Helgaas @ 2008-05-15 22:07 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, Takashi Iwai [-- 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] 18+ messages in thread
* [patch 4/4] PNP: dont sort by type in /sys/.../resources @ 2008-05-15 22:07 ` Bjorn Helgaas 0 siblings, 0 replies; 18+ messages in thread From: Bjorn Helgaas @ 2008-05-15 22:07 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, Takashi Iwai [-- 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] 18+ messages in thread
* Re: [patch 4/4] PNP: dont sort by type in /sys/.../resources 2008-05-15 22:07 ` Bjorn Helgaas (?) @ 2008-05-19 22:42 ` Adam M Belay 2008-05-19 23:41 ` Bjorn Helgaas -1 siblings, 1 reply; 18+ messages in thread From: Adam M Belay @ 2008-05-19 22:42 UTC (permalink / raw) To: Bjorn Helgaas Cc: Len Brown, linux-acpi, linux-kernel, Adam Belay, Li Shaohua, Matthieu Castet, Thomas Renninger, Rene Herman, Jaroslav Kysela, Andrew Morton, Takashi Iwai Does this fix a bug or is it just a more convenient representation? Perhaps, if we're going to change the interface we could switch to PCI style sysfs resources. Thanks, Adam Quoting Bjorn Helgaas <bjorn.helgaas@hp.com>: > 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] 18+ messages in thread
* Re: [patch 4/4] PNP: dont sort by type in /sys/.../resources 2008-05-19 22:42 ` Adam M Belay @ 2008-05-19 23:41 ` Bjorn Helgaas 0 siblings, 0 replies; 18+ messages in thread From: Bjorn Helgaas @ 2008-05-19 23:41 UTC (permalink / raw) To: Adam M Belay Cc: Len Brown, linux-acpi, linux-kernel, Adam Belay, Li Shaohua, Matthieu Castet, Thomas Renninger, Rene Herman, Jaroslav Kysela, Andrew Morton, Takashi Iwai On Monday 19 May 2008 04:42:28 pm Adam M Belay wrote: > Does this fix a bug or is it just a more convenient representation? Mostly just simpler code. It is important to maintain the order of resources within each type, of course, so we can encode them in the correct order when we set device configuration. But we could still sort them by type if you think that's important. There is some risk with changing the order, but there's also some benefit in reducing the fiddling we do with the data from the firmware. It's been conceptually liberating for me to see the data correlate more exactly with _CRS and friends. > Perhaps, if > we're going to change the interface we could switch to PCI style sysfs > resources. You mean like resource_show(), which looks like this? $ cat /sys/devices/pci0000:00/0000:00:00.0/resource 0x0000000000000000 0x0000000000000000 0x0000000000000000 0x0000000000000000 0x0000000000000000 0x0000000000000000 0x0000000000000000 0x0000000000000000 0x0000000000000000 0x0000000000000000 0x0000000000000000 0x0000000000000000 0x0000000000000000 0x0000000000000000 0x0000000000000000 0x0000000000000000 0x0000000000000000 0x0000000000000000 0x0000000000000000 0x0000000000000000 0x0000000000000000 I'm all in favor of converging with the PCI style of doing things, but I'm not sure I understand your idea in this case. We still do quite a bit of manual poking in the PNP resource file, and the labels make it nicer for human consumption. Bjorn > Quoting Bjorn Helgaas <bjorn.helgaas@hp.com>: > > > 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] 18+ messages in thread
* Re: [patch 4/4] PNP: dont sort by type in /sys/.../resources 2008-05-15 22:07 ` Bjorn Helgaas (?) (?) @ 2008-05-19 23:22 ` Rene Herman -1 siblings, 0 replies; 18+ messages in thread From: Rene Herman @ 2008-05-19 23:22 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, Takashi Iwai On 16-05-08 00:07, Bjorn Helgaas wrote: > 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> Acked-by: Rene Herman <rene.herman@gmail.com> Rene. ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2008-05-22 21:18 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-05-15 22:07 [patch 0/4] PNP: convert resource table to dynamic list, v2 Bjorn Helgaas
2008-05-15 22:07 ` Bjorn Helgaas
2008-05-15 22:07 ` [patch 1/4] PNP: make pnp_{port,mem,etc}_start(), et al work for invalid resources Bjorn Helgaas
2008-05-15 22:07 ` Bjorn Helgaas
2008-05-19 22:23 ` Rene Herman
2008-05-15 22:07 ` [patch 2/4] PNP: replace pnp_resource_table with dynamically allocated resources Bjorn Helgaas
2008-05-15 22:07 ` Bjorn Helgaas
2008-05-19 23:14 ` Rene Herman
2008-05-19 23:41 ` Bjorn Helgaas
2008-05-22 21:18 ` Rene Herman
2008-05-15 22:07 ` [patch 3/4] PNP: remove ratelimit on add resource failures Bjorn Helgaas
2008-05-15 22:07 ` Bjorn Helgaas
2008-05-19 23:15 ` Rene Herman
2008-05-15 22:07 ` [patch 4/4] PNP: dont sort by type in /sys/.../resources Bjorn Helgaas
2008-05-15 22:07 ` Bjorn Helgaas
2008-05-19 22:42 ` Adam M Belay
2008-05-19 23:41 ` Bjorn Helgaas
2008-05-19 23:22 ` Rene Herman
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.