On Sun, Jul 13, 2014 at 4:59 AM, Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx> wrote: > Hi Linus, Dmitry, > > On Sat, Jul 12, 2014 at 11:31:21AM +0200, Linus Walleij wrote: >> From: Dmitry Artamonow <mad_soft@xxxxxxxx> > > So is this Dmitry's or Alessandro's work? Multiple authors. Alessandro, Dmitry and me. In order of sign-off... The only viable rule I have for multiple authors is to attribute it to the author who wrote the bulk of the code, and that comes from Dmitry. >> +#include <linux/mfd/ipaq-micro.h> > > Has this been merged already? Yes. v3.16 merge window. >> +static void micro_ts_receive(void *data, int len, unsigned char *msg) >> +{ >> + struct touchscreen_data *ts = data; >> + >> + if (len == 4) { >> + input_report_abs(ts->input, ABS_X, (msg[2]<<8)+msg[3]); > > be16_to_cpup()? Yes! Replacing everywhere. >> + input_report_abs(ts->input, ABS_Y, (msg[0]<<8)+msg[1]); >> + input_report_abs(ts->input, ABS_PRESSURE, 1); > > Please no fake pressure events, update your tslib instead. > >> + input_report_key(ts->input, BTN_TOUCH, 0); > > This seems completely backwards. Yeah... I wonder what happened here. >> + ts->input = input_allocate_device(); > > Error handling please; also consider using devm_input_allocate_device(). > >> + >> + ts->input->evbit[0] = BIT(EV_ABS); >> + ts->input->absbit[0] = BIT(ABS_X) | BIT(ABS_Y) | BIT(ABS_PRESSURE); >> + input_set_abs_params(ts->input, ABS_X, 0, 1023, 0, 0); >> + input_set_abs_params(ts->input, ABS_Y, 0, 1023, 0, 0); >> + input_set_abs_params(ts->input, ABS_PRESSURE, 0x0, 0x1, 0, 0); >> + > > You are missing setting BTN_TOUCH in capabilities so events you are > trying to send in micro_ts_receive will not go anywhere. Yeah. And I switched this to using just input_set_capability(). >> + spin_lock(&ts->micro->lock); >> + ts->micro->ts = NULL; >> + ts->micro->ts_data = NULL; >> + spin_unlock(&ts->micro->lock); > > What is the point here? Unregistering the callbacks so the MFD core device will stop calling them. Or we will crash the kernel when removing the driver as next event arrives. Sending v2 soon. Yours, Linus Walleij -- 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