* [RFCv3 1/7] HID: Add map_msc() to avoid boilerplate code
2021-12-01 16:42 [RFCv3 0/7] USI stylus support series Tero Kristo
@ 2021-12-01 16:42 ` Tero Kristo
2021-12-08 15:03 ` Benjamin Tissoires
2021-12-01 16:42 ` [RFCv3 2/7] HID: hid-input: Add suffix also for HID_DG_PEN Tero Kristo
` (6 subsequent siblings)
7 siblings, 1 reply; 16+ messages in thread
From: Tero Kristo @ 2021-12-01 16:42 UTC (permalink / raw)
To: linux-input, benjamin.tissoires, jikos, mika.westerberg, tero.kristo
Cc: linux-kernel, dmitry.torokhov, peter.hutterer
From: Mika Westerberg <mika.westerberg@linux.intel.com>
Since we are going to have more MSC events too, add map_msc() that can
be used to fill in necessary fields and avoid boilerplate code.
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
drivers/hid/hid-input.c | 6 ++----
include/linux/hid.h | 4 ++++
2 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index 2c72ce4147b1..39ebedb2323b 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -52,6 +52,7 @@ static const struct {
#define map_rel(c) hid_map_usage(hidinput, usage, &bit, &max, EV_REL, (c))
#define map_key(c) hid_map_usage(hidinput, usage, &bit, &max, EV_KEY, (c))
#define map_led(c) hid_map_usage(hidinput, usage, &bit, &max, EV_LED, (c))
+#define map_msc(c) hid_map_usage(hidinput, usage, &bit, &max, EV_MSC, (c))
#define map_abs_clear(c) hid_map_usage_clear(hidinput, usage, &bit, \
&max, EV_ABS, (c))
@@ -872,10 +873,7 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
case 0x5b: /* TransducerSerialNumber */
case 0x6e: /* TransducerSerialNumber2 */
- usage->type = EV_MSC;
- usage->code = MSC_SERIAL;
- bit = input->mscbit;
- max = MSC_MAX;
+ map_msc(MSC_SERIAL);
break;
default: goto unknown;
diff --git a/include/linux/hid.h b/include/linux/hid.h
index e95800bab56a..96eaca0d5322 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -766,6 +766,10 @@ static inline void hid_map_usage(struct hid_input *hidinput,
bmap = input->ledbit;
limit = LED_MAX;
break;
+ case EV_MSC:
+ bmap = input->mscbit;
+ limit = MSC_MAX;
+ break;
}
if (unlikely(c > limit || !bmap)) {
--
2.25.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [RFCv3 1/7] HID: Add map_msc() to avoid boilerplate code
2021-12-01 16:42 ` [RFCv3 1/7] HID: Add map_msc() to avoid boilerplate code Tero Kristo
@ 2021-12-08 15:03 ` Benjamin Tissoires
0 siblings, 0 replies; 16+ messages in thread
From: Benjamin Tissoires @ 2021-12-08 15:03 UTC (permalink / raw)
To: Tero Kristo
Cc: open list:HID CORE LAYER, Jiri Kosina, Mika Westerberg, lkml,
Dmitry Torokhov, Peter Hutterer
On Wed, Dec 1, 2021 at 5:43 PM Tero Kristo <tero.kristo@linux.intel.com> wrote:
>
> From: Mika Westerberg <mika.westerberg@linux.intel.com>
>
> Since we are going to have more MSC events too, add map_msc() that can
> be used to fill in necessary fields and avoid boilerplate code.
>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
This one looks good to me and I should be able to take it in v5.17.
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Cheers,
Benjamin
> ---
> drivers/hid/hid-input.c | 6 ++----
> include/linux/hid.h | 4 ++++
> 2 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
> index 2c72ce4147b1..39ebedb2323b 100644
> --- a/drivers/hid/hid-input.c
> +++ b/drivers/hid/hid-input.c
> @@ -52,6 +52,7 @@ static const struct {
> #define map_rel(c) hid_map_usage(hidinput, usage, &bit, &max, EV_REL, (c))
> #define map_key(c) hid_map_usage(hidinput, usage, &bit, &max, EV_KEY, (c))
> #define map_led(c) hid_map_usage(hidinput, usage, &bit, &max, EV_LED, (c))
> +#define map_msc(c) hid_map_usage(hidinput, usage, &bit, &max, EV_MSC, (c))
>
> #define map_abs_clear(c) hid_map_usage_clear(hidinput, usage, &bit, \
> &max, EV_ABS, (c))
> @@ -872,10 +873,7 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
>
> case 0x5b: /* TransducerSerialNumber */
> case 0x6e: /* TransducerSerialNumber2 */
> - usage->type = EV_MSC;
> - usage->code = MSC_SERIAL;
> - bit = input->mscbit;
> - max = MSC_MAX;
> + map_msc(MSC_SERIAL);
> break;
>
> default: goto unknown;
> diff --git a/include/linux/hid.h b/include/linux/hid.h
> index e95800bab56a..96eaca0d5322 100644
> --- a/include/linux/hid.h
> +++ b/include/linux/hid.h
> @@ -766,6 +766,10 @@ static inline void hid_map_usage(struct hid_input *hidinput,
> bmap = input->ledbit;
> limit = LED_MAX;
> break;
> + case EV_MSC:
> + bmap = input->mscbit;
> + limit = MSC_MAX;
> + break;
> }
>
> if (unlikely(c > limit || !bmap)) {
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [RFCv3 2/7] HID: hid-input: Add suffix also for HID_DG_PEN
2021-12-01 16:42 [RFCv3 0/7] USI stylus support series Tero Kristo
2021-12-01 16:42 ` [RFCv3 1/7] HID: Add map_msc() to avoid boilerplate code Tero Kristo
@ 2021-12-01 16:42 ` Tero Kristo
2021-12-08 15:06 ` Benjamin Tissoires
2021-12-01 16:42 ` [RFCv3 3/7] HID: core: Add support for USI style events Tero Kristo
` (5 subsequent siblings)
7 siblings, 1 reply; 16+ messages in thread
From: Tero Kristo @ 2021-12-01 16:42 UTC (permalink / raw)
To: linux-input, benjamin.tissoires, jikos, mika.westerberg, tero.kristo
Cc: linux-kernel, dmitry.torokhov, peter.hutterer
From: Mika Westerberg <mika.westerberg@linux.intel.com>
This and HID_DG_STYLUS are pretty much the same thing so add suffix for
HID_DG_PEN too. This makes the input device name look better.
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
drivers/hid/hid-input.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index 39ebedb2323b..73c2edda742e 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -1737,6 +1737,7 @@ static struct hid_input *hidinput_allocate(struct hid_device *hid,
case HID_GD_MOUSE:
suffix = "Mouse";
break;
+ case HID_DG_PEN:
case HID_DG_STYLUS:
suffix = "Pen";
break;
--
2.25.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [RFCv3 2/7] HID: hid-input: Add suffix also for HID_DG_PEN
2021-12-01 16:42 ` [RFCv3 2/7] HID: hid-input: Add suffix also for HID_DG_PEN Tero Kristo
@ 2021-12-08 15:06 ` Benjamin Tissoires
0 siblings, 0 replies; 16+ messages in thread
From: Benjamin Tissoires @ 2021-12-08 15:06 UTC (permalink / raw)
To: Tero Kristo
Cc: open list:HID CORE LAYER, Jiri Kosina, Mika Westerberg, lkml,
Dmitry Torokhov, Peter Hutterer
On Wed, Dec 1, 2021 at 5:43 PM Tero Kristo <tero.kristo@linux.intel.com> wrote:
>
> From: Mika Westerberg <mika.westerberg@linux.intel.com>
>
> This and HID_DG_STYLUS are pretty much the same thing so add suffix for
> HID_DG_PEN too. This makes the input device name look better.
>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> ---
> drivers/hid/hid-input.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
> index 39ebedb2323b..73c2edda742e 100644
> --- a/drivers/hid/hid-input.c
> +++ b/drivers/hid/hid-input.c
> @@ -1737,6 +1737,7 @@ static struct hid_input *hidinput_allocate(struct hid_device *hid,
> case HID_GD_MOUSE:
> suffix = "Mouse";
> break;
> + case HID_DG_PEN:
hid-multitouch overrides this in mt_input_configured() and sets
"Stylus" instead.
A common code would be best, but now I realizes that this would mean:
HID_DG_PEN -> "Stylus"
HID_DG_STYLUS -> "Pen"
This is part of the configuration API, because we can have
configuration snippets based on the device name :)
So ideally, we need:
this patch to add DG_PEN support in hid-input.c and remove the special
case from hid-multitouch, with a comment explaining the Pen/Stylus
mistake :)
Cheers,
Benjamin
> case HID_DG_STYLUS:
> suffix = "Pen";
> break;
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [RFCv3 3/7] HID: core: Add support for USI style events
2021-12-01 16:42 [RFCv3 0/7] USI stylus support series Tero Kristo
2021-12-01 16:42 ` [RFCv3 1/7] HID: Add map_msc() to avoid boilerplate code Tero Kristo
2021-12-01 16:42 ` [RFCv3 2/7] HID: hid-input: Add suffix also for HID_DG_PEN Tero Kristo
@ 2021-12-01 16:42 ` Tero Kristo
2021-12-08 15:18 ` Benjamin Tissoires
2021-12-01 16:42 ` [RFCv3 4/7] HID: input: Make hidinput_find_field() static Tero Kristo
` (4 subsequent siblings)
7 siblings, 1 reply; 16+ messages in thread
From: Tero Kristo @ 2021-12-01 16:42 UTC (permalink / raw)
To: linux-input, benjamin.tissoires, jikos, mika.westerberg, tero.kristo
Cc: linux-kernel, dmitry.torokhov, peter.hutterer
Add support for Universal Stylus Interface (USI) style events to the HID
core and input layers.
Signed-off-by: Tero Kristo <tero.kristo@linux.intel.com>
---
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);
+ 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;
+
+ case 0x70:
+ case 0x71:
+ case 0x72:
+ case 0x73:
+ case 0x74:
+ case 0x75:
+ case 0x76:
+ case 0x77:
+ map_msc(MSC_PEN_LINE_STYLE);
+ 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
#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
+#define MSC_PEN_LINE_STYLE 0x09
+/* TODO: Add USI diagnostic & battery events too */
+#define MSC_MAX 0x09
+#define MSC_CNT (MSC_MAX + 1)
/*
* LEDs
--
2.25.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [RFCv3 3/7] HID: core: Add support for USI style events
2021-12-01 16:42 ` [RFCv3 3/7] HID: core: Add support for USI style events Tero Kristo
@ 2021-12-08 15:18 ` Benjamin Tissoires
2021-12-08 16:33 ` Benjamin Tissoires
2021-12-10 10:31 ` Tero Kristo
0 siblings, 2 replies; 16+ messages in thread
From: Benjamin Tissoires @ 2021-12-08 15:18 UTC (permalink / raw)
To: Tero Kristo
Cc: open list:HID CORE LAYER, Jiri Kosina, Mika Westerberg, lkml,
Dmitry Torokhov, Peter Hutterer
On Wed, Dec 1, 2021 at 5:43 PM Tero Kristo <tero.kristo@linux.intel.com> wrote:
>
> Add support for Universal Stylus Interface (USI) style events to the HID
> core and input layers.
>
> Signed-off-by: Tero Kristo <tero.kristo@linux.intel.com>
> ---
> 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.
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
>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [RFCv3 3/7] HID: core: Add support for USI style events
2021-12-08 15:18 ` Benjamin Tissoires
@ 2021-12-08 16:33 ` Benjamin Tissoires
2021-12-10 10:31 ` Tero Kristo
1 sibling, 0 replies; 16+ messages in thread
From: Benjamin Tissoires @ 2021-12-08 16:33 UTC (permalink / raw)
To: Tero Kristo
Cc: open list:HID CORE LAYER, Jiri Kosina, Mika Westerberg, lkml,
Dmitry Torokhov, Peter Hutterer
On 12/8/21 16:18, Benjamin Tissoires wrote:
> On Wed, Dec 1, 2021 at 5:43 PM Tero Kristo <tero.kristo@linux.intel.com> wrote:
>>
>> Add support for Universal Stylus Interface (USI) style events to the HID
>> core and input layers.
>>
>> Signed-off-by: Tero Kristo <tero.kristo@linux.intel.com>
>> ---
>> 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
>>
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [RFCv3 3/7] HID: core: Add support for USI style events
2021-12-08 15:18 ` Benjamin Tissoires
2021-12-08 16:33 ` Benjamin Tissoires
@ 2021-12-10 10:31 ` Tero Kristo
1 sibling, 0 replies; 16+ messages in thread
From: Tero Kristo @ 2021-12-10 10:31 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: open list:HID CORE LAYER, Jiri Kosina, Mika Westerberg, lkml,
Dmitry Torokhov, Peter Hutterer
Hi Benjamin,
On 08/12/2021 17:18, Benjamin Tissoires wrote:
> On Wed, Dec 1, 2021 at 5:43 PM Tero Kristo <tero.kristo@linux.intel.com> wrote:
>> Add support for Universal Stylus Interface (USI) style events to the HID
>> core and input layers.
>>
>> Signed-off-by: Tero Kristo <tero.kristo@linux.intel.com>
>> ---
>> 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.
Afaik, current controllers (panels) only support single pen at a time.
You can use multiple pens with them and they effectively re-use the same
pen-id, but you can't use them at the same time.
>
>> + 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.
Ok, I can switch to use this one.
>
>> +
>> + 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.
>
> 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.
Ok will update this, thanks.
-Tero
>
> Cheers,
> Benjamin
>
>> +/* TODO: Add USI diagnostic & battery events too */
>> +#define MSC_MAX 0x09
>> +#define MSC_CNT (MSC_MAX + 1)
>>
>> /*
>> * LEDs
>> --
>> 2.25.1
>>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [RFCv3 4/7] HID: input: Make hidinput_find_field() static
2021-12-01 16:42 [RFCv3 0/7] USI stylus support series Tero Kristo
` (2 preceding siblings ...)
2021-12-01 16:42 ` [RFCv3 3/7] HID: core: Add support for USI style events Tero Kristo
@ 2021-12-01 16:42 ` Tero Kristo
2021-12-08 15:19 ` Benjamin Tissoires
2021-12-01 16:42 ` [RFCv3 5/7] HID: debug: Add USI usages Tero Kristo
` (3 subsequent siblings)
7 siblings, 1 reply; 16+ messages in thread
From: Tero Kristo @ 2021-12-01 16:42 UTC (permalink / raw)
To: linux-input, benjamin.tissoires, jikos, mika.westerberg, tero.kristo
Cc: linux-kernel, dmitry.torokhov, peter.hutterer
From: Mika Westerberg <mika.westerberg@linux.intel.com>
This function is not called outside of hid-input.c so we can make it
static.
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
drivers/hid/hid-input.c | 4 ++--
include/linux/hid.h | 1 -
2 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index b428ee9b4d9b..f6332d269d49 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -1477,7 +1477,8 @@ void hidinput_report_event(struct hid_device *hid, struct hid_report *report)
}
EXPORT_SYMBOL_GPL(hidinput_report_event);
-int hidinput_find_field(struct hid_device *hid, unsigned int type, unsigned int code, struct hid_field **field)
+static int hidinput_find_field(struct hid_device *hid, unsigned int type,
+ unsigned int code, struct hid_field **field)
{
struct hid_report *report;
int i, j;
@@ -1492,7 +1493,6 @@ int hidinput_find_field(struct hid_device *hid, unsigned int type, unsigned int
}
return -1;
}
-EXPORT_SYMBOL_GPL(hidinput_find_field);
struct hid_field *hidinput_get_led_field(struct hid_device *hid)
{
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 96eaca0d5322..3f1fd4fcf1a9 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -636,7 +636,6 @@ extern void hidinput_disconnect(struct hid_device *);
int hid_set_field(struct hid_field *, unsigned, __s32);
int hid_input_report(struct hid_device *, int type, u8 *, u32, int);
-int hidinput_find_field(struct hid_device *hid, unsigned int type, unsigned int code, struct hid_field **field);
struct hid_field *hidinput_get_led_field(struct hid_device *hid);
unsigned int hidinput_count_leds(struct hid_device *hid);
__s32 hidinput_calc_abs_res(const struct hid_field *field, __u16 code);
--
2.25.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [RFCv3 4/7] HID: input: Make hidinput_find_field() static
2021-12-01 16:42 ` [RFCv3 4/7] HID: input: Make hidinput_find_field() static Tero Kristo
@ 2021-12-08 15:19 ` Benjamin Tissoires
0 siblings, 0 replies; 16+ messages in thread
From: Benjamin Tissoires @ 2021-12-08 15:19 UTC (permalink / raw)
To: Tero Kristo
Cc: open list:HID CORE LAYER, Jiri Kosina, Mika Westerberg, lkml,
Dmitry Torokhov, Peter Hutterer
On Wed, Dec 1, 2021 at 5:43 PM Tero Kristo <tero.kristo@linux.intel.com> wrote:
>
> From: Mika Westerberg <mika.westerberg@linux.intel.com>
>
> This function is not called outside of hid-input.c so we can make it
> static.
>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Cheers,
Benjamin
> ---
> drivers/hid/hid-input.c | 4 ++--
> include/linux/hid.h | 1 -
> 2 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
> index b428ee9b4d9b..f6332d269d49 100644
> --- a/drivers/hid/hid-input.c
> +++ b/drivers/hid/hid-input.c
> @@ -1477,7 +1477,8 @@ void hidinput_report_event(struct hid_device *hid, struct hid_report *report)
> }
> EXPORT_SYMBOL_GPL(hidinput_report_event);
>
> -int hidinput_find_field(struct hid_device *hid, unsigned int type, unsigned int code, struct hid_field **field)
> +static int hidinput_find_field(struct hid_device *hid, unsigned int type,
> + unsigned int code, struct hid_field **field)
> {
> struct hid_report *report;
> int i, j;
> @@ -1492,7 +1493,6 @@ int hidinput_find_field(struct hid_device *hid, unsigned int type, unsigned int
> }
> return -1;
> }
> -EXPORT_SYMBOL_GPL(hidinput_find_field);
>
> struct hid_field *hidinput_get_led_field(struct hid_device *hid)
> {
> diff --git a/include/linux/hid.h b/include/linux/hid.h
> index 96eaca0d5322..3f1fd4fcf1a9 100644
> --- a/include/linux/hid.h
> +++ b/include/linux/hid.h
> @@ -636,7 +636,6 @@ extern void hidinput_disconnect(struct hid_device *);
>
> int hid_set_field(struct hid_field *, unsigned, __s32);
> int hid_input_report(struct hid_device *, int type, u8 *, u32, int);
> -int hidinput_find_field(struct hid_device *hid, unsigned int type, unsigned int code, struct hid_field **field);
> struct hid_field *hidinput_get_led_field(struct hid_device *hid);
> unsigned int hidinput_count_leds(struct hid_device *hid);
> __s32 hidinput_calc_abs_res(const struct hid_field *field, __u16 code);
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [RFCv3 5/7] HID: debug: Add USI usages
2021-12-01 16:42 [RFCv3 0/7] USI stylus support series Tero Kristo
` (3 preceding siblings ...)
2021-12-01 16:42 ` [RFCv3 4/7] HID: input: Make hidinput_find_field() static Tero Kristo
@ 2021-12-01 16:42 ` Tero Kristo
2021-12-01 16:43 ` [RFCv3 6/7] samples: hid: add new hid-usi sample Tero Kristo
` (2 subsequent siblings)
7 siblings, 0 replies; 16+ messages in thread
From: Tero Kristo @ 2021-12-01 16:42 UTC (permalink / raw)
To: linux-input, benjamin.tissoires, jikos, mika.westerberg, tero.kristo
Cc: linux-kernel, dmitry.torokhov, peter.hutterer
From: Mika Westerberg <mika.westerberg@linux.intel.com>
Add USI defined usages to the HID debug code.
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
[tero.kristo: squashed in EV_MSC definitions]
Signed-off-by: Tero Kristo <tero.kristo@linux.intel.com>
---
drivers/hid/hid-debug.c | 40 +++++++++++++++++++++++++++++++++++++++-
1 file changed, 39 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/hid-debug.c b/drivers/hid/hid-debug.c
index 7a92e2a04a09..cec0470e5485 100644
--- a/drivers/hid/hid-debug.c
+++ b/drivers/hid/hid-debug.c
@@ -141,8 +141,10 @@ static const struct hid_usage_entry hid_usage_table[] = {
{0, 0x33, "Touch"},
{0, 0x34, "UnTouch"},
{0, 0x35, "Tap"},
+ {0, 0x38, "Transducer Index"},
{0, 0x39, "TabletFunctionKey"},
{0, 0x3a, "ProgramChangeKey"},
+ {0, 0x3B, "Battery Strength"},
{0, 0x3c, "Invert"},
{0, 0x42, "TipSwitch"},
{0, 0x43, "SecondaryTipSwitch"},
@@ -160,7 +162,40 @@ static const struct hid_usage_entry hid_usage_table[] = {
{0, 0x59, "ButtonType"},
{0, 0x5A, "SecondaryBarrelSwitch"},
{0, 0x5B, "TransducerSerialNumber"},
+ {0, 0x5C, "Preferred Color"},
+ {0, 0x5D, "Preferred Color is Locked"},
+ {0, 0x5E, "Preferred Line Width"},
+ {0, 0x5F, "Preferred Line Width is Locked"},
{0, 0x6e, "TransducerSerialNumber2"},
+ {0, 0x70, "Preferred Line Style"},
+ {0, 0x71, "Preferred Line Style is Locked"},
+ {0, 0x72, "Ink"},
+ {0, 0x73, "Pencil"},
+ {0, 0x74, "Highlighter"},
+ {0, 0x75, "Chisel Marker"},
+ {0, 0x76, "Brush"},
+ {0, 0x77, "No Preference"},
+ {0, 0x80, "Digitizer Diagnostic"},
+ {0, 0x81, "Digitizer Error"},
+ {0, 0x82, "Err Normal Status"},
+ {0, 0x83, "Err Transducers Exceeded"},
+ {0, 0x84, "Err Full Trans Features Unavailable"},
+ {0, 0x85, "Err Charge Low"},
+ {0, 0x90, "Transducer Software Info"},
+ {0, 0x91, "Transducer Vendor Id"},
+ {0, 0x92, "Transducer Product Id"},
+ {0, 0x93, "Device Supported Protocols"},
+ {0, 0x94, "Transducer Supported Protocols"},
+ {0, 0x95, "No Protocol"},
+ {0, 0x96, "Wacom AES Protocol"},
+ {0, 0x97, "USI Protocol"},
+ {0, 0x98, "Microsoft Pen Protocol"},
+ {0, 0xA0, "Supported Report Rates"},
+ {0, 0xA1, "Report Rate"},
+ {0, 0xA2, "Transducer Connected"},
+ {0, 0xA3, "Switch Disabled"},
+ {0, 0xA4, "Switch Unimplemented"},
+ {0, 0xA5, "Transducer Switches"},
{ 15, 0, "PhysicalInterfaceDevice" },
{0, 0x00, "Undefined"},
{0, 0x01, "Physical_Interface_Device"},
@@ -990,7 +1025,10 @@ static const char *absolutes[ABS_CNT] = {
static const char *misc[MSC_MAX + 1] = {
[MSC_SERIAL] = "Serial", [MSC_PULSELED] = "Pulseled",
- [MSC_GESTURE] = "Gesture", [MSC_RAW] = "RawData"
+ [MSC_GESTURE] = "Gesture", [MSC_RAW] = "RawData",
+ [MSC_PEN_ID] = "PenID", [MSC_PEN_COLOR] "PenColor",
+ [MSC_PEN_LINE_WIDTH] = "PenLineWidth",
+ [MSC_PEN_LINE_STYLE] = "PenLineStyle",
};
static const char *leds[LED_MAX + 1] = {
--
2.25.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [RFCv3 6/7] samples: hid: add new hid-usi sample
2021-12-01 16:42 [RFCv3 0/7] USI stylus support series Tero Kristo
` (4 preceding siblings ...)
2021-12-01 16:42 ` [RFCv3 5/7] HID: debug: Add USI usages Tero Kristo
@ 2021-12-01 16:43 ` Tero Kristo
2021-12-01 16:43 ` [RFCv3 7/7] samples: hid: convert USI sample to use unix socket for comms Tero Kristo
2021-12-08 15:30 ` [RFCv3 0/7] USI stylus support series Benjamin Tissoires
7 siblings, 0 replies; 16+ messages in thread
From: Tero Kristo @ 2021-12-01 16:43 UTC (permalink / raw)
To: linux-input, benjamin.tissoires, jikos, mika.westerberg, tero.kristo
Cc: linux-kernel, dmitry.torokhov, peter.hutterer
HID-usi sample adds support for writing USI pen variables. When the
sample is run, it creates a unix fifo under /tmp/usi, which can be
written with "<param> set <val>", where param is in [color,width,style],
and val is an integer value for the parameter.
Signed-off-by: Tero Kristo <tero.kristo@linux.intel.com>
---
samples/bpf/Makefile | 3 +
samples/bpf/hid_usi.h | 21 ++++
samples/bpf/hid_usi_kern.c | 230 ++++++++++++++++++++++++++++++++++
samples/bpf/hid_usi_user.c | 247 +++++++++++++++++++++++++++++++++++++
4 files changed, 501 insertions(+)
create mode 100644 samples/bpf/hid_usi.h
create mode 100644 samples/bpf/hid_usi_kern.c
create mode 100644 samples/bpf/hid_usi_user.c
diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
index 11f6f0cfab9e..6f45f6048979 100644
--- a/samples/bpf/Makefile
+++ b/samples/bpf/Makefile
@@ -62,6 +62,7 @@ tprogs-y += xdp_monitor
tprogs-y += hid_mouse
tprogs-y += hid_surface_dial
+tprogs-y += hid_usi
# Libbpf dependencies
LIBBPF_SRC = $(TOOLS_PATH)/lib/bpf
@@ -129,6 +130,7 @@ xdp_redirect-objs := xdp_redirect_user.o $(XDP_SAMPLE)
xdp_monitor-objs := xdp_monitor_user.o $(XDP_SAMPLE)
hid_mouse-objs := hid_mouse_user.o
hid_surface_dial-objs := hid_surface_dial_user.o
+hid_usi-objs := hid_usi_user.o
# Tell kbuild to always build the programs
always-y := $(tprogs-y)
@@ -188,6 +190,7 @@ always-y += hbm_edt_kern.o
always-y += xdpsock_kern.o
always-y += hid_mouse_kern.o
always-y += hid_surface_dial_kern.o
+always-y += hid_usi_kern.o
ifeq ($(ARCH), arm)
# Strip all except -D__LINUX_ARM_ARCH__ option needed to handle linux
diff --git a/samples/bpf/hid_usi.h b/samples/bpf/hid_usi.h
new file mode 100644
index 000000000000..90803375d9c1
--- /dev/null
+++ b/samples/bpf/hid_usi.h
@@ -0,0 +1,21 @@
+/* SPDX-License-Identifier: GPL-2.0
+ *
+ * Copyright(c) 2021 Intel Corporation.
+ */
+
+#ifndef HID_USI_H_
+#define HID_USI_H_
+
+#include <linux/bits.h>
+
+enum {
+ USI_PEN_ID = 0,
+ USI_PEN_IN_RANGE,
+ USI_PEN_TOUCHING,
+ USI_PEN_COLOR,
+ USI_PEN_LINE_WIDTH,
+ USI_PEN_LINE_STYLE,
+ USI_NUM_PARAMS
+};
+
+#endif /* HID_USI_H */
diff --git a/samples/bpf/hid_usi_kern.c b/samples/bpf/hid_usi_kern.c
new file mode 100644
index 000000000000..1ba32eef99c3
--- /dev/null
+++ b/samples/bpf/hid_usi_kern.c
@@ -0,0 +1,230 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright(c) 2021 Intel Corporation. */
+#include <linux/version.h>
+#include <uapi/linux/bpf.h>
+#include <uapi/linux/hid.h>
+#include <uapi/linux/bpf_hid.h>
+#include <bpf/bpf_helpers.h>
+
+#include "hid_usi.h"
+
+static const char param_names[USI_NUM_PARAMS][6] = {
+ "id",
+ "flags",
+ "color",
+ "width",
+ "style",
+};
+
+struct {
+ __uint(type, BPF_MAP_TYPE_RINGBUF);
+ __uint(max_entries, 4096);
+} ringbuf SEC(".maps");
+
+struct {
+ __uint(type, BPF_MAP_TYPE_ARRAY);
+ __type(key, int);
+ __type(value, int);
+ __uint(max_entries, USI_NUM_PARAMS);
+} cache SEC(".maps");
+
+struct {
+ __uint(type, BPF_MAP_TYPE_ARRAY);
+ __type(key, int);
+ __type(value, int);
+ __uint(max_entries, USI_NUM_PARAMS);
+} wr_cache SEC(".maps");
+
+struct rep_data {
+ int offset;
+ int size;
+ int idx;
+};
+
+static struct rep_data inputs[USI_NUM_PARAMS];
+static struct rep_data features[USI_NUM_PARAMS];
+
+SEC("hid/raw_event")
+int hid_raw_event(struct hid_bpf_ctx *ctx)
+{
+ u32 i;
+ u32 tmp;
+ int val;
+ int *wrc, *c;
+ u8 *buf;
+ u32 flags = 0;
+ int offset;
+ int size;
+ int in_range;
+ int touching;
+ bool cache_valid;
+
+ if (ctx->event.data[0] != inputs[USI_PEN_IN_RANGE].idx)
+ return 0;
+
+ in_range = bpf_hid_get_data(ctx, inputs[USI_PEN_IN_RANGE].offset,
+ inputs[USI_PEN_IN_RANGE].size);
+ touching = bpf_hid_get_data(ctx, inputs[USI_PEN_TOUCHING].offset,
+ inputs[USI_PEN_TOUCHING].size);
+
+ bpf_printk("flags: in_range=%d, touching=%d", in_range, touching);
+
+ if (!touching && cache_valid) {
+ for (i = USI_PEN_COLOR; i < USI_NUM_PARAMS; i++) {
+ val = 0;
+
+ /*
+ * Make a local copy of i so that we can get a
+ * pointer reference to it. i itself must remain
+ * inside a register so that BPF verifier can
+ * calculate its value properly.
+ */
+ tmp = i;
+ bpf_map_update_elem(&wr_cache, &tmp, &val, 0);
+ bpf_map_update_elem(&cache, &tmp, &val, 0);
+ }
+ cache_valid = false;
+ }
+
+ if (!touching)
+ return 0;
+
+ for (i = USI_PEN_COLOR; i < USI_NUM_PARAMS; i++) {
+ offset = inputs[i].offset;
+ size = inputs[i].size;
+ val = bpf_hid_get_data(ctx, offset, size);
+
+ /*
+ * Again, make a local copy of i which we can refer via a
+ * pointer to satisfy BPF verifier.
+ */
+ tmp = i;
+
+ wrc = bpf_map_lookup_elem(&wr_cache, &tmp);
+ c = bpf_map_lookup_elem(&cache, &tmp);
+ if (!wrc || !c)
+ continue;
+
+ bpf_printk("raw[%d]: %s = %02x", i, param_names[i], val);
+ bpf_printk(" (c=%02x, wr=%02x)", *c, *wrc);
+
+ if (*wrc != *c) {
+ val = *wrc;
+
+ buf = bpf_ringbuf_reserve(&ringbuf, 16, 0);
+ if (!buf)
+ continue;
+
+ buf[0] = features[i].idx;
+ buf[1] = 1;
+ buf[2] = val;
+
+ bpf_hid_raw_request(ctx, buf, 3,
+ HID_FEATURE_REPORT,
+ HID_REQ_SET_REPORT);
+
+ bpf_ringbuf_discard(buf, 0);
+ }
+
+ if (val != *c && !cache_valid) {
+ bpf_map_update_elem(&cache, &tmp, &val, 0);
+ bpf_map_update_elem(&wr_cache, &tmp, &val, 0);
+ cache_valid = true;
+ } else {
+ bpf_hid_set_data(ctx, offset, size, *c);
+ }
+ }
+
+ return 0;
+}
+
+SEC("hid/event")
+int set_haptic(struct hid_bpf_ctx *ctx)
+{
+ return 0;
+}
+
+static void process_tag(struct hid_bpf_ctx *ctx, struct rep_data *data,
+ struct hid_bpf_parser *parser, u64 *idx)
+{
+ u32 i;
+ int id;
+ u32 offset;
+
+ for (i = 0; i < parser->local.usage_index && i < 16; i++) {
+ offset = parser->report.current_offset +
+ i * parser->global.report_size;
+
+ switch (parser->local.usage[i]) {
+ case HID_DG_PEN_COLOR:
+ id = USI_PEN_COLOR;
+ break;
+ case HID_DG_PEN_LINE_WIDTH:
+ id = USI_PEN_LINE_WIDTH;
+ break;
+ case HID_DG_PEN_LINE_STYLE_INK:
+ id = USI_PEN_LINE_STYLE;
+ /*
+ * Force flags for line style. This makes it act
+ * as a simple variable from HID core point of view.
+ */
+ bpf_hid_set_data(ctx, (*idx + 1) << 3, 8, 0x2);
+ break;
+ case HID_DG_INRANGE:
+ if (parser->local.usage_index == 1)
+ continue;
+
+ id = USI_PEN_IN_RANGE;
+ break;
+ case HID_DG_TIPSWITCH:
+ if (parser->local.usage_index == 1)
+ continue;
+
+ id = USI_PEN_TOUCHING;
+ break;
+ default:
+ continue;
+ }
+
+ data[id].offset = offset + 8;
+ data[id].size = parser->global.report_size;
+ data[id].idx = parser->report.id;
+
+ bpf_printk("parsed id=%d, offset=%d, idx=%d",
+ id, data[id].offset, data[id].idx);
+ }
+}
+
+static u64 process_hid_rdesc_item(struct hid_bpf_ctx *ctx,
+ struct hid_bpf_parser *parser, u64 *idx,
+ void *data)
+{
+ struct hid_bpf_item *item = &parser->item;
+
+ switch (item->type) {
+ case HID_ITEM_TYPE_MAIN:
+ if (item->tag == HID_MAIN_ITEM_TAG_INPUT)
+ process_tag(ctx, inputs, parser, idx);
+ if (item->tag == HID_MAIN_ITEM_TAG_FEATURE)
+ process_tag(ctx, features, parser, idx);
+ }
+
+ return 0;
+}
+
+SEC("hid/rdesc_fixup")
+int hid_rdesc_fixup(struct hid_bpf_ctx *ctx)
+{
+ int ret;
+
+ if (ctx->type != HID_BPF_RDESC_FIXUP)
+ return 0;
+
+ ret = bpf_hid_foreach_rdesc_item(ctx, process_hid_rdesc_item, (void *)0, 0);
+ bpf_printk("ret: %d", ret);
+
+ return 0;
+}
+
+char _license[] SEC("license") = "GPL";
+u32 _version SEC("version") = LINUX_VERSION_CODE;
diff --git a/samples/bpf/hid_usi_user.c b/samples/bpf/hid_usi_user.c
new file mode 100644
index 000000000000..b05a3f768835
--- /dev/null
+++ b/samples/bpf/hid_usi_user.c
@@ -0,0 +1,247 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2021, Intel Corporation
+ */
+#include <linux/bpf.h>
+#include <linux/if_link.h>
+#include <assert.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <signal.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <libgen.h>
+#include <sys/resource.h>
+#include <getopt.h>
+
+#include "bpf_util.h"
+#include <bpf/bpf.h>
+#include <bpf/libbpf.h>
+
+#include <sys/stat.h>
+
+#include "hid_usi.h"
+
+static char *sysfs_path;
+static char *fifoname = "/tmp/usi";
+static int sysfs_fd;
+static int fifo_fd;
+static int prog_count;
+static int cache, wr_cache;
+
+static const struct option long_options[] = {
+ { "help", no_argument, NULL, 'h' },
+ { "fifo", required_argument, NULL, 'f' },
+};
+
+struct prog {
+ int fd;
+ enum bpf_attach_type type;
+};
+
+static struct prog progs[10];
+
+static void int_exit(int sig)
+{
+ int ret;
+
+ for (prog_count--; prog_count >= 0 ; prog_count--) {
+ ret = bpf_prog_detach2(progs[prog_count].fd, sysfs_fd, progs[prog_count].type);
+ if (ret)
+ fprintf(stderr, "bpf_prog_detach2: returned %m\n");
+ }
+
+ close(sysfs_fd);
+ close(fifo_fd);
+ remove(fifoname);
+ exit(0);
+}
+
+static void usage(const char *prog)
+{
+ fprintf(stderr,
+ "usage: %s [-f <fifoname>] /dev/HIDRAW\n\n",
+ prog);
+}
+
+static int param_to_idx(const char *param)
+{
+ if (!strcmp(param, "color"))
+ return USI_PEN_COLOR;
+ if (!strcmp(param, "width"))
+ return USI_PEN_LINE_WIDTH;
+ if (!strcmp(param, "style"))
+ return USI_PEN_LINE_STYLE;
+
+ return -EINVAL;
+}
+
+static int write_value(const char *param, int value)
+{
+ int err;
+ int idx = param_to_idx(param);
+
+ printf("%s: param=%s (%d), value=%d\n", __func__, param, idx, value);
+ err = bpf_map_update_elem(wr_cache, &idx, &value, BPF_ANY);
+ if (err)
+ printf("Update failed for %d, err=%d\n", idx, err);
+
+ return 0;
+}
+
+static int read_value(const char *param)
+{
+ int value;
+ int idx = param_to_idx(param);
+
+ printf("%s: param=%s (%d)\n", __func__, param, idx);
+
+ if (bpf_map_lookup_elem(cache, &idx, &value))
+ printf("Value missing for %d\n", idx);
+ else
+ printf("Value for %d = %d\n", idx, value);
+
+ return 0;
+}
+
+static int attach_progs(int argc, char **argv)
+{
+ char filename[256];
+ struct bpf_prog_info info = {};
+ __u32 info_len = sizeof(info);
+ struct bpf_object *obj;
+ struct bpf_program *prog;
+ int err = 0;
+ char buf[BUFSIZ];
+ char param[16];
+ char op[8];
+ int value;
+ int m, n;
+
+ snprintf(filename, sizeof(filename), "%s_kern.o", argv[0]);
+ obj = bpf_object__open_file(filename, NULL);
+ err = libbpf_get_error(obj);
+ if (err) {
+ fprintf(stderr, "ERROR: opening BPF object file failed\n");
+ obj = NULL;
+ err = 1;
+ goto cleanup;
+ }
+
+ /* load BPF program */
+ err = bpf_object__load(obj);
+ if (err) {
+ fprintf(stderr, "ERROR: loading BPF object file failed\n");
+ goto cleanup;
+ }
+
+ sysfs_fd = open(sysfs_path, O_RDONLY);
+
+ bpf_object__for_each_program(prog, obj) {
+ progs[prog_count].fd = bpf_program__fd(prog);
+ progs[prog_count].type = bpf_program__get_expected_attach_type(prog);
+
+ err = bpf_prog_attach(progs[prog_count].fd,
+ sysfs_fd,
+ progs[prog_count].type,
+ 0);
+ if (err) {
+ fprintf(stderr, "bpf_prog_attach: err=%m\n");
+ progs[prog_count].fd = 0;
+ goto cleanup;
+ }
+ printf("attached BPF program with FD=%d, type=%d\n",
+ progs[prog_count].fd, progs[prog_count].type);
+ prog_count++;
+ }
+
+ signal(SIGINT, int_exit);
+ signal(SIGTERM, int_exit);
+
+ err = bpf_obj_get_info_by_fd(progs[0].fd, &info, &info_len);
+ if (err) {
+ printf("can't get prog info - %s\n", strerror(errno));
+ goto cleanup;
+ }
+
+ cache = bpf_object__find_map_fd_by_name(obj, "cache");
+ if (cache < 0) {
+ printf("can't get 'cache' shared mem from object - %m\n");
+ goto cleanup;
+ }
+
+ wr_cache = bpf_object__find_map_fd_by_name(obj, "wr_cache");
+ if (wr_cache < 0) {
+ printf("can't get 'wr_cache' shared mem from object - %m\n");
+ goto cleanup;
+ }
+
+ mkfifo(fifoname, 0666);
+
+ fifo_fd = open(fifoname, O_RDWR);
+ if (fifo_fd < 0) {
+ perror("Fifo open error.\n");
+ err = fifo_fd;
+ goto cleanup;
+ }
+
+ while (1) {
+ n = read(fifo_fd, buf, BUFSIZ);
+ if (n < 0)
+ break;
+ buf[n] = 0;
+
+ printf("%s: received '%s'\n", __func__, buf);
+
+ m = sscanf(buf, "%16s %8s %d", param, op, &value);
+ if (m == 2 && strcmp(op, "get") == 0)
+ read_value(param);
+ else if (m == 3 && strcmp(op, "set") == 0)
+ write_value(param, value);
+ }
+
+ return 0;
+
+ cleanup:
+ for (prog_count--; prog_count >= 0; prog_count--) {
+ if (bpf_prog_detach2(progs[prog_count].fd, sysfs_fd, progs[prog_count].type))
+ fprintf(stderr, "bpf_prog_detach2: returned %m\n");
+ }
+
+ bpf_object__close(obj);
+ return err;
+}
+
+int main(int argc, char **argv)
+{
+ int opt;
+
+ while ((opt = getopt_long(argc, argv, "f:", long_options,
+ NULL)) != -1) {
+ switch (opt) {
+ case 'f':
+ fifoname = optarg;
+ break;
+ default:
+ usage(basename(argv[0]));
+ return 1;
+ }
+ }
+
+ if (optind == argc) {
+ usage(basename(argv[0]));
+ return 1;
+ }
+
+ sysfs_path = argv[optind];
+ if (!sysfs_path) {
+ perror("hidraw");
+ return 1;
+ }
+
+ printf("fifoname: %s\n", fifoname);
+ printf("sysfs_path: %s\n", sysfs_path);
+
+ return attach_progs(argc, argv);
+}
--
2.25.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [RFCv3 7/7] samples: hid: convert USI sample to use unix socket for comms
2021-12-01 16:42 [RFCv3 0/7] USI stylus support series Tero Kristo
` (5 preceding siblings ...)
2021-12-01 16:43 ` [RFCv3 6/7] samples: hid: add new hid-usi sample Tero Kristo
@ 2021-12-01 16:43 ` Tero Kristo
2021-12-08 15:30 ` [RFCv3 0/7] USI stylus support series Benjamin Tissoires
7 siblings, 0 replies; 16+ messages in thread
From: Tero Kristo @ 2021-12-01 16:43 UTC (permalink / raw)
To: linux-input, benjamin.tissoires, jikos, mika.westerberg, tero.kristo
Cc: linux-kernel, dmitry.torokhov, peter.hutterer
Convert USI hid sample userspace interface from using unix fifo to unix
socket. This allows sending results back via the communication channel
also.
Signed-off-by: Tero Kristo <tero.kristo@linux.intel.com>
---
samples/bpf/hid_usi_user.c | 73 ++++++++++++++++++++++++++------------
1 file changed, 50 insertions(+), 23 deletions(-)
diff --git a/samples/bpf/hid_usi_user.c b/samples/bpf/hid_usi_user.c
index b05a3f768835..5bf5ebc21a30 100644
--- a/samples/bpf/hid_usi_user.c
+++ b/samples/bpf/hid_usi_user.c
@@ -14,6 +14,8 @@
#include <libgen.h>
#include <sys/resource.h>
#include <getopt.h>
+#include <sys/socket.h>
+#include <sys/un.h>
#include "bpf_util.h"
#include <bpf/bpf.h>
@@ -24,15 +26,15 @@
#include "hid_usi.h"
static char *sysfs_path;
-static char *fifoname = "/tmp/usi";
+static char *sockname = "/tmp/usi";
static int sysfs_fd;
-static int fifo_fd;
+static int sock_fd;
static int prog_count;
static int cache, wr_cache;
static const struct option long_options[] = {
{ "help", no_argument, NULL, 'h' },
- { "fifo", required_argument, NULL, 'f' },
+ { "sock", required_argument, NULL, 's' },
};
struct prog {
@@ -53,15 +55,15 @@ static void int_exit(int sig)
}
close(sysfs_fd);
- close(fifo_fd);
- remove(fifoname);
+ close(sock_fd);
+ remove(sockname);
exit(0);
}
static void usage(const char *prog)
{
fprintf(stderr,
- "usage: %s [-f <fifoname>] /dev/HIDRAW\n\n",
+ "usage: %s [-s <sockname>] /dev/HIDRAW\n\n",
prog);
}
@@ -84,15 +86,17 @@ static int write_value(const char *param, int value)
printf("%s: param=%s (%d), value=%d\n", __func__, param, idx, value);
err = bpf_map_update_elem(wr_cache, &idx, &value, BPF_ANY);
- if (err)
+ if (err) {
printf("Update failed for %d, err=%d\n", idx, err);
+ return err;
+ }
return 0;
}
static int read_value(const char *param)
{
- int value;
+ int value = -ENOENT;
int idx = param_to_idx(param);
printf("%s: param=%s (%d)\n", __func__, param, idx);
@@ -102,7 +106,7 @@ static int read_value(const char *param)
else
printf("Value for %d = %d\n", idx, value);
- return 0;
+ return value;
}
static int attach_progs(int argc, char **argv)
@@ -118,6 +122,9 @@ static int attach_progs(int argc, char **argv)
char op[8];
int value;
int m, n;
+ struct sockaddr_un addr;
+ struct sockaddr_un from;
+ socklen_t from_len = sizeof(from);
snprintf(filename, sizeof(filename), "%s_kern.o", argv[0]);
obj = bpf_object__open_file(filename, NULL);
@@ -177,28 +184,48 @@ static int attach_progs(int argc, char **argv)
goto cleanup;
}
- mkfifo(fifoname, 0666);
+ sock_fd = socket(PF_UNIX, SOCK_DGRAM, 0);
+ if (sock_fd < 0) {
+ perror("socket open error.\n");
+ err = sock_fd;
+ goto cleanup;
+ }
+
+ addr.sun_family = AF_UNIX;
+ strcpy(addr.sun_path, sockname);
+ unlink(sockname);
- fifo_fd = open(fifoname, O_RDWR);
- if (fifo_fd < 0) {
- perror("Fifo open error.\n");
- err = fifo_fd;
+ if (bind(sock_fd, (struct sockaddr *)&addr, sizeof(addr)) < 0) {
+ perror("bind");
goto cleanup;
}
while (1) {
- n = read(fifo_fd, buf, BUFSIZ);
+ n = recvfrom(sock_fd, buf, BUFSIZ, 0, (struct sockaddr *)&from,
+ &from_len);
if (n < 0)
break;
buf[n] = 0;
printf("%s: received '%s'\n", __func__, buf);
+ printf("%s: from_len=%d, from=%s\n", __func__, from_len,
+ from.sun_path);
+
m = sscanf(buf, "%16s %8s %d", param, op, &value);
- if (m == 2 && strcmp(op, "get") == 0)
- read_value(param);
- else if (m == 3 && strcmp(op, "set") == 0)
- write_value(param, value);
+ if (m == 2 && strcmp(op, "get") == 0) {
+ value = read_value(param);
+ sprintf(buf, "%s: %d\n", param, value);
+ printf("%s: sending '%s'\n", __func__, buf);
+ sendto(sock_fd, buf, strlen(buf) + 1, 0,
+ (struct sockaddr *)&from, from_len);
+ } else if (m == 3 && strcmp(op, "set") == 0) {
+ err = write_value(param, value);
+ sprintf(buf, "%s: %d, err=%d\n", param, value, err);
+ printf("%s: sending '%s'\n", __func__, buf);
+ sendto(sock_fd, buf, strlen(buf) + 1, 0,
+ (struct sockaddr *)&from, from_len);
+ }
}
return 0;
@@ -217,11 +244,11 @@ int main(int argc, char **argv)
{
int opt;
- while ((opt = getopt_long(argc, argv, "f:", long_options,
+ while ((opt = getopt_long(argc, argv, "s:", long_options,
NULL)) != -1) {
switch (opt) {
- case 'f':
- fifoname = optarg;
+ case 's':
+ sockname = optarg;
break;
default:
usage(basename(argv[0]));
@@ -240,7 +267,7 @@ int main(int argc, char **argv)
return 1;
}
- printf("fifoname: %s\n", fifoname);
+ printf("sockname: %s\n", sockname);
printf("sysfs_path: %s\n", sysfs_path);
return attach_progs(argc, argv);
--
2.25.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [RFCv3 0/7] USI stylus support series
2021-12-01 16:42 [RFCv3 0/7] USI stylus support series Tero Kristo
` (6 preceding siblings ...)
2021-12-01 16:43 ` [RFCv3 7/7] samples: hid: convert USI sample to use unix socket for comms Tero Kristo
@ 2021-12-08 15:30 ` Benjamin Tissoires
2021-12-09 7:51 ` Tero Kristo
7 siblings, 1 reply; 16+ messages in thread
From: Benjamin Tissoires @ 2021-12-08 15:30 UTC (permalink / raw)
To: Tero Kristo
Cc: open list:HID CORE LAYER, Jiri Kosina, Mika Westerberg, lkml,
Dmitry Torokhov, Peter Hutterer
Hi Tero,
On Wed, Dec 1, 2021 at 5:43 PM Tero Kristo <tero.kristo@linux.intel.com> wrote:
>
> Hi,
>
> Another update here based on received feedback. Main change compared to v2:
If that's OK with you, I'd like to cherry-pick some patches from the
series: 1/7, 2/7 (the version from v4 because I requested changes),
4/7 and a lighter 5/7 where we don't have the MSC events we are still
discussing.
So Ideally, can you post a v4 based on top of hid.git/for-next
(without my hid-bpf changes) with those 4 patches?
Patch 3 is still up for discussion, and patches 6 and 7 are obviously RFC.
Actually, Patch 3 could be split into a part where you add the HID
usages and a part with the mapping of them. The HID usages part can be
integrated now too, and we'll have the USI mapping that would require
Dmitry's ack in a separate patch.
But if you prefer having everything in one series, that's fine by me too.
Cheers,
Benjamin
>
> - Dropped patch #5 : HID: core: map USI pen style reports directly
> This is not needed anymore, as the same result can be reached by
> modifying the flags of the relevant field in rdesc. This is done now
> as part of patch #7 in the BPF code.
>
> I also dropped one of the fixes from my test branch [1] as pointed out
> by Benjamin, it is not needed if the BPF program is executed with the
> modalias link.
>
> Updated test branch can be found from [2].
>
> -Tero
>
> [1]
> https://github.com/t-kristo/linux/commit/81b27fd46780ce67c2706d586c0f4a437e87dbf6
> (HID: bpf: fix file mapping)
> [2] https://github.com/t-kristo/linux/commits/usi-5.16-rfc-v3-bpf
>
>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [RFCv3 0/7] USI stylus support series
2021-12-08 15:30 ` [RFCv3 0/7] USI stylus support series Benjamin Tissoires
@ 2021-12-09 7:51 ` Tero Kristo
0 siblings, 0 replies; 16+ messages in thread
From: Tero Kristo @ 2021-12-09 7:51 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: open list:HID CORE LAYER, Jiri Kosina, Mika Westerberg, lkml,
Dmitry Torokhov, Peter Hutterer
Hi Benjamin,
On 08/12/2021 17:30, Benjamin Tissoires wrote:
> Hi Tero,
>
> On Wed, Dec 1, 2021 at 5:43 PM Tero Kristo <tero.kristo@linux.intel.com> wrote:
>> Hi,
>>
>> Another update here based on received feedback. Main change compared to v2:
> If that's OK with you, I'd like to cherry-pick some patches from the
> series: 1/7, 2/7 (the version from v4 because I requested changes),
> 4/7 and a lighter 5/7 where we don't have the MSC events we are still
> discussing.
>
> So Ideally, can you post a v4 based on top of hid.git/for-next
> (without my hid-bpf changes) with those 4 patches?
Yes, I can post these.
>
> Patch 3 is still up for discussion, and patches 6 and 7 are obviously RFC.
>
> Actually, Patch 3 could be split into a part where you add the HID
> usages and a part with the mapping of them. The HID usages part can be
> integrated now too, and we'll have the USI mapping that would require
> Dmitry's ack in a separate patch.
Ok, I'll take a look at this part also.
>
> But if you prefer having everything in one series, that's fine by me too.
No, I am fine with splitting things up and start getting things integrated.
-Tero
>
> Cheers,
> Benjamin
>
>> - Dropped patch #5 : HID: core: map USI pen style reports directly
>> This is not needed anymore, as the same result can be reached by
>> modifying the flags of the relevant field in rdesc. This is done now
>> as part of patch #7 in the BPF code.
>>
>> I also dropped one of the fixes from my test branch [1] as pointed out
>> by Benjamin, it is not needed if the BPF program is executed with the
>> modalias link.
>>
>> Updated test branch can be found from [2].
>>
>> -Tero
>>
>> [1]
>> https://github.com/t-kristo/linux/commit/81b27fd46780ce67c2706d586c0f4a437e87dbf6
>> (HID: bpf: fix file mapping)
>> [2] https://github.com/t-kristo/linux/commits/usi-5.16-rfc-v3-bpf
>>
>>
^ permalink raw reply [flat|nested] 16+ messages in thread