* [PATCH] binman: Add option for pointing to external description @ 2024-10-07 13:05 Michal Simek 2024-10-09 1:55 ` Simon Glass 0 siblings, 1 reply; 12+ messages in thread From: Michal Simek @ 2024-10-07 13:05 UTC (permalink / raw) To: u-boot, git, Simon Glass Cc: AKASHI Takahiro, Andrew Davis, Bryan Brattlof, Heinrich Schuchardt, Ilias Apalodimas, Leon M. Busch-George, Rasmus Villemoes, Sean Anderson, Sughosh Ganu, Sumit Garg, Tom Rini Adding binman node with target images description can be unwanted feature but as of today there is no way to disable it. Also on size constrained systems it is not useful to add binman description to DTB. Introduce BINMAN_EXTERNAL_DTB Kconfig symbol which allows separate DTB for target from DTB for binman itself. Signed-off-by: Michal Simek <michal.simek@amd.com> --- Makefile | 2 +- lib/Kconfig | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index af24de4165e4..8043016ac279 100644 --- a/Makefile +++ b/Makefile @@ -1384,7 +1384,7 @@ cmd_binman = $(srctree)/tools/binman/binman $(if $(BINMAN_DEBUG),-D) \ $(foreach f,$(BINMAN_TOOLPATHS),--toolpath $(f)) \ --toolpath $(objtree)/tools \ $(if $(BINMAN_VERBOSE),-v$(BINMAN_VERBOSE)) \ - build -u -d u-boot.dtb -O . -m \ + build -u -d $(CONFIG_BINMAN_EXTERNAL_DTB) -O . -m \ --allow-missing $(if $(BINMAN_ALLOW_MISSING),--ignore-missing) \ -I . -I $(srctree) -I $(srctree)/board/$(BOARDDIR) \ $(foreach f,$(of_list_dirs),-I $(f)) -a of-list=$(of_list) \ diff --git a/lib/Kconfig b/lib/Kconfig index 1448b4ac2d64..491f265012b8 100644 --- a/lib/Kconfig +++ b/lib/Kconfig @@ -45,6 +45,16 @@ config BINMAN_FDT locate entries in the firmware image. See binman.h for the available functionality. +config BINMAN_EXTERNAL_DTB + string "External binman description" + depends on BINMAN + default "u-boot.dtb" + help + This enables option to point to different DTB file with binman node which + is outside of DTB used by the firmware. Use this option if information + about generated images shouldn't be the part of target binary. Or on system + with limited storage. + config CC_OPTIMIZE_LIBS_FOR_SPEED bool "Optimize libraries for speed" help -- 2.43.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] binman: Add option for pointing to external description 2024-10-07 13:05 [PATCH] binman: Add option for pointing to external description Michal Simek @ 2024-10-09 1:55 ` Simon Glass 2024-10-09 13:21 ` Michal Simek 0 siblings, 1 reply; 12+ messages in thread From: Simon Glass @ 2024-10-09 1:55 UTC (permalink / raw) To: Michal Simek Cc: u-boot, git, AKASHI Takahiro, Andrew Davis, Bryan Brattlof, Heinrich Schuchardt, Ilias Apalodimas, Leon M. Busch-George, Rasmus Villemoes, Sean Anderson, Sughosh Ganu, Sumit Garg, Tom Rini Hi Michal, On Mon, 7 Oct 2024 at 07:05, Michal Simek <michal.simek@amd.com> wrote: > > Adding binman node with target images description can be unwanted feature > but as of today there is no way to disable it. > Also on size constrained systems it is not useful to add binman description > to DTB. > Introduce BINMAN_EXTERNAL_DTB Kconfig symbol which allows separate DTB for > target from DTB for binman itself. > > Signed-off-by: Michal Simek <michal.simek@amd.com> > --- > > Makefile | 2 +- > lib/Kconfig | 10 ++++++++++ > 2 files changed, 11 insertions(+), 1 deletion(-) > Doesn't this defeat one of the purposes of Binman, i.e. to document images? We do want the .dts to include the image description. What sort of problem is this causing? Regards, Simon ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] binman: Add option for pointing to external description 2024-10-09 1:55 ` Simon Glass @ 2024-10-09 13:21 ` Michal Simek 2024-10-09 21:14 ` Simon Glass 0 siblings, 1 reply; 12+ messages in thread From: Michal Simek @ 2024-10-09 13:21 UTC (permalink / raw) To: Simon Glass Cc: u-boot, git, AKASHI Takahiro, Andrew Davis, Bryan Brattlof, Heinrich Schuchardt, Ilias Apalodimas, Leon M. Busch-George, Rasmus Villemoes, Sean Anderson, Sughosh Ganu, Sumit Garg, Tom Rini Hi, On 10/9/24 03:55, Simon Glass wrote: > Hi Michal, > > On Mon, 7 Oct 2024 at 07:05, Michal Simek <michal.simek@amd.com> wrote: >> >> Adding binman node with target images description can be unwanted feature >> but as of today there is no way to disable it. >> Also on size constrained systems it is not useful to add binman description >> to DTB. >> Introduce BINMAN_EXTERNAL_DTB Kconfig symbol which allows separate DTB for >> target from DTB for binman itself. >> >> Signed-off-by: Michal Simek <michal.simek@amd.com> >> --- >> >> Makefile | 2 +- >> lib/Kconfig | 10 ++++++++++ >> 2 files changed, 11 insertions(+), 1 deletion(-) >> > > Doesn't this defeat one of the purposes of Binman, i.e. to document > images? We do want the .dts to include the image description. What > sort of problem is this causing? We have two boot flows. The first one (default one) is using Xilinx FSBL for SOM initialization with fit image (DTBS) + u-boot.elf + tfa. The second one is using U-Boot SPL instead of FSBL. This flow is used by buildroot for example. In perfect world I should describe both of these flows. I sent description for the second as RFC here. https://lore.kernel.org/r/de1b8dbabd5ab7f20d7aac217ec4f5074d39f1da.1728462767.git.michal.simek@amd.com but it is also reasonable to describe the first flow but I really don't want both descriptions ends up in the target image. The second part is if you look at RFC and how fit-dtb.blob is composed. It is one DTB + DTBS which are composed from overlays. xilinx_zynqmp_kria_defconfig has CONFIG_DEFAULT_DEVICE_TREE="zynqmp-smk-k26-revA" That's why binman node should go to this DTB but because other images are composed with overlays binman node is spread to all DTBs inside FIT image. It means one binman description is in fit-dtb.blob 14 times which is far from ideal. Third part is that I can't see binman node in DT schema or bindings that's why I expect this will be reported and I can't see any code which removes it before handing off to OS which is required for System Ready IR. And IIRC removing is also problematic for measured boot. Thanks, Michal ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] binman: Add option for pointing to external description 2024-10-09 13:21 ` Michal Simek @ 2024-10-09 21:14 ` Simon Glass 2024-10-10 13:03 ` Michal Simek 0 siblings, 1 reply; 12+ messages in thread From: Simon Glass @ 2024-10-09 21:14 UTC (permalink / raw) To: Michal Simek Cc: u-boot, git, AKASHI Takahiro, Andrew Davis, Bryan Brattlof, Heinrich Schuchardt, Ilias Apalodimas, Leon M. Busch-George, Rasmus Villemoes, Sean Anderson, Sughosh Ganu, Sumit Garg, Tom Rini Hi Michal, On Wed, 9 Oct 2024 at 07:21, Michal Simek <michal.simek@amd.com> wrote: > > Hi, > > On 10/9/24 03:55, Simon Glass wrote: > > Hi Michal, > > > > On Mon, 7 Oct 2024 at 07:05, Michal Simek <michal.simek@amd.com> wrote: > >> > >> Adding binman node with target images description can be unwanted feature > >> but as of today there is no way to disable it. > >> Also on size constrained systems it is not useful to add binman description > >> to DTB. > >> Introduce BINMAN_EXTERNAL_DTB Kconfig symbol which allows separate DTB for > >> target from DTB for binman itself. > >> > >> Signed-off-by: Michal Simek <michal.simek@amd.com> > >> --- > >> > >> Makefile | 2 +- > >> lib/Kconfig | 10 ++++++++++ > >> 2 files changed, 11 insertions(+), 1 deletion(-) > >> > > > > Doesn't this defeat one of the purposes of Binman, i.e. to document > > images? We do want the .dts to include the image description. What > > sort of problem is this causing? > > We have two boot flows. > The first one (default one) is using Xilinx FSBL for SOM initialization with fit > image (DTBS) + u-boot.elf + tfa. > > The second one is using U-Boot SPL instead of FSBL. This flow is used by > buildroot for example. > > In perfect world I should describe both of these flows. I sent description for > the second as RFC here. > https://lore.kernel.org/r/de1b8dbabd5ab7f20d7aac217ec4f5074d39f1da.1728462767.git.michal.simek@amd.com OK I'll take a look. > > but it is also reasonable to describe the first flow but I really don't want > both descriptions ends up in the target image. Why not? Knowing what is in the firmware is one of the goals of Binman. > > The second part is if you look at RFC and how fit-dtb.blob is composed. It is > one DTB + DTBS which are composed from overlays. > > xilinx_zynqmp_kria_defconfig has > CONFIG_DEFAULT_DEVICE_TREE="zynqmp-smk-k26-revA" > > That's why binman node should go to this DTB but because other images are > composed with overlays binman node is spread to all DTBs inside FIT image. > > It means one binman description is in fit-dtb.blob 14 times which is far from > ideal. Yes, but I think what you are saying is that U-Boot doesn't need the description, so you don't need it to appear in the dtbs in the FIT. Is that right? If so, then I think we should add a way to remove it, in Binman, perhaps with a property in the top-level binman image. > > Third part is that I can't see binman node in DT schema or bindings that's why I > expect this will be reported and I can't see any code which removes it before > handing off to OS which is required for System Ready IR. > And IIRC removing is also problematic for measured boot. I did start this e.g. [1] but have not got back to it. Help would be appreciated if it is important to you. I am not sure about System Ready IR, but we shouldn't need to remove this. Also, please add an fdtmap somewhere so the image can be listed. Regards, Simon [1] Documentation/devicetree/bindings/mtd/partitions/binman.yaml ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] binman: Add option for pointing to external description 2024-10-09 21:14 ` Simon Glass @ 2024-10-10 13:03 ` Michal Simek 2024-10-15 12:48 ` Simon Glass 0 siblings, 1 reply; 12+ messages in thread From: Michal Simek @ 2024-10-10 13:03 UTC (permalink / raw) To: Simon Glass Cc: u-boot, git, AKASHI Takahiro, Andrew Davis, Bryan Brattlof, Heinrich Schuchardt, Ilias Apalodimas, Leon M. Busch-George, Rasmus Villemoes, Sean Anderson, Sughosh Ganu, Sumit Garg, Tom Rini On 10/9/24 23:14, Simon Glass wrote: > Hi Michal, > > On Wed, 9 Oct 2024 at 07:21, Michal Simek <michal.simek@amd.com> wrote: >> >> Hi, >> >> On 10/9/24 03:55, Simon Glass wrote: >>> Hi Michal, >>> >>> On Mon, 7 Oct 2024 at 07:05, Michal Simek <michal.simek@amd.com> wrote: >>>> >>>> Adding binman node with target images description can be unwanted feature >>>> but as of today there is no way to disable it. >>>> Also on size constrained systems it is not useful to add binman description >>>> to DTB. >>>> Introduce BINMAN_EXTERNAL_DTB Kconfig symbol which allows separate DTB for >>>> target from DTB for binman itself. >>>> >>>> Signed-off-by: Michal Simek <michal.simek@amd.com> >>>> --- >>>> >>>> Makefile | 2 +- >>>> lib/Kconfig | 10 ++++++++++ >>>> 2 files changed, 11 insertions(+), 1 deletion(-) >>>> >>> >>> Doesn't this defeat one of the purposes of Binman, i.e. to document >>> images? We do want the .dts to include the image description. What >>> sort of problem is this causing? >> >> We have two boot flows. >> The first one (default one) is using Xilinx FSBL for SOM initialization with fit >> image (DTBS) + u-boot.elf + tfa. >> >> The second one is using U-Boot SPL instead of FSBL. This flow is used by >> buildroot for example. >> >> In perfect world I should describe both of these flows. I sent description for >> the second as RFC here. >> https://lore.kernel.org/r/de1b8dbabd5ab7f20d7aac217ec4f5074d39f1da.1728462767.git.michal.simek@amd.com > > OK I'll take a look. > >> >> but it is also reasonable to describe the first flow but I really don't want >> both descriptions ends up in the target image. > > Why not? Knowing what is in the firmware is one of the goals of Binman. If this is single binary composition with clear layout then likely fine. In our case where we target evaluation boards which can boot out of different boot devices it will be more confusing. For these I want to generated all images also for testing purpose not only images which you will burn to qspi. >> >> The second part is if you look at RFC and how fit-dtb.blob is composed. It is >> one DTB + DTBS which are composed from overlays. >> >> xilinx_zynqmp_kria_defconfig has >> CONFIG_DEFAULT_DEVICE_TREE="zynqmp-smk-k26-revA" >> >> That's why binman node should go to this DTB but because other images are >> composed with overlays binman node is spread to all DTBs inside FIT image. >> >> It means one binman description is in fit-dtb.blob 14 times which is far from >> ideal. > > Yes, but I think what you are saying is that U-Boot doesn't need the > description, so you don't need it to appear in the dtbs in the FIT. Is > that right? Yes. I know that there is a code around it but as of now I don't want to use any of this feature. > If so, then I think we should add a way to remove it, in Binman, > perhaps with a property in the top-level binman image. Works for me but keep in your mind that for SOM this should be removed from all combinations and for me it is easier not to add that description there instead of adding it and removing it. >> >> Third part is that I can't see binman node in DT schema or bindings that's why I >> expect this will be reported and I can't see any code which removes it before >> handing off to OS which is required for System Ready IR. >> And IIRC removing is also problematic for measured boot. > > I did start this e.g. [1] but have not got back to it. Help would be > appreciated if it is important to you. I am not sure about System > Ready IR, but we shouldn't need to remove this. I have seen this description but this target only boot images inside MTD. SR IR requires DT to pass dt-schema. It means make no sense to add binman node to DT if this is going to be reported. > Also, please add an fdtmap somewhere so the image can be listed. Thanks for reminder. I remember this option and will explore to see what it does. Thanks, Michal ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] binman: Add option for pointing to external description 2024-10-10 13:03 ` Michal Simek @ 2024-10-15 12:48 ` Simon Glass 2024-10-16 5:59 ` Michal Simek 0 siblings, 1 reply; 12+ messages in thread From: Simon Glass @ 2024-10-15 12:48 UTC (permalink / raw) To: Michal Simek Cc: u-boot, git, AKASHI Takahiro, Andrew Davis, Bryan Brattlof, Heinrich Schuchardt, Ilias Apalodimas, Leon M. Busch-George, Rasmus Villemoes, Sean Anderson, Sughosh Ganu, Sumit Garg, Tom Rini Hi Michal, On Thu, 10 Oct 2024 at 07:03, Michal Simek <michal.simek@amd.com> wrote: > > > > On 10/9/24 23:14, Simon Glass wrote: > > Hi Michal, > > > > On Wed, 9 Oct 2024 at 07:21, Michal Simek <michal.simek@amd.com> wrote: > >> > >> Hi, > >> > >> On 10/9/24 03:55, Simon Glass wrote: > >>> Hi Michal, > >>> > >>> On Mon, 7 Oct 2024 at 07:05, Michal Simek <michal.simek@amd.com> wrote: > >>>> > >>>> Adding binman node with target images description can be unwanted feature > >>>> but as of today there is no way to disable it. > >>>> Also on size constrained systems it is not useful to add binman description > >>>> to DTB. > >>>> Introduce BINMAN_EXTERNAL_DTB Kconfig symbol which allows separate DTB for > >>>> target from DTB for binman itself. > >>>> > >>>> Signed-off-by: Michal Simek <michal.simek@amd.com> > >>>> --- > >>>> > >>>> Makefile | 2 +- > >>>> lib/Kconfig | 10 ++++++++++ > >>>> 2 files changed, 11 insertions(+), 1 deletion(-) > >>>> > >>> > >>> Doesn't this defeat one of the purposes of Binman, i.e. to document > >>> images? We do want the .dts to include the image description. What > >>> sort of problem is this causing? > >> > >> We have two boot flows. > >> The first one (default one) is using Xilinx FSBL for SOM initialization with fit > >> image (DTBS) + u-boot.elf + tfa. > >> > >> The second one is using U-Boot SPL instead of FSBL. This flow is used by > >> buildroot for example. > >> > >> In perfect world I should describe both of these flows. I sent description for > >> the second as RFC here. > >> https://lore.kernel.org/r/de1b8dbabd5ab7f20d7aac217ec4f5074d39f1da.1728462767.git.michal.simek@amd.com > > > > OK I'll take a look. > > > >> > >> but it is also reasonable to describe the first flow but I really don't want > >> both descriptions ends up in the target image. > > > > Why not? Knowing what is in the firmware is one of the goals of Binman. > > If this is single binary composition with clear layout then likely fine. > In our case where we target evaluation boards which can boot out of different > boot devices it will be more confusing. > For these I want to generated all images also for testing purpose not only > images which you will burn to qspi. > > >> > >> The second part is if you look at RFC and how fit-dtb.blob is composed. It is > >> one DTB + DTBS which are composed from overlays. > >> > >> xilinx_zynqmp_kria_defconfig has > >> CONFIG_DEFAULT_DEVICE_TREE="zynqmp-smk-k26-revA" > >> > >> That's why binman node should go to this DTB but because other images are > >> composed with overlays binman node is spread to all DTBs inside FIT image. > >> > >> It means one binman description is in fit-dtb.blob 14 times which is far from > >> ideal. > > > > Yes, but I think what you are saying is that U-Boot doesn't need the > > description, so you don't need it to appear in the dtbs in the FIT. Is > > that right? > > Yes. > I know that there is a code around it but as of now I don't want to use any of > this feature. > > > If so, then I think we should add a way to remove it, in Binman, > > perhaps with a property in the top-level binman image. > > Works for me but keep in your mind that for SOM this should be removed from all > combinations and for me it is easier not to add that description there instead > of adding it and removing it. OK, I think you are saying that the description is repeated in each .dtb since each is built by U-Boot's build system and then they are added to the FIT. But what is to stop people from not bothering to fill in the binman description in U-Boot? I worry that vendors will have instructions like 'build U-Boot with the in-tree devicetree, which has no binman node, but pass this option to use this other file (not in mainline, just our special vendor branch), just for Binman's use', Where do you plan to keep this other file? > > >> > >> Third part is that I can't see binman node in DT schema or bindings that's why I > >> expect this will be reported and I can't see any code which removes it before > >> handing off to OS which is required for System Ready IR. > >> And IIRC removing is also problematic for measured boot. > > > > I did start this e.g. [1] but have not got back to it. Help would be > > appreciated if it is important to you. I am not sure about System > > Ready IR, but we shouldn't need to remove this. > > I have seen this description but this target only boot images inside MTD. > > SR IR requires DT to pass dt-schema. It means make no sense to add binman node > to DT if this is going to be reported. We need to get the schema in, then. IMO that whole idea (or checking dt-schema) makes little sense, unless the schema includes firmware nodes. Is there somewhere that explains why this is useful? > > > Also, please add an fdtmap somewhere so the image can be listed. > > Thanks for reminder. I remember this option and will explore to see what it does. OK Regards, Simon ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] binman: Add option for pointing to external description 2024-10-15 12:48 ` Simon Glass @ 2024-10-16 5:59 ` Michal Simek 2024-10-17 17:54 ` Tom Rini 2024-10-17 23:23 ` Simon Glass 0 siblings, 2 replies; 12+ messages in thread From: Michal Simek @ 2024-10-16 5:59 UTC (permalink / raw) To: Simon Glass Cc: u-boot, git, AKASHI Takahiro, Andrew Davis, Bryan Brattlof, Heinrich Schuchardt, Ilias Apalodimas, Leon M. Busch-George, Rasmus Villemoes, Sean Anderson, Sughosh Ganu, Sumit Garg, Tom Rini Hi Simon, On 10/15/24 14:48, Simon Glass wrote: > Hi Michal, > > On Thu, 10 Oct 2024 at 07:03, Michal Simek <michal.simek@amd.com> wrote: >> >> >> >> On 10/9/24 23:14, Simon Glass wrote: >>> Hi Michal, >>> >>> On Wed, 9 Oct 2024 at 07:21, Michal Simek <michal.simek@amd.com> wrote: >>>> >>>> Hi, >>>> >>>> On 10/9/24 03:55, Simon Glass wrote: >>>>> Hi Michal, >>>>> >>>>> On Mon, 7 Oct 2024 at 07:05, Michal Simek <michal.simek@amd.com> wrote: >>>>>> >>>>>> Adding binman node with target images description can be unwanted feature >>>>>> but as of today there is no way to disable it. >>>>>> Also on size constrained systems it is not useful to add binman description >>>>>> to DTB. >>>>>> Introduce BINMAN_EXTERNAL_DTB Kconfig symbol which allows separate DTB for >>>>>> target from DTB for binman itself. >>>>>> >>>>>> Signed-off-by: Michal Simek <michal.simek@amd.com> >>>>>> --- >>>>>> >>>>>> Makefile | 2 +- >>>>>> lib/Kconfig | 10 ++++++++++ >>>>>> 2 files changed, 11 insertions(+), 1 deletion(-) >>>>>> >>>>> >>>>> Doesn't this defeat one of the purposes of Binman, i.e. to document >>>>> images? We do want the .dts to include the image description. What >>>>> sort of problem is this causing? >>>> >>>> We have two boot flows. >>>> The first one (default one) is using Xilinx FSBL for SOM initialization with fit >>>> image (DTBS) + u-boot.elf + tfa. >>>> >>>> The second one is using U-Boot SPL instead of FSBL. This flow is used by >>>> buildroot for example. >>>> >>>> In perfect world I should describe both of these flows. I sent description for >>>> the second as RFC here. >>>> https://lore.kernel.org/r/de1b8dbabd5ab7f20d7aac217ec4f5074d39f1da.1728462767.git.michal.simek@amd.com >>> >>> OK I'll take a look. >>> >>>> >>>> but it is also reasonable to describe the first flow but I really don't want >>>> both descriptions ends up in the target image. >>> >>> Why not? Knowing what is in the firmware is one of the goals of Binman. >> >> If this is single binary composition with clear layout then likely fine. >> In our case where we target evaluation boards which can boot out of different >> boot devices it will be more confusing. >> For these I want to generated all images also for testing purpose not only >> images which you will burn to qspi. >> >>>> >>>> The second part is if you look at RFC and how fit-dtb.blob is composed. It is >>>> one DTB + DTBS which are composed from overlays. >>>> >>>> xilinx_zynqmp_kria_defconfig has >>>> CONFIG_DEFAULT_DEVICE_TREE="zynqmp-smk-k26-revA" >>>> >>>> That's why binman node should go to this DTB but because other images are >>>> composed with overlays binman node is spread to all DTBs inside FIT image. >>>> >>>> It means one binman description is in fit-dtb.blob 14 times which is far from >>>> ideal. >>> >>> Yes, but I think what you are saying is that U-Boot doesn't need the >>> description, so you don't need it to appear in the dtbs in the FIT. Is >>> that right? >> >> Yes. >> I know that there is a code around it but as of now I don't want to use any of >> this feature. >> >>> If so, then I think we should add a way to remove it, in Binman, >>> perhaps with a property in the top-level binman image. >> >> Works for me but keep in your mind that for SOM this should be removed from all >> combinations and for me it is easier not to add that description there instead >> of adding it and removing it. > > OK, I think you are saying that the description is repeated in each > .dtb since each is built by U-Boot's build system and then they are > added to the FIT. yep > > But what is to stop people from not bothering to fill in the binman > description in U-Boot? I worry that vendors will have instructions > like 'build U-Boot with the in-tree devicetree, which has no binman > node, but pass this option to use this other file (not in mainline, > just our special vendor branch), just for Binman's use', > > Where do you plan to keep this other file? In u-boot repo of course. And all configurations which makes sense. And pretty much if vendors wants to hide it they can no matter of this patch. I understand your concern but vendors can do it today. Thanks, Michal ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] binman: Add option for pointing to external description 2024-10-16 5:59 ` Michal Simek @ 2024-10-17 17:54 ` Tom Rini 2024-10-18 5:57 ` Michal Simek 2024-10-17 23:23 ` Simon Glass 1 sibling, 1 reply; 12+ messages in thread From: Tom Rini @ 2024-10-17 17:54 UTC (permalink / raw) To: Michal Simek Cc: Simon Glass, u-boot, git, AKASHI Takahiro, Andrew Davis, Bryan Brattlof, Heinrich Schuchardt, Ilias Apalodimas, Leon M. Busch-George, Rasmus Villemoes, Sean Anderson, Sughosh Ganu, Sumit Garg [-- Attachment #1: Type: text/plain, Size: 4915 bytes --] On Wed, Oct 16, 2024 at 07:59:59AM +0200, Michal Simek wrote: > Hi Simon, > > On 10/15/24 14:48, Simon Glass wrote: > > Hi Michal, > > > > On Thu, 10 Oct 2024 at 07:03, Michal Simek <michal.simek@amd.com> wrote: > > > > > > > > > > > > On 10/9/24 23:14, Simon Glass wrote: > > > > Hi Michal, > > > > > > > > On Wed, 9 Oct 2024 at 07:21, Michal Simek <michal.simek@amd.com> wrote: > > > > > > > > > > Hi, > > > > > > > > > > On 10/9/24 03:55, Simon Glass wrote: > > > > > > Hi Michal, > > > > > > > > > > > > On Mon, 7 Oct 2024 at 07:05, Michal Simek <michal.simek@amd.com> wrote: > > > > > > > > > > > > > > Adding binman node with target images description can be unwanted feature > > > > > > > but as of today there is no way to disable it. > > > > > > > Also on size constrained systems it is not useful to add binman description > > > > > > > to DTB. > > > > > > > Introduce BINMAN_EXTERNAL_DTB Kconfig symbol which allows separate DTB for > > > > > > > target from DTB for binman itself. > > > > > > > > > > > > > > Signed-off-by: Michal Simek <michal.simek@amd.com> > > > > > > > --- > > > > > > > > > > > > > > Makefile | 2 +- > > > > > > > lib/Kconfig | 10 ++++++++++ > > > > > > > 2 files changed, 11 insertions(+), 1 deletion(-) > > > > > > > > > > > > > > > > > > > Doesn't this defeat one of the purposes of Binman, i.e. to document > > > > > > images? We do want the .dts to include the image description. What > > > > > > sort of problem is this causing? > > > > > > > > > > We have two boot flows. > > > > > The first one (default one) is using Xilinx FSBL for SOM initialization with fit > > > > > image (DTBS) + u-boot.elf + tfa. > > > > > > > > > > The second one is using U-Boot SPL instead of FSBL. This flow is used by > > > > > buildroot for example. > > > > > > > > > > In perfect world I should describe both of these flows. I sent description for > > > > > the second as RFC here. > > > > > https://lore.kernel.org/r/de1b8dbabd5ab7f20d7aac217ec4f5074d39f1da.1728462767.git.michal.simek@amd.com > > > > > > > > OK I'll take a look. > > > > > > > > > > > > > > but it is also reasonable to describe the first flow but I really don't want > > > > > both descriptions ends up in the target image. > > > > > > > > Why not? Knowing what is in the firmware is one of the goals of Binman. > > > > > > If this is single binary composition with clear layout then likely fine. > > > In our case where we target evaluation boards which can boot out of different > > > boot devices it will be more confusing. > > > For these I want to generated all images also for testing purpose not only > > > images which you will burn to qspi. > > > > > > > > > > > > > The second part is if you look at RFC and how fit-dtb.blob is composed. It is > > > > > one DTB + DTBS which are composed from overlays. > > > > > > > > > > xilinx_zynqmp_kria_defconfig has > > > > > CONFIG_DEFAULT_DEVICE_TREE="zynqmp-smk-k26-revA" > > > > > > > > > > That's why binman node should go to this DTB but because other images are > > > > > composed with overlays binman node is spread to all DTBs inside FIT image. > > > > > > > > > > It means one binman description is in fit-dtb.blob 14 times which is far from > > > > > ideal. > > > > > > > > Yes, but I think what you are saying is that U-Boot doesn't need the > > > > description, so you don't need it to appear in the dtbs in the FIT. Is > > > > that right? > > > > > > Yes. > > > I know that there is a code around it but as of now I don't want to use any of > > > this feature. > > > > > > > If so, then I think we should add a way to remove it, in Binman, > > > > perhaps with a property in the top-level binman image. > > > > > > Works for me but keep in your mind that for SOM this should be removed from all > > > combinations and for me it is easier not to add that description there instead > > > of adding it and removing it. > > > > OK, I think you are saying that the description is repeated in each > > .dtb since each is built by U-Boot's build system and then they are > > added to the FIT. > > yep > > > > > But what is to stop people from not bothering to fill in the binman > > description in U-Boot? I worry that vendors will have instructions > > like 'build U-Boot with the in-tree devicetree, which has no binman > > node, but pass this option to use this other file (not in mainline, > > just our special vendor branch), just for Binman's use', > > > > Where do you plan to keep this other file? > > In u-boot repo of course. And all configurations which makes sense. > And pretty much if vendors wants to hide it they can no matter of this patch. > I understand your concern but vendors can do it today. Will resolving this let us finally remove SPL_FIT_GENERATOR as well? -- Tom [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 659 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] binman: Add option for pointing to external description 2024-10-17 17:54 ` Tom Rini @ 2024-10-18 5:57 ` Michal Simek 0 siblings, 0 replies; 12+ messages in thread From: Michal Simek @ 2024-10-18 5:57 UTC (permalink / raw) To: Tom Rini Cc: Simon Glass, u-boot, git, AKASHI Takahiro, Andrew Davis, Bryan Brattlof, Heinrich Schuchardt, Ilias Apalodimas, Leon M. Busch-George, Rasmus Villemoes, Sean Anderson, Sughosh Ganu, Sumit Garg On 10/17/24 19:54, Tom Rini wrote: > On Wed, Oct 16, 2024 at 07:59:59AM +0200, Michal Simek wrote: >> Hi Simon, >> >> On 10/15/24 14:48, Simon Glass wrote: >>> Hi Michal, >>> >>> On Thu, 10 Oct 2024 at 07:03, Michal Simek <michal.simek@amd.com> wrote: >>>> >>>> >>>> >>>> On 10/9/24 23:14, Simon Glass wrote: >>>>> Hi Michal, >>>>> >>>>> On Wed, 9 Oct 2024 at 07:21, Michal Simek <michal.simek@amd.com> wrote: >>>>>> >>>>>> Hi, >>>>>> >>>>>> On 10/9/24 03:55, Simon Glass wrote: >>>>>>> Hi Michal, >>>>>>> >>>>>>> On Mon, 7 Oct 2024 at 07:05, Michal Simek <michal.simek@amd.com> wrote: >>>>>>>> >>>>>>>> Adding binman node with target images description can be unwanted feature >>>>>>>> but as of today there is no way to disable it. >>>>>>>> Also on size constrained systems it is not useful to add binman description >>>>>>>> to DTB. >>>>>>>> Introduce BINMAN_EXTERNAL_DTB Kconfig symbol which allows separate DTB for >>>>>>>> target from DTB for binman itself. >>>>>>>> >>>>>>>> Signed-off-by: Michal Simek <michal.simek@amd.com> >>>>>>>> --- >>>>>>>> >>>>>>>> Makefile | 2 +- >>>>>>>> lib/Kconfig | 10 ++++++++++ >>>>>>>> 2 files changed, 11 insertions(+), 1 deletion(-) >>>>>>>> >>>>>>> >>>>>>> Doesn't this defeat one of the purposes of Binman, i.e. to document >>>>>>> images? We do want the .dts to include the image description. What >>>>>>> sort of problem is this causing? >>>>>> >>>>>> We have two boot flows. >>>>>> The first one (default one) is using Xilinx FSBL for SOM initialization with fit >>>>>> image (DTBS) + u-boot.elf + tfa. >>>>>> >>>>>> The second one is using U-Boot SPL instead of FSBL. This flow is used by >>>>>> buildroot for example. >>>>>> >>>>>> In perfect world I should describe both of these flows. I sent description for >>>>>> the second as RFC here. >>>>>> https://lore.kernel.org/r/de1b8dbabd5ab7f20d7aac217ec4f5074d39f1da.1728462767.git.michal.simek@amd.com >>>>> >>>>> OK I'll take a look. >>>>> >>>>>> >>>>>> but it is also reasonable to describe the first flow but I really don't want >>>>>> both descriptions ends up in the target image. >>>>> >>>>> Why not? Knowing what is in the firmware is one of the goals of Binman. >>>> >>>> If this is single binary composition with clear layout then likely fine. >>>> In our case where we target evaluation boards which can boot out of different >>>> boot devices it will be more confusing. >>>> For these I want to generated all images also for testing purpose not only >>>> images which you will burn to qspi. >>>> >>>>>> >>>>>> The second part is if you look at RFC and how fit-dtb.blob is composed. It is >>>>>> one DTB + DTBS which are composed from overlays. >>>>>> >>>>>> xilinx_zynqmp_kria_defconfig has >>>>>> CONFIG_DEFAULT_DEVICE_TREE="zynqmp-smk-k26-revA" >>>>>> >>>>>> That's why binman node should go to this DTB but because other images are >>>>>> composed with overlays binman node is spread to all DTBs inside FIT image. >>>>>> >>>>>> It means one binman description is in fit-dtb.blob 14 times which is far from >>>>>> ideal. >>>>> >>>>> Yes, but I think what you are saying is that U-Boot doesn't need the >>>>> description, so you don't need it to appear in the dtbs in the FIT. Is >>>>> that right? >>>> >>>> Yes. >>>> I know that there is a code around it but as of now I don't want to use any of >>>> this feature. >>>> >>>>> If so, then I think we should add a way to remove it, in Binman, >>>>> perhaps with a property in the top-level binman image. >>>> >>>> Works for me but keep in your mind that for SOM this should be removed from all >>>> combinations and for me it is easier not to add that description there instead >>>> of adding it and removing it. >>> >>> OK, I think you are saying that the description is repeated in each >>> .dtb since each is built by U-Boot's build system and then they are >>> added to the FIT. >> >> yep >> >>> >>> But what is to stop people from not bothering to fill in the binman >>> description in U-Boot? I worry that vendors will have instructions >>> like 'build U-Boot with the in-tree devicetree, which has no binman >>> node, but pass this option to use this other file (not in mainline, >>> just our special vendor branch), just for Binman's use', >>> >>> Where do you plan to keep this other file? >> >> In u-boot repo of course. And all configurations which makes sense. >> And pretty much if vendors wants to hide it they can no matter of this patch. >> I understand your concern but vendors can do it today. > > Will resolving this let us finally remove SPL_FIT_GENERATOR as well? Yes, that's the plan. I have that patch prepared but we need to resolve this first. Thanks, Michal ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] binman: Add option for pointing to external description 2024-10-16 5:59 ` Michal Simek 2024-10-17 17:54 ` Tom Rini @ 2024-10-17 23:23 ` Simon Glass 2024-10-18 6:01 ` Michal Simek 1 sibling, 1 reply; 12+ messages in thread From: Simon Glass @ 2024-10-17 23:23 UTC (permalink / raw) To: Michal Simek Cc: u-boot, git, AKASHI Takahiro, Andrew Davis, Bryan Brattlof, Heinrich Schuchardt, Ilias Apalodimas, Leon M. Busch-George, Rasmus Villemoes, Sean Anderson, Sughosh Ganu, Sumit Garg, Tom Rini Hi Michal, On Wed, 16 Oct 2024 at 00:00, Michal Simek <michal.simek@amd.com> wrote: > > Hi Simon, > > On 10/15/24 14:48, Simon Glass wrote: > > Hi Michal, > > > > On Thu, 10 Oct 2024 at 07:03, Michal Simek <michal.simek@amd.com> wrote: > >> > >> > >> > >> On 10/9/24 23:14, Simon Glass wrote: > >>> Hi Michal, > >>> > >>> On Wed, 9 Oct 2024 at 07:21, Michal Simek <michal.simek@amd.com> wrote: > >>>> > >>>> Hi, > >>>> > >>>> On 10/9/24 03:55, Simon Glass wrote: > >>>>> Hi Michal, > >>>>> > >>>>> On Mon, 7 Oct 2024 at 07:05, Michal Simek <michal.simek@amd.com> wrote: > >>>>>> > >>>>>> Adding binman node with target images description can be unwanted feature > >>>>>> but as of today there is no way to disable it. > >>>>>> Also on size constrained systems it is not useful to add binman description > >>>>>> to DTB. > >>>>>> Introduce BINMAN_EXTERNAL_DTB Kconfig symbol which allows separate DTB for > >>>>>> target from DTB for binman itself. > >>>>>> > >>>>>> Signed-off-by: Michal Simek <michal.simek@amd.com> > >>>>>> --- > >>>>>> > >>>>>> Makefile | 2 +- > >>>>>> lib/Kconfig | 10 ++++++++++ > >>>>>> 2 files changed, 11 insertions(+), 1 deletion(-) > >>>>>> > >>>>> > >>>>> Doesn't this defeat one of the purposes of Binman, i.e. to document > >>>>> images? We do want the .dts to include the image description. What > >>>>> sort of problem is this causing? > >>>> > >>>> We have two boot flows. > >>>> The first one (default one) is using Xilinx FSBL for SOM initialization with fit > >>>> image (DTBS) + u-boot.elf + tfa. > >>>> > >>>> The second one is using U-Boot SPL instead of FSBL. This flow is used by > >>>> buildroot for example. > >>>> > >>>> In perfect world I should describe both of these flows. I sent description for > >>>> the second as RFC here. > >>>> https://lore.kernel.org/r/de1b8dbabd5ab7f20d7aac217ec4f5074d39f1da.1728462767.git.michal.simek@amd.com > >>> > >>> OK I'll take a look. > >>> > >>>> > >>>> but it is also reasonable to describe the first flow but I really don't want > >>>> both descriptions ends up in the target image. > >>> > >>> Why not? Knowing what is in the firmware is one of the goals of Binman. > >> > >> If this is single binary composition with clear layout then likely fine. > >> In our case where we target evaluation boards which can boot out of different > >> boot devices it will be more confusing. > >> For these I want to generated all images also for testing purpose not only > >> images which you will burn to qspi. > >> > >>>> > >>>> The second part is if you look at RFC and how fit-dtb.blob is composed. It is > >>>> one DTB + DTBS which are composed from overlays. > >>>> > >>>> xilinx_zynqmp_kria_defconfig has > >>>> CONFIG_DEFAULT_DEVICE_TREE="zynqmp-smk-k26-revA" > >>>> > >>>> That's why binman node should go to this DTB but because other images are > >>>> composed with overlays binman node is spread to all DTBs inside FIT image. > >>>> > >>>> It means one binman description is in fit-dtb.blob 14 times which is far from > >>>> ideal. > >>> > >>> Yes, but I think what you are saying is that U-Boot doesn't need the > >>> description, so you don't need it to appear in the dtbs in the FIT. Is > >>> that right? > >> > >> Yes. > >> I know that there is a code around it but as of now I don't want to use any of > >> this feature. > >> > >>> If so, then I think we should add a way to remove it, in Binman, > >>> perhaps with a property in the top-level binman image. > >> > >> Works for me but keep in your mind that for SOM this should be removed from all > >> combinations and for me it is easier not to add that description there instead > >> of adding it and removing it. > > > > OK, I think you are saying that the description is repeated in each > > .dtb since each is built by U-Boot's build system and then they are > > added to the FIT. > > yep OK, got it. I think we should add an way to make the binman node optional. > > > > > But what is to stop people from not bothering to fill in the binman > > description in U-Boot? I worry that vendors will have instructions > > like 'build U-Boot with the in-tree devicetree, which has no binman > > node, but pass this option to use this other file (not in mainline, > > just our special vendor branch), just for Binman's use', > > > > Where do you plan to keep this other file? > > In u-boot repo of course. And all configurations which makes sense. > And pretty much if vendors wants to hide it they can no matter of this patch. > I understand your concern but vendors can do it today. So what value are you going to use for BINMAN_EXTERNAL_DTB ? Is there a patch for that? Perhaps it should be renamed, since it suggests that the file is out of tree. Regards, Simon ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] binman: Add option for pointing to external description 2024-10-17 23:23 ` Simon Glass @ 2024-10-18 6:01 ` Michal Simek 2024-10-18 14:59 ` Simon Glass 0 siblings, 1 reply; 12+ messages in thread From: Michal Simek @ 2024-10-18 6:01 UTC (permalink / raw) To: Simon Glass Cc: u-boot, git, AKASHI Takahiro, Andrew Davis, Bryan Brattlof, Heinrich Schuchardt, Ilias Apalodimas, Leon M. Busch-George, Rasmus Villemoes, Sean Anderson, Sughosh Ganu, Sumit Garg, Tom Rini Hi Simon, On 10/18/24 01:23, Simon Glass wrote: > Hi Michal, > > On Wed, 16 Oct 2024 at 00:00, Michal Simek <michal.simek@amd.com> wrote: >> >> Hi Simon, >> >> On 10/15/24 14:48, Simon Glass wrote: >>> Hi Michal, >>> >>> On Thu, 10 Oct 2024 at 07:03, Michal Simek <michal.simek@amd.com> wrote: >>>> >>>> >>>> >>>> On 10/9/24 23:14, Simon Glass wrote: >>>>> Hi Michal, >>>>> >>>>> On Wed, 9 Oct 2024 at 07:21, Michal Simek <michal.simek@amd.com> wrote: >>>>>> >>>>>> Hi, >>>>>> >>>>>> On 10/9/24 03:55, Simon Glass wrote: >>>>>>> Hi Michal, >>>>>>> >>>>>>> On Mon, 7 Oct 2024 at 07:05, Michal Simek <michal.simek@amd.com> wrote: >>>>>>>> >>>>>>>> Adding binman node with target images description can be unwanted feature >>>>>>>> but as of today there is no way to disable it. >>>>>>>> Also on size constrained systems it is not useful to add binman description >>>>>>>> to DTB. >>>>>>>> Introduce BINMAN_EXTERNAL_DTB Kconfig symbol which allows separate DTB for >>>>>>>> target from DTB for binman itself. >>>>>>>> >>>>>>>> Signed-off-by: Michal Simek <michal.simek@amd.com> >>>>>>>> --- >>>>>>>> >>>>>>>> Makefile | 2 +- >>>>>>>> lib/Kconfig | 10 ++++++++++ >>>>>>>> 2 files changed, 11 insertions(+), 1 deletion(-) >>>>>>>> >>>>>>> >>>>>>> Doesn't this defeat one of the purposes of Binman, i.e. to document >>>>>>> images? We do want the .dts to include the image description. What >>>>>>> sort of problem is this causing? >>>>>> >>>>>> We have two boot flows. >>>>>> The first one (default one) is using Xilinx FSBL for SOM initialization with fit >>>>>> image (DTBS) + u-boot.elf + tfa. >>>>>> >>>>>> The second one is using U-Boot SPL instead of FSBL. This flow is used by >>>>>> buildroot for example. >>>>>> >>>>>> In perfect world I should describe both of these flows. I sent description for >>>>>> the second as RFC here. >>>>>> https://lore.kernel.org/r/de1b8dbabd5ab7f20d7aac217ec4f5074d39f1da.1728462767.git.michal.simek@amd.com >>>>> >>>>> OK I'll take a look. >>>>> >>>>>> >>>>>> but it is also reasonable to describe the first flow but I really don't want >>>>>> both descriptions ends up in the target image. >>>>> >>>>> Why not? Knowing what is in the firmware is one of the goals of Binman. >>>> >>>> If this is single binary composition with clear layout then likely fine. >>>> In our case where we target evaluation boards which can boot out of different >>>> boot devices it will be more confusing. >>>> For these I want to generated all images also for testing purpose not only >>>> images which you will burn to qspi. >>>> >>>>>> >>>>>> The second part is if you look at RFC and how fit-dtb.blob is composed. It is >>>>>> one DTB + DTBS which are composed from overlays. >>>>>> >>>>>> xilinx_zynqmp_kria_defconfig has >>>>>> CONFIG_DEFAULT_DEVICE_TREE="zynqmp-smk-k26-revA" >>>>>> >>>>>> That's why binman node should go to this DTB but because other images are >>>>>> composed with overlays binman node is spread to all DTBs inside FIT image. >>>>>> >>>>>> It means one binman description is in fit-dtb.blob 14 times which is far from >>>>>> ideal. >>>>> >>>>> Yes, but I think what you are saying is that U-Boot doesn't need the >>>>> description, so you don't need it to appear in the dtbs in the FIT. Is >>>>> that right? >>>> >>>> Yes. >>>> I know that there is a code around it but as of now I don't want to use any of >>>> this feature. >>>> >>>>> If so, then I think we should add a way to remove it, in Binman, >>>>> perhaps with a property in the top-level binman image. >>>> >>>> Works for me but keep in your mind that for SOM this should be removed from all >>>> combinations and for me it is easier not to add that description there instead >>>> of adding it and removing it. >>> >>> OK, I think you are saying that the description is repeated in each >>> .dtb since each is built by U-Boot's build system and then they are >>> added to the FIT. >> >> yep > > OK, got it. I think we should add an way to make the binman node optional. I expect binman node is optional even today. No binman no means no image generation. Also I have one more use case where adding binman node can be misleading. With our FSBL boot flow only u-boot.elf is taken. If binman node in appended dtb is there people can think that bootimage was compose by binman but it doesn't need to be the case. That's why I want to have freedom and move decision about composing images to end users. >>> >>> But what is to stop people from not bothering to fill in the binman >>> description in U-Boot? I worry that vendors will have instructions >>> like 'build U-Boot with the in-tree devicetree, which has no binman >>> node, but pass this option to use this other file (not in mainline, >>> just our special vendor branch), just for Binman's use', >>> >>> Where do you plan to keep this other file? >> >> In u-boot repo of course. And all configurations which makes sense. >> And pretty much if vendors wants to hide it they can no matter of this patch. >> I understand your concern but vendors can do it today. > > So what value are you going to use for BINMAN_EXTERNAL_DTB ? Is there > a patch for that? Sorry I see that I didn't include defconfig change. In SOM case it should look like this. CONFIG_BINMAN_EXTERNAL_DTB="arch/arm/dts/zynqmp-som-binman.dtb" I had it as the part of SPL_FIT_GENERATOR removal. > Perhaps it should be renamed, since it suggests that > the file is out of tree. I am fine with renaming it. Do you have any suggestion? Thanks, Michal ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] binman: Add option for pointing to external description 2024-10-18 6:01 ` Michal Simek @ 2024-10-18 14:59 ` Simon Glass 0 siblings, 0 replies; 12+ messages in thread From: Simon Glass @ 2024-10-18 14:59 UTC (permalink / raw) To: Michal Simek Cc: u-boot, git, AKASHI Takahiro, Andrew Davis, Bryan Brattlof, Heinrich Schuchardt, Ilias Apalodimas, Leon M. Busch-George, Rasmus Villemoes, Sean Anderson, Sughosh Ganu, Sumit Garg, Tom Rini Hi Michal, On Fri, 18 Oct 2024 at 00:02, Michal Simek <michal.simek@amd.com> wrote: > > Hi Simon, > > On 10/18/24 01:23, Simon Glass wrote: > > Hi Michal, > > > > On Wed, 16 Oct 2024 at 00:00, Michal Simek <michal.simek@amd.com> wrote: > >> > >> Hi Simon, > >> > >> On 10/15/24 14:48, Simon Glass wrote: > >>> Hi Michal, > >>> > >>> On Thu, 10 Oct 2024 at 07:03, Michal Simek <michal.simek@amd.com> wrote: > >>>> > >>>> > >>>> > >>>> On 10/9/24 23:14, Simon Glass wrote: > >>>>> Hi Michal, > >>>>> > >>>>> On Wed, 9 Oct 2024 at 07:21, Michal Simek <michal.simek@amd.com> wrote: > >>>>>> > >>>>>> Hi, > >>>>>> > >>>>>> On 10/9/24 03:55, Simon Glass wrote: > >>>>>>> Hi Michal, > >>>>>>> > >>>>>>> On Mon, 7 Oct 2024 at 07:05, Michal Simek <michal.simek@amd.com> wrote: > >>>>>>>> > >>>>>>>> Adding binman node with target images description can be unwanted feature > >>>>>>>> but as of today there is no way to disable it. > >>>>>>>> Also on size constrained systems it is not useful to add binman description > >>>>>>>> to DTB. > >>>>>>>> Introduce BINMAN_EXTERNAL_DTB Kconfig symbol which allows separate DTB for > >>>>>>>> target from DTB for binman itself. > >>>>>>>> > >>>>>>>> Signed-off-by: Michal Simek <michal.simek@amd.com> > >>>>>>>> --- > >>>>>>>> > >>>>>>>> Makefile | 2 +- > >>>>>>>> lib/Kconfig | 10 ++++++++++ > >>>>>>>> 2 files changed, 11 insertions(+), 1 deletion(-) > >>>>>>>> > >>>>>>> > >>>>>>> Doesn't this defeat one of the purposes of Binman, i.e. to document > >>>>>>> images? We do want the .dts to include the image description. What > >>>>>>> sort of problem is this causing? > >>>>>> > >>>>>> We have two boot flows. > >>>>>> The first one (default one) is using Xilinx FSBL for SOM initialization with fit > >>>>>> image (DTBS) + u-boot.elf + tfa. > >>>>>> > >>>>>> The second one is using U-Boot SPL instead of FSBL. This flow is used by > >>>>>> buildroot for example. > >>>>>> > >>>>>> In perfect world I should describe both of these flows. I sent description for > >>>>>> the second as RFC here. > >>>>>> https://lore.kernel.org/r/de1b8dbabd5ab7f20d7aac217ec4f5074d39f1da.1728462767.git.michal.simek@amd.com > >>>>> > >>>>> OK I'll take a look. > >>>>> > >>>>>> > >>>>>> but it is also reasonable to describe the first flow but I really don't want > >>>>>> both descriptions ends up in the target image. > >>>>> > >>>>> Why not? Knowing what is in the firmware is one of the goals of Binman. > >>>> > >>>> If this is single binary composition with clear layout then likely fine. > >>>> In our case where we target evaluation boards which can boot out of different > >>>> boot devices it will be more confusing. > >>>> For these I want to generated all images also for testing purpose not only > >>>> images which you will burn to qspi. > >>>> > >>>>>> > >>>>>> The second part is if you look at RFC and how fit-dtb.blob is composed. It is > >>>>>> one DTB + DTBS which are composed from overlays. > >>>>>> > >>>>>> xilinx_zynqmp_kria_defconfig has > >>>>>> CONFIG_DEFAULT_DEVICE_TREE="zynqmp-smk-k26-revA" > >>>>>> > >>>>>> That's why binman node should go to this DTB but because other images are > >>>>>> composed with overlays binman node is spread to all DTBs inside FIT image. > >>>>>> > >>>>>> It means one binman description is in fit-dtb.blob 14 times which is far from > >>>>>> ideal. > >>>>> > >>>>> Yes, but I think what you are saying is that U-Boot doesn't need the > >>>>> description, so you don't need it to appear in the dtbs in the FIT. Is > >>>>> that right? > >>>> > >>>> Yes. > >>>> I know that there is a code around it but as of now I don't want to use any of > >>>> this feature. > >>>> > >>>>> If so, then I think we should add a way to remove it, in Binman, > >>>>> perhaps with a property in the top-level binman image. > >>>> > >>>> Works for me but keep in your mind that for SOM this should be removed from all > >>>> combinations and for me it is easier not to add that description there instead > >>>> of adding it and removing it. > >>> > >>> OK, I think you are saying that the description is repeated in each > >>> .dtb since each is built by U-Boot's build system and then they are > >>> added to the FIT. > >> > >> yep > > > > OK, got it. I think we should add an way to make the binman node optional. > > I expect binman node is optional even today. No binman no means no image generation. > > Also I have one more use case where adding binman node can be misleading. > With our FSBL boot flow only u-boot.elf is taken. If binman node in appended dtb > is there people can think that bootimage was compose by binman but it doesn't > need to be the case. That's why I want to have freedom and move decision about > composing images to end users. OK. > > > >>> > >>> But what is to stop people from not bothering to fill in the binman > >>> description in U-Boot? I worry that vendors will have instructions > >>> like 'build U-Boot with the in-tree devicetree, which has no binman > >>> node, but pass this option to use this other file (not in mainline, > >>> just our special vendor branch), just for Binman's use', > >>> > >>> Where do you plan to keep this other file? > >> > >> In u-boot repo of course. And all configurations which makes sense. > >> And pretty much if vendors wants to hide it they can no matter of this patch. > >> I understand your concern but vendors can do it today. > > > > So what value are you going to use for BINMAN_EXTERNAL_DTB ? Is there > > a patch for that? > > Sorry I see that I didn't include defconfig change. In SOM case it should look > like this. > > CONFIG_BINMAN_EXTERNAL_DTB="arch/arm/dts/zynqmp-som-binman.dtb" > > I had it as the part of SPL_FIT_GENERATOR removal. > > > Perhaps it should be renamed, since it suggests that > > the file is out of tree. > > I am fine with renaming it. Do you have any suggestion? Yes, just CONFIG_BINMAN_DTB seems OK to me. Please make sure that the path has a "./" prefix so that absolute paths are not allowed. Let's see how this goes in practice. The Makefile logic should be adjusted to put the CONFIG value into a separate variable, so that CONFIG_BINMAN_DTB can be empty and the Makefile then defaults it to u-boot.dtb I am thinking at some point we might want binman to accept both dtbs, but let's see... Regards, SImon ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2024-10-18 14:59 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-10-07 13:05 [PATCH] binman: Add option for pointing to external description Michal Simek 2024-10-09 1:55 ` Simon Glass 2024-10-09 13:21 ` Michal Simek 2024-10-09 21:14 ` Simon Glass 2024-10-10 13:03 ` Michal Simek 2024-10-15 12:48 ` Simon Glass 2024-10-16 5:59 ` Michal Simek 2024-10-17 17:54 ` Tom Rini 2024-10-18 5:57 ` Michal Simek 2024-10-17 23:23 ` Simon Glass 2024-10-18 6:01 ` Michal Simek 2024-10-18 14:59 ` Simon Glass
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.