| Message ID | 1710418312-6559-3-git-send-email-quic_amrianan@quicinc.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Add board-id support for multiple DT selection | expand |
On Thu, 14 Mar 2024 17:41:52 +0530, 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". > > 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(+) > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13): yamllint warnings/errors: ./Documentation/devicetree/bindings/arm/qcom.yaml:1159:8: [warning] wrong indentation: expected 6 but found 7 (indentation) ./Documentation/devicetree/bindings/arm/qcom.yaml:1162:8: [warning] wrong indentation: expected 6 but found 7 (indentation) ./Documentation/devicetree/bindings/arm/qcom.yaml:1165:15: [warning] wrong indentation: expected 13 but found 14 (indentation) dtschema/dtc warnings/errors: doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/1710418312-6559-3-git-send-email-quic_amrianan@quicinc.com The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.
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> > + - 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> > + - 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. 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>; > + }; > + > + > ...
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. 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? >> + - 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. > > 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>; >> + }; >> + >> + >> ... >
On Thu, Mar 14, 2024 at 02:20:38PM +0000, Caleb Connolly wrote: > Hi Amrit, > > On 14/03/2024 12:11, Amrit Anand wrote: ... > > > > +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. > I agree with the concerns you listed. One point I wanted to bring is that if you provide a boot image that has only one DTB, all production Qualcomm bootloaders I'm aware of will use that DTB so long as "qcom,board-id" isn't a mismatch. I believe this is what most everyone is doing if using the DTBs from kernel.org. We'd like to use an open standard for DTB selection and that would very likely be incompatible with existing bootloaders that don't have whatever that standard is. > 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. One concern we have with U-Boot's approach is that it's based on metadata external to the DTB and, in our experience, makes it hard to track which board goes to which DTB. This approach also isn't necessarily portable to other image types/boot flows. Thanks, Elliot
On Thu, Mar 14, 2024 at 02:20:38PM +0000, Caleb Connolly wrote: > On 14/03/2024 12:11, Amrit Anand wrote: > 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. I pointed out previously that the Chromebook guys had some similar issues with dtb selection when the OEM varies parts but there does not seem to be any of them on CC here. There's definitely a generic problem to be solved here but I don't see an effort made to involve the other people known to have the same problems and this is all highly vendor specific.
On Sat, Mar 16, 2024 at 04:20:03PM +0000, Conor Dooley wrote: > On Thu, Mar 14, 2024 at 02:20:38PM +0000, Caleb Connolly wrote: > > On 14/03/2024 12:11, Amrit Anand wrote: > > 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. > > I pointed out previously that the Chromebook guys had some similar > issues with dtb selection when the OEM varies parts but there does not > seem to be any of them on CC here. That's maybe a bit harsh of me actually, I see that there's a chrome-platform address on CC, but I don't know if that's gonna reach the guys that work on these devices (Chen-Yu Tsai and Doug Anderson in particular).
Hi, On Sat, Mar 16, 2024 at 9:51 AM Conor Dooley <conor@kernel.org> wrote: > > On Sat, Mar 16, 2024 at 04:20:03PM +0000, Conor Dooley wrote: > > On Thu, Mar 14, 2024 at 02:20:38PM +0000, Caleb Connolly wrote: > > > On 14/03/2024 12:11, Amrit Anand wrote: > > > 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. > > > > I pointed out previously that the Chromebook guys had some similar > > issues with dtb selection when the OEM varies parts but there does not > > seem to be any of them on CC here. > > That's maybe a bit harsh of me actually, I see that there's a > chrome-platform address on CC, but I don't know if that's gonna reach > the guys that work on these devices (Chen-Yu Tsai and Doug Anderson in > particular). Thanks for the CC. Yeah, I don't watch the "chrome-platform" list myself, though maybe I should... The Chromebook boot flow and how we've handled this so far is documented in the kernel [1]. This method is what we've been using (with slight modifications over the years) since the earlier ARM Chromebooks and is, I believe, supported in both the depthcharge loader (used in Chromebooks) and also in U-Boot, though it's possible (?) that the U-Boot rules might vary ever so slightly. I haven't tried using U-Boot to boot a Chromebook in years. The current way things work for Chromebooks involves a heavy amount of duplication. We bundle an entire "DTB" for every relevant board/rev/sku combination even though many of those DTBs are 99% the same as the other ones. The DTBs have been relatively small and we compress them so this hasn't been a massive deal, but it's always been on the TODO list to come up with a scheme to use DT overlays. We've also talked about bundling a device tree that has the superset of components and then using an in-kernel driver to set the status of some components to okay and there is some overlap there in the possible way to represent board variants. I think Chen-Yu is going to talk about a few of these topics next month at EOSS [2]. In terms of looking at your specific proposal, it's definitely trying to factor in a lot more things than the current one that Chromebooks use. The Chromebook model was "simple" enough that we could just leverage the compatible string, though the way we leverage it has ended up controversial over the years. Yours is definitely too complicated to work the same way. It seems like device tree overlays would be a better fit? I'm not an expert so maybe this is already solved somewhere, but I'd imagine the hard part is getting everyone to agree on how to specify stuff in the DT overlay that allows the bootloader to know whether to overlay it or not... [1] https://docs.kernel.org/arch/arm/google/chromebook-boot-flow.html [2] https://eoss24.sched.com/event/1aBGe/second-source-component-probing-on-device-tree-platforms-chen-yu-tsai-google-llc
On Tue, Mar 19, 2024 at 5:36 AM Doug Anderson <dianders@chromium.org> wrote: > > Hi, > > On Sat, Mar 16, 2024 at 9:51 AM Conor Dooley <conor@kernel.org> wrote: > > > > On Sat, Mar 16, 2024 at 04:20:03PM +0000, Conor Dooley wrote: > > > On Thu, Mar 14, 2024 at 02:20:38PM +0000, Caleb Connolly wrote: > > > > On 14/03/2024 12:11, Amrit Anand wrote: > > > > 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. > > > > > > I pointed out previously that the Chromebook guys had some similar > > > issues with dtb selection when the OEM varies parts but there does not > > > seem to be any of them on CC here. > > > > That's maybe a bit harsh of me actually, I see that there's a > > chrome-platform address on CC, but I don't know if that's gonna reach > > the guys that work on these devices (Chen-Yu Tsai and Doug Anderson in > > particular). > > Thanks for the CC. Yeah, I don't watch the "chrome-platform" list > myself, though maybe I should... > > The Chromebook boot flow and how we've handled this so far is > documented in the kernel [1]. This method is what we've been using > (with slight modifications over the years) since the earlier ARM > Chromebooks and is, I believe, supported in both the depthcharge > loader (used in Chromebooks) and also in U-Boot, though it's possible > (?) that the U-Boot rules might vary ever so slightly. I haven't tried > using U-Boot to boot a Chromebook in years. > > The current way things work for Chromebooks involves a heavy amount of > duplication. We bundle an entire "DTB" for every relevant > board/rev/sku combination even though many of those DTBs are 99% the > same as the other ones. The DTBs have been relatively small and we > compress them so this hasn't been a massive deal, but it's always been > on the TODO list to come up with a scheme to use DT overlays. We've > also talked about bundling a device tree that has the superset of > components and then using an in-kernel driver to set the status of > some components to okay and there is some overlap there in the > possible way to represent board variants. I think Chen-Yu is going to > talk about a few of these topics next month at EOSS [2]. That's right. It's the last talk before the closing game on the last day. Elliot is also presenting the board-id proposal at EOSS [3], which is the last talk of day 2. > In terms of looking at your specific proposal, it's definitely trying > to factor in a lot more things than the current one that Chromebooks > use. The Chromebook model was "simple" enough that we could just > leverage the compatible string, though the way we leverage it has > ended up controversial over the years. Yours is definitely too > complicated to work the same way. It seems like device tree overlays > would be a better fit? I'm not an expert so maybe this is already > solved somewhere, but I'd imagine the hard part is getting everyone to > agree on how to specify stuff in the DT overlay that allows the > bootloader to know whether to overlay it or not... I think qcom,board-id + qcom,oem-id maps to our board name + SKU ID + revision. The difference is probably because we make our device codenames public? We don't actually have integer identifiers for each board family, though I do see the appeal of it. So it looks like this proposal is more about identifying boards without polluting (?) the compatible namespace with all sorts of combinations and hacks like we have. > [1] https://docs.kernel.org/arch/arm/google/chromebook-boot-flow.html > [2] https://eoss24.sched.com/event/1aBGe/second-source-component-probing-on-device-tree-platforms-chen-yu-tsai-google-llc [3] https://eoss24.sched.com/event/1aBFy/shipping-multiple-devicetrees-how-to-identify-which-dtb-is-for-my-board-elliot-berman-qualcomm
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>; >>> + }; >>> + >>> + >>> ...
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> + - 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> + - 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"; + + #address-cells = <2>; + #size-cells = <2>; + }; + + ...
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". 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(+)