lkml.org 
[lkml]   [2006]   [Mar]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH, RFC] Stop using tasklet in ds1374 RTC driver
Jean Delvare wrote:
>
> Well I already have a patch for ds1374. This is basically a mix between
> Randy's and my original patch: I've kept the dedicated workqueue as my
> patch had, but preserved the in_interrupt() call as in Randy's. Here's
> the result:
>
> * * * * *
>
> A tasklet is not suitable for what the ds1374 driver does: neither sleeping
> nor mutex operations are allowed in tasklets, and ds1374_set_tlet may do
> both.
>
> We can use a workqueue instead, where both sleeping and mutex operations
> are allowed.
>
> Signed-off-by: Jean Delvare <khali@linux-fr.org>
> ---
> drivers/i2c/chips/ds1374.c | 16 ++++++++++------
> 1 file changed, 10 insertions(+), 6 deletions(-)
>
> --- linux-2.6.16-git.orig/drivers/i2c/chips/ds1374.c 2006-03-27 18:25:17.000000000 +0200
> +++ linux-2.6.16-git/drivers/i2c/chips/ds1374.c 2006-03-27 18:59:05.000000000 +0200
> @@ -27,6 +27,7 @@
> #include <linux/rtc.h>
> #include <linux/bcd.h>
> #include <linux/mutex.h>
> +#include <linux/workqueue.h>
>
> #define DS1374_REG_TOD0 0x00
> #define DS1374_REG_TOD1 0x01
> @@ -139,7 +140,7 @@
> return t1;
> }
>
> -static void ds1374_set_tlet(ulong arg)
> +static void ds1374_set_work(void *arg)
> {
> ulong t1, t2;
> int limit = 10; /* arbitrary retry limit */
> @@ -168,17 +169,18 @@
>
> static ulong new_time;
>
> -static DECLARE_TASKLET_DISABLED(ds1374_tasklet, ds1374_set_tlet,
> - (ulong) & new_time);
> +static struct workqueue_struct *ds1374_workqueue;
> +
> +static DECLARE_WORK(ds1374_work, ds1374_set_work, &new_time);
>
> int ds1374_set_rtc_time(ulong nowtime)
> {
> new_time = nowtime;
>
> if (in_interrupt())
> - tasklet_schedule(&ds1374_tasklet);
> + queue_work(ds1374_workqueue, &ds1374_work);
> else
> - ds1374_set_tlet((ulong) & new_time);
> + ds1374_set_work(&new_time);
>
> return 0;
> }
> @@ -204,6 +206,8 @@
> client->adapter = adap;
> client->driver = &ds1374_driver;
>
> + ds1374_workqueue = create_singlethread_workqueue("ds1374");
> +
> if ((rc = i2c_attach_client(client)) != 0) {
> kfree(client);
> return rc;
> @@ -227,7 +231,7 @@
>
> if ((rc = i2c_detach_client(client)) == 0) {
> kfree(i2c_get_clientdata(client));
> - tasklet_kill(&ds1374_tasklet);
> + destroy_workqueue(ds1374_workqueue);
> }
> return rc;
> }
>
> * * * * *
>
> If it's OK, then Randy can just sign it off and I'll push it to Greg
> quickly. If it's not OK for some reason, I'll just wait for a new patch
> from Randy.
I've tested the patch. It works fine for me.

Randy V.
A tasklet is not suitable for what the ds1374 driver does: neither sleeping
nor mutex operations are allowed in tasklets, and ds1374_set_tlet may do
both.

We can use a workqueue instead, where both sleeping and mutex operations
are allowed.

Acked-by: Randy Vinson <rvinson@mvista.com>
Signed-off-by: Jean Delvare <khali@linux-fr.org>
---
drivers/i2c/chips/ds1374.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)

--- linux-2.6.16-git.orig/drivers/i2c/chips/ds1374.c 2006-03-27 18:25:17.000000000 +0200
+++ linux-2.6.16-git/drivers/i2c/chips/ds1374.c 2006-03-27 18:59:05.000000000 +0200
@@ -27,6 +27,7 @@
#include <linux/rtc.h>
#include <linux/bcd.h>
#include <linux/mutex.h>
+#include <linux/workqueue.h>

#define DS1374_REG_TOD0 0x00
#define DS1374_REG_TOD1 0x01
@@ -139,7 +140,7 @@
return t1;
}

-static void ds1374_set_tlet(ulong arg)
+static void ds1374_set_work(void *arg)
{
ulong t1, t2;
int limit = 10; /* arbitrary retry limit */
@@ -168,17 +169,18 @@

static ulong new_time;

-static DECLARE_TASKLET_DISABLED(ds1374_tasklet, ds1374_set_tlet,
- (ulong) & new_time);
+static struct workqueue_struct *ds1374_workqueue;
+
+static DECLARE_WORK(ds1374_work, ds1374_set_work, &new_time);

int ds1374_set_rtc_time(ulong nowtime)
{
new_time = nowtime;

if (in_interrupt())
- tasklet_schedule(&ds1374_tasklet);
+ queue_work(ds1374_workqueue, &ds1374_work);
else
- ds1374_set_tlet((ulong) & new_time);
+ ds1374_set_work(&new_time);

return 0;
}
@@ -204,6 +206,8 @@
client->adapter = adap;
client->driver = &ds1374_driver;

+ ds1374_workqueue = create_singlethread_workqueue("ds1374");
+
if ((rc = i2c_attach_client(client)) != 0) {
kfree(client);
return rc;
@@ -227,7 +231,7 @@

if ((rc = i2c_detach_client(client)) == 0) {
kfree(i2c_get_clientdata(client));
- tasklet_kill(&ds1374_tasklet);
+ destroy_workqueue(ds1374_workqueue);
}
return rc;
}
\
 
 \ /
  Last update: 2006-03-28 00:34    [W:0.054 / U:0.084 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site