Re: i810_audio MMIO patch

[Herbert Xu]

John Linville <linville@xxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxx>
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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/