| Message ID | 1441907100-4141-1-git-send-email-kvans32@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
On Thu, Sep 10, 2015 at 12:45:00PM -0500, Kyle Evans wrote: > Do not write initialize magic on systems that do not have > feature query 0xb. Fixes Bug #82451. > > Redefine FEATURE_QUERY to align with 0xb and FEATURE2 with 0xd > for code clearity. > > Add a new test function, hp_wmi_bios_2008_later() & simplify > hp_wmi_bios_2009_later(), which fixes a bug in cases where > an improper value is returned. Probably also fixes Bug #69131. > > Signed-off-by: Kyle Evans <kvans32@gmail.com> > --- > Since v1: > - Refactored feature query 0xb into separate function > - Redefine FEATURE_QUERY to align with 0xb and FEATURE2 with 0xd > > Since v2: > - Simplify hp_wmi_bios_200x_later functions. No longer returns true > (4) when the test fails. However, if state is somehow useful, that is lost. > > Since v3: > - Fix whitespace, email client reformatting. Thanks, this one applies cleanly, however: drivers/platform/x86/hp-wmi.c: In function ‘hp_wmi_input_setup’: drivers/platform/x86/hp-wmi.c:675:2: error: implicit declaration of function ‘hp_wmi_2008_later’ [-Werror=implicit-function-declaration] if (!hp_wmi_bios_2009_later() && hp_wmi_2008_later()) ^ drivers/platform/x86/hp-wmi.c: At top level: drivers/platform/x86/hp-wmi.c:299:19: warning: ‘hp_wmi_bios_2008_later’ defined but not used [-Wunused-function] static int __init hp_wmi_bios_2008_later(void) Looks like you missed "bios" in the call to hp_wmi_bios_2008_later. Which suggests this version was not compile tested. As I cannot test the code myself without hardware, I depend on submitters even more to do the testing, so it is really important that you have tested the exact patch that I send to Linus.
On 09/10/2015 04:21 PM, Darren Hart wrote: > On Thu, Sep 10, 2015 at 12:45:00PM -0500, Kyle Evans wrote: >> Do not write initialize magic on systems that do not have >> feature query 0xb. Fixes Bug #82451. >> >> Redefine FEATURE_QUERY to align with 0xb and FEATURE2 with 0xd >> for code clearity. >> >> Add a new test function, hp_wmi_bios_2008_later() & simplify >> hp_wmi_bios_2009_later(), which fixes a bug in cases where >> an improper value is returned. Probably also fixes Bug #69131. >> >> Signed-off-by: Kyle Evans <kvans32@gmail.com> >> --- >> Since v1: >> - Refactored feature query 0xb into separate function >> - Redefine FEATURE_QUERY to align with 0xb and FEATURE2 with 0xd >> >> Since v2: >> - Simplify hp_wmi_bios_200x_later functions. No longer returns true >> (4) when the test fails. However, if state is somehow useful, that is lost. >> >> Since v3: >> - Fix whitespace, email client reformatting. > > Thanks, this one applies cleanly, however: > > drivers/platform/x86/hp-wmi.c: In function ‘hp_wmi_input_setup’: > drivers/platform/x86/hp-wmi.c:675:2: error: implicit declaration of function ‘hp_wmi_2008_later’ [-Werror=implicit-function-declaration] > if (!hp_wmi_bios_2009_later() && hp_wmi_2008_later()) > ^ > drivers/platform/x86/hp-wmi.c: At top level: > drivers/platform/x86/hp-wmi.c:299:19: warning: ‘hp_wmi_bios_2008_later’ defined but not used [-Wunused-function] > static int __init hp_wmi_bios_2008_later(void) > > Looks like you missed "bios" in the call to hp_wmi_bios_2008_later. Which > suggests this version was not compile tested. As I cannot test the code myself > without hardware, I depend on submitters even more to do the testing, so it is > really important that you have tested the exact patch that I send to Linus. > Sorry for all the rookie mistakes. I did run a compile and reboot script. Unfortunately, the error logic was commented out when I was debugging the reboot code and never re-enabled. Lesson learned. I'm on it. -- 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
diff --git a/drivers/platform/x86/hp-wmi.c b/drivers/platform/x86/hp-wmi.c index 0669731..45a9eaa 100644 --- a/drivers/platform/x86/hp-wmi.c +++ b/drivers/platform/x86/hp-wmi.c @@ -54,8 +54,9 @@ MODULE_ALIAS("wmi:5FB7F034-2C63-45e9-BE91-3D44E2C707E4"); #define HPWMI_HARDWARE_QUERY 0x4 #define HPWMI_WIRELESS_QUERY 0x5 #define HPWMI_BIOS_QUERY 0x9 +#define HPWMI_FEATURE_QUERY 0xb #define HPWMI_HOTKEY_QUERY 0xc -#define HPWMI_FEATURE_QUERY 0xd +#define HPWMI_FEATURE2_QUERY 0xd #define HPWMI_WIRELESS2_QUERY 0x1b #define HPWMI_POSTCODEERROR_QUERY 0x2a @@ -295,25 +296,33 @@ static int hp_wmi_tablet_state(void) return (state & 0x4) ? 1 : 0; } -static int __init hp_wmi_bios_2009_later(void) +static int __init hp_wmi_bios_2008_later(void) { int state = 0; int ret = hp_wmi_perform_query(HPWMI_FEATURE_QUERY, 0, &state, sizeof(state), sizeof(state)); - if (ret) - return ret; + if (!ret) + return 1; - return (state & 0x10) ? 1 : 0; + return (ret == HPWMI_RET_UNKNOWN_CMDTYPE) ? 0 : -ENXIO; } -static int hp_wmi_enable_hotkeys(void) +static int __init hp_wmi_bios_2009_later(void) { - int ret; - int query = 0x6e; + int state = 0; + int ret = hp_wmi_perform_query(HPWMI_FEATURE2_QUERY, 0, &state, + sizeof(state), sizeof(state)); + if (!ret) + return 1; - ret = hp_wmi_perform_query(HPWMI_BIOS_QUERY, 1, &query, sizeof(query), - 0); + return (ret == HPWMI_RET_UNKNOWN_CMDTYPE) ? 0 : -ENXIO; +} +static int hp_wmi_enable_hotkeys(void) +{ + int value = 0x6e; + int ret = hp_wmi_perform_query(HPWMI_BIOS_QUERY, 1, &value, + sizeof(value), 0); if (ret) return -EINVAL; return 0; @@ -663,7 +672,7 @@ static int __init hp_wmi_input_setup(void) hp_wmi_tablet_state()); input_sync(hp_wmi_input_dev); - if (hp_wmi_bios_2009_later() == 4) + if (!hp_wmi_bios_2009_later() && hp_wmi_2008_later()) hp_wmi_enable_hotkeys(); status = wmi_install_notify_handler(HPWMI_EVENT_GUID, hp_wmi_notify, NULL);
Do not write initialize magic on systems that do not have feature query 0xb. Fixes Bug #82451. Redefine FEATURE_QUERY to align with 0xb and FEATURE2 with 0xd for code clearity. Add a new test function, hp_wmi_bios_2008_later() & simplify hp_wmi_bios_2009_later(), which fixes a bug in cases where an improper value is returned. Probably also fixes Bug #69131. Signed-off-by: Kyle Evans <kvans32@gmail.com> --- Since v1: - Refactored feature query 0xb into separate function - Redefine FEATURE_QUERY to align with 0xb and FEATURE2 with 0xd Since v2: - Simplify hp_wmi_bios_200x_later functions. No longer returns true (4) when the test fails. However, if state is somehow useful, that is lost. Since v3: - Fix whitespace, email client reformatting. drivers/platform/x86/hp-wmi.c | 31 ++++++++++++++++++++----------- 1 file changed, 20 insertions(+), 11 deletions(-)