On 7/20/05, Pavel Machek <[email protected]> wrote:
> Hi!
>
> This adds support for touchscreen of sharp zaurus sl-5500. I got the
> patches from John Lenz <[email protected]>, but lots of copyrights are
> Russell King. To do so, it needs to add quite a bit of
> infrastructure. If there's better place for some code, please let me
> know (I already moved touchscreen parts to drivers/input).
<snip>
> diff --git a/drivers/input/touchscreen/collie_ts.c
> b/drivers/input/touchscreen/collie_ts.c
> new file mode 100644
> --- /dev/null
> +++ b/drivers/input/touchscreen/collie_ts.c
<snip>
> +static int ucb1x00_thread(void *_ts)
> +{
> + struct ucb1x00_ts *ts = _ts;
> + struct task_struct *tsk = current;
> + int valid;
> +
> + ts->rtask = tsk;
> +
> + daemonize("ktsd");
> + /* only want to receive SIGKILL */
> + allow_signal(SIGKILL);
> +
> + /*
> + * 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;
> +
> + complete(&ts->init_exit);
> +
> + valid = 0;
> +
> + for (;;) {
> + unsigned int x, y, p, val;
> +
> + ts->restart = 0;
> +
> + ucb1x00_adc_enable(ts->ucb);
> +
> + x = ucb1x00_ts_read_xpos(ts);
> + y = ucb1x00_ts_read_ypos(ts);
> + p = ucb1x00_ts_read_pressure(ts);
> +
> + /*
> + * Switch back to interrupt mode.
> + */
> + ucb1x00_ts_mode_int(ts);
> + ucb1x00_adc_disable(ts->ucb);
> +
> + set_task_state(tsk, TASK_UNINTERRUPTIBLE);
> + schedule_timeout(HZ / 100);
> + if (signal_pending(tsk))
> + break;
You specifically allow SIGKILL, but then sleep uninterruptibly? And
then you check if signal_pending() :) I think you may want
TASK_INTERRUPTIBLE? Or, go one better and use msleep_interruptible(),
as I don't see any wait-queues in the immediate area of this code...
<snip>
> + set_task_state(tsk, TASK_INTERRUPTIBLE);
> + schedule_timeout(HZ / 100);
> + }
> +
> + if (signal_pending(tsk))
> + break;
Then you use INTERRUPTIBLE and check signal_pending() again, so I'm
pretty sure you wanted INTERRUPTIBLE above...But this sleep can be
msleep_interruptible(), as well?
<snip>
> diff --git a/drivers/misc/ucb1x00-core.c b/drivers/misc/ucb1x00-core.c
> new file mode 100644
> --- /dev/null
> +++ b/drivers/misc/ucb1x00-core.c
<snip>
> +/**
> + * ucb1x00_adc_read - read the specified ADC channel
> + * @ucb: UCB1x00 structure describing chip
> + * @adc_channel: ADC channel mask
> + * @sync: wait for syncronisation pulse.
> + *
> + * Start an ADC conversion and wait for the result. Note that
> + * synchronised ADC conversions (via the ADCSYNC pin) must wait
> + * until the trigger is asserted and the conversion is finished.
> + *
> + * This function currently spins waiting for the conversion to
> + * complete (2 frames max without sync).
You technically sleep (schedule_timeout()), not spin...
> + *
> + * If called for a synchronised ADC conversion, it may sleep
> + * with the ADC semaphore held.
> + */
> +unsigned int ucb1x00_adc_read(struct ucb1x00 *ucb, int adc_channel, int sync)
> +{
> + unsigned int val;
> +
> + if (sync)
> + adc_channel |= UCB_ADC_SYNC_ENA;
> +
> + ucb1x00_reg_write(ucb, UCB_ADC_CR, ucb->adc_cr | adc_channel);
> + ucb1x00_reg_write(ucb, UCB_ADC_CR, ucb->adc_cr | adc_channel | UCB_ADC_START);
> +
> + for (;;) {
> + val = ucb1x00_reg_read(ucb, UCB_ADC_DATA);
> + if (val & UCB_ADC_DAT_VAL)
> + break;
> + /* yield to other processes */
> + set_current_state(TASK_INTERRUPTIBLE);
> + schedule_timeout(1);
> + }
If I ever add a poll_event() interface to the kernel, this would be a
good user. You don't check if signal_pending(), though, even though
you are in INTERRUPTIBLE state... Maybe this case can use
UNINTERRUPTIBLE?
Thanks,
Nish
-
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/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
| |
 |