Re: [PATCH] bttv driver II

From: Alan Cox (alan@lxorguk.ukuu.org.uk)
Date: Mon May 22 2000 - 06:55:36 EST


> - dep_tristate ' BT848 Video For Linux' CONFIG_VIDEO_BT848 $CONFIG_VIDEO_DEV $CONFIG_PCI $CONFIG_I2C_ALGOBIT
> + dep_tristate ' BT848 Video For Linux' CONFIG_VIDEO_BT848 $CONFIG_VIDEO_DEV $CONFIG_PCI $CONFIG_I2C_ALGOBIT $CONFIG_SOUND

BT848 is not dependant on the sound layer.

> /* read EEPROM */
> -static void readee(struct bttv *btv, unsigned char *eedata, int addr)
> +static void __init readee(struct bttv *btv, unsigned char *eedata, int addr)

This function can be called from ioctls so cannot be __init

> -static void
> -dump_eeprom(struct bttv *btv,int addr)
> +static void __init dump_eeprom(struct bttv *btv,int addr)

Again this is used outside of init code

> + spin_unlock_irqrestore(&btv->s_lock, flags);
> interruptible_sleep_on(&btv->vbiq);
> - sti();

Nope. You added a race. The existing code is not right either. You need
to change this to use add_wait_queue and remove the race.

> - btwrite(0xfffffUL, BT848_INT_STAT);
> + btwrite(~0x0UL, BT848_INT_STAT);

Nope. You changed it - notice it is 5x 0xF

> + spin_unlock_irqrestore(&btv->s_lock, flags);
> interruptible_sleep_on(&btv->vbiq);
> - sti();

Again you need to fix this with add_wait_queue

> + return pci_module_init(&bttv_pci_driver);
> }

And there are (rare alas) cardbus BT879 based capture cards so I suspect
most of the other __init changes are wrong for HOTPLUG=y. I think you
ned __devinit.

Oh dear, that looks like I trashed the patch. Most of it is right, the other
bugs want fixing anyway so dont take it as a reject!

Alan

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/



This archive was generated by hypermail 2b29 : Tue May 23 2000 - 21:00:21 EST