diff mbox series

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

Message ID 20220529202629.47588-5-krzysztof.kozlowski@linaro.org
State New
Headers show
Series dt-bindings: arm: qcom: qcom,board-id and qcom,msm-id | expand

Commit Message

Krzysztof Kozlowski May 29, 2022, 8:26 p.m. UTC
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(-)

Comments

Krzysztof Kozlowski June 5, 2022, 3:42 p.m. UTC | #1
On 03/06/2022 21:02, Stephan Gerhold wrote:
> +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.

Thanks for bringing up this topic. I think we should include this
quadruple-set in the DT schema.

> 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...

Generic answer is: yes, we want constraints because we want to define
interface which is followed by bootloader. Following up answer is - in
practice this might not be possible...

I wish I could say that DTS abusing bindings will not be accepted, but
unfortunately vendor (OnePlus or whoever) simply does not care at all,
so this would affect only the community. Therefore rejecting such DTS is
not a viable option which leads me to first option - try to describe it
in schema, as much as possible.

Even if it means some "oneOf:" set for different vendors.


Best regards,
Krzysztof
diff mbox series

Patch

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>;
 
 	battery: battery {
 		compatible = "simple-battery";
diff --git a/arch/arm64/boot/dts/qcom/sdm845-oneplus-fajita.dts b/arch/arm64/boot/dts/qcom/sdm845-oneplus-fajita.dts
index 1b6b5bf368df..193cbd27b8b4 100644
--- a/arch/arm64/boot/dts/qcom/sdm845-oneplus-fajita.dts
+++ b/arch/arm64/boot/dts/qcom/sdm845-oneplus-fajita.dts
@@ -12,7 +12,7 @@  / {
 	compatible = "oneplus,fajita", "qcom,sdm845";
 	chassis-type = "handset";
 	qcom,msm-id = <0x141 0x20001>;
-	qcom,board-id = <8 0 18801 41>;
+	qcom,board-id = <8 0>, <18801 41>;
 
 	battery: battery {
 		compatible = "simple-battery";