From: Koba Ko <[email protected]>
Error messge is issued,
"platform::micmute: Setting an LED's brightness failed (-19)",
Even the device isn't presented.
Get the related tokens of SMBIOS, GLOBAL_MIC_MUTE_DISABLE/ENABLE.
If one of two tokens doesn't exist, don't register platform::micmute.
Signed-off-by: Koba Ko <[email protected]>
---
drivers/platform/x86/dell-laptop.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c
index 1e46022fb2c5..afc1ded83e56 100644
--- a/drivers/platform/x86/dell-laptop.c
+++ b/drivers/platform/x86/dell-laptop.c
@@ -2208,10 +2208,13 @@ static int __init dell_init(void)
dell_laptop_register_notifier(&dell_laptop_notifier);
- micmute_led_cdev.brightness = ledtrig_audio_get(LED_AUDIO_MICMUTE);
- ret = led_classdev_register(&platform_device->dev, &micmute_led_cdev);
- if (ret < 0)
- goto fail_led;
+ if (dell_smbios_find_token(GLOBAL_MIC_MUTE_DISABLE) &&
+ dell_smbios_find_token(GLOBAL_MIC_MUTE_ENABLE)) {
+ micmute_led_cdev.brightness = ledtrig_audio_get(LED_AUDIO_MICMUTE);
+ ret = led_classdev_register(&platform_device->dev, &micmute_led_cdev);
+ if (ret < 0)
+ goto fail_led;
+ }
if (acpi_video_get_backlight_type() != acpi_backlight_vendor)
return 0;
--
2.17.1
+Cc: Mario
On Thu, May 7, 2020 at 12:42 PM <[email protected]> wrote:
>
> From: Koba Ko <[email protected]>
>
> Error messge is issued,
> "platform::micmute: Setting an LED's brightness failed (-19)",
> Even the device isn't presented.
>
> Get the related tokens of SMBIOS, GLOBAL_MIC_MUTE_DISABLE/ENABLE.
> If one of two tokens doesn't exist, don't register platform::micmute.
>
What the exact platform you are experiencing that on?
> Signed-off-by: Koba Ko <[email protected]>
> ---
> drivers/platform/x86/dell-laptop.c | 11 +++++++----
> 1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c
> index 1e46022fb2c5..afc1ded83e56 100644
> --- a/drivers/platform/x86/dell-laptop.c
> +++ b/drivers/platform/x86/dell-laptop.c
> @@ -2208,10 +2208,13 @@ static int __init dell_init(void)
>
> dell_laptop_register_notifier(&dell_laptop_notifier);
>
> - micmute_led_cdev.brightness = ledtrig_audio_get(LED_AUDIO_MICMUTE);
> - ret = led_classdev_register(&platform_device->dev, &micmute_led_cdev);
> - if (ret < 0)
> - goto fail_led;
> + if (dell_smbios_find_token(GLOBAL_MIC_MUTE_DISABLE) &&
> + dell_smbios_find_token(GLOBAL_MIC_MUTE_ENABLE)) {
> + micmute_led_cdev.brightness = ledtrig_audio_get(LED_AUDIO_MICMUTE);
> + ret = led_classdev_register(&platform_device->dev, &micmute_led_cdev);
> + if (ret < 0)
> + goto fail_led;
> + }
>
> if (acpi_video_get_backlight_type() != acpi_backlight_vendor)
> return 0;
> --
> 2.17.1
>
--
With Best Regards,
Andy Shevchenko
On Thursday 07 May 2020 17:42:42 [email protected] wrote:
> From: Koba Ko <[email protected]>
>
> Error messge is issued,
> "platform::micmute: Setting an LED's brightness failed (-19)",
> Even the device isn't presented.
>
> Get the related tokens of SMBIOS, GLOBAL_MIC_MUTE_DISABLE/ENABLE.
> If one of two tokens doesn't exist, don't register platform::micmute.
>
> Signed-off-by: Koba Ko <[email protected]>
> ---
> drivers/platform/x86/dell-laptop.c | 11 +++++++----
> 1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c
> index 1e46022fb2c5..afc1ded83e56 100644
> --- a/drivers/platform/x86/dell-laptop.c
> +++ b/drivers/platform/x86/dell-laptop.c
> @@ -2208,10 +2208,13 @@ static int __init dell_init(void)
>
> dell_laptop_register_notifier(&dell_laptop_notifier);
>
> - micmute_led_cdev.brightness = ledtrig_audio_get(LED_AUDIO_MICMUTE);
> - ret = led_classdev_register(&platform_device->dev, &micmute_led_cdev);
> - if (ret < 0)
> - goto fail_led;
> + if (dell_smbios_find_token(GLOBAL_MIC_MUTE_DISABLE) &&
> + dell_smbios_find_token(GLOBAL_MIC_MUTE_ENABLE)) {
> + micmute_led_cdev.brightness = ledtrig_audio_get(LED_AUDIO_MICMUTE);
> + ret = led_classdev_register(&platform_device->dev, &micmute_led_cdev);
> + if (ret < 0)
> + goto fail_led;
> + }
Hello! I think that this is correct approach. Changing micmute LED is
done via those GLOBAL_MIC_MUTE_DISABLE and GLOBAL_MIC_MUTE_ENABLE
tokens. And if these tokens are not supported by hardware then linux
kernel should not register micmute LED device. There are lot of Dell
machines without led diode for microphone and these machines obviously
would not support those tokens.
But this change is incomplete as registration of led class dev would be
optional. So deregistration also needs to be optional.
And I think there is missing better description / explanation of this
change to make it clear what really happens.
>
> if (acpi_video_get_backlight_type() != acpi_backlight_vendor)
> return 0;
> --
> 2.17.1
>
On Thursday 07 May 2020 19:27:47 Koba Ko wrote:
> Hi Pali,
> don't understand "registration and deregistration would be optional',
> could you explain more!?
After your patch led_classdev_register() function is not always called.
And led_classdev_unregister() should not be called when there is no
device registered.
> I will modify the comment of patch.
>
> On Thu, May 7, 2020 at 7:13 PM Pali Rohár <[email protected]> wrote:
>
> > On Thursday 07 May 2020 17:42:42 [email protected] wrote:
> > > From: Koba Ko <[email protected]>
> > >
> > > Error messge is issued,
> > > "platform::micmute: Setting an LED's brightness failed (-19)",
> > > Even the device isn't presented.
> > >
> > > Get the related tokens of SMBIOS, GLOBAL_MIC_MUTE_DISABLE/ENABLE.
> > > If one of two tokens doesn't exist, don't register platform::micmute.
> > >
> > > Signed-off-by: Koba Ko <[email protected]>
> > > ---
> > > drivers/platform/x86/dell-laptop.c | 11 +++++++----
> > > 1 file changed, 7 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/platform/x86/dell-laptop.c
> > b/drivers/platform/x86/dell-laptop.c
> > > index 1e46022fb2c5..afc1ded83e56 100644
> > > --- a/drivers/platform/x86/dell-laptop.c
> > > +++ b/drivers/platform/x86/dell-laptop.c
> > > @@ -2208,10 +2208,13 @@ static int __init dell_init(void)
> > >
> > > dell_laptop_register_notifier(&dell_laptop_notifier);
> > >
> > > - micmute_led_cdev.brightness = ledtrig_audio_get(LED_AUDIO_MICMUTE);
> > > - ret = led_classdev_register(&platform_device->dev,
> > &micmute_led_cdev);
> > > - if (ret < 0)
> > > - goto fail_led;
> > > + if (dell_smbios_find_token(GLOBAL_MIC_MUTE_DISABLE) &&
> > > + dell_smbios_find_token(GLOBAL_MIC_MUTE_ENABLE)) {
> > > + micmute_led_cdev.brightness =
> > ledtrig_audio_get(LED_AUDIO_MICMUTE);
> > > + ret = led_classdev_register(&platform_device->dev,
> > &micmute_led_cdev);
> > > + if (ret < 0)
> > > + goto fail_led;
> > > + }
> >
> > Hello! I think that this is correct approach. Changing micmute LED is
> > done via those GLOBAL_MIC_MUTE_DISABLE and GLOBAL_MIC_MUTE_ENABLE
> > tokens. And if these tokens are not supported by hardware then linux
> > kernel should not register micmute LED device. There are lot of Dell
> > machines without led diode for microphone and these machines obviously
> > would not support those tokens.
> >
> > But this change is incomplete as registration of led class dev would be
> > optional. So deregistration also needs to be optional.
> >
> > And I think there is missing better description / explanation of this
> > change to make it clear what really happens.
> >
> > >
> > > if (acpi_video_get_backlight_type() != acpi_backlight_vendor)
> > > return 0;
> > > --
> > > 2.17.1
> > >
> >
On Thu, May 7, 2020 at 2:45 PM Pali Rohár <[email protected]> wrote:
> On Thursday 07 May 2020 19:27:47 Koba Ko wrote:
> > don't understand "registration and deregistration would be optional',
> > could you explain more!?
>
> After your patch led_classdev_register() function is not always called.
> And led_classdev_unregister() should not be called when there is no
> device registered.
I think it's not a strong requirement after the commit
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit?id=1dbb9fb4082ce2a2f1cf9596881ddece062d15d0
--
With Best Regards,
Andy Shevchenko
On Thursday 07 May 2020 15:54:06 Andy Shevchenko wrote:
> On Thu, May 7, 2020 at 2:45 PM Pali Rohár <[email protected]> wrote:
> > On Thursday 07 May 2020 19:27:47 Koba Ko wrote:
>
> > > don't understand "registration and deregistration would be optional',
> > > could you explain more!?
> >
> > After your patch led_classdev_register() function is not always called.
> > And led_classdev_unregister() should not be called when there is no
> > device registered.
>
> I think it's not a strong requirement after the commit
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit?id=1dbb9fb4082ce2a2f1cf9596881ddece062d15d0
Thank you for update. I did know about this change.