Re: [patch,rfc] Support for touchscreen on sharp zaurus sl-5500

[Dmitry Torokhov <dtor_core@ameritech.net>]

Hi,

On Thursday 21 July 2005 00:24, Pavel Machek wrote:
>  
> +config TOUCHSCREEN_COLLIE
> +	tristate "Collie touchscreen (for Sharp SL-5500)"
> +	depends on MCP_UCB1200 && INPUT

I don't think you need && INPUT here.

>  
>  obj-$(CONFIG_TOUCHSCREEN_BITSY)	+= h3600_ts_input.o
>  obj-$(CONFIG_TOUCHSCREEN_CORGI)	+= corgi_ts.o
> +obj-$(CONFIG_TOUCHSCREEN_COLLIE)+= collie_ts.o

A tab would be nice.

> +
> +/*
> + * This is a RT kernel thread that handles the ADC accesses
> + * (mainly so we can use semaphores in the UCB1200 core code
> + * to serialise accesses to the ADC).  The UCB1400 access
> + * functions are expected to be able to sleep as well.
> + */
> +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);
> +

This should be converted to kthread interface.


> +static int ucb1x00_ts_open(struct input_dev *idev)
> +{
> +	struct ucb1x00_ts *ts = (struct ucb1x00_ts *)idev;
> +	int ret = 0;
> +
> +	if (down_interruptible(&ts->sem))
> +		return -EINTR;
> +
> +	if (ts->use_count++ != 0)
> +		goto out;
> +

Please kill both ts->sem and ts->use_count - input core already serializes
input_open and input_close.

> +	if (ts->rtask)
> +		panic("ucb1x00: rtask running?");
> +
> +	sema_init(&ts->irq_wait, 0);
> +	ret = ucb1x00_hook_irq(ts->ucb, UCB_IRQ_TSPX, ucb1x00_ts_irq, ts);
> +	if (ret < 0)
> +		goto out;
> +
> +	/*
> +	 * If we do this at all, we should allow the user to
> +	 * measure and read the X and Y resistance at any time.
> +	 */
> +	ucb1x00_adc_enable(ts->ucb);
> +	ts->x_res = ucb1x00_ts_read_xres(ts);
> +	ts->y_res = ucb1x00_ts_read_yres(ts);
> +	ucb1x00_adc_disable(ts->ucb);
> +
> +	init_completion(&ts->init_exit);
> +	ret = kernel_thread(ucb1x00_thread, ts, CLONE_KERNEL);
> +	if (ret >= 0) {
> +		wait_for_completion(&ts->init_exit);
> +		ret = 0;
> +	} else {
> +		ucb1x00_free_irq(ts->ucb, UCB_IRQ_TSPX, ts);
> +	}

	kthread_run(ucb1x00_thread, ts, "ktsd")...

> +
> +MODULE_PARM(adcsync, "i");
> +MODULE_PARM_DESC(adcsync, "Enable use of ADCSYNC signal");

Die, MODULE_PARM, die!

-- 
Dmitry