LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* Re: [PATCH] use new msleep() in ADT746x driver
       [not found] <200405291908.i4TJ8Acm011281@hera.kernel.org>
@ 2004-05-29 19:54 ` Jeff Garzik
  2004-05-29 23:26   ` Benjamin Herrenschmidt
  0 siblings, 1 reply; 4+ messages in thread
From: Jeff Garzik @ 2004-05-29 19:54 UTC (permalink / raw)
  To: benh; +Cc: Linux Kernel Mailing List, Andrew Morton

Linux Kernel Mailing List wrote:
> diff -Nru a/drivers/macintosh/therm_adt746x.c b/drivers/macintosh/therm_adt746x.c
> --- a/drivers/macintosh/therm_adt746x.c	2004-05-29 12:08:19 -07:00
> +++ b/drivers/macintosh/therm_adt746x.c	2004-05-29 12:08:19 -07:00
> @@ -246,8 +246,7 @@
>  
>  	while(monitor_running)
>  	{
> -		set_task_state(current, TASK_UNINTERRUPTIBLE);
> -		schedule_timeout(2*HZ);
> +		msleep(2000);


IMO this is moving the code away from what the coder appeared to intend.

A "sleep(2)" would be preferred, and more clear.

	Jeff



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] use new msleep() in ADT746x driver
  2004-05-29 19:54 ` [PATCH] use new msleep() in ADT746x driver Jeff Garzik
@ 2004-05-29 23:26   ` Benjamin Herrenschmidt
  2004-06-02 20:22     ` Jeff Garzik
  0 siblings, 1 reply; 4+ messages in thread
From: Benjamin Herrenschmidt @ 2004-05-29 23:26 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: Linux Kernel Mailing List, Andrew Morton

On Sun, 2004-05-30 at 05:54, Jeff Garzik wrote:
> Linux Kernel Mailing List wrote:
> > diff -Nru a/drivers/macintosh/therm_adt746x.c b/drivers/macintosh/therm_adt746x.c
> > --- a/drivers/macintosh/therm_adt746x.c	2004-05-29 12:08:19 -07:00
> > +++ b/drivers/macintosh/therm_adt746x.c	2004-05-29 12:08:19 -07:00
> > @@ -246,8 +246,7 @@
> >  
> >  	while(monitor_running)
> >  	{
> > -		set_task_state(current, TASK_UNINTERRUPTIBLE);
> > -		schedule_timeout(2*HZ);
> > +		msleep(2000);
> 
> 
> IMO this is moving the code away from what the coder appeared to intend.
> 
> A "sleep(2)" would be preferred, and more clear.

This patch was done by the original author of the driver

Ben.



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] use new msleep() in ADT746x driver
  2004-05-29 23:26   ` Benjamin Herrenschmidt
@ 2004-06-02 20:22     ` Jeff Garzik
  0 siblings, 0 replies; 4+ messages in thread
From: Jeff Garzik @ 2004-06-02 20:22 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: Linux Kernel Mailing List, Andrew Morton

Benjamin Herrenschmidt wrote:
> On Sun, 2004-05-30 at 05:54, Jeff Garzik wrote:
> 
>>Linux Kernel Mailing List wrote:
>>
>>>diff -Nru a/drivers/macintosh/therm_adt746x.c b/drivers/macintosh/therm_adt746x.c
>>>--- a/drivers/macintosh/therm_adt746x.c	2004-05-29 12:08:19 -07:00
>>>+++ b/drivers/macintosh/therm_adt746x.c	2004-05-29 12:08:19 -07:00
>>>@@ -246,8 +246,7 @@
>>> 
>>> 	while(monitor_running)
>>> 	{
>>>-		set_task_state(current, TASK_UNINTERRUPTIBLE);
>>>-		schedule_timeout(2*HZ);
>>>+		msleep(2000);
>>
>>
>>IMO this is moving the code away from what the coder appeared to intend.
>>
>>A "sleep(2)" would be preferred, and more clear.
> 
> 
> This patch was done by the original author of the driver


Well, I think the author is making his driver less readable...  "2 
seconds" is more clear than "2000 msec", and I am willing to bet that 
sleep() would have been used, had it existed.

	Jeff



^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH] use new msleep() in ADT746x driver
@ 2004-05-28  9:23 Benjamin Herrenschmidt
  0 siblings, 0 replies; 4+ messages in thread
From: Benjamin Herrenschmidt @ 2004-05-28  9:23 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Linus Torvalds, Linux Kernel list

This patch replaces schedule_timeout() with the new msleep().

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Signed-off-by: Colin Leroy <colin@colino.net>

--- drivers/macintosh/therm_adt746x.c.orig	2004-05-27 08:55:17.000000000 +0200
+++ drivers/macintosh/therm_adt746x.c	2004-05-28 08:59:27.589388936 +0200
@@ -247,8 +247,7 @@
 
 	while(monitor_running)
 	{
-		set_task_state(current, TASK_UNINTERRUPTIBLE);
-		schedule_timeout(2*HZ);
+		msleep(2000);
 
 		/* Check status */
 		/* local   : chip */
-- 
Benjamin Herrenschmidt <benh@kernel.crashing.org>


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2004-06-02 20:28 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <200405291908.i4TJ8Acm011281@hera.kernel.org>
2004-05-29 19:54 ` [PATCH] use new msleep() in ADT746x driver Jeff Garzik
2004-05-29 23:26   ` Benjamin Herrenschmidt
2004-06-02 20:22     ` Jeff Garzik
2004-05-28  9:23 Benjamin Herrenschmidt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).