| Message ID | 20200507122703.14617-1-koba.ko@canonical.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | Andy Shevchenko |
| Headers | show |
| Series | platform/x86: dell-laptop: don't register platform::micmute if the related tokens don't exist. | expand |
On Thu, May 7, 2020 at 3:36 PM Koba Ko <koba.ko@canonical.com> wrote: > > Hi All, > I enhanced the patch and refine the commet. > please check it again. Thank you. What you should or shouldn't have done: - you need to use version in the patch (-v<n>, where <n> is a version) - you need to write a changelog after cutter line '---' instead of separate messages or so - you need to avoid top postings like this very email you send - you need to wait for a while (usually one day or so) to give a change for people to look at it
> -----Original Message----- > From: koba.ko@canonical.com <koba.ko@canonical.com> > Sent: Thursday, May 7, 2020 7:27 AM > To: Matthew Garrett; Pali Rohár; Darren Hart; Andy Shevchenko; platform-driver- > x86@vger.kernel.org; linux-kernel@vger.kernel.org; Limonciello, Mario > Subject: [PATCH] platform/x86: dell-laptop: don't register platform::micmute if > the related tokens don't exist. > > > [EXTERNAL EMAIL] > > From: Koba Ko <koba.ko@canonical.com> > > During boot up, Error messge is issued, > "platform::micmute: Setting an LED's brightness failed (-19)", > but 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 call led_classdev_register() for platform::micmute. > After that, you wouldn't see the platform::micmute in /sys/class/leds/, > and the error message wouldn't see in dmesg. > Unless it's a pre-release platform, I think it's worth mentioning in commit message what the hardware is. If it's pre-release, then perhaps mention "pre-release vostro platform" here. This is useful information in case in the future something in this driver must change for a full class/line of systems. > Signed-off-by: Koba Ko <koba.ko@canonical.com> > --- > drivers/platform/x86/dell-laptop.c | 17 +++++++++++------ > 1 file changed, 11 insertions(+), 6 deletions(-) > > diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell- > laptop.c > index 74e988f839e8..e315185dbdd6 100644 > --- a/drivers/platform/x86/dell-laptop.c > +++ b/drivers/platform/x86/dell-laptop.c > @@ -2164,7 +2164,7 @@ static struct led_classdev micmute_led_cdev = { > static int __init dell_init(void) > { > struct calling_interface_token *token; > - int max_intensity = 0; > + int max_intensity = 0, is_micmute_exist = 0; As a nit, please declare variables on their own line and I don't like is_micmute_exist as this is bad grammar. Perhaps "micmute_exists" instead. Also this variable can be a boolean type instead with it's usage. > int ret; > > if (!dmi_check_system(dell_device_table)) > @@ -2204,10 +2204,14 @@ 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; > + is_micmute_exist = 1; > + } > > if (acpi_video_get_backlight_type() != acpi_backlight_vendor) > return 0; > @@ -2254,7 +2258,8 @@ static int __init dell_init(void) > fail_get_brightness: > backlight_device_unregister(dell_backlight_device); > fail_backlight: > - led_classdev_unregister(&micmute_led_cdev); > + if (is_micmute_exist) > + led_classdev_unregister(&micmute_led_cdev); > fail_led: > dell_cleanup_rfkill(); > fail_rfkill: > -- > 2.25.1
diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c index 74e988f839e8..e315185dbdd6 100644 --- a/drivers/platform/x86/dell-laptop.c +++ b/drivers/platform/x86/dell-laptop.c @@ -2164,7 +2164,7 @@ static struct led_classdev micmute_led_cdev = { static int __init dell_init(void) { struct calling_interface_token *token; - int max_intensity = 0; + int max_intensity = 0, is_micmute_exist = 0; int ret; if (!dmi_check_system(dell_device_table)) @@ -2204,10 +2204,14 @@ 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; + is_micmute_exist = 1; + } if (acpi_video_get_backlight_type() != acpi_backlight_vendor) return 0; @@ -2254,7 +2258,8 @@ static int __init dell_init(void) fail_get_brightness: backlight_device_unregister(dell_backlight_device); fail_backlight: - led_classdev_unregister(&micmute_led_cdev); + if (is_micmute_exist) + led_classdev_unregister(&micmute_led_cdev); fail_led: dell_cleanup_rfkill(); fail_rfkill: