Re: [PATCH] input: Synaptics USB device driver

On Thu, Jan 05, 2012 at 05:39:57AM +0000, Jan Steinhoff wrote:

On Wed, 4 Jan 2012 01:41:03 -0800
Dmitry Torokhov [off-list ref] wrote:

quoted

Om, so I have been looking at the driver and the following seems to be
still working with my Lenovo composite touchpad/stick.

Changes:

- the stick is reported in relative mode; we might consider addig
  sensitivity control similar to trackpoint driver. Press-to-select -
  maybe in driver, or in client; undecided.

- got rid of BTN_MISC/BTN_MIDDLE option (at least for now);

Please make BTN_MIDDLE the default in this case for the cPad. This
button is actually located where the middle button should be. Only if
one uses the cPad's background display it obtains a "special" meaning.
(Then pressing it should launch a menu on the background display and the
touchpad works as a touchscreen for this display.)

OK, let's change it to MTN_MIDDLE. We may consider changing it to
BTN_MISC if cPad display support is merged and active.

quoted

- added devices to HID blacklist;

I think it is more flexible and save to use the manual driver binding
capability of the USB core instead:

http://lwn.net/Articles/143397/

Which driver to use for which device can then be set, e.g., with the
help of udev config files.

However it is more work for distributions and users. As it stands now
adding blacklist and compiling synaptics_usb does not require any
additional userspace support (everyone packages and installs synaptics X
driver nowadays).

Blacklisting has the big disadvantage that the device will not work at
all if the synaptics-usb kernel module is not loaded/present, whereas
otherwise usbhid is used as the default. At least the corresponding
part in the blacklist should be enclosed in
#ifdef CONFIG_MOUSE_SYNAPTICS_USB ... #endif

This is a good idea, I'll add ifdefs.

Further, as noted in the initial comment, not all devices have been
tested with this driver. This is another reason to allow the user to
choose which driver to use.

I'll remove touchscreen from blacklist/device table as it really should
generate different set of events; the rest I hope we'll be able to fix
reasonably quickly if there are complaints.

quoted

- runtime PM (hopefully I got it right);

- open/close.

I do not understand why needs_remote_wakeup is flipped in open/close.
As far as I know, the device can not autosuspend after
usb_autopm_get_interface was called anyway?

We do usb_autopm_put_interface as last action in open() so the device
will be suspended and needs wakeups.

quoted

Still TODO:

- query device for supported rages instead of having module
parameters.

At least for the cPad the device does unfortunately not provide this
information. I tried to read the HID descriptors once but just got
errors. However, the defaults work quite well for all tested devices.
The module parameters are actually meant to just provide a simple
workaround in case one of the untested devices (especially the
touchscreen) might need them.

OK, so why don't we replace module parameters with default ranges and
maybe later Synaptics guys will tell us how to query ranges properly?
Default ranges worked reasonably well for PS/2 version for several
years.

Thanks.

-- 
Dmitry