Thread (27 messages) 27 messages, 2 authors, 2021-11-16

Re: [RESEND PATCH v2 04/13] backlight: qcom-wled: Fix off-by-one maximum with default num_strings

From: Daniel Thompson <hidden>
Date: 2021-11-12 13:19:33
Also in: dri-devel, linux-arm-msm, linux-devicetree, lkml, phone-devel

On Fri, Nov 12, 2021 at 01:35:01PM +0100, Marijn Suijten wrote:
On 2021-11-12 12:08:39, Daniel Thompson wrote:
quoted
On Fri, Nov 12, 2021 at 01:26:57AM +0100, Marijn Suijten wrote:
quoted
+		if (string_len > 0) {
+			dev_warn(dev, "qcom,num-strings and qcom,enabled-strings are ambiguous\n");
The warning should also be below the error message on the next if statement.
Agreed.
quoted
This warning occurs even when there is no ambiguity.

This could be:

	if (string_len > 0 && val != string_len)

Combined these changes allows us to give a much more helpful and assertive
warning message:

qcom,num-strings mis-matches and will partially override
qcom,enabled-strings (remove qcom,num-strings?)
I want to let the user know it's set regardless of whether they're
equivalent; no need to set both.

How about:

    Only one of qcom,num-strings or qcom,enabled-strings should be set

That should be more descriptive?  Otherwise, let me know if you really
want to allow users to (unnecessarily) set both - or if it can / should
be caught in DT validation instead.
Yes. I can live with that text. Let's use that.

Maybe I wouldn't if there gazilions of existing DTs with both
properties but IIRC the number is likely to be small or zero
(although we couldn't be 100% sure which).


Daniel.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help