Hi

On Mon, Sep 9, 2013 at 10:25 AM, Florian Echtler [off-list ref] wrote:

On 06.09.2013 15:25, Benjamin Tissoires wrote:

quoted

Some generic comments:
- please always inline the code in the message, it is *much* easier to review and comment it
- please directly use a patch format: if the code is good, Dmitry can take it directly through his tree
- add the following people for further submissions:
  * Henrik - multitouch maintainer in the kernel, and I'm sure he will be happy to see this stuff
  * Dmitry - input maintainer, the driver will go through his tree, so it's better to let him know as soon as possible of the different discussions.
- please stick to the kernel formatting guidelines (without orders: do not use C99-style ("// ..."), do not mix tabs and spaces, stick to 80 columns, etc..). The whole documentation is in Documentation/CodingStyle, and use the script scripts/checkpatch.pl to validate most of these.
- I don't think a separate ".h" will be accepted as the declarations will not be used outside of the driver. Just merge the header on top of you .c file.

Benjamin and David, thanks for your feedback. I will integrate this and
get back to you with a proper patch ASAP.

I have one additional question which is more about "future-proofing"
this driver. The SUR40 hardware does also provide a raw video image from
the touch sensor over another USB _endpoint_. Since it's not a different
_interface_, this can only be handled in the same driver as far as I can
tell. At some point in the future, I would like to add a V4L2 interface
to also support the video endpoint - are there any issues I should
already try to watch out for in the input part?

Media drivers use input-devices to report input data. So no, there is
no reason to change your driver's design. Once you want
media-functionality, you simply add a v4l2 device during hid_probe()
and remove it in hid_remove(). User-space can see the relation between
both via sysfs.
It is never wrong to think about the API _now_. But as long as it's
only a single input-dev and one v4l2-dev, I'd recommend to put both
below your usb-device and you're fine.
Regarding internal driver-design, you should not bother now. Make it
work for input and make it work well.

Cheers
David