arm64: dts: qcom: sa8775p: add symbols to dtb

PATCH In review Eric Chanudet 6 participants 9 messages Mar 9, 2023 – Mar 24, 2023

Subsystems (3)

Device tree bindings Arm Qualcomm mailing list Arm Qualcomm support

lore

Share & tools

lore

Eric Chanudet

author

2023-03-09 23:44 UTC #0 lore raw

PATCH 1/1 arm64: dts: qcom: sa8775p: add symbols to dtb

ABL uses the __symbols__ section to process the DTB before passing it forward. Without it, the bootstrap is interrupted.

Signed-off-by Eric Chanudet <[email protected]>

---
Depends on initial sa8775p-ride.dts:
https://lore.kernel.org/all/[email protected]/

 arch/arm64/boot/dts/qcom/Makefile | 4 ++++
 1 file changed, 4 insertions(+)

arch/arm64/boot/dts/qcom/Makefile +4 -0

index b63cd1861e68..72e85ab31d74 100644
@@ -1,4 +1,8 @@
 # SPDX-License-Identifier: GPL-2.0
+
+# Enable support for device-tree overlays required on sa8775p-ride.
+DTC_FLAGS_sa8775p-ride := -@
+
 dtb-$(CONFIG_ARCH_QCOM)	+= apq8016-sbc.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= apq8094-sony-xperia-kitakami-karin_windy.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= apq8096-db820c.dtb

Signature · 1 line

2.39.1

Konrad Dybcio

2023-03-09 23:48 UTC #1 lore raw

Quoted 5 lines · Eric Chanudet

ABL uses the __symbols__ section to process the DTB before passing it forward. Without it, the bootstrap is interrupted.
Signed-off-by Eric Chanudet <[email protected]>
---

Fix your ABL.

Konrad

Quoted 19 lines

Depends on initial sa8775p-ride.dts:
https://lore.kernel.org/all/[email protected]/

 arch/arm64/boot/dts/qcom/Makefile | 4 ++++
 1 file changed, 4 insertions(+)

arch/arm64/boot/dts/qcom/Makefile +4 -0

index b63cd1861e68..72e85ab31d74 100644
@@ -1,4 +1,8 @@
 # SPDX-License-Identifier: GPL-2.0
+
+# Enable support for device-tree overlays required on sa8775p-ride.
+DTC_FLAGS_sa8775p-ride := -@
+
 dtb-$(CONFIG_ARCH_QCOM)	+= apq8016-sbc.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= apq8094-sony-xperia-kitakami-karin_windy.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= apq8096-db820c.dtb

Prasad Sodagudi

2023-03-14 04:49 UTC #2 lore raw

Quoted 9 lines · Konrad Dybcio

Quoted 5 lines · Eric Chanudet
ABL uses the __symbols__ section to process the DTB before passing it forward. Without it, the bootstrap is interrupted.
Signed-off-by Eric Chanudet <[email protected]>
---
Fix your ABL.

Hi Konrad,

Apps boot-loader need __symbols__ for dynamic overlay operation. Qualcomm firmware passes an overlay file to apps boot-loader to overlay some of the nodes based on firmware configuration. Without __symbols__ apps boot-loader is not able to overlay.

Qualcomm hypervisor/gunyah would like to overlay arch timer node with always-on property, So adding __symbols__ helps boot-loader to overlay.

I think, commit text is misleading here and I will request Eric to fix the commit text.

-Thanks, Prasad

Quoted 23 lines

Konrad

Quoted 19 lines

Depends on initial sa8775p-ride.dts:
https://lore.kernel.org/all/[email protected]/

  arch/arm64/boot/dts/qcom/Makefile | 4 ++++
  1 file changed, 4 insertions(+)

arch/arm64/boot/dts/qcom/Makefile +4 -0

index b63cd1861e68..72e85ab31d74 100644
@@ -1,4 +1,8 @@
  # SPDX-License-Identifier: GPL-2.0
+
+# Enable support for device-tree overlays required on sa8775p-ride.
+DTC_FLAGS_sa8775p-ride := -@
+
  dtb-$(CONFIG_ARCH_QCOM)	+= apq8016-sbc.dtb
  dtb-$(CONFIG_ARCH_QCOM)	+= apq8094-sony-xperia-kitakami-karin_windy.dtb
  dtb-$(CONFIG_ARCH_QCOM)	+= apq8096-db820c.dtb

Konrad Dybcio

2023-03-14 09:59 UTC #3 lore raw

Quoted 9 lines · Prasad Sodagudi

Quoted 6 lines · Konrad Dybcio
Quoted 3 lines · Eric Chanudet
ABL uses the __symbols__ section to process the DTB before passing it forward. Without it, the bootstrap is interrupted.

Quoted 6 lines

Quoted 3 lines
Quoted 2 lines
Signed-off-by Eric Chanudet <[email protected]>
---
Fix your ABL.
Hi Konrad,
Apps boot-loader need __symbols__ for dynamic overlay operation. Qualcomm firmware passes an overlay file to apps boot-loader to overlay some of the nodes based on firmware configuration. Without __symbols__ apps boot-loader is not able to overlay.

Yes/no. There are a plenty of libfdt functions that let you do that, especially if you know something about the node.

Quoted 2 lines

Qualcomm hypervisor/gunyah would like to overlay arch timer node with always-on property, So adding __symbols__ helps boot-loader to overlay.

For the arch timer, you can simply iterate over the top-level nodes (or in the worst case scenario, over all nodes which would not take very long on cortex-a / cortex-x cores) and look for the specific timer compatible that has been with us for like 10 years at this point and use libfdt's fdt_add_property without overlays.

Quoted 2 lines

I think, commit text is misleading here and I will request Eric to fix the commit text.

Sort of, but then the design you explained is very error-prone as it's dependent on the device tree always satisfying your hypervisor's needs.

Konrad

Quoted 26 lines

-Thanks, Prasad
Quoted 23 lines
Konrad
Quoted 19 lines
Depends on initial sa8775p-ride.dts:
https://lore.kernel.org/all/[email protected]/
arch/arm64/boot/dts/qcom/Makefile | 4 ++++
  1 file changed, 4 insertions(+)
arch/arm64/boot/dts/qcom/Makefile +4 -0
index b63cd1861e68..72e85ab31d74 100644
@@ -1,4 +1,8 @@
   # SPDX-License-Identifier: GPL-2.0
+
+# Enable support for device-tree overlays required on sa8775p-ride.
+DTC_FLAGS_sa8775p-ride := -@
+
dtb-$(CONFIG_ARCH_QCOM) += apq8016-sbc.dtb dtb-$(CONFIG_ARCH_QCOM) += apq8094-sony-xperia-kitakami-karin_windy.dtb dtb-$(CONFIG_ARCH_QCOM) += apq8096-db820c.dtb

Krzysztof Kozlowski

2023-03-16 06:55 UTC #4 lore raw

In reply to Prasad Sodagudi

Quoted 14 lines · Prasad Sodagudi

Quoted 9 lines · Konrad Dybcio
Quoted 5 lines · Eric Chanudet
ABL uses the __symbols__ section to process the DTB before passing it forward. Without it, the bootstrap is interrupted.
Signed-off-by Eric Chanudet <[email protected]>
---
Fix your ABL.
Hi Konrad,

Quoted 7 lines

Apps boot-loader need __symbols__ for dynamic overlay operation. Qualcomm firmware passes an overlay file to apps boot-loader to overlay some of the nodes based on firmware configuration. Without __symbols__ apps boot-loader is not able to overlay.
Qualcomm hypervisor/gunyah would like to overlay arch timer node with always-on property, So adding __symbols__ helps boot-loader to overlay.

Since none of these overlays are upstream, we can argue that it is not a valid reason. If you decide to keep overlays downstream then also you can keep patch enabling the overlays there...

Best regards,
Krzysztof

Bjorn Andersson

maintainer

2023-03-13 12:46 UTC #5 lore raw

In reply to Eric Chanudet

Quoted 4 lines · Eric Chanudet

ABL uses the __symbols__ section to process the DTB before passing it forward. Without it, the bootstrap is interrupted.
Signed-off-by Eric Chanudet <[email protected]>

@Rob, @Krzysztof, it seems useful to be able to use the upstream generated DTBs with overlays.

Do you suggest that we enable this on a per-board basis when it's being requested, across all devices, or tell the users that they have to re-generate the dtbs themselves?

Thanks,
Bjorn

Quoted 23 lines

---
Depends on initial sa8775p-ride.dts:
https://lore.kernel.org/all/[email protected]/

 arch/arm64/boot/dts/qcom/Makefile | 4 ++++
 1 file changed, 4 insertions(+)

arch/arm64/boot/dts/qcom/Makefile +4 -0

index b63cd1861e68..72e85ab31d74 100644
@@ -1,4 +1,8 @@
 # SPDX-License-Identifier: GPL-2.0
+
+# Enable support for device-tree overlays required on sa8775p-ride.
+DTC_FLAGS_sa8775p-ride := -@
+
 dtb-$(CONFIG_ARCH_QCOM)	+= apq8016-sbc.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= apq8094-sony-xperia-kitakami-karin_windy.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= apq8096-db820c.dtb

Signature · 1 line

2.39.1

Rob Herring

2023-03-14 15:15 UTC #6 lore raw

Quoted 4 lines · Bjorn Andersson

Quoted 2 lines · Eric Chanudet
ABL uses the __symbols__ section to process the DTB before passing it forward. Without it, the bootstrap is interrupted.

No clue what ABL is... Put this in high level terms, "I want to be able to apply overlays", not some unknown acronym and low-level detail of how overlays work.

Quoted 9 lines

Quoted 2 lines
Signed-off-by Eric Chanudet <[email protected]>
@Rob, @Krzysztof, it seems useful to be able to use the upstream generated DTBs with overlays.
Do you suggest that we enable this on a per-board basis when it's being requested, across all devices, or tell the users that they have to re-generate the dtbs themselves?

It's up to you whether you want the bloat in the dtbs or not. Labels become an ABI when symbols are enabled.

If it was my platform(s), I'd require the overlays to be upstream too, but I'm just paranoid about the modifications people want to make.

Rob

Bjorn Andersson

maintainer

2023-03-22 02:52 UTC #7 lore raw

In reply to Eric Chanudet

Quoted 3 lines · Eric Chanudet

ABL uses the __symbols__ section to process the DTB before passing it forward. Without it, the bootstrap is interrupted.

If the reason is that ABL refuses to boot without it, then please have ABL fixed. If on the other hand there is a valid reason for ABL to require the dtb to have __symbols__ defined, please describe that - if nothing else so that others know when this is supposed to be used.

Thanks,
Bjorn

Quoted 24 lines

Signed-off-by Eric Chanudet <[email protected]>

---
Depends on initial sa8775p-ride.dts:
https://lore.kernel.org/all/[email protected]/

 arch/arm64/boot/dts/qcom/Makefile | 4 ++++
 1 file changed, 4 insertions(+)

arch/arm64/boot/dts/qcom/Makefile +4 -0

index b63cd1861e68..72e85ab31d74 100644
@@ -1,4 +1,8 @@
 # SPDX-License-Identifier: GPL-2.0
+
+# Enable support for device-tree overlays required on sa8775p-ride.
+DTC_FLAGS_sa8775p-ride := -@
+
 dtb-$(CONFIG_ARCH_QCOM)	+= apq8016-sbc.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= apq8094-sony-xperia-kitakami-karin_windy.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= apq8096-db820c.dtb

Signature · 1 line

2.39.1

Eric Chanudet

author

2023-03-24 17:20 UTC #8 lore raw

Quoted 9 lines · Bjorn Andersson

Quoted 3 lines · Eric Chanudet
ABL uses the __symbols__ section to process the DTB before passing it forward. Without it, the bootstrap is interrupted.
If the reason is that ABL refuses to boot without it, then please have ABL fixed. If on the other hand there is a valid reason for ABL to require the dtb to have __symbols__ defined, please describe that - if nothing else so that others know when this is supposed to be used.

Here is what I understand from the ABL sources and discussions with Prasad:

Android Boot Loader (ABL), the UEFI application to run before executing the kernel, implements the Qualcomm SCM protocol to call into TZ. One of these SCM call is trapped by the hypervisor, itself provided with the firmware package for the board, and returns to ABL some information about our VM. These information may include one or more DTBO. ABL then proceeds and tries to apply the overlays on the DTB it loaded from the Android Boot Image it is trying to boot.

If there is an hypervisor and it returned at least one DTBO, ABL treats a failure to apply the DTBO (e.g, if __symbols__ are not available in the DTB) as critical and ends the boot. I was only ever given a firmware package that included the hypervisor and it always returned at least one DTBO. So enabling overlays is required to run this board, using the firmware I know of, with an upstream kernel and DTB at time of writing.

I suppose ABL could be made to handle such failure as a warning and continue booting? Which comes down to ignoring the DTBO provided by the hypervisor. Maybe that still allows the kernel to run the board with limited functionality?

Prior cases in the git history for enabling overlays covered board variants and extension headers (ti and nvidia). These do not fit what is happening here. In hindsight, I should have sent this as an RFC, with the above explanation to begin with, to ask about the limits and requirements.

Maybe Prasad, or someone with a more comprehensive knowledge of this board, can fill the remaining gaps or correct my understanding of the boot sequence if I got something wrong?

Thanks,

Signature · 1 line

Eric Chanudet