linux-kernel - Re: [PATCH 2/6] backlight/generic-bl: remove DRIVER1 state

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: <20180117171340.GZ2759@phenom.ffwll.local>
Date:   Wed, 17 Jan 2018 18:13:40 +0100
From:   Daniel Vetter <daniel@...ll.ch>
To:     Daniel Thompson <daniel.thompson@...aro.org>
Cc:     Daniel Vetter <daniel.vetter@...ll.ch>,
        DRI Development <dri-devel@...ts.freedesktop.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Lee Jones <lee.jones@...aro.org>,
        Jingoo Han <jingoohan1@...il.com>,
        Daniel Vetter <daniel.vetter@...el.com>
Subject: Re: [PATCH 2/6] backlight/generic-bl: remove DRIVER1 state

On Wed, Jan 17, 2018 at 04:44:00PM +0000, Daniel Thompson wrote:
> On 17/01/18 14:01, Daniel Vetter wrote:
> > Nothing in the entire tree ever sets this, which means this is dead
> > code. Remove it.
> > 
> > Cc: Lee Jones <lee.jones@...aro.org>
> > Cc: Daniel Thompson <daniel.thompson@...aro.org>
> > Cc: Jingoo Han <jingoohan1@...il.com>
> > Signed-off-by: Daniel Vetter <daniel.vetter@...el.com>
> 
> Not sure whether to ack this one or not.
> 
> There is nothing wrong with the change but having taken a closer look the
> driver seems like it exists mostly to allow mach-XXX code to plug in
> function pointers and we don't do that sort of thing any more.
> 
> I think the entire driver is dead code!

Well I can also supply a patch to outright nuke the code, but figuring out
whether that's the right thing to do is definitely way above may pay grade
:-)

I only really stitched these together after a long discussion with Meghana
about why backlight seems to have 3+ different ways to enable/disable a
backlight. Just trying to help a bit with getting the
backlight_enable/disable stuff going, so that long-term, at least for
newer drivers, we have one blessed way to do that.

btw that kind of display pm simplification matches what we've done when
implementing atomic modesetting about 3 years ago: We've smashed all the
various power states drm (and fbdev/fbcon) knew about into a simple "is it
on?" boolean. Todays digital hw doesn't really know anything in-between.
Ofc there's tons of components to switch on/off to get the entire display
pipe up, and they might want different autosuspend delays to optimize the
overall system, but that's orthogonal (well, driver internal
implementation detail) really.

Cheers, Daniel

> 
> 
> Daniel.
> 
> 
> > --- drivers/video/backlight/generic_bl.c | 5 ----- 1 file changed, 5
> > deletions(-)
> > 
> > diff --git a/drivers/video/backlight/generic_bl.c
> > b/drivers/video/backlight/generic_bl.c index
> > 67dfb939a514..4dea91acea13 100644 ---
> > a/drivers/video/backlight/generic_bl.c +++
> > b/drivers/video/backlight/generic_bl.c @@ -21,9 +21,6 @@ static int
> > genericbl_intensity; static struct backlight_device
> > *generic_backlight_device; static struct generic_bl_info *bl_machinfo;
> > -/* Flag to signal when the battery is low */ -#define
> > GENERICBL_BATTLOW       BL_CORE_DRIVER1 - static int
> > genericbl_send_intensity(struct backlight_device *bd) { int intensity
> > = bd->props.brightness; @@ -34,8 +31,6 @@ static int
> > genericbl_send_intensity(struct backlight_device *bd) intensity = 0;
> > if (bd->props.state & BL_CORE_SUSPENDED) intensity = 0; -	if
> > (bd->props.state & GENERICBL_BATTLOW) -		intensity &=
> > bl_machinfo->limit_mask; bl_machinfo->set_bl_intensity(intensity);
> > 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch