linux-kernel - Re: [PATCH 2/2] backlight: aw99706: Add support for Awinic AW99706 backlight

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 for Android: free password hash cracker in your pocket

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

Message-ID: <aQJUmx5elYOW2TvO@aspen.lan>
Date: Wed, 29 Oct 2025 17:53:31 +0000
From: Daniel Thompson <danielt@...nel.org>
To: Junjie Cao <caojunjie650@...il.com>
Cc: Lee Jones <lee@...nel.org>, Jingoo Han <jingoohan1@...il.com>,
	Pavel Machek <pavel@...nel.org>, Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>, Helge Deller <deller@....de>,
	dri-devel@...ts.freedesktop.org, linux-leds@...r.kernel.org,
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-fbdev@...r.kernel.org, Pengyu Luo <mitltlatltl@...il.com>
Subject: Re: [PATCH 2/2] backlight: aw99706: Add support for Awinic AW99706
 backlight

On Wed, Oct 29, 2025 at 07:49:35PM +0800, Junjie Cao wrote:
> On Tue, Oct 28, 2025 at 9:21 PM Daniel Thompson <danielt@...nel.org> wrote:
> >
> > On Sun, Oct 26, 2025 at 08:39:23PM +0800, Junjie Cao wrote:
> > > Add support for Awinic AW99706 backlight, which can be found in
> > > tablet and notebook backlight, one case is the Lenovo Legion Y700
> > > Gen4. This driver refers to the official datasheets and android
> > > driver, they can be found in [1].
> > >
> > > [1] https://www.awinic.com/en/productDetail/AW99706QNR
> > >
> > > Signed-off-by: Pengyu Luo <mitltlatltl@...il.com>
> > > Signed-off-by: Junjie Cao <caojunjie650@...il.com>
> > > ---
> > > diff --git a/drivers/video/backlight/aw99706.c b/drivers/video/backlight/aw99706.c
> > > <snip>
> > > +static void aw99706_dt_parse(struct aw99706_device *aw)
> > > +{
> > > +     struct aw99706_dt_prop *prop;
> > > +     int ret, i;
> > > +
> > > +     for (i = 0; i < ARRAY_SIZE(aw99706_dt_props); i++) {
> > > +             prop = &aw99706_dt_props[i];
> > > +             ret = device_property_read_u32(aw->dev, prop->name,
> > > +                                            &prop->raw_val);
> > > +             if (ret < 0) {
> > > +                     dev_warn(aw->dev, "Missing property %s: %d\n",
> > > +                              prop->name, ret);
> >
> > Why is there a warning when an optional property is not present. A DT
> > not including an optional property needs no message at all.
> >
>
> They are mandatory in the downstream, and providing all properties is
> difficult sometimes, so I set a default value if one is missing. But
> one device may use a configuration different from the component
> vendor's. These default values may be not optimal, so I issue a
> warning for property missing. (I forgot to address it)

All sensible but to be clear...

>From my point-of-view the driver should match the upstream bindings.
Either the properties are required (in which case missing them can be
dev_err() and/or fail to probe) or they are optional (in which case
there should be no warnings).

Similarly if missing values is likely to lead to very sub-optimal
behavior (or something that has a risk of over-current or component
failure) then consider making the options mandatory.


Daniel.