Re: [PATCH] platform/x86: dell-laptop: don't register platform::micmute if the related tokens don't exist.

[Pali RohÃr]

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 <pali@xxxxxxxxxx> wrote:
> 
> > On Thursday 07 May 2020 17:42:42 koba.ko@xxxxxxxxxxxxx wrote:
> > > From: Koba Ko <koba.ko@xxxxxxxxxxxxx>
> > >
> > > 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 <koba.ko@xxxxxxxxxxxxx>
> > > ---
> > >  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
> > >
> >