Re: [PATCH] Allocate pnp resources dynamically via krealloc -working version - Addon patch 1
From: Thomas Renninger
Date: Mon Jan 28 2008 - 11:05:00 EST
On Mon, 2008-01-28 at 16:00 +0100, Rene Herman wrote:
> On 28-01-08 15:21, Thomas Renninger wrote:
>
> > I think I know what is going on.
> > While pnpbios and pnpacpi theoretically do not have limits, isapnp has
> > spec restrictions (AFAIK, I have not read this up, but taken over from
> > previous implementation...).
> > Therefore in isapnp I wanted to stay with:
> > #define PNP_MAX_PORT 8
> > #define PNP_MAX_MEM 4
> > #define PNP_MAX_IRQ 2
> > #define PNP_MAX_DMA 2
> > but I have forgotten to malloc one portion for each at init time, or
> > even better one portion as soon as one is needed.
>
> Yup.
>
> > As said, isapnp is more or less untested, thanks a lot for trying out.
> > I will send an updated version soon.
>
> I"m not sure of the flow of things by the way but if it makes better/nicer
> code to just pretend that ISAPnP is also unlimited then I'd say to simply do
> so. ISAPnP is getting obsolete anyway, not anything to optimise for...
Can you try these two "on top" patches pls.
This one should fix the real cause (the "pnp: Port resource 0 not
allocated, cannot assign value" messages). The next should avoid the
NULL pointer in error case:
Also allocate memory for dependent/independent resource lists
Signed-off-by: Thomas Renninger <trenn@xxxxxxx>
---
drivers/pnp/manager.c | 138 ++++++++++++++++++++++----------------------------
1 file changed, 61 insertions(+), 77 deletions(-)
Index: linux-2.6.23/drivers/pnp/manager.c
===================================================================
--- linux-2.6.23.orig/drivers/pnp/manager.c
+++ linux-2.6.23/drivers/pnp/manager.c
@@ -341,99 +341,89 @@ int pnp_assign_resource(struct pnp_resou
static int pnp_assign_port(struct pnp_dev *dev, struct pnp_port *rule, int idx)
{
- resource_size_t *start, *end;
- unsigned long *flags;
-
- if (!pnp_port_ok(dev, idx)) {
- pnp_print_assign_err("Port", idx);
- /* pretend we were successful so at least the manager won't try again */
- return 1;
- }
+ struct resource res;
/* check if this resource has been manually set, if so skip */
if (!(dev->res.port_resource[idx].flags & IORESOURCE_AUTO))
return 1;
- start = &dev->res.port_resource[idx].start;
- end = &dev->res.port_resource[idx].end;
- flags = &dev->res.port_resource[idx].flags;
+ res.start = dev->res.port_resource[idx].start;
+ res.end = dev->res.port_resource[idx].end;
+ res.flags = dev->res.port_resource[idx].flags;
/* set the initial values */
- *flags |= rule->flags | IORESOURCE_IO;
- *flags &= ~IORESOURCE_UNSET;
+ res.flags |= rule->flags | IORESOURCE_IO;
+ res.flags &= ~IORESOURCE_UNSET;
if (!rule->size) {
- *flags |= IORESOURCE_DISABLED;
+ res.flags |= IORESOURCE_DISABLED;
+ pnp_assign_resource(&dev->res, &res);
return 1; /* skip disabled resource requests */
}
- *start = rule->min;
- *end = *start + rule->size - 1;
+ res.start = rule->min;
+ res.end = res.start + rule->size - 1;
/* run through until pnp_check_port is happy */
while (!pnp_check_port(dev, idx)) {
- *start += rule->align;
- *end = *start + rule->size - 1;
- if (*start > rule->max || !rule->align)
- return 0;
+ res.start += rule->align;
+ res.end = res.start + rule->size - 1;
+ if (res.start > rule->max || !rule->align)
+ return pnp_assign_resource(&dev->res, &res);
}
+ pnp_assign_resource(&dev->res, &res);
return 1;
}
static int pnp_assign_mem(struct pnp_dev *dev, struct pnp_mem *rule, int idx)
{
- resource_size_t *start, *end;
- unsigned long *flags;
-
- if (!pnp_mem_ok(dev, idx)) {
- pnp_print_assign_err("System Memory", idx);
- return 1;
- }
+ struct resource res;
/* check if this resource has been manually set, if so skip */
if (!(dev->res.mem_resource[idx].flags & IORESOURCE_AUTO))
return 1;
- start = &dev->res.mem_resource[idx].start;
- end = &dev->res.mem_resource[idx].end;
- flags = &dev->res.mem_resource[idx].flags;
+ res.start = dev->res.mem_resource[idx].start;
+ res.end = dev->res.mem_resource[idx].end;
+ res.flags = dev->res.mem_resource[idx].flags;
/* set the initial values */
- *flags |= rule->flags | IORESOURCE_MEM;
- *flags &= ~IORESOURCE_UNSET;
+ res.flags |= rule->flags | IORESOURCE_MEM;
+ res.flags &= ~IORESOURCE_UNSET;
/* convert pnp flags to standard Linux flags */
if (!(rule->flags & IORESOURCE_MEM_WRITEABLE))
- *flags |= IORESOURCE_READONLY;
+ res.flags |= IORESOURCE_READONLY;
if (rule->flags & IORESOURCE_MEM_CACHEABLE)
- *flags |= IORESOURCE_CACHEABLE;
+ res.flags |= IORESOURCE_CACHEABLE;
if (rule->flags & IORESOURCE_MEM_RANGELENGTH)
- *flags |= IORESOURCE_RANGELENGTH;
+ res.flags |= IORESOURCE_RANGELENGTH;
if (rule->flags & IORESOURCE_MEM_SHADOWABLE)
- *flags |= IORESOURCE_SHADOWABLE;
+ res.flags |= IORESOURCE_SHADOWABLE;
if (!rule->size) {
- *flags |= IORESOURCE_DISABLED;
+ res.flags |= IORESOURCE_DISABLED;
+ pnp_assign_resource(&dev->res, &res);
return 1; /* skip disabled resource requests */
}
- *start = rule->min;
- *end = *start + rule->size - 1;
+ res.start = rule->min;
+ res.end = res.start + rule->size - 1;
/* run through until pnp_check_mem is happy */
while (!pnp_check_mem(dev, idx)) {
- *start += rule->align;
- *end = *start + rule->size - 1;
- if (*start > rule->max || !rule->align)
- return 0;
+ res.start += rule->align;
+ res.end = res.start + rule->size - 1;
+ if (res.start > rule->max || !rule->align)
+ return pnp_assign_resource(&dev->res, &res);
}
+ pnp_assign_resource(&dev->res, &res);
return 1;
}
static int pnp_assign_irq(struct pnp_dev *dev, struct pnp_irq *rule, int idx)
{
- resource_size_t *start, *end;
- unsigned long *flags;
+ struct resource res;
int i;
/* IRQ priority: this table is good for i386 */
@@ -441,48 +431,44 @@ static int pnp_assign_irq(struct pnp_dev
5, 10, 11, 12, 9, 14, 15, 7, 3, 4, 13, 0, 1, 6, 8, 2
};
- if (!pnp_irq_ok(dev, idx)) {
- pnp_print_assign_err("Irq", idx);
- return 1;
- }
-
/* check if this resource has been manually set, if so skip */
if (!(dev->res.irq_resource[idx].flags & IORESOURCE_AUTO))
return 1;
- start = &dev->res.irq_resource[idx].start;
- end = &dev->res.irq_resource[idx].end;
- flags = &dev->res.irq_resource[idx].flags;
+ res.start = dev->res.irq_resource[idx].start;
+ res.end = dev->res.irq_resource[idx].end;
+ res.flags = dev->res.irq_resource[idx].flags;
/* set the initial values */
- *flags |= rule->flags | IORESOURCE_IRQ;
- *flags &= ~IORESOURCE_UNSET;
+ res.flags |= rule->flags | IORESOURCE_IRQ;
+ res.flags &= ~IORESOURCE_UNSET;
if (bitmap_empty(rule->map, PNP_IRQ_NR)) {
- *flags |= IORESOURCE_DISABLED;
+ res.flags |= IORESOURCE_DISABLED;
+ pnp_assign_resource(&dev->res, &res);
return 1; /* skip disabled resource requests */
}
/* TBD: need check for >16 IRQ */
- *start = find_next_bit(rule->map, PNP_IRQ_NR, 16);
- if (*start < PNP_IRQ_NR) {
- *end = *start;
+ res.start = find_next_bit(rule->map, PNP_IRQ_NR, 16);
+ if (res.start < PNP_IRQ_NR) {
+ res.end = res.start;
+ pnp_assign_resource(&dev->res, &res);
return 1;
}
for (i = 0; i < 16; i++) {
if (test_bit(xtab[i], rule->map)) {
- *start = *end = xtab[i];
+ res.start = res.end = xtab[i];
if (pnp_check_irq(dev, idx))
return 1;
}
}
- return 0;
+ return pnp_assign_resource(&dev->res, &res);
}
static void pnp_assign_dma(struct pnp_dev *dev, struct pnp_dma *rule, int idx)
{
- resource_size_t *start, *end;
- unsigned long *flags;
+ struct resource res;
int i;
/* DMA priority: this table is good for i386 */
@@ -490,34 +476,32 @@ static void pnp_assign_dma(struct pnp_de
1, 3, 5, 6, 7, 0, 2, 4
};
- if (!pnp_dma_ok(dev, idx)) {
- pnp_print_assign_err("DMA", idx);
- return;
- }
-
/* check if this resource has been manually set, if so skip */
if (!(dev->res.dma_resource[idx].flags & IORESOURCE_AUTO))
return;
- start = &dev->res.dma_resource[idx].start;
- end = &dev->res.dma_resource[idx].end;
- flags = &dev->res.dma_resource[idx].flags;
+ res.start = dev->res.dma_resource[idx].start;
+ res.end = dev->res.dma_resource[idx].end;
+ res.flags = dev->res.dma_resource[idx].flags;
/* set the initial values */
- *flags |= rule->flags | IORESOURCE_DMA;
- *flags &= ~IORESOURCE_UNSET;
+ res.flags |= rule->flags | IORESOURCE_DMA;
+ res.flags &= ~IORESOURCE_UNSET;
for (i = 0; i < 8; i++) {
if (rule->map & (1 << xtab[i])) {
- *start = *end = xtab[i];
- if (pnp_check_dma(dev, idx))
+ res.start = res.end = xtab[i];
+ if (pnp_check_dma(dev, idx)){
+ pnp_assign_resource(&dev->res, &res);
return;
+ }
}
}
#ifdef MAX_DMA_CHANNELS
- *start = *end = MAX_DMA_CHANNELS;
+ res.start = res.end = MAX_DMA_CHANNELS;
#endif
- *flags |= IORESOURCE_UNSET | IORESOURCE_DISABLED;
+ res.flags |= IORESOURCE_UNSET | IORESOURCE_DISABLED;
+ pnp_assign_resource(&dev->res, &res);
}
/**
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/