Re: [RFC][PATCH] USB touch screen driver, all-in-one

[Dmitry Torokhov]

On Saturday 11 March 2006 15:55, Daniel Ritz wrote:
> hi
> 
> here my merge of the USB touchscreen drivers, based on my patch from
> thursday for touchkitusb. this time it's a new driver...
> 
> and of course it's untested. i can test the egalax part next week...
> 
> [ also cc'ing the authors of the other drivers ]
> 
> the sizes for comparison:
>    text    data     bss     dec     hex filename
>    2942     724       4    3670     e56 touchkitusb.ko
>    2647     660       0    3307     ceb mtouchusb.ko
>    2448     628       0    3076     c04 itmtouch.ko
>    4097    1012       4    5113    13f9 usbtouchscreen.ko
> 
> comments?
> 

I like it.

> +
> +	input_dev->evbit[0] = BIT(EV_KEY) | BIT(EV_ABS);
> +	input_dev->keybit[LONG(BTN_TOUCH)] = BIT(BTN_TOUCH);
> +	input_set_abs_params(input_dev, ABS_X, type->min_xc, type->max_xc, 0, 0);
> +	input_set_abs_params(input_dev, ABS_Y, type->min_yc, type->max_yc, 0, 0);
> +	input_set_abs_params(input_dev, ABS_PRESSURE, type->min_press, type->max_press, 0, 0);
> +

Not all devices report pressure; driver should only advertise ABS_PRESSURE for
devices that actually support it.

> +	usb_fill_int_urb(usbtouch->irq, usbtouch->udev,
> +			 usb_rcvintpipe(usbtouch->udev, 0x81),
> +			 usbtouch->data, type->rept_size,
> +			 usbtouch_irq, usbtouch, endpoint->bInterval);
> +
> +	usbtouch->irq->transfer_dma = usbtouch->data_dma;
> +	usbtouch->irq->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
> +
> +	input_register_device(usbtouch->input);
> +

Please add error handling now that input_register_device() returns errors.

> +	usb_set_intfdata(intf, usbtouch);
> +
> +	/* device specific init */
> +	if (type->init)
> +		type->init(usbtouch);
> +

Should we do device-specific init before registering input device and also
handle errors here?

-- 
Dmitry
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/