Received-SPF: pass (google.com: domain of linux-kernel+bounces-32703-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Message-ID: <391f8f48-d1f5-702d-20d4-ae8b8a7ace58@quicinc.com>
Date: Mon, 22 Jan 2024 15:37:14 +0530
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.9.0
Subject: Re: [PATCH 2/2] dt-bindings: hwinfo: Add Qualcomm's board-id types
To: Konrad Dybcio <konrad.dybcio@linaro.org>, <robh+dt@kernel.org>,
        <krzysztof.kozlowski+dt@linaro.org>, <conor+dt@kernel.org>,
        <agross@kernel.org>, <andersson@kernel.org>
CC: <devicetree@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <linux-arm-msm@vger.kernel.org>, <kernel@quicinc.com>,
        Elliot Berman
	<quic_eberman@quicinc.com>
References: <1705749649-4708-1-git-send-email-quic_amrianan@quicinc.com>
 <1705749649-4708-3-git-send-email-quic_amrianan@quicinc.com>
 <54426665-90c5-4355-a174-f512004e11e5@linaro.org>
Content-Language: en-US
From: Amrit Anand <quic_amrianan@quicinc.com>
In-Reply-To: <54426665-90c5-4355-a174-f512004e11e5@linaro.org>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit


On 1/20/2024 7:02 PM, Konrad Dybcio wrote:
> On 20.01.2024 12:20, Amrit Anand wrote:
>> Qualcomm based DT uses two or three different identifiers. The SoC
>> based idenfier which signifies chipset and the revision for those
>> chipsets. The board based identifier is used to distinguish different
>> boards (e.g. IDP, MTP) along with the different types of same boards.
>> The PMIC attached to the board can also be used as a identifier for
>> device tree.
>>
>> Signed-off-by: Elliot Berman <quic_eberman@quicinc.com>
>> Signed-off-by: Amrit Anand <quic_amrianan@quicinc.com>
>> ---
>>   .../devicetree/bindings/hwinfo/qcom,board-id.yaml  | 86 ++++++++++++++++++++++
>>   include/dt-bindings/arm/qcom,ids.h                 | 68 +++++++++++++++--
>>   2 files changed, 146 insertions(+), 8 deletions(-)
>>   create mode 100644 Documentation/devicetree/bindings/hwinfo/qcom,board-id.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/hwinfo/qcom,board-id.yaml b/Documentation/devicetree/bindings/hwinfo/qcom,board-id.yaml
>> new file mode 100644
>> index 0000000..807f134
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/hwinfo/qcom,board-id.yaml
>> @@ -0,0 +1,86 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/hwinfo/qcom,board-id.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: QCOM Board Identifier for Devicetree Selection
>> +
>> +maintainers:
>> +  - Amrit Anand <quic_amrianan@quicinc.com>
>> +  - Elliot Berman <quic_eberman@quicinc.com>
>> +
>> +description: |
> The '|'s are unnecessary in both commits, IIRC they're used for
> preserving formatting which we don't really need for non-styled
> plaintext
Sure, will do.
>> +  Qualcomm uses two and sometimes three hardware identifiers to describe
>> +  its boards
>> +      - a SoC identifier is used to match chipsets (e.g. sm8550 vs sm8450)
>> +      - a board identifier is used to match board form factor (e.g. MTP, QRD,
>> +        ADP, CRD)
>> +      - a PMIC identifier is occasionally used when different PMICs are used
>> +        for a given board/SoC combination.
>> +  Each field and helper macros are defined at::
>> +      - include/dt-bindings/arm/qcom,ids.h
>> +
>> +  For example,
>> +    / {
>> +        #board-id-cells = <2>;
>> +        board-id = <456 0>, <457 0>, <10 0>;
>> +        board-id-types = "qcom,soc-id", "qcom,soc-id", "qcom,board-id";
>> +     }
>> +
>> +allOf:
>> +  - $ref: board-id.yaml#
>> +
>> +properties:
>> +  board-id:
>> +    minItems: 2
> I believe some older platforms match exclusively based on socid, so
> perhaps 1 would be okay as well.
>
> [...]

Ok, considering legacy targets we can make it 1.

But i think ideally it should always be recommended to have a board ID 
associated with a SoC ID, correct me if my understanding is wrong.

>> +examples:
>> +   - |
>> +     #include <dt-bindings/arm/qcom,ids.h>
>> +     / {
>> +         model = "Qualcomm Technologies, Inc. sc7280 IDP SKU1 platform";
>> +         compatible = "qcom,sc7280-idp", "google,senor", "qcom,sc7280";
>> +
>> +         #board-id-cells = <2>;
>> +         board-id = <QCOM_SOC_ID(SC7280) QCOM_SOC_REVISION(1)>,
>> +                    <QCOM_SOC_ID(SC7280) QCOM_SOC_REVISION(2)>,
>> +                    <QCOM_BOARD_ID(IDP, 1, 0) QCOM_BOARD_SUBTYPE(UFS, ANY, 1)>;
>> +         board-id-types = "qcom,soc-id",
>> +                          "qcom,soc-id",
>> +                          "qcom,board-id";
> So, would the matching here would be:
>
> loop over disctinct board-id-types
> 	check if there's at least 1 match for all of them
> 		use this dtb if that's the case
>
> stop booting / "best guess match"
>
> ?
>
> [...]

Yes, But the "if" checking would have preference in place.
The preference logic would look something like this,

First will check for SoC-ID, if we have an exact match for SoC-ID then 
will proceed for board-ID match. Otherwise the DT would be discarded.
Once (exact) board-ID found, will proceed for subtype , pmic and so on.
Exact match and best match logic is used. Parameters like SoC-ID, 
board-ID are required to be best matched. Other few fields follow best 
match logic and best of the DT can be picked.

>> +#define QCOM_BOARD_ID_MTP		0x8
>> +#define QCOM_BOARD_ID_DRAGONBOARD	0x10
>> +#define QCOM_BOARD_ID_QRD		0x11
>> +#define QCOM_BOARD_ID_HDK		0x1F
>> +#define QCOM_BOARD_ID_ATP		0x21
>> +#define QCOM_BOARD_ID_IDP		0x22
>> +#define QCOM_BOARD_ID_SBC		0x24
>> +#define QCOM_BOARD_ID_QXR		0x26
>> +#define QCOM_BOARD_ID_CRD		0x28
> Missing ADP/QCP/Ride (if they're separate)

Sure, will update. Would need to work with teams.

Thanks,
Amrit.