phone-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Daniel Thompson <daniel.thompson@linaro.org>
To: Marijn Suijten <marijn.suijten@somainline.org>
Cc: phone-devel@vger.kernel.org, Andy Gross <agross@kernel.org>,
	Bjorn Andersson <bjorn.andersson@linaro.org>,
	Lee Jones <lee.jones@linaro.org>,
	Jingoo Han <jingoohan1@gmail.com>,
	~postmarketos/upstreaming@lists.sr.ht,
	AngeloGioacchino Del Regno 
	<angelogioacchino.delregno@somainline.org>,
	Konrad Dybcio <konrad.dybcio@somainline.org>,
	Martin Botka <martin.botka@somainline.org>,
	Jami Kettunen <jami.kettunen@somainline.org>,
	Pavel Dubrova <pashadubrova@gmail.com>,
	Kiran Gunda <kgunda@codeaurora.org>,
	Courtney Cavin <courtney.cavin@sonymobile.com>,
	Bryan Wu <cooloney@gmail.com>,
	linux-arm-msm@vger.kernel.org, dri-devel@lists.freedesktop.org,
	linux-fbdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 05/10] backlight: qcom-wled: Fix off-by-one maximum with default num_strings
Date: Wed, 6 Oct 2021 15:44:44 +0100	[thread overview]
Message-ID: <20211006144444.6q3qm3bzfrhzwa46@maple.lan> (raw)
In-Reply-To: <20211005173400.lyu3gabbalv2l3uq@SoMainline.org>

On Tue, Oct 05, 2021 at 07:34:00PM +0200, Marijn Suijten wrote:
> On 2021-10-05 17:24:53, Daniel Thompson wrote:
> > On Tue, Oct 05, 2021 at 05:23:26PM +0200, Marijn Suijten wrote:
> > > Since there don't seem to be any substantial platforms/PMICs using this
> > > functionality in a working manner, can I talk you into agreeing with
> > > fixing the DT instead?
> > 
> > I've no objections to seeing the DT updated. However I don't really see
> > what benefit we get from breaking existing DTs in order to do so.
> > 
> > "Cleaning up annoying legacy" is seldom a good reason to break existing
> > DTs since, if we could break DTs whenever we choose, there would never
> > be any annoying legacy to worry about. When conflicting properties
> > result in uninterpretable DTs then a break may be justified but that is
> > not the case here.
> 
> As mentioned in my message and repeated by Konrad, the only "existing
> DT" that could possibly be broken is a platform that's brought up by us
> (SoMainline) and we're more than happy to improve the driver and leave
> legacy DT behind us, unless there's more DT in circulation that hasn't
> landed in Linux mainline but should be taken into account?

Devicetrees are supposed to be the domain of firmware (e.g. not part of
the kernel).

I'm therefore reluctant to adopt an "it only exists if it is upstream"
approach for documented DT bindings. Doubly so when it is our bugs that
causes DTs to be written in a manner which we then retrospectively
declare to be wrong.


> Anyway the plan is to leave qcom,num-strings in place so that the
> default enabled_strings list in this driver actually serves a purpose.
> Then, if num-strings and enabled-strings is provided the former has
> precedence (assuming it doesn't exceed the size of the latter) but
> we'll print a warning about this (now unnecessary) ambiguity, and if
> possible at all - haven't found an example yet - make the properties
> mutually exclusive in dt-bindings.
> 
> Disallowing both cases would only simplify the code in the end but we
> can spend a few lines to support the desired legacy.

Yes, warning is OK for me.


Daniel.

  reply	other threads:[~2021-10-06 14:44 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-04 19:27 [PATCH 00/10] backlight: qcom-wled: fix and solidify handling of enabled-strings Marijn Suijten
2021-10-04 19:27 ` [PATCH 01/10] backlight: qcom-wled: Pass number of elements to read to read_u32_array Marijn Suijten
2021-10-05  9:05   ` Daniel Thompson
2021-10-04 19:27 ` [PATCH 02/10] backlight: qcom-wled: Use cpu_to_le16 macro to perform conversion Marijn Suijten
2021-10-05  9:06   ` Daniel Thompson
2021-10-04 19:27 ` [PATCH 03/10] backlight: qcom-wled: Override num-strings when enabled-strings is set Marijn Suijten
2021-10-05  9:38   ` Daniel Thompson
2021-10-04 19:27 ` [PATCH 04/10] backlight: qcom-wled: Validate enabled string indices in DT Marijn Suijten
2021-10-05  9:14   ` Daniel Thompson
2021-10-05 10:03     ` Marijn Suijten
2021-10-05 10:42       ` Daniel Thompson
2021-10-04 19:27 ` [PATCH 05/10] backlight: qcom-wled: Fix off-by-one maximum with default num_strings Marijn Suijten
2021-10-05  9:19   ` Daniel Thompson
2021-10-05 10:06     ` Marijn Suijten
2021-10-05 10:38       ` Daniel Thompson
2021-10-05 10:53         ` Daniel Thompson
2021-10-05 11:44           ` Marijn Suijten
2021-10-05 14:03             ` Daniel Thompson
2021-10-05 15:23               ` Marijn Suijten
2021-10-05 16:24                 ` Daniel Thompson
2021-10-05 16:50                   ` Konrad Dybcio
2021-10-05 17:34                   ` Marijn Suijten
2021-10-06 14:44                     ` Daniel Thompson [this message]
2021-10-07 21:28                       ` Marijn Suijten
2021-10-04 19:27 ` [PATCH 06/10] backlight: qcom-wled: Remove unnecessary 4th default string in wled3 Marijn Suijten
2021-10-05  9:20   ` Daniel Thompson
2021-10-04 19:27 ` [PATCH 07/10] backlight: qcom-wled: Provide enabled_strings default for wled 4 and 5 Marijn Suijten
2021-10-05  9:21   ` Daniel Thompson
2021-10-04 19:27 ` [PATCH 08/10] backlight: qcom-wled: Remove unnecessary double whitespace Marijn Suijten
2021-10-05  9:21   ` Daniel Thompson
2021-10-04 19:27 ` [PATCH 09/10] backlight: qcom-wled: Consistently use enabled-strings in set_brightness Marijn Suijten
2021-10-05  9:33   ` Daniel Thompson
2021-10-05 10:12     ` Marijn Suijten
2021-10-04 19:27 ` [PATCH 10/10] backlight: qcom-wled: Consider enabled_strings in autodetection Marijn Suijten

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=20211006144444.6q3qm3bzfrhzwa46@maple.lan \
    --to=daniel.thompson@linaro.org \
    --cc=agross@kernel.org \
    --cc=angelogioacchino.delregno@somainline.org \
    --cc=bjorn.andersson@linaro.org \
    --cc=cooloney@gmail.com \
    --cc=courtney.cavin@sonymobile.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=jami.kettunen@somainline.org \
    --cc=jingoohan1@gmail.com \
    --cc=kgunda@codeaurora.org \
    --cc=konrad.dybcio@somainline.org \
    --cc=lee.jones@linaro.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-fbdev@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marijn.suijten@somainline.org \
    --cc=martin.botka@somainline.org \
    --cc=pashadubrova@gmail.com \
    --cc=phone-devel@vger.kernel.org \
    --cc=~postmarketos/upstreaming@lists.sr.ht \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).