Received-SPF: pass (google.com: domain of linux-kernel+bounces-33946-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Message-ID: <065601d3-92e7-46cc-a7aa-116cd02b3c36@quicinc.com>
Date: Mon, 22 Jan 2024 10:10:22 -0800
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 2/2] dt-bindings: hwinfo: Add Qualcomm's board-id types
Content-Language: en-US
To: Amrit Anand <quic_amrianan@quicinc.com>,
        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>
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>
 <391f8f48-d1f5-702d-20d4-ae8b8a7ace58@quicinc.com>
From: Elliot Berman <quic_eberman@quicinc.com>
In-Reply-To: <391f8f48-d1f5-702d-20d4-ae8b8a7ace58@quicinc.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8bit


On 1/22/2024 2:07 AM, Amrit Anand wrote:
> 
> 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.
> 

There is no "legacy" support needed here: Qualcomm's bootloaders
need to be updated to adhere to the new proposed spec. I suppose
we need to consider whether we have targets that only need SoC to
differentiate?

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

There are probably more boards that we aren't aware of.

Amrit, please add board IDs for all the boards that are
in kernel.org.