On 21/06/2022 21:49, Dmitry Baryshkov wrote:
> On Tue, 21 Jun 2022 at 22:32, Krzysztof Kozlowski
> <krzysztof.kozlowski@xxxxxxxxxx> wrote:
>>
>> On 21/06/2022 21:26, Dmitry Baryshkov wrote:
>>> 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.
>>
>> Don't ask me, I am new to this.
>>
>> https://lore.kernel.org/all/02ab0276-b078-fe66-8596-fcec4378722b@xxxxxxxxxxxxxx/
>
>
>
>
>>
>>>
>>>>
>>>> 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@xxxxxxxxxx/
>>>> Co-developed-by: Kumar Gala <galak@xxxxxxxxxxxxxx>
>>>> Signed-off-by: Kumar Gala <galak@xxxxxxxxxxxxxx>
>>>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx>
>>>> ---
>>>> .../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
>>
>> Right, thanks.
>>
>>>
>>>> + - 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?
>>
>> Second paragraph:
>> https://lore.kernel.org/all/20220522195138.35943-1-konrad.dybcio@xxxxxxxxxxxxxx/
>
> I think this is something peculiar to Sony. Public lahaina (sm8350)
> dts files contain both these properties:
>
> https://github.com/MiCode/kernel_devicetree/blob/zeus-s-oss/qcom/lahaina-hdk.dts
> https://github.com/MiCode/kernel_devicetree/blob/zeus-s-oss/qcom/lahaina-v2.1.dtsi
>
>>
>> Plus consensus with Rob:
>> https://lore.kernel.org/all/CAL_JsqKL-mtAQ8Q9H4vLGM8izVVzDPbUAVWSdS8AmGjN6X6kcA@xxxxxxxxxxxxxx/
>
> I'm not sure here. But sm8350 and sm8450 dtsi files use these
> properties. I've linked lahaina files above.
> The waiptio dtsi (sm8450) are present at the same URL.
If you did not like where the consensus is going during the discussion
last week, I would expect to join the discussion. Not to comment after I
implement it.
>
>>
>>> 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.
>>
>> If you have any email addresses in mind, please Cc them to invite in
>> discussions. Otherwise I am afraid it won't be allowed. The feedback I
>> got before was that SM8350 and newer do not require this property. Feel
>> free to propose other way to solve comments (see "consensus with Rob"
>> above).
>
> Amit is in CC list. In the past he used these properties to allow
> single-image booting of RB3 and RB5.
> In fact I might prefer adding more of these properties to the dts
> files, where it makes sense, to allow adding more dt files to the
> images we create.
> I'd really like to be able to boot a single image on all my boards
> (rb3, rb5, db410c, db820, ifc6560, etc).
You have several options here. Use the board-compatible-encoded-scheme,
which was merged like 6 years ago or something. Bootloader could parse
it, dtbTool as well. Add a generic property, like Rob wanted (and
probably fix bootloader). Or find any other way to satisfy Rob's
comments. These properties were not accepted 6 years ago and the board
compatible approach was merged instead. If 6 years is not enough to
change the bootloaders, nothing will happen here ever, so we need to
make some statement.
Best regards,
Krzysztof
