Re: [U-Boot] [PATCH v4 8/9] spl: dm: Make it possible for the SPL to pick its own DTB from a FIT
Hi Jean-Jacques, On 12 September 2017 at 07:23, Jean-Jacques Hiblot wrote: > Hi Simon, > > It's has been a month since you've reviewed the series, I think it's time > that I send a v5. > I tried to address all your remarks but I stumbled on this one below: > > JJ > > On 13/08/2017 23:35, Simon Glass wrote: >> >> diff --git a/dts/Kconfig b/dts/Kconfig >>> >>> index c78438a..ec91a71 100644 >>> --- a/dts/Kconfig >>> +++ b/dts/Kconfig >>> @@ -118,6 +118,89 @@ config MULTI_DTB_FIT >>>the correct DTB to be used. Use this if you need to support >>>multiple DTBs but don't use the SPL. >>> >>> + >>> +config SPL_MULTI_DTB_FIT >>> + depends on SPL_LOAD_FIT && SPL_OF_CONTROL && !SPL_OF_PLATDATA >>> + bool "support embedding several DTBs in a FIT image for the SPL" >> >> Can you please capitalise the options in this file, so 'Bool'* > > I tried to do that, but it didn't work. Could it be a matter of the host > configuration ? That's me being silly, sorry. I mean: bool "Support ..." Regards, Simon ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v4 8/9] spl: dm: Make it possible for the SPL to pick its own DTB from a FIT
On Tue, Sep 12, 2017 at 03:23:38PM +0200, Jean-Jacques Hiblot wrote: > Hi Simon, > > It's has been a month since you've reviewed the series, I think it's > time that I send a v5. > I tried to address all your remarks but I stumbled on this one below: > > JJ > > On 13/08/2017 23:35, Simon Glass wrote: > >diff --git a/dts/Kconfig b/dts/Kconfig > >>index c78438a..ec91a71 100644 > >>--- a/dts/Kconfig > >>+++ b/dts/Kconfig > >>@@ -118,6 +118,89 @@ config MULTI_DTB_FIT > >> the correct DTB to be used. Use this if you need to support > >> multiple DTBs but don't use the SPL. > >> > >>+ > >>+config SPL_MULTI_DTB_FIT > >>+ depends on SPL_LOAD_FIT && SPL_OF_CONTROL && !SPL_OF_PLATDATA > >>+ bool "support embedding several DTBs in a FIT image for the SPL" > >Can you please capitalise the options in this file, so 'Bool'* > I tried to do that, but it didn't work. Could it be a matter of the > host configuration ? Er, it must be lower case config, depends, bool, help, etc. -- Tom signature.asc Description: Digital signature ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v4 8/9] spl: dm: Make it possible for the SPL to pick its own DTB from a FIT
Hi Simon, It's has been a month since you've reviewed the series, I think it's time that I send a v5. I tried to address all your remarks but I stumbled on this one below: JJ On 13/08/2017 23:35, Simon Glass wrote: diff --git a/dts/Kconfig b/dts/Kconfig index c78438a..ec91a71 100644 --- a/dts/Kconfig +++ b/dts/Kconfig @@ -118,6 +118,89 @@ config MULTI_DTB_FIT the correct DTB to be used. Use this if you need to support multiple DTBs but don't use the SPL. + +config SPL_MULTI_DTB_FIT + depends on SPL_LOAD_FIT && SPL_OF_CONTROL && !SPL_OF_PLATDATA + bool "support embedding several DTBs in a FIT image for the SPL" Can you please capitalise the options in this file, so 'Bool'* I tried to do that, but it didn't work. Could it be a matter of the host configuration ? + help + This option provides the SPL with the ability to select its own + DTB at runtime from an appended FIT image containing several DTBs. + This allows using the same SPL binary on multiple platforms. + The primary purpose is to handle different versions of + the same platform without tweaking the platform code if the + differences can be expressed in the DTBs (common examples are: bus + capabilities, pad configurations). + +config SPL_OF_LIST + string "List of device tree files to include for DT control in SPL" + depends on SPL_MULTI_DTB_FIT + default OF_LIST + help + This option specifies a list of device tree files to use for DT + control in the SPL. These will be packaged into a FIT. At run-time, + the SPL will select the correct DT to use by examining the + hardware (e.g. reading a board ID value). This is a list of + device tree files (without the directory or .dtb suffix) + separated by . + +choice + prompt "SPL OF LIST compression" + depends on SPL_MULTI_DTB_FIT + default SPL_MULTI_DTB_FIT_LZO + +config SPL_MULTI_DTB_FIT_LZO + bool "LZO" + depends on SYS_MALLOC_F + select SPL_LZO + help + Compress the FIT image containing the DTBs available for the SPL + using LZO compression. (requires lzop on host). + +config SPL_MULTI_DTB_FIT_GZ ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v4 8/9] spl: dm: Make it possible for the SPL to pick its own DTB from a FIT
Hi Jean-Jacques,
On 24 August 2017 at 04:52, Jean-Jacques Hiblot wrote:
> Hi Simon,
>
> On 13/08/2017 23:35, Simon Glass wrote:
>>
>> Hi Jean-Jacques,
>>
>> On 7 August 2017 at 04:07, Jean-Jacques Hiblot wrote:
>>>
>>> u-boot can be embedded within a FIT image with multiple DTBs. It then
>>> selects at run-time which one is best suited for the platform.
>>> Use the same principle here for the SPL: put the DTBs in a FIT image,
>>> compress it (LZO, GZIP, or no compression) and append it at the end of
>>> the
>>> SPL.
>>>
>>> Signed-off-by: Jean-Jacques Hiblot
>>> ---
>>>
>>> change since v3: updated the help in Kconfig to take in account commit
>>> f1896c
>>> ("spl: make SPL and normal u-boot stage use independent
>>> SYS_MALLOC_F_LEN")
>>>
>>> doc/README.multi-dtb-fit | 44 +++--
>>> dts/Kconfig | 83
>>> ++
>>> lib/fdtdec.c | 85
>>> +++-
>>> scripts/Makefile.spl | 35 +++-
>>> 4 files changed, 235 insertions(+), 12 deletions(-)
>>>
>> Unfortunately I have quite a few comments and a few nits below. I feel
>> this is an important feature and most of my comments relate to polish.
>> It's a nice implementation.
>>
> Thanks
[..]
>>> @@ -1243,7 +1292,25 @@ int fdtdec_setup(void)
>>> gd->fdt_blob = (void *)getenv_ulong("fdtcontroladdr", 16,
>>>
>>> (uintptr_t)gd->fdt_blob);
>>> # endif
>>> +
>>> +# if CONFIG_IS_ENABLED(MULTI_DTB_FIT)
>>> + /*
>>> +*try and uncompress the blob.
>>> +* max input size is set arbitrarily to 16MB (should more than
>>> enough)
>>
>> This seems ugly. Can you call fdt_totalsize() instead?
>
> Unfortunately we cannot. The data itself is not part of the fdt, it is
> appended after it. In the fdt, we find the descriptions, and for each binary
> the size and the offset relative to the end of the fdt.
>
So I think we need header so we can find out:
- the compression algorithm (since otherwise I'm not sure how you tell)
- the compressed size
-the original uncompressed size
without having to decompress to find out. Perhaps add a magic value also?
> JJ
[..]
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v4 8/9] spl: dm: Make it possible for the SPL to pick its own DTB from a FIT
Hi Simon,
On 13/08/2017 23:35, Simon Glass wrote:
Hi Jean-Jacques,
On 7 August 2017 at 04:07, Jean-Jacques Hiblot wrote:
u-boot can be embedded within a FIT image with multiple DTBs. It then
selects at run-time which one is best suited for the platform.
Use the same principle here for the SPL: put the DTBs in a FIT image,
compress it (LZO, GZIP, or no compression) and append it at the end of the
SPL.
Signed-off-by: Jean-Jacques Hiblot
---
change since v3: updated the help in Kconfig to take in account commit f1896c
("spl: make SPL and normal u-boot stage use independent SYS_MALLOC_F_LEN")
doc/README.multi-dtb-fit | 44 +++--
dts/Kconfig | 83 ++
lib/fdtdec.c | 85 +++-
scripts/Makefile.spl | 35 +++-
4 files changed, 235 insertions(+), 12 deletions(-)
Unfortunately I have quite a few comments and a few nits below. I feel
this is an important feature and most of my comments relate to polish.
It's a nice implementation.
Thanks
diff --git a/doc/README.multi-dtb-fit b/doc/README.multi-dtb-fit
index d182d4e..5e593c8 100644
--- a/doc/README.multi-dtb-fit
+++ b/doc/README.multi-dtb-fit
@@ -1,9 +1,49 @@
MULTI DTB FIT
-The purpose of this feature is to enable u-boot to select its DTB from a FIT
-image appended at the end of the binary.
+The purpose of this feature is to enable u-boot or the SPL to select its DTB
+from a FIT image appended at the end of the binary.
Since FIT means Flat Image Tree (I think) we should probably write
'FIT' everywhere instead of 'FIT image'.
+It comes in two flavor: u-boot (CONFIG_MULTI_DTB_FIT) and SPL
+(CONFIG_SPL_MULTI_DTB_FIT)
+u-boot flavor:
U-Boot (please fix throughout)
Usually the DTB is selected by the SPL and passed down to u-boot. But some
platforms don't use the SPL. In this case MULTI_DTB_FIT can used to provide
u-boot with a choice of DTBs. The FIT image is automatically image at the end
of the compilation and appended to u-boot.bin
'automatically image' - does that mean automatically built? Or
(looking below) generated?
+
+
+
+SPL flavor:
+the SPL uses only a small subset of the DTB and it usually depends more
+on the SOC than on the board. So it's usually fine to include a DTB in the
+SPL that doesn't exactly match the board. There are howerver somes cases
however some
+where it's not possible. In the later case, in order to support multiple
+boards (or board revisions) with the same SPL binary, SPL_MULTI_DTB_FIT
+can be used. The list of DTB is given in CONFIG_SPL_OF_LIST which by default
+is the same list as CONFIG_OF_LIST
The DTB files are packed into a FIT
+The FIT image is automatically generated at the end of the compilation,
+compressed and appended to u-boot-spl.bin
so that SPL can locate it and select the correct DTB from inside the FIT.
I think this needs expanding a bit:
- board_fit_config_name_match() is used to find the correct DTB within the FIT
- mention an example board that uses this feature (so people can trace the code)
- how memory is allocated for decompression
- mention compression - it is automatic in the build and SPL
decompresses the correct DTB
+
+The impact of this option is relatively small. Here are some numbers measured
+for a TI DRA7 platform:
+
+ ++
+ | Size|delta |boot-time| delta |
+ | (bytes) |(bytes) |(ms) | (ms) |
++---+
+| reference| 120185 | | 1331 ||
++---+
+| feature | | | ||
+| deactiVated | 120185 | 0 | 1330 | -1|
++---+
+| 1 DTB LZO | 120208 | 23 | 1331 | 0 |
++---+
+| 4 DTB LZO | 120810 | 625| 1336 | 5 |
++---+
+| 4 DTB LZO | | | ||
+| no malloc| 120746 | 561| 1343 | 12|
++---+
+| 4 DTB GZIP | 128552 | 8367 | 1353 | 22|
++---+
+| 4 DTB No comp | 132352 | 12167 | 1351 | 20|
++--+--+--+-++
diff --git a/dts/Kconfig b/dts/Kconfig
index c78438a..ec91a71 100644
--- a/dts/Kconfig
+++ b/dts/Kconfig
@@ -118,6 +118,89 @@ config MULTI_DTB_FIT
the correct DTB to be used. Use this if you need to support
multiple DTBs but don't use the SPL.
+
+config SPL
Re: [U-Boot] [PATCH v4 8/9] spl: dm: Make it possible for the SPL to pick its own DTB from a FIT
Hi Jean-Jacques,
On 7 August 2017 at 04:07, Jean-Jacques Hiblot wrote:
> u-boot can be embedded within a FIT image with multiple DTBs. It then
> selects at run-time which one is best suited for the platform.
> Use the same principle here for the SPL: put the DTBs in a FIT image,
> compress it (LZO, GZIP, or no compression) and append it at the end of the
> SPL.
>
> Signed-off-by: Jean-Jacques Hiblot
> ---
>
> change since v3: updated the help in Kconfig to take in account commit f1896c
> ("spl: make SPL and normal u-boot stage use independent SYS_MALLOC_F_LEN")
>
> doc/README.multi-dtb-fit | 44 +++--
> dts/Kconfig | 83 ++
> lib/fdtdec.c | 85
> +++-
> scripts/Makefile.spl | 35 +++-
> 4 files changed, 235 insertions(+), 12 deletions(-)
>
Unfortunately I have quite a few comments and a few nits below. I feel
this is an important feature and most of my comments relate to polish.
It's a nice implementation.
> diff --git a/doc/README.multi-dtb-fit b/doc/README.multi-dtb-fit
> index d182d4e..5e593c8 100644
> --- a/doc/README.multi-dtb-fit
> +++ b/doc/README.multi-dtb-fit
> @@ -1,9 +1,49 @@
> MULTI DTB FIT
>
> -The purpose of this feature is to enable u-boot to select its DTB from a FIT
> -image appended at the end of the binary.
> +The purpose of this feature is to enable u-boot or the SPL to select its DTB
> +from a FIT image appended at the end of the binary.
Since FIT means Flat Image Tree (I think) we should probably write
'FIT' everywhere instead of 'FIT image'.
> +It comes in two flavor: u-boot (CONFIG_MULTI_DTB_FIT) and SPL
> +(CONFIG_SPL_MULTI_DTB_FIT)
>
> +u-boot flavor:
U-Boot (please fix throughout)
> Usually the DTB is selected by the SPL and passed down to u-boot. But some
> platforms don't use the SPL. In this case MULTI_DTB_FIT can used to provide
> u-boot with a choice of DTBs. The FIT image is automatically image at the end
> of the compilation and appended to u-boot.bin
'automatically image' - does that mean automatically built? Or
(looking below) generated?
> +
> +
> +
> +SPL flavor:
> +the SPL uses only a small subset of the DTB and it usually depends more
> +on the SOC than on the board. So it's usually fine to include a DTB in the
> +SPL that doesn't exactly match the board. There are howerver somes cases
however some
> +where it's not possible. In the later case, in order to support multiple
> +boards (or board revisions) with the same SPL binary, SPL_MULTI_DTB_FIT
> +can be used. The list of DTB is given in CONFIG_SPL_OF_LIST which by default
> +is the same list as CONFIG_OF_LIST
The DTB files are packed into a FIT
> +The FIT image is automatically generated at the end of the compilation,
> +compressed and appended to u-boot-spl.bin
so that SPL can locate it and select the correct DTB from inside the FIT.
I think this needs expanding a bit:
- board_fit_config_name_match() is used to find the correct DTB within the FIT
- mention an example board that uses this feature (so people can trace the code)
- how memory is allocated for decompression
- mention compression - it is automatic in the build and SPL
decompresses the correct DTB
> +
> +The impact of this option is relatively small. Here are some numbers measured
> +for a TI DRA7 platform:
> +
> + ++
> + | Size|delta |boot-time| delta |
> + | (bytes) |(bytes) |(ms) | (ms) |
> ++---+
> +| reference| 120185 | | 1331 ||
> ++---+
> +| feature | | | ||
> +| deactiVated | 120185 | 0 | 1330 | -1|
> ++---+
> +| 1 DTB LZO | 120208 | 23 | 1331 | 0 |
> ++---+
> +| 4 DTB LZO | 120810 | 625| 1336 | 5 |
> ++---+
> +| 4 DTB LZO | | | ||
> +| no malloc| 120746 | 561| 1343 | 12|
> ++---+
> +| 4 DTB GZIP | 128552 | 8367 | 1353 | 22|
> ++---+
> +| 4 DTB No comp | 132352 | 12167 | 1351 | 20|
> ++--+--+--+-++
> diff --git a/dts/Kconfig b/dts/Kconfig
> index c78438a..ec91a71 100644
> --- a/dts/Kconfig
> +++ b/dts/Kconfig
> @@ -118,6 +118,89 @@ config MULTI_DTB_FIT
> the correct DTB to be used. Use this if you ne
Re: [U-Boot] [PATCH v4 8/9] spl: dm: Make it possible for the SPL to pick its own DTB from a FIT
On Mon, Aug 07, 2017 at 12:07:53PM +0200, Jean-Jacques Hiblot wrote: > u-boot can be embedded within a FIT image with multiple DTBs. It then > selects at run-time which one is best suited for the platform. > Use the same principle here for the SPL: put the DTBs in a FIT image, > compress it (LZO, GZIP, or no compression) and append it at the end of the > SPL. > > Signed-off-by: Jean-Jacques Hiblot Reviewed-by: Tom Rini -- Tom signature.asc Description: Digital signature ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v4 8/9] spl: dm: Make it possible for the SPL to pick its own DTB from a FIT
u-boot can be embedded within a FIT image with multiple DTBs. It then
selects at run-time which one is best suited for the platform.
Use the same principle here for the SPL: put the DTBs in a FIT image,
compress it (LZO, GZIP, or no compression) and append it at the end of the
SPL.
Signed-off-by: Jean-Jacques Hiblot
---
change since v3: updated the help in Kconfig to take in account commit f1896c
("spl: make SPL and normal u-boot stage use independent SYS_MALLOC_F_LEN")
doc/README.multi-dtb-fit | 44 +++--
dts/Kconfig | 83 ++
lib/fdtdec.c | 85 +++-
scripts/Makefile.spl | 35 +++-
4 files changed, 235 insertions(+), 12 deletions(-)
diff --git a/doc/README.multi-dtb-fit b/doc/README.multi-dtb-fit
index d182d4e..5e593c8 100644
--- a/doc/README.multi-dtb-fit
+++ b/doc/README.multi-dtb-fit
@@ -1,9 +1,49 @@
MULTI DTB FIT
-The purpose of this feature is to enable u-boot to select its DTB from a FIT
-image appended at the end of the binary.
+The purpose of this feature is to enable u-boot or the SPL to select its DTB
+from a FIT image appended at the end of the binary.
+It comes in two flavor: u-boot (CONFIG_MULTI_DTB_FIT) and SPL
+(CONFIG_SPL_MULTI_DTB_FIT)
+u-boot flavor:
Usually the DTB is selected by the SPL and passed down to u-boot. But some
platforms don't use the SPL. In this case MULTI_DTB_FIT can used to provide
u-boot with a choice of DTBs. The FIT image is automatically image at the end
of the compilation and appended to u-boot.bin
+
+
+
+SPL flavor:
+the SPL uses only a small subset of the DTB and it usually depends more
+on the SOC than on the board. So it's usually fine to include a DTB in the
+SPL that doesn't exactly match the board. There are howerver somes cases
+where it's not possible. In the later case, in order to support multiple
+boards (or board revisions) with the same SPL binary, SPL_MULTI_DTB_FIT
+can be used. The list of DTB is given in CONFIG_SPL_OF_LIST which by default
+is the same list as CONFIG_OF_LIST
+The FIT image is automatically generated at the end of the compilation,
+compressed and appended to u-boot-spl.bin
+
+The impact of this option is relatively small. Here are some numbers measured
+for a TI DRA7 platform:
+
+ ++
+ | Size|delta |boot-time| delta |
+ | (bytes) |(bytes) |(ms) | (ms) |
++---+
+| reference| 120185 | | 1331 ||
++---+
+| feature | | | ||
+| deactiVated | 120185 | 0 | 1330 | -1|
++---+
+| 1 DTB LZO | 120208 | 23 | 1331 | 0 |
++---+
+| 4 DTB LZO | 120810 | 625| 1336 | 5 |
++---+
+| 4 DTB LZO | | | ||
+| no malloc| 120746 | 561| 1343 | 12|
++---+
+| 4 DTB GZIP | 128552 | 8367 | 1353 | 22|
++---+
+| 4 DTB No comp | 132352 | 12167 | 1351 | 20|
++--+--+--+-++
diff --git a/dts/Kconfig b/dts/Kconfig
index c78438a..ec91a71 100644
--- a/dts/Kconfig
+++ b/dts/Kconfig
@@ -118,6 +118,89 @@ config MULTI_DTB_FIT
the correct DTB to be used. Use this if you need to support
multiple DTBs but don't use the SPL.
+
+config SPL_MULTI_DTB_FIT
+ depends on SPL_LOAD_FIT && SPL_OF_CONTROL && !SPL_OF_PLATDATA
+ bool "support embedding several DTBs in a FIT image for the SPL"
+ help
+ This option provides the SPL with the ability to select its own
+ DTB at runtime from an appended FIT image containing several DTBs.
+ This allows using the same SPL binary on multiple platforms.
+ The primary purpose is to handle different versions of
+ the same platform without tweaking the platform code if the
+ differences can be expressed in the DTBs (common examples are: bus
+ capabilities, pad configurations).
+
+config SPL_OF_LIST
+ string "List of device tree files to include for DT control in SPL"
+ depends on SPL_MULTI_DTB_FIT
+ default OF_LIST
+ help
+ This option specifies a list of device tree files to use for DT
+ control in the SPL. These will be packaged into a FIT. At run-time,
+ the SPL will select the correct DT t
