RE: [PATCH] HID: plantronics: Additional PID for double volume key presses quirk

Wang, Wade
2024-09-16T09:06:05Z

Hi Benjamin,

I have sent 3rd patch per your comments. And this patch does not depend on other patch, and can be applied directly by "git am -3"\

Thanks
Wade

-----Original Message-----
From: Benjamin Tissoires <bentiss@kernel.org>
Sent: Friday, September 13, 2024 10:01 PM
To: Wang, Wade <wade.wang@hp.com>
Cc: jikos@kernel.org; linux-input@vger.kernel.org; linux-kernel@vger.kernel.org; stable@vger.kernel.org
Subject: Re: [PATCH] HID: plantronics: Additional PID for double volume key presses quirk

CAUTION: External Email

On Sep 13 2024, Wade Wang wrote:
> Add the below headsets for double volume key presses quirk
> Plantronics EncorePro 500 Series (047f:431e)
> Plantronics Blackwire_3325 Series (047f:430c)
>
> Quote from previous patch by Maxim Mikityanskiy and Terry Junge
> 'commit f567d6ef8606 ("HID: plantronics: Workaround for double volume
> key presses")'
> 'commit 3d57f36c89d8 ("HID: plantronics: Additional PIDs for double
> volume key presses quirk")'
> These Plantronics Series headset sends an opposite volume key following
> each volume key press. This patch adds a quirk to hid-plantronics for this
> product ID, which will ignore the second opposite volume key press if it
> happens within 250 ms from the last one that was handled.

This commit message is very confusing:
- you mention that you are quoting 2 commits,
- but then you don't quote them but slightly reword the content
- and then, and most of all, you insert in the driver a new timeout of
250 ms, which seems to not be with the same bases than f567d6ef8606
where it is mentioned that "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." -> 250 ms gets in the roughly
same range than 3 times per seconds, so some more explanations is
required

Please also follow Greg's advice and, as you replied in your last message
that didn't made the list (HTML), please resubmit the series with a
clear v3 indicator and a description of changes.

Ideally, I'd like to also have the other plantronics patch you sent
today in the same series, so I know which order I should apply them, in
case one rely on the other.

Cheers,
Benjamin

>
> Cc: stable@vger.kernel.org
> Signed-off-by: Wade Wang <wade.wang@hp.com>
> ---
> drivers/hid/hid-ids.h | 2 ++
> drivers/hid/hid-plantronics.c | 11 +++++++++++
> 2 files changed, 13 insertions(+)
>
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index 781c5aa29859..a0aaac98a891 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -1050,6 +1050,8 @@
> #define USB_DEVICE_ID_PLANTRONICS_BLACKWIRE_3220_SERIES 0xc056
> #define USB_DEVICE_ID_PLANTRONICS_BLACKWIRE_3215_SERIES 0xc057
> #define USB_DEVICE_ID_PLANTRONICS_BLACKWIRE_3225_SERIES 0xc058
> +#define USB_DEVICE_ID_PLANTRONICS_BLACKWIRE_3325_SERIES 0x430c
> +#define USB_DEVICE_ID_PLANTRONICS_ENCOREPRO_500_SERIES 0x431e
>
> #define USB_VENDOR_ID_PANASONIC 0x04da
> #define USB_DEVICE_ID_PANABOARD_UBT780 0x1044
> diff --git a/drivers/hid/hid-plantronics.c b/drivers/hid/hid-plantronics.c
> index 3d414ae194ac..2a19f3646ecb 100644
> --- a/drivers/hid/hid-plantronics.c
> +++ b/drivers/hid/hid-plantronics.c
> @@ -38,8 +38,10 @@
> (usage->hid & HID_USAGE_PAGE) == HID_UP_CONSUMER)