Re: [PATCH 1/3] HID: input - Add microphone mute LED support

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
To: Bernhard Seibold <mail@bernhard-seibold.de>
Cc: linux-input@vger.kernel.org, Jiri Kosina <jikos@kernel.org>,
	Benjamin Tissoires <benjamin.tissoires@redhat.com>,
	Hans de Goede <hdegoede@redhat.com>,
	Jamie Lentin <jm@lentin.co.uk>
Subject: Re: [PATCH 1/3] HID: input - Add microphone mute LED support
Date: Thu, 4 Jan 2024 09:55:42 -0800	[thread overview]
Message-ID: <ZZbxHpibdyNY_zUt@google.com> (raw)
In-Reply-To: <20240104154941.6919-1-mail@bernhard-seibold.de>

Hi Bernhard,

On Thu, Jan 04, 2024 at 04:49:39PM +0100, Bernhard Seibold wrote:
> Define an input event code for micmute led and enable sending it via HID
> 
> Signed-off-by: Bernhard Seibold <mail@bernhard-seibold.de>
> ---
>  drivers/hid/hid-input.c                | 1 +
>  include/uapi/linux/input-event-codes.h | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
> index c8b20d44b147..96c595bb14ce 100644
> --- a/drivers/hid/hid-input.c
> +++ b/drivers/hid/hid-input.c
> @@ -928,6 +928,7 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
>  		case 0x03:  map_led (LED_SCROLLL);  break;    /*   "Scroll Lock"              */
>  		case 0x04:  map_led (LED_COMPOSE);  break;    /*   "Compose"                  */
>  		case 0x05:  map_led (LED_KANA);     break;    /*   "Kana"                     */
> +		case 0x21:  map_led (LED_MICMUTE);  break;    /*   "Microphone"               */
>  		case 0x27:  map_led (LED_SLEEP);    break;    /*   "Stand-By"                 */
>  		case 0x4c:  map_led (LED_SUSPEND);  break;    /*   "System Suspend"           */
>  		case 0x09:  map_led (LED_MUTE);     break;    /*   "Mute"                     */
> diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h
> index 022a520e31fc..f9a4f9040c59 100644
> --- a/include/uapi/linux/input-event-codes.h
> +++ b/include/uapi/linux/input-event-codes.h
> @@ -952,6 +952,7 @@
>  #define LED_MISC		0x08
>  #define LED_MAIL		0x09
>  #define LED_CHARGING		0x0a
> +#define LED_MICMUTE		0x0b

No, please do not add new LEDs to input subsystem, and instead use "new"
LED APIs that are much more flexible. That is why we built the input
leds bridge in direction of input->leds and on the other way around.

The existing input LED definitions are grandfathered because they are
exposed to userspace (via evdev), but we will not be adding new ones.

Thanks.

-- 
Dmitry