Re: [PATCH] thinkpad-acpi: setup hotkey polling after changinghotkey_driver_mask

[Thadeu Lima de Souza Cascardo]

On Mon, Feb 08, 2010 at 11:37:36PM -0200, Henrique de Moraes Holschuh wrote:
> Thadeu, thanks for the detailed analysis of the problem.
> 
> Please test the commit below, it should fix things cleanly.  I am quite
> tired right now, so I might have missed some details, but it survived a fast
> testing and compiled without warnings both with and without poll support
> compiled in.
> 
> Tested in a T43 by crippling the input device open handle to not start the
> poller, and by crippling hotkey_mask support to force the driver to think it
> needs to default to poll mode.  Both volume and brightness reporting worked
> fine in the test.
> 
> If it does fixes the issues you observed, I will add the tested-by, and send
> it to Len.
> 

Sorry for the delay. Takes a while to build a kernel in my old notebook
with a not-so-large config.

It works nice for me. This was a fix that has crossed my mind, but since
other init was doing the other way (hotkey_init), I did prefer that too.
Anyway, your way is cleaner and the right way.  :-)

Tested-by: Thadeu Lima de Souza Cascardo <cascardo@xxxxxxxxxxxxxx>

Thanks and my best regards,
Cascardo.

> 
> commit 8e05920a6cb236b21f31391b4479ec29ce65ccdf
> Author: Henrique de Moraes Holschuh <hmh@xxxxxxxxxx>
> Date:   Mon Feb 8 22:40:28 2010 -0200
> 
>     thinkpad-acpi: make driver events work in NVRAM poll mode
>     
>     Thadeu Lima de Souza Cascardo reports this:
>     
>     Brightness notification does not work until the user writes to
>     hotkey_mask attribute.  That's because the polling thread will only run
>     if hotkey_user_mask is set and someone is reading the input device or
>     if hotkey_driver_mask is set.  In this second case, this condition is
>     not tested after the mask is changed, because the brightness and
>     volume drivers are started after the hotkey drivers.
>     
>     Fix tpacpi_hotkey_driver_mask_set() to call hotkey_poll_setup(), so
>     that the poller kthread will be started when needed.
>     
>     Reported-by: Thadeu Lima de Souza Cascardo <cascardo@xxxxxxxxxxxxxx>
>     Signed-off-by: Henrique de Moraes Holschuh <hmh@xxxxxxxxxx>
>     Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
> 
> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
> index d12b61b..09c1fe6 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -2086,6 +2086,7 @@ static struct attribute_set *hotkey_dev_attributes;
>  
>  static void tpacpi_driver_event(const unsigned int hkey_event);
>  static void hotkey_driver_event(const unsigned int scancode);
> +static void hotkey_poll_setup(const bool may_warn);
>  
>  /* HKEY.MHKG() return bits */
>  #define TP_HOTKEY_TABLET_MASK (1 << 3)
> @@ -2268,6 +2269,8 @@ static int tpacpi_hotkey_driver_mask_set(const u32 mask)
>  
>  	rc = hotkey_mask_set((hotkey_acpi_mask | hotkey_driver_mask) &
>  							~hotkey_source_mask);
> +	hotkey_poll_setup(true);
> +
>  	mutex_unlock(&hotkey_mutex);
>  
>  	return rc;
> @@ -2552,7 +2555,7 @@ static void hotkey_poll_stop_sync(void)
>  }
>  
>  /* call with hotkey_mutex held */
> -static void hotkey_poll_setup(bool may_warn)
> +static void hotkey_poll_setup(const bool may_warn)
>  {
>  	const u32 poll_driver_mask = hotkey_driver_mask & hotkey_source_mask;
>  	const u32 poll_user_mask = hotkey_user_mask & hotkey_source_mask;
> @@ -2583,7 +2586,7 @@ static void hotkey_poll_setup(bool may_warn)
>  	}
>  }
>  
> -static void hotkey_poll_setup_safe(bool may_warn)
> +static void hotkey_poll_setup_safe(const bool may_warn)
>  {
>  	mutex_lock(&hotkey_mutex);
>  	hotkey_poll_setup(may_warn);
> @@ -2601,7 +2604,11 @@ static void hotkey_poll_set_freq(unsigned int freq)
>  
>  #else /* CONFIG_THINKPAD_ACPI_HOTKEY_POLL */
>  
> -static void hotkey_poll_setup_safe(bool __unused)
> +static void hotkey_poll_setup(const bool __unused)
> +{
> +}
> +
> +static void hotkey_poll_setup_safe(const bool __unused)
>  {
>  }
>  
> 
> -- 
>   "One disk to rule them all, One disk to find them. One disk to bring
>   them all and in the darkness grind them. In the Land of Redmond
>   where the shadows lie." -- The Silicon Valley Tarot
>   Henrique Holschuh

Attachment: signature.asc
Description: Digital signature