* Re: [PATCH] HID: use standard debug APIs
From: Benjamin Tissoires @ 2023-02-06 15:24 UTC (permalink / raw)
To: Jiri Kosina, Thomas Weißschuh; +Cc: linux-input, linux-kernel
In-Reply-To: <20221223-hid-dbg-v1-1-5dcf8794f7f9@weissschuh.net>
On Fri, 23 Dec 2022 21:30:19 +0000, Thomas Weißschuh wrote:
> The custom "debug" module parameter is fairly inflexible.
> It can only manage debugging for all calls dbg_hid() at the same time.
>
> Furthermore it creates a mismatch between calls to hid_dbg() which can
> be managed by CONFIG_DYNAMIC_DEBUG and dbg_hid() which is managed by the
> module parameter.
>
> [...]
Applied to hid/hid.git (for-6.3/hid-core), thanks!
[1/1] HID: use standard debug APIs
https://git.kernel.org/hid/hid/c/3f16ba1c0768
Cheers,
--
Benjamin Tissoires <benjamin.tissoires@redhat.com>
^ permalink raw reply
* Re: [PATCH v3] HID: kye: Add support for all kye tablets
From: Benjamin Tissoires @ 2023-02-06 15:32 UTC (permalink / raw)
To: yangfl; +Cc: Jiri Kosina, linux-input, linux-kernel
In-Reply-To: <20230120115308.2610-1-yangfl@users.noreply.github.com>
Hi david,
On Jan 20 2023, yangfl wrote:
> From: David Yang <mmyangfl@gmail.com>
>
> Genius digitizer tablets send incorrect report descriptor by default. This
> patch collects their physical data from Windows driver, and use it to
> generate correct HID reports.
>
> Signed-off-by: David Yang <mmyangfl@gmail.com>
Thanks for the patch and sorry for not going back to you sooner.
The problem I have with this patch is that it is massive, it changes
behaviour of existing devices and it adds more. This whole makes it
impossible to review.
We don't mind refactoring of code, as long as they are self-contained.
So please make refactoring/reordering in their own patches, with a clear
comment whether or not you are changing the code path. Splitting this
series should hopefully get you some more constructive feedback and
eventually get it merged.
Cheers,
Benjamin
> ---
> v2: fix missing rsize assignment
> v3: fix geometry
> drivers/hid/hid-ids.h | 9 +-
> drivers/hid/hid-kye.c | 886 ++++++++++++++++-----------------------
> drivers/hid/hid-quirks.c | 14 +-
> 3 files changed, 392 insertions(+), 517 deletions(-)
>
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index 0f8c11842a3a..11153d897db7 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -712,12 +712,19 @@
> #define USB_DEVICE_ID_GENIUS_MANTICORE 0x0153
> #define USB_DEVICE_ID_GENIUS_GX_IMPERATOR 0x4018
> #define USB_DEVICE_ID_KYE_GPEN_560 0x5003
> +#define USB_DEVICE_ID_KYE_EASYPEN_M406 0x5005
> +#define USB_DEVICE_ID_KYE_EASYPEN_M506 0x500F
> #define USB_DEVICE_ID_KYE_EASYPEN_I405X 0x5010
> #define USB_DEVICE_ID_KYE_MOUSEPEN_I608X 0x5011
> -#define USB_DEVICE_ID_KYE_MOUSEPEN_I608X_V2 0x501a
> +#define USB_DEVICE_ID_KYE_EASYPEN_M406W 0x5012
> #define USB_DEVICE_ID_KYE_EASYPEN_M610X 0x5013
> +#define USB_DEVICE_ID_KYE_EASYPEN_340 0x5014
> #define USB_DEVICE_ID_KYE_PENSKETCH_M912 0x5015
> +#define USB_DEVICE_ID_KYE_MOUSEPEN_M508WX 0x5016
> +#define USB_DEVICE_ID_KYE_MOUSEPEN_M508X 0x5017
> #define USB_DEVICE_ID_KYE_EASYPEN_M406XE 0x5019
> +#define USB_DEVICE_ID_KYE_MOUSEPEN_I608X_V2 0x501A
> +#define USB_DEVICE_ID_KYE_PENSKETCH_T609A 0x501B
>
> #define USB_VENDOR_ID_LABTEC 0x1020
> #define USB_DEVICE_ID_LABTEC_WIRELESS_KEYBOARD 0x0006
> diff --git a/drivers/hid/hid-kye.c b/drivers/hid/hid-kye.c
> index da903138eee4..42a7182ffb3a 100644
> --- a/drivers/hid/hid-kye.c
> +++ b/drivers/hid/hid-kye.c
> @@ -5,361 +5,267 @@
> * Copyright (c) 2009 Jiri Kosina
> * Copyright (c) 2009 Tomas Hanak
> * Copyright (c) 2012 Nikolai Kondrashov
> + * Copyright (c) 2022 David Yang
> */
>
> -/*
> - */
> -
> +#include <asm-generic/unaligned.h>
> #include <linux/device.h>
> #include <linux/hid.h>
> #include <linux/module.h>
>
> #include "hid-ids.h"
>
> -/* Original EasyPen i405X report descriptor size */
> -#define EASYPEN_I405X_RDESC_ORIG_SIZE 476
> -
> -/* Fixed EasyPen i405X report descriptor */
> -static __u8 easypen_i405x_rdesc_fixed[] = {
> - 0x06, 0x00, 0xFF, /* Usage Page (FF00h), */
> - 0x09, 0x01, /* Usage (01h), */
> - 0xA1, 0x01, /* Collection (Application), */
> - 0x85, 0x05, /* Report ID (5), */
> - 0x09, 0x01, /* Usage (01h), */
> - 0x15, 0x80, /* Logical Minimum (-128), */
> - 0x25, 0x7F, /* Logical Maximum (127), */
> - 0x75, 0x08, /* Report Size (8), */
> - 0x95, 0x07, /* Report Count (7), */
> - 0xB1, 0x02, /* Feature (Variable), */
> - 0xC0, /* End Collection, */
> - 0x05, 0x0D, /* Usage Page (Digitizer), */
> - 0x09, 0x01, /* Usage (Digitizer), */
> - 0xA1, 0x01, /* Collection (Application), */
> - 0x85, 0x10, /* Report ID (16), */
> - 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, 0x04, /* Report Count (4), */
> - 0x81, 0x03, /* Input (Constant, Variable), */
> - 0x09, 0x32, /* Usage (In Range), */
> - 0x95, 0x01, /* Report Count (1), */
> - 0x81, 0x02, /* Input (Variable), */
> - 0x75, 0x10, /* Report Size (16), */
> - 0x95, 0x01, /* Report Count (1), */
> - 0xA4, /* Push, */
> - 0x05, 0x01, /* Usage Page (Desktop), */
> - 0x55, 0xFD, /* Unit Exponent (-3), */
> - 0x65, 0x13, /* Unit (Inch), */
> - 0x34, /* Physical Minimum (0), */
> - 0x09, 0x30, /* Usage (X), */
> - 0x46, 0x7C, 0x15, /* Physical Maximum (5500), */
> - 0x26, 0x00, 0x37, /* Logical Maximum (14080), */
> - 0x81, 0x02, /* Input (Variable), */
> - 0x09, 0x31, /* Usage (Y), */
> - 0x46, 0xA0, 0x0F, /* Physical Maximum (4000), */
> - 0x26, 0x00, 0x28, /* Logical Maximum (10240), */
> - 0x81, 0x02, /* Input (Variable), */
> - 0xB4, /* Pop, */
> - 0x09, 0x30, /* Usage (Tip Pressure), */
> - 0x26, 0xFF, 0x03, /* Logical Maximum (1023), */
> - 0x81, 0x02, /* Input (Variable), */
> - 0xC0, /* End Collection, */
> - 0xC0 /* End Collection */
> +/* Data gathered from Database/VID0458_PID????/Vista/TBoard/default.xml in ioTablet driver
> + *
> + * TODO:
> + * - Add battery and sleep support for EasyPen M406W and MousePen M508WX
> + * - Investigate ScrollZ.MiceFMT buttons of EasyPen M406
> + */
> +
> +static const __u8 easypen_m406_control_rdesc[] = {
> + 0x05, 0x0C, /* Usage Page (Consumer), */
> + 0x09, 0x01, /* Usage (Consumer Control), */
> + 0xA1, 0x01, /* Collection (Application), */
> + 0x85, 0x12, /* Report ID (18), */
> + 0x0A, 0x45, 0x02, /* Usage (AC Rotate), */
> + 0x09, 0x40, /* Usage (Menu), */
> + 0x0A, 0x2F, 0x02, /* Usage (AC Zoom), */
> + 0x0A, 0x46, 0x02, /* Usage (AC Resize), */
> + 0x0A, 0x1A, 0x02, /* Usage (AC Undo), */
> + 0x0A, 0x6A, 0x02, /* Usage (AC Delete), */
> + 0x0A, 0x24, 0x02, /* Usage (AC Back), */
> + 0x0A, 0x25, 0x02, /* Usage (AC Forward), */
> + 0x14, /* Logical Minimum (0), */
> + 0x25, 0x01, /* Logical Maximum (1), */
> + 0x75, 0x01, /* Report Size (1), */
> + 0x95, 0x08, /* Report Count (8), */
> + 0x81, 0x02, /* Input (Variable), */
> + 0x95, 0x30, /* Report Count (48), */
> + 0x81, 0x01, /* Input (Constant), */
> + 0xC0 /* End Collection */
> };
>
> -/* Original MousePen i608X report descriptor size */
> -#define MOUSEPEN_I608X_RDESC_ORIG_SIZE 476
> -
> -/* Fixed MousePen i608X report descriptor */
> -static __u8 mousepen_i608x_rdesc_fixed[] = {
> - 0x06, 0x00, 0xFF, /* Usage Page (FF00h), */
> - 0x09, 0x01, /* Usage (01h), */
> - 0xA1, 0x01, /* Collection (Application), */
> - 0x85, 0x05, /* Report ID (5), */
> - 0x09, 0x01, /* Usage (01h), */
> - 0x15, 0x80, /* Logical Minimum (-128), */
> - 0x25, 0x7F, /* Logical Maximum (127), */
> - 0x75, 0x08, /* Report Size (8), */
> - 0x95, 0x07, /* Report Count (7), */
> - 0xB1, 0x02, /* Feature (Variable), */
> - 0xC0, /* End Collection, */
> - 0x05, 0x0D, /* Usage Page (Digitizer), */
> - 0x09, 0x01, /* Usage (Digitizer), */
> - 0xA1, 0x01, /* Collection (Application), */
> - 0x85, 0x10, /* Report ID (16), */
> - 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, 0x04, /* Report Count (4), */
> - 0x81, 0x03, /* Input (Constant, Variable), */
> - 0x09, 0x32, /* Usage (In Range), */
> - 0x95, 0x01, /* Report Count (1), */
> - 0x81, 0x02, /* Input (Variable), */
> - 0x75, 0x10, /* Report Size (16), */
> - 0x95, 0x01, /* Report Count (1), */
> - 0xA4, /* Push, */
> - 0x05, 0x01, /* Usage Page (Desktop), */
> - 0x55, 0xFD, /* Unit Exponent (-3), */
> - 0x65, 0x13, /* Unit (Inch), */
> - 0x34, /* Physical Minimum (0), */
> - 0x09, 0x30, /* Usage (X), */
> - 0x46, 0x40, 0x1F, /* Physical Maximum (8000), */
> - 0x26, 0x00, 0x50, /* Logical Maximum (20480), */
> - 0x81, 0x02, /* Input (Variable), */
> - 0x09, 0x31, /* Usage (Y), */
> - 0x46, 0x70, 0x17, /* Physical Maximum (6000), */
> - 0x26, 0x00, 0x3C, /* Logical Maximum (15360), */
> - 0x81, 0x02, /* Input (Variable), */
> - 0xB4, /* Pop, */
> - 0x09, 0x30, /* Usage (Tip Pressure), */
> - 0x26, 0xFF, 0x03, /* Logical Maximum (1023), */
> - 0x81, 0x02, /* Input (Variable), */
> - 0xC0, /* End Collection, */
> - 0xC0, /* End Collection, */
> - 0x05, 0x01, /* Usage Page (Desktop), */
> - 0x09, 0x02, /* Usage (Mouse), */
> - 0xA1, 0x01, /* Collection (Application), */
> - 0x85, 0x11, /* Report ID (17), */
> - 0x09, 0x01, /* Usage (Pointer), */
> - 0xA0, /* Collection (Physical), */
> - 0x14, /* Logical Minimum (0), */
> - 0xA4, /* Push, */
> - 0x05, 0x09, /* Usage Page (Button), */
> - 0x75, 0x01, /* Report Size (1), */
> - 0x19, 0x01, /* Usage Minimum (01h), */
> - 0x29, 0x03, /* Usage Maximum (03h), */
> - 0x25, 0x01, /* Logical Maximum (1), */
> - 0x95, 0x03, /* Report Count (3), */
> - 0x81, 0x02, /* Input (Variable), */
> - 0x95, 0x05, /* Report Count (5), */
> - 0x81, 0x01, /* Input (Constant), */
> - 0xB4, /* Pop, */
> - 0x95, 0x01, /* Report Count (1), */
> - 0xA4, /* Push, */
> - 0x55, 0xFD, /* Unit Exponent (-3), */
> - 0x65, 0x13, /* Unit (Inch), */
> - 0x34, /* Physical Minimum (0), */
> - 0x75, 0x10, /* Report Size (16), */
> - 0x09, 0x30, /* Usage (X), */
> - 0x46, 0x40, 0x1F, /* Physical Maximum (8000), */
> - 0x26, 0x00, 0x50, /* Logical Maximum (20480), */
> - 0x81, 0x02, /* Input (Variable), */
> - 0x09, 0x31, /* Usage (Y), */
> - 0x46, 0x70, 0x17, /* Physical Maximum (6000), */
> - 0x26, 0x00, 0x3C, /* Logical Maximum (15360), */
> - 0x81, 0x02, /* Input (Variable), */
> - 0xB4, /* Pop, */
> - 0x75, 0x08, /* Report Size (8), */
> - 0x09, 0x38, /* Usage (Wheel), */
> - 0x15, 0xFF, /* Logical Minimum (-1), */
> - 0x25, 0x01, /* Logical Maximum (1), */
> - 0x81, 0x06, /* Input (Variable, Relative), */
> - 0x81, 0x01, /* Input (Constant), */
> - 0xC0, /* End Collection, */
> - 0xC0 /* End Collection */
> +static const __u8 easypen_m506_control_rdesc[] = {
> + 0x05, 0x0C, /* Usage Page (Consumer), */
> + 0x09, 0x01, /* Usage (Consumer Control), */
> + 0xA1, 0x01, /* Collection (Application), */
> + 0x85, 0x12, /* Report ID (18), */
> + 0x0A, 0x6A, 0x02, /* Usage (AC Delete), */
> + 0x0A, 0x1A, 0x02, /* Usage (AC Undo), */
> + 0x0A, 0x2D, 0x02, /* Usage (AC Zoom In), */
> + 0x0A, 0x2E, 0x02, /* Usage (AC Zoom Out), */
> + 0x14, /* Logical Minimum (0), */
> + 0x25, 0x01, /* Logical Maximum (1), */
> + 0x75, 0x01, /* Report Size (1), */
> + 0x95, 0x04, /* Report Count (4), */
> + 0x81, 0x02, /* Input (Variable), */
> + 0x95, 0x34, /* Report Count (52), */
> + 0x81, 0x01, /* Input (Constant), */
> + 0xC0 /* End Collection */
> };
>
> -/* Original MousePen i608X v2 report descriptor size */
> -#define MOUSEPEN_I608X_V2_RDESC_ORIG_SIZE 482
> +static const __u8 easypen_m406w_control_rdesc[] = {
> + 0x05, 0x0C, /* Usage Page (Consumer), */
> + 0x09, 0x01, /* Usage (Consumer Control), */
> + 0xA1, 0x01, /* Collection (Application), */
> + 0x85, 0x12, /* Report ID (18), */
> + 0x0A, 0x6A, 0x02, /* Usage (AC Delete), */
> + 0x0A, 0x1A, 0x02, /* Usage (AC Undo), */
> + 0x0A, 0x01, 0x02, /* Usage (AC New), */
> + 0x09, 0x40, /* Usage (Menu), */
> + 0x14, /* Logical Minimum (0), */
> + 0x25, 0x01, /* Logical Maximum (1), */
> + 0x75, 0x01, /* Report Size (1), */
> + 0x95, 0x04, /* Report Count (4), */
> + 0x81, 0x02, /* Input (Variable), */
> + 0x95, 0x34, /* Report Count (52), */
> + 0x81, 0x01, /* Input (Constant), */
> + 0xC0 /* End Collection */
> +};
>
> -/* Fixed MousePen i608X v2 report descriptor */
> -static __u8 mousepen_i608x_v2_rdesc_fixed[] = {
> - 0x06, 0x00, 0xFF, /* Usage Page (FF00h), */
> - 0x09, 0x01, /* Usage (01h), */
> - 0xA1, 0x01, /* Collection (Application), */
> - 0x85, 0x05, /* Report ID (5), */
> - 0x09, 0x01, /* Usage (01h), */
> - 0x15, 0x80, /* Logical Minimum (-128), */
> - 0x25, 0x7F, /* Logical Maximum (127), */
> - 0x75, 0x08, /* Report Size (8), */
> - 0x95, 0x07, /* Report Count (7), */
> - 0xB1, 0x02, /* Feature (Variable), */
> - 0xC0, /* End Collection, */
> - 0x05, 0x0D, /* Usage Page (Digitizer), */
> - 0x09, 0x01, /* Usage (Digitizer), */
> - 0xA1, 0x01, /* Collection (Application), */
> - 0x85, 0x10, /* Report ID (16), */
> - 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, 0x04, /* Report Count (4), */
> - 0x81, 0x03, /* Input (Constant, Variable), */
> - 0x09, 0x32, /* Usage (In Range), */
> - 0x95, 0x01, /* Report Count (1), */
> - 0x81, 0x02, /* Input (Variable), */
> - 0x75, 0x10, /* Report Size (16), */
> - 0x95, 0x01, /* Report Count (1), */
> - 0xA4, /* Push, */
> - 0x05, 0x01, /* Usage Page (Desktop), */
> - 0x55, 0xFD, /* Unit Exponent (-3), */
> - 0x65, 0x13, /* Unit (Inch), */
> - 0x34, /* Physical Minimum (0), */
> - 0x09, 0x30, /* Usage (X), */
> - 0x46, 0x40, 0x1F, /* Physical Maximum (8000), */
> - 0x27, 0x00, 0xA0, 0x00, 0x00, /* Logical Maximum (40960), */
> - 0x81, 0x02, /* Input (Variable), */
> - 0x09, 0x31, /* Usage (Y), */
> - 0x46, 0x70, 0x17, /* Physical Maximum (6000), */
> - 0x26, 0x00, 0x78, /* Logical Maximum (30720), */
> - 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, */
> - 0x05, 0x01, /* Usage Page (Desktop), */
> - 0x09, 0x02, /* Usage (Mouse), */
> - 0xA1, 0x01, /* Collection (Application), */
> - 0x85, 0x11, /* Report ID (17), */
> - 0x09, 0x01, /* Usage (Pointer), */
> - 0xA0, /* Collection (Physical), */
> - 0x14, /* Logical Minimum (0), */
> - 0xA4, /* Push, */
> - 0x05, 0x09, /* Usage Page (Button), */
> - 0x75, 0x01, /* Report Size (1), */
> - 0x19, 0x01, /* Usage Minimum (01h), */
> - 0x29, 0x03, /* Usage Maximum (03h), */
> - 0x25, 0x01, /* Logical Maximum (1), */
> - 0x95, 0x03, /* Report Count (3), */
> - 0x81, 0x02, /* Input (Variable), */
> - 0x95, 0x05, /* Report Count (5), */
> - 0x81, 0x01, /* Input (Constant), */
> - 0xB4, /* Pop, */
> - 0x95, 0x01, /* Report Count (1), */
> - 0xA4, /* Push, */
> - 0x55, 0xFD, /* Unit Exponent (-3), */
> - 0x65, 0x13, /* Unit (Inch), */
> - 0x34, /* Physical Minimum (0), */
> - 0x75, 0x10, /* Report Size (16), */
> - 0x09, 0x30, /* Usage (X), */
> - 0x46, 0x40, 0x1F, /* Physical Maximum (8000), */
> - 0x27, 0x00, 0xA0, 0x00, 0x00, /* Logical Maximum (40960), */
> - 0x81, 0x02, /* Input (Variable), */
> - 0x09, 0x31, /* Usage (Y), */
> - 0x46, 0x70, 0x17, /* Physical Maximum (6000), */
> - 0x26, 0x00, 0x78, /* Logical Maximum (30720), */
> - 0x81, 0x02, /* Input (Variable), */
> - 0xB4, /* Pop, */
> - 0x75, 0x08, /* Report Size (8), */
> - 0x09, 0x38, /* Usage (Wheel), */
> - 0x15, 0xFF, /* Logical Minimum (-1), */
> - 0x25, 0x01, /* Logical Maximum (1), */
> - 0x81, 0x06, /* Input (Variable, Relative), */
> - 0x81, 0x01, /* Input (Constant), */
> - 0xC0, /* End Collection, */
> - 0xC0 /* End Collection */
> +static const __u8 easypen_m610x_control_rdesc[] = {
> + 0x05, 0x0C, /* Usage Page (Consumer), */
> + 0x09, 0x01, /* Usage (Consumer Control), */
> + 0xA1, 0x01, /* Collection (Application), */
> + 0x85, 0x12, /* Report ID (18), */
> + 0x0A, 0x1A, 0x02, /* Usage (AC Undo), */
> + 0x0A, 0x79, 0x02, /* Usage (AC Redo Or Repeat), */
> + 0x0A, 0x2D, 0x02, /* Usage (AC Zoom In), */
> + 0x0A, 0x2E, 0x02, /* Usage (AC Zoom Out), */
> + 0x14, /* Logical Minimum (0), */
> + 0x25, 0x01, /* Logical Maximum (1), */
> + 0x75, 0x01, /* Report Size (1), */
> + 0x95, 0x04, /* Report Count (4), */
> + 0x81, 0x02, /* Input (Variable), */
> + 0x95, 0x34, /* Report Count (52), */
> + 0x81, 0x01, /* Input (Constant), */
> + 0xC0 /* End Collection */
> };
>
> -/* Original EasyPen M610X report descriptor size */
> -#define EASYPEN_M610X_RDESC_ORIG_SIZE 476
> +static const __u8 pensketch_m912_control_rdesc[] = {
> + 0x05, 0x0C, /* Usage Page (Consumer), */
> + 0x09, 0x01, /* Usage (Consumer Control), */
> + 0xA1, 0x01, /* Collection (Application), */
> + 0x85, 0x12, /* Report ID (18), */
> + 0x14, /* Logical Minimum (0), */
> + 0x25, 0x01, /* Logical Maximum (1), */
> + 0x75, 0x01, /* Report Size (1), */
> + 0x95, 0x08, /* Report Count (8), */
> + 0x05, 0x0C, /* Usage Page (Consumer), */
> + 0x0A, 0x6A, 0x02, /* Usage (AC Delete), */
> + 0x0A, 0x1A, 0x02, /* Usage (AC Undo), */
> + 0x0A, 0x01, 0x02, /* Usage (AC New), */
> + 0x0A, 0x2F, 0x02, /* Usage (AC Zoom), */
> + 0x0A, 0x25, 0x02, /* Usage (AC Forward), */
> + 0x0A, 0x24, 0x02, /* Usage (AC Back), */
> + 0x0A, 0x2D, 0x02, /* Usage (AC Zoom In), */
> + 0x0A, 0x2E, 0x02, /* Usage (AC Zoom Out), */
> + 0x81, 0x02, /* Input (Variable), */
> + 0x95, 0x30, /* Report Count (48), */
> + 0x81, 0x03, /* Input (Constant, Variable), */
> + 0xC0 /* End Collection */
> +};
>
> -/* Fixed EasyPen M610X report descriptor */
> -static __u8 easypen_m610x_rdesc_fixed[] = {
> - 0x06, 0x00, 0xFF, /* Usage Page (FF00h), */
> - 0x09, 0x01, /* Usage (01h), */
> - 0xA1, 0x01, /* Collection (Application), */
> - 0x85, 0x05, /* Report ID (5), */
> - 0x09, 0x01, /* Usage (01h), */
> - 0x15, 0x80, /* Logical Minimum (-128), */
> - 0x25, 0x7F, /* Logical Maximum (127), */
> - 0x75, 0x08, /* Report Size (8), */
> - 0x95, 0x07, /* Report Count (7), */
> - 0xB1, 0x02, /* Feature (Variable), */
> - 0xC0, /* End Collection, */
> - 0x05, 0x0D, /* Usage Page (Digitizer), */
> - 0x09, 0x01, /* Usage (Digitizer), */
> - 0xA1, 0x01, /* Collection (Application), */
> - 0x85, 0x10, /* Report ID (16), */
> - 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, 0x04, /* Report Count (4), */
> - 0x81, 0x03, /* Input (Constant, Variable), */
> - 0x09, 0x32, /* Usage (In Range), */
> - 0x95, 0x01, /* Report Count (1), */
> - 0x81, 0x02, /* Input (Variable), */
> - 0x75, 0x10, /* Report Size (16), */
> - 0x95, 0x01, /* Report Count (1), */
> - 0xA4, /* Push, */
> - 0x05, 0x01, /* Usage Page (Desktop), */
> - 0x55, 0xFD, /* Unit Exponent (-3), */
> - 0x65, 0x13, /* Unit (Inch), */
> - 0x34, /* Physical Minimum (0), */
> - 0x09, 0x30, /* Usage (X), */
> - 0x46, 0x10, 0x27, /* Physical Maximum (10000), */
> - 0x27, 0x00, 0xA0, 0x00, 0x00, /* Logical Maximum (40960), */
> - 0x81, 0x02, /* Input (Variable), */
> - 0x09, 0x31, /* Usage (Y), */
> - 0x46, 0x6A, 0x18, /* Physical Maximum (6250), */
> - 0x26, 0x00, 0x64, /* Logical Maximum (25600), */
> - 0x81, 0x02, /* Input (Variable), */
> - 0xB4, /* Pop, */
> - 0x09, 0x30, /* Usage (Tip Pressure), */
> - 0x26, 0xFF, 0x03, /* Logical Maximum (1023), */
> - 0x81, 0x02, /* Input (Variable), */
> - 0xC0, /* End Collection, */
> - 0xC0, /* End Collection, */
> - 0x05, 0x0C, /* Usage Page (Consumer), */
> - 0x09, 0x01, /* Usage (Consumer Control), */
> - 0xA1, 0x01, /* Collection (Application), */
> - 0x85, 0x12, /* Report ID (18), */
> - 0x14, /* Logical Minimum (0), */
> - 0x25, 0x01, /* Logical Maximum (1), */
> - 0x75, 0x01, /* Report Size (1), */
> - 0x95, 0x04, /* Report Count (4), */
> - 0x0A, 0x1A, 0x02, /* Usage (AC Undo), */
> - 0x0A, 0x79, 0x02, /* Usage (AC Redo Or Repeat), */
> - 0x0A, 0x2D, 0x02, /* Usage (AC Zoom In), */
> - 0x0A, 0x2E, 0x02, /* Usage (AC Zoom Out), */
> - 0x81, 0x02, /* Input (Variable), */
> - 0x95, 0x01, /* Report Count (1), */
> - 0x75, 0x14, /* Report Size (20), */
> - 0x81, 0x03, /* Input (Constant, Variable), */
> - 0x75, 0x20, /* Report Size (32), */
> - 0x81, 0x03, /* Input (Constant, Variable), */
> - 0xC0 /* End Collection */
> +static const __u8 mousepen_m508wx_control_rdesc[] = {
> + 0x05, 0x0C, /* Usage Page (Consumer), */
> + 0x09, 0x01, /* Usage (Consumer Control), */
> + 0xA1, 0x01, /* Collection (Application), */
> + 0x85, 0x12, /* Report ID (18), */
> + 0x0A, 0x1A, 0x02, /* Usage (AC Undo), */
> + 0x0A, 0x6A, 0x02, /* Usage (AC Delete), */
> + 0x0A, 0x2D, 0x02, /* Usage (AC Zoom In), */
> + 0x0A, 0x2E, 0x02, /* Usage (AC Zoom Out), */
> + 0x14, /* Logical Minimum (0), */
> + 0x25, 0x01, /* Logical Maximum (1), */
> + 0x75, 0x01, /* Report Size (1), */
> + 0x95, 0x04, /* Report Count (4), */
> + 0x81, 0x02, /* Input (Variable), */
> + 0x95, 0x34, /* Report Count (52), */
> + 0x81, 0x01, /* Input (Constant), */
> + 0xC0 /* End Collection */
> };
>
> +static const __u8 mousepen_m508x_control_rdesc[] = {
> + 0x05, 0x0C, /* Usage Page (Consumer), */
> + 0x09, 0x01, /* Usage (Consumer Control), */
> + 0xA1, 0x01, /* Collection (Application), */
> + 0x85, 0x12, /* Report ID (18), */
> + 0x0A, 0x01, 0x02, /* Usage (AC New), */
> + 0x09, 0x40, /* Usage (Menu), */
> + 0x0A, 0x6A, 0x02, /* Usage (AC Delete), */
> + 0x0A, 0x1A, 0x02, /* Usage (AC Undo), */
> + 0x14, /* Logical Minimum (0), */
> + 0x25, 0x01, /* Logical Maximum (1), */
> + 0x75, 0x01, /* Report Size (1), */
> + 0x95, 0x04, /* Report Count (4), */
> + 0x81, 0x02, /* Input (Variable), */
> + 0x81, 0x01, /* Input (Constant), */
> + 0x15, 0xFF, /* Logical Minimum (-1), */
> + 0x95, 0x10, /* Report Count (16), */
> + 0x81, 0x01, /* Input (Constant), */
> + 0x0A, 0x35, 0x02, /* Usage (AC Scroll), */
> + 0x0A, 0x2F, 0x02, /* Usage (AC Zoom), */
> + 0x0A, 0x38, 0x02, /* Usage (AC Pan), */
> + 0x75, 0x08, /* Report Size (8), */
> + 0x95, 0x03, /* Report Count (3), */
> + 0x81, 0x06, /* Input (Variable, Relative), */
> + 0x95, 0x01, /* Report Count (1), */
> + 0x81, 0x01, /* Input (Constant), */
> + 0xC0 /* End Collection */
> +};
>
> -/* Original PenSketch M912 report descriptor size */
> -#define PENSKETCH_M912_RDESC_ORIG_SIZE 482
> +static const __u8 easypen_m406xe_control_rdesc[] = {
> + 0x05, 0x0C, /* Usage Page (Consumer), */
> + 0x09, 0x01, /* Usage (Consumer Control), */
> + 0xA1, 0x01, /* Collection (Application), */
> + 0x85, 0x12, /* Report ID (18), */
> + 0x14, /* Logical Minimum (0), */
> + 0x25, 0x01, /* Logical Maximum (1), */
> + 0x75, 0x01, /* Report Size (1), */
> + 0x95, 0x04, /* Report Count (4), */
> + 0x0A, 0x79, 0x02, /* Usage (AC Redo Or Repeat), */
> + 0x0A, 0x1A, 0x02, /* Usage (AC Undo), */
> + 0x0A, 0x2D, 0x02, /* Usage (AC Zoom In), */
> + 0x0A, 0x2E, 0x02, /* Usage (AC Zoom Out), */
> + 0x81, 0x02, /* Input (Variable), */
> + 0x95, 0x34, /* Report Count (52), */
> + 0x81, 0x03, /* Input (Constant, Variable), */
> + 0xC0 /* End Collection */
> +};
> +
> +static const __u8 pensketch_t609a_control_rdesc[] = {
> + 0x05, 0x0C, /* Usage Page (Consumer), */
> + 0x09, 0x01, /* Usage (Consumer Control), */
> + 0xA1, 0x01, /* Collection (Application), */
> + 0x85, 0x12, /* Report ID (18), */
> + 0x0A, 0x6A, 0x02, /* Usage (AC Delete), */
> + 0x14, /* Logical Minimum (0), */
> + 0x25, 0x01, /* Logical Maximum (1), */
> + 0x75, 0x01, /* Report Size (1), */
> + 0x95, 0x08, /* Report Count (8), */
> + 0x81, 0x02, /* Input (Variable), */
> + 0x95, 0x37, /* Report Count (55), */
> + 0x81, 0x01, /* Input (Constant), */
> + 0xC0 /* End Collection */
> +};
>
> -/* Fixed PenSketch M912 report descriptor */
> -static __u8 pensketch_m912_rdesc_fixed[] = {
> - 0x05, 0x01, /* Usage Page (Desktop), */
> - 0x08, /* Usage (00h), */
> +static const struct kye_tablet_info {
> + __u32 product;
> + __s32 x_logical_maximum;
> + __s32 y_logical_maximum;
> + __s32 pressure_logical_maximum;
> + __s32 x_physical_maximum;
> + __s32 y_physical_maximum;
> + __s8 unit_exponent;
> + __s8 unit;
> + bool has_punk;
> + unsigned int control_rsize;
> + const __u8 *control_rdesc;
> +} kye_tablets_info[] = {
> + {USB_DEVICE_ID_KYE_EASYPEN_M406, /* 0x5005 */
> + 15360, 10240, 1023, 6, 4, 0, 0x13, false,
> + sizeof(easypen_m406_control_rdesc), easypen_m406_control_rdesc},
> + {USB_DEVICE_ID_KYE_EASYPEN_M506, /* 0x500F */
> + 24576, 20480, 1023, 6, 5, 0, 0x13, false,
> + sizeof(easypen_m506_control_rdesc), easypen_m506_control_rdesc},
> + {USB_DEVICE_ID_KYE_EASYPEN_I405X, /* 0x5010 */
> + 14080, 10240, 1023, 55, 40, -1, 0x13, false},
> + {USB_DEVICE_ID_KYE_MOUSEPEN_I608X, /* 0x5011 */
> + 20480, 15360, 2047, 8, 6, 0, 0x13, true},
> + {USB_DEVICE_ID_KYE_EASYPEN_M406W, /* 0x5012 */
> + 15360, 10240, 1023, 6, 4, 0, 0x13, false,
> + sizeof(easypen_m406w_control_rdesc), easypen_m406w_control_rdesc},
> + {USB_DEVICE_ID_KYE_EASYPEN_M610X, /* 0x5013 */
> + 40960, 25600, 1023, 1000, 625, -2, 0x13, false,
> + sizeof(easypen_m610x_control_rdesc), easypen_m610x_control_rdesc},
> + {USB_DEVICE_ID_KYE_EASYPEN_340, /* 0x5014 */
> + 10240, 7680, 1023, 4, 3, 0, 0x13, false},
> + {USB_DEVICE_ID_KYE_PENSKETCH_M912, /* 0x5015 */
> + 61440, 46080, 2047, 12, 9, 0, 0x13, true,
> + sizeof(pensketch_m912_control_rdesc), pensketch_m912_control_rdesc},
> + {USB_DEVICE_ID_KYE_MOUSEPEN_M508WX, /* 0x5016 */
> + 40960, 25600, 2047, 8, 5, 0, 0x13, true,
> + sizeof(mousepen_m508wx_control_rdesc), mousepen_m508wx_control_rdesc},
> + {USB_DEVICE_ID_KYE_MOUSEPEN_M508X, /* 0x5017 */
> + 40960, 25600, 2047, 8, 5, 0, 0x13, true,
> + sizeof(mousepen_m508x_control_rdesc), mousepen_m508x_control_rdesc},
> + {USB_DEVICE_ID_KYE_EASYPEN_M406XE, /* 0x5019 */
> + 15360, 10240, 1023, 6, 4, 0, 0x13, false,
> + sizeof(easypen_m406xe_control_rdesc), easypen_m406xe_control_rdesc},
> + {USB_DEVICE_ID_KYE_MOUSEPEN_I608X_V2, /* 0x501A */
> + 40960, 30720, 2047, 8, 6, 0, 0x13, true},
> + {USB_DEVICE_ID_KYE_PENSKETCH_T609A, /* 0x501B */
> + 43520, 28160, 1023, 85, 55, -1, 0x13, false,
> + sizeof(pensketch_t609a_control_rdesc), pensketch_t609a_control_rdesc},
> + {}
> +};
> +
> +/* Fix indexes in kye_tablet_report_fixup if you change this */
> +static const __u8 kye_tablet_rdesc[] = {
> + 0x06, 0x00, 0xFF, /* Usage Page (FF00h), */
> + 0x09, 0x01, /* Usage (01h), */
> 0xA1, 0x01, /* Collection (Application), */
> 0x85, 0x05, /* Report ID (5), */
> - 0x06, 0x00, 0xFF, /* Usage Page (FF00h), */
> 0x09, 0x01, /* Usage (01h), */
> 0x15, 0x81, /* Logical Minimum (-127), */
> 0x25, 0x7F, /* Logical Maximum (127), */
> @@ -382,30 +288,29 @@ static __u8 pensketch_m912_rdesc_fixed[] = {
> 0x95, 0x03, /* Report Count (3), */
> 0x81, 0x02, /* Input (Variable), */
> 0x95, 0x04, /* Report Count (4), */
> - 0x81, 0x03, /* Input (Constant, Variable), */
> + 0x81, 0x01, /* Input (Constant), */
> 0x09, 0x32, /* Usage (In Range), */
> 0x95, 0x01, /* Report Count (1), */
> 0x81, 0x02, /* Input (Variable), */
> 0x75, 0x10, /* Report Size (16), */
> - 0x95, 0x01, /* Report Count (1), */
> 0xA4, /* Push, */
> 0x05, 0x01, /* Usage Page (Desktop), */
> - 0x55, 0xFD, /* Unit Exponent (-3), */
> - 0x65, 0x13, /* Unit (Inch), */
> - 0x14, /* Logical Minimum (0), */
> - 0x34, /* Physical Minimum (0), */
> 0x09, 0x30, /* Usage (X), */
> - 0x27, 0x00, 0xF0, 0x00, 0x00, /* Logical Maximum (61440), */
> - 0x46, 0xE0, 0x2E, /* Physical Maximum (12000), */
> + 0x27, 0xFF, 0x7F, 0x00, 0x00, /* Logical Maximum (32767), */
> + 0x34, /* Physical Minimum (0), */
> + 0x47, 0x00, 0x00, 0x00, 0x00, /* Physical Maximum (0), */
> + 0x65, 0x11, /* Unit (Centimeter), */
> + 0x55, 0x00, /* Unit Exponent (0), */
> + 0x75, 0x10, /* Report Size (16), */
> 0x81, 0x02, /* Input (Variable), */
> 0x09, 0x31, /* Usage (Y), */
> - 0x27, 0x00, 0xB4, 0x00, 0x00, /* Logical Maximum (46080), */
> - 0x46, 0x28, 0x23, /* Physical Maximum (9000), */
> + 0x27, 0xFF, 0x7F, 0x00, 0x00, /* Logical Maximum (32767), */
> + 0x47, 0x00, 0x00, 0x00, 0x00, /* Physical Maximum (0), */
> 0x81, 0x02, /* Input (Variable), */
> 0xB4, /* Pop, */
> + 0x05, 0x0D, /* Usage Page (Digitizer), */
> 0x09, 0x30, /* Usage (Tip Pressure), */
> - 0x14, /* Logical Minimum (0), */
> - 0x26, 0xFF, 0x07, /* Logical Maximum (2047), */
> + 0x27, 0xFF, 0x07, 0x00, 0x00, /* Logical Maximum (2047), */
> 0x81, 0x02, /* Input (Variable), */
> 0xC0, /* End Collection, */
> 0xC0, /* End Collection, */
> @@ -416,146 +321,47 @@ static __u8 pensketch_m912_rdesc_fixed[] = {
> 0x09, 0x21, /* Usage (Puck), */
> 0xA0, /* Collection (Physical), */
> 0x05, 0x09, /* Usage Page (Button), */
> - 0x75, 0x01, /* Report Size (1), */
> 0x19, 0x01, /* Usage Minimum (01h), */
> 0x29, 0x03, /* Usage Maximum (03h), */
> 0x14, /* Logical Minimum (0), */
> 0x25, 0x01, /* Logical Maximum (1), */
> + 0x75, 0x01, /* Report Size (1), */
> 0x95, 0x03, /* Report Count (3), */
> 0x81, 0x02, /* Input (Variable), */
> 0x95, 0x04, /* Report Count (4), */
> 0x81, 0x01, /* Input (Constant), */
> + 0x05, 0x0D, /* Usage Page (Digitizer), */
> + 0x09, 0x32, /* Usage (In Range), */
> 0x95, 0x01, /* Report Count (1), */
> - 0x0B, 0x32, 0x00, 0x0D, 0x00, /* Usage (Digitizer In Range), */
> - 0x14, /* Logical Minimum (0), */
> - 0x25, 0x01, /* Logical Maximum (1), */
> 0x81, 0x02, /* Input (Variable), */
> - 0xA4, /* Push, */
> 0x05, 0x01, /* Usage Page (Desktop), */
> - 0x75, 0x10, /* Report Size (16), */
> - 0x95, 0x01, /* Report Count (1), */
> - 0x55, 0xFD, /* Unit Exponent (-3), */
> - 0x65, 0x13, /* Unit (Inch), */
> - 0x14, /* Logical Minimum (0), */
> - 0x34, /* Physical Minimum (0), */
> + 0xA4, /* Push, */
> 0x09, 0x30, /* Usage (X), */
> - 0x27, 0x00, 0xF0, 0x00, 0x00, /* Logical Maximum (61440), */
> - 0x46, 0xE0, 0x2E, /* Physical Maximum (12000), */
> + 0x27, 0xFF, 0x7F, 0x00, 0x00, /* Logical Maximum (32767), */
> + 0x34, /* Physical Minimum (0), */
> + 0x47, 0x00, 0x00, 0x00, 0x00, /* Physical Maximum (0), */
> + 0x65, 0x11, /* Unit (Centimeter), */
> + 0x55, 0x00, /* Unit Exponent (0), */
> + 0x75, 0x10, /* Report Size (16), */
> 0x81, 0x02, /* Input (Variable), */
> 0x09, 0x31, /* Usage (Y), */
> - 0x27, 0x00, 0xB4, 0x00, 0x00, /* Logical Maximum (46080), */
> - 0x46, 0x28, 0x23, /* Physical Maximum (9000), */
> + 0x27, 0xFF, 0x7F, 0x00, 0x00, /* Logical Maximum (32767), */
> + 0x47, 0x00, 0x00, 0x00, 0x00, /* Physical Maximum (0), */
> 0x81, 0x02, /* Input (Variable), */
> + 0xB4, /* Pop, */
> 0x09, 0x38, /* Usage (Wheel), */
> + 0x15, 0xFF, /* Logical Minimum (-1), */
> 0x75, 0x08, /* Report Size (8), */
> 0x95, 0x01, /* Report Count (1), */
> - 0x15, 0xFF, /* Logical Minimum (-1), */
> - 0x25, 0x01, /* Logical Maximum (1), */
> - 0x34, /* Physical Minimum (0), */
> - 0x44, /* Physical Maximum (0), */
> 0x81, 0x06, /* Input (Variable, Relative), */
> - 0xB4, /* Pop, */
> + 0x81, 0x01, /* Input (Constant), */
> 0xC0, /* End Collection, */
> - 0xC0, /* End Collection, */
> - 0x05, 0x0C, /* Usage Page (Consumer), */
> - 0x09, 0x01, /* Usage (Consumer Control), */
> - 0xA1, 0x01, /* Collection (Application), */
> - 0x85, 0x12, /* Report ID (18), */
> - 0x14, /* Logical Minimum (0), */
> - 0x25, 0x01, /* Logical Maximum (1), */
> - 0x75, 0x01, /* Report Size (1), */
> - 0x95, 0x08, /* Report Count (8), */
> - 0x05, 0x0C, /* Usage Page (Consumer), */
> - 0x0A, 0x6A, 0x02, /* Usage (AC Delete), */
> - 0x0A, 0x1A, 0x02, /* Usage (AC Undo), */
> - 0x0A, 0x01, 0x02, /* Usage (AC New), */
> - 0x0A, 0x2F, 0x02, /* Usage (AC Zoom), */
> - 0x0A, 0x25, 0x02, /* Usage (AC Forward), */
> - 0x0A, 0x24, 0x02, /* Usage (AC Back), */
> - 0x0A, 0x2D, 0x02, /* Usage (AC Zoom In), */
> - 0x0A, 0x2E, 0x02, /* Usage (AC Zoom Out), */
> - 0x81, 0x02, /* Input (Variable), */
> - 0x95, 0x30, /* Report Count (48), */
> - 0x81, 0x03, /* Input (Constant, Variable), */
> 0xC0 /* End Collection */
> };
>
> -/* Original EasyPen M406XE report descriptor size */
> -#define EASYPEN_M406XE_RDESC_ORIG_SIZE 476
> -
> -/* Fixed EasyPen M406XE report descriptor */
> -static __u8 easypen_m406xe_rdesc_fixed[] = {
> - 0x05, 0x01, /* Usage Page (Desktop), */
> - 0x09, 0x01, /* Usage (01h), */
> - 0xA1, 0x01, /* Collection (Application), */
> - 0x85, 0x05, /* Report ID (5), */
> - 0x09, 0x01, /* Usage (01h), */
> - 0x15, 0x80, /* Logical Minimum (-128), */
> - 0x25, 0x7F, /* Logical Maximum (127), */
> - 0x75, 0x08, /* Report Size (8), */
> - 0x95, 0x07, /* Report Count (7), */
> - 0xB1, 0x02, /* Feature (Variable), */
> - 0xC0, /* End Collection, */
> - 0x05, 0x0D, /* Usage Page (Digitizer), */
> - 0x09, 0x01, /* Usage (Digitizer), */
> - 0xA1, 0x01, /* Collection (Application), */
> - 0x85, 0x10, /* Report ID (16), */
> - 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, 0x04, /* Report Count (4), */
> - 0x81, 0x03, /* Input (Constant, Variable), */
> - 0x09, 0x32, /* Usage (In Range), */
> - 0x95, 0x01, /* Report Count (1), */
> - 0x81, 0x02, /* Input (Variable), */
> - 0x75, 0x10, /* Report Size (16), */
> - 0x95, 0x01, /* Report Count (1), */
> - 0xA4, /* Push, */
> - 0x05, 0x01, /* Usage Page (Desktop), */
> - 0x55, 0xFD, /* Unit Exponent (-3), */
> - 0x65, 0x13, /* Unit (Inch), */
> - 0x34, /* Physical Minimum (0), */
> - 0x09, 0x30, /* Usage (X), */
> - 0x46, 0x70, 0x17, /* Physical Maximum (6000), */
> - 0x26, 0x00, 0x3C, /* Logical Maximum (15360), */
> - 0x81, 0x02, /* Input (Variable), */
> - 0x09, 0x31, /* Usage (Y), */
> - 0x46, 0xA0, 0x0F, /* Physical Maximum (4000), */
> - 0x26, 0x00, 0x28, /* Logical Maximum (10240), */
> - 0x81, 0x02, /* Input (Variable), */
> - 0xB4, /* Pop, */
> - 0x09, 0x30, /* Usage (Tip Pressure), */
> - 0x26, 0xFF, 0x03, /* Logical Maximum (1023), */
> - 0x81, 0x02, /* Input (Variable), */
> - 0xC0, /* End Collection, */
> - 0xC0, /* End Collection */
> - 0x05, 0x0C, /* Usage Page (Consumer), */
> - 0x09, 0x01, /* Usage (Consumer Control), */
> - 0xA1, 0x01, /* Collection (Application), */
> - 0x85, 0x12, /* Report ID (18), */
> - 0x14, /* Logical Minimum (0), */
> - 0x25, 0x01, /* Logical Maximum (1), */
> - 0x75, 0x01, /* Report Size (1), */
> - 0x95, 0x04, /* Report Count (4), */
> - 0x0A, 0x79, 0x02, /* Usage (AC Redo Or Repeat), */
> - 0x0A, 0x1A, 0x02, /* Usage (AC Undo), */
> - 0x0A, 0x2D, 0x02, /* Usage (AC Zoom In), */
> - 0x0A, 0x2E, 0x02, /* Usage (AC Zoom Out), */
> - 0x81, 0x02, /* Input (Variable), */
> - 0x95, 0x34, /* Report Count (52), */
> - 0x81, 0x03, /* Input (Constant, Variable), */
> - 0xC0 /* End Collection */
> -};
> -
> static __u8 *kye_consumer_control_fixup(struct hid_device *hdev, __u8 *rdesc,
> - unsigned int *rsize, int offset, const char *device_name) {
> + unsigned int *rsize, int offset, const char *device_name)
> +{
> /*
> * the fixup that need to be done:
> * - change Usage Maximum in the Consumer Control
> @@ -574,6 +380,60 @@ static __u8 *kye_consumer_control_fixup(struct hid_device *hdev, __u8 *rdesc,
> return rdesc;
> }
>
> +static __u8 *kye_tablet_fixup(struct hid_device *hdev, __u8 *rdesc, unsigned int *rsize)
> +{
> + const struct kye_tablet_info *info;
> + unsigned int newsize;
> +
> + if (*rsize < sizeof(kye_tablet_rdesc)) {
> + hid_warn(hdev,
> + "tablet report size too small, or kye_tablet_rdesc unexpectedly large\n");
> + return rdesc;
> + }
> +
> + for (info = kye_tablets_info; info->product; info++) {
> + if (hdev->product == info->product)
> + break;
> + }
> +
> + if (!info->product) {
> + hid_err(hdev, "tablet unknown, someone forget to add kye_tablet_info entry?\n");
> + return rdesc;
> + }
> +
> + newsize = info->has_punk ? sizeof(kye_tablet_rdesc) : 112;
> + memcpy(rdesc, kye_tablet_rdesc, newsize);
> +
> + put_unaligned_le32(info->x_logical_maximum, rdesc + 66);
> + put_unaligned_le32(info->x_physical_maximum, rdesc + 72);
> + rdesc[77] = info->unit;
> + rdesc[79] = info->unit_exponent;
> + put_unaligned_le32(info->y_logical_maximum, rdesc + 87);
> + put_unaligned_le32(info->y_physical_maximum, rdesc + 92);
> + put_unaligned_le32(info->pressure_logical_maximum, rdesc + 104);
> +
> + if (info->has_punk) {
> + put_unaligned_le32(info->x_logical_maximum, rdesc + 156);
> + put_unaligned_le32(info->x_physical_maximum, rdesc + 162);
> + rdesc[167] = info->unit;
> + rdesc[169] = info->unit_exponent;
> + put_unaligned_le32(info->y_logical_maximum, rdesc + 177);
> + put_unaligned_le32(info->y_physical_maximum, rdesc + 182);
> + }
> +
> + if (info->control_rsize) {
> + if (newsize + info->control_rsize > *rsize)
> + hid_err(hdev, "control rdesc unexpectedly large");
> + else {
> + memcpy(rdesc + newsize, info->control_rdesc, info->control_rsize);
> + newsize += info->control_rsize;
> + }
> + }
> +
> + *rsize = newsize;
> + return rdesc;
> +}
> +
> static __u8 *kye_report_fixup(struct hid_device *hdev, __u8 *rdesc,
> unsigned int *rsize)
> {
> @@ -602,42 +462,6 @@ static __u8 *kye_report_fixup(struct hid_device *hdev, __u8 *rdesc,
> rdesc[74] = 0x08;
> }
> break;
> - case USB_DEVICE_ID_KYE_EASYPEN_I405X:
> - if (*rsize == EASYPEN_I405X_RDESC_ORIG_SIZE) {
> - rdesc = easypen_i405x_rdesc_fixed;
> - *rsize = sizeof(easypen_i405x_rdesc_fixed);
> - }
> - break;
> - case USB_DEVICE_ID_KYE_MOUSEPEN_I608X:
> - if (*rsize == MOUSEPEN_I608X_RDESC_ORIG_SIZE) {
> - rdesc = mousepen_i608x_rdesc_fixed;
> - *rsize = sizeof(mousepen_i608x_rdesc_fixed);
> - }
> - break;
> - case USB_DEVICE_ID_KYE_MOUSEPEN_I608X_V2:
> - if (*rsize == MOUSEPEN_I608X_V2_RDESC_ORIG_SIZE) {
> - rdesc = mousepen_i608x_v2_rdesc_fixed;
> - *rsize = sizeof(mousepen_i608x_v2_rdesc_fixed);
> - }
> - break;
> - case USB_DEVICE_ID_KYE_EASYPEN_M610X:
> - if (*rsize == EASYPEN_M610X_RDESC_ORIG_SIZE) {
> - rdesc = easypen_m610x_rdesc_fixed;
> - *rsize = sizeof(easypen_m610x_rdesc_fixed);
> - }
> - break;
> - case USB_DEVICE_ID_KYE_EASYPEN_M406XE:
> - if (*rsize == EASYPEN_M406XE_RDESC_ORIG_SIZE) {
> - rdesc = easypen_m406xe_rdesc_fixed;
> - *rsize = sizeof(easypen_m406xe_rdesc_fixed);
> - }
> - break;
> - case USB_DEVICE_ID_KYE_PENSKETCH_M912:
> - if (*rsize == PENSKETCH_M912_RDESC_ORIG_SIZE) {
> - rdesc = pensketch_m912_rdesc_fixed;
> - *rsize = sizeof(pensketch_m912_rdesc_fixed);
> - }
> - break;
> case USB_DEVICE_ID_GENIUS_GILA_GAMING_MOUSE:
> rdesc = kye_consumer_control_fixup(hdev, rdesc, rsize, 104,
> "Genius Gila Gaming Mouse");
> @@ -650,18 +474,25 @@ static __u8 *kye_report_fixup(struct hid_device *hdev, __u8 *rdesc,
> rdesc = kye_consumer_control_fixup(hdev, rdesc, rsize, 104,
> "Genius Manticore Keyboard");
> break;
> + case USB_DEVICE_ID_KYE_EASYPEN_M406:
> + case USB_DEVICE_ID_KYE_EASYPEN_M506:
> + case USB_DEVICE_ID_KYE_EASYPEN_I405X:
> + case USB_DEVICE_ID_KYE_MOUSEPEN_I608X:
> + case USB_DEVICE_ID_KYE_EASYPEN_M406W:
> + case USB_DEVICE_ID_KYE_EASYPEN_M610X:
> + case USB_DEVICE_ID_KYE_EASYPEN_340:
> + case USB_DEVICE_ID_KYE_PENSKETCH_M912:
> + case USB_DEVICE_ID_KYE_MOUSEPEN_M508WX:
> + case USB_DEVICE_ID_KYE_MOUSEPEN_M508X:
> + case USB_DEVICE_ID_KYE_EASYPEN_M406XE:
> + case USB_DEVICE_ID_KYE_MOUSEPEN_I608X_V2:
> + case USB_DEVICE_ID_KYE_PENSKETCH_T609A:
> + rdesc = kye_tablet_fixup(hdev, rdesc, rsize);
> + break;
> }
> return rdesc;
> }
>
> -/**
> - * kye_tablet_enable() - Enable fully-functional tablet mode by setting a special feature report.
> - *
> - * @hdev: HID device
> - *
> - * The specific report ID and data were discovered by sniffing the
> - * Windows driver traffic.
> - */
> static int kye_tablet_enable(struct hid_device *hdev)
> {
> struct list_head *list;
> @@ -688,6 +519,14 @@ static int kye_tablet_enable(struct hid_device *hdev)
>
> value = report->field[0]->value;
>
> + /*
> + * The code is for DataFormat 2 of config xml. They have no obvious
> + * meaning (at least not configurable in Windows driver) except enabling
> + * fully-functional tablet mode (absolute mouse).
> + *
> + * Though there're magic codes for DataFormat 3 and 4, no devices use
> + * these DataFormats.
> + */
> value[0] = 0x12;
> value[1] = 0x10;
> value[2] = 0x11;
> @@ -717,12 +556,19 @@ static int kye_probe(struct hid_device *hdev, const struct hid_device_id *id)
> }
>
> switch (id->product) {
> + case USB_DEVICE_ID_KYE_EASYPEN_M406:
> + case USB_DEVICE_ID_KYE_EASYPEN_M506:
> case USB_DEVICE_ID_KYE_EASYPEN_I405X:
> case USB_DEVICE_ID_KYE_MOUSEPEN_I608X:
> - case USB_DEVICE_ID_KYE_MOUSEPEN_I608X_V2:
> + case USB_DEVICE_ID_KYE_EASYPEN_M406W:
> case USB_DEVICE_ID_KYE_EASYPEN_M610X:
> - case USB_DEVICE_ID_KYE_EASYPEN_M406XE:
> + case USB_DEVICE_ID_KYE_EASYPEN_340:
> case USB_DEVICE_ID_KYE_PENSKETCH_M912:
> + case USB_DEVICE_ID_KYE_MOUSEPEN_M508WX:
> + case USB_DEVICE_ID_KYE_MOUSEPEN_M508X:
> + case USB_DEVICE_ID_KYE_EASYPEN_M406XE:
> + case USB_DEVICE_ID_KYE_MOUSEPEN_I608X_V2:
> + case USB_DEVICE_ID_KYE_PENSKETCH_T609A:
> ret = kye_tablet_enable(hdev);
> if (ret) {
> hid_err(hdev, "tablet enabling failed\n");
> @@ -748,24 +594,38 @@ static int kye_probe(struct hid_device *hdev, const struct hid_device_id *id)
>
> static const struct hid_device_id kye_devices[] = {
> { HID_USB_DEVICE(USB_VENDOR_ID_KYE, USB_DEVICE_ID_KYE_ERGO_525V) },
> + { HID_USB_DEVICE(USB_VENDOR_ID_KYE,
> + USB_DEVICE_ID_GENIUS_GILA_GAMING_MOUSE) },
> + { HID_USB_DEVICE(USB_VENDOR_ID_KYE,
> + USB_DEVICE_ID_GENIUS_MANTICORE) },
> + { HID_USB_DEVICE(USB_VENDOR_ID_KYE,
> + USB_DEVICE_ID_GENIUS_GX_IMPERATOR) },
> + { HID_USB_DEVICE(USB_VENDOR_ID_KYE,
> + USB_DEVICE_ID_KYE_EASYPEN_M406) },
> + { HID_USB_DEVICE(USB_VENDOR_ID_KYE,
> + USB_DEVICE_ID_KYE_EASYPEN_M506) },
> { HID_USB_DEVICE(USB_VENDOR_ID_KYE,
> USB_DEVICE_ID_KYE_EASYPEN_I405X) },
> { HID_USB_DEVICE(USB_VENDOR_ID_KYE,
> USB_DEVICE_ID_KYE_MOUSEPEN_I608X) },
> { HID_USB_DEVICE(USB_VENDOR_ID_KYE,
> - USB_DEVICE_ID_KYE_MOUSEPEN_I608X_V2) },
> + USB_DEVICE_ID_KYE_EASYPEN_M406W) },
> { HID_USB_DEVICE(USB_VENDOR_ID_KYE,
> USB_DEVICE_ID_KYE_EASYPEN_M610X) },
> { HID_USB_DEVICE(USB_VENDOR_ID_KYE,
> - USB_DEVICE_ID_KYE_EASYPEN_M406XE) },
> + USB_DEVICE_ID_KYE_EASYPEN_340) },
> { HID_USB_DEVICE(USB_VENDOR_ID_KYE,
> - USB_DEVICE_ID_GENIUS_GILA_GAMING_MOUSE) },
> + USB_DEVICE_ID_KYE_PENSKETCH_M912) },
> { HID_USB_DEVICE(USB_VENDOR_ID_KYE,
> - USB_DEVICE_ID_GENIUS_GX_IMPERATOR) },
> + USB_DEVICE_ID_KYE_MOUSEPEN_M508WX) },
> { HID_USB_DEVICE(USB_VENDOR_ID_KYE,
> - USB_DEVICE_ID_GENIUS_MANTICORE) },
> + USB_DEVICE_ID_KYE_MOUSEPEN_M508X) },
> { HID_USB_DEVICE(USB_VENDOR_ID_KYE,
> - USB_DEVICE_ID_KYE_PENSKETCH_M912) },
> + USB_DEVICE_ID_KYE_EASYPEN_M406XE) },
> + { HID_USB_DEVICE(USB_VENDOR_ID_KYE,
> + USB_DEVICE_ID_KYE_MOUSEPEN_I608X_V2) },
> + { HID_USB_DEVICE(USB_VENDOR_ID_KYE,
> + USB_DEVICE_ID_KYE_PENSKETCH_T609A) },
> { }
> };
> MODULE_DEVICE_TABLE(hid, kye_devices);
> diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
> index be3ad02573de..fdb75af75b53 100644
> --- a/drivers/hid/hid-quirks.c
> +++ b/drivers/hid/hid-quirks.c
> @@ -104,12 +104,20 @@ static const struct hid_device_id hid_quirks[] = {
> { HID_USB_DEVICE(USB_VENDOR_ID_HP, USB_PRODUCT_ID_HP_PIXART_OEM_USB_OPTICAL_MOUSE_1f4a), HID_QUIRK_ALWAYS_POLL },
> { HID_USB_DEVICE(USB_VENDOR_ID_IDEACOM, USB_DEVICE_ID_IDEACOM_IDC6680), HID_QUIRK_MULTI_INPUT },
> { HID_USB_DEVICE(USB_VENDOR_ID_INNOMEDIA, USB_DEVICE_ID_INNEX_GENESIS_ATARI), HID_QUIRK_MULTI_INPUT },
> - { HID_USB_DEVICE(USB_VENDOR_ID_KYE, USB_DEVICE_ID_KYE_EASYPEN_M610X), HID_QUIRK_MULTI_INPUT },
> + { HID_USB_DEVICE(USB_VENDOR_ID_KYE, USB_DEVICE_ID_PIXART_USB_OPTICAL_MOUSE_ID2), HID_QUIRK_ALWAYS_POLL },
> + { HID_USB_DEVICE(USB_VENDOR_ID_KYE, USB_DEVICE_ID_KYE_EASYPEN_M406), HID_QUIRK_MULTI_INPUT },
> + { HID_USB_DEVICE(USB_VENDOR_ID_KYE, USB_DEVICE_ID_KYE_EASYPEN_M506), HID_QUIRK_MULTI_INPUT },
> + { HID_USB_DEVICE(USB_VENDOR_ID_KYE, USB_DEVICE_ID_KYE_EASYPEN_I405X), HID_QUIRK_MULTI_INPUT },
> { HID_USB_DEVICE(USB_VENDOR_ID_KYE, USB_DEVICE_ID_KYE_MOUSEPEN_I608X), HID_QUIRK_MULTI_INPUT },
> - { HID_USB_DEVICE(USB_VENDOR_ID_KYE, USB_DEVICE_ID_KYE_MOUSEPEN_I608X_V2), HID_QUIRK_MULTI_INPUT },
> + { HID_USB_DEVICE(USB_VENDOR_ID_KYE, USB_DEVICE_ID_KYE_EASYPEN_M406W), HID_QUIRK_MULTI_INPUT },
> + { HID_USB_DEVICE(USB_VENDOR_ID_KYE, USB_DEVICE_ID_KYE_EASYPEN_M610X), HID_QUIRK_MULTI_INPUT },
> + { HID_USB_DEVICE(USB_VENDOR_ID_KYE, USB_DEVICE_ID_KYE_EASYPEN_340), HID_QUIRK_MULTI_INPUT },
> { HID_USB_DEVICE(USB_VENDOR_ID_KYE, USB_DEVICE_ID_KYE_PENSKETCH_M912), HID_QUIRK_MULTI_INPUT },
> + { HID_USB_DEVICE(USB_VENDOR_ID_KYE, USB_DEVICE_ID_KYE_MOUSEPEN_M508WX), HID_QUIRK_MULTI_INPUT },
> + { HID_USB_DEVICE(USB_VENDOR_ID_KYE, USB_DEVICE_ID_KYE_MOUSEPEN_M508X), HID_QUIRK_MULTI_INPUT },
> { HID_USB_DEVICE(USB_VENDOR_ID_KYE, USB_DEVICE_ID_KYE_EASYPEN_M406XE), HID_QUIRK_MULTI_INPUT },
> - { HID_USB_DEVICE(USB_VENDOR_ID_KYE, USB_DEVICE_ID_PIXART_USB_OPTICAL_MOUSE_ID2), HID_QUIRK_ALWAYS_POLL },
> + { HID_USB_DEVICE(USB_VENDOR_ID_KYE, USB_DEVICE_ID_KYE_MOUSEPEN_I608X_V2), HID_QUIRK_MULTI_INPUT },
> + { HID_USB_DEVICE(USB_VENDOR_ID_KYE, USB_DEVICE_ID_KYE_PENSKETCH_T609A), HID_QUIRK_MULTI_INPUT },
> { HID_USB_DEVICE(USB_VENDOR_ID_LENOVO, USB_DEVICE_ID_LENOVO_OPTICAL_USB_MOUSE_600E), HID_QUIRK_ALWAYS_POLL },
> { HID_USB_DEVICE(USB_VENDOR_ID_LENOVO, USB_DEVICE_ID_LENOVO_PIXART_USB_MOUSE_608D), HID_QUIRK_ALWAYS_POLL },
> { HID_USB_DEVICE(USB_VENDOR_ID_LENOVO, USB_DEVICE_ID_LENOVO_PIXART_USB_MOUSE_6019), HID_QUIRK_ALWAYS_POLL },
> --
> 2.39.0
>
^ permalink raw reply
* Re: [PATCH v4 1/4] dt-bindings: i2c: Add CP2112 HID USB to SMBus Bridge
From: Krzysztof Kozlowski @ 2023-02-06 16:17 UTC (permalink / raw)
To: Danny Kaehn, robh+dt, krzysztof.kozlowski+dt, jikos,
benjamin.tissoires
Cc: devicetree, linux-input, ethan.twardy
In-Reply-To: <20230206135016.6737-2-kaehndan@gmail.com>
On 06/02/2023 14:50, Danny Kaehn wrote:
> This is a USB HID device which includes an I2C controller and 8 GPIO pins.
>
> The binding allows describing the chip's gpio and i2c controller in DT
> using the subnodes named "gpio" and "i2c", respectively. This is
> intended to be used in configurations where the CP2112 is permanently
> connected in hardware.
>
> Signed-off-by: Danny Kaehn <kaehndan@gmail.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v3 4/4] HID: cp2112: Devicetree Support
From: Rob Herring @ 2023-02-06 16:39 UTC (permalink / raw)
To: Danny Kaehn, bartosz.golaszewski
Cc: krzysztof.kozlowski+dt, jikos, benjamin.tissoires, devicetree,
linux-input, ethan.twardy
In-Reply-To: <20230205145450.3396-5-kaehndan@gmail.com>
+Bartosz
On Sun, Feb 05, 2023 at 08:54:50AM -0600, Danny Kaehn wrote:
> Bind i2c and gpio interfaces to subnodes with names
> "i2c" and "gpio" if they exist, respectively. This
> allows the gpio and i2c controllers to be described
> in DT as usual. Additionally, support configuring the
> i2c bus speed from the clock-frequency property.
>
> Signed-off-by: Danny Kaehn <kaehndan@gmail.com>
> ---
> drivers/hid/hid-cp2112.c | 22 ++++++++++++++++++++--
> 1 file changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hid/hid-cp2112.c b/drivers/hid/hid-cp2112.c
> index 27cadadda7c9..aa634accdfb0 100644
> --- a/drivers/hid/hid-cp2112.c
> +++ b/drivers/hid/hid-cp2112.c
> @@ -1234,6 +1234,7 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id)
> u8 buf[3];
> struct cp2112_smbus_config_report config;
> struct gpio_irq_chip *girq;
> + struct i2c_timings timings;
> int ret;
>
> dev = devm_kzalloc(&hdev->dev, sizeof(*dev), GFP_KERNEL);
> @@ -1292,6 +1293,10 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id)
> goto err_power_normal;
> }
>
> + dev->adap.dev.of_node = of_get_child_by_name(hdev->dev.of_node, "i2c");
> + i2c_parse_fw_timings(&dev->adap.dev, &timings, true);
> +
> + config.clock_speed = cpu_to_be32(timings.bus_freq_hz);
> config.retry_time = cpu_to_be16(1);
>
> ret = cp2112_hid_output(hdev, (u8 *)&config, sizeof(config),
> @@ -1300,7 +1305,7 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id)
> hid_err(hdev, "error setting SMBus config\n");
> if (ret >= 0)
> ret = -EIO;
> - goto err_power_normal;
> + goto err_free_i2c_of;
> }
>
> hid_set_drvdata(hdev, (void *)dev);
> @@ -1322,7 +1327,7 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id)
>
> if (ret) {
> hid_err(hdev, "error registering i2c adapter\n");
> - goto err_power_normal;
> + goto err_free_i2c_of;
> }
>
> hid_dbg(hdev, "adapter registered\n");
> @@ -1336,6 +1341,9 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id)
> dev->gc.ngpio = 8;
> dev->gc.can_sleep = 1;
> dev->gc.parent = &hdev->dev;
> +#if IS_ENABLED(CONFIG_OF_GPIO)
> + dev->gc.of_node = of_get_child_by_name(hdev->dev.of_node, "gpio");
> +#endif
The scarcity of CONFIG_OF_GPIO ifdefs in the tree tells me this is
wrong. I think you want to use the fwnode pointer instead. GPIO
maintainers should review this.
Rob
^ permalink raw reply
* Re: [PATCH v3 1/4] dt-bindings: input: Add CP2112 HID USB to SMBus Bridge
From: Rob Herring @ 2023-02-06 16:51 UTC (permalink / raw)
To: Danny Kaehn
Cc: krzysztof.kozlowski+dt, jikos, benjamin.tissoires, devicetree,
linux-input, ethan.twardy
In-Reply-To: <20230205145450.3396-2-kaehndan@gmail.com>
On Sun, Feb 05, 2023 at 08:54:47AM -0600, Danny Kaehn wrote:
> This is a USB HID device which includes an I2C controller and 8 GPIO pins.
>
> The binding allows describing the chip's gpio and i2c controller in DT
> using the subnodes named "gpio" and "i2c", respectively. This is
> intended to be used in configurations where the CP2112 is permanently
> connected in hardware.
>
> Signed-off-by: Danny Kaehn <kaehndan@gmail.com>
> ---
> .../bindings/input/silabs,cp2112.yaml | 112 ++++++++++++++++++
> 1 file changed, 112 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/input/silabs,cp2112.yaml
>
> diff --git a/Documentation/devicetree/bindings/input/silabs,cp2112.yaml b/Documentation/devicetree/bindings/input/silabs,cp2112.yaml
> new file mode 100644
> index 000000000000..eb2e89edb80a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/silabs,cp2112.yaml
> @@ -0,0 +1,112 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/input/silabs,cp2112.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: CP2112 HID USB to SMBus/I2C Bridge
> +
> +maintainers:
> + - Danny Kaehn <kaehndan@gmail.com>
> +
> +description:
> + The CP2112 is a USB HID device which includes an integrated I2C controller
> + and 8 GPIO pins. Its GPIO pins can each be configured as inputs, open-drain
> + outputs, or push-pull outputs.
> +
> +properties:
> + compatible:
> + const: usb10c4,ea90
> +
> + reg:
> + maxItems: 1
> + description: The USB port number on the host controller
> +
> + i2c:
> + description: The SMBus/I2C controller node for the CP2112
> + $ref: /schemas/i2c/i2c-controller.yaml#
> + unevaluatedProperties: false
> + properties:
> + clock-frequency:
> + minimum: 10000
> + default: 100000
> + maximum: 400000
> +
> + gpio:
> + description: The GPIO controller node for the CP2112
> + type: object
> + properties:
> + interrupt-controller: true
> + "#interrupt-cells":
> + const: 2
> +
> + gpio-controller: true
> + "#gpio-cells":
> + const: 2
> +
> + ngpios:
> + const: 8
If this can only be 1 value, then it doesn't need to be in DT.
> +
> + gpio-line-names:
> + minItems: 1
> + maxItems: 8
> +
> + patternProperties:
> + "^(hog-[0-9]+|.+-hog(-[0-9]+)?)$":
Pick one naming scheme, not everything we allow.
> + type: object
> + properties:
> + gpio-hog: true
> + input: true
> + output-high: true
> + output-low: true
> + line-name: true
> + gpios:
> + minItems: 1
> + maxItems: 8
> +
> + required:
> + - gpio-hog
> + - gpios
> +
> + additionalProperties: false
You shouldn't need all this for the hog nodes, just need the following
and the common schema will check the rest:
required:
- gpio-hog
> +
> + unevaluatedProperties: false
Move this above 'properties'. Easier to read rather than after
a long indented block.
> +
> +required:
> + - compatible
> + - reg
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/input/input.h>
> + #include <dt-bindings/interrupt-controller/irq.h>
> +
> + usb {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + device@1 {
> + compatible = "usb10c4,ea90";
> + reg = <1>;
> +
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + temp@48 {
> + compatible = "national,lm75";
> + reg = <0x48>;
> + };
> + };
> +
> + gpio {
> + gpio-controller;
> + interrupt-controller;
> + #gpio-cells = <2>;
> + gpio-line-names = "TEST0", "TEST1", "TEST2",
> + "TEST3", "TEST4", "TEST5", "TEST6", "TEST7";
Put a hog to test the schema.
> + };
> + };
> + };
> --
> 2.25.1
>
^ permalink raw reply
* Re: [PATCH 1/3] HID: logitech-hidpp: Add more debug statements
From: kernel test robot @ 2023-02-06 16:53 UTC (permalink / raw)
To: Bastien Nocera, linux-input
Cc: oe-kbuild-all, linux-kernel, Jiri Kosina, Benjamin Tissoires,
Peter F . Patel-Schneider, Filipe Laíns, Nestor Lopez Casado
In-Reply-To: <20230206135808.8840-1-hadess@hadess.net>
Hi Bastien,
I love your patch! Perhaps something to improve:
[auto build test WARNING on hid/for-next]
[also build test WARNING on linus/master v6.2-rc7 next-20230206]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Bastien-Nocera/HID-logitech-hidpp-Retry-commands-when-device-is-busy/20230206-215940
base: https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git for-next
patch link: https://lore.kernel.org/r/20230206135808.8840-1-hadess%40hadess.net
patch subject: [PATCH 1/3] HID: logitech-hidpp: Add more debug statements
config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20230207/202302070034.GqDB3Cje-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/f201298961e2cb71de94a0c8632cb9376975959f
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Bastien-Nocera/HID-logitech-hidpp-Retry-commands-when-device-is-busy/20230206-215940
git checkout f201298961e2cb71de94a0c8632cb9376975959f
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=m68k olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=m68k SHELL=/bin/bash drivers/hid/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
In file included from include/linux/printk.h:566,
from include/asm-generic/bug.h:22,
from arch/m68k/include/asm/bug.h:32,
from include/linux/bug.h:5,
from include/linux/thread_info.h:13,
from include/asm-generic/preempt.h:5,
from ./arch/m68k/include/generated/asm/preempt.h:1,
from include/linux/preempt.h:78,
from arch/m68k/include/asm/irqflags.h:6,
from include/linux/irqflags.h:16,
from arch/m68k/include/asm/atomic.h:6,
from include/linux/atomic.h:7,
from include/linux/rcupdate.h:25,
from include/linux/rculist.h:11,
from include/linux/pid.h:5,
from include/linux/sched.h:14,
from include/linux/ratelimit.h:6,
from include/linux/dev_printk.h:16,
from include/linux/device.h:15,
from drivers/hid/hid-logitech-hidpp.c:13:
drivers/hid/hid-logitech-hidpp.c: In function 'hidpp_send_fap_command_sync':
>> drivers/hid/hid-logitech-hidpp.c:343:25: warning: format '%ld' expects argument of type 'long int', but argument 5 has type 'unsigned int' [-Wformat=]
343 | "Invalid number of parameters passed to command (%d != %ld)\n",
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/dynamic_debug.h:223:29: note: in definition of macro '__dynamic_func_call_cls'
223 | func(&id, ##__VA_ARGS__); \
| ^~~~~~~~~~~
include/linux/dynamic_debug.h:249:9: note: in expansion of macro '_dynamic_func_call_cls'
249 | _dynamic_func_call_cls(_DPRINTK_CLASS_DFLT, fmt, func, ##__VA_ARGS__)
| ^~~~~~~~~~~~~~~~~~~~~~
include/linux/dynamic_debug.h:272:9: note: in expansion of macro '_dynamic_func_call'
272 | _dynamic_func_call(fmt, __dynamic_dev_dbg, \
| ^~~~~~~~~~~~~~~~~~
include/linux/dev_printk.h:155:9: note: in expansion of macro 'dynamic_dev_dbg'
155 | dynamic_dev_dbg(dev, dev_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~~~~~~~~~
include/linux/dev_printk.h:155:30: note: in expansion of macro 'dev_fmt'
155 | dynamic_dev_dbg(dev, dev_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~
include/linux/hid.h:1202:9: note: in expansion of macro 'dev_dbg'
1202 | dev_dbg(&(hid)->dev, fmt, ##__VA_ARGS__)
| ^~~~~~~
drivers/hid/hid-logitech-hidpp.c:342:17: note: in expansion of macro 'hid_dbg'
342 | hid_dbg(hidpp->hid_dev,
| ^~~~~~~
drivers/hid/hid-logitech-hidpp.c:343:82: note: format string is defined here
343 | "Invalid number of parameters passed to command (%d != %ld)\n",
| ~~^
| |
| long int
| %d
vim +343 drivers/hid/hid-logitech-hidpp.c
333
334 static int hidpp_send_fap_command_sync(struct hidpp_device *hidpp,
335 u8 feat_index, u8 funcindex_clientid, u8 *params, int param_count,
336 struct hidpp_report *response)
337 {
338 struct hidpp_report *message;
339 int ret;
340
341 if (param_count > sizeof(message->fap.params)) {
342 hid_dbg(hidpp->hid_dev,
> 343 "Invalid number of parameters passed to command (%d != %ld)\n",
344 param_count, sizeof(message->fap.params));
345 return -EINVAL;
346 }
347
348 message = kzalloc(sizeof(struct hidpp_report), GFP_KERNEL);
349 if (!message)
350 return -ENOMEM;
351
352 if (param_count > (HIDPP_REPORT_LONG_LENGTH - 4))
353 message->report_id = REPORT_ID_HIDPP_VERY_LONG;
354 else
355 message->report_id = REPORT_ID_HIDPP_LONG;
356 message->fap.feature_index = feat_index;
357 message->fap.funcindex_clientid = funcindex_clientid | LINUX_KERNEL_SW_ID;
358 memcpy(&message->fap.params, params, param_count);
359
360 ret = hidpp_send_message_sync(hidpp, message, response);
361 kfree(message);
362 return ret;
363 }
364
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
^ permalink raw reply
* Re: [PATCH] HID: Ignore battery for ELAN touchscreen 29DF on HP
From: Benjamin Tissoires @ 2023-02-06 17:13 UTC (permalink / raw)
To: jikos, Luka Guzenko; +Cc: linux-input, linux-kernel
In-Reply-To: <20230120223741.3007-1-l.guzenko@web.de>
On Fri, 20 Jan 2023 23:37:41 +0100, Luka Guzenko wrote:
> The touchscreen reports a battery status of 0% and jumps to 1% when a
> stylus is used. The device ID was added and the battery ignore quirk was
> enabled for it.
>
>
Applied to hid/hid.git (for-6.2/upstream-fixes), thanks!
[1/1] HID: Ignore battery for ELAN touchscreen 29DF on HP
https://git.kernel.org/hid/hid/c/ebebf05a4b06
Cheers,
--
Benjamin Tissoires <benjamin.tissoires@redhat.com>
^ permalink raw reply
* Re: [PATCH v3 1/1] HID: evision: Add preliminary support for EVision keyboards
From: Benjamin Tissoires @ 2023-02-06 17:47 UTC (permalink / raw)
To: Jiri Kosina, linux-kernel, linux-input, Philippe Valembois
In-Reply-To: <20230125211511.12266-1-lephilousophe@gmail.com>
On Wed, 25 Jan 2023 22:15:10 +0100, Philippe Valembois wrote:
> For now only supports one model and only filters out bogus reports sent
> when the keyboard has been configured through hidraw.
> Without this, as events are not released, soft repeat floods userspace
> with unknown key events.
>
>
Applied to https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git (for-6.3/evision), thanks!
[1/1] HID: evision: Add preliminary support for EVision keyboards
https://git.kernel.org/hid/hid/c/f5cd71cfdb5c
Cheers,
--
Benjamin Tissoires <benjamin.tissoires@redhat.com>
^ permalink raw reply
* [PATCH v2 2/3] HID: logitech-hidpp: Retry commands when device is busy
From: Bastien Nocera @ 2023-02-06 22:12 UTC (permalink / raw)
To: linux-input
Cc: linux-kernel, Jiri Kosina, Benjamin Tissoires,
Peter F . Patel-Schneider, Filipe Laíns, Nestor Lopez Casado
In-Reply-To: <20230206221256.129198-1-hadess@hadess.net>
Handle the busy error coming from the device or receiver. The
documentation says a busy error can be returned when:
"
Device (or receiver) cannot answer immediately to this request
for any reason i.e:
- already processing a request from the same or another SW
- pipe full
"
Signed-off-by: Bastien Nocera <hadess@hadess.net>
---
Same as v1
drivers/hid/hid-logitech-hidpp.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 1952d8d3b6b2..9e94026de437 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -295,6 +295,7 @@ static int hidpp_send_message_sync(struct hidpp_device *hidpp,
*/
*response = *message;
+retry:
ret = __hidpp_send_report(hidpp->hid_dev, message);
if (ret) {
@@ -321,6 +322,10 @@ static int hidpp_send_message_sync(struct hidpp_device *hidpp,
response->report_id == REPORT_ID_HIDPP_VERY_LONG) &&
response->fap.feature_index == HIDPP20_ERROR) {
ret = response->fap.params[1];
+ if (ret == HIDPP20_ERROR_BUSY) {
+ dbg_hid("%s:got busy hidpp 2.0 error %02X, retrying\n", __func__, ret);
+ goto retry;
+ }
dbg_hid("%s:got hidpp 2.0 error %02X\n", __func__, ret);
goto exit;
}
--
2.39.1
^ permalink raw reply related
* [PATCH v2 1/3] HID: logitech-hidpp: Add more debug statements
From: Bastien Nocera @ 2023-02-06 22:12 UTC (permalink / raw)
To: linux-input
Cc: linux-kernel, Jiri Kosina, Benjamin Tissoires,
Peter F . Patel-Schneider, Filipe Laíns, Nestor Lopez Casado
This should help us figure out some hairy problems with some devices.
Signed-off-by: Bastien Nocera <hadess@hadess.net>
---
Fixed kernel test bot warning:
drivers/hid/hid-logitech-hidpp.c: In function 'hidpp_send_fap_command_sync':
>> drivers/hid/hid-logitech-hidpp.c:343:25: warning: format '%ld' expects argument of type 'long int', but argument 5 has type 'unsigned int' [-Wformat=]
343 | "Invalid number of parameters passed to command (%d != %ld)\n",
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/hid/hid-logitech-hidpp.c | 17 ++++++++++++++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index f44ba7be3cc5..1952d8d3b6b2 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -338,8 +338,13 @@ static int hidpp_send_fap_command_sync(struct hidpp_device *hidpp,
struct hidpp_report *message;
int ret;
- if (param_count > sizeof(message->fap.params))
+ if (param_count > sizeof(message->fap.params)) {
+ hid_dbg(hidpp->hid_dev,
+ "Invalid number of parameters passed to command (%d != %llu)\n",
+ param_count,
+ (unsigned long long) sizeof(message->fap.params));
return -EINVAL;
+ }
message = kzalloc(sizeof(struct hidpp_report), GFP_KERNEL);
if (!message)
@@ -3440,11 +3445,17 @@ static int hi_res_scroll_enable(struct hidpp_device *hidpp)
ret = hidpp10_enable_scrolling_acceleration(hidpp);
multiplier = 8;
}
- if (ret)
+ if (ret) {
+ hid_dbg(hidpp->hid_dev,
+ "Could not enable hi-res scrolling: %d\n", ret);
return ret;
+ }
- if (multiplier == 0)
+ if (multiplier == 0) {
+ hid_dbg(hidpp->hid_dev,
+ "Invalid multiplier 0 from device, setting it to 1\n");
multiplier = 1;
+ }
hidpp->vertical_wheel_counter.wheel_multiplier = multiplier;
hid_dbg(hidpp->hid_dev, "wheel multiplier = %d\n", multiplier);
--
2.39.1
^ permalink raw reply related
* [PATCH v2 3/3] HID: logitech-hidpp: Add myself to authors
From: Bastien Nocera @ 2023-02-06 22:12 UTC (permalink / raw)
To: linux-input
Cc: linux-kernel, Jiri Kosina, Benjamin Tissoires,
Peter F . Patel-Schneider, Filipe Laíns, Nestor Lopez Casado
In-Reply-To: <20230206221256.129198-1-hadess@hadess.net>
Signed-off-by: Bastien Nocera <hadess@hadess.net>
---
Same as v1
drivers/hid/hid-logitech-hidpp.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 9e94026de437..03b77ca03081 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -30,6 +30,7 @@
MODULE_LICENSE("GPL");
MODULE_AUTHOR("Benjamin Tissoires <benjamin.tissoires@gmail.com>");
MODULE_AUTHOR("Nestor Lopez Casado <nlopezcasad@logitech.com>");
+MODULE_AUTHOR("Bastien Nocera <hadess@hadess.net>");
static bool disable_tap_to_click;
module_param(disable_tap_to_click, bool, 0644);
--
2.39.1
^ permalink raw reply related
* [PATCH] HID: Ignore battery for ELAN touchscreen on HP ENVY X360 13-ar0xxx
From: Luka Guzenko @ 2023-02-06 22:32 UTC (permalink / raw)
To: benjamin.tissoires; +Cc: linux-input, linux-kernel, Luka Guzenko
Ignore the reported battery level of the built-in touchscreen to suppress
battery warnings when a stylus is used. The device ID was added and the
battery ignore quirk was enabled.
Signed-off-by: Luka Guzenko <l.guzenko@web.de>
---
drivers/hid/hid-ids.h | 1 +
drivers/hid/hid-input.c | 2 ++
2 files changed, 3 insertions(+)
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 0f8c11842a3a..ea6cd7dfa459 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -409,6 +409,7 @@
#define USB_DEVICE_ID_TOSHIBA_CLICK_L9W 0x0401
#define USB_DEVICE_ID_HP_X2 0x074d
#define USB_DEVICE_ID_HP_X2_10_COVER 0x0755
+#define I2C_DEVICE_ID_HP_ENVY_X360_13_AR0XXX 0x23B9
#define I2C_DEVICE_ID_HP_ENVY_X360_15 0x2d05
#define I2C_DEVICE_ID_HP_ENVY_X360_15T_DR100 0x29CF
#define I2C_DEVICE_ID_HP_ENVY_X360_EU0009NV 0x2CF9
diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index 9b59e436df0a..8dc090d9f2d6 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -376,6 +376,8 @@ static const struct hid_device_id hid_battery_quirks[] = {
HID_BATTERY_QUIRK_IGNORE },
{ HID_USB_DEVICE(USB_VENDOR_ID_UGEE, USB_DEVICE_ID_UGEE_XPPEN_TABLET_DECO_L),
HID_BATTERY_QUIRK_AVOID_QUERY },
+ { HID_USB_DEVICE(USB_VENDOR_ID_ELAN, I2C_DEVICE_ID_HP_ENVY_X360_13_AR0XXX),
+ HID_BATTERY_QUIRK_IGNORE },
{ HID_I2C_DEVICE(USB_VENDOR_ID_ELAN, I2C_DEVICE_ID_HP_ENVY_X360_15),
HID_BATTERY_QUIRK_IGNORE },
{ HID_I2C_DEVICE(USB_VENDOR_ID_ELAN, I2C_DEVICE_ID_HP_ENVY_X360_15T_DR100),
--
2.39.1
^ permalink raw reply related
* Re: [PATCH v4 2/4] HID: usbhid: Share USB device devicetree node with child HID device
From: Dmitry Torokhov @ 2023-02-06 23:14 UTC (permalink / raw)
To: Danny Kaehn
Cc: robh+dt, krzysztof.kozlowski+dt, jikos, benjamin.tissoires,
devicetree, linux-input, ethan.twardy
In-Reply-To: <20230206135016.6737-3-kaehndan@gmail.com>
Hi Danny,
On Mon, Feb 06, 2023 at 07:50:14AM -0600, Danny Kaehn wrote:
> USB HID core now shares its devicetree of_node with its child HID device.
> Since there can only be one HID device on a USB interface, it is redundant
> to specify a hid node under the USB device (and further, binding this way
> isn't currently possible, as hid_driver does not support of_match_table).
Why do we do that only for OF? Can we use device_set_node() instead?
>
> Signed-off-by: Danny Kaehn <kaehndan@gmail.com>
> ---
> drivers/hid/usbhid/hid-core.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
> index be4c731aaa65..b6c968af258f 100644
> --- a/drivers/hid/usbhid/hid-core.c
> +++ b/drivers/hid/usbhid/hid-core.c
> @@ -33,6 +33,7 @@
> #include <linux/hiddev.h>
> #include <linux/hid-debug.h>
> #include <linux/hidraw.h>
> +#include <linux/device.h>
> #include "usbhid.h"
>
> /*
> @@ -1369,6 +1370,7 @@ static int usbhid_probe(struct usb_interface *intf, const struct usb_device_id *
> hid->hiddev_report_event = hiddev_report_event;
> #endif
> hid->dev.parent = &intf->dev;
> + device_set_of_node_from_dev(&hid->dev, &intf->dev);
> hid->bus = BUS_USB;
> hid->vendor = le16_to_cpu(dev->descriptor.idVendor);
> hid->product = le16_to_cpu(dev->descriptor.idProduct);
> --
> 2.25.1
>
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v4 3/4] HID: cp2112: Fix driver not registering GPIO IRQ chip as threaded
From: Dmitry Torokhov @ 2023-02-06 23:15 UTC (permalink / raw)
To: Danny Kaehn
Cc: robh+dt, krzysztof.kozlowski+dt, jikos, benjamin.tissoires,
devicetree, linux-input, ethan.twardy
In-Reply-To: <20230206135016.6737-4-kaehndan@gmail.com>
On Mon, Feb 06, 2023 at 07:50:15AM -0600, Danny Kaehn wrote:
> The CP2112 generates interrupts from a polling routine on a thread,
> and can only support threaded interrupts. This patch configures the
> gpiochip irq chip with this flag, disallowing consumers to request
> a hard IRQ from this driver, which resulted in a segfault previously.
This looks like a bugfix not dependent on anything else in the series
and can be applied separately...
>
> Signed-off-by: Danny Kaehn <kaehndan@gmail.com>
> ---
> drivers/hid/hid-cp2112.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/hid/hid-cp2112.c b/drivers/hid/hid-cp2112.c
> index 1e16b0fa310d..27cadadda7c9 100644
> --- a/drivers/hid/hid-cp2112.c
> +++ b/drivers/hid/hid-cp2112.c
> @@ -1354,6 +1354,7 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id)
> girq->parents = NULL;
> girq->default_type = IRQ_TYPE_NONE;
> girq->handler = handle_simple_irq;
> + girq->threaded = true;
>
> ret = gpiochip_add_data(&dev->gc, dev);
> if (ret < 0) {
> --
> 2.25.1
>
--
Dmitry
^ permalink raw reply
* Re: [PATCH v4 4/4] HID: cp2112: Devicetree Support
From: Dmitry Torokhov @ 2023-02-06 23:18 UTC (permalink / raw)
To: Danny Kaehn
Cc: robh+dt, krzysztof.kozlowski+dt, jikos, benjamin.tissoires,
devicetree, linux-input, ethan.twardy, Andy Shevchenko
In-Reply-To: <20230206135016.6737-5-kaehndan@gmail.com>
On Mon, Feb 06, 2023 at 07:50:16AM -0600, Danny Kaehn wrote:
> Bind i2c and gpio interfaces to subnodes with names
> "i2c" and "gpio" if they exist, respectively. This
> allows the gpio and i2c controllers to be described
> in DT as usual. Additionally, support configuring the
> i2c bus speed from the clock-frequency property.
>
> Signed-off-by: Danny Kaehn <kaehndan@gmail.com>
> ---
> drivers/hid/hid-cp2112.c | 22 ++++++++++++++++++++--
> 1 file changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hid/hid-cp2112.c b/drivers/hid/hid-cp2112.c
> index 27cadadda7c9..aa634accdfb0 100644
> --- a/drivers/hid/hid-cp2112.c
> +++ b/drivers/hid/hid-cp2112.c
> @@ -1234,6 +1234,7 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id)
> u8 buf[3];
> struct cp2112_smbus_config_report config;
> struct gpio_irq_chip *girq;
> + struct i2c_timings timings;
> int ret;
>
> dev = devm_kzalloc(&hdev->dev, sizeof(*dev), GFP_KERNEL);
> @@ -1292,6 +1293,10 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id)
> goto err_power_normal;
> }
>
> + dev->adap.dev.of_node = of_get_child_by_name(hdev->dev.of_node, "i2c");
> + i2c_parse_fw_timings(&dev->adap.dev, &timings, true);
> +
> + config.clock_speed = cpu_to_be32(timings.bus_freq_hz);
> config.retry_time = cpu_to_be16(1);
>
> ret = cp2112_hid_output(hdev, (u8 *)&config, sizeof(config),
> @@ -1300,7 +1305,7 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id)
> hid_err(hdev, "error setting SMBus config\n");
> if (ret >= 0)
> ret = -EIO;
> - goto err_power_normal;
> + goto err_free_i2c_of;
> }
>
> hid_set_drvdata(hdev, (void *)dev);
> @@ -1322,7 +1327,7 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id)
>
> if (ret) {
> hid_err(hdev, "error registering i2c adapter\n");
> - goto err_power_normal;
> + goto err_free_i2c_of;
> }
>
> hid_dbg(hdev, "adapter registered\n");
> @@ -1336,6 +1341,9 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id)
> dev->gc.ngpio = 8;
> dev->gc.can_sleep = 1;
> dev->gc.parent = &hdev->dev;
> +#if IS_ENABLED(CONFIG_OF_GPIO)
> + dev->gc.of_node = of_get_child_by_name(hdev->dev.of_node, "gpio");
I believe Andy is actively trying to get rid of of_node from GPIO chips.
And in general, we should be using fwnode and generic device properties
as much as possible.
> +#endif
>
> dev->irq.name = "cp2112-gpio";
> dev->irq.irq_startup = cp2112_gpio_irq_startup;
> @@ -1376,7 +1384,12 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id)
> err_gpiochip_remove:
> gpiochip_remove(&dev->gc);
> err_free_i2c:
> +#if IS_ENABLED(CONFIG_OF_GPIO)
> + of_node_put(dev->gc.of_node);
> +#endif
> i2c_del_adapter(&dev->adap);
> +err_free_i2c_of:
> + of_node_put(dev->adap.dev.of_node);
> err_power_normal:
> hid_hw_power(hdev, PM_HINT_NORMAL);
> err_hid_close:
> @@ -1391,6 +1404,11 @@ static void cp2112_remove(struct hid_device *hdev)
> struct cp2112_device *dev = hid_get_drvdata(hdev);
> int i;
>
> + of_node_put(dev->adap.dev.of_node);
> +#if IS_ENABLED(CONFIG_OF_GPIO)
> + of_node_put(dev->gc.of_node);
> +#endif
> +
> sysfs_remove_group(&hdev->dev.kobj, &cp2112_attr_group);
> i2c_del_adapter(&dev->adap);
>
> --
> 2.25.1
>
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v2] HID: add KEY_CAMERA_FOCUS event in HID
From: qi feng @ 2023-02-07 2:35 UTC (permalink / raw)
To: jikos; +Cc: benjamin.tissoires, rydberg, linux-input, linux-kernel, fengqi
In-Reply-To: <20230131114632.14078-1-fengqi706@gmail.com>
Hi，
Please help to review this v2 code
thanks a lot
Qi Feng <fengqi706@gmail.com> 于2023年1月31日周二 19:46写道：
>
> From: fengqi <fengqi@xiaomi.com>
>
> Our HID device need KEY_CAMERA_FOCUS event to control camera, but this
> event is non-existent in current HID driver.we add this event in hid-input.c
> We committed this v2 version following your previous suggestion
>
> Signed-off-by: fengqi <fengqi@xiaomi.com>
> ---
> drivers/hid/hid-input.c | 7 +++++++
> include/linux/hid.h | 1 +
> 2 files changed, 8 insertions(+)
>
> diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
> index 9b59e436df0a..05fa3e191574 100644
> --- a/drivers/hid/hid-input.c
> +++ b/drivers/hid/hid-input.c
> @@ -1221,6 +1221,13 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
> return;
> }
> goto unknown;
> + case HID_UP_CAMERA:
> + switch (usage->hid & HID_USAGE){
> + case 0x020: map_key_clear(KEY_CAMERA_FOCUS); break;
> + case 0x021: map_key_clear(KEY_CAMERA); break;
> + default: goto ignore;
> + }
> + break;
>
> case HID_UP_HPVENDOR: /* Reported on a Dutch layout HP5308 */
> set_bit(EV_REP, input->evbit);
> diff --git a/include/linux/hid.h b/include/linux/hid.h
> index 8677ae38599e..e3daf2c7739c 100644
> --- a/include/linux/hid.h
> +++ b/include/linux/hid.h
> @@ -155,6 +155,7 @@ struct hid_item {
> #define HID_UP_DIGITIZER 0x000d0000
> #define HID_UP_PID 0x000f0000
> #define HID_UP_BATTERY 0x00850000
> +#define HID_UP_CAMERA 0x00900000
> #define HID_UP_HPVENDOR 0xff7f0000
> #define HID_UP_HPVENDOR2 0xff010000
> #define HID_UP_MSVENDOR 0xff000000
> --
> 2.39.0
>
^ permalink raw reply
* [PATCH 0/7] arm: qcom: Fix touchscreen voltage for sc7280-herobrine boards
From: Douglas Anderson @ 2023-02-07 2:48 UTC (permalink / raw)
To: Bjorn Andersson, Rob Herring, Krzysztof Kozlowski, Jiri Kosina,
Benjamin Tissoires
Cc: linux-input, linux-arm-msm, Dmitry Torokhov, devicetree,
Stephen Kitt, Konrad Dybcio, Douglas Anderson, Andy Gross,
linux-kernel
Trying to figure out how to talk to the touchscreen properly on
sc7280-herobrine boards was a long and difficult process. Many
Engineering hours were spent deciding how exactly one should talk over
i2c to a peripheral. In the end, a solution has been found and this
patch series attempts to implement it in a way that will work for all
herobrine-based boards.
Validation of this code has actually been done on
sc7280-herobrine-villager. Those patches are device-tree only and are
placed first. They shouldn't be super controversial, I hope.
In order to make this work, we also need to support
sc7280-herobrine-evoker. For evoker, things are a bit tricker, though
(I think) the solution we ended up with is not terrible. See the
"Goodix" bindings patch for the full details. Unfortunately, I haven't
tested the final version of these patches on evoker hardware. Thus
those patches are at the end of the series and marked as such. It
still wouldn't hurt to land them, if people are OK with it, since
nobody in the wild has this hardware and and the evoker-specific parts
of the patch are very easy to validate.
Douglas Anderson (7):
arm64: dts: qcom: sc7280: On QCard, regulator L3C should be 1.8V
arm64: dts: qcom: sc7280: Add 3ms ramp to herobrine's
pp3300_left_in_mlb
arm64: dts: qcom: sc7280: Hook up the touchscreen IO rail on villager
HID: i2c-hid: goodix: Stop tying the reset line to the regulator
dt-bindings: HID: i2c-hid: goodix: Add mainboard-vddio-supply
HID: i2c-hid: goodix: Add mainboard-vddio-supply
arm64: dts: qcom: sc7280: Hook up the touchscreen IO rail on evoker
.../bindings/input/goodix,gt7375p.yaml | 7 ++
.../dts/qcom/sc7280-herobrine-evoker.dtsi | 1 +
.../dts/qcom/sc7280-herobrine-villager.dtsi | 1 +
.../arm64/boot/dts/qcom/sc7280-herobrine.dtsi | 2 +
arch/arm64/boot/dts/qcom/sc7280-qcard.dtsi | 10 +-
drivers/hid/i2c-hid/i2c-hid-of-goodix.c | 98 +++++--------------
6 files changed, 42 insertions(+), 77 deletions(-)
--
2.39.1.519.gcb327c4b5f-goog
^ permalink raw reply
* [PATCH 1/7] arm64: dts: qcom: sc7280: On QCard, regulator L3C should be 1.8V
From: Douglas Anderson @ 2023-02-07 2:48 UTC (permalink / raw)
To: Bjorn Andersson, Rob Herring, Krzysztof Kozlowski, Jiri Kosina,
Benjamin Tissoires
Cc: linux-input, linux-arm-msm, Dmitry Torokhov, devicetree,
Stephen Kitt, Konrad Dybcio, Douglas Anderson, Andy Gross,
linux-kernel
In-Reply-To: <20230207024816.525938-1-dianders@chromium.org>
On the first sc7280 QCards the L3C rail was never really used for
anything. Stuffing options on the QCard meant that the QCard itself
didn't use this rail for anything. This rail did get sent to the
mainboard, but no existing mainboards ever did anything with it other
that route it to a testpoint.
On later sc7280 QCards, the L3C rail was repurposed. Instead of being
a (nominally) 3.3V rail, it was decided to make it a 1.8V rail. It is
now provided to the display connector (which might route it to the
touchscreen) and also used to power some buffers relating to
touchscreen IO. This rail is getting the additional tag "ts_avccio",
though some places still refer to it as "vreg_l3c_3p0" despite the
fact that the name now specifies the wrong voltage.
Since it never hurts for this rail to be 1.8V (even on old QCards /
old boards), let's just change it to 1.8V across the board and add the
extra "ts_avccio" moniker as a label in the device tree.
Future patches will start using this rail in their touchscreens.
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
arch/arm64/boot/dts/qcom/sc7280-qcard.dtsi | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/sc7280-qcard.dtsi b/arch/arm64/boot/dts/qcom/sc7280-qcard.dtsi
index df49564ae6dc..50d963957303 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-qcard.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280-qcard.dtsi
@@ -230,9 +230,15 @@ vreg_l2c_1p8: ldo2 {
regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
};
+ /*
+ * The initial design of this regulator was to use it as 3.3V,
+ * but due to later changes in design it was changed to 1.8V.
+ * The original name is kept due to same schematic.
+ */
+ ts_avccio:
vreg_l3c_3p0: ldo3 {
- regulator-min-microvolt = <2800000>;
- regulator-max-microvolt = <3540000>;
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
};
--
2.39.1.519.gcb327c4b5f-goog
^ permalink raw reply related
* [PATCH 2/7] arm64: dts: qcom: sc7280: Add 3ms ramp to herobrine's pp3300_left_in_mlb
From: Douglas Anderson @ 2023-02-07 2:48 UTC (permalink / raw)
To: Bjorn Andersson, Rob Herring, Krzysztof Kozlowski, Jiri Kosina,
Benjamin Tissoires
Cc: linux-input, linux-arm-msm, Dmitry Torokhov, devicetree,
Stephen Kitt, Konrad Dybcio, Douglas Anderson, Andy Gross,
linux-kernel
In-Reply-To: <20230207024816.525938-1-dianders@chromium.org>
The "pp3300_left_in_mlb" rail on herobrine eventually connects up to
"vreg_edp_3p3" on the qcard. On several herobrine designs this rail
has been measured to need more than 1ms to turn on.
While technically a herobrine derivative (defined as anyone including
the "herobrine.dtsi") could change the board to make the rail rise
faster or slower, the fact that two boards (evoker and villager) both
measured it as taking more than 1ms implies that it's probably going
to be the norm. Thus, let's add a "regulator-enable-ramp-delay"
straight into the herobrine.dtsi to handle this. If a particular
derivative board needs a faster or slower one then they can override
it, though that feels unlikely.
While we measured something a bit over 1ms, we'll choose 3ms to give
us a tiny bit of margin. This isn't a rail that turns off and on all
the time anyway and 3ms is nothing compared to the total amount of
time to power on a panel.
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
index 27f479ff9d80..ded36b5d28c7 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
@@ -108,6 +108,8 @@ pp3300_left_in_mlb: pp3300-left-in-mlb-regulator {
pinctrl-names = "default";
pinctrl-0 = <&en_pp3300_dx_edp>;
+ regulator-enable-ramp-delay = <3000>;
+
vin-supply = <&pp3300_z1>;
};
--
2.39.1.519.gcb327c4b5f-goog
^ permalink raw reply related
* [PATCH 3/7] arm64: dts: qcom: sc7280: Hook up the touchscreen IO rail on villager
From: Douglas Anderson @ 2023-02-07 2:48 UTC (permalink / raw)
To: Bjorn Andersson, Rob Herring, Krzysztof Kozlowski, Jiri Kosina,
Benjamin Tissoires
Cc: linux-input, linux-arm-msm, Dmitry Torokhov, devicetree,
Stephen Kitt, Konrad Dybcio, Douglas Anderson, Andy Gross,
linux-kernel
In-Reply-To: <20230207024816.525938-1-dianders@chromium.org>
On never revs of sc7280-herobrine-villager (rev2+) the L3C rail is
provided to the touchscreen as the IO voltage rail. Let's add it in
the device tree.
NOTE: Even though this is only really needed on rev2+ villagers (-rev0
had non-functioning touchscreen and -rev1 had some hacky hardware
magic), it doesn't actually hurt to do this for old villager revs. As
talked about in the patch ("arm64: dts: qcom: sc7280: On QCard,
regulator L3C should be 1.8V") the L3C regulator didn't go anywhere at
all on older revs. That means that turning it on for older revs
doesn't hurt other than drawing a tiny bit of extra power. Since -rev0
and -rev1 villagers will never make it to real customers and it's nice
not to have too many old device trees, the better tradeoff seems to be
to enable it everywhere.
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
arch/arm64/boot/dts/qcom/sc7280-herobrine-villager.dtsi | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine-villager.dtsi b/arch/arm64/boot/dts/qcom/sc7280-herobrine-villager.dtsi
index 17553e0fd6fd..818d4046d2c7 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-herobrine-villager.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine-villager.dtsi
@@ -55,6 +55,7 @@ ap_ts: touchscreen@10 {
reset-gpios = <&tlmm 54 GPIO_ACTIVE_LOW>;
vcc33-supply = <&ts_avdd>;
+ vccio-supply = <&ts_avccio>;
};
};
--
2.39.1.519.gcb327c4b5f-goog
^ permalink raw reply related
* [PATCH 4/7] HID: i2c-hid: goodix: Stop tying the reset line to the regulator
From: Douglas Anderson @ 2023-02-07 2:48 UTC (permalink / raw)
To: Bjorn Andersson, Rob Herring, Krzysztof Kozlowski, Jiri Kosina,
Benjamin Tissoires
Cc: linux-input, linux-arm-msm, Dmitry Torokhov, devicetree,
Stephen Kitt, Konrad Dybcio, Douglas Anderson, linux-kernel
In-Reply-To: <20230207024816.525938-1-dianders@chromium.org>
In commit 18eeef46d359 ("HID: i2c-hid: goodix: Tie the reset line to
true state of the regulator"), we started tying the reset line of
Goodix touchscreens to the regulator.
The primary motivation for that patch was some pre-production hardware
(specifically sc7180-trogdor-homestar) where it was proposed to hook
the touchscreen's main 3.3V power rail to an always-on supply. In such
a case, when we turned "off" the touchscreen in Linux it was bad to
assert the "reset" GPIO because that was causing a power drain. The
patch accomplished that goal and did it in a general sort of way that
didn't require special properties to be added in the device tree for
homestar.
It turns out that the design of using an always-on power rail for the
touchscreen was rejected soon after the patch was written and long
before sc7180-trogdor-homestar went into production. The final design
of homestar actually fully separates the rail for the touchscreen and
the display panel and both can be powered off and on. That means that
the original motivation for the feature is gone.
There are 3 other users of the goodix i2c-hid driver in mainline.
I'll first talk about 2 of the other users in mainline: coachz and
mrbland. On both coachz and mrbland the touchscreen power and panel
power _are_ shared. That means that the patch to tie the reset line to
the true state of the regulator _is_ doing something on those
boards. Specifically, the patch reduced power consumption by tens of
mA in the case where we turned the touchscreen off but left the panel
on. Other than saving a small bit of power, the patch wasn't truly
necessary. That being said, even though a small bit of power was saved
in the state of "panel on + touchscreen off", that's not actually a
state we ever expect to be in, except perhaps for very short periods
of time at boot or during suspend/resume. Thus, the patch is truly not
necessary. It should be further noted that, as documented in the
original patch, the current code still didn't optimize power for every
corner case of the "shared rail" situation.
The last user in mainline was very recently added: evoker. Evoker is
actually the motivation for me removing this bit of code. It turns out
that for evoker we need to manage a second power rail for IO to the
touchscreen. Trying to fit the management of this IO rail into the
regulator notifiers turns out to be extremely hard. To avoid lockdep
splats you shouldn't enable/disable other regulators in regulator
notifiers and trying to find a way around this was going to be fairly
difficult.
Given the lack of any true motivation to tie the reset line to the
regulator, lets go back to the simpler days and remove the code. This
is, effectively, a revert of commit bdbc65eb77ee ("HID: i2c-hid:
goodix: Fix a lockdep splat"), commit 25ddd7cfc582 ("HID: i2c-hid:
goodix: Use the devm variant of regulator_register_notifier()"), and
commit 18eeef46d359 ("HID: i2c-hid: goodix: Tie the reset line to true
state of the regulator").
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
drivers/hid/i2c-hid/i2c-hid-of-goodix.c | 88 ++++---------------------
1 file changed, 13 insertions(+), 75 deletions(-)
diff --git a/drivers/hid/i2c-hid/i2c-hid-of-goodix.c b/drivers/hid/i2c-hid/i2c-hid-of-goodix.c
index 29c6cb174032..584d833dc0aa 100644
--- a/drivers/hid/i2c-hid/i2c-hid-of-goodix.c
+++ b/drivers/hid/i2c-hid/i2c-hid-of-goodix.c
@@ -26,28 +26,28 @@ struct i2c_hid_of_goodix {
struct i2chid_ops ops;
struct regulator *vdd;
- struct notifier_block nb;
struct gpio_desc *reset_gpio;
const struct goodix_i2c_hid_timing_data *timings;
};
-static void goodix_i2c_hid_deassert_reset(struct i2c_hid_of_goodix *ihid_goodix,
- bool regulator_just_turned_on)
+static int goodix_i2c_hid_power_up(struct i2chid_ops *ops)
{
- if (regulator_just_turned_on && ihid_goodix->timings->post_power_delay_ms)
+ struct i2c_hid_of_goodix *ihid_goodix =
+ container_of(ops, struct i2c_hid_of_goodix, ops);
+ int ret;
+
+ ret = regulator_enable(ihid_goodix->vdd);
+ if (ret)
+ return ret;
+
+ if (ihid_goodix->timings->post_power_delay_ms)
msleep(ihid_goodix->timings->post_power_delay_ms);
gpiod_set_value_cansleep(ihid_goodix->reset_gpio, 0);
if (ihid_goodix->timings->post_gpio_reset_delay_ms)
msleep(ihid_goodix->timings->post_gpio_reset_delay_ms);
-}
-
-static int goodix_i2c_hid_power_up(struct i2chid_ops *ops)
-{
- struct i2c_hid_of_goodix *ihid_goodix =
- container_of(ops, struct i2c_hid_of_goodix, ops);
- return regulator_enable(ihid_goodix->vdd);
+ return 0;
}
static void goodix_i2c_hid_power_down(struct i2chid_ops *ops)
@@ -55,42 +55,14 @@ static void goodix_i2c_hid_power_down(struct i2chid_ops *ops)
struct i2c_hid_of_goodix *ihid_goodix =
container_of(ops, struct i2c_hid_of_goodix, ops);
+ gpiod_set_value_cansleep(ihid_goodix->reset_gpio, 1);
regulator_disable(ihid_goodix->vdd);
}
-static int ihid_goodix_vdd_notify(struct notifier_block *nb,
- unsigned long event,
- void *ignored)
-{
- struct i2c_hid_of_goodix *ihid_goodix =
- container_of(nb, struct i2c_hid_of_goodix, nb);
- int ret = NOTIFY_OK;
-
- switch (event) {
- case REGULATOR_EVENT_PRE_DISABLE:
- gpiod_set_value_cansleep(ihid_goodix->reset_gpio, 1);
- break;
-
- case REGULATOR_EVENT_ENABLE:
- goodix_i2c_hid_deassert_reset(ihid_goodix, true);
- break;
-
- case REGULATOR_EVENT_ABORT_DISABLE:
- goodix_i2c_hid_deassert_reset(ihid_goodix, false);
- break;
-
- default:
- ret = NOTIFY_DONE;
- break;
- }
-
- return ret;
-}
-
static int i2c_hid_of_goodix_probe(struct i2c_client *client)
{
struct i2c_hid_of_goodix *ihid_goodix;
- int ret;
+
ihid_goodix = devm_kzalloc(&client->dev, sizeof(*ihid_goodix),
GFP_KERNEL);
if (!ihid_goodix)
@@ -111,40 +83,6 @@ static int i2c_hid_of_goodix_probe(struct i2c_client *client)
ihid_goodix->timings = device_get_match_data(&client->dev);
- /*
- * We need to control the "reset" line in lockstep with the regulator
- * actually turning on an off instead of just when we make the request.
- * This matters if the regulator is shared with another consumer.
- * - If the regulator is off then we must assert reset. The reset
- * line is active low and on some boards it could cause a current
- * leak if left high.
- * - If the regulator is on then we don't want reset asserted for very
- * long. Holding the controller in reset apparently draws extra
- * power.
- */
- ihid_goodix->nb.notifier_call = ihid_goodix_vdd_notify;
- ret = devm_regulator_register_notifier(ihid_goodix->vdd, &ihid_goodix->nb);
- if (ret)
- return dev_err_probe(&client->dev, ret,
- "regulator notifier request failed\n");
-
- /*
- * If someone else is holding the regulator on (or the regulator is
- * an always-on one) we might never be told to deassert reset. Do it
- * now... and temporarily bump the regulator reference count just to
- * make sure it is impossible for this to race with our own notifier!
- * We also assume that someone else might have _just barely_ turned
- * the regulator on so we'll do the full "post_power_delay" just in
- * case.
- */
- if (ihid_goodix->reset_gpio && regulator_is_enabled(ihid_goodix->vdd)) {
- ret = regulator_enable(ihid_goodix->vdd);
- if (ret)
- return ret;
- goodix_i2c_hid_deassert_reset(ihid_goodix, true);
- regulator_disable(ihid_goodix->vdd);
- }
-
return i2c_hid_core_probe(client, &ihid_goodix->ops, 0x0001, 0);
}
--
2.39.1.519.gcb327c4b5f-goog
^ permalink raw reply related
* [PATCH 5/7] dt-bindings: HID: i2c-hid: goodix: Add mainboard-vddio-supply
From: Douglas Anderson @ 2023-02-07 2:48 UTC (permalink / raw)
To: Bjorn Andersson, Rob Herring, Krzysztof Kozlowski, Jiri Kosina,
Benjamin Tissoires
Cc: linux-input, linux-arm-msm, Dmitry Torokhov, devicetree,
Stephen Kitt, Konrad Dybcio, Douglas Anderson, linux-kernel
In-Reply-To: <20230207024816.525938-1-dianders@chromium.org>
The goodix i2c-hid bindings currently support two models of
touchscreen: GT7375P and GT7986U. The datasheets of both touchscreens
show the following things:
* The mainboard that the touchscreen is connected to is only expected
to supply one voltage to the touchscreen: 3.3V.
* The touchscreen, depending on stuffing options, can accept IO to the
touchscreen as either 3.3V or 1.8V. Presumably this means that the
touchscreen has its own way internally to make or deal with 1.8V
signals when it's configured for 1.8V IO.
NOTE: you've got to look very carefully at the datasheet for the
touchscreen to see that the above bullets are true. Specifically, the
datasheet shows a signal called VDDIO and one might think that this is
where a mainboard would provide VDDIO to the touchscreen. Upon closer
inspection, however, a footnote can be found that says "When VDDIO is
left floating, the logic level is 1.8V [...]; when VDDIO is connected
to AVDD, the logic level is AVDD.". Thus the VDDIO pin on the
touchscreen IC is actually a selector and not a pin whre the mainboard
would pass a reference voltage.
The fact that the touchscreen isn't supplied 1.8V by the mainboard
means that when I originally submitted bindings for these touchscreens
I only listed the 3.3V rail in the bindings. It can be noted that the
original bindings and driver were added for sc7180-trogdor boards and
these boards all use 3.3V IO via a level shifter on the mainboard.
It turns out that with sc7280-herobrine-evoker, we've got a bit of a
strange monkey on our hands. Due to some very interesting but
(unfortunately) set-in-stone hardware design, we are doing 1.8V IO to
the touchscreen but we _also_ have some extra buffers on the mainboard
that need to be powered up to make the IO lines work. After much
pondering about this, it seems like the best way to handle this is to
add an optional "mainboard-vddio" rail to the bindings that is used to
power up the buffers. Specifically, the fact that the touchscreen
datasheet documents that its IOs can be at a different voltage level
than its main power rail means that there truly are two voltage rails
associated with the touchscreen, even if we don't actually provide the
IO rail to it. Thus it doesn't feel absurd for the DT node on the host
to have a 1.8V rail to power up anything related to its 1.8V logic.
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
.../devicetree/bindings/input/goodix,gt7375p.yaml | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/Documentation/devicetree/bindings/input/goodix,gt7375p.yaml b/Documentation/devicetree/bindings/input/goodix,gt7375p.yaml
index 1c191bc5a178..ce18d7dadae2 100644
--- a/Documentation/devicetree/bindings/input/goodix,gt7375p.yaml
+++ b/Documentation/devicetree/bindings/input/goodix,gt7375p.yaml
@@ -36,6 +36,13 @@ properties:
vdd-supply:
description: The 3.3V supply to the touchscreen.
+ mainboard-vddio-supply:
+ description:
+ The supply on the main board needed to power up IO signals going
+ to the touchscreen. This supply need not go to the touchscreen
+ itself as long as it allows the main board to make signals compatible
+ with what the touchscreen is expecting for its IO rails.
+
required:
- compatible
- reg
--
2.39.1.519.gcb327c4b5f-goog
^ permalink raw reply related
* [PATCH 6/7] HID: i2c-hid: goodix: Add mainboard-vddio-supply
From: Douglas Anderson @ 2023-02-07 2:48 UTC (permalink / raw)
To: Bjorn Andersson, Rob Herring, Krzysztof Kozlowski, Jiri Kosina,
Benjamin Tissoires
Cc: linux-input, linux-arm-msm, Dmitry Torokhov, devicetree,
Stephen Kitt, Konrad Dybcio, Douglas Anderson, linux-kernel
In-Reply-To: <20230207024816.525938-1-dianders@chromium.org>
As talked about in the patch ("dt-bindings: HID: i2c-hid: goodix: Add
mainboard-vddio-supply") we may need to power up a 1.8V rail on the
host associated with touchscreen IO. Let's add support in the driver
for it.
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
Unfortunately, I haven't been able to actually test this on real
hardware yet. However, the change is very simple, I believe it is
correct, and it doesn't break other boards I've tested it on.
drivers/hid/i2c-hid/i2c-hid-of-goodix.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/hid/i2c-hid/i2c-hid-of-goodix.c b/drivers/hid/i2c-hid/i2c-hid-of-goodix.c
index 584d833dc0aa..0060e3dcd775 100644
--- a/drivers/hid/i2c-hid/i2c-hid-of-goodix.c
+++ b/drivers/hid/i2c-hid/i2c-hid-of-goodix.c
@@ -26,6 +26,7 @@ struct i2c_hid_of_goodix {
struct i2chid_ops ops;
struct regulator *vdd;
+ struct regulator *vddio;
struct gpio_desc *reset_gpio;
const struct goodix_i2c_hid_timing_data *timings;
};
@@ -40,6 +41,10 @@ static int goodix_i2c_hid_power_up(struct i2chid_ops *ops)
if (ret)
return ret;
+ ret = regulator_enable(ihid_goodix->vddio);
+ if (ret)
+ return ret;
+
if (ihid_goodix->timings->post_power_delay_ms)
msleep(ihid_goodix->timings->post_power_delay_ms);
@@ -56,6 +61,7 @@ static void goodix_i2c_hid_power_down(struct i2chid_ops *ops)
container_of(ops, struct i2c_hid_of_goodix, ops);
gpiod_set_value_cansleep(ihid_goodix->reset_gpio, 1);
+ regulator_disable(ihid_goodix->vddio);
regulator_disable(ihid_goodix->vdd);
}
@@ -81,6 +87,10 @@ static int i2c_hid_of_goodix_probe(struct i2c_client *client)
if (IS_ERR(ihid_goodix->vdd))
return PTR_ERR(ihid_goodix->vdd);
+ ihid_goodix->vddio = devm_regulator_get(&client->dev, "mainboard-vddio");
+ if (IS_ERR(ihid_goodix->vddio))
+ return PTR_ERR(ihid_goodix->vddio);
+
ihid_goodix->timings = device_get_match_data(&client->dev);
return i2c_hid_core_probe(client, &ihid_goodix->ops, 0x0001, 0);
--
2.39.1.519.gcb327c4b5f-goog
^ permalink raw reply related
* [PATCH 7/7] arm64: dts: qcom: sc7280: Hook up the touchscreen IO rail on evoker
From: Douglas Anderson @ 2023-02-07 2:48 UTC (permalink / raw)
To: Bjorn Andersson, Rob Herring, Krzysztof Kozlowski, Jiri Kosina,
Benjamin Tissoires
Cc: linux-input, linux-arm-msm, Dmitry Torokhov, devicetree,
Stephen Kitt, Konrad Dybcio, Douglas Anderson, Andy Gross,
linux-kernel
In-Reply-To: <20230207024816.525938-1-dianders@chromium.org>
On older revisions of evoker, the touchscreen was either
non-functional or needed special hardware magic to get it talking
properly. It's been decided that the proper way going forward is to
use L3C to power some buffers on the QCard and then configure the
touchscreens for 1.8V. Let's do that.
Note that this is safe to do even on older revs even if it might not
make the touchscreen work there (because they didn't have a properly
stuffed QCard). As talked about in the patch ("arm64: dts: qcom:
sc7280: On QCard, regulator L3C should be 1.8V") the L3C regulator
didn't go anywhere at all on older revs.
This patch relies on the patch ("HID: i2c-hid: goodix: Add
mainboard-vddio-supply") in order to function properly. Without that
patch this one won't do any harm but it won't actually accomplish its
goal.
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
I haven't yet received real hardware to test this on, but it's a very
simple patch and, in the very least, highly unlikely to make anything
worse. No real users have these boards yet.
arch/arm64/boot/dts/qcom/sc7280-herobrine-evoker.dtsi | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine-evoker.dtsi b/arch/arm64/boot/dts/qcom/sc7280-herobrine-evoker.dtsi
index 3d639c70a06e..0add7a2a099c 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-herobrine-evoker.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine-evoker.dtsi
@@ -55,6 +55,7 @@ ap_ts: touchscreen@5d {
reset-gpios = <&tlmm 54 GPIO_ACTIVE_LOW>;
vdd-supply = <&ts_avdd>;
+ mainboard-vddio-supply = <&ts_avccio>;
};
};
--
2.39.1.519.gcb327c4b5f-goog
^ permalink raw reply related
* Re: [PATCH 4/7] HID: i2c-hid: goodix: Stop tying the reset line to the regulator
From: Dmitry Torokhov @ 2023-02-07 3:58 UTC (permalink / raw)
To: Douglas Anderson
Cc: Bjorn Andersson, Rob Herring, Krzysztof Kozlowski, Jiri Kosina,
Benjamin Tissoires, linux-input, linux-arm-msm, devicetree,
Stephen Kitt, Konrad Dybcio, linux-kernel
In-Reply-To: <20230206184744.4.I085b32b6140c7d1ac4e7e97b712bff9dd5962b62@changeid>
On Mon, Feb 06, 2023 at 06:48:13PM -0800, Douglas Anderson wrote:
> In commit 18eeef46d359 ("HID: i2c-hid: goodix: Tie the reset line to
> true state of the regulator"), we started tying the reset line of
> Goodix touchscreens to the regulator.
>
> The primary motivation for that patch was some pre-production hardware
> (specifically sc7180-trogdor-homestar) where it was proposed to hook
> the touchscreen's main 3.3V power rail to an always-on supply. In such
> a case, when we turned "off" the touchscreen in Linux it was bad to
> assert the "reset" GPIO because that was causing a power drain. The
> patch accomplished that goal and did it in a general sort of way that
> didn't require special properties to be added in the device tree for
> homestar.
>
> It turns out that the design of using an always-on power rail for the
> touchscreen was rejected soon after the patch was written and long
> before sc7180-trogdor-homestar went into production. The final design
> of homestar actually fully separates the rail for the touchscreen and
> the display panel and both can be powered off and on. That means that
> the original motivation for the feature is gone.
>
> There are 3 other users of the goodix i2c-hid driver in mainline.
>
> I'll first talk about 2 of the other users in mainline: coachz and
> mrbland. On both coachz and mrbland the touchscreen power and panel
> power _are_ shared. That means that the patch to tie the reset line to
> the true state of the regulator _is_ doing something on those
> boards. Specifically, the patch reduced power consumption by tens of
> mA in the case where we turned the touchscreen off but left the panel
> on. Other than saving a small bit of power, the patch wasn't truly
> necessary. That being said, even though a small bit of power was saved
> in the state of "panel on + touchscreen off", that's not actually a
> state we ever expect to be in, except perhaps for very short periods
> of time at boot or during suspend/resume. Thus, the patch is truly not
> necessary. It should be further noted that, as documented in the
> original patch, the current code still didn't optimize power for every
> corner case of the "shared rail" situation.
>
> The last user in mainline was very recently added: evoker. Evoker is
> actually the motivation for me removing this bit of code. It turns out
> that for evoker we need to manage a second power rail for IO to the
> touchscreen. Trying to fit the management of this IO rail into the
> regulator notifiers turns out to be extremely hard. To avoid lockdep
> splats you shouldn't enable/disable other regulators in regulator
> notifiers and trying to find a way around this was going to be fairly
> difficult.
>
> Given the lack of any true motivation to tie the reset line to the
> regulator, lets go back to the simpler days and remove the code. This
> is, effectively, a revert of commit bdbc65eb77ee ("HID: i2c-hid:
> goodix: Fix a lockdep splat"), commit 25ddd7cfc582 ("HID: i2c-hid:
> goodix: Use the devm variant of regulator_register_notifier()"), and
> commit 18eeef46d359 ("HID: i2c-hid: goodix: Tie the reset line to true
> state of the regulator").
>
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
Reviewed-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
>
> drivers/hid/i2c-hid/i2c-hid-of-goodix.c | 88 ++++---------------------
> 1 file changed, 13 insertions(+), 75 deletions(-)
>
> diff --git a/drivers/hid/i2c-hid/i2c-hid-of-goodix.c b/drivers/hid/i2c-hid/i2c-hid-of-goodix.c
> index 29c6cb174032..584d833dc0aa 100644
> --- a/drivers/hid/i2c-hid/i2c-hid-of-goodix.c
> +++ b/drivers/hid/i2c-hid/i2c-hid-of-goodix.c
> @@ -26,28 +26,28 @@ struct i2c_hid_of_goodix {
> struct i2chid_ops ops;
>
> struct regulator *vdd;
> - struct notifier_block nb;
> struct gpio_desc *reset_gpio;
> const struct goodix_i2c_hid_timing_data *timings;
> };
>
> -static void goodix_i2c_hid_deassert_reset(struct i2c_hid_of_goodix *ihid_goodix,
> - bool regulator_just_turned_on)
> +static int goodix_i2c_hid_power_up(struct i2chid_ops *ops)
> {
> - if (regulator_just_turned_on && ihid_goodix->timings->post_power_delay_ms)
> + struct i2c_hid_of_goodix *ihid_goodix =
> + container_of(ops, struct i2c_hid_of_goodix, ops);
> + int ret;
> +
> + ret = regulator_enable(ihid_goodix->vdd);
> + if (ret)
> + return ret;
> +
> + if (ihid_goodix->timings->post_power_delay_ms)
> msleep(ihid_goodix->timings->post_power_delay_ms);
>
> gpiod_set_value_cansleep(ihid_goodix->reset_gpio, 0);
> if (ihid_goodix->timings->post_gpio_reset_delay_ms)
> msleep(ihid_goodix->timings->post_gpio_reset_delay_ms);
> -}
> -
> -static int goodix_i2c_hid_power_up(struct i2chid_ops *ops)
> -{
> - struct i2c_hid_of_goodix *ihid_goodix =
> - container_of(ops, struct i2c_hid_of_goodix, ops);
>
> - return regulator_enable(ihid_goodix->vdd);
> + return 0;
> }
>
> static void goodix_i2c_hid_power_down(struct i2chid_ops *ops)
> @@ -55,42 +55,14 @@ static void goodix_i2c_hid_power_down(struct i2chid_ops *ops)
> struct i2c_hid_of_goodix *ihid_goodix =
> container_of(ops, struct i2c_hid_of_goodix, ops);
>
> + gpiod_set_value_cansleep(ihid_goodix->reset_gpio, 1);
> regulator_disable(ihid_goodix->vdd);
> }
>
> -static int ihid_goodix_vdd_notify(struct notifier_block *nb,
> - unsigned long event,
> - void *ignored)
> -{
> - struct i2c_hid_of_goodix *ihid_goodix =
> - container_of(nb, struct i2c_hid_of_goodix, nb);
> - int ret = NOTIFY_OK;
> -
> - switch (event) {
> - case REGULATOR_EVENT_PRE_DISABLE:
> - gpiod_set_value_cansleep(ihid_goodix->reset_gpio, 1);
> - break;
> -
> - case REGULATOR_EVENT_ENABLE:
> - goodix_i2c_hid_deassert_reset(ihid_goodix, true);
> - break;
> -
> - case REGULATOR_EVENT_ABORT_DISABLE:
> - goodix_i2c_hid_deassert_reset(ihid_goodix, false);
> - break;
> -
> - default:
> - ret = NOTIFY_DONE;
> - break;
> - }
> -
> - return ret;
> -}
> -
> static int i2c_hid_of_goodix_probe(struct i2c_client *client)
> {
> struct i2c_hid_of_goodix *ihid_goodix;
> - int ret;
> +
> ihid_goodix = devm_kzalloc(&client->dev, sizeof(*ihid_goodix),
> GFP_KERNEL);
> if (!ihid_goodix)
> @@ -111,40 +83,6 @@ static int i2c_hid_of_goodix_probe(struct i2c_client *client)
>
> ihid_goodix->timings = device_get_match_data(&client->dev);
>
> - /*
> - * We need to control the "reset" line in lockstep with the regulator
> - * actually turning on an off instead of just when we make the request.
> - * This matters if the regulator is shared with another consumer.
> - * - If the regulator is off then we must assert reset. The reset
> - * line is active low and on some boards it could cause a current
> - * leak if left high.
> - * - If the regulator is on then we don't want reset asserted for very
> - * long. Holding the controller in reset apparently draws extra
> - * power.
> - */
> - ihid_goodix->nb.notifier_call = ihid_goodix_vdd_notify;
> - ret = devm_regulator_register_notifier(ihid_goodix->vdd, &ihid_goodix->nb);
> - if (ret)
> - return dev_err_probe(&client->dev, ret,
> - "regulator notifier request failed\n");
> -
> - /*
> - * If someone else is holding the regulator on (or the regulator is
> - * an always-on one) we might never be told to deassert reset. Do it
> - * now... and temporarily bump the regulator reference count just to
> - * make sure it is impossible for this to race with our own notifier!
> - * We also assume that someone else might have _just barely_ turned
> - * the regulator on so we'll do the full "post_power_delay" just in
> - * case.
> - */
> - if (ihid_goodix->reset_gpio && regulator_is_enabled(ihid_goodix->vdd)) {
> - ret = regulator_enable(ihid_goodix->vdd);
> - if (ret)
> - return ret;
> - goodix_i2c_hid_deassert_reset(ihid_goodix, true);
> - regulator_disable(ihid_goodix->vdd);
> - }
> -
> return i2c_hid_core_probe(client, &ihid_goodix->ops, 0x0001, 0);
> }
>
> --
> 2.39.1.519.gcb327c4b5f-goog
>
--
Dmitry
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox