2023-03-06 15:04:30

by Koba Ko

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

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 <[email protected]>
---
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

2023-03-07 12:11:29

by Hans de Goede

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

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 <[email protected]>

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;
>

2023-03-08 06:26:06

by Koba Ko

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

On Tue, Mar 7, 2023 at 8:10 PM Hans de Goede <[email protected]> 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 <[email protected]>
>
> 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;
> >
>