Hi Justin, On Fri, Apr 25, 2008 at 02:56:37PM -0400, Justin Waters wrote: > The atmel tsadcc is a touchscreen/adc controller found on the AT91SAM9RL SoC. > This driver provides basic support for this controller for use as a > touchscreen. Some future improvements include suspend/resume capabilities and > debugfs support. > > Signed-off-by: Justin Waters <justin.waters@xxxxxxxxxxx> Thank you for the patch. Some comments: > diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile > index 35d4097..3302e27 100644 > --- a/drivers/input/touchscreen/Makefile > +++ b/drivers/input/touchscreen/Makefile > @@ -18,4 +18,5 @@ obj-$(CONFIG_TOUCHSCREEN_USB_COMPOSITE) += usbtouchscreen.o > obj-$(CONFIG_TOUCHSCREEN_PENMOUNT) += penmount.o > obj-$(CONFIG_TOUCHSCREEN_TOUCHRIGHT) += touchright.o > obj-$(CONFIG_TOUCHSCREEN_TOUCHWIN) += touchwin.o > +obj-$(CONFIG_TOUCHSCREEN_ATMEL_TSADCC) += atmel_tsadcc.o If it was sorted alphabetically it would help me. > + > + /* Pen remains down */ > + atmel_tsadcc_getevent(ts); > + > + input_report_abs(ts->input, ABS_X, ts->event.x); > + input_report_abs(ts->input, ABS_Y, ts->event.y); > + input_report_abs(ts->input, ABS_PRESSURE, 7500); > + The device does not really support pressure reading, please don't try to provide fake reports. Signal touch with BTN_TOUCH. > +static int __devexit atmel_tsadcc_remove(struct platform_device *pdev) > +{ > + struct atmel_tsadcc *ts = dev_get_drvdata(&pdev->dev); > + > + input_unregister_device(ts->input); > + > + free_irq(ts->irq, ts); You should free IRQ first, otherwise IRQ may fire just was device is unregistered and bad things may happed. Also please run checkpatch on the driver since there are some formatting and whitespace issues (note that I dont particularly care about 80 column limit as long as code is readable). Thank you. -- Dmitry -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html