Hi!
> > I have made quite a lot of cleanups to touchscreen part, and it seems
> > to be acceptable by input people. I think it should go into
> > drivers/input/touchscreen/collie_ts.c... Also it looks to me like
> > mcp.h should go into asm/arch-sa1100, so that other drivers can use it...
>
> I have couple of nitpicks (below) and one bigger concern - I am
> surprised that a driver for a physical device is implemented as an
> interface to a class device. This precludes implementing any kind of
> power management in the driver and pushes it into the parent and is
> generally speaking is a wrong thing to do (IMHO).
I'll port my changes to newer version of rmk's tree, that should solve
it.
> > +static int ucb1x00_thread(void *_ts)
> > +{
> > + struct ucb1x00_ts *ts = _ts;
> > + struct task_struct *tsk = current;
> > + int valid;
> > +
> > + ts->rtask = tsk;
>
> Just move that assignment into ucb1x00_input_open and kill all this
> "current" stuff.
It will still want to set the priority, but yes, it cleaned it.
> > + /*
> > + * We run as a real-time thread. However, thus far
> > + * this doesn't seem to be necessary.
> > + */
> > + tsk->policy = SCHED_FIFO;
> > + tsk->rt_priority = 1;
> > +
> > + valid = 0;
> > + for (;;) {
>
> Can we change this to "while (!kthread_should_stop())" to make me
> completely happy?
:-) Ok.
[Just FYI, I'll post agregated patch when I solve file placement and
port to newer version.]
Cleanups suggested by Dmitri.
---
commit 60814924ed695d863fa226c24b3d4e96054c8b66
tree 0e88e8272c23926c5654ae10bfe14d4cb2af84ab
parent ff30d8505b88064a5f6e6e70bd42028150a864e2
author <[EMAIL PROTECTED](none)> Mon, 25 Jul 2005 23:35:21 +0200
committer <[EMAIL PROTECTED](none)> Mon, 25 Jul 2005 23:35:21 +0200
drivers/input/touchscreen/collie_ts.c | 12 ++++--------
1 files changed, 4 insertions(+), 8 deletions(-)
diff --git a/drivers/input/touchscreen/collie_ts.c
b/drivers/input/touchscreen/collie_ts.c
--- a/drivers/input/touchscreen/collie_ts.c
+++ b/drivers/input/touchscreen/collie_ts.c
@@ -158,8 +158,6 @@ static int ucb1x00_thread(void *_ts)
struct task_struct *tsk = current;
int valid;
- ts->rtask = tsk;
-
/*
* We run as a real-time thread. However, thus far
* this doesn't seem to be necessary.
@@ -168,7 +166,7 @@ static int ucb1x00_thread(void *_ts)
tsk->rt_priority = 1;
valid = 0;
- for (;;) {
+ while (!kthread_should_stop()) {
unsigned int x, y, p, val;
ts->restart = 0;
@@ -231,9 +229,6 @@ static int ucb1x00_thread(void *_ts)
msleep_interruptible(10);
}
-
- if (kthread_should_stop())
- break;
}
ts->rtask = NULL;
@@ -273,11 +268,12 @@ static int ucb1x00_ts_open(struct input_
ts->y_res = ucb1x00_ts_read_yres(ts);
ucb1x00_adc_disable(ts->ucb);
- task = kthread_run(ucb1x00_thread, ts, "ktsd");
- if (!IS_ERR(task)) {
+ ts->rtask = kthread_run(ucb1x00_thread, ts, "ktsd");
+ if (!IS_ERR(ts->task)) {
ret = 0;
} else {
ucb1x00_free_irq(ts->ucb, UCB_IRQ_TSPX, ts);
+ ts->rtask = NULL;
ret = -EFAULT;
}
--
teflon -- maybe it is a trademark, but it should not be.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
