linux-kernel - Re: [PATCH 09/15] staging: fbtft: fb_ssd1351.c: Introduce backlight_is

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Y7sntztwrNqw41+i@ravnborg.org>
Date:   Sun, 8 Jan 2023 21:29:43 +0100
From:   Sam Ravnborg <sam@...nborg.org>
To:     Stephen Kitt <steve@....org>
Cc:     Sam Ravnborg via B4 Submission Endpoint 
        <devnull+sam.ravnborg.org@...nel.org>,
        Alexandre Belloni <alexandre.belloni@...tlin.com>,
        linux-staging@...ts.linux.dev,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        linux-fbdev@...r.kernel.org, dri-devel@...ts.freedesktop.org,
        linux-kernel@...r.kernel.org, Paul Mackerras <paulus@...ba.org>,
        Daniel Thompson <daniel.thompson@...aro.org>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Helge Deller <deller@....de>, Lee Jones <lee@...nel.org>,
        Miguel Ojeda <ojeda@...nel.org>,
        Antonino Daplas <adaplas@...il.com>,
        linux-omap@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        Robin van der Gracht <robin@...tonic.nl>,
        Nicolas Ferre <nicolas.ferre@...rochip.com>,
        Jingoo Han <jingoohan1@...il.com>,
        linuxppc-dev@...ts.ozlabs.org,
        Claudiu Beznea <claudiu.beznea@...rochip.com>
Subject: Re: [PATCH 09/15] staging: fbtft: fb_ssd1351.c: Introduce
 backlight_is_blank()

Hi Stephen,

On Sun, Jan 08, 2023 at 08:28:17PM +0100, Stephen Kitt wrote:
> On Sat, 07 Jan 2023 19:26:23 +0100, Sam Ravnborg via B4 Submission Endpoint
> <devnull+sam.ravnborg.org@...nel.org> wrote:
> 
> > From: Sam Ravnborg <sam@...nborg.org>
> > 
> > Avoiding direct access to backlight_properties.props.
> > 
> > Access to the deprecated props.fb_blank replaced by backlight_is_blank().
> > Access to props.power is dropped - it was only used for debug.
> > 
> > Signed-off-by: Sam Ravnborg <sam@...nborg.org>
> > Cc: Stephen Kitt <steve@....org>
> > Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> > Cc: Daniel Thompson <daniel.thompson@...aro.org>
> > Cc: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
> > Cc: linux-fbdev@...r.kernel.org
> > ---
> >  drivers/staging/fbtft/fb_ssd1351.c | 9 +++------
> >  1 file changed, 3 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/staging/fbtft/fb_ssd1351.c
> > b/drivers/staging/fbtft/fb_ssd1351.c index b8d55aa8c5c7..995fbd2f3dc6 100644
> > --- a/drivers/staging/fbtft/fb_ssd1351.c
> > +++ b/drivers/staging/fbtft/fb_ssd1351.c
> > @@ -190,15 +190,12 @@ static struct fbtft_display display = {
> >  static int update_onboard_backlight(struct backlight_device *bd)
> >  {
> >  	struct fbtft_par *par = bl_get_data(bd);
> > -	bool on;
> > +	bool blank = backlight_is_blank(bd);
> >  
> > -	fbtft_par_dbg(DEBUG_BACKLIGHT, par,
> > -		      "%s: power=%d, fb_blank=%d\n",
> > -		      __func__, bd->props.power, bd->props.fb_blank);
> > +	fbtft_par_dbg(DEBUG_BACKLIGHT, par, "%s: blank=%d\n", __func__,
> > blank); 
> > -	on = !backlight_is_blank(bd);
> >  	/* Onboard backlight connected to GPIO0 on SSD1351, GPIO1 unused */
> > -	write_reg(par, 0xB5, on ? 0x03 : 0x02);
> > +	write_reg(par, 0xB5, !blank ? 0x03 : 0x02);
> >  
> >  	return 0;
> >  }
> > 
> > -- 
> > 2.34.1
> 
> For debugging purposes here, would there be any point in logging props.state?
> As in
> 
>         fbtft_par_dbg(DEBUG_BACKLIGHT, par,
> -                     "%s: power=%d, fb_blank=%d\n",
> -                     __func__, bd->props.power, bd->props.fb_blank);
> +                     "%s: power=%d, state=%u\n",
> +                     __func__, bd->props.power, bd->props.state);

Thanks for the suggestion - and the reviews!

I was tempted to just remove the debugging.
If we require debugging, then this could be added in the backlight core,
thus everyone would benefit from it.

The solution above avoid any direct use of backlight_properties
which I consider a layer violation outside the backlight core.
(We cannot avoid it today with the current interface - but we can
minimize it).

	Sam