Re: [PATCHv4] hp-wmi: limit hotkey enable

2015-09-11 Thread Kyle Evans

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 
---
  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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCHv4] hp-wmi: limit hotkey enable

2015-09-10 Thread Darren Hart
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 
> ---
>  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.

-- 
Darren Hart
Intel Open Source Technology Center
--
To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" 
in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html