[PATCH] v4l: i2c cleanups

From: Gerd Knorr <kraxel@bytesex.org>

This patch brings a few cleanups/fixes for the v4l-related i2c modules:

* fix "badness in interruptible_sleep_on"

* use completions instead of semaphores to sync rmmod + kernel thread
  exit

* drop some some obsolete code.

* minor tweaks for some tv cards.

drivers/media/video/msp3400.c

@@ -87,11 +87,13 @@ struct msp3400c {
 	int dfp_regs[DFP_COUNT];
 
 	/* thread */
-	struct task_struct  *thread;
+	pid_t                tpid;
+	struct completion    texit;
 	wait_queue_head_t    wq;
 
-	struct semaphore    *notify;
-	int                  active,restart,rmmod;
+	int                  active:1;
+	int                  restart:1;
+	int                  rmmod:1;
 
 	int                  watch_stereo;
 	struct timer_list    wake_stereo;
@@ -101,14 +103,12 @@ struct msp3400c {
 #define HAVE_SIMPLE(msp)  ((msp->rev1      & 0xff) >= 'D'-'@')
 #define HAVE_RADIO(msp)   ((msp->rev1      & 0xff) >= 'G'-'@')
 
-#define MSP3400_MAX 4
-static struct i2c_client *msps[MSP3400_MAX];
-
 #define VIDEO_MODE_RADIO 16      /* norm magic for radio mode */
 
 /* ---------------------------------------------------------------------- */
 
-#define dprintk     if (debug) printk
+#define dprintk      if (debug >= 1) printk
+#define d2printk     if (debug >= 2) printk
 
 MODULE_PARM(once,"i");
 MODULE_PARM(debug,"i");
@@ -735,6 +735,22 @@ autodetect_stereo(struct i2c_client *client)
  * in the ioctl while doing the sound carrier & stereo detect
  */
 
+static int msp34xx_sleep(struct msp3400c *msp, int timeout)
+{
+	DECLARE_WAITQUEUE(wait, current);
+
+	add_wait_queue(&msp->wq, &wait);
+	if (!msp->rmmod) {
+		set_current_state(TASK_INTERRUPTIBLE);
+		if (timeout < 0)
+			schedule();
+		else
+			schedule_timeout(timeout);
+	}
+	remove_wait_queue(&msp->wq, &wait);
+	return msp->rmmod || signal_pending(current);
+}
+
 static void msp3400c_stereo_wake(unsigned long data)
 {
 	struct msp3400c *msp = (struct msp3400c*)data;   /* XXX alpha ??? */
@@ -771,26 +787,16 @@ static int msp3400c_thread(void *data)
 	struct CARRIER_DETECT *cd;
 	int count, max1,max2,val1,val2, val,this;
 	
-	lock_kernel();
 	daemonize("msp3400");
-	msp->thread = current;
-	unlock_kernel();
-
+	allow_signal(SIGTERM);
 	printk("msp3400: daemon started\n");
-	if(msp->notify != NULL)
-		up(msp->notify);
 
 	for (;;) {
-		if (msp->rmmod)
-			goto done;
-		if (debug > 1)
-			printk("msp3400: thread: sleep\n");
-		interruptible_sleep_on(&msp->wq);
-		if (debug > 1)
-			printk("msp3400: thread: wakeup\n");
-		if (msp->rmmod || signal_pending(current))
+		d2printk("msp3400: thread: sleep\n");
+		if (msp34xx_sleep(msp,-1))
 			goto done;
 
+		d2printk("msp3400: thread: wakeup\n");
 		msp->active = 1;
 
 		if (msp->watch_stereo) {
@@ -800,9 +806,7 @@ static int msp3400c_thread(void *data)
 		}
 
 		/* some time for the tuner to sync */
-		set_current_state(TASK_INTERRUPTIBLE);
-		schedule_timeout(HZ/5);
-		if (signal_pending(current))
+		if (msp34xx_sleep(msp,HZ/5))
 			goto done;
 		
 	restart:
@@ -835,9 +839,7 @@ static int msp3400c_thread(void *data)
 		for (this = 0; this < count; this++) {
 			msp3400c_setcarrier(client, cd[this].cdo,cd[this].cdo);
 
-			set_current_state(TASK_INTERRUPTIBLE);
-			schedule_timeout(HZ/10);
-			if (signal_pending(current))
+			if (msp34xx_sleep(msp,HZ/10))
 				goto done;
 			if (msp->restart)
 				msp->restart = 0;
@@ -872,9 +874,7 @@ static int msp3400c_thread(void *data)
 		for (this = 0; this < count; this++) {
 			msp3400c_setcarrier(client, cd[this].cdo,cd[this].cdo);
 
-			set_current_state(TASK_INTERRUPTIBLE);
-			schedule_timeout(HZ/10);
-			if (signal_pending(current))
+			if (msp34xx_sleep(msp,HZ/10))
 				goto done;
 			if (msp->restart)
 				goto restart;
@@ -973,13 +973,9 @@ static int msp3400c_thread(void *data)
 	}
 
 done:
-	dprintk(KERN_DEBUG "msp3400: thread: exit\n");
 	msp->active = 0;
-	msp->thread = NULL;
-
-	if(msp->notify != NULL)
-		up(msp->notify);
-	return 0;
+	dprintk(KERN_DEBUG "msp3400: thread: exit\n");
+        complete_and_exit(&msp->texit, 0);
 }
 
 /* ----------------------------------------------------------------------- */
@@ -1019,26 +1015,16 @@ static int msp3410d_thread(void *data)
 	struct msp3400c *msp = i2c_get_clientdata(client);
 	int mode,val,i,std;
     
-	lock_kernel();
 	daemonize("msp3410 [auto]");
-	msp->thread = current;
-	unlock_kernel();
-
+	allow_signal(SIGTERM);
 	printk("msp3410: daemon started\n");
-	if(msp->notify != NULL)
-		up(msp->notify);
-		
+
 	for (;;) {
-		if (msp->rmmod)
-			goto done;
-		if (debug > 1)
-			printk(KERN_DEBUG "msp3410: thread: sleep\n");
-		interruptible_sleep_on(&msp->wq);
-		if (debug > 1)
-			printk(KERN_DEBUG "msp3410: thread: wakeup\n");
-		if (msp->rmmod || signal_pending(current))
+		d2printk(KERN_DEBUG "msp3410: thread: sleep\n");
+		if (msp34xx_sleep(msp,-1))
 			goto done;
 
+		d2printk(KERN_DEBUG "msp3410: thread: wakeup\n");
 		msp->active = 1;
 
 		if (msp->watch_stereo) {
@@ -1048,9 +1034,7 @@ static int msp3410d_thread(void *data)
 		}
 	
 		/* some time for the tuner to sync */
-		set_current_state(TASK_INTERRUPTIBLE);
-		schedule_timeout(HZ/5);
-		if (signal_pending(current))
+		if (msp34xx_sleep(msp,HZ/5))
 			goto done;
 
 	restart:
@@ -1109,9 +1093,7 @@ static int msp3410d_thread(void *data)
 		} else {
 			/* triggered autodetect */
 			for (;;) {
-				set_current_state(TASK_INTERRUPTIBLE);
-				schedule_timeout(HZ/10);
-				if (signal_pending(current))
+				if (msp34xx_sleep(msp,HZ/10))
 					goto done;
 				if (msp->restart)
 					goto restart;
@@ -1222,12 +1204,9 @@ static int msp3410d_thread(void *data)
 	}
 
 done:
-	dprintk(KERN_DEBUG "msp3410: thread: exit\n");
 	msp->active = 0;
-	msp->thread = NULL;
-
-	if(msp->notify != NULL)
-		up(msp->notify);
+	dprintk(KERN_DEBUG "msp3410: thread: exit\n");
+        complete_and_exit(&msp->texit, 0);
 	return 0;
 }
 
@@ -1257,10 +1236,9 @@ static struct i2c_client client_template =
 
 static int msp_attach(struct i2c_adapter *adap, int addr, int kind)
 {
-	DECLARE_MUTEX_LOCKED(sem);
 	struct msp3400c *msp;
         struct i2c_client *c;
-	int i, rc;
+	int i;
 
         client_template.adapter = adap;
         client_template.addr = addr;
@@ -1342,24 +1320,13 @@ static int msp_attach(struct i2c_adapter *adap, int addr, int kind)
 	printk("\n");
 
 	/* startup control thread */
-	msp->notify = &sem;
-	rc = kernel_thread(msp->simple ? msp3410d_thread : msp3400c_thread,
-			   (void *)c, 0);
-	if (rc < 0)
+	init_completion(&msp->texit);
+	msp->tpid = kernel_thread(msp->simple ? msp3410d_thread : msp3400c_thread,
+				  (void *)c, 0);
+	if (msp->tpid < 0)
 		printk(KERN_WARNING "msp34xx: kernel_thread() failed\n");
-	else
-		down(&sem);
-	msp->notify = NULL;
 	wake_up_interruptible(&msp->wq);
 
-	/* update our own array */
-	for (i = 0; i < MSP3400_MAX; i++) {
-		if (NULL == msps[i]) {
-			msps[i] = c;
-			break;
-		}
-	}
-	
 	/* done */
         i2c_attach_client(c);
 	return 0;
@@ -1367,30 +1334,17 @@ static int msp_attach(struct i2c_adapter *adap, int addr, int kind)
 
 static int msp_detach(struct i2c_client *client)
 {
-	DECLARE_MUTEX_LOCKED(sem);
 	struct msp3400c *msp  = i2c_get_clientdata(client);
-	int i;
 	
 	/* shutdown control thread */
-	del_timer(&msp->wake_stereo);
-	if (msp->thread) 
-	{
-		msp->notify = &sem;
+	del_timer_sync(&msp->wake_stereo);
+	if (msp->tpid >= 0) {
 		msp->rmmod = 1;
 		wake_up_interruptible(&msp->wq);
-		down(&sem);
-		msp->notify = NULL;
+		wait_for_completion(&msp->texit);
 	}
     	msp3400c_reset(client);
 
-        /* update our own array */
-	for (i = 0; i < MSP3400_MAX; i++) {
-		if (client == msps[i]) {
-			msps[i] = NULL;
-			break;
-		}
-	}
-
 	i2c_detach_client(client);
 	kfree(msp);
 	kfree(client);
@@ -1403,8 +1357,13 @@ static int msp_probe(struct i2c_adapter *adap)
 	if (adap->class & I2C_ADAP_CLASS_TV_ANALOG)
 		return i2c_probe(adap, &addr_data, msp_attach);
 #else
-	if (adap->id == (I2C_ALGO_BIT | I2C_HW_B_BT848))
+	switch (adap->id) {
+	case I2C_ALGO_BIT | I2C_HW_SMBUS_VOODOO3:
+	case I2C_ALGO_BIT | I2C_HW_B_BT848:
+	//case I2C_ALGO_SAA7134:
 		return i2c_probe(adap, &addr_data, msp_attach);
+		break;
+	}
 #endif
 	return 0;
 }

drivers/media/video/tda9887.c

@@ -24,8 +24,12 @@
     
 
 /* Addresses to scan */
-static unsigned short normal_i2c[] = {I2C_CLIENT_END};
-static unsigned short normal_i2c_range[] = {0x86>>1,0x86>>1,I2C_CLIENT_END};
+static unsigned short normal_i2c[] = {
+	0x86 >>1,
+	0x96 >>1,
+	I2C_CLIENT_END,
+};
+static unsigned short normal_i2c_range[] = {I2C_CLIENT_END,I2C_CLIENT_END};
 I2C_CLIENT_INSMOD;
 
 /* insmod options */

drivers/media/video/tuner.c

@@ -746,6 +746,7 @@ static int microtune_init(struct i2c_client *c)
         printk("tuner: microtune: companycode=%04x part=%02x rev=%02x\n",
 	       company_code,buf[0x13],buf[0x14]);
 	switch (company_code) {
+	case 0x30bf:
 	case 0x3cbf:
 	case 0x3dbf:
 	case 0x4d54:
@@ -1060,6 +1061,7 @@ static int tuner_probe(struct i2c_adapter *adap)
 		return i2c_probe(adap, &addr_data, tuner_attach);
 #else
 	switch (adap->id) {
+	case I2C_ALGO_BIT | I2C_HW_SMBUS_VOODOO3:
 	case I2C_ALGO_BIT | I2C_HW_B_BT848:
 	case I2C_ALGO_BIT | I2C_HW_B_RIVA:
 	case I2C_ALGO_SAA7134:

drivers/media/video/tvaudio.c

@@ -122,9 +122,10 @@ struct CHIPSTATE {
 	__u16 left,right,treble,bass,mode;
 	int prevmode;
 	int norm;
+
 	/* thread */
-	struct task_struct  *thread;
-	struct semaphore    *notify;
+	pid_t                tpid;
+	struct completion    texit;
 	wait_queue_head_t    wq;
 	struct timer_list    wt;
 	int                  done;
@@ -269,23 +270,24 @@ static void chip_thread_wake(unsigned long data)
 
 static int chip_thread(void *data)
 {
+	DECLARE_WAITQUEUE(wait, current);
         struct CHIPSTATE *chip = data;
 	struct CHIPDESC  *desc = chiplist + chip->type;
 	
-	lock_kernel();
 	daemonize("%s",i2c_clientname(&chip->c));
-	chip->thread = current;
-	unlock_kernel();
-
+	allow_signal(SIGTERM);
 	dprintk("%s: thread started\n", i2c_clientname(&chip->c));
-	if(chip->notify != NULL)
-		up(chip->notify);
 
 	for (;;) {
-		interruptible_sleep_on(&chip->wq);
-		dprintk("%s: thread wakeup\n", i2c_clientname(&chip->c));
+		add_wait_queue(&chip->wq, &wait);
+		if (!chip->done) {
+			set_current_state(TASK_INTERRUPTIBLE);
+			schedule();
+		}
+		remove_wait_queue(&chip->wq, &wait);
 		if (chip->done || signal_pending(current))
 			break;
+		dprintk("%s: thread wakeup\n", i2c_clientname(&chip->c));
 
 		/* don't do anything for radio or if mode != auto */
 		if (chip->norm == VIDEO_MODE_RADIO || chip->mode != 0)
@@ -298,11 +300,8 @@ static int chip_thread(void *data)
 		mod_timer(&chip->wt, jiffies+2*HZ);
 	}
 
-	chip->thread = NULL;
 	dprintk("%s: thread exiting\n", i2c_clientname(&chip->c));
-	if(chip->notify != NULL)
-		up(chip->notify);
-
+        complete_and_exit(&chip->texit, 0);
 	return 0;
 }
 
@@ -1420,7 +1419,6 @@ static int chip_attach(struct i2c_adapter *adap, int addr, int kind)
 {
 	struct CHIPSTATE *chip;
 	struct CHIPDESC  *desc;
-	int rc;
 
 	chip = kmalloc(sizeof(*chip),GFP_KERNEL);
 	if (!chip)
@@ -1480,21 +1478,18 @@ static int chip_attach(struct i2c_adapter *adap, int addr, int kind)
 		chip_write(chip,desc->treblereg,desc->treblefunc(chip->treble));
 	}
 
+	chip->tpid = -1;
 	if (desc->checkmode) {
 		/* start async thread */
-		DECLARE_MUTEX_LOCKED(sem);
-		chip->notify = &sem;
 		init_timer(&chip->wt);
 		chip->wt.function = chip_thread_wake;
 		chip->wt.data     = (unsigned long)chip;
 		init_waitqueue_head(&chip->wq);
-		rc = kernel_thread(chip_thread,(void *)chip,0);
-		if (rc < 0)
+		init_completion(&chip->texit);
+		chip->tpid = kernel_thread(chip_thread,(void *)chip,0);
+		if (chip->tpid < 0)
 			printk(KERN_WARNING "%s: kernel_thread() failed\n",
 			       i2c_clientname(&chip->c));
-		else
-			down(&sem);
-		chip->notify = NULL;
 		wake_up_interruptible(&chip->wq);
 	}
 	return 0;
@@ -1520,15 +1515,12 @@ static int chip_detach(struct i2c_client *client)
 {
 	struct CHIPSTATE *chip = i2c_get_clientdata(client);
 
-	del_timer(&chip->wt);
-	if (NULL != chip->thread) {
+	del_timer_sync(&chip->wt);
+	if (chip->tpid >= 0) {
 		/* shutdown async thread */
-		DECLARE_MUTEX_LOCKED(sem);
-		chip->notify = &sem;
 		chip->done = 1;
 		wake_up_interruptible(&chip->wq);
-		down(&sem);
-		chip->notify = NULL;
+		wait_for_completion(&chip->texit);
 	}
 	
 	i2c_detach_client(&chip->c);

drivers/media/video/tvmixer.c

@@ -134,6 +134,8 @@ static int tvmixer_ioctl(struct inode *inode, struct file *file, unsigned int cm
 		va.volume  = max(left,right);
 		va.balance = (32768*min(left,right)) / (va.volume ? va.volume : 1);
 		va.balance = (left<right) ? (65535-va.balance) : va.balance;
+		if (va.volume)
+			va.flags &= ~VIDEO_AUDIO_MUTE;
 		client->driver->command(client,VIDIOCSAUDIO,&va);
 		client->driver->command(client,VIDIOCGAUDIO,&va);
 		/* fall throuth */
@@ -267,6 +269,7 @@ static int tvmixer_clients(struct i2c_client *client)
 #else
 	/* TV card ??? */
 	switch (client->adapter->id) {
+	case I2C_ALGO_BIT | I2C_HW_SMBUS_VOODOO3:
 	case I2C_ALGO_BIT | I2C_HW_B_BT848:
 	case I2C_ALGO_BIT | I2C_HW_B_RIVA:
 		/* ok, have a look ... */