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

From: Sam Ravnborg
Date: Sun Oct 25 2020 - 11:44:01 EST


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