* [PATCH] platform/x86: asus-wmi: Only Tell EC the OS will handle display hotkeys from asus_nb_wmi
@ 2019-05-21 6:28 Hans de Goede
2019-05-24 16:43 ` Andy Shevchenko
2019-05-28 21:22 ` João Paulo Rechi Vita
0 siblings, 2 replies; 9+ messages in thread
From: Hans de Goede @ 2019-05-21 6:28 UTC (permalink / raw)
To: Darren Hart, Andy Shevchenko, Corentin Chary
Cc: Hans de Goede, acpi4asus-user, platform-driver-x86, linux-kernel,
João Paulo Rechi Vita
Commit 78f3ac76d9e5 ("platform/x86: asus-wmi: Tell the EC the OS will
handle the display off hotkey") causes the backlight to be permanently off
on various EeePC laptop models using the eeepc-wmi driver (Asus EeePC
1015BX, Asus EeePC 1025C).
The asus_wmi_set_devstate(ASUS_WMI_DEVID_BACKLIGHT, 2, NULL) call added
by that commit is made conditional in this commit and only enabled in
the quirk_entry structs in the asus-nb-wmi driver fixing the broken
display / backlight on various EeePC laptop models.
Cc: João Paulo Rechi Vita <jprvita@endlessm.com>
Fixes: 78f3ac76d9e5 ("platform/x86: asus-wmi: Tell the EC the OS will handle the display off hotkey")
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/platform/x86/asus-nb-wmi.c | 8 ++++++++
drivers/platform/x86/asus-wmi.c | 2 +-
drivers/platform/x86/asus-wmi.h | 1 +
3 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/platform/x86/asus-nb-wmi.c b/drivers/platform/x86/asus-nb-wmi.c
index b6f2ff95c3ed..59f3a37a44d7 100644
--- a/drivers/platform/x86/asus-nb-wmi.c
+++ b/drivers/platform/x86/asus-nb-wmi.c
@@ -78,10 +78,12 @@ static bool asus_q500a_i8042_filter(unsigned char data, unsigned char str,
static struct quirk_entry quirk_asus_unknown = {
.wapf = 0,
+ .wmi_backlight_set_devstate = true,
};
static struct quirk_entry quirk_asus_q500a = {
.i8042_filter = asus_q500a_i8042_filter,
+ .wmi_backlight_set_devstate = true,
};
/*
@@ -92,26 +94,32 @@ static struct quirk_entry quirk_asus_q500a = {
static struct quirk_entry quirk_asus_x55u = {
.wapf = 4,
.wmi_backlight_power = true,
+ .wmi_backlight_set_devstate = true,
.no_display_toggle = true,
};
static struct quirk_entry quirk_asus_wapf4 = {
.wapf = 4,
+ .wmi_backlight_set_devstate = true,
};
static struct quirk_entry quirk_asus_x200ca = {
.wapf = 2,
+ .wmi_backlight_set_devstate = true,
};
static struct quirk_entry quirk_asus_ux303ub = {
.wmi_backlight_native = true,
+ .wmi_backlight_set_devstate = true,
};
static struct quirk_entry quirk_asus_x550lb = {
+ .wmi_backlight_set_devstate = true,
.xusb2pr = 0x01D9,
};
static struct quirk_entry quirk_asus_forceals = {
+ .wmi_backlight_set_devstate = true,
.wmi_force_als_set = true,
};
diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index ee1fa93708ec..a66e99500c12 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -2131,7 +2131,7 @@ static int asus_wmi_add(struct platform_device *pdev)
err = asus_wmi_backlight_init(asus);
if (err && err != -ENODEV)
goto fail_backlight;
- } else
+ } else if (asus->driver->quirks->wmi_backlight_set_devstate)
err = asus_wmi_set_devstate(ASUS_WMI_DEVID_BACKLIGHT, 2, NULL);
status = wmi_install_notify_handler(asus->driver->event_guid,
diff --git a/drivers/platform/x86/asus-wmi.h b/drivers/platform/x86/asus-wmi.h
index 6c1311f4b04d..57a79bddb286 100644
--- a/drivers/platform/x86/asus-wmi.h
+++ b/drivers/platform/x86/asus-wmi.h
@@ -44,6 +44,7 @@ struct quirk_entry {
bool store_backlight_power;
bool wmi_backlight_power;
bool wmi_backlight_native;
+ bool wmi_backlight_set_devstate;
bool wmi_force_als_set;
int wapf;
/*
--
2.21.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH] platform/x86: asus-wmi: Only Tell EC the OS will handle display hotkeys from asus_nb_wmi
2019-05-21 6:28 [PATCH] platform/x86: asus-wmi: Only Tell EC the OS will handle display hotkeys from asus_nb_wmi Hans de Goede
@ 2019-05-24 16:43 ` Andy Shevchenko
2019-05-28 13:37 ` Hans de Goede
2019-05-28 21:22 ` João Paulo Rechi Vita
1 sibling, 1 reply; 9+ messages in thread
From: Andy Shevchenko @ 2019-05-24 16:43 UTC (permalink / raw)
To: Hans de Goede
Cc: Darren Hart, Andy Shevchenko, Corentin Chary, acpi4asus-user,
Platform Driver, Linux Kernel Mailing List,
João Paulo Rechi Vita
On Tue, May 21, 2019 at 9:28 AM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Commit 78f3ac76d9e5 ("platform/x86: asus-wmi: Tell the EC the OS will
> handle the display off hotkey") causes the backlight to be permanently off
> on various EeePC laptop models using the eeepc-wmi driver (Asus EeePC
> 1015BX, Asus EeePC 1025C).
>
> The asus_wmi_set_devstate(ASUS_WMI_DEVID_BACKLIGHT, 2, NULL) call added
> by that commit is made conditional in this commit and only enabled in
> the quirk_entry structs in the asus-nb-wmi driver fixing the broken
> display / backlight on various EeePC laptop models.
>
Hmm... doesn't apply.
> Cc: João Paulo Rechi Vita <jprvita@endlessm.com>
> Fixes: 78f3ac76d9e5 ("platform/x86: asus-wmi: Tell the EC the OS will handle the display off hotkey")
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> drivers/platform/x86/asus-nb-wmi.c | 8 ++++++++
> drivers/platform/x86/asus-wmi.c | 2 +-
> drivers/platform/x86/asus-wmi.h | 1 +
> 3 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/platform/x86/asus-nb-wmi.c b/drivers/platform/x86/asus-nb-wmi.c
> index b6f2ff95c3ed..59f3a37a44d7 100644
> --- a/drivers/platform/x86/asus-nb-wmi.c
> +++ b/drivers/platform/x86/asus-nb-wmi.c
> @@ -78,10 +78,12 @@ static bool asus_q500a_i8042_filter(unsigned char data, unsigned char str,
>
> static struct quirk_entry quirk_asus_unknown = {
> .wapf = 0,
> + .wmi_backlight_set_devstate = true,
> };
>
> static struct quirk_entry quirk_asus_q500a = {
> .i8042_filter = asus_q500a_i8042_filter,
> + .wmi_backlight_set_devstate = true,
> };
>
> /*
> @@ -92,26 +94,32 @@ static struct quirk_entry quirk_asus_q500a = {
> static struct quirk_entry quirk_asus_x55u = {
> .wapf = 4,
> .wmi_backlight_power = true,
> + .wmi_backlight_set_devstate = true,
> .no_display_toggle = true,
> };
>
> static struct quirk_entry quirk_asus_wapf4 = {
> .wapf = 4,
> + .wmi_backlight_set_devstate = true,
> };
>
> static struct quirk_entry quirk_asus_x200ca = {
> .wapf = 2,
> + .wmi_backlight_set_devstate = true,
> };
>
> static struct quirk_entry quirk_asus_ux303ub = {
> .wmi_backlight_native = true,
> + .wmi_backlight_set_devstate = true,
> };
>
> static struct quirk_entry quirk_asus_x550lb = {
> + .wmi_backlight_set_devstate = true,
> .xusb2pr = 0x01D9,
> };
>
> static struct quirk_entry quirk_asus_forceals = {
> + .wmi_backlight_set_devstate = true,
> .wmi_force_als_set = true,
> };
>
> diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
> index ee1fa93708ec..a66e99500c12 100644
> --- a/drivers/platform/x86/asus-wmi.c
> +++ b/drivers/platform/x86/asus-wmi.c
> @@ -2131,7 +2131,7 @@ static int asus_wmi_add(struct platform_device *pdev)
> err = asus_wmi_backlight_init(asus);
> if (err && err != -ENODEV)
> goto fail_backlight;
> - } else
> + } else if (asus->driver->quirks->wmi_backlight_set_devstate)
> err = asus_wmi_set_devstate(ASUS_WMI_DEVID_BACKLIGHT, 2, NULL);
>
> status = wmi_install_notify_handler(asus->driver->event_guid,
> diff --git a/drivers/platform/x86/asus-wmi.h b/drivers/platform/x86/asus-wmi.h
> index 6c1311f4b04d..57a79bddb286 100644
> --- a/drivers/platform/x86/asus-wmi.h
> +++ b/drivers/platform/x86/asus-wmi.h
> @@ -44,6 +44,7 @@ struct quirk_entry {
> bool store_backlight_power;
> bool wmi_backlight_power;
> bool wmi_backlight_native;
> + bool wmi_backlight_set_devstate;
> bool wmi_force_als_set;
> int wapf;
> /*
> --
> 2.21.0
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] platform/x86: asus-wmi: Only Tell EC the OS will handle display hotkeys from asus_nb_wmi
2019-05-24 16:43 ` Andy Shevchenko
@ 2019-05-28 13:37 ` Hans de Goede
2019-06-05 18:15 ` Hans de Goede
0 siblings, 1 reply; 9+ messages in thread
From: Hans de Goede @ 2019-05-28 13:37 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Darren Hart, Andy Shevchenko, Corentin Chary, acpi4asus-user,
Platform Driver, Linux Kernel Mailing List,
João Paulo Rechi Vita
Hi,
On 24-05-19 18:43, Andy Shevchenko wrote:
> On Tue, May 21, 2019 at 9:28 AM Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> Commit 78f3ac76d9e5 ("platform/x86: asus-wmi: Tell the EC the OS will
>> handle the display off hotkey") causes the backlight to be permanently off
>> on various EeePC laptop models using the eeepc-wmi driver (Asus EeePC
>> 1015BX, Asus EeePC 1025C).
>>
>> The asus_wmi_set_devstate(ASUS_WMI_DEVID_BACKLIGHT, 2, NULL) call added
>> by that commit is made conditional in this commit and only enabled in
>> the quirk_entry structs in the asus-nb-wmi driver fixing the broken
>> display / backlight on various EeePC laptop models.
>>
>
> Hmm... doesn't apply.
That is likely because your review-andy branch contain a whole-bunch
of asus-wmi patches.
Since this is a patch fixing a regression this should go to the fixes
branch. As the other patches so far are only in your review-andy branch
it is probably best to merge this patch to for-next + fixes first and
then rebase the set you have in review-andy on top, so that this can go
to fixes without getting a merge conflict when the rest of the patches
show up...
Regards,
Hans
>
>> Cc: João Paulo Rechi Vita <jprvita@endlessm.com>
>> Fixes: 78f3ac76d9e5 ("platform/x86: asus-wmi: Tell the EC the OS will handle the display off hotkey")
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>> drivers/platform/x86/asus-nb-wmi.c | 8 ++++++++
>> drivers/platform/x86/asus-wmi.c | 2 +-
>> drivers/platform/x86/asus-wmi.h | 1 +
>> 3 files changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/platform/x86/asus-nb-wmi.c b/drivers/platform/x86/asus-nb-wmi.c
>> index b6f2ff95c3ed..59f3a37a44d7 100644
>> --- a/drivers/platform/x86/asus-nb-wmi.c
>> +++ b/drivers/platform/x86/asus-nb-wmi.c
>> @@ -78,10 +78,12 @@ static bool asus_q500a_i8042_filter(unsigned char data, unsigned char str,
>>
>> static struct quirk_entry quirk_asus_unknown = {
>> .wapf = 0,
>> + .wmi_backlight_set_devstate = true,
>> };
>>
>> static struct quirk_entry quirk_asus_q500a = {
>> .i8042_filter = asus_q500a_i8042_filter,
>> + .wmi_backlight_set_devstate = true,
>> };
>>
>> /*
>> @@ -92,26 +94,32 @@ static struct quirk_entry quirk_asus_q500a = {
>> static struct quirk_entry quirk_asus_x55u = {
>> .wapf = 4,
>> .wmi_backlight_power = true,
>> + .wmi_backlight_set_devstate = true,
>> .no_display_toggle = true,
>> };
>>
>> static struct quirk_entry quirk_asus_wapf4 = {
>> .wapf = 4,
>> + .wmi_backlight_set_devstate = true,
>> };
>>
>> static struct quirk_entry quirk_asus_x200ca = {
>> .wapf = 2,
>> + .wmi_backlight_set_devstate = true,
>> };
>>
>> static struct quirk_entry quirk_asus_ux303ub = {
>> .wmi_backlight_native = true,
>> + .wmi_backlight_set_devstate = true,
>> };
>>
>> static struct quirk_entry quirk_asus_x550lb = {
>> + .wmi_backlight_set_devstate = true,
>> .xusb2pr = 0x01D9,
>> };
>>
>> static struct quirk_entry quirk_asus_forceals = {
>> + .wmi_backlight_set_devstate = true,
>> .wmi_force_als_set = true,
>> };
>>
>> diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
>> index ee1fa93708ec..a66e99500c12 100644
>> --- a/drivers/platform/x86/asus-wmi.c
>> +++ b/drivers/platform/x86/asus-wmi.c
>> @@ -2131,7 +2131,7 @@ static int asus_wmi_add(struct platform_device *pdev)
>> err = asus_wmi_backlight_init(asus);
>> if (err && err != -ENODEV)
>> goto fail_backlight;
>> - } else
>> + } else if (asus->driver->quirks->wmi_backlight_set_devstate)
>> err = asus_wmi_set_devstate(ASUS_WMI_DEVID_BACKLIGHT, 2, NULL);
>>
>> status = wmi_install_notify_handler(asus->driver->event_guid,
>> diff --git a/drivers/platform/x86/asus-wmi.h b/drivers/platform/x86/asus-wmi.h
>> index 6c1311f4b04d..57a79bddb286 100644
>> --- a/drivers/platform/x86/asus-wmi.h
>> +++ b/drivers/platform/x86/asus-wmi.h
>> @@ -44,6 +44,7 @@ struct quirk_entry {
>> bool store_backlight_power;
>> bool wmi_backlight_power;
>> bool wmi_backlight_native;
>> + bool wmi_backlight_set_devstate;
>> bool wmi_force_als_set;
>> int wapf;
>> /*
>> --
>> 2.21.0
>>
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] platform/x86: asus-wmi: Only Tell EC the OS will handle display hotkeys from asus_nb_wmi
2019-05-28 13:37 ` Hans de Goede
@ 2019-06-05 18:15 ` Hans de Goede
2019-06-11 19:15 ` Andy Shevchenko
0 siblings, 1 reply; 9+ messages in thread
From: Hans de Goede @ 2019-06-05 18:15 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Darren Hart, Andy Shevchenko, Corentin Chary, acpi4asus-user,
Platform Driver, Linux Kernel Mailing List,
João Paulo Rechi Vita
Hi,
On 28-05-19 15:37, Hans de Goede wrote:
> Hi,
>
> On 24-05-19 18:43, Andy Shevchenko wrote:
>> On Tue, May 21, 2019 at 9:28 AM Hans de Goede <hdegoede@redhat.com> wrote:
>>>
>>> Commit 78f3ac76d9e5 ("platform/x86: asus-wmi: Tell the EC the OS will
>>> handle the display off hotkey") causes the backlight to be permanently off
>>> on various EeePC laptop models using the eeepc-wmi driver (Asus EeePC
>>> 1015BX, Asus EeePC 1025C).
>>>
>>> The asus_wmi_set_devstate(ASUS_WMI_DEVID_BACKLIGHT, 2, NULL) call added
>>> by that commit is made conditional in this commit and only enabled in
>>> the quirk_entry structs in the asus-nb-wmi driver fixing the broken
>>> display / backlight on various EeePC laptop models.
>>>
>>
>> Hmm... doesn't apply.
>
> That is likely because your review-andy branch contain a whole-bunch
> of asus-wmi patches.
>
> Since this is a patch fixing a regression this should go to the fixes
> branch. As the other patches so far are only in your review-andy branch
> it is probably best to merge this patch to for-next + fixes first and
> then rebase the set you have in review-andy on top, so that this can go
> to fixes without getting a merge conflict when the rest of the patches
> show up...
Ping? This is a 5.x regression fix, as such it would be really good to
get this into 5.2 and then cherry-picked into 5.y.z.
Regards,
Hans
>>> Cc: João Paulo Rechi Vita <jprvita@endlessm.com>
>>> Fixes: 78f3ac76d9e5 ("platform/x86: asus-wmi: Tell the EC the OS will handle the display off hotkey")
>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>> ---
>>> drivers/platform/x86/asus-nb-wmi.c | 8 ++++++++
>>> drivers/platform/x86/asus-wmi.c | 2 +-
>>> drivers/platform/x86/asus-wmi.h | 1 +
>>> 3 files changed, 10 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/platform/x86/asus-nb-wmi.c b/drivers/platform/x86/asus-nb-wmi.c
>>> index b6f2ff95c3ed..59f3a37a44d7 100644
>>> --- a/drivers/platform/x86/asus-nb-wmi.c
>>> +++ b/drivers/platform/x86/asus-nb-wmi.c
>>> @@ -78,10 +78,12 @@ static bool asus_q500a_i8042_filter(unsigned char data, unsigned char str,
>>>
>>> static struct quirk_entry quirk_asus_unknown = {
>>> .wapf = 0,
>>> + .wmi_backlight_set_devstate = true,
>>> };
>>>
>>> static struct quirk_entry quirk_asus_q500a = {
>>> .i8042_filter = asus_q500a_i8042_filter,
>>> + .wmi_backlight_set_devstate = true,
>>> };
>>>
>>> /*
>>> @@ -92,26 +94,32 @@ static struct quirk_entry quirk_asus_q500a = {
>>> static struct quirk_entry quirk_asus_x55u = {
>>> .wapf = 4,
>>> .wmi_backlight_power = true,
>>> + .wmi_backlight_set_devstate = true,
>>> .no_display_toggle = true,
>>> };
>>>
>>> static struct quirk_entry quirk_asus_wapf4 = {
>>> .wapf = 4,
>>> + .wmi_backlight_set_devstate = true,
>>> };
>>>
>>> static struct quirk_entry quirk_asus_x200ca = {
>>> .wapf = 2,
>>> + .wmi_backlight_set_devstate = true,
>>> };
>>>
>>> static struct quirk_entry quirk_asus_ux303ub = {
>>> .wmi_backlight_native = true,
>>> + .wmi_backlight_set_devstate = true,
>>> };
>>>
>>> static struct quirk_entry quirk_asus_x550lb = {
>>> + .wmi_backlight_set_devstate = true,
>>> .xusb2pr = 0x01D9,
>>> };
>>>
>>> static struct quirk_entry quirk_asus_forceals = {
>>> + .wmi_backlight_set_devstate = true,
>>> .wmi_force_als_set = true,
>>> };
>>>
>>> diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
>>> index ee1fa93708ec..a66e99500c12 100644
>>> --- a/drivers/platform/x86/asus-wmi.c
>>> +++ b/drivers/platform/x86/asus-wmi.c
>>> @@ -2131,7 +2131,7 @@ static int asus_wmi_add(struct platform_device *pdev)
>>> err = asus_wmi_backlight_init(asus);
>>> if (err && err != -ENODEV)
>>> goto fail_backlight;
>>> - } else
>>> + } else if (asus->driver->quirks->wmi_backlight_set_devstate)
>>> err = asus_wmi_set_devstate(ASUS_WMI_DEVID_BACKLIGHT, 2, NULL);
>>>
>>> status = wmi_install_notify_handler(asus->driver->event_guid,
>>> diff --git a/drivers/platform/x86/asus-wmi.h b/drivers/platform/x86/asus-wmi.h
>>> index 6c1311f4b04d..57a79bddb286 100644
>>> --- a/drivers/platform/x86/asus-wmi.h
>>> +++ b/drivers/platform/x86/asus-wmi.h
>>> @@ -44,6 +44,7 @@ struct quirk_entry {
>>> bool store_backlight_power;
>>> bool wmi_backlight_power;
>>> bool wmi_backlight_native;
>>> + bool wmi_backlight_set_devstate;
>>> bool wmi_force_als_set;
>>> int wapf;
>>> /*
>>> --
>>> 2.21.0
>>>
>>
>>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] platform/x86: asus-wmi: Only Tell EC the OS will handle display hotkeys from asus_nb_wmi
2019-06-05 18:15 ` Hans de Goede
@ 2019-06-11 19:15 ` Andy Shevchenko
2019-06-12 6:59 ` Hans de Goede
0 siblings, 1 reply; 9+ messages in thread
From: Andy Shevchenko @ 2019-06-11 19:15 UTC (permalink / raw)
To: Hans de Goede
Cc: Darren Hart, Andy Shevchenko, Corentin Chary, acpi4asus-user,
Platform Driver, Linux Kernel Mailing List,
João Paulo Rechi Vita
On Wed, Jun 5, 2019 at 9:15 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi,
>
> On 28-05-19 15:37, Hans de Goede wrote:
> > Hi,
> >
> > On 24-05-19 18:43, Andy Shevchenko wrote:
> >> On Tue, May 21, 2019 at 9:28 AM Hans de Goede <hdegoede@redhat.com> wrote:
> >>>
> >>> Commit 78f3ac76d9e5 ("platform/x86: asus-wmi: Tell the EC the OS will
> >>> handle the display off hotkey") causes the backlight to be permanently off
> >>> on various EeePC laptop models using the eeepc-wmi driver (Asus EeePC
> >>> 1015BX, Asus EeePC 1025C).
> >>>
> >>> The asus_wmi_set_devstate(ASUS_WMI_DEVID_BACKLIGHT, 2, NULL) call added
> >>> by that commit is made conditional in this commit and only enabled in
> >>> the quirk_entry structs in the asus-nb-wmi driver fixing the broken
> >>> display / backlight on various EeePC laptop models.
> >>>
> >>
> >> Hmm... doesn't apply.
> >
> > That is likely because your review-andy branch contain a whole-bunch
> > of asus-wmi patches.
> >
> > Since this is a patch fixing a regression this should go to the fixes
> > branch. As the other patches so far are only in your review-andy branch
> > it is probably best to merge this patch to for-next + fixes first and
> > then rebase the set you have in review-andy on top, so that this can go
> > to fixes without getting a merge conflict when the rest of the patches
> > show up...
>
> Ping? This is a 5.x regression fix, as such it would be really good to
> get this into 5.2 and then cherry-picked into 5.y.z.
>
It doesn't apply to our fixes branch.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] platform/x86: asus-wmi: Only Tell EC the OS will handle display hotkeys from asus_nb_wmi
2019-06-11 19:15 ` Andy Shevchenko
@ 2019-06-12 6:59 ` Hans de Goede
0 siblings, 0 replies; 9+ messages in thread
From: Hans de Goede @ 2019-06-12 6:59 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Darren Hart, Andy Shevchenko, Corentin Chary, acpi4asus-user,
Platform Driver, Linux Kernel Mailing List,
João Paulo Rechi Vita
Hi,
On 11-06-19 21:15, Andy Shevchenko wrote:
> On Wed, Jun 5, 2019 at 9:15 PM Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> Hi,
>>
>> On 28-05-19 15:37, Hans de Goede wrote:
>>> Hi,
>>>
>>> On 24-05-19 18:43, Andy Shevchenko wrote:
>>>> On Tue, May 21, 2019 at 9:28 AM Hans de Goede <hdegoede@redhat.com> wrote:
>>>>>
>>>>> Commit 78f3ac76d9e5 ("platform/x86: asus-wmi: Tell the EC the OS will
>>>>> handle the display off hotkey") causes the backlight to be permanently off
>>>>> on various EeePC laptop models using the eeepc-wmi driver (Asus EeePC
>>>>> 1015BX, Asus EeePC 1025C).
>>>>>
>>>>> The asus_wmi_set_devstate(ASUS_WMI_DEVID_BACKLIGHT, 2, NULL) call added
>>>>> by that commit is made conditional in this commit and only enabled in
>>>>> the quirk_entry structs in the asus-nb-wmi driver fixing the broken
>>>>> display / backlight on various EeePC laptop models.
>>>>>
>>>>
>>>> Hmm... doesn't apply.
>>>
>>> That is likely because your review-andy branch contain a whole-bunch
>>> of asus-wmi patches.
>>>
>>> Since this is a patch fixing a regression this should go to the fixes
>>> branch. As the other patches so far are only in your review-andy branch
>>> it is probably best to merge this patch to for-next + fixes first and
>>> then rebase the set you have in review-andy on top, so that this can go
>>> to fixes without getting a merge conflict when the rest of the patches
>>> show up...
>>
>> Ping? This is a 5.x regression fix, as such it would be really good to
>> get this into 5.2 and then cherry-picked into 5.y.z.
>>
>
> It doesn't apply to our fixes branch.
Weird. I will resend the patch rebased on top of 5.2-rc4.
Regards,
Hans
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] platform/x86: asus-wmi: Only Tell EC the OS will handle display hotkeys from asus_nb_wmi
2019-05-21 6:28 [PATCH] platform/x86: asus-wmi: Only Tell EC the OS will handle display hotkeys from asus_nb_wmi Hans de Goede
2019-05-24 16:43 ` Andy Shevchenko
@ 2019-05-28 21:22 ` João Paulo Rechi Vita
2019-05-29 8:55 ` Hans de Goede
1 sibling, 1 reply; 9+ messages in thread
From: João Paulo Rechi Vita @ 2019-05-28 21:22 UTC (permalink / raw)
To: Hans de Goede
Cc: Darren Hart, Andy Shevchenko, Corentin Chary, acpi4asus-user,
Platform Driver, Linux Kernel Mailing List
On Mon, May 20, 2019 at 11:28 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Commit 78f3ac76d9e5 ("platform/x86: asus-wmi: Tell the EC the OS will
> handle the display off hotkey") causes the backlight to be permanently off
> on various EeePC laptop models using the eeepc-wmi driver (Asus EeePC
> 1015BX, Asus EeePC 1025C).
>
> The asus_wmi_set_devstate(ASUS_WMI_DEVID_BACKLIGHT, 2, NULL) call added
> by that commit is made conditional in this commit and only enabled in
> the quirk_entry structs in the asus-nb-wmi driver fixing the broken
> display / backlight on various EeePC laptop models.
>
> Cc: João Paulo Rechi Vita <jprvita@endlessm.com>
> Fixes: 78f3ac76d9e5 ("platform/x86: asus-wmi: Tell the EC the OS will handle the display off hotkey")
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> drivers/platform/x86/asus-nb-wmi.c | 8 ++++++++
> drivers/platform/x86/asus-wmi.c | 2 +-
> drivers/platform/x86/asus-wmi.h | 1 +
> 3 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/platform/x86/asus-nb-wmi.c b/drivers/platform/x86/asus-nb-wmi.c
> index b6f2ff95c3ed..59f3a37a44d7 100644
> --- a/drivers/platform/x86/asus-nb-wmi.c
> +++ b/drivers/platform/x86/asus-nb-wmi.c
> @@ -78,10 +78,12 @@ static bool asus_q500a_i8042_filter(unsigned char data, unsigned char str,
>
> static struct quirk_entry quirk_asus_unknown = {
> .wapf = 0,
> + .wmi_backlight_set_devstate = true,
> };
>
> static struct quirk_entry quirk_asus_q500a = {
> .i8042_filter = asus_q500a_i8042_filter,
> + .wmi_backlight_set_devstate = true,
> };
>
> /*
> @@ -92,26 +94,32 @@ static struct quirk_entry quirk_asus_q500a = {
> static struct quirk_entry quirk_asus_x55u = {
> .wapf = 4,
> .wmi_backlight_power = true,
> + .wmi_backlight_set_devstate = true,
> .no_display_toggle = true,
> };
>
> static struct quirk_entry quirk_asus_wapf4 = {
> .wapf = 4,
> + .wmi_backlight_set_devstate = true,
> };
>
> static struct quirk_entry quirk_asus_x200ca = {
> .wapf = 2,
> + .wmi_backlight_set_devstate = true,
> };
>
> static struct quirk_entry quirk_asus_ux303ub = {
> .wmi_backlight_native = true,
> + .wmi_backlight_set_devstate = true,
> };
>
> static struct quirk_entry quirk_asus_x550lb = {
> + .wmi_backlight_set_devstate = true,
> .xusb2pr = 0x01D9,
> };
>
> static struct quirk_entry quirk_asus_forceals = {
> + .wmi_backlight_set_devstate = true,
> .wmi_force_als_set = true,
> };
>
> diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
> index ee1fa93708ec..a66e99500c12 100644
> --- a/drivers/platform/x86/asus-wmi.c
> +++ b/drivers/platform/x86/asus-wmi.c
> @@ -2131,7 +2131,7 @@ static int asus_wmi_add(struct platform_device *pdev)
> err = asus_wmi_backlight_init(asus);
> if (err && err != -ENODEV)
> goto fail_backlight;
> - } else
> + } else if (asus->driver->quirks->wmi_backlight_set_devstate)
> err = asus_wmi_set_devstate(ASUS_WMI_DEVID_BACKLIGHT, 2, NULL);
>
> status = wmi_install_notify_handler(asus->driver->event_guid,
> diff --git a/drivers/platform/x86/asus-wmi.h b/drivers/platform/x86/asus-wmi.h
> index 6c1311f4b04d..57a79bddb286 100644
> --- a/drivers/platform/x86/asus-wmi.h
> +++ b/drivers/platform/x86/asus-wmi.h
> @@ -44,6 +44,7 @@ struct quirk_entry {
> bool store_backlight_power;
> bool wmi_backlight_power;
> bool wmi_backlight_native;
> + bool wmi_backlight_set_devstate;
Wouldn't it be better to add this field to struct asus_wmi_driver
instead, and set it in asus_nb_wmi_driver only? This way we wouldn't
need to make sure it is present in all quirk entries from this driver,
current and future.
I've tested both the original patch and my suggestion above and in
both cases the "turn off backlight" hotkey continued to work fine on a
machine where asus-nb-wmi is used (I don't have access to any machine
using the eeepc driver).
> bool wmi_force_als_set;
> int wapf;
> /*
> --
> 2.21.0
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] platform/x86: asus-wmi: Only Tell EC the OS will handle display hotkeys from asus_nb_wmi
2019-05-28 21:22 ` João Paulo Rechi Vita
@ 2019-05-29 8:55 ` Hans de Goede
2019-05-29 17:37 ` João Paulo Rechi Vita
0 siblings, 1 reply; 9+ messages in thread
From: Hans de Goede @ 2019-05-29 8:55 UTC (permalink / raw)
To: João Paulo Rechi Vita
Cc: Darren Hart, Andy Shevchenko, Corentin Chary, acpi4asus-user,
Platform Driver, Linux Kernel Mailing List
Hi João,
On 5/28/19 11:22 PM, João Paulo Rechi Vita wrote:
> On Mon, May 20, 2019 at 11:28 PM Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> Commit 78f3ac76d9e5 ("platform/x86: asus-wmi: Tell the EC the OS will
>> handle the display off hotkey") causes the backlight to be permanently off
>> on various EeePC laptop models using the eeepc-wmi driver (Asus EeePC
>> 1015BX, Asus EeePC 1025C).
>>
>> The asus_wmi_set_devstate(ASUS_WMI_DEVID_BACKLIGHT, 2, NULL) call added
>> by that commit is made conditional in this commit and only enabled in
>> the quirk_entry structs in the asus-nb-wmi driver fixing the broken
>> display / backlight on various EeePC laptop models.
>>
>> Cc: João Paulo Rechi Vita <jprvita@endlessm.com>
>> Fixes: 78f3ac76d9e5 ("platform/x86: asus-wmi: Tell the EC the OS will handle the display off hotkey")
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>> drivers/platform/x86/asus-nb-wmi.c | 8 ++++++++
>> drivers/platform/x86/asus-wmi.c | 2 +-
>> drivers/platform/x86/asus-wmi.h | 1 +
>> 3 files changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/platform/x86/asus-nb-wmi.c b/drivers/platform/x86/asus-nb-wmi.c
>> index b6f2ff95c3ed..59f3a37a44d7 100644
>> --- a/drivers/platform/x86/asus-nb-wmi.c
>> +++ b/drivers/platform/x86/asus-nb-wmi.c
>> @@ -78,10 +78,12 @@ static bool asus_q500a_i8042_filter(unsigned char data, unsigned char str,
>>
>> static struct quirk_entry quirk_asus_unknown = {
>> .wapf = 0,
>> + .wmi_backlight_set_devstate = true,
>> };
>>
>> static struct quirk_entry quirk_asus_q500a = {
>> .i8042_filter = asus_q500a_i8042_filter,
>> + .wmi_backlight_set_devstate = true,
>> };
>>
>> /*
>> @@ -92,26 +94,32 @@ static struct quirk_entry quirk_asus_q500a = {
>> static struct quirk_entry quirk_asus_x55u = {
>> .wapf = 4,
>> .wmi_backlight_power = true,
>> + .wmi_backlight_set_devstate = true,
>> .no_display_toggle = true,
>> };
>>
>> static struct quirk_entry quirk_asus_wapf4 = {
>> .wapf = 4,
>> + .wmi_backlight_set_devstate = true,
>> };
>>
>> static struct quirk_entry quirk_asus_x200ca = {
>> .wapf = 2,
>> + .wmi_backlight_set_devstate = true,
>> };
>>
>> static struct quirk_entry quirk_asus_ux303ub = {
>> .wmi_backlight_native = true,
>> + .wmi_backlight_set_devstate = true,
>> };
>>
>> static struct quirk_entry quirk_asus_x550lb = {
>> + .wmi_backlight_set_devstate = true,
>> .xusb2pr = 0x01D9,
>> };
>>
>> static struct quirk_entry quirk_asus_forceals = {
>> + .wmi_backlight_set_devstate = true,
>> .wmi_force_als_set = true,
>> };
>>
>> diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
>> index ee1fa93708ec..a66e99500c12 100644
>> --- a/drivers/platform/x86/asus-wmi.c
>> +++ b/drivers/platform/x86/asus-wmi.c
>> @@ -2131,7 +2131,7 @@ static int asus_wmi_add(struct platform_device *pdev)
>> err = asus_wmi_backlight_init(asus);
>> if (err && err != -ENODEV)
>> goto fail_backlight;
>> - } else
>> + } else if (asus->driver->quirks->wmi_backlight_set_devstate)
>> err = asus_wmi_set_devstate(ASUS_WMI_DEVID_BACKLIGHT, 2, NULL);
>>
>> status = wmi_install_notify_handler(asus->driver->event_guid,
>> diff --git a/drivers/platform/x86/asus-wmi.h b/drivers/platform/x86/asus-wmi.h
>> index 6c1311f4b04d..57a79bddb286 100644
>> --- a/drivers/platform/x86/asus-wmi.h
>> +++ b/drivers/platform/x86/asus-wmi.h
>> @@ -44,6 +44,7 @@ struct quirk_entry {
>> bool store_backlight_power;
>> bool wmi_backlight_power;
>> bool wmi_backlight_native;
>> + bool wmi_backlight_set_devstate;
>
> Wouldn't it be better to add this field to struct asus_wmi_driver
> instead, and set it in asus_nb_wmi_driver only? This way we wouldn't
> need to make sure it is present in all quirk entries from this driver,
> current and future.
>
> I've tested both the original patch and my suggestion above and in
> both cases the "turn off backlight" hotkey continued to work fine on a
> machine where asus-nb-wmi is used (I don't have access to any machine
> using the eeepc driver).
I deliberately put in the quirks struct so that if necessary we can
enable / disable it easily on a per model (rather then per driver)
case in the future.
Regards,
Hans
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] platform/x86: asus-wmi: Only Tell EC the OS will handle display hotkeys from asus_nb_wmi
2019-05-29 8:55 ` Hans de Goede
@ 2019-05-29 17:37 ` João Paulo Rechi Vita
0 siblings, 0 replies; 9+ messages in thread
From: João Paulo Rechi Vita @ 2019-05-29 17:37 UTC (permalink / raw)
To: Hans de Goede
Cc: Darren Hart, Andy Shevchenko, Corentin Chary, acpi4asus-user,
Platform Driver, Linux Kernel Mailing List
On Wed, May 29, 2019 at 1:55 AM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi João,
>
> On 5/28/19 11:22 PM, João Paulo Rechi Vita wrote:
> > On Mon, May 20, 2019 at 11:28 PM Hans de Goede <hdegoede@redhat.com> wrote:
> >>
> >> Commit 78f3ac76d9e5 ("platform/x86: asus-wmi: Tell the EC the OS will
> >> handle the display off hotkey") causes the backlight to be permanently off
> >> on various EeePC laptop models using the eeepc-wmi driver (Asus EeePC
> >> 1015BX, Asus EeePC 1025C).
> >>
> >> The asus_wmi_set_devstate(ASUS_WMI_DEVID_BACKLIGHT, 2, NULL) call added
> >> by that commit is made conditional in this commit and only enabled in
> >> the quirk_entry structs in the asus-nb-wmi driver fixing the broken
> >> display / backlight on various EeePC laptop models.
> >>
> >> Cc: João Paulo Rechi Vita <jprvita@endlessm.com>
> >> Fixes: 78f3ac76d9e5 ("platform/x86: asus-wmi: Tell the EC the OS will handle the display off hotkey")
> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> >> ---
> >> drivers/platform/x86/asus-nb-wmi.c | 8 ++++++++
> >> drivers/platform/x86/asus-wmi.c | 2 +-
> >> drivers/platform/x86/asus-wmi.h | 1 +
> >> 3 files changed, 10 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/platform/x86/asus-nb-wmi.c b/drivers/platform/x86/asus-nb-wmi.c
> >> index b6f2ff95c3ed..59f3a37a44d7 100644
> >> --- a/drivers/platform/x86/asus-nb-wmi.c
> >> +++ b/drivers/platform/x86/asus-nb-wmi.c
> >> @@ -78,10 +78,12 @@ static bool asus_q500a_i8042_filter(unsigned char data, unsigned char str,
> >>
> >> static struct quirk_entry quirk_asus_unknown = {
> >> .wapf = 0,
> >> + .wmi_backlight_set_devstate = true,
> >> };
> >>
> >> static struct quirk_entry quirk_asus_q500a = {
> >> .i8042_filter = asus_q500a_i8042_filter,
> >> + .wmi_backlight_set_devstate = true,
> >> };
> >>
> >> /*
> >> @@ -92,26 +94,32 @@ static struct quirk_entry quirk_asus_q500a = {
> >> static struct quirk_entry quirk_asus_x55u = {
> >> .wapf = 4,
> >> .wmi_backlight_power = true,
> >> + .wmi_backlight_set_devstate = true,
> >> .no_display_toggle = true,
> >> };
> >>
> >> static struct quirk_entry quirk_asus_wapf4 = {
> >> .wapf = 4,
> >> + .wmi_backlight_set_devstate = true,
> >> };
> >>
> >> static struct quirk_entry quirk_asus_x200ca = {
> >> .wapf = 2,
> >> + .wmi_backlight_set_devstate = true,
> >> };
> >>
> >> static struct quirk_entry quirk_asus_ux303ub = {
> >> .wmi_backlight_native = true,
> >> + .wmi_backlight_set_devstate = true,
> >> };
> >>
> >> static struct quirk_entry quirk_asus_x550lb = {
> >> + .wmi_backlight_set_devstate = true,
> >> .xusb2pr = 0x01D9,
> >> };
> >>
> >> static struct quirk_entry quirk_asus_forceals = {
> >> + .wmi_backlight_set_devstate = true,
> >> .wmi_force_als_set = true,
> >> };
> >>
> >> diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
> >> index ee1fa93708ec..a66e99500c12 100644
> >> --- a/drivers/platform/x86/asus-wmi.c
> >> +++ b/drivers/platform/x86/asus-wmi.c
> >> @@ -2131,7 +2131,7 @@ static int asus_wmi_add(struct platform_device *pdev)
> >> err = asus_wmi_backlight_init(asus);
> >> if (err && err != -ENODEV)
> >> goto fail_backlight;
> >> - } else
> >> + } else if (asus->driver->quirks->wmi_backlight_set_devstate)
> >> err = asus_wmi_set_devstate(ASUS_WMI_DEVID_BACKLIGHT, 2, NULL);
> >>
> >> status = wmi_install_notify_handler(asus->driver->event_guid,
> >> diff --git a/drivers/platform/x86/asus-wmi.h b/drivers/platform/x86/asus-wmi.h
> >> index 6c1311f4b04d..57a79bddb286 100644
> >> --- a/drivers/platform/x86/asus-wmi.h
> >> +++ b/drivers/platform/x86/asus-wmi.h
> >> @@ -44,6 +44,7 @@ struct quirk_entry {
> >> bool store_backlight_power;
> >> bool wmi_backlight_power;
> >> bool wmi_backlight_native;
> >> + bool wmi_backlight_set_devstate;
> >
> > Wouldn't it be better to add this field to struct asus_wmi_driver
> > instead, and set it in asus_nb_wmi_driver only? This way we wouldn't
> > need to make sure it is present in all quirk entries from this driver,
> > current and future.
> >
> > I've tested both the original patch and my suggestion above and in
> > both cases the "turn off backlight" hotkey continued to work fine on a
> > machine where asus-nb-wmi is used (I don't have access to any machine
> > using the eeepc driver).
>
> I deliberately put in the quirks struct so that if necessary we can
> enable / disable it easily on a per model (rather then per driver)
> case in the future.
>
You are right that it will be easier to change it if we ever need to,
although I don't expect it to happen in the near future (famous last
words). It would be nice to not have to add it to every quirk entry,
current and new though. But I do not have another suggestion atm, so
I'm fine with your original approach.
Reviewed-by: João Paulo Rechi Vita <jprvita@endlessm.com>
......................................................................................
João Paulo Rechi Vita | Endless
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-06-12 6:59 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-21 6:28 [PATCH] platform/x86: asus-wmi: Only Tell EC the OS will handle display hotkeys from asus_nb_wmi Hans de Goede
2019-05-24 16:43 ` Andy Shevchenko
2019-05-28 13:37 ` Hans de Goede
2019-06-05 18:15 ` Hans de Goede
2019-06-11 19:15 ` Andy Shevchenko
2019-06-12 6:59 ` Hans de Goede
2019-05-28 21:22 ` João Paulo Rechi Vita
2019-05-29 8:55 ` Hans de Goede
2019-05-29 17:37 ` João Paulo Rechi Vita
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.