All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] platform/x86: sony-laptop: Don't turn off 0x153 keyboard backlight during probe
@ 2022-12-13 12:29 Hans de Goede
  2022-12-13 13:03 ` Andy Shevchenko
       [not found] ` <CANER=bYHYNSi3fTwqAt89n-6uS5dSV+o+6H4oD6doeSzgtoZoQ@mail.gmail.com>
  0 siblings, 2 replies; 6+ messages in thread
From: Hans de Goede @ 2022-12-13 12:29 UTC (permalink / raw)
  To: Mattia Dongili, Mark Gross, Andy Shevchenko
  Cc: Hans de Goede, platform-driver-x86

The 0x153 version of the kbd backlight control SNC handle has no separate
address to probe if the backlight is there.

This turns the probe call into a set keyboard backlight call with a value
of 0 turning off the keyboard backlight.

Skip probing when there is no separate probe address to avoid this.

Link: https://bugzilla.redhat.com/show_bug.cgi?id=1583752
Fixes: 800f20170dcf ("Keyboard backlight control for some Vaio Fit models")
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/platform/x86/sony-laptop.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/drivers/platform/x86/sony-laptop.c b/drivers/platform/x86/sony-laptop.c
index 765fcaba4d12..5ff5aaf92b56 100644
--- a/drivers/platform/x86/sony-laptop.c
+++ b/drivers/platform/x86/sony-laptop.c
@@ -1888,14 +1888,21 @@ static int sony_nc_kbd_backlight_setup(struct platform_device *pd,
 		break;
 	}
 
-	ret = sony_call_snc_handle(handle, probe_base, &result);
-	if (ret)
-		return ret;
+	/*
+	 * Only probe if there is a separate probe_base, otherwise the probe call
+	 * is equivalent to __sony_nc_kbd_backlight_mode_set(0), resulting in
+	 * the keyboard backlight being turned off.
+	 */
+	if (probe_base) {
+		ret = sony_call_snc_handle(handle, probe_base, &result);
+		if (ret)
+			return ret;
 
-	if ((handle == 0x0137 && !(result & 0x02)) ||
-			!(result & 0x01)) {
-		dprintk("no backlight keyboard found\n");
-		return 0;
+		if ((handle == 0x0137 && !(result & 0x02)) ||
+				!(result & 0x01)) {
+			dprintk("no backlight keyboard found\n");
+			return 0;
+		}
 	}
 
 	kbdbl_ctl = kzalloc(sizeof(*kbdbl_ctl), GFP_KERNEL);
-- 
2.38.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] platform/x86: sony-laptop: Don't turn off 0x153 keyboard backlight during probe
  2022-12-13 12:29 [PATCH] platform/x86: sony-laptop: Don't turn off 0x153 keyboard backlight during probe Hans de Goede
@ 2022-12-13 13:03 ` Andy Shevchenko
  2022-12-13 13:35   ` Hans de Goede
       [not found] ` <CANER=bYHYNSi3fTwqAt89n-6uS5dSV+o+6H4oD6doeSzgtoZoQ@mail.gmail.com>
  1 sibling, 1 reply; 6+ messages in thread
From: Andy Shevchenko @ 2022-12-13 13:03 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Mattia Dongili, Mark Gross, Andy Shevchenko, platform-driver-x86

On Tue, Dec 13, 2022 at 2:29 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> The 0x153 version of the kbd backlight control SNC handle has no separate
> address to probe if the backlight is there.
>
> This turns the probe call into a set keyboard backlight call with a value
> of 0 turning off the keyboard backlight.
>
> Skip probing when there is no separate probe address to avoid this.

...

> +       /*
> +        * Only probe if there is a separate probe_base, otherwise the probe call
> +        * is equivalent to __sony_nc_kbd_backlight_mode_set(0), resulting in
> +        * the keyboard backlight being turned off.
> +        */
> +       if (probe_base) {

I'm wondering if it wouldn't be better to split this into the helper
and hence just call it here.

>         }

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] platform/x86: sony-laptop: Don't turn off 0x153 keyboard backlight during probe
  2022-12-13 13:03 ` Andy Shevchenko
@ 2022-12-13 13:35   ` Hans de Goede
  0 siblings, 0 replies; 6+ messages in thread
From: Hans de Goede @ 2022-12-13 13:35 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Mattia Dongili, Mark Gross, Andy Shevchenko, platform-driver-x86

Hi,

On 12/13/22 14:03, Andy Shevchenko wrote:
> On Tue, Dec 13, 2022 at 2:29 PM Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> The 0x153 version of the kbd backlight control SNC handle has no separate
>> address to probe if the backlight is there.
>>
>> This turns the probe call into a set keyboard backlight call with a value
>> of 0 turning off the keyboard backlight.
>>
>> Skip probing when there is no separate probe address to avoid this.
> 
> ...
> 
>> +       /*
>> +        * Only probe if there is a separate probe_base, otherwise the probe call
>> +        * is equivalent to __sony_nc_kbd_backlight_mode_set(0), resulting in
>> +        * the keyboard backlight being turned off.
>> +        */
>> +       if (probe_base) {
> 
> I'm wondering if it wouldn't be better to split this into the helper
> and hence just call it here.

The helper here is sony_call_snc_handle() which is not only used for
probing it is a generic helper.

Calling sony_call_snc_handle() with an argument of 0 is valid, specifically
this turns of the kbd backlight on the affected laptop models.

Which is a valid thing to do if the user requests it, but it is not a valid
thing to do as a "probe" to see if there is a kbd-backlight.

Regards,

Hans



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] platform/x86: sony-laptop: Don't turn off 0x153 keyboard backlight during probe
       [not found] ` <CANER=bYHYNSi3fTwqAt89n-6uS5dSV+o+6H4oD6doeSzgtoZoQ@mail.gmail.com>
@ 2022-12-14  9:12   ` Hans de Goede
  2022-12-14  9:49     ` Mattia Dongili
  0 siblings, 1 reply; 6+ messages in thread
From: Hans de Goede @ 2022-12-14  9:12 UTC (permalink / raw)
  To: Mattia Dongili; +Cc: Mark Gross, Andy Shevchenko, platform-driver-x86

Hi Mattia,

On 12/14/22 09:55, Mattia Dongili wrote:
> On Tue, 13 Dec 2022 at 21:29, Hans de Goede <hdegoede@redhat.com <mailto:hdegoede@redhat.com>> wrote:
> 
>     The 0x153 version of the kbd backlight control SNC handle has no separate
>     address to probe if the backlight is there.
> 
>     This turns the probe call into a set keyboard backlight call with a value
>     of 0 turning off the keyboard backlight.
> 
>     Skip probing when there is no separate probe address to avoid this.
> 
>     Link: https://bugzilla.redhat.com/show_bug.cgi?id=1583752 <https://bugzilla.redhat.com/show_bug.cgi?id=1583752>
>     Fixes: 800f20170dcf ("Keyboard backlight control for some Vaio Fit models")
>     Signed-off-by: Hans de Goede <hdegoede@redhat.com <mailto:hdegoede@redhat.com>>
> 
> 
> Signed-off-by: Mattia Dongili <malattia@linux.it <mailto:malattia@linux.it>>
>  
> 
>     ---
>      drivers/platform/x86/sony-laptop.c | 21 ++++++++++++++-------
>      1 file changed, 14 insertions(+), 7 deletions(-)
> 
>     diff --git a/drivers/platform/x86/sony-laptop.c b/drivers/platform/x86/sony-laptop.c
>     index 765fcaba4d12..5ff5aaf92b56 100644
>     --- a/drivers/platform/x86/sony-laptop.c
>     +++ b/drivers/platform/x86/sony-laptop.c
>     @@ -1888,14 +1888,21 @@ static int sony_nc_kbd_backlight_setup(struct platform_device *pd,
>                     break;
>             }
> 
>     -       ret = sony_call_snc_handle(handle, probe_base, &result);
>     -       if (ret)
>     -               return ret;
>     +       /*
>     +        * Only probe if there is a separate probe_base, otherwise the probe call
>     +        * is equivalent to __sony_nc_kbd_backlight_mode_set(0), resulting in
>     +        * the keyboard backlight being turned off.
>     +        */
>     +       if (probe_base) {
>     +               ret = sony_call_snc_handle(handle, probe_base, &result);
>     +               if (ret)
>     +                       return ret;
> 
>     -       if ((handle == 0x0137 && !(result & 0x02)) ||
>     -                       !(result & 0x01)) {
>     -               dprintk("no backlight keyboard found\n");
>     -               return 0;
>     +               if ((handle == 0x0137 && !(result & 0x02)) ||
>     +                               !(result & 0x01)) {
>     +                       dprintk("no backlight keyboard found\n");
>     +                       return 0;
>     +               }
>             }
> 
>             kbdbl_ctl = kzalloc(sizeof(*kbdbl_ctl), GFP_KERNEL);
>     -- 
>     2.38.1
> 
> ---
> 
> Aha, looking at the bug report and the commit that caused it I think this fix makes sense.
> You can add my sign-off too.

I think you mean Reviewed-by? Singed-off-by: is only for patches passing
through you. E.g. it was send to you personally and you then submit it
to the list.

Regards,

Hans



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] platform/x86: sony-laptop: Don't turn off 0x153 keyboard backlight during probe
  2022-12-14  9:12   ` Hans de Goede
@ 2022-12-14  9:49     ` Mattia Dongili
  2023-01-12 16:29       ` Hans de Goede
  0 siblings, 1 reply; 6+ messages in thread
From: Mattia Dongili @ 2022-12-14  9:49 UTC (permalink / raw)
  To: Hans de Goede; +Cc: Mark Gross, Andy Shevchenko, platform-driver-x86

On Wed, 14 Dec 2022 at 18:13, Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi Mattia,
>
> On 12/14/22 09:55, Mattia Dongili wrote:
> > On Tue, 13 Dec 2022 at 21:29, Hans de Goede <hdegoede@redhat.com <mailto:hdegoede@redhat.com>> wrote:
> >
> >     The 0x153 version of the kbd backlight control SNC handle has no separate
> >     address to probe if the backlight is there.
> >
> >     This turns the probe call into a set keyboard backlight call with a value
> >     of 0 turning off the keyboard backlight.
> >
> >     Skip probing when there is no separate probe address to avoid this.
> >
> >     Link: https://bugzilla.redhat.com/show_bug.cgi?id=1583752 <https://bugzilla.redhat.com/show_bug.cgi?id=1583752>
> >     Fixes: 800f20170dcf ("Keyboard backlight control for some Vaio Fit models")
> >     Signed-off-by: Hans de Goede <hdegoede@redhat.com <mailto:hdegoede@redhat.com>>
> >
> >
> > Signed-off-by: Mattia Dongili <malattia@linux.it <mailto:malattia@linux.it>>
> >
> >
> >     ---
> >      drivers/platform/x86/sony-laptop.c | 21 ++++++++++++++-------
> >      1 file changed, 14 insertions(+), 7 deletions(-)
> >
> >     diff --git a/drivers/platform/x86/sony-laptop.c b/drivers/platform/x86/sony-laptop.c
> >     index 765fcaba4d12..5ff5aaf92b56 100644
> >     --- a/drivers/platform/x86/sony-laptop.c
> >     +++ b/drivers/platform/x86/sony-laptop.c
> >     @@ -1888,14 +1888,21 @@ static int sony_nc_kbd_backlight_setup(struct platform_device *pd,
> >                     break;
> >             }
> >
> >     -       ret = sony_call_snc_handle(handle, probe_base, &result);
> >     -       if (ret)
> >     -               return ret;
> >     +       /*
> >     +        * Only probe if there is a separate probe_base, otherwise the probe call
> >     +        * is equivalent to __sony_nc_kbd_backlight_mode_set(0), resulting in
> >     +        * the keyboard backlight being turned off.
> >     +        */
> >     +       if (probe_base) {
> >     +               ret = sony_call_snc_handle(handle, probe_base, &result);
> >     +               if (ret)
> >     +                       return ret;
> >
> >     -       if ((handle == 0x0137 && !(result & 0x02)) ||
> >     -                       !(result & 0x01)) {
> >     -               dprintk("no backlight keyboard found\n");
> >     -               return 0;
> >     +               if ((handle == 0x0137 && !(result & 0x02)) ||
> >     +                               !(result & 0x01)) {
> >     +                       dprintk("no backlight keyboard found\n");
> >     +                       return 0;
> >     +               }
> >             }
> >
> >             kbdbl_ctl = kzalloc(sizeof(*kbdbl_ctl), GFP_KERNEL);
> >     --
> >     2.38.1
> >
> > ---
> >
> > Aha, looking at the bug report and the commit that caused it I think this fix makes sense.
> > You can add my sign-off too.
>
> I think you mean Reviewed-by? Singed-off-by: is only for patches passing
> through you. E.g. it was send to you personally and you then submit it
> to the list.

Sigh yeah... It's been too long.

Reviewed-by: Mattia Dongili <malattia@linux.it>

Thanks for pointing this out and most importantly for the fix!
-- 
mattia
:wq!

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] platform/x86: sony-laptop: Don't turn off 0x153 keyboard backlight during probe
  2022-12-14  9:49     ` Mattia Dongili
@ 2023-01-12 16:29       ` Hans de Goede
  0 siblings, 0 replies; 6+ messages in thread
From: Hans de Goede @ 2023-01-12 16:29 UTC (permalink / raw)
  To: Mattia Dongili; +Cc: Mark Gross, Andy Shevchenko, platform-driver-x86

Hi,

On 12/14/22 10:49, Mattia Dongili wrote:
> On Wed, 14 Dec 2022 at 18:13, Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> Hi Mattia,
>>
>> On 12/14/22 09:55, Mattia Dongili wrote:
>>> On Tue, 13 Dec 2022 at 21:29, Hans de Goede <hdegoede@redhat.com <mailto:hdegoede@redhat.com>> wrote:
>>>
>>>     The 0x153 version of the kbd backlight control SNC handle has no separate
>>>     address to probe if the backlight is there.
>>>
>>>     This turns the probe call into a set keyboard backlight call with a value
>>>     of 0 turning off the keyboard backlight.
>>>
>>>     Skip probing when there is no separate probe address to avoid this.
>>>
>>>     Link: https://bugzilla.redhat.com/show_bug.cgi?id=1583752 <https://bugzilla.redhat.com/show_bug.cgi?id=1583752>
>>>     Fixes: 800f20170dcf ("Keyboard backlight control for some Vaio Fit models")
>>>     Signed-off-by: Hans de Goede <hdegoede@redhat.com <mailto:hdegoede@redhat.com>>
>>>
>>>
>>> Signed-off-by: Mattia Dongili <malattia@linux.it <mailto:malattia@linux.it>>
>>>
>>>
>>>     ---
>>>      drivers/platform/x86/sony-laptop.c | 21 ++++++++++++++-------
>>>      1 file changed, 14 insertions(+), 7 deletions(-)
>>>
>>>     diff --git a/drivers/platform/x86/sony-laptop.c b/drivers/platform/x86/sony-laptop.c
>>>     index 765fcaba4d12..5ff5aaf92b56 100644
>>>     --- a/drivers/platform/x86/sony-laptop.c
>>>     +++ b/drivers/platform/x86/sony-laptop.c
>>>     @@ -1888,14 +1888,21 @@ static int sony_nc_kbd_backlight_setup(struct platform_device *pd,
>>>                     break;
>>>             }
>>>
>>>     -       ret = sony_call_snc_handle(handle, probe_base, &result);
>>>     -       if (ret)
>>>     -               return ret;
>>>     +       /*
>>>     +        * Only probe if there is a separate probe_base, otherwise the probe call
>>>     +        * is equivalent to __sony_nc_kbd_backlight_mode_set(0), resulting in
>>>     +        * the keyboard backlight being turned off.
>>>     +        */
>>>     +       if (probe_base) {
>>>     +               ret = sony_call_snc_handle(handle, probe_base, &result);
>>>     +               if (ret)
>>>     +                       return ret;
>>>
>>>     -       if ((handle == 0x0137 && !(result & 0x02)) ||
>>>     -                       !(result & 0x01)) {
>>>     -               dprintk("no backlight keyboard found\n");
>>>     -               return 0;
>>>     +               if ((handle == 0x0137 && !(result & 0x02)) ||
>>>     +                               !(result & 0x01)) {
>>>     +                       dprintk("no backlight keyboard found\n");
>>>     +                       return 0;
>>>     +               }
>>>             }
>>>
>>>             kbdbl_ctl = kzalloc(sizeof(*kbdbl_ctl), GFP_KERNEL);
>>>     --
>>>     2.38.1
>>>
>>> ---
>>>
>>> Aha, looking at the bug report and the commit that caused it I think this fix makes sense.
>>> You can add my sign-off too.
>>
>> I think you mean Reviewed-by? Singed-off-by: is only for patches passing
>> through you. E.g. it was send to you personally and you then submit it
>> to the list.
> 
> Sigh yeah... It's been too long.
> 
> Reviewed-by: Mattia Dongili <malattia@linux.it>

Thanks.

I've added this to my review-hans (soon to be for-next) branch now.

Regards,

Hans



^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2023-01-12 16:32 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-13 12:29 [PATCH] platform/x86: sony-laptop: Don't turn off 0x153 keyboard backlight during probe Hans de Goede
2022-12-13 13:03 ` Andy Shevchenko
2022-12-13 13:35   ` Hans de Goede
     [not found] ` <CANER=bYHYNSi3fTwqAt89n-6uS5dSV+o+6H4oD6doeSzgtoZoQ@mail.gmail.com>
2022-12-14  9:12   ` Hans de Goede
2022-12-14  9:49     ` Mattia Dongili
2023-01-12 16:29       ` Hans de Goede

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.