[PATCH v3 1/5] leds: core: Add support for poll()ing the sysfs brightness attr for changes.

[PATCH v3 1/5] leds: core: Add support for poll()ing the sysfs brightness attr for changes.

[Hans de Goede]

Add support for userspace using poll() for POLL_PRI on the sysfs brightness
attr to watch for brightness changes.

This commit adds a led_notify_brightness_change helper function for
waking up any poll() waiters; and calls this after any brightness changes
(after any led_classdev->brightness_set[_blocking] calls have completed).

In some use-cases led hardware may autonomously change its brightness,
e.g. the keyboard backlight used on some laptops is controlled by a
hardwired (firmware handled) hotkey. led_notify_brightness_change is
exported for use by drivers which can detect such autonomous changes.

This commit also updates the Documentation/ABI/testing/sysfs-class-led
documentation to document that userspace may now poll on the brightness
attribute.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
-Wakeup / notify userspace on any brightness changes, not just on
 autonomous changes done by the hw
Changes in v3:
-Rebase on linux-leds/for-next
---
 Documentation/ABI/testing/sysfs-class-led |  8 ++++++--
 drivers/leds/led-class.c                  |  9 +++++++++
 drivers/leds/led-core.c                   | 16 +++++++++++++++-
 include/linux/leds.h                      | 12 ++++++++++++
 4 files changed, 42 insertions(+), 3 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-class-led b/Documentation/ABI/testing/sysfs-class-led
index 491cdee..0af5191 100644
--- a/Documentation/ABI/testing/sysfs-class-led
+++ b/Documentation/ABI/testing/sysfs-class-led
@@ -1,12 +1,16 @@
 What:		/sys/class/leds/<led>/brightness
-Date:		March 2006
-KernelVersion:	2.6.17
+Date:		March 2006 (poll October 2016)
+KernelVersion:	2.6.17 (poll since 4.10)
 Contact:	Richard Purdie <rpurdie@rpsys.net>
 Description:
 		Set the brightness of the LED. Most LEDs don't
 		have hardware brightness support, so will just be turned on for
 		non-zero brightness settings. The value is between 0 and
 		/sys/class/leds/<led>/max_brightness.
+		The file supports poll() to detect brightness changes, in
+		some cases the hardware / firmware may change the brightness
+		autonomously, poll() should be woken up in this case too,
+		but not all drivers may support this.
 
 		Writing 0 to this file clears active trigger.
 
diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index 731e4eb..c8d2d67 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -204,6 +204,14 @@ int led_classdev_register(struct device *parent, struct led_classdev *led_cdev)
 		dev_warn(parent, "Led %s renamed to %s due to name collision",
 				led_cdev->name, dev_name(led_cdev->dev));
 
+	led_cdev->brightness_kn = sysfs_get_dirent(led_cdev->dev->kobj.sd,
+						   "brightness");
+	if (!led_cdev->brightness_kn) {
+		dev_err(led_cdev->dev, "Error getting brightness kernfs_node\n");
+		device_unregister(led_cdev->dev);
+		return -ENODEV;
+	}
+
 #ifdef CONFIG_LEDS_TRIGGERS
 	init_rwsem(&led_cdev->trigger_lock);
 #endif
@@ -255,6 +263,7 @@ void led_classdev_unregister(struct led_classdev *led_cdev)
 
 	flush_work(&led_cdev->set_brightness_work);
 
+	sysfs_put(led_cdev->brightness_kn);
 	device_unregister(led_cdev->dev);
 
 	down_write(&leds_list_lock);
diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
index 2d0c75a..af78279 100644
--- a/drivers/leds/led-core.c
+++ b/drivers/leds/led-core.c
@@ -33,16 +33,24 @@ static int __led_set_brightness(struct led_classdev *led_cdev,
 
 	led_cdev->brightness_set(led_cdev, value);
 
+	led_notify_brightness_change(led_cdev);
+
 	return 0;
 }
 
 static int __led_set_brightness_blocking(struct led_classdev *led_cdev,
 					 enum led_brightness value)
 {
+	int ret;
+
 	if (!led_cdev->brightness_set_blocking)
 		return -ENOTSUPP;
 
-	return led_cdev->brightness_set_blocking(led_cdev, value);
+	ret = led_cdev->brightness_set_blocking(led_cdev, value);
+	if (ret >= 0)
+		led_notify_brightness_change(led_cdev);
+
+	return ret;
 }
 
 static void led_timer_function(unsigned long data)
@@ -308,6 +316,12 @@ int led_update_brightness(struct led_classdev *led_cdev)
 }
 EXPORT_SYMBOL_GPL(led_update_brightness);
 
+void led_notify_brightness_change(struct led_classdev *led_cdev)
+{
+	sysfs_notify_dirent(led_cdev->brightness_kn);
+}
+EXPORT_SYMBOL_GPL(led_notify_brightness_change);
+
 /* Caller must ensure led_cdev->led_access held */
 void led_sysfs_disable(struct led_classdev *led_cdev)
 {
diff --git a/include/linux/leds.h b/include/linux/leds.h
index 52993de..f034c2d 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -13,6 +13,7 @@
 #define __LINUX_LEDS_H_INCLUDED
 
 #include <linux/device.h>
+#include <linux/kernfs.h>
 #include <linux/list.h>
 #include <linux/mutex.h>
 #include <linux/rwsem.h>
@@ -93,6 +94,8 @@ struct led_classdev {
 
 	struct work_struct	set_brightness_work;
 
+	struct kernfs_node	*brightness_kn;
+
 #ifdef CONFIG_LEDS_TRIGGERS
 	/* Protects the trigger data below */
 	struct rw_semaphore	 trigger_lock;
@@ -192,6 +195,15 @@ extern int led_set_brightness_sync(struct led_classdev *led_cdev,
 extern int led_update_brightness(struct led_classdev *led_cdev);
 
 /**
+ * led_notify_brightness_change - Notify userspace of brightness changes
+ * @led_cdev: the LED to do the notify on
+ *
+ * Let any users waiting for POLL_PRI on the led's brightness sysfs
+ * atrribute know that the brightness has been changed.
+ */
+extern void led_notify_brightness_change(struct led_classdev *led_cdev);
+
+/**
  * led_sysfs_disable - disable LED sysfs interface
  * @led_cdev: the LED to set
  *
-- 
2.9.3

[PATCH v3 2/5] platform: x86: thinkpad: Call led_notify_brightness_change on kbd brightness change

[Hans de Goede]

Make thinkpad_acpi call led_notify_brightness_change on the kbd_led
led_classdev registered by thinkpad_acpi when the kbd backlight
brightness changes.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v3:
-This is a new patch in v3 of this patch-set
---
 drivers/platform/x86/thinkpad_acpi.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index b65ce75..5dcd7d8b 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -162,6 +162,7 @@ enum tpacpi_hkey_event_t {
 	TP_HKEY_EV_HOTKEY_BASE		= 0x1001, /* first hotkey (FN+F1) */
 	TP_HKEY_EV_BRGHT_UP		= 0x1010, /* Brightness up */
 	TP_HKEY_EV_BRGHT_DOWN		= 0x1011, /* Brightness down */
+	TP_HKEY_EV_THINKLIGHT		= 0x1012, /* Thinklight/kbd backlight */
 	TP_HKEY_EV_VOL_UP		= 0x1015, /* Volume up or unmute */
 	TP_HKEY_EV_VOL_DOWN		= 0x1016, /* Volume down or unmute */
 	TP_HKEY_EV_VOL_MUTE		= 0x1017, /* Mixer output mute */
@@ -5167,6 +5168,8 @@ static int __init kbdlight_init(struct ibm_init_struct *iibm)
 		return rc;
 	}
 
+	tpacpi_hotkey_driver_mask_set(hotkey_driver_mask |
+				      TP_ACPI_HKEY_THNKLGHT_MASK);
 	return 0;
 }
 
@@ -9114,6 +9117,8 @@ static void tpacpi_driver_event(const unsigned int hkey_event)
 			volume_alsa_notify_change();
 		}
 	}
+	if (tp_features.kbdlight && hkey_event == TP_HKEY_EV_THINKLIGHT)
+		led_notify_brightness_change(&tpacpi_led_kbdlight.led_classdev);
 }
 
 static void hotkey_driver_event(const unsigned int scancode)
-- 
2.9.3

[PATCH v3 3/5] platform: x86: dell-smbios: Add a generic dell-smbios notifier chain

[Hans de Goede]

There are several cases where events handled in one of the dell-* drivers
need to be propagated to another dell-* driver.

This commits add 3 generic functions:
dell_smbios_register_notifier()
dell_smbios_unregister_notifier()
dell_smbios_call_notifier()

It currently only defines 1 action:
dell_smbios_kbd_backlight_brightness_changed

Which is intended to propagate kbd_backlight_brightness_changed wmi
events from dell-wmi to dell-laptop (which contains the actual kbd
backlight driver).

This is only somewhat dell smbios related, both dell-wmi and dell-laptop
use smbios functions and I do not want to put the notifier head in
either driver, as that will make the 2 drivers depend on each other.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
-This is a new patch in v2 of this patch-set
Changes in v3:
-No changes
---
 drivers/platform/x86/dell-smbios.c | 20 ++++++++++++++++++++
 drivers/platform/x86/dell-smbios.h | 11 +++++++++++
 2 files changed, 31 insertions(+)

diff --git a/drivers/platform/x86/dell-smbios.c b/drivers/platform/x86/dell-smbios.c
index d2412ab..8b96bdf 100644
--- a/drivers/platform/x86/dell-smbios.c
+++ b/drivers/platform/x86/dell-smbios.c
@@ -105,6 +105,26 @@ struct calling_interface_token *dell_smbios_find_token(int tokenid)
 }
 EXPORT_SYMBOL_GPL(dell_smbios_find_token);
 
+static ATOMIC_NOTIFIER_HEAD(dell_smbios_chain_head);
+
+int dell_smbios_register_notifier(struct notifier_block *nb)
+{
+	return atomic_notifier_chain_register(&dell_smbios_chain_head, nb);
+}
+EXPORT_SYMBOL_GPL(dell_smbios_register_notifier);
+
+int dell_smbios_unregister_notifier(struct notifier_block *nb)
+{
+	return atomic_notifier_chain_unregister(&dell_smbios_chain_head, nb);
+}
+EXPORT_SYMBOL_GPL(dell_smbios_unregister_notifier);
+
+void dell_smbios_call_notifier(unsigned long action, void *data)
+{
+	atomic_notifier_call_chain(&dell_smbios_chain_head, action, data);
+}
+EXPORT_SYMBOL_GPL(dell_smbios_call_notifier);
+
 static void __init parse_da_table(const struct dmi_header *dm)
 {
 	/* Final token is a terminator, so we don't want to copy it */
diff --git a/drivers/platform/x86/dell-smbios.h b/drivers/platform/x86/dell-smbios.h
index ec7d40a..e91f13f 100644
--- a/drivers/platform/x86/dell-smbios.h
+++ b/drivers/platform/x86/dell-smbios.h
@@ -16,6 +16,8 @@
 #ifndef _DELL_SMBIOS_H_
 #define _DELL_SMBIOS_H_
 
+struct notifier_block;
+
 /* This structure will be modified by the firmware when we enter
  * system management mode, hence the volatiles */
 
@@ -43,4 +45,13 @@ void dell_smbios_release_buffer(void);
 void dell_smbios_send_request(int class, int select);
 
 struct calling_interface_token *dell_smbios_find_token(int tokenid);
+
+enum dell_smbios_notifier_actions {
+	dell_smbios_kbd_backlight_brightness_changed,
+};
+
+int dell_smbios_register_notifier(struct notifier_block *nb);
+int dell_smbios_unregister_notifier(struct notifier_block *nb);
+void dell_smbios_call_notifier(unsigned long action, void *data);
+
 #endif
-- 
2.9.3

[PATCH v3 4/5] platform: x86: dell-*: Call led_notify_brightness_change on kbd brightness change

[Hans de Goede]

Make dell-wmi call led_notify_brightness_change on the kbd_led led_classdev
registered by dell-laptop when the kbd backlight brightness changes.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
-Use the new dell_smbios*notify functionality
Changes in v3:
-Simplify the if condition for calling led_notify_brightness_change
---
 drivers/platform/x86/dell-laptop.c | 28 +++++++++++++++++++++++++++-
 drivers/platform/x86/dell-wmi.c    | 14 +++++++++-----
 2 files changed, 36 insertions(+), 6 deletions(-)

diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c
index 2c2f02b..5a96c25 100644
--- a/drivers/platform/x86/dell-laptop.c
+++ b/drivers/platform/x86/dell-laptop.c
@@ -1942,6 +1942,8 @@ static struct led_classdev kbd_led = {
 
 static int __init kbd_led_init(struct device *dev)
 {
+	int ret;
+
 	kbd_init();
 	if (!kbd_led_present)
 		return -ENODEV;
@@ -1953,7 +1955,11 @@ static int __init kbd_led_init(struct device *dev)
 		if (kbd_led.max_brightness)
 			kbd_led.max_brightness--;
 	}
-	return led_classdev_register(dev, &kbd_led);
+	ret = led_classdev_register(dev, &kbd_led);
+	if (ret)
+		kbd_led_present = false;
+
+	return ret;
 }
 
 static void brightness_set_exit(struct led_classdev *led_cdev,
@@ -1970,6 +1976,23 @@ static void kbd_led_exit(void)
 	led_classdev_unregister(&kbd_led);
 }
 
+static int dell_laptop_notifier_call(struct notifier_block *nb,
+				     unsigned long action, void *data)
+{
+	switch (action) {
+	case dell_smbios_kbd_backlight_brightness_changed:
+		if (kbd_led_present)
+			led_notify_brightness_change(&kbd_led);
+		break;
+	}
+
+	return NOTIFY_OK;
+}
+
+static struct notifier_block dell_laptop_notifier = {
+	.notifier_call = dell_laptop_notifier_call,
+};
+
 static int __init dell_init(void)
 {
 	struct calling_interface_buffer *buffer;
@@ -2013,6 +2036,8 @@ static int __init dell_init(void)
 		debugfs_create_file("rfkill", 0444, dell_laptop_dir, NULL,
 				    &dell_debugfs_fops);
 
+	dell_smbios_register_notifier(&dell_laptop_notifier);
+
 	if (acpi_video_get_backlight_type() != acpi_backlight_vendor)
 		return 0;
 
@@ -2064,6 +2089,7 @@ static int __init dell_init(void)
 
 static void __exit dell_exit(void)
 {
+	dell_smbios_unregister_notifier(&dell_laptop_notifier);
 	debugfs_remove_recursive(dell_laptop_dir);
 	if (quirks && quirks->touchpad_led)
 		touchpad_led_exit();
diff --git a/drivers/platform/x86/dell-wmi.c b/drivers/platform/x86/dell-wmi.c
index da2fe18..9eb0200 100644
--- a/drivers/platform/x86/dell-wmi.c
+++ b/drivers/platform/x86/dell-wmi.c
@@ -287,11 +287,11 @@ static const struct key_entry dell_wmi_keymap_type_0011[] __initconst = {
 	{ KE_IGNORE, 0xfff1, { KEY_RESERVED } },
 
 	/* Keyboard backlight level changed */
-	{ KE_IGNORE, 0x01e1, { KEY_RESERVED } },
-	{ KE_IGNORE, 0x02ea, { KEY_RESERVED } },
-	{ KE_IGNORE, 0x02eb, { KEY_RESERVED } },
-	{ KE_IGNORE, 0x02ec, { KEY_RESERVED } },
-	{ KE_IGNORE, 0x02f6, { KEY_RESERVED } },
+	{ KE_IGNORE, 0x01e1, { KEY_KBDILLUMTOGGLE } },
+	{ KE_IGNORE, 0x02ea, { KEY_KBDILLUMTOGGLE } },
+	{ KE_IGNORE, 0x02eb, { KEY_KBDILLUMTOGGLE } },
+	{ KE_IGNORE, 0x02ec, { KEY_KBDILLUMTOGGLE } },
+	{ KE_IGNORE, 0x02f6, { KEY_KBDILLUMTOGGLE } },
 };
 
 static struct input_dev *dell_wmi_input_dev;
@@ -319,6 +319,10 @@ static void dell_wmi_process_key(int type, int code)
 	if (type == 0x0000 && code == 0xe025 && !wmi_requires_smbios_request)
 		return;
 
+	if (key->keycode == KEY_KBDILLUMTOGGLE)
+		dell_smbios_call_notifier(
+			dell_smbios_kbd_backlight_brightness_changed, NULL);
+
 	sparse_keymap_report_entry(dell_wmi_input_dev, key, 1, true);
 }
 
-- 
2.9.3

[PATCH v3 5/5] platform: x86: dell-*: Simplify dell-rbtn integration with dell-laptop [untested]

[Hans de Goede]

Use dell_smbios*notifier for dell-laptop to listen to dell-rbtn slider
events, replace dell_rbtn_notifier_register() /
dell_rbtn_notifier_unregister() with a single dell_rbtn_has_rfkill() used
by dell-laptop to decide whether or not to use the i8042 filter and used
by dell-rbtn to auto-remove its rfkill interface when called.

This results in a nice cleanup, downside is that the rfkill interface
of dell-rbtn is not automatically re-enabled on rmmod dell-laptop, this
now requires rmmod + insmod of dell-rbtn. But people who do not want
dell-laptop for some reason will have it blacklisted anyways, so this
is not an issue and there is a work-around.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
-This is a new patch in v2 of my platform/x86/dell-* notifier set intended
 to show how dell_smbios*notifier can be used to improve the dell-rbtn
 integration too
Changes in v3:
-Call dell_rbtn_has_rfkill_func instead of dell_rbtn_has_rfkill, so that the
 dynamic symbol dance we do to allow loading without dell-rbtn actually works.
---
 drivers/platform/x86/Kconfig       |  1 +
 drivers/platform/x86/dell-laptop.c | 53 +++++++-------------------------
 drivers/platform/x86/dell-rbtn.c   | 63 +++++++++-----------------------------
 drivers/platform/x86/dell-rbtn.h   |  5 +--
 drivers/platform/x86/dell-smbios.h |  1 +
 5 files changed, 29 insertions(+), 94 deletions(-)

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 81b8dcc..30291e1 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -158,6 +158,7 @@ config DELL_RBTN
 	depends on ACPI
 	depends on INPUT
 	depends on RFKILL
+	depends on DELL_SMBIOS
 	---help---
 	  Say Y here if you want to support Dell Airplane Mode Switch ACPI
 	  device on Dell laptops. Sometimes it has names: DELLABCE or DELRBTN.
diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c
index 5a96c25..a894b18 100644
--- a/drivers/platform/x86/dell-laptop.c
+++ b/drivers/platform/x86/dell-laptop.c
@@ -670,25 +670,12 @@ static bool dell_laptop_i8042_filter(unsigned char data, unsigned char str,
 	return false;
 }
 
-static int (*dell_rbtn_notifier_register_func)(struct notifier_block *);
-static int (*dell_rbtn_notifier_unregister_func)(struct notifier_block *);
-
-static int dell_laptop_rbtn_notifier_call(struct notifier_block *nb,
-					  unsigned long action, void *data)
-{
-	schedule_delayed_work(&dell_rfkill_work, 0);
-	return NOTIFY_OK;
-}
-
-static struct notifier_block dell_laptop_rbtn_notifier = {
-	.notifier_call = dell_laptop_rbtn_notifier_call,
-};
-
 static int __init dell_setup_rfkill(void)
 {
 	struct calling_interface_buffer *buffer;
 	int status, ret, whitelisted;
 	const char *product;
+	int (*dell_rbtn_has_rfkill_func)(void);
 
 	/*
 	 * rfkill support causes trouble on various models, mostly Inspirons.
@@ -773,29 +760,13 @@ static int __init dell_setup_rfkill(void)
 	 *
 	 * To prevent duplicate rfkill devices which control and do same thing,
 	 * dell-rbtn driver will automatically remove its own rfkill devices
-	 * once function dell_rbtn_notifier_register() is called.
+	 * once function dell_rbtn_has_rfkill() is called.
 	 */
 
-	dell_rbtn_notifier_register_func =
-		symbol_request(dell_rbtn_notifier_register);
-	if (dell_rbtn_notifier_register_func) {
-		dell_rbtn_notifier_unregister_func =
-			symbol_request(dell_rbtn_notifier_unregister);
-		if (!dell_rbtn_notifier_unregister_func) {
-			symbol_put(dell_rbtn_notifier_register);
-			dell_rbtn_notifier_register_func = NULL;
-		}
-	}
-
-	if (dell_rbtn_notifier_register_func) {
-		ret = dell_rbtn_notifier_register_func(
-			&dell_laptop_rbtn_notifier);
-		symbol_put(dell_rbtn_notifier_register);
-		dell_rbtn_notifier_register_func = NULL;
-		if (ret != 0) {
-			symbol_put(dell_rbtn_notifier_unregister);
-			dell_rbtn_notifier_unregister_func = NULL;
-		}
+	dell_rbtn_has_rfkill_func = symbol_request(dell_rbtn_has_rfkill);
+	if (dell_rbtn_has_rfkill_func) {
+		ret = dell_rbtn_has_rfkill_func();
+		symbol_put(dell_rbtn_has_rfkill);
 	} else {
 		pr_info("Symbols from dell-rbtn acpi driver are not available\n");
 		ret = -ENODEV;
@@ -835,13 +806,7 @@ static int __init dell_setup_rfkill(void)
 
 static void dell_cleanup_rfkill(void)
 {
-	if (dell_rbtn_notifier_unregister_func) {
-		dell_rbtn_notifier_unregister_func(&dell_laptop_rbtn_notifier);
-		symbol_put(dell_rbtn_notifier_unregister);
-		dell_rbtn_notifier_unregister_func = NULL;
-	} else {
-		i8042_remove_filter(dell_laptop_i8042_filter);
-	}
+	i8042_remove_filter(dell_laptop_i8042_filter);
 	cancel_delayed_work_sync(&dell_rfkill_work);
 	if (wifi_rfkill) {
 		rfkill_unregister(wifi_rfkill);
@@ -1984,6 +1949,10 @@ static int dell_laptop_notifier_call(struct notifier_block *nb,
 		if (kbd_led_present)
 			led_notify_brightness_change(&kbd_led);
 		break;
+	case dell_smbios_rbtn_slider:
+		if (wifi_rfkill || bluetooth_rfkill || wwan_rfkill)
+			schedule_delayed_work(&dell_rfkill_work, 0);
+		break;
 	}
 
 	return NOTIFY_OK;
diff --git a/drivers/platform/x86/dell-rbtn.c b/drivers/platform/x86/dell-rbtn.c
index dcd9f40..b6c03d4 100644
--- a/drivers/platform/x86/dell-rbtn.c
+++ b/drivers/platform/x86/dell-rbtn.c
@@ -17,6 +17,8 @@
 #include <linux/acpi.h>
 #include <linux/rfkill.h>
 #include <linux/input.h>
+#include "dell-smbios.h"
+#include "dell-rbtn.h"
 
 enum rbtn_type {
 	RBTN_UNKNOWN,
@@ -295,12 +297,11 @@ static struct acpi_driver rbtn_driver = {
 
 
 /*
- * notifier export functions
+ * Functions to coordinate registering only 1 rfkill with dell-laptop
  */
 
 static bool auto_remove_rfkill = true;
-
-static ATOMIC_NOTIFIER_HEAD(rbtn_chain_head);
+static bool init_rfkill = true;
 
 static int rbtn_inc_count(struct device *dev, void *data)
 {
@@ -314,26 +315,14 @@ static int rbtn_inc_count(struct device *dev, void *data)
 	return 0;
 }
 
-static int rbtn_switch_dev(struct device *dev, void *data)
+static int rbtn_disable_rfkill(struct device *dev, void *data)
 {
-	struct acpi_device *device = to_acpi_device(dev);
-	struct rbtn_data *rbtn_data = device->driver_data;
-	bool enable = data;
-
-	if (rbtn_data->type != RBTN_SLIDER)
-		return 0;
-
-	if (enable)
-		rbtn_rfkill_init(device);
-	else
-		rbtn_rfkill_exit(device);
-
+	rbtn_rfkill_exit(to_acpi_device(dev));
 	return 0;
 }
 
-int dell_rbtn_notifier_register(struct notifier_block *nb)
+int dell_rbtn_has_rfkill(void)
 {
-	bool first;
 	int count;
 	int ret;
 
@@ -343,35 +332,15 @@ int dell_rbtn_notifier_register(struct notifier_block *nb)
 	if (ret || count == 0)
 		return -ENODEV;
 
-	first = !rbtn_chain_head.head;
-
-	ret = atomic_notifier_chain_register(&rbtn_chain_head, nb);
-	if (ret != 0)
-		return ret;
-
-	if (auto_remove_rfkill && first)
-		ret = driver_for_each_device(&rbtn_driver.drv, NULL,
-					     (void *)false, rbtn_switch_dev);
-
-	return ret;
-}
-EXPORT_SYMBOL_GPL(dell_rbtn_notifier_register);
-
-int dell_rbtn_notifier_unregister(struct notifier_block *nb)
-{
-	int ret;
-
-	ret = atomic_notifier_chain_unregister(&rbtn_chain_head, nb);
-	if (ret != 0)
-		return ret;
-
-	if (auto_remove_rfkill && !rbtn_chain_head.head)
-		ret = driver_for_each_device(&rbtn_driver.drv, NULL,
-					     (void *)true, rbtn_switch_dev);
+	if (auto_remove_rfkill) {
+		init_rfkill = false;
+		ret = driver_for_each_device(&rbtn_driver.drv, NULL, NULL,
+					     rbtn_disable_rfkill);
+	}
 
 	return ret;
 }
-EXPORT_SYMBOL_GPL(dell_rbtn_notifier_unregister);
+EXPORT_SYMBOL_GPL(dell_rbtn_has_rfkill);
 
 
 /*
@@ -408,9 +377,7 @@ static int rbtn_add(struct acpi_device *device)
 		ret = rbtn_input_init(rbtn_data);
 		break;
 	case RBTN_SLIDER:
-		if (auto_remove_rfkill && rbtn_chain_head.head)
-			ret = 0;
-		else
+		if (init_rfkill)
 			ret = rbtn_rfkill_init(device);
 		break;
 	default:
@@ -467,7 +434,7 @@ static void rbtn_notify(struct acpi_device *device, u32 event)
 		break;
 	case RBTN_SLIDER:
 		rbtn_rfkill_event(device);
-		atomic_notifier_call_chain(&rbtn_chain_head, event, device);
+		dell_smbios_call_notifier(dell_smbios_rbtn_slider, NULL);
 		break;
 	default:
 		break;
diff --git a/drivers/platform/x86/dell-rbtn.h b/drivers/platform/x86/dell-rbtn.h
index c59cc6b..97578e8 100644
--- a/drivers/platform/x86/dell-rbtn.h
+++ b/drivers/platform/x86/dell-rbtn.h
@@ -16,9 +16,6 @@
 #ifndef _DELL_RBTN_H_
 #define _DELL_RBTN_H_
 
-struct notifier_block;
-
-int dell_rbtn_notifier_register(struct notifier_block *nb);
-int dell_rbtn_notifier_unregister(struct notifier_block *nb);
+int dell_rbtn_has_rfkill(void);
 
 #endif
diff --git a/drivers/platform/x86/dell-smbios.h b/drivers/platform/x86/dell-smbios.h
index e91f13f..92817a1 100644
--- a/drivers/platform/x86/dell-smbios.h
+++ b/drivers/platform/x86/dell-smbios.h
@@ -48,6 +48,7 @@ struct calling_interface_token *dell_smbios_find_token(int tokenid);
 
 enum dell_smbios_notifier_actions {
 	dell_smbios_kbd_backlight_brightness_changed,
+	dell_smbios_rbtn_slider,
 };
 
 int dell_smbios_register_notifier(struct notifier_block *nb);
-- 
2.9.3

Re: [PATCH v3 2/5] platform: x86: thinkpad: Call led_notify_brightness_change on kbd brightness change

[Pali Rohár]

[-- Attachment #1: Type: Text/Plain, Size: 342 bytes --]

On Wednesday 26 October 2016 19:41:15 Hans de Goede wrote:
> +	TP_HKEY_EV_THINKLIGHT		= 0x1012, /* Thinklight/kbd backlight */

Hi! Do you have some documentation for this number? When it is send? 
When keyboard baklight level is changed or when thinklight is turned 
on/off? Or in both cases?

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

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH v3 5/5] platform: x86: dell-*: Simplify dell-rbtn integration with dell-laptop [untested]

[Pali Rohár]

On Wednesday 26 October 2016 19:41:18 Hans de Goede wrote:
> Use dell_smbios*notifier for dell-laptop to listen to dell-rbtn slider
> events, replace dell_rbtn_notifier_register() /
> dell_rbtn_notifier_unregister() with a single dell_rbtn_has_rfkill() used
> by dell-laptop to decide whether or not to use the i8042 filter and used
> by dell-rbtn to auto-remove its rfkill interface when called.
> 
> This results in a nice cleanup, downside is that the rfkill interface
> of dell-rbtn is not automatically re-enabled on rmmod dell-laptop, this
> now requires rmmod + insmod of dell-rbtn. But people who do not want
> dell-laptop for some reason will have it blacklisted anyways, so this
> is not an issue and there is a work-around.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Changes in v2:
> -This is a new patch in v2 of my platform/x86/dell-* notifier set intended
>  to show how dell_smbios*notifier can be used to improve the dell-rbtn
>  integration too
> Changes in v3:
> -Call dell_rbtn_has_rfkill_func instead of dell_rbtn_has_rfkill, so that the
>  dynamic symbol dance we do to allow loading without dell-rbtn actually works.
> ---
>  drivers/platform/x86/Kconfig       |  1 +
>  drivers/platform/x86/dell-laptop.c | 53 +++++++-------------------------
>  drivers/platform/x86/dell-rbtn.c   | 63 +++++++++-----------------------------
>  drivers/platform/x86/dell-rbtn.h   |  5 +--
>  drivers/platform/x86/dell-smbios.h |  1 +
>  5 files changed, 29 insertions(+), 94 deletions(-)

Looks like that for preventing sending event that rfkill switch was
changed by hardware slider we must always drop atk i8042 keycode...

Needs to check if key is really send by both dell-rbtn and also by atk
i8042 keyboard driver and if yes then i8042_install_filter() is always
needed (if rbtn is there or not)...

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

Re: [PATCH v3 5/5] platform: x86: dell-*: Simplify dell-rbtn integration with dell-laptop [untested]

[Hans de Goede]

Hi,

On 27-10-16 12:38, Pali Rohár wrote:
> On Wednesday 26 October 2016 19:41:18 Hans de Goede wrote:
>> Use dell_smbios*notifier for dell-laptop to listen to dell-rbtn slider
>> events, replace dell_rbtn_notifier_register() /
>> dell_rbtn_notifier_unregister() with a single dell_rbtn_has_rfkill() used
>> by dell-laptop to decide whether or not to use the i8042 filter and used
>> by dell-rbtn to auto-remove its rfkill interface when called.
>>
>> This results in a nice cleanup, downside is that the rfkill interface
>> of dell-rbtn is not automatically re-enabled on rmmod dell-laptop, this
>> now requires rmmod + insmod of dell-rbtn. But people who do not want
>> dell-laptop for some reason will have it blacklisted anyways, so this
>> is not an issue and there is a work-around.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>> Changes in v2:
>> -This is a new patch in v2 of my platform/x86/dell-* notifier set intended
>>  to show how dell_smbios*notifier can be used to improve the dell-rbtn
>>  integration too
>> Changes in v3:
>> -Call dell_rbtn_has_rfkill_func instead of dell_rbtn_has_rfkill, so that the
>>  dynamic symbol dance we do to allow loading without dell-rbtn actually works.
>> ---
>>  drivers/platform/x86/Kconfig       |  1 +
>>  drivers/platform/x86/dell-laptop.c | 53 +++++++-------------------------
>>  drivers/platform/x86/dell-rbtn.c   | 63 +++++++++-----------------------------
>>  drivers/platform/x86/dell-rbtn.h   |  5 +--
>>  drivers/platform/x86/dell-smbios.h |  1 +
>>  5 files changed, 29 insertions(+), 94 deletions(-)
>
> Looks like that for preventing sending event that rfkill switch was
> changed by hardware slider we must always drop atk i8042 keycode...
>
> Needs to check if key is really send by both dell-rbtn and also by atk
> i8042 keyboard driver and if yes then i8042_install_filter() is always
> needed (if rbtn is there or not)...

But this is not related to this patch, right? This patch does not change
any behavior. Other then your concerns about where to put the notifier,
do you like the approach of this patch?

Regards,

Hans

------------------------------------------------------------------------------
The Command Line: Reinvented for Modern Developers
Did the resurgence of CLI tooling catch you by surprise?
Reconnect with the command line and become more productive. 
Learn the new .NET and ASP.NET CLI. Get your free copy!
http://sdm.link/telerik
_______________________________________________
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel

Re: [PATCH v3 5/5] platform: x86: dell-*: Simplify dell-rbtn integration with dell-laptop [untested]

[Pali Rohár]

On Thursday 27 October 2016 13:46:33 Hans de Goede wrote:
> Hi,
> 
> On 27-10-16 12:38, Pali Rohár wrote:
> >On Wednesday 26 October 2016 19:41:18 Hans de Goede wrote:
> >>Use dell_smbios*notifier for dell-laptop to listen to dell-rbtn slider
> >>events, replace dell_rbtn_notifier_register() /
> >>dell_rbtn_notifier_unregister() with a single dell_rbtn_has_rfkill() used
> >>by dell-laptop to decide whether or not to use the i8042 filter and used
> >>by dell-rbtn to auto-remove its rfkill interface when called.
> >>
> >>This results in a nice cleanup, downside is that the rfkill interface
> >>of dell-rbtn is not automatically re-enabled on rmmod dell-laptop, this
> >>now requires rmmod + insmod of dell-rbtn. But people who do not want
> >>dell-laptop for some reason will have it blacklisted anyways, so this
> >>is not an issue and there is a work-around.
> >>
> >>Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> >>---
> >>Changes in v2:
> >>-This is a new patch in v2 of my platform/x86/dell-* notifier set intended
> >> to show how dell_smbios*notifier can be used to improve the dell-rbtn
> >> integration too
> >>Changes in v3:
> >>-Call dell_rbtn_has_rfkill_func instead of dell_rbtn_has_rfkill, so that the
> >> dynamic symbol dance we do to allow loading without dell-rbtn actually works.
> >>---
> >> drivers/platform/x86/Kconfig       |  1 +
> >> drivers/platform/x86/dell-laptop.c | 53 +++++++-------------------------
> >> drivers/platform/x86/dell-rbtn.c   | 63 +++++++++-----------------------------
> >> drivers/platform/x86/dell-rbtn.h   |  5 +--
> >> drivers/platform/x86/dell-smbios.h |  1 +
> >> 5 files changed, 29 insertions(+), 94 deletions(-)
> >
> >Looks like that for preventing sending event that rfkill switch was
> >changed by hardware slider we must always drop atk i8042 keycode...
> >
> >Needs to check if key is really send by both dell-rbtn and also by atk
> >i8042 keyboard driver and if yes then i8042_install_filter() is always
> >needed (if rbtn is there or not)...
> 
> But this is not related to this patch, right? This patch does not change
> any behavior. Other then your concerns about where to put the notifier,
> do you like the approach of this patch?

It is not related, but if above is truth, then another rewrite (also of
those your changes!) is needed. So maybe it would be better to postpone
(or drop this patch from your patch series) so we do not invest time for
something which will be again rewritten...

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

Re: [PATCH v3 5/5] platform: x86: dell-*: Simplify dell-rbtn integration with dell-laptop [untested]

[Hans de Goede]

Hi,

On 27-10-16 13:59, Pali Rohár wrote:
> On Thursday 27 October 2016 13:46:33 Hans de Goede wrote:
>> Hi,
>>
>> On 27-10-16 12:38, Pali Rohár wrote:
>>> On Wednesday 26 October 2016 19:41:18 Hans de Goede wrote:
>>>> Use dell_smbios*notifier for dell-laptop to listen to dell-rbtn slider
>>>> events, replace dell_rbtn_notifier_register() /
>>>> dell_rbtn_notifier_unregister() with a single dell_rbtn_has_rfkill() used
>>>> by dell-laptop to decide whether or not to use the i8042 filter and used
>>>> by dell-rbtn to auto-remove its rfkill interface when called.
>>>>
>>>> This results in a nice cleanup, downside is that the rfkill interface
>>>> of dell-rbtn is not automatically re-enabled on rmmod dell-laptop, this
>>>> now requires rmmod + insmod of dell-rbtn. But people who do not want
>>>> dell-laptop for some reason will have it blacklisted anyways, so this
>>>> is not an issue and there is a work-around.
>>>>
>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>>> ---
>>>> Changes in v2:
>>>> -This is a new patch in v2 of my platform/x86/dell-* notifier set intended
>>>> to show how dell_smbios*notifier can be used to improve the dell-rbtn
>>>> integration too
>>>> Changes in v3:
>>>> -Call dell_rbtn_has_rfkill_func instead of dell_rbtn_has_rfkill, so that the
>>>> dynamic symbol dance we do to allow loading without dell-rbtn actually works.
>>>> ---
>>>> drivers/platform/x86/Kconfig       |  1 +
>>>> drivers/platform/x86/dell-laptop.c | 53 +++++++-------------------------
>>>> drivers/platform/x86/dell-rbtn.c   | 63 +++++++++-----------------------------
>>>> drivers/platform/x86/dell-rbtn.h   |  5 +--
>>>> drivers/platform/x86/dell-smbios.h |  1 +
>>>> 5 files changed, 29 insertions(+), 94 deletions(-)
>>>
>>> Looks like that for preventing sending event that rfkill switch was
>>> changed by hardware slider we must always drop atk i8042 keycode...
>>>
>>> Needs to check if key is really send by both dell-rbtn and also by atk
>>> i8042 keyboard driver and if yes then i8042_install_filter() is always
>>> needed (if rbtn is there or not)...
>>
>> But this is not related to this patch, right? This patch does not change
>> any behavior. Other then your concerns about where to put the notifier,
>> do you like the approach of this patch?
>
> It is not related, but if above is truth, then another rewrite (also of
> those your changes!) is needed. So maybe it would be better to postpone
> (or drop this patch from your patch series) so we do not invest time for
> something which will be again rewritten...

Ok, I'm fine with delaying this cleanup till your questions are answered.

Maybe we should then also move ahead with the notifier in dell-smbios,
because without this patch it is only used by dell-wmi and dell-laptop
both of which already depend on dell-smbios.

Regards,

Hans

------------------------------------------------------------------------------
The Command Line: Reinvented for Modern Developers
Did the resurgence of CLI tooling catch you by surprise?
Reconnect with the command line and become more productive. 
Learn the new .NET and ASP.NET CLI. Get your free copy!
http://sdm.link/telerik
_______________________________________________
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel

Re: [PATCH v3 5/5] platform: x86: dell-*: Simplify dell-rbtn integration with dell-laptop [untested]

[Darren Hart]

On Thu, Oct 27, 2016 at 02:42:55PM +0200, Hans de Goede wrote:
> Hi,
> 
> On 27-10-16 13:59, Pali Rohár wrote:
> > On Thursday 27 October 2016 13:46:33 Hans de Goede wrote:
> > > Hi,
> > > 
> > > On 27-10-16 12:38, Pali Rohár wrote:
> > > > On Wednesday 26 October 2016 19:41:18 Hans de Goede wrote:
> > > > > Use dell_smbios*notifier for dell-laptop to listen to dell-rbtn slider
> > > > > events, replace dell_rbtn_notifier_register() /
> > > > > dell_rbtn_notifier_unregister() with a single dell_rbtn_has_rfkill() used
> > > > > by dell-laptop to decide whether or not to use the i8042 filter and used
> > > > > by dell-rbtn to auto-remove its rfkill interface when called.
> > > > > 
> > > > > This results in a nice cleanup, downside is that the rfkill interface
> > > > > of dell-rbtn is not automatically re-enabled on rmmod dell-laptop, this
> > > > > now requires rmmod + insmod of dell-rbtn. But people who do not want
> > > > > dell-laptop for some reason will have it blacklisted anyways, so this
> > > > > is not an issue and there is a work-around.
> > > > > 
> > > > > Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> > > > > ---
> > > > > Changes in v2:
> > > > > -This is a new patch in v2 of my platform/x86/dell-* notifier set intended
> > > > > to show how dell_smbios*notifier can be used to improve the dell-rbtn
> > > > > integration too
> > > > > Changes in v3:
> > > > > -Call dell_rbtn_has_rfkill_func instead of dell_rbtn_has_rfkill, so that the
> > > > > dynamic symbol dance we do to allow loading without dell-rbtn actually works.
> > > > > ---
> > > > > drivers/platform/x86/Kconfig       |  1 +
> > > > > drivers/platform/x86/dell-laptop.c | 53 +++++++-------------------------
> > > > > drivers/platform/x86/dell-rbtn.c   | 63 +++++++++-----------------------------
> > > > > drivers/platform/x86/dell-rbtn.h   |  5 +--
> > > > > drivers/platform/x86/dell-smbios.h |  1 +
> > > > > 5 files changed, 29 insertions(+), 94 deletions(-)
> > > > 
> > > > Looks like that for preventing sending event that rfkill switch was
> > > > changed by hardware slider we must always drop atk i8042 keycode...
> > > > 
> > > > Needs to check if key is really send by both dell-rbtn and also by atk
> > > > i8042 keyboard driver and if yes then i8042_install_filter() is always
> > > > needed (if rbtn is there or not)...
> > > 
> > > But this is not related to this patch, right? This patch does not change
> > > any behavior. Other then your concerns about where to put the notifier,
> > > do you like the approach of this patch?
> > 
> > It is not related, but if above is truth, then another rewrite (also of
> > those your changes!) is needed. So maybe it would be better to postpone
> > (or drop this patch from your patch series) so we do not invest time for
> > something which will be again rewritten...
> 
> Ok, I'm fine with delaying this cleanup till your questions are answered.
> 
> Maybe we should then also move ahead with the notifier in dell-smbios,
> because without this patch it is only used by dell-wmi and dell-laptop
> both of which already depend on dell-smbios.

This one has been at the bottom of my queue for a while, but as far as I know we
haven't moved forward with the testing and/or clarification we needed. I'm
dropping for now (from the patchwork list), but please feel free to resurrect if
I've missed something, or new information is available.

-- 
Darren Hart
Intel Open Source Technology Center