From: Sam Ravnborg <sam@ravnborg.org>
To: Lubomir Rintel <lkundrak@v3.sk>
Cc: Andrzej Hajda <a.hajda@samsung.com>,
devicetree@vger.kernel.org,
Jernej Skrabec <jernej.skrabec@siol.net>,
Jonas Karlman <jonas@kwiboo.se>, David Airlie <airlied@linux.ie>,
Neil Armstrong <narmstrong@baylibre.com>,
linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
Rob Herring <robh+dt@kernel.org>,
Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
Subject: Re: [RESEND PATCH v5 2/2] drm/bridge: hx8837: add a Himax HX8837 display controller driver
Date: Sun, 25 Oct 2020 16:43:46 +0100 [thread overview]
Message-ID: <20201025154346.GA37040@ravnborg.org> (raw)
In-Reply-To: <20201025151942.GA526002@demiurge.local>
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
WARNING: multiple messages have this Message-ID (diff)
From: Sam Ravnborg <sam@ravnborg.org>
To: Lubomir Rintel <lkundrak@v3.sk>
Cc: devicetree@vger.kernel.org,
Jernej Skrabec <jernej.skrabec@siol.net>,
Jonas Karlman <jonas@kwiboo.se>, David Airlie <airlied@linux.ie>,
Neil Armstrong <narmstrong@baylibre.com>,
linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
Andrzej Hajda <a.hajda@samsung.com>,
Rob Herring <robh+dt@kernel.org>,
Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
Subject: Re: [RESEND PATCH v5 2/2] drm/bridge: hx8837: add a Himax HX8837 display controller driver
Date: Sun, 25 Oct 2020 16:43:46 +0100 [thread overview]
Message-ID: <20201025154346.GA37040@ravnborg.org> (raw)
In-Reply-To: <20201025151942.GA526002@demiurge.local>
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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2020-10-25 15:43 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-26 0:07 [RESEND PATCH v5 0/2] Add a Himax HX8837 display controller driver Lubomir Rintel
2020-09-26 0:07 ` Lubomir Rintel
2020-09-26 0:07 ` [RESEND PATCH v5 1/2] dt-bindings: display: himax,hx8837: Add Himax HX8837 bindings Lubomir Rintel
2020-09-26 0:07 ` [RESEND PATCH v5 1/2] dt-bindings: display: himax, hx8837: " Lubomir Rintel
2020-09-26 0:07 ` [RESEND PATCH v5 2/2] drm/bridge: hx8837: add a Himax HX8837 display controller driver Lubomir Rintel
2020-09-26 0:07 ` Lubomir Rintel
2020-10-16 20:07 ` Sam Ravnborg
2020-10-16 20:07 ` Sam Ravnborg
2020-10-25 15:19 ` Lubomir Rintel
2020-10-25 15:19 ` Lubomir Rintel
2020-10-25 15:43 ` Sam Ravnborg [this message]
2020-10-25 15:43 ` Sam Ravnborg
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20201025154346.GA37040@ravnborg.org \
--to=sam@ravnborg.org \
--cc=Laurent.pinchart@ideasonboard.com \
--cc=a.hajda@samsung.com \
--cc=airlied@linux.ie \
--cc=devicetree@vger.kernel.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=jernej.skrabec@siol.net \
--cc=jonas@kwiboo.se \
--cc=linux-kernel@vger.kernel.org \
--cc=lkundrak@v3.sk \
--cc=narmstrong@baylibre.com \
--cc=robh+dt@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.