Message-ID: <609d1f0f-daca-4fde-3315-5d8e171ca5c4@linaro.org>
Date:   Tue, 21 Jun 2022 22:26:36 +0300
From:   Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To:     Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
        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
Cc:     Konrad Dybcio <konrad.dybcio@...ainline.org>,
        Amit Pundir <amit.pundir@...aro.org>,
        Trilok Soni <quic_tsoni@...cinc.com>,
        Rob Clark <robdclark@...il.com>,
        Stephan Gerhold <stephan@...hold.net>,
        Kumar Gala <galak@...eaurora.org>,
        Stephen Boyd <sboyd@...nel.org>
Subject: Re: [PATCH v2 1/2] dt-bindings: arm: qcom: document qcom,msm-id and
 qcom,board-id

On 21/06/2022 21:56, Krzysztof Kozlowski wrote:
> The top level qcom,msm-id and qcom,board-id properties are utilized by
> bootloaders on Qualcomm MSM platforms to determine which device tree
> should be used and passed to the kernel.
> 
> The commit b32e592d3c28 ("devicetree: bindings: Document qcom board
> compatible format") from 2015 was a consensus during discussion about
> upstreaming qcom,msm-id and qcom,board-id fields.  There are however still
> problems with that consensus:
> 1. It was reached 7 years ago but it turned out its implementation did
>     not reach all possible products.
> 
> 2. Initially additional tool (dtbTool) was needed for parsing these
>     fields to create a QCDT image consisting of multiple DTBs, later the
>     bootloaders were improved and they use these qcom,msm-id and
>     qcom,board-id properties directly.

I might be mistaken here. I think it was expected that dtbTool would use 
board compat strings to generate qcom,msm-id and qcom,board-id 
properties. It's not that the bootloaders were improved.

> 
> 3. Extracting relevant information from the board compatible requires
>     this additional tool (dtbTool), which makes the build process more
>     complicated and not easily reproducible (DTBs are modified after the
>     kernel build).
> 
> 4. Some versions of Qualcomm bootloaders expect these properties even
>     when booting with a single DTB.  The community is stuck with these
>     bootloaders thus they require properties in the DTBs.
> 
> Since several upstreamed Qualcomm SoC-based boards require these
> properties to properly boot and the properties are reportedly used by
> bootloaders, document them.
> 
> Link: https://lore.kernel.org/r/a3c932d1-a102-ce18-deea-18cbbd05ecab@linaro.org/
> Co-developed-by: Kumar Gala <galak@...eaurora.org>
> Signed-off-by: Kumar Gala <galak@...eaurora.org>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
> ---
>   .../devicetree/bindings/arm/qcom.yaml         | 123 ++++++++++++++++++
>   include/dt-bindings/arm/qcom,ids.h            |  30 +++++
>   2 files changed, 153 insertions(+)
>   create mode 100644 include/dt-bindings/arm/qcom,ids.h
> 
> diff --git a/Documentation/devicetree/bindings/arm/qcom.yaml b/Documentation/devicetree/bindings/arm/qcom.yaml
> index 6c38c1387afd..05b98cde4653 100644
> --- a/Documentation/devicetree/bindings/arm/qcom.yaml
> +++ b/Documentation/devicetree/bindings/arm/qcom.yaml
> @@ -403,6 +403,129 @@ properties:
>                 - qcom,sm8450-qrd
>             - const: qcom,sm8450
>   
> +  # Board compatibles go above
> +
> +  qcom,msm-id:
> +    $ref: /schemas/types.yaml#/definitions/uint32-matrix
> +    minItems: 1
> +    maxItems: 8
> +    items:
> +      items:
> +        - description: |
> +            MSM chipset ID - an exact match value consisting of three bitfields::

two bitfields

> +             - bits 0-15  - The unique MSM chipset ID
> +             - bits 16-31 - Reserved; should be 0
> +        - description: |
> +            Hardware revision ID - a chipset specific 32-bit ID representing
> +            the version of the chipset.  It is best a match value - the
> +            bootloader will look for the closest possible match.
> +    deprecated: true
> +    description:
> +      The MSM chipset and hardware revision use by Qualcomm bootloaders.  It
> +      can optionally be an array of these to indicate multiple hardware that
> +      use the same device tree.  It is expected that the bootloader will use
> +      this information at boot-up to decide which device tree to use when given
> +      multiple device trees, some of which may not be compatible with the
> +      actual hardware.  It is the bootloader's responsibility to pass the
> +      correct device tree to the kernel.
> +      The property is deprecated - it is not expected on newer boards
> +      (starting with SM8350).

Could you please elaborate this? If the AOSP team were to add e.g. 
SM8350-HDK to their single RB3+RB5 images, they would still need the 
qcom,board-id/qcom,msm-id properties to let the bootloader choose proper 
DTB.

> +
> +  qcom,board-id:
> +    $ref: /schemas/types.yaml#/definitions/uint32-matrix
> +    minItems: 1
> +    maxItems: 8
> +    items:
> +      oneOf:
> +        - maxItems: 2
> +          items:
> +            - description: |
> +                Board ID consisting of three bitfields::
> +                  - bits 31-24 - Unused
> +                  - bits 23-16 - Platform Version Major
> +                  - bits 15-8  - Platform Version Minor
> +                  - bits 7-0   - Platform Type
> +                Platform Type field is an exact match value.  The
> +                Platform Major/Minor field is a best match.  The bootloader will
> +                look for the closest possible match.
> +            - description: |
> +                Subtype ID unique to a Platform Type/Chipset ID.  For a given
> +                Platform Type, there will typically only be a single board and the
> +                subtype_id will be 0.  However in some cases board variants may
> +                need to be distinguished by different subtype_id values.
> +        # OnePlus uses a variant of board-id with four elements:
> +        - minItems: 4
> +          items:
> +            - const: 8
> +            - const: 0
> +            - description: OnePlus board ID
> +            - description: OnePlus subtype ID
> +    deprecated: true
> +    description:
> +      The board type and revision information.  It can optionally be an array
> +      of these to indicate multiple boards that use the same device tree.  It
> +      is expected that the bootloader will use this information at boot-up to
> +      decide which device tree to use when given multiple device trees, some of
> +      which may not be compatible with the actual hardware.  It is the
> +      bootloader's responsibility to pass the correct device tree to the
> +      kernel
> +      The property is deprecated - it is not expected on newer boards
> +      (starting with SM8350).
> +
> +allOf:
> +  # Explicit allow-list for older SoCs. The legacy properties are not allowed
> +  # on newer SoCs.
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - qcom,apq8026
> +              - qcom,apq8094
> +              - qcom,apq8096
> +              - qcom,msm8992
> +              - qcom,msm8994
> +              - qcom,msm8996
> +              - qcom,msm8998
> +              - qcom,sdm630
> +              - qcom,sdm632
> +              - qcom,sdm845
> +              - qcom,sdx55
> +              - qcom,sdx65
> +              - qcom,sm6125
> +              - qcom,sm6350
> +              - qcom,sm7225
> +              - qcom,sm8150
> +              - qcom,sm8250
> +    then:
> +      properties:
> +        qcom,board-id: true
> +        qcom,msm-id: true
> +    else:
> +      properties:
> +        qcom,board-id: false
> +        qcom,msm-id: false
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - oneplus,cheeseburger
> +              - oneplus,dumpling
> +              - oneplus,enchilada
> +              - oneplus,fajita
> +    then:
> +      properties:
> +        qcom,board-id:
> +          items:
> +            minItems: 4
> +    else:
> +      properties:
> +        qcom,board-id:
> +          items:
> +            maxItems: 2
> +
>   additionalProperties: true
>   
>   ...
> diff --git a/include/dt-bindings/arm/qcom,ids.h b/include/dt-bindings/arm/qcom,ids.h
> new file mode 100644
> index 000000000000..eaf86c18650f
> --- /dev/null
> +++ b/include/dt-bindings/arm/qcom,ids.h
> @@ -0,0 +1,30 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (c) 2015, The Linux Foundation. All rights reserved.
> + * Copyright (c) 2022 Linaro Ltd
> + * Author: Krzysztof Kozlowski <krzk@...nel.org> based on previous work of Kumar Gala.
> + */
> +#ifndef _DT_BINDINGS_ARM_QCOM_IDS_H
> +#define _DT_BINDINGS_ARM_QCOM_IDS_H
> +
> +/* qcom,msm-id */
> +#define QCOM_ID_APQ8026				199
> +#define QCOM_ID_MSM8916				206
> +#define QCOM_ID_MSM8994				207
> +#define QCOM_ID_MSM8996_3_0			246

2_0 too.
And then (according to 3.18):
8996-pro 305
8996-pro-auto 315
8996-auto 310

> +#define QCOM_ID_APQ8016				247
> +#define QCOM_ID_MSM8216				248
> +#define QCOM_ID_MSM8116				249
> +#define QCOM_ID_MSM8616				250
> +#define QCOM_ID_MSM8998				292
> +#define QCOM_ID_SDM845				321

sdm845-v2.1-rb3.dts:	qcom,msm-id = <341 0x20001>;
But this might be a typo

> +
> +/* qcom,board-id */
> +#define QCOM_BOARD_ID(a, major, minor) \
> +	(((major & 0xff) << 16) | ((minor & 0xff) << 8) | QCOM_BOARD_ID_##a)
> +
> +#define QCOM_BOARD_ID_MTP			8
> +#define QCOM_BOARD_ID_DRAGONBOARD		10
> +#define QCOM_BOARD_ID_SBC			24
> +
> +#endif /* _DT_BINDINGS_ARM_QCOM_IDS_H */


-- 
With best wishes
Dmitry

Powered by blists - more mailing lists