linux-kernel - Re: [RESEND PATCH v5 2/2] drm/bridge: hx8837: add a Himax HX8837 display controller driver

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] [day] [month] [year] [list]

Message-ID: <20201025154346.GA37040@ravnborg.org>
Date:   Sun, 25 Oct 2020 16:43:46 +0100
From:   Sam Ravnborg <sam@...nborg.org>
To:     Lubomir Rintel <lkundrak@...sk>
Cc:     Andrzej Hajda <a.hajda@...sung.com>, devicetree@...r.kernel.org,
        Jernej Skrabec <jernej.skrabec@...l.net>,
        Jonas Karlman <jonas@...boo.se>,
        David Airlie <airlied@...ux.ie>,
        Neil Armstrong <narmstrong@...libre.com>,
        linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org,
        Rob Herring <robh+dt@...nel.org>,
        Laurent Pinchart <Laurent.pinchart@...asonboard.com>
Subject: Re: [RESEND PATCH v5 2/2] drm/bridge: hx8837: add a Himax HX8837
 display controller driver

Hi Lubomir.

> > > +static int hx8837_bl_update_status(struct backlight_device *bl)
> > > +{
> > > +	struct hx8837_priv *priv = bl_get_data(bl);
> > > +	unsigned int val;
> > > +	int ret;
> > > +
> > > +	ret = regmap_update_bits(priv->regmap, DCON_REG_BRIGHT,
> > > +					       0x000f,
> > > +					       bl->props.brightness);
> > 
> > Use backlight_get_brightness() to get the brightness.
> > This will also make sure 0 is returned when backlight is off so the
> > logic a few lines down is correct.
> 
> I'm not sure I understand this one. I'm wondering if you could help me out
> with it before I follow up with v4.
> 
> Currently I read in the current brightness level in probe() (which
> prevents struct backlight_properties, below, from being const) and the
> nthe brightness is entirely in control of the driver via
> update_status().
> 
> What would I need get_brightness() for? We know that whatever the driver
> set is the current level. It doesn't seem to be called on backlight
> device registration so it doesn't make the readin in probe()
> unnecessary either.

The request here is to replace the direct access to backlight properties
"bl->props.brightness" with the helper backlight_get_brightness(bl).

	Sam