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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sun, 5 Jun 2022 17:42:23 +0200
From:   Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To:     Stephan Gerhold <stephan@...hold.net>
Cc:     Andy Gross <agross@...nel.org>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, Stephen Boyd <sboyd@...nel.org>,
        Konrad Dybcio <konrad.dybcio@...ainline.org>,
        Amit Pundir <amit.pundir@...aro.org>,
        Trilok Soni <quic_tsoni@...cinc.com>,
        Rob Clark <robdclark@...il.com>,
        Caleb Connolly <caleb@...nolly.tech>
Subject: Re: [PATCH 4/4] arm64: dts: qcom: sdm845-oneplus: split qcom,board-id
 into tuples

On 03/06/2022 21:02, Stephan Gerhold wrote:
> +Cc Caleb Connolly <caleb@...nolly.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@...aro.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