Linux Input/HID development
 help / color / mirror / Atom feed
* Re: [PATCH 4/5] hid: huion: Switch to generating report descriptor
From: Benjamin Tissoires @ 2014-07-23 14:42 UTC (permalink / raw)
  To: Nikolai Kondrashov; +Cc: Jiri Kosina, linux-input, DIGImend-devel
In-Reply-To: <1406119378-24551-5-git-send-email-spbnick@gmail.com>

On Wed, Jul 23, 2014 at 8:42 AM, Nikolai Kondrashov <spbnick@gmail.com> wrote:
> Switch to generating tablet pen report descriptor from a template and
> parameters retrieved from string descriptor 0x64.
>
> Signed-off-by: Nikolai Kondrashov <spbnick@gmail.com>
> ---

This is a nice solution.

I still have few remarks/questions inlined:

>  drivers/hid/hid-huion.c | 245 +++++++++++++++++++++++++++++++++---------------
>  1 file changed, 167 insertions(+), 78 deletions(-)
>
> diff --git a/drivers/hid/hid-huion.c b/drivers/hid/hid-huion.c
> index e5f1e22..e6f5338 100644
> --- a/drivers/hid/hid-huion.c
> +++ b/drivers/hid/hid-huion.c
> @@ -16,67 +16,89 @@
>  #include <linux/hid.h>
>  #include <linux/module.h>
>  #include <linux/usb.h>
> +#include <asm/unaligned.h>
>  #include "usbhid/usbhid.h"
>
>  #include "hid-ids.h"
>
> -/* Original tablet report descriptor size */
> -#define HUION_TABLET_RDESC_ORIG_SIZE   177
> -
> -/* Fixed tablet report descriptor */
> -static __u8 huion_tablet_rdesc_fixed[] = {
> -       0x05, 0x0D,         /*  Usage Page (Digitizer),             */
> -       0x09, 0x02,         /*  Usage (Pen),                        */
> -       0xA1, 0x01,         /*  Collection (Application),           */
> -       0x85, 0x07,         /*      Report ID (7),                  */
> -       0x09, 0x20,         /*      Usage (Stylus),                 */
> -       0xA0,               /*      Collection (Physical),          */
> -       0x14,               /*          Logical Minimum (0),        */
> -       0x25, 0x01,         /*          Logical Maximum (1),        */
> -       0x75, 0x01,         /*          Report Size (1),            */
> -       0x09, 0x42,         /*          Usage (Tip Switch),         */
> -       0x09, 0x44,         /*          Usage (Barrel Switch),      */
> -       0x09, 0x46,         /*          Usage (Tablet Pick),        */
> -       0x95, 0x03,         /*          Report Count (3),           */
> -       0x81, 0x02,         /*          Input (Variable),           */
> -       0x95, 0x03,         /*          Report Count (3),           */
> -       0x81, 0x03,         /*          Input (Constant, Variable), */
> -       0x09, 0x32,         /*          Usage (In Range),           */
> -       0x95, 0x01,         /*          Report Count (1),           */
> -       0x81, 0x02,         /*          Input (Variable),           */
> -       0x95, 0x01,         /*          Report Count (1),           */
> -       0x81, 0x03,         /*          Input (Constant, Variable), */
> -       0x75, 0x10,         /*          Report Size (16),           */
> -       0x95, 0x01,         /*          Report Count (1),           */
> -       0xA4,               /*          Push,                       */
> -       0x05, 0x01,         /*          Usage Page (Desktop),       */
> -       0x65, 0x13,         /*          Unit (Inch),                */
> -       0x55, 0xFD,         /*          Unit Exponent (-3),         */
> -       0x34,               /*          Physical Minimum (0),       */
> -       0x09, 0x30,         /*          Usage (X),                  */
> -       0x46, 0x40, 0x1F,   /*          Physical Maximum (8000),    */
> -       0x26, 0x00, 0x7D,   /*          Logical Maximum (32000),    */
> -       0x81, 0x02,         /*          Input (Variable),           */
> -       0x09, 0x31,         /*          Usage (Y),                  */
> -       0x46, 0x88, 0x13,   /*          Physical Maximum (5000),    */
> -       0x26, 0x20, 0x4E,   /*          Logical Maximum (20000),    */
> -       0x81, 0x02,         /*          Input (Variable),           */
> -       0xB4,               /*          Pop,                        */
> -       0x09, 0x30,         /*          Usage (Tip Pressure),       */
> -       0x26, 0xFF, 0x07,   /*          Logical Maximum (2047),     */
> -       0x81, 0x02,         /*          Input (Variable),           */
> -       0xC0,               /*      End Collection,                 */
> -       0xC0                /*  End Collection                      */
> +/* Report descriptor template placeholder head */
> +#define HUION_PH_HEAD  0xFE, 0xED, 0x1D
> +
> +/* Report descriptor template placeholder IDs */
> +enum huion_ph_id {
> +       HUION_PH_ID_X_LM,
> +       HUION_PH_ID_X_PM,
> +       HUION_PH_ID_Y_LM,
> +       HUION_PH_ID_Y_PM,
> +       HUION_PH_ID_PRESSURE_LM,
> +       HUION_PH_ID_NUM
> +};
> +
> +/* Report descriptor template placeholder */
> +#define HUION_PH(_ID) HUION_PH_HEAD, HUION_PH_ID_##_ID
> +
> +/* Fixed report descriptor template */
> +static const __u8 huion_tablet_rdesc_template[] = {
> +       0x05, 0x0D,             /*  Usage Page (Digitizer),                 */
> +       0x09, 0x02,             /*  Usage (Pen),                            */
> +       0xA1, 0x01,             /*  Collection (Application),               */
> +       0x85, 0x07,             /*      Report ID (7),                      */
> +       0x09, 0x20,             /*      Usage (Stylus),                     */
> +       0xA0,                   /*      Collection (Physical),              */
> +       0x14,                   /*          Logical Minimum (0),            */
> +       0x25, 0x01,             /*          Logical Maximum (1),            */
> +       0x75, 0x01,             /*          Report Size (1),                */
> +       0x09, 0x42,             /*          Usage (Tip Switch),             */
> +       0x09, 0x44,             /*          Usage (Barrel Switch),          */
> +       0x09, 0x46,             /*          Usage (Tablet Pick),            */
> +       0x95, 0x03,             /*          Report Count (3),               */
> +       0x81, 0x02,             /*          Input (Variable),               */
> +       0x95, 0x03,             /*          Report Count (3),               */
> +       0x81, 0x03,             /*          Input (Constant, Variable),     */
> +       0x09, 0x32,             /*          Usage (In Range),               */
> +       0x95, 0x01,             /*          Report Count (1),               */
> +       0x81, 0x02,             /*          Input (Variable),               */
> +       0x95, 0x01,             /*          Report Count (1),               */
> +       0x81, 0x03,             /*          Input (Constant, Variable),     */
> +       0x75, 0x10,             /*          Report Size (16),               */
> +       0x95, 0x01,             /*          Report Count (1),               */
> +       0xA4,                   /*          Push,                           */
> +       0x05, 0x01,             /*          Usage Page (Desktop),           */
> +       0x65, 0x13,             /*          Unit (Inch),                    */
> +       0x55, 0xFD,             /*          Unit Exponent (-3),             */
> +       0x34,                   /*          Physical Minimum (0),           */
> +       0x09, 0x30,             /*          Usage (X),                      */
> +       0x27, HUION_PH(X_LM),   /*          Logical Maximum (PLACEHOLDER),  */
> +       0x47, HUION_PH(X_PM),   /*          Physical Maximum (PLACEHOLDER), */
> +       0x81, 0x02,             /*          Input (Variable),               */
> +       0x09, 0x31,             /*          Usage (Y),                      */
> +       0x27, HUION_PH(Y_LM),   /*          Logical Maximum (PLACEHOLDER),  */
> +       0x47, HUION_PH(Y_PM),   /*          Physical Maximum (PLACEHOLDER), */
> +       0x81, 0x02,             /*          Input (Variable),               */
> +       0xB4,                   /*          Pop,                            */
> +       0x09, 0x30,             /*          Usage (Tip Pressure),           */
> +       0x27,
> +       HUION_PH(PRESSURE_LM),  /*          Logical Maximum (PLACEHOLDER),  */
> +       0x81, 0x02,             /*          Input (Variable),               */
> +       0xC0,                   /*      End Collection,                     */
> +       0xC0                    /*  End Collection                          */
> +};
> +
> +/* Driver data */
> +struct huion_drvdata {
> +       __u8 *rdesc;
> +       unsigned int rsize;
>  };
>
>  static __u8 *huion_report_fixup(struct hid_device *hdev, __u8 *rdesc,
>                 unsigned int *rsize)
>  {
> +       struct huion_drvdata *drvdata = hid_get_drvdata(hdev);
>         switch (hdev->product) {
>         case USB_DEVICE_ID_HUION_TABLET:
> -               if (*rsize == HUION_TABLET_RDESC_ORIG_SIZE) {
> -                       rdesc = huion_tablet_rdesc_fixed;
> -                       *rsize = sizeof(huion_tablet_rdesc_fixed);
> +               if (drvdata->rdesc != NULL) {
> +                       rdesc = drvdata->rdesc;
> +                       *rsize = drvdata->rsize;
>                 }
>                 break;
>         }
> @@ -84,57 +106,124 @@ static __u8 *huion_report_fixup(struct hid_device *hdev, __u8 *rdesc,
>  }
>
>  /**
> - * Enable fully-functional tablet mode by reading special string
> - * descriptor.
> + * Enable fully-functional tablet mode and determine device parameters.
>   *
>   * @hdev:      HID device
> - *
> - * The specific string descriptor and data were discovered by sniffing
> - * the Windows driver traffic.
>   */
>  static int huion_tablet_enable(struct hid_device *hdev)
>  {
>         int rc;
> -       char buf[22];
> +       struct usb_device *usb_dev = hid_to_usb_dev(hdev);
> +       struct huion_drvdata *drvdata = hid_get_drvdata(hdev);
> +       u16 buf[6];
>
> -       rc = usb_string(hid_to_usb_dev(hdev), 0x64, buf, sizeof(buf));
> -       if (rc < 0)
> -               return rc;
> +       /*
> +        * Read string descriptor containing tablet parameters. The specific
> +        * string descriptor and data were discovered by sniffing the Windows
> +        * driver traffic.
> +        * NOTE: This enables fully-functional tablet mode.
> +        */
> +       rc = usb_control_msg(usb_dev, usb_rcvctrlpipe(usb_dev, 0),
> +                               USB_REQ_GET_DESCRIPTOR, USB_DIR_IN,
> +                               (USB_DT_STRING << 8) + 0x64,
> +                               0x0409, buf, sizeof(buf),
> +                               USB_CTRL_GET_TIMEOUT);

Just out of curiosity, you are replacing usb_string() by
usb_control_msg(). They both seem to do the same, so why bother
changing the call?

> +       if (rc == -EPIPE)
> +               hid_warn(hdev, "device parameters not found\n");
> +       else if (rc < 0)
> +               hid_warn(hdev, "failed to get device parameters: %d\n", rc);
> +       else if (rc != sizeof(buf))
> +               hid_warn(hdev, "invalid device parameters\n");
> +       else {
> +               s32 params[HUION_PH_ID_NUM];
> +               s32 resolution;
> +               __u8 *p;
> +               s32 v;
> +
> +               /* Extract device parameters */
> +               params[HUION_PH_ID_X_LM] = le16_to_cpu(buf[1]);
> +               params[HUION_PH_ID_Y_LM] = le16_to_cpu(buf[2]);
> +               params[HUION_PH_ID_PRESSURE_LM] = le16_to_cpu(buf[4]);
> +               resolution = le16_to_cpu(buf[5]);
> +               if (resolution == 0) {
> +                       params[HUION_PH_ID_X_PM] = 0;
> +                       params[HUION_PH_ID_Y_PM] = 0;

This is not good (not your code I mean). I know OEMs tend to not care
about resolution, but Wayland will for the tablet protocol. The idea
will be to report the coordinate in mm so that we can have a constant
reporting across vendors/products.

Did you ever fall in this case? and if so, isn't there any way of
retrieving the actual resolution or an approximation?

> +               } else {
> +                       params[HUION_PH_ID_X_PM] = params[HUION_PH_ID_X_LM] *
> +                                                       1000 / resolution;
> +                       params[HUION_PH_ID_Y_PM] = params[HUION_PH_ID_Y_LM] *
> +                                                       1000 / resolution;
> +               }
> +
> +               /* Allocate fixed report descriptor */
> +               drvdata->rdesc = devm_kmalloc(&hdev->dev,
> +                                       sizeof(huion_tablet_rdesc_template),
> +                                       GFP_KERNEL);
> +               if (drvdata->rdesc == NULL) {
> +                       hid_err(hdev, "failed to allocate fixed rdesc\n");
> +                       return -ENOMEM;
> +               }
> +               drvdata->rsize = sizeof(huion_tablet_rdesc_template);
> +
> +               /* Format fixed report descriptor */
> +               memcpy(drvdata->rdesc, huion_tablet_rdesc_template,
> +                       drvdata->rsize);
> +               for (p = drvdata->rdesc;
> +                    p <= drvdata->rdesc + drvdata->rsize - 4;) {
> +                       if (p[0] == 0xFE && p[1] == 0xED && p[2] == 0x1D &&
> +                           p[3] < sizeof(params)) {
> +                               v = params[p[3]];
> +                               put_unaligned(cpu_to_le32(v), (s32 *)p);
> +                               p += 4;
> +                       } else {
> +                               p++;
> +                       }
> +               }
> +       }
>
>         return 0;
>  }
>
>  static int huion_probe(struct hid_device *hdev, const struct hid_device_id *id)
>  {
> -       int ret;
> -
> -       ret = hid_parse(hdev);
> -       if (ret) {
> -               hid_err(hdev, "parse failed\n");
> -               goto err;
> -       }
> +       int rc;
> +       struct usb_interface *intf = to_usb_interface(hdev->dev.parent);
> +       struct huion_drvdata *drvdata;
>
> -       ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
> -       if (ret) {
> -               hid_err(hdev, "hw start failed\n");
> -               goto err;
> +       /* Allocate and assign driver data */
> +       drvdata = devm_kzalloc(&hdev->dev, sizeof(*drvdata), GFP_KERNEL);
> +       if (drvdata == NULL) {
> +               hid_err(hdev, "failed to allocate driver data\n");
> +               return -ENOMEM;
>         }
> +       hid_set_drvdata(hdev, drvdata);
>
>         switch (id->product) {
>         case USB_DEVICE_ID_HUION_TABLET:
> -               ret = huion_tablet_enable(hdev);
> -               if (ret) {
> -                       hid_err(hdev, "tablet enabling failed\n");
> -                       goto enabling_err;
> +               /* If this is the pen interface */
> +               if (intf->cur_altsetting->desc.bInterfaceNumber == 0) {
> +                       rc = huion_tablet_enable(hdev);
> +                       if (rc) {
> +                               hid_err(hdev, "tablet enabling failed\n");
> +                               return rc;
> +                       }
>                 }
>                 break;
>         }
>
> +       rc = hid_parse(hdev);
> +       if (rc) {
> +               hid_err(hdev, "parse failed\n");
> +               return rc;
> +       }
> +
> +       rc = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
> +       if (rc) {
> +               hid_err(hdev, "hw start failed\n");
> +               return rc;
> +       }
> +
>         return 0;
> -enabling_err:
> -       hid_hw_stop(hdev);
> -err:
> -       return ret;
>  }
>
>  static int huion_raw_event(struct hid_device *hdev, struct hid_report *report,
> --
> 2.0.1
>


The rest looks fair enough, so even with my questions:
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

Cheers,
Benjamin

^ permalink raw reply

* Re: [PATCH 3/5] hid: huion: Don't ignore other interfaces
From: Benjamin Tissoires @ 2014-07-23 14:43 UTC (permalink / raw)
  To: Nikolai Kondrashov; +Cc: Jiri Kosina, linux-input, DIGImend-devel
In-Reply-To: <1406119378-24551-4-git-send-email-spbnick@gmail.com>

On Wed, Jul 23, 2014 at 8:42 AM, Nikolai Kondrashov <spbnick@gmail.com> wrote:
> Don't ignore non pen-reporting interfaces as they may be used by some
> models reusing the same product ID.
>
> Signed-off-by: Nikolai Kondrashov <spbnick@gmail.com>
> ---

Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

>  drivers/hid/hid-huion.c | 16 ++++------------
>  1 file changed, 4 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/hid/hid-huion.c b/drivers/hid/hid-huion.c
> index db24472..e5f1e22 100644
> --- a/drivers/hid/hid-huion.c
> +++ b/drivers/hid/hid-huion.c
> @@ -107,17 +107,6 @@ static int huion_tablet_enable(struct hid_device *hdev)
>  static int huion_probe(struct hid_device *hdev, const struct hid_device_id *id)
>  {
>         int ret;
> -       struct usb_interface *intf = to_usb_interface(hdev->dev.parent);
> -
> -       /* Ignore interfaces 1 (mouse) and 2 (keyboard) for tablet,
> -        * as they are not used
> -        */
> -       switch (id->product) {
> -       case USB_DEVICE_ID_HUION_TABLET:
> -               if (intf->cur_altsetting->desc.bInterfaceNumber != 0x00)
> -                       return -ENODEV;
> -               break;
> -       }
>
>         ret = hid_parse(hdev);
>         if (ret) {
> @@ -151,10 +140,13 @@ err:
>  static int huion_raw_event(struct hid_device *hdev, struct hid_report *report,
>                         u8 *data, int size)
>  {
> +       struct usb_interface *intf = to_usb_interface(hdev->dev.parent);
> +
>         switch (hdev->product) {
>         case USB_DEVICE_ID_HUION_TABLET:
>                 /* If this is a pen input report */
> -               if (report->type == HID_INPUT_REPORT &&
> +               if (intf->cur_altsetting->desc.bInterfaceNumber == 0 &&
> +                   report->type == HID_INPUT_REPORT &&
>                     report->id == 0x07 && size >= 2)
>                         /* Invert the in-range bit */
>                         data[1] ^= 0x40;
> --
> 2.0.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 5/5] hid: huion: Handle tablets with UC-Logic vendor ID
From: Benjamin Tissoires @ 2014-07-23 14:43 UTC (permalink / raw)
  To: Nikolai Kondrashov; +Cc: Jiri Kosina, linux-input, DIGImend-devel
In-Reply-To: <1406119378-24551-6-git-send-email-spbnick@gmail.com>

On Wed, Jul 23, 2014 at 8:42 AM, Nikolai Kondrashov <spbnick@gmail.com> wrote:
> Add handling of Huion tablets with UC-Logic vendor ID, but the same
> product ID to hid-huion driver. A Huion H580 was seen using it.
>
> Signed-off-by: Nikolai Kondrashov <spbnick@gmail.com>
> ---

Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

>  drivers/hid/hid-huion.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/hid/hid-huion.c b/drivers/hid/hid-huion.c
> index e6f5338..1006c2a 100644
> --- a/drivers/hid/hid-huion.c
> +++ b/drivers/hid/hid-huion.c
> @@ -247,6 +247,7 @@ static int huion_raw_event(struct hid_device *hdev, struct hid_report *report,
>
>  static const struct hid_device_id huion_devices[] = {
>         { HID_USB_DEVICE(USB_VENDOR_ID_HUION, USB_DEVICE_ID_HUION_TABLET) },
> +       { HID_USB_DEVICE(USB_VENDOR_ID_UCLOGIC, USB_DEVICE_ID_HUION_TABLET) },
>         { }
>  };
>  MODULE_DEVICE_TABLE(hid, huion_devices);
> --
> 2.0.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 4/5] hid: huion: Switch to generating report descriptor
From: Nikolai Kondrashov @ 2014-07-23 14:59 UTC (permalink / raw)
  To: Benjamin Tissoires; +Cc: Jiri Kosina, linux-input, DIGImend-devel
In-Reply-To: <CAN+gG=FapAjC3C5oMg2UvpAvXN6pzuG5ovjfXPygGorjPjTKcw@mail.gmail.com>

On 07/23/2014 05:42 PM, Benjamin Tissoires wrote:
> On Wed, Jul 23, 2014 at 8:42 AM, Nikolai Kondrashov <spbnick@gmail.com> wrote:
>> Switch to generating tablet pen report descriptor from a template and
>> parameters retrieved from string descriptor 0x64.
>>
>> Signed-off-by: Nikolai Kondrashov <spbnick@gmail.com>
>> ---
>
> This is a nice solution.

Thanks, Benjamin :)

>> -       rc = usb_string(hid_to_usb_dev(hdev), 0x64, buf, sizeof(buf));
>> -       if (rc < 0)
>> -               return rc;
>> +       /*
>> +        * Read string descriptor containing tablet parameters. The specific
>> +        * string descriptor and data were discovered by sniffing the Windows
>> +        * driver traffic.
>> +        * NOTE: This enables fully-functional tablet mode.
>> +        */
>> +       rc = usb_control_msg(usb_dev, usb_rcvctrlpipe(usb_dev, 0),
>> +                               USB_REQ_GET_DESCRIPTOR, USB_DIR_IN,
>> +                               (USB_DT_STRING << 8) + 0x64,
>> +                               0x0409, buf, sizeof(buf),
>> +                               USB_CTRL_GET_TIMEOUT);
>
> Just out of curiosity, you are replacing usb_string() by
> usb_control_msg(). They both seem to do the same, so why bother
> changing the call?

Well, actually they don't seem to do the same and the main difference is that
usb_string attempts to convert the data from UTF-16LE to UTF-8, which would be
undesirable for the binary contents we expect there.

>> +               resolution = le16_to_cpu(buf[5]);
>> +               if (resolution == 0) {
>> +                       params[HUION_PH_ID_X_PM] = 0;
>> +                       params[HUION_PH_ID_Y_PM] = 0;
>
> This is not good (not your code I mean). I know OEMs tend to not care
> about resolution, but Wayland will for the tablet protocol. The idea
> will be to report the coordinate in mm so that we can have a constant
> reporting across vendors/products.
>
> Did you ever fall in this case? and if so, isn't there any way of
> retrieving the actual resolution or an approximation?

Thankfully not. This is merely a protection against division-by-zero
exception in case they start doing that.

BTW, it's nice that Wayland tries to actually use the resolution.

> The rest looks fair enough, so even with my questions:
> Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

Thanks for the review, Benjamin!

Will send a new version soon.

Sincerely,
Nick

^ permalink raw reply

* Re: [PATCH 04/15] Input: atmel_mxt_ts - implement device tree support
From: Nick Dyer @ 2014-07-23 15:13 UTC (permalink / raw)
  To: Stephen Warren
  Cc: Dmitry Torokhov, Yufeng Shen, Daniel Kurtz, Henrik Rydberg,
	Joonyoung Shim, Alan Bowens, linux-input, linux-kernel,
	Peter Meerwald, Benson Leung, Olof Johansson, Sekhar Nori,
	Stephen Warren
In-Reply-To: <53CECB6C.2030207@wwwdotorg.org>

On 22/07/14 21:37, Stephen Warren wrote:
> On 07/03/2014 09:01 AM, nick.dyer@itdev.co.uk wrote:
>> From: Stephen Warren <swarren@nvidia.com>
> 
>> diff --git a/Documentation/devicetree/bindings/input/atmel,maxtouch.txt b/Documentation/devicetree/bindings/input/atmel,maxtouch.txt
> 
>> +	touch@4b {
>> +		compatible = "atmel,maxtouch";
>> +		reg = <0x4b>;
>> +		interrupt-parent = <&gpio>;
>> +		interrupts = <TEGRA_GPIO(W, 3) GPIO_ACTIVE_HIGH>;
> 
> Oops. s/GPIO_ACTIVE_HIGH/IRQ_TYPE_LEVEL_LOW/ there; I evidently typed
> something stupid into the doc when I wrote it!

OK, fixed this. I thought it was some tegra quirk I wasn't aware of!

^ permalink raw reply

* Re: [PATCH 00/15] atmel_mxt_ts - device tree, bootloader, etc
From: Nick Dyer @ 2014-07-23 15:30 UTC (permalink / raw)
  To: Stephen Warren
  Cc: Dmitry Torokhov, Yufeng Shen, Daniel Kurtz, Henrik Rydberg,
	Joonyoung Shim, Alan Bowens, linux-input, linux-kernel,
	Peter Meerwald, Benson Leung, Olof Johansson, Sekhar Nori
In-Reply-To: <53CECAEC.8080905@wwwdotorg.org>

On 22/07/14 21:34, Stephen Warren wrote:
> Unfortunately, I still can't get these to work on my system.
> 
> Per your "Re: atmel_mxt_ts: defaulting irqflags to
> IRQF_TRIGGER_FALLING", I set up the IRQ type in the Tegra DT file, and
> then applied this series on top of next-20140721. The driver appears to
> initialize OK, but neither X nor evtest see any events from the device.
> The IRQ count in /proc/interrupts doesn't increase when I touch the
> touchpad, but does when I press it hard enough to trigger the physical
> button.

You're using the T19/GPIO support, then? In which case, there appears to be
something wrong on the touch controller rather than the driver itself.

> A boot log with debug enabled follows. No additional kernel log
> messages are generated by touches or clicks.

Perhaps I should add some debug to mxt_input_button() - currently it will
not debug the fact that a click is received, although I guess that you will
see it in getevent.

> Do you have any idea what I should try?

I am suspicious that it may be that the power sequencing isn't quite right,
which sometimes leads to parts of the chip not working properly (eg GPIO
buttons working, but no touch).

The patch "use deep sleep when stopped" removes the reset on every resume
(which would otherwise kill resume performance). But that reset tends to
paper over a device which hasn't been powered up properly in the first place.

Could you try issuing a manual reset and see if the touch starts working? I
would normally do this by compiling our obp-utils software from
https://github.com/atmel-maxtouch/obp-utils using ndk-build and doing
something like:

mxt-app -d i2c-dev:1-004b --reset

(you need CONFIG_I2C_DEBUG to make /dev/i2c-1 appear)

If this works, then we need to adjust the board file that powers it on, the
correct power on sequence is contained in this later patch:
https://github.com/ndyer/linux/commit/97e938fa5863

>> [    1.831998] atmel_mxt_ts 1-004b: Interrupt triggered but zero messages

FWIW, this warning generally means you should be using the CHG line mode 1
in T18 COMMSCONFIG. It's benign, though.

^ permalink raw reply

* [PATCHES v2] Add support for more Huion tablets
From: Nikolai Kondrashov @ 2014-07-23 16:31 UTC (permalink / raw)
  To: Jiri Kosina; +Cc: linux-input, DIGImend-devel
In-Reply-To: <53CFC950.90703@gmail.com>

Hi everyone,

This is the second version of the patch set adding support for more Huion
tablets [1].

This version has the "hid: huion: Invert in-range on specific product" patch
removed as requested by Benjamin Tissoires.

Tested with Huion H610N.

Nick

[PATCH 1/4] hid: huion: Use "tablet" instead of specific model
[PATCH 2/4] hid: huion: Don't ignore other interfaces
[PATCH 3/4] hid: huion: Switch to generating report descriptor
[PATCH 4/4] hid: huion: Handle tablets with UC-Logic vendor ID

 drivers/hid/hid-core.c  |   2 +-
 drivers/hid/hid-huion.c | 265 ++++++++++++++++++++++++++++++++----------------
 drivers/hid/hid-ids.h   |   2 +-
 3 files changed, 177 insertions(+), 92 deletions(-)

[1] http://thread.gmane.org/gmane.linux.kernel.input/37187

^ permalink raw reply

* [PATCH 1/4] hid: huion: Use "tablet" instead of specific model
From: Nikolai Kondrashov @ 2014-07-23 16:31 UTC (permalink / raw)
  To: Jiri Kosina; +Cc: linux-input, DIGImend-devel, Nikolai Kondrashov
In-Reply-To: <1406133117-29243-1-git-send-email-spbnick@gmail.com>

Use word "tablet" in identifiers and comments instead of referring to specific
Huion tablet model name, as the product ID is used by at least several tablet
models.

Signed-off-by: Nikolai Kondrashov <spbnick@gmail.com>
---
 drivers/hid/hid-core.c  |  2 +-
 drivers/hid/hid-huion.c | 25 +++++++++++++------------
 drivers/hid/hid-ids.h   |  2 +-
 3 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 8ed66fd..84ed7d4 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1782,7 +1782,7 @@ static const struct hid_device_id hid_have_special_driver[] = {
 	{ HID_USB_DEVICE(USB_VENDOR_ID_HOLTEK_ALT, USB_DEVICE_ID_HOLTEK_ALT_MOUSE_A070) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_HOLTEK_ALT, USB_DEVICE_ID_HOLTEK_ALT_MOUSE_A072) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_HOLTEK_ALT, USB_DEVICE_ID_HOLTEK_ALT_MOUSE_A081) },
-	{ HID_USB_DEVICE(USB_VENDOR_ID_HUION, USB_DEVICE_ID_HUION_580) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_HUION, USB_DEVICE_ID_HUION_TABLET) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_JESS2, USB_DEVICE_ID_JESS2_COLOR_RUMBLE_PAD) },
 	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_ION, USB_DEVICE_ID_ICADE) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_KENSINGTON, USB_DEVICE_ID_KS_SLIMBLADE) },
diff --git a/drivers/hid/hid-huion.c b/drivers/hid/hid-huion.c
index cbf4da4..25d01cd 100644
--- a/drivers/hid/hid-huion.c
+++ b/drivers/hid/hid-huion.c
@@ -2,6 +2,7 @@
  *  HID driver for Huion devices not fully compliant with HID standard
  *
  *  Copyright (c) 2013 Martin Rusko
+ *  Copyright (c) 2014 Nikolai Kondrashov
  */
 
 /*
@@ -19,11 +20,11 @@
 
 #include "hid-ids.h"
 
-/* Original Huion 580 report descriptor size */
-#define HUION_580_RDESC_ORIG_SIZE	177
+/* Original tablet report descriptor size */
+#define HUION_TABLET_RDESC_ORIG_SIZE	177
 
-/* Fixed Huion 580 report descriptor */
-static __u8 huion_580_rdesc_fixed[] = {
+/* Fixed tablet report descriptor */
+static __u8 huion_tablet_rdesc_fixed[] = {
 	0x05, 0x0D,         /*  Usage Page (Digitizer),             */
 	0x09, 0x02,         /*  Usage (Pen),                        */
 	0xA1, 0x01,         /*  Collection (Application),           */
@@ -72,10 +73,10 @@ static __u8 *huion_report_fixup(struct hid_device *hdev, __u8 *rdesc,
 		unsigned int *rsize)
 {
 	switch (hdev->product) {
-	case USB_DEVICE_ID_HUION_580:
-		if (*rsize == HUION_580_RDESC_ORIG_SIZE) {
-			rdesc = huion_580_rdesc_fixed;
-			*rsize = sizeof(huion_580_rdesc_fixed);
+	case USB_DEVICE_ID_HUION_TABLET:
+		if (*rsize == HUION_TABLET_RDESC_ORIG_SIZE) {
+			rdesc = huion_tablet_rdesc_fixed;
+			*rsize = sizeof(huion_tablet_rdesc_fixed);
 		}
 		break;
 	}
@@ -108,11 +109,11 @@ static int huion_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	int ret;
 	struct usb_interface *intf = to_usb_interface(hdev->dev.parent);
 
-	/* Ignore interfaces 1 (mouse) and 2 (keyboard) for Huion 580 tablet,
+	/* Ignore interfaces 1 (mouse) and 2 (keyboard) for tablet,
 	 * as they are not used
 	 */
 	switch (id->product) {
-	case USB_DEVICE_ID_HUION_580:
+	case USB_DEVICE_ID_HUION_TABLET:
 		if (intf->cur_altsetting->desc.bInterfaceNumber != 0x00)
 			return -ENODEV;
 		break;
@@ -131,7 +132,7 @@ static int huion_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	}
 
 	switch (id->product) {
-	case USB_DEVICE_ID_HUION_580:
+	case USB_DEVICE_ID_HUION_TABLET:
 		ret = huion_tablet_enable(hdev);
 		if (ret) {
 			hid_err(hdev, "tablet enabling failed\n");
@@ -158,7 +159,7 @@ static int huion_raw_event(struct hid_device *hdev, struct hid_report *report,
 }
 
 static const struct hid_device_id huion_devices[] = {
-	{ HID_USB_DEVICE(USB_VENDOR_ID_HUION, USB_DEVICE_ID_HUION_580) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_HUION, USB_DEVICE_ID_HUION_TABLET) },
 	{ }
 };
 MODULE_DEVICE_TABLE(hid, huion_devices);
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 48b66bb..fe2f163 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -448,7 +448,7 @@
 #define USB_DEVICE_ID_UGCI_FIGHTING	0x0030
 
 #define USB_VENDOR_ID_HUION		0x256c
-#define USB_DEVICE_ID_HUION_580		0x006e
+#define USB_DEVICE_ID_HUION_TABLET	0x006e
 
 #define USB_VENDOR_ID_IDEACOM		0x1cb6
 #define USB_DEVICE_ID_IDEACOM_IDC6650	0x6650
-- 
2.0.1


^ permalink raw reply related

* [PATCH 2/4] hid: huion: Don't ignore other interfaces
From: Nikolai Kondrashov @ 2014-07-23 16:31 UTC (permalink / raw)
  To: Jiri Kosina; +Cc: linux-input, DIGImend-devel, Nikolai Kondrashov
In-Reply-To: <1406133117-29243-1-git-send-email-spbnick@gmail.com>

Don't ignore non pen-reporting interfaces as they may be used by some
models reusing the same product ID.

Signed-off-by: Nikolai Kondrashov <spbnick@gmail.com>
---
 drivers/hid/hid-huion.c | 20 +++++++-------------
 1 file changed, 7 insertions(+), 13 deletions(-)

diff --git a/drivers/hid/hid-huion.c b/drivers/hid/hid-huion.c
index 25d01cd..46c425b 100644
--- a/drivers/hid/hid-huion.c
+++ b/drivers/hid/hid-huion.c
@@ -107,17 +107,6 @@ static int huion_tablet_enable(struct hid_device *hdev)
 static int huion_probe(struct hid_device *hdev, const struct hid_device_id *id)
 {
 	int ret;
-	struct usb_interface *intf = to_usb_interface(hdev->dev.parent);
-
-	/* Ignore interfaces 1 (mouse) and 2 (keyboard) for tablet,
-	 * as they are not used
-	 */
-	switch (id->product) {
-	case USB_DEVICE_ID_HUION_TABLET:
-		if (intf->cur_altsetting->desc.bInterfaceNumber != 0x00)
-			return -ENODEV;
-		break;
-	}
 
 	ret = hid_parse(hdev);
 	if (ret) {
@@ -151,8 +140,13 @@ err:
 static int huion_raw_event(struct hid_device *hdev, struct hid_report *report,
 			u8 *data, int size)
 {
-	/* If this is a pen input report then invert the in-range bit */
-	if (report->type == HID_INPUT_REPORT && report->id == 0x07 && size >= 2)
+	struct usb_interface *intf = to_usb_interface(hdev->dev.parent);
+
+	/* If this is a pen input report */
+	if (intf->cur_altsetting->desc.bInterfaceNumber == 0 &&
+	    report->type == HID_INPUT_REPORT &&
+	    report->id == 0x07 && size >= 2)
+		/* Invert the in-range bit */
 		data[1] ^= 0x40;
 
 	return 0;
-- 
2.0.1


^ permalink raw reply related

* [PATCH 3/4] hid: huion: Switch to generating report descriptor
From: Nikolai Kondrashov @ 2014-07-23 16:31 UTC (permalink / raw)
  To: Jiri Kosina; +Cc: linux-input, DIGImend-devel, Nikolai Kondrashov
In-Reply-To: <1406133117-29243-1-git-send-email-spbnick@gmail.com>

Switch to generating tablet pen report descriptor from a template and
parameters retrieved from string descriptor 0x64.

Signed-off-by: Nikolai Kondrashov <spbnick@gmail.com>
---
 drivers/hid/hid-huion.c | 245 +++++++++++++++++++++++++++++++++---------------
 1 file changed, 167 insertions(+), 78 deletions(-)

diff --git a/drivers/hid/hid-huion.c b/drivers/hid/hid-huion.c
index 46c425b..6c811c1 100644
--- a/drivers/hid/hid-huion.c
+++ b/drivers/hid/hid-huion.c
@@ -16,67 +16,89 @@
 #include <linux/hid.h>
 #include <linux/module.h>
 #include <linux/usb.h>
+#include <asm/unaligned.h>
 #include "usbhid/usbhid.h"
 
 #include "hid-ids.h"
 
-/* Original tablet report descriptor size */
-#define HUION_TABLET_RDESC_ORIG_SIZE	177
-
-/* Fixed tablet report descriptor */
-static __u8 huion_tablet_rdesc_fixed[] = {
-	0x05, 0x0D,         /*  Usage Page (Digitizer),             */
-	0x09, 0x02,         /*  Usage (Pen),                        */
-	0xA1, 0x01,         /*  Collection (Application),           */
-	0x85, 0x07,         /*      Report ID (7),                  */
-	0x09, 0x20,         /*      Usage (Stylus),                 */
-	0xA0,               /*      Collection (Physical),          */
-	0x14,               /*          Logical Minimum (0),        */
-	0x25, 0x01,         /*          Logical Maximum (1),        */
-	0x75, 0x01,         /*          Report Size (1),            */
-	0x09, 0x42,         /*          Usage (Tip Switch),         */
-	0x09, 0x44,         /*          Usage (Barrel Switch),      */
-	0x09, 0x46,         /*          Usage (Tablet Pick),        */
-	0x95, 0x03,         /*          Report Count (3),           */
-	0x81, 0x02,         /*          Input (Variable),           */
-	0x95, 0x03,         /*          Report Count (3),           */
-	0x81, 0x03,         /*          Input (Constant, Variable), */
-	0x09, 0x32,         /*          Usage (In Range),           */
-	0x95, 0x01,         /*          Report Count (1),           */
-	0x81, 0x02,         /*          Input (Variable),           */
-	0x95, 0x01,         /*          Report Count (1),           */
-	0x81, 0x03,         /*          Input (Constant, Variable), */
-	0x75, 0x10,         /*          Report Size (16),           */
-	0x95, 0x01,         /*          Report Count (1),           */
-	0xA4,               /*          Push,                       */
-	0x05, 0x01,         /*          Usage Page (Desktop),       */
-	0x65, 0x13,         /*          Unit (Inch),                */
-	0x55, 0xFD,         /*          Unit Exponent (-3),         */
-	0x34,               /*          Physical Minimum (0),       */
-	0x09, 0x30,         /*          Usage (X),                  */
-	0x46, 0x40, 0x1F,   /*          Physical Maximum (8000),    */
-	0x26, 0x00, 0x7D,   /*          Logical Maximum (32000),    */
-	0x81, 0x02,         /*          Input (Variable),           */
-	0x09, 0x31,         /*          Usage (Y),                  */
-	0x46, 0x88, 0x13,   /*          Physical Maximum (5000),    */
-	0x26, 0x20, 0x4E,   /*          Logical Maximum (20000),    */
-	0x81, 0x02,         /*          Input (Variable),           */
-	0xB4,               /*          Pop,                        */
-	0x09, 0x30,         /*          Usage (Tip Pressure),       */
-	0x26, 0xFF, 0x07,   /*          Logical Maximum (2047),     */
-	0x81, 0x02,         /*          Input (Variable),           */
-	0xC0,               /*      End Collection,                 */
-	0xC0                /*  End Collection                      */
+/* Report descriptor template placeholder head */
+#define HUION_PH_HEAD	0xFE, 0xED, 0x1D
+
+/* Report descriptor template placeholder IDs */
+enum huion_ph_id {
+	HUION_PH_ID_X_LM,
+	HUION_PH_ID_X_PM,
+	HUION_PH_ID_Y_LM,
+	HUION_PH_ID_Y_PM,
+	HUION_PH_ID_PRESSURE_LM,
+	HUION_PH_ID_NUM
+};
+
+/* Report descriptor template placeholder */
+#define HUION_PH(_ID) HUION_PH_HEAD, HUION_PH_ID_##_ID
+
+/* Fixed report descriptor template */
+static const __u8 huion_tablet_rdesc_template[] = {
+	0x05, 0x0D,             /*  Usage Page (Digitizer),                 */
+	0x09, 0x02,             /*  Usage (Pen),                            */
+	0xA1, 0x01,             /*  Collection (Application),               */
+	0x85, 0x07,             /*      Report ID (7),                      */
+	0x09, 0x20,             /*      Usage (Stylus),                     */
+	0xA0,                   /*      Collection (Physical),              */
+	0x14,                   /*          Logical Minimum (0),            */
+	0x25, 0x01,             /*          Logical Maximum (1),            */
+	0x75, 0x01,             /*          Report Size (1),                */
+	0x09, 0x42,             /*          Usage (Tip Switch),             */
+	0x09, 0x44,             /*          Usage (Barrel Switch),          */
+	0x09, 0x46,             /*          Usage (Tablet Pick),            */
+	0x95, 0x03,             /*          Report Count (3),               */
+	0x81, 0x02,             /*          Input (Variable),               */
+	0x95, 0x03,             /*          Report Count (3),               */
+	0x81, 0x03,             /*          Input (Constant, Variable),     */
+	0x09, 0x32,             /*          Usage (In Range),               */
+	0x95, 0x01,             /*          Report Count (1),               */
+	0x81, 0x02,             /*          Input (Variable),               */
+	0x95, 0x01,             /*          Report Count (1),               */
+	0x81, 0x03,             /*          Input (Constant, Variable),     */
+	0x75, 0x10,             /*          Report Size (16),               */
+	0x95, 0x01,             /*          Report Count (1),               */
+	0xA4,                   /*          Push,                           */
+	0x05, 0x01,             /*          Usage Page (Desktop),           */
+	0x65, 0x13,             /*          Unit (Inch),                    */
+	0x55, 0xFD,             /*          Unit Exponent (-3),             */
+	0x34,                   /*          Physical Minimum (0),           */
+	0x09, 0x30,             /*          Usage (X),                      */
+	0x27, HUION_PH(X_LM),   /*          Logical Maximum (PLACEHOLDER),  */
+	0x47, HUION_PH(X_PM),   /*          Physical Maximum (PLACEHOLDER), */
+	0x81, 0x02,             /*          Input (Variable),               */
+	0x09, 0x31,             /*          Usage (Y),                      */
+	0x27, HUION_PH(Y_LM),   /*          Logical Maximum (PLACEHOLDER),  */
+	0x47, HUION_PH(Y_PM),   /*          Physical Maximum (PLACEHOLDER), */
+	0x81, 0x02,             /*          Input (Variable),               */
+	0xB4,                   /*          Pop,                            */
+	0x09, 0x30,             /*          Usage (Tip Pressure),           */
+	0x27,
+	HUION_PH(PRESSURE_LM),  /*          Logical Maximum (PLACEHOLDER),  */
+	0x81, 0x02,             /*          Input (Variable),               */
+	0xC0,                   /*      End Collection,                     */
+	0xC0                    /*  End Collection                          */
+};
+
+/* Driver data */
+struct huion_drvdata {
+	__u8 *rdesc;
+	unsigned int rsize;
 };
 
 static __u8 *huion_report_fixup(struct hid_device *hdev, __u8 *rdesc,
 		unsigned int *rsize)
 {
+	struct huion_drvdata *drvdata = hid_get_drvdata(hdev);
 	switch (hdev->product) {
 	case USB_DEVICE_ID_HUION_TABLET:
-		if (*rsize == HUION_TABLET_RDESC_ORIG_SIZE) {
-			rdesc = huion_tablet_rdesc_fixed;
-			*rsize = sizeof(huion_tablet_rdesc_fixed);
+		if (drvdata->rdesc != NULL) {
+			rdesc = drvdata->rdesc;
+			*rsize = drvdata->rsize;
 		}
 		break;
 	}
@@ -84,57 +106,124 @@ static __u8 *huion_report_fixup(struct hid_device *hdev, __u8 *rdesc,
 }
 
 /**
- * Enable fully-functional tablet mode by reading special string
- * descriptor.
+ * Enable fully-functional tablet mode and determine device parameters.
  *
  * @hdev:	HID device
- *
- * The specific string descriptor and data were discovered by sniffing
- * the Windows driver traffic.
  */
 static int huion_tablet_enable(struct hid_device *hdev)
 {
 	int rc;
-	char buf[22];
+	struct usb_device *usb_dev = hid_to_usb_dev(hdev);
+	struct huion_drvdata *drvdata = hid_get_drvdata(hdev);
+	u16 buf[6];
 
-	rc = usb_string(hid_to_usb_dev(hdev), 0x64, buf, sizeof(buf));
-	if (rc < 0)
-		return rc;
+	/*
+	 * Read string descriptor containing tablet parameters. The specific
+	 * string descriptor and data were discovered by sniffing the Windows
+	 * driver traffic.
+	 * NOTE: This enables fully-functional tablet mode.
+	 */
+	rc = usb_control_msg(usb_dev, usb_rcvctrlpipe(usb_dev, 0),
+				USB_REQ_GET_DESCRIPTOR, USB_DIR_IN,
+				(USB_DT_STRING << 8) + 0x64,
+				0x0409, buf, sizeof(buf),
+				USB_CTRL_GET_TIMEOUT);
+	if (rc == -EPIPE)
+		hid_warn(hdev, "device parameters not found\n");
+	else if (rc < 0)
+		hid_warn(hdev, "failed to get device parameters: %d\n", rc);
+	else if (rc != sizeof(buf))
+		hid_warn(hdev, "invalid device parameters\n");
+	else {
+		s32 params[HUION_PH_ID_NUM];
+		s32 resolution;
+		__u8 *p;
+		s32 v;
+
+		/* Extract device parameters */
+		params[HUION_PH_ID_X_LM] = le16_to_cpu(buf[1]);
+		params[HUION_PH_ID_Y_LM] = le16_to_cpu(buf[2]);
+		params[HUION_PH_ID_PRESSURE_LM] = le16_to_cpu(buf[4]);
+		resolution = le16_to_cpu(buf[5]);
+		if (resolution == 0) {
+			params[HUION_PH_ID_X_PM] = 0;
+			params[HUION_PH_ID_Y_PM] = 0;
+		} else {
+			params[HUION_PH_ID_X_PM] = params[HUION_PH_ID_X_LM] *
+							1000 / resolution;
+			params[HUION_PH_ID_Y_PM] = params[HUION_PH_ID_Y_LM] *
+							1000 / resolution;
+		}
+
+		/* Allocate fixed report descriptor */
+		drvdata->rdesc = devm_kmalloc(&hdev->dev,
+					sizeof(huion_tablet_rdesc_template),
+					GFP_KERNEL);
+		if (drvdata->rdesc == NULL) {
+			hid_err(hdev, "failed to allocate fixed rdesc\n");
+			return -ENOMEM;
+		}
+		drvdata->rsize = sizeof(huion_tablet_rdesc_template);
+
+		/* Format fixed report descriptor */
+		memcpy(drvdata->rdesc, huion_tablet_rdesc_template,
+			drvdata->rsize);
+		for (p = drvdata->rdesc;
+		     p <= drvdata->rdesc + drvdata->rsize - 4;) {
+			if (p[0] == 0xFE && p[1] == 0xED && p[2] == 0x1D &&
+			    p[3] < sizeof(params)) {
+				v = params[p[3]];
+				put_unaligned(cpu_to_le32(v), (s32 *)p);
+				p += 4;
+			} else {
+				p++;
+			}
+		}
+	}
 
 	return 0;
 }
 
 static int huion_probe(struct hid_device *hdev, const struct hid_device_id *id)
 {
-	int ret;
-
-	ret = hid_parse(hdev);
-	if (ret) {
-		hid_err(hdev, "parse failed\n");
-		goto err;
-	}
+	int rc;
+	struct usb_interface *intf = to_usb_interface(hdev->dev.parent);
+	struct huion_drvdata *drvdata;
 
-	ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
-	if (ret) {
-		hid_err(hdev, "hw start failed\n");
-		goto err;
+	/* Allocate and assign driver data */
+	drvdata = devm_kzalloc(&hdev->dev, sizeof(*drvdata), GFP_KERNEL);
+	if (drvdata == NULL) {
+		hid_err(hdev, "failed to allocate driver data\n");
+		return -ENOMEM;
 	}
+	hid_set_drvdata(hdev, drvdata);
 
 	switch (id->product) {
 	case USB_DEVICE_ID_HUION_TABLET:
-		ret = huion_tablet_enable(hdev);
-		if (ret) {
-			hid_err(hdev, "tablet enabling failed\n");
-			goto enabling_err;
+		/* If this is the pen interface */
+		if (intf->cur_altsetting->desc.bInterfaceNumber == 0) {
+			rc = huion_tablet_enable(hdev);
+			if (rc) {
+				hid_err(hdev, "tablet enabling failed\n");
+				return rc;
+			}
 		}
 		break;
 	}
 
+	rc = hid_parse(hdev);
+	if (rc) {
+		hid_err(hdev, "parse failed\n");
+		return rc;
+	}
+
+	rc = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
+	if (rc) {
+		hid_err(hdev, "hw start failed\n");
+		return rc;
+	}
+
 	return 0;
-enabling_err:
-	hid_hw_stop(hdev);
-err:
-	return ret;
 }
 
 static int huion_raw_event(struct hid_device *hdev, struct hid_report *report,
-- 
2.0.1


^ permalink raw reply related

* [PATCH 4/4] hid: huion: Handle tablets with UC-Logic vendor ID
From: Nikolai Kondrashov @ 2014-07-23 16:31 UTC (permalink / raw)
  To: Jiri Kosina; +Cc: linux-input, DIGImend-devel, Nikolai Kondrashov
In-Reply-To: <1406133117-29243-1-git-send-email-spbnick@gmail.com>

Add handling of Huion tablets with UC-Logic vendor ID, but the same
product ID to hid-huion driver. A Huion H580 was seen using it.

Signed-off-by: Nikolai Kondrashov <spbnick@gmail.com>
---
 drivers/hid/hid-huion.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/hid/hid-huion.c b/drivers/hid/hid-huion.c
index 6c811c1..438b54e 100644
--- a/drivers/hid/hid-huion.c
+++ b/drivers/hid/hid-huion.c
@@ -243,6 +243,7 @@ static int huion_raw_event(struct hid_device *hdev, struct hid_report *report,
 
 static const struct hid_device_id huion_devices[] = {
 	{ HID_USB_DEVICE(USB_VENDOR_ID_HUION, USB_DEVICE_ID_HUION_TABLET) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_UCLOGIC, USB_DEVICE_ID_HUION_TABLET) },
 	{ }
 };
 MODULE_DEVICE_TABLE(hid, huion_devices);
-- 
2.0.1


^ permalink raw reply related

* Re: [PATCH 00/15] atmel_mxt_ts - device tree, bootloader, etc
From: Stephen Warren @ 2014-07-23 17:22 UTC (permalink / raw)
  To: Nick Dyer
  Cc: Dmitry Torokhov, Yufeng Shen, Daniel Kurtz, Henrik Rydberg,
	Joonyoung Shim, Alan Bowens, linux-input, linux-kernel,
	Peter Meerwald, Benson Leung, Olof Johansson, Sekhar Nori
In-Reply-To: <53CFD50C.4040509@itdev.co.uk>

On 07/23/2014 09:30 AM, Nick Dyer wrote:
> On 22/07/14 21:34, Stephen Warren wrote:
>> Unfortunately, I still can't get these to work on my system.
>>
>> Per your "Re: atmel_mxt_ts: defaulting irqflags to
>> IRQF_TRIGGER_FALLING", I set up the IRQ type in the Tegra DT file, and
>> then applied this series on top of next-20140721. The driver appears to
>> initialize OK, but neither X nor evtest see any events from the device.
>> The IRQ count in /proc/interrupts doesn't increase when I touch the
>> touchpad, but does when I press it hard enough to trigger the physical
>> button.
> 
> You're using the T19/GPIO support, then? In which case, there appears to be
> something wrong on the touch controller rather than the driver itself.

I assume I'm using T19, since there's a physical click action on the
touchpad along with the normal touch detection.

>> A boot log with debug enabled follows. No additional kernel log
>> messages are generated by touches or clicks.
> 
> Perhaps I should add some debug to mxt_input_button() - currently it will
> not debug the fact that a click is received, although I guess that you will
> see it in getevent.
> 
>> Do you have any idea what I should try?
> 
> I am suspicious that it may be that the power sequencing isn't quite right,
> which sometimes leads to parts of the chip not working properly (eg GPIO
> buttons working, but no touch).
> 
> The patch "use deep sleep when stopped" removes the reset on every resume
> (which would otherwise kill resume performance). But that reset tends to
> paper over a device which hasn't been powered up properly in the first place.
> 
> Could you try issuing a manual reset and see if the touch starts working? I
> would normally do this by compiling our obp-utils software from
> https://github.com/atmel-maxtouch/obp-utils using ndk-build and doing
> something like:
> 
> mxt-app -d i2c-dev:1-004b --reset
> 
> (you need CONFIG_I2C_DEBUG to make /dev/i2c-1 appear)

That didn't make any difference.

I also tried the tool interactively. the "Display raw (M)essages" option
never displayed anything, and the couple of self-tests I tried just
timed out. "Read (I)nfo block" did display some values that seemed like
they might be correct rather than random data.

Interestingly though, I did bisect the series and found "Input:
atmel_mxt_ts - use deep sleep mode when stopped" causes the problem. If
I apply the whole series and revert that one patch, the touchpad works
for mouse movement, but interestingly not for taps or physical clicks.

...
>>> [    1.831998] atmel_mxt_ts 1-004b: Interrupt triggered but zero messages
> 
> FWIW, this warning generally means you should be using the CHG line mode 1
> in T18 COMMSCONFIG. It's benign, though.

I'm not sure how I would adjust this; all firmware/config/... was
flashed into the touchpad device itself before I received the system,
and I don't have any firmware files on my host system.

^ permalink raw reply

* Re: [PATCH v2] input: driver for touchscreen on iPaq h3xxx
From: Dmitry Torokhov @ 2014-07-23 17:32 UTC (permalink / raw)
  To: Linus Walleij; +Cc: linux-input, Dmitry Artamonow, Alessandro GARDICH
In-Reply-To: <1406103614-12438-1-git-send-email-linus.walleij@linaro.org>

Hi Linus,

On Wed, Jul 23, 2014 at 10:20:14AM +0200, Linus Walleij wrote:
> From: Dmitry Artamonow <mad_soft@inbox.ru>
> 
> This adds a driver for the touchscreen connected to the
> Atmel microcontroller on the iPAQ h3xxx series.
> 
> Based on a driver from handhelds.org 2.6.21 kernel, written
> by Alessandro GARDICH, with the bulk of the code for the
> new input architecture rewritten by Dmitry Atamonow, and
> the final polish by Linus Walleij.
> 
> Signed-off-by: Alessandro GARDICH <gremlin@gremlin.it>
> Signed-off-by: Dmitry Artamonow <mad_soft@inbox.ru>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> ChangeLog v1->v2:
> - More verbose Kconfig selection text including module info.
> - Use be16_to_cpup() to deserialize protocol.
> - Cut fake pressure event reporting.
> - Report button events properly.
> - Use devm_input_allocate_device() to allocate input device.
> - Use input_set_capability() to set up capabilities.
> ---
>  drivers/input/touchscreen/Kconfig         |  12 +++
>  drivers/input/touchscreen/Makefile        |   1 +
>  drivers/input/touchscreen/ipaq-micro-ts.c | 141 ++++++++++++++++++++++++++++++
>  3 files changed, 154 insertions(+)
>  create mode 100644 drivers/input/touchscreen/ipaq-micro-ts.c
> 
> diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
> index a23a94bb4bcb..6bb9a7dd23b6 100644
> --- a/drivers/input/touchscreen/Kconfig
> +++ b/drivers/input/touchscreen/Kconfig
> @@ -471,6 +471,18 @@ config TOUCHSCREEN_HP7XX
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called jornada720_ts.
>  
> +config TOUCHSCREEN_IPAQ_MICRO
> +	tristate "HP iPAQ Atmel Micro ASIC touchscreen"
> +	depends on MFD_IPAQ_MICRO
> +	help
> +	  Say Y here to enable support for the touchscreen attached to
> +	  the Atmel Micro peripheral controller on iPAQ h3100/h3600/h3700
> +
> +	  If unsure, say N.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called ipaq-micro-ts.
> +
>  config TOUCHSCREEN_HTCPEN
>  	tristate "HTC Shift X9500 touchscreen"
>  	depends on ISA
> diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
> index 126479d8c29a..4be94fce41af 100644
> --- a/drivers/input/touchscreen/Makefile
> +++ b/drivers/input/touchscreen/Makefile
> @@ -46,6 +46,7 @@ obj-$(CONFIG_TOUCHSCREEN_MTOUCH)	+= mtouch.o
>  obj-$(CONFIG_TOUCHSCREEN_MK712)		+= mk712.o
>  obj-$(CONFIG_TOUCHSCREEN_HP600)		+= hp680_ts_input.o
>  obj-$(CONFIG_TOUCHSCREEN_HP7XX)		+= jornada720_ts.o
> +obj-$(CONFIG_TOUCHSCREEN_IPAQ_MICRO)	+= ipaq-micro-ts.o
>  obj-$(CONFIG_TOUCHSCREEN_HTCPEN)	+= htcpen.o
>  obj-$(CONFIG_TOUCHSCREEN_USB_COMPOSITE)	+= usbtouchscreen.o
>  obj-$(CONFIG_TOUCHSCREEN_PCAP)		+= pcap_ts.o
> diff --git a/drivers/input/touchscreen/ipaq-micro-ts.c b/drivers/input/touchscreen/ipaq-micro-ts.c
> new file mode 100644
> index 000000000000..d044b06b1bfb
> --- /dev/null
> +++ b/drivers/input/touchscreen/ipaq-micro-ts.c
> @@ -0,0 +1,141 @@
> +/*
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * h3600 atmel micro companion support, touchscreen subdevice
> + * Author : Alessandro Gardich <gremlin@gremlin.it>
> + * Author : Dmitry Artamonow <mad_soft@inbox.ru>
> + * Author : Linus Walleij <linus.walleij@linaro.org>
> + *
> + */
> +
> +#include <asm/byteorder.h>
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/interrupt.h>
> +#include <linux/pm.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/input.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/mfd/ipaq-micro.h>
> +
> +struct touchscreen_data {
> +	struct input_dev *input;
> +	struct ipaq_micro *micro;
> +};
> +
> +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,
> +				 be16_to_cpup((__be16 *) &msg[2]));
> +		input_report_abs(ts->input, ABS_Y,
> +				 be16_to_cpup((__be16 *) &msg[0]));
> +		input_report_key(ts->input, BTN_TOUCH, 1);
> +	}
> +	if (len == 0) {
> +		input_report_abs(ts->input, ABS_X, 0);
> +		input_report_abs(ts->input, ABS_Y, 0);
> +		input_report_key(ts->input, BTN_TOUCH, 0);
> +	}
> +	input_sync(ts->input);

We should probably pull input_sync() up into these 2 branches so that we
do not try to send sync on empty packets.

> +}
> +
> +
> +static int micro_ts_probe(struct platform_device *pdev)
> +{
> +	struct touchscreen_data *ts;
> +	int ret;
> +
> +	ts = devm_kzalloc(&pdev->dev, sizeof(*ts), GFP_KERNEL);
> +	if (!ts)
> +		return -ENOMEM;
> +	ts->micro = dev_get_drvdata(pdev->dev.parent);
> +
> +	platform_set_drvdata(pdev, ts);
> +
> +	ts->input = devm_input_allocate_device(&pdev->dev);
> +	if (!ts->input) {
> +		dev_err(&pdev->dev, "failed to allocate input device\n");
> +		return -ENOMEM;
> +        }
> +
> +	input_set_capability(ts->input, EV_KEY, BTN_TOUCH);
> +	input_set_capability(ts->input, EV_ABS, ABS_X);
> +	input_set_capability(ts->input, EV_ABS, ABS_Y);
> +	input_set_abs_params(ts->input, ABS_X, 0, 1023, 0, 0);
> +	input_set_abs_params(ts->input, ABS_Y, 0, 1023, 0, 0);
> +
> +	ts->input->name = "ipaq micro ts";
> +
> +	ret = input_register_device(ts->input);
> +	if (ret) {
> +		dev_err(&pdev->dev, "error registering touch input\n");
> +		return ret;
> +	}
> +
> +	spin_lock(&ts->micro->lock);
> +	ts->micro->ts = micro_ts_receive;
> +	ts->micro->ts_data = ts;
> +	spin_unlock(&ts->micro->lock);

The receiver callback is called form interrupt context, as far as I can
see, so these must be _irq() or _irqsave().

> +
> +	dev_info(&pdev->dev, "iPAQ micro touchscreen\n");
> +	return 0;
> +}
> +
> +static int micro_ts_remove(struct platform_device *pdev)
> +{
> +	struct touchscreen_data *ts = platform_get_drvdata(pdev);
> +
> +	spin_lock(&ts->micro->lock);
> +	ts->micro->ts = NULL;
> +	ts->micro->ts_data = NULL;
> +	spin_unlock(&ts->micro->lock);
> +
> +	return 0;
> +}
> +
> +static int micro_ts_suspend(struct device *dev)
> +{
> +	struct touchscreen_data *ts = dev_get_drvdata(dev);
> +
> +	spin_lock(&ts->micro->lock);
> +	ts->micro->ts = NULL;
> +	ts->micro->ts_data = NULL;
> +	spin_unlock(&ts->micro->lock);
> +	return 0;
> +}
> +
> +static int micro_ts_resume(struct device *dev)
> +{
> +	struct touchscreen_data *ts = dev_get_drvdata(dev);
> +
> +	spin_lock(&ts->micro->lock);
> +	ts->micro->ts = micro_ts_receive;
> +	ts->micro->ts_data = ts;
> +	spin_unlock(&ts->micro->lock);
> +	return 0;
> +}

We should protect these with CONFIG_PM_SLEEP to avoid "unused" warnings.

> +
> +static const struct dev_pm_ops micro_ts_dev_pm_ops = {
> +	SET_SYSTEM_SLEEP_PM_OPS(micro_ts_suspend, micro_ts_resume)
> +};
> +
> +struct platform_driver micro_ts_device_driver = {
> +	.driver  = {
> +		.name    = "ipaq-micro-ts",
> +		.pm	= &micro_ts_dev_pm_ops,
> +	},
> +	.probe   = micro_ts_probe,
> +	.remove  = micro_ts_remove,
> +};
> +module_platform_driver(micro_ts_device_driver);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("driver for iPAQ Atmel micro touchscreen");
> +MODULE_ALIAS("platform:ipaq-micro-ts");

I've done the changes locally, please take a look.

Thanks.

-- 
Dmitry


Input: driver for touchscreen on iPaq h3xxx

From: Dmitry Artamonow <mad_soft@inbox.ru>

This adds a driver for the touchscreen connected to the Atmel
microcontroller on the iPAQ h3xxx series.

Based on a driver from handhelds.org 2.6.21 kernel, written by Alessandro
GARDICH, with the bulk of the code for the new input architecture rewritten
by Dmitry Atamonow, and the final polish by Linus Walleij.

Signed-off-by: Alessandro GARDICH <gremlin@gremlin.it>
Signed-off-by: Dmitry Artamonow <mad_soft@inbox.ru>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/touchscreen/Kconfig         |   12 ++
 drivers/input/touchscreen/Makefile        |    1 
 drivers/input/touchscreen/ipaq-micro-ts.c |  142 +++++++++++++++++++++++++++++
 3 files changed, 155 insertions(+)
 create mode 100644 drivers/input/touchscreen/ipaq-micro-ts.c

diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
index a23a94b..6bb9a7d 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -471,6 +471,18 @@ config TOUCHSCREEN_HP7XX
 	  To compile this driver as a module, choose M here: the
 	  module will be called jornada720_ts.
 
+config TOUCHSCREEN_IPAQ_MICRO
+	tristate "HP iPAQ Atmel Micro ASIC touchscreen"
+	depends on MFD_IPAQ_MICRO
+	help
+	  Say Y here to enable support for the touchscreen attached to
+	  the Atmel Micro peripheral controller on iPAQ h3100/h3600/h3700
+
+	  If unsure, say N.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called ipaq-micro-ts.
+
 config TOUCHSCREEN_HTCPEN
 	tristate "HTC Shift X9500 touchscreen"
 	depends on ISA
diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
index 126479d..4be94fc 100644
--- a/drivers/input/touchscreen/Makefile
+++ b/drivers/input/touchscreen/Makefile
@@ -46,6 +46,7 @@ obj-$(CONFIG_TOUCHSCREEN_MTOUCH)	+= mtouch.o
 obj-$(CONFIG_TOUCHSCREEN_MK712)		+= mk712.o
 obj-$(CONFIG_TOUCHSCREEN_HP600)		+= hp680_ts_input.o
 obj-$(CONFIG_TOUCHSCREEN_HP7XX)		+= jornada720_ts.o
+obj-$(CONFIG_TOUCHSCREEN_IPAQ_MICRO)	+= ipaq-micro-ts.o
 obj-$(CONFIG_TOUCHSCREEN_HTCPEN)	+= htcpen.o
 obj-$(CONFIG_TOUCHSCREEN_USB_COMPOSITE)	+= usbtouchscreen.o
 obj-$(CONFIG_TOUCHSCREEN_PCAP)		+= pcap_ts.o
diff --git a/drivers/input/touchscreen/ipaq-micro-ts.c b/drivers/input/touchscreen/ipaq-micro-ts.c
new file mode 100644
index 0000000..53fa5c3
--- /dev/null
+++ b/drivers/input/touchscreen/ipaq-micro-ts.c
@@ -0,0 +1,142 @@
+/*
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * h3600 atmel micro companion support, touchscreen subdevice
+ * Author : Alessandro Gardich <gremlin@gremlin.it>
+ * Author : Dmitry Artamonow <mad_soft@inbox.ru>
+ * Author : Linus Walleij <linus.walleij@linaro.org>
+ *
+ */
+
+#include <asm/byteorder.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/pm.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/input.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/mfd/ipaq-micro.h>
+
+struct touchscreen_data {
+	struct input_dev *input;
+	struct ipaq_micro *micro;
+};
+
+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,
+				 be16_to_cpup((__be16 *) &msg[2]));
+		input_report_abs(ts->input, ABS_Y,
+				 be16_to_cpup((__be16 *) &msg[0]));
+		input_report_key(ts->input, BTN_TOUCH, 1);
+		input_sync(ts->input);
+	} else if (len == 0) {
+		input_report_abs(ts->input, ABS_X, 0);
+		input_report_abs(ts->input, ABS_Y, 0);
+		input_report_key(ts->input, BTN_TOUCH, 0);
+		input_sync(ts->input);
+	}
+}
+
+static int micro_ts_probe(struct platform_device *pdev)
+{
+	struct touchscreen_data *ts;
+	int ret;
+
+	ts = devm_kzalloc(&pdev->dev, sizeof(*ts), GFP_KERNEL);
+	if (!ts)
+		return -ENOMEM;
+	ts->micro = dev_get_drvdata(pdev->dev.parent);
+
+	platform_set_drvdata(pdev, ts);
+
+	ts->input = devm_input_allocate_device(&pdev->dev);
+	if (!ts->input) {
+		dev_err(&pdev->dev, "failed to allocate input device\n");
+		return -ENOMEM;
+	}
+
+	input_set_capability(ts->input, EV_KEY, BTN_TOUCH);
+	input_set_capability(ts->input, EV_ABS, ABS_X);
+	input_set_capability(ts->input, EV_ABS, ABS_Y);
+	input_set_abs_params(ts->input, ABS_X, 0, 1023, 0, 0);
+	input_set_abs_params(ts->input, ABS_Y, 0, 1023, 0, 0);
+
+	ts->input->name = "ipaq micro ts";
+
+	ret = input_register_device(ts->input);
+	if (ret) {
+		dev_err(&pdev->dev, "error registering touch input\n");
+		return ret;
+	}
+
+	spin_lock_irq(&ts->micro->lock);
+	ts->micro->ts = micro_ts_receive;
+	ts->micro->ts_data = ts;
+	spin_unlock_irq(&ts->micro->lock);
+
+	dev_info(&pdev->dev, "iPAQ micro touchscreen\n");
+	return 0;
+}
+
+static int micro_ts_remove(struct platform_device *pdev)
+{
+	struct touchscreen_data *ts = platform_get_drvdata(pdev);
+
+	spin_lock_irq(&ts->micro->lock);
+	ts->micro->ts = NULL;
+	ts->micro->ts_data = NULL;
+	spin_unlock_irq(&ts->micro->lock);
+
+	return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int micro_ts_suspend(struct device *dev)
+{
+	struct touchscreen_data *ts = dev_get_drvdata(dev);
+
+	spin_lock_irq(&ts->micro->lock);
+	ts->micro->ts = NULL;
+	ts->micro->ts_data = NULL;
+	spin_unlock_irq(&ts->micro->lock);
+	return 0;
+}
+
+static int micro_ts_resume(struct device *dev)
+{
+	struct touchscreen_data *ts = dev_get_drvdata(dev);
+
+	spin_lock_irq(&ts->micro->lock);
+	ts->micro->ts = micro_ts_receive;
+	ts->micro->ts_data = ts;
+	spin_unlock_irq(&ts->micro->lock);
+	return 0;
+}
+#endif
+
+static const struct dev_pm_ops micro_ts_dev_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(micro_ts_suspend, micro_ts_resume)
+};
+
+static struct platform_driver micro_ts_device_driver = {
+	.driver	= {
+		.name	= "ipaq-micro-ts",
+		.pm	= &micro_ts_dev_pm_ops,
+	},
+	.probe	= micro_ts_probe,
+	.remove	= micro_ts_remove,
+};
+module_platform_driver(micro_ts_device_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("driver for iPAQ Atmel micro touchscreen");
+MODULE_ALIAS("platform:ipaq-micro-ts");

^ permalink raw reply related

* [PATCH] Input: ipaq-micro-ts - introduce open/close
From: Dmitry Torokhov @ 2014-07-23 17:36 UTC (permalink / raw)
  To: linux-input
  Cc: Alessandro GARDICH, Dmitry Artamonow, Linus Walleij, linux-kernel

Wire up open/close so we do not try to send events until someone uses them;
this also allows us to remove micro_ts_remove() and rely fully on managed
resources.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/touchscreen/ipaq-micro-ts.c | 88 ++++++++++++++++++++-----------
 1 file changed, 56 insertions(+), 32 deletions(-)

diff --git a/drivers/input/touchscreen/ipaq-micro-ts.c b/drivers/input/touchscreen/ipaq-micro-ts.c
index 53fa5c3..62c8976 100644
--- a/drivers/input/touchscreen/ipaq-micro-ts.c
+++ b/drivers/input/touchscreen/ipaq-micro-ts.c
@@ -46,17 +46,50 @@ static void micro_ts_receive(void *data, int len, unsigned char *msg)
 	}
 }
 
+static void micro_ts_toggle_receive(struct touchscreen_data *ts, bool enable)
+{
+	struct ipaq_micro *micro = ts->micro;
+
+	spin_lock_irq(&micro->lock);
+
+	if (enable) {
+		micro->ts = micro_ts_receive;
+		micro->ts_data = ts;
+	} else {
+		micro->ts = NULL;
+		micro->ts_data = NULL;
+	}
+
+	spin_unlock_irq(&ts->micro->lock);
+}
+
+static int micro_ts_open(struct input_dev *input)
+{
+	struct touchscreen_data *ts = input_get_drvdata(input);
+
+	micro_ts_toggle_receive(ts, true);
+
+	return 0;
+}
+
+static void micro_ts_close(struct input_dev *input)
+{
+	struct touchscreen_data *ts = input_get_drvdata(input);
+
+	micro_ts_toggle_receive(ts, false);
+}
+
 static int micro_ts_probe(struct platform_device *pdev)
 {
+	struct ipaq_micro *micro = dev_get_drvdata(pdev->dev.parent);
 	struct touchscreen_data *ts;
-	int ret;
+	int error;
 
 	ts = devm_kzalloc(&pdev->dev, sizeof(*ts), GFP_KERNEL);
 	if (!ts)
 		return -ENOMEM;
-	ts->micro = dev_get_drvdata(pdev->dev.parent);
 
-	platform_set_drvdata(pdev, ts);
+	ts->micro = micro;
 
 	ts->input = devm_input_allocate_device(&pdev->dev);
 	if (!ts->input) {
@@ -64,37 +97,27 @@ static int micro_ts_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
+	ts->input->name = "ipaq micro ts";
+	ts->input->open = micro_ts_open;
+	ts->input->close = micro_ts_close;
+
+	input_set_drvdata(ts->input, ts);
+
 	input_set_capability(ts->input, EV_KEY, BTN_TOUCH);
 	input_set_capability(ts->input, EV_ABS, ABS_X);
 	input_set_capability(ts->input, EV_ABS, ABS_Y);
 	input_set_abs_params(ts->input, ABS_X, 0, 1023, 0, 0);
 	input_set_abs_params(ts->input, ABS_Y, 0, 1023, 0, 0);
 
-	ts->input->name = "ipaq micro ts";
-
-	ret = input_register_device(ts->input);
-	if (ret) {
+	error = input_register_device(ts->input);
+	if (error) {
 		dev_err(&pdev->dev, "error registering touch input\n");
-		return ret;
+		return error;
 	}
 
-	spin_lock_irq(&ts->micro->lock);
-	ts->micro->ts = micro_ts_receive;
-	ts->micro->ts_data = ts;
-	spin_unlock_irq(&ts->micro->lock);
+	platform_set_drvdata(pdev, ts);
 
 	dev_info(&pdev->dev, "iPAQ micro touchscreen\n");
-	return 0;
-}
-
-static int micro_ts_remove(struct platform_device *pdev)
-{
-	struct touchscreen_data *ts = platform_get_drvdata(pdev);
-
-	spin_lock_irq(&ts->micro->lock);
-	ts->micro->ts = NULL;
-	ts->micro->ts_data = NULL;
-	spin_unlock_irq(&ts->micro->lock);
 
 	return 0;
 }
@@ -104,21 +127,23 @@ static int micro_ts_suspend(struct device *dev)
 {
 	struct touchscreen_data *ts = dev_get_drvdata(dev);
 
-	spin_lock_irq(&ts->micro->lock);
-	ts->micro->ts = NULL;
-	ts->micro->ts_data = NULL;
-	spin_unlock_irq(&ts->micro->lock);
+	micro_ts_toggle_receive(ts, false);
+
 	return 0;
 }
 
 static int micro_ts_resume(struct device *dev)
 {
 	struct touchscreen_data *ts = dev_get_drvdata(dev);
+	struct input_dev *input = ts->input;
+
+	mutex_lock(&input->mutex);
+
+	if (input->users)
+		micro_ts_toggle_receive(ts, true);
+
+	mutex_unlock(&input->mutex);
 
-	spin_lock_irq(&ts->micro->lock);
-	ts->micro->ts = micro_ts_receive;
-	ts->micro->ts_data = ts;
-	spin_unlock_irq(&ts->micro->lock);
 	return 0;
 }
 #endif
@@ -133,7 +158,6 @@ static struct platform_driver micro_ts_device_driver = {
 		.pm	= &micro_ts_dev_pm_ops,
 	},
 	.probe	= micro_ts_probe,
-	.remove	= micro_ts_remove,
 };
 module_platform_driver(micro_ts_device_driver);
 
-- 
2.0.0.526.g5318336


-- 
Dmitry

^ permalink raw reply related

* Re: [PATCH 00/15] atmel_mxt_ts - device tree, bootloader, etc
From: Dmitry Torokhov @ 2014-07-23 20:29 UTC (permalink / raw)
  To: Stephen Warren
  Cc: Nick Dyer, Yufeng Shen, Daniel Kurtz, Henrik Rydberg,
	Joonyoung Shim, Alan Bowens, linux-input, linux-kernel,
	Peter Meerwald, Benson Leung, Olof Johansson, Sekhar Nori
In-Reply-To: <53CFEF6E.2060905@wwwdotorg.org>

On Wed, Jul 23, 2014 at 11:22:54AM -0600, Stephen Warren wrote:
> On 07/23/2014 09:30 AM, Nick Dyer wrote:
> > On 22/07/14 21:34, Stephen Warren wrote:
> >> Unfortunately, I still can't get these to work on my system.
> >>
> >> Per your "Re: atmel_mxt_ts: defaulting irqflags to
> >> IRQF_TRIGGER_FALLING", I set up the IRQ type in the Tegra DT file, and
> >> then applied this series on top of next-20140721. The driver appears to
> >> initialize OK, but neither X nor evtest see any events from the device.
> >> The IRQ count in /proc/interrupts doesn't increase when I touch the
> >> touchpad, but does when I press it hard enough to trigger the physical
> >> button.
> > 
> > You're using the T19/GPIO support, then? In which case, there appears to be
> > something wrong on the touch controller rather than the driver itself.
> 
> I assume I'm using T19, since there's a physical click action on the
> touchpad along with the normal touch detection.
> 
> >> A boot log with debug enabled follows. No additional kernel log
> >> messages are generated by touches or clicks.
> > 
> > Perhaps I should add some debug to mxt_input_button() - currently it will
> > not debug the fact that a click is received, although I guess that you will
> > see it in getevent.
> > 
> >> Do you have any idea what I should try?
> > 
> > I am suspicious that it may be that the power sequencing isn't quite right,
> > which sometimes leads to parts of the chip not working properly (eg GPIO
> > buttons working, but no touch).
> > 
> > The patch "use deep sleep when stopped" removes the reset on every resume
> > (which would otherwise kill resume performance). But that reset tends to
> > paper over a device which hasn't been powered up properly in the first place.
> > 
> > Could you try issuing a manual reset and see if the touch starts working? I
> > would normally do this by compiling our obp-utils software from
> > https://github.com/atmel-maxtouch/obp-utils using ndk-build and doing
> > something like:
> > 
> > mxt-app -d i2c-dev:1-004b --reset
> > 
> > (you need CONFIG_I2C_DEBUG to make /dev/i2c-1 appear)
> 
> That didn't make any difference.
> 
> I also tried the tool interactively. the "Display raw (M)essages" option
> never displayed anything, and the couple of self-tests I tried just
> timed out. "Read (I)nfo block" did display some values that seemed like
> they might be correct rather than random data.
> 
> Interestingly though, I did bisect the series and found "Input:
> atmel_mxt_ts - use deep sleep mode when stopped" causes the problem. If
> I apply the whole series and revert that one patch, the touchpad works
> for mouse movement, but interestingly not for taps or physical clicks.

I ended up applying everything but the "deep sleep" patch. I wonder if
you have any keys defined to indicate that it is a touchpad and activate
tap-to-click support in userspace.

Thanks.

-- 
Dmitry

^ permalink raw reply

* [PATCH] hid: hid-apple: Fixed 2 spacing style issues
From: Brad Slayter @ 2014-07-23 20:47 UTC (permalink / raw)
  To: jkosina; +Cc: linux-input, linux-kernel, Brad Slayter

Fixed a spacing issue in a struct def and a funcion call.

Signed-off-by: Brad Slayter <slayterkernel@gmail.com>
---
 drivers/hid/hid-apple.c |   10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c
index f822fd2..8e0a0a4 100644
--- a/drivers/hid/hid-apple.c
+++ b/drivers/hid/hid-apple.c
@@ -157,10 +157,10 @@ static const struct apple_key_translation apple_iso_keyboard[] = {
 };
 
 static const struct apple_key_translation swapped_option_cmd_keys[] = {
-	{ KEY_LEFTALT,	KEY_LEFTMETA },
-	{ KEY_LEFTMETA,	KEY_LEFTALT },
-	{ KEY_RIGHTALT,	KEY_RIGHTMETA },
-	{ KEY_RIGHTMETA,KEY_RIGHTALT },
+	{ KEY_LEFTALT,	 KEY_LEFTMETA },
+	{ KEY_LEFTMETA,	 KEY_LEFTALT },
+	{ KEY_RIGHTALT,	 KEY_RIGHTMETA },
+	{ KEY_RIGHTMETA, KEY_RIGHTALT },
 	{ }
 };
 
@@ -200,7 +200,7 @@ static int hidinput_apple_event(struct hid_device *hid, struct input_dev *input,
 		else
 			table = apple_fn_keys;
 
-		trans = apple_find_translation (table, usage->code);
+		trans = apple_find_translation(table, usage->code);
 
 		if (trans) {
 			if (test_bit(usage->code, asc->pressed_fn))
-- 
1.7.10.4

^ permalink raw reply related

* Re: [PATCH 04/15] Input: atmel_mxt_ts - implement device tree support
From: Stephen Warren @ 2014-07-23 21:36 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: nick.dyer, Yufeng Shen, Daniel Kurtz, Henrik Rydberg,
	Joonyoung Shim, Alan Bowens, linux-input, linux-kernel,
	Peter Meerwald, Benson Leung, Olof Johansson, Sekhar Nori,
	Stephen Warren
In-Reply-To: <1404399697-26484-5-git-send-email-nick.dyer@itdev.co.uk>

On 07/03/2014 09:01 AM, nick.dyer@itdev.co.uk wrote:
> From: Stephen Warren <swarren@nvidia.com>
> 
> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> Signed-off-by: Nick Dyer <nick.dyer@itdev.co.uk>

Hmm. It looks like the version of this patch that was actually applied
varies quite a bit from what was posted, in particular in the
implementation and call site of mxt_parse_dt(). It'd be nice if the
commit log had been adjusted to mention this, so it didn't look like *I*
caused all those compile errors and warnings that the 0-day builder just
found:-(

^ permalink raw reply

* Re: [PATCH 00/15] atmel_mxt_ts - device tree, bootloader, etc
From: Stephen Warren @ 2014-07-23 21:39 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Nick Dyer, Yufeng Shen, Daniel Kurtz, Henrik Rydberg,
	Joonyoung Shim, Alan Bowens, linux-input, linux-kernel,
	Peter Meerwald, Benson Leung, Olof Johansson, Sekhar Nori
In-Reply-To: <20140723202949.GA958@core.coreip.homeip.net>

On 07/23/2014 02:29 PM, Dmitry Torokhov wrote:
> On Wed, Jul 23, 2014 at 11:22:54AM -0600, Stephen Warren wrote:
>> On 07/23/2014 09:30 AM, Nick Dyer wrote:
>>> On 22/07/14 21:34, Stephen Warren wrote:
>>>> Unfortunately, I still can't get these to work on my system.
>>>>
>>>> Per your "Re: atmel_mxt_ts: defaulting irqflags to
>>>> IRQF_TRIGGER_FALLING", I set up the IRQ type in the Tegra DT file, and
>>>> then applied this series on top of next-20140721. The driver appears to
>>>> initialize OK, but neither X nor evtest see any events from the device.
>>>> The IRQ count in /proc/interrupts doesn't increase when I touch the
>>>> touchpad, but does when I press it hard enough to trigger the physical
>>>> button.
>>>
>>> You're using the T19/GPIO support, then? In which case, there appears to be
>>> something wrong on the touch controller rather than the driver itself.
>>
>> I assume I'm using T19, since there's a physical click action on the
>> touchpad along with the normal touch detection.
>>
>>>> A boot log with debug enabled follows. No additional kernel log
>>>> messages are generated by touches or clicks.
>>>
>>> Perhaps I should add some debug to mxt_input_button() - currently it will
>>> not debug the fact that a click is received, although I guess that you will
>>> see it in getevent.
>>>
>>>> Do you have any idea what I should try?
>>>
>>> I am suspicious that it may be that the power sequencing isn't quite right,
>>> which sometimes leads to parts of the chip not working properly (eg GPIO
>>> buttons working, but no touch).
>>>
>>> The patch "use deep sleep when stopped" removes the reset on every resume
>>> (which would otherwise kill resume performance). But that reset tends to
>>> paper over a device which hasn't been powered up properly in the first place.
>>>
>>> Could you try issuing a manual reset and see if the touch starts working? I
>>> would normally do this by compiling our obp-utils software from
>>> https://github.com/atmel-maxtouch/obp-utils using ndk-build and doing
>>> something like:
>>>
>>> mxt-app -d i2c-dev:1-004b --reset
>>>
>>> (you need CONFIG_I2C_DEBUG to make /dev/i2c-1 appear)
>>
>> That didn't make any difference.
>>
>> I also tried the tool interactively. the "Display raw (M)essages" option
>> never displayed anything, and the couple of self-tests I tried just
>> timed out. "Read (I)nfo block" did display some values that seemed like
>> they might be correct rather than random data.
>>
>> Interestingly though, I did bisect the series and found "Input:
>> atmel_mxt_ts - use deep sleep mode when stopped" causes the problem. If
>> I apply the whole series and revert that one patch, the touchpad works
>> for mouse movement, but interestingly not for taps or physical clicks.
> 
> I ended up applying everything but the "deep sleep" patch. I wonder if
> you have any keys defined to indicate that it is a touchpad and activate
> tap-to-click support in userspace.

Yes, I have one GPIO defined in the keymap, for the physical
push-to-click button:

>                 trackpad@4b {
>                         compatible = "atmel,maxtouch";
>                         reg = <0x4b>;
>                         interrupt-parent = <&gpio>;
>                         interrupts = <TEGRA_GPIO(W, 3) IRQ_TYPE_LEVEL_LOW>;
>                         linux,gpio-keymap = <0 0 0 BTN_LEFT>;
>                 };

(at some point long ago, this did work fine, just by adding my DT
binding/parsing patch on top of what was in linux-next).

^ permalink raw reply

* [PATCH] Input: atmel_mxt_ts - simplify mxt_initialize a bit
From: Dmitry Torokhov @ 2014-07-23 21:50 UTC (permalink / raw)
  To: linux-input
  Cc: Nick Dyer, Yufeng Shen, Benson Leung, Daniel Kurtz, Iiro Valkonen,
	linux-kernel

I think having control flow with 2 goto/labels/flags is quite hard to read,
this version is a bit more readable IMO.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/touchscreen/atmel_mxt_ts.c | 81 +++++++++++++++++---------------
 1 file changed, 42 insertions(+), 39 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index c6dfd0a..5882352 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -355,7 +355,6 @@ static int mxt_bootloader_read(struct mxt_data *data,
 	msg.buf = val;
 
 	ret = i2c_transfer(data->client->adapter, &msg, 1);
-
 	if (ret == 1) {
 		ret = 0;
 	} else {
@@ -410,6 +409,7 @@ static int mxt_lookup_bootloader_address(struct mxt_data *data, bool retry)
 	case 0x5b:
 		bootloader = appmode - 0x26;
 		break;
+
 	default:
 		dev_err(&data->client->dev,
 			"Appmode i2c address 0x%02x not found\n",
@@ -421,20 +421,20 @@ static int mxt_lookup_bootloader_address(struct mxt_data *data, bool retry)
 	return 0;
 }
 
-static int mxt_probe_bootloader(struct mxt_data *data, bool retry)
+static int mxt_probe_bootloader(struct mxt_data *data, bool alt_address)
 {
 	struct device *dev = &data->client->dev;
-	int ret;
+	int error;
 	u8 val;
 	bool crc_failure;
 
-	ret = mxt_lookup_bootloader_address(data, retry);
-	if (ret)
-		return ret;
+	error = mxt_lookup_bootloader_address(data, alt_address);
+	if (error)
+		return error;
 
-	ret = mxt_bootloader_read(data, &val, 1);
-	if (ret)
-		return ret;
+	error = mxt_bootloader_read(data, &val, 1);
+	if (error)
+		return error;
 
 	/* Check app crc fail mode */
 	crc_failure = (val & ~MXT_BOOT_STATUS_MASK) == MXT_APP_CRC_FAIL;
@@ -1653,41 +1653,39 @@ static void mxt_config_cb(const struct firmware *cfg, void *ctx)
 static int mxt_initialize(struct mxt_data *data)
 {
 	struct i2c_client *client = data->client;
+	int recovery_attempts = 0;
 	int error;
-	bool alt_bootloader_addr = false;
-	bool retry = false;
 
-retry_info:
-	error = mxt_get_info(data);
-	if (error) {
-retry_bootloader:
-		error = mxt_probe_bootloader(data, alt_bootloader_addr);
+	while (1) {
+		error = mxt_get_info(data);
+		if (!error)
+			break;
+
+		/* Check bootloader state */
+		error = mxt_probe_bootloader(data, false);
 		if (error) {
-			if (alt_bootloader_addr) {
+			dev_info(&client->dev, "Trying alternate bootloader address\n");
+			error = mxt_probe_bootloader(data, true);
+			if (error) {
 				/* Chip is not in appmode or bootloader mode */
 				return error;
 			}
+		}
 
-			dev_info(&client->dev, "Trying alternate bootloader address\n");
-			alt_bootloader_addr = true;
-			goto retry_bootloader;
-		} else {
-			if (retry) {
-				dev_err(&client->dev, "Could not recover from bootloader mode\n");
-				/*
-				 * We can reflash from this state, so do not
-				 * abort init
-				 */
-				data->in_bootloader = true;
-				return 0;
-			}
-
-			/* Attempt to exit bootloader into app mode */
-			mxt_send_bootloader_cmd(data, false);
-			msleep(MXT_FW_RESET_TIME);
-			retry = true;
-			goto retry_info;
+		/* OK, we are in bootloader, see if we can recover */
+		if (++recovery_attempts > 1) {
+			dev_err(&client->dev, "Could not recover from bootloader mode\n");
+			/*
+			 * We can reflash from this state, so do not
+			 * abort initialization.
+			 */
+			data->in_bootloader = true;
+			return 0;
 		}
+
+		/* Attempt to exit bootloader into app mode */
+		mxt_send_bootloader_cmd(data, false);
+		msleep(MXT_FW_RESET_TIME);
 	}
 
 	/* Get object table information */
@@ -1701,9 +1699,14 @@ retry_bootloader:
 	if (error)
 		goto err_free_object_table;
 
-	request_firmware_nowait(THIS_MODULE, true, MXT_CFG_NAME,
-				&data->client->dev, GFP_KERNEL, data,
-				mxt_config_cb);
+	error = request_firmware_nowait(THIS_MODULE, true, MXT_CFG_NAME,
+					&client->dev, GFP_KERNEL, data,
+					mxt_config_cb);
+	if (error) {
+		dev_err(&client->dev, "Failed to invoke firmware loader: %d\n",
+			error);
+		goto err_free_object_table;
+	}
 
 	return 0;
 
-- 
2.0.0.526.g5318336


-- 
Dmitry

^ permalink raw reply related

* [PATCH v5 0/4] HID: lenovo: Add support for Lenovo Compact Keyboard
From: Jamie Lentin @ 2014-07-23 22:30 UTC (permalink / raw)
  To: Jiri Kosina, Antonio Ospite, Hans de Goede
  Cc: linux-input, linux-kernel, Jamie Lentin

This patchset follows on from my previous attempts to add support for
these keyboards from Lenovo.

Antonio, I've used cptkbd_data and left data_pointer for now. It'd be
an easy patch to change data_pointer to tpkbd_data, but is drifting
away from what this patchset is supposed to do. Could send that
afterwards though.

Changes since v4: https://lkml.org/lkml/2014/7/13/14
* Remember to add prefixes to subjects (whooops) [Antonio Ospite]
* Don't allocate drvdata for non-interesting half of USB
  keyboard [Antonio Ospite]
* Some pointer/trackpoint comment clarifications [Antonio Ospite]
* Change tpcsc -> cptkbd_data [Antonio Ospite]
* Only bother doing sysfs_remove_group if there's a data_pointer
  [Antonio Ospite]

Changes since v3: https://lkml.org/lkml/2014/6/15/159
* Break up first patch [Antonio Ospite]
* Rename ABI docs too [Jiri Kosina]
* Spelling and Grammar, style fixes [Antonio Ospite]
* Driver should just be called 'lenovo' [Antonio Ospite]
* Remove useless id from probe functions [Antonio Ospite]
* Use KEY_SCALE to match Thinkpad ??40 models [Hans de Goede]
* Add ABI documentation for fn_lock setting [Jiri Kosina]
* Use a bool for fn_lock instead of uint [Antonio Ospite]
* hid_output_raw_report has been replaced with hid_hw_raw_request [Antonio Ospite]

Changes since v2: https://lkml.org/lkml/2014/6/10/730
* Rename hid-lenovo-tpkbd to hid-lenovo, to make it obvious this is
  for any Lenovo-manufactured device [Antonio Ospite, Jiri Kosina]
* Style fixes: function calls in conditions, combine checks for 
  both USB & BT keyboards [Antonio Ospite]

Changes since v1: https://lkml.org/lkml/2014/3/25/535
* Merge driver into hid-lenovo-tpkbd.c instead of creating our own
  driver for the hardware [Jiri Kosina]
* Remove key mappings which are now supported by standard
* Use KEY_FILE for Fn-F12 (opens My Computer on Windows)
* Support the USB variant as well as Bluetooth
* Expose the Fn Lock setting as a sysfs attribute instead of trying to
  build a mechanism to toggle into the kernel

Applies against v3.16-rc4, tested with Bluetooth and USB variants of the
Compact Keyboard with Trackpoint.

Cheers,

Jamie Lentin (4):
  HID: lenovo: Rename hid-lenovo-tpkbd to hid-lenovo
  HID: lenovo: Prepare support for adding other devices
  HID: lenovo: Style fixes
  HID: lenovo: Add support for Compact (BT|USB) keyboard

 ...er-hid-lenovo-tpkbd => sysfs-driver-hid-lenovo} |  12 +
 drivers/hid/Kconfig                                |  16 +-
 drivers/hid/Makefile                               |   2 +-
 drivers/hid/hid-core.c                             |   4 +-
 drivers/hid/hid-ids.h                              |   2 +
 drivers/hid/hid-lenovo-tpkbd.c                     | 462 --------------
 drivers/hid/hid-lenovo.c                           | 708 +++++++++++++++++++++
 include/linux/hid.h                                |   1 +
 8 files changed, 736 insertions(+), 471 deletions(-)
 rename Documentation/ABI/testing/{sysfs-driver-hid-lenovo-tpkbd => sysfs-driver-hid-lenovo} (75%)
 delete mode 100644 drivers/hid/hid-lenovo-tpkbd.c
 create mode 100644 drivers/hid/hid-lenovo.c

-- 
2.0.0

^ permalink raw reply

* [PATCH v5 1/4] HID: lenovo: Rename hid-lenovo-tpkbd to hid-lenovo
From: Jamie Lentin @ 2014-07-23 22:30 UTC (permalink / raw)
  To: Jiri Kosina, Antonio Ospite, Hans de Goede
  Cc: linux-input, linux-kernel, Jamie Lentin
In-Reply-To: <1406154648-14000-1-git-send-email-jm@lentin.co.uk>

Rename module and all functions within so we can add support for other
keyboards in the same file. Rename the _tp postfix to _tpkbd, to
signify functions relevant to the TP USB keyboard.

Signed-off-by: Jamie Lentin <jm@lentin.co.uk>
---
 ...er-hid-lenovo-tpkbd => sysfs-driver-hid-lenovo} |   0
 drivers/hid/Kconfig                                |  14 +-
 drivers/hid/Makefile                               |   2 +-
 drivers/hid/hid-core.c                             |   2 +-
 drivers/hid/{hid-lenovo-tpkbd.c => hid-lenovo.c}   | 185 +++++++++++----------
 5 files changed, 102 insertions(+), 101 deletions(-)
 rename Documentation/ABI/testing/{sysfs-driver-hid-lenovo-tpkbd => sysfs-driver-hid-lenovo} (100%)
 rename drivers/hid/{hid-lenovo-tpkbd.c => hid-lenovo.c} (64%)

diff --git a/Documentation/ABI/testing/sysfs-driver-hid-lenovo-tpkbd b/Documentation/ABI/testing/sysfs-driver-hid-lenovo
similarity index 100%
rename from Documentation/ABI/testing/sysfs-driver-hid-lenovo-tpkbd
rename to Documentation/ABI/testing/sysfs-driver-hid-lenovo
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 800c8b6..9b7acfc 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -331,18 +331,18 @@ config HID_LCPOWER
 	---help---
 	Support for LC-Power RC1000MCE RF remote control.
 
-config HID_LENOVO_TPKBD
-	tristate "Lenovo ThinkPad USB Keyboard with TrackPoint"
+config HID_LENOVO
+	tristate "Lenovo / Thinkpad devices"
 	depends on HID
 	select NEW_LEDS
 	select LEDS_CLASS
 	---help---
-	Support for the Lenovo ThinkPad USB Keyboard with TrackPoint.
+	Support for Lenovo devices that are not fully compliant with HID standard.
 
-	Say Y here if you have a Lenovo ThinkPad USB Keyboard with TrackPoint
-	and would like to use device-specific features like changing the
-	sensitivity of the trackpoint, using the microphone mute button or
-	controlling the mute and microphone mute LEDs.
+	Say Y if you want support for the non-compliant features of the Lenovo
+	Thinkpad standalone keyboards, e.g:
+	- ThinkPad USB Keyboard with TrackPoint (supports extra LEDs and trackpoint
+	  configuration)
 
 config HID_LOGITECH
 	tristate "Logitech devices" if EXPERT
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index a6fa6ba..5e96be3 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -59,7 +59,7 @@ obj-$(CONFIG_HID_KENSINGTON)	+= hid-kensington.o
 obj-$(CONFIG_HID_KEYTOUCH)	+= hid-keytouch.o
 obj-$(CONFIG_HID_KYE)		+= hid-kye.o
 obj-$(CONFIG_HID_LCPOWER)       += hid-lcpower.o
-obj-$(CONFIG_HID_LENOVO_TPKBD)	+= hid-lenovo-tpkbd.o
+obj-$(CONFIG_HID_LENOVO)	+= hid-lenovo.o
 obj-$(CONFIG_HID_LOGITECH)	+= hid-logitech.o
 obj-$(CONFIG_HID_LOGITECH_DJ)	+= hid-logitech-dj.o
 obj-$(CONFIG_HID_MAGICMOUSE)    += hid-magicmouse.o
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 8ed66fd..55841bd 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1796,7 +1796,7 @@ static const struct hid_device_id hid_have_special_driver[] = {
 	{ HID_USB_DEVICE(USB_VENDOR_ID_KYE, USB_DEVICE_ID_KYE_EASYPEN_M610X) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_LABTEC, USB_DEVICE_ID_LABTEC_WIRELESS_KEYBOARD) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_LCPOWER, USB_DEVICE_ID_LCPOWER_LC1000 ) },
-#if IS_ENABLED(CONFIG_HID_LENOVO_TPKBD)
+#if IS_ENABLED(CONFIG_HID_LENOVO)
 	{ HID_USB_DEVICE(USB_VENDOR_ID_LENOVO, USB_DEVICE_ID_LENOVO_TPKBD) },
 #endif
 	{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_MX3000_RECEIVER) },
diff --git a/drivers/hid/hid-lenovo-tpkbd.c b/drivers/hid/hid-lenovo.c
similarity index 64%
rename from drivers/hid/hid-lenovo-tpkbd.c
rename to drivers/hid/hid-lenovo.c
index 2d25b6c..0320b96 100644
--- a/drivers/hid/hid-lenovo-tpkbd.c
+++ b/drivers/hid/hid-lenovo.c
@@ -20,8 +20,7 @@
 
 #include "hid-ids.h"
 
-/* This is only used for the trackpoint part of the driver, hence _tp */
-struct tpkbd_data_pointer {
+struct lenovo_drvdata_tpkbd {
 	int led_state;
 	struct led_classdev led_mute;
 	struct led_classdev led_micmute;
@@ -35,7 +34,7 @@ struct tpkbd_data_pointer {
 
 #define map_key_clear(c) hid_map_usage_clear(hi, usage, bit, max, EV_KEY, (c))
 
-static int tpkbd_input_mapping(struct hid_device *hdev,
+static int lenovo_input_mapping_tpkbd(struct hid_device *hdev,
 		struct hid_input *hi, struct hid_field *field,
 		struct hid_usage *usage, unsigned long **bit, int *max)
 {
@@ -50,10 +49,10 @@ static int tpkbd_input_mapping(struct hid_device *hdev,
 
 #undef map_key_clear
 
-static int tpkbd_features_set(struct hid_device *hdev)
+static int lenovo_features_set_tpkbd(struct hid_device *hdev)
 {
 	struct hid_report *report;
-	struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev);
+	struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev);
 
 	report = hdev->report_enum[HID_FEATURE_REPORT].report_id_hash[4];
 
@@ -69,23 +68,23 @@ static int tpkbd_features_set(struct hid_device *hdev)
 	return 0;
 }
 
-static ssize_t pointer_press_to_select_show(struct device *dev,
+static ssize_t attr_press_to_select_show_tpkbd(struct device *dev,
 		struct device_attribute *attr,
 		char *buf)
 {
 	struct hid_device *hdev = container_of(dev, struct hid_device, dev);
-	struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev);
+	struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev);
 
 	return snprintf(buf, PAGE_SIZE, "%u\n", data_pointer->press_to_select);
 }
 
-static ssize_t pointer_press_to_select_store(struct device *dev,
+static ssize_t attr_press_to_select_store_tpkbd(struct device *dev,
 		struct device_attribute *attr,
 		const char *buf,
 		size_t count)
 {
 	struct hid_device *hdev = container_of(dev, struct hid_device, dev);
-	struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev);
+	struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev);
 	int value;
 
 	if (kstrtoint(buf, 10, &value))
@@ -94,28 +93,28 @@ static ssize_t pointer_press_to_select_store(struct device *dev,
 		return -EINVAL;
 
 	data_pointer->press_to_select = value;
-	tpkbd_features_set(hdev);
+	lenovo_features_set_tpkbd(hdev);
 
 	return count;
 }
 
-static ssize_t pointer_dragging_show(struct device *dev,
+static ssize_t attr_dragging_show_tpkbd(struct device *dev,
 		struct device_attribute *attr,
 		char *buf)
 {
 	struct hid_device *hdev = container_of(dev, struct hid_device, dev);
-	struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev);
+	struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev);
 
 	return snprintf(buf, PAGE_SIZE, "%u\n", data_pointer->dragging);
 }
 
-static ssize_t pointer_dragging_store(struct device *dev,
+static ssize_t attr_dragging_store_tpkbd(struct device *dev,
 		struct device_attribute *attr,
 		const char *buf,
 		size_t count)
 {
 	struct hid_device *hdev = container_of(dev, struct hid_device, dev);
-	struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev);
+	struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev);
 	int value;
 
 	if (kstrtoint(buf, 10, &value))
@@ -124,28 +123,28 @@ static ssize_t pointer_dragging_store(struct device *dev,
 		return -EINVAL;
 
 	data_pointer->dragging = value;
-	tpkbd_features_set(hdev);
+	lenovo_features_set_tpkbd(hdev);
 
 	return count;
 }
 
-static ssize_t pointer_release_to_select_show(struct device *dev,
+static ssize_t attr_release_to_select_show_tpkbd(struct device *dev,
 		struct device_attribute *attr,
 		char *buf)
 {
 	struct hid_device *hdev = container_of(dev, struct hid_device, dev);
-	struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev);
+	struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev);
 
 	return snprintf(buf, PAGE_SIZE, "%u\n", data_pointer->release_to_select);
 }
 
-static ssize_t pointer_release_to_select_store(struct device *dev,
+static ssize_t attr_release_to_select_store_tpkbd(struct device *dev,
 		struct device_attribute *attr,
 		const char *buf,
 		size_t count)
 {
 	struct hid_device *hdev = container_of(dev, struct hid_device, dev);
-	struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev);
+	struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev);
 	int value;
 
 	if (kstrtoint(buf, 10, &value))
@@ -154,28 +153,28 @@ static ssize_t pointer_release_to_select_store(struct device *dev,
 		return -EINVAL;
 
 	data_pointer->release_to_select = value;
-	tpkbd_features_set(hdev);
+	lenovo_features_set_tpkbd(hdev);
 
 	return count;
 }
 
-static ssize_t pointer_select_right_show(struct device *dev,
+static ssize_t attr_select_right_show_tpkbd(struct device *dev,
 		struct device_attribute *attr,
 		char *buf)
 {
 	struct hid_device *hdev = container_of(dev, struct hid_device, dev);
-	struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev);
+	struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev);
 
 	return snprintf(buf, PAGE_SIZE, "%u\n", data_pointer->select_right);
 }
 
-static ssize_t pointer_select_right_store(struct device *dev,
+static ssize_t attr_select_right_store_tpkbd(struct device *dev,
 		struct device_attribute *attr,
 		const char *buf,
 		size_t count)
 {
 	struct hid_device *hdev = container_of(dev, struct hid_device, dev);
-	struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev);
+	struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev);
 	int value;
 
 	if (kstrtoint(buf, 10, &value))
@@ -184,119 +183,119 @@ static ssize_t pointer_select_right_store(struct device *dev,
 		return -EINVAL;
 
 	data_pointer->select_right = value;
-	tpkbd_features_set(hdev);
+	lenovo_features_set_tpkbd(hdev);
 
 	return count;
 }
 
-static ssize_t pointer_sensitivity_show(struct device *dev,
+static ssize_t attr_sensitivity_show_tpkbd(struct device *dev,
 		struct device_attribute *attr,
 		char *buf)
 {
 	struct hid_device *hdev = container_of(dev, struct hid_device, dev);
-	struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev);
+	struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev);
 
 	return snprintf(buf, PAGE_SIZE, "%u\n",
 		data_pointer->sensitivity);
 }
 
-static ssize_t pointer_sensitivity_store(struct device *dev,
+static ssize_t attr_sensitivity_store_tpkbd(struct device *dev,
 		struct device_attribute *attr,
 		const char *buf,
 		size_t count)
 {
 	struct hid_device *hdev = container_of(dev, struct hid_device, dev);
-	struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev);
+	struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev);
 	int value;
 
 	if (kstrtoint(buf, 10, &value) || value < 1 || value > 255)
 		return -EINVAL;
 
 	data_pointer->sensitivity = value;
-	tpkbd_features_set(hdev);
+	lenovo_features_set_tpkbd(hdev);
 
 	return count;
 }
 
-static ssize_t pointer_press_speed_show(struct device *dev,
+static ssize_t attr_press_speed_show_tpkbd(struct device *dev,
 		struct device_attribute *attr,
 		char *buf)
 {
 	struct hid_device *hdev = container_of(dev, struct hid_device, dev);
-	struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev);
+	struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev);
 
 	return snprintf(buf, PAGE_SIZE, "%u\n",
 		data_pointer->press_speed);
 }
 
-static ssize_t pointer_press_speed_store(struct device *dev,
+static ssize_t attr_press_speed_store_tpkbd(struct device *dev,
 		struct device_attribute *attr,
 		const char *buf,
 		size_t count)
 {
 	struct hid_device *hdev = container_of(dev, struct hid_device, dev);
-	struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev);
+	struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev);
 	int value;
 
 	if (kstrtoint(buf, 10, &value) || value < 1 || value > 255)
 		return -EINVAL;
 
 	data_pointer->press_speed = value;
-	tpkbd_features_set(hdev);
+	lenovo_features_set_tpkbd(hdev);
 
 	return count;
 }
 
-static struct device_attribute dev_attr_pointer_press_to_select =
+static struct device_attribute dev_attr_press_to_select_tpkbd =
 	__ATTR(press_to_select, S_IWUSR | S_IRUGO,
-			pointer_press_to_select_show,
-			pointer_press_to_select_store);
+			attr_press_to_select_show_tpkbd,
+			attr_press_to_select_store_tpkbd);
 
-static struct device_attribute dev_attr_pointer_dragging =
+static struct device_attribute dev_attr_dragging_tpkbd =
 	__ATTR(dragging, S_IWUSR | S_IRUGO,
-			pointer_dragging_show,
-			pointer_dragging_store);
+			attr_dragging_show_tpkbd,
+			attr_dragging_store_tpkbd);
 
-static struct device_attribute dev_attr_pointer_release_to_select =
+static struct device_attribute dev_attr_release_to_select_tpkbd =
 	__ATTR(release_to_select, S_IWUSR | S_IRUGO,
-			pointer_release_to_select_show,
-			pointer_release_to_select_store);
+			attr_release_to_select_show_tpkbd,
+			attr_release_to_select_store_tpkbd);
 
-static struct device_attribute dev_attr_pointer_select_right =
+static struct device_attribute dev_attr_select_right_tpkbd =
 	__ATTR(select_right, S_IWUSR | S_IRUGO,
-			pointer_select_right_show,
-			pointer_select_right_store);
+			attr_select_right_show_tpkbd,
+			attr_select_right_store_tpkbd);
 
-static struct device_attribute dev_attr_pointer_sensitivity =
+static struct device_attribute dev_attr_sensitivity_tpkbd =
 	__ATTR(sensitivity, S_IWUSR | S_IRUGO,
-			pointer_sensitivity_show,
-			pointer_sensitivity_store);
+			attr_sensitivity_show_tpkbd,
+			attr_sensitivity_store_tpkbd);
 
-static struct device_attribute dev_attr_pointer_press_speed =
+static struct device_attribute dev_attr_press_speed_tpkbd =
 	__ATTR(press_speed, S_IWUSR | S_IRUGO,
-			pointer_press_speed_show,
-			pointer_press_speed_store);
-
-static struct attribute *tpkbd_attributes_pointer[] = {
-	&dev_attr_pointer_press_to_select.attr,
-	&dev_attr_pointer_dragging.attr,
-	&dev_attr_pointer_release_to_select.attr,
-	&dev_attr_pointer_select_right.attr,
-	&dev_attr_pointer_sensitivity.attr,
-	&dev_attr_pointer_press_speed.attr,
+			attr_press_speed_show_tpkbd,
+			attr_press_speed_store_tpkbd);
+
+static struct attribute *lenovo_attributes_tpkbd[] = {
+	&dev_attr_press_to_select_tpkbd.attr,
+	&dev_attr_dragging_tpkbd.attr,
+	&dev_attr_release_to_select_tpkbd.attr,
+	&dev_attr_select_right_tpkbd.attr,
+	&dev_attr_sensitivity_tpkbd.attr,
+	&dev_attr_press_speed_tpkbd.attr,
 	NULL
 };
 
-static const struct attribute_group tpkbd_attr_group_pointer = {
-	.attrs = tpkbd_attributes_pointer,
+static const struct attribute_group lenovo_attr_group_tpkbd = {
+	.attrs = lenovo_attributes_tpkbd,
 };
 
-static enum led_brightness tpkbd_led_brightness_get(
+static enum led_brightness lenovo_led_brightness_get_tpkbd(
 			struct led_classdev *led_cdev)
 {
 	struct device *dev = led_cdev->dev->parent;
 	struct hid_device *hdev = container_of(dev, struct hid_device, dev);
-	struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev);
+	struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev);
 	int led_nr = 0;
 
 	if (led_cdev == &data_pointer->led_micmute)
@@ -307,12 +306,12 @@ static enum led_brightness tpkbd_led_brightness_get(
 				: LED_OFF;
 }
 
-static void tpkbd_led_brightness_set(struct led_classdev *led_cdev,
+static void lenovo_led_brightness_set_tpkbd(struct led_classdev *led_cdev,
 			enum led_brightness value)
 {
 	struct device *dev = led_cdev->dev->parent;
 	struct hid_device *hdev = container_of(dev, struct hid_device, dev);
-	struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev);
+	struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev);
 	struct hid_report *report;
 	int led_nr = 0;
 
@@ -330,10 +329,10 @@ static void tpkbd_led_brightness_set(struct led_classdev *led_cdev,
 	hid_hw_request(hdev, report, HID_REQ_SET_REPORT);
 }
 
-static int tpkbd_probe_tp(struct hid_device *hdev)
+static int lenovo_probe_tpkbd(struct hid_device *hdev)
 {
 	struct device *dev = &hdev->dev;
-	struct tpkbd_data_pointer *data_pointer;
+	struct lenovo_drvdata_tpkbd *data_pointer;
 	size_t name_sz = strlen(dev_name(dev)) + 16;
 	char *name_mute, *name_micmute;
 	int i;
@@ -347,12 +346,12 @@ static int tpkbd_probe_tp(struct hid_device *hdev)
 		return -ENODEV;
 
 	if (sysfs_create_group(&hdev->dev.kobj,
-				&tpkbd_attr_group_pointer)) {
+				&lenovo_attr_group_tpkbd)) {
 		hid_warn(hdev, "Could not create sysfs group\n");
 	}
 
 	data_pointer = devm_kzalloc(&hdev->dev,
-				    sizeof(struct tpkbd_data_pointer),
+				    sizeof(struct lenovo_drvdata_tpkbd),
 				    GFP_KERNEL);
 	if (data_pointer == NULL) {
 		hid_err(hdev, "Could not allocate memory for driver data\n");
@@ -375,23 +374,25 @@ static int tpkbd_probe_tp(struct hid_device *hdev)
 	hid_set_drvdata(hdev, data_pointer);
 
 	data_pointer->led_mute.name = name_mute;
-	data_pointer->led_mute.brightness_get = tpkbd_led_brightness_get;
-	data_pointer->led_mute.brightness_set = tpkbd_led_brightness_set;
+	data_pointer->led_mute.brightness_get = lenovo_led_brightness_get_tpkbd;
+	data_pointer->led_mute.brightness_set = lenovo_led_brightness_set_tpkbd;
 	data_pointer->led_mute.dev = dev;
 	led_classdev_register(dev, &data_pointer->led_mute);
 
 	data_pointer->led_micmute.name = name_micmute;
-	data_pointer->led_micmute.brightness_get = tpkbd_led_brightness_get;
-	data_pointer->led_micmute.brightness_set = tpkbd_led_brightness_set;
+	data_pointer->led_micmute.brightness_get =
+		lenovo_led_brightness_get_tpkbd;
+	data_pointer->led_micmute.brightness_set =
+		lenovo_led_brightness_set_tpkbd;
 	data_pointer->led_micmute.dev = dev;
 	led_classdev_register(dev, &data_pointer->led_micmute);
 
-	tpkbd_features_set(hdev);
+	lenovo_features_set_tpkbd(hdev);
 
 	return 0;
 }
 
-static int tpkbd_probe(struct hid_device *hdev,
+static int lenovo_probe(struct hid_device *hdev,
 		const struct hid_device_id *id)
 {
 	int ret;
@@ -410,7 +411,7 @@ static int tpkbd_probe(struct hid_device *hdev,
 
 	if (hid_get_drvdata(hdev)) {
 		hid_set_drvdata(hdev, NULL);
-		ret = tpkbd_probe_tp(hdev);
+		ret = lenovo_probe_tpkbd(hdev);
 		if (ret)
 			goto err_hid;
 	}
@@ -422,12 +423,12 @@ err:
 	return ret;
 }
 
-static void tpkbd_remove_tp(struct hid_device *hdev)
+static void lenovo_remove_tpkbd(struct hid_device *hdev)
 {
-	struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev);
+	struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev);
 
 	sysfs_remove_group(&hdev->dev.kobj,
-			&tpkbd_attr_group_pointer);
+			&lenovo_attr_group_tpkbd);
 
 	led_classdev_unregister(&data_pointer->led_micmute);
 	led_classdev_unregister(&data_pointer->led_mute);
@@ -435,28 +436,28 @@ static void tpkbd_remove_tp(struct hid_device *hdev)
 	hid_set_drvdata(hdev, NULL);
 }
 
-static void tpkbd_remove(struct hid_device *hdev)
+static void lenovo_remove(struct hid_device *hdev)
 {
 	if (hid_get_drvdata(hdev))
-		tpkbd_remove_tp(hdev);
+		lenovo_remove_tpkbd(hdev);
 
 	hid_hw_stop(hdev);
 }
 
-static const struct hid_device_id tpkbd_devices[] = {
+static const struct hid_device_id lenovo_devices[] = {
 	{ HID_USB_DEVICE(USB_VENDOR_ID_LENOVO, USB_DEVICE_ID_LENOVO_TPKBD) },
 	{ }
 };
 
-MODULE_DEVICE_TABLE(hid, tpkbd_devices);
+MODULE_DEVICE_TABLE(hid, lenovo_devices);
 
-static struct hid_driver tpkbd_driver = {
-	.name = "lenovo_tpkbd",
-	.id_table = tpkbd_devices,
-	.input_mapping = tpkbd_input_mapping,
-	.probe = tpkbd_probe,
-	.remove = tpkbd_remove,
+static struct hid_driver lenovo_driver = {
+	.name = "lenovo",
+	.id_table = lenovo_devices,
+	.input_mapping = lenovo_input_mapping_tpkbd,
+	.probe = lenovo_probe,
+	.remove = lenovo_remove,
 };
-module_hid_driver(tpkbd_driver);
+module_hid_driver(lenovo_driver);
 
 MODULE_LICENSE("GPL");
-- 
2.0.0

^ permalink raw reply related

* [PATCH v5 2/4] HID: lenovo: Prepare support for adding other devices
From: Jamie Lentin @ 2014-07-23 22:30 UTC (permalink / raw)
  To: Jiri Kosina, Antonio Ospite, Hans de Goede
  Cc: linux-input, linux-kernel, Jamie Lentin
In-Reply-To: <1406154648-14000-1-git-send-email-jm@lentin.co.uk>

Ensure all tpkbd specifics are within a postfixed function, the
main functions for the driver should just switch to the appropriate
function depending on product ID. Given this, we can add extra devices
by including extra postfixed functions.

Signed-off-by: Jamie Lentin <jm@lentin.co.uk>
---
 drivers/hid/hid-lenovo.c | 53 ++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 45 insertions(+), 8 deletions(-)

diff --git a/drivers/hid/hid-lenovo.c b/drivers/hid/hid-lenovo.c
index 0320b96..a56b9e7 100644
--- a/drivers/hid/hid-lenovo.c
+++ b/drivers/hid/hid-lenovo.c
@@ -1,5 +1,6 @@
 /*
- *  HID driver for Lenovo ThinkPad USB Keyboard with TrackPoint
+ *  HID driver for Lenovo:
+ *  - ThinkPad USB Keyboard with TrackPoint (tpkbd)
  *
  *  Copyright (c) 2012 Bernhard Seibold
  */
@@ -39,7 +40,7 @@ static int lenovo_input_mapping_tpkbd(struct hid_device *hdev,
 		struct hid_usage *usage, unsigned long **bit, int *max)
 {
 	if (usage->hid == (HID_UP_BUTTON | 0x0010)) {
-		/* mark the device as pointer */
+		/* This sub-device contains trackpoint, mark it */
 		hid_set_drvdata(hdev, (void *)1);
 		map_key_clear(KEY_MICMUTE);
 		return 1;
@@ -47,6 +48,19 @@ static int lenovo_input_mapping_tpkbd(struct hid_device *hdev,
 	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)
+{
+	switch (hdev->product) {
+	case USB_DEVICE_ID_LENOVO_TPKBD:
+		return lenovo_input_mapping_tpkbd(hdev, hi, field,
+							usage, bit, max);
+	default:
+		return 0;
+	}
+}
+
 #undef map_key_clear
 
 static int lenovo_features_set_tpkbd(struct hid_device *hdev)
@@ -337,6 +351,15 @@ static int lenovo_probe_tpkbd(struct hid_device *hdev)
 	char *name_mute, *name_micmute;
 	int i;
 
+	/*
+	 * Only register extra settings against subdevice where input_mapping
+	 * set drvdata to 1, i.e. the trackpoint.
+	 */
+	if (!hid_get_drvdata(hdev))
+		return 0;
+
+	hid_set_drvdata(hdev, NULL);
+
 	/* Validate required reports. */
 	for (i = 0; i < 4; i++) {
 		if (!hid_validate_values(hdev, HID_FEATURE_REPORT, 4, i, 1))
@@ -409,12 +432,16 @@ static int lenovo_probe(struct hid_device *hdev,
 		goto err;
 	}
 
-	if (hid_get_drvdata(hdev)) {
-		hid_set_drvdata(hdev, NULL);
+	switch (hdev->product) {
+	case USB_DEVICE_ID_LENOVO_TPKBD:
 		ret = lenovo_probe_tpkbd(hdev);
-		if (ret)
-			goto err_hid;
+		break;
+	default:
+		ret = 0;
+		break;
 	}
+	if (ret)
+		goto err_hid;
 
 	return 0;
 err_hid:
@@ -427,6 +454,13 @@ static void lenovo_remove_tpkbd(struct hid_device *hdev)
 {
 	struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev);
 
+	/*
+	 * Only the trackpoint half of the keyboard has drvdata and stuff that
+	 * needs unregistering.
+	 */
+	if (data_pointer == NULL)
+		return;
+
 	sysfs_remove_group(&hdev->dev.kobj,
 			&lenovo_attr_group_tpkbd);
 
@@ -438,8 +472,11 @@ static void lenovo_remove_tpkbd(struct hid_device *hdev)
 
 static void lenovo_remove(struct hid_device *hdev)
 {
-	if (hid_get_drvdata(hdev))
+	switch (hdev->product) {
+	case USB_DEVICE_ID_LENOVO_TPKBD:
 		lenovo_remove_tpkbd(hdev);
+		break;
+	}
 
 	hid_hw_stop(hdev);
 }
@@ -454,7 +491,7 @@ MODULE_DEVICE_TABLE(hid, lenovo_devices);
 static struct hid_driver lenovo_driver = {
 	.name = "lenovo",
 	.id_table = lenovo_devices,
-	.input_mapping = lenovo_input_mapping_tpkbd,
+	.input_mapping = lenovo_input_mapping,
 	.probe = lenovo_probe,
 	.remove = lenovo_remove,
 };
-- 
2.0.0

^ permalink raw reply related

* [PATCH v5 3/4] HID: lenovo: Style fixes
From: Jamie Lentin @ 2014-07-23 22:30 UTC (permalink / raw)
  To: Jiri Kosina, Antonio Ospite, Hans de Goede
  Cc: linux-input, linux-kernel, Jamie Lentin
In-Reply-To: <1406154648-14000-1-git-send-email-jm@lentin.co.uk>

Signed-off-by: Jamie Lentin <jm@lentin.co.uk>
---
 drivers/hid/hid-lenovo.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/hid/hid-lenovo.c b/drivers/hid/hid-lenovo.c
index a56b9e7..6d55939 100644
--- a/drivers/hid/hid-lenovo.c
+++ b/drivers/hid/hid-lenovo.c
@@ -350,6 +350,7 @@ static int lenovo_probe_tpkbd(struct hid_device *hdev)
 	size_t name_sz = strlen(dev_name(dev)) + 16;
 	char *name_mute, *name_micmute;
 	int i;
+	int ret;
 
 	/*
 	 * Only register extra settings against subdevice where input_mapping
@@ -368,10 +369,9 @@ static int lenovo_probe_tpkbd(struct hid_device *hdev)
 	if (!hid_validate_values(hdev, HID_OUTPUT_REPORT, 3, 0, 2))
 		return -ENODEV;
 
-	if (sysfs_create_group(&hdev->dev.kobj,
-				&lenovo_attr_group_tpkbd)) {
+	ret = sysfs_create_group(&hdev->dev.kobj, &lenovo_attr_group_tpkbd);
+	if (ret)
 		hid_warn(hdev, "Could not create sysfs group\n");
-	}
 
 	data_pointer = devm_kzalloc(&hdev->dev,
 				    sizeof(struct lenovo_drvdata_tpkbd),
-- 
2.0.0

^ permalink raw reply related

* [PATCH v5 4/4] HID: lenovo: Add support for Compact (BT|USB) keyboard
From: Jamie Lentin @ 2014-07-23 22:30 UTC (permalink / raw)
  To: Jiri Kosina, Antonio Ospite, Hans de Goede
  Cc: linux-input, linux-kernel, Jamie Lentin
In-Reply-To: <1406154648-14000-1-git-send-email-jm@lentin.co.uk>

Add support for both ThinkPad Compact Bluetooth Keyboard with
TrackPoint and ThinkPad Compact USB Keyboard with TrackPoint.

Signed-off-by: Jamie Lentin <jm@lentin.co.uk>
---
 Documentation/ABI/testing/sysfs-driver-hid-lenovo |  12 ++
 drivers/hid/Kconfig                               |   2 +
 drivers/hid/hid-core.c                            |   2 +
 drivers/hid/hid-ids.h                             |   2 +
 drivers/hid/hid-lenovo.c                          | 208 ++++++++++++++++++++++
 include/linux/hid.h                               |   1 +
 6 files changed, 227 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-driver-hid-lenovo b/Documentation/ABI/testing/sysfs-driver-hid-lenovo
index 57b92cb..53a0725 100644
--- a/Documentation/ABI/testing/sysfs-driver-hid-lenovo
+++ b/Documentation/ABI/testing/sysfs-driver-hid-lenovo
@@ -4,18 +4,21 @@ Contact:	linux-input@vger.kernel.org
 Description:	This controls if mouse clicks should be generated if the trackpoint is quickly pressed. How fast this press has to be
 		is being controlled by press_speed.
 		Values are 0 or 1.
+		Applies to Thinkpad USB Keyboard with TrackPoint.
 
 What:		/sys/bus/usb/devices/<busnum>-<devnum>:<config num>.<interface num>/<hid-bus>:<vendor-id>:<product-id>.<num>/dragging
 Date:		July 2011
 Contact:	linux-input@vger.kernel.org
 Description:	If this setting is enabled, it is possible to do dragging by pressing the trackpoint. This requires press_to_select to be enabled.
 		Values are 0 or 1.
+		Applies to Thinkpad USB Keyboard with TrackPoint.
 
 What:		/sys/bus/usb/devices/<busnum>-<devnum>:<config num>.<interface num>/<hid-bus>:<vendor-id>:<product-id>.<num>/release_to_select
 Date:		July 2011
 Contact:	linux-input@vger.kernel.org
 Description:	For details regarding this setting please refer to http://www.pc.ibm.com/ww/healthycomputing/trkpntb.html
 		Values are 0 or 1.
+		Applies to Thinkpad USB Keyboard with TrackPoint.
 
 What:		/sys/bus/usb/devices/<busnum>-<devnum>:<config num>.<interface num>/<hid-bus>:<vendor-id>:<product-id>.<num>/select_right
 Date:		July 2011
@@ -23,16 +26,25 @@ Contact:	linux-input@vger.kernel.org
 Description:	This setting controls if the mouse click events generated by pressing the trackpoint (if press_to_select is enabled) generate
 		a left or right mouse button click.
 		Values are 0 or 1.
+		Applies to Thinkpad USB Keyboard with TrackPoint.
 
 What:		/sys/bus/usb/devices/<busnum>-<devnum>:<config num>.<interface num>/<hid-bus>:<vendor-id>:<product-id>.<num>/sensitivity
 Date:		July 2011
 Contact:	linux-input@vger.kernel.org
 Description:	This file contains the trackpoint sensitivity.
 		Values are decimal integers from 1 (lowest sensitivity) to 255 (highest sensitivity).
+		Applies to Thinkpad USB Keyboard with TrackPoint.
 
 What:		/sys/bus/usb/devices/<busnum>-<devnum>:<config num>.<interface num>/<hid-bus>:<vendor-id>:<product-id>.<num>/press_speed
 Date:		July 2011
 Contact:	linux-input@vger.kernel.org
 Description:	This setting controls how fast the trackpoint needs to be pressed to generate a mouse click if press_to_select is enabled.
 		Values are decimal integers from 1 (slowest) to 255 (fastest).
+		Applies to Thinkpad USB Keyboard with TrackPoint.
 
+What:		/sys/bus/usb/devices/<busnum>-<devnum>:<config num>.<interface num>/<hid-bus>:<vendor-id>:<product-id>.<num>/fn_lock
+Date:		July 2014
+Contact:	linux-input@vger.kernel.org
+Description:	This setting controls whether Fn Lock is enabled on the keyboard (i.e. if F1 is Mute or F1)
+		Values are 0 or 1
+		Applies to ThinkPad Compact (USB|Bluetooth) Keyboard with TrackPoint.
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 9b7acfc..1e19292 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -343,6 +343,8 @@ config HID_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)
+	- ThinkPad Compact USB Keyboard with TrackPoint (supports Fn keys)
 
 config HID_LOGITECH
 	tristate "Logitech devices" if EXPERT
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 55841bd..81b3bb6 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1798,6 +1798,8 @@ static const struct hid_device_id hid_have_special_driver[] = {
 	{ HID_USB_DEVICE(USB_VENDOR_ID_LCPOWER, USB_DEVICE_ID_LCPOWER_LC1000 ) },
 #if IS_ENABLED(CONFIG_HID_LENOVO)
 	{ HID_USB_DEVICE(USB_VENDOR_ID_LENOVO, USB_DEVICE_ID_LENOVO_TPKBD) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_LENOVO, USB_DEVICE_ID_LENOVO_CUSBKBD) },
+	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LENOVO, USB_DEVICE_ID_LENOVO_CBTKBD) },
 #endif
 	{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_MX3000_RECEIVER) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_S510_RECEIVER) },
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 6d00bb9..d2e2a96 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -560,6 +560,8 @@
 
 #define USB_VENDOR_ID_LENOVO		0x17ef
 #define USB_DEVICE_ID_LENOVO_TPKBD	0x6009
+#define USB_DEVICE_ID_LENOVO_CUSBKBD	0x6047
+#define USB_DEVICE_ID_LENOVO_CBTKBD	0x6048
 
 #define USB_VENDOR_ID_LG		0x1fd2
 #define USB_DEVICE_ID_LG_MULTITOUCH	0x0064
diff --git a/drivers/hid/hid-lenovo.c b/drivers/hid/hid-lenovo.c
index 6d55939..eb5493e 100644
--- a/drivers/hid/hid-lenovo.c
+++ b/drivers/hid/hid-lenovo.c
@@ -1,8 +1,11 @@
 /*
  *  HID driver for Lenovo:
  *  - ThinkPad USB Keyboard with TrackPoint (tpkbd)
+ *  - ThinkPad Compact Bluetooth Keyboard with TrackPoint (cptkbd)
+ *  - ThinkPad Compact USB Keyboard with TrackPoint (cptkbd)
  *
  *  Copyright (c) 2012 Bernhard Seibold
+ *  Copyright (c) 2014 Jamie Lentin <jm@lentin.co.uk>
  */
 
 /*
@@ -33,6 +36,10 @@ struct lenovo_drvdata_tpkbd {
 	int press_speed;
 };
 
+struct lenovo_drvdata_cptkbd {
+	bool fn_lock;
+};
+
 #define map_key_clear(c) hid_map_usage_clear(hi, usage, bit, max, EV_KEY, (c))
 
 static int lenovo_input_mapping_tpkbd(struct hid_device *hdev,
@@ -48,6 +55,49 @@ static int lenovo_input_mapping_tpkbd(struct hid_device *hdev,
 	return 0;
 }
 
+static int lenovo_input_mapping_cptkbd(struct hid_device *hdev,
+		struct hid_input *hi, struct hid_field *field,
+		struct hid_usage *usage, unsigned long **bit, int *max)
+{
+	/* HID_UP_LNVENDOR = USB, HID_UP_MSVENDOR = BT */
+	if ((usage->hid & HID_USAGE_PAGE) == HID_UP_MSVENDOR ||
+	    (usage->hid & HID_USAGE_PAGE) == HID_UP_LNVENDOR) {
+		set_bit(EV_REP, hi->input->evbit);
+		switch (usage->hid & HID_USAGE) {
+		case 0x00f1: /* Fn-F4: Mic mute */
+			map_key_clear(KEY_MICMUTE);
+			return 1;
+		case 0x00f2: /* Fn-F5: Brightness down */
+			map_key_clear(KEY_BRIGHTNESSDOWN);
+			return 1;
+		case 0x00f3: /* Fn-F6: Brightness up */
+			map_key_clear(KEY_BRIGHTNESSUP);
+			return 1;
+		case 0x00f4: /* Fn-F7: External display (projector) */
+			map_key_clear(KEY_SWITCHVIDEOMODE);
+			return 1;
+		case 0x00f5: /* Fn-F8: Wireless */
+			map_key_clear(KEY_WLAN);
+			return 1;
+		case 0x00f6: /* Fn-F9: Control panel */
+			map_key_clear(KEY_CONFIG);
+			return 1;
+		case 0x00f8: /* Fn-F11: View open applications (3 boxes) */
+			map_key_clear(KEY_SCALE);
+			return 1;
+		case 0x00fa: /* Fn-Esc: Fn-lock toggle */
+			map_key_clear(KEY_FN_ESC);
+			return 1;
+		case 0x00fb: /* Fn-F12: Open My computer (6 boxes) USB-only */
+			/* NB: This mapping is invented in raw_event below */
+			map_key_clear(KEY_FILE);
+			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)
@@ -56,6 +106,10 @@ static int lenovo_input_mapping(struct hid_device *hdev,
 	case USB_DEVICE_ID_LENOVO_TPKBD:
 		return lenovo_input_mapping_tpkbd(hdev, hi, field,
 							usage, bit, max);
+	case USB_DEVICE_ID_LENOVO_CUSBKBD:
+	case USB_DEVICE_ID_LENOVO_CBTKBD:
+		return lenovo_input_mapping_cptkbd(hdev, hi, field,
+							usage, bit, max);
 	default:
 		return 0;
 	}
@@ -63,6 +117,103 @@ static int lenovo_input_mapping(struct hid_device *hdev,
 
 #undef map_key_clear
 
+/* Send a config command to the keyboard */
+static int lenovo_send_cmd_cptkbd(struct hid_device *hdev,
+			unsigned char byte2, unsigned char byte3)
+{
+	int ret;
+	unsigned char buf[] = {0x18, byte2, byte3};
+
+	switch (hdev->product) {
+	case USB_DEVICE_ID_LENOVO_CUSBKBD:
+		ret = hid_hw_raw_request(hdev, 0x13, buf, sizeof(buf),
+					HID_FEATURE_REPORT, HID_REQ_SET_REPORT);
+		break;
+	case USB_DEVICE_ID_LENOVO_CBTKBD:
+		ret = hid_hw_output_report(hdev, buf, sizeof(buf));
+		break;
+	default:
+		ret = -EINVAL;
+		break;
+	}
+
+	return ret < 0 ? ret : 0; /* BT returns 0, USB returns sizeof(buf) */
+}
+
+static void lenovo_features_set_cptkbd(struct hid_device *hdev)
+{
+	int ret;
+	struct lenovo_drvdata_cptkbd *cptkbd_data = hid_get_drvdata(hdev);
+
+	ret = lenovo_send_cmd_cptkbd(hdev, 0x05, cptkbd_data->fn_lock);
+	if (ret)
+		hid_err(hdev, "Fn-lock setting failed: %d\n", ret);
+}
+
+static ssize_t attr_fn_lock_show_cptkbd(struct device *dev,
+		struct device_attribute *attr,
+		char *buf)
+{
+	struct hid_device *hdev = container_of(dev, struct hid_device, dev);
+	struct lenovo_drvdata_cptkbd *cptkbd_data = hid_get_drvdata(hdev);
+
+	return snprintf(buf, PAGE_SIZE, "%u\n", cptkbd_data->fn_lock);
+}
+
+static ssize_t attr_fn_lock_store_cptkbd(struct device *dev,
+		struct device_attribute *attr,
+		const char *buf,
+		size_t count)
+{
+	struct hid_device *hdev = container_of(dev, struct hid_device, dev);
+	struct lenovo_drvdata_cptkbd *cptkbd_data = hid_get_drvdata(hdev);
+	int value;
+
+	if (kstrtoint(buf, 10, &value))
+		return -EINVAL;
+	if (value < 0 || value > 1)
+		return -EINVAL;
+
+	cptkbd_data->fn_lock = !!value;
+	lenovo_features_set_cptkbd(hdev);
+
+	return count;
+}
+
+static struct device_attribute dev_attr_fn_lock_cptkbd =
+	__ATTR(fn_lock, S_IWUSR | S_IRUGO,
+			attr_fn_lock_show_cptkbd,
+			attr_fn_lock_store_cptkbd);
+
+static struct attribute *lenovo_attributes_cptkbd[] = {
+	&dev_attr_fn_lock_cptkbd.attr,
+	NULL
+};
+
+static const struct attribute_group lenovo_attr_group_cptkbd = {
+	.attrs = lenovo_attributes_cptkbd,
+};
+
+static int lenovo_raw_event(struct hid_device *hdev,
+			struct hid_report *report, u8 *data, int size)
+{
+	/*
+	 * Compact USB keyboard's Fn-F12 report holds down many other keys, and
+	 * its own key is outside the usage page range. Remove extra
+	 * keypresses and remap to inside usage page.
+	 */
+	if (unlikely(hdev->product == USB_DEVICE_ID_LENOVO_CUSBKBD
+			&& size == 3
+			&& data[0] == 0x15
+			&& data[1] == 0x94
+			&& data[2] == 0x01)) {
+		data[1] = 0x0;
+		data[2] = 0x4;
+	}
+
+	return 0;
+}
+
 static int lenovo_features_set_tpkbd(struct hid_device *hdev)
 {
 	struct hid_report *report;
@@ -415,6 +566,46 @@ static int lenovo_probe_tpkbd(struct hid_device *hdev)
 	return 0;
 }
 
+static int lenovo_probe_cptkbd(struct hid_device *hdev)
+{
+	int ret;
+	struct lenovo_drvdata_cptkbd *cptkbd_data;
+
+	/* All the custom action happens on the USBMOUSE device for USB */
+	if (hdev->product == USB_DEVICE_ID_LENOVO_CUSBKBD
+			&& hdev->type != HID_TYPE_USBMOUSE) {
+		hid_dbg(hdev, "Ignoring keyboard half of device\n");
+		return 0;
+	}
+
+	cptkbd_data = devm_kzalloc(&hdev->dev,
+					sizeof(*cptkbd_data),
+					GFP_KERNEL);
+	if (cptkbd_data == NULL) {
+		hid_err(hdev, "can't alloc keyboard descriptor\n");
+		return -ENOMEM;
+	}
+	hid_set_drvdata(hdev, cptkbd_data);
+
+	/*
+	 * Tell the keyboard a driver understands it, and turn F7, F9, F11 into
+	 * regular keys
+	 */
+	ret = lenovo_send_cmd_cptkbd(hdev, 0x01, 0x03);
+	if (ret)
+		hid_warn(hdev, "Failed to switch F7/9/11 mode: %d\n", ret);
+
+	/* Turn Fn-Lock on by default */
+	cptkbd_data->fn_lock = true;
+	lenovo_features_set_cptkbd(hdev);
+
+	ret = sysfs_create_group(&hdev->dev.kobj, &lenovo_attr_group_cptkbd);
+	if (ret)
+		hid_warn(hdev, "Could not create sysfs group: %d\n", ret);
+
+	return 0;
+}
+
 static int lenovo_probe(struct hid_device *hdev,
 		const struct hid_device_id *id)
 {
@@ -436,6 +627,10 @@ static int lenovo_probe(struct hid_device *hdev,
 	case USB_DEVICE_ID_LENOVO_TPKBD:
 		ret = lenovo_probe_tpkbd(hdev);
 		break;
+	case USB_DEVICE_ID_LENOVO_CUSBKBD:
+	case USB_DEVICE_ID_LENOVO_CBTKBD:
+		ret = lenovo_probe_cptkbd(hdev);
+		break;
 	default:
 		ret = 0;
 		break;
@@ -470,12 +665,22 @@ static void lenovo_remove_tpkbd(struct hid_device *hdev)
 	hid_set_drvdata(hdev, NULL);
 }
 
+static void lenovo_remove_cptkbd(struct hid_device *hdev)
+{
+	sysfs_remove_group(&hdev->dev.kobj,
+			&lenovo_attr_group_cptkbd);
+}
+
 static void lenovo_remove(struct hid_device *hdev)
 {
 	switch (hdev->product) {
 	case USB_DEVICE_ID_LENOVO_TPKBD:
 		lenovo_remove_tpkbd(hdev);
 		break;
+	case USB_DEVICE_ID_LENOVO_CUSBKBD:
+	case USB_DEVICE_ID_LENOVO_CBTKBD:
+		lenovo_remove_cptkbd(hdev);
+		break;
 	}
 
 	hid_hw_stop(hdev);
@@ -483,6 +688,8 @@ static void lenovo_remove(struct hid_device *hdev)
 
 static const struct hid_device_id lenovo_devices[] = {
 	{ HID_USB_DEVICE(USB_VENDOR_ID_LENOVO, USB_DEVICE_ID_LENOVO_TPKBD) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_LENOVO, USB_DEVICE_ID_LENOVO_CUSBKBD) },
+	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LENOVO, USB_DEVICE_ID_LENOVO_CBTKBD) },
 	{ }
 };
 
@@ -494,6 +701,7 @@ static struct hid_driver lenovo_driver = {
 	.input_mapping = lenovo_input_mapping,
 	.probe = lenovo_probe,
 	.remove = lenovo_remove,
+	.raw_event = lenovo_raw_event,
 };
 module_hid_driver(lenovo_driver);
 
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 77632cf..fca74f1 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -167,6 +167,7 @@ struct hid_item {
 #define HID_UP_MSVENDOR		0xff000000
 #define HID_UP_CUSTOM		0x00ff0000
 #define HID_UP_LOGIVENDOR	0xffbc0000
+#define HID_UP_LNVENDOR		0xffa00000
 #define HID_UP_SENSOR		0x00200000
 
 #define HID_USAGE		0x0000ffff
-- 
2.0.0


^ permalink raw reply related

* [PATCH] Input: atmel_mxt_ts - split config update a bit
From: Dmitry Torokhov @ 2014-07-23 23:29 UTC (permalink / raw)
  To: linux-input
  Cc: Nick Dyer, Yufeng Shen, Benson Leung, Daniel Kurtz, linux-kernel

Let's split config update code a bit so it is hopefully a bit easier to
read. Also, the firmware update callback should be the entity releasing
firmware blob, not lower layers.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---

As always, not tested...

 drivers/input/touchscreen/atmel_mxt_ts.c | 270 +++++++++++++++++--------------
 1 file changed, 145 insertions(+), 125 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 5882352..9e0181a 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -1074,6 +1074,133 @@ static u32 mxt_calculate_crc(u8 *base, off_t start_off, off_t end_off)
 	return crc;
 }
 
+static int mxt_prepare_cfg_mem(struct mxt_data *data,
+			       const struct firmware *cfg,
+			       unsigned int data_pos,
+			       unsigned int cfg_start_ofs,
+			       u8 *config_mem,
+			       size_t config_mem_size)
+{
+	struct device *dev = &data->client->dev;
+	struct mxt_object *object;
+	unsigned int type, instance, size, byte_offset;
+	int offset;
+	int ret;
+	int i;
+	u16 reg;
+	u8 val;
+
+	while (data_pos < cfg->size) {
+		/* Read type, instance, length */
+		ret = sscanf(cfg->data + data_pos, "%x %x %x%n",
+			     &type, &instance, &size, &offset);
+		if (ret == 0) {
+			/* EOF */
+			break;
+		} else if (ret != 3) {
+			dev_err(dev, "Bad format: failed to parse object\n");
+			return -EINVAL;
+		}
+		data_pos += offset;
+
+		object = mxt_get_object(data, type);
+		if (!object) {
+			/* Skip object */
+			for (i = 0; i < size; i++) {
+				ret = sscanf(cfg->data + data_pos, "%hhx%n",
+					     &val,
+					     &offset);
+				data_pos += offset;
+			}
+			continue;
+		}
+
+		if (size > mxt_obj_size(object)) {
+			/*
+			 * Either we are in fallback mode due to wrong
+			 * config or config from a later fw version,
+			 * or the file is corrupt or hand-edited.
+			 */
+			dev_warn(dev, "Discarding %zu byte(s) in T%u\n",
+				 size - mxt_obj_size(object), type);
+		} else if (mxt_obj_size(object) > size) {
+			/*
+			 * If firmware is upgraded, new bytes may be added to
+			 * end of objects. It is generally forward compatible
+			 * to zero these bytes - previous behaviour will be
+			 * retained. However this does invalidate the CRC and
+			 * will force fallback mode until the configuration is
+			 * updated. We warn here but do nothing else - the
+			 * malloc has zeroed the entire configuration.
+			 */
+			dev_warn(dev, "Zeroing %zu byte(s) in T%d\n",
+				 mxt_obj_size(object) - size, type);
+		}
+
+		if (instance >= mxt_obj_instances(object)) {
+			dev_err(dev, "Object instances exceeded!\n");
+			return -EINVAL;
+		}
+
+		reg = object->start_address + mxt_obj_size(object) * instance;
+
+		for (i = 0; i < size; i++) {
+			ret = sscanf(cfg->data + data_pos, "%hhx%n",
+				     &val,
+				     &offset);
+			if (ret != 1) {
+				dev_err(dev, "Bad format in T%d\n", type);
+				return -EINVAL;
+			}
+			data_pos += offset;
+
+			if (i > mxt_obj_size(object))
+				continue;
+
+			byte_offset = reg + i - cfg_start_ofs;
+
+			if (byte_offset >= 0 &&
+			    byte_offset <= config_mem_size) {
+				*(config_mem + byte_offset) = val;
+			} else {
+				dev_err(dev, "Bad object: reg:%d, T%d, ofs=%d\n",
+					reg, object->type, byte_offset);
+				return -EINVAL;
+			}
+		}
+	}
+
+	return 0;
+}
+
+static int mxt_upload_cfg_mem(struct mxt_data *data, unsigned int cfg_start,
+			      u8 *config_mem, size_t config_mem_size)
+{
+	unsigned int byte_offset = 0;
+	int error;
+
+	/* Write configuration as blocks */
+	while (byte_offset < config_mem_size) {
+		unsigned int size = config_mem_size - byte_offset;
+
+		if (size > MXT_MAX_BLOCK_WRITE)
+			size = MXT_MAX_BLOCK_WRITE;
+
+		error = __mxt_write_reg(data->client,
+					cfg_start + byte_offset,
+					size, config_mem + byte_offset);
+		if (error) {
+			dev_err(&data->client->dev,
+				"Config write error, ret=%d\n", error);
+			return error;
+		}
+
+		byte_offset += size;
+	}
+
+	return 0;
+}
+
 /*
  * mxt_update_cfg - download configuration to chip
  *
@@ -1097,26 +1224,20 @@ static int mxt_update_cfg(struct mxt_data *data, const struct firmware *cfg)
 {
 	struct device *dev = &data->client->dev;
 	struct mxt_info cfg_info;
-	struct mxt_object *object;
 	int ret;
 	int offset;
 	int data_pos;
-	int byte_offset;
 	int i;
 	int cfg_start_ofs;
 	u32 info_crc, config_crc, calculated_crc;
 	u8 *config_mem;
 	size_t config_mem_size;
-	unsigned int type, instance, size;
-	u8 val;
-	u16 reg;
 
 	mxt_update_crc(data, MXT_COMMAND_REPORTALL, 1);
 
 	if (strncmp(cfg->data, MXT_CFG_MAGIC, strlen(MXT_CFG_MAGIC))) {
 		dev_err(dev, "Unrecognised config file\n");
-		ret = -EINVAL;
-		goto release;
+		return -EINVAL;
 	}
 
 	data_pos = strlen(MXT_CFG_MAGIC);
@@ -1128,8 +1249,7 @@ static int mxt_update_cfg(struct mxt_data *data, const struct firmware *cfg)
 			     &offset);
 		if (ret != 1) {
 			dev_err(dev, "Bad format\n");
-			ret = -EINVAL;
-			goto release;
+			return -EINVAL;
 		}
 
 		data_pos += offset;
@@ -1137,30 +1257,26 @@ static int mxt_update_cfg(struct mxt_data *data, const struct firmware *cfg)
 
 	if (cfg_info.family_id != data->info.family_id) {
 		dev_err(dev, "Family ID mismatch!\n");
-		ret = -EINVAL;
-		goto release;
+		return -EINVAL;
 	}
 
 	if (cfg_info.variant_id != data->info.variant_id) {
 		dev_err(dev, "Variant ID mismatch!\n");
-		ret = -EINVAL;
-		goto release;
+		return -EINVAL;
 	}
 
 	/* Read CRCs */
 	ret = sscanf(cfg->data + data_pos, "%x%n", &info_crc, &offset);
 	if (ret != 1) {
 		dev_err(dev, "Bad format: failed to parse Info CRC\n");
-		ret = -EINVAL;
-		goto release;
+		return -EINVAL;
 	}
 	data_pos += offset;
 
 	ret = sscanf(cfg->data + data_pos, "%x%n", &config_crc, &offset);
 	if (ret != 1) {
 		dev_err(dev, "Bad format: failed to parse Config CRC\n");
-		ret = -EINVAL;
-		goto release;
+		return -EINVAL;
 	}
 	data_pos += offset;
 
@@ -1176,8 +1292,7 @@ static int mxt_update_cfg(struct mxt_data *data, const struct firmware *cfg)
 		} else if (config_crc == data->config_crc) {
 			dev_dbg(dev, "Config CRC 0x%06X: OK\n",
 				 data->config_crc);
-			ret = 0;
-			goto release;
+			return 0;
 		} else {
 			dev_info(dev, "Config CRC 0x%06X: does not match file 0x%06X\n",
 				 data->config_crc, config_crc);
@@ -1196,93 +1311,13 @@ static int mxt_update_cfg(struct mxt_data *data, const struct firmware *cfg)
 	config_mem = kzalloc(config_mem_size, GFP_KERNEL);
 	if (!config_mem) {
 		dev_err(dev, "Failed to allocate memory\n");
-		ret = -ENOMEM;
-		goto release;
+		return -ENOMEM;
 	}
 
-	while (data_pos < cfg->size) {
-		/* Read type, instance, length */
-		ret = sscanf(cfg->data + data_pos, "%x %x %x%n",
-			     &type, &instance, &size, &offset);
-		if (ret == 0) {
-			/* EOF */
-			break;
-		} else if (ret != 3) {
-			dev_err(dev, "Bad format: failed to parse object\n");
-			ret = -EINVAL;
-			goto release_mem;
-		}
-		data_pos += offset;
-
-		object = mxt_get_object(data, type);
-		if (!object) {
-			/* Skip object */
-			for (i = 0; i < size; i++) {
-				ret = sscanf(cfg->data + data_pos, "%hhx%n",
-					     &val,
-					     &offset);
-				data_pos += offset;
-			}
-			continue;
-		}
-
-		if (size > mxt_obj_size(object)) {
-			/*
-			 * Either we are in fallback mode due to wrong
-			 * config or config from a later fw version,
-			 * or the file is corrupt or hand-edited.
-			 */
-			dev_warn(dev, "Discarding %zu byte(s) in T%u\n",
-				 size - mxt_obj_size(object), type);
-		} else if (mxt_obj_size(object) > size) {
-			/*
-			 * If firmware is upgraded, new bytes may be added to
-			 * end of objects. It is generally forward compatible
-			 * to zero these bytes - previous behaviour will be
-			 * retained. However this does invalidate the CRC and
-			 * will force fallback mode until the configuration is
-			 * updated. We warn here but do nothing else - the
-			 * malloc has zeroed the entire configuration.
-			 */
-			dev_warn(dev, "Zeroing %zu byte(s) in T%d\n",
-				 mxt_obj_size(object) - size, type);
-		}
-
-		if (instance >= mxt_obj_instances(object)) {
-			dev_err(dev, "Object instances exceeded!\n");
-			ret = -EINVAL;
-			goto release_mem;
-		}
-
-		reg = object->start_address + mxt_obj_size(object) * instance;
-
-		for (i = 0; i < size; i++) {
-			ret = sscanf(cfg->data + data_pos, "%hhx%n",
-				     &val,
-				     &offset);
-			if (ret != 1) {
-				dev_err(dev, "Bad format in T%d\n", type);
-				ret = -EINVAL;
-				goto release_mem;
-			}
-			data_pos += offset;
-
-			if (i > mxt_obj_size(object))
-				continue;
-
-			byte_offset = reg + i - cfg_start_ofs;
-
-			if ((byte_offset >= 0)
-			    && (byte_offset <= config_mem_size)) {
-				*(config_mem + byte_offset) = val;
-			} else {
-				dev_err(dev, "Bad object: reg:%d, T%d, ofs=%d\n",
-					reg, object->type, byte_offset);
-				ret = -EINVAL;
-				goto release_mem;
-			}
-		}
-	}
+	ret = mxt_prepare_cfg_mem(data, cfg, data_pos, cfg_start_ofs,
+				  config_mem, config_mem_size);
+	if (ret)
+		goto release_mem;
 
 	/* Calculate crc of the received configs (not the raw config file) */
 	if (data->T7_address < cfg_start_ofs) {
@@ -1296,28 +1331,14 @@ static int mxt_update_cfg(struct mxt_data *data, const struct firmware *cfg)
 					   data->T7_address - cfg_start_ofs,
 					   config_mem_size);
 
-	if (config_crc > 0 && (config_crc != calculated_crc))
+	if (config_crc > 0 && config_crc != calculated_crc)
 		dev_warn(dev, "Config CRC error, calculated=%06X, file=%06X\n",
 			 calculated_crc, config_crc);
 
-	/* Write configuration as blocks */
-	byte_offset = 0;
-	while (byte_offset < config_mem_size) {
-		size = config_mem_size - byte_offset;
-
-		if (size > MXT_MAX_BLOCK_WRITE)
-			size = MXT_MAX_BLOCK_WRITE;
-
-		ret = __mxt_write_reg(data->client,
-				      cfg_start_ofs + byte_offset,
-				      size, config_mem + byte_offset);
-		if (ret != 0) {
-			dev_err(dev, "Config write error, ret=%d\n", ret);
-			goto release_mem;
-		}
-
-		byte_offset += size;
-	}
+	ret = mxt_upload_cfg_mem(data, cfg_start_ofs,
+				 config_mem, config_mem_size);
+	if (ret)
+		goto release_mem;
 
 	mxt_update_crc(data, MXT_COMMAND_BACKUPNV, MXT_BACKUP_VALUE);
 
@@ -1329,8 +1350,6 @@ static int mxt_update_cfg(struct mxt_data *data, const struct firmware *cfg)
 
 release_mem:
 	kfree(config_mem);
-release:
-	release_firmware(cfg);
 	return ret;
 }
 
@@ -1648,6 +1667,7 @@ static int mxt_configure_objects(struct mxt_data *data,
 static void mxt_config_cb(const struct firmware *cfg, void *ctx)
 {
 	mxt_configure_objects(ctx, cfg);
+	release_firmware(cfg);
 }
 
 static int mxt_initialize(struct mxt_data *data)
-- 
2.0.0.526.g5318336


-- 
Dmitry

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox