All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/6] msi-laptop: Add MSI Wind U90/U100 support
@ 2012-11-24 22:28 Maxim Mikityanskiy
  2012-11-24 22:28 ` [PATCH 1/6] msi-laptop: Use proper return codes instead of -1 Maxim Mikityanskiy
                   ` (5 more replies)
  0 siblings, 6 replies; 25+ messages in thread
From: Maxim Mikityanskiy @ 2012-11-24 22:28 UTC (permalink / raw)
  To: platform-driver-x86, jlee, mjg

This series adds support for MSI Wind U90/U100 laptops to msi-laptop
platform driver and adds new driver msi-wind-wmi that provides basic
support for WMI found in MSI Wind laptops.

Tested on MSI Wind U100Plus laptop.

----------------------------------------------------------------
Maxim Mikityanskiy (6):
      msi-laptop: Use proper return codes instead of -1
      msi-laptop: Work around gcc warning
      msi-laptop: Code cleanups
      msi-laptop: Add MSI Wind U90/U100 support
      msi-laptop: Add missing ABI documentation
      Add MSI Wind WMI support

 Documentation/ABI/testing/sysfs-platform-msi-laptop |  83 +++++++++++++++++++++++++
 drivers/platform/x86/Kconfig                        |  13 ++++
 drivers/platform/x86/Makefile                       |   1 +
 drivers/platform/x86/msi-laptop.c                   | 276 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----------------------
 drivers/platform/x86/msi-wind-wmi.c                 | 169 +++++++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 466 insertions(+), 76 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-platform-msi-laptop
 create mode 100644 drivers/platform/x86/msi-wind-wmi.c

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

* [PATCH 1/6] msi-laptop: Use proper return codes instead of -1
  2012-11-24 22:28 [PATCH 0/6] msi-laptop: Add MSI Wind U90/U100 support Maxim Mikityanskiy
@ 2012-11-24 22:28 ` Maxim Mikityanskiy
  2012-11-28  2:07   ` joeyli
  2012-11-24 22:28 ` [PATCH 2/6] msi-laptop: Work around gcc warning Maxim Mikityanskiy
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 25+ messages in thread
From: Maxim Mikityanskiy @ 2012-11-24 22:28 UTC (permalink / raw)
  To: platform-driver-x86, jlee, mjg; +Cc: Maxim Mikityanskiy


Signed-off-by: Maxim Mikityanskiy <maxtram95@gmail.com>
---
 drivers/platform/x86/msi-laptop.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/platform/x86/msi-laptop.c b/drivers/platform/x86/msi-laptop.c
index 2111dbb..063113c 100644
--- a/drivers/platform/x86/msi-laptop.c
+++ b/drivers/platform/x86/msi-laptop.c
@@ -198,7 +198,7 @@ static ssize_t set_device_state(const char *buf, size_t count, u8 mask)
 	/* read current device state */
 	result = ec_read(MSI_STANDARD_EC_COMMAND_ADDRESS, &rdata);
 	if (result < 0)
-		return -EINVAL;
+		return result;
 
 	if (!!(rdata & mask) != status) {
 		/* reverse device bit */
@@ -209,7 +209,7 @@ static ssize_t set_device_state(const char *buf, size_t count, u8 mask)
 
 		result = ec_write(MSI_STANDARD_EC_COMMAND_ADDRESS, wdata);
 		if (result < 0)
-			return -EINVAL;
+			return result;
 	}
 
 	return count;
@@ -222,7 +222,7 @@ static int get_wireless_state(int *wlan, int *bluetooth)
 
 	result = ec_transaction(MSI_EC_COMMAND_WIRELESS, &wdata, 1, &rdata, 1);
 	if (result < 0)
-		return -1;
+		return result;
 
 	if (wlan)
 		*wlan = !!(rdata & 8);
@@ -240,7 +240,7 @@ static int get_wireless_state_ec_standard(void)
 
 	result = ec_read(MSI_STANDARD_EC_COMMAND_ADDRESS, &rdata);
 	if (result < 0)
-		return -1;
+		return result;
 
 	wlan_s = !!(rdata & MSI_STANDARD_EC_WLAN_MASK);
 
@@ -258,7 +258,7 @@ static int get_threeg_exists(void)
 
 	result = ec_read(MSI_STANDARD_EC_DEVICES_EXISTS_ADDRESS, &rdata);
 	if (result < 0)
-		return -1;
+		return result;
 
 	threeg_exists = !!(rdata & MSI_STANDARD_EC_3G_MASK);
 
@@ -343,7 +343,7 @@ static ssize_t show_threeg(struct device *dev,
 
 	/* old msi ec not support 3G */
 	if (old_ec_model)
-		return -1;
+		return -ENODEV;
 
 	ret = get_wireless_state_ec_standard();
 	if (ret < 0)
-- 
1.8.0

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

* [PATCH 2/6] msi-laptop: Work around gcc warning
  2012-11-24 22:28 [PATCH 0/6] msi-laptop: Add MSI Wind U90/U100 support Maxim Mikityanskiy
  2012-11-24 22:28 ` [PATCH 1/6] msi-laptop: Use proper return codes instead of -1 Maxim Mikityanskiy
@ 2012-11-24 22:28 ` Maxim Mikityanskiy
  2012-11-28  2:47   ` joeyli
  2012-11-24 22:28 ` [PATCH 3/6] msi-laptop: Code cleanups Maxim Mikityanskiy
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 25+ messages in thread
From: Maxim Mikityanskiy @ 2012-11-24 22:28 UTC (permalink / raw)
  To: platform-driver-x86, jlee, mjg; +Cc: Maxim Mikityanskiy


Signed-off-by: Maxim Mikityanskiy <maxtram95@gmail.com>
---
 drivers/platform/x86/msi-laptop.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/msi-laptop.c b/drivers/platform/x86/msi-laptop.c
index 063113c..7ba107a 100644
--- a/drivers/platform/x86/msi-laptop.c
+++ b/drivers/platform/x86/msi-laptop.c
@@ -291,7 +291,7 @@ static ssize_t show_wlan(struct device *dev,
 	struct device_attribute *attr, char *buf)
 {
 
-	int ret, enabled;
+	int ret, enabled = 0;
 
 	if (old_ec_model) {
 		ret = get_wireless_state(&enabled, NULL);
@@ -315,7 +315,7 @@ static ssize_t show_bluetooth(struct device *dev,
 	struct device_attribute *attr, char *buf)
 {
 
-	int ret, enabled;
+	int ret, enabled = 0;
 
 	if (old_ec_model) {
 		ret = get_wireless_state(NULL, &enabled);
-- 
1.8.0

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

* [PATCH 3/6] msi-laptop: Code cleanups
  2012-11-24 22:28 [PATCH 0/6] msi-laptop: Add MSI Wind U90/U100 support Maxim Mikityanskiy
  2012-11-24 22:28 ` [PATCH 1/6] msi-laptop: Use proper return codes instead of -1 Maxim Mikityanskiy
  2012-11-24 22:28 ` [PATCH 2/6] msi-laptop: Work around gcc warning Maxim Mikityanskiy
@ 2012-11-24 22:28 ` Maxim Mikityanskiy
  2012-11-28  3:11   ` joeyli
  2012-11-24 22:28 ` [PATCH 4/6] msi-laptop: Add MSI Wind U90/U100 support Maxim Mikityanskiy
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 25+ messages in thread
From: Maxim Mikityanskiy @ 2012-11-24 22:28 UTC (permalink / raw)
  To: platform-driver-x86, jlee, mjg; +Cc: Maxim Mikityanskiy

Replaced msi_init_rfkill() by msi_update_rfkill(), removed delays by
replacing delayed works by just works and made msi_laptop_i8042_filter()
filter out KEY_TOUCHPAD_TOGGLE when sending KEY_TOUCHPAD_ON and
KEY_TOUCHPAD_OFF.

Signed-off-by: Maxim Mikityanskiy <maxtram95@gmail.com>
---
 drivers/platform/x86/msi-laptop.c | 67 ++++++++++++++-------------------------
 1 file changed, 24 insertions(+), 43 deletions(-)

diff --git a/drivers/platform/x86/msi-laptop.c b/drivers/platform/x86/msi-laptop.c
index 7ba107a..3b6f494 100644
--- a/drivers/platform/x86/msi-laptop.c
+++ b/drivers/platform/x86/msi-laptop.c
@@ -600,8 +600,23 @@ static const struct rfkill_ops rfkill_threeg_ops = {
 	.set_block = rfkill_threeg_set
 };
 
+static void msi_update_rfkill(struct work_struct *ignored)
+{
+	get_wireless_state_ec_standard();
+
+	if (rfk_wlan)
+		rfkill_set_sw_state(rfk_wlan, !wlan_s);
+	if (rfk_bluetooth)
+		rfkill_set_sw_state(rfk_bluetooth, !bluetooth_s);
+	if (rfk_threeg)
+		rfkill_set_sw_state(rfk_threeg, !threeg_s);
+}
+static DECLARE_WORK(msi_rfkill_work, msi_update_rfkill);
+
 static void rfkill_cleanup(void)
 {
+	cancel_work_sync(&msi_rfkill_work);
+
 	if (rfk_bluetooth) {
 		rfkill_unregister(rfk_bluetooth);
 		rfkill_destroy(rfk_bluetooth);
@@ -618,19 +633,6 @@ static void rfkill_cleanup(void)
 	}
 }
 
-static void msi_update_rfkill(struct work_struct *ignored)
-{
-	get_wireless_state_ec_standard();
-
-	if (rfk_wlan)
-		rfkill_set_sw_state(rfk_wlan, !wlan_s);
-	if (rfk_bluetooth)
-		rfkill_set_sw_state(rfk_bluetooth, !bluetooth_s);
-	if (rfk_threeg)
-		rfkill_set_sw_state(rfk_threeg, !threeg_s);
-}
-static DECLARE_DELAYED_WORK(msi_rfkill_work, msi_update_rfkill);
-
 static void msi_send_touchpad_key(struct work_struct *ignored)
 {
 	u8 rdata;
@@ -644,7 +646,7 @@ static void msi_send_touchpad_key(struct work_struct *ignored)
 		(rdata & MSI_STANDARD_EC_TOUCHPAD_MASK) ?
 		KEY_TOUCHPAD_ON : KEY_TOUCHPAD_OFF, 1, true);
 }
-static DECLARE_DELAYED_WORK(msi_touchpad_work, msi_send_touchpad_key);
+static DECLARE_WORK(msi_touchpad_work, msi_send_touchpad_key);
 
 static bool msi_laptop_i8042_filter(unsigned char data, unsigned char str,
 				struct serio *port)
@@ -662,14 +664,16 @@ static bool msi_laptop_i8042_filter(unsigned char data, unsigned char str,
 		extended = false;
 		switch (data) {
 		case 0xE4:
-			schedule_delayed_work(&msi_touchpad_work,
-				round_jiffies_relative(0.5 * HZ));
-			break;
+			schedule_work(&msi_touchpad_work);
+		case 0x64:
+			/* Filter out KEY_TOUCHPAD_TOGGLE and send only
+			 * KEY_TOUCHPAD_ON and KEY_TOUCHPAD_OFF
+			 */
+			return true;
 		case 0x54:
 		case 0x62:
 		case 0x76:
-			schedule_delayed_work(&msi_rfkill_work,
-				round_jiffies_relative(0.5 * HZ));
+			schedule_work(&msi_rfkill_work);
 			break;
 		}
 	}
@@ -677,31 +681,11 @@ static bool msi_laptop_i8042_filter(unsigned char data, unsigned char str,
 	return false;
 }
 
-static void msi_init_rfkill(struct work_struct *ignored)
-{
-	if (rfk_wlan) {
-		rfkill_set_sw_state(rfk_wlan, !wlan_s);
-		rfkill_wlan_set(NULL, !wlan_s);
-	}
-	if (rfk_bluetooth) {
-		rfkill_set_sw_state(rfk_bluetooth, !bluetooth_s);
-		rfkill_bluetooth_set(NULL, !bluetooth_s);
-	}
-	if (rfk_threeg) {
-		rfkill_set_sw_state(rfk_threeg, !threeg_s);
-		rfkill_threeg_set(NULL, !threeg_s);
-	}
-}
-static DECLARE_DELAYED_WORK(msi_rfkill_init, msi_init_rfkill);
-
 static int rfkill_init(struct platform_device *sdev)
 {
 	/* add rfkill */
 	int retval;
 
-	/* keep the hardware wireless state */
-	get_wireless_state_ec_standard();
-
 	rfk_bluetooth = rfkill_alloc("msi-bluetooth", &sdev->dev,
 				RFKILL_TYPE_BLUETOOTH,
 				&rfkill_bluetooth_ops, NULL);
@@ -736,8 +720,7 @@ static int rfkill_init(struct platform_device *sdev)
 	}
 
 	/* schedule to run rfkill state initial */
-	schedule_delayed_work(&msi_rfkill_init,
-				round_jiffies_relative(1 * HZ));
+	schedule_work(&msi_rfkill_work);
 
 	return 0;
 
@@ -951,7 +934,6 @@ fail_platform_device2:
 
 	if (load_scm_model) {
 		i8042_remove_filter(msi_laptop_i8042_filter);
-		cancel_delayed_work_sync(&msi_rfkill_work);
 		rfkill_cleanup();
 	}
 	platform_device_del(msipf_device);
@@ -976,7 +958,6 @@ static void __exit msi_cleanup(void)
 	if (load_scm_model) {
 		i8042_remove_filter(msi_laptop_i8042_filter);
 		msi_laptop_input_destroy();
-		cancel_delayed_work_sync(&msi_rfkill_work);
 		rfkill_cleanup();
 	}
 
-- 
1.8.0

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

* [PATCH 4/6] msi-laptop: Add MSI Wind U90/U100 support
  2012-11-24 22:28 [PATCH 0/6] msi-laptop: Add MSI Wind U90/U100 support Maxim Mikityanskiy
                   ` (2 preceding siblings ...)
  2012-11-24 22:28 ` [PATCH 3/6] msi-laptop: Code cleanups Maxim Mikityanskiy
@ 2012-11-24 22:28 ` Maxim Mikityanskiy
  2012-11-28  3:32   ` joeyli
  2012-11-24 22:29 ` [PATCH 5/6] msi-laptop: Add missing ABI documentation Maxim Mikityanskiy
  2012-11-24 22:29 ` [PATCH 6/6] Add MSI Wind WMI support Maxim Mikityanskiy
  5 siblings, 1 reply; 25+ messages in thread
From: Maxim Mikityanskiy @ 2012-11-24 22:28 UTC (permalink / raw)
  To: platform-driver-x86, jlee, mjg; +Cc: Maxim Mikityanskiy

Add MSI Wind U90/U100 to separate DMI table, add U90/U100 specific
workarounds and add some missing EC features support such as basic fan
control, turbo and ECO modes and touchpad state.

Signed-off-by: Maxim Mikityanskiy <maxtram95@gmail.com>
---
 drivers/platform/x86/msi-laptop.c | 199 ++++++++++++++++++++++++++++++++------
 1 file changed, 171 insertions(+), 28 deletions(-)

diff --git a/drivers/platform/x86/msi-laptop.c b/drivers/platform/x86/msi-laptop.c
index 3b6f494..16e9863 100644
--- a/drivers/platform/x86/msi-laptop.c
+++ b/drivers/platform/x86/msi-laptop.c
@@ -82,8 +82,19 @@
 #define MSI_STANDARD_EC_SCM_LOAD_ADDRESS	0x2d
 #define MSI_STANDARD_EC_SCM_LOAD_MASK		(1 << 0)
 
-#define MSI_STANDARD_EC_TOUCHPAD_ADDRESS	0xe4
+#define MSI_STANDARD_EC_POWER_ADDRESS		0xe4
+/* Power LED is orange - Turbo mode */
+#define MSI_STANDARD_EC_TURBO_MASK		(1 << 1)
+/* Power LED is green - ECO mode */
+#define MSI_STANDARD_EC_ECO_MASK		(1 << 3)
+/* Touchpad is turned on */
 #define MSI_STANDARD_EC_TOUCHPAD_MASK		(1 << 4)
+/* If this bit != bit 1, turbo mode can't be toggled */
+#define MSI_STANDARD_EC_TURBO_COOLDOWN_MASK	(1 << 7)
+
+#define MSI_STANDARD_EC_FAN_ADDRESS		0x33
+/* If zero, fan rotates at maximal speed */
+#define MSI_STANDARD_EC_AUTOFAN_MASK		(1 << 0)
 
 #ifdef CONFIG_PM_SLEEP
 static int msi_laptop_resume(struct device *device);
@@ -123,6 +134,13 @@ static int threeg_exists;
  * e.g. MSI N034 netbook
  */
 static bool load_scm_model;
+
+/* Some MSI Wind netbooks (e.g. MSI Wind U100) need loading SCM to get some
+ * features working (e.g. ECO mode), but we cannot change Wlan/Bluetooth state
+ * in software and we can only read its state.
+ */
+static bool ec_read_only;
+
 static struct rfkill *rfk_wlan, *rfk_bluetooth, *rfk_threeg;
 
 /* Hardware access */
@@ -195,6 +213,9 @@ static ssize_t set_device_state(const char *buf, size_t count, u8 mask)
 	if (sscanf(buf, "%i", &status) != 1 || (status < 0 || status > 1))
 		return -EINVAL;
 
+	if (ec_read_only)
+		return -EOPNOTSUPP;
+
 	/* read current device state */
 	result = ec_read(MSI_STANDARD_EC_COMMAND_ADDRESS, &rdata);
 	if (result < 0)
@@ -417,18 +438,115 @@ static ssize_t store_auto_brightness(struct device *dev,
 	return count;
 }
 
+static ssize_t show_touchpad(struct device *dev,
+	struct device_attribute *attr, char *buf)
+{
+
+	u8 rdata;
+	int result;
+
+	result = ec_read(MSI_STANDARD_EC_POWER_ADDRESS, &rdata);
+	if (result < 0)
+		return result;
+
+	return sprintf(buf, "%i\n", !!(rdata & MSI_STANDARD_EC_TOUCHPAD_MASK));
+}
+
+static ssize_t show_turbo(struct device *dev,
+	struct device_attribute *attr, char *buf)
+{
+
+	u8 rdata;
+	int result;
+
+	result = ec_read(MSI_STANDARD_EC_POWER_ADDRESS, &rdata);
+	if (result < 0)
+		return result;
+
+	return sprintf(buf, "%i\n", !!(rdata & MSI_STANDARD_EC_TURBO_MASK));
+}
+
+static ssize_t show_eco(struct device *dev,
+	struct device_attribute *attr, char *buf)
+{
+
+	u8 rdata;
+	int result;
+
+	result = ec_read(MSI_STANDARD_EC_POWER_ADDRESS, &rdata);
+	if (result < 0)
+		return result;
+
+	return sprintf(buf, "%i\n", !!(rdata & MSI_STANDARD_EC_ECO_MASK));
+}
+
+static ssize_t show_turbo_cooldown(struct device *dev,
+	struct device_attribute *attr, char *buf)
+{
+
+	u8 rdata;
+	int result;
+
+	result = ec_read(MSI_STANDARD_EC_POWER_ADDRESS, &rdata);
+	if (result < 0)
+		return result;
+
+	return sprintf(buf, "%i\n", (!!(rdata & MSI_STANDARD_EC_TURBO_MASK)) |
+		(!!(rdata & MSI_STANDARD_EC_TURBO_COOLDOWN_MASK) << 1));
+}
+
+static ssize_t show_auto_fan(struct device *dev,
+	struct device_attribute *attr, char *buf)
+{
+
+	u8 rdata;
+	int result;
+
+	result = ec_read(MSI_STANDARD_EC_FAN_ADDRESS, &rdata);
+	if (result < 0)
+		return result;
+
+	return sprintf(buf, "%i\n", !!(rdata & MSI_STANDARD_EC_AUTOFAN_MASK));
+}
+
+static ssize_t store_auto_fan(struct device *dev,
+	struct device_attribute *attr, const char *buf, size_t count)
+{
+
+	int enable, result;
+
+	if (sscanf(buf, "%i", &enable) != 1 || (enable != (enable & 1)))
+		return -EINVAL;
+
+	result = ec_write(MSI_STANDARD_EC_FAN_ADDRESS, enable);
+	if (result < 0)
+		return result;
+
+	return count;
+}
+
 static DEVICE_ATTR(lcd_level, 0644, show_lcd_level, store_lcd_level);
 static DEVICE_ATTR(auto_brightness, 0644, show_auto_brightness,
 		   store_auto_brightness);
 static DEVICE_ATTR(bluetooth, 0444, show_bluetooth, NULL);
 static DEVICE_ATTR(wlan, 0444, show_wlan, NULL);
 static DEVICE_ATTR(threeg, 0444, show_threeg, NULL);
+static DEVICE_ATTR(touchpad, 0444, show_touchpad, NULL);
+static DEVICE_ATTR(turbo_mode, 0444, show_turbo, NULL);
+static DEVICE_ATTR(eco_mode, 0444, show_eco, NULL);
+static DEVICE_ATTR(turbo_cooldown, 0444, show_turbo_cooldown, NULL);
+static DEVICE_ATTR(auto_fan, 0644, show_auto_fan, store_auto_fan);
 
 static struct attribute *msipf_attributes[] = {
 	&dev_attr_lcd_level.attr,
 	&dev_attr_auto_brightness.attr,
 	&dev_attr_bluetooth.attr,
 	&dev_attr_wlan.attr,
+	&dev_attr_touchpad.attr,
+	&dev_attr_turbo_mode.attr,
+	&dev_attr_eco_mode.attr,
+	&dev_attr_turbo_cooldown.attr,
+	&dev_attr_auto_fan.attr,
 	NULL
 };
 
@@ -553,6 +671,19 @@ static struct dmi_system_id __initdata msi_load_scm_models_dmi_table[] = {
 	{ }
 };
 
+static struct dmi_system_id __initdata msi_load_scm_ro_models_dmi_table[] = {
+	{
+		.ident = "MSI U90/U100",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR,
+				"MICRO-STAR INTERNATIONAL CO., LTD"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "U90/U100"),
+		},
+		.callback = dmi_check_cb
+	},
+	{ }
+};
+
 static int rfkill_bluetooth_set(void *data, bool blocked)
 {
 	/* Do something with blocked...*/
@@ -560,32 +691,26 @@ static int rfkill_bluetooth_set(void *data, bool blocked)
 	 * blocked == false is on
 	 * blocked == true is off
 	 */
-	if (blocked)
-		set_device_state("0", 0, MSI_STANDARD_EC_BLUETOOTH_MASK);
-	else
-		set_device_state("1", 0, MSI_STANDARD_EC_BLUETOOTH_MASK);
+	int result = set_device_state(blocked ? "0" : "1", 0,
+		MSI_STANDARD_EC_BLUETOOTH_MASK);
 
-	return 0;
+	return result < 0 ? result : 0;
 }
 
 static int rfkill_wlan_set(void *data, bool blocked)
 {
-	if (blocked)
-		set_device_state("0", 0, MSI_STANDARD_EC_WLAN_MASK);
-	else
-		set_device_state("1", 0, MSI_STANDARD_EC_WLAN_MASK);
+	int result = set_device_state(blocked ? "0" : "1", 0,
+		MSI_STANDARD_EC_WLAN_MASK);
 
-	return 0;
+	return result < 0 ? result : 0;
 }
 
 static int rfkill_threeg_set(void *data, bool blocked)
 {
-	if (blocked)
-		set_device_state("0", 0, MSI_STANDARD_EC_3G_MASK);
-	else
-		set_device_state("1", 0, MSI_STANDARD_EC_3G_MASK);
+	int result = set_device_state(blocked ? "0" : "1", 0,
+		MSI_STANDARD_EC_3G_MASK);
 
-	return 0;
+	return result < 0 ? result : 0;
 }
 
 static const struct rfkill_ops rfkill_bluetooth_ops = {
@@ -600,16 +725,24 @@ static const struct rfkill_ops rfkill_threeg_ops = {
 	.set_block = rfkill_threeg_set
 };
 
+static bool msi_rfkill_set_state(struct rfkill *rfkill, bool blocked)
+{
+	if (ec_read_only)
+		return rfkill_set_hw_state(rfkill, blocked);
+	else
+		return rfkill_set_sw_state(rfkill, blocked);
+}
+
 static void msi_update_rfkill(struct work_struct *ignored)
 {
 	get_wireless_state_ec_standard();
 
 	if (rfk_wlan)
-		rfkill_set_sw_state(rfk_wlan, !wlan_s);
+		msi_rfkill_set_state(rfk_wlan, !wlan_s);
 	if (rfk_bluetooth)
-		rfkill_set_sw_state(rfk_bluetooth, !bluetooth_s);
+		msi_rfkill_set_state(rfk_bluetooth, !bluetooth_s);
 	if (rfk_threeg)
-		rfkill_set_sw_state(rfk_threeg, !threeg_s);
+		msi_rfkill_set_state(rfk_threeg, !threeg_s);
 }
 static DECLARE_WORK(msi_rfkill_work, msi_update_rfkill);
 
@@ -638,7 +771,7 @@ static void msi_send_touchpad_key(struct work_struct *ignored)
 	u8 rdata;
 	int result;
 
-	result = ec_read(MSI_STANDARD_EC_TOUCHPAD_ADDRESS, &rdata);
+	result = ec_read(MSI_STANDARD_EC_POWER_ADDRESS, &rdata);
 	if (result < 0)
 		return;
 
@@ -802,13 +935,15 @@ static int __init load_scm_model_init(struct platform_device *sdev)
 	u8 data;
 	int result;
 
-	/* allow userland write sysfs file  */
-	dev_attr_bluetooth.store = store_bluetooth;
-	dev_attr_wlan.store = store_wlan;
-	dev_attr_threeg.store = store_threeg;
-	dev_attr_bluetooth.attr.mode |= S_IWUSR;
-	dev_attr_wlan.attr.mode |= S_IWUSR;
-	dev_attr_threeg.attr.mode |= S_IWUSR;
+	if (!ec_read_only) {
+		/* allow userland write sysfs file  */
+		dev_attr_bluetooth.store = store_bluetooth;
+		dev_attr_wlan.store = store_wlan;
+		dev_attr_threeg.store = store_threeg;
+		dev_attr_bluetooth.attr.mode |= S_IWUSR;
+		dev_attr_wlan.attr.mode |= S_IWUSR;
+		dev_attr_threeg.attr.mode |= S_IWUSR;
+	}
 
 	/* disable hardware control by fn key */
 	result = ec_read(MSI_STANDARD_EC_SCM_LOAD_ADDRESS, &data);
@@ -860,8 +995,15 @@ static int __init msi_init(void)
 	if (force || dmi_check_system(msi_dmi_table))
 		old_ec_model = 1;
 
-	if (!old_ec_model)
+	if (!old_ec_model) {
 		get_threeg_exists();
+		if (dmi_check_system(msi_load_scm_models_dmi_table))
+			load_scm_model = 1;
+		else if (dmi_check_system(msi_load_scm_ro_models_dmi_table)) {
+			load_scm_model = 1;
+			ec_read_only = 1;
+		}
+	}
 
 	if (!old_ec_model && dmi_check_system(msi_load_scm_models_dmi_table))
 		load_scm_model = 1;
@@ -992,3 +1134,4 @@ MODULE_ALIAS("dmi:*:svnMICRO-STARINTERNATIONAL*:pnMS-N051:*");
 MODULE_ALIAS("dmi:*:svnMICRO-STARINTERNATIONAL*:pnMS-N014:*");
 MODULE_ALIAS("dmi:*:svnMicro-StarInternational*:pnCR620:*");
 MODULE_ALIAS("dmi:*:svnMicro-StarInternational*:pnU270series:*");
+MODULE_ALIAS("dmi:*:svnMICRO-STARINTERNATIONAL*:pnU90/U100:*");
-- 
1.8.0

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

* [PATCH 5/6] msi-laptop: Add missing ABI documentation
  2012-11-24 22:28 [PATCH 0/6] msi-laptop: Add MSI Wind U90/U100 support Maxim Mikityanskiy
                   ` (3 preceding siblings ...)
  2012-11-24 22:28 ` [PATCH 4/6] msi-laptop: Add MSI Wind U90/U100 support Maxim Mikityanskiy
@ 2012-11-24 22:29 ` Maxim Mikityanskiy
  2012-11-28  3:36   ` joeyli
  2012-11-24 22:29 ` [PATCH 6/6] Add MSI Wind WMI support Maxim Mikityanskiy
  5 siblings, 1 reply; 25+ messages in thread
From: Maxim Mikityanskiy @ 2012-11-24 22:29 UTC (permalink / raw)
  To: platform-driver-x86, jlee, mjg; +Cc: Maxim Mikityanskiy


Signed-off-by: Maxim Mikityanskiy <maxtram95@gmail.com>
---
 .../ABI/testing/sysfs-platform-msi-laptop          | 83 ++++++++++++++++++++++
 1 file changed, 83 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-platform-msi-laptop

diff --git a/Documentation/ABI/testing/sysfs-platform-msi-laptop b/Documentation/ABI/testing/sysfs-platform-msi-laptop
new file mode 100644
index 0000000..f621027
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-platform-msi-laptop
@@ -0,0 +1,83 @@
+What:		/sys/devices/platform/msi-laptop-pf/lcd_level
+Date:		Oct 2006
+KernelVersion:	2.6.19
+Contact:	"Lennart Poettering <mzxreary@0pointer.de>"
+Description:
+		Screen brightness: contains a single integer in the range 0..8.
+
+What:		/sys/devices/platform/msi-laptop-pf/auto_brightness
+Date:		Oct 2006
+KernelVersion:	2.6.19
+Contact:	"Lennart Poettering <mzxreary@0pointer.de>"
+Description:
+		Enable automatic brightness control: contains either 0 or 1. If
+		set to 1 the hardware adjusts the screen brightness
+		automatically when the power cord is plugged/unplugged.
+
+What:		/sys/devices/platform/msi-laptop-pf/wlan
+Date:		Oct 2006
+KernelVersion:	2.6.19
+Contact:	"Lennart Poettering <mzxreary@0pointer.de>"
+Description:
+		WLAN subsystem enabled: contains either 0 or 1.
+
+What:		/sys/devices/platform/msi-laptop-pf/bluetooth
+Date:		Oct 2006
+KernelVersion:	2.6.19
+Contact:	"Lennart Poettering <mzxreary@0pointer.de>"
+Description:
+		Bluetooth subsystem enabled: contains either 0 or 1. Please
+		note that this file is constantly 0 if no Bluetooth hardware is
+		available.
+
+What:		/sys/devices/platform/msi-laptop-pf/touchpad
+Date:		Nov 2012
+KernelVersion:	3.7
+Contact:	"Maxim Mikityanskiy <maxtram95@gmail.com>"
+Description:
+		Contains either 0 or 1 and indicates if touchpad is turned on.
+		Touchpad state can only be toggled by pressing Fn+F3.
+
+What:		/sys/devices/platform/msi-laptop-pf/turbo_mode
+Date:		Nov 2012
+KernelVersion:	3.7
+Contact:	"Maxim Mikityanskiy <maxtram95@gmail.com>"
+Description:
+		Contains either 0 or 1 and indicates if turbo mode is turned
+		on. In turbo mode power LED is orange and processor is
+		overclocked. Turbo mode is available only if charging. It is
+		only possible to toggle turbo mode state by pressing Fn+F10,
+		and there is a few seconds cooldown between subsequent toggles.
+		If user presses Fn+F10 too frequent, turbo mode state is not
+		changed.
+
+What:		/sys/devices/platform/msi-laptop-pf/eco_mode
+Date:		Nov 2012
+KernelVersion:	3.7
+Contact:	"Maxim Mikityanskiy <maxtram95@gmail.com>"
+Description:
+		Contains either 0 or 1 and indicates if ECO mode is turned on.
+		In ECO mode power LED is green and userspace should do some
+		powersaving actions. ECO mode is available only on battery
+		power. ECO mode can only be toggled by pressing Fn+F10.
+
+What:		/sys/devices/platform/msi-laptop-pf/turbo_cooldown
+Date:		Nov 2012
+KernelVersion:	3.7
+Contact:	"Maxim Mikityanskiy <maxtram95@gmail.com>"
+Description:
+		Contains value in range 0..3:
+			* 0 -> Turbo mode is off
+			* 1 -> Turbo mode is on, cannot be turned off yet
+			* 2 -> Turbo mode is off, cannot be turned on yet
+			* 3 -> Turbo mode is on
+
+What:		/sys/devices/platform/msi-laptop-pf/auto_fan
+Date:		Nov 2012
+KernelVersion:	3.7
+Contact:	"Maxim Mikityanskiy <maxtram95@gmail.com>"
+Description:
+		Contains either 0 or 1 and indicates if fan speed is controlled
+		automatically (1) or fan runs at maximal speed (0). Can be
+		toggled in software.
+
-- 
1.8.0

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

* [PATCH 6/6] Add MSI Wind WMI support
  2012-11-24 22:28 [PATCH 0/6] msi-laptop: Add MSI Wind U90/U100 support Maxim Mikityanskiy
                   ` (4 preceding siblings ...)
  2012-11-24 22:29 ` [PATCH 5/6] msi-laptop: Add missing ABI documentation Maxim Mikityanskiy
@ 2012-11-24 22:29 ` Maxim Mikityanskiy
  2012-11-27  8:14   ` joeyli
  5 siblings, 1 reply; 25+ messages in thread
From: Maxim Mikityanskiy @ 2012-11-24 22:29 UTC (permalink / raw)
  To: platform-driver-x86, jlee, mjg; +Cc: Maxim Mikityanskiy

Create a driver to support WMI found on MSI Wind laptops. This driver
allows us to receive some additional keycodes for keys that are not
handled without this driver.

Signed-off-by: Maxim Mikityanskiy <maxtram95@gmail.com>
---
 drivers/platform/x86/Kconfig        |  13 +++
 drivers/platform/x86/Makefile       |   1 +
 drivers/platform/x86/msi-wind-wmi.c | 169 ++++++++++++++++++++++++++++++++++++
 3 files changed, 183 insertions(+)
 create mode 100644 drivers/platform/x86/msi-wind-wmi.c

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index c86bae8..46f269e5 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -561,6 +561,19 @@ config MSI_WMI
 	 To compile this driver as a module, choose M here: the module will
 	 be called msi-wmi.
 
+config MSI_WIND_WMI
+	tristate "MSI Wind WMI Driver"
+	depends on ACPI_WMI
+	depends on INPUT
+	select INPUT_SPARSEKMAP
+	---help---
+	 Say Y here if you want to support WMI-based hotkeys on MSI Wind
+	 laptops. MSI Wind WMI differs from WMI found on other MSI laptops, so
+	 say Y here if you have MSI Wind, otherwise select "MSI WMI extras".
+
+	 To compile this driver as a module, choose M here: the module will
+	 be called msi-wind-wmi.
+
 config TOPSTAR_LAPTOP
 	tristate "Topstar Laptop Extras"
 	depends on ACPI
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index bf7e4f9..44389c0 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -30,6 +30,7 @@ obj-$(CONFIG_INTEL_MENLOW)	+= intel_menlow.o
 obj-$(CONFIG_ACPI_WMI)		+= wmi.o
 obj-$(CONFIG_MSI_WMI)		+= msi-wmi.o
 obj-$(CONFIG_TOPSTAR_LAPTOP)	+= topstar-laptop.o
+obj-$(CONFIG_MSI_WIND_WMI)	+= msi-wind-wmi.o
 
 # toshiba_acpi must link after wmi to ensure that wmi devices are found
 # before toshiba_acpi initializes
diff --git a/drivers/platform/x86/msi-wind-wmi.c b/drivers/platform/x86/msi-wind-wmi.c
new file mode 100644
index 0000000..4f19434
--- /dev/null
+++ b/drivers/platform/x86/msi-wind-wmi.c
@@ -0,0 +1,169 @@
+/*
+ * MSI Wind WMI hotkeys
+ *
+ * Copyright (C) 2012 Maxim Mikityanskiy <maxtram95@gmail.com>
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/module.h>
+#include <linux/input.h>
+#include <linux/input/sparse-keymap.h>
+#include <linux/acpi.h>
+
+MODULE_AUTHOR("Maxim Mikityanskiy <maxtram95@gmail.com>");
+MODULE_DESCRIPTION("MSI Wind laptop WMI hotkeys driver");
+MODULE_LICENSE("GPL");
+
+#define WMI_EVENT_GUID "5B3CC38A-40D9-7245-8AE6-1145B751BE3F"
+
+MODULE_ALIAS("wmi:" WMI_EVENT_GUID);
+
+/* Fn+F3 touchpad toggle */
+#define WIND_KEY_TOUCHPAD	0x08
+/* Fn+F11 Bluetooth toggle */
+#define WIND_KEY_BLUETOOTH	0x56
+/* Fn+F6 webcam toggle */
+#define WIND_KEY_CAMERA		0x57
+/* Fn+F11 Wi-Fi toggle */
+#define WIND_KEY_WLAN		0x5f
+/* Fn+F10 turbo mode toggle */
+#define WIND_KEY_TURBO		0x60
+/* Fn+F10 ECO mode toggle */
+#define WIND_KEY_ECO		0x69
+
+static struct key_entry wind_keymap[] = {
+	/* These keys work without WMI. Ignore them to avoid double keycodes */
+	{ KE_IGNORE, WIND_KEY_TOUCHPAD,		{ KEY_TOUCHPAD_TOGGLE } },
+	{ KE_IGNORE, WIND_KEY_BLUETOOTH,	{ KEY_BLUETOOTH } },
+	{ KE_IGNORE, WIND_KEY_CAMERA,		{ KEY_CAMERA } },
+	{ KE_IGNORE, WIND_KEY_WLAN,		{ KEY_WLAN } },
+	/* These are keys that should be handled via WMI */
+	{ KE_KEY,    WIND_KEY_TURBO,		{ KEY_PROG1 } },
+	{ KE_KEY,    WIND_KEY_ECO,		{ KEY_PROG2 } },
+	{ KE_END, 0 }
+};
+
+static struct input_dev *wind_input_dev;
+
+static void wind_wmi_notify(u32 value, void *context)
+{
+	struct acpi_buffer response = {
+		.length		= ACPI_ALLOCATE_BUFFER,
+		.pointer	= NULL
+	};
+	acpi_status status = wmi_get_event_data(value, &response);
+	union acpi_object *obj = response.pointer;
+	int code;
+
+	if (status != AE_OK) {
+		pr_warn("Bad event status %#x\n", status);
+		return;
+	}
+
+	if (!obj || obj->type != ACPI_TYPE_INTEGER) {
+		pr_warn("Unknown event received\n");
+		goto wind_wmi_notify_free;
+	}
+
+	code = obj->integer.value;
+	pr_debug("Event code: %#x\n", code);
+
+	if (code == 0x00 || code == 0x62 || code == 0x63) {
+		/* Unknown events - drop them for now */
+		goto wind_wmi_notify_free;
+	}
+
+	if (!sparse_keymap_report_event(wind_input_dev, code, 1, true))
+		pr_warn("Unknown key %#x pressed\n", code);
+
+wind_wmi_notify_free:
+	kfree(response.pointer);
+}
+
+static int __init wind_input_setup(void)
+{
+	int err;
+
+	wind_input_dev = input_allocate_device();
+	if (!wind_input_dev)
+		return -ENOMEM;
+
+	wind_input_dev->name = "MSI WMI hotkeys";
+	wind_input_dev->phys = "wmi/input0";
+	wind_input_dev->id.bustype = BUS_HOST;
+
+	err = sparse_keymap_setup(wind_input_dev, wind_keymap, NULL);
+	if (err)
+		goto wind_input_setup_free_dev;
+
+	err = input_register_device(wind_input_dev);
+	if (err)
+		goto wind_input_setup_free_keymap;
+
+	return 0;
+
+wind_input_setup_free_keymap:
+	sparse_keymap_free(wind_input_dev);
+wind_input_setup_free_dev:
+	input_free_device(wind_input_dev);
+	return err;
+}
+
+static int __init wind_wmi_init(void)
+{
+	int err;
+
+	if (!wmi_has_guid(WMI_EVENT_GUID)) {
+		pr_err("No MSI Wind WMI found\n");
+		return -ENODEV;
+	}
+
+	err = wind_input_setup();
+	if (err) {
+		pr_err("Unable to setup input device\n");
+		return err;
+	}
+
+	if (ACPI_FAILURE(wmi_install_notify_handler(WMI_EVENT_GUID,
+		wind_wmi_notify, NULL))) {
+		pr_err("Unable to install WMI notify handler\n");
+		err = -EIO;
+		goto wind_wmi_init_unregister_input;
+	}
+
+	pr_debug("Event handler installed\n");
+
+	return 0;
+
+wind_wmi_init_unregister_input:
+	input_unregister_device(wind_input_dev);
+	sparse_keymap_free(wind_input_dev);
+	input_free_device(wind_input_dev);
+	return err;
+}
+
+static void __exit wind_wmi_exit(void)
+{
+	wmi_remove_notify_handler(WMI_EVENT_GUID);
+	input_unregister_device(wind_input_dev);
+	sparse_keymap_free(wind_input_dev);
+	input_free_device(wind_input_dev);
+}
+
+module_init(wind_wmi_init);
+module_exit(wind_wmi_exit);
-- 
1.8.0

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

* Re: [PATCH 6/6] Add MSI Wind WMI support
  2012-11-24 22:29 ` [PATCH 6/6] Add MSI Wind WMI support Maxim Mikityanskiy
@ 2012-11-27  8:14   ` joeyli
  2012-11-27  8:50     ` Corentin Chary
  0 siblings, 1 reply; 25+ messages in thread
From: joeyli @ 2012-11-27  8:14 UTC (permalink / raw)
  To: Maxim Mikityanskiy; +Cc: platform-driver-x86, jlee, mjg

Hi Maxim, 

於 日,2012-11-25 於 00:29 +0200,Maxim Mikityanskiy 提到:
> Create a driver to support WMI found on MSI Wind laptops. This driver
> allows us to receive some additional keycodes for keys that are not
> handled without this driver.
> 
> Signed-off-by: Maxim Mikityanskiy <maxtram95@gmail.com>

I simply review this patch and looks there have many code the same with
msi-wmi.c. Does there have any reason didn't direct patch msi-wmi but
need generate a new driver?


Thanks a lot!
Joey Lee

> ---
>  drivers/platform/x86/Kconfig        |  13 +++
>  drivers/platform/x86/Makefile       |   1 +
>  drivers/platform/x86/msi-wind-wmi.c | 169 ++++++++++++++++++++++++++++++++++++
>  3 files changed, 183 insertions(+)
>  create mode 100644 drivers/platform/x86/msi-wind-wmi.c
> 
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index c86bae8..46f269e5 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -561,6 +561,19 @@ config MSI_WMI
>  	 To compile this driver as a module, choose M here: the module will
>  	 be called msi-wmi.
>  
> +config MSI_WIND_WMI
> +	tristate "MSI Wind WMI Driver"
> +	depends on ACPI_WMI
> +	depends on INPUT
> +	select INPUT_SPARSEKMAP
> +	---help---
> +	 Say Y here if you want to support WMI-based hotkeys on MSI Wind
> +	 laptops. MSI Wind WMI differs from WMI found on other MSI laptops, so
> +	 say Y here if you have MSI Wind, otherwise select "MSI WMI extras".
> +
> +	 To compile this driver as a module, choose M here: the module will
> +	 be called msi-wind-wmi.
> +
>  config TOPSTAR_LAPTOP
>  	tristate "Topstar Laptop Extras"
>  	depends on ACPI
> diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
> index bf7e4f9..44389c0 100644
> --- a/drivers/platform/x86/Makefile
> +++ b/drivers/platform/x86/Makefile
> @@ -30,6 +30,7 @@ obj-$(CONFIG_INTEL_MENLOW)	+= intel_menlow.o
>  obj-$(CONFIG_ACPI_WMI)		+= wmi.o
>  obj-$(CONFIG_MSI_WMI)		+= msi-wmi.o
>  obj-$(CONFIG_TOPSTAR_LAPTOP)	+= topstar-laptop.o
> +obj-$(CONFIG_MSI_WIND_WMI)	+= msi-wind-wmi.o
>  
>  # toshiba_acpi must link after wmi to ensure that wmi devices are found
>  # before toshiba_acpi initializes
> diff --git a/drivers/platform/x86/msi-wind-wmi.c b/drivers/platform/x86/msi-wind-wmi.c
> new file mode 100644
> index 0000000..4f19434
> --- /dev/null
> +++ b/drivers/platform/x86/msi-wind-wmi.c
> @@ -0,0 +1,169 @@
> +/*
> + * MSI Wind WMI hotkeys
> + *
> + * Copyright (C) 2012 Maxim Mikityanskiy <maxtram95@gmail.com>
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation; either version 2 of the License, or
> + *  (at your option) any later version.
> + *
> + *  This program is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with this program; if not, write to the Free Software
> + *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
> + */
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <linux/module.h>
> +#include <linux/input.h>
> +#include <linux/input/sparse-keymap.h>
> +#include <linux/acpi.h>
> +
> +MODULE_AUTHOR("Maxim Mikityanskiy <maxtram95@gmail.com>");
> +MODULE_DESCRIPTION("MSI Wind laptop WMI hotkeys driver");
> +MODULE_LICENSE("GPL");
> +
> +#define WMI_EVENT_GUID "5B3CC38A-40D9-7245-8AE6-1145B751BE3F"
> +
> +MODULE_ALIAS("wmi:" WMI_EVENT_GUID);
> +
> +/* Fn+F3 touchpad toggle */
> +#define WIND_KEY_TOUCHPAD	0x08
> +/* Fn+F11 Bluetooth toggle */
> +#define WIND_KEY_BLUETOOTH	0x56
> +/* Fn+F6 webcam toggle */
> +#define WIND_KEY_CAMERA		0x57
> +/* Fn+F11 Wi-Fi toggle */
> +#define WIND_KEY_WLAN		0x5f
> +/* Fn+F10 turbo mode toggle */
> +#define WIND_KEY_TURBO		0x60
> +/* Fn+F10 ECO mode toggle */
> +#define WIND_KEY_ECO		0x69
> +
> +static struct key_entry wind_keymap[] = {
> +	/* These keys work without WMI. Ignore them to avoid double keycodes */
> +	{ KE_IGNORE, WIND_KEY_TOUCHPAD,		{ KEY_TOUCHPAD_TOGGLE } },
> +	{ KE_IGNORE, WIND_KEY_BLUETOOTH,	{ KEY_BLUETOOTH } },
> +	{ KE_IGNORE, WIND_KEY_CAMERA,		{ KEY_CAMERA } },
> +	{ KE_IGNORE, WIND_KEY_WLAN,		{ KEY_WLAN } },
> +	/* These are keys that should be handled via WMI */
> +	{ KE_KEY,    WIND_KEY_TURBO,		{ KEY_PROG1 } },
> +	{ KE_KEY,    WIND_KEY_ECO,		{ KEY_PROG2 } },
> +	{ KE_END, 0 }
> +};
> +
> +static struct input_dev *wind_input_dev;
> +
> +static void wind_wmi_notify(u32 value, void *context)
> +{
> +	struct acpi_buffer response = {
> +		.length		= ACPI_ALLOCATE_BUFFER,
> +		.pointer	= NULL
> +	};
> +	acpi_status status = wmi_get_event_data(value, &response);
> +	union acpi_object *obj = response.pointer;
> +	int code;
> +
> +	if (status != AE_OK) {
> +		pr_warn("Bad event status %#x\n", status);
> +		return;
> +	}
> +
> +	if (!obj || obj->type != ACPI_TYPE_INTEGER) {
> +		pr_warn("Unknown event received\n");
> +		goto wind_wmi_notify_free;
> +	}
> +
> +	code = obj->integer.value;
> +	pr_debug("Event code: %#x\n", code);
> +
> +	if (code == 0x00 || code == 0x62 || code == 0x63) {
> +		/* Unknown events - drop them for now */
> +		goto wind_wmi_notify_free;
> +	}
> +
> +	if (!sparse_keymap_report_event(wind_input_dev, code, 1, true))
> +		pr_warn("Unknown key %#x pressed\n", code);
> +
> +wind_wmi_notify_free:
> +	kfree(response.pointer);
> +}
> +
> +static int __init wind_input_setup(void)
> +{
> +	int err;
> +
> +	wind_input_dev = input_allocate_device();
> +	if (!wind_input_dev)
> +		return -ENOMEM;
> +
> +	wind_input_dev->name = "MSI WMI hotkeys";
> +	wind_input_dev->phys = "wmi/input0";
> +	wind_input_dev->id.bustype = BUS_HOST;
> +
> +	err = sparse_keymap_setup(wind_input_dev, wind_keymap, NULL);
> +	if (err)
> +		goto wind_input_setup_free_dev;
> +
> +	err = input_register_device(wind_input_dev);
> +	if (err)
> +		goto wind_input_setup_free_keymap;
> +
> +	return 0;
> +
> +wind_input_setup_free_keymap:
> +	sparse_keymap_free(wind_input_dev);
> +wind_input_setup_free_dev:
> +	input_free_device(wind_input_dev);
> +	return err;
> +}
> +
> +static int __init wind_wmi_init(void)
> +{
> +	int err;
> +
> +	if (!wmi_has_guid(WMI_EVENT_GUID)) {
> +		pr_err("No MSI Wind WMI found\n");
> +		return -ENODEV;
> +	}
> +
> +	err = wind_input_setup();
> +	if (err) {
> +		pr_err("Unable to setup input device\n");
> +		return err;
> +	}
> +
> +	if (ACPI_FAILURE(wmi_install_notify_handler(WMI_EVENT_GUID,
> +		wind_wmi_notify, NULL))) {
> +		pr_err("Unable to install WMI notify handler\n");
> +		err = -EIO;
> +		goto wind_wmi_init_unregister_input;
> +	}
> +
> +	pr_debug("Event handler installed\n");
> +
> +	return 0;
> +
> +wind_wmi_init_unregister_input:
> +	input_unregister_device(wind_input_dev);
> +	sparse_keymap_free(wind_input_dev);
> +	input_free_device(wind_input_dev);
> +	return err;
> +}
> +
> +static void __exit wind_wmi_exit(void)
> +{
> +	wmi_remove_notify_handler(WMI_EVENT_GUID);
> +	input_unregister_device(wind_input_dev);
> +	sparse_keymap_free(wind_input_dev);
> +	input_free_device(wind_input_dev);
> +}
> +
> +module_init(wind_wmi_init);
> +module_exit(wind_wmi_exit);

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

* Re: [PATCH 6/6] Add MSI Wind WMI support
  2012-11-27  8:14   ` joeyli
@ 2012-11-27  8:50     ` Corentin Chary
  2012-11-27 15:55       ` Maxim Mikityanskiy
  0 siblings, 1 reply; 25+ messages in thread
From: Corentin Chary @ 2012-11-27  8:50 UTC (permalink / raw)
  To: joeyli
  Cc: Maxim Mikityanskiy, platform-driver-x86, Lee, Chun-Yi, Matthew Garrett

On Tue, Nov 27, 2012 at 8:14 AM, joeyli <jlee@suse.com> wrote:
> Hi Maxim,
>
> 於 日,2012-11-25 於 00:29 +0200,Maxim Mikityanskiy 提到:
>> Create a driver to support WMI found on MSI Wind laptops. This driver
>> allows us to receive some additional keycodes for keys that are not
>> handled without this driver.
>>
>> Signed-off-by: Maxim Mikityanskiy <maxtram95@gmail.com>
>
> I simply review this patch and looks there have many code the same with
> msi-wmi.c. Does there have any reason didn't direct patch msi-wmi but
> need generate a new driver?

Right, if it's only a keymap and guid change, plus 1 or 2 quirks, it
may makes sense to be in msi-wind.c

>
> Thanks a lot!
> Joey Lee
>
>> ---
>>  drivers/platform/x86/Kconfig        |  13 +++
>>  drivers/platform/x86/Makefile       |   1 +
>>  drivers/platform/x86/msi-wind-wmi.c | 169 ++++++++++++++++++++++++++++++++++++
>>  3 files changed, 183 insertions(+)
>>  create mode 100644 drivers/platform/x86/msi-wind-wmi.c
>>
>> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
>> index c86bae8..46f269e5 100644
>> --- a/drivers/platform/x86/Kconfig
>> +++ b/drivers/platform/x86/Kconfig
>> @@ -561,6 +561,19 @@ config MSI_WMI
>>        To compile this driver as a module, choose M here: the module will
>>        be called msi-wmi.
>>
>> +config MSI_WIND_WMI
>> +     tristate "MSI Wind WMI Driver"
>> +     depends on ACPI_WMI
>> +     depends on INPUT
>> +     select INPUT_SPARSEKMAP
>> +     ---help---
>> +      Say Y here if you want to support WMI-based hotkeys on MSI Wind
>> +      laptops. MSI Wind WMI differs from WMI found on other MSI laptops, so
>> +      say Y here if you have MSI Wind, otherwise select "MSI WMI extras".
>> +
>> +      To compile this driver as a module, choose M here: the module will
>> +      be called msi-wind-wmi.
>> +
>>  config TOPSTAR_LAPTOP
>>       tristate "Topstar Laptop Extras"
>>       depends on ACPI
>> diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
>> index bf7e4f9..44389c0 100644
>> --- a/drivers/platform/x86/Makefile
>> +++ b/drivers/platform/x86/Makefile
>> @@ -30,6 +30,7 @@ obj-$(CONFIG_INTEL_MENLOW)  += intel_menlow.o
>>  obj-$(CONFIG_ACPI_WMI)               += wmi.o
>>  obj-$(CONFIG_MSI_WMI)                += msi-wmi.o
>>  obj-$(CONFIG_TOPSTAR_LAPTOP) += topstar-laptop.o
>> +obj-$(CONFIG_MSI_WIND_WMI)   += msi-wind-wmi.o
>>
>>  # toshiba_acpi must link after wmi to ensure that wmi devices are found
>>  # before toshiba_acpi initializes
>> diff --git a/drivers/platform/x86/msi-wind-wmi.c b/drivers/platform/x86/msi-wind-wmi.c
>> new file mode 100644
>> index 0000000..4f19434
>> --- /dev/null
>> +++ b/drivers/platform/x86/msi-wind-wmi.c
>> @@ -0,0 +1,169 @@
>> +/*
>> + * MSI Wind WMI hotkeys
>> + *
>> + * Copyright (C) 2012 Maxim Mikityanskiy <maxtram95@gmail.com>
>> + *
>> + *  This program is free software; you can redistribute it and/or modify
>> + *  it under the terms of the GNU General Public License as published by
>> + *  the Free Software Foundation; either version 2 of the License, or
>> + *  (at your option) any later version.
>> + *
>> + *  This program is distributed in the hope that it will be useful,
>> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + *  GNU General Public License for more details.
>> + *
>> + *  You should have received a copy of the GNU General Public License
>> + *  along with this program; if not, write to the Free Software
>> + *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
>> + */
>> +
>> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>> +
>> +#include <linux/module.h>
>> +#include <linux/input.h>
>> +#include <linux/input/sparse-keymap.h>
>> +#include <linux/acpi.h>
>> +
>> +MODULE_AUTHOR("Maxim Mikityanskiy <maxtram95@gmail.com>");
>> +MODULE_DESCRIPTION("MSI Wind laptop WMI hotkeys driver");
>> +MODULE_LICENSE("GPL");
>> +
>> +#define WMI_EVENT_GUID "5B3CC38A-40D9-7245-8AE6-1145B751BE3F"
>> +
>> +MODULE_ALIAS("wmi:" WMI_EVENT_GUID);
>> +
>> +/* Fn+F3 touchpad toggle */
>> +#define WIND_KEY_TOUCHPAD    0x08
>> +/* Fn+F11 Bluetooth toggle */
>> +#define WIND_KEY_BLUETOOTH   0x56
>> +/* Fn+F6 webcam toggle */
>> +#define WIND_KEY_CAMERA              0x57
>> +/* Fn+F11 Wi-Fi toggle */
>> +#define WIND_KEY_WLAN                0x5f
>> +/* Fn+F10 turbo mode toggle */
>> +#define WIND_KEY_TURBO               0x60
>> +/* Fn+F10 ECO mode toggle */
>> +#define WIND_KEY_ECO         0x69
>> +
>> +static struct key_entry wind_keymap[] = {
>> +     /* These keys work without WMI. Ignore them to avoid double keycodes */
>> +     { KE_IGNORE, WIND_KEY_TOUCHPAD,         { KEY_TOUCHPAD_TOGGLE } },
>> +     { KE_IGNORE, WIND_KEY_BLUETOOTH,        { KEY_BLUETOOTH } },
>> +     { KE_IGNORE, WIND_KEY_CAMERA,           { KEY_CAMERA } },
>> +     { KE_IGNORE, WIND_KEY_WLAN,             { KEY_WLAN } },
>> +     /* These are keys that should be handled via WMI */
>> +     { KE_KEY,    WIND_KEY_TURBO,            { KEY_PROG1 } },
>> +     { KE_KEY,    WIND_KEY_ECO,              { KEY_PROG2 } },
>> +     { KE_END, 0 }
>> +};
>> +
>> +static struct input_dev *wind_input_dev;
>> +
>> +static void wind_wmi_notify(u32 value, void *context)
>> +{
>> +     struct acpi_buffer response = {
>> +             .length         = ACPI_ALLOCATE_BUFFER,
>> +             .pointer        = NULL
>> +     };
>> +     acpi_status status = wmi_get_event_data(value, &response);
>> +     union acpi_object *obj = response.pointer;
>> +     int code;
>> +
>> +     if (status != AE_OK) {
>> +             pr_warn("Bad event status %#x\n", status);
>> +             return;
>> +     }
>> +
>> +     if (!obj || obj->type != ACPI_TYPE_INTEGER) {
>> +             pr_warn("Unknown event received\n");
>> +             goto wind_wmi_notify_free;
>> +     }
>> +
>> +     code = obj->integer.value;
>> +     pr_debug("Event code: %#x\n", code);
>> +
>> +     if (code == 0x00 || code == 0x62 || code == 0x63) {
>> +             /* Unknown events - drop them for now */
>> +             goto wind_wmi_notify_free;
>> +     }
>> +
>> +     if (!sparse_keymap_report_event(wind_input_dev, code, 1, true))
>> +             pr_warn("Unknown key %#x pressed\n", code);
>> +
>> +wind_wmi_notify_free:
>> +     kfree(response.pointer);
>> +}
>> +
>> +static int __init wind_input_setup(void)
>> +{
>> +     int err;
>> +
>> +     wind_input_dev = input_allocate_device();
>> +     if (!wind_input_dev)
>> +             return -ENOMEM;
>> +
>> +     wind_input_dev->name = "MSI WMI hotkeys";
>> +     wind_input_dev->phys = "wmi/input0";
>> +     wind_input_dev->id.bustype = BUS_HOST;
>> +
>> +     err = sparse_keymap_setup(wind_input_dev, wind_keymap, NULL);
>> +     if (err)
>> +             goto wind_input_setup_free_dev;
>> +
>> +     err = input_register_device(wind_input_dev);
>> +     if (err)
>> +             goto wind_input_setup_free_keymap;
>> +
>> +     return 0;
>> +
>> +wind_input_setup_free_keymap:
>> +     sparse_keymap_free(wind_input_dev);
>> +wind_input_setup_free_dev:
>> +     input_free_device(wind_input_dev);
>> +     return err;
>> +}
>> +
>> +static int __init wind_wmi_init(void)
>> +{
>> +     int err;
>> +
>> +     if (!wmi_has_guid(WMI_EVENT_GUID)) {
>> +             pr_err("No MSI Wind WMI found\n");
>> +             return -ENODEV;
>> +     }
>> +
>> +     err = wind_input_setup();
>> +     if (err) {
>> +             pr_err("Unable to setup input device\n");
>> +             return err;
>> +     }
>> +
>> +     if (ACPI_FAILURE(wmi_install_notify_handler(WMI_EVENT_GUID,
>> +             wind_wmi_notify, NULL))) {
>> +             pr_err("Unable to install WMI notify handler\n");
>> +             err = -EIO;
>> +             goto wind_wmi_init_unregister_input;
>> +     }
>> +
>> +     pr_debug("Event handler installed\n");
>> +
>> +     return 0;
>> +
>> +wind_wmi_init_unregister_input:
>> +     input_unregister_device(wind_input_dev);
>> +     sparse_keymap_free(wind_input_dev);
>> +     input_free_device(wind_input_dev);
>> +     return err;
>> +}
>> +
>> +static void __exit wind_wmi_exit(void)
>> +{
>> +     wmi_remove_notify_handler(WMI_EVENT_GUID);
>> +     input_unregister_device(wind_input_dev);
>> +     sparse_keymap_free(wind_input_dev);
>> +     input_free_device(wind_input_dev);
>> +}
>> +
>> +module_init(wind_wmi_init);
>> +module_exit(wind_wmi_exit);
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



--
Corentin Chary
http://xf.iksaif.net

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

* Re: [PATCH 6/6] Add MSI Wind WMI support
  2012-11-27  8:50     ` Corentin Chary
@ 2012-11-27 15:55       ` Maxim Mikityanskiy
  2012-11-27 16:34         ` Corentin Chary
  2012-11-27 23:20         ` joeyli
  0 siblings, 2 replies; 25+ messages in thread
From: Maxim Mikityanskiy @ 2012-11-27 15:55 UTC (permalink / raw)
  To: Corentin Chary; +Cc: joeyli, platform-driver-x86, Lee, Chun-Yi, Matthew Garrett

2012/11/27 Corentin Chary <corentin.chary@gmail.com>:
> On Tue, Nov 27, 2012 at 8:14 AM, joeyli <jlee@suse.com> wrote:
>> Hi Maxim,
>>
>> 於 日,2012-11-25 於 00:29 +0200,Maxim Mikityanskiy 提到:
>>> Create a driver to support WMI found on MSI Wind laptops. This driver
>>> allows us to receive some additional keycodes for keys that are not
>>> handled without this driver.
>>>
>>> Signed-off-by: Maxim Mikityanskiy <maxtram95@gmail.com>
>>
>> I simply review this patch and looks there have many code the same with
>> msi-wmi.c. Does there have any reason didn't direct patch msi-wmi but
>> need generate a new driver?
>
> Right, if it's only a keymap and guid change, plus 1 or 2 quirks, it
> may makes sense to be in msi-wind.c

Compare msi-wind-wmi.c and dell-wmi-aio.c. They look very similar,
even more similar than msi-wind-wmi.c and msi-wmi.c. Why do you
recommend to merge msi-wind-wmi features into msi-wmi, but not
dell-wmi-aio?

Look at dell-wmi.c, dell-wmi-aio.c and msi-wmi.c. They have lots of
common code and they differ only in GUIDs, keymap tables and some
quirks, but they are still different modules.

msi-wind-wmi is WMI driver that only supports hotkey handling. Each
WMI driver providing this feature contains such code, so there is
nothing strange in that msi-wmi driver also contains such code.
msi-wmi is a driver for fully different laptop with fully different
WMI. It also handles backlight brightness and needs a quirk for hotkey
handling. MSI Wind WMI does not send brightness change events and does
not need that quirk.

Do you still think I should merge msi-wind-wmi code into msi-wmi?

>
>>
>> Thanks a lot!
>> Joey Lee
>>
>>> ---
>>>  drivers/platform/x86/Kconfig        |  13 +++
>>>  drivers/platform/x86/Makefile       |   1 +
>>>  drivers/platform/x86/msi-wind-wmi.c | 169 ++++++++++++++++++++++++++++++++++++
>>>  3 files changed, 183 insertions(+)
>>>  create mode 100644 drivers/platform/x86/msi-wind-wmi.c
>>>
>>> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
>>> index c86bae8..46f269e5 100644
>>> --- a/drivers/platform/x86/Kconfig
>>> +++ b/drivers/platform/x86/Kconfig
>>> @@ -561,6 +561,19 @@ config MSI_WMI
>>>        To compile this driver as a module, choose M here: the module will
>>>        be called msi-wmi.
>>>
>>> +config MSI_WIND_WMI
>>> +     tristate "MSI Wind WMI Driver"
>>> +     depends on ACPI_WMI
>>> +     depends on INPUT
>>> +     select INPUT_SPARSEKMAP
>>> +     ---help---
>>> +      Say Y here if you want to support WMI-based hotkeys on MSI Wind
>>> +      laptops. MSI Wind WMI differs from WMI found on other MSI laptops, so
>>> +      say Y here if you have MSI Wind, otherwise select "MSI WMI extras".
>>> +
>>> +      To compile this driver as a module, choose M here: the module will
>>> +      be called msi-wind-wmi.
>>> +
>>>  config TOPSTAR_LAPTOP
>>>       tristate "Topstar Laptop Extras"
>>>       depends on ACPI
>>> diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
>>> index bf7e4f9..44389c0 100644
>>> --- a/drivers/platform/x86/Makefile
>>> +++ b/drivers/platform/x86/Makefile
>>> @@ -30,6 +30,7 @@ obj-$(CONFIG_INTEL_MENLOW)  += intel_menlow.o
>>>  obj-$(CONFIG_ACPI_WMI)               += wmi.o
>>>  obj-$(CONFIG_MSI_WMI)                += msi-wmi.o
>>>  obj-$(CONFIG_TOPSTAR_LAPTOP) += topstar-laptop.o
>>> +obj-$(CONFIG_MSI_WIND_WMI)   += msi-wind-wmi.o
>>>
>>>  # toshiba_acpi must link after wmi to ensure that wmi devices are found
>>>  # before toshiba_acpi initializes
>>> diff --git a/drivers/platform/x86/msi-wind-wmi.c b/drivers/platform/x86/msi-wind-wmi.c
>>> new file mode 100644
>>> index 0000000..4f19434
>>> --- /dev/null
>>> +++ b/drivers/platform/x86/msi-wind-wmi.c
>>> @@ -0,0 +1,169 @@
>>> +/*
>>> + * MSI Wind WMI hotkeys
>>> + *
>>> + * Copyright (C) 2012 Maxim Mikityanskiy <maxtram95@gmail.com>
>>> + *
>>> + *  This program is free software; you can redistribute it and/or modify
>>> + *  it under the terms of the GNU General Public License as published by
>>> + *  the Free Software Foundation; either version 2 of the License, or
>>> + *  (at your option) any later version.
>>> + *
>>> + *  This program is distributed in the hope that it will be useful,
>>> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>>> + *  GNU General Public License for more details.
>>> + *
>>> + *  You should have received a copy of the GNU General Public License
>>> + *  along with this program; if not, write to the Free Software
>>> + *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
>>> + */
>>> +
>>> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>>> +
>>> +#include <linux/module.h>
>>> +#include <linux/input.h>
>>> +#include <linux/input/sparse-keymap.h>
>>> +#include <linux/acpi.h>
>>> +
>>> +MODULE_AUTHOR("Maxim Mikityanskiy <maxtram95@gmail.com>");
>>> +MODULE_DESCRIPTION("MSI Wind laptop WMI hotkeys driver");
>>> +MODULE_LICENSE("GPL");
>>> +
>>> +#define WMI_EVENT_GUID "5B3CC38A-40D9-7245-8AE6-1145B751BE3F"
>>> +
>>> +MODULE_ALIAS("wmi:" WMI_EVENT_GUID);
>>> +
>>> +/* Fn+F3 touchpad toggle */
>>> +#define WIND_KEY_TOUCHPAD    0x08
>>> +/* Fn+F11 Bluetooth toggle */
>>> +#define WIND_KEY_BLUETOOTH   0x56
>>> +/* Fn+F6 webcam toggle */
>>> +#define WIND_KEY_CAMERA              0x57
>>> +/* Fn+F11 Wi-Fi toggle */
>>> +#define WIND_KEY_WLAN                0x5f
>>> +/* Fn+F10 turbo mode toggle */
>>> +#define WIND_KEY_TURBO               0x60
>>> +/* Fn+F10 ECO mode toggle */
>>> +#define WIND_KEY_ECO         0x69
>>> +
>>> +static struct key_entry wind_keymap[] = {
>>> +     /* These keys work without WMI. Ignore them to avoid double keycodes */
>>> +     { KE_IGNORE, WIND_KEY_TOUCHPAD,         { KEY_TOUCHPAD_TOGGLE } },
>>> +     { KE_IGNORE, WIND_KEY_BLUETOOTH,        { KEY_BLUETOOTH } },
>>> +     { KE_IGNORE, WIND_KEY_CAMERA,           { KEY_CAMERA } },
>>> +     { KE_IGNORE, WIND_KEY_WLAN,             { KEY_WLAN } },
>>> +     /* These are keys that should be handled via WMI */
>>> +     { KE_KEY,    WIND_KEY_TURBO,            { KEY_PROG1 } },
>>> +     { KE_KEY,    WIND_KEY_ECO,              { KEY_PROG2 } },
>>> +     { KE_END, 0 }
>>> +};
>>> +
>>> +static struct input_dev *wind_input_dev;
>>> +
>>> +static void wind_wmi_notify(u32 value, void *context)
>>> +{
>>> +     struct acpi_buffer response = {
>>> +             .length         = ACPI_ALLOCATE_BUFFER,
>>> +             .pointer        = NULL
>>> +     };
>>> +     acpi_status status = wmi_get_event_data(value, &response);
>>> +     union acpi_object *obj = response.pointer;
>>> +     int code;
>>> +
>>> +     if (status != AE_OK) {
>>> +             pr_warn("Bad event status %#x\n", status);
>>> +             return;
>>> +     }
>>> +
>>> +     if (!obj || obj->type != ACPI_TYPE_INTEGER) {
>>> +             pr_warn("Unknown event received\n");
>>> +             goto wind_wmi_notify_free;
>>> +     }
>>> +
>>> +     code = obj->integer.value;
>>> +     pr_debug("Event code: %#x\n", code);
>>> +
>>> +     if (code == 0x00 || code == 0x62 || code == 0x63) {
>>> +             /* Unknown events - drop them for now */
>>> +             goto wind_wmi_notify_free;
>>> +     }
>>> +
>>> +     if (!sparse_keymap_report_event(wind_input_dev, code, 1, true))
>>> +             pr_warn("Unknown key %#x pressed\n", code);
>>> +
>>> +wind_wmi_notify_free:
>>> +     kfree(response.pointer);
>>> +}
>>> +
>>> +static int __init wind_input_setup(void)
>>> +{
>>> +     int err;
>>> +
>>> +     wind_input_dev = input_allocate_device();
>>> +     if (!wind_input_dev)
>>> +             return -ENOMEM;
>>> +
>>> +     wind_input_dev->name = "MSI WMI hotkeys";
>>> +     wind_input_dev->phys = "wmi/input0";
>>> +     wind_input_dev->id.bustype = BUS_HOST;
>>> +
>>> +     err = sparse_keymap_setup(wind_input_dev, wind_keymap, NULL);
>>> +     if (err)
>>> +             goto wind_input_setup_free_dev;
>>> +
>>> +     err = input_register_device(wind_input_dev);
>>> +     if (err)
>>> +             goto wind_input_setup_free_keymap;
>>> +
>>> +     return 0;
>>> +
>>> +wind_input_setup_free_keymap:
>>> +     sparse_keymap_free(wind_input_dev);
>>> +wind_input_setup_free_dev:
>>> +     input_free_device(wind_input_dev);
>>> +     return err;
>>> +}
>>> +
>>> +static int __init wind_wmi_init(void)
>>> +{
>>> +     int err;
>>> +
>>> +     if (!wmi_has_guid(WMI_EVENT_GUID)) {
>>> +             pr_err("No MSI Wind WMI found\n");
>>> +             return -ENODEV;
>>> +     }
>>> +
>>> +     err = wind_input_setup();
>>> +     if (err) {
>>> +             pr_err("Unable to setup input device\n");
>>> +             return err;
>>> +     }
>>> +
>>> +     if (ACPI_FAILURE(wmi_install_notify_handler(WMI_EVENT_GUID,
>>> +             wind_wmi_notify, NULL))) {
>>> +             pr_err("Unable to install WMI notify handler\n");
>>> +             err = -EIO;
>>> +             goto wind_wmi_init_unregister_input;
>>> +     }
>>> +
>>> +     pr_debug("Event handler installed\n");
>>> +
>>> +     return 0;
>>> +
>>> +wind_wmi_init_unregister_input:
>>> +     input_unregister_device(wind_input_dev);
>>> +     sparse_keymap_free(wind_input_dev);
>>> +     input_free_device(wind_input_dev);
>>> +     return err;
>>> +}
>>> +
>>> +static void __exit wind_wmi_exit(void)
>>> +{
>>> +     wmi_remove_notify_handler(WMI_EVENT_GUID);
>>> +     input_unregister_device(wind_input_dev);
>>> +     sparse_keymap_free(wind_input_dev);
>>> +     input_free_device(wind_input_dev);
>>> +}
>>> +
>>> +module_init(wind_wmi_init);
>>> +module_exit(wind_wmi_exit);
>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>
>
> --
> Corentin Chary
> http://xf.iksaif.net

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

* Re: [PATCH 6/6] Add MSI Wind WMI support
  2012-11-27 15:55       ` Maxim Mikityanskiy
@ 2012-11-27 16:34         ` Corentin Chary
  2012-11-27 16:49           ` Maxim Mikityanskiy
  2012-11-27 23:20         ` joeyli
  1 sibling, 1 reply; 25+ messages in thread
From: Corentin Chary @ 2012-11-27 16:34 UTC (permalink / raw)
  To: Maxim Mikityanskiy
  Cc: joeyli, platform-driver-x86, Lee, Chun-Yi, Matthew Garrett

On Tue, Nov 27, 2012 at 3:55 PM, Maxim Mikityanskiy <maxtram95@gmail.com> wrote:
> 2012/11/27 Corentin Chary <corentin.chary@gmail.com>:
>> On Tue, Nov 27, 2012 at 8:14 AM, joeyli <jlee@suse.com> wrote:
>>> Hi Maxim,
>>>
>>> 於 日,2012-11-25 於 00:29 +0200,Maxim Mikityanskiy 提到:
>>>> Create a driver to support WMI found on MSI Wind laptops. This driver
>>>> allows us to receive some additional keycodes for keys that are not
>>>> handled without this driver.
>>>>
>>>> Signed-off-by: Maxim Mikityanskiy <maxtram95@gmail.com>
>>>
>>> I simply review this patch and looks there have many code the same with
>>> msi-wmi.c. Does there have any reason didn't direct patch msi-wmi but
>>> need generate a new driver?
>>
>> Right, if it's only a keymap and guid change, plus 1 or 2 quirks, it
>> may makes sense to be in msi-wind.c
>
> Compare msi-wind-wmi.c and dell-wmi-aio.c. They look very similar,
> even more similar than msi-wind-wmi.c and msi-wmi.c. Why do you
> recommend to merge msi-wind-wmi features into msi-wmi, but not
> dell-wmi-aio?

Well, because both are msi ? But as long as you took a look and think
that's not worth it, I'm ok with that.

> Look at dell-wmi.c, dell-wmi-aio.c and msi-wmi.c. They have lots of
> common code and they differ only in GUIDs, keymap tables and some
> quirks, but they are still different modules.
>
> msi-wind-wmi is WMI driver that only supports hotkey handling. Each
> WMI driver providing this feature contains such code, so there is
> nothing strange in that msi-wmi driver also contains such code.
> msi-wmi is a driver for fully different laptop with fully different
> WMI. It also handles backlight brightness and needs a quirk for hotkey
> handling. MSI Wind WMI does not send brightness change events and does
> not need that quirk.
>
> Do you still think I should merge msi-wind-wmi code into msi-wmi?
>
>>
>>>
>>> Thanks a lot!
>>> Joey Lee
>>>
>>>> ---
>>>>  drivers/platform/x86/Kconfig        |  13 +++
>>>>  drivers/platform/x86/Makefile       |   1 +
>>>>  drivers/platform/x86/msi-wind-wmi.c | 169 ++++++++++++++++++++++++++++++++++++
>>>>  3 files changed, 183 insertions(+)
>>>>  create mode 100644 drivers/platform/x86/msi-wind-wmi.c
>>>>
>>>> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
>>>> index c86bae8..46f269e5 100644
>>>> --- a/drivers/platform/x86/Kconfig
>>>> +++ b/drivers/platform/x86/Kconfig
>>>> @@ -561,6 +561,19 @@ config MSI_WMI
>>>>        To compile this driver as a module, choose M here: the module will
>>>>        be called msi-wmi.
>>>>
>>>> +config MSI_WIND_WMI
>>>> +     tristate "MSI Wind WMI Driver"
>>>> +     depends on ACPI_WMI
>>>> +     depends on INPUT
>>>> +     select INPUT_SPARSEKMAP
>>>> +     ---help---
>>>> +      Say Y here if you want to support WMI-based hotkeys on MSI Wind
>>>> +      laptops. MSI Wind WMI differs from WMI found on other MSI laptops, so
>>>> +      say Y here if you have MSI Wind, otherwise select "MSI WMI extras".
>>>> +
>>>> +      To compile this driver as a module, choose M here: the module will
>>>> +      be called msi-wind-wmi.
>>>> +
>>>>  config TOPSTAR_LAPTOP
>>>>       tristate "Topstar Laptop Extras"
>>>>       depends on ACPI
>>>> diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
>>>> index bf7e4f9..44389c0 100644
>>>> --- a/drivers/platform/x86/Makefile
>>>> +++ b/drivers/platform/x86/Makefile
>>>> @@ -30,6 +30,7 @@ obj-$(CONFIG_INTEL_MENLOW)  += intel_menlow.o
>>>>  obj-$(CONFIG_ACPI_WMI)               += wmi.o
>>>>  obj-$(CONFIG_MSI_WMI)                += msi-wmi.o
>>>>  obj-$(CONFIG_TOPSTAR_LAPTOP) += topstar-laptop.o
>>>> +obj-$(CONFIG_MSI_WIND_WMI)   += msi-wind-wmi.o
>>>>
>>>>  # toshiba_acpi must link after wmi to ensure that wmi devices are found
>>>>  # before toshiba_acpi initializes
>>>> diff --git a/drivers/platform/x86/msi-wind-wmi.c b/drivers/platform/x86/msi-wind-wmi.c
>>>> new file mode 100644
>>>> index 0000000..4f19434
>>>> --- /dev/null
>>>> +++ b/drivers/platform/x86/msi-wind-wmi.c
>>>> @@ -0,0 +1,169 @@
>>>> +/*
>>>> + * MSI Wind WMI hotkeys
>>>> + *
>>>> + * Copyright (C) 2012 Maxim Mikityanskiy <maxtram95@gmail.com>
>>>> + *
>>>> + *  This program is free software; you can redistribute it and/or modify
>>>> + *  it under the terms of the GNU General Public License as published by
>>>> + *  the Free Software Foundation; either version 2 of the License, or
>>>> + *  (at your option) any later version.
>>>> + *
>>>> + *  This program is distributed in the hope that it will be useful,
>>>> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
>>>> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>>>> + *  GNU General Public License for more details.
>>>> + *
>>>> + *  You should have received a copy of the GNU General Public License
>>>> + *  along with this program; if not, write to the Free Software
>>>> + *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
>>>> + */
>>>> +
>>>> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>>>> +
>>>> +#include <linux/module.h>
>>>> +#include <linux/input.h>
>>>> +#include <linux/input/sparse-keymap.h>
>>>> +#include <linux/acpi.h>
>>>> +
>>>> +MODULE_AUTHOR("Maxim Mikityanskiy <maxtram95@gmail.com>");
>>>> +MODULE_DESCRIPTION("MSI Wind laptop WMI hotkeys driver");
>>>> +MODULE_LICENSE("GPL");
>>>> +
>>>> +#define WMI_EVENT_GUID "5B3CC38A-40D9-7245-8AE6-1145B751BE3F"
>>>> +
>>>> +MODULE_ALIAS("wmi:" WMI_EVENT_GUID);
>>>> +
>>>> +/* Fn+F3 touchpad toggle */
>>>> +#define WIND_KEY_TOUCHPAD    0x08
>>>> +/* Fn+F11 Bluetooth toggle */
>>>> +#define WIND_KEY_BLUETOOTH   0x56
>>>> +/* Fn+F6 webcam toggle */
>>>> +#define WIND_KEY_CAMERA              0x57
>>>> +/* Fn+F11 Wi-Fi toggle */
>>>> +#define WIND_KEY_WLAN                0x5f
>>>> +/* Fn+F10 turbo mode toggle */
>>>> +#define WIND_KEY_TURBO               0x60
>>>> +/* Fn+F10 ECO mode toggle */
>>>> +#define WIND_KEY_ECO         0x69
>>>> +
>>>> +static struct key_entry wind_keymap[] = {
>>>> +     /* These keys work without WMI. Ignore them to avoid double keycodes */
>>>> +     { KE_IGNORE, WIND_KEY_TOUCHPAD,         { KEY_TOUCHPAD_TOGGLE } },
>>>> +     { KE_IGNORE, WIND_KEY_BLUETOOTH,        { KEY_BLUETOOTH } },
>>>> +     { KE_IGNORE, WIND_KEY_CAMERA,           { KEY_CAMERA } },
>>>> +     { KE_IGNORE, WIND_KEY_WLAN,             { KEY_WLAN } },
>>>> +     /* These are keys that should be handled via WMI */
>>>> +     { KE_KEY,    WIND_KEY_TURBO,            { KEY_PROG1 } },
>>>> +     { KE_KEY,    WIND_KEY_ECO,              { KEY_PROG2 } },
>>>> +     { KE_END, 0 }
>>>> +};
>>>> +
>>>> +static struct input_dev *wind_input_dev;
>>>> +
>>>> +static void wind_wmi_notify(u32 value, void *context)
>>>> +{
>>>> +     struct acpi_buffer response = {
>>>> +             .length         = ACPI_ALLOCATE_BUFFER,
>>>> +             .pointer        = NULL
>>>> +     };
>>>> +     acpi_status status = wmi_get_event_data(value, &response);
>>>> +     union acpi_object *obj = response.pointer;
>>>> +     int code;
>>>> +
>>>> +     if (status != AE_OK) {
>>>> +             pr_warn("Bad event status %#x\n", status);
>>>> +             return;
>>>> +     }
>>>> +
>>>> +     if (!obj || obj->type != ACPI_TYPE_INTEGER) {
>>>> +             pr_warn("Unknown event received\n");
>>>> +             goto wind_wmi_notify_free;
>>>> +     }
>>>> +
>>>> +     code = obj->integer.value;
>>>> +     pr_debug("Event code: %#x\n", code);
>>>> +
>>>> +     if (code == 0x00 || code == 0x62 || code == 0x63) {
>>>> +             /* Unknown events - drop them for now */
>>>> +             goto wind_wmi_notify_free;
>>>> +     }
>>>> +
>>>> +     if (!sparse_keymap_report_event(wind_input_dev, code, 1, true))
>>>> +             pr_warn("Unknown key %#x pressed\n", code);
>>>> +
>>>> +wind_wmi_notify_free:
>>>> +     kfree(response.pointer);
>>>> +}
>>>> +
>>>> +static int __init wind_input_setup(void)
>>>> +{
>>>> +     int err;
>>>> +
>>>> +     wind_input_dev = input_allocate_device();
>>>> +     if (!wind_input_dev)
>>>> +             return -ENOMEM;
>>>> +
>>>> +     wind_input_dev->name = "MSI WMI hotkeys";
>>>> +     wind_input_dev->phys = "wmi/input0";
>>>> +     wind_input_dev->id.bustype = BUS_HOST;
>>>> +
>>>> +     err = sparse_keymap_setup(wind_input_dev, wind_keymap, NULL);
>>>> +     if (err)
>>>> +             goto wind_input_setup_free_dev;
>>>> +
>>>> +     err = input_register_device(wind_input_dev);
>>>> +     if (err)
>>>> +             goto wind_input_setup_free_keymap;
>>>> +
>>>> +     return 0;
>>>> +
>>>> +wind_input_setup_free_keymap:
>>>> +     sparse_keymap_free(wind_input_dev);
>>>> +wind_input_setup_free_dev:
>>>> +     input_free_device(wind_input_dev);
>>>> +     return err;
>>>> +}
>>>> +
>>>> +static int __init wind_wmi_init(void)
>>>> +{
>>>> +     int err;
>>>> +
>>>> +     if (!wmi_has_guid(WMI_EVENT_GUID)) {
>>>> +             pr_err("No MSI Wind WMI found\n");
>>>> +             return -ENODEV;
>>>> +     }
>>>> +
>>>> +     err = wind_input_setup();
>>>> +     if (err) {
>>>> +             pr_err("Unable to setup input device\n");
>>>> +             return err;
>>>> +     }
>>>> +
>>>> +     if (ACPI_FAILURE(wmi_install_notify_handler(WMI_EVENT_GUID,
>>>> +             wind_wmi_notify, NULL))) {
>>>> +             pr_err("Unable to install WMI notify handler\n");
>>>> +             err = -EIO;
>>>> +             goto wind_wmi_init_unregister_input;
>>>> +     }
>>>> +
>>>> +     pr_debug("Event handler installed\n");
>>>> +
>>>> +     return 0;
>>>> +
>>>> +wind_wmi_init_unregister_input:
>>>> +     input_unregister_device(wind_input_dev);
>>>> +     sparse_keymap_free(wind_input_dev);
>>>> +     input_free_device(wind_input_dev);
>>>> +     return err;
>>>> +}
>>>> +
>>>> +static void __exit wind_wmi_exit(void)
>>>> +{
>>>> +     wmi_remove_notify_handler(WMI_EVENT_GUID);
>>>> +     input_unregister_device(wind_input_dev);
>>>> +     sparse_keymap_free(wind_input_dev);
>>>> +     input_free_device(wind_input_dev);
>>>> +}
>>>> +
>>>> +module_init(wind_wmi_init);
>>>> +module_exit(wind_wmi_exit);
>>>
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>>
>>
>> --
>> Corentin Chary
>> http://xf.iksaif.net



--
Corentin Chary
http://xf.iksaif.net

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

* Re: [PATCH 6/6] Add MSI Wind WMI support
  2012-11-27 16:34         ` Corentin Chary
@ 2012-11-27 16:49           ` Maxim Mikityanskiy
  0 siblings, 0 replies; 25+ messages in thread
From: Maxim Mikityanskiy @ 2012-11-27 16:49 UTC (permalink / raw)
  To: Corentin Chary; +Cc: joeyli, platform-driver-x86, Lee, Chun-Yi, Matthew Garrett

2012/11/27 Corentin Chary <corentin.chary@gmail.com>:
> On Tue, Nov 27, 2012 at 3:55 PM, Maxim Mikityanskiy <maxtram95@gmail.com> wrote:
>> 2012/11/27 Corentin Chary <corentin.chary@gmail.com>:
>>> On Tue, Nov 27, 2012 at 8:14 AM, joeyli <jlee@suse.com> wrote:
>>>> Hi Maxim,
>>>>
>>>> 於 日,2012-11-25 於 00:29 +0200,Maxim Mikityanskiy 提到:
>>>>> Create a driver to support WMI found on MSI Wind laptops. This driver
>>>>> allows us to receive some additional keycodes for keys that are not
>>>>> handled without this driver.
>>>>>
>>>>> Signed-off-by: Maxim Mikityanskiy <maxtram95@gmail.com>
>>>>
>>>> I simply review this patch and looks there have many code the same with
>>>> msi-wmi.c. Does there have any reason didn't direct patch msi-wmi but
>>>> need generate a new driver?
>>>
>>> Right, if it's only a keymap and guid change, plus 1 or 2 quirks, it
>>> may makes sense to be in msi-wind.c
>>
>> Compare msi-wind-wmi.c and dell-wmi-aio.c. They look very similar,
>> even more similar than msi-wind-wmi.c and msi-wmi.c. Why do you
>> recommend to merge msi-wind-wmi features into msi-wmi, but not
>> dell-wmi-aio?
>
> Well, because both are msi ?

In spite of this, that's totally different laptops, although they both are MSI.

> But as long as you took a look and think
> that's not worth it, I'm ok with that.

OK, thanks.

>> Look at dell-wmi.c, dell-wmi-aio.c and msi-wmi.c. They have lots of
>> common code and they differ only in GUIDs, keymap tables and some
>> quirks, but they are still different modules.
>>
>> msi-wind-wmi is WMI driver that only supports hotkey handling. Each
>> WMI driver providing this feature contains such code, so there is
>> nothing strange in that msi-wmi driver also contains such code.
>> msi-wmi is a driver for fully different laptop with fully different
>> WMI. It also handles backlight brightness and needs a quirk for hotkey
>> handling. MSI Wind WMI does not send brightness change events and does
>> not need that quirk.
>>
>> Do you still think I should merge msi-wind-wmi code into msi-wmi?
>>
>>>
>>>>
>>>> Thanks a lot!
>>>> Joey Lee
>>>>
>>>>> ---
>>>>>  drivers/platform/x86/Kconfig        |  13 +++
>>>>>  drivers/platform/x86/Makefile       |   1 +
>>>>>  drivers/platform/x86/msi-wind-wmi.c | 169 ++++++++++++++++++++++++++++++++++++
>>>>>  3 files changed, 183 insertions(+)
>>>>>  create mode 100644 drivers/platform/x86/msi-wind-wmi.c
>>>>>
>>>>> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
>>>>> index c86bae8..46f269e5 100644
>>>>> --- a/drivers/platform/x86/Kconfig
>>>>> +++ b/drivers/platform/x86/Kconfig
>>>>> @@ -561,6 +561,19 @@ config MSI_WMI
>>>>>        To compile this driver as a module, choose M here: the module will
>>>>>        be called msi-wmi.
>>>>>
>>>>> +config MSI_WIND_WMI
>>>>> +     tristate "MSI Wind WMI Driver"
>>>>> +     depends on ACPI_WMI
>>>>> +     depends on INPUT
>>>>> +     select INPUT_SPARSEKMAP
>>>>> +     ---help---
>>>>> +      Say Y here if you want to support WMI-based hotkeys on MSI Wind
>>>>> +      laptops. MSI Wind WMI differs from WMI found on other MSI laptops, so
>>>>> +      say Y here if you have MSI Wind, otherwise select "MSI WMI extras".
>>>>> +
>>>>> +      To compile this driver as a module, choose M here: the module will
>>>>> +      be called msi-wind-wmi.
>>>>> +
>>>>>  config TOPSTAR_LAPTOP
>>>>>       tristate "Topstar Laptop Extras"
>>>>>       depends on ACPI
>>>>> diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
>>>>> index bf7e4f9..44389c0 100644
>>>>> --- a/drivers/platform/x86/Makefile
>>>>> +++ b/drivers/platform/x86/Makefile
>>>>> @@ -30,6 +30,7 @@ obj-$(CONFIG_INTEL_MENLOW)  += intel_menlow.o
>>>>>  obj-$(CONFIG_ACPI_WMI)               += wmi.o
>>>>>  obj-$(CONFIG_MSI_WMI)                += msi-wmi.o
>>>>>  obj-$(CONFIG_TOPSTAR_LAPTOP) += topstar-laptop.o
>>>>> +obj-$(CONFIG_MSI_WIND_WMI)   += msi-wind-wmi.o
>>>>>
>>>>>  # toshiba_acpi must link after wmi to ensure that wmi devices are found
>>>>>  # before toshiba_acpi initializes
>>>>> diff --git a/drivers/platform/x86/msi-wind-wmi.c b/drivers/platform/x86/msi-wind-wmi.c
>>>>> new file mode 100644
>>>>> index 0000000..4f19434
>>>>> --- /dev/null
>>>>> +++ b/drivers/platform/x86/msi-wind-wmi.c
>>>>> @@ -0,0 +1,169 @@
>>>>> +/*
>>>>> + * MSI Wind WMI hotkeys
>>>>> + *
>>>>> + * Copyright (C) 2012 Maxim Mikityanskiy <maxtram95@gmail.com>
>>>>> + *
>>>>> + *  This program is free software; you can redistribute it and/or modify
>>>>> + *  it under the terms of the GNU General Public License as published by
>>>>> + *  the Free Software Foundation; either version 2 of the License, or
>>>>> + *  (at your option) any later version.
>>>>> + *
>>>>> + *  This program is distributed in the hope that it will be useful,
>>>>> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
>>>>> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>>>>> + *  GNU General Public License for more details.
>>>>> + *
>>>>> + *  You should have received a copy of the GNU General Public License
>>>>> + *  along with this program; if not, write to the Free Software
>>>>> + *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
>>>>> + */
>>>>> +
>>>>> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>>>>> +
>>>>> +#include <linux/module.h>
>>>>> +#include <linux/input.h>
>>>>> +#include <linux/input/sparse-keymap.h>
>>>>> +#include <linux/acpi.h>
>>>>> +
>>>>> +MODULE_AUTHOR("Maxim Mikityanskiy <maxtram95@gmail.com>");
>>>>> +MODULE_DESCRIPTION("MSI Wind laptop WMI hotkeys driver");
>>>>> +MODULE_LICENSE("GPL");
>>>>> +
>>>>> +#define WMI_EVENT_GUID "5B3CC38A-40D9-7245-8AE6-1145B751BE3F"
>>>>> +
>>>>> +MODULE_ALIAS("wmi:" WMI_EVENT_GUID);
>>>>> +
>>>>> +/* Fn+F3 touchpad toggle */
>>>>> +#define WIND_KEY_TOUCHPAD    0x08
>>>>> +/* Fn+F11 Bluetooth toggle */
>>>>> +#define WIND_KEY_BLUETOOTH   0x56
>>>>> +/* Fn+F6 webcam toggle */
>>>>> +#define WIND_KEY_CAMERA              0x57
>>>>> +/* Fn+F11 Wi-Fi toggle */
>>>>> +#define WIND_KEY_WLAN                0x5f
>>>>> +/* Fn+F10 turbo mode toggle */
>>>>> +#define WIND_KEY_TURBO               0x60
>>>>> +/* Fn+F10 ECO mode toggle */
>>>>> +#define WIND_KEY_ECO         0x69
>>>>> +
>>>>> +static struct key_entry wind_keymap[] = {
>>>>> +     /* These keys work without WMI. Ignore them to avoid double keycodes */
>>>>> +     { KE_IGNORE, WIND_KEY_TOUCHPAD,         { KEY_TOUCHPAD_TOGGLE } },
>>>>> +     { KE_IGNORE, WIND_KEY_BLUETOOTH,        { KEY_BLUETOOTH } },
>>>>> +     { KE_IGNORE, WIND_KEY_CAMERA,           { KEY_CAMERA } },
>>>>> +     { KE_IGNORE, WIND_KEY_WLAN,             { KEY_WLAN } },
>>>>> +     /* These are keys that should be handled via WMI */
>>>>> +     { KE_KEY,    WIND_KEY_TURBO,            { KEY_PROG1 } },
>>>>> +     { KE_KEY,    WIND_KEY_ECO,              { KEY_PROG2 } },
>>>>> +     { KE_END, 0 }
>>>>> +};
>>>>> +
>>>>> +static struct input_dev *wind_input_dev;
>>>>> +
>>>>> +static void wind_wmi_notify(u32 value, void *context)
>>>>> +{
>>>>> +     struct acpi_buffer response = {
>>>>> +             .length         = ACPI_ALLOCATE_BUFFER,
>>>>> +             .pointer        = NULL
>>>>> +     };
>>>>> +     acpi_status status = wmi_get_event_data(value, &response);
>>>>> +     union acpi_object *obj = response.pointer;
>>>>> +     int code;
>>>>> +
>>>>> +     if (status != AE_OK) {
>>>>> +             pr_warn("Bad event status %#x\n", status);
>>>>> +             return;
>>>>> +     }
>>>>> +
>>>>> +     if (!obj || obj->type != ACPI_TYPE_INTEGER) {
>>>>> +             pr_warn("Unknown event received\n");
>>>>> +             goto wind_wmi_notify_free;
>>>>> +     }
>>>>> +
>>>>> +     code = obj->integer.value;
>>>>> +     pr_debug("Event code: %#x\n", code);
>>>>> +
>>>>> +     if (code == 0x00 || code == 0x62 || code == 0x63) {
>>>>> +             /* Unknown events - drop them for now */
>>>>> +             goto wind_wmi_notify_free;
>>>>> +     }
>>>>> +
>>>>> +     if (!sparse_keymap_report_event(wind_input_dev, code, 1, true))
>>>>> +             pr_warn("Unknown key %#x pressed\n", code);
>>>>> +
>>>>> +wind_wmi_notify_free:
>>>>> +     kfree(response.pointer);
>>>>> +}
>>>>> +
>>>>> +static int __init wind_input_setup(void)
>>>>> +{
>>>>> +     int err;
>>>>> +
>>>>> +     wind_input_dev = input_allocate_device();
>>>>> +     if (!wind_input_dev)
>>>>> +             return -ENOMEM;
>>>>> +
>>>>> +     wind_input_dev->name = "MSI WMI hotkeys";
>>>>> +     wind_input_dev->phys = "wmi/input0";
>>>>> +     wind_input_dev->id.bustype = BUS_HOST;
>>>>> +
>>>>> +     err = sparse_keymap_setup(wind_input_dev, wind_keymap, NULL);
>>>>> +     if (err)
>>>>> +             goto wind_input_setup_free_dev;
>>>>> +
>>>>> +     err = input_register_device(wind_input_dev);
>>>>> +     if (err)
>>>>> +             goto wind_input_setup_free_keymap;
>>>>> +
>>>>> +     return 0;
>>>>> +
>>>>> +wind_input_setup_free_keymap:
>>>>> +     sparse_keymap_free(wind_input_dev);
>>>>> +wind_input_setup_free_dev:
>>>>> +     input_free_device(wind_input_dev);
>>>>> +     return err;
>>>>> +}
>>>>> +
>>>>> +static int __init wind_wmi_init(void)
>>>>> +{
>>>>> +     int err;
>>>>> +
>>>>> +     if (!wmi_has_guid(WMI_EVENT_GUID)) {
>>>>> +             pr_err("No MSI Wind WMI found\n");
>>>>> +             return -ENODEV;
>>>>> +     }
>>>>> +
>>>>> +     err = wind_input_setup();
>>>>> +     if (err) {
>>>>> +             pr_err("Unable to setup input device\n");
>>>>> +             return err;
>>>>> +     }
>>>>> +
>>>>> +     if (ACPI_FAILURE(wmi_install_notify_handler(WMI_EVENT_GUID,
>>>>> +             wind_wmi_notify, NULL))) {
>>>>> +             pr_err("Unable to install WMI notify handler\n");
>>>>> +             err = -EIO;
>>>>> +             goto wind_wmi_init_unregister_input;
>>>>> +     }
>>>>> +
>>>>> +     pr_debug("Event handler installed\n");
>>>>> +
>>>>> +     return 0;
>>>>> +
>>>>> +wind_wmi_init_unregister_input:
>>>>> +     input_unregister_device(wind_input_dev);
>>>>> +     sparse_keymap_free(wind_input_dev);
>>>>> +     input_free_device(wind_input_dev);
>>>>> +     return err;
>>>>> +}
>>>>> +
>>>>> +static void __exit wind_wmi_exit(void)
>>>>> +{
>>>>> +     wmi_remove_notify_handler(WMI_EVENT_GUID);
>>>>> +     input_unregister_device(wind_input_dev);
>>>>> +     sparse_keymap_free(wind_input_dev);
>>>>> +     input_free_device(wind_input_dev);
>>>>> +}
>>>>> +
>>>>> +module_init(wind_wmi_init);
>>>>> +module_exit(wind_wmi_exit);
>>>>
>>>>
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in
>>>> the body of a message to majordomo@vger.kernel.org
>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>
>>>
>>>
>>> --
>>> Corentin Chary
>>> http://xf.iksaif.net
>
>
>
> --
> Corentin Chary
> http://xf.iksaif.net

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

* Re: [PATCH 6/6] Add MSI Wind WMI support
  2012-11-27 15:55       ` Maxim Mikityanskiy
  2012-11-27 16:34         ` Corentin Chary
@ 2012-11-27 23:20         ` joeyli
  2012-11-28 18:03           ` Maxim Mikityanskiy
  1 sibling, 1 reply; 25+ messages in thread
From: joeyli @ 2012-11-27 23:20 UTC (permalink / raw)
  To: Maxim Mikityanskiy
  Cc: Corentin Chary, platform-driver-x86, Lee, Chun-Yi, Matthew Garrett

於 二,2012-11-27 於 17:55 +0200,Maxim Mikityanskiy 提到:
> 2012/11/27 Corentin Chary <corentin.chary@gmail.com>:
> > On Tue, Nov 27, 2012 at 8:14 AM, joeyli <jlee@suse.com> wrote:
> >> Hi Maxim,
> >>
> >> 於 日,2012-11-25 於 00:29 +0200,Maxim Mikityanskiy 提到:
> >>> Create a driver to support WMI found on MSI Wind laptops. This driver
> >>> allows us to receive some additional keycodes for keys that are not
> >>> handled without this driver.
> >>>
> >>> Signed-off-by: Maxim Mikityanskiy <maxtram95@gmail.com>
> >>
> >> I simply review this patch and looks there have many code the same with
> >> msi-wmi.c. Does there have any reason didn't direct patch msi-wmi but
> >> need generate a new driver?
> >
> > Right, if it's only a keymap and guid change, plus 1 or 2 quirks, it
> > may makes sense to be in msi-wind.c
> 
> Compare msi-wind-wmi.c and dell-wmi-aio.c. They look very similar,
> even more similar than msi-wind-wmi.c and msi-wmi.c. Why do you
> recommend to merge msi-wind-wmi features into msi-wmi, but not
> dell-wmi-aio?

Because MSI and DELL are different manufacturers. For the similar code
between different manufacturers we should extract those similar code to
acpi wmi or platform framework. 

> 
> Look at dell-wmi.c, dell-wmi-aio.c and msi-wmi.c. They have lots of
> common code and they differ only in GUIDs, keymap tables and some
> quirks, but they are still different modules.

Yes, because different manufacturers. For those common code need extract
to framework.

> 
> msi-wind-wmi is WMI driver that only supports hotkey handling. Each
> WMI driver providing this feature contains such code, so there is
> nothing strange in that msi-wmi driver also contains such code.
> msi-wmi is a driver for fully different laptop with fully different
> WMI. It also handles backlight brightness and needs a quirk for hotkey
> handling. MSI Wind WMI does not send brightness change events and does
> not need that quirk.
> 
> Do you still think I should merge msi-wind-wmi code into msi-wmi?

Yes, your contribution is very good for MSI wind user, but I still think
it's not worth to create a new driver for a small series. 

In Kconfig, it separate different platform driver by manufacturers, then
the drivers of the same manufacturer are separate to different interface
type (platform or wmi), or different hardware series like Laptop and
AIO. 

Current help of MSI_WMI in Kconfig is:
  help
   Say Y here if you want to support WMI-based hotkeys on MSI laptops.

Per this define, MSI wind series is undoubted a subset of MSI laptops. 

We can say the MSI wind is a series that should separate to different driver,
but I don't think MSI wind series is a big and complex series that worth to do that.


Thanks a lot!
Joey Lee

> 
> >
> >>
> >> Thanks a lot!
> >> Joey Lee
> >>
> >>> ---
> >>>  drivers/platform/x86/Kconfig        |  13 +++
> >>>  drivers/platform/x86/Makefile       |   1 +
> >>>  drivers/platform/x86/msi-wind-wmi.c | 169 ++++++++++++++++++++++++++++++++++++
> >>>  3 files changed, 183 insertions(+)
> >>>  create mode 100644 drivers/platform/x86/msi-wind-wmi.c
> >>>
> >>> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> >>> index c86bae8..46f269e5 100644
> >>> --- a/drivers/platform/x86/Kconfig
> >>> +++ b/drivers/platform/x86/Kconfig
> >>> @@ -561,6 +561,19 @@ config MSI_WMI
> >>>        To compile this driver as a module, choose M here: the module will
> >>>        be called msi-wmi.
> >>>
> >>> +config MSI_WIND_WMI
> >>> +     tristate "MSI Wind WMI Driver"
> >>> +     depends on ACPI_WMI
> >>> +     depends on INPUT
> >>> +     select INPUT_SPARSEKMAP
> >>> +     ---help---
> >>> +      Say Y here if you want to support WMI-based hotkeys on MSI Wind
> >>> +      laptops. MSI Wind WMI differs from WMI found on other MSI laptops, so
> >>> +      say Y here if you have MSI Wind, otherwise select "MSI WMI extras".
> >>> +
> >>> +      To compile this driver as a module, choose M here: the module will
> >>> +      be called msi-wind-wmi.
> >>> +
> >>>  config TOPSTAR_LAPTOP
> >>>       tristate "Topstar Laptop Extras"
> >>>       depends on ACPI
> >>> diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
> >>> index bf7e4f9..44389c0 100644
> >>> --- a/drivers/platform/x86/Makefile
> >>> +++ b/drivers/platform/x86/Makefile
> >>> @@ -30,6 +30,7 @@ obj-$(CONFIG_INTEL_MENLOW)  += intel_menlow.o
> >>>  obj-$(CONFIG_ACPI_WMI)               += wmi.o
> >>>  obj-$(CONFIG_MSI_WMI)                += msi-wmi.o
> >>>  obj-$(CONFIG_TOPSTAR_LAPTOP) += topstar-laptop.o
> >>> +obj-$(CONFIG_MSI_WIND_WMI)   += msi-wind-wmi.o
> >>>
> >>>  # toshiba_acpi must link after wmi to ensure that wmi devices are found
> >>>  # before toshiba_acpi initializes
> >>> diff --git a/drivers/platform/x86/msi-wind-wmi.c b/drivers/platform/x86/msi-wind-wmi.c
> >>> new file mode 100644
> >>> index 0000000..4f19434
> >>> --- /dev/null
> >>> +++ b/drivers/platform/x86/msi-wind-wmi.c
> >>> @@ -0,0 +1,169 @@
> >>> +/*
> >>> + * MSI Wind WMI hotkeys
> >>> + *
> >>> + * Copyright (C) 2012 Maxim Mikityanskiy <maxtram95@gmail.com>
> >>> + *
> >>> + *  This program is free software; you can redistribute it and/or modify
> >>> + *  it under the terms of the GNU General Public License as published by
> >>> + *  the Free Software Foundation; either version 2 of the License, or
> >>> + *  (at your option) any later version.
> >>> + *
> >>> + *  This program is distributed in the hope that it will be useful,
> >>> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> >>> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> >>> + *  GNU General Public License for more details.
> >>> + *
> >>> + *  You should have received a copy of the GNU General Public License
> >>> + *  along with this program; if not, write to the Free Software
> >>> + *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
> >>> + */
> >>> +
> >>> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> >>> +
> >>> +#include <linux/module.h>
> >>> +#include <linux/input.h>
> >>> +#include <linux/input/sparse-keymap.h>
> >>> +#include <linux/acpi.h>
> >>> +
> >>> +MODULE_AUTHOR("Maxim Mikityanskiy <maxtram95@gmail.com>");
> >>> +MODULE_DESCRIPTION("MSI Wind laptop WMI hotkeys driver");
> >>> +MODULE_LICENSE("GPL");
> >>> +
> >>> +#define WMI_EVENT_GUID "5B3CC38A-40D9-7245-8AE6-1145B751BE3F"
> >>> +
> >>> +MODULE_ALIAS("wmi:" WMI_EVENT_GUID);
> >>> +
> >>> +/* Fn+F3 touchpad toggle */
> >>> +#define WIND_KEY_TOUCHPAD    0x08
> >>> +/* Fn+F11 Bluetooth toggle */
> >>> +#define WIND_KEY_BLUETOOTH   0x56
> >>> +/* Fn+F6 webcam toggle */
> >>> +#define WIND_KEY_CAMERA              0x57
> >>> +/* Fn+F11 Wi-Fi toggle */
> >>> +#define WIND_KEY_WLAN                0x5f
> >>> +/* Fn+F10 turbo mode toggle */
> >>> +#define WIND_KEY_TURBO               0x60
> >>> +/* Fn+F10 ECO mode toggle */
> >>> +#define WIND_KEY_ECO         0x69
> >>> +
> >>> +static struct key_entry wind_keymap[] = {
> >>> +     /* These keys work without WMI. Ignore them to avoid double keycodes */
> >>> +     { KE_IGNORE, WIND_KEY_TOUCHPAD,         { KEY_TOUCHPAD_TOGGLE } },
> >>> +     { KE_IGNORE, WIND_KEY_BLUETOOTH,        { KEY_BLUETOOTH } },
> >>> +     { KE_IGNORE, WIND_KEY_CAMERA,           { KEY_CAMERA } },
> >>> +     { KE_IGNORE, WIND_KEY_WLAN,             { KEY_WLAN } },
> >>> +     /* These are keys that should be handled via WMI */
> >>> +     { KE_KEY,    WIND_KEY_TURBO,            { KEY_PROG1 } },
> >>> +     { KE_KEY,    WIND_KEY_ECO,              { KEY_PROG2 } },
> >>> +     { KE_END, 0 }
> >>> +};
> >>> +
> >>> +static struct input_dev *wind_input_dev;
> >>> +
> >>> +static void wind_wmi_notify(u32 value, void *context)
> >>> +{
> >>> +     struct acpi_buffer response = {
> >>> +             .length         = ACPI_ALLOCATE_BUFFER,
> >>> +             .pointer        = NULL
> >>> +     };
> >>> +     acpi_status status = wmi_get_event_data(value, &response);
> >>> +     union acpi_object *obj = response.pointer;
> >>> +     int code;
> >>> +
> >>> +     if (status != AE_OK) {
> >>> +             pr_warn("Bad event status %#x\n", status);
> >>> +             return;
> >>> +     }
> >>> +
> >>> +     if (!obj || obj->type != ACPI_TYPE_INTEGER) {
> >>> +             pr_warn("Unknown event received\n");
> >>> +             goto wind_wmi_notify_free;
> >>> +     }
> >>> +
> >>> +     code = obj->integer.value;
> >>> +     pr_debug("Event code: %#x\n", code);
> >>> +
> >>> +     if (code == 0x00 || code == 0x62 || code == 0x63) {
> >>> +             /* Unknown events - drop them for now */
> >>> +             goto wind_wmi_notify_free;
> >>> +     }
> >>> +
> >>> +     if (!sparse_keymap_report_event(wind_input_dev, code, 1, true))
> >>> +             pr_warn("Unknown key %#x pressed\n", code);
> >>> +
> >>> +wind_wmi_notify_free:
> >>> +     kfree(response.pointer);
> >>> +}
> >>> +
> >>> +static int __init wind_input_setup(void)
> >>> +{
> >>> +     int err;
> >>> +
> >>> +     wind_input_dev = input_allocate_device();
> >>> +     if (!wind_input_dev)
> >>> +             return -ENOMEM;
> >>> +
> >>> +     wind_input_dev->name = "MSI WMI hotkeys";
> >>> +     wind_input_dev->phys = "wmi/input0";
> >>> +     wind_input_dev->id.bustype = BUS_HOST;
> >>> +
> >>> +     err = sparse_keymap_setup(wind_input_dev, wind_keymap, NULL);
> >>> +     if (err)
> >>> +             goto wind_input_setup_free_dev;
> >>> +
> >>> +     err = input_register_device(wind_input_dev);
> >>> +     if (err)
> >>> +             goto wind_input_setup_free_keymap;
> >>> +
> >>> +     return 0;
> >>> +
> >>> +wind_input_setup_free_keymap:
> >>> +     sparse_keymap_free(wind_input_dev);
> >>> +wind_input_setup_free_dev:
> >>> +     input_free_device(wind_input_dev);
> >>> +     return err;
> >>> +}
> >>> +
> >>> +static int __init wind_wmi_init(void)
> >>> +{
> >>> +     int err;
> >>> +
> >>> +     if (!wmi_has_guid(WMI_EVENT_GUID)) {
> >>> +             pr_err("No MSI Wind WMI found\n");
> >>> +             return -ENODEV;
> >>> +     }
> >>> +
> >>> +     err = wind_input_setup();
> >>> +     if (err) {
> >>> +             pr_err("Unable to setup input device\n");
> >>> +             return err;
> >>> +     }
> >>> +
> >>> +     if (ACPI_FAILURE(wmi_install_notify_handler(WMI_EVENT_GUID,
> >>> +             wind_wmi_notify, NULL))) {
> >>> +             pr_err("Unable to install WMI notify handler\n");
> >>> +             err = -EIO;
> >>> +             goto wind_wmi_init_unregister_input;
> >>> +     }
> >>> +
> >>> +     pr_debug("Event handler installed\n");
> >>> +
> >>> +     return 0;
> >>> +
> >>> +wind_wmi_init_unregister_input:
> >>> +     input_unregister_device(wind_input_dev);
> >>> +     sparse_keymap_free(wind_input_dev);
> >>> +     input_free_device(wind_input_dev);
> >>> +     return err;
> >>> +}
> >>> +
> >>> +static void __exit wind_wmi_exit(void)
> >>> +{
> >>> +     wmi_remove_notify_handler(WMI_EVENT_GUID);
> >>> +     input_unregister_device(wind_input_dev);
> >>> +     sparse_keymap_free(wind_input_dev);
> >>> +     input_free_device(wind_input_dev);
> >>> +}
> >>> +
> >>> +module_init(wind_wmi_init);
> >>> +module_exit(wind_wmi_exit);
> >>
> >>
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in
> >> the body of a message to majordomo@vger.kernel.org
> >> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >
> >
> >
> > --
> > Corentin Chary
> > http://xf.iksaif.net
> 

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

* Re: [PATCH 1/6] msi-laptop: Use proper return codes instead of -1
  2012-11-24 22:28 ` [PATCH 1/6] msi-laptop: Use proper return codes instead of -1 Maxim Mikityanskiy
@ 2012-11-28  2:07   ` joeyli
  0 siblings, 0 replies; 25+ messages in thread
From: joeyli @ 2012-11-28  2:07 UTC (permalink / raw)
  To: Maxim Mikityanskiy; +Cc: platform-driver-x86, jlee, mjg

於 日,2012-11-25 於 00:28 +0200,Maxim Mikityanskiy 提到:
> Signed-off-by: Maxim Mikityanskiy <maxtram95@gmail.com>

Signed-off-by: Lee, Chun-Yi <jlee@suse.com>

Thanks for your patch!
Joey Lee

> ---
>  drivers/platform/x86/msi-laptop.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/platform/x86/msi-laptop.c b/drivers/platform/x86/msi-laptop.c
> index 2111dbb..063113c 100644
> --- a/drivers/platform/x86/msi-laptop.c
> +++ b/drivers/platform/x86/msi-laptop.c
> @@ -198,7 +198,7 @@ static ssize_t set_device_state(const char *buf, size_t count, u8 mask)
>  	/* read current device state */
>  	result = ec_read(MSI_STANDARD_EC_COMMAND_ADDRESS, &rdata);
>  	if (result < 0)
> -		return -EINVAL;
> +		return result;
>  
>  	if (!!(rdata & mask) != status) {
>  		/* reverse device bit */
> @@ -209,7 +209,7 @@ static ssize_t set_device_state(const char *buf, size_t count, u8 mask)
>  
>  		result = ec_write(MSI_STANDARD_EC_COMMAND_ADDRESS, wdata);
>  		if (result < 0)
> -			return -EINVAL;
> +			return result;
>  	}
>  
>  	return count;
> @@ -222,7 +222,7 @@ static int get_wireless_state(int *wlan, int *bluetooth)
>  
>  	result = ec_transaction(MSI_EC_COMMAND_WIRELESS, &wdata, 1, &rdata, 1);
>  	if (result < 0)
> -		return -1;
> +		return result;
>  
>  	if (wlan)
>  		*wlan = !!(rdata & 8);
> @@ -240,7 +240,7 @@ static int get_wireless_state_ec_standard(void)
>  
>  	result = ec_read(MSI_STANDARD_EC_COMMAND_ADDRESS, &rdata);
>  	if (result < 0)
> -		return -1;
> +		return result;
>  
>  	wlan_s = !!(rdata & MSI_STANDARD_EC_WLAN_MASK);
>  
> @@ -258,7 +258,7 @@ static int get_threeg_exists(void)
>  
>  	result = ec_read(MSI_STANDARD_EC_DEVICES_EXISTS_ADDRESS, &rdata);
>  	if (result < 0)
> -		return -1;
> +		return result;
>  
>  	threeg_exists = !!(rdata & MSI_STANDARD_EC_3G_MASK);
>  
> @@ -343,7 +343,7 @@ static ssize_t show_threeg(struct device *dev,
>  
>  	/* old msi ec not support 3G */
>  	if (old_ec_model)
> -		return -1;
> +		return -ENODEV;
>  
>  	ret = get_wireless_state_ec_standard();
>  	if (ret < 0)

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

* Re: [PATCH 2/6] msi-laptop: Work around gcc warning
  2012-11-24 22:28 ` [PATCH 2/6] msi-laptop: Work around gcc warning Maxim Mikityanskiy
@ 2012-11-28  2:47   ` joeyli
  0 siblings, 0 replies; 25+ messages in thread
From: joeyli @ 2012-11-28  2:47 UTC (permalink / raw)
  To: Maxim Mikityanskiy; +Cc: platform-driver-x86, jlee, mjg

於 日,2012-11-25 於 00:28 +0200,Maxim Mikityanskiy 提到:
> Signed-off-by: Maxim Mikityanskiy <maxtram95@gmail.com>

Signed-off-by: Lee, Chun-Yi <jlee@suse.com>

Thanks for your patch!
Joey Lee

> ---
>  drivers/platform/x86/msi-laptop.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/platform/x86/msi-laptop.c b/drivers/platform/x86/msi-laptop.c
> index 063113c..7ba107a 100644
> --- a/drivers/platform/x86/msi-laptop.c
> +++ b/drivers/platform/x86/msi-laptop.c
> @@ -291,7 +291,7 @@ static ssize_t show_wlan(struct device *dev,
>  	struct device_attribute *attr, char *buf)
>  {
>  
> -	int ret, enabled;
> +	int ret, enabled = 0;
>  
>  	if (old_ec_model) {
>  		ret = get_wireless_state(&enabled, NULL);
> @@ -315,7 +315,7 @@ static ssize_t show_bluetooth(struct device *dev,
>  	struct device_attribute *attr, char *buf)
>  {
>  
> -	int ret, enabled;
> +	int ret, enabled = 0;
>  
>  	if (old_ec_model) {
>  		ret = get_wireless_state(NULL, &enabled);

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

* Re: [PATCH 3/6] msi-laptop: Code cleanups
  2012-11-24 22:28 ` [PATCH 3/6] msi-laptop: Code cleanups Maxim Mikityanskiy
@ 2012-11-28  3:11   ` joeyli
  2012-11-28 17:32     ` Maxim Mikityanskiy
  0 siblings, 1 reply; 25+ messages in thread
From: joeyli @ 2012-11-28  3:11 UTC (permalink / raw)
  To: Maxim Mikityanskiy; +Cc: platform-driver-x86, jlee, mjg

於 日,2012-11-25 於 00:28 +0200,Maxim Mikityanskiy 提到:
> Replaced msi_init_rfkill() by msi_update_rfkill(), removed delays by
> replacing delayed works by just works and made msi_laptop_i8042_filter()
> filter out KEY_TOUCHPAD_TOGGLE when sending KEY_TOUCHPAD_ON and
> KEY_TOUCHPAD_OFF.

Please separate to 2 patches, one for touchpad key and other one for
rfkill.

And, 
I am not agree for the clean up to msi_init_rfkill unless it causes
problem on U90/U100. Did you find any issue on U90/U100?

I delay 1 magic second for msi_rfkill_init because there have other MSI
model need a delay time after we set SCM mode through write EC
register. 
So, please don't change the code for msi_init_rfkill delay work, just
keep it works with other MSI shipping machines.

> 
> Signed-off-by: Maxim Mikityanskiy <maxtram95@gmail.com>
> ---
>  drivers/platform/x86/msi-laptop.c | 67 ++++++++++++++-------------------------
>  1 file changed, 24 insertions(+), 43 deletions(-)
> 
> diff --git a/drivers/platform/x86/msi-laptop.c b/drivers/platform/x86/msi-laptop.c
> index 7ba107a..3b6f494 100644
> --- a/drivers/platform/x86/msi-laptop.c
> +++ b/drivers/platform/x86/msi-laptop.c
> @@ -600,8 +600,23 @@ static const struct rfkill_ops rfkill_threeg_ops = {
>  	.set_block = rfkill_threeg_set
>  };
>  
> +static void msi_update_rfkill(struct work_struct *ignored)
> +{
> +	get_wireless_state_ec_standard();
> +
> +	if (rfk_wlan)
> +		rfkill_set_sw_state(rfk_wlan, !wlan_s);
> +	if (rfk_bluetooth)
> +		rfkill_set_sw_state(rfk_bluetooth, !bluetooth_s);
> +	if (rfk_threeg)
> +		rfkill_set_sw_state(rfk_threeg, !threeg_s);
> +}
> +static DECLARE_WORK(msi_rfkill_work, msi_update_rfkill);
> +
>  static void rfkill_cleanup(void)
>  {
> +	cancel_work_sync(&msi_rfkill_work);
> +
>  	if (rfk_bluetooth) {
>  		rfkill_unregister(rfk_bluetooth);
>  		rfkill_destroy(rfk_bluetooth);
> @@ -618,19 +633,6 @@ static void rfkill_cleanup(void)
>  	}
>  }
>  
> -static void msi_update_rfkill(struct work_struct *ignored)
> -{
> -	get_wireless_state_ec_standard();
> -
> -	if (rfk_wlan)
> -		rfkill_set_sw_state(rfk_wlan, !wlan_s);
> -	if (rfk_bluetooth)
> -		rfkill_set_sw_state(rfk_bluetooth, !bluetooth_s);
> -	if (rfk_threeg)
> -		rfkill_set_sw_state(rfk_threeg, !threeg_s);
> -}
> -static DECLARE_DELAYED_WORK(msi_rfkill_work, msi_update_rfkill);
> -
>  static void msi_send_touchpad_key(struct work_struct *ignored)
>  {
>  	u8 rdata;
> @@ -644,7 +646,7 @@ static void msi_send_touchpad_key(struct work_struct *ignored)
>  		(rdata & MSI_STANDARD_EC_TOUCHPAD_MASK) ?
>  		KEY_TOUCHPAD_ON : KEY_TOUCHPAD_OFF, 1, true);
>  }
> -static DECLARE_DELAYED_WORK(msi_touchpad_work, msi_send_touchpad_key);
> +static DECLARE_WORK(msi_touchpad_work, msi_send_touchpad_key);
>  
>  static bool msi_laptop_i8042_filter(unsigned char data, unsigned char str,
>  				struct serio *port)
> @@ -662,14 +664,16 @@ static bool msi_laptop_i8042_filter(unsigned char data, unsigned char str,
>  		extended = false;
>  		switch (data) {
>  		case 0xE4:
> -			schedule_delayed_work(&msi_touchpad_work,
> -				round_jiffies_relative(0.5 * HZ));
> -			break;
> +			schedule_work(&msi_touchpad_work);

Don't remove the 0.5 magic delay, please! It will cause other shipped
MSI machines got problem could not capture the real change of EC
register.

> +		case 0x64:
> +			/* Filter out KEY_TOUCHPAD_TOGGLE and send only
> +			 * KEY_TOUCHPAD_ON and KEY_TOUCHPAD_OFF
> +			 */
> +			return true;

Why this patch filter out 0x64 key? What's the scan code emitted when
press the touchpad on/off Fn key on U100? Does it emit
KEY_TOUCHPAD_TOGGLE and KEY_TOUCHPAD_ON/OFF at the same time?

>  		case 0x54:
>  		case 0x62:
>  		case 0x76:
> -			schedule_delayed_work(&msi_rfkill_work,
> -				round_jiffies_relative(0.5 * HZ));
> +			schedule_work(&msi_rfkill_work);
>  			break;
>  		}
>  	}
> @@ -677,31 +681,11 @@ static bool msi_laptop_i8042_filter(unsigned char data, unsigned char str,
>  	return false;
>  }
>  
> -static void msi_init_rfkill(struct work_struct *ignored)
> -{
> -	if (rfk_wlan) {
> -		rfkill_set_sw_state(rfk_wlan, !wlan_s);
> -		rfkill_wlan_set(NULL, !wlan_s);
> -	}
> -	if (rfk_bluetooth) {
> -		rfkill_set_sw_state(rfk_bluetooth, !bluetooth_s);
> -		rfkill_bluetooth_set(NULL, !bluetooth_s);
> -	}
> -	if (rfk_threeg) {
> -		rfkill_set_sw_state(rfk_threeg, !threeg_s);
> -		rfkill_threeg_set(NULL, !threeg_s);
> -	}
> -}
> -static DECLARE_DELAYED_WORK(msi_rfkill_init, msi_init_rfkill);
> -
>  static int rfkill_init(struct platform_device *sdev)
>  {
>  	/* add rfkill */
>  	int retval;
>  
> -	/* keep the hardware wireless state */
> -	get_wireless_state_ec_standard();
> -
>  	rfk_bluetooth = rfkill_alloc("msi-bluetooth", &sdev->dev,
>  				RFKILL_TYPE_BLUETOOTH,
>  				&rfkill_bluetooth_ops, NULL);
> @@ -736,8 +720,7 @@ static int rfkill_init(struct platform_device *sdev)
>  	}
>  
>  	/* schedule to run rfkill state initial */
> -	schedule_delayed_work(&msi_rfkill_init,
> -				round_jiffies_relative(1 * HZ));
> +	schedule_work(&msi_rfkill_work);
>  
>  	return 0;
>  
> @@ -951,7 +934,6 @@ fail_platform_device2:
>  
>  	if (load_scm_model) {
>  		i8042_remove_filter(msi_laptop_i8042_filter);
> -		cancel_delayed_work_sync(&msi_rfkill_work);
>  		rfkill_cleanup();
>  	}
>  	platform_device_del(msipf_device);
> @@ -976,7 +958,6 @@ static void __exit msi_cleanup(void)
>  	if (load_scm_model) {
>  		i8042_remove_filter(msi_laptop_i8042_filter);
>  		msi_laptop_input_destroy();
> -		cancel_delayed_work_sync(&msi_rfkill_work);
>  		rfkill_cleanup();
>  	}
>  

Thanks a lot!
Joey Lee

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

* Re: [PATCH 4/6] msi-laptop: Add MSI Wind U90/U100 support
  2012-11-24 22:28 ` [PATCH 4/6] msi-laptop: Add MSI Wind U90/U100 support Maxim Mikityanskiy
@ 2012-11-28  3:32   ` joeyli
  2012-11-28 17:41     ` Maxim Mikityanskiy
  0 siblings, 1 reply; 25+ messages in thread
From: joeyli @ 2012-11-28  3:32 UTC (permalink / raw)
  To: Maxim Mikityanskiy; +Cc: platform-driver-x86, jlee, mjg

於 日,2012-11-25 於 00:28 +0200,Maxim Mikityanskiy 提到:
> Add MSI Wind U90/U100 to separate DMI table, add U90/U100 specific
> workarounds and add some missing EC features support such as basic fan
> control, turbo and ECO modes and touchpad state.
> 
> Signed-off-by: Maxim Mikityanskiy <maxtram95@gmail.com>
> ---
>  drivers/platform/x86/msi-laptop.c | 199 ++++++++++++++++++++++++++++++++------
>  1 file changed, 171 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/platform/x86/msi-laptop.c b/drivers/platform/x86/msi-laptop.c
> index 3b6f494..16e9863 100644
> --- a/drivers/platform/x86/msi-laptop.c
> +++ b/drivers/platform/x86/msi-laptop.c
> @@ -82,8 +82,19 @@
>  #define MSI_STANDARD_EC_SCM_LOAD_ADDRESS	0x2d
>  #define MSI_STANDARD_EC_SCM_LOAD_MASK		(1 << 0)
>  
> -#define MSI_STANDARD_EC_TOUCHPAD_ADDRESS	0xe4
> +#define MSI_STANDARD_EC_POWER_ADDRESS		0xe4

If 0xE4 register is not just for touchpad and power, then I suggest use
a more general name like: MSI_STANDARD_EC_FUNCTIONS_ADDRESS or other.

> +/* Power LED is orange - Turbo mode */
> +#define MSI_STANDARD_EC_TURBO_MASK		(1 << 1)
> +/* Power LED is green - ECO mode */
> +#define MSI_STANDARD_EC_ECO_MASK		(1 << 3)
> +/* Touchpad is turned on */
>  #define MSI_STANDARD_EC_TOUCHPAD_MASK		(1 << 4)
> +/* If this bit != bit 1, turbo mode can't be toggled */
> +#define MSI_STANDARD_EC_TURBO_COOLDOWN_MASK	(1 << 7)
> +
> +#define MSI_STANDARD_EC_FAN_ADDRESS		0x33
> +/* If zero, fan rotates at maximal speed */
> +#define MSI_STANDARD_EC_AUTOFAN_MASK		(1 << 0)
>  
>  #ifdef CONFIG_PM_SLEEP
>  static int msi_laptop_resume(struct device *device);
> @@ -123,6 +134,13 @@ static int threeg_exists;
>   * e.g. MSI N034 netbook
>   */
>  static bool load_scm_model;
> +
> +/* Some MSI Wind netbooks (e.g. MSI Wind U100) need loading SCM to get some
> + * features working (e.g. ECO mode), but we cannot change Wlan/Bluetooth state
> + * in software and we can only read its state.
> + */
> +static bool ec_read_only;
> +
>  static struct rfkill *rfk_wlan, *rfk_bluetooth, *rfk_threeg;
>  
>  /* Hardware access */
> @@ -195,6 +213,9 @@ static ssize_t set_device_state(const char *buf, size_t count, u8 mask)
>  	if (sscanf(buf, "%i", &status) != 1 || (status < 0 || status > 1))
>  		return -EINVAL;
>  
> +	if (ec_read_only)
> +		return -EOPNOTSUPP;
> +
>  	/* read current device state */
>  	result = ec_read(MSI_STANDARD_EC_COMMAND_ADDRESS, &rdata);
>  	if (result < 0)
> @@ -417,18 +438,115 @@ static ssize_t store_auto_brightness(struct device *dev,
>  	return count;
>  }
>  
> +static ssize_t show_touchpad(struct device *dev,
> +	struct device_attribute *attr, char *buf)
> +{
> +
> +	u8 rdata;
> +	int result;
> +
> +	result = ec_read(MSI_STANDARD_EC_POWER_ADDRESS, &rdata);
> +	if (result < 0)
> +		return result;
> +
> +	return sprintf(buf, "%i\n", !!(rdata & MSI_STANDARD_EC_TOUCHPAD_MASK));
> +}

I don't think we need create a touchpad attribute interface because
there already have key code raise the change to user space.

> +
> +static ssize_t show_turbo(struct device *dev,
> +	struct device_attribute *attr, char *buf)
> +{
> +
> +	u8 rdata;
> +	int result;
> +
> +	result = ec_read(MSI_STANDARD_EC_POWER_ADDRESS, &rdata);
> +	if (result < 0)
> +		return result;
> +
> +	return sprintf(buf, "%i\n", !!(rdata & MSI_STANDARD_EC_TURBO_MASK));
> +}
> +
> +static ssize_t show_eco(struct device *dev,
> +	struct device_attribute *attr, char *buf)
> +{
> +
> +	u8 rdata;
> +	int result;
> +
> +	result = ec_read(MSI_STANDARD_EC_POWER_ADDRESS, &rdata);
> +	if (result < 0)
> +		return result;
> +
> +	return sprintf(buf, "%i\n", !!(rdata & MSI_STANDARD_EC_ECO_MASK));
> +}
> +
> +static ssize_t show_turbo_cooldown(struct device *dev,
> +	struct device_attribute *attr, char *buf)
> +{
> +
> +	u8 rdata;
> +	int result;
> +
> +	result = ec_read(MSI_STANDARD_EC_POWER_ADDRESS, &rdata);
> +	if (result < 0)
> +		return result;
> +
> +	return sprintf(buf, "%i\n", (!!(rdata & MSI_STANDARD_EC_TURBO_MASK)) |
> +		(!!(rdata & MSI_STANDARD_EC_TURBO_COOLDOWN_MASK) << 1));
> +}
> +
> +static ssize_t show_auto_fan(struct device *dev,
> +	struct device_attribute *attr, char *buf)
> +{
> +
> +	u8 rdata;
> +	int result;
> +
> +	result = ec_read(MSI_STANDARD_EC_FAN_ADDRESS, &rdata);
> +	if (result < 0)
> +		return result;
> +
> +	return sprintf(buf, "%i\n", !!(rdata & MSI_STANDARD_EC_AUTOFAN_MASK));
> +}
> +
> +static ssize_t store_auto_fan(struct device *dev,
> +	struct device_attribute *attr, const char *buf, size_t count)
> +{
> +
> +	int enable, result;
> +
> +	if (sscanf(buf, "%i", &enable) != 1 || (enable != (enable & 1)))
> +		return -EINVAL;
> +
> +	result = ec_write(MSI_STANDARD_EC_FAN_ADDRESS, enable);
> +	if (result < 0)
> +		return result;
> +
> +	return count;
> +}
> +
>  static DEVICE_ATTR(lcd_level, 0644, show_lcd_level, store_lcd_level);
>  static DEVICE_ATTR(auto_brightness, 0644, show_auto_brightness,
>  		   store_auto_brightness);
>  static DEVICE_ATTR(bluetooth, 0444, show_bluetooth, NULL);
>  static DEVICE_ATTR(wlan, 0444, show_wlan, NULL);
>  static DEVICE_ATTR(threeg, 0444, show_threeg, NULL);
> +static DEVICE_ATTR(touchpad, 0444, show_touchpad, NULL);
> +static DEVICE_ATTR(turbo_mode, 0444, show_turbo, NULL);
> +static DEVICE_ATTR(eco_mode, 0444, show_eco, NULL);
> +static DEVICE_ATTR(turbo_cooldown, 0444, show_turbo_cooldown, NULL);
> +static DEVICE_ATTR(auto_fan, 0644, show_auto_fan, store_auto_fan);
>  
>  static struct attribute *msipf_attributes[] = {
>  	&dev_attr_lcd_level.attr,
>  	&dev_attr_auto_brightness.attr,
>  	&dev_attr_bluetooth.attr,
>  	&dev_attr_wlan.attr,
> +	&dev_attr_touchpad.attr,
> +	&dev_attr_turbo_mode.attr,
> +	&dev_attr_eco_mode.attr,
> +	&dev_attr_turbo_cooldown.attr,
> +	&dev_attr_auto_fan.attr,
>  	NULL
>  };
>  
> @@ -553,6 +671,19 @@ static struct dmi_system_id __initdata msi_load_scm_models_dmi_table[] = {
>  	{ }
>  };
>  
> +static struct dmi_system_id __initdata msi_load_scm_ro_models_dmi_table[] = {
> +	{
> +		.ident = "MSI U90/U100",
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR,
> +				"MICRO-STAR INTERNATIONAL CO., LTD"),
> +			DMI_MATCH(DMI_PRODUCT_NAME, "U90/U100"),
> +		},
> +		.callback = dmi_check_cb
> +	},
> +	{ }
> +};
> +
>  static int rfkill_bluetooth_set(void *data, bool blocked)
>  {
>  	/* Do something with blocked...*/
> @@ -560,32 +691,26 @@ static int rfkill_bluetooth_set(void *data, bool blocked)
>  	 * blocked == false is on
>  	 * blocked == true is off
>  	 */
> -	if (blocked)
> -		set_device_state("0", 0, MSI_STANDARD_EC_BLUETOOTH_MASK);
> -	else
> -		set_device_state("1", 0, MSI_STANDARD_EC_BLUETOOTH_MASK);
> +	int result = set_device_state(blocked ? "0" : "1", 0,
> +		MSI_STANDARD_EC_BLUETOOTH_MASK);
>  
> -	return 0;
> +	return result < 0 ? result : 0;
>  }
>  
>  static int rfkill_wlan_set(void *data, bool blocked)
>  {
> -	if (blocked)
> -		set_device_state("0", 0, MSI_STANDARD_EC_WLAN_MASK);
> -	else
> -		set_device_state("1", 0, MSI_STANDARD_EC_WLAN_MASK);
> +	int result = set_device_state(blocked ? "0" : "1", 0,
> +		MSI_STANDARD_EC_WLAN_MASK);
>  
> -	return 0;
> +	return result < 0 ? result : 0;
>  }
>  
>  static int rfkill_threeg_set(void *data, bool blocked)
>  {
> -	if (blocked)
> -		set_device_state("0", 0, MSI_STANDARD_EC_3G_MASK);
> -	else
> -		set_device_state("1", 0, MSI_STANDARD_EC_3G_MASK);
> +	int result = set_device_state(blocked ? "0" : "1", 0,
> +		MSI_STANDARD_EC_3G_MASK);
>  
> -	return 0;
> +	return result < 0 ? result : 0;
>  }
>  
>  static const struct rfkill_ops rfkill_bluetooth_ops = {
> @@ -600,16 +725,24 @@ static const struct rfkill_ops rfkill_threeg_ops = {
>  	.set_block = rfkill_threeg_set
>  };
>  
> +static bool msi_rfkill_set_state(struct rfkill *rfkill, bool blocked)
> +{
> +	if (ec_read_only)
> +		return rfkill_set_hw_state(rfkill, blocked);
> +	else
> +		return rfkill_set_sw_state(rfkill, blocked);
> +}
> +
>  static void msi_update_rfkill(struct work_struct *ignored)
>  {
>  	get_wireless_state_ec_standard();
>  
>  	if (rfk_wlan)
> -		rfkill_set_sw_state(rfk_wlan, !wlan_s);
> +		msi_rfkill_set_state(rfk_wlan, !wlan_s);
>  	if (rfk_bluetooth)
> -		rfkill_set_sw_state(rfk_bluetooth, !bluetooth_s);
> +		msi_rfkill_set_state(rfk_bluetooth, !bluetooth_s);
>  	if (rfk_threeg)
> -		rfkill_set_sw_state(rfk_threeg, !threeg_s);
> +		msi_rfkill_set_state(rfk_threeg, !threeg_s);
>  }
>  static DECLARE_WORK(msi_rfkill_work, msi_update_rfkill);
>  
> @@ -638,7 +771,7 @@ static void msi_send_touchpad_key(struct work_struct *ignored)
>  	u8 rdata;
>  	int result;
>  
> -	result = ec_read(MSI_STANDARD_EC_TOUCHPAD_ADDRESS, &rdata);
> +	result = ec_read(MSI_STANDARD_EC_POWER_ADDRESS, &rdata);
>  	if (result < 0)
>  		return;
>  
> @@ -802,13 +935,15 @@ static int __init load_scm_model_init(struct platform_device *sdev)
>  	u8 data;
>  	int result;
>  
> -	/* allow userland write sysfs file  */
> -	dev_attr_bluetooth.store = store_bluetooth;
> -	dev_attr_wlan.store = store_wlan;
> -	dev_attr_threeg.store = store_threeg;
> -	dev_attr_bluetooth.attr.mode |= S_IWUSR;
> -	dev_attr_wlan.attr.mode |= S_IWUSR;
> -	dev_attr_threeg.attr.mode |= S_IWUSR;
> +	if (!ec_read_only) {
> +		/* allow userland write sysfs file  */
> +		dev_attr_bluetooth.store = store_bluetooth;
> +		dev_attr_wlan.store = store_wlan;
> +		dev_attr_threeg.store = store_threeg;
> +		dev_attr_bluetooth.attr.mode |= S_IWUSR;
> +		dev_attr_wlan.attr.mode |= S_IWUSR;
> +		dev_attr_threeg.attr.mode |= S_IWUSR;
> +	}
>  
>  	/* disable hardware control by fn key */
>  	result = ec_read(MSI_STANDARD_EC_SCM_LOAD_ADDRESS, &data);
> @@ -860,8 +995,15 @@ static int __init msi_init(void)
>  	if (force || dmi_check_system(msi_dmi_table))
>  		old_ec_model = 1;
>  
> -	if (!old_ec_model)
> +	if (!old_ec_model) {
>  		get_threeg_exists();
> +		if (dmi_check_system(msi_load_scm_models_dmi_table))
> +			load_scm_model = 1;
> +		else if (dmi_check_system(msi_load_scm_ro_models_dmi_table)) {
> +			load_scm_model = 1;
> +			ec_read_only = 1;
> +		}
> +	}
>  
>  	if (!old_ec_model && dmi_check_system(msi_load_scm_models_dmi_table))
>  		load_scm_model = 1;

hmm... the load_scm_model dmi table check 2 times? I think you can just
remove the duplicate code.

> @@ -992,3 +1134,4 @@ MODULE_ALIAS("dmi:*:svnMICRO-STARINTERNATIONAL*:pnMS-N051:*");
>  MODULE_ALIAS("dmi:*:svnMICRO-STARINTERNATIONAL*:pnMS-N014:*");
>  MODULE_ALIAS("dmi:*:svnMicro-StarInternational*:pnCR620:*");
>  MODULE_ALIAS("dmi:*:svnMicro-StarInternational*:pnU270series:*");
> +MODULE_ALIAS("dmi:*:svnMICRO-STARINTERNATIONAL*:pnU90/U100:*");


Thanks a lot!
Joey Lee

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

* Re: [PATCH 5/6] msi-laptop: Add missing ABI documentation
  2012-11-24 22:29 ` [PATCH 5/6] msi-laptop: Add missing ABI documentation Maxim Mikityanskiy
@ 2012-11-28  3:36   ` joeyli
  0 siblings, 0 replies; 25+ messages in thread
From: joeyli @ 2012-11-28  3:36 UTC (permalink / raw)
  To: Maxim Mikityanskiy; +Cc: platform-driver-x86, jlee, mjg

於 日,2012-11-25 於 00:29 +0200,Maxim Mikityanskiy 提到:
> Signed-off-by: Maxim Mikityanskiy <maxtram95@gmail.com>
> ---
>  .../ABI/testing/sysfs-platform-msi-laptop          | 83 ++++++++++++++++++++++
>  1 file changed, 83 insertions(+)
>  create mode 100644 Documentation/ABI/testing/sysfs-platform-msi-laptop
> 
> diff --git a/Documentation/ABI/testing/sysfs-platform-msi-laptop b/Documentation/ABI/testing/sysfs-platform-msi-laptop
> new file mode 100644
> index 0000000..f621027
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-platform-msi-laptop
> @@ -0,0 +1,83 @@
> +What:		/sys/devices/platform/msi-laptop-pf/lcd_level
> +Date:		Oct 2006
> +KernelVersion:	2.6.19
> +Contact:	"Lennart Poettering <mzxreary@0pointer.de>"
> +Description:
> +		Screen brightness: contains a single integer in the range 0..8.
> +
> +What:		/sys/devices/platform/msi-laptop-pf/auto_brightness
> +Date:		Oct 2006
> +KernelVersion:	2.6.19
> +Contact:	"Lennart Poettering <mzxreary@0pointer.de>"
> +Description:
> +		Enable automatic brightness control: contains either 0 or 1. If
> +		set to 1 the hardware adjusts the screen brightness
> +		automatically when the power cord is plugged/unplugged.
> +
> +What:		/sys/devices/platform/msi-laptop-pf/wlan
> +Date:		Oct 2006
> +KernelVersion:	2.6.19
> +Contact:	"Lennart Poettering <mzxreary@0pointer.de>"
> +Description:
> +		WLAN subsystem enabled: contains either 0 or 1.
> +
> +What:		/sys/devices/platform/msi-laptop-pf/bluetooth
> +Date:		Oct 2006
> +KernelVersion:	2.6.19
> +Contact:	"Lennart Poettering <mzxreary@0pointer.de>"
> +Description:
> +		Bluetooth subsystem enabled: contains either 0 or 1. Please
> +		note that this file is constantly 0 if no Bluetooth hardware is
> +		available.
> +
> +What:		/sys/devices/platform/msi-laptop-pf/touchpad
> +Date:		Nov 2012
> +KernelVersion:	3.7
> +Contact:	"Maxim Mikityanskiy <maxtram95@gmail.com>"
> +Description:
> +		Contains either 0 or 1 and indicates if touchpad is turned on.
> +		Touchpad state can only be toggled by pressing Fn+F3.
> +

I think we don't need another interface to raise the touchpad status
because we already send key code to user space.

Except touchpad, other content in this documentation patch is good!

Signed-off-by: Lee, Chun-Yi <jlee@suse.com>


Thanks a lot!
Joey Lee

> +What:		/sys/devices/platform/msi-laptop-pf/turbo_mode
> +Date:		Nov 2012
> +KernelVersion:	3.7
> +Contact:	"Maxim Mikityanskiy <maxtram95@gmail.com>"
> +Description:
> +		Contains either 0 or 1 and indicates if turbo mode is turned
> +		on. In turbo mode power LED is orange and processor is
> +		overclocked. Turbo mode is available only if charging. It is
> +		only possible to toggle turbo mode state by pressing Fn+F10,
> +		and there is a few seconds cooldown between subsequent toggles.
> +		If user presses Fn+F10 too frequent, turbo mode state is not
> +		changed.
> +
> +What:		/sys/devices/platform/msi-laptop-pf/eco_mode
> +Date:		Nov 2012
> +KernelVersion:	3.7
> +Contact:	"Maxim Mikityanskiy <maxtram95@gmail.com>"
> +Description:
> +		Contains either 0 or 1 and indicates if ECO mode is turned on.
> +		In ECO mode power LED is green and userspace should do some
> +		powersaving actions. ECO mode is available only on battery
> +		power. ECO mode can only be toggled by pressing Fn+F10.
> +
> +What:		/sys/devices/platform/msi-laptop-pf/turbo_cooldown
> +Date:		Nov 2012
> +KernelVersion:	3.7
> +Contact:	"Maxim Mikityanskiy <maxtram95@gmail.com>"
> +Description:
> +		Contains value in range 0..3:
> +			* 0 -> Turbo mode is off
> +			* 1 -> Turbo mode is on, cannot be turned off yet
> +			* 2 -> Turbo mode is off, cannot be turned on yet
> +			* 3 -> Turbo mode is on
> +
> +What:		/sys/devices/platform/msi-laptop-pf/auto_fan
> +Date:		Nov 2012
> +KernelVersion:	3.7
> +Contact:	"Maxim Mikityanskiy <maxtram95@gmail.com>"
> +Description:
> +		Contains either 0 or 1 and indicates if fan speed is controlled
> +		automatically (1) or fan runs at maximal speed (0). Can be
> +		toggled in software.
> +

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

* Re: [PATCH 3/6] msi-laptop: Code cleanups
  2012-11-28  3:11   ` joeyli
@ 2012-11-28 17:32     ` Maxim Mikityanskiy
  2012-11-29  8:10       ` joeyli
  0 siblings, 1 reply; 25+ messages in thread
From: Maxim Mikityanskiy @ 2012-11-28 17:32 UTC (permalink / raw)
  To: joeyli; +Cc: platform-driver-x86, Lee, Chun-Yi, Matthew Garrett

2012/11/28 joeyli <jlee@suse.com>:
> 於 日,2012-11-25 於 00:28 +0200,Maxim Mikityanskiy 提到:
>> Replaced msi_init_rfkill() by msi_update_rfkill(), removed delays by
>> replacing delayed works by just works and made msi_laptop_i8042_filter()
>> filter out KEY_TOUCHPAD_TOGGLE when sending KEY_TOUCHPAD_ON and
>> KEY_TOUCHPAD_OFF.
>
> Please separate to 2 patches, one for touchpad key and other one for
> rfkill.
>
> And,
> I am not agree for the clean up to msi_init_rfkill unless it causes
> problem on U90/U100. Did you find any issue on U90/U100?

In original code, we call get_wireless_state_ec_standard() in
rfkill_init() in order to get initial rfkill state from hardware and
after a second we call msi_init_rfkill() that sets rfkill state and
writes it to hardware. msi_update_rfkill() reads rfkill state from
hardware and sets it immediately. On U90/U100 writing to EC confuses
it. In patch 4 I add a quirk in set_device_state() that blocks writing
to EC so msi_init_rfkill() should not cause any issue, but we would
have unneeded 1 second delay and function calls.

> I delay 1 magic second for msi_rfkill_init because there have other MSI
> model need a delay time after we set SCM mode through write EC
> register.
> So, please don't change the code for msi_init_rfkill delay work, just
> keep it works with other MSI shipping machines.

What do you think if I remove delay only for U90/U100 and keep it for
other models?

>>
>> Signed-off-by: Maxim Mikityanskiy <maxtram95@gmail.com>
>> ---
>>  drivers/platform/x86/msi-laptop.c | 67 ++++++++++++++-------------------------
>>  1 file changed, 24 insertions(+), 43 deletions(-)
>>
>> diff --git a/drivers/platform/x86/msi-laptop.c b/drivers/platform/x86/msi-laptop.c
>> index 7ba107a..3b6f494 100644
>> --- a/drivers/platform/x86/msi-laptop.c
>> +++ b/drivers/platform/x86/msi-laptop.c
>> @@ -600,8 +600,23 @@ static const struct rfkill_ops rfkill_threeg_ops = {
>>       .set_block = rfkill_threeg_set
>>  };
>>
>> +static void msi_update_rfkill(struct work_struct *ignored)
>> +{
>> +     get_wireless_state_ec_standard();
>> +
>> +     if (rfk_wlan)
>> +             rfkill_set_sw_state(rfk_wlan, !wlan_s);
>> +     if (rfk_bluetooth)
>> +             rfkill_set_sw_state(rfk_bluetooth, !bluetooth_s);
>> +     if (rfk_threeg)
>> +             rfkill_set_sw_state(rfk_threeg, !threeg_s);
>> +}
>> +static DECLARE_WORK(msi_rfkill_work, msi_update_rfkill);
>> +
>>  static void rfkill_cleanup(void)
>>  {
>> +     cancel_work_sync(&msi_rfkill_work);
>> +
>>       if (rfk_bluetooth) {
>>               rfkill_unregister(rfk_bluetooth);
>>               rfkill_destroy(rfk_bluetooth);
>> @@ -618,19 +633,6 @@ static void rfkill_cleanup(void)
>>       }
>>  }
>>
>> -static void msi_update_rfkill(struct work_struct *ignored)
>> -{
>> -     get_wireless_state_ec_standard();
>> -
>> -     if (rfk_wlan)
>> -             rfkill_set_sw_state(rfk_wlan, !wlan_s);
>> -     if (rfk_bluetooth)
>> -             rfkill_set_sw_state(rfk_bluetooth, !bluetooth_s);
>> -     if (rfk_threeg)
>> -             rfkill_set_sw_state(rfk_threeg, !threeg_s);
>> -}
>> -static DECLARE_DELAYED_WORK(msi_rfkill_work, msi_update_rfkill);
>> -
>>  static void msi_send_touchpad_key(struct work_struct *ignored)
>>  {
>>       u8 rdata;
>> @@ -644,7 +646,7 @@ static void msi_send_touchpad_key(struct work_struct *ignored)
>>               (rdata & MSI_STANDARD_EC_TOUCHPAD_MASK) ?
>>               KEY_TOUCHPAD_ON : KEY_TOUCHPAD_OFF, 1, true);
>>  }
>> -static DECLARE_DELAYED_WORK(msi_touchpad_work, msi_send_touchpad_key);
>> +static DECLARE_WORK(msi_touchpad_work, msi_send_touchpad_key);
>>
>>  static bool msi_laptop_i8042_filter(unsigned char data, unsigned char str,
>>                               struct serio *port)
>> @@ -662,14 +664,16 @@ static bool msi_laptop_i8042_filter(unsigned char data, unsigned char str,
>>               extended = false;
>>               switch (data) {
>>               case 0xE4:
>> -                     schedule_delayed_work(&msi_touchpad_work,
>> -                             round_jiffies_relative(0.5 * HZ));
>> -                     break;
>> +                     schedule_work(&msi_touchpad_work);
>
> Don't remove the 0.5 magic delay, please! It will cause other shipped
> MSI machines got problem could not capture the real change of EC
> register.

The same question here.

>> +             case 0x64:
>> +                     /* Filter out KEY_TOUCHPAD_TOGGLE and send only
>> +                      * KEY_TOUCHPAD_ON and KEY_TOUCHPAD_OFF
>> +                      */
>> +                     return true;
>
> Why this patch filter out 0x64 key? What's the scan code emitted when
> press the touchpad on/off Fn key on U100? Does it emit
> KEY_TOUCHPAD_TOGGLE and KEY_TOUCHPAD_ON/OFF at the same time?

0xE4 stands for Fn+F3 release and 0x64 stands for Fn+F3 press. So, if
we do not use msi-laptop driver, we will get KEY_TOUCHPAD_TOGGLE
(maybe from i8042 driver). If we load msi-laptop driver, it hooks
Fn+F3 release and sends KEY_TOUCHPAD_ON/OFF, so userspace will receive
KEY_TOUCHPAD_TOGGLE before KEY_TOUCHPAD_ON/OFF. So we need to filter
out both Fn+F3 press and release, so that KEY_TOUCHPAD_TOGGLE will not
be sent.

>>               case 0x54:
>>               case 0x62:
>>               case 0x76:
>> -                     schedule_delayed_work(&msi_rfkill_work,
>> -                             round_jiffies_relative(0.5 * HZ));
>> +                     schedule_work(&msi_rfkill_work);
>>                       break;
>>               }
>>       }
>> @@ -677,31 +681,11 @@ static bool msi_laptop_i8042_filter(unsigned char data, unsigned char str,
>>       return false;
>>  }
>>
>> -static void msi_init_rfkill(struct work_struct *ignored)
>> -{
>> -     if (rfk_wlan) {
>> -             rfkill_set_sw_state(rfk_wlan, !wlan_s);
>> -             rfkill_wlan_set(NULL, !wlan_s);
>> -     }
>> -     if (rfk_bluetooth) {
>> -             rfkill_set_sw_state(rfk_bluetooth, !bluetooth_s);
>> -             rfkill_bluetooth_set(NULL, !bluetooth_s);
>> -     }
>> -     if (rfk_threeg) {
>> -             rfkill_set_sw_state(rfk_threeg, !threeg_s);
>> -             rfkill_threeg_set(NULL, !threeg_s);
>> -     }
>> -}
>> -static DECLARE_DELAYED_WORK(msi_rfkill_init, msi_init_rfkill);
>> -
>>  static int rfkill_init(struct platform_device *sdev)
>>  {
>>       /* add rfkill */
>>       int retval;
>>
>> -     /* keep the hardware wireless state */
>> -     get_wireless_state_ec_standard();
>> -
>>       rfk_bluetooth = rfkill_alloc("msi-bluetooth", &sdev->dev,
>>                               RFKILL_TYPE_BLUETOOTH,
>>                               &rfkill_bluetooth_ops, NULL);
>> @@ -736,8 +720,7 @@ static int rfkill_init(struct platform_device *sdev)
>>       }
>>
>>       /* schedule to run rfkill state initial */
>> -     schedule_delayed_work(&msi_rfkill_init,
>> -                             round_jiffies_relative(1 * HZ));
>> +     schedule_work(&msi_rfkill_work);
>>
>>       return 0;
>>
>> @@ -951,7 +934,6 @@ fail_platform_device2:
>>
>>       if (load_scm_model) {
>>               i8042_remove_filter(msi_laptop_i8042_filter);
>> -             cancel_delayed_work_sync(&msi_rfkill_work);
>>               rfkill_cleanup();
>>       }
>>       platform_device_del(msipf_device);
>> @@ -976,7 +958,6 @@ static void __exit msi_cleanup(void)
>>       if (load_scm_model) {
>>               i8042_remove_filter(msi_laptop_i8042_filter);
>>               msi_laptop_input_destroy();
>> -             cancel_delayed_work_sync(&msi_rfkill_work);
>>               rfkill_cleanup();
>>       }
>>
>
> Thanks a lot!
> Joey Lee
>
>

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

* Re: [PATCH 4/6] msi-laptop: Add MSI Wind U90/U100 support
  2012-11-28  3:32   ` joeyli
@ 2012-11-28 17:41     ` Maxim Mikityanskiy
  2012-11-29  4:14       ` joeyli
  0 siblings, 1 reply; 25+ messages in thread
From: Maxim Mikityanskiy @ 2012-11-28 17:41 UTC (permalink / raw)
  To: joeyli; +Cc: platform-driver-x86, Lee, Chun-Yi, Matthew Garrett

2012/11/28 joeyli <jlee@suse.com>:
> 於 日,2012-11-25 於 00:28 +0200,Maxim Mikityanskiy 提到:
>> Add MSI Wind U90/U100 to separate DMI table, add U90/U100 specific
>> workarounds and add some missing EC features support such as basic fan
>> control, turbo and ECO modes and touchpad state.
>>
>> Signed-off-by: Maxim Mikityanskiy <maxtram95@gmail.com>
>> ---
>>  drivers/platform/x86/msi-laptop.c | 199 ++++++++++++++++++++++++++++++++------
>>  1 file changed, 171 insertions(+), 28 deletions(-)
>>
>> diff --git a/drivers/platform/x86/msi-laptop.c b/drivers/platform/x86/msi-laptop.c
>> index 3b6f494..16e9863 100644
>> --- a/drivers/platform/x86/msi-laptop.c
>> +++ b/drivers/platform/x86/msi-laptop.c
>> @@ -82,8 +82,19 @@
>>  #define MSI_STANDARD_EC_SCM_LOAD_ADDRESS     0x2d
>>  #define MSI_STANDARD_EC_SCM_LOAD_MASK                (1 << 0)
>>
>> -#define MSI_STANDARD_EC_TOUCHPAD_ADDRESS     0xe4
>> +#define MSI_STANDARD_EC_POWER_ADDRESS                0xe4
>
> If 0xE4 register is not just for touchpad and power, then I suggest use
> a more general name like: MSI_STANDARD_EC_FUNCTIONS_ADDRESS or other.

OK, I will rename it.

>> +/* Power LED is orange - Turbo mode */
>> +#define MSI_STANDARD_EC_TURBO_MASK           (1 << 1)
>> +/* Power LED is green - ECO mode */
>> +#define MSI_STANDARD_EC_ECO_MASK             (1 << 3)
>> +/* Touchpad is turned on */
>>  #define MSI_STANDARD_EC_TOUCHPAD_MASK                (1 << 4)
>> +/* If this bit != bit 1, turbo mode can't be toggled */
>> +#define MSI_STANDARD_EC_TURBO_COOLDOWN_MASK  (1 << 7)
>> +
>> +#define MSI_STANDARD_EC_FAN_ADDRESS          0x33
>> +/* If zero, fan rotates at maximal speed */
>> +#define MSI_STANDARD_EC_AUTOFAN_MASK         (1 << 0)
>>
>>  #ifdef CONFIG_PM_SLEEP
>>  static int msi_laptop_resume(struct device *device);
>> @@ -123,6 +134,13 @@ static int threeg_exists;
>>   * e.g. MSI N034 netbook
>>   */
>>  static bool load_scm_model;
>> +
>> +/* Some MSI Wind netbooks (e.g. MSI Wind U100) need loading SCM to get some
>> + * features working (e.g. ECO mode), but we cannot change Wlan/Bluetooth state
>> + * in software and we can only read its state.
>> + */
>> +static bool ec_read_only;
>> +
>>  static struct rfkill *rfk_wlan, *rfk_bluetooth, *rfk_threeg;
>>
>>  /* Hardware access */
>> @@ -195,6 +213,9 @@ static ssize_t set_device_state(const char *buf, size_t count, u8 mask)
>>       if (sscanf(buf, "%i", &status) != 1 || (status < 0 || status > 1))
>>               return -EINVAL;
>>
>> +     if (ec_read_only)
>> +             return -EOPNOTSUPP;
>> +
>>       /* read current device state */
>>       result = ec_read(MSI_STANDARD_EC_COMMAND_ADDRESS, &rdata);
>>       if (result < 0)
>> @@ -417,18 +438,115 @@ static ssize_t store_auto_brightness(struct device *dev,
>>       return count;
>>  }
>>
>> +static ssize_t show_touchpad(struct device *dev,
>> +     struct device_attribute *attr, char *buf)
>> +{
>> +
>> +     u8 rdata;
>> +     int result;
>> +
>> +     result = ec_read(MSI_STANDARD_EC_POWER_ADDRESS, &rdata);
>> +     if (result < 0)
>> +             return result;
>> +
>> +     return sprintf(buf, "%i\n", !!(rdata & MSI_STANDARD_EC_TOUCHPAD_MASK));
>> +}
>
> I don't think we need create a touchpad attribute interface because
> there already have key code raise the change to user space.

Key codes only indicate touchpad state change. We cannot determine
initial touchpad state on boot using only key codes. I think, it might
be useful to have a file in sysfs that always keeps actual touchpad
state, so that we can get initial touchpad state on boot. Do you
disagree with that?

>> +
>> +static ssize_t show_turbo(struct device *dev,
>> +     struct device_attribute *attr, char *buf)
>> +{
>> +
>> +     u8 rdata;
>> +     int result;
>> +
>> +     result = ec_read(MSI_STANDARD_EC_POWER_ADDRESS, &rdata);
>> +     if (result < 0)
>> +             return result;
>> +
>> +     return sprintf(buf, "%i\n", !!(rdata & MSI_STANDARD_EC_TURBO_MASK));
>> +}
>> +
>> +static ssize_t show_eco(struct device *dev,
>> +     struct device_attribute *attr, char *buf)
>> +{
>> +
>> +     u8 rdata;
>> +     int result;
>> +
>> +     result = ec_read(MSI_STANDARD_EC_POWER_ADDRESS, &rdata);
>> +     if (result < 0)
>> +             return result;
>> +
>> +     return sprintf(buf, "%i\n", !!(rdata & MSI_STANDARD_EC_ECO_MASK));
>> +}
>> +
>> +static ssize_t show_turbo_cooldown(struct device *dev,
>> +     struct device_attribute *attr, char *buf)
>> +{
>> +
>> +     u8 rdata;
>> +     int result;
>> +
>> +     result = ec_read(MSI_STANDARD_EC_POWER_ADDRESS, &rdata);
>> +     if (result < 0)
>> +             return result;
>> +
>> +     return sprintf(buf, "%i\n", (!!(rdata & MSI_STANDARD_EC_TURBO_MASK)) |
>> +             (!!(rdata & MSI_STANDARD_EC_TURBO_COOLDOWN_MASK) << 1));
>> +}
>> +
>> +static ssize_t show_auto_fan(struct device *dev,
>> +     struct device_attribute *attr, char *buf)
>> +{
>> +
>> +     u8 rdata;
>> +     int result;
>> +
>> +     result = ec_read(MSI_STANDARD_EC_FAN_ADDRESS, &rdata);
>> +     if (result < 0)
>> +             return result;
>> +
>> +     return sprintf(buf, "%i\n", !!(rdata & MSI_STANDARD_EC_AUTOFAN_MASK));
>> +}
>> +
>> +static ssize_t store_auto_fan(struct device *dev,
>> +     struct device_attribute *attr, const char *buf, size_t count)
>> +{
>> +
>> +     int enable, result;
>> +
>> +     if (sscanf(buf, "%i", &enable) != 1 || (enable != (enable & 1)))
>> +             return -EINVAL;
>> +
>> +     result = ec_write(MSI_STANDARD_EC_FAN_ADDRESS, enable);
>> +     if (result < 0)
>> +             return result;
>> +
>> +     return count;
>> +}
>> +
>>  static DEVICE_ATTR(lcd_level, 0644, show_lcd_level, store_lcd_level);
>>  static DEVICE_ATTR(auto_brightness, 0644, show_auto_brightness,
>>                  store_auto_brightness);
>>  static DEVICE_ATTR(bluetooth, 0444, show_bluetooth, NULL);
>>  static DEVICE_ATTR(wlan, 0444, show_wlan, NULL);
>>  static DEVICE_ATTR(threeg, 0444, show_threeg, NULL);
>> +static DEVICE_ATTR(touchpad, 0444, show_touchpad, NULL);
>> +static DEVICE_ATTR(turbo_mode, 0444, show_turbo, NULL);
>> +static DEVICE_ATTR(eco_mode, 0444, show_eco, NULL);
>> +static DEVICE_ATTR(turbo_cooldown, 0444, show_turbo_cooldown, NULL);
>> +static DEVICE_ATTR(auto_fan, 0644, show_auto_fan, store_auto_fan);
>>
>>  static struct attribute *msipf_attributes[] = {
>>       &dev_attr_lcd_level.attr,
>>       &dev_attr_auto_brightness.attr,
>>       &dev_attr_bluetooth.attr,
>>       &dev_attr_wlan.attr,
>> +     &dev_attr_touchpad.attr,
>> +     &dev_attr_turbo_mode.attr,
>> +     &dev_attr_eco_mode.attr,
>> +     &dev_attr_turbo_cooldown.attr,
>> +     &dev_attr_auto_fan.attr,
>>       NULL
>>  };
>>
>> @@ -553,6 +671,19 @@ static struct dmi_system_id __initdata msi_load_scm_models_dmi_table[] = {
>>       { }
>>  };
>>
>> +static struct dmi_system_id __initdata msi_load_scm_ro_models_dmi_table[] = {
>> +     {
>> +             .ident = "MSI U90/U100",
>> +             .matches = {
>> +                     DMI_MATCH(DMI_SYS_VENDOR,
>> +                             "MICRO-STAR INTERNATIONAL CO., LTD"),
>> +                     DMI_MATCH(DMI_PRODUCT_NAME, "U90/U100"),
>> +             },
>> +             .callback = dmi_check_cb
>> +     },
>> +     { }
>> +};
>> +
>>  static int rfkill_bluetooth_set(void *data, bool blocked)
>>  {
>>       /* Do something with blocked...*/
>> @@ -560,32 +691,26 @@ static int rfkill_bluetooth_set(void *data, bool blocked)
>>        * blocked == false is on
>>        * blocked == true is off
>>        */
>> -     if (blocked)
>> -             set_device_state("0", 0, MSI_STANDARD_EC_BLUETOOTH_MASK);
>> -     else
>> -             set_device_state("1", 0, MSI_STANDARD_EC_BLUETOOTH_MASK);
>> +     int result = set_device_state(blocked ? "0" : "1", 0,
>> +             MSI_STANDARD_EC_BLUETOOTH_MASK);
>>
>> -     return 0;
>> +     return result < 0 ? result : 0;
>>  }
>>
>>  static int rfkill_wlan_set(void *data, bool blocked)
>>  {
>> -     if (blocked)
>> -             set_device_state("0", 0, MSI_STANDARD_EC_WLAN_MASK);
>> -     else
>> -             set_device_state("1", 0, MSI_STANDARD_EC_WLAN_MASK);
>> +     int result = set_device_state(blocked ? "0" : "1", 0,
>> +             MSI_STANDARD_EC_WLAN_MASK);
>>
>> -     return 0;
>> +     return result < 0 ? result : 0;
>>  }
>>
>>  static int rfkill_threeg_set(void *data, bool blocked)
>>  {
>> -     if (blocked)
>> -             set_device_state("0", 0, MSI_STANDARD_EC_3G_MASK);
>> -     else
>> -             set_device_state("1", 0, MSI_STANDARD_EC_3G_MASK);
>> +     int result = set_device_state(blocked ? "0" : "1", 0,
>> +             MSI_STANDARD_EC_3G_MASK);
>>
>> -     return 0;
>> +     return result < 0 ? result : 0;
>>  }
>>
>>  static const struct rfkill_ops rfkill_bluetooth_ops = {
>> @@ -600,16 +725,24 @@ static const struct rfkill_ops rfkill_threeg_ops = {
>>       .set_block = rfkill_threeg_set
>>  };
>>
>> +static bool msi_rfkill_set_state(struct rfkill *rfkill, bool blocked)
>> +{
>> +     if (ec_read_only)
>> +             return rfkill_set_hw_state(rfkill, blocked);
>> +     else
>> +             return rfkill_set_sw_state(rfkill, blocked);
>> +}
>> +
>>  static void msi_update_rfkill(struct work_struct *ignored)
>>  {
>>       get_wireless_state_ec_standard();
>>
>>       if (rfk_wlan)
>> -             rfkill_set_sw_state(rfk_wlan, !wlan_s);
>> +             msi_rfkill_set_state(rfk_wlan, !wlan_s);
>>       if (rfk_bluetooth)
>> -             rfkill_set_sw_state(rfk_bluetooth, !bluetooth_s);
>> +             msi_rfkill_set_state(rfk_bluetooth, !bluetooth_s);
>>       if (rfk_threeg)
>> -             rfkill_set_sw_state(rfk_threeg, !threeg_s);
>> +             msi_rfkill_set_state(rfk_threeg, !threeg_s);
>>  }
>>  static DECLARE_WORK(msi_rfkill_work, msi_update_rfkill);
>>
>> @@ -638,7 +771,7 @@ static void msi_send_touchpad_key(struct work_struct *ignored)
>>       u8 rdata;
>>       int result;
>>
>> -     result = ec_read(MSI_STANDARD_EC_TOUCHPAD_ADDRESS, &rdata);
>> +     result = ec_read(MSI_STANDARD_EC_POWER_ADDRESS, &rdata);
>>       if (result < 0)
>>               return;
>>
>> @@ -802,13 +935,15 @@ static int __init load_scm_model_init(struct platform_device *sdev)
>>       u8 data;
>>       int result;
>>
>> -     /* allow userland write sysfs file  */
>> -     dev_attr_bluetooth.store = store_bluetooth;
>> -     dev_attr_wlan.store = store_wlan;
>> -     dev_attr_threeg.store = store_threeg;
>> -     dev_attr_bluetooth.attr.mode |= S_IWUSR;
>> -     dev_attr_wlan.attr.mode |= S_IWUSR;
>> -     dev_attr_threeg.attr.mode |= S_IWUSR;
>> +     if (!ec_read_only) {
>> +             /* allow userland write sysfs file  */
>> +             dev_attr_bluetooth.store = store_bluetooth;
>> +             dev_attr_wlan.store = store_wlan;
>> +             dev_attr_threeg.store = store_threeg;
>> +             dev_attr_bluetooth.attr.mode |= S_IWUSR;
>> +             dev_attr_wlan.attr.mode |= S_IWUSR;
>> +             dev_attr_threeg.attr.mode |= S_IWUSR;
>> +     }
>>
>>       /* disable hardware control by fn key */
>>       result = ec_read(MSI_STANDARD_EC_SCM_LOAD_ADDRESS, &data);
>> @@ -860,8 +995,15 @@ static int __init msi_init(void)
>>       if (force || dmi_check_system(msi_dmi_table))
>>               old_ec_model = 1;
>>
>> -     if (!old_ec_model)
>> +     if (!old_ec_model) {
>>               get_threeg_exists();
>> +             if (dmi_check_system(msi_load_scm_models_dmi_table))
>> +                     load_scm_model = 1;
>> +             else if (dmi_check_system(msi_load_scm_ro_models_dmi_table)) {
>> +                     load_scm_model = 1;
>> +                     ec_read_only = 1;
>> +             }
>> +     }
>>
>>       if (!old_ec_model && dmi_check_system(msi_load_scm_models_dmi_table))
>>               load_scm_model = 1;
>
> hmm... the load_scm_model dmi table check 2 times? I think you can just
> remove the duplicate code.

Oops, I forgot to remove the last two lines when I were preparing patches.

Do I need to resend all patches after doing all needed fixes?

>> @@ -992,3 +1134,4 @@ MODULE_ALIAS("dmi:*:svnMICRO-STARINTERNATIONAL*:pnMS-N051:*");
>>  MODULE_ALIAS("dmi:*:svnMICRO-STARINTERNATIONAL*:pnMS-N014:*");
>>  MODULE_ALIAS("dmi:*:svnMicro-StarInternational*:pnCR620:*");
>>  MODULE_ALIAS("dmi:*:svnMicro-StarInternational*:pnU270series:*");
>> +MODULE_ALIAS("dmi:*:svnMICRO-STARINTERNATIONAL*:pnU90/U100:*");
>
>
> Thanks a lot!
> Joey Lee
>
>

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

* Re: [PATCH 6/6] Add MSI Wind WMI support
  2012-11-27 23:20         ` joeyli
@ 2012-11-28 18:03           ` Maxim Mikityanskiy
  0 siblings, 0 replies; 25+ messages in thread
From: Maxim Mikityanskiy @ 2012-11-28 18:03 UTC (permalink / raw)
  To: joeyli; +Cc: Corentin Chary, platform-driver-x86, Lee, Chun-Yi, Matthew Garrett

2012/11/28 joeyli <jlee@suse.com>:
> 於 二,2012-11-27 於 17:55 +0200,Maxim Mikityanskiy 提到:
>> 2012/11/27 Corentin Chary <corentin.chary@gmail.com>:
>> > On Tue, Nov 27, 2012 at 8:14 AM, joeyli <jlee@suse.com> wrote:
>> >> Hi Maxim,
>> >>
>> >> 於 日,2012-11-25 於 00:29 +0200,Maxim Mikityanskiy 提到:
>> >>> Create a driver to support WMI found on MSI Wind laptops. This driver
>> >>> allows us to receive some additional keycodes for keys that are not
>> >>> handled without this driver.
>> >>>
>> >>> Signed-off-by: Maxim Mikityanskiy <maxtram95@gmail.com>
>> >>
>> >> I simply review this patch and looks there have many code the same with
>> >> msi-wmi.c. Does there have any reason didn't direct patch msi-wmi but
>> >> need generate a new driver?
>> >
>> > Right, if it's only a keymap and guid change, plus 1 or 2 quirks, it
>> > may makes sense to be in msi-wind.c
>>
>> Compare msi-wind-wmi.c and dell-wmi-aio.c. They look very similar,
>> even more similar than msi-wind-wmi.c and msi-wmi.c. Why do you
>> recommend to merge msi-wind-wmi features into msi-wmi, but not
>> dell-wmi-aio?
>
> Because MSI and DELL are different manufacturers. For the similar code
> between different manufacturers we should extract those similar code to
> acpi wmi or platform framework.
>
>>
>> Look at dell-wmi.c, dell-wmi-aio.c and msi-wmi.c. They have lots of
>> common code and they differ only in GUIDs, keymap tables and some
>> quirks, but they are still different modules.
>
> Yes, because different manufacturers. For those common code need extract
> to framework.
>
>>
>> msi-wind-wmi is WMI driver that only supports hotkey handling. Each
>> WMI driver providing this feature contains such code, so there is
>> nothing strange in that msi-wmi driver also contains such code.
>> msi-wmi is a driver for fully different laptop with fully different
>> WMI. It also handles backlight brightness and needs a quirk for hotkey
>> handling. MSI Wind WMI does not send brightness change events and does
>> not need that quirk.
>>
>> Do you still think I should merge msi-wind-wmi code into msi-wmi?
>
> Yes, your contribution is very good for MSI wind user, but I still think
> it's not worth to create a new driver for a small series.
>
> In Kconfig, it separate different platform driver by manufacturers, then
> the drivers of the same manufacturer are separate to different interface
> type (platform or wmi), or different hardware series like Laptop and
> AIO.
>
> Current help of MSI_WMI in Kconfig is:
>   help
>    Say Y here if you want to support WMI-based hotkeys on MSI laptops.
>
> Per this define, MSI wind series is undoubted a subset of MSI laptops.
>
> We can say the MSI wind is a series that should separate to different driver,
> but I don't think MSI wind series is a big and complex series that worth to do that.

Okay, okay, if you really want to preserve one vendor - one driver
logic, I can make a patch for msi-wmi, but I think it will lead to
complication of code, lots of ifs and lots of code unused by U90/U100
but loaded in RAM.

>
> Thanks a lot!
> Joey Lee
>
>>
>> >
>> >>
>> >> Thanks a lot!
>> >> Joey Lee
>> >>
>> >>> ---
>> >>>  drivers/platform/x86/Kconfig        |  13 +++
>> >>>  drivers/platform/x86/Makefile       |   1 +
>> >>>  drivers/platform/x86/msi-wind-wmi.c | 169 ++++++++++++++++++++++++++++++++++++
>> >>>  3 files changed, 183 insertions(+)
>> >>>  create mode 100644 drivers/platform/x86/msi-wind-wmi.c
>> >>>
>> >>> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
>> >>> index c86bae8..46f269e5 100644
>> >>> --- a/drivers/platform/x86/Kconfig
>> >>> +++ b/drivers/platform/x86/Kconfig
>> >>> @@ -561,6 +561,19 @@ config MSI_WMI
>> >>>        To compile this driver as a module, choose M here: the module will
>> >>>        be called msi-wmi.
>> >>>
>> >>> +config MSI_WIND_WMI
>> >>> +     tristate "MSI Wind WMI Driver"
>> >>> +     depends on ACPI_WMI
>> >>> +     depends on INPUT
>> >>> +     select INPUT_SPARSEKMAP
>> >>> +     ---help---
>> >>> +      Say Y here if you want to support WMI-based hotkeys on MSI Wind
>> >>> +      laptops. MSI Wind WMI differs from WMI found on other MSI laptops, so
>> >>> +      say Y here if you have MSI Wind, otherwise select "MSI WMI extras".
>> >>> +
>> >>> +      To compile this driver as a module, choose M here: the module will
>> >>> +      be called msi-wind-wmi.
>> >>> +
>> >>>  config TOPSTAR_LAPTOP
>> >>>       tristate "Topstar Laptop Extras"
>> >>>       depends on ACPI
>> >>> diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
>> >>> index bf7e4f9..44389c0 100644
>> >>> --- a/drivers/platform/x86/Makefile
>> >>> +++ b/drivers/platform/x86/Makefile
>> >>> @@ -30,6 +30,7 @@ obj-$(CONFIG_INTEL_MENLOW)  += intel_menlow.o
>> >>>  obj-$(CONFIG_ACPI_WMI)               += wmi.o
>> >>>  obj-$(CONFIG_MSI_WMI)                += msi-wmi.o
>> >>>  obj-$(CONFIG_TOPSTAR_LAPTOP) += topstar-laptop.o
>> >>> +obj-$(CONFIG_MSI_WIND_WMI)   += msi-wind-wmi.o
>> >>>
>> >>>  # toshiba_acpi must link after wmi to ensure that wmi devices are found
>> >>>  # before toshiba_acpi initializes
>> >>> diff --git a/drivers/platform/x86/msi-wind-wmi.c b/drivers/platform/x86/msi-wind-wmi.c
>> >>> new file mode 100644
>> >>> index 0000000..4f19434
>> >>> --- /dev/null
>> >>> +++ b/drivers/platform/x86/msi-wind-wmi.c
>> >>> @@ -0,0 +1,169 @@
>> >>> +/*
>> >>> + * MSI Wind WMI hotkeys
>> >>> + *
>> >>> + * Copyright (C) 2012 Maxim Mikityanskiy <maxtram95@gmail.com>
>> >>> + *
>> >>> + *  This program is free software; you can redistribute it and/or modify
>> >>> + *  it under the terms of the GNU General Public License as published by
>> >>> + *  the Free Software Foundation; either version 2 of the License, or
>> >>> + *  (at your option) any later version.
>> >>> + *
>> >>> + *  This program is distributed in the hope that it will be useful,
>> >>> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
>> >>> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> >>> + *  GNU General Public License for more details.
>> >>> + *
>> >>> + *  You should have received a copy of the GNU General Public License
>> >>> + *  along with this program; if not, write to the Free Software
>> >>> + *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
>> >>> + */
>> >>> +
>> >>> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>> >>> +
>> >>> +#include <linux/module.h>
>> >>> +#include <linux/input.h>
>> >>> +#include <linux/input/sparse-keymap.h>
>> >>> +#include <linux/acpi.h>
>> >>> +
>> >>> +MODULE_AUTHOR("Maxim Mikityanskiy <maxtram95@gmail.com>");
>> >>> +MODULE_DESCRIPTION("MSI Wind laptop WMI hotkeys driver");
>> >>> +MODULE_LICENSE("GPL");
>> >>> +
>> >>> +#define WMI_EVENT_GUID "5B3CC38A-40D9-7245-8AE6-1145B751BE3F"
>> >>> +
>> >>> +MODULE_ALIAS("wmi:" WMI_EVENT_GUID);
>> >>> +
>> >>> +/* Fn+F3 touchpad toggle */
>> >>> +#define WIND_KEY_TOUCHPAD    0x08
>> >>> +/* Fn+F11 Bluetooth toggle */
>> >>> +#define WIND_KEY_BLUETOOTH   0x56
>> >>> +/* Fn+F6 webcam toggle */
>> >>> +#define WIND_KEY_CAMERA              0x57
>> >>> +/* Fn+F11 Wi-Fi toggle */
>> >>> +#define WIND_KEY_WLAN                0x5f
>> >>> +/* Fn+F10 turbo mode toggle */
>> >>> +#define WIND_KEY_TURBO               0x60
>> >>> +/* Fn+F10 ECO mode toggle */
>> >>> +#define WIND_KEY_ECO         0x69
>> >>> +
>> >>> +static struct key_entry wind_keymap[] = {
>> >>> +     /* These keys work without WMI. Ignore them to avoid double keycodes */
>> >>> +     { KE_IGNORE, WIND_KEY_TOUCHPAD,         { KEY_TOUCHPAD_TOGGLE } },
>> >>> +     { KE_IGNORE, WIND_KEY_BLUETOOTH,        { KEY_BLUETOOTH } },
>> >>> +     { KE_IGNORE, WIND_KEY_CAMERA,           { KEY_CAMERA } },
>> >>> +     { KE_IGNORE, WIND_KEY_WLAN,             { KEY_WLAN } },
>> >>> +     /* These are keys that should be handled via WMI */
>> >>> +     { KE_KEY,    WIND_KEY_TURBO,            { KEY_PROG1 } },
>> >>> +     { KE_KEY,    WIND_KEY_ECO,              { KEY_PROG2 } },
>> >>> +     { KE_END, 0 }
>> >>> +};
>> >>> +
>> >>> +static struct input_dev *wind_input_dev;
>> >>> +
>> >>> +static void wind_wmi_notify(u32 value, void *context)
>> >>> +{
>> >>> +     struct acpi_buffer response = {
>> >>> +             .length         = ACPI_ALLOCATE_BUFFER,
>> >>> +             .pointer        = NULL
>> >>> +     };
>> >>> +     acpi_status status = wmi_get_event_data(value, &response);
>> >>> +     union acpi_object *obj = response.pointer;
>> >>> +     int code;
>> >>> +
>> >>> +     if (status != AE_OK) {
>> >>> +             pr_warn("Bad event status %#x\n", status);
>> >>> +             return;
>> >>> +     }
>> >>> +
>> >>> +     if (!obj || obj->type != ACPI_TYPE_INTEGER) {
>> >>> +             pr_warn("Unknown event received\n");
>> >>> +             goto wind_wmi_notify_free;
>> >>> +     }
>> >>> +
>> >>> +     code = obj->integer.value;
>> >>> +     pr_debug("Event code: %#x\n", code);
>> >>> +
>> >>> +     if (code == 0x00 || code == 0x62 || code == 0x63) {
>> >>> +             /* Unknown events - drop them for now */
>> >>> +             goto wind_wmi_notify_free;
>> >>> +     }
>> >>> +
>> >>> +     if (!sparse_keymap_report_event(wind_input_dev, code, 1, true))
>> >>> +             pr_warn("Unknown key %#x pressed\n", code);
>> >>> +
>> >>> +wind_wmi_notify_free:
>> >>> +     kfree(response.pointer);
>> >>> +}
>> >>> +
>> >>> +static int __init wind_input_setup(void)
>> >>> +{
>> >>> +     int err;
>> >>> +
>> >>> +     wind_input_dev = input_allocate_device();
>> >>> +     if (!wind_input_dev)
>> >>> +             return -ENOMEM;
>> >>> +
>> >>> +     wind_input_dev->name = "MSI WMI hotkeys";
>> >>> +     wind_input_dev->phys = "wmi/input0";
>> >>> +     wind_input_dev->id.bustype = BUS_HOST;
>> >>> +
>> >>> +     err = sparse_keymap_setup(wind_input_dev, wind_keymap, NULL);
>> >>> +     if (err)
>> >>> +             goto wind_input_setup_free_dev;
>> >>> +
>> >>> +     err = input_register_device(wind_input_dev);
>> >>> +     if (err)
>> >>> +             goto wind_input_setup_free_keymap;
>> >>> +
>> >>> +     return 0;
>> >>> +
>> >>> +wind_input_setup_free_keymap:
>> >>> +     sparse_keymap_free(wind_input_dev);
>> >>> +wind_input_setup_free_dev:
>> >>> +     input_free_device(wind_input_dev);
>> >>> +     return err;
>> >>> +}
>> >>> +
>> >>> +static int __init wind_wmi_init(void)
>> >>> +{
>> >>> +     int err;
>> >>> +
>> >>> +     if (!wmi_has_guid(WMI_EVENT_GUID)) {
>> >>> +             pr_err("No MSI Wind WMI found\n");
>> >>> +             return -ENODEV;
>> >>> +     }
>> >>> +
>> >>> +     err = wind_input_setup();
>> >>> +     if (err) {
>> >>> +             pr_err("Unable to setup input device\n");
>> >>> +             return err;
>> >>> +     }
>> >>> +
>> >>> +     if (ACPI_FAILURE(wmi_install_notify_handler(WMI_EVENT_GUID,
>> >>> +             wind_wmi_notify, NULL))) {
>> >>> +             pr_err("Unable to install WMI notify handler\n");
>> >>> +             err = -EIO;
>> >>> +             goto wind_wmi_init_unregister_input;
>> >>> +     }
>> >>> +
>> >>> +     pr_debug("Event handler installed\n");
>> >>> +
>> >>> +     return 0;
>> >>> +
>> >>> +wind_wmi_init_unregister_input:
>> >>> +     input_unregister_device(wind_input_dev);
>> >>> +     sparse_keymap_free(wind_input_dev);
>> >>> +     input_free_device(wind_input_dev);
>> >>> +     return err;
>> >>> +}
>> >>> +
>> >>> +static void __exit wind_wmi_exit(void)
>> >>> +{
>> >>> +     wmi_remove_notify_handler(WMI_EVENT_GUID);
>> >>> +     input_unregister_device(wind_input_dev);
>> >>> +     sparse_keymap_free(wind_input_dev);
>> >>> +     input_free_device(wind_input_dev);
>> >>> +}
>> >>> +
>> >>> +module_init(wind_wmi_init);
>> >>> +module_exit(wind_wmi_exit);
>> >>
>> >>
>> >> --
>> >> To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in
>> >> the body of a message to majordomo@vger.kernel.org
>> >> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> >
>> >
>> >
>> > --
>> > Corentin Chary
>> > http://xf.iksaif.net
>>
>
>

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

* Re: [PATCH 4/6] msi-laptop: Add MSI Wind U90/U100 support
  2012-11-28 17:41     ` Maxim Mikityanskiy
@ 2012-11-29  4:14       ` joeyli
  2012-11-29 16:31         ` Maxim Mikityanskiy
  0 siblings, 1 reply; 25+ messages in thread
From: joeyli @ 2012-11-29  4:14 UTC (permalink / raw)
  To: Maxim Mikityanskiy; +Cc: platform-driver-x86, Lee, Chun-Yi, Matthew Garrett

於 三,2012-11-28 於 19:41 +0200,Maxim Mikityanskiy 提到:
> 2012/11/28 joeyli <jlee@suse.com>:
> > 於 日,2012-11-25 於 00:28 +0200,Maxim Mikityanskiy 提到:
> >> Add MSI Wind U90/U100 to separate DMI table, add U90/U100 specific
> >> workarounds and add some missing EC features support such as basic fan
> >> control, turbo and ECO modes and touchpad state.
> >>
> >> Signed-off-by: Maxim Mikityanskiy <maxtram95@gmail.com>
> >> ---
> >>  drivers/platform/x86/msi-laptop.c | 199 ++++++++++++++++++++++++++++++++------
> >>  1 file changed, 171 insertions(+), 28 deletions(-)
> >>
> >> diff --git a/drivers/platform/x86/msi-laptop.c b/drivers/platform/x86/msi-laptop.c
> >> index 3b6f494..16e9863 100644
> >> --- a/drivers/platform/x86/msi-laptop.c
> >> +++ b/drivers/platform/x86/msi-laptop.c
> >> @@ -82,8 +82,19 @@
> >>  #define MSI_STANDARD_EC_SCM_LOAD_ADDRESS     0x2d
> >>  #define MSI_STANDARD_EC_SCM_LOAD_MASK                (1 << 0)
> >>
> >> -#define MSI_STANDARD_EC_TOUCHPAD_ADDRESS     0xe4
> >> +#define MSI_STANDARD_EC_POWER_ADDRESS                0xe4
> >
> > If 0xE4 register is not just for touchpad and power, then I suggest use
> > a more general name like: MSI_STANDARD_EC_FUNCTIONS_ADDRESS or other.
> 
> OK, I will rename it.

thanks

> >> +static ssize_t show_touchpad(struct device *dev,
> >> +     struct device_attribute *attr, char *buf)
> >> +{
> >> +
> >> +     u8 rdata;
> >> +     int result;
> >> +
> >> +     result = ec_read(MSI_STANDARD_EC_POWER_ADDRESS, &rdata);
> >> +     if (result < 0)
> >> +             return result;
> >> +
> >> +     return sprintf(buf, "%i\n", !!(rdata & MSI_STANDARD_EC_TOUCHPAD_MASK));
> >> +}
> >
> > I don't think we need create a touchpad attribute interface because
> > there already have key code raise the change to user space.
> 
> Key codes only indicate touchpad state change. We cannot determine
> initial touchpad state on boot using only key codes. I think, it might
> be useful to have a file in sysfs that always keeps actual touchpad
> state, so that we can get initial touchpad state on boot. Do you
> disagree with that?
> 

I agree this reason for add a sysfs interface for raise touchpad status
to userland.
But this interface will be a special api and I am not sure there already
have standard way in input subsystem for raise touchpad status. If there
have no standard api to raise it, I think add it is make sense.

> >> +
> >> +static ssize_t show_turbo(struct device *dev,
> >> +     struct device_attribute *attr, char *buf)
> >> +{
...
> >> -     if (!old_ec_model)
> >> +     if (!old_ec_model) {
> >>               get_threeg_exists();
> >> +             if (dmi_check_system(msi_load_scm_models_dmi_table))
> >> +                     load_scm_model = 1;
> >> +             else if (dmi_check_system(msi_load_scm_ro_models_dmi_table)) {
> >> +                     load_scm_model = 1;
> >> +                     ec_read_only = 1;
> >> +             }
> >> +     }
> >>
> >>       if (!old_ec_model && dmi_check_system(msi_load_scm_models_dmi_table))
> >>               load_scm_model = 1;
> >
> > hmm... the load_scm_model dmi table check 2 times? I think you can just
> > remove the duplicate code.
> 
> Oops, I forgot to remove the last two lines when I were preparing patches.
> 
> Do I need to resend all patches after doing all needed fixes?

Yes, please modify your patches for all changed and I suggest re-send
them by add v2 to subject, like:
	[PATCH 0/6 v2] msi-laptop: Add MSI Wind U90/U100 support
	...
	[PATCH 4/6 v2] msi-laptop: Add MSI Wind U90/U100 support

Please write down what's the v2 change for each patch on description of
patch.


Thanks a lot!
Joey Lee

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

* Re: [PATCH 3/6] msi-laptop: Code cleanups
  2012-11-28 17:32     ` Maxim Mikityanskiy
@ 2012-11-29  8:10       ` joeyli
  2012-11-29 16:31         ` Maxim Mikityanskiy
  0 siblings, 1 reply; 25+ messages in thread
From: joeyli @ 2012-11-29  8:10 UTC (permalink / raw)
  To: Maxim Mikityanskiy; +Cc: platform-driver-x86, Lee, Chun-Yi, Matthew Garrett

於 三,2012-11-28 於 19:32 +0200,Maxim Mikityanskiy 提到:
> 2012/11/28 joeyli <jlee@suse.com>:
> > 於 日,2012-11-25 於 00:28 +0200,Maxim Mikityanskiy 提到:
> >> Replaced msi_init_rfkill() by msi_update_rfkill(), removed delays by
> >> replacing delayed works by just works and made msi_laptop_i8042_filter()
> >> filter out KEY_TOUCHPAD_TOGGLE when sending KEY_TOUCHPAD_ON and
> >> KEY_TOUCHPAD_OFF.
> >
> > Please separate to 2 patches, one for touchpad key and other one for
> > rfkill.
> >
> > And,
> > I am not agree for the clean up to msi_init_rfkill unless it causes
> > problem on U90/U100. Did you find any issue on U90/U100?
> 
> In original code, we call get_wireless_state_ec_standard() in
> rfkill_init() in order to get initial rfkill state from hardware and
> after a second we call msi_init_rfkill() that sets rfkill state and
> writes it to hardware. msi_update_rfkill() reads rfkill state from
> hardware and sets it immediately. On U90/U100 writing to EC confuses
> it. In patch 4 I add a quirk in set_device_state() that blocks writing
> to EC so msi_init_rfkill() should not cause any issue, but we would
> have unneeded 1 second delay and function calls.
> 
> > I delay 1 magic second for msi_rfkill_init because there have other MSI
> > model need a delay time after we set SCM mode through write EC
> > register.
> > So, please don't change the code for msi_init_rfkill delay work, just
> > keep it works with other MSI shipping machines.
> 
> What do you think if I remove delay only for U90/U100 and keep it for
> other models?

Please consider to include this patch[1] to your msi-laptop patch set.
This patch introduced quirk_entry and merged quirk tables to
msi_dmi_table.

Currently I don't have msi machine on my hand, so I didn't test this
patch. Just feel free to modify or even separate it for put to your
patches.

And, I put a quirk that name is ec_delay, please consider to use this
quirk in your patch for add the 1 second (and 0.5 second) for other SCM
machines.

> 
> >>
> >> Signed-off-by: Maxim Mikityanskiy <maxtram95@gmail.com>
> >> ---
> >>  drivers/platform/x86/msi-laptop.c | 67 ++++++++++++++-------------------------
> >>  1 file changed, 24 insertions(+), 43 deletions(-)
> >>
> >> diff --git a/drivers/platform/x86/msi-laptop.c b/drivers/platform/x86/msi-laptop.c
> >> index 7ba107a..3b6f494 100644
> >> --- a/drivers/platform/x86/msi-laptop.c
> >> +++ b/drivers/platform/x86/msi-laptop.c
> >> @@ -600,8 +600,23 @@ static const struct rfkill_ops rfkill_threeg_ops = {
> >>       .set_block = rfkill_threeg_set
> >>  };
...
> >>  static bool msi_laptop_i8042_filter(unsigned char data, unsigned char str,
> >>                               struct serio *port)
> >> @@ -662,14 +664,16 @@ static bool msi_laptop_i8042_filter(unsigned char data, unsigned char str,
> >>               extended = false;
> >>               switch (data) {
> >>               case 0xE4:
> >> -                     schedule_delayed_work(&msi_touchpad_work,
> >> -                             round_jiffies_relative(0.5 * HZ));
> >> -                     break;
> >> +                     schedule_work(&msi_touchpad_work);
> >
> > Don't remove the 0.5 magic delay, please! It will cause other shipped
> > MSI machines got problem could not capture the real change of EC
> > register.
> 
> The same question here.
> 

Here the same, please consider to use this quirk in your patch for add
the 0.5 second for other SCM machines.

> >> +             case 0x64:
> >> +                     /* Filter out KEY_TOUCHPAD_TOGGLE and send only
> >> +                      * KEY_TOUCHPAD_ON and KEY_TOUCHPAD_OFF
> >> +                      */
> >> +                     return true;
> >
> > Why this patch filter out 0x64 key? What's the scan code emitted when
> > press the touchpad on/off Fn key on U100? Does it emit
> > KEY_TOUCHPAD_TOGGLE and KEY_TOUCHPAD_ON/OFF at the same time?
> 
> 0xE4 stands for Fn+F3 release and 0x64 stands for Fn+F3 press. So, if
> we do not use msi-laptop driver, we will get KEY_TOUCHPAD_TOGGLE
> (maybe from i8042 driver). If we load msi-laptop driver, it hooks
> Fn+F3 release and sends KEY_TOUCHPAD_ON/OFF, so userspace will receive
> KEY_TOUCHPAD_TOGGLE before KEY_TOUCHPAD_ON/OFF. So we need to filter
> out both Fn+F3 press and release, so that KEY_TOUCHPAD_TOGGLE will not
> be sent.

OK, understood!

> >>               case 0x54:
> >>               case 0x62:
> >>               case 0x76:
> >> -                     schedule_delayed_work(&msi_rfkill_work,
> >> -                             round_jiffies_relative(0.5 * HZ));
> >> +                     schedule_work(&msi_rfkill_work);
> >>                       break;
> >>               }
...

> >>       platform_device_del(msipf_device);
> >> @@ -976,7 +958,6 @@ static void __exit msi_cleanup(void)
> >>       if (load_scm_model) {
> >>               i8042_remove_filter(msi_laptop_i8042_filter);
> >>               msi_laptop_input_destroy();
> >> -             cancel_delayed_work_sync(&msi_rfkill_work);
> >>               rfkill_cleanup();
> >>       }


Thanks a lot!
Joey Lee

[1]

From 79db5e187ac10aea5f142ce0a352dcd17442c190 Mon Sep 17 00:00:00 2001
From: "Lee, Chun-Yi" <jlee@suse.com>
Date: Fri, 30 Nov 2012 01:22:21 +0800
Subject: [PATCH] msi-laptop: merge quirk tables to one

This patch introduced a quirk_entry struct, then we merged all quirk tables to
msi_dmi_table. Then we can more easily to set different quirk attributes for different
machine.

Signed-off-by: Lee, Chun-Yi <jlee@suse.com>
---
 drivers/platform/x86/msi-laptop.c |  125 +++++++++++++++++++++----------------
 1 file changed, 72 insertions(+), 53 deletions(-)

diff --git a/drivers/platform/x86/msi-laptop.c b/drivers/platform/x86/msi-laptop.c
index 16e9863..2447128 100644
--- a/drivers/platform/x86/msi-laptop.c
+++ b/drivers/platform/x86/msi-laptop.c
@@ -119,29 +119,35 @@ static const struct key_entry msi_laptop_keymap[] = {
 
 static struct input_dev *msi_laptop_input_dev;
 
-static bool old_ec_model;
 static int wlan_s, bluetooth_s, threeg_s;
 static int threeg_exists;
+static struct rfkill *rfk_wlan, *rfk_bluetooth, *rfk_threeg;
 
-/* Some MSI 3G netbook only have one fn key to control Wlan/Bluetooth/3G,
- * those netbook will load the SCM (windows app) to disable the original
- * Wlan/Bluetooth control by BIOS when user press fn key, then control
- * Wlan/Bluetooth/3G by SCM (software control by OS). Without SCM, user
- * cann't on/off 3G module on those 3G netbook.
- * On Linux, msi-laptop driver will do the same thing to disable the
- * original BIOS control, then might need use HAL or other userland
- * application to do the software control that simulate with SCM.
- * e.g. MSI N034 netbook
- */
-static bool load_scm_model;
+/* MSI laptop quirks */
+struct quirk_entry {
+	u8 old_ec_model;
+
+	/* Some MSI 3G netbook only have one fn key to control Wlan/Bluetooth/3G,
+	 * those netbook will load the SCM (windows app) to disable the original
+	 * Wlan/Bluetooth control by BIOS when user press fn key, then control
+	 * Wlan/Bluetooth/3G by SCM (software control by OS). Without SCM, user
+	 * cann't on/off 3G module on those 3G netbook.
+	 * On Linux, msi-laptop driver will do the same thing to disable the
+	 * original BIOS control, then might need use HAL or other userland
+	 * application to do the software control that simulate with SCM.
+	 * e.g. MSI N034 netbook
+	 */
+	u8 load_scm_model;
+	u8 ec_delay;
 
-/* Some MSI Wind netbooks (e.g. MSI Wind U100) need loading SCM to get some
- * features working (e.g. ECO mode), but we cannot change Wlan/Bluetooth state
- * in software and we can only read its state.
- */
-static bool ec_read_only;
+	/* Some MSI Wind netbooks (e.g. MSI Wind U100) need loading SCM to get some
+	 * features working (e.g. ECO mode), but we cannot change Wlan/Bluetooth state
+	 * in software and we can only read its state.
+	 */
+	u8 ec_read_only;
+};
 
-static struct rfkill *rfk_wlan, *rfk_bluetooth, *rfk_threeg;
+static struct quirk_entry *quirks;
 
 /* Hardware access */
 
@@ -213,7 +219,7 @@ static ssize_t set_device_state(const char *buf, size_t count, u8 mask)
 	if (sscanf(buf, "%i", &status) != 1 || (status < 0 || status > 1))
 		return -EINVAL;
 
-	if (ec_read_only)
+	if (quirks->ec_read_only)
 		return -EOPNOTSUPP;
 
 	/* read current device state */
@@ -314,7 +320,7 @@ static ssize_t show_wlan(struct device *dev,
 
 	int ret, enabled = 0;
 
-	if (old_ec_model) {
+	if (quirks->old_ec_model) {
 		ret = get_wireless_state(&enabled, NULL);
 	} else {
 		ret = get_wireless_state_ec_standard();
@@ -338,7 +344,7 @@ static ssize_t show_bluetooth(struct device *dev,
 
 	int ret, enabled = 0;
 
-	if (old_ec_model) {
+	if (quirks->old_ec_model) {
 		ret = get_wireless_state(NULL, &enabled);
 	} else {
 		ret = get_wireless_state_ec_standard();
@@ -363,7 +369,7 @@ static ssize_t show_threeg(struct device *dev,
 	int ret;
 
 	/* old msi ec not support 3G */
-	if (old_ec_model)
+	if (quirks->old_ec_model)
 		return -ENODEV;
 
 	ret = get_wireless_state_ec_standard();
@@ -566,9 +572,29 @@ static struct platform_device *msipf_device;
 
 /* Initialization */
 
-static int dmi_check_cb(const struct dmi_system_id *id)
+static struct quirk_entry quirk_old_ec_model = {
+	.old_ec_model = 1,
+};
+
+static struct quirk_entry quirk_load_scm_model = {
+	.load_scm_model = 1,
+	.ec_delay = 1,
+};
+
+static struct quirk_entry quirk_load_scm_ro_model = {
+	.load_scm_model = 1,
+	.ec_read_only = 1,
+};
+
+static int dmi_check_cb(const struct dmi_system_id *dmi)
 {
-	pr_info("Identified laptop model '%s'\n", id->ident);
+	pr_info("Identified laptop model '%s'\n", dmi->ident);
+
+	if (dmi->driver_data)
+		quirks = dmi->driver_data;
+	else
+		quirks = &quirk_load_scm_model;
+
 	return 1;
 }
 
@@ -582,6 +608,7 @@ static struct dmi_system_id __initdata msi_dmi_table[] = {
 			DMI_MATCH(DMI_CHASSIS_VENDOR,
 				  "MICRO-STAR INT'L CO.,LTD")
 		},
+		.driver_data = &quirk_old_ec_model,
 		.callback = dmi_check_cb
 	},
 	{
@@ -592,6 +619,7 @@ static struct dmi_system_id __initdata msi_dmi_table[] = {
 			DMI_MATCH(DMI_PRODUCT_VERSION, "0581"),
 			DMI_MATCH(DMI_BOARD_NAME, "MS-1058")
 		},
+		.driver_data = &quirk_old_ec_model,
 		.callback = dmi_check_cb
 	},
 	{
@@ -602,6 +630,7 @@ static struct dmi_system_id __initdata msi_dmi_table[] = {
 			DMI_MATCH(DMI_BOARD_VENDOR, "MSI"),
 			DMI_MATCH(DMI_BOARD_NAME, "MS-1412")
 		},
+		.driver_data = &quirk_old_ec_model,
 		.callback = dmi_check_cb
 	},
 	{
@@ -613,12 +642,9 @@ static struct dmi_system_id __initdata msi_dmi_table[] = {
 			DMI_MATCH(DMI_CHASSIS_VENDOR,
 				  "MICRO-STAR INT'L CO.,LTD")
 		},
+		.driver_data = &quirk_old_ec_model,
 		.callback = dmi_check_cb
 	},
-	{ }
-};
-
-static struct dmi_system_id __initdata msi_load_scm_models_dmi_table[] = {
 	{
 		.ident = "MSI N034",
 		.matches = {
@@ -628,6 +654,7 @@ static struct dmi_system_id __initdata msi_load_scm_models_dmi_table[] = {
 			DMI_MATCH(DMI_CHASSIS_VENDOR,
 			"MICRO-STAR INTERNATIONAL CO., LTD")
 		},
+		.driver_data = &quirk_load_scm_model,
 		.callback = dmi_check_cb
 	},
 	{
@@ -639,6 +666,7 @@ static struct dmi_system_id __initdata msi_load_scm_models_dmi_table[] = {
 			DMI_MATCH(DMI_CHASSIS_VENDOR,
 			"MICRO-STAR INTERNATIONAL CO., LTD")
 		},
+		.driver_data = &quirk_load_scm_model,
 		.callback = dmi_check_cb
 	},
 	{
@@ -648,6 +676,7 @@ static struct dmi_system_id __initdata msi_load_scm_models_dmi_table[] = {
 				"MICRO-STAR INTERNATIONAL CO., LTD"),
 			DMI_MATCH(DMI_PRODUCT_NAME, "MS-N014"),
 		},
+		.driver_data = &quirk_load_scm_model,
 		.callback = dmi_check_cb
 	},
 	{
@@ -657,6 +686,7 @@ static struct dmi_system_id __initdata msi_load_scm_models_dmi_table[] = {
 				"Micro-Star International"),
 			DMI_MATCH(DMI_PRODUCT_NAME, "CR620"),
 		},
+		.driver_data = &quirk_load_scm_model,
 		.callback = dmi_check_cb
 	},
 	{
@@ -666,12 +696,9 @@ static struct dmi_system_id __initdata msi_load_scm_models_dmi_table[] = {
 				"Micro-Star International Co., Ltd."),
 			DMI_MATCH(DMI_PRODUCT_NAME, "U270 series"),
 		},
+		.driver_data = &quirk_load_scm_model,
 		.callback = dmi_check_cb
 	},
-	{ }
-};
-
-static struct dmi_system_id __initdata msi_load_scm_ro_models_dmi_table[] = {
 	{
 		.ident = "MSI U90/U100",
 		.matches = {
@@ -679,6 +706,7 @@ static struct dmi_system_id __initdata msi_load_scm_ro_models_dmi_table[] = {
 				"MICRO-STAR INTERNATIONAL CO., LTD"),
 			DMI_MATCH(DMI_PRODUCT_NAME, "U90/U100"),
 		},
+		.driver_data = &quirk_load_scm_ro_model,
 		.callback = dmi_check_cb
 	},
 	{ }
@@ -727,7 +755,7 @@ static const struct rfkill_ops rfkill_threeg_ops = {
 
 static bool msi_rfkill_set_state(struct rfkill *rfkill, bool blocked)
 {
-	if (ec_read_only)
+	if (quirks->ec_read_only)
 		return rfkill_set_hw_state(rfkill, blocked);
 	else
 		return rfkill_set_sw_state(rfkill, blocked);
@@ -877,7 +905,7 @@ static int msi_laptop_resume(struct device *device)
 	u8 data;
 	int result;
 
-	if (!load_scm_model)
+	if (!quirks->load_scm_model)
 		return 0;
 
 	/* set load SCM to disable hardware control by fn key */
@@ -935,7 +963,7 @@ static int __init load_scm_model_init(struct platform_device *sdev)
 	u8 data;
 	int result;
 
-	if (!ec_read_only) {
+	if (!quirks->ec_read_only) {
 		/* allow userland write sysfs file  */
 		dev_attr_bluetooth.store = store_bluetooth;
 		dev_attr_wlan.store = store_wlan;
@@ -992,21 +1020,12 @@ static int __init msi_init(void)
 	if (acpi_disabled)
 		return -ENODEV;
 
-	if (force || dmi_check_system(msi_dmi_table))
-		old_ec_model = 1;
+	dmi_check_system(msi_dmi_table);
+	if (force)
+		quirks = &quirk_old_ec_model;
 
-	if (!old_ec_model) {
+	if (!quirks->old_ec_model)
 		get_threeg_exists();
-		if (dmi_check_system(msi_load_scm_models_dmi_table))
-			load_scm_model = 1;
-		else if (dmi_check_system(msi_load_scm_ro_models_dmi_table)) {
-			load_scm_model = 1;
-			ec_read_only = 1;
-		}
-	}
-
-	if (!old_ec_model && dmi_check_system(msi_load_scm_models_dmi_table))
-		load_scm_model = 1;
 
 	if (auto_brightness < 0 || auto_brightness > 2)
 		return -EINVAL;
@@ -1043,7 +1062,7 @@ static int __init msi_init(void)
 	if (ret)
 		goto fail_platform_device1;
 
-	if (load_scm_model && (load_scm_model_init(msipf_device) < 0)) {
+	if (quirks->load_scm_model && (load_scm_model_init(msipf_device) < 0)) {
 		ret = -EINVAL;
 		goto fail_platform_device1;
 	}
@@ -1053,7 +1072,7 @@ static int __init msi_init(void)
 	if (ret)
 		goto fail_platform_device2;
 
-	if (!old_ec_model) {
+	if (!quirks->old_ec_model) {
 		if (threeg_exists)
 			ret = device_create_file(&msipf_device->dev,
 						&dev_attr_threeg);
@@ -1074,7 +1093,7 @@ static int __init msi_init(void)
 
 fail_platform_device2:
 
-	if (load_scm_model) {
+	if (quirks->load_scm_model) {
 		i8042_remove_filter(msi_laptop_i8042_filter);
 		rfkill_cleanup();
 	}
@@ -1097,14 +1116,14 @@ fail_backlight:
 
 static void __exit msi_cleanup(void)
 {
-	if (load_scm_model) {
+	if (quirks->load_scm_model) {
 		i8042_remove_filter(msi_laptop_i8042_filter);
 		msi_laptop_input_destroy();
 		rfkill_cleanup();
 	}
 
 	sysfs_remove_group(&msipf_device->dev.kobj, &msipf_attribute_group);
-	if (!old_ec_model && threeg_exists)
+	if (!quirks->old_ec_model && threeg_exists)
 		device_remove_file(&msipf_device->dev, &dev_attr_threeg);
 	platform_device_unregister(msipf_device);
 	platform_driver_unregister(&msipf_driver);
-- 
1.7.10.4

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

* Re: [PATCH 4/6] msi-laptop: Add MSI Wind U90/U100 support
  2012-11-29  4:14       ` joeyli
@ 2012-11-29 16:31         ` Maxim Mikityanskiy
  0 siblings, 0 replies; 25+ messages in thread
From: Maxim Mikityanskiy @ 2012-11-29 16:31 UTC (permalink / raw)
  To: joeyli; +Cc: platform-driver-x86, Lee, Chun-Yi, Matthew Garrett

2012/11/29 joeyli <jlee@suse.com>:
> 於 三,2012-11-28 於 19:41 +0200,Maxim Mikityanskiy 提到:
>> 2012/11/28 joeyli <jlee@suse.com>:
>> > 於 日,2012-11-25 於 00:28 +0200,Maxim Mikityanskiy 提到:
>> >> Add MSI Wind U90/U100 to separate DMI table, add U90/U100 specific
>> >> workarounds and add some missing EC features support such as basic fan
>> >> control, turbo and ECO modes and touchpad state.
>> >>
>> >> Signed-off-by: Maxim Mikityanskiy <maxtram95@gmail.com>
>> >> ---
>> >>  drivers/platform/x86/msi-laptop.c | 199 ++++++++++++++++++++++++++++++++------
>> >>  1 file changed, 171 insertions(+), 28 deletions(-)
>> >>
>> >> diff --git a/drivers/platform/x86/msi-laptop.c b/drivers/platform/x86/msi-laptop.c
>> >> index 3b6f494..16e9863 100644
>> >> --- a/drivers/platform/x86/msi-laptop.c
>> >> +++ b/drivers/platform/x86/msi-laptop.c
>> >> @@ -82,8 +82,19 @@
>> >>  #define MSI_STANDARD_EC_SCM_LOAD_ADDRESS     0x2d
>> >>  #define MSI_STANDARD_EC_SCM_LOAD_MASK                (1 << 0)
>> >>
>> >> -#define MSI_STANDARD_EC_TOUCHPAD_ADDRESS     0xe4
>> >> +#define MSI_STANDARD_EC_POWER_ADDRESS                0xe4
>> >
>> > If 0xE4 register is not just for touchpad and power, then I suggest use
>> > a more general name like: MSI_STANDARD_EC_FUNCTIONS_ADDRESS or other.
>>
>> OK, I will rename it.
>
> thanks
>
>> >> +static ssize_t show_touchpad(struct device *dev,
>> >> +     struct device_attribute *attr, char *buf)
>> >> +{
>> >> +
>> >> +     u8 rdata;
>> >> +     int result;
>> >> +
>> >> +     result = ec_read(MSI_STANDARD_EC_POWER_ADDRESS, &rdata);
>> >> +     if (result < 0)
>> >> +             return result;
>> >> +
>> >> +     return sprintf(buf, "%i\n", !!(rdata & MSI_STANDARD_EC_TOUCHPAD_MASK));
>> >> +}
>> >
>> > I don't think we need create a touchpad attribute interface because
>> > there already have key code raise the change to user space.
>>
>> Key codes only indicate touchpad state change. We cannot determine
>> initial touchpad state on boot using only key codes. I think, it might
>> be useful to have a file in sysfs that always keeps actual touchpad
>> state, so that we can get initial touchpad state on boot. Do you
>> disagree with that?
>>
>
> I agree this reason for add a sysfs interface for raise touchpad status
> to userland.
> But this interface will be a special api and I am not sure there already
> have standard way in input subsystem for raise touchpad status. If there
> have no standard api to raise it, I think add it is make sense.

AFAIK, there is no generic interface that exposes touchpad state. As
for now, the only way is to create such special platform-specific file
in sysfs. asus-wmi uses the same approach.

>> >> +
>> >> +static ssize_t show_turbo(struct device *dev,
>> >> +     struct device_attribute *attr, char *buf)
>> >> +{
> ...
>> >> -     if (!old_ec_model)
>> >> +     if (!old_ec_model) {
>> >>               get_threeg_exists();
>> >> +             if (dmi_check_system(msi_load_scm_models_dmi_table))
>> >> +                     load_scm_model = 1;
>> >> +             else if (dmi_check_system(msi_load_scm_ro_models_dmi_table)) {
>> >> +                     load_scm_model = 1;
>> >> +                     ec_read_only = 1;
>> >> +             }
>> >> +     }
>> >>
>> >>       if (!old_ec_model && dmi_check_system(msi_load_scm_models_dmi_table))
>> >>               load_scm_model = 1;
>> >
>> > hmm... the load_scm_model dmi table check 2 times? I think you can just
>> > remove the duplicate code.
>>
>> Oops, I forgot to remove the last two lines when I were preparing patches.
>>
>> Do I need to resend all patches after doing all needed fixes?
>
> Yes, please modify your patches for all changed and I suggest re-send
> them by add v2 to subject, like:
>         [PATCH 0/6 v2] msi-laptop: Add MSI Wind U90/U100 support
>         ...
>         [PATCH 4/6 v2] msi-laptop: Add MSI Wind U90/U100 support
>
> Please write down what's the v2 change for each patch on description of
> patch.

OK, I'll do that when v2 will be ready.

>
> Thanks a lot!
> Joey Lee
>
>

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

* Re: [PATCH 3/6] msi-laptop: Code cleanups
  2012-11-29  8:10       ` joeyli
@ 2012-11-29 16:31         ` Maxim Mikityanskiy
  0 siblings, 0 replies; 25+ messages in thread
From: Maxim Mikityanskiy @ 2012-11-29 16:31 UTC (permalink / raw)
  To: joeyli; +Cc: platform-driver-x86, Lee, Chun-Yi, Matthew Garrett

2012/11/29 joeyli <jlee@suse.com>:
> 於 三,2012-11-28 於 19:32 +0200,Maxim Mikityanskiy 提到:
>> 2012/11/28 joeyli <jlee@suse.com>:
>> > 於 日,2012-11-25 於 00:28 +0200,Maxim Mikityanskiy 提到:
>> >> Replaced msi_init_rfkill() by msi_update_rfkill(), removed delays by
>> >> replacing delayed works by just works and made msi_laptop_i8042_filter()
>> >> filter out KEY_TOUCHPAD_TOGGLE when sending KEY_TOUCHPAD_ON and
>> >> KEY_TOUCHPAD_OFF.
>> >
>> > Please separate to 2 patches, one for touchpad key and other one for
>> > rfkill.
>> >
>> > And,
>> > I am not agree for the clean up to msi_init_rfkill unless it causes
>> > problem on U90/U100. Did you find any issue on U90/U100?
>>
>> In original code, we call get_wireless_state_ec_standard() in
>> rfkill_init() in order to get initial rfkill state from hardware and
>> after a second we call msi_init_rfkill() that sets rfkill state and
>> writes it to hardware. msi_update_rfkill() reads rfkill state from
>> hardware and sets it immediately. On U90/U100 writing to EC confuses
>> it. In patch 4 I add a quirk in set_device_state() that blocks writing
>> to EC so msi_init_rfkill() should not cause any issue, but we would
>> have unneeded 1 second delay and function calls.
>>
>> > I delay 1 magic second for msi_rfkill_init because there have other MSI
>> > model need a delay time after we set SCM mode through write EC
>> > register.
>> > So, please don't change the code for msi_init_rfkill delay work, just
>> > keep it works with other MSI shipping machines.
>>
>> What do you think if I remove delay only for U90/U100 and keep it for
>> other models?
>
> Please consider to include this patch[1] to your msi-laptop patch set.
> This patch introduced quirk_entry and merged quirk tables to
> msi_dmi_table.
>
> Currently I don't have msi machine on my hand, so I didn't test this
> patch. Just feel free to modify or even separate it for put to your
> patches.

Thank you, I'll test it on my laptop and include it when I will resend
fixed patches. I'm going to modify this patch so that it could be
applied before patch that adds support for U90/U100.

> And, I put a quirk that name is ec_delay, please consider to use this
> quirk in your patch for add the 1 second (and 0.5 second) for other SCM
> machines.

OK, I'll use this quirk.

>>
>> >>
>> >> Signed-off-by: Maxim Mikityanskiy <maxtram95@gmail.com>
>> >> ---
>> >>  drivers/platform/x86/msi-laptop.c | 67 ++++++++++++++-------------------------
>> >>  1 file changed, 24 insertions(+), 43 deletions(-)
>> >>
>> >> diff --git a/drivers/platform/x86/msi-laptop.c b/drivers/platform/x86/msi-laptop.c
>> >> index 7ba107a..3b6f494 100644
>> >> --- a/drivers/platform/x86/msi-laptop.c
>> >> +++ b/drivers/platform/x86/msi-laptop.c
>> >> @@ -600,8 +600,23 @@ static const struct rfkill_ops rfkill_threeg_ops = {
>> >>       .set_block = rfkill_threeg_set
>> >>  };
> ...
>> >>  static bool msi_laptop_i8042_filter(unsigned char data, unsigned char str,
>> >>                               struct serio *port)
>> >> @@ -662,14 +664,16 @@ static bool msi_laptop_i8042_filter(unsigned char data, unsigned char str,
>> >>               extended = false;
>> >>               switch (data) {
>> >>               case 0xE4:
>> >> -                     schedule_delayed_work(&msi_touchpad_work,
>> >> -                             round_jiffies_relative(0.5 * HZ));
>> >> -                     break;
>> >> +                     schedule_work(&msi_touchpad_work);
>> >
>> > Don't remove the 0.5 magic delay, please! It will cause other shipped
>> > MSI machines got problem could not capture the real change of EC
>> > register.
>>
>> The same question here.
>>
>
> Here the same, please consider to use this quirk in your patch for add
> the 0.5 second for other SCM machines.
>
>> >> +             case 0x64:
>> >> +                     /* Filter out KEY_TOUCHPAD_TOGGLE and send only
>> >> +                      * KEY_TOUCHPAD_ON and KEY_TOUCHPAD_OFF
>> >> +                      */
>> >> +                     return true;
>> >
>> > Why this patch filter out 0x64 key? What's the scan code emitted when
>> > press the touchpad on/off Fn key on U100? Does it emit
>> > KEY_TOUCHPAD_TOGGLE and KEY_TOUCHPAD_ON/OFF at the same time?
>>
>> 0xE4 stands for Fn+F3 release and 0x64 stands for Fn+F3 press. So, if
>> we do not use msi-laptop driver, we will get KEY_TOUCHPAD_TOGGLE
>> (maybe from i8042 driver). If we load msi-laptop driver, it hooks
>> Fn+F3 release and sends KEY_TOUCHPAD_ON/OFF, so userspace will receive
>> KEY_TOUCHPAD_TOGGLE before KEY_TOUCHPAD_ON/OFF. So we need to filter
>> out both Fn+F3 press and release, so that KEY_TOUCHPAD_TOGGLE will not
>> be sent.
>
> OK, understood!
>
>> >>               case 0x54:
>> >>               case 0x62:
>> >>               case 0x76:
>> >> -                     schedule_delayed_work(&msi_rfkill_work,
>> >> -                             round_jiffies_relative(0.5 * HZ));
>> >> +                     schedule_work(&msi_rfkill_work);
>> >>                       break;
>> >>               }
> ...
>
>> >>       platform_device_del(msipf_device);
>> >> @@ -976,7 +958,6 @@ static void __exit msi_cleanup(void)
>> >>       if (load_scm_model) {
>> >>               i8042_remove_filter(msi_laptop_i8042_filter);
>> >>               msi_laptop_input_destroy();
>> >> -             cancel_delayed_work_sync(&msi_rfkill_work);
>> >>               rfkill_cleanup();
>> >>       }
>
>
> Thanks a lot!
> Joey Lee
>
> [1]
>
> >From 79db5e187ac10aea5f142ce0a352dcd17442c190 Mon Sep 17 00:00:00 2001
> From: "Lee, Chun-Yi" <jlee@suse.com>
> Date: Fri, 30 Nov 2012 01:22:21 +0800
> Subject: [PATCH] msi-laptop: merge quirk tables to one
>
> This patch introduced a quirk_entry struct, then we merged all quirk tables to
> msi_dmi_table. Then we can more easily to set different quirk attributes for different
> machine.
>
> Signed-off-by: Lee, Chun-Yi <jlee@suse.com>
> ---
>  drivers/platform/x86/msi-laptop.c |  125 +++++++++++++++++++++----------------
>  1 file changed, 72 insertions(+), 53 deletions(-)
>
> diff --git a/drivers/platform/x86/msi-laptop.c b/drivers/platform/x86/msi-laptop.c
> index 16e9863..2447128 100644
> --- a/drivers/platform/x86/msi-laptop.c
> +++ b/drivers/platform/x86/msi-laptop.c
> @@ -119,29 +119,35 @@ static const struct key_entry msi_laptop_keymap[] = {
>
>  static struct input_dev *msi_laptop_input_dev;
>
> -static bool old_ec_model;
>  static int wlan_s, bluetooth_s, threeg_s;
>  static int threeg_exists;
> +static struct rfkill *rfk_wlan, *rfk_bluetooth, *rfk_threeg;
>
> -/* Some MSI 3G netbook only have one fn key to control Wlan/Bluetooth/3G,
> - * those netbook will load the SCM (windows app) to disable the original
> - * Wlan/Bluetooth control by BIOS when user press fn key, then control
> - * Wlan/Bluetooth/3G by SCM (software control by OS). Without SCM, user
> - * cann't on/off 3G module on those 3G netbook.
> - * On Linux, msi-laptop driver will do the same thing to disable the
> - * original BIOS control, then might need use HAL or other userland
> - * application to do the software control that simulate with SCM.
> - * e.g. MSI N034 netbook
> - */
> -static bool load_scm_model;
> +/* MSI laptop quirks */
> +struct quirk_entry {
> +       u8 old_ec_model;
> +
> +       /* Some MSI 3G netbook only have one fn key to control Wlan/Bluetooth/3G,
> +        * those netbook will load the SCM (windows app) to disable the original
> +        * Wlan/Bluetooth control by BIOS when user press fn key, then control
> +        * Wlan/Bluetooth/3G by SCM (software control by OS). Without SCM, user
> +        * cann't on/off 3G module on those 3G netbook.
> +        * On Linux, msi-laptop driver will do the same thing to disable the
> +        * original BIOS control, then might need use HAL or other userland
> +        * application to do the software control that simulate with SCM.
> +        * e.g. MSI N034 netbook
> +        */
> +       u8 load_scm_model;
> +       u8 ec_delay;
>
> -/* Some MSI Wind netbooks (e.g. MSI Wind U100) need loading SCM to get some
> - * features working (e.g. ECO mode), but we cannot change Wlan/Bluetooth state
> - * in software and we can only read its state.
> - */
> -static bool ec_read_only;
> +       /* Some MSI Wind netbooks (e.g. MSI Wind U100) need loading SCM to get some
> +        * features working (e.g. ECO mode), but we cannot change Wlan/Bluetooth state
> +        * in software and we can only read its state.
> +        */
> +       u8 ec_read_only;
> +};
>
> -static struct rfkill *rfk_wlan, *rfk_bluetooth, *rfk_threeg;
> +static struct quirk_entry *quirks;
>
>  /* Hardware access */
>
> @@ -213,7 +219,7 @@ static ssize_t set_device_state(const char *buf, size_t count, u8 mask)
>         if (sscanf(buf, "%i", &status) != 1 || (status < 0 || status > 1))
>                 return -EINVAL;
>
> -       if (ec_read_only)
> +       if (quirks->ec_read_only)
>                 return -EOPNOTSUPP;
>
>         /* read current device state */
> @@ -314,7 +320,7 @@ static ssize_t show_wlan(struct device *dev,
>
>         int ret, enabled = 0;
>
> -       if (old_ec_model) {
> +       if (quirks->old_ec_model) {
>                 ret = get_wireless_state(&enabled, NULL);
>         } else {
>                 ret = get_wireless_state_ec_standard();
> @@ -338,7 +344,7 @@ static ssize_t show_bluetooth(struct device *dev,
>
>         int ret, enabled = 0;
>
> -       if (old_ec_model) {
> +       if (quirks->old_ec_model) {
>                 ret = get_wireless_state(NULL, &enabled);
>         } else {
>                 ret = get_wireless_state_ec_standard();
> @@ -363,7 +369,7 @@ static ssize_t show_threeg(struct device *dev,
>         int ret;
>
>         /* old msi ec not support 3G */
> -       if (old_ec_model)
> +       if (quirks->old_ec_model)
>                 return -ENODEV;
>
>         ret = get_wireless_state_ec_standard();
> @@ -566,9 +572,29 @@ static struct platform_device *msipf_device;
>
>  /* Initialization */
>
> -static int dmi_check_cb(const struct dmi_system_id *id)
> +static struct quirk_entry quirk_old_ec_model = {
> +       .old_ec_model = 1,
> +};
> +
> +static struct quirk_entry quirk_load_scm_model = {
> +       .load_scm_model = 1,
> +       .ec_delay = 1,
> +};
> +
> +static struct quirk_entry quirk_load_scm_ro_model = {
> +       .load_scm_model = 1,
> +       .ec_read_only = 1,
> +};
> +
> +static int dmi_check_cb(const struct dmi_system_id *dmi)
>  {
> -       pr_info("Identified laptop model '%s'\n", id->ident);
> +       pr_info("Identified laptop model '%s'\n", dmi->ident);
> +
> +       if (dmi->driver_data)
> +               quirks = dmi->driver_data;
> +       else
> +               quirks = &quirk_load_scm_model;
> +
>         return 1;
>  }
>
> @@ -582,6 +608,7 @@ static struct dmi_system_id __initdata msi_dmi_table[] = {
>                         DMI_MATCH(DMI_CHASSIS_VENDOR,
>                                   "MICRO-STAR INT'L CO.,LTD")
>                 },
> +               .driver_data = &quirk_old_ec_model,
>                 .callback = dmi_check_cb
>         },
>         {
> @@ -592,6 +619,7 @@ static struct dmi_system_id __initdata msi_dmi_table[] = {
>                         DMI_MATCH(DMI_PRODUCT_VERSION, "0581"),
>                         DMI_MATCH(DMI_BOARD_NAME, "MS-1058")
>                 },
> +               .driver_data = &quirk_old_ec_model,
>                 .callback = dmi_check_cb
>         },
>         {
> @@ -602,6 +630,7 @@ static struct dmi_system_id __initdata msi_dmi_table[] = {
>                         DMI_MATCH(DMI_BOARD_VENDOR, "MSI"),
>                         DMI_MATCH(DMI_BOARD_NAME, "MS-1412")
>                 },
> +               .driver_data = &quirk_old_ec_model,
>                 .callback = dmi_check_cb
>         },
>         {
> @@ -613,12 +642,9 @@ static struct dmi_system_id __initdata msi_dmi_table[] = {
>                         DMI_MATCH(DMI_CHASSIS_VENDOR,
>                                   "MICRO-STAR INT'L CO.,LTD")
>                 },
> +               .driver_data = &quirk_old_ec_model,
>                 .callback = dmi_check_cb
>         },
> -       { }
> -};
> -
> -static struct dmi_system_id __initdata msi_load_scm_models_dmi_table[] = {
>         {
>                 .ident = "MSI N034",
>                 .matches = {
> @@ -628,6 +654,7 @@ static struct dmi_system_id __initdata msi_load_scm_models_dmi_table[] = {
>                         DMI_MATCH(DMI_CHASSIS_VENDOR,
>                         "MICRO-STAR INTERNATIONAL CO., LTD")
>                 },
> +               .driver_data = &quirk_load_scm_model,
>                 .callback = dmi_check_cb
>         },
>         {
> @@ -639,6 +666,7 @@ static struct dmi_system_id __initdata msi_load_scm_models_dmi_table[] = {
>                         DMI_MATCH(DMI_CHASSIS_VENDOR,
>                         "MICRO-STAR INTERNATIONAL CO., LTD")
>                 },
> +               .driver_data = &quirk_load_scm_model,
>                 .callback = dmi_check_cb
>         },
>         {
> @@ -648,6 +676,7 @@ static struct dmi_system_id __initdata msi_load_scm_models_dmi_table[] = {
>                                 "MICRO-STAR INTERNATIONAL CO., LTD"),
>                         DMI_MATCH(DMI_PRODUCT_NAME, "MS-N014"),
>                 },
> +               .driver_data = &quirk_load_scm_model,
>                 .callback = dmi_check_cb
>         },
>         {
> @@ -657,6 +686,7 @@ static struct dmi_system_id __initdata msi_load_scm_models_dmi_table[] = {
>                                 "Micro-Star International"),
>                         DMI_MATCH(DMI_PRODUCT_NAME, "CR620"),
>                 },
> +               .driver_data = &quirk_load_scm_model,
>                 .callback = dmi_check_cb
>         },
>         {
> @@ -666,12 +696,9 @@ static struct dmi_system_id __initdata msi_load_scm_models_dmi_table[] = {
>                                 "Micro-Star International Co., Ltd."),
>                         DMI_MATCH(DMI_PRODUCT_NAME, "U270 series"),
>                 },
> +               .driver_data = &quirk_load_scm_model,
>                 .callback = dmi_check_cb
>         },
> -       { }
> -};
> -
> -static struct dmi_system_id __initdata msi_load_scm_ro_models_dmi_table[] = {
>         {
>                 .ident = "MSI U90/U100",
>                 .matches = {
> @@ -679,6 +706,7 @@ static struct dmi_system_id __initdata msi_load_scm_ro_models_dmi_table[] = {
>                                 "MICRO-STAR INTERNATIONAL CO., LTD"),
>                         DMI_MATCH(DMI_PRODUCT_NAME, "U90/U100"),
>                 },
> +               .driver_data = &quirk_load_scm_ro_model,
>                 .callback = dmi_check_cb
>         },
>         { }
> @@ -727,7 +755,7 @@ static const struct rfkill_ops rfkill_threeg_ops = {
>
>  static bool msi_rfkill_set_state(struct rfkill *rfkill, bool blocked)
>  {
> -       if (ec_read_only)
> +       if (quirks->ec_read_only)
>                 return rfkill_set_hw_state(rfkill, blocked);
>         else
>                 return rfkill_set_sw_state(rfkill, blocked);
> @@ -877,7 +905,7 @@ static int msi_laptop_resume(struct device *device)
>         u8 data;
>         int result;
>
> -       if (!load_scm_model)
> +       if (!quirks->load_scm_model)
>                 return 0;
>
>         /* set load SCM to disable hardware control by fn key */
> @@ -935,7 +963,7 @@ static int __init load_scm_model_init(struct platform_device *sdev)
>         u8 data;
>         int result;
>
> -       if (!ec_read_only) {
> +       if (!quirks->ec_read_only) {
>                 /* allow userland write sysfs file  */
>                 dev_attr_bluetooth.store = store_bluetooth;
>                 dev_attr_wlan.store = store_wlan;
> @@ -992,21 +1020,12 @@ static int __init msi_init(void)
>         if (acpi_disabled)
>                 return -ENODEV;
>
> -       if (force || dmi_check_system(msi_dmi_table))
> -               old_ec_model = 1;
> +       dmi_check_system(msi_dmi_table);
> +       if (force)
> +               quirks = &quirk_old_ec_model;
>
> -       if (!old_ec_model) {
> +       if (!quirks->old_ec_model)
>                 get_threeg_exists();
> -               if (dmi_check_system(msi_load_scm_models_dmi_table))
> -                       load_scm_model = 1;
> -               else if (dmi_check_system(msi_load_scm_ro_models_dmi_table)) {
> -                       load_scm_model = 1;
> -                       ec_read_only = 1;
> -               }
> -       }
> -
> -       if (!old_ec_model && dmi_check_system(msi_load_scm_models_dmi_table))
> -               load_scm_model = 1;
>
>         if (auto_brightness < 0 || auto_brightness > 2)
>                 return -EINVAL;
> @@ -1043,7 +1062,7 @@ static int __init msi_init(void)
>         if (ret)
>                 goto fail_platform_device1;
>
> -       if (load_scm_model && (load_scm_model_init(msipf_device) < 0)) {
> +       if (quirks->load_scm_model && (load_scm_model_init(msipf_device) < 0)) {
>                 ret = -EINVAL;
>                 goto fail_platform_device1;
>         }
> @@ -1053,7 +1072,7 @@ static int __init msi_init(void)
>         if (ret)
>                 goto fail_platform_device2;
>
> -       if (!old_ec_model) {
> +       if (!quirks->old_ec_model) {
>                 if (threeg_exists)
>                         ret = device_create_file(&msipf_device->dev,
>                                                 &dev_attr_threeg);
> @@ -1074,7 +1093,7 @@ static int __init msi_init(void)
>
>  fail_platform_device2:
>
> -       if (load_scm_model) {
> +       if (quirks->load_scm_model) {
>                 i8042_remove_filter(msi_laptop_i8042_filter);
>                 rfkill_cleanup();
>         }
> @@ -1097,14 +1116,14 @@ fail_backlight:
>
>  static void __exit msi_cleanup(void)
>  {
> -       if (load_scm_model) {
> +       if (quirks->load_scm_model) {
>                 i8042_remove_filter(msi_laptop_i8042_filter);
>                 msi_laptop_input_destroy();
>                 rfkill_cleanup();
>         }
>
>         sysfs_remove_group(&msipf_device->dev.kobj, &msipf_attribute_group);
> -       if (!old_ec_model && threeg_exists)
> +       if (!quirks->old_ec_model && threeg_exists)
>                 device_remove_file(&msipf_device->dev, &dev_attr_threeg);
>         platform_device_unregister(msipf_device);
>         platform_driver_unregister(&msipf_driver);
> --
> 1.7.10.4
>
>
>

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

end of thread, other threads:[~2012-11-29 16:31 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-11-24 22:28 [PATCH 0/6] msi-laptop: Add MSI Wind U90/U100 support Maxim Mikityanskiy
2012-11-24 22:28 ` [PATCH 1/6] msi-laptop: Use proper return codes instead of -1 Maxim Mikityanskiy
2012-11-28  2:07   ` joeyli
2012-11-24 22:28 ` [PATCH 2/6] msi-laptop: Work around gcc warning Maxim Mikityanskiy
2012-11-28  2:47   ` joeyli
2012-11-24 22:28 ` [PATCH 3/6] msi-laptop: Code cleanups Maxim Mikityanskiy
2012-11-28  3:11   ` joeyli
2012-11-28 17:32     ` Maxim Mikityanskiy
2012-11-29  8:10       ` joeyli
2012-11-29 16:31         ` Maxim Mikityanskiy
2012-11-24 22:28 ` [PATCH 4/6] msi-laptop: Add MSI Wind U90/U100 support Maxim Mikityanskiy
2012-11-28  3:32   ` joeyli
2012-11-28 17:41     ` Maxim Mikityanskiy
2012-11-29  4:14       ` joeyli
2012-11-29 16:31         ` Maxim Mikityanskiy
2012-11-24 22:29 ` [PATCH 5/6] msi-laptop: Add missing ABI documentation Maxim Mikityanskiy
2012-11-28  3:36   ` joeyli
2012-11-24 22:29 ` [PATCH 6/6] Add MSI Wind WMI support Maxim Mikityanskiy
2012-11-27  8:14   ` joeyli
2012-11-27  8:50     ` Corentin Chary
2012-11-27 15:55       ` Maxim Mikityanskiy
2012-11-27 16:34         ` Corentin Chary
2012-11-27 16:49           ` Maxim Mikityanskiy
2012-11-27 23:20         ` joeyli
2012-11-28 18:03           ` Maxim Mikityanskiy

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.