Re: i810_audio MMIO patch

From: Herbert Xu (herbert_at_gondor.apana.org.au)
Date: 06/30/04

  • Next message: Meelis Roos: "irq 11: nobody cared? (usb?)" 
    To: linville@redhat.com (John Linville)
Date:	Wed, 30 Jun 2004 20:22:14 +1000

    John Linville <linville@redhat.com> wrote:
    >
    > @@ -452,12 +452,34 @@ struct i810_card {
    > /* extract register offset from codec struct */
    > #define IO_REG_OFF(codec) (((struct i810_card *) codec->private_data)->ac97_id_map[codec->id])
    >
    > -#define GET_CIV(port) MODULOP2(inb((port) + OFF_CIV), SG_LEN)
    > -#define GET_LVI(port) MODULOP2(inb((port) + OFF_LVI), SG_LEN)
    > +#define I810_IOREAD(size, type, card, off) \
    > +({ \
    > + type val; \
    > + if (card->use_mmio) val=read##size(card->iobase_mmio+off); \
    > + else val=in##size(card->iobase+off); \
    > + val; \
    > +})

    Please indent I810_IOWRITE and this like normal code.
     
    > @@ -716,9 +738,9 @@ static inline unsigned i810_get_dma_addr

    How about making card a local variable where it's used over and over again
    as in this function?

    > @@ -2798,7 +2819,8 @@ static int i810_ac97_power_up_bus(struct
    > * before we start doing DMA stuff
    > */
    > /* see i810_ac97_init for the next 7 lines (jsaw) */
    > - inw(card->ac97base);
    > + if (card->use_mmio) readw(card->ac97base_mmio);
    > + else inw(card->ac97base);

    Please indent these ones too.

    > @@ -3141,6 +3162,11 @@ static int __devinit i810_probe(struct p
    > }
    > }
    >
    > + if (!(card->use_mmio) && !(card->iobase)) {
    > + printk(KERN_ERR "i810_audio: No I/O resources available.\n");
    > + goto out_mem;
    > + }
    > +

    To be safe we should check card->ac97base as well.

    Otherwise the patch looks good.

    Cheers, 

    -- 
Visit Openswan at http://www.openswan.org/
Email:  Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
  • Next message: Meelis Roos: "irq 11: nobody cared? (usb?)"

    Relevant Pages