Re: [PATCH 4/4] arm64: dts: qcom: sdm845-oneplus: split qcom,board-id into tuples

From: Stephan Gerhold <stephan@gerhold.net>
To: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Cc: Andy Gross <agross@kernel.org>,
	Bjorn Andersson <bjorn.andersson@linaro.org>,
	Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	linux-arm-msm@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, Stephen Boyd <sboyd@kernel.org>,
	Konrad Dybcio <konrad.dybcio@somainline.org>,
	Amit Pundir <amit.pundir@linaro.org>,
	Trilok Soni <quic_tsoni@quicinc.com>,
	Rob Clark <robdclark@gmail.com>,
	Caleb Connolly <caleb@connolly.tech>
Subject: Re: [PATCH 4/4] arm64: dts: qcom: sdm845-oneplus: split qcom,board-id into tuples
Date: Fri, 3 Jun 2022 21:02:14 +0200	[thread overview]
Message-ID: <Yppatj7KuQLPdDW1@gerhold.net> (raw)
In-Reply-To: <20220529202629.47588-5-krzysztof.kozlowski@linaro.org>

+Cc Caleb Connolly <caleb@connolly.tech>

On Sun, May 29, 2022 at 10:26:29PM +0200, Krzysztof Kozlowski wrote:
> The qcom,board-id is an uint32 matrix, so a list of tuples.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>  arch/arm64/boot/dts/qcom/sdm845-oneplus-enchilada.dts | 2 +-
>  arch/arm64/boot/dts/qcom/sdm845-oneplus-fajita.dts    | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sdm845-oneplus-enchilada.dts b/arch/arm64/boot/dts/qcom/sdm845-oneplus-enchilada.dts
> index bf2cf92e8976..8897a2f4cfe3 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845-oneplus-enchilada.dts
> +++ b/arch/arm64/boot/dts/qcom/sdm845-oneplus-enchilada.dts
> @@ -12,7 +12,7 @@ / {
>  	compatible = "oneplus,enchilada", "qcom,sdm845";
>  	chassis-type = "handset";
>  	qcom,msm-id = <0x141 0x20001>;
> -	qcom,board-id = <8 0 17819 22>;
> +	qcom,board-id = <8 0>, <17819 22>;

FWIW: While it's just a cosmetic change this is a bit misleading in my
opinion. Having two tuples suggests this should be interpreted as:

"This device tree is suitable for two different boards:
 board-id = <8 0> (aka sdm845-mtp, a standard qcom reference board)
 OR, alternatively: board-id = <17819 22>"

Since this device tree is clearly not meant for sdm845-mtp one could now
argue that the <8 0> could be removed, and only the second tuple covers
the actual device. It might be worth a try (maybe Caleb can try?), but
I suspect the bootloader will not accept that...

I think the bootloader from OPPO/OnePlus is actually looking for
quadruples instead of tuples on this board. I have seen similar hacks on
several other OPPO devices as well. They usually add their project ID
(here: 17819) somewhere and look for that in the bootloader.

In this case maybe adding a short comment would be sufficient, just to
make it more obvious that this doesn't actually follow the binding
documentation.

But this kind of brings up the question if it's worth making any
constraints in the DT schema at all, if some of the device trees
can not follow it.

For example, older OPPO bootloaders actually look for triples instead,
e.g.: (This is from a real device!)
	qcom,board-id = <8 0 15009>;

So maybe it's just a matter of time until someone tries to add a DT
with a format that cannot be changed cosmetically to fit the DT schema...

Stephan