From: <Mario.Limonciello@dell.com> To: <koba.ko@canonical.com>, <mjg59@srcf.ucam.org>, <pali.rohar@gmail.com>, <dvhart@infradead.org>, <andy@infradead.org>, <platform-driver-x86@vger.kernel.org>, <linux-kernel@vger.kernel.org> Subject: RE: [PATCH] platform/x86: dell-laptop: don't register platform::micmute if the related tokens don't exist. Date: Thu, 7 May 2020 20:11:08 +0000 [thread overview] Message-ID: <31e54254d233424f937ea767c5a26f57@AUSX13MPC101.AMER.DELL.COM> (raw) In-Reply-To: <20200507122703.14617-1-koba.ko@canonical.com> > -----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
WARNING: multiple messages have this Message-ID (diff)
From: <Mario.Limonciello@dell.com> To: koba.ko@canonical.com, mjg59@srcf.ucam.org, pali.rohar@gmail.com, dvhart@infradead.org, andy@infradead.org, platform-driver-x86@vger.kernel.org, linux-kernel@vger.kernel.org Subject: RE: [PATCH] platform/x86: dell-laptop: don't register platform::micmute if the related tokens don't exist. Date: Thu, 7 May 2020 20:11:08 +0000 [thread overview] Message-ID: <31e54254d233424f937ea767c5a26f57@AUSX13MPC101.AMER.DELL.COM> (raw) In-Reply-To: <20200507122703.14617-1-koba.ko@canonical.com> > -----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
next prev parent reply other threads:[~2020-05-07 20:11 UTC|newest] Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-05-07 12:27 [PATCH] platform/x86: dell-laptop: don't register platform::micmute if the related tokens don't exist koba.ko [not found] ` <CAJB-X+XwshJp1ud32kXxRGkE+8WdM+=e1MSewmOUvgERk2yGMw@mail.gmail.com> 2020-05-07 12:58 ` Andy Shevchenko 2020-05-07 20:11 ` Mario.Limonciello [this message] 2020-05-07 20:11 ` Mario.Limonciello -- strict thread matches above, loose matches on Subject: below -- 2020-05-11 1:16 koba.ko 2020-05-07 9:42 koba.ko 2020-05-07 10:41 ` Andy Shevchenko [not found] ` <CAJB-X+U9vOzKNt3Z-BzZrEJhyU0Gd_5rVM=yqfy3TZTWNn2_YA@mail.gmail.com> 2020-05-07 20:26 ` Mario.Limonciello 2020-05-07 20:26 ` Mario.Limonciello [not found] ` <CAJB-X+UTzknY63mL2iY5mXNDYm9ohm2ZeQBdPDUBZ+MSkcUAFQ@mail.gmail.com> 2020-05-08 17:25 ` Mario.Limonciello 2020-05-08 17:25 ` Mario.Limonciello 2020-05-07 11:13 ` Pali Rohár [not found] ` <CAJB-X+WKqrWuKK0=BWtj7f8AovsMzbCO-QaLi2ZaP0_Q6321WQ@mail.gmail.com> 2020-05-07 11:45 ` Pali Rohár 2020-05-07 12:54 ` Andy Shevchenko 2020-05-07 12:57 ` Pali Rohár 2020-05-07 8:49 koba.ko 2020-05-07 9:02 ` Andy Shevchenko
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=31e54254d233424f937ea767c5a26f57@AUSX13MPC101.AMER.DELL.COM \ --to=mario.limonciello@dell.com \ --cc=andy@infradead.org \ --cc=dvhart@infradead.org \ --cc=koba.ko@canonical.com \ --cc=linux-kernel@vger.kernel.org \ --cc=mjg59@srcf.ucam.org \ --cc=pali.rohar@gmail.com \ --cc=platform-driver-x86@vger.kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.