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
