On 12/8/21 16:18, Benjamin Tissoires wrote:
On Wed, Dec 1, 2021 at 5:43 PM Tero Kristo <tero.kristo@xxxxxxxxxxxxxxx> wrote:
Add support for Universal Stylus Interface (USI) style events to the HID
core and input layers.
Signed-off-by: Tero Kristo <tero.kristo@xxxxxxxxxxxxxxx>
---
drivers/hid/hid-input.c | 18 ++++++++++++++++++
include/linux/mod_devicetable.h | 2 +-
include/uapi/linux/hid.h | 10 ++++++++++
include/uapi/linux/input-event-codes.h | 22 ++++++++++++++--------
4 files changed, 43 insertions(+), 9 deletions(-)
diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index 73c2edda742e..b428ee9b4d9b 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -829,6 +829,10 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
}
break;
+ case 0x38: /* Transducer Index */
+ map_msc(MSC_PEN_ID);
This is the new slot version for pens, I am not sure we really want to
blindly introduce that without testing.
Do you have a panel that supports multiple values (at once) for this
usage (2 pens???). If not, I would simply skip that usage until we get
an actual hardware that makes use of it.
+ break;
+
case 0x3b: /* Battery Strength */
hidinput_setup_battery(device, HID_INPUT_REPORT, field, false);
usage->type = EV_PWR;
@@ -876,6 +880,20 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
map_msc(MSC_SERIAL);
break;
+ case 0x5c: map_msc(MSC_PEN_COLOR); break;
+ case 0x5e: map_msc(MSC_PEN_LINE_WIDTH); break;
We already have ABS_TOOL_WIDTH which seems to relate to that very closely.
+
+ case 0x70:
+ case 0x71:
+ case 0x72:
+ case 0x73:
+ case 0x74:
+ case 0x75:
+ case 0x76:
+ case 0x77:
+ map_msc(MSC_PEN_LINE_STYLE);
Nope, this is wrong. It took me a long time to understand the report
descriptor (see my last reply to your v2).
Basically, we need one input event for each value between 0x72 and
0x77. HID core should translate the array of 1 element into a set of
{depressed usage, pressed usage} and from the user-space, we will get
"MSC_PEN_STYLE_CHISEL_MARKER 1" for instance.
This seems to be working (on top of this RFC):
---
diff --git a/drivers/hid/hid-debug.c b/drivers/hid/hid-debug.c
index cec0470e5485..18035c63c358 100644
--- a/drivers/hid/hid-debug.c
+++ b/drivers/hid/hid-debug.c
@@ -1027,8 +1027,12 @@ static const char *misc[MSC_MAX + 1] = {
[MSC_SERIAL] = "Serial", [MSC_PULSELED] = "Pulseled",
[MSC_GESTURE] = "Gesture", [MSC_RAW] = "RawData",
[MSC_PEN_ID] = "PenID", [MSC_PEN_COLOR] "PenColor",
- [MSC_PEN_LINE_WIDTH] = "PenLineWidth",
- [MSC_PEN_LINE_STYLE] = "PenLineStyle",
+ [MSC_PEN_LINE_STYLE_INK] = "PenLineStyleInk",
+ [MSC_PEN_LINE_STYLE_PENCIL] = "PenLineStylePencil",
+ [MSC_PEN_LINE_STYLE_HIGHLIGHTER] = "PenLineStyleHighlighter",
+ [MSC_PEN_LINE_STYLE_CHISEL_MARKER] = "PenLineStyleChiselMarker",
+ [MSC_PEN_LINE_STYLE_BRUSH] = "PenLineStyleBrush",
+ [MSC_PEN_LINE_STYLE_NO_PREFERENCE] = "PenLineStyleNoPreference",
};
static const char *leds[LED_MAX + 1] = {
diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index 079e67c5168f..837585f4e673 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -832,8 +832,7 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
break;
case 0x38: /* Transducer Index */
- map_msc(MSC_PEN_ID);
- break;
+ goto ignore;
case 0x3b: /* Battery Strength */
hidinput_setup_battery(device, HID_INPUT_REPORT, field, false);
@@ -882,18 +881,36 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
map_msc(MSC_SERIAL);
break;
- case 0x5c: map_msc(MSC_PEN_COLOR); break;
- case 0x5e: map_msc(MSC_PEN_LINE_WIDTH); break;
-
- case 0x70:
- case 0x71:
- case 0x72:
- case 0x73:
- case 0x74:
- case 0x75:
- case 0x76:
- case 0x77:
- map_msc(MSC_PEN_LINE_STYLE);
+ case 0x5c: /* Digitizer Preferred Color */
+ map_msc(MSC_PEN_COLOR);
+ break;
+
+ case 0x5e: /* Digitizer Preferred Line Width */
+ map_abs_clear(ABS_TOOL_WIDTH);
+ break;
+
+
+ case 0x70: /* Preferred Line Style -> not an input usage*/
+ case 0x71: /* Preferred Line Style is Locked */
+ goto ignore;
+
+ case 0x72: /* Ink */
+ map_msc(MSC_PEN_LINE_STYLE_INK);
+ break;
+ case 0x73: /* Pencil */
+ map_msc(MSC_PEN_LINE_STYLE_PENCIL);
+ break;
+ case 0x74: /* Highlighter */
+ map_msc(MSC_PEN_LINE_STYLE_HIGHLIGHTER);
+ break;
+ case 0x75: /* Chisel Marker */
+ map_msc(MSC_PEN_LINE_STYLE_CHISEL_MARKER);
+ break;
+ case 0x76: /* Brush */
+ map_msc(MSC_PEN_LINE_STYLE_BRUSH);
+ break;
+ case 0x77: /* No Preference */
+ map_msc(MSC_PEN_LINE_STYLE_NO_PREFERENCE);
break;
default: goto unknown;
diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
index a76d37082ab5..bf2b76a6e7e8 100644
--- a/include/linux/mod_devicetable.h
+++ b/include/linux/mod_devicetable.h
@@ -322,7 +322,7 @@ struct pcmcia_device_id {
#define INPUT_DEVICE_ID_KEY_MAX 0x2ff
#define INPUT_DEVICE_ID_REL_MAX 0x0f
#define INPUT_DEVICE_ID_ABS_MAX 0x3f
-#define INPUT_DEVICE_ID_MSC_MAX 0x09
+#define INPUT_DEVICE_ID_MSC_MAX 0x0d
#define INPUT_DEVICE_ID_LED_MAX 0x0f
#define INPUT_DEVICE_ID_SND_MAX 0x07
#define INPUT_DEVICE_ID_FF_MAX 0x7f
diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h
index 98295f71941a..ff705245b7ae 100644
--- a/include/uapi/linux/input-event-codes.h
+++ b/include/uapi/linux/input-event-codes.h
@@ -908,12 +908,16 @@
#define MSC_SCAN 0x04
#define MSC_TIMESTAMP 0x05
/* USI Pen events */
-#define MSC_PEN_ID 0x06
-#define MSC_PEN_COLOR 0x07
-#define MSC_PEN_LINE_WIDTH 0x08
-#define MSC_PEN_LINE_STYLE 0x09
+#define MSC_PEN_ID 0x06
+#define MSC_PEN_COLOR 0x07
+#define MSC_PEN_LINE_STYLE_INK 0x08
+#define MSC_PEN_LINE_STYLE_PENCIL 0x09
+#define MSC_PEN_LINE_STYLE_HIGHLIGHTER 0x0a
+#define MSC_PEN_LINE_STYLE_CHISEL_MARKER 0x0b
+#define MSC_PEN_LINE_STYLE_BRUSH 0x0c
+#define MSC_PEN_LINE_STYLE_NO_PREFERENCE 0x0d
/* TODO: Add USI diagnostic & battery events too */
-#define MSC_MAX 0x09
+#define MSC_MAX 0x0d
#define MSC_CNT (MSC_MAX + 1)
/*
---
Cheers,
Benjamin
And *maybe* we could even use KEY events there, so we could remap them
(but that's a very distant maybe).
+ break;
+
default: goto unknown;
}
break;
diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
index ae2e75d15b21..4ff40be7676b 100644
--- a/include/linux/mod_devicetable.h
+++ b/include/linux/mod_devicetable.h
@@ -322,7 +322,7 @@ struct pcmcia_device_id {
#define INPUT_DEVICE_ID_KEY_MAX 0x2ff
#define INPUT_DEVICE_ID_REL_MAX 0x0f
#define INPUT_DEVICE_ID_ABS_MAX 0x3f
-#define INPUT_DEVICE_ID_MSC_MAX 0x07
+#define INPUT_DEVICE_ID_MSC_MAX 0x09
#define INPUT_DEVICE_ID_LED_MAX 0x0f
#define INPUT_DEVICE_ID_SND_MAX 0x07
#define INPUT_DEVICE_ID_FF_MAX 0x7f
diff --git a/include/uapi/linux/hid.h b/include/uapi/linux/hid.h
index 861bfbbfc565..60ef9b615a1a 100644
--- a/include/uapi/linux/hid.h
+++ b/include/uapi/linux/hid.h
@@ -255,6 +255,7 @@
#define HID_DG_TOUCH 0x000d0033
#define HID_DG_UNTOUCH 0x000d0034
#define HID_DG_TAP 0x000d0035
+#define HID_DG_TRANSDUCER_INDEX 0x000d0038
#define HID_DG_TABLETFUNCTIONKEY 0x000d0039
#define HID_DG_PROGRAMCHANGEKEY 0x000d003a
#define HID_DG_BATTERYSTRENGTH 0x000d003b
@@ -267,6 +268,15 @@
#define HID_DG_BARRELSWITCH 0x000d0044
#define HID_DG_ERASER 0x000d0045
#define HID_DG_TABLETPICK 0x000d0046
+#define HID_DG_PEN_COLOR 0x000d005c
+#define HID_DG_PEN_LINE_WIDTH 0x000d005e
+#define HID_DG_PEN_LINE_STYLE 0x000d0070
+#define HID_DG_PEN_LINE_STYLE_INK 0x000d0072
+#define HID_DG_PEN_LINE_STYLE_PENCIL 0x000d0073
+#define HID_DG_PEN_LINE_STYLE_HIGHLIGHTER 0x000d0074
+#define HID_DG_PEN_LINE_STYLE_CHISEL_MARKER 0x000d0075
+#define HID_DG_PEN_LINE_STYLE_BRUSH 0x000d0076
+#define HID_DG_PEN_LINE_STYLE_NO_PREFERENCE 0x000d0077
Could you integrate that patch before my patch "HID: export the
various HID defines from the spec in the uapi"? And also have it as a
separate patch from the mapping?
This way I can schedule that part earlier before we settle on the
actual user space usages.
#define HID_CP_CONSUMERCONTROL 0x000c0001
#define HID_CP_NUMERICKEYPAD 0x000c0002
diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h
index 225ec87d4f22..98295f71941a 100644
--- a/include/uapi/linux/input-event-codes.h
+++ b/include/uapi/linux/input-event-codes.h
@@ -901,14 +901,20 @@
* Misc events
*/
-#define MSC_SERIAL 0x00
-#define MSC_PULSELED 0x01
-#define MSC_GESTURE 0x02
-#define MSC_RAW 0x03
-#define MSC_SCAN 0x04
-#define MSC_TIMESTAMP 0x05
-#define MSC_MAX 0x07
-#define MSC_CNT (MSC_MAX+1)
+#define MSC_SERIAL 0x00
+#define MSC_PULSELED 0x01
+#define MSC_GESTURE 0x02
+#define MSC_RAW 0x03
+#define MSC_SCAN 0x04
+#define MSC_TIMESTAMP 0x05
+/* USI Pen events */
+#define MSC_PEN_ID 0x06
+#define MSC_PEN_COLOR 0x07
+#define MSC_PEN_LINE_WIDTH 0x08
PEN_LINE_WIDTH should be dropped.
+#define MSC_PEN_LINE_STYLE 0x09
Again, PEN_LINE_STYLE is HID only and we should only map the values,
not the title of the array.
Cheers,
Benjamin
+/* TODO: Add USI diagnostic & battery events too */
+#define MSC_MAX 0x09
+#define MSC_CNT (MSC_MAX + 1)
/*
* LEDs
--
2.25.1
