Re: [PATCH] HID: plantronics: Workaround for double volume key presses

2021-03-08 Thread Jiri Kosina
On Sun, 7 Feb 2021, Maxim Mikityanskiy wrote:

> Plantronics Blackwire 3220 Series (047f:c056) sends HID reports twice
> for each volume key press. This patch adds a quirk to hid-plantronics
> for this product ID, which will ignore the second volume key press if
> it happens within 5 ms from the last one that was handled.
> 
> The patch was tested on the mentioned model only, it shouldn't affect
> other models, however, this quirk might be needed for them too.
> Auto-repeat (when a key is held pressed) is not affected, because the
> rate is about 3 times per second, which is far less frequent than once
> in 5 ms.
> 
> Fixes: 81bb773faed7 ("HID: plantronics: Update to map volume up/down 
> controls")
> Signed-off-by: Maxim Mikityanskiy 
> ---
> People from Plantronics, maybe you could advise on a better fix than
> filtering duplicate events on driver level? Do you happen to know why
> they occur in the first place? Are any other headsets affected?

Makes one wonder how the Windows driver is dealing with this indeed.
Anyway, as there doesn't seem to be better cure available for now, I have 
applied the patch. Thanks,

-- 
Jiri Kosina
SUSE Labs



[PATCH] HID: plantronics: Workaround for double volume key presses

2021-02-07 Thread Maxim Mikityanskiy
Plantronics Blackwire 3220 Series (047f:c056) sends HID reports twice
for each volume key press. This patch adds a quirk to hid-plantronics
for this product ID, which will ignore the second volume key press if
it happens within 5 ms from the last one that was handled.

The patch was tested on the mentioned model only, it shouldn't affect
other models, however, this quirk might be needed for them too.
Auto-repeat (when a key is held pressed) is not affected, because the
rate is about 3 times per second, which is far less frequent than once
in 5 ms.

Fixes: 81bb773faed7 ("HID: plantronics: Update to map volume up/down controls")
Signed-off-by: Maxim Mikityanskiy 
---
People from Plantronics, maybe you could advise on a better fix than
filtering duplicate events on driver level? Do you happen to know why
they occur in the first place? Are any other headsets affected?

 drivers/hid/hid-ids.h |  1 +
 drivers/hid/hid-plantronics.c | 60 +--
 include/linux/hid.h   |  2 ++
 3 files changed, 61 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 5ba0aa1d2335..5ac52b4d900d 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -937,6 +937,7 @@
 #define USB_DEVICE_ID_ORTEK_IHOME_IMAC_A210S   0x8003
 
 #define USB_VENDOR_ID_PLANTRONICS  0x047f
+#define USB_DEVICE_ID_PLANTRONICS_BLACKWIRE_3220_SERIES0xc056
 
 #define USB_VENDOR_ID_PANASONIC0x04da
 #define USB_DEVICE_ID_PANABOARD_UBT780 0x1044
diff --git a/drivers/hid/hid-plantronics.c b/drivers/hid/hid-plantronics.c
index 85b685efc12f..e81b7cec2d12 100644
--- a/drivers/hid/hid-plantronics.c
+++ b/drivers/hid/hid-plantronics.c
@@ -13,6 +13,7 @@
 
 #include 
 #include 
+#include 
 
 #define PLT_HID_1_0_PAGE   0xffa0
 #define PLT_HID_2_0_PAGE   0xffa2
@@ -36,6 +37,16 @@
 #define PLT_ALLOW_CONSUMER (field->application == HID_CP_CONSUMERCONTROL && \
(usage->hid & HID_USAGE_PAGE) == HID_UP_CONSUMER)
 
+#define PLT_QUIRK_DOUBLE_VOLUME_KEYS BIT(0)
+
+#define PLT_DOUBLE_KEY_TIMEOUT 5 /* ms */
+
+struct plt_drv_data {
+   unsigned long device_type;
+   unsigned long last_volume_key_ts;
+   u32 quirks;
+};
+
 static int plantronics_input_mapping(struct hid_device *hdev,
 struct hid_input *hi,
 struct hid_field *field,
@@ -43,7 +54,8 @@ static int plantronics_input_mapping(struct hid_device *hdev,
 unsigned long **bit, int *max)
 {
unsigned short mapped_key;
-   unsigned long plt_type = (unsigned long)hid_get_drvdata(hdev);
+   struct plt_drv_data *drv_data = hid_get_drvdata(hdev);
+   unsigned long plt_type = drv_data->device_type;
 
/* special case for PTT products */
if (field->application == HID_GD_JOYSTICK)
@@ -105,6 +117,30 @@ static int plantronics_input_mapping(struct hid_device 
*hdev,
return 1;
 }
 
+static int plantronics_event(struct hid_device *hdev, struct hid_field *field,
+struct hid_usage *usage, __s32 value)
+{
+   struct plt_drv_data *drv_data = hid_get_drvdata(hdev);
+
+   if (drv_data->quirks & PLT_QUIRK_DOUBLE_VOLUME_KEYS) {
+   unsigned long prev_ts, cur_ts;
+
+   /* Usages are filtered in plantronics_usages. */
+
+   if (!value) /* Handle key presses only. */
+   return 0;
+
+   prev_ts = drv_data->last_volume_key_ts;
+   cur_ts = jiffies;
+   if (jiffies_to_msecs(cur_ts - prev_ts) <= 
PLT_DOUBLE_KEY_TIMEOUT)
+   return 1; /* Ignore the repeated key. */
+
+   drv_data->last_volume_key_ts = cur_ts;
+   }
+
+   return 0;
+}
+
 static unsigned long plantronics_device_type(struct hid_device *hdev)
 {
unsigned i, col_page;
@@ -133,15 +169,24 @@ static unsigned long plantronics_device_type(struct 
hid_device *hdev)
 static int plantronics_probe(struct hid_device *hdev,
 const struct hid_device_id *id)
 {
+   struct plt_drv_data *drv_data;
int ret;
 
+   drv_data = devm_kzalloc(&hdev->dev, sizeof(*drv_data), GFP_KERNEL);
+   if (!drv_data)
+   return -ENOMEM;
+
ret = hid_parse(hdev);
if (ret) {
hid_err(hdev, "parse failed\n");
goto err;
}
 
-   hid_set_drvdata(hdev, (void *)plantronics_device_type(hdev));
+   drv_data->device_type = plantronics_device_type(hdev);
+   drv_data->quirks = id->driver_data;
+   drv_data->last_volume_key_ts = jiffies - 
msecs_to_jiffies(PLT_DOUBLE_KEY_TIMEOUT);
+
+   hid_set_drvdata(hdev, drv_data);
 
ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT |
HID_CONNECT_HIDINPUT_FORCE | HID_CONNECT_HIDDEV_FORCE);
@@ -153,15 +198,26 @@ static int plantronics_probe(struct hid_device *hdev