Re: [PATCH, RFC] Stop using tasklet in ds1374 RTC driver

[Randy Vinson <rvinson@mvista.com>]

Jean Delvare wrote:
> Hi all,
>
> I have the following patch, which addresses a might-sleep-in-tasklet
> issue in the ds1374 driver. I'm not too sure if the new code is right
> or not, as I have never been using workqueues before, and I also don't
> have a DS1374 chip to test my changes on.
>
> Can anyone comment on the patch and tell me if anything is wrong?
>
> Can anyone with a DS1374 chip please test it?

I've attached a similar patch that has been tested using the DS1374 on the Freescale MPC8349MDS reference system. It is patterned after a similar change made to the m41t00 driver. The changes work, but I am also unfamiliar with workqueues, so my patch may not be any better.

> I want this to be fixed now, because in -mm the ds1374 driver also uses
> the new mutex implementation, which is not allowed in tasklets, but is
> OK in workqueues.
>
> Thanks.

This patch changes the DS1374 driver to use workqueues (taken from a
similar change to the m41t00.c driver.) This patch has been tested on the
Freescale MPC8349MDS.

Signed-off-by: Randy Vinson <rvinson@mvista.com>
Index: linux-2.6.10_wrk/drivers/i2c/chips/ds1374.c
===================================================================
--- linux-2.6.10_wrk.orig/drivers/i2c/chips/ds1374.c
+++ linux-2.6.10_wrk/drivers/i2c/chips/ds1374.c
@@ -26,6 +26,7 @@
 #include <linux/i2c.h>
 #include <linux/rtc.h>
 #include <linux/bcd.h>
+#include <linux/workqueue.h>
 
 #include <asm/time.h>
 
@@ -51,6 +52,8 @@ static struct i2c_client *save_client;
 static unsigned short ignore[] = { I2C_CLIENT_END };
 static unsigned short normal_addr[] = { 0x68, I2C_CLIENT_END };
 
+static struct work_struct set_rtc_time_task;
+
 static struct i2c_client_address_data addr_data = {
 	.normal_i2c = normal_addr,
 	.normal_i2c_range = ignore,
@@ -180,7 +183,7 @@ int ds1374_set_rtc_time(ulong nowtime)
 	new_time = nowtime;
 
 	if (in_interrupt())
-		tasklet_schedule(&ds1374_tasklet);
+		schedule_work(&set_rtc_time_task);
 	else
 		ds1374_set_tlet((ulong) & new_time);
 
@@ -215,6 +218,9 @@ static int ds1374_probe(struct i2c_adapt
 		return rc;
 	}
 
+	INIT_WORK(&set_rtc_time_task,
+			(void (*)(void *))&ds1374_set_tlet, &new_time);
+
 	save_client = client;
 
 	ds1374_check_rtc_status();
@@ -233,7 +239,7 @@ static int ds1374_detach(struct i2c_clie
 
 	if ((rc = i2c_detach_client(client)) == 0) {
 		kfree(i2c_get_clientdata(client));
-		tasklet_kill(&ds1374_tasklet);
+		flush_scheduled_work();
 	}
 	return rc;
 }