Messages in this thread | | | Date | Mon, 25 Jul 2005 10:16:05 -0500 | From | Dmitry Torokhov <> | Subject | Re: [patch 1/2] Touchscreen support for sharp sl-5500 |
| |
Hi Pavel,
On 7/24/05, Pavel Machek <pavel@suse.cz> wrote: > > 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).
If the problem is that you have a single piece of hardware you need to bind several drivers to - I guess you will have to create a new sub-device bus for that. Or just register sub-devices on the same bus the parent device is registered on - I am not sure what is best in this particular case - I am not familiar with the arch. It is my understanding that the purpose of interfaces to to present different "views" to userspace and therefore they are not quie suited for what you are trying to do...
> +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.
> + > + /* > + * 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?
Thanks!
-- Dmitry - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |