Messages in this thread | | | From | Herbert Xu <> | Subject | Re: i810_audio MMIO patch | 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/
| |