All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/6] Move dell-led to drivers/platform/x86
@ 2017-01-16 13:21 Michał Kępień
  2017-01-16 13:21 ` [PATCH v2 1/6] dell-led: remove GUID check from dell_micmute_led_set() Michał Kępień
                   ` (8 more replies)
  0 siblings, 9 replies; 43+ messages in thread
From: Michał Kępień @ 2017-01-16 13:21 UTC (permalink / raw)
  To: Richard Purdie, Jacek Anaszewski, Pavel Machek, Pali Rohár,
	Darren Hart, Jaroslav Kysela, Takashi Iwai
  Cc: Andy Shevchenko, Anthony Wong, linux-leds, platform-driver-x86,
	alsa-devel, linux-kernel

This patch series moves the dell-led driver from the LED subsystem to
the x86 platform driver subsystem.

The original motivation behind this effort was to move all code using
the dell-smbios module to the x86 platform driver subsystem.  While I
was investigating the possibilities to do that, it quickly emerged that
dell-led can and in fact should be moved to the x86 platform driver
subsystem in its entirety.

dell-led consists of two major parts:

  - the part exposing a microphone mute LED interface, introduced in
    db6d8cc00773 ("dell-led: add mic mute led interface"); this
    interface is used by sound/pci/hda/dell_wmi_helper.c; while the
    original implementation used a WMI interface, it was changed to use
    dell-smbios in cf0d7ea33596 ("dell-led: use dell_smbios_find_token()
    for finding mic DMI tokens") and 0c41a08e131d ("dell-led: use
    dell_smbios_send_request() for performing SMBIOS calls"),

  - the part handling an activity LED present in Dell Latitude 2100
    netbooks, introduced in 72dcd8d08aca ("leds: Add Dell Business Class
    Netbook LED driver"); it binds to a specific WMI GUID and then
    registers a LED device which is controlled using WMI (i.e. it is
    essentially a WMI driver).

Patches 1 and 2 clean up the microphone mute LED interface to minimize
the amount of code moved around.

Patch 3 updates a variable name in sound/pci/hda/dell_wmi_helper.c so
that it better matches that variable's role.

Patch 4 moves the microphone mute LED interface to
drivers/platform/x86/dell-laptop.c, effectively causing
sound/pci/hda/dell_wmi_helper.c to depend on CONFIG_DELL_LAPTOP instead
of CONFIG_LEDS_DELL_NETBOOKS.

Patch 5 reverts dell-led to the state it was in after its initial commit
72dcd8d08aca ("leds: Add Dell Business Class Netbook LED driver") by
removing all remnants of the microphone mute LED handling code.

Patch 6 moves all that is left of dell-led (i.e. the activity LED part,
as originally implemented), to a new module which is placed in
drivers/platform/x86/dell-wmi-led.c.

As all patches except patch 3 in this series affect the LED subsystem,
the series is based on linux-leds/for-4.11.

Anthony, I would be grateful if you could test this patch series on the
Dell machines you have access to that were previously supported by
dell-led as Jacek needs a Tested-by from someone to sign off on these
changes.  Please note the Kconfig option rename done by the last patch.
Thanks!

Changes from v1:

  - Squash patches 2-4 from v1 into a single patch (#2 in v2).

  - Add patch 3.

  - Fix subject pattern in patch 4.

  - Slight commit message adjustments, including fixing a typo
    ("COFIG_LEDS_DELL_NETBOOKS") in patch 6.

  - Remove the name of the module's source file from the header comment
    in drivers/platform/x86/dell-wmi-led.c to avoid the need to update
    it in the future.

 drivers/leds/Kconfig                               |  9 ---
 drivers/leds/Makefile                              |  1 -
 drivers/platform/x86/Kconfig                       |  8 +++
 drivers/platform/x86/Makefile                      |  1 +
 drivers/platform/x86/dell-laptop.c                 | 28 ++++++++
 .../dell-led.c => platform/x86/dell-wmi-led.c}     | 75 ++--------------------
 include/linux/dell-led.h                           |  6 +-
 sound/pci/hda/dell_wmi_helper.c                    | 30 ++++-----
 8 files changed, 60 insertions(+), 98 deletions(-)
 rename drivers/{leds/dell-led.c => platform/x86/dell-wmi-led.c} (73%)

-- 
2.11.0

^ permalink raw reply	[flat|nested] 43+ messages in thread

* [PATCH v2 1/6] dell-led: remove GUID check from dell_micmute_led_set()
  2017-01-16 13:21 [PATCH v2 0/6] Move dell-led to drivers/platform/x86 Michał Kępień
@ 2017-01-16 13:21 ` Michał Kępień
  2017-01-16 13:22 ` [PATCH v2 2/6] ALSA: hda - use dell_micmute_led_set() instead of dell_app_wmi_led_set() Michał Kępień
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 43+ messages in thread
From: Michał Kępień @ 2017-01-16 13:21 UTC (permalink / raw)
  To: Richard Purdie, Jacek Anaszewski, Pavel Machek, Pali Rohár,
	Darren Hart, Jaroslav Kysela, Takashi Iwai
  Cc: Andy Shevchenko, Anthony Wong, linux-leds, platform-driver-x86,
	alsa-devel, linux-kernel

As dell_micmute_led_set() no longer uses the dell_wmi_perform_query()
method, which was removed in commit 0c41a08e131d ("dell-led: use
dell_smbios_send_request() for performing SMBIOS calls"), the
DELL_APP_GUID check is redundant and thus can be safely removed.

Signed-off-by: Michał Kępień <kernel@kempniu.pl>
Reviewed-by: Pali Rohár <pali.rohar@gmail.com>
---
 drivers/leds/dell-led.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/leds/dell-led.c b/drivers/leds/dell-led.c
index b3d6e9c15cf9..e8e8f67224c1 100644
--- a/drivers/leds/dell-led.c
+++ b/drivers/leds/dell-led.c
@@ -51,9 +51,6 @@ static int dell_micmute_led_set(int state)
 	struct calling_interface_buffer *buffer;
 	struct calling_interface_token *token;
 
-	if (!wmi_has_guid(DELL_APP_GUID))
-		return -ENODEV;
-
 	if (state == 0)
 		token = dell_smbios_find_token(GLOBAL_MIC_MUTE_DISABLE);
 	else if (state == 1)
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PATCH v2 2/6] ALSA: hda - use dell_micmute_led_set() instead of dell_app_wmi_led_set()
  2017-01-16 13:21 [PATCH v2 0/6] Move dell-led to drivers/platform/x86 Michał Kępień
  2017-01-16 13:21 ` [PATCH v2 1/6] dell-led: remove GUID check from dell_micmute_led_set() Michał Kępień
@ 2017-01-16 13:22 ` Michał Kępień
  2017-01-16 13:22 ` [PATCH v2 3/6] ALSA: hda - rename dell_led_set_func to dell_micmute_led_set_func Michał Kępień
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 43+ messages in thread
From: Michał Kępień @ 2017-01-16 13:22 UTC (permalink / raw)
  To: Richard Purdie, Jacek Anaszewski, Pavel Machek, Pali Rohár,
	Darren Hart, Jaroslav Kysela, Takashi Iwai
  Cc: Andy Shevchenko, Anthony Wong, linux-leds, platform-driver-x86,
	alsa-devel, linux-kernel

The dell_app_wmi_led_set() method introduced in commit db6d8cc00773
("dell-led: add mic mute led interface") was implemented as an easily
extensible entry point for other modules to set the state of various
LEDs.  However, almost three years later it is still only used to
control the mic mute LED, so it will be replaced with direct calls to
dell_micmute_led_set().

Signed-off-by: Michał Kępień <kernel@kempniu.pl>
Reviewed-by: Pali Rohár <pali.rohar@gmail.com>
---
 drivers/leds/dell-led.c         | 20 ++------------------
 include/linux/dell-led.h        |  6 +-----
 sound/pci/hda/dell_wmi_helper.c | 12 ++++++------
 3 files changed, 9 insertions(+), 29 deletions(-)

diff --git a/drivers/leds/dell-led.c b/drivers/leds/dell-led.c
index e8e8f67224c1..f9002d9bb757 100644
--- a/drivers/leds/dell-led.c
+++ b/drivers/leds/dell-led.c
@@ -46,7 +46,7 @@ MODULE_ALIAS("wmi:" DELL_LED_BIOS_GUID);
 #define GLOBAL_MIC_MUTE_ENABLE	0x364
 #define GLOBAL_MIC_MUTE_DISABLE	0x365
 
-static int dell_micmute_led_set(int state)
+int dell_micmute_led_set(int state)
 {
 	struct calling_interface_buffer *buffer;
 	struct calling_interface_token *token;
@@ -69,23 +69,7 @@ static int dell_micmute_led_set(int state)
 
 	return state;
 }
-
-int dell_app_wmi_led_set(int whichled, int on)
-{
-	int state = 0;
-
-	switch (whichled) {
-	case DELL_LED_MICMUTE:
-		state = dell_micmute_led_set(on);
-		break;
-	default:
-		pr_warn("led type %x is not supported\n", whichled);
-		break;
-	}
-
-	return state;
-}
-EXPORT_SYMBOL_GPL(dell_app_wmi_led_set);
+EXPORT_SYMBOL_GPL(dell_micmute_led_set);
 
 struct bios_args {
 	unsigned char length;
diff --git a/include/linux/dell-led.h b/include/linux/dell-led.h
index 7009b8bec77b..3f033c48071e 100644
--- a/include/linux/dell-led.h
+++ b/include/linux/dell-led.h
@@ -1,10 +1,6 @@
 #ifndef __DELL_LED_H__
 #define __DELL_LED_H__
 
-enum {
-	DELL_LED_MICMUTE,
-};
-
-int dell_app_wmi_led_set(int whichled, int on);
+int dell_micmute_led_set(int on);
 
 #endif
diff --git a/sound/pci/hda/dell_wmi_helper.c b/sound/pci/hda/dell_wmi_helper.c
index 19d41da79f93..e128c8096772 100644
--- a/sound/pci/hda/dell_wmi_helper.c
+++ b/sound/pci/hda/dell_wmi_helper.c
@@ -6,7 +6,7 @@
 #include <linux/dell-led.h>
 
 static int dell_led_value;
-static int (*dell_led_set_func)(int, int);
+static int (*dell_led_set_func)(int);
 static void (*dell_old_cap_hook)(struct hda_codec *,
 			         struct snd_kcontrol *,
 				 struct snd_ctl_elem_value *);
@@ -27,7 +27,7 @@ static void update_dell_wmi_micmute_led(struct hda_codec *codec,
 			return;
 		dell_led_value = val;
 		if (dell_led_set_func)
-			dell_led_set_func(DELL_LED_MICMUTE, dell_led_value);
+			dell_led_set_func(dell_led_value);
 	}
 }
 
@@ -40,14 +40,14 @@ static void alc_fixup_dell_wmi(struct hda_codec *codec,
 
 	if (action == HDA_FIXUP_ACT_PROBE) {
 		if (!dell_led_set_func)
-			dell_led_set_func = symbol_request(dell_app_wmi_led_set);
+			dell_led_set_func = symbol_request(dell_micmute_led_set);
 		if (!dell_led_set_func) {
-			codec_warn(codec, "Failed to find dell wmi symbol dell_app_wmi_led_set\n");
+			codec_warn(codec, "Failed to find dell wmi symbol dell_micmute_led_set\n");
 			return;
 		}
 
 		removefunc = true;
-		if (dell_led_set_func(DELL_LED_MICMUTE, false) >= 0) {
+		if (dell_led_set_func(false) >= 0) {
 			dell_led_value = 0;
 			if (spec->gen.num_adc_nids > 1 && !spec->gen.dyn_adc_switch)
 				codec_dbg(codec, "Skipping micmute LED control due to several ADCs");
@@ -61,7 +61,7 @@ static void alc_fixup_dell_wmi(struct hda_codec *codec,
 	}
 
 	if (dell_led_set_func && (action == HDA_FIXUP_ACT_FREE || removefunc)) {
-		symbol_put(dell_app_wmi_led_set);
+		symbol_put(dell_micmute_led_set);
 		dell_led_set_func = NULL;
 		dell_old_cap_hook = NULL;
 	}
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PATCH v2 3/6] ALSA: hda - rename dell_led_set_func to dell_micmute_led_set_func
  2017-01-16 13:21 [PATCH v2 0/6] Move dell-led to drivers/platform/x86 Michał Kępień
  2017-01-16 13:21 ` [PATCH v2 1/6] dell-led: remove GUID check from dell_micmute_led_set() Michał Kępień
  2017-01-16 13:22 ` [PATCH v2 2/6] ALSA: hda - use dell_micmute_led_set() instead of dell_app_wmi_led_set() Michał Kępień
@ 2017-01-16 13:22 ` Michał Kępień
  2017-01-17 11:12   ` Pali Rohár
  2017-01-17 21:20   ` Jacek Anaszewski
  2017-01-16 13:22 ` [PATCH v2 4/6] platform/x86: dell-laptop: import dell_micmute_led_set() from drivers/leds/dell-led.c Michał Kępień
                   ` (5 subsequent siblings)
  8 siblings, 2 replies; 43+ messages in thread
From: Michał Kępień @ 2017-01-16 13:22 UTC (permalink / raw)
  To: Richard Purdie, Jacek Anaszewski, Pavel Machek, Pali Rohár,
	Darren Hart, Jaroslav Kysela, Takashi Iwai
  Cc: Andy Shevchenko, Anthony Wong, linux-leds, platform-driver-x86,
	alsa-devel, linux-kernel

With dell_app_wmi_led_set() replaced by dell_micmute_led_set(), rename
the function pointer to the latter for consistency.

Signed-off-by: Michał Kępień <kernel@kempniu.pl>
---
 sound/pci/hda/dell_wmi_helper.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/sound/pci/hda/dell_wmi_helper.c b/sound/pci/hda/dell_wmi_helper.c
index e128c8096772..516237ad6ef5 100644
--- a/sound/pci/hda/dell_wmi_helper.c
+++ b/sound/pci/hda/dell_wmi_helper.c
@@ -6,7 +6,7 @@
 #include <linux/dell-led.h>
 
 static int dell_led_value;
-static int (*dell_led_set_func)(int);
+static int (*dell_micmute_led_set_func)(int);
 static void (*dell_old_cap_hook)(struct hda_codec *,
 			         struct snd_kcontrol *,
 				 struct snd_ctl_elem_value *);
@@ -18,7 +18,7 @@ static void update_dell_wmi_micmute_led(struct hda_codec *codec,
 	if (dell_old_cap_hook)
 		dell_old_cap_hook(codec, kcontrol, ucontrol);
 
-	if (!ucontrol || !dell_led_set_func)
+	if (!ucontrol || !dell_micmute_led_set_func)
 		return;
 	if (strcmp("Capture Switch", ucontrol->id.name) == 0 && ucontrol->id.index == 0) {
 		/* TODO: How do I verify if it's a mono or stereo here? */
@@ -26,8 +26,8 @@ static void update_dell_wmi_micmute_led(struct hda_codec *codec,
 		if (val == dell_led_value)
 			return;
 		dell_led_value = val;
-		if (dell_led_set_func)
-			dell_led_set_func(dell_led_value);
+		if (dell_micmute_led_set_func)
+			dell_micmute_led_set_func(dell_led_value);
 	}
 }
 
@@ -39,15 +39,15 @@ static void alc_fixup_dell_wmi(struct hda_codec *codec,
 	bool removefunc = false;
 
 	if (action == HDA_FIXUP_ACT_PROBE) {
-		if (!dell_led_set_func)
-			dell_led_set_func = symbol_request(dell_micmute_led_set);
-		if (!dell_led_set_func) {
+		if (!dell_micmute_led_set_func)
+			dell_micmute_led_set_func = symbol_request(dell_micmute_led_set);
+		if (!dell_micmute_led_set_func) {
 			codec_warn(codec, "Failed to find dell wmi symbol dell_micmute_led_set\n");
 			return;
 		}
 
 		removefunc = true;
-		if (dell_led_set_func(false) >= 0) {
+		if (dell_micmute_led_set_func(false) >= 0) {
 			dell_led_value = 0;
 			if (spec->gen.num_adc_nids > 1 && !spec->gen.dyn_adc_switch)
 				codec_dbg(codec, "Skipping micmute LED control due to several ADCs");
@@ -60,9 +60,9 @@ static void alc_fixup_dell_wmi(struct hda_codec *codec,
 
 	}
 
-	if (dell_led_set_func && (action == HDA_FIXUP_ACT_FREE || removefunc)) {
+	if (dell_micmute_led_set_func && (action == HDA_FIXUP_ACT_FREE || removefunc)) {
 		symbol_put(dell_micmute_led_set);
-		dell_led_set_func = NULL;
+		dell_micmute_led_set_func = NULL;
 		dell_old_cap_hook = NULL;
 	}
 }
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PATCH v2 4/6] platform/x86: dell-laptop: import dell_micmute_led_set() from drivers/leds/dell-led.c
  2017-01-16 13:21 [PATCH v2 0/6] Move dell-led to drivers/platform/x86 Michał Kępień
                   ` (2 preceding siblings ...)
  2017-01-16 13:22 ` [PATCH v2 3/6] ALSA: hda - rename dell_led_set_func to dell_micmute_led_set_func Michał Kępień
@ 2017-01-16 13:22 ` Michał Kępień
  2017-01-17 11:23   ` Pali Rohár
  2017-01-18 19:12   ` Andy Shevchenko
  2017-01-16 13:22 ` [PATCH v2 5/6] dell-led: remove code related to mic mute LED Michał Kępień
                   ` (4 subsequent siblings)
  8 siblings, 2 replies; 43+ messages in thread
From: Michał Kępień @ 2017-01-16 13:22 UTC (permalink / raw)
  To: Richard Purdie, Jacek Anaszewski, Pavel Machek, Pali Rohár,
	Darren Hart, Jaroslav Kysela, Takashi Iwai
  Cc: Andy Shevchenko, Anthony Wong, linux-leds, platform-driver-x86,
	alsa-devel, linux-kernel

To ensure all users of dell-smbios are in drivers/platform/x86, move the
dell_micmute_led_set() method from drivers/leds/dell-led.c to
drivers/platform/x86/dell-laptop.c.

Signed-off-by: Michał Kępień <kernel@kempniu.pl>
---
 drivers/leds/dell-led.c            | 29 -----------------------------
 drivers/platform/x86/dell-laptop.c | 28 ++++++++++++++++++++++++++++
 sound/pci/hda/dell_wmi_helper.c    |  6 +++---
 3 files changed, 31 insertions(+), 32 deletions(-)

diff --git a/drivers/leds/dell-led.c b/drivers/leds/dell-led.c
index f9002d9bb757..c9cc36a7c890 100644
--- a/drivers/leds/dell-led.c
+++ b/drivers/leds/dell-led.c
@@ -16,7 +16,6 @@
 #include <linux/slab.h>
 #include <linux/module.h>
 #include <linux/dmi.h>
-#include <linux/dell-led.h>
 #include "../platform/x86/dell-smbios.h"
 
 MODULE_AUTHOR("Louis Davis/Jim Dailey");
@@ -43,34 +42,6 @@ MODULE_ALIAS("wmi:" DELL_LED_BIOS_GUID);
 #define CMD_LED_OFF	17
 #define CMD_LED_BLINK	18
 
-#define GLOBAL_MIC_MUTE_ENABLE	0x364
-#define GLOBAL_MIC_MUTE_DISABLE	0x365
-
-int dell_micmute_led_set(int state)
-{
-	struct calling_interface_buffer *buffer;
-	struct calling_interface_token *token;
-
-	if (state == 0)
-		token = dell_smbios_find_token(GLOBAL_MIC_MUTE_DISABLE);
-	else if (state == 1)
-		token = dell_smbios_find_token(GLOBAL_MIC_MUTE_ENABLE);
-	else
-		return -EINVAL;
-
-	if (!token)
-		return -ENODEV;
-
-	buffer = dell_smbios_get_buffer();
-	buffer->input[0] = token->location;
-	buffer->input[1] = token->value;
-	dell_smbios_send_request(1, 0);
-	dell_smbios_release_buffer();
-
-	return state;
-}
-EXPORT_SYMBOL_GPL(dell_micmute_led_set);
-
 struct bios_args {
 	unsigned char length;
 	unsigned char result_code;
diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c
index 2c2f02b2e08a..277656c74343 100644
--- a/drivers/platform/x86/dell-laptop.c
+++ b/drivers/platform/x86/dell-laptop.c
@@ -30,6 +30,7 @@
 #include <linux/i8042.h>
 #include <linux/debugfs.h>
 #include <linux/seq_file.h>
+#include <linux/dell-led.h>
 #include <acpi/video.h>
 #include "dell-rbtn.h"
 #include "dell-smbios.h"
@@ -42,6 +43,8 @@
 #define KBD_LED_AUTO_50_TOKEN 0x02EB
 #define KBD_LED_AUTO_75_TOKEN 0x02EC
 #define KBD_LED_AUTO_100_TOKEN 0x02F6
+#define GLOBAL_MIC_MUTE_ENABLE 0x364
+#define GLOBAL_MIC_MUTE_DISABLE 0x365
 
 struct quirk_entry {
 	u8 touchpad_led;
@@ -1970,6 +1973,31 @@ static void kbd_led_exit(void)
 	led_classdev_unregister(&kbd_led);
 }
 
+int dell_micmute_led_set(int state)
+{
+	struct calling_interface_buffer *buffer;
+	struct calling_interface_token *token;
+
+	if (state == 0)
+		token = dell_smbios_find_token(GLOBAL_MIC_MUTE_DISABLE);
+	else if (state == 1)
+		token = dell_smbios_find_token(GLOBAL_MIC_MUTE_ENABLE);
+	else
+		return -EINVAL;
+
+	if (!token)
+		return -ENODEV;
+
+	buffer = dell_smbios_get_buffer();
+	buffer->input[0] = token->location;
+	buffer->input[1] = token->value;
+	dell_smbios_send_request(1, 0);
+	dell_smbios_release_buffer();
+
+	return state;
+}
+EXPORT_SYMBOL_GPL(dell_micmute_led_set);
+
 static int __init dell_init(void)
 {
 	struct calling_interface_buffer *buffer;
diff --git a/sound/pci/hda/dell_wmi_helper.c b/sound/pci/hda/dell_wmi_helper.c
index 516237ad6ef5..7efa7bd7acb2 100644
--- a/sound/pci/hda/dell_wmi_helper.c
+++ b/sound/pci/hda/dell_wmi_helper.c
@@ -2,7 +2,7 @@
  * to be included from codec driver
  */
 
-#if IS_ENABLED(CONFIG_LEDS_DELL_NETBOOKS)
+#if IS_ENABLED(CONFIG_DELL_LAPTOP)
 #include <linux/dell-led.h>
 
 static int dell_led_value;
@@ -67,10 +67,10 @@ static void alc_fixup_dell_wmi(struct hda_codec *codec,
 	}
 }
 
-#else /* CONFIG_LEDS_DELL_NETBOOKS */
+#else /* CONFIG_DELL_LAPTOP */
 static void alc_fixup_dell_wmi(struct hda_codec *codec,
 			       const struct hda_fixup *fix, int action)
 {
 }
 
-#endif /* CONFIG_LEDS_DELL_NETBOOKS */
+#endif /* CONFIG_DELL_LAPTOP */
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PATCH v2 5/6] dell-led: remove code related to mic mute LED
  2017-01-16 13:21 [PATCH v2 0/6] Move dell-led to drivers/platform/x86 Michał Kępień
                   ` (3 preceding siblings ...)
  2017-01-16 13:22 ` [PATCH v2 4/6] platform/x86: dell-laptop: import dell_micmute_led_set() from drivers/leds/dell-led.c Michał Kępień
@ 2017-01-16 13:22 ` Michał Kępień
  2017-01-17 11:24   ` Pali Rohár
  2017-01-16 13:22 ` [PATCH v2 6/6] dell-led: move driver to drivers/platform/x86/dell-wmi-led.c Michał Kępień
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 43+ messages in thread
From: Michał Kępień @ 2017-01-16 13:22 UTC (permalink / raw)
  To: Richard Purdie, Jacek Anaszewski, Pavel Machek, Pali Rohár,
	Darren Hart, Jaroslav Kysela, Takashi Iwai
  Cc: Andy Shevchenko, Anthony Wong, linux-leds, platform-driver-x86,
	alsa-devel, linux-kernel

With dell_micmute_led_set() moved to drivers/platform/x86/dell-laptop.c,
all remnants of the mic mute LED handling code can be removed from
drivers/leds/dell-led.c, restoring it back to the state it was in before
commit db6d8cc00773 ("dell-led: add mic mute led interface").

Signed-off-by: Michał Kępień <kernel@kempniu.pl>
---
 drivers/leds/Kconfig    |  1 -
 drivers/leds/dell-led.c | 25 +++++++------------------
 2 files changed, 7 insertions(+), 19 deletions(-)

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index c621cbbb5768..f29b8698ca88 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -458,7 +458,6 @@ config LEDS_DELL_NETBOOKS
 	tristate "External LED on Dell Business Netbooks"
 	depends on LEDS_CLASS
 	depends on X86 && ACPI_WMI
-	depends on DELL_SMBIOS
 	help
 	  This adds support for the Latitude 2100 and similar
 	  notebooks that have an external LED.
diff --git a/drivers/leds/dell-led.c b/drivers/leds/dell-led.c
index c9cc36a7c890..e5c57389efd6 100644
--- a/drivers/leds/dell-led.c
+++ b/drivers/leds/dell-led.c
@@ -15,15 +15,12 @@
 #include <linux/leds.h>
 #include <linux/slab.h>
 #include <linux/module.h>
-#include <linux/dmi.h>
-#include "../platform/x86/dell-smbios.h"
 
 MODULE_AUTHOR("Louis Davis/Jim Dailey");
 MODULE_DESCRIPTION("Dell LED Control Driver");
 MODULE_LICENSE("GPL");
 
 #define DELL_LED_BIOS_GUID "F6E4FE6E-909D-47cb-8BAB-C9F6F2F8D396"
-#define DELL_APP_GUID "A80593CE-A997-11DA-B012-B622A1EF5492"
 MODULE_ALIAS("wmi:" DELL_LED_BIOS_GUID);
 
 /* Error Result Codes: */
@@ -184,29 +181,21 @@ static int __init dell_led_init(void)
 {
 	int error = 0;
 
-	if (!wmi_has_guid(DELL_LED_BIOS_GUID) && !wmi_has_guid(DELL_APP_GUID))
+	if (!wmi_has_guid(DELL_LED_BIOS_GUID))
 		return -ENODEV;
 
-	if (wmi_has_guid(DELL_LED_BIOS_GUID)) {
-		error = led_off();
-		if (error != 0)
-			return -ENODEV;
-
-		error = led_classdev_register(NULL, &dell_led);
-	}
+	error = led_off();
+	if (error != 0)
+		return -ENODEV;
 
-	return error;
+	return led_classdev_register(NULL, &dell_led);
 }
 
 static void __exit dell_led_exit(void)
 {
-	int error = 0;
+	led_classdev_unregister(&dell_led);
 
-	if (wmi_has_guid(DELL_LED_BIOS_GUID)) {
-		error = led_off();
-		if (error == 0)
-			led_classdev_unregister(&dell_led);
-	}
+	led_off();
 }
 
 module_init(dell_led_init);
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PATCH v2 6/6] dell-led: move driver to drivers/platform/x86/dell-wmi-led.c
  2017-01-16 13:21 [PATCH v2 0/6] Move dell-led to drivers/platform/x86 Michał Kępień
                   ` (4 preceding siblings ...)
  2017-01-16 13:22 ` [PATCH v2 5/6] dell-led: remove code related to mic mute LED Michał Kępień
@ 2017-01-16 13:22 ` Michał Kępień
  2017-01-16 20:49   ` Jacek Anaszewski
  2017-01-18 19:08   ` Andy Shevchenko
  2017-01-17  7:17 ` [PATCH v2 6+/6] platform/x86: dell-wmi-led: fix coding style issues Michał Kępień
                   ` (2 subsequent siblings)
  8 siblings, 2 replies; 43+ messages in thread
From: Michał Kępień @ 2017-01-16 13:22 UTC (permalink / raw)
  To: Richard Purdie, Jacek Anaszewski, Pavel Machek, Pali Rohár,
	Darren Hart, Jaroslav Kysela, Takashi Iwai
  Cc: Andy Shevchenko, Anthony Wong, linux-leds, platform-driver-x86,
	alsa-devel, linux-kernel

The dell-led driver handles a specific WMI GUID present on some Dell
laptops and as such it belongs in the x86 platform driver subsystem.
Source code is moved along with the relevant Kconfig and Makefile
entries, with some minor modifications:

  - Kconfig option is renamed from CONFIG_LEDS_DELL_NETBOOKS to
    CONFIG_DELL_WMI_LED,

  - the X86 Kconfig dependency is removed as the whole
    drivers/platform/x86 menu depends on it, so there is no need to
    duplicate it,

  - the name of the module's source file is removed from the header
    comment to avoid the need to update it in the future.

Signed-off-by: Michał Kępień <kernel@kempniu.pl>
---
 drivers/leds/Kconfig                                     | 8 --------
 drivers/leds/Makefile                                    | 1 -
 drivers/platform/x86/Kconfig                             | 8 ++++++++
 drivers/platform/x86/Makefile                            | 1 +
 drivers/{leds/dell-led.c => platform/x86/dell-wmi-led.c} | 2 --
 5 files changed, 9 insertions(+), 11 deletions(-)
 rename drivers/{leds/dell-led.c => platform/x86/dell-wmi-led.c} (99%)

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index f29b8698ca88..5af3fb2f52e4 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -454,14 +454,6 @@ config LEDS_ADP5520
 	  To compile this driver as a module, choose M here: the module will
 	  be called leds-adp5520.
 
-config LEDS_DELL_NETBOOKS
-	tristate "External LED on Dell Business Netbooks"
-	depends on LEDS_CLASS
-	depends on X86 && ACPI_WMI
-	help
-	  This adds support for the Latitude 2100 and similar
-	  notebooks that have an external LED.
-
 config LEDS_MC13783
 	tristate "LED Support for MC13XXX PMIC"
 	depends on LEDS_CLASS
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index 6b8273736478..558d24675454 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -52,7 +52,6 @@ obj-$(CONFIG_LEDS_REGULATOR)		+= leds-regulator.o
 obj-$(CONFIG_LEDS_INTEL_SS4200)		+= leds-ss4200.o
 obj-$(CONFIG_LEDS_LT3593)		+= leds-lt3593.o
 obj-$(CONFIG_LEDS_ADP5520)		+= leds-adp5520.o
-obj-$(CONFIG_LEDS_DELL_NETBOOKS)	+= dell-led.o
 obj-$(CONFIG_LEDS_MC13783)		+= leds-mc13783.o
 obj-$(CONFIG_LEDS_NS2)			+= leds-ns2.o
 obj-$(CONFIG_LEDS_NETXBIG)		+= leds-netxbig.o
diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 81b8dcca8891..f9018e8c1e49 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -143,6 +143,14 @@ config DELL_WMI_AIO
 	  To compile this driver as a module, choose M here: the module will
 	  be called dell-wmi-aio.
 
+config DELL_WMI_LED
+	tristate "External LED on Dell Business Netbooks"
+	depends on LEDS_CLASS
+	depends on ACPI_WMI
+	help
+	  This adds support for the Latitude 2100 and similar
+	  notebooks that have an external LED.
+
 config DELL_SMO8800
 	tristate "Dell Latitude freefall driver (ACPI SMO88XX)"
 	depends on ACPI
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index 2efa86d2a1a7..b061817ecaf1 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -15,6 +15,7 @@ obj-$(CONFIG_DELL_SMBIOS)	+= dell-smbios.o
 obj-$(CONFIG_DELL_LAPTOP)	+= dell-laptop.o
 obj-$(CONFIG_DELL_WMI)		+= dell-wmi.o
 obj-$(CONFIG_DELL_WMI_AIO)	+= dell-wmi-aio.o
+obj-$(CONFIG_DELL_WMI_LED)	+= dell-wmi-led.o
 obj-$(CONFIG_DELL_SMO8800)	+= dell-smo8800.o
 obj-$(CONFIG_DELL_RBTN)		+= dell-rbtn.o
 obj-$(CONFIG_ACER_WMI)		+= acer-wmi.o
diff --git a/drivers/leds/dell-led.c b/drivers/platform/x86/dell-wmi-led.c
similarity index 99%
rename from drivers/leds/dell-led.c
rename to drivers/platform/x86/dell-wmi-led.c
index e5c57389efd6..d0232c7f1909 100644
--- a/drivers/leds/dell-led.c
+++ b/drivers/platform/x86/dell-wmi-led.c
@@ -1,6 +1,4 @@
 /*
- * dell_led.c - Dell LED Driver
- *
  * Copyright (C) 2010 Dell Inc.
  * Louis Davis <louis_davis@dell.com>
  * Jim Dailey <jim_dailey@dell.com>
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 43+ messages in thread

* Re: [PATCH v2 6/6] dell-led: move driver to drivers/platform/x86/dell-wmi-led.c
  2017-01-16 13:22 ` [PATCH v2 6/6] dell-led: move driver to drivers/platform/x86/dell-wmi-led.c Michał Kępień
@ 2017-01-16 20:49   ` Jacek Anaszewski
  2017-01-17 11:08     ` Pavel Machek
  2017-01-18 19:08   ` Andy Shevchenko
  1 sibling, 1 reply; 43+ messages in thread
From: Jacek Anaszewski @ 2017-01-16 20:49 UTC (permalink / raw)
  To: Michał Kępień,
	Richard Purdie, Pavel Machek, Pali Rohár, Darren Hart,
	Jaroslav Kysela, Takashi Iwai
  Cc: Andy Shevchenko, Anthony Wong, linux-leds, platform-driver-x86,
	alsa-devel, linux-kernel

Hi Michał,

Thanks for the patch set.

Would you mind fixing also the following issues raised by
checkpatch.pl?:

WARNING: Missing a blank line after declarations
#376: FILE: drivers/platform/x86/dell-wmi-led.c:64:
+	struct bios_args args;
+	args.length = length;

WARNING: Block comments use * on subsequent lines
#460: FILE: drivers/platform/x86/dell-wmi-led.c:148:
+	/* The Dell LED delay is based on 125ms intervals.
+	   Need to round up to next interval. */

WARNING: Block comments use a trailing */ on a separate line
#460: FILE: drivers/platform/x86/dell-wmi-led.c:148:
+	   Need to round up to next interval. */

WARNING: Comparisons should place the constant on the right side of the test
#463: FILE: drivers/platform/x86/dell-wmi-led.c:151:
+	if (0 == on_eighths)

WARNING: Comparisons should place the constant on the right side of the test
#470: FILE: drivers/platform/x86/dell-wmi-led.c:158:
+	if (0 == off_eighths)

The fixes could be placed in an incremental patch after this one.

Best regards,
Jacek Anaszewski


On 01/16/2017 02:22 PM, Michał Kępień wrote:
> The dell-led driver handles a specific WMI GUID present on some Dell
> laptops and as such it belongs in the x86 platform driver subsystem.
> Source code is moved along with the relevant Kconfig and Makefile
> entries, with some minor modifications:
> 
>   - Kconfig option is renamed from CONFIG_LEDS_DELL_NETBOOKS to
>     CONFIG_DELL_WMI_LED,
> 
>   - the X86 Kconfig dependency is removed as the whole
>     drivers/platform/x86 menu depends on it, so there is no need to
>     duplicate it,
> 
>   - the name of the module's source file is removed from the header
>     comment to avoid the need to update it in the future.
> 
> Signed-off-by: Michał Kępień <kernel@kempniu.pl>
> ---
>  drivers/leds/Kconfig                                     | 8 --------
>  drivers/leds/Makefile                                    | 1 -
>  drivers/platform/x86/Kconfig                             | 8 ++++++++
>  drivers/platform/x86/Makefile                            | 1 +
>  drivers/{leds/dell-led.c => platform/x86/dell-wmi-led.c} | 2 --
>  5 files changed, 9 insertions(+), 11 deletions(-)
>  rename drivers/{leds/dell-led.c => platform/x86/dell-wmi-led.c} (99%)
> 
> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> index f29b8698ca88..5af3fb2f52e4 100644
> --- a/drivers/leds/Kconfig
> +++ b/drivers/leds/Kconfig
> @@ -454,14 +454,6 @@ config LEDS_ADP5520
>  	  To compile this driver as a module, choose M here: the module will
>  	  be called leds-adp5520.
>  
> -config LEDS_DELL_NETBOOKS
> -	tristate "External LED on Dell Business Netbooks"
> -	depends on LEDS_CLASS
> -	depends on X86 && ACPI_WMI
> -	help
> -	  This adds support for the Latitude 2100 and similar
> -	  notebooks that have an external LED.
> -
>  config LEDS_MC13783
>  	tristate "LED Support for MC13XXX PMIC"
>  	depends on LEDS_CLASS
> diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
> index 6b8273736478..558d24675454 100644
> --- a/drivers/leds/Makefile
> +++ b/drivers/leds/Makefile
> @@ -52,7 +52,6 @@ obj-$(CONFIG_LEDS_REGULATOR)		+= leds-regulator.o
>  obj-$(CONFIG_LEDS_INTEL_SS4200)		+= leds-ss4200.o
>  obj-$(CONFIG_LEDS_LT3593)		+= leds-lt3593.o
>  obj-$(CONFIG_LEDS_ADP5520)		+= leds-adp5520.o
> -obj-$(CONFIG_LEDS_DELL_NETBOOKS)	+= dell-led.o
>  obj-$(CONFIG_LEDS_MC13783)		+= leds-mc13783.o
>  obj-$(CONFIG_LEDS_NS2)			+= leds-ns2.o
>  obj-$(CONFIG_LEDS_NETXBIG)		+= leds-netxbig.o
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index 81b8dcca8891..f9018e8c1e49 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -143,6 +143,14 @@ config DELL_WMI_AIO
>  	  To compile this driver as a module, choose M here: the module will
>  	  be called dell-wmi-aio.
>  
> +config DELL_WMI_LED
> +	tristate "External LED on Dell Business Netbooks"
> +	depends on LEDS_CLASS
> +	depends on ACPI_WMI
> +	help
> +	  This adds support for the Latitude 2100 and similar
> +	  notebooks that have an external LED.
> +
>  config DELL_SMO8800
>  	tristate "Dell Latitude freefall driver (ACPI SMO88XX)"
>  	depends on ACPI
> diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
> index 2efa86d2a1a7..b061817ecaf1 100644
> --- a/drivers/platform/x86/Makefile
> +++ b/drivers/platform/x86/Makefile
> @@ -15,6 +15,7 @@ obj-$(CONFIG_DELL_SMBIOS)	+= dell-smbios.o
>  obj-$(CONFIG_DELL_LAPTOP)	+= dell-laptop.o
>  obj-$(CONFIG_DELL_WMI)		+= dell-wmi.o
>  obj-$(CONFIG_DELL_WMI_AIO)	+= dell-wmi-aio.o
> +obj-$(CONFIG_DELL_WMI_LED)	+= dell-wmi-led.o
>  obj-$(CONFIG_DELL_SMO8800)	+= dell-smo8800.o
>  obj-$(CONFIG_DELL_RBTN)		+= dell-rbtn.o
>  obj-$(CONFIG_ACER_WMI)		+= acer-wmi.o
> diff --git a/drivers/leds/dell-led.c b/drivers/platform/x86/dell-wmi-led.c
> similarity index 99%
> rename from drivers/leds/dell-led.c
> rename to drivers/platform/x86/dell-wmi-led.c
> index e5c57389efd6..d0232c7f1909 100644
> --- a/drivers/leds/dell-led.c
> +++ b/drivers/platform/x86/dell-wmi-led.c
> @@ -1,6 +1,4 @@
>  /*
> - * dell_led.c - Dell LED Driver
> - *
>   * Copyright (C) 2010 Dell Inc.
>   * Louis Davis <louis_davis@dell.com>
>   * Jim Dailey <jim_dailey@dell.com>
> 

^ permalink raw reply	[flat|nested] 43+ messages in thread

* [PATCH v2 6+/6] platform/x86: dell-wmi-led: fix coding style issues
  2017-01-16 13:21 [PATCH v2 0/6] Move dell-led to drivers/platform/x86 Michał Kępień
                   ` (5 preceding siblings ...)
  2017-01-16 13:22 ` [PATCH v2 6/6] dell-led: move driver to drivers/platform/x86/dell-wmi-led.c Michał Kępień
@ 2017-01-17  7:17 ` Michał Kępień
  2017-01-17  8:21   ` Joe Perches
                     ` (2 more replies)
  2017-01-17 21:20 ` [PATCH v2 0/6] Move dell-led to drivers/platform/x86 Jacek Anaszewski
  2017-02-13 11:26 ` Michał Kępień
  8 siblings, 3 replies; 43+ messages in thread
From: Michał Kępień @ 2017-01-17  7:17 UTC (permalink / raw)
  To: Richard Purdie, Jacek Anaszewski, Pavel Machek, Pali Rohár,
	Darren Hart, Jaroslav Kysela, Takashi Iwai
  Cc: Andy Shevchenko, Anthony Wong, linux-leds, platform-driver-x86,
	alsa-devel, linux-kernel

Fix coding style issues in dell-wmi-led which checkpatch complains about
to make sure the module gets a clean start in the x86 platform driver
subsystem.

Signed-off-by: Michał Kępień <kernel@kempniu.pl>
---
This is an extra patch that Jacek asked for [1].

[1] https://lkml.org/lkml/2017/1/16/631

 drivers/platform/x86/dell-wmi-led.c | 41 +++++++++++++++----------------------
 1 file changed, 16 insertions(+), 25 deletions(-)

diff --git a/drivers/platform/x86/dell-wmi-led.c b/drivers/platform/x86/dell-wmi-led.c
index d0232c7f1909..8753c4fc36b8 100644
--- a/drivers/platform/x86/dell-wmi-led.c
+++ b/drivers/platform/x86/dell-wmi-led.c
@@ -46,21 +46,16 @@ struct bios_args {
 	unsigned char off_time;
 };
 
-static int dell_led_perform_fn(u8 length,
-		u8 result_code,
-		u8 device_id,
-		u8 command,
-		u8 on_time,
-		u8 off_time)
+static int dell_led_perform_fn(u8 length, u8 result_code, u8 device_id,
+			       u8 command, u8 on_time, u8 off_time)
 {
-	struct bios_args *bios_return;
-	u8 return_code;
-	union acpi_object *obj;
 	struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
+	struct bios_args *bios_return, args;
 	struct acpi_buffer input;
+	union acpi_object *obj;
 	acpi_status status;
+	u8 return_code;
 
-	struct bios_args args;
 	args.length = length;
 	args.result_code = result_code;
 	args.device_id = device_id;
@@ -71,11 +66,7 @@ static int dell_led_perform_fn(u8 length,
 	input.length = sizeof(struct bios_args);
 	input.pointer = &args;
 
-	status = wmi_evaluate_method(DELL_LED_BIOS_GUID,
-		1,
-		1,
-		&input,
-		&output);
+	status = wmi_evaluate_method(DELL_LED_BIOS_GUID, 1, 1, &input, &output);
 
 	if (ACPI_FAILURE(status))
 		return status;
@@ -84,7 +75,7 @@ static int dell_led_perform_fn(u8 length,
 
 	if (!obj)
 		return -EINVAL;
-	else if (obj->type != ACPI_TYPE_BUFFER) {
+	if (obj->type != ACPI_TYPE_BUFFER) {
 		kfree(obj);
 		return -EINVAL;
 	}
@@ -117,8 +108,7 @@ static int led_off(void)
 		0);			/* not used */
 }
 
-static int led_blink(unsigned char on_eighths,
-		unsigned char off_eighths)
+static int led_blink(unsigned char on_eighths, unsigned char off_eighths)
 {
 	return dell_led_perform_fn(5,	/* Length of command */
 		INTERFACE_ERROR,	/* Init to  INTERFACE_ERROR */
@@ -129,7 +119,7 @@ static int led_blink(unsigned char on_eighths,
 }
 
 static void dell_led_set(struct led_classdev *led_cdev,
-		enum led_brightness value)
+			 enum led_brightness value)
 {
 	if (value == LED_OFF)
 		led_off();
@@ -138,24 +128,25 @@ static void dell_led_set(struct led_classdev *led_cdev,
 }
 
 static int dell_led_blink(struct led_classdev *led_cdev,
-		unsigned long *delay_on,
-		unsigned long *delay_off)
+			  unsigned long *delay_on, unsigned long *delay_off)
 {
 	unsigned long on_eighths;
 	unsigned long off_eighths;
 
-	/* The Dell LED delay is based on 125ms intervals.
-	   Need to round up to next interval. */
+	/*
+	 * The Dell LED delay is based on 125ms intervals.
+	 * Need to round up to next interval.
+	 */
 
 	on_eighths = (*delay_on + 124) / 125;
-	if (0 == on_eighths)
+	if (on_eighths == 0)
 		on_eighths = 1;
 	if (on_eighths > 255)
 		on_eighths = 255;
 	*delay_on = on_eighths * 125;
 
 	off_eighths = (*delay_off + 124) / 125;
-	if (0 == off_eighths)
+	if (off_eighths == 0)
 		off_eighths = 1;
 	if (off_eighths > 255)
 		off_eighths = 255;
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 43+ messages in thread

* Re: [PATCH v2 6+/6] platform/x86: dell-wmi-led: fix coding style issues
  2017-01-17  7:17 ` [PATCH v2 6+/6] platform/x86: dell-wmi-led: fix coding style issues Michał Kępień
@ 2017-01-17  8:21   ` Joe Perches
  2017-01-17  9:19     ` Michał Kępień
  2017-01-17 11:08   ` Pavel Machek
  2017-01-18 19:06   ` Andy Shevchenko
  2 siblings, 1 reply; 43+ messages in thread
From: Joe Perches @ 2017-01-17  8:21 UTC (permalink / raw)
  To: Michał Kępień,
	Richard Purdie, Jacek Anaszewski, Pavel Machek, Pali Rohár,
	Darren Hart, Jaroslav Kysela, Takashi Iwai
  Cc: Andy Shevchenko, Anthony Wong, linux-leds, platform-driver-x86,
	alsa-devel, linux-kernel

On Tue, 2017-01-17 at 08:17 +0100, Michał Kępień wrote:
> Fix coding style issues in dell-wmi-led which checkpatch complains about
> to make sure the module gets a clean start in the x86 platform driver
> subsystem.

trivia:

> Signed-off-by: Michał Kępień <kernel@kempniu.pl>
> ---
> This is an extra patch that Jacek asked for [1].
> 
> [1] https://lkml.org/lkml/2017/1/16/631
> 
>  drivers/platform/x86/dell-wmi-led.c | 41 +++++++++++++++----------------------
>  1 file changed, 16 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/platform/x86/dell-wmi-led.c b/drivers/platform/x86/dell-wmi-led.c
[]
> @@ -46,21 +46,16 @@ struct bios_args {
>  	unsigned char off_time;
>  };
>  
> -static int dell_led_perform_fn(u8 length,
> -		u8 result_code,
> -		u8 device_id,
> -		u8 command,
> -		u8 on_time,
> -		u8 off_time)
> +static int dell_led_perform_fn(u8 length, u8 result_code, u8 device_id,
> +			       u8 command, u8 on_time, u8 off_time)
>  {
> -	struct bios_args *bios_return;
> -	u8 return_code;
> -	union acpi_object *obj;
>  	struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
> +	struct bios_args *bios_return, args;
>  	struct acpi_buffer input;
> +	union acpi_object *obj;
>  	acpi_status status;
> +	u8 return_code;
>  
> -	struct bios_args args;
>  	args.length = length;
>  	args.result_code = result_code;
>  	args.device_id = device_id;

This declaration might be nicer using

	struct bios_args args = {
		.length = length,
		.result_code = result_code,
		.device_id = device_id,
		[...]
	};

[]

> @@ -138,24 +128,25 @@ static void dell_led_set(struct led_classdev *led_cdev,
>  }
>  
>  static int dell_led_blink(struct led_classdev *led_cdev,
> -		unsigned long *delay_on,
> -		unsigned long *delay_off)
> +			  unsigned long *delay_on, unsigned long *delay_off)
>  {
>  	unsigned long on_eighths;
>  	unsigned long off_eighths;
>  
> -	/* The Dell LED delay is based on 125ms intervals.
> -	   Need to round up to next interval. */
> +	/*
> +	 * The Dell LED delay is based on 125ms intervals.
> +	 * Need to round up to next interval.
> +	 */
>  
>  	on_eighths = (*delay_on + 124) / 125;
> -	if (0 == on_eighths)
> +	if (on_eighths == 0)
>  		on_eighths = 1;
>  	if (on_eighths > 255)
>  		on_eighths = 255;
>  	*delay_on = on_eighths * 125;
>  
>  	off_eighths = (*delay_off + 124) / 125;
> -	if (0 == off_eighths)
> +	if (off_eighths == 0)
>  		off_eighths = 1;
>  	if (off_eighths > 255)
>  		off_eighths = 255;

These could use DIV_ROUND_UP and clamp()

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v2 6+/6] platform/x86: dell-wmi-led: fix coding style issues
  2017-01-17  8:21   ` Joe Perches
@ 2017-01-17  9:19     ` Michał Kępień
  2017-01-17 21:20       ` Jacek Anaszewski
  0 siblings, 1 reply; 43+ messages in thread
From: Michał Kępień @ 2017-01-17  9:19 UTC (permalink / raw)
  To: Joe Perches, Jacek Anaszewski
  Cc: Richard Purdie, Pavel Machek, Pali Rohár, Darren Hart,
	Jaroslav Kysela, Takashi Iwai, Andy Shevchenko, Anthony Wong,
	linux-leds, platform-driver-x86, alsa-devel, linux-kernel

> On Tue, 2017-01-17 at 08:17 +0100, Michał Kępień wrote:
> > Fix coding style issues in dell-wmi-led which checkpatch complains about
> > to make sure the module gets a clean start in the x86 platform driver
> > subsystem.
> 
> trivia:
> 
> > Signed-off-by: Michał Kępień <kernel@kempniu.pl>
> > ---
> > This is an extra patch that Jacek asked for [1].
> > 
> > [1] https://lkml.org/lkml/2017/1/16/631
> > 
> >  drivers/platform/x86/dell-wmi-led.c | 41 +++++++++++++++----------------------
> >  1 file changed, 16 insertions(+), 25 deletions(-)
> > 
> > diff --git a/drivers/platform/x86/dell-wmi-led.c b/drivers/platform/x86/dell-wmi-led.c
> []
> > @@ -46,21 +46,16 @@ struct bios_args {
> >  	unsigned char off_time;
> >  };
> >  
> > -static int dell_led_perform_fn(u8 length,
> > -		u8 result_code,
> > -		u8 device_id,
> > -		u8 command,
> > -		u8 on_time,
> > -		u8 off_time)
> > +static int dell_led_perform_fn(u8 length, u8 result_code, u8 device_id,
> > +			       u8 command, u8 on_time, u8 off_time)
> >  {
> > -	struct bios_args *bios_return;
> > -	u8 return_code;
> > -	union acpi_object *obj;
> >  	struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
> > +	struct bios_args *bios_return, args;
> >  	struct acpi_buffer input;
> > +	union acpi_object *obj;
> >  	acpi_status status;
> > +	u8 return_code;
> >  
> > -	struct bios_args args;
> >  	args.length = length;
> >  	args.result_code = result_code;
> >  	args.device_id = device_id;
> 
> This declaration might be nicer using
> 
> 	struct bios_args args = {
> 		.length = length,
> 		.result_code = result_code,
> 		.device_id = device_id,
> 		[...]
> 	};
> 
> []
> 
> > @@ -138,24 +128,25 @@ static void dell_led_set(struct led_classdev *led_cdev,
> >  }
> >  
> >  static int dell_led_blink(struct led_classdev *led_cdev,
> > -		unsigned long *delay_on,
> > -		unsigned long *delay_off)
> > +			  unsigned long *delay_on, unsigned long *delay_off)
> >  {
> >  	unsigned long on_eighths;
> >  	unsigned long off_eighths;
> >  
> > -	/* The Dell LED delay is based on 125ms intervals.
> > -	   Need to round up to next interval. */
> > +	/*
> > +	 * The Dell LED delay is based on 125ms intervals.
> > +	 * Need to round up to next interval.
> > +	 */
> >  
> >  	on_eighths = (*delay_on + 124) / 125;
> > -	if (0 == on_eighths)
> > +	if (on_eighths == 0)
> >  		on_eighths = 1;
> >  	if (on_eighths > 255)
> >  		on_eighths = 255;
> >  	*delay_on = on_eighths * 125;
> >  
> >  	off_eighths = (*delay_off + 124) / 125;
> > -	if (0 == off_eighths)
> > +	if (off_eighths == 0)
> >  		off_eighths = 1;
> >  	if (off_eighths > 255)
> >  		off_eighths = 255;
> 
> These could use DIV_ROUND_UP and clamp()

Thanks for taking a look, Joe, I can certainly fix these.

Jacek, as resending an updated version of this patch with Joe's
suggestions taken into account would be even more confusing than the
"PATCH v2 6+/6" subject I already resorted to, I suggest the following:
if this series goes to v3, I will include an updated version of this
patch in v3, but in case the remaining patches get acked in their
current shape by all maintainers, I will send an updated version of this
extra patch separately, after the rest of the series gets applied.  Does
this sound reasonable?

-- 
Best regards,
Michał Kępień

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v2 6/6] dell-led: move driver to drivers/platform/x86/dell-wmi-led.c
  2017-01-16 20:49   ` Jacek Anaszewski
@ 2017-01-17 11:08     ` Pavel Machek
  2017-01-17 11:28       ` Pali Rohár
  0 siblings, 1 reply; 43+ messages in thread
From: Pavel Machek @ 2017-01-17 11:08 UTC (permalink / raw)
  To: Jacek Anaszewski
  Cc: Michał Kępień,
	Richard Purdie, Pali Rohár, Darren Hart, Jaroslav Kysela,
	Takashi Iwai, Andy Shevchenko, Anthony Wong, linux-leds,
	platform-driver-x86, alsa-devel, linux-kernel

Hi!

> Thanks for the patch set.
> 
> Would you mind fixing also the following issues raised by
> checkpatch.pl?:
> 
> WARNING: Missing a blank line after declarations
> #376: FILE: drivers/platform/x86/dell-wmi-led.c:64:
> +	struct bios_args args;
...
> The fixes could be placed in an incremental patch after this one.

Anyway, move itself is ok (and the checkpatch issues are not that serious anyway.)

Thanks,

Acked-by: Pavel Machek <pavel@ucw.cz>

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v2 6+/6] platform/x86: dell-wmi-led: fix coding style issues
  2017-01-17  7:17 ` [PATCH v2 6+/6] platform/x86: dell-wmi-led: fix coding style issues Michał Kępień
  2017-01-17  8:21   ` Joe Perches
@ 2017-01-17 11:08   ` Pavel Machek
  2017-01-18 19:06   ` Andy Shevchenko
  2 siblings, 0 replies; 43+ messages in thread
From: Pavel Machek @ 2017-01-17 11:08 UTC (permalink / raw)
  To: Michał Kępień
  Cc: Richard Purdie, Jacek Anaszewski, Pali Rohár, Darren Hart,
	Jaroslav Kysela, Takashi Iwai, Andy Shevchenko, Anthony Wong,
	linux-leds, platform-driver-x86, alsa-devel, linux-kernel

On Tue 2017-01-17 08:17:14, Michał Kępień wrote:
> Fix coding style issues in dell-wmi-led which checkpatch complains about
> to make sure the module gets a clean start in the x86 platform driver
> subsystem.
> 
> Signed-off-by: Michał Kępień <kernel@kempniu.pl>

Acked-by: Pavel Machek <pavel@ucw.cz>

> ---
> This is an extra patch that Jacek asked for [1].
> 
> [1] https://lkml.org/lkml/2017/1/16/631
> 
>  drivers/platform/x86/dell-wmi-led.c | 41 +++++++++++++++----------------------
>  1 file changed, 16 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/platform/x86/dell-wmi-led.c b/drivers/platform/x86/dell-wmi-led.c
> index d0232c7f1909..8753c4fc36b8 100644
> --- a/drivers/platform/x86/dell-wmi-led.c
> +++ b/drivers/platform/x86/dell-wmi-led.c
> @@ -46,21 +46,16 @@ struct bios_args {
>  	unsigned char off_time;
>  };
>  
> -static int dell_led_perform_fn(u8 length,
> -		u8 result_code,
> -		u8 device_id,
> -		u8 command,
> -		u8 on_time,
> -		u8 off_time)
> +static int dell_led_perform_fn(u8 length, u8 result_code, u8 device_id,
> +			       u8 command, u8 on_time, u8 off_time)
>  {
> -	struct bios_args *bios_return;
> -	u8 return_code;
> -	union acpi_object *obj;
>  	struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
> +	struct bios_args *bios_return, args;
>  	struct acpi_buffer input;
> +	union acpi_object *obj;
>  	acpi_status status;
> +	u8 return_code;
>  
> -	struct bios_args args;
>  	args.length = length;
>  	args.result_code = result_code;
>  	args.device_id = device_id;
> @@ -71,11 +66,7 @@ static int dell_led_perform_fn(u8 length,
>  	input.length = sizeof(struct bios_args);
>  	input.pointer = &args;
>  
> -	status = wmi_evaluate_method(DELL_LED_BIOS_GUID,
> -		1,
> -		1,
> -		&input,
> -		&output);
> +	status = wmi_evaluate_method(DELL_LED_BIOS_GUID, 1, 1, &input, &output);
>  
>  	if (ACPI_FAILURE(status))
>  		return status;
> @@ -84,7 +75,7 @@ static int dell_led_perform_fn(u8 length,
>  
>  	if (!obj)
>  		return -EINVAL;
> -	else if (obj->type != ACPI_TYPE_BUFFER) {
> +	if (obj->type != ACPI_TYPE_BUFFER) {
>  		kfree(obj);
>  		return -EINVAL;
>  	}
> @@ -117,8 +108,7 @@ static int led_off(void)
>  		0);			/* not used */
>  }
>  
> -static int led_blink(unsigned char on_eighths,
> -		unsigned char off_eighths)
> +static int led_blink(unsigned char on_eighths, unsigned char off_eighths)
>  {
>  	return dell_led_perform_fn(5,	/* Length of command */
>  		INTERFACE_ERROR,	/* Init to  INTERFACE_ERROR */
> @@ -129,7 +119,7 @@ static int led_blink(unsigned char on_eighths,
>  }
>  
>  static void dell_led_set(struct led_classdev *led_cdev,
> -		enum led_brightness value)
> +			 enum led_brightness value)
>  {
>  	if (value == LED_OFF)
>  		led_off();
> @@ -138,24 +128,25 @@ static void dell_led_set(struct led_classdev *led_cdev,
>  }
>  
>  static int dell_led_blink(struct led_classdev *led_cdev,
> -		unsigned long *delay_on,
> -		unsigned long *delay_off)
> +			  unsigned long *delay_on, unsigned long *delay_off)
>  {
>  	unsigned long on_eighths;
>  	unsigned long off_eighths;
>  
> -	/* The Dell LED delay is based on 125ms intervals.
> -	   Need to round up to next interval. */
> +	/*
> +	 * The Dell LED delay is based on 125ms intervals.
> +	 * Need to round up to next interval.
> +	 */
>  
>  	on_eighths = (*delay_on + 124) / 125;
> -	if (0 == on_eighths)
> +	if (on_eighths == 0)
>  		on_eighths = 1;
>  	if (on_eighths > 255)
>  		on_eighths = 255;
>  	*delay_on = on_eighths * 125;
>  
>  	off_eighths = (*delay_off + 124) / 125;
> -	if (0 == off_eighths)
> +	if (off_eighths == 0)
>  		off_eighths = 1;
>  	if (off_eighths > 255)
>  		off_eighths = 255;
> -- 
> 2.11.0

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v2 3/6] ALSA: hda - rename dell_led_set_func to dell_micmute_led_set_func
  2017-01-16 13:22 ` [PATCH v2 3/6] ALSA: hda - rename dell_led_set_func to dell_micmute_led_set_func Michał Kępień
@ 2017-01-17 11:12   ` Pali Rohár
  2017-01-17 21:20   ` Jacek Anaszewski
  1 sibling, 0 replies; 43+ messages in thread
From: Pali Rohár @ 2017-01-17 11:12 UTC (permalink / raw)
  To: Michał Kępień
  Cc: Richard Purdie, Jacek Anaszewski, Pavel Machek, Darren Hart,
	Jaroslav Kysela, Takashi Iwai, Andy Shevchenko, Anthony Wong,
	linux-leds, platform-driver-x86, alsa-devel, linux-kernel

On Monday 16 January 2017 14:22:01 Michał Kępień wrote:
> With dell_app_wmi_led_set() replaced by dell_micmute_led_set(), rename
> the function pointer to the latter for consistency.
> 
> Signed-off-by: Michał Kępień <kernel@kempniu.pl>

Reviewed-by: Pali Rohár <pali.rohar@gmail.com>

-- 
Pali Rohár
pali.rohar@gmail.com

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v2 4/6] platform/x86: dell-laptop: import dell_micmute_led_set() from drivers/leds/dell-led.c
  2017-01-16 13:22 ` [PATCH v2 4/6] platform/x86: dell-laptop: import dell_micmute_led_set() from drivers/leds/dell-led.c Michał Kępień
@ 2017-01-17 11:23   ` Pali Rohár
  2017-01-18 19:12   ` Andy Shevchenko
  1 sibling, 0 replies; 43+ messages in thread
From: Pali Rohár @ 2017-01-17 11:23 UTC (permalink / raw)
  To: Michał Kępień
  Cc: Richard Purdie, Jacek Anaszewski, Pavel Machek, Darren Hart,
	Jaroslav Kysela, Takashi Iwai, Andy Shevchenko, Anthony Wong,
	linux-leds, platform-driver-x86, alsa-devel, linux-kernel

On Monday 16 January 2017 14:22:02 Michał Kępień wrote:
> To ensure all users of dell-smbios are in drivers/platform/x86, move the
> dell_micmute_led_set() method from drivers/leds/dell-led.c to
> drivers/platform/x86/dell-laptop.c.
> 
> Signed-off-by: Michał Kępień <kernel@kempniu.pl>
> ---
>  drivers/leds/dell-led.c            | 29 -----------------------------
>  drivers/platform/x86/dell-laptop.c | 28 ++++++++++++++++++++++++++++
>  sound/pci/hda/dell_wmi_helper.c    |  6 +++---
>  3 files changed, 31 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/leds/dell-led.c b/drivers/leds/dell-led.c
> index f9002d9bb757..c9cc36a7c890 100644
> --- a/drivers/leds/dell-led.c
> +++ b/drivers/leds/dell-led.c
> @@ -16,7 +16,6 @@
>  #include <linux/slab.h>
>  #include <linux/module.h>
>  #include <linux/dmi.h>
> -#include <linux/dell-led.h>
>  #include "../platform/x86/dell-smbios.h"
>  
>  MODULE_AUTHOR("Louis Davis/Jim Dailey");
> @@ -43,34 +42,6 @@ MODULE_ALIAS("wmi:" DELL_LED_BIOS_GUID);
>  #define CMD_LED_OFF	17
>  #define CMD_LED_BLINK	18
>  
> -#define GLOBAL_MIC_MUTE_ENABLE	0x364
> -#define GLOBAL_MIC_MUTE_DISABLE	0x365
> -
> -int dell_micmute_led_set(int state)
> -{
> -	struct calling_interface_buffer *buffer;
> -	struct calling_interface_token *token;
> -
> -	if (state == 0)
> -		token = dell_smbios_find_token(GLOBAL_MIC_MUTE_DISABLE);
> -	else if (state == 1)
> -		token = dell_smbios_find_token(GLOBAL_MIC_MUTE_ENABLE);
> -	else
> -		return -EINVAL;
> -
> -	if (!token)
> -		return -ENODEV;
> -
> -	buffer = dell_smbios_get_buffer();
> -	buffer->input[0] = token->location;
> -	buffer->input[1] = token->value;
> -	dell_smbios_send_request(1, 0);
> -	dell_smbios_release_buffer();
> -
> -	return state;
> -}
> -EXPORT_SYMBOL_GPL(dell_micmute_led_set);
> -
>  struct bios_args {
>  	unsigned char length;
>  	unsigned char result_code;
> diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c
> index 2c2f02b2e08a..277656c74343 100644
> --- a/drivers/platform/x86/dell-laptop.c
> +++ b/drivers/platform/x86/dell-laptop.c
> @@ -30,6 +30,7 @@
>  #include <linux/i8042.h>
>  #include <linux/debugfs.h>
>  #include <linux/seq_file.h>
> +#include <linux/dell-led.h>
>  #include <acpi/video.h>
>  #include "dell-rbtn.h"
>  #include "dell-smbios.h"
> @@ -42,6 +43,8 @@
>  #define KBD_LED_AUTO_50_TOKEN 0x02EB
>  #define KBD_LED_AUTO_75_TOKEN 0x02EC
>  #define KBD_LED_AUTO_100_TOKEN 0x02F6
> +#define GLOBAL_MIC_MUTE_ENABLE 0x364
> +#define GLOBAL_MIC_MUTE_DISABLE 0x365

For consistency with other constants, please add leading zero (0x0364) so
all constants will be fully 16bit.

Otherwise OK and you can add my Reviewed-by.

>  
>  struct quirk_entry {
>  	u8 touchpad_led;
> @@ -1970,6 +1973,31 @@ static void kbd_led_exit(void)
>  	led_classdev_unregister(&kbd_led);
>  }
>  
> +int dell_micmute_led_set(int state)
> +{
> +	struct calling_interface_buffer *buffer;
> +	struct calling_interface_token *token;
> +
> +	if (state == 0)
> +		token = dell_smbios_find_token(GLOBAL_MIC_MUTE_DISABLE);
> +	else if (state == 1)
> +		token = dell_smbios_find_token(GLOBAL_MIC_MUTE_ENABLE);
> +	else
> +		return -EINVAL;
> +
> +	if (!token)
> +		return -ENODEV;
> +
> +	buffer = dell_smbios_get_buffer();
> +	buffer->input[0] = token->location;
> +	buffer->input[1] = token->value;
> +	dell_smbios_send_request(1, 0);
> +	dell_smbios_release_buffer();
> +
> +	return state;
> +}
> +EXPORT_SYMBOL_GPL(dell_micmute_led_set);
> +
>  static int __init dell_init(void)
>  {
>  	struct calling_interface_buffer *buffer;
> diff --git a/sound/pci/hda/dell_wmi_helper.c b/sound/pci/hda/dell_wmi_helper.c
> index 516237ad6ef5..7efa7bd7acb2 100644
> --- a/sound/pci/hda/dell_wmi_helper.c
> +++ b/sound/pci/hda/dell_wmi_helper.c
> @@ -2,7 +2,7 @@
>   * to be included from codec driver
>   */
>  
> -#if IS_ENABLED(CONFIG_LEDS_DELL_NETBOOKS)
> +#if IS_ENABLED(CONFIG_DELL_LAPTOP)
>  #include <linux/dell-led.h>
>  
>  static int dell_led_value;
> @@ -67,10 +67,10 @@ static void alc_fixup_dell_wmi(struct hda_codec *codec,
>  	}
>  }
>  
> -#else /* CONFIG_LEDS_DELL_NETBOOKS */
> +#else /* CONFIG_DELL_LAPTOP */
>  static void alc_fixup_dell_wmi(struct hda_codec *codec,
>  			       const struct hda_fixup *fix, int action)
>  {
>  }
>  
> -#endif /* CONFIG_LEDS_DELL_NETBOOKS */
> +#endif /* CONFIG_DELL_LAPTOP */

-- 
Pali Rohár
pali.rohar@gmail.com

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v2 5/6] dell-led: remove code related to mic mute LED
  2017-01-16 13:22 ` [PATCH v2 5/6] dell-led: remove code related to mic mute LED Michał Kępień
@ 2017-01-17 11:24   ` Pali Rohár
  0 siblings, 0 replies; 43+ messages in thread
From: Pali Rohár @ 2017-01-17 11:24 UTC (permalink / raw)
  To: Michał Kępień
  Cc: Richard Purdie, Jacek Anaszewski, Pavel Machek, Darren Hart,
	Jaroslav Kysela, Takashi Iwai, Andy Shevchenko, Anthony Wong,
	linux-leds, platform-driver-x86, alsa-devel, linux-kernel

On Monday 16 January 2017 14:22:03 Michał Kępień wrote:
> With dell_micmute_led_set() moved to drivers/platform/x86/dell-laptop.c,
> all remnants of the mic mute LED handling code can be removed from
> drivers/leds/dell-led.c, restoring it back to the state it was in before
> commit db6d8cc00773 ("dell-led: add mic mute led interface").
> 
> Signed-off-by: Michał Kępień <kernel@kempniu.pl>

Reviewed-by: Pali Rohár <pali.rohar@gmail.com>

-- 
Pali Rohár
pali.rohar@gmail.com

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v2 6/6] dell-led: move driver to drivers/platform/x86/dell-wmi-led.c
  2017-01-17 11:08     ` Pavel Machek
@ 2017-01-17 11:28       ` Pali Rohár
  0 siblings, 0 replies; 43+ messages in thread
From: Pali Rohár @ 2017-01-17 11:28 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Jacek Anaszewski, Michał Kępień,
	Richard Purdie, Darren Hart, Jaroslav Kysela, Takashi Iwai,
	Andy Shevchenko, Anthony Wong, linux-leds, platform-driver-x86,
	alsa-devel, linux-kernel

On Tuesday 17 January 2017 12:08:09 Pavel Machek wrote:
> Hi!
> 
> > Thanks for the patch set.
> > 
> > Would you mind fixing also the following issues raised by
> > checkpatch.pl?:
> > 
> > WARNING: Missing a blank line after declarations
> > #376: FILE: drivers/platform/x86/dell-wmi-led.c:64:
> > +	struct bios_args args;
> ...
> > The fixes could be placed in an incremental patch after this one.
> 
> Anyway, move itself is ok (and the checkpatch issues are not that serious anyway.)
> 
> Thanks,
> 
> Acked-by: Pavel Machek <pavel@ucw.cz>

Yes, this patch does not change code, just move one file to other
location. You can add my Reviewed-by.

-- 
Pali Rohár
pali.rohar@gmail.com

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v2 3/6] ALSA: hda - rename dell_led_set_func to dell_micmute_led_set_func
  2017-01-16 13:22 ` [PATCH v2 3/6] ALSA: hda - rename dell_led_set_func to dell_micmute_led_set_func Michał Kępień
  2017-01-17 11:12   ` Pali Rohár
@ 2017-01-17 21:20   ` Jacek Anaszewski
  1 sibling, 0 replies; 43+ messages in thread
From: Jacek Anaszewski @ 2017-01-17 21:20 UTC (permalink / raw)
  To: Jaroslav Kysela, Takashi Iwai
  Cc: Michał Kępień,
	Richard Purdie, Pavel Machek, Pali Rohár, Darren Hart,
	Andy Shevchenko, Anthony Wong, linux-leds, platform-driver-x86,
	alsa-devel, linux-kernel

Hi Jaroslav, Takashi,

I need an ack from sound sybsystem maintainer.
Are you OK with this patch going through LED tree?

Best regards,
Jacek Anaszewski

On 01/16/2017 02:22 PM, Michał Kępień wrote:
> With dell_app_wmi_led_set() replaced by dell_micmute_led_set(), rename
> the function pointer to the latter for consistency.
> 
> Signed-off-by: Michał Kępień <kernel@kempniu.pl>
> ---
>  sound/pci/hda/dell_wmi_helper.c | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/sound/pci/hda/dell_wmi_helper.c b/sound/pci/hda/dell_wmi_helper.c
> index e128c8096772..516237ad6ef5 100644
> --- a/sound/pci/hda/dell_wmi_helper.c
> +++ b/sound/pci/hda/dell_wmi_helper.c
> @@ -6,7 +6,7 @@
>  #include <linux/dell-led.h>
>  
>  static int dell_led_value;
> -static int (*dell_led_set_func)(int);
> +static int (*dell_micmute_led_set_func)(int);
>  static void (*dell_old_cap_hook)(struct hda_codec *,
>  			         struct snd_kcontrol *,
>  				 struct snd_ctl_elem_value *);
> @@ -18,7 +18,7 @@ static void update_dell_wmi_micmute_led(struct hda_codec *codec,
>  	if (dell_old_cap_hook)
>  		dell_old_cap_hook(codec, kcontrol, ucontrol);
>  
> -	if (!ucontrol || !dell_led_set_func)
> +	if (!ucontrol || !dell_micmute_led_set_func)
>  		return;
>  	if (strcmp("Capture Switch", ucontrol->id.name) == 0 && ucontrol->id.index == 0) {
>  		/* TODO: How do I verify if it's a mono or stereo here? */
> @@ -26,8 +26,8 @@ static void update_dell_wmi_micmute_led(struct hda_codec *codec,
>  		if (val == dell_led_value)
>  			return;
>  		dell_led_value = val;
> -		if (dell_led_set_func)
> -			dell_led_set_func(dell_led_value);
> +		if (dell_micmute_led_set_func)
> +			dell_micmute_led_set_func(dell_led_value);
>  	}
>  }
>  
> @@ -39,15 +39,15 @@ static void alc_fixup_dell_wmi(struct hda_codec *codec,
>  	bool removefunc = false;
>  
>  	if (action == HDA_FIXUP_ACT_PROBE) {
> -		if (!dell_led_set_func)
> -			dell_led_set_func = symbol_request(dell_micmute_led_set);
> -		if (!dell_led_set_func) {
> +		if (!dell_micmute_led_set_func)
> +			dell_micmute_led_set_func = symbol_request(dell_micmute_led_set);
> +		if (!dell_micmute_led_set_func) {
>  			codec_warn(codec, "Failed to find dell wmi symbol dell_micmute_led_set\n");
>  			return;
>  		}
>  
>  		removefunc = true;
> -		if (dell_led_set_func(false) >= 0) {
> +		if (dell_micmute_led_set_func(false) >= 0) {
>  			dell_led_value = 0;
>  			if (spec->gen.num_adc_nids > 1 && !spec->gen.dyn_adc_switch)
>  				codec_dbg(codec, "Skipping micmute LED control due to several ADCs");
> @@ -60,9 +60,9 @@ static void alc_fixup_dell_wmi(struct hda_codec *codec,
>  
>  	}
>  
> -	if (dell_led_set_func && (action == HDA_FIXUP_ACT_FREE || removefunc)) {
> +	if (dell_micmute_led_set_func && (action == HDA_FIXUP_ACT_FREE || removefunc)) {
>  		symbol_put(dell_micmute_led_set);
> -		dell_led_set_func = NULL;
> +		dell_micmute_led_set_func = NULL;
>  		dell_old_cap_hook = NULL;
>  	}
>  }
> 

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v2 0/6] Move dell-led to drivers/platform/x86
  2017-01-16 13:21 [PATCH v2 0/6] Move dell-led to drivers/platform/x86 Michał Kępień
                   ` (6 preceding siblings ...)
  2017-01-17  7:17 ` [PATCH v2 6+/6] platform/x86: dell-wmi-led: fix coding style issues Michał Kępień
@ 2017-01-17 21:20 ` Jacek Anaszewski
  2017-02-13 11:26 ` Michał Kępień
  8 siblings, 0 replies; 43+ messages in thread
From: Jacek Anaszewski @ 2017-01-17 21:20 UTC (permalink / raw)
  To: Darren Hart, Takashi Iwai
  Cc: Michał Kępień,
	Richard Purdie, Pavel Machek, Pali Rohár, Jaroslav Kysela,
	Andy Shevchenko, Anthony Wong, linux-leds, platform-driver-x86,
	alsa-devel, linux-kernel

Hi Darren,

Are you OK with the patches 4/6 and 6/6? If so, could you give
your ack so that I could apply the whole patch set to the LED tree?

Best regards,
Jacek Anaszewski

On 01/16/2017 02:21 PM, Michał Kępień wrote:
> This patch series moves the dell-led driver from the LED subsystem to
> the x86 platform driver subsystem.
> 
> The original motivation behind this effort was to move all code using
> the dell-smbios module to the x86 platform driver subsystem.  While I
> was investigating the possibilities to do that, it quickly emerged that
> dell-led can and in fact should be moved to the x86 platform driver
> subsystem in its entirety.
> 
> dell-led consists of two major parts:
> 
>   - the part exposing a microphone mute LED interface, introduced in
>     db6d8cc00773 ("dell-led: add mic mute led interface"); this
>     interface is used by sound/pci/hda/dell_wmi_helper.c; while the
>     original implementation used a WMI interface, it was changed to use
>     dell-smbios in cf0d7ea33596 ("dell-led: use dell_smbios_find_token()
>     for finding mic DMI tokens") and 0c41a08e131d ("dell-led: use
>     dell_smbios_send_request() for performing SMBIOS calls"),
> 
>   - the part handling an activity LED present in Dell Latitude 2100
>     netbooks, introduced in 72dcd8d08aca ("leds: Add Dell Business Class
>     Netbook LED driver"); it binds to a specific WMI GUID and then
>     registers a LED device which is controlled using WMI (i.e. it is
>     essentially a WMI driver).
> 
> Patches 1 and 2 clean up the microphone mute LED interface to minimize
> the amount of code moved around.
> 
> Patch 3 updates a variable name in sound/pci/hda/dell_wmi_helper.c so
> that it better matches that variable's role.
> 
> Patch 4 moves the microphone mute LED interface to
> drivers/platform/x86/dell-laptop.c, effectively causing
> sound/pci/hda/dell_wmi_helper.c to depend on CONFIG_DELL_LAPTOP instead
> of CONFIG_LEDS_DELL_NETBOOKS.
> 
> Patch 5 reverts dell-led to the state it was in after its initial commit
> 72dcd8d08aca ("leds: Add Dell Business Class Netbook LED driver") by
> removing all remnants of the microphone mute LED handling code.
> 
> Patch 6 moves all that is left of dell-led (i.e. the activity LED part,
> as originally implemented), to a new module which is placed in
> drivers/platform/x86/dell-wmi-led.c.
> 
> As all patches except patch 3 in this series affect the LED subsystem,
> the series is based on linux-leds/for-4.11.
> 
> Anthony, I would be grateful if you could test this patch series on the
> Dell machines you have access to that were previously supported by
> dell-led as Jacek needs a Tested-by from someone to sign off on these
> changes.  Please note the Kconfig option rename done by the last patch.
> Thanks!
> 
> Changes from v1:
> 
>   - Squash patches 2-4 from v1 into a single patch (#2 in v2).
> 
>   - Add patch 3.
> 
>   - Fix subject pattern in patch 4.
> 
>   - Slight commit message adjustments, including fixing a typo
>     ("COFIG_LEDS_DELL_NETBOOKS") in patch 6.
> 
>   - Remove the name of the module's source file from the header comment
>     in drivers/platform/x86/dell-wmi-led.c to avoid the need to update
>     it in the future.
> 
>  drivers/leds/Kconfig                               |  9 ---
>  drivers/leds/Makefile                              |  1 -
>  drivers/platform/x86/Kconfig                       |  8 +++
>  drivers/platform/x86/Makefile                      |  1 +
>  drivers/platform/x86/dell-laptop.c                 | 28 ++++++++
>  .../dell-led.c => platform/x86/dell-wmi-led.c}     | 75 ++--------------------
>  include/linux/dell-led.h                           |  6 +-
>  sound/pci/hda/dell_wmi_helper.c                    | 30 ++++-----
>  8 files changed, 60 insertions(+), 98 deletions(-)
>  rename drivers/{leds/dell-led.c => platform/x86/dell-wmi-led.c} (73%)
> 

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v2 6+/6] platform/x86: dell-wmi-led: fix coding style issues
  2017-01-17  9:19     ` Michał Kępień
@ 2017-01-17 21:20       ` Jacek Anaszewski
  0 siblings, 0 replies; 43+ messages in thread
From: Jacek Anaszewski @ 2017-01-17 21:20 UTC (permalink / raw)
  To: Michał Kępień, Joe Perches
  Cc: Richard Purdie, Pavel Machek, Pali Rohár, Darren Hart,
	Jaroslav Kysela, Takashi Iwai, Andy Shevchenko, Anthony Wong,
	linux-leds, platform-driver-x86, alsa-devel, linux-kernel

Hi Michał,

On 01/17/2017 10:19 AM, Michał Kępień wrote:
>> On Tue, 2017-01-17 at 08:17 +0100, Michał Kępień wrote:
>>> Fix coding style issues in dell-wmi-led which checkpatch complains about
>>> to make sure the module gets a clean start in the x86 platform driver
>>> subsystem.
>>
>> trivia:
>>
>>> Signed-off-by: Michał Kępień <kernel@kempniu.pl>
>>> ---
>>> This is an extra patch that Jacek asked for [1].
>>>
>>> [1] https://lkml.org/lkml/2017/1/16/631
>>>
>>>  drivers/platform/x86/dell-wmi-led.c | 41 +++++++++++++++----------------------
>>>  1 file changed, 16 insertions(+), 25 deletions(-)
>>>
>>> diff --git a/drivers/platform/x86/dell-wmi-led.c b/drivers/platform/x86/dell-wmi-led.c
>> []
>>> @@ -46,21 +46,16 @@ struct bios_args {
>>>  	unsigned char off_time;
>>>  };
>>>  
>>> -static int dell_led_perform_fn(u8 length,
>>> -		u8 result_code,
>>> -		u8 device_id,
>>> -		u8 command,
>>> -		u8 on_time,
>>> -		u8 off_time)
>>> +static int dell_led_perform_fn(u8 length, u8 result_code, u8 device_id,
>>> +			       u8 command, u8 on_time, u8 off_time)
>>>  {
>>> -	struct bios_args *bios_return;
>>> -	u8 return_code;
>>> -	union acpi_object *obj;
>>>  	struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
>>> +	struct bios_args *bios_return, args;
>>>  	struct acpi_buffer input;
>>> +	union acpi_object *obj;
>>>  	acpi_status status;
>>> +	u8 return_code;
>>>  
>>> -	struct bios_args args;
>>>  	args.length = length;
>>>  	args.result_code = result_code;
>>>  	args.device_id = device_id;
>>
>> This declaration might be nicer using
>>
>> 	struct bios_args args = {
>> 		.length = length,
>> 		.result_code = result_code,
>> 		.device_id = device_id,
>> 		[...]
>> 	};
>>
>> []
>>
>>> @@ -138,24 +128,25 @@ static void dell_led_set(struct led_classdev *led_cdev,
>>>  }
>>>  
>>>  static int dell_led_blink(struct led_classdev *led_cdev,
>>> -		unsigned long *delay_on,
>>> -		unsigned long *delay_off)
>>> +			  unsigned long *delay_on, unsigned long *delay_off)
>>>  {
>>>  	unsigned long on_eighths;
>>>  	unsigned long off_eighths;
>>>  
>>> -	/* The Dell LED delay is based on 125ms intervals.
>>> -	   Need to round up to next interval. */
>>> +	/*
>>> +	 * The Dell LED delay is based on 125ms intervals.
>>> +	 * Need to round up to next interval.
>>> +	 */
>>>  
>>>  	on_eighths = (*delay_on + 124) / 125;
>>> -	if (0 == on_eighths)
>>> +	if (on_eighths == 0)
>>>  		on_eighths = 1;
>>>  	if (on_eighths > 255)
>>>  		on_eighths = 255;
>>>  	*delay_on = on_eighths * 125;
>>>  
>>>  	off_eighths = (*delay_off + 124) / 125;
>>> -	if (0 == off_eighths)
>>> +	if (off_eighths == 0)
>>>  		off_eighths = 1;
>>>  	if (off_eighths > 255)
>>>  		off_eighths = 255;
>>
>> These could use DIV_ROUND_UP and clamp()
> 
> Thanks for taking a look, Joe, I can certainly fix these.
> 
> Jacek, as resending an updated version of this patch with Joe's
> suggestions taken into account would be even more confusing than the
> "PATCH v2 6+/6" subject I already resorted to, I suggest the following:
> if this series goes to v3, I will include an updated version of this
> patch in v3, but in case the remaining patches get acked in their
> current shape by all maintainers, I will send an updated version of this
> extra patch separately, after the rest of the series gets applied.  Does
> this sound reasonable?

Sure. I'll merge whole patch set after getting acks from sound
and x86 platform drivers maintainers.

-- 
Best regards,
Jacek Anaszewski

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v2 6+/6] platform/x86: dell-wmi-led: fix coding style issues
  2017-01-17  7:17 ` [PATCH v2 6+/6] platform/x86: dell-wmi-led: fix coding style issues Michał Kępień
  2017-01-17  8:21   ` Joe Perches
  2017-01-17 11:08   ` Pavel Machek
@ 2017-01-18 19:06   ` Andy Shevchenko
  2 siblings, 0 replies; 43+ messages in thread
From: Andy Shevchenko @ 2017-01-18 19:06 UTC (permalink / raw)
  To: Michał Kępień
  Cc: Richard Purdie, Jacek Anaszewski, Pavel Machek, Pali Rohár,
	Darren Hart, Jaroslav Kysela, Takashi Iwai, Anthony Wong,
	linux-leds, Platform Driver, ALSA Development Mailing List,
	linux-kernel

On Tue, Jan 17, 2017 at 9:17 AM, Michał Kępień <kernel@kempniu.pl> wrote:
> Fix coding style issues in dell-wmi-led which checkpatch complains about
> to make sure the module gets a clean start in the x86 platform driver
> subsystem.

> +       status = wmi_evaluate_method(DELL_LED_BIOS_GUID, 1, 1, &input, &output);
>

You may remove extra line.

>         if (ACPI_FAILURE(status))
>                 return status;

> -       if (0 == on_eighths)
> +       if (on_eighths == 0)
>                 on_eighths = 1;
>         if (on_eighths > 255)
>                 on_eighths = 255;

> -       if (0 == off_eighths)
> +       if (off_eighths == 0)
>                 off_eighths = 1;
>         if (off_eighths > 255)
>                 off_eighths = 255;

Obviously they both are custom clamp{_t}().

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v2 6/6] dell-led: move driver to drivers/platform/x86/dell-wmi-led.c
  2017-01-16 13:22 ` [PATCH v2 6/6] dell-led: move driver to drivers/platform/x86/dell-wmi-led.c Michał Kępień
  2017-01-16 20:49   ` Jacek Anaszewski
@ 2017-01-18 19:08   ` Andy Shevchenko
  1 sibling, 0 replies; 43+ messages in thread
From: Andy Shevchenko @ 2017-01-18 19:08 UTC (permalink / raw)
  To: Michał Kępień
  Cc: Richard Purdie, Jacek Anaszewski, Pavel Machek, Pali Rohár,
	Darren Hart, Jaroslav Kysela, Takashi Iwai, Anthony Wong,
	linux-leds, Platform Driver, ALSA Development Mailing List,
	linux-kernel

On Mon, Jan 16, 2017 at 3:22 PM, Michał Kępień <kernel@kempniu.pl> wrote:
> The dell-led driver handles a specific WMI GUID present on some Dell
> laptops and as such it belongs in the x86 platform driver subsystem.
> Source code is moved along with the relevant Kconfig and Makefile
> entries, with some minor modifications:
>
>   - Kconfig option is renamed from CONFIG_LEDS_DELL_NETBOOKS to
>     CONFIG_DELL_WMI_LED,
>
>   - the X86 Kconfig dependency is removed as the whole
>     drivers/platform/x86 menu depends on it, so there is no need to
>     duplicate it,
>
>   - the name of the module's source file is removed from the header
>     comment to avoid the need to update it in the future.
>

Acked-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> Signed-off-by: Michał Kępień <kernel@kempniu.pl>
> ---
>  drivers/leds/Kconfig                                     | 8 --------
>  drivers/leds/Makefile                                    | 1 -
>  drivers/platform/x86/Kconfig                             | 8 ++++++++
>  drivers/platform/x86/Makefile                            | 1 +
>  drivers/{leds/dell-led.c => platform/x86/dell-wmi-led.c} | 2 --
>  5 files changed, 9 insertions(+), 11 deletions(-)
>  rename drivers/{leds/dell-led.c => platform/x86/dell-wmi-led.c} (99%)
>
> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> index f29b8698ca88..5af3fb2f52e4 100644
> --- a/drivers/leds/Kconfig
> +++ b/drivers/leds/Kconfig
> @@ -454,14 +454,6 @@ config LEDS_ADP5520
>           To compile this driver as a module, choose M here: the module will
>           be called leds-adp5520.
>
> -config LEDS_DELL_NETBOOKS
> -       tristate "External LED on Dell Business Netbooks"
> -       depends on LEDS_CLASS
> -       depends on X86 && ACPI_WMI
> -       help
> -         This adds support for the Latitude 2100 and similar
> -         notebooks that have an external LED.
> -
>  config LEDS_MC13783
>         tristate "LED Support for MC13XXX PMIC"
>         depends on LEDS_CLASS
> diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
> index 6b8273736478..558d24675454 100644
> --- a/drivers/leds/Makefile
> +++ b/drivers/leds/Makefile
> @@ -52,7 +52,6 @@ obj-$(CONFIG_LEDS_REGULATOR)          += leds-regulator.o
>  obj-$(CONFIG_LEDS_INTEL_SS4200)                += leds-ss4200.o
>  obj-$(CONFIG_LEDS_LT3593)              += leds-lt3593.o
>  obj-$(CONFIG_LEDS_ADP5520)             += leds-adp5520.o
> -obj-$(CONFIG_LEDS_DELL_NETBOOKS)       += dell-led.o
>  obj-$(CONFIG_LEDS_MC13783)             += leds-mc13783.o
>  obj-$(CONFIG_LEDS_NS2)                 += leds-ns2.o
>  obj-$(CONFIG_LEDS_NETXBIG)             += leds-netxbig.o
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index 81b8dcca8891..f9018e8c1e49 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -143,6 +143,14 @@ config DELL_WMI_AIO
>           To compile this driver as a module, choose M here: the module will
>           be called dell-wmi-aio.
>
> +config DELL_WMI_LED
> +       tristate "External LED on Dell Business Netbooks"
> +       depends on LEDS_CLASS
> +       depends on ACPI_WMI
> +       help
> +         This adds support for the Latitude 2100 and similar
> +         notebooks that have an external LED.
> +
>  config DELL_SMO8800
>         tristate "Dell Latitude freefall driver (ACPI SMO88XX)"
>         depends on ACPI
> diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
> index 2efa86d2a1a7..b061817ecaf1 100644
> --- a/drivers/platform/x86/Makefile
> +++ b/drivers/platform/x86/Makefile
> @@ -15,6 +15,7 @@ obj-$(CONFIG_DELL_SMBIOS)     += dell-smbios.o
>  obj-$(CONFIG_DELL_LAPTOP)      += dell-laptop.o
>  obj-$(CONFIG_DELL_WMI)         += dell-wmi.o
>  obj-$(CONFIG_DELL_WMI_AIO)     += dell-wmi-aio.o
> +obj-$(CONFIG_DELL_WMI_LED)     += dell-wmi-led.o
>  obj-$(CONFIG_DELL_SMO8800)     += dell-smo8800.o
>  obj-$(CONFIG_DELL_RBTN)                += dell-rbtn.o
>  obj-$(CONFIG_ACER_WMI)         += acer-wmi.o
> diff --git a/drivers/leds/dell-led.c b/drivers/platform/x86/dell-wmi-led.c
> similarity index 99%
> rename from drivers/leds/dell-led.c
> rename to drivers/platform/x86/dell-wmi-led.c
> index e5c57389efd6..d0232c7f1909 100644
> --- a/drivers/leds/dell-led.c
> +++ b/drivers/platform/x86/dell-wmi-led.c
> @@ -1,6 +1,4 @@
>  /*
> - * dell_led.c - Dell LED Driver
> - *
>   * Copyright (C) 2010 Dell Inc.
>   * Louis Davis <louis_davis@dell.com>
>   * Jim Dailey <jim_dailey@dell.com>
> --
> 2.11.0
>



-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v2 4/6] platform/x86: dell-laptop: import dell_micmute_led_set() from drivers/leds/dell-led.c
  2017-01-16 13:22 ` [PATCH v2 4/6] platform/x86: dell-laptop: import dell_micmute_led_set() from drivers/leds/dell-led.c Michał Kępień
  2017-01-17 11:23   ` Pali Rohár
@ 2017-01-18 19:12   ` Andy Shevchenko
  1 sibling, 0 replies; 43+ messages in thread
From: Andy Shevchenko @ 2017-01-18 19:12 UTC (permalink / raw)
  To: Michał Kępień
  Cc: Richard Purdie, Jacek Anaszewski, Pavel Machek, Pali Rohár,
	Darren Hart, Jaroslav Kysela, Takashi Iwai, Anthony Wong,
	linux-leds, Platform Driver, ALSA Development Mailing List,
	linux-kernel

On Mon, Jan 16, 2017 at 3:22 PM, Michał Kępień <kernel@kempniu.pl> wrote:
> To ensure all users of dell-smbios are in drivers/platform/x86, move the
> dell_micmute_led_set() method from drivers/leds/dell-led.c to
> drivers/platform/x86/dell-laptop.c.
>

Acked-by: Andy Shevchenko <andy.shevchenko@gmail.com>

Couple of nitpicks below.

> --- a/drivers/platform/x86/dell-laptop.c
> +++ b/drivers/platform/x86/dell-laptop.c
> @@ -30,6 +30,7 @@
>  #include <linux/i8042.h>
>  #include <linux/debugfs.h>

>  #include <linux/seq_file.h>
> +#include <linux/dell-led.h>

Perhaps move this one line above

> +#define GLOBAL_MIC_MUTE_ENABLE 0x364
> +#define GLOBAL_MIC_MUTE_DISABLE 0x365

Same as Pali told.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v2 0/6] Move dell-led to drivers/platform/x86
  2017-01-16 13:21 [PATCH v2 0/6] Move dell-led to drivers/platform/x86 Michał Kępień
                   ` (7 preceding siblings ...)
  2017-01-17 21:20 ` [PATCH v2 0/6] Move dell-led to drivers/platform/x86 Jacek Anaszewski
@ 2017-02-13 11:26 ` Michał Kępień
  2017-02-15 11:56     ` Alex Hung
  2017-02-15 20:28     ` Takashi Iwai
  8 siblings, 2 replies; 43+ messages in thread
From: Michał Kępień @ 2017-02-13 11:26 UTC (permalink / raw)
  To: Jaroslav Kysela, Takashi Iwai, Anthony Wong
  Cc: Richard Purdie, Jacek Anaszewski, Pavel Machek, Pali Rohár,
	Darren Hart, Andy Shevchenko, linux-leds, platform-driver-x86,
	alsa-devel, linux-kernel

Hi everyone,

It has been almost a month since I posted v2, so I hope sending a
reminder is okay.

Jaroslav, Takashi, could you please ack this patch series from the sound
subsystem perspective?  Patches 2-4 touch it.

Anthony, have you and your team perhaps had a chance to test this patch
series on actual hardware?

Thanks,

-- 
Best regards,
Michał Kępień

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v2 0/6] Move dell-led to drivers/platform/x86
  2017-02-13 11:26 ` Michał Kępień
@ 2017-02-15 11:56     ` Alex Hung
  2017-02-15 20:28     ` Takashi Iwai
  1 sibling, 0 replies; 43+ messages in thread
From: Alex Hung @ 2017-02-15 11:56 UTC (permalink / raw)
  To: Michał Kępień
  Cc: platform-driver-x86, alsa-devel, linux-kernel, Anthony Wong,
	Takashi Iwai, Andy Shevchenko, Richard Purdie, Jacek Anaszewski,
	Pavel Machek, Pali Rohár, Darren Hart, Linux LED Subsystem

Hi,

I tested the patches with the following setup. Please note I tested
the microphone mute led by GUI, as the hotkey does not work on this
system.

1. Downloaded the below patches from patchwork

9518737 Awaiting Upstream [v2,1/6] dell-led: remove GUID check from
dell_micmute_led_set()
9518761 Awaiting Upstream [v2,2/6] ALSA: hda - use
dell_micmute_led_set() instead of dell_app_wmi_led_set()
9518741 Awaiting Upstream [v2,3/6] ALSA: hda - rename
dell_led_set_func to dell_micmute_led_set_func
9518725 Awaiting Upstream [v2,4/6] platform/x86: dell-laptop: import
dell_micmute_led_set() from drivers/leds/dell-led.c
9518727 Awaiting Upstream [v2,5/6] dell-led: remove code related to mic mute LED
9518735 Awaiting Upstream [v2,6/6] dell-led: move driver to
drivers/platform/x86/dell-wmi-led.c
9520053 Awaiting Upstream [v2,6+/6] platform/x86: dell-wmi-led: fix
coding style issues

2. Installed Ubuntu 16.10 (Linux kernel 4.8) on Dell Latitude 7180

  -> Microphone mute led works as expected

3. Applied downloaded patches to kernel 4.8 without any conflicts, but
kernel will not compile successfully.

4. Installed Ubuntu Zesty kernel (Linux kernel 4.9)

  -> Microphone mute led works as expected

5. Applied and compiled downloaded patches to kernel 4.9

  -> Microphone mute led does not work

6. Compiled and installed Linux kernel 4.10 rc8

  -> Microphone mute led does not work

7. Applied and compiled downloaded patches to kernel 4.10 rc8

  -> Microphone mute led does not work



On Mon, Feb 13, 2017 at 7:26 PM, Michał Kępień <kernel@kempniu.pl> wrote:
> Hi everyone,
>
> It has been almost a month since I posted v2, so I hope sending a
> reminder is okay.
>
> Jaroslav, Takashi, could you please ack this patch series from the sound
> subsystem perspective?  Patches 2-4 touch it.
>
> Anthony, have you and your team perhaps had a chance to test this patch
> series on actual hardware?
>
> Thanks,
>
> --
> Best regards,
> Michał Kępień



-- 
Cheers,
Alex Hung
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v2 0/6] Move dell-led to drivers/platform/x86
@ 2017-02-15 11:56     ` Alex Hung
  0 siblings, 0 replies; 43+ messages in thread
From: Alex Hung @ 2017-02-15 11:56 UTC (permalink / raw)
  To: Michał Kępień
  Cc: Jaroslav Kysela, Takashi Iwai, Anthony Wong, Richard Purdie,
	Jacek Anaszewski, Pavel Machek, Pali Rohár, Darren Hart,
	Andy Shevchenko, Linux LED Subsystem, platform-driver-x86,
	alsa-devel, linux-kernel

Hi,

I tested the patches with the following setup. Please note I tested
the microphone mute led by GUI, as the hotkey does not work on this
system.

1. Downloaded the below patches from patchwork

9518737 Awaiting Upstream [v2,1/6] dell-led: remove GUID check from
dell_micmute_led_set()
9518761 Awaiting Upstream [v2,2/6] ALSA: hda - use
dell_micmute_led_set() instead of dell_app_wmi_led_set()
9518741 Awaiting Upstream [v2,3/6] ALSA: hda - rename
dell_led_set_func to dell_micmute_led_set_func
9518725 Awaiting Upstream [v2,4/6] platform/x86: dell-laptop: import
dell_micmute_led_set() from drivers/leds/dell-led.c
9518727 Awaiting Upstream [v2,5/6] dell-led: remove code related to mic mute LED
9518735 Awaiting Upstream [v2,6/6] dell-led: move driver to
drivers/platform/x86/dell-wmi-led.c
9520053 Awaiting Upstream [v2,6+/6] platform/x86: dell-wmi-led: fix
coding style issues

2. Installed Ubuntu 16.10 (Linux kernel 4.8) on Dell Latitude 7180

  -> Microphone mute led works as expected

3. Applied downloaded patches to kernel 4.8 without any conflicts, but
kernel will not compile successfully.

4. Installed Ubuntu Zesty kernel (Linux kernel 4.9)

  -> Microphone mute led works as expected

5. Applied and compiled downloaded patches to kernel 4.9

  -> Microphone mute led does not work

6. Compiled and installed Linux kernel 4.10 rc8

  -> Microphone mute led does not work

7. Applied and compiled downloaded patches to kernel 4.10 rc8

  -> Microphone mute led does not work



On Mon, Feb 13, 2017 at 7:26 PM, Michał Kępień <kernel@kempniu.pl> wrote:
> Hi everyone,
>
> It has been almost a month since I posted v2, so I hope sending a
> reminder is okay.
>
> Jaroslav, Takashi, could you please ack this patch series from the sound
> subsystem perspective?  Patches 2-4 touch it.
>
> Anthony, have you and your team perhaps had a chance to test this patch
> series on actual hardware?
>
> Thanks,
>
> --
> Best regards,
> Michał Kępień



-- 
Cheers,
Alex Hung

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v2 0/6] Move dell-led to drivers/platform/x86
  2017-02-15 11:56     ` Alex Hung
  (?)
@ 2017-02-15 13:54     ` Michał Kępień
  2017-02-15 14:31         ` Alex Hung
  -1 siblings, 1 reply; 43+ messages in thread
From: Michał Kępień @ 2017-02-15 13:54 UTC (permalink / raw)
  To: Alex Hung
  Cc: Jaroslav Kysela, Takashi Iwai, Anthony Wong, Richard Purdie,
	Jacek Anaszewski, Pavel Machek, Pali Rohár, Darren Hart,
	Andy Shevchenko, Linux LED Subsystem, platform-driver-x86,
	alsa-devel, linux-kernel

Alex, 

> I tested the patches with the following setup. Please note I tested
> the microphone mute led by GUI, as the hotkey does not work on this
> system.

Thank you for taking the time to test this series, it is appreciated.  I
would not expect the switching method (GUI/hotkey) to matter here, as
long as we can compare behavior of two kernel versions by consistently
using one of them.

> 
> 1. Downloaded the below patches from patchwork
> 
> 9518737 Awaiting Upstream [v2,1/6] dell-led: remove GUID check from
> dell_micmute_led_set()
> 9518761 Awaiting Upstream [v2,2/6] ALSA: hda - use
> dell_micmute_led_set() instead of dell_app_wmi_led_set()
> 9518741 Awaiting Upstream [v2,3/6] ALSA: hda - rename
> dell_led_set_func to dell_micmute_led_set_func
> 9518725 Awaiting Upstream [v2,4/6] platform/x86: dell-laptop: import
> dell_micmute_led_set() from drivers/leds/dell-led.c
> 9518727 Awaiting Upstream [v2,5/6] dell-led: remove code related to mic mute LED
> 9518735 Awaiting Upstream [v2,6/6] dell-led: move driver to
> drivers/platform/x86/dell-wmi-led.c
> 9520053 Awaiting Upstream [v2,6+/6] platform/x86: dell-wmi-led: fix
> coding style issues
> 
> 2. Installed Ubuntu 16.10 (Linux kernel 4.8) on Dell Latitude 7180

Just to be sure, I assume you meant 7280?

> 
>   -> Microphone mute led works as expected
> 
> 3. Applied downloaded patches to kernel 4.8 without any conflicts, but
> kernel will not compile successfully.

I am confused.  This patch series should apply cleanly to 4.9 and its
surroundings.  Between 4.8 and 4.9, commit 4875a5f72180 ("ALSA: hda -
Fix a failure of micmute led when having multi adcs") happened.   This
makes it impossible to apply this patch series cleanly on top of 4.8
without reducing context to one line.

Moreover, I applied this patch series (with reduced context) to 4.8 and
successfully compiled the kernel.  Could you shed some light on where
exactly did your build crash?

> 
> 4. Installed Ubuntu Zesty kernel (Linux kernel 4.9)
> 
>   -> Microphone mute led works as expected
> 
> 5. Applied and compiled downloaded patches to kernel 4.9
> 
>   -> Microphone mute led does not work

Any chance of a git bisect?

> 
> 6. Compiled and installed Linux kernel 4.10 rc8
> 
>   -> Microphone mute led does not work

This is even more confusing.  Between 4.9, which you claim works fine,
and 4.10-rc8, which you claim does not work, I can see no changes to
either drivers/leds/dell-led.c or sound/pci/hda/dell_wmi_helper.c.
Kernel configuration issue?

> 
> 7. Applied and compiled downloaded patches to kernel 4.10 rc8
> 
>   -> Microphone mute led does not work

No surprises here if it really does not work with 4.10-rc8.

-- 
Best regards,
Michał Kępień

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v2 0/6] Move dell-led to drivers/platform/x86
  2017-02-15 13:54     ` Michał Kępień
@ 2017-02-15 14:31         ` Alex Hung
  0 siblings, 0 replies; 43+ messages in thread
From: Alex Hung @ 2017-02-15 14:31 UTC (permalink / raw)
  To: Michał Kępień
  Cc: platform-driver-x86, alsa-devel, linux-kernel, Anthony Wong,
	Takashi Iwai, Andy Shevchenko, Richard Purdie, Jacek Anaszewski,
	Pavel Machek, Pali Rohár, Darren Hart, Linux LED Subsystem

On Wed, Feb 15, 2017 at 9:54 PM, Michał Kępień <kernel@kempniu.pl> wrote:
> Alex,
>
>> I tested the patches with the following setup. Please note I tested
>> the microphone mute led by GUI, as the hotkey does not work on this
>> system.
>
> Thank you for taking the time to test this series, it is appreciated.  I
> would not expect the switching method (GUI/hotkey) to matter here, as
> long as we can compare behavior of two kernel versions by consistently
> using one of them.
>
>>
>> 1. Downloaded the below patches from patchwork
>>
>> 9518737 Awaiting Upstream [v2,1/6] dell-led: remove GUID check from
>> dell_micmute_led_set()
>> 9518761 Awaiting Upstream [v2,2/6] ALSA: hda - use
>> dell_micmute_led_set() instead of dell_app_wmi_led_set()
>> 9518741 Awaiting Upstream [v2,3/6] ALSA: hda - rename
>> dell_led_set_func to dell_micmute_led_set_func
>> 9518725 Awaiting Upstream [v2,4/6] platform/x86: dell-laptop: import
>> dell_micmute_led_set() from drivers/leds/dell-led.c
>> 9518727 Awaiting Upstream [v2,5/6] dell-led: remove code related to mic mute LED
>> 9518735 Awaiting Upstream [v2,6/6] dell-led: move driver to
>> drivers/platform/x86/dell-wmi-led.c
>> 9520053 Awaiting Upstream [v2,6+/6] platform/x86: dell-wmi-led: fix
>> coding style issues
>>
>> 2. Installed Ubuntu 16.10 (Linux kernel 4.8) on Dell Latitude 7180
>
> Just to be sure, I assume you meant 7280?

This is a typo, and it should be 7480, which should be the same as 7280.

>
>>
>>   -> Microphone mute led works as expected
>>
>> 3. Applied downloaded patches to kernel 4.8 without any conflicts, but
>> kernel will not compile successfully.
>
> I am confused.  This patch series should apply cleanly to 4.9 and its
> surroundings.  Between 4.8 and 4.9, commit 4875a5f72180 ("ALSA: hda -
> Fix a failure of micmute led when having multi adcs") happened.   This
> makes it impossible to apply this patch series cleanly on top of 4.8
> without reducing context to one line.
>
> Moreover, I applied this patch series (with reduced context) to 4.8 and
> successfully compiled the kernel.  Could you shed some light on where
> exactly did your build crash?

The error messages such as below, so it is probably not "fails to
compile". Since 4.9 works better, I probably will focus on 4.9 & 4.10,
though.

find /home/alexhung/src/kernel/ubuntu-yakkety/debian/build/build-generic/
-name \*.ko | \
    sed -e 's/.*\/\([^\/]*\)\.ko/\1/' | sort >
/home/alexhung/src/kernel/ubuntu-yakkety/debian.master/abi/4.8.0-37.39/amd64/generic.modules
II: Checking modules for generic...
   reading new modules...read 4821 modules.
   reading old modules...
      MISS: dell-led
      NEW : dell-wmi-led
      read 4821 modules : new(1)  missing(1)
EE: Missing modules (start begging for mercy)
debian/rules.d/4-checks.mk:12: recipe for target 'module-check-generic' failed
make: *** [module-check-generic] Error 1


>
>>
>> 4. Installed Ubuntu Zesty kernel (Linux kernel 4.9)
>>
>>   -> Microphone mute led works as expected
>>
>> 5. Applied and compiled downloaded patches to kernel 4.9
>>
>>   -> Microphone mute led does not work
>
> Any chance of a git bisect?

Certainly. Give me some time.

>
>>
>> 6. Compiled and installed Linux kernel 4.10 rc8
>>
>>   -> Microphone mute led does not work
>
> This is even more confusing.  Between 4.9, which you claim works fine,
> and 4.10-rc8, which you claim does not work, I can see no changes to
> either drivers/leds/dell-led.c or sound/pci/hda/dell_wmi_helper.c.
> Kernel configuration issue?

I was surprised too, and I compiled it twice and installed on both
Ubuntu 16.04 and 16.10.

I would guess it is config too. On 4.9, it was using Ubuntu default.
On 4.10rc8, I am pretty much using the instruction on
https://wiki.ubuntu.com/KernelTeam/GitKernelBuild. The .config is
available @ http://paste.ubuntu.com/24001113/


>
>>
>> 7. Applied and compiled downloaded patches to kernel 4.10 rc8
>>
>>   -> Microphone mute led does not work
>
> No surprises here if it really does not work with 4.10-rc8.
>
> --
> Best regards,
> Michał Kępień



-- 
Cheers,
Alex Hung
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v2 0/6] Move dell-led to drivers/platform/x86
@ 2017-02-15 14:31         ` Alex Hung
  0 siblings, 0 replies; 43+ messages in thread
From: Alex Hung @ 2017-02-15 14:31 UTC (permalink / raw)
  To: Michał Kępień
  Cc: Jaroslav Kysela, Takashi Iwai, Anthony Wong, Richard Purdie,
	Jacek Anaszewski, Pavel Machek, Pali Rohár, Darren Hart,
	Andy Shevchenko, Linux LED Subsystem, platform-driver-x86,
	alsa-devel, linux-kernel

On Wed, Feb 15, 2017 at 9:54 PM, Michał Kępień <kernel@kempniu.pl> wrote:
> Alex,
>
>> I tested the patches with the following setup. Please note I tested
>> the microphone mute led by GUI, as the hotkey does not work on this
>> system.
>
> Thank you for taking the time to test this series, it is appreciated.  I
> would not expect the switching method (GUI/hotkey) to matter here, as
> long as we can compare behavior of two kernel versions by consistently
> using one of them.
>
>>
>> 1. Downloaded the below patches from patchwork
>>
>> 9518737 Awaiting Upstream [v2,1/6] dell-led: remove GUID check from
>> dell_micmute_led_set()
>> 9518761 Awaiting Upstream [v2,2/6] ALSA: hda - use
>> dell_micmute_led_set() instead of dell_app_wmi_led_set()
>> 9518741 Awaiting Upstream [v2,3/6] ALSA: hda - rename
>> dell_led_set_func to dell_micmute_led_set_func
>> 9518725 Awaiting Upstream [v2,4/6] platform/x86: dell-laptop: import
>> dell_micmute_led_set() from drivers/leds/dell-led.c
>> 9518727 Awaiting Upstream [v2,5/6] dell-led: remove code related to mic mute LED
>> 9518735 Awaiting Upstream [v2,6/6] dell-led: move driver to
>> drivers/platform/x86/dell-wmi-led.c
>> 9520053 Awaiting Upstream [v2,6+/6] platform/x86: dell-wmi-led: fix
>> coding style issues
>>
>> 2. Installed Ubuntu 16.10 (Linux kernel 4.8) on Dell Latitude 7180
>
> Just to be sure, I assume you meant 7280?

This is a typo, and it should be 7480, which should be the same as 7280.

>
>>
>>   -> Microphone mute led works as expected
>>
>> 3. Applied downloaded patches to kernel 4.8 without any conflicts, but
>> kernel will not compile successfully.
>
> I am confused.  This patch series should apply cleanly to 4.9 and its
> surroundings.  Between 4.8 and 4.9, commit 4875a5f72180 ("ALSA: hda -
> Fix a failure of micmute led when having multi adcs") happened.   This
> makes it impossible to apply this patch series cleanly on top of 4.8
> without reducing context to one line.
>
> Moreover, I applied this patch series (with reduced context) to 4.8 and
> successfully compiled the kernel.  Could you shed some light on where
> exactly did your build crash?

The error messages such as below, so it is probably not "fails to
compile". Since 4.9 works better, I probably will focus on 4.9 & 4.10,
though.

find /home/alexhung/src/kernel/ubuntu-yakkety/debian/build/build-generic/
-name \*.ko | \
    sed -e 's/.*\/\([^\/]*\)\.ko/\1/' | sort >
/home/alexhung/src/kernel/ubuntu-yakkety/debian.master/abi/4.8.0-37.39/amd64/generic.modules
II: Checking modules for generic...
   reading new modules...read 4821 modules.
   reading old modules...
      MISS: dell-led
      NEW : dell-wmi-led
      read 4821 modules : new(1)  missing(1)
EE: Missing modules (start begging for mercy)
debian/rules.d/4-checks.mk:12: recipe for target 'module-check-generic' failed
make: *** [module-check-generic] Error 1


>
>>
>> 4. Installed Ubuntu Zesty kernel (Linux kernel 4.9)
>>
>>   -> Microphone mute led works as expected
>>
>> 5. Applied and compiled downloaded patches to kernel 4.9
>>
>>   -> Microphone mute led does not work
>
> Any chance of a git bisect?

Certainly. Give me some time.

>
>>
>> 6. Compiled and installed Linux kernel 4.10 rc8
>>
>>   -> Microphone mute led does not work
>
> This is even more confusing.  Between 4.9, which you claim works fine,
> and 4.10-rc8, which you claim does not work, I can see no changes to
> either drivers/leds/dell-led.c or sound/pci/hda/dell_wmi_helper.c.
> Kernel configuration issue?

I was surprised too, and I compiled it twice and installed on both
Ubuntu 16.04 and 16.10.

I would guess it is config too. On 4.9, it was using Ubuntu default.
On 4.10rc8, I am pretty much using the instruction on
https://wiki.ubuntu.com/KernelTeam/GitKernelBuild. The .config is
available @ http://paste.ubuntu.com/24001113/


>
>>
>> 7. Applied and compiled downloaded patches to kernel 4.10 rc8
>>
>>   -> Microphone mute led does not work
>
> No surprises here if it really does not work with 4.10-rc8.
>
> --
> Best regards,
> Michał Kępień



-- 
Cheers,
Alex Hung

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v2 0/6] Move dell-led to drivers/platform/x86
  2017-02-15 14:31         ` Alex Hung
  (?)
@ 2017-02-15 15:12         ` Pali Rohár
  2017-02-16  9:33             ` Michał Kępień
  -1 siblings, 1 reply; 43+ messages in thread
From: Pali Rohár @ 2017-02-15 15:12 UTC (permalink / raw)
  To: Alex Hung
  Cc: Michał Kępień,
	Jaroslav Kysela, Takashi Iwai, Anthony Wong, Richard Purdie,
	Jacek Anaszewski, Pavel Machek, Darren Hart, Andy Shevchenko,
	Linux LED Subsystem, platform-driver-x86, alsa-devel,
	linux-kernel

On Wednesday 15 February 2017 22:31:39 Alex Hung wrote:
> On Wed, Feb 15, 2017 at 9:54 PM, Michał Kępień <kernel@kempniu.pl> wrote:
> > Alex,
> >
> >> I tested the patches with the following setup. Please note I tested
> >> the microphone mute led by GUI, as the hotkey does not work on this
> >> system.
> >
> > Thank you for taking the time to test this series, it is appreciated.  I
> > would not expect the switching method (GUI/hotkey) to matter here, as
> > long as we can compare behavior of two kernel versions by consistently
> > using one of them.
> >
> >>
> >> 1. Downloaded the below patches from patchwork
> >>
> >> 9518737 Awaiting Upstream [v2,1/6] dell-led: remove GUID check from
> >> dell_micmute_led_set()
> >> 9518761 Awaiting Upstream [v2,2/6] ALSA: hda - use
> >> dell_micmute_led_set() instead of dell_app_wmi_led_set()
> >> 9518741 Awaiting Upstream [v2,3/6] ALSA: hda - rename
> >> dell_led_set_func to dell_micmute_led_set_func
> >> 9518725 Awaiting Upstream [v2,4/6] platform/x86: dell-laptop: import
> >> dell_micmute_led_set() from drivers/leds/dell-led.c
> >> 9518727 Awaiting Upstream [v2,5/6] dell-led: remove code related to mic mute LED
> >> 9518735 Awaiting Upstream [v2,6/6] dell-led: move driver to
> >> drivers/platform/x86/dell-wmi-led.c
> >> 9520053 Awaiting Upstream [v2,6+/6] platform/x86: dell-wmi-led: fix
> >> coding style issues
> >>
> >> 2. Installed Ubuntu 16.10 (Linux kernel 4.8) on Dell Latitude 7180
> >
> > Just to be sure, I assume you meant 7280?
> 
> This is a typo, and it should be 7480, which should be the same as 7280.
> 
> >
> >>
> >>   -> Microphone mute led works as expected
> >>
> >> 3. Applied downloaded patches to kernel 4.8 without any conflicts, but
> >> kernel will not compile successfully.
> >
> > I am confused.  This patch series should apply cleanly to 4.9 and its
> > surroundings.  Between 4.8 and 4.9, commit 4875a5f72180 ("ALSA: hda -
> > Fix a failure of micmute led when having multi adcs") happened.   This
> > makes it impossible to apply this patch series cleanly on top of 4.8
> > without reducing context to one line.
> >
> > Moreover, I applied this patch series (with reduced context) to 4.8 and
> > successfully compiled the kernel.  Could you shed some light on where
> > exactly did your build crash?
> 
> The error messages such as below, so it is probably not "fails to
> compile". Since 4.9 works better, I probably will focus on 4.9 & 4.10,
> though.
> 
> find /home/alexhung/src/kernel/ubuntu-yakkety/debian/build/build-generic/
> -name \*.ko | \
>     sed -e 's/.*\/\([^\/]*\)\.ko/\1/' | sort >
> /home/alexhung/src/kernel/ubuntu-yakkety/debian.master/abi/4.8.0-37.39/amd64/generic.modules
> II: Checking modules for generic...
>    reading new modules...read 4821 modules.
>    reading old modules...
>       MISS: dell-led
>       NEW : dell-wmi-led
>       read 4821 modules : new(1)  missing(1)
> EE: Missing modules (start begging for mercy)
> debian/rules.d/4-checks.mk:12: recipe for target 'module-check-generic' failed
> make: *** [module-check-generic] Error 1
> 
> 
> >
> >>
> >> 4. Installed Ubuntu Zesty kernel (Linux kernel 4.9)
> >>
> >>   -> Microphone mute led works as expected
> >>
> >> 5. Applied and compiled downloaded patches to kernel 4.9
> >>
> >>   -> Microphone mute led does not work
> >
> > Any chance of a git bisect?
> 
> Certainly. Give me some time.
> 
> >
> >>
> >> 6. Compiled and installed Linux kernel 4.10 rc8
> >>
> >>   -> Microphone mute led does not work
> >
> > This is even more confusing.  Between 4.9, which you claim works fine,
> > and 4.10-rc8, which you claim does not work, I can see no changes to
> > either drivers/leds/dell-led.c or sound/pci/hda/dell_wmi_helper.c.
> > Kernel configuration issue?
> 
> I was surprised too, and I compiled it twice and installed on both
> Ubuntu 16.04 and 16.10.
> 
> I would guess it is config too. On 4.9, it was using Ubuntu default.
> On 4.10rc8, I am pretty much using the instruction on
> https://wiki.ubuntu.com/KernelTeam/GitKernelBuild. The .config is
> available @ http://paste.ubuntu.com/24001113/

In Patch 6/6 is: Kconfig option is renamed from
CONFIG_LEDS_DELL_NETBOOKS to CONFIG_DELL_WMI_LED.
So you need to update your config for testing.

> 
> >
> >>
> >> 7. Applied and compiled downloaded patches to kernel 4.10 rc8
> >>
> >>   -> Microphone mute led does not work
> >
> > No surprises here if it really does not work with 4.10-rc8.
> >
> > --
> > Best regards,
> > Michał Kępień
> 
> 
> 

-- 
Pali Rohár
pali.rohar@gmail.com

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v2 0/6] Move dell-led to drivers/platform/x86
  2017-02-13 11:26 ` Michał Kępień
@ 2017-02-15 20:28     ` Takashi Iwai
  2017-02-15 20:28     ` Takashi Iwai
  1 sibling, 0 replies; 43+ messages in thread
From: Takashi Iwai @ 2017-02-15 20:28 UTC (permalink / raw)
  To: "Michał Kępień"
  Cc: alsa-devel, linux-kernel, Anthony Wong, Andy Shevchenko,
	Richard Purdie, Jacek Anaszewski, Pavel Machek, pali.rohar,
	Darren Hart, platform-driver-x86, linux-leds

On Mon, 13 Feb 2017 12:26:40 +0100,
Michał Kępień wrote:
> 
> Hi everyone,
> 
> It has been almost a month since I posted v2, so I hope sending a
> reminder is okay.
> 
> Jaroslav, Takashi, could you please ack this patch series from the sound
> subsystem perspective?  Patches 2-4 touch it.

Sorry to be late.  Yes, feel free to take my ack>

Acked-by: Takashi Iwai <tiwai@suse.de>


thanks,

Takashi
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v2 0/6] Move dell-led to drivers/platform/x86
@ 2017-02-15 20:28     ` Takashi Iwai
  0 siblings, 0 replies; 43+ messages in thread
From: Takashi Iwai @ 2017-02-15 20:28 UTC (permalink / raw)
  To: "Michał Kępień"
  Cc: Anthony Wong, Jaroslav Kysela, alsa-devel, Andy Shevchenko,
	Jacek Anaszewski, pali.rohar, Darren Hart, Richard Purdie,
	Pavel Machek, linux-kernel, linux-leds, platform-driver-x86

On Mon, 13 Feb 2017 12:26:40 +0100,
Michał Kępień wrote:
> 
> Hi everyone,
> 
> It has been almost a month since I posted v2, so I hope sending a
> reminder is okay.
> 
> Jaroslav, Takashi, could you please ack this patch series from the sound
> subsystem perspective?  Patches 2-4 touch it.

Sorry to be late.  Yes, feel free to take my ack>

Acked-by: Takashi Iwai <tiwai@suse.de>


thanks,

Takashi

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v2 0/6] Move dell-led to drivers/platform/x86
  2017-02-15 14:31         ` Alex Hung
  (?)
  (?)
@ 2017-02-16  9:27         ` Michał Kępień
  2017-02-16 10:35           ` Alex Hung
  -1 siblings, 1 reply; 43+ messages in thread
From: Michał Kępień @ 2017-02-16  9:27 UTC (permalink / raw)
  To: Alex Hung
  Cc: Jaroslav Kysela, Takashi Iwai, Anthony Wong, Richard Purdie,
	Jacek Anaszewski, Pavel Machek, Pali Rohár, Darren Hart,
	Andy Shevchenko, Linux LED Subsystem, platform-driver-x86,
	alsa-devel, linux-kernel

> On Wed, Feb 15, 2017 at 9:54 PM, Michał Kępień <kernel@kempniu.pl> wrote:
> > Alex,
> >
> >> I tested the patches with the following setup. Please note I tested
> >> the microphone mute led by GUI, as the hotkey does not work on this
> >> system.
> >
> > Thank you for taking the time to test this series, it is appreciated.  I
> > would not expect the switching method (GUI/hotkey) to matter here, as
> > long as we can compare behavior of two kernel versions by consistently
> > using one of them.
> >
> >>
> >> 1. Downloaded the below patches from patchwork
> >>
> >> 9518737 Awaiting Upstream [v2,1/6] dell-led: remove GUID check from
> >> dell_micmute_led_set()
> >> 9518761 Awaiting Upstream [v2,2/6] ALSA: hda - use
> >> dell_micmute_led_set() instead of dell_app_wmi_led_set()
> >> 9518741 Awaiting Upstream [v2,3/6] ALSA: hda - rename
> >> dell_led_set_func to dell_micmute_led_set_func
> >> 9518725 Awaiting Upstream [v2,4/6] platform/x86: dell-laptop: import
> >> dell_micmute_led_set() from drivers/leds/dell-led.c
> >> 9518727 Awaiting Upstream [v2,5/6] dell-led: remove code related to mic mute LED
> >> 9518735 Awaiting Upstream [v2,6/6] dell-led: move driver to
> >> drivers/platform/x86/dell-wmi-led.c
> >> 9520053 Awaiting Upstream [v2,6+/6] platform/x86: dell-wmi-led: fix
> >> coding style issues
> >>
> >> 2. Installed Ubuntu 16.10 (Linux kernel 4.8) on Dell Latitude 7180
> >
> > Just to be sure, I assume you meant 7280?
> 
> This is a typo, and it should be 7480, which should be the same as 7280.
> 
> >
> >>
> >>   -> Microphone mute led works as expected
> >>
> >> 3. Applied downloaded patches to kernel 4.8 without any conflicts, but
> >> kernel will not compile successfully.
> >
> > I am confused.  This patch series should apply cleanly to 4.9 and its
> > surroundings.  Between 4.8 and 4.9, commit 4875a5f72180 ("ALSA: hda -
> > Fix a failure of micmute led when having multi adcs") happened.   This
> > makes it impossible to apply this patch series cleanly on top of 4.8
> > without reducing context to one line.
> >
> > Moreover, I applied this patch series (with reduced context) to 4.8 and
> > successfully compiled the kernel.  Could you shed some light on where
> > exactly did your build crash?
> 
> The error messages such as below, so it is probably not "fails to
> compile". Since 4.9 works better, I probably will focus on 4.9 & 4.10,
> though.

I agree as I see no reason for this patch series to be backported.

> 
> find /home/alexhung/src/kernel/ubuntu-yakkety/debian/build/build-generic/
> -name \*.ko | \
>     sed -e 's/.*\/\([^\/]*\)\.ko/\1/' | sort >
> /home/alexhung/src/kernel/ubuntu-yakkety/debian.master/abi/4.8.0-37.39/amd64/generic.modules
> II: Checking modules for generic...
>    reading new modules...read 4821 modules.
>    reading old modules...
>       MISS: dell-led
>       NEW : dell-wmi-led
>       read 4821 modules : new(1)  missing(1)
> EE: Missing modules (start begging for mercy)
> debian/rules.d/4-checks.mk:12: recipe for target 'module-check-generic' failed
> make: *** [module-check-generic] Error 1

Ah, okay, so you are using the Debian/Ubuntu build method.  The messages
it spits out are correct as the purpose of this series is to remove
dell-led and create dell-wmi-led.  However, I have never compiled a
kernel using the Debian/Ubuntu way, so I am unable to give you any tips
for getting rid of the "Missing modules" error message.

> 
> 
> >
> >>
> >> 4. Installed Ubuntu Zesty kernel (Linux kernel 4.9)
> >>
> >>   -> Microphone mute led works as expected
> >>
> >> 5. Applied and compiled downloaded patches to kernel 4.9
> >>
> >>   -> Microphone mute led does not work
> >
> > Any chance of a git bisect?
> 
> Certainly. Give me some time.

Sure, no rush, though before you start bisecting, please check whether
CONFIG_DELL_LAPTOP is enabled in your kernel configuration for 4.9.

> 
> >
> >>
> >> 6. Compiled and installed Linux kernel 4.10 rc8
> >>
> >>   -> Microphone mute led does not work
> >
> > This is even more confusing.  Between 4.9, which you claim works fine,
> > and 4.10-rc8, which you claim does not work, I can see no changes to
> > either drivers/leds/dell-led.c or sound/pci/hda/dell_wmi_helper.c.
> > Kernel configuration issue?
> 
> I was surprised too, and I compiled it twice and installed on both
> Ubuntu 16.04 and 16.10.
> 
> I would guess it is config too. On 4.9, it was using Ubuntu default.
> On 4.10rc8, I am pretty much using the instruction on
> https://wiki.ubuntu.com/KernelTeam/GitKernelBuild. The .config is
> available @ http://paste.ubuntu.com/24001113/

CONFIG_DELL_SMBIOS is not set in that .config.  That prevents you from
enabling CONFIG_LEDS_DELL_NETBOOKS, which is required for the microphone
mute LED to work without this patch series applied.

> 
> 
> >
> >>
> >> 7. Applied and compiled downloaded patches to kernel 4.10 rc8
> >>
> >>   -> Microphone mute led does not work
> >
> > No surprises here if it really does not work with 4.10-rc8.

My remark regarding CONFIG_DELL_LAPTOP also applies to 4.10-rc8 with
this patch series applied.

Thanks again for your work on this,

-- 
Best regards,
Michał Kępień

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v2 0/6] Move dell-led to drivers/platform/x86
  2017-02-15 15:12         ` Pali Rohár
@ 2017-02-16  9:33             ` Michał Kępień
  0 siblings, 0 replies; 43+ messages in thread
From: Michał Kępień @ 2017-02-16  9:33 UTC (permalink / raw)
  To: Pali Rohár
  Cc: Alex Hung, Jaroslav Kysela, Takashi Iwai, Anthony Wong,
	Richard Purdie, Jacek Anaszewski, Pavel Machek, Darren Hart,
	Andy Shevchenko, Linux LED Subsystem, platform-driver-x86,
	alsa-devel, linux-kernel

> > >> 6. Compiled and installed Linux kernel 4.10 rc8
> > >>
> > >>   -> Microphone mute led does not work
> > >
> > > This is even more confusing.  Between 4.9, which you claim works fine,
> > > and 4.10-rc8, which you claim does not work, I can see no changes to
> > > either drivers/leds/dell-led.c or sound/pci/hda/dell_wmi_helper.c.
> > > Kernel configuration issue?
> > 
> > I was surprised too, and I compiled it twice and installed on both
> > Ubuntu 16.04 and 16.10.
> > 
> > I would guess it is config too. On 4.9, it was using Ubuntu default.
> > On 4.10rc8, I am pretty much using the instruction on
> > https://wiki.ubuntu.com/KernelTeam/GitKernelBuild. The .config is
> > available @ http://paste.ubuntu.com/24001113/
> 
> In Patch 6/6 is: Kconfig option is renamed from
> CONFIG_LEDS_DELL_NETBOOKS to CONFIG_DELL_WMI_LED.
> So you need to update your config for testing.

I also thought about this, but CONFIG_DELL_WMI_LED is only needed to
support the Dell Activity LED (present in the Dell Latitude 2100
netbook).  Meanwhile, Alex is only testing the microphone mute LED,
which requires CONFIG_DELL_LAPTOP to be enabled.  The cover letter for
this patch series provides a more thorough explanation.

-- 
Best regards,
Michał Kępień

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v2 0/6] Move dell-led to drivers/platform/x86
@ 2017-02-16  9:33             ` Michał Kępień
  0 siblings, 0 replies; 43+ messages in thread
From: Michał Kępień @ 2017-02-16  9:33 UTC (permalink / raw)
  To: Pali Rohár
  Cc: Alex Hung, Jaroslav Kysela, Takashi Iwai, Anthony Wong,
	Richard Purdie, Jacek Anaszewski, Pavel Machek, Darren Hart,
	Andy Shevchenko, Linux LED Subsystem, platform-driver-x86,
	alsa-devel, linux-kernel

> > >> 6. Compiled and installed Linux kernel 4.10 rc8
> > >>
> > >>   -> Microphone mute led does not work
> > >
> > > This is even more confusing.  Between 4.9, which you claim works fine,
> > > and 4.10-rc8, which you claim does not work, I can see no changes to
> > > either drivers/leds/dell-led.c or sound/pci/hda/dell_wmi_helper.c.
> > > Kernel configuration issue?
> > 
> > I was surprised too, and I compiled it twice and installed on both
> > Ubuntu 16.04 and 16.10.
> > 
> > I would guess it is config too. On 4.9, it was using Ubuntu default.
> > On 4.10rc8, I am pretty much using the instruction on
> > https://wiki.ubuntu.com/KernelTeam/GitKernelBuild. The .config is
> > available @ http://paste.ubuntu.com/24001113/
> 
> In Patch 6/6 is: Kconfig option is renamed from
> CONFIG_LEDS_DELL_NETBOOKS to CONFIG_DELL_WMI_LED.
> So you need to update your config for testing.

I also thought about this, but CONFIG_DELL_WMI_LED is only needed to
support the Dell Activity LED (present in the Dell Latitude 2100
netbook).  Meanwhile, Alex is only testing the microphone mute LED,
which requires CONFIG_DELL_LAPTOP to be enabled.  The cover letter for
this patch series provides a more thorough explanation.

-- 
Best regards,
Michał Kępień

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v2 0/6] Move dell-led to drivers/platform/x86
  2017-02-16  9:27         ` Michał Kępień
@ 2017-02-16 10:35           ` Alex Hung
  2017-02-16 11:32             ` Michał Kępień
  0 siblings, 1 reply; 43+ messages in thread
From: Alex Hung @ 2017-02-16 10:35 UTC (permalink / raw)
  To: Michał Kępień
  Cc: Jaroslav Kysela, Takashi Iwai, Anthony Wong, Richard Purdie,
	Jacek Anaszewski, Pavel Machek, Pali Rohár, Darren Hart,
	Andy Shevchenko, Linux LED Subsystem, platform-driver-x86,
	alsa-devel, linux-kernel

On Thu, Feb 16, 2017 at 5:27 PM, Michał Kępień <kernel@kempniu.pl> wrote:
>> On Wed, Feb 15, 2017 at 9:54 PM, Michał Kępień <kernel@kempniu.pl> wrote:
>> > Alex,
>> >
>> >> I tested the patches with the following setup. Please note I tested
>> >> the microphone mute led by GUI, as the hotkey does not work on this
>> >> system.
>> >
>> > Thank you for taking the time to test this series, it is appreciated.  I
>> > would not expect the switching method (GUI/hotkey) to matter here, as
>> > long as we can compare behavior of two kernel versions by consistently
>> > using one of them.
>> >
>> >>
>> >> 1. Downloaded the below patches from patchwork
>> >>
>> >> 9518737 Awaiting Upstream [v2,1/6] dell-led: remove GUID check from
>> >> dell_micmute_led_set()
>> >> 9518761 Awaiting Upstream [v2,2/6] ALSA: hda - use
>> >> dell_micmute_led_set() instead of dell_app_wmi_led_set()
>> >> 9518741 Awaiting Upstream [v2,3/6] ALSA: hda - rename
>> >> dell_led_set_func to dell_micmute_led_set_func
>> >> 9518725 Awaiting Upstream [v2,4/6] platform/x86: dell-laptop: import
>> >> dell_micmute_led_set() from drivers/leds/dell-led.c
>> >> 9518727 Awaiting Upstream [v2,5/6] dell-led: remove code related to mic mute LED
>> >> 9518735 Awaiting Upstream [v2,6/6] dell-led: move driver to
>> >> drivers/platform/x86/dell-wmi-led.c
>> >> 9520053 Awaiting Upstream [v2,6+/6] platform/x86: dell-wmi-led: fix
>> >> coding style issues
>> >>
>> >> 2. Installed Ubuntu 16.10 (Linux kernel 4.8) on Dell Latitude 7180
>> >
>> > Just to be sure, I assume you meant 7280?
>>
>> This is a typo, and it should be 7480, which should be the same as 7280.
>>
>> >
>> >>
>> >>   -> Microphone mute led works as expected
>> >>
>> >> 3. Applied downloaded patches to kernel 4.8 without any conflicts, but
>> >> kernel will not compile successfully.
>> >
>> > I am confused.  This patch series should apply cleanly to 4.9 and its
>> > surroundings.  Between 4.8 and 4.9, commit 4875a5f72180 ("ALSA: hda -
>> > Fix a failure of micmute led when having multi adcs") happened.   This
>> > makes it impossible to apply this patch series cleanly on top of 4.8
>> > without reducing context to one line.
>> >
>> > Moreover, I applied this patch series (with reduced context) to 4.8 and
>> > successfully compiled the kernel.  Could you shed some light on where
>> > exactly did your build crash?
>>
>> The error messages such as below, so it is probably not "fails to
>> compile". Since 4.9 works better, I probably will focus on 4.9 & 4.10,
>> though.
>
> I agree as I see no reason for this patch series to be backported.
>
>>
>> find /home/alexhung/src/kernel/ubuntu-yakkety/debian/build/build-generic/
>> -name \*.ko | \
>>     sed -e 's/.*\/\([^\/]*\)\.ko/\1/' | sort >
>> /home/alexhung/src/kernel/ubuntu-yakkety/debian.master/abi/4.8.0-37.39/amd64/generic.modules
>> II: Checking modules for generic...
>>    reading new modules...read 4821 modules.
>>    reading old modules...
>>       MISS: dell-led
>>       NEW : dell-wmi-led
>>       read 4821 modules : new(1)  missing(1)
>> EE: Missing modules (start begging for mercy)
>> debian/rules.d/4-checks.mk:12: recipe for target 'module-check-generic' failed
>> make: *** [module-check-generic] Error 1
>
> Ah, okay, so you are using the Debian/Ubuntu build method.  The messages
> it spits out are correct as the purpose of this series is to remove
> dell-led and create dell-wmi-led.  However, I have never compiled a
> kernel using the Debian/Ubuntu way, so I am unable to give you any tips
> for getting rid of the "Missing modules" error message.
>
>>
>>
>> >
>> >>
>> >> 4. Installed Ubuntu Zesty kernel (Linux kernel 4.9)
>> >>
>> >>   -> Microphone mute led works as expected
>> >>
>> >> 5. Applied and compiled downloaded patches to kernel 4.9
>> >>
>> >>   -> Microphone mute led does not work
>> >
>> > Any chance of a git bisect?
>>
>> Certainly. Give me some time.
>
> Sure, no rush, though before you start bisecting, please check whether
> CONFIG_DELL_LAPTOP is enabled in your kernel configuration for 4.9.

The led stops working after 4th patch "platform/x86: dell-laptop:
import dell_micmute_led_set() from drivers/leds/dell-led.c", and the
.config is with CONFIG_DELL_LAPTOP=m

However, I found dell-laptop is not loaded on Latitude 7480 as its
chassis type is Notebook (10), instead of 8 (Portable) or 9 (Laptop).
That's also what I see for newer Dell laptops.

After I added the below entry into dell-laptop, the led works fine.
This is not in kernel 4.10 rc8 either, and I will submit a patch for
it.

        {
                .matches = {
                        DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
                        DMI_MATCH(DMI_CHASSIS_TYPE, "10"), /*Notebook*/
                },
        },


>
>>
>> >
>> >>
>> >> 6. Compiled and installed Linux kernel 4.10 rc8
>> >>
>> >>   -> Microphone mute led does not work
>> >
>> > This is even more confusing.  Between 4.9, which you claim works fine,
>> > and 4.10-rc8, which you claim does not work, I can see no changes to
>> > either drivers/leds/dell-led.c or sound/pci/hda/dell_wmi_helper.c.
>> > Kernel configuration issue?
>>
>> I was surprised too, and I compiled it twice and installed on both
>> Ubuntu 16.04 and 16.10.
>>
>> I would guess it is config too. On 4.9, it was using Ubuntu default.
>> On 4.10rc8, I am pretty much using the instruction on
>> https://wiki.ubuntu.com/KernelTeam/GitKernelBuild. The .config is
>> available @ http://paste.ubuntu.com/24001113/
>
> CONFIG_DELL_SMBIOS is not set in that .config.  That prevents you from
> enabling CONFIG_LEDS_DELL_NETBOOKS, which is required for the microphone
> mute LED to work without this patch series applied.
>
>>
>>
>> >
>> >>
>> >> 7. Applied and compiled downloaded patches to kernel 4.10 rc8
>> >>
>> >>   -> Microphone mute led does not work
>> >
>> > No surprises here if it really does not work with 4.10-rc8.
>
> My remark regarding CONFIG_DELL_LAPTOP also applies to 4.10-rc8 with
> this patch series applied.
>
> Thanks again for your work on this,
>
> --
> Best regards,
> Michał Kępień



-- 
Cheers,
Alex Hung

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v2 0/6] Move dell-led to drivers/platform/x86
  2017-02-16 10:35           ` Alex Hung
@ 2017-02-16 11:32             ` Michał Kępień
  2017-02-16 11:38               ` Alex Hung
                                 ` (2 more replies)
  0 siblings, 3 replies; 43+ messages in thread
From: Michał Kępień @ 2017-02-16 11:32 UTC (permalink / raw)
  To: Alex Hung, Jacek Anaszewski, Pavel Machek
  Cc: Jaroslav Kysela, Takashi Iwai, Anthony Wong, Richard Purdie,
	Pali Rohár, Darren Hart, Andy Shevchenko,
	Linux LED Subsystem, platform-driver-x86, alsa-devel,
	linux-kernel

> The led stops working after 4th patch "platform/x86: dell-laptop:
> import dell_micmute_led_set() from drivers/leds/dell-led.c", and the
> .config is with CONFIG_DELL_LAPTOP=m
> 
> However, I found dell-laptop is not loaded on Latitude 7480 as its
> chassis type is Notebook (10), instead of 8 (Portable) or 9 (Laptop).
> That's also what I see for newer Dell laptops.
> 
> After I added the below entry into dell-laptop, the led works fine.
> This is not in kernel 4.10 rc8 either, and I will submit a patch for
> it.
> 
>         {
>                 .matches = {
>                         DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
>                         DMI_MATCH(DMI_CHASSIS_TYPE, "10"), /*Notebook*/
>                 },
>         },

Great, so it all makes sense now.  Thanks once again for investigating.

For maintainers' convenience, I am planning to submit a v3 with all the
tags supplied by everyone who reviewed this series and also some minor
coding style tweaks suggested along the way.

Alex, in the light of your findings quoted above, would it be okay to
also apply your Tested-by to all patches?

Jacek, Pavel, which linux-leds branch would you like me to base v3 on?

-- 
Best regards,
Michał Kępień

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v2 0/6] Move dell-led to drivers/platform/x86
  2017-02-16 11:32             ` Michał Kępień
@ 2017-02-16 11:38               ` Alex Hung
  2017-02-16 11:41               ` Andy Shevchenko
  2017-02-16 22:11               ` Jacek Anaszewski
  2 siblings, 0 replies; 43+ messages in thread
From: Alex Hung @ 2017-02-16 11:38 UTC (permalink / raw)
  To: Michał Kępień
  Cc: Jacek Anaszewski, Pavel Machek, Jaroslav Kysela, Takashi Iwai,
	Anthony Wong, Richard Purdie, Pali Rohár, Darren Hart,
	Andy Shevchenko, Linux LED Subsystem, platform-driver-x86,
	alsa-devel, linux-kernel

On Thu, Feb 16, 2017 at 7:32 PM, Michał Kępień <kernel@kempniu.pl> wrote:
>> The led stops working after 4th patch "platform/x86: dell-laptop:
>> import dell_micmute_led_set() from drivers/leds/dell-led.c", and the
>> .config is with CONFIG_DELL_LAPTOP=m
>>
>> However, I found dell-laptop is not loaded on Latitude 7480 as its
>> chassis type is Notebook (10), instead of 8 (Portable) or 9 (Laptop).
>> That's also what I see for newer Dell laptops.
>>
>> After I added the below entry into dell-laptop, the led works fine.
>> This is not in kernel 4.10 rc8 either, and I will submit a patch for
>> it.
>>
>>         {
>>                 .matches = {
>>                         DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
>>                         DMI_MATCH(DMI_CHASSIS_TYPE, "10"), /*Notebook*/
>>                 },
>>         },
>
> Great, so it all makes sense now.  Thanks once again for investigating.
>
> For maintainers' convenience, I am planning to submit a v3 with all the
> tags supplied by everyone who reviewed this series and also some minor
> coding style tweaks suggested along the way.
>
> Alex, in the light of your findings quoted above, would it be okay to
> also apply your Tested-by to all patches?

Certainly.

Tested-by: Alex Hung <alex.hung@canonical.com>

>
> Jacek, Pavel, which linux-leds branch would you like me to base v3 on?
>
> --
> Best regards,
> Michał Kępień



-- 
Cheers,
Alex Hung

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v2 0/6] Move dell-led to drivers/platform/x86
  2017-02-16 11:32             ` Michał Kępień
  2017-02-16 11:38               ` Alex Hung
@ 2017-02-16 11:41               ` Andy Shevchenko
  2017-02-16 12:01                 ` Michał Kępień
  2017-02-16 22:11               ` Jacek Anaszewski
  2 siblings, 1 reply; 43+ messages in thread
From: Andy Shevchenko @ 2017-02-16 11:41 UTC (permalink / raw)
  To: Michał Kępień
  Cc: Alex Hung, Jacek Anaszewski, Pavel Machek, Jaroslav Kysela,
	Takashi Iwai, Anthony Wong, Richard Purdie, Pali Rohár,
	Darren Hart, Linux LED Subsystem, platform-driver-x86,
	ALSA Development Mailing List, linux-kernel

On Thu, Feb 16, 2017 at 1:32 PM, Michał Kępień <kernel@kempniu.pl> wrote:

> For maintainers' convenience, I am planning to submit a v3 with all the
> tags supplied by everyone who reviewed this series and also some minor
> coding style tweaks suggested along the way.

Thanks!
I'm a bit busy right now, but I definitely will apply the series this week.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v2 0/6] Move dell-led to drivers/platform/x86
  2017-02-16 11:41               ` Andy Shevchenko
@ 2017-02-16 12:01                 ` Michał Kępień
  2017-02-16 13:11                     ` Andy Shevchenko
  0 siblings, 1 reply; 43+ messages in thread
From: Michał Kępień @ 2017-02-16 12:01 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Alex Hung, Jacek Anaszewski, Pavel Machek, Jaroslav Kysela,
	Takashi Iwai, Anthony Wong, Richard Purdie, Pali Rohár,
	Darren Hart, Linux LED Subsystem, platform-driver-x86,
	ALSA Development Mailing List, linux-kernel

> On Thu, Feb 16, 2017 at 1:32 PM, Michał Kępień <kernel@kempniu.pl> wrote:
> 
> > For maintainers' convenience, I am planning to submit a v3 with all the
> > tags supplied by everyone who reviewed this series and also some minor
> > coding style tweaks suggested along the way.
> 
> Thanks!
> I'm a bit busy right now, but I definitely will apply the series this week.

Not sure if I understand correctly, but this series was supposed to go
through the LED subsystem.  The cover letter said:

> As all patches except patch 3 in this series affect the LED subsystem,
> the series is based on linux-leds/for-4.11.

See also:

    https://www.spinics.net/lists/platform-driver-x86/msg10235.html
    https://www.spinics.net/lists/platform-driver-x86/msg10237.html
    https://www.spinics.net/lists/platform-driver-x86/msg10250.html
    https://www.spinics.net/lists/platform-driver-x86/msg10253.html

-- 
Best regards,
Michał Kępień

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v2 0/6] Move dell-led to drivers/platform/x86
  2017-02-16 12:01                 ` Michał Kępień
@ 2017-02-16 13:11                     ` Andy Shevchenko
  0 siblings, 0 replies; 43+ messages in thread
From: Andy Shevchenko @ 2017-02-16 13:11 UTC (permalink / raw)
  To: Michał Kępień
  Cc: platform-driver-x86, ALSA Development Mailing List, linux-kernel,
	Anthony Wong, Takashi Iwai, Alex Hung, Richard Purdie,
	Jacek Anaszewski, Pavel Machek, Pali Rohár, Darren Hart,
	Linux LED Subsystem

On Thu, Feb 16, 2017 at 2:01 PM, Michał Kępień <kernel@kempniu.pl> wrote:
>> On Thu, Feb 16, 2017 at 1:32 PM, Michał Kępień <kernel@kempniu.pl> wrote:
>>
>> > For maintainers' convenience, I am planning to submit a v3 with all the
>> > tags supplied by everyone who reviewed this series and also some minor
>> > coding style tweaks suggested along the way.
>>
>> Thanks!
>> I'm a bit busy right now, but I definitely will apply the series this week.
>
> Not sure if I understand correctly, but this series was supposed to go
> through the LED subsystem.  The cover letter said:

Ah, that's cool!

Seems I mixed up this one and the one where Thinkpad is involved.

>> As all patches except patch 3 in this series affect the LED subsystem,
>> the series is based on linux-leds/for-4.11.
>
> See also:
>
>     https://www.spinics.net/lists/platform-driver-x86/msg10235.html
>     https://www.spinics.net/lists/platform-driver-x86/msg10237.html
>     https://www.spinics.net/lists/platform-driver-x86/msg10250.html
>     https://www.spinics.net/lists/platform-driver-x86/msg10253.html
>
> --
> Best regards,
> Michał Kępień



-- 
With Best Regards,
Andy Shevchenko
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v2 0/6] Move dell-led to drivers/platform/x86
@ 2017-02-16 13:11                     ` Andy Shevchenko
  0 siblings, 0 replies; 43+ messages in thread
From: Andy Shevchenko @ 2017-02-16 13:11 UTC (permalink / raw)
  To: Michał Kępień
  Cc: Alex Hung, Jacek Anaszewski, Pavel Machek, Jaroslav Kysela,
	Takashi Iwai, Anthony Wong, Richard Purdie, Pali Rohár,
	Darren Hart, Linux LED Subsystem, platform-driver-x86,
	ALSA Development Mailing List, linux-kernel

On Thu, Feb 16, 2017 at 2:01 PM, Michał Kępień <kernel@kempniu.pl> wrote:
>> On Thu, Feb 16, 2017 at 1:32 PM, Michał Kępień <kernel@kempniu.pl> wrote:
>>
>> > For maintainers' convenience, I am planning to submit a v3 with all the
>> > tags supplied by everyone who reviewed this series and also some minor
>> > coding style tweaks suggested along the way.
>>
>> Thanks!
>> I'm a bit busy right now, but I definitely will apply the series this week.
>
> Not sure if I understand correctly, but this series was supposed to go
> through the LED subsystem.  The cover letter said:

Ah, that's cool!

Seems I mixed up this one and the one where Thinkpad is involved.

>> As all patches except patch 3 in this series affect the LED subsystem,
>> the series is based on linux-leds/for-4.11.
>
> See also:
>
>     https://www.spinics.net/lists/platform-driver-x86/msg10235.html
>     https://www.spinics.net/lists/platform-driver-x86/msg10237.html
>     https://www.spinics.net/lists/platform-driver-x86/msg10250.html
>     https://www.spinics.net/lists/platform-driver-x86/msg10253.html
>
> --
> Best regards,
> Michał Kępień



-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v2 0/6] Move dell-led to drivers/platform/x86
  2017-02-16 11:32             ` Michał Kępień
  2017-02-16 11:38               ` Alex Hung
  2017-02-16 11:41               ` Andy Shevchenko
@ 2017-02-16 22:11               ` Jacek Anaszewski
  2 siblings, 0 replies; 43+ messages in thread
From: Jacek Anaszewski @ 2017-02-16 22:11 UTC (permalink / raw)
  To: Michał Kępień, Alex Hung, Pavel Machek
  Cc: Jaroslav Kysela, Takashi Iwai, Anthony Wong, Richard Purdie,
	Pali Rohár, Darren Hart, Andy Shevchenko,
	Linux LED Subsystem, platform-driver-x86, alsa-devel,
	linux-kernel

Hi Michał,

On 02/16/2017 12:32 PM, Michał Kępień wrote:
>> The led stops working after 4th patch "platform/x86: dell-laptop:
>> import dell_micmute_led_set() from drivers/leds/dell-led.c", and the
>> .config is with CONFIG_DELL_LAPTOP=m
>>
>> However, I found dell-laptop is not loaded on Latitude 7480 as its
>> chassis type is Notebook (10), instead of 8 (Portable) or 9 (Laptop).
>> That's also what I see for newer Dell laptops.
>>
>> After I added the below entry into dell-laptop, the led works fine.
>> This is not in kernel 4.10 rc8 either, and I will submit a patch for
>> it.
>>
>>         {
>>                 .matches = {
>>                         DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
>>                         DMI_MATCH(DMI_CHASSIS_TYPE, "10"), /*Notebook*/
>>                 },
>>         },
> 
> Great, so it all makes sense now.  Thanks once again for investigating.
> 
> For maintainers' convenience, I am planning to submit a v3 with all the
> tags supplied by everyone who reviewed this series and also some minor
> coding style tweaks suggested along the way.
> 
> Alex, in the light of your findings quoted above, would it be okay to
> also apply your Tested-by to all patches?
> 
> Jacek, Pavel, which linux-leds branch would you like me to base v3 on?

Please use for-next branch for that.

-- 
Best regards,
Jacek Anaszewski

^ permalink raw reply	[flat|nested] 43+ messages in thread

end of thread, other threads:[~2017-02-16 22:11 UTC | newest]

Thread overview: 43+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-16 13:21 [PATCH v2 0/6] Move dell-led to drivers/platform/x86 Michał Kępień
2017-01-16 13:21 ` [PATCH v2 1/6] dell-led: remove GUID check from dell_micmute_led_set() Michał Kępień
2017-01-16 13:22 ` [PATCH v2 2/6] ALSA: hda - use dell_micmute_led_set() instead of dell_app_wmi_led_set() Michał Kępień
2017-01-16 13:22 ` [PATCH v2 3/6] ALSA: hda - rename dell_led_set_func to dell_micmute_led_set_func Michał Kępień
2017-01-17 11:12   ` Pali Rohár
2017-01-17 21:20   ` Jacek Anaszewski
2017-01-16 13:22 ` [PATCH v2 4/6] platform/x86: dell-laptop: import dell_micmute_led_set() from drivers/leds/dell-led.c Michał Kępień
2017-01-17 11:23   ` Pali Rohár
2017-01-18 19:12   ` Andy Shevchenko
2017-01-16 13:22 ` [PATCH v2 5/6] dell-led: remove code related to mic mute LED Michał Kępień
2017-01-17 11:24   ` Pali Rohár
2017-01-16 13:22 ` [PATCH v2 6/6] dell-led: move driver to drivers/platform/x86/dell-wmi-led.c Michał Kępień
2017-01-16 20:49   ` Jacek Anaszewski
2017-01-17 11:08     ` Pavel Machek
2017-01-17 11:28       ` Pali Rohár
2017-01-18 19:08   ` Andy Shevchenko
2017-01-17  7:17 ` [PATCH v2 6+/6] platform/x86: dell-wmi-led: fix coding style issues Michał Kępień
2017-01-17  8:21   ` Joe Perches
2017-01-17  9:19     ` Michał Kępień
2017-01-17 21:20       ` Jacek Anaszewski
2017-01-17 11:08   ` Pavel Machek
2017-01-18 19:06   ` Andy Shevchenko
2017-01-17 21:20 ` [PATCH v2 0/6] Move dell-led to drivers/platform/x86 Jacek Anaszewski
2017-02-13 11:26 ` Michał Kępień
2017-02-15 11:56   ` Alex Hung
2017-02-15 11:56     ` Alex Hung
2017-02-15 13:54     ` Michał Kępień
2017-02-15 14:31       ` Alex Hung
2017-02-15 14:31         ` Alex Hung
2017-02-15 15:12         ` Pali Rohár
2017-02-16  9:33           ` Michał Kępień
2017-02-16  9:33             ` Michał Kępień
2017-02-16  9:27         ` Michał Kępień
2017-02-16 10:35           ` Alex Hung
2017-02-16 11:32             ` Michał Kępień
2017-02-16 11:38               ` Alex Hung
2017-02-16 11:41               ` Andy Shevchenko
2017-02-16 12:01                 ` Michał Kępień
2017-02-16 13:11                   ` Andy Shevchenko
2017-02-16 13:11                     ` Andy Shevchenko
2017-02-16 22:11               ` Jacek Anaszewski
2017-02-15 20:28   ` Takashi Iwai
2017-02-15 20:28     ` Takashi Iwai

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.