Alan Cox wrote:
>
> > - 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.
If you compile the kernel with sound support then $CONFIG_VIDEO_BT848 and
$CONFIG_SOUND
must have the same value. It's not what I wrote above but there is some
dependence
(trough the msp3400.c file).
>
> > /* 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
>
Which driver are you looking at? They are declared static, not exported,
not referenced in any non __init function and monolithic compilation didn't
fail...
> > + 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
I know. The comment above this line (in one occasion) is:
/* clear interrupt status */
btwrite(0xfffffUL, BT848_INT_STAT);
But then they only reset the lower 16bit. I'm not looking at documentation
so I can be wrong.
> > + spin_unlock_irqrestore(&btv->s_lock, flags);
> > interruptible_sleep_on(&btv->vbiq);
> > - sti();
>
> Again you need to fix this with add_wait_queue
>
I always though this piece of code was right... Can you explain
the race? Is it that the wake_up() should be protected by the lock and
this way it may come before we wait on it?
> > + 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.
Ok. Didn't know about this ones.
Rui Sousa
-
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:22 EST