[v2] auxdisplay: ht16k33: Use backlight helper

[PATCH v2] auxdisplay: ht16k33: Use backlight helper

Posted by Stephen Kitt 3 years, 1 month ago

backlight_properties.fb_blank is deprecated. The states it represents
are handled by other properties; but instead of accessing those
properties directly, drivers should use the helpers provided by
backlight.h.

Instead of retrieving the backlight brightness in struct
backlight_properties manually, and then checking whether the backlight
should be on at all, use backlight_get_brightness() which does all
this and insulates this from future changes.

This means that BL_CORE_SUSPENDED is now taken into account, as it
should be.

Signed-off-by: Stephen Kitt <steve@sk2.org>
---

Notes:
    Changes since v1: mention BL_CORE_SUSPENDED.

 drivers/auxdisplay/ht16k33.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/auxdisplay/ht16k33.c b/drivers/auxdisplay/ht16k33.c
index 02425991c159..15ab118c80f5 100644
--- a/drivers/auxdisplay/ht16k33.c
+++ b/drivers/auxdisplay/ht16k33.c
@@ -314,14 +314,9 @@ static int ht16k33_initialize(struct ht16k33_priv *priv)
 
 static int ht16k33_bl_update_status(struct backlight_device *bl)
 {
-	int brightness = bl->props.brightness;
+	int brightness = backlight_get_brightness(bl);
 	struct ht16k33_priv *priv = bl_get_data(bl);
 
-	if (bl->props.power != FB_BLANK_UNBLANK ||
-	    bl->props.fb_blank != FB_BLANK_UNBLANK ||
-	    bl->props.state & BL_CORE_FBBLANK)
-		brightness = 0;
-
 	return ht16k33_brightness_set(priv, brightness);
 }
 
-- 
2.30.2

Re: [PATCH v2] auxdisplay: ht16k33: Use backlight helper

Posted by Robin van der Gracht 3 years ago

On 2023-01-06 15:30, Stephen Kitt wrote:
> backlight_properties.fb_blank is deprecated. The states it represents
> are handled by other properties; but instead of accessing those
> properties directly, drivers should use the helpers provided by
> backlight.h.
> 
> Instead of retrieving the backlight brightness in struct
> backlight_properties manually, and then checking whether the backlight
> should be on at all, use backlight_get_brightness() which does all
> this and insulates this from future changes.
> 
> This means that BL_CORE_SUSPENDED is now taken into account, as it
> should be.
> 
> Signed-off-by: Stephen Kitt <steve@sk2.org>
> ---
> 
> Notes:
>     Changes since v1: mention BL_CORE_SUSPENDED.
> 
>  drivers/auxdisplay/ht16k33.c | 7 +------
>  1 file changed, 1 insertion(+), 6 deletions(-)
> 
> diff --git a/drivers/auxdisplay/ht16k33.c 
> b/drivers/auxdisplay/ht16k33.c
> index 02425991c159..15ab118c80f5 100644
> --- a/drivers/auxdisplay/ht16k33.c
> +++ b/drivers/auxdisplay/ht16k33.c
> @@ -314,14 +314,9 @@ static int ht16k33_initialize(struct ht16k33_priv 
> *priv)
> 
>  static int ht16k33_bl_update_status(struct backlight_device *bl)
>  {
> -	int brightness = bl->props.brightness;
> +	int brightness = backlight_get_brightness(bl);
>  	struct ht16k33_priv *priv = bl_get_data(bl);
> 
> -	if (bl->props.power != FB_BLANK_UNBLANK ||
> -	    bl->props.fb_blank != FB_BLANK_UNBLANK ||
> -	    bl->props.state & BL_CORE_FBBLANK)
> -		brightness = 0;
> -
>  	return ht16k33_brightness_set(priv, brightness);
>  }

Thank you Stephen (and Sam), looks good!

Reviewed-by: Robin van der Gracht <robin@protonic.nl>

Re: [PATCH v2] auxdisplay: ht16k33: Use backlight helper

Posted by Geert Uytterhoeven 3 years, 1 month ago

On Fri, Jan 6, 2023 at 4:10 PM Stephen Kitt <steve@sk2.org> wrote:
> backlight_properties.fb_blank is deprecated. The states it represents
> are handled by other properties; but instead of accessing those
> properties directly, drivers should use the helpers provided by
> backlight.h.
>
> Instead of retrieving the backlight brightness in struct
> backlight_properties manually, and then checking whether the backlight
> should be on at all, use backlight_get_brightness() which does all
> this and insulates this from future changes.
>
> This means that BL_CORE_SUSPENDED is now taken into account, as it
> should be.
>
> Signed-off-by: Stephen Kitt <steve@sk2.org>

Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v2] auxdisplay: ht16k33: Use backlight helper

Posted by Miguel Ojeda 3 years ago

On Sun, Jan 8, 2023 at 11:03 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org>

This will likely go through drm-misc as part of a bigger series:

    https://lore.kernel.org/lkml/20230107-sam-video-backlight-drop-fb_blank-v1-0-1bd9bafb351f@ravnborg.org/

So you may want to copy-paste your tag there.

Cheers,
Miguel

Re: [PATCH v2] auxdisplay: ht16k33: Use backlight helper

Posted by Geert Uytterhoeven 3 years ago

Hi Miguel,

On Sun, Jan 8, 2023 at 3:57 PM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
> On Sun, Jan 8, 2023 at 11:03 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org>
>
> This will likely go through drm-misc as part of a bigger series:
>
>     https://lore.kernel.org/lkml/20230107-sam-video-backlight-drop-fb_blank-v1-0-1bd9bafb351f@ravnborg.org/
>
> So you may want to copy-paste your tag there.

Oh, I sent it here because of
https://lore.kernel.org/all/Y7qM+ZlG5gQiOW4K@ravnborg.org ?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v2] auxdisplay: ht16k33: Use backlight helper

Posted by Miguel Ojeda 3 years ago

On Sun, Jan 8, 2023 at 7:32 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Oh, I sent it here because of
> https://lore.kernel.org/all/Y7qM+ZlG5gQiOW4K@ravnborg.org ?

Yeah, sorry. I mentioned it just in case, since Sam asked for Robin's
Ack in that thread, but I should have probably just Cc'd him here
(done!)...

Cheers,
Miguel

Re: [PATCH v2] auxdisplay: ht16k33: Use backlight helper

Posted by Sam Ravnborg 3 years ago

Hi all.

On Sun, Jan 08, 2023 at 08:11:52PM +0100, Miguel Ojeda wrote:
> On Sun, Jan 8, 2023 at 7:32 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> >
> > Oh, I sent it here because of
> > https://lore.kernel.org/all/Y7qM+ZlG5gQiOW4K@ravnborg.org ?
> 
> Yeah, sorry. I mentioned it just in case, since Sam asked for Robin's
> Ack in that thread, but I should have probably just Cc'd him here
> (done!)...

Stephen notified me that the patch is already applied - so no need for
anything now. Sorry for the noise!

	Sam

Re: [PATCH v2] auxdisplay: ht16k33: Use backlight helper

Posted by Stephen Kitt 3 years ago

On Sun, 8 Jan 2023 21:17:43 +0100, Sam Ravnborg <sam@ravnborg.org> wrote:
> On Sun, Jan 08, 2023 at 08:11:52PM +0100, Miguel Ojeda wrote:
> > On Sun, Jan 8, 2023 at 7:32 PM Geert Uytterhoeven <geert@linux-m68k.org>
> > wrote:  
> > >
> > > Oh, I sent it here because of
> > > https://lore.kernel.org/all/Y7qM+ZlG5gQiOW4K@ravnborg.org ?  
> > 
> > Yeah, sorry. I mentioned it just in case, since Sam asked for Robin's
> > Ack in that thread, but I should have probably just Cc'd him here
> > (done!)...  
> 
> Stephen notified me that the patch is already applied - so no need for
> anything now. Sorry for the noise!

That was the via-pmu-backlight patch; AFAIK the ht16k33 patch hasn’t been
applied anywhere yet.

Regards,

Stephen

Re: [PATCH v2] auxdisplay: ht16k33: Use backlight helper

Posted by Miguel Ojeda 3 years, 1 month ago

On Fri, Jan 6, 2023 at 3:30 PM Stephen Kitt <steve@sk2.org> wrote:
>
>     Changes since v1: mention BL_CORE_SUSPENDED.

Thanks Stephen.

Robin, it would be great to get your ack/review (or if you can give it
a quick test, that would be great too).

Cheers,
Miguel