| Message ID | 20240327023451.2288523-1-j-humphreys@ti.com |
|---|---|
| State | RFC |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | [RFC] Makefile.lib: find capsule ESL dtsi file when CONFIG_OF_UPSTREAM=y | expand |
hi Jonathan, On Wed, 27 Mar 2024 at 08:05, Jonathan Humphreys <j-humphreys@ti.com> wrote: > > When CONFIG_OF_UPSTREAM is enabled, DTS files are in SOC subdirectories (vs the > top level dts directory), but when CONFIG_EFI_CAPSULE_AUTHENTICATE is enabled, > the dynamically created dtsi file containing the capsule ESL DT node is in the > parent directory. This results in a build failure because the #include inserted > in the DTS file is local to the current directory. Update Makefile to have the > DT preprocessing of #includes search in the parent (dts top level) directory > too. > > I'm not sure if this is the best solution. > > I was also tempted to just manually include the capsule-key property in the > board dts, and avoid the Makefile implicit inclusion trickery. I would actually > prefer this approach as everything is more explicit. But this isn't an option > because if CONFIG_EFI_CAPSULE_AUTHENTICATE is enabled, the implicit inclusion of > the dtsi file happens. It would be better, IMO, if we only included the > generated dtsi file if CONFIG_EFI_CAPSULE_ESL_FILE is defined. Was only > supporting the implicit inclusiong approach an intentional design choice? I was not sure if users would want to manually insert the contents of the ESL file, which is a binary file(a few hundred bytes at least) into the DTS. If you prefer having such an option, we can change the logic to what you propose. Thanks. -sughosh > > Thanks > Jon > > Signed-off-by: Jonathan Humphreys <j-humphreys@ti.com> > --- > scripts/Makefile.lib | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib > index 12857316c58..62f87517c09 100644 > --- a/scripts/Makefile.lib > +++ b/scripts/Makefile.lib > @@ -334,7 +334,7 @@ cmd_dtc = mkdir -p $(dir ${dtc-tmp}) ; \ > (cat $< > $(pre-tmp)); \ > $(foreach f,$(subst $(quote),,$(dtsi_include_list)), \ > echo '$(pound)include "$(f)"' >> $(pre-tmp);) \ > - $(HOSTCC) -E $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) $(pre-tmp) ; \ > + $(HOSTCC) -E $(dtc_cpp_flags) -I$(obj) -x assembler-with-cpp -o $(dtc-tmp) $(pre-tmp) ; \ > $(DTC) -O dtb -o $@ -b 0 \ > -i $(dir $<) -i $(u_boot_dtsi_loc) $(DTC_FLAGS) \ > -d $(depfile).dtc.tmp $(dtc-tmp) || \ > -- > 2.34.1 >
Sughosh Ganu <sughosh.ganu@linaro.org> writes: > hi Jonathan, > > On Wed, 27 Mar 2024 at 08:05, Jonathan Humphreys <j-humphreys@ti.com> wrote: >> >> When CONFIG_OF_UPSTREAM is enabled, DTS files are in SOC subdirectories (vs the >> top level dts directory), but when CONFIG_EFI_CAPSULE_AUTHENTICATE is enabled, >> the dynamically created dtsi file containing the capsule ESL DT node is in the >> parent directory. This results in a build failure because the #include inserted >> in the DTS file is local to the current directory. Update Makefile to have the >> DT preprocessing of #includes search in the parent (dts top level) directory >> too. >> >> I'm not sure if this is the best solution. >> >> I was also tempted to just manually include the capsule-key property in the >> board dts, and avoid the Makefile implicit inclusion trickery. I would actually >> prefer this approach as everything is more explicit. But this isn't an option >> because if CONFIG_EFI_CAPSULE_AUTHENTICATE is enabled, the implicit inclusion of >> the dtsi file happens. It would be better, IMO, if we only included the >> generated dtsi file if CONFIG_EFI_CAPSULE_ESL_FILE is defined. Was only >> supporting the implicit inclusiong approach an intentional design choice? > > I was not sure if users would want to manually insert the contents of > the ESL file, which is a binary file(a few hundred bytes at least) > into the DTS. If you prefer having such an option, we can change the > logic to what you propose. Thanks. What I was thinking is that one would explictly add the capsule-key property to the board dts but do it just as the generated dtsi does where it references the .esl file. Jon > > -sughosh > >> >> Thanks >> Jon >> >> Signed-off-by: Jonathan Humphreys <j-humphreys@ti.com> >> --- >> scripts/Makefile.lib | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib >> index 12857316c58..62f87517c09 100644 >> --- a/scripts/Makefile.lib >> +++ b/scripts/Makefile.lib >> @@ -334,7 +334,7 @@ cmd_dtc = mkdir -p $(dir ${dtc-tmp}) ; \ >> (cat $< > $(pre-tmp)); \ >> $(foreach f,$(subst $(quote),,$(dtsi_include_list)), \ >> echo '$(pound)include "$(f)"' >> $(pre-tmp);) \ >> - $(HOSTCC) -E $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) $(pre-tmp) ; \ >> + $(HOSTCC) -E $(dtc_cpp_flags) -I$(obj) -x assembler-with-cpp -o $(dtc-tmp) $(pre-tmp) ; \ >> $(DTC) -O dtb -o $@ -b 0 \ >> -i $(dir $<) -i $(u_boot_dtsi_loc) $(DTC_FLAGS) \ >> -d $(depfile).dtc.tmp $(dtc-tmp) || \ >> -- >> 2.34.1 >>
On Thu, 28 Mar 2024 at 09:34, Jon Humphreys <j-humphreys@ti.com> wrote: > > Sughosh Ganu <sughosh.ganu@linaro.org> writes: > > > hi Jonathan, > > > > On Wed, 27 Mar 2024 at 08:05, Jonathan Humphreys <j-humphreys@ti.com> wrote: > >> > >> When CONFIG_OF_UPSTREAM is enabled, DTS files are in SOC subdirectories (vs the > >> top level dts directory), but when CONFIG_EFI_CAPSULE_AUTHENTICATE is enabled, > >> the dynamically created dtsi file containing the capsule ESL DT node is in the > >> parent directory. This results in a build failure because the #include inserted > >> in the DTS file is local to the current directory. Update Makefile to have the > >> DT preprocessing of #includes search in the parent (dts top level) directory > >> too. > >> > >> I'm not sure if this is the best solution. > >> > >> I was also tempted to just manually include the capsule-key property in the > >> board dts, and avoid the Makefile implicit inclusion trickery. I would actually > >> prefer this approach as everything is more explicit. But this isn't an option > >> because if CONFIG_EFI_CAPSULE_AUTHENTICATE is enabled, the implicit inclusion of > >> the dtsi file happens. It would be better, IMO, if we only included the > >> generated dtsi file if CONFIG_EFI_CAPSULE_ESL_FILE is defined. Was only > >> supporting the implicit inclusiong approach an intentional design choice? > > > > I was not sure if users would want to manually insert the contents of > > the ESL file, which is a binary file(a few hundred bytes at least) > > into the DTS. If you prefer having such an option, we can change the > > logic to what you propose. Thanks. > > What I was thinking is that one would explictly add the capsule-key > property to the board dts but do it just as the generated dtsi does > where it references the .esl file. Okay. In any case, that can be done by having the addition of the capsule_esl_dtsi to the dtsi_include_list under the CONFIG_EFI_CAPSULE_ESL_FILE conditional, isn't it? That way, we have both solutions. Users can then define the config flag to automate the inclusion of the capsule-key. Those who prefer to explicitly include the node to the board dts can do so on similar lines to how it is defined in the lib/efi_loader/capsule_esl.dtsi.in file. -sughosh > > > Jon > > > > > -sughosh > > > >> > >> Thanks > >> Jon > >> > >> Signed-off-by: Jonathan Humphreys <j-humphreys@ti.com> > >> --- > >> scripts/Makefile.lib | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib > >> index 12857316c58..62f87517c09 100644 > >> --- a/scripts/Makefile.lib > >> +++ b/scripts/Makefile.lib > >> @@ -334,7 +334,7 @@ cmd_dtc = mkdir -p $(dir ${dtc-tmp}) ; \ > >> (cat $< > $(pre-tmp)); \ > >> $(foreach f,$(subst $(quote),,$(dtsi_include_list)), \ > >> echo '$(pound)include "$(f)"' >> $(pre-tmp);) \ > >> - $(HOSTCC) -E $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) $(pre-tmp) ; \ > >> + $(HOSTCC) -E $(dtc_cpp_flags) -I$(obj) -x assembler-with-cpp -o $(dtc-tmp) $(pre-tmp) ; \ > >> $(DTC) -O dtb -o $@ -b 0 \ > >> -i $(dir $<) -i $(u_boot_dtsi_loc) $(DTC_FLAGS) \ > >> -d $(depfile).dtc.tmp $(dtc-tmp) || \ > >> -- > >> 2.34.1 > >>
Sughosh Ganu <sughosh.ganu@linaro.org> writes: > On Thu, 28 Mar 2024 at 09:34, Jon Humphreys <j-humphreys@ti.com> wrote: >> >> Sughosh Ganu <sughosh.ganu@linaro.org> writes: >> >> > hi Jonathan, >> > >> > On Wed, 27 Mar 2024 at 08:05, Jonathan Humphreys <j-humphreys@ti.com> wrote: >> >> >> >> When CONFIG_OF_UPSTREAM is enabled, DTS files are in SOC subdirectories (vs the >> >> top level dts directory), but when CONFIG_EFI_CAPSULE_AUTHENTICATE is enabled, >> >> the dynamically created dtsi file containing the capsule ESL DT node is in the >> >> parent directory. This results in a build failure because the #include inserted >> >> in the DTS file is local to the current directory. Update Makefile to have the >> >> DT preprocessing of #includes search in the parent (dts top level) directory >> >> too. >> >> >> >> I'm not sure if this is the best solution. >> >> >> >> I was also tempted to just manually include the capsule-key property in the >> >> board dts, and avoid the Makefile implicit inclusion trickery. I would actually >> >> prefer this approach as everything is more explicit. But this isn't an option >> >> because if CONFIG_EFI_CAPSULE_AUTHENTICATE is enabled, the implicit inclusion of >> >> the dtsi file happens. It would be better, IMO, if we only included the >> >> generated dtsi file if CONFIG_EFI_CAPSULE_ESL_FILE is defined. Was only >> >> supporting the implicit inclusiong approach an intentional design choice? >> > >> > I was not sure if users would want to manually insert the contents of >> > the ESL file, which is a binary file(a few hundred bytes at least) >> > into the DTS. If you prefer having such an option, we can change the >> > logic to what you propose. Thanks. >> >> What I was thinking is that one would explictly add the capsule-key >> property to the board dts but do it just as the generated dtsi does >> where it references the .esl file. > > Okay. In any case, that can be done by having the addition of the > capsule_esl_dtsi to the dtsi_include_list under the > CONFIG_EFI_CAPSULE_ESL_FILE conditional, isn't it? That way, we have > both solutions. Users can then define the config flag to automate the > inclusion of the capsule-key. Those who prefer to explicitly include > the node to the board dts can do so on similar lines to how it is > defined in the lib/efi_loader/capsule_esl.dtsi.in file. > exactly! Jon > -sughosh > >> >> >> Jon >> >> > >> > -sughosh >> > >> >> >> >> Thanks >> >> Jon >> >> >> >> Signed-off-by: Jonathan Humphreys <j-humphreys@ti.com> >> >> --- >> >> scripts/Makefile.lib | 2 +- >> >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> >> >> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib >> >> index 12857316c58..62f87517c09 100644 >> >> --- a/scripts/Makefile.lib >> >> +++ b/scripts/Makefile.lib >> >> @@ -334,7 +334,7 @@ cmd_dtc = mkdir -p $(dir ${dtc-tmp}) ; \ >> >> (cat $< > $(pre-tmp)); \ >> >> $(foreach f,$(subst $(quote),,$(dtsi_include_list)), \ >> >> echo '$(pound)include "$(f)"' >> $(pre-tmp);) \ >> >> - $(HOSTCC) -E $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) $(pre-tmp) ; \ >> >> + $(HOSTCC) -E $(dtc_cpp_flags) -I$(obj) -x assembler-with-cpp -o $(dtc-tmp) $(pre-tmp) ; \ >> >> $(DTC) -O dtb -o $@ -b 0 \ >> >> -i $(dir $<) -i $(u_boot_dtsi_loc) $(DTC_FLAGS) \ >> >> -d $(depfile).dtc.tmp $(dtc-tmp) || \ >> >> -- >> >> 2.34.1 >> >>
On Fri, 29 Mar 2024 at 14:04, Jon Humphreys <j-humphreys@ti.com> wrote: > > Sughosh Ganu <sughosh.ganu@linaro.org> writes: > > > On Thu, 28 Mar 2024 at 09:34, Jon Humphreys <j-humphreys@ti.com> wrote: > >> > >> Sughosh Ganu <sughosh.ganu@linaro.org> writes: > >> > >> > hi Jonathan, > >> > > >> > On Wed, 27 Mar 2024 at 08:05, Jonathan Humphreys <j-humphreys@ti.com> wrote: > >> >> > >> >> When CONFIG_OF_UPSTREAM is enabled, DTS files are in SOC subdirectories (vs the > >> >> top level dts directory), but when CONFIG_EFI_CAPSULE_AUTHENTICATE is enabled, > >> >> the dynamically created dtsi file containing the capsule ESL DT node is in the > >> >> parent directory. This results in a build failure because the #include inserted > >> >> in the DTS file is local to the current directory. Update Makefile to have the > >> >> DT preprocessing of #includes search in the parent (dts top level) directory > >> >> too. > >> >> > >> >> I'm not sure if this is the best solution. > >> >> > >> >> I was also tempted to just manually include the capsule-key property in the > >> >> board dts, and avoid the Makefile implicit inclusion trickery. I would actually > >> >> prefer this approach as everything is more explicit. But this isn't an option > >> >> because if CONFIG_EFI_CAPSULE_AUTHENTICATE is enabled, the implicit inclusion of > >> >> the dtsi file happens. It would be better, IMO, if we only included the > >> >> generated dtsi file if CONFIG_EFI_CAPSULE_ESL_FILE is defined. Was only > >> >> supporting the implicit inclusiong approach an intentional design choice? > >> > > >> > I was not sure if users would want to manually insert the contents of > >> > the ESL file, which is a binary file(a few hundred bytes at least) > >> > into the DTS. If you prefer having such an option, we can change the > >> > logic to what you propose. Thanks. > >> > >> What I was thinking is that one would explictly add the capsule-key > >> property to the board dts but do it just as the generated dtsi does > >> where it references the .esl file. > > > > Okay. In any case, that can be done by having the addition of the > > capsule_esl_dtsi to the dtsi_include_list under the > > CONFIG_EFI_CAPSULE_ESL_FILE conditional, isn't it? That way, we have > > both solutions. Users can then define the config flag to automate the > > inclusion of the capsule-key. Those who prefer to explicitly include > > the node to the board dts can do so on similar lines to how it is > > defined in the lib/efi_loader/capsule_esl.dtsi.in file. > > > > exactly! Ah okay. Then I guess we are on the same page. But you might want to figure out how you are going to provide the path to the ESL file. IIRC, providing the path relative to the u-boot tree does not work with out of tree builds. And I was told that the paths used in u-boot need to be relative to the U-Boot top-of-tree. -sughosh > > Jon > > > -sughosh > > > >> > >> > >> Jon > >> > >> > > >> > -sughosh > >> > > >> >> > >> >> Thanks > >> >> Jon > >> >> > >> >> Signed-off-by: Jonathan Humphreys <j-humphreys@ti.com> > >> >> --- > >> >> scripts/Makefile.lib | 2 +- > >> >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> >> > >> >> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib > >> >> index 12857316c58..62f87517c09 100644 > >> >> --- a/scripts/Makefile.lib > >> >> +++ b/scripts/Makefile.lib > >> >> @@ -334,7 +334,7 @@ cmd_dtc = mkdir -p $(dir ${dtc-tmp}) ; \ > >> >> (cat $< > $(pre-tmp)); \ > >> >> $(foreach f,$(subst $(quote),,$(dtsi_include_list)), \ > >> >> echo '$(pound)include "$(f)"' >> $(pre-tmp);) \ > >> >> - $(HOSTCC) -E $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) $(pre-tmp) ; \ > >> >> + $(HOSTCC) -E $(dtc_cpp_flags) -I$(obj) -x assembler-with-cpp -o $(dtc-tmp) $(pre-tmp) ; \ > >> >> $(DTC) -O dtb -o $@ -b 0 \ > >> >> -i $(dir $<) -i $(u_boot_dtsi_loc) $(DTC_FLAGS) \ > >> >> -d $(depfile).dtc.tmp $(dtc-tmp) || \ > >> >> -- > >> >> 2.34.1 > >> >>
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib index 12857316c58..62f87517c09 100644 --- a/scripts/Makefile.lib +++ b/scripts/Makefile.lib @@ -334,7 +334,7 @@ cmd_dtc = mkdir -p $(dir ${dtc-tmp}) ; \ (cat $< > $(pre-tmp)); \ $(foreach f,$(subst $(quote),,$(dtsi_include_list)), \ echo '$(pound)include "$(f)"' >> $(pre-tmp);) \ - $(HOSTCC) -E $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) $(pre-tmp) ; \ + $(HOSTCC) -E $(dtc_cpp_flags) -I$(obj) -x assembler-with-cpp -o $(dtc-tmp) $(pre-tmp) ; \ $(DTC) -O dtb -o $@ -b 0 \ -i $(dir $<) -i $(u_boot_dtsi_loc) $(DTC_FLAGS) \ -d $(depfile).dtc.tmp $(dtc-tmp) || \
When CONFIG_OF_UPSTREAM is enabled, DTS files are in SOC subdirectories (vs the top level dts directory), but when CONFIG_EFI_CAPSULE_AUTHENTICATE is enabled, the dynamically created dtsi file containing the capsule ESL DT node is in the parent directory. This results in a build failure because the #include inserted in the DTS file is local to the current directory. Update Makefile to have the DT preprocessing of #includes search in the parent (dts top level) directory too. I'm not sure if this is the best solution. I was also tempted to just manually include the capsule-key property in the board dts, and avoid the Makefile implicit inclusion trickery. I would actually prefer this approach as everything is more explicit. But this isn't an option because if CONFIG_EFI_CAPSULE_AUTHENTICATE is enabled, the implicit inclusion of the dtsi file happens. It would be better, IMO, if we only included the generated dtsi file if CONFIG_EFI_CAPSULE_ESL_FILE is defined. Was only supporting the implicit inclusiong approach an intentional design choice? Thanks Jon Signed-off-by: Jonathan Humphreys <j-humphreys@ti.com> --- scripts/Makefile.lib | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)