[v1] platform/x86: dell-laptop: Register ctl-led for speaker-mute

[PATCH] platform/x86: dell-laptop: Register ctl-led for speaker-mute

Posted by Koba Ko 3 years, 1 month ago

Some platforms have the speaker-mute led and
current driver doesn't control it.

If the platform support the control of speaker-mute led, register it

Signed-off-by: Koba Ko <koba.ko@canonical.com>
---
 drivers/platform/x86/dell/dell-laptop.c | 43 +++++++++++++++++++++++++
 drivers/platform/x86/dell/dell-smbios.h |  2 ++
 2 files changed, 45 insertions(+)

diff --git a/drivers/platform/x86/dell/dell-laptop.c b/drivers/platform/x86/dell/dell-laptop.c
index 1321687d923ed..38d95bae8e3ab 100644
--- a/drivers/platform/x86/dell/dell-laptop.c
+++ b/drivers/platform/x86/dell/dell-laptop.c
@@ -97,6 +97,7 @@ static struct rfkill *bluetooth_rfkill;
 static struct rfkill *wwan_rfkill;
 static bool force_rfkill;
 static bool micmute_led_registered;
+static bool mute_led_registered;
 
 module_param(force_rfkill, bool, 0444);
 MODULE_PARM_DESC(force_rfkill, "enable rfkill on non whitelisted models");
@@ -2177,6 +2178,34 @@ static struct led_classdev micmute_led_cdev = {
 	.default_trigger = "audio-micmute",
 };
 
+static int mute_led_set(struct led_classdev *led_cdev,
+			   enum led_brightness brightness)
+{
+	struct calling_interface_buffer buffer;
+	struct calling_interface_token *token;
+	int state = brightness != LED_OFF;
+
+	if (state == 0)
+		token = dell_smbios_find_token(GLOBAL_MUTE_DISABLE);
+	else
+		token = dell_smbios_find_token(GLOBAL_MUTE_ENABLE);
+
+	if (!token)
+		return -ENODEV;
+
+	dell_fill_request(&buffer, token->location, token->value, 0, 0);
+	dell_send_request(&buffer, CLASS_TOKEN_WRITE, SELECT_TOKEN_STD);
+
+	return 0;
+}
+
+static struct led_classdev mute_led_cdev = {
+	.name = "platform::mute",
+	.max_brightness = 1,
+	.brightness_set_blocking = mute_led_set,
+	.default_trigger = "audio-mute",
+};
+
 static int __init dell_init(void)
 {
 	struct calling_interface_token *token;
@@ -2230,6 +2259,16 @@ static int __init dell_init(void)
 		micmute_led_registered = true;
 	}
 
+	if (dell_smbios_find_token(GLOBAL_MUTE_DISABLE) &&
+	    dell_smbios_find_token(GLOBAL_MUTE_ENABLE) &&
+	    !dell_privacy_has_mic_mute()) {
+		mute_led_cdev.brightness = ledtrig_audio_get(LED_AUDIO_MUTE);
+		ret = led_classdev_register(&platform_device->dev, &mute_led_cdev);
+		if (ret < 0)
+			goto fail_led;
+		mute_led_registered = true;
+	}
+
 	if (acpi_video_get_backlight_type() != acpi_backlight_vendor)
 		return 0;
 
@@ -2277,6 +2316,8 @@ static int __init dell_init(void)
 fail_backlight:
 	if (micmute_led_registered)
 		led_classdev_unregister(&micmute_led_cdev);
+	if (mute_led_registered)
+		led_classdev_unregister(&mute_led_cdev);
 fail_led:
 	dell_cleanup_rfkill();
 fail_rfkill:
@@ -2299,6 +2340,8 @@ static void __exit dell_exit(void)
 	backlight_device_unregister(dell_backlight_device);
 	if (micmute_led_registered)
 		led_classdev_unregister(&micmute_led_cdev);
+	if (mute_led_registered)
+		led_classdev_unregister(&mute_led_cdev);
 	dell_cleanup_rfkill();
 	if (platform_device) {
 		platform_device_unregister(platform_device);
diff --git a/drivers/platform/x86/dell/dell-smbios.h b/drivers/platform/x86/dell/dell-smbios.h
index 75fa8ea0476dc..eb341bf000c67 100644
--- a/drivers/platform/x86/dell/dell-smbios.h
+++ b/drivers/platform/x86/dell/dell-smbios.h
@@ -34,6 +34,8 @@
 #define KBD_LED_AUTO_100_TOKEN	0x02F6
 #define GLOBAL_MIC_MUTE_ENABLE	0x0364
 #define GLOBAL_MIC_MUTE_DISABLE	0x0365
+#define GLOBAL_MUTE_ENABLE	0x058C
+#define GLOBAL_MUTE_DISABLE	0x058D
 
 struct notifier_block;
 
-- 
2.25.1

Re: [PATCH] platform/x86: dell-laptop: Register ctl-led for speaker-mute

Posted by Hans de Goede 3 years, 1 month ago

Hi,

On 3/6/23 15:24, Koba Ko wrote:
> Some platforms have the speaker-mute led and
> current driver doesn't control it.
> 
> If the platform support the control of speaker-mute led, register it
> 
> Signed-off-by: Koba Ko <koba.ko@canonical.com>

Thank you for your patch, one small remark below.

> ---
>  drivers/platform/x86/dell/dell-laptop.c | 43 +++++++++++++++++++++++++
>  drivers/platform/x86/dell/dell-smbios.h |  2 ++
>  2 files changed, 45 insertions(+)
> 
> diff --git a/drivers/platform/x86/dell/dell-laptop.c b/drivers/platform/x86/dell/dell-laptop.c
> index 1321687d923ed..38d95bae8e3ab 100644
> --- a/drivers/platform/x86/dell/dell-laptop.c
> +++ b/drivers/platform/x86/dell/dell-laptop.c
> @@ -97,6 +97,7 @@ static struct rfkill *bluetooth_rfkill;
>  static struct rfkill *wwan_rfkill;
>  static bool force_rfkill;
>  static bool micmute_led_registered;
> +static bool mute_led_registered;
>  
>  module_param(force_rfkill, bool, 0444);
>  MODULE_PARM_DESC(force_rfkill, "enable rfkill on non whitelisted models");
> @@ -2177,6 +2178,34 @@ static struct led_classdev micmute_led_cdev = {
>  	.default_trigger = "audio-micmute",
>  };
>  
> +static int mute_led_set(struct led_classdev *led_cdev,
> +			   enum led_brightness brightness)
> +{
> +	struct calling_interface_buffer buffer;
> +	struct calling_interface_token *token;
> +	int state = brightness != LED_OFF;
> +
> +	if (state == 0)
> +		token = dell_smbios_find_token(GLOBAL_MUTE_DISABLE);
> +	else
> +		token = dell_smbios_find_token(GLOBAL_MUTE_ENABLE);
> +
> +	if (!token)
> +		return -ENODEV;
> +
> +	dell_fill_request(&buffer, token->location, token->value, 0, 0);
> +	dell_send_request(&buffer, CLASS_TOKEN_WRITE, SELECT_TOKEN_STD);
> +
> +	return 0;
> +}
> +
> +static struct led_classdev mute_led_cdev = {
> +	.name = "platform::mute",
> +	.max_brightness = 1,
> +	.brightness_set_blocking = mute_led_set,
> +	.default_trigger = "audio-mute",
> +};
> +
>  static int __init dell_init(void)
>  {
>  	struct calling_interface_token *token;
> @@ -2230,6 +2259,16 @@ static int __init dell_init(void)
>  		micmute_led_registered = true;
>  	}
>  
> +	if (dell_smbios_find_token(GLOBAL_MUTE_DISABLE) &&
> +	    dell_smbios_find_token(GLOBAL_MUTE_ENABLE) &&
> +	    !dell_privacy_has_mic_mute()) {

Since this is a speaker mute LED and since the Dell hw privacy
stuff does not deal with the speaker at all, I believe that you
should drop the "&& !dell_privacy_has_mic_mute()" part of
the if condition here ?

Can you please send a new version with this dropped?

Regards,

Hans


> +		mute_led_cdev.brightness = ledtrig_audio_get(LED_AUDIO_MUTE);
> +		ret = led_classdev_register(&platform_device->dev, &mute_led_cdev);
> +		if (ret < 0)
> +			goto fail_led;
> +		mute_led_registered = true;
> +	}
> +
>  	if (acpi_video_get_backlight_type() != acpi_backlight_vendor)
>  		return 0;
>  
> @@ -2277,6 +2316,8 @@ static int __init dell_init(void)
>  fail_backlight:
>  	if (micmute_led_registered)
>  		led_classdev_unregister(&micmute_led_cdev);
> +	if (mute_led_registered)
> +		led_classdev_unregister(&mute_led_cdev);
>  fail_led:
>  	dell_cleanup_rfkill();
>  fail_rfkill:
> @@ -2299,6 +2340,8 @@ static void __exit dell_exit(void)
>  	backlight_device_unregister(dell_backlight_device);
>  	if (micmute_led_registered)
>  		led_classdev_unregister(&micmute_led_cdev);
> +	if (mute_led_registered)
> +		led_classdev_unregister(&mute_led_cdev);
>  	dell_cleanup_rfkill();
>  	if (platform_device) {
>  		platform_device_unregister(platform_device);
> diff --git a/drivers/platform/x86/dell/dell-smbios.h b/drivers/platform/x86/dell/dell-smbios.h
> index 75fa8ea0476dc..eb341bf000c67 100644
> --- a/drivers/platform/x86/dell/dell-smbios.h
> +++ b/drivers/platform/x86/dell/dell-smbios.h
> @@ -34,6 +34,8 @@
>  #define KBD_LED_AUTO_100_TOKEN	0x02F6
>  #define GLOBAL_MIC_MUTE_ENABLE	0x0364
>  #define GLOBAL_MIC_MUTE_DISABLE	0x0365
> +#define GLOBAL_MUTE_ENABLE	0x058C
> +#define GLOBAL_MUTE_DISABLE	0x058D
>  
>  struct notifier_block;
>

Re: [PATCH] platform/x86: dell-laptop: Register ctl-led for speaker-mute

Posted by Koba Ko 3 years, 1 month ago

On Tue, Mar 7, 2023 at 8:10 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi,
>
> On 3/6/23 15:24, Koba Ko wrote:
> > Some platforms have the speaker-mute led and
> > current driver doesn't control it.
> >
> > If the platform support the control of speaker-mute led, register it
> >
> > Signed-off-by: Koba Ko <koba.ko@canonical.com>
>
> Thank you for your patch, one small remark below.
>
> > ---
> >  drivers/platform/x86/dell/dell-laptop.c | 43 +++++++++++++++++++++++++
> >  drivers/platform/x86/dell/dell-smbios.h |  2 ++
> >  2 files changed, 45 insertions(+)
> >
> > diff --git a/drivers/platform/x86/dell/dell-laptop.c b/drivers/platform/x86/dell/dell-laptop.c
> > index 1321687d923ed..38d95bae8e3ab 100644
> > --- a/drivers/platform/x86/dell/dell-laptop.c
> > +++ b/drivers/platform/x86/dell/dell-laptop.c
> > @@ -97,6 +97,7 @@ static struct rfkill *bluetooth_rfkill;
> >  static struct rfkill *wwan_rfkill;
> >  static bool force_rfkill;
> >  static bool micmute_led_registered;
> > +static bool mute_led_registered;
> >
> >  module_param(force_rfkill, bool, 0444);
> >  MODULE_PARM_DESC(force_rfkill, "enable rfkill on non whitelisted models");
> > @@ -2177,6 +2178,34 @@ static struct led_classdev micmute_led_cdev = {
> >       .default_trigger = "audio-micmute",
> >  };
> >
> > +static int mute_led_set(struct led_classdev *led_cdev,
> > +                        enum led_brightness brightness)
> > +{
> > +     struct calling_interface_buffer buffer;
> > +     struct calling_interface_token *token;
> > +     int state = brightness != LED_OFF;
> > +
> > +     if (state == 0)
> > +             token = dell_smbios_find_token(GLOBAL_MUTE_DISABLE);
> > +     else
> > +             token = dell_smbios_find_token(GLOBAL_MUTE_ENABLE);
> > +
> > +     if (!token)
> > +             return -ENODEV;
> > +
> > +     dell_fill_request(&buffer, token->location, token->value, 0, 0);
> > +     dell_send_request(&buffer, CLASS_TOKEN_WRITE, SELECT_TOKEN_STD);
> > +
> > +     return 0;
> > +}
> > +
> > +static struct led_classdev mute_led_cdev = {
> > +     .name = "platform::mute",
> > +     .max_brightness = 1,
> > +     .brightness_set_blocking = mute_led_set,
> > +     .default_trigger = "audio-mute",
> > +};
> > +
> >  static int __init dell_init(void)
> >  {
> >       struct calling_interface_token *token;
> > @@ -2230,6 +2259,16 @@ static int __init dell_init(void)
> >               micmute_led_registered = true;
> >       }
> >
> > +     if (dell_smbios_find_token(GLOBAL_MUTE_DISABLE) &&
> > +         dell_smbios_find_token(GLOBAL_MUTE_ENABLE) &&
> > +         !dell_privacy_has_mic_mute()) {
>
> Since this is a speaker mute LED and since the Dell hw privacy
> stuff does not deal with the speaker at all, I believe that you
> should drop the "&& !dell_privacy_has_mic_mute()" part of
> the if condition here ?
>
> Can you please send a new version with this dropped?

Sure, have sent v2 with dell_privacy_has_mic_mute dropped
Thanks

>
> Regards,
>
> Hans
>
>
> > +             mute_led_cdev.brightness = ledtrig_audio_get(LED_AUDIO_MUTE);
> > +             ret = led_classdev_register(&platform_device->dev, &mute_led_cdev);
> > +             if (ret < 0)
> > +                     goto fail_led;
> > +             mute_led_registered = true;
> > +     }
> > +
> >       if (acpi_video_get_backlight_type() != acpi_backlight_vendor)
> >               return 0;
> >
> > @@ -2277,6 +2316,8 @@ static int __init dell_init(void)
> >  fail_backlight:
> >       if (micmute_led_registered)
> >               led_classdev_unregister(&micmute_led_cdev);
> > +     if (mute_led_registered)
> > +             led_classdev_unregister(&mute_led_cdev);
> >  fail_led:
> >       dell_cleanup_rfkill();
> >  fail_rfkill:
> > @@ -2299,6 +2340,8 @@ static void __exit dell_exit(void)
> >       backlight_device_unregister(dell_backlight_device);
> >       if (micmute_led_registered)
> >               led_classdev_unregister(&micmute_led_cdev);
> > +     if (mute_led_registered)
> > +             led_classdev_unregister(&mute_led_cdev);
> >       dell_cleanup_rfkill();
> >       if (platform_device) {
> >               platform_device_unregister(platform_device);
> > diff --git a/drivers/platform/x86/dell/dell-smbios.h b/drivers/platform/x86/dell/dell-smbios.h
> > index 75fa8ea0476dc..eb341bf000c67 100644
> > --- a/drivers/platform/x86/dell/dell-smbios.h
> > +++ b/drivers/platform/x86/dell/dell-smbios.h
> > @@ -34,6 +34,8 @@
> >  #define KBD_LED_AUTO_100_TOKEN       0x02F6
> >  #define GLOBAL_MIC_MUTE_ENABLE       0x0364
> >  #define GLOBAL_MIC_MUTE_DISABLE      0x0365
> > +#define GLOBAL_MUTE_ENABLE   0x058C
> > +#define GLOBAL_MUTE_DISABLE  0x058D
> >
> >  struct notifier_block;
> >
>