All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Thompson <danielt@kernel.org>
To: Junjie Cao <caojunjie650@gmail.com>
Cc: Lee Jones <lee@kernel.org>, Jingoo Han <jingoohan1@gmail.com>,
	Pavel Machek <pavel@kernel.org>, Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>, Helge Deller <deller@gmx.de>,
	dri-devel@lists.freedesktop.org, linux-leds@vger.kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-fbdev@vger.kernel.org, Pengyu Luo <mitltlatltl@gmail.com>
Subject: Re: [PATCH 2/2] backlight: aw99706: Add support for Awinic AW99706 backlight
Date: Wed, 29 Oct 2025 17:53:31 +0000	[thread overview]
Message-ID: <aQJUmx5elYOW2TvO@aspen.lan> (raw)
In-Reply-To: <CAK6c68h3Mc0=JbbbVAmo_cYeOR_T-_rRy5EacgYQh7HgQZOPBg@mail.gmail.com>

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@kernel.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@gmail.com>
> > > Signed-off-by: Junjie Cao <caojunjie650@gmail.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.

      reply	other threads:[~2025-10-29 17:52 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20251026123923.1531727-1-caojunjie650@gmail.com>
2025-10-26 12:39 ` [PATCH 1/2] dt-bindings: leds: backlight: Add Awinic AW99706 backlight Junjie Cao
2025-10-26 13:47   ` Krzysztof Kozlowski
2025-10-27  6:58     ` Junjie Cao
2025-10-27  8:38       ` Krzysztof Kozlowski
2025-10-27 10:29         ` Junjie Cao
2025-10-26 12:39 ` [PATCH 2/2] backlight: aw99706: Add support for " Junjie Cao
2025-10-27 12:06   ` kernel test robot
2025-10-27 18:59   ` kernel test robot
2025-10-28 13:22   ` Daniel Thompson
2025-10-29 11:49     ` Junjie Cao
2025-10-29 17:53       ` Daniel Thompson [this message]

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=aQJUmx5elYOW2TvO@aspen.lan \
    --to=danielt@kernel.org \
    --cc=caojunjie650@gmail.com \
    --cc=conor+dt@kernel.org \
    --cc=deller@gmx.de \
    --cc=devicetree@vger.kernel.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=jingoohan1@gmail.com \
    --cc=krzk+dt@kernel.org \
    --cc=lee@kernel.org \
    --cc=linux-fbdev@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-leds@vger.kernel.org \
    --cc=mitltlatltl@gmail.com \
    --cc=pavel@kernel.org \
    --cc=robh@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.