use kthread infrastructure in envctrl

envctrl currently uses very odd ways to stop a thread, using various things that should be exposed to drivers at all.

This patch (which is untested as I don't have sparc hardware) switches it to use the proper kthread infrastructure.

Index: linux-2.6/drivers/sbus/char/envctrl.c
=================================--- linux-2.6.orig/drivers/sbus/char/envctrl.c 2005-07-28 19:32:23.000000000 +0200
+++ linux-2.6/drivers/sbus/char/envctrl.c 2005-08-09 15:48:45.000000000 +0200
@@ -24,6 +24,7 @@
#include <linux/config.h>
#include <linux/module.h>
#include <linux/sched.h>
+#include <linux/kthread.h>
#include <linux/errno.h>
#include <linux/delay.h>
#include <linux/ioport.h>
@@ -1010,16 +1011,13 @@

poll_interval = 5000; /* TODO env_mon_interval */

- daemonize("kenvctrld");
- allow_signal(SIGKILL);
-
- kenvctrld_task = current;
-
printk(KERN_INFO "envctrl: %s starting...\n", current->comm); for (;;) {
- if(msleep_interruptible(poll_interval))
- break;
+ msleep_interruptible(poll_interval);

+ if (kthread_should_stop())
+ break;
+
for (whichcpu = 0; whichcpu < ENVCTRL_MAX_CPU; ++whichcpu) {
if (0 < envctrl_read_cpu_info(whichcpu, cputemp,
ENVCTRL_CPUTEMP_MON,
@@ -1118,9 +1116,11 @@
i2c_childlist[i].addr, (0 = i) ? ("\n") : (" "));
}

- err = kernel_thread(kenvctrld, NULL, CLONE_FS | CLONE_FILES);
- if (err < 0)
+ kenvctrld_task = kthread_run(kenvctrld, NULL, "kenvctrld");
+ if (IS_ERR(kenvctrld_task)) {
+ err = ERR_PTR(kenvctrld_task);
goto out_deregister;
+ }

return 0;

@@ -1142,28 +1142,7 @@
{
int i;

- if (NULL != kenvctrld_task) {
- force_sig(SIGKILL, kenvctrld_task);
- for (;;) {
- struct task_struct *p;
- int found = 0;
-
- read_lock(&tasklist_lock);
- for_each_process(p) {
- if (p = kenvctrld_task) {
- found = 1;
- break;
- }
- }
- read_unlock(&tasklist_lock);
-
- if (!found)
- break;
-
- msleep(1000);
- }
- kenvctrld_task = NULL;
- }
+ kthread_stop(kenvctrld_task);

iounmap(i2c);
misc_deregister(&envctrl_dev);

From: Christoph Hellwig <[email protected]>
Date: Tue, 9 Aug 2005 15:51:28 +0200

Looks good, applied.

Same exact kind of gross code (written by me of course) exists in bbc_envctrl.c as well. Would you mind sending me a patch to fix up that driver as well?

Thanks Christoph.

Sure. While we're at it, is there a specific reason various sbus drivers reimplement i2c functionality instead of using the common code?

Index: linux-2.6/drivers/sbus/char/bbc_envctrl.c
=================================--- linux-2.6.orig/drivers/sbus/char/bbc_envctrl.c 2005-07-28 19:32:23.000000000 +0200
+++ linux-2.6/drivers/sbus/char/bbc_envctrl.c 2005-08-09 22:27:48.000000000 +0200
@@ -7,6 +7,7 @@
#define __KERNEL_SYSCALLS__

#include <linux/kernel.h>
+#include <linux/kthread.h>
#include <linux/sched.h>
#include <linux/slab.h>
#include <linux/delay.h>
@@ -459,10 +460,6 @@

static int kenvctrld(void *__unused)
{
- daemonize("kenvctrld");
- allow_signal(SIGKILL);
- kenvctrld_task = current;
-
printk(KERN_INFO "bbc_envctrl: kenvctrld starting...\n"); last_warning_jiffies = jiffies - WARN_INTERVAL; for (;;) {
@@ -470,7 +467,7 @@
struct bbc_fan_control *fp;

msleep_interruptible(POLL_INTERVAL);
- if (signal_pending(current))
+ if (kthread_should_stop())
break;

for (tp = all_bbc_temps; tp; tp = tp->next) {
@@ -577,7 +574,6 @@
int temp_index = 0;
int fan_index = 0;
int devidx = 0;
- int err = 0;

while ((echild = bbc_i2c_getdev(devidx++)) != NULL) {
if (!strcmp(echild->prom_name, "temperature"))
@@ -585,9 +581,13 @@
if (!strcmp(echild->prom_name, "fan-control"))
attach_one_fan(echild, fan_index++);
}
- if (temp_index != 0 && fan_index != 0)
- err = kernel_thread(kenvctrld, NULL, CLONE_FS | CLONE_FILES);
- return err;
+ if (temp_index != 0 && fan_index != 0) {
+ kenvctrld_task = kthread_run(kenvctrld, NULL, "kenvctrld");
+ if (IS_ERR(kenvctrld_task))
+ return PTR_ERR(kenvctrld_task);
+ }
+
+ return 0;
}

static void destroy_one_temp(struct bbc_cpu_temperature *tp)
@@ -607,26 +607,7 @@
struct bbc_cpu_temperature *tp;
struct bbc_fan_control *fp;

- if (kenvctrld_task != NULL) {
- force_sig(SIGKILL, kenvctrld_task);
- for (;;) {
- struct task_struct *p;
- int found = 0;
-
- read_lock(&tasklist_lock);
- for_each_process(p) {
- if (p = kenvctrld_task) {
- found = 1;
- break;
- }
- }
- read_unlock(&tasklist_lock);
- if (!found)
- break;
- msleep(1000);
- }
- kenvctrld_task = NULL;
- }
+ kthread_stop(kenvctrld_task);

tp = all_bbc_temps;
while (tp != NULL) {

From: Christoph Hellwig <[email protected]>
Date: Tue, 9 Aug 2005 22:28:59 +0200

There wer no suitable i2c client registry, sharing, and mutual exclusion facilities in the i2c layer at the time. That has, of course, changed.

Another problem is that the device probing this driver wants to do is via the string names that are in the firmware device tree, and the i2c layer has no way for me to really plug in that stuff.

Anyways, I'll apply this patch, thanks a lot.