Re: [PATCH v2 2/3] arm: dts: rockchip: Separate rockchip-binman.dtsi

Thu, 03 Nov 2022 03:03:16 -0700 

Hi Jagan,

On 11/3/22 07:19, Jagan Teki wrote:

rockchip-u-boot.dtsi has the FIT image for the final stage of
binman image creation.

If the actual binman node is part of this dtsi then there are
build issues to use optee as input to this final stage binman
image since optee is built via another binman image creation
unlike ATF built via tools like make_fit_atf.py.

    binman: Filename 'u-boot.itb' not found in input path

Fix this by separating binman FIT image in rockchip-binman.dtsi
My understanding is that this is a work-around for something that should be implemented in binman instead (e.g. dependency between images). If i'm not mistaken, what you're suggesting is to not build u-boot-rockchip.bin for some platforms? IIRC the plan for this binary was that it would apply to all Rockchip platforms, and this patch makes this "promise" go away. 


rockchip-u-boot.dtsi: binman node
rockchip-binman.dtsi: binman FIT image node

The inclusion of rockchip-binman.dtsi is always to be last in
included files as it has a FIT image node for final image creation.
You are not respecting this in your patch. Please update or remove this section if not required. (I assume you have this limitation because you use a binman phandle, meaning the node needs to be defined before). 

Also, rockchip-u-boot.dtsi content is now literally:
/ {
        binman: binman {
                multiple-images;
        };
};

which is pretty much useless.
Since you want to work around your build issue, why just not include rockchip-u-boot.dtsi instead of moving part of it to another file without any added benefit (at least at first glance, I may be missing some context).
BTW, we were discussing some months ago on moving away from make_fit_atf.py to binman for all Rockchip platforms, c.f. the long discussion here: https://lore.kernel.org/u-boot/20220725172953.GD2029@begut/ So maybe we should just do this and that might fix the problem you're trying to work-around? 

In any case, can you provide a bit more context on the failing platform(s)?

Cheers,
Quentin


Sample example of optee used rk3288 looks like
    b/arch/arm/dts/rk3288-u-boot.dtsi
    #include "rockchip-u-boot.dtsi"
    #include "rockchip-optee.dtsi"
    #include "rockchip-binman.dtsi"

Signed-off-by: Jagan Teki <ja...@edgeble.ai>
---
Changes for v2:
- excluded rv1126

  arch/arm/dts/px30-u-boot.dtsi              |  1 +
  arch/arm/dts/rk3036-u-boot.dtsi            |  1 +
  arch/arm/dts/rk3066a-u-boot.dtsi           |  1 +
  arch/arm/dts/rk3188-u-boot.dtsi            |  1 +
  arch/arm/dts/rk322x-u-boot.dtsi            |  1 +
  arch/arm/dts/rk3288-u-boot.dtsi            |  1 +
  arch/arm/dts/rk3308-u-boot.dtsi            |  1 +
  arch/arm/dts/rk3326-odroid-go2-u-boot.dtsi |  1 +
  arch/arm/dts/rk3328-u-boot.dtsi            |  1 +
  arch/arm/dts/rk3368-u-boot.dtsi            |  1 +
  arch/arm/dts/rk3399-u-boot.dtsi            |  1 +
  arch/arm/dts/rk356x-u-boot.dtsi            |  1 +
  arch/arm/dts/rockchip-binman.dtsi          | 67 ++++++++++++++++++++++
  arch/arm/dts/rockchip-u-boot.dtsi          | 61 --------------------
  arch/arm/dts/rv1108-u-boot.dtsi            |  1 +
  15 files changed, 80 insertions(+), 61 deletions(-)
  create mode 100644 arch/arm/dts/rockchip-binman.dtsi

diff --git a/arch/arm/dts/px30-u-boot.dtsi b/arch/arm/dts/px30-u-boot.dtsi
index 462eaf68f8..7cea48181c 100644
--- a/arch/arm/dts/px30-u-boot.dtsi
+++ b/arch/arm/dts/px30-u-boot.dtsi
@@ -4,6 +4,7 @@
   */
#include "rockchip-u-boot.dtsi" 
+#include "rockchip-binman.dtsi"
/ { 
        aliases {
diff --git a/arch/arm/dts/rk3036-u-boot.dtsi b/arch/arm/dts/rk3036-u-boot.dtsi
index 41ac054b81..a49526c63a 100644
--- a/arch/arm/dts/rk3036-u-boot.dtsi
+++ b/arch/arm/dts/rk3036-u-boot.dtsi
@@ -4,3 +4,4 @@
   */
#include "rockchip-u-boot.dtsi" 
+#include "rockchip-binman.dtsi"
diff --git a/arch/arm/dts/rk3066a-u-boot.dtsi b/arch/arm/dts/rk3066a-u-boot.dtsi
index bc6e609d02..a85f752477 100644
--- a/arch/arm/dts/rk3066a-u-boot.dtsi
+++ b/arch/arm/dts/rk3066a-u-boot.dtsi
@@ -1,4 +1,5 @@
  // SPDX-License-Identifier: GPL-2.0+
#include "rockchip-u-boot.dtsi" 
+#include "rockchip-binman.dtsi"
  #include "rk3xxx-u-boot.dtsi"
diff --git a/arch/arm/dts/rk3188-u-boot.dtsi b/arch/arm/dts/rk3188-u-boot.dtsi
index 735776c16b..bc028a8c1b 100644
--- a/arch/arm/dts/rk3188-u-boot.dtsi
+++ b/arch/arm/dts/rk3188-u-boot.dtsi
@@ -4,6 +4,7 @@
   */
#include "rockchip-u-boot.dtsi" 
+#include "rockchip-binman.dtsi"
  #include "rk3xxx-u-boot.dtsi"
&global_timer { 
diff --git a/arch/arm/dts/rk322x-u-boot.dtsi b/arch/arm/dts/rk322x-u-boot.dtsi
index 79c41e481b..bd31c3bdc4 100644
--- a/arch/arm/dts/rk322x-u-boot.dtsi
+++ b/arch/arm/dts/rk322x-u-boot.dtsi
@@ -1,6 +1,7 @@
  // SPDX-License-Identifier: GPL-2.0+
#include "rockchip-u-boot.dtsi" 
+#include "rockchip-binman.dtsi"
/ { 
        bus_intmem@10080000 {
diff --git a/arch/arm/dts/rk3288-u-boot.dtsi b/arch/arm/dts/rk3288-u-boot.dtsi
index e411445ed6..a24d590e20 100644
--- a/arch/arm/dts/rk3288-u-boot.dtsi
+++ b/arch/arm/dts/rk3288-u-boot.dtsi
@@ -5,6 +5,7 @@
#include "rockchip-u-boot.dtsi" 
  #include "rockchip-optee.dtsi"
+#include "rockchip-binman.dtsi"
/ { 
        aliases {
diff --git a/arch/arm/dts/rk3308-u-boot.dtsi b/arch/arm/dts/rk3308-u-boot.dtsi
index ab5bfc2ce9..665582ddd3 100644
--- a/arch/arm/dts/rk3308-u-boot.dtsi
+++ b/arch/arm/dts/rk3308-u-boot.dtsi
@@ -4,6 +4,7 @@
   */
#include "rockchip-u-boot.dtsi" 
+#include "rockchip-binman.dtsi"
/ { 
        aliases {
diff --git a/arch/arm/dts/rk3326-odroid-go2-u-boot.dtsi 
b/arch/arm/dts/rk3326-odroid-go2-u-boot.dtsi
index 16c33735eb..cd5d43b3c4 100644
--- a/arch/arm/dts/rk3326-odroid-go2-u-boot.dtsi
+++ b/arch/arm/dts/rk3326-odroid-go2-u-boot.dtsi
@@ -4,6 +4,7 @@
   */
#include "rockchip-u-boot.dtsi" 
+#include "rockchip-binman.dtsi"
/ { 
        chosen {
diff --git a/arch/arm/dts/rk3328-u-boot.dtsi b/arch/arm/dts/rk3328-u-boot.dtsi
index d4a7540a92..407286b176 100644
--- a/arch/arm/dts/rk3328-u-boot.dtsi
+++ b/arch/arm/dts/rk3328-u-boot.dtsi
@@ -4,6 +4,7 @@
   */
#include "rockchip-u-boot.dtsi" 
+#include "rockchip-binman.dtsi"
/ { 
        aliases {
diff --git a/arch/arm/dts/rk3368-u-boot.dtsi b/arch/arm/dts/rk3368-u-boot.dtsi
index 811d59ac34..c33604266a 100644
--- a/arch/arm/dts/rk3368-u-boot.dtsi
+++ b/arch/arm/dts/rk3368-u-boot.dtsi
@@ -5,6 +5,7 @@
#include <dt-bindings/memory/rk3368-dmc.h> 
  #include "rockchip-u-boot.dtsi"
+#include "rockchip-binman.dtsi"
/ { 
        dmc: dmc@ff610000 {
diff --git a/arch/arm/dts/rk3399-u-boot.dtsi b/arch/arm/dts/rk3399-u-boot.dtsi
index 3c1a15fe51..03a328b295 100644
--- a/arch/arm/dts/rk3399-u-boot.dtsi
+++ b/arch/arm/dts/rk3399-u-boot.dtsi
@@ -5,6 +5,7 @@
  #define USB_CLASS_HUB                 9
#include "rockchip-u-boot.dtsi" 
+#include "rockchip-binman.dtsi"
/ { 
        aliases {
diff --git a/arch/arm/dts/rk356x-u-boot.dtsi b/arch/arm/dts/rk356x-u-boot.dtsi
index ccb8db0001..8265e1a0bd 100644
--- a/arch/arm/dts/rk356x-u-boot.dtsi
+++ b/arch/arm/dts/rk356x-u-boot.dtsi
@@ -4,6 +4,7 @@
   */
#include "rockchip-u-boot.dtsi" 
+#include "rockchip-binman.dtsi"
/ { 
        aliases {
diff --git a/arch/arm/dts/rockchip-binman.dtsi 
b/arch/arm/dts/rockchip-binman.dtsi
new file mode 100644
index 0000000000..8f7b3ee53b
--- /dev/null
+++ b/arch/arm/dts/rockchip-binman.dtsi
@@ -0,0 +1,67 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2019 Jagan Teki <ja...@amarulasolutions.com>
+ */
+
+#include <config.h>
+
+#ifdef CONFIG_SPL
+&binman {
+       simple-bin {
+               filename = "u-boot-rockchip.bin";
+               pad-byte = <0xff>;
+
+               mkimage {
+                       filename = "idbloader.img";
+                       args = "-n", CONFIG_SYS_SOC, "-T", "rksd";
+#ifdef CONFIG_TPL
+                       multiple-data-files;
+
+                       u-boot-tpl {
+                       };
+#endif
+                       u-boot-spl {
+                       };
+               };
+
+#ifdef CONFIG_ARM64
+               blob {
+                       filename = "u-boot.itb";
+#else
+               u-boot-img {
+#endif
+                       offset = <CONFIG_SPL_PAD_TO>;
+               };
+       };
+
+#ifdef CONFIG_ROCKCHIP_SPI_IMAGE
+       simple-bin-spi {
+               filename = "u-boot-rockchip-spi.bin";
+               pad-byte = <0xff>;
+
+               mkimage {
+                       filename = "idbloader-spi.img";
+                       args = "-n", CONFIG_SYS_SOC, "-T", "rkspi";
+#ifdef CONFIG_TPL
+                       multiple-data-files;
+
+                       u-boot-tpl {
+                       };
+#endif
+                       u-boot-spl {
+                       };
+               };
+
+#ifdef CONFIG_ARM64
+               blob {
+                       filename = "u-boot.itb";
+#else
+               u-boot-img {
+#endif
+                       /* Sync with u-boot,spl-payload-offset if present */
+                       offset = <CONFIG_SYS_SPI_U_BOOT_OFFS>;
+               };
+       };
+#endif
+};
+#endif
diff --git a/arch/arm/dts/rockchip-u-boot.dtsi 
b/arch/arm/dts/rockchip-u-boot.dtsi
index 584f21eb5b..3d55553e44 100644
--- a/arch/arm/dts/rockchip-u-boot.dtsi
+++ b/arch/arm/dts/rockchip-u-boot.dtsi
@@ -10,64 +10,3 @@
                multiple-images;
        };
  };
-
-#ifdef CONFIG_SPL
-&binman {
-       simple-bin {
-               filename = "u-boot-rockchip.bin";
-               pad-byte = <0xff>;
-
-               mkimage {
-                       filename = "idbloader.img";
-                       args = "-n", CONFIG_SYS_SOC, "-T", "rksd";
-#ifdef CONFIG_TPL
-                       multiple-data-files;
-
-                       u-boot-tpl {
-                       };
-#endif
-                       u-boot-spl {
-                       };
-               };
-
-#ifdef CONFIG_ARM64
-               blob {
-                       filename = "u-boot.itb";
-#else
-               u-boot-img {
-#endif
-                       offset = <CONFIG_SPL_PAD_TO>;
-               };
-       };
-
-#ifdef CONFIG_ROCKCHIP_SPI_IMAGE
-       simple-bin-spi {
-               filename = "u-boot-rockchip-spi.bin";
-               pad-byte = <0xff>;
-
-               mkimage {
-                       filename = "idbloader-spi.img";
-                       args = "-n", CONFIG_SYS_SOC, "-T", "rkspi";
-#ifdef CONFIG_TPL
-                       multiple-data-files;
-
-                       u-boot-tpl {
-                       };
-#endif
-                       u-boot-spl {
-                       };
-               };
-
-#ifdef CONFIG_ARM64
-               blob {
-                       filename = "u-boot.itb";
-#else
-               u-boot-img {
-#endif
-                       /* Sync with u-boot,spl-payload-offset if present */
-                       offset = <CONFIG_SYS_SPI_U_BOOT_OFFS>;
-               };
-       };
-#endif
-};
-#endif
diff --git a/arch/arm/dts/rv1108-u-boot.dtsi b/arch/arm/dts/rv1108-u-boot.dtsi
index 6a2098b8d4..e6cb7b2f24 100644
--- a/arch/arm/dts/rv1108-u-boot.dtsi
+++ b/arch/arm/dts/rv1108-u-boot.dtsi
@@ -4,6 +4,7 @@
   */
#include "rockchip-u-boot.dtsi" 
+#include "rockchip-binman.dtsi"
&grf { 
        u-boot,dm-pre-reloc;

Reply via email to