於 三，2012-11-28 於 19:32 +0200，Maxim Mikityanskiy 提到：
> 2012/11/28 joeyli <jlee@xxxxxxxx>:
> > 於 日，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@xxxxxxxxx>
> >> ---
> >> 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@xxxxxxxx>
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@xxxxxxxx>
---
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
--
To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
