diff mbox series

HID: plantronics: Update to map micmute controls

Message ID 20240913060800.1325954-1-wade.wang@hp.com
State Superseded
Headers show
Series HID: plantronics: Update to map micmute controls | expand

Commit Message

Wang, Wade Sept. 13, 2024, 6:08 a.m. UTC
telephony page of Plantronics headset is ignored currently, it caused
micmute button no function, Now follow native HID key mapping for
telephony page map, telephony micmute key is enabled by default

Cc: stable@vger.kernel.org
Signed-off-by: Wade Wang <wade.wang@hp.com>
---
 drivers/hid/hid-plantronics.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Wang, Wade Sept. 16, 2024, 8:13 a.m. UTC | #1
Hi Benjamin,

This patch is for all Poly HS devices, and it does not depends on other patches, it can apply directly by " git am -3".

With this patch, MicMute button key event will be send to user space, I have tested on the below Poly devices:
        Plantronics EncorePro 500 Series
        Plantronics Blackwire_3325 Series
        Poly Voyager 4320 HS + BT700 Dongle

Regards
Wade

-----Original Message-----
From: Benjamin Tissoires <bentiss@kernel.org> 
Sent: Friday, September 13, 2024 10:04 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: Update to map micmute controls

CAUTION: External Email

On Sep 13 2024, Wade Wang wrote:
> telephony page of Plantronics headset is ignored currently, it caused
> micmute button no function, Now follow native HID key mapping for
> telephony page map, telephony micmute key is enabled by default

For which devices this patch is required? Is it related to the other
patch you sent today? If so please make a mention of the concerned
devices and make sure both patches are sent in a single v3 series.

Also, have you tested this change with other Plantronics headsets? Where
there any changes in behavior from them?

Cheers,
Benjamin

>
> Cc: stable@vger.kernel.org
> Signed-off-by: Wade Wang <wade.wang@hp.com>
> ---
>  drivers/hid/hid-plantronics.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hid/hid-plantronics.c b/drivers/hid/hid-plantronics.c
> index 2a19f3646ecb..2d17534fce61 100644
> --- a/drivers/hid/hid-plantronics.c
> +++ b/drivers/hid/hid-plantronics.c
> @@ -77,10 +77,10 @@ static int plantronics_input_mapping(struct hid_device *hdev,
>               }
>       }
>       /* handle standard types - plt_type is 0xffa0uuuu or 0xffa2uuuu */
> -     /* 'basic telephony compliant' - allow default consumer page map */
> +     /* 'basic telephony compliant' - allow default consumer & telephony page map */
>       else if ((plt_type & HID_USAGE) >= PLT_BASIC_TELEPHONY &&
>                (plt_type & HID_USAGE) != PLT_BASIC_EXCEPTION) {
> -             if (PLT_ALLOW_CONSUMER)
> +             if (PLT_ALLOW_CONSUMER || (usage->hid & HID_USAGE_PAGE) == HID_UP_TELEPHONY)
>                       goto defaulted;
>       }
>       /* not 'basic telephony' - apply legacy mapping */
> --
> 2.34.1
>
Wang, Wade Sept. 24, 2024, 9 a.m. UTC | #2
Hi Benjamin and Greg,

May I know the review progress and anything I need to change? Thanks

Regards
Wade

-----Original Message-----
From: Wang, Wade 
Sent: Monday, September 16, 2024 4:13 PM
To: Benjamin Tissoires <bentiss@kernel.org>
Cc: jikos@kernel.org; linux-input@vger.kernel.org; linux-kernel@vger.kernel.org; stable@vger.kernel.org
Subject: RE: [PATCH] HID: plantronics: Update to map micmute controls

Hi Benjamin,

This patch is for all Poly HS devices, and it does not depends on other patches, it can apply directly by " git am -3".

With this patch, MicMute button key event will be send to user space, I have tested on the below Poly devices:
        Plantronics EncorePro 500 Series
        Plantronics Blackwire_3325 Series
        Poly Voyager 4320 HS + BT700 Dongle

Regards
Wade

-----Original Message-----
From: Benjamin Tissoires <bentiss@kernel.org>
Sent: Friday, September 13, 2024 10:04 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: Update to map micmute controls

CAUTION: External Email

On Sep 13 2024, Wade Wang wrote:
> telephony page of Plantronics headset is ignored currently, it caused 
> micmute button no function, Now follow native HID key mapping for 
> telephony page map, telephony micmute key is enabled by default

For which devices this patch is required? Is it related to the other patch you sent today? If so please make a mention of the concerned devices and make sure both patches are sent in a single v3 series.

Also, have you tested this change with other Plantronics headsets? Where there any changes in behavior from them?

Cheers,
Benjamin

>
> Cc: stable@vger.kernel.org
> Signed-off-by: Wade Wang <wade.wang@hp.com>
> ---
>  drivers/hid/hid-plantronics.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hid/hid-plantronics.c 
> b/drivers/hid/hid-plantronics.c index 2a19f3646ecb..2d17534fce61 
> 100644
> --- a/drivers/hid/hid-plantronics.c
> +++ b/drivers/hid/hid-plantronics.c
> @@ -77,10 +77,10 @@ static int plantronics_input_mapping(struct hid_device *hdev,
>               }
>       }
>       /* handle standard types - plt_type is 0xffa0uuuu or 0xffa2uuuu */
> -     /* 'basic telephony compliant' - allow default consumer page map */
> +     /* 'basic telephony compliant' - allow default consumer & 
> + telephony page map */
>       else if ((plt_type & HID_USAGE) >= PLT_BASIC_TELEPHONY &&
>                (plt_type & HID_USAGE) != PLT_BASIC_EXCEPTION) {
> -             if (PLT_ALLOW_CONSUMER)
> +             if (PLT_ALLOW_CONSUMER || (usage->hid & HID_USAGE_PAGE) 
> + == HID_UP_TELEPHONY)
>                       goto defaulted;
>       }
>       /* not 'basic telephony' - apply legacy mapping */
> --
> 2.34.1
>
Terry Junge Sept. 25, 2024, 3:31 a.m. UTC | #3
Hi Wade,

I retired from Plantronics in 2020. The original driver did not allow 
mute button to be mapped as there were mute synchronization issues.

The headset needs to receive some type of feedback from the host when it 
sends the mute event in order to synchronize with the host, ideally the 
host setting or clearing the mute control in the audio control interface 
but setting/clearing the mute LED would also work.

At the time Ubuntu and Chromebooks did not feedback mute state and it 
was possible to mute from the headset and then unmute from the mixer or 
keyboard and the headset would stay muted. The only way to unmute was 
with the headset button. This was an unacceptable user experience so we 
blocked mapping.

If you want to try mapping mute event then you also need to allow 
mapping the mute LED for possible host feedback.

(HID_UP_TELEPHONY | 0x2f) is telephony page mute button
(HID_UP_LED | 0x09) is LED page mute LED

Then you need to test more than just the event getting to user space. 
You need to check mute synchronization with the host mixer under all 
mute/unmute use cases.

Regards,
Terry Junge


On 9/24/24 2:00 AM, Wang, Wade wrote:
> Hi Benjamin and Greg,
> 
> May I know the review progress and anything I need to change? Thanks
> 
> Regards
> Wade
> 
> -----Original Message-----
> From: Wang, Wade
> Sent: Monday, September 16, 2024 4:13 PM
> To: Benjamin Tissoires <bentiss@kernel.org>
> Cc: jikos@kernel.org; linux-input@vger.kernel.org; linux-kernel@vger.kernel.org; stable@vger.kernel.org
> Subject: RE: [PATCH] HID: plantronics: Update to map micmute controls
> 
> Hi Benjamin,
> 
> This patch is for all Poly HS devices, and it does not depends on other patches, it can apply directly by " git am -3".
> 
> With this patch, MicMute button key event will be send to user space, I have tested on the below Poly devices:
>          Plantronics EncorePro 500 Series
>          Plantronics Blackwire_3325 Series
>          Poly Voyager 4320 HS + BT700 Dongle
> 
> Regards
> Wade
> 
> -----Original Message-----
> From: Benjamin Tissoires <bentiss@kernel.org>
> Sent: Friday, September 13, 2024 10:04 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: Update to map micmute controls
> 
> CAUTION: External Email
> 
> On Sep 13 2024, Wade Wang wrote:
>> telephony page of Plantronics headset is ignored currently, it caused
>> micmute button no function, Now follow native HID key mapping for
>> telephony page map, telephony micmute key is enabled by default
> 
> For which devices this patch is required? Is it related to the other patch you sent today? If so please make a mention of the concerned devices and make sure both patches are sent in a single v3 series.
> 
> Also, have you tested this change with other Plantronics headsets? Where there any changes in behavior from them?
> 
> Cheers,
> Benjamin
> 
>>
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Wade Wang <wade.wang@hp.com>
>> ---
>>   drivers/hid/hid-plantronics.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/hid/hid-plantronics.c
>> b/drivers/hid/hid-plantronics.c index 2a19f3646ecb..2d17534fce61
>> 100644
>> --- a/drivers/hid/hid-plantronics.c
>> +++ b/drivers/hid/hid-plantronics.c
>> @@ -77,10 +77,10 @@ static int plantronics_input_mapping(struct hid_device *hdev,
>>                }
>>        }
>>        /* handle standard types - plt_type is 0xffa0uuuu or 0xffa2uuuu */
>> -     /* 'basic telephony compliant' - allow default consumer page map */
>> +     /* 'basic telephony compliant' - allow default consumer &
>> + telephony page map */
>>        else if ((plt_type & HID_USAGE) >= PLT_BASIC_TELEPHONY &&
>>                 (plt_type & HID_USAGE) != PLT_BASIC_EXCEPTION) {
>> -             if (PLT_ALLOW_CONSUMER)
>> +             if (PLT_ALLOW_CONSUMER || (usage->hid & HID_USAGE_PAGE)
>> + == HID_UP_TELEPHONY)
>>                        goto defaulted;
>>        }
>>        /* not 'basic telephony' - apply legacy mapping */
>> --
>> 2.34.1
>>
> 
>
Wang, Wade Oct. 11, 2024, 4:03 a.m. UTC | #4
Hi Terry,

Is it OK to apply? At least we will have a chance to improve user experience in userspace after apply this patch. Looking forward to your comments. Thanks

Regards
Wade

-----Original Message-----
From: Wang, Wade 
Sent: Thursday, September 26, 2024 9:58 AM
To: Terry Junge <linuxhid@cosmicgizmosystems.com>; Benjamin Tissoires <bentiss@kernel.org>
Cc: jikos@kernel.org; linux-input@vger.kernel.org; linux-kernel@vger.kernel.org; stable@vger.kernel.org
Subject: RE: [PATCH] HID: plantronics: Update to map micmute controls

Hi Terry,

1. Per our testing, Poly headset will maintain Mute status at headset side, whatever host send feedback or not.
2. Mute led is off when Poly USB headset connect to host, so host will keep same Mute status with headset because of toggle Mute key event.
3. Even Ubuntu and Chromebooks have to feedback Poly headset mute state, it should be done at user space instead of kernel. The precondition is kernel should report Mute key event first, then user space has chance to do this kind of improvement in future

So following standard HID rule is necessary.

BTW, on MSFT Windows, After receive mute key, the host switch the mute control status of the audio control interface, whatever mute status in headset FW is correct or not. I think it make sense than LED page mute LED.

Thanks,
Wade

-----Original Message-----
From: Terry Junge <linuxhid@cosmicgizmosystems.com>
Sent: Wednesday, September 25, 2024 11:32 AM
To: Wang, Wade <wade.wang@hp.com>; Benjamin Tissoires <bentiss@kernel.org>
Cc: jikos@kernel.org; linux-input@vger.kernel.org; linux-kernel@vger.kernel.org; stable@vger.kernel.org
Subject: Re: [PATCH] HID: plantronics: Update to map micmute controls

CAUTION: External Email

Hi Wade,

I retired from Plantronics in 2020. The original driver did not allow mute button to be mapped as there were mute synchronization issues.

The headset needs to receive some type of feedback from the host when it sends the mute event in order to synchronize with the host, ideally the host setting or clearing the mute control in the audio control interface but setting/clearing the mute LED would also work.

At the time Ubuntu and Chromebooks did not feedback mute state and it was possible to mute from the headset and then unmute from the mixer or keyboard and the headset would stay muted. The only way to unmute was with the headset button. This was an unacceptable user experience so we blocked mapping.

If you want to try mapping mute event then you also need to allow mapping the mute LED for possible host feedback.

(HID_UP_TELEPHONY | 0x2f) is telephony page mute button (HID_UP_LED | 0x09) is LED page mute LED

Then you need to test more than just the event getting to user space.
You need to check mute synchronization with the host mixer under all mute/unmute use cases.

Regards,
Terry Junge


On 9/24/24 2:00 AM, Wang, Wade wrote:
> Hi Benjamin and Greg,
>
> May I know the review progress and anything I need to change? Thanks
>
> Regards
> Wade
>
> -----Original Message-----
> From: Wang, Wade
> Sent: Monday, September 16, 2024 4:13 PM
> To: Benjamin Tissoires <bentiss@kernel.org>
> Cc: jikos@kernel.org; linux-input@vger.kernel.org; 
> linux-kernel@vger.kernel.org; stable@vger.kernel.org
> Subject: RE: [PATCH] HID: plantronics: Update to map micmute controls
>
> Hi Benjamin,
>
> This patch is for all Poly HS devices, and it does not depends on other patches, it can apply directly by " git am -3".
>
> With this patch, MicMute button key event will be send to user space, I have tested on the below Poly devices:
>          Plantronics EncorePro 500 Series
>          Plantronics Blackwire_3325 Series
>          Poly Voyager 4320 HS + BT700 Dongle
>
> Regards
> Wade
>
> -----Original Message-----
> From: Benjamin Tissoires <bentiss@kernel.org>
> Sent: Friday, September 13, 2024 10:04 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: Update to map micmute controls
>
> CAUTION: External Email
>
> On Sep 13 2024, Wade Wang wrote:
>> telephony page of Plantronics headset is ignored currently, it caused 
>> micmute button no function, Now follow native HID key mapping for 
>> telephony page map, telephony micmute key is enabled by default
>
> For which devices this patch is required? Is it related to the other patch you sent today? If so please make a mention of the concerned devices and make sure both patches are sent in a single v3 series.
>
> Also, have you tested this change with other Plantronics headsets? Where there any changes in behavior from them?
>
> Cheers,
> Benjamin
>
>>
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Wade Wang <wade.wang@hp.com>
>> ---
>>   drivers/hid/hid-plantronics.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/hid/hid-plantronics.c 
>> b/drivers/hid/hid-plantronics.c index 2a19f3646ecb..2d17534fce61
>> 100644
>> --- a/drivers/hid/hid-plantronics.c
>> +++ b/drivers/hid/hid-plantronics.c
>> @@ -77,10 +77,10 @@ static int plantronics_input_mapping(struct hid_device *hdev,
>>                }
>>        }
>>        /* handle standard types - plt_type is 0xffa0uuuu or 0xffa2uuuu */
>> -     /* 'basic telephony compliant' - allow default consumer page map */
>> +     /* 'basic telephony compliant' - allow default consumer & 
>> + telephony page map */
>>        else if ((plt_type & HID_USAGE) >= PLT_BASIC_TELEPHONY &&
>>                 (plt_type & HID_USAGE) != PLT_BASIC_EXCEPTION) {
>> -             if (PLT_ALLOW_CONSUMER)
>> +             if (PLT_ALLOW_CONSUMER || (usage->hid & HID_USAGE_PAGE) 
>> + == HID_UP_TELEPHONY)
>>                        goto defaulted;
>>        }
>>        /* not 'basic telephony' - apply legacy mapping */
>> --
>> 2.34.1
>>
>
>
diff mbox series

Patch

diff --git a/drivers/hid/hid-plantronics.c b/drivers/hid/hid-plantronics.c
index 2a19f3646ecb..2d17534fce61 100644
--- a/drivers/hid/hid-plantronics.c
+++ b/drivers/hid/hid-plantronics.c
@@ -77,10 +77,10 @@  static int plantronics_input_mapping(struct hid_device *hdev,
 		}
 	}
 	/* handle standard types - plt_type is 0xffa0uuuu or 0xffa2uuuu */
-	/* 'basic telephony compliant' - allow default consumer page map */
+	/* 'basic telephony compliant' - allow default consumer & telephony page map */
 	else if ((plt_type & HID_USAGE) >= PLT_BASIC_TELEPHONY &&
 		 (plt_type & HID_USAGE) != PLT_BASIC_EXCEPTION) {
-		if (PLT_ALLOW_CONSUMER)
+		if (PLT_ALLOW_CONSUMER || (usage->hid & HID_USAGE_PAGE) == HID_UP_TELEPHONY)
 			goto defaulted;
 	}
 	/* not 'basic telephony' - apply legacy mapping */