From: Amrit Anand <quic_amrianan@quicinc.com>
To: Caleb Connolly <caleb.connolly@linaro.org>, <robh@kernel.org>,
<krzysztof.kozlowski+dt@linaro.org>, <conor+dt@kernel.org>,
<agross@kernel.org>, <andersson@kernel.org>,
<konrad.dybcio@linaro.org>
Cc: <devicetree@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
<linux-arm-msm@vger.kernel.org>, <kernel@quicinc.com>,
<peter.griffin@linaro.org>, <linux-riscv@lists.infradead.org>,
<chrome-platform@lists.linux.dev>,
<linux-mediatek@lists.infradead.org>,
Simon Glass <sjg@chromium.org>
Subject: Re: [PATCH v2 2/2] dt-bindings: qcom: Update DT bindings for multiple DT
Date: Tue, 14 May 2024 08:44:22 +0530 [thread overview]
Message-ID: <3f6faea5-575d-46db-3a7b-ed6dbde060ea@quicinc.com> (raw)
In-Reply-To: <a5eac9d5-1e88-4d7a-b8e8-677f6d116782@linaro.org>
On 3/14/2024 8:05 PM, Caleb Connolly wrote:
> On 14/03/2024 14:20, Caleb Connolly wrote:
>> Hi Amrit,
>>
>> On 14/03/2024 12:11, Amrit Anand wrote:
>>> Qualcomm produces a lot of "unique" boards with slight differences in
>>> SoC's and board's configuration. For eg, there can be SM8150v1 on MTPv1,
>>> SM8150v1 on MTPv2, SM8150v2 on MTPv2, SM8150v2 on MTPv2 with a different
>>> PMIC, SM8150v2 with no modem support and so on. For instance, suppose we
>>> have 3 SoC, each with 4 boards supported, along with 2 PMIC support for
>>> each case which would lead to total of 24 DTB files. Along with these
>>> configurations, OEMs may also add certain additional board variants. Thus
>>> a mechanism is required to pick the correct DTB for the corresponding board.
>>>
>>> Introduce mechanism to select required DTB using newly introduced device
>>> tree properties "board-id" and "board-id-type". "board-id" will contain
>>> the list of values of "qcom,soc-id", "qcom,board-id", "qcom,pmic-id" or
>>> "qcom,oem-id". "board-id-types" contains the type of parameter which is
>>> entered. It can be either "qcom,soc-id", "qcom,board-id", "qcom,pmic-id"
>>> or "qcom,oem-id".
>> Thanks for working on this, it's nice to finally see this logic
>> documented in the kernel.
>>> Qualcomm based bootloader will use these properties to pick the best
>>> matched DTB to boot the device with.
>>>
>>> Signed-off-by: Amrit Anand<quic_amrianan@quicinc.com>
>>> ---
>>> Documentation/devicetree/bindings/arm/qcom.yaml | 90 +++++++++++++++++++++++++
>>> 1 file changed, 90 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/arm/qcom.yaml b/Documentation/devicetree/bindings/arm/qcom.yaml
>>> index 7f80f48..dc66ae9 100644
>>> --- a/Documentation/devicetree/bindings/arm/qcom.yaml
>>> +++ b/Documentation/devicetree/bindings/arm/qcom.yaml
>>> @@ -1100,6 +1100,76 @@ properties:
>>> kernel
>>> The property is deprecated.
>>>
>>> + board-id:
>>> + $ref: /schemas/types.yaml#/definitions/uint32-matrix
>>> + minItems: 2
>>> + description: |
>>> + Qualcomm specific bootloader uses multiple different identifiers
>>> + (qcom,soc-id, qcom,board-id, qcom,pmic-id, qcom,oem-id) to select
>>> + single Devicetree among list of Devicetrees. For different identifiers,
>>> + the selection can be done either based on exact match (where the
>>> + identifiers information coming from firmware should exactly match
>>> + the ones described in devicetree) or best match (firmware provided
>>> + identifier information closely matches with the one of the Devicetree).
>>> + Below table describes matching criteria for each identifier::
>>> + |----------------------------------------------------------------------|
>>> + | DT property | Individual fields | Exact | Best | Default |
>>> + |----------------------------------------------------------------------|
>>> + | qcom,soc-id | |
>>> + | | Chipset Id | Y | N | - |
>>> + | | SoC Revision | N | Y | - |
>>> + | qcom,board-id | |
>>> + | | Board Id | Y | N | - |
>>> + | | Board Major | N | Y | - |
>>> + | | Board Minor | N | Y | - |
>>> + | | Subtype | Y | N | 0 |
>>> + | | DDRtype | Y | N | 0 |
>>> + | | BootDevice Type | Y | N | 0 |
>>> + | qcom,pmic-id | |
>>> + | | Slave Id | Y | N | 0 |
>>> + | | PMIC Id | Y | N | 0 |
>>> + | | PMIC Major | N | Y | 0 |
>>> + | | PMIC Minor | N | Y | 0 |
>>> + | qcom,oem-id | |
>>> + | | OEM Id | Y | N | 0 |
>>> + |----------------------------------------------------------------------|
>>> + For best match, identifiers are matched based on following priority order::
>>> + SoC Revision > Board Major > Board Minor > PMIC Major > PMIC Minor
>>> +
>>> + board-id-types:
>>> + $ref: /schemas/types.yaml#/definitions/non-unique-string-array
>>> + description:
>>> + Each field and helper macros are defined at include/dt-bindings/arm/qcom,ids.
>>> + minItems: 2
>>> + items:
>>> + oneOf:
>>> + - const: qcom,soc-id
>>> + description:
>>> + Matches Qualcomm Technologies, Inc. boards with the specified SoC.
>>> + 2 integers are needed to describe a soc-id. The first integer is the
>>> + SoC ID and the second integer is the SoC revision.
>>> + qcom,soc-id = <soc-id soc-revision>
>>> + - const: qcom,board-id
>>> + description: |
>>> + Matches Qualcomm Technologies, Inc. boards with the specified board.
>>> + 2 integers are needed to describe a board-id. The first integer is the
>>> + board ID. The second integer is the board-subtype.
>>> + qcom,board-id = <board-id board-subtype>
> This is a recursive definition. You partially described the individual
> fields above, you should do that here.
The information about these fields are documented in header
include/dt-bindings/arm/qcom,ids.h
sent in patch 1.
> What is DDR type? What information is encoded that doesn't make sense to
> describe elsewhere in DT?
>
> Same for "bootdevice type", why would you pick a different DT based on
> whether the bootloader was loaded from UFS or NAND? Why does this
> information belong in DT?
We can have multiple DT for different DDR types and boot device types.
In order to distinguish different DT when
all other parameters are same, we are using DDR type, boot device type
as distinguishable parameters.
>>> + - const: qcom,pmic-id
>>> + description: |
>>> + Qualcomm boards can be attached to multiple PMICs where slave-id (SID)
>>> + indicates the address of the bus on which the PMIC is attached. It can be
>>> + any number. The model for a PMIC indicates the PMIC name attached to bus
>>> + described by SID along with major and minor version. 2 integers are needed
>>> + to describe qcom,pmic-id. The first integer is the slave-id and the second integer
>>> + is the pmic model.
>>> + qcom,pmic-id = <pmic-sid pmic-model>
> Same questions here, why don't you just walk the DT and read the
> compatible properties of PMIC nodes?
>>> + - const: qcom,oem-id
>>> + description: |
>>> + Matches Qualcomm Technologies, Inc. boards with the specified OEM ID.
>>> + 1 integer is needed to describe the oem-id.
>>> + qcom,oem-id = <oem-id>
>>> +
>>> allOf:
>>> # Explicit allow-list for older SoCs. The legacy properties are not allowed
>>> # on newer SoCs.
>>> @@ -1167,4 +1237,24 @@ allOf:
>>>
>>> additionalProperties: true
>>>
>>> +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";
>> Forgive me if this is a particularly cynical view, but this seems
>> incredibly blatant, the "qcom,board-id" property is deprecated for
>> various good reasons, just using a key/value map where "qcom,board-id"
>> is a key doesn't change that. There are two main issues I have with the
>> proposal here:
>>
>> 1. This breaks backwards compatibility, millions of production devices
>> with bootloaders that will never receive another update might be
>> compatible with the downstream "qcom,board-id" property, but they won't
>> work with this.
>> 2. A top level board-id property that isn't namespaced implies that it
>> isn't vendor specific, but the proposed implementation doesn't even
>> pretend to be vendor agnostic.
ok, will try to redefine it. Meantime, since Elliot has some suggestions
from his EOSS conference presentation,
will also co-work with him towards making another attempt at vendor
agnostic approach as well.
>>
>> U-Boot also has some ideas around this issue, there you can pass in
>> multiple DTBs and provide some board specific "best match" function.
>> I think there's definitely some value in exposing this information, but
>> there's no good reason to define the same data as `qcom,board-id` while
>> breaking production bootloaders.
>>> +
>>> + #address-cells = <2>;
>>> + #size-cells = <2>;
>>> + };
>>> +
>>> +
>>> ...
next prev parent reply other threads:[~2024-05-14 3:14 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-14 12:11 [PATCH v2 0/2] Add board-id support for multiple DT selection Amrit Anand
2024-03-14 12:11 ` [PATCH v2 1/2] dt-bindings: arm: qcom: Update Devicetree identifiers Amrit Anand
2024-03-14 14:29 ` Caleb Connolly
2024-05-07 5:07 ` Amrit Anand
2024-03-14 12:11 ` [PATCH v2 2/2] dt-bindings: qcom: Update DT bindings for multiple DT Amrit Anand
2024-03-14 13:35 ` Rob Herring
2024-03-14 14:20 ` Caleb Connolly
2024-03-14 14:35 ` Caleb Connolly
2024-05-14 3:14 ` Amrit Anand [this message]
2024-03-14 20:07 ` Elliot Berman
2024-03-16 16:20 ` Conor Dooley
2024-03-16 16:51 ` Conor Dooley
2024-03-18 21:36 ` Doug Anderson
2024-03-28 5:49 ` Chen-Yu Tsai
2024-03-28 8:50 ` [PATCH v2 0/2] Add board-id support for multiple DT selection Krzysztof Kozlowski
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=3f6faea5-575d-46db-3a7b-ed6dbde060ea@quicinc.com \
--to=quic_amrianan@quicinc.com \
--cc=agross@kernel.org \
--cc=andersson@kernel.org \
--cc=caleb.connolly@linaro.org \
--cc=chrome-platform@lists.linux.dev \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=kernel@quicinc.com \
--cc=konrad.dybcio@linaro.org \
--cc=krzysztof.kozlowski+dt@linaro.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mediatek@lists.infradead.org \
--cc=linux-riscv@lists.infradead.org \
--cc=peter.griffin@linaro.org \
--cc=robh@kernel.org \
--cc=sjg@chromium.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).