On 13-04-18, Benjamin Tissoires wrote: > Hi Peter, > > On Thu, Apr 12, 2018 at 7:36 PM, pgzh <peter.ganzhorn@xxxxxxxxx> wrote: > > The IBM/Lenovo Scrollpoint mice feature a trackpoint-like stick instead of a > > scrolling wheel capable of 2-D (vertical+horizontal) scrolling. > > hid-generic does only expose 1-D (vertical) scrolling functionality for these mice. > > This patch adds support for horizontal scrolling for the IBM/Lenovo Scrollpoint > > mice to hid-lenovo. > > > > changes in v2: > > - commit message enhanced > > - removed hid-quirk.c hunk > > - merged separate patches into one single patch > > Just FYI, the changes should go after the first '---'. This way they > will not be added in the final commit message. > > As a trick, I add in my commit messages '---' then the changes. They > are kept accross revisions in my tree, but when Jiri applies the > patch, they magically disappear :) > > Appart from that, the patch looks good. > > I just have one tiny remark inlined below: > > > > > Signed-off-by: Peter Ganzhorn <peter.ganzhorn@xxxxxxxxx> > > --- > > drivers/hid/Kconfig | 7 ++++--- > > drivers/hid/hid-ids.h | 8 ++++++++ > > drivers/hid/hid-lenovo.c | 36 ++++++++++++++++++++++++++++++++++++ > > 3 files changed, 48 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig > > index 60252fd796f6..ab5e84018345 100644 > > --- a/drivers/hid/Kconfig > > +++ b/drivers/hid/Kconfig > > @@ -462,10 +462,11 @@ config HID_LENOVO > > select NEW_LEDS > > select LEDS_CLASS > > ---help--- > > - Support for Lenovo devices that are not fully compliant with HID standard. > > + Support for IBM/Lenovo devices that are not fully compliant with HID standard. > > > > - Say Y if you want support for the non-compliant features of the Lenovo > > - Thinkpad standalone keyboards, e.g: > > + Say Y if you want support for horizontal scrolling of the IBM/Lenovo > > + Scrollpoint mice or the non-compliant features of the Lenovo Thinkpad > > + standalone keyboards, e.g: > > - ThinkPad USB Keyboard with TrackPoint (supports extra LEDs and trackpoint > > configuration) > > - ThinkPad Compact Bluetooth Keyboard with TrackPoint (supports Fn keys) > > diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h > > index 5a3a7ead3012..484511a58a7b 100644 > > --- a/drivers/hid/hid-ids.h > > +++ b/drivers/hid/hid-ids.h > > @@ -549,6 +549,13 @@ > > #define USB_VENDOR_ID_HUION 0x256c > > #define USB_DEVICE_ID_HUION_TABLET 0x006e > > > > +#define USB_VENDOR_ID_IBM 0x04b3 > > +#define USB_DEVICE_ID_IBM_SCROLLPOINT_III 0x3100 > > +#define USB_DEVICE_ID_IBM_SCROLLPOINT_PRO 0x3103 > > +#define USB_DEVICE_ID_IBM_SCROLLPOINT_OPTICAL 0x3105 > > +#define USB_DEVICE_ID_IBM_SCROLLPOINT_800DPI_OPTICAL 0x3108 > > +#define USB_DEVICE_ID_IBM_SCROLLPOINT_800DPI_OPTICAL_PRO 0x3109 > > + > > #define USB_VENDOR_ID_IDEACOM 0x1cb6 > > #define USB_DEVICE_ID_IDEACOM_IDC6650 0x6650 > > #define USB_DEVICE_ID_IDEACOM_IDC6651 0x6651 > > @@ -681,6 +688,7 @@ > > #define USB_DEVICE_ID_LENOVO_TPKBD 0x6009 > > #define USB_DEVICE_ID_LENOVO_CUSBKBD 0x6047 > > #define USB_DEVICE_ID_LENOVO_CBTKBD 0x6048 > > +#define USB_DEVICE_ID_LENOVO_SCROLLPOINT_OPTICAL 0x6049 > > #define USB_DEVICE_ID_LENOVO_TPPRODOCK 0x6067 > > #define USB_DEVICE_ID_LENOVO_X1_COVER 0x6085 > > #define USB_DEVICE_ID_LENOVO_X1_TAB 0x60a3 > > diff --git a/drivers/hid/hid-lenovo.c b/drivers/hid/hid-lenovo.c > > index 1ac4ff4d57a6..643b6eb54442 100644 > > --- a/drivers/hid/hid-lenovo.c > > +++ b/drivers/hid/hid-lenovo.c > > @@ -6,6 +6,17 @@ > > * > > * Copyright (c) 2012 Bernhard Seibold > > * Copyright (c) 2014 Jamie Lentin <jm@xxxxxxxxxxxx> > > + * > > + * Linux IBM/Lenovo Scrollpoint mouse driver: > > + * - IBM Scrollpoint III > > + * - IBM Scrollpoint Pro > > + * - IBM Scrollpoint Optical > > + * - IBM Scrollpoint Optical 800dpi > > + * - IBM Scrollpoint Optical 800dpi Pro > > + * - Lenovo Scrollpoint Optical > > + * > > + * Copyright (c) 2012 Peter De Wachter <pdewacht@xxxxxxxxx> > > Where is that copyright coming from? > > [further google search later] > > Peter (De Wachter), could you add your signed-off by on this patch too > given that I found > https://github.com/pdewacht/hid-scrollpoint/blob/master/hid-scrollpoint.c > which seems to be the base for this patch. > > With Peter's ACK, this is: > Reviewed-by: Benjamin Tissoires <benjamin.tissoires@xxxxxxxxxx> > > Cheers, > Benjamin > > > + * Copyright (c) 2018 Peter Ganzhorn <peter.ganzhorn@xxxxxxxxx> > > */ > > > > /* > > @@ -160,6 +171,17 @@ static int lenovo_input_mapping_cptkbd(struct hid_device *hdev, > > return 0; > > } > > > > +static int lenovo_input_mapping_scrollpoint(struct hid_device *hdev, > > + struct hid_input *hi, struct hid_field *field, > > + struct hid_usage *usage, unsigned long **bit, int *max) > > +{ > > + if (usage->hid == HID_GD_Z) { > > + hid_map_usage(hi, usage, bit, max, EV_REL, REL_HWHEEL); > > + return 1; > > + } > > + return 0; > > +} > > + > > static int lenovo_input_mapping(struct hid_device *hdev, > > struct hid_input *hi, struct hid_field *field, > > struct hid_usage *usage, unsigned long **bit, int *max) > > @@ -172,6 +194,14 @@ static int lenovo_input_mapping(struct hid_device *hdev, > > case USB_DEVICE_ID_LENOVO_CBTKBD: > > return lenovo_input_mapping_cptkbd(hdev, hi, field, > > usage, bit, max); > > + case USB_DEVICE_ID_IBM_SCROLLPOINT_III: > > + case USB_DEVICE_ID_IBM_SCROLLPOINT_PRO: > > + case USB_DEVICE_ID_IBM_SCROLLPOINT_OPTICAL: > > + case USB_DEVICE_ID_IBM_SCROLLPOINT_800DPI_OPTICAL: > > + case USB_DEVICE_ID_IBM_SCROLLPOINT_800DPI_OPTICAL_PRO: > > + case USB_DEVICE_ID_LENOVO_SCROLLPOINT_OPTICAL: > > + return lenovo_input_mapping_scrollpoint(hdev, hi, field, > > + usage, bit, max); > > default: > > return 0; > > } > > @@ -883,6 +913,12 @@ static const struct hid_device_id lenovo_devices[] = { > > { HID_USB_DEVICE(USB_VENDOR_ID_LENOVO, USB_DEVICE_ID_LENOVO_CUSBKBD) }, > > { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LENOVO, USB_DEVICE_ID_LENOVO_CBTKBD) }, > > { HID_USB_DEVICE(USB_VENDOR_ID_LENOVO, USB_DEVICE_ID_LENOVO_TPPRODOCK) }, > > + { HID_USB_DEVICE(USB_VENDOR_ID_IBM, USB_DEVICE_ID_IBM_SCROLLPOINT_III) }, > > + { HID_USB_DEVICE(USB_VENDOR_ID_IBM, USB_DEVICE_ID_IBM_SCROLLPOINT_PRO) }, > > + { HID_USB_DEVICE(USB_VENDOR_ID_IBM, USB_DEVICE_ID_IBM_SCROLLPOINT_OPTICAL) }, > > + { HID_USB_DEVICE(USB_VENDOR_ID_IBM, USB_DEVICE_ID_IBM_SCROLLPOINT_800DPI_OPTICAL) }, > > + { HID_USB_DEVICE(USB_VENDOR_ID_IBM, USB_DEVICE_ID_IBM_SCROLLPOINT_800DPI_OPTICAL_PRO) }, > > + { HID_USB_DEVICE(USB_VENDOR_ID_LENOVO, USB_DEVICE_ID_LENOVO_SCROLLPOINT_OPTICAL) }, > > { } > > }; > > > > -- > > 2.11.0 > > -- Hi Benjamin, thanks for the tip, I will keep this in mind the next time. I kept the copyright of Peter De Wachter since he is the original author of the scrollpoint driver you found on Github. He also tried to submit a patch in 2013 (https://lkml.org/lkml/2013/7/2/549) but never answered Jiris feedback since he never saw the reply. I already mentioned his work and previous submission in my first email. Since my patch is based on his work, I wanted to keep his name and also contacted him before submitting my patch. Thank you for giving me so much feedback, I really appreciate it! Is there anything else that needs to be done to get the patch merged? Peter -- 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
