Messages in this thread Patch in this message | | | Date | Fri, 16 Jan 2004 00:35:45 +0100 | From | Rene Herman <> | Subject | Re: [PATCH] Re: ALSA in 2.6 failing to find the OPL chip of the sb cards |
| |
Adam Belay wrote:
> Here's the patch. Any testing would be appreciated.
Tested with two more ISA-Pnp soundcards, ES1868 and OPTi 82c933, and their ALSA drivers, snd-es18xx and snd-opti93x, and both work the same as they do without the patch (not quite right that is, but nothing to do with PnP). Also tested with ISA-PnP IDE (on the ES1868), ISA-PnP NE2000 (RTL8019), and ISA-PnP modem. All fine.
Minor point about the patch itself though. In pnp.h, you do:
> -#define pnp_port_valid(dev,bar) (pnp_port_flags((dev),(bar)) & > IORESOURCE_IO) > +#define pnp_port_valid(dev,bar) \ + > ((pnp_port_flags((dev),(bar)) & IORESOURCE_IO) && \ + > !(pnp_port_flags((dev),(bar)) & IORESOURCE_UNSET))
and the same for mem,irq,dma. It seems you could roll these two tests into one with:
#define pnp_port_valid(dev,bar) \ ((pnp_port_flags((dev),(bar)) & (IORESOURCE_IO | IORESOURCE_UNSET)) == IORESOURCE_IO)
Basically just an optimisation I guess (just checked and gcc doesn't do this itself) but this also stops the macro arguments from being accessed more than once.
One more point, in isapnp/core.c:isapnp_set_resources()
> - for (tmp = 0; tmp < PNP_MAX_PORT && res->port_resource[tmp].flags & IORESOURCE_IO; tmp++) > + for (tmp = 0; tmp < PNP_MAX_PORT && !(res->port_resource[tmp].flags & IORESOURCE_UNSET); tmp++)
and again same for mem,irq,dma. That is, it goes from only checking IORESOURCE_<TYPE> to only checking IORESOURCE_UNSET. Also checking for the type does sound like a valid sanity check, so would it be better to also check both flags here? Ie:
for (tmp = 0; tmp < PNP_MAX_PORT && (res->port_resource[tmp].flags & (IORESOURCE_IO | IORESOURCE_UNSET)) == IORESOURCE_IO; tmp++)
Incremental patch attached, in case you agree. Compiled, booted and tested.
Rene.
diff -urN linux-2.6.1-pnp/drivers/pnp/isapnp/core.c linux-2.6.1-pnp-incr/drivers/pnp/isapnp/core.c --- linux-2.6.1-pnp/drivers/pnp/isapnp/core.c 2004-01-16 00:02:40.000000000 +0100 +++ linux-2.6.1-pnp-incr/drivers/pnp/isapnp/core.c 2004-01-15 23:58:23.000000000 +0100 @@ -1039,17 +1039,17 @@ isapnp_cfg_begin(dev->card->number, dev->number); dev->active = 1; - for (tmp = 0; tmp < PNP_MAX_PORT && !(res->port_resource[tmp].flags & IORESOURCE_UNSET); tmp++) + for (tmp = 0; tmp < PNP_MAX_PORT && (res->port_resource[tmp].flags & (IORESOURCE_IO | IORESOURCE_UNSET)) == IORESOURCE_IO; tmp++) isapnp_write_word(ISAPNP_CFG_PORT+(tmp<<1), res->port_resource[tmp].start); - for (tmp = 0; tmp < PNP_MAX_IRQ && !(res->irq_resource[tmp].flags & IORESOURCE_UNSET); tmp++) { + for (tmp = 0; tmp < PNP_MAX_IRQ && (res->irq_resource[tmp].flags & (IORESOURCE_IRQ | IORESOURCE_UNSET)) == IORESOURCE_IRQ; tmp++) { int irq = res->irq_resource[tmp].start; if (irq == 2) irq = 9; isapnp_write_byte(ISAPNP_CFG_IRQ+(tmp<<1), irq); } - for (tmp = 0; tmp < PNP_MAX_DMA && !(res->dma_resource[tmp].flags & IORESOURCE_UNSET); tmp++) + for (tmp = 0; tmp < PNP_MAX_DMA && (res->dma_resource[tmp].flags & (IORESOURCE_DMA | IORESOURCE_UNSET)) == IORESOURCE_DMA; tmp++) isapnp_write_byte(ISAPNP_CFG_DMA+tmp, res->dma_resource[tmp].start); - for (tmp = 0; tmp < PNP_MAX_MEM && !(res->mem_resource[tmp].flags & IORESOURCE_UNSET); tmp++) + for (tmp = 0; tmp < PNP_MAX_MEM && (res->mem_resource[tmp].flags & (IORESOURCE_MEM | IORESOURCE_UNSET)) == IORESOURCE_MEM; tmp++) isapnp_write_word(ISAPNP_CFG_MEM+(tmp<<2), (res->mem_resource[tmp].start >> 8) & 0xffff); /* FIXME: We aren't handling 32bit mems properly here */ isapnp_activate(dev->number); diff -urN linux-2.6.1-pnp/include/linux/pnp.h linux-2.6.1-pnp-incr/include/linux/pnp.h --- linux-2.6.1-pnp/include/linux/pnp.h 2004-01-16 00:02:40.000000000 +0100 +++ linux-2.6.1-pnp-incr/include/linux/pnp.h 2004-01-15 23:56:31.000000000 +0100 @@ -34,8 +34,8 @@ #define pnp_port_end(dev,bar) ((dev)->res.port_resource[(bar)].end) #define pnp_port_flags(dev,bar) ((dev)->res.port_resource[(bar)].flags) #define pnp_port_valid(dev,bar) \ - ((pnp_port_flags((dev),(bar)) & IORESOURCE_IO) && \ - !(pnp_port_flags((dev),(bar)) & IORESOURCE_UNSET)) + ((pnp_port_flags((dev),(bar)) & (IORESOURCE_IO | IORESOURCE_UNSET)) \ + == IORESOURCE_IO) #define pnp_port_len(dev,bar) \ ((pnp_port_start((dev),(bar)) == 0 && \ pnp_port_end((dev),(bar)) == \ @@ -48,8 +48,8 @@ #define pnp_mem_end(dev,bar) ((dev)->res.mem_resource[(bar)].end) #define pnp_mem_flags(dev,bar) ((dev)->res.mem_resource[(bar)].flags) #define pnp_mem_valid(dev,bar) \ - ((pnp_mem_flags((dev),(bar)) & IORESOURCE_MEM) && \ - !(pnp_mem_flags((dev),(bar)) & IORESOURCE_UNSET)) + ((pnp_mem_flags((dev),(bar)) & (IORESOURCE_MEM | IORESOURCE_UNSET)) \ + == IORESOURCE_MEM) #define pnp_mem_len(dev,bar) \ ((pnp_mem_start((dev),(bar)) == 0 && \ pnp_mem_end((dev),(bar)) == \ @@ -61,14 +61,14 @@ #define pnp_irq(dev,bar) ((dev)->res.irq_resource[(bar)].start) #define pnp_irq_flags(dev,bar) ((dev)->res.irq_resource[(bar)].flags) #define pnp_irq_valid(dev,bar) \ - ((pnp_irq_flags((dev),(bar)) & IORESOURCE_IRQ) && \ - !(pnp_irq_flags((dev),(bar)) & IORESOURCE_UNSET)) + ((pnp_irq_flags((dev),(bar)) & (IORESOURCE_IRQ | IORESOURCE_UNSET)) \ + == IORESOURCE_IRQ) #define pnp_dma(dev,bar) ((dev)->res.dma_resource[(bar)].start) #define pnp_dma_flags(dev,bar) ((dev)->res.dma_resource[(bar)].flags) #define pnp_dma_valid(dev,bar) \ - ((pnp_dma_flags((dev),(bar)) & IORESOURCE_DMA) && \ - !(pnp_dma_flags((dev),(bar)) & IORESOURCE_UNSET)) + ((pnp_dma_flags((dev),(bar)) & (IORESOURCE_DMA | IORESOURCE_UNSET)) \ + == IORESOURCE_DMA) #define PNP_PORT_FLAG_16BITADDR (1<<0) #define PNP_PORT_FLAG_FIXED (1<<1)
| |