Re: [PATCH] bttv driver II

From: Alan Cox (alan@lxorguk.ukuu.org.uk)
Date: Mon May 22 2000 - 13:19:45 EST


> > 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).

The msp3400 driver is not required

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

Ok my fault. The default kernel version doesn thave the BTTV_READEE/WRITEE
ioctls that are in my tree.

> btwrite(0xfffffUL, BT848_INT_STAT);
>
> But then they only reset the lower 16bit. I'm not looking at documentation
> so I can be wrong.

Lower 20bits which is I believe right

> > > + 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?

We can go

        spin_unlock
                                        wake_up
        interruptible_sleep_on
        
the existing code is wrong on 2.3.x as well btw. Take a look how the
tty code handles it with add_wait_queue/remove_wait_queue - that kills all the
races.

> > ned __devinit.
>
> Ok. Didn't know about this ones.

No problem 8).

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:22 EST