[v6,011/102] fdt: Show the preprocessed .dts file on error
diff mbox series

Message ID 20191206213936.v6.11.If3f82501255df1ffcf086953cbd1ed9b65432073@changeid
State Accepted
Delegated to: Bin Meng
Headers show
Series
  • x86: Add initial support for apollolake
Related show

Commit Message

Simon Glass Dec. 7, 2019, 4:41 a.m. UTC
When device-tree compilation fails it is sometimes tricky to see which
line is broken, since the input file to dtc is a pre-processed version
of the device tree.

Add a line that points to the file that needs to be checked:

When the error is in the main .dts file, output is something like this:

   output: 'Error: arch/x86/dts/.chromebook_coral.dtb.pre.tmp:478.46-47
	syntax error
   FATAL ERROR: Unable to parse input tree

but in fact looking at that file shows nothing useful:

   PAD_CFG_NF_IOSSTATE_IOSTERM(GPIO_157, UP_20K, DEEP, NF1, HIZCRX1, DISPUPD)

Instead we need to look at the preprocessed file, which shows:

   163 ((1U << 30) | (1 << 10)) ((0xb << 10) | PAD_CFG1_IOSSTATE_HIZCRX1)

Here it is clear that PAD_CFG1_IOSSTATE_HIZCRX1 is not defined and so is
not being resolved by the preprocessor.

This commit adds an additional useful message:

   Check arch/x86/dts/.chromebook_coral.dtb.dts.tmp for errors

Note that if the error is reported in an included file, such as
u-boot.dtsi then the output is the following:

   Error: arch/x86/dts/u-boot.dtsi:137.14-15 syntax error
   FATAL ERROR: Unable to parse input tree

But again, if the error is due to a preprocessor failure, like this:

   filename = CONFIG_IFW_INPUT_FILE;

then you can't tell what the problem is by looking at the source. All you
see is the original code:

	intel-ifwi {
		filename = CONFIG_IFW_INPUT_FILE;
		...
		};
	};
	intel-fsp-m {
		filename = CONFIG_FSP_FILE_M;
	};

Everything looks fine. But looking at the output of the preprocessor:

 intel-ifwi {
  filename = CONFIG_IFW_INPUT_FILE;
  ...
 };
 intel-fsp-m {
  filename = "fsp_m.bin";
 };

This shows that the filename (normally "fitimage.bin") has not been
inserted the preprocess, leading to the realisation that the value should
be CONFIG_IFWI_INPUT_FILE.

If the above does not make sense, I encourage people to try introducing
errors in the device tree preprocessed values.

Signed-off-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
---

Changes in v6: None
Changes in v5: None
Changes in v4:
- One last desperate attempt to try to explain the purpose of this commit
- Update the message to mention the preprocessed file, not un-preprocessed

Changes in v3:
- Update example error message to better show the intended purpose

Changes in v2: None

 scripts/Makefile.lib | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Bin Meng Dec. 8, 2019, 2:02 a.m. UTC | #1
On Sat, Dec 7, 2019 at 12:46 PM Simon Glass <sjg@chromium.org> wrote:
>
> When device-tree compilation fails it is sometimes tricky to see which
> line is broken, since the input file to dtc is a pre-processed version
> of the device tree.
>
> Add a line that points to the file that needs to be checked:
>
> When the error is in the main .dts file, output is something like this:
>
>    output: 'Error: arch/x86/dts/.chromebook_coral.dtb.pre.tmp:478.46-47
>         syntax error
>    FATAL ERROR: Unable to parse input tree
>
> but in fact looking at that file shows nothing useful:
>
>    PAD_CFG_NF_IOSSTATE_IOSTERM(GPIO_157, UP_20K, DEEP, NF1, HIZCRX1, DISPUPD)
>
> Instead we need to look at the preprocessed file, which shows:
>
>    163 ((1U << 30) | (1 << 10)) ((0xb << 10) | PAD_CFG1_IOSSTATE_HIZCRX1)
>
> Here it is clear that PAD_CFG1_IOSSTATE_HIZCRX1 is not defined and so is
> not being resolved by the preprocessor.
>
> This commit adds an additional useful message:
>
>    Check arch/x86/dts/.chromebook_coral.dtb.dts.tmp for errors
>
> Note that if the error is reported in an included file, such as
> u-boot.dtsi then the output is the following:
>
>    Error: arch/x86/dts/u-boot.dtsi:137.14-15 syntax error
>    FATAL ERROR: Unable to parse input tree
>
> But again, if the error is due to a preprocessor failure, like this:
>
>    filename = CONFIG_IFW_INPUT_FILE;
>
> then you can't tell what the problem is by looking at the source. All you
> see is the original code:
>
>         intel-ifwi {
>                 filename = CONFIG_IFW_INPUT_FILE;
>                 ...
>                 };
>         };
>         intel-fsp-m {
>                 filename = CONFIG_FSP_FILE_M;
>         };
>
> Everything looks fine. But looking at the output of the preprocessor:
>
>  intel-ifwi {
>   filename = CONFIG_IFW_INPUT_FILE;
>   ...
>  };
>  intel-fsp-m {
>   filename = "fsp_m.bin";
>  };
>
> This shows that the filename (normally "fitimage.bin") has not been
> inserted the preprocess, leading to the realisation that the value should
> be CONFIG_IFWI_INPUT_FILE.
>
> If the above does not make sense, I encourage people to try introducing
> errors in the device tree preprocessed values.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> ---
>
> Changes in v6: None
> Changes in v5: None
> Changes in v4:
> - One last desperate attempt to try to explain the purpose of this commit
> - Update the message to mention the preprocessed file, not un-preprocessed
>
> Changes in v3:
> - Update example error message to better show the intended purpose
>
> Changes in v2: None
>
>  scripts/Makefile.lib | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>

applied to u-boot-x86/next, thanks!

Patch
diff mbox series

diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index ef116e0e0a..c10cd83a0a 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -300,7 +300,9 @@  cmd_dtc = mkdir -p $(dir ${dtc-tmp}) ; \
 	$(CPP) $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) $(pre-tmp) ; \
 	$(DTC) -O dtb -o $@ -b 0 \
 		-i $(dir $<) $(DTC_FLAGS) \
-		-d $(depfile).dtc.tmp $(dtc-tmp) ; \
+		-d $(depfile).dtc.tmp $(dtc-tmp) || \
+		(echo "Check $(shell pwd)/$(pre-tmp) for errors" && false) \
+		; \
 	cat $(depfile).pre.tmp $(depfile).dtc.tmp > $(depfile) ; \
 	sed -i "s:$(pre-tmp):$(<):" $(depfile)