Re: [RFC] surface-input

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi

On Thu, Sep 5, 2013 at 4:15 PM, Florian Echtler <floe@xxxxxxxxxxxxxx> wrote:
> Hello everyone,
>
> as mentioned earlier, I'm currently writing a multitouch input driver
> for the Pixelsense (formerly Surface 2.0). It's now at a point where I'd
> consider it mostly done, but a) I have very limited experience with
> kernel drivers and b) there are some additional questions I have, so I'm
> attaching the current state of my source code and would like to ask for
> your comments.
>
> Open question: it looks like just calling
>
> input_mt_init_slots(poll_dev->input, MAX_CONTACTS, INPUT_MT_DIRECT);
>
> in the initialization routine isn't enough. hid-multitouch doesn't seem
> to use any other init commands, though, or did I overlook something?
>
> Are there any other caveats of the input-mt library which I should be
> aware of?
>
> Thanks for your input, and best regards, Florian

Please inline the code as Benjamin noted. Otherwise, it's cumbersome
to comment on specific lines (please see "git help format-patch" and
"git help send-email").
I did review the core setup/lifetime/destruction code and it looks
good. But you need to fix several error-paths. If a core function
returns "int", you should check it for "r < 0" and then forward the
code. Don't overwrite it with EINVAL.
Furthermore, correctly free your memory. Things like this leak memory
(surface might be non-NULL):
  if (!surface || !poll_dev)
    return -ENOMEM;

I can do a more complete review for v2 if you send it inline. But
looks good overall.

Thanks
David
--
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




[Index of Archives]     [Linux Media Devel]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Linux Wireless Networking]     [Linux Omap]

  Powered by Linux