於 三，2012-11-28 於 19:32 +0200，Maxim Mikityanskiy 提到： > 2012/11/28 joeyli <[email protected]>: > > 於 日，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 <[email protected]> > >> --- > >> 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" <[email protected]> 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 <[email protected]> --- 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 [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
