mbox series

[u-boot-mvebu,00/31] kwboot / kwbimage improvements

Message ID 20210708173032.27999-1-marek.behun@nic.cz
Headers show
Series kwboot / kwbimage improvements | expand

Message

Marek Behún July 8, 2021, 5:30 p.m. UTC
Hi Stefan and others,

this is a series of improvements to kwboot, kwbimage and mvebu.

The main goal of this series is to correctly use BootROM's code
for loading U-Boot from NOR / NAND: currently only SPL is read by
BootROM and the main U-Boot is read by SPL. By using BootROM to also
load main U-Boot we can reduce the size of SPL image, since it does
not need to contain code for reading NOR / NAND.

Before merging, this series should be tested on as many relevant
boards as possible.

Marek & Pali

Marek Behún (2):
  tools: kwbimage: Add constant for SDIO bootfrom
  tools: kwbimage: Cosmetic fix - remove redundant space character

Pali Rohár (29):
  tools: kwbimage: Fix compilation without CONFIG_SYS_U_BOOT_OFFS
  tools: kwbimage: Simplify aligning and calculating checksum
  tools: kwbimage: Align SPI and NAND images to 256 bytes
  tools: kwbimage: Fix generation of SATA, SDIO and PCIe images
  tools: kwbimage: Don't crash when binary file name does not contain
    '/'
  tools: kwbimage: Fix check for v0 extended header checksum
  tools: kwbimage: Validate extended headers of v1 images
  tools: kwbimage: Validate data checksum of v1 images
  tools: kwbimage: Print size of binary header in
    kwbimage_print_header()
  tools: kwboot: Fix wrong parameter passed to read()
  tools: kwboot: Fix restoring terminal
  tools: kwboot: Print trailing newline after terminal is terminated
  tools: kwboot: Cosmetic fix - add missing curly brackets
  tools: kwboot: Check for v1 header size
  tools: kwbimage: Use -a parameter (load address) for v1 images
  arm: mvebu: Fix return_to_bootrom()
  arm: mvebu: Mark return_to_bootrom() as a noreturn function
  arm: mvebu: Implement return_to_bootrom() via U-Boot's SPL framework
  arm: mvebu: Use U-Boot's SPL BootROM framework for booting from
    NAND/UART
  arm: mvebu: Always use BootROM for loading the rest of U-Boot's binary
  arm: mvebu: gdsys: Remove custom spl_board_init()
  arm: mvebu: Remove legacy U-Boot header from kwbimage v1 files
  tools: kwbimage: Remove v1 kwbimage SPL padding to
    CONFIG_SYS_U_BOOT_OFFS bytes
  arm: mvebu: Remove unused macro CONFIG_SYS_U_BOOT_OFFS
  tools: kwbimage: Add support for more BINARY headers
  tools: kwbimage: Don't parse PAYLOAD keyword
  tools: kwbimage: Add support for DATA command also for v1 images
  tools: kwbimage: Add support for a new DATA_DELAY command
  tools: kwbimage: Do not hide usage of secure header under
    CONFIG_ARMADA_38X

 Makefile                               |   2 +-
 arch/arm/mach-mvebu/Kconfig            |  16 +-
 arch/arm/mach-mvebu/include/mach/cpu.h |   2 +-
 arch/arm/mach-mvebu/lowlevel_spl.S     |   3 +-
 arch/arm/mach-mvebu/spl.c              |  90 +------
 board/gdsys/a38x/Makefile              |   2 +-
 board/gdsys/a38x/spl.c                 |  20 --
 include/configs/clearfog.h             |   6 +-
 include/configs/controlcenterdc.h      |   8 +-
 include/configs/db-88f6720.h           |   3 -
 include/configs/db-88f6820-amc.h       |   5 -
 include/configs/db-88f6820-gp.h        |   6 -
 include/configs/db-mv784mp-gp.h        |   3 -
 include/configs/ds414.h                |   5 -
 include/configs/helios4.h              |   6 +-
 include/configs/theadorable.h          |   3 -
 include/configs/turris_omnia.h         |   6 -
 include/configs/x530.h                 |   3 -
 scripts/config_whitelist.txt           |   1 -
 tools/Makefile                         |   8 -
 tools/kwbimage.c                       | 339 +++++++++++++++++--------
 tools/kwbimage.h                       |  30 ++-
 tools/kwboot.c                         |  14 +-
 23 files changed, 296 insertions(+), 285 deletions(-)
 delete mode 100644 board/gdsys/a38x/spl.c

Comments

Stefan Roese July 9, 2021, 6:05 a.m. UTC | #1
Hi Marek & Pali,

On 08.07.21 19:30, Marek Behún wrote:
> Hi Stefan and others,
> 
> this is a series of improvements to kwboot, kwbimage and mvebu.
> 
> The main goal of this series is to correctly use BootROM's code
> for loading U-Boot from NOR / NAND: currently only SPL is read by
> BootROM and the main U-Boot is read by SPL. By using BootROM to also
> load main U-Boot we can reduce the size of SPL image, since it does
> not need to contain code for reading NOR / NAND.

Before going into a review of the patches, let me ask about the
motivation of this patchset. Is the reduction of the SPL image size the
main motivation for this series? Or did you experiece some problems
with the SPL code for U-Boot proper loading?

BTW: This patch / mail subject "kwboot / kwbimage improvements" does
not really match its content AFAIU. Here, the SPL returns always back
to the BootROM for U-Boot proper loading part is missing. Or do I
misunderstand something?

BTW2: Could you please list the affected MVEBU SoC's that are affected
by this series so that this is clear?

> Before merging, this series should be tested on as many relevant
> boards as possible.

I fully agree. I very much welcome any "Tested-by" tags and reviews
from others.

Thanks,
Stefan

> Marek & Pali
> 
> Marek Behún (2):
>    tools: kwbimage: Add constant for SDIO bootfrom
>    tools: kwbimage: Cosmetic fix - remove redundant space character
> 
> Pali Rohár (29):
>    tools: kwbimage: Fix compilation without CONFIG_SYS_U_BOOT_OFFS
>    tools: kwbimage: Simplify aligning and calculating checksum
>    tools: kwbimage: Align SPI and NAND images to 256 bytes
>    tools: kwbimage: Fix generation of SATA, SDIO and PCIe images
>    tools: kwbimage: Don't crash when binary file name does not contain
>      '/'
>    tools: kwbimage: Fix check for v0 extended header checksum
>    tools: kwbimage: Validate extended headers of v1 images
>    tools: kwbimage: Validate data checksum of v1 images
>    tools: kwbimage: Print size of binary header in
>      kwbimage_print_header()
>    tools: kwboot: Fix wrong parameter passed to read()
>    tools: kwboot: Fix restoring terminal
>    tools: kwboot: Print trailing newline after terminal is terminated
>    tools: kwboot: Cosmetic fix - add missing curly brackets
>    tools: kwboot: Check for v1 header size
>    tools: kwbimage: Use -a parameter (load address) for v1 images
>    arm: mvebu: Fix return_to_bootrom()
>    arm: mvebu: Mark return_to_bootrom() as a noreturn function
>    arm: mvebu: Implement return_to_bootrom() via U-Boot's SPL framework
>    arm: mvebu: Use U-Boot's SPL BootROM framework for booting from
>      NAND/UART
>    arm: mvebu: Always use BootROM for loading the rest of U-Boot's binary
>    arm: mvebu: gdsys: Remove custom spl_board_init()
>    arm: mvebu: Remove legacy U-Boot header from kwbimage v1 files
>    tools: kwbimage: Remove v1 kwbimage SPL padding to
>      CONFIG_SYS_U_BOOT_OFFS bytes
>    arm: mvebu: Remove unused macro CONFIG_SYS_U_BOOT_OFFS
>    tools: kwbimage: Add support for more BINARY headers
>    tools: kwbimage: Don't parse PAYLOAD keyword
>    tools: kwbimage: Add support for DATA command also for v1 images
>    tools: kwbimage: Add support for a new DATA_DELAY command
>    tools: kwbimage: Do not hide usage of secure header under
>      CONFIG_ARMADA_38X
> 
>   Makefile                               |   2 +-
>   arch/arm/mach-mvebu/Kconfig            |  16 +-
>   arch/arm/mach-mvebu/include/mach/cpu.h |   2 +-
>   arch/arm/mach-mvebu/lowlevel_spl.S     |   3 +-
>   arch/arm/mach-mvebu/spl.c              |  90 +------
>   board/gdsys/a38x/Makefile              |   2 +-
>   board/gdsys/a38x/spl.c                 |  20 --
>   include/configs/clearfog.h             |   6 +-
>   include/configs/controlcenterdc.h      |   8 +-
>   include/configs/db-88f6720.h           |   3 -
>   include/configs/db-88f6820-amc.h       |   5 -
>   include/configs/db-88f6820-gp.h        |   6 -
>   include/configs/db-mv784mp-gp.h        |   3 -
>   include/configs/ds414.h                |   5 -
>   include/configs/helios4.h              |   6 +-
>   include/configs/theadorable.h          |   3 -
>   include/configs/turris_omnia.h         |   6 -
>   include/configs/x530.h                 |   3 -
>   scripts/config_whitelist.txt           |   1 -
>   tools/Makefile                         |   8 -
>   tools/kwbimage.c                       | 339 +++++++++++++++++--------
>   tools/kwbimage.h                       |  30 ++-
>   tools/kwboot.c                         |  14 +-
>   23 files changed, 296 insertions(+), 285 deletions(-)
>   delete mode 100644 board/gdsys/a38x/spl.c
> 


Viele Grüße,
Stefan
Marek Behún July 9, 2021, 11:22 a.m. UTC | #2
Hi Stefan

On Fri, 9 Jul 2021 08:05:40 +0200
Stefan Roese <sr@denx.de> wrote:

> > The main goal of this series is to correctly use BootROM's code
> > for loading U-Boot from NOR / NAND: currently only SPL is read by
> > BootROM and the main U-Boot is read by SPL. By using BootROM to also
> > load main U-Boot we can reduce the size of SPL image, since it does
> > not need to contain code for reading NOR / NAND.  
> 
> Before going into a review of the patches, let me ask about the
> motivation of this patchset. Is the reduction of the SPL image size
> the main motivation for this series? Or did you experiece some
> problems with the SPL code for U-Boot proper loading?

The motivation is reduction of SPL code size.
Pali started working on something different (adding support for higher
baudrates to kwboot, so that a bricked board can be booted over UART
faster), and he is still working on it, but when he started studying the
kwbimage/kwboot code and A38x documentation, he noticed that these
things could be improved, so first he did this series.

> BTW: This patch / mail subject "kwboot / kwbimage improvements" does
> not really match its content AFAIU. Here, the SPL returns always back
> to the BootROM for U-Boot proper loading part is missing. Or do I
> misunderstand something?

You are right, the subject of the series could have been better. I
shall change it in v2.

> 
> BTW2: Could you please list the affected MVEBU SoC's that are affected
> by this series so that this is clear?

OK.

Marek
Stefan Roese July 9, 2021, 12:35 p.m. UTC | #3
Hi Marek,

On 09.07.21 13:22, Marek Behún wrote:
> Hi Stefan
> 
> On Fri, 9 Jul 2021 08:05:40 +0200
> Stefan Roese <sr@denx.de> wrote:
> 
>>> The main goal of this series is to correctly use BootROM's code
>>> for loading U-Boot from NOR / NAND: currently only SPL is read by
>>> BootROM and the main U-Boot is read by SPL. By using BootROM to also
>>> load main U-Boot we can reduce the size of SPL image, since it does
>>> not need to contain code for reading NOR / NAND.
>>
>> Before going into a review of the patches, let me ask about the
>> motivation of this patchset. Is the reduction of the SPL image size
>> the main motivation for this series? Or did you experiece some
>> problems with the SPL code for U-Boot proper loading?
> 
> The motivation is reduction of SPL code size.
> Pali started working on something different (adding support for higher
> baudrates to kwboot, so that a bricked board can be booted over UART
> faster), and he is still working on it, but when he started studying the
> kwbimage/kwboot code and A38x documentation, he noticed that these
> things could be improved, so first he did this series.
> 
>> BTW: This patch / mail subject "kwboot / kwbimage improvements" does
>> not really match its content AFAIU. Here, the SPL returns always back
>> to the BootROM for U-Boot proper loading part is missing. Or do I
>> misunderstand something?
> 
> You are right, the subject of the series could have been better. I
> shall change it in v2.
> 
>>
>> BTW2: Could you please list the affected MVEBU SoC's that are affected
>> by this series so that this is clear?
> 
> OK.

Thanks. And could you please also do one (or more) boot-time
comparisons, old vs. new version? This way we can see, if and how the
boot-time is affected (perhaps improved, which would be great) by this
patch series.

Thanks,
Stefan

> Marek
> 


Viele Grüße,
Stefan
Marek Behún July 9, 2021, 2:08 p.m. UTC | #4
On Fri, 9 Jul 2021 14:35:15 +0200
Stefan Roese <sr@denx.de> wrote:

> Thanks. And could you please also do one (or more) boot-time
> comparisons, old vs. new version? This way we can see, if and how the
> boot-time is affected (perhaps improved, which would be great) by this
> patch series.

For Turris Omnia, the size of the u-boot-spl.kwb binary decreases by
~37 KiB, mainly due to not needing padding anymore. The size of the SPL
binary itself (u-boot-spl.bin) decreases by 7.5 KiB (due to code
reduction - no SPI NOR code).

Unfortunately the boot time increases by 815 ms, from 2341 ms (on
average) to 3156 ms (on average).
This is probably because BootROM read the memory differently (lower
frequency or some other reason).

I think this 815 ms increase in boot time into U-Boot prompt is worth
the 37 KiB saved space for U-Boot code (mainly because on Omnia we have
960 KiB for U-Boot and currently we are at 860 KiB, so enabling some
other features in the future may push us to the limit), but others may
not agree.

I am going to put this information also into v2 cover letter.

Marek
Baruch Siach July 9, 2021, 2:54 p.m. UTC | #5
Hi Marek, Pali,

On Thu, Jul 08 2021, Marek Behún wrote:
> Hi Stefan and others,
>
> this is a series of improvements to kwboot, kwbimage and mvebu.
>
> The main goal of this series is to correctly use BootROM's code
> for loading U-Boot from NOR / NAND: currently only SPL is read by
> BootROM and the main U-Boot is read by SPL. By using BootROM to also
> load main U-Boot we can reduce the size of SPL image, since it does
> not need to contain code for reading NOR / NAND.
>
> Before merging, this series should be tested on as many relevant
> boards as possible.

Is this series available for pull at a public git repo? If not, what
commit is this series based on?

What platforms have you tested?

baruch

> Marek Behún (2):
>   tools: kwbimage: Add constant for SDIO bootfrom
>   tools: kwbimage: Cosmetic fix - remove redundant space character
>
> Pali Rohár (29):
>   tools: kwbimage: Fix compilation without CONFIG_SYS_U_BOOT_OFFS
>   tools: kwbimage: Simplify aligning and calculating checksum
>   tools: kwbimage: Align SPI and NAND images to 256 bytes
>   tools: kwbimage: Fix generation of SATA, SDIO and PCIe images
>   tools: kwbimage: Don't crash when binary file name does not contain
>     '/'
>   tools: kwbimage: Fix check for v0 extended header checksum
>   tools: kwbimage: Validate extended headers of v1 images
>   tools: kwbimage: Validate data checksum of v1 images
>   tools: kwbimage: Print size of binary header in
>     kwbimage_print_header()
>   tools: kwboot: Fix wrong parameter passed to read()
>   tools: kwboot: Fix restoring terminal
>   tools: kwboot: Print trailing newline after terminal is terminated
>   tools: kwboot: Cosmetic fix - add missing curly brackets
>   tools: kwboot: Check for v1 header size
>   tools: kwbimage: Use -a parameter (load address) for v1 images
>   arm: mvebu: Fix return_to_bootrom()
>   arm: mvebu: Mark return_to_bootrom() as a noreturn function
>   arm: mvebu: Implement return_to_bootrom() via U-Boot's SPL framework
>   arm: mvebu: Use U-Boot's SPL BootROM framework for booting from
>     NAND/UART
>   arm: mvebu: Always use BootROM for loading the rest of U-Boot's binary
>   arm: mvebu: gdsys: Remove custom spl_board_init()
>   arm: mvebu: Remove legacy U-Boot header from kwbimage v1 files
>   tools: kwbimage: Remove v1 kwbimage SPL padding to
>     CONFIG_SYS_U_BOOT_OFFS bytes
>   arm: mvebu: Remove unused macro CONFIG_SYS_U_BOOT_OFFS
>   tools: kwbimage: Add support for more BINARY headers
>   tools: kwbimage: Don't parse PAYLOAD keyword
>   tools: kwbimage: Add support for DATA command also for v1 images
>   tools: kwbimage: Add support for a new DATA_DELAY command
>   tools: kwbimage: Do not hide usage of secure header under
>     CONFIG_ARMADA_38X
>
>  Makefile                               |   2 +-
>  arch/arm/mach-mvebu/Kconfig            |  16 +-
>  arch/arm/mach-mvebu/include/mach/cpu.h |   2 +-
>  arch/arm/mach-mvebu/lowlevel_spl.S     |   3 +-
>  arch/arm/mach-mvebu/spl.c              |  90 +------
>  board/gdsys/a38x/Makefile              |   2 +-
>  board/gdsys/a38x/spl.c                 |  20 --
>  include/configs/clearfog.h             |   6 +-
>  include/configs/controlcenterdc.h      |   8 +-
>  include/configs/db-88f6720.h           |   3 -
>  include/configs/db-88f6820-amc.h       |   5 -
>  include/configs/db-88f6820-gp.h        |   6 -
>  include/configs/db-mv784mp-gp.h        |   3 -
>  include/configs/ds414.h                |   5 -
>  include/configs/helios4.h              |   6 +-
>  include/configs/theadorable.h          |   3 -
>  include/configs/turris_omnia.h         |   6 -
>  include/configs/x530.h                 |   3 -
>  scripts/config_whitelist.txt           |   1 -
>  tools/Makefile                         |   8 -
>  tools/kwbimage.c                       | 339 +++++++++++++++++--------
>  tools/kwbimage.h                       |  30 ++-
>  tools/kwboot.c                         |  14 +-
>  23 files changed, 296 insertions(+), 285 deletions(-)
>  delete mode 100644 board/gdsys/a38x/spl.c
Marek Behún July 9, 2021, 2:57 p.m. UTC | #6
On Fri, 09 Jul 2021 17:54:25 +0300
Baruch Siach <baruch@tkos.co.il> wrote:

> Hi Marek, Pali,
> 
> On Thu, Jul 08 2021, Marek Behún wrote:
> > Hi Stefan and others,
> >
> > this is a series of improvements to kwboot, kwbimage and mvebu.
> >
> > The main goal of this series is to correctly use BootROM's code
> > for loading U-Boot from NOR / NAND: currently only SPL is read by
> > BootROM and the main U-Boot is read by SPL. By using BootROM to also
> > load main U-Boot we can reduce the size of SPL image, since it does
> > not need to contain code for reading NOR / NAND.
> >
> > Before merging, this series should be tested on as many relevant
> > boards as possible.  
> 
> Is this series available for pull at a public git repo? If not, what
> commit is this series based on?
> 
> What platforms have you tested?

Based on current u-boot master, commit
fd075f77ca56ffb07e0b1979f0cb47fc8831600f

Tested only on Turris Omnia (Armada 385).

Marek
Stefan Roese July 10, 2021, 12:31 a.m. UTC | #7
Hi Marek,

On 09.07.21 16:08, Marek Behún wrote:
> On Fri, 9 Jul 2021 14:35:15 +0200
> Stefan Roese <sr@denx.de> wrote:
> 
>> Thanks. And could you please also do one (or more) boot-time
>> comparisons, old vs. new version? This way we can see, if and how the
>> boot-time is affected (perhaps improved, which would be great) by this
>> patch series.
> 
> For Turris Omnia, the size of the u-boot-spl.kwb binary decreases by
> ~37 KiB, mainly due to not needing padding anymore. The size of the SPL
> binary itself (u-boot-spl.bin) decreases by 7.5 KiB (due to code
> reduction - no SPI NOR code).
> 
> Unfortunately the boot time increases by 815 ms, from 2341 ms (on
> average) to 3156 ms (on average).

Ups. This boot-time increase is quite big. And there might be some
Armada / MVEBU U-Boot targets that are tuned for fast booting. I can
speak for the Armada XP theadorable board, where boot-time is quite
critical. I remember that we tuned and optimized here and an increase
of perhaps 100ms would be acceptable. But an increase of nearly 1
second is definitely not acceptable here, I'm afraid.

Please see below.

> This is probably because BootROM read the memory differently (lower
> frequency or some other reason).
> 
> I think this 815 ms increase in boot time into U-Boot prompt is worth
> the 37 KiB saved space for U-Boot code (mainly because on Omnia we have
> 960 KiB for U-Boot and currently we are at 860 KiB, so enabling some
> other features in the future may push us to the limit), but others may
> not agree.

Yes, I disagree here. Boot-time is critical for some platforms and this
increase is just too high. I might be able to do some tests on
theadorable to get actual numbers for this platform.

Could you perhaps add this "SPL returns to BootROM" support as an
optional feature that can be selected on a per-board basis?

Thanks,
Stefan

> I am going to put this information also into v2 cover letter.
> 
> Marek
>
Pali Rohár July 10, 2021, 12:43 a.m. UTC | #8
On Saturday 10 July 2021 02:31:32 Stefan Roese wrote:
> Could you perhaps add this "SPL returns to BootROM" support as an
> optional feature that can be selected on a per-board basis?

Hi Stefan! I was thinking about it and it should not be hard. Based on
defconfig options you decide if you want to use SPL NOR code or stick
with return to BootROM. It is acceptable?

What is needed is just to extend SPL SPI NOR code to parse also kwbimage
header (at offset zero) instead of legacy U-Boot header which is
removing by this patch series and which was on variable offset.
Pali Rohár July 10, 2021, 12:59 a.m. UTC | #9
On Saturday 10 July 2021 02:43:12 Pali Rohár wrote:
> On Saturday 10 July 2021 02:31:32 Stefan Roese wrote:
> > Could you perhaps add this "SPL returns to BootROM" support as an
> > optional feature that can be selected on a per-board basis?
> 
> Hi Stefan! I was thinking about it and it should not be hard. Based on
> defconfig options you decide if you want to use SPL NOR code or stick
> with return to BootROM. It is acceptable?
> 
> What is needed is just to extend SPL SPI NOR code to parse also kwbimage
> header (at offset zero) instead of legacy U-Boot header which is
> removing by this patch series and which was on variable offset.

Anyway, this switch "return to BootROM" is done in patch 22. Previous
patches 01 - 21 can be applied and used without later patches. So
meanwhile could you look at patches 01 - 21 if they are OK and if they
could be merged independently of "return to BootROM" patches?
Stefan Roese July 11, 2021, 7:11 a.m. UTC | #10
On 10.07.21 02:43, Pali Rohár wrote:
> On Saturday 10 July 2021 02:31:32 Stefan Roese wrote:
>> Could you perhaps add this "SPL returns to BootROM" support as an
>> optional feature that can be selected on a per-board basis?
> 
> Hi Stefan! I was thinking about it and it should not be hard. Based on
> defconfig options you decide if you want to use SPL NOR code or stick
> with return to BootROM. It is acceptable?

I think so, yes. If you patches add features / enhancements for some
boards and this can be selected on a per-board basis to not affect other
targets then yes, this can be accepted AFAICT.

> What is needed is just to extend SPL SPI NOR code to parse also kwbimage
> header (at offset zero) instead of legacy U-Boot header which is
> removing by this patch series and which was on variable offset.

It would be helpful, if you could add a Kconfig option to selectively
enable "return to BootROM" - disabled per default and enabled for your
targets to this patchset.

Thanks,
Stefan
Stefan Roese July 11, 2021, 7:11 a.m. UTC | #11
On 10.07.21 02:59, Pali Rohár wrote:
> On Saturday 10 July 2021 02:43:12 Pali Rohár wrote:
>> On Saturday 10 July 2021 02:31:32 Stefan Roese wrote:
>>> Could you perhaps add this "SPL returns to BootROM" support as an
>>> optional feature that can be selected on a per-board basis?
>>
>> Hi Stefan! I was thinking about it and it should not be hard. Based on
>> defconfig options you decide if you want to use SPL NOR code or stick
>> with return to BootROM. It is acceptable?
>>
>> What is needed is just to extend SPL SPI NOR code to parse also kwbimage
>> header (at offset zero) instead of legacy U-Boot header which is
>> removing by this patch series and which was on variable offset.
> 
> Anyway, this switch "return to BootROM" is done in patch 22. Previous
> patches 01 - 21 can be applied and used without later patches. So
> meanwhile could you look at patches 01 - 21 if they are OK and if they
> could be merged independently of "return to BootROM" patches?

I'll try to find some time next week to review these patches.

Thanks,
Stefan
Chris Packham July 11, 2021, 9:56 p.m. UTC | #12
On Fri, Jul 9, 2021 at 5:30 AM Marek Behún <marek.behun@nic.cz> wrote:
>
> Hi Stefan and others,
>
> this is a series of improvements to kwboot, kwbimage and mvebu.
>
> The main goal of this series is to correctly use BootROM's code
> for loading U-Boot from NOR / NAND: currently only SPL is read by
> BootROM and the main U-Boot is read by SPL. By using BootROM to also
> load main U-Boot we can reduce the size of SPL image, since it does
> not need to contain code for reading NOR / NAND.
>
> Before merging, this series should be tested on as many relevant
> boards as possible.

I've given the series a look over. Aside from a couple of typos in one
commit message which I replied to, the rest looked pretty good.

Review-by: Chris Packham <judge.packham@gmail.com>

For DB-88f6820-AMC and x530

Tested-by: Chris Packham <judge.packham@gmail.com>

>
> Marek & Pali
>
> Marek Behún (2):
>   tools: kwbimage: Add constant for SDIO bootfrom
>   tools: kwbimage: Cosmetic fix - remove redundant space character
>
> Pali Rohár (29):
>   tools: kwbimage: Fix compilation without CONFIG_SYS_U_BOOT_OFFS
>   tools: kwbimage: Simplify aligning and calculating checksum
>   tools: kwbimage: Align SPI and NAND images to 256 bytes
>   tools: kwbimage: Fix generation of SATA, SDIO and PCIe images
>   tools: kwbimage: Don't crash when binary file name does not contain
>     '/'
>   tools: kwbimage: Fix check for v0 extended header checksum
>   tools: kwbimage: Validate extended headers of v1 images
>   tools: kwbimage: Validate data checksum of v1 images
>   tools: kwbimage: Print size of binary header in
>     kwbimage_print_header()
>   tools: kwboot: Fix wrong parameter passed to read()
>   tools: kwboot: Fix restoring terminal
>   tools: kwboot: Print trailing newline after terminal is terminated
>   tools: kwboot: Cosmetic fix - add missing curly brackets
>   tools: kwboot: Check for v1 header size
>   tools: kwbimage: Use -a parameter (load address) for v1 images
>   arm: mvebu: Fix return_to_bootrom()
>   arm: mvebu: Mark return_to_bootrom() as a noreturn function
>   arm: mvebu: Implement return_to_bootrom() via U-Boot's SPL framework
>   arm: mvebu: Use U-Boot's SPL BootROM framework for booting from
>     NAND/UART
>   arm: mvebu: Always use BootROM for loading the rest of U-Boot's binary
>   arm: mvebu: gdsys: Remove custom spl_board_init()
>   arm: mvebu: Remove legacy U-Boot header from kwbimage v1 files
>   tools: kwbimage: Remove v1 kwbimage SPL padding to
>     CONFIG_SYS_U_BOOT_OFFS bytes
>   arm: mvebu: Remove unused macro CONFIG_SYS_U_BOOT_OFFS
>   tools: kwbimage: Add support for more BINARY headers
>   tools: kwbimage: Don't parse PAYLOAD keyword
>   tools: kwbimage: Add support for DATA command also for v1 images
>   tools: kwbimage: Add support for a new DATA_DELAY command
>   tools: kwbimage: Do not hide usage of secure header under
>     CONFIG_ARMADA_38X
>
>  Makefile                               |   2 +-
>  arch/arm/mach-mvebu/Kconfig            |  16 +-
>  arch/arm/mach-mvebu/include/mach/cpu.h |   2 +-
>  arch/arm/mach-mvebu/lowlevel_spl.S     |   3 +-
>  arch/arm/mach-mvebu/spl.c              |  90 +------
>  board/gdsys/a38x/Makefile              |   2 +-
>  board/gdsys/a38x/spl.c                 |  20 --
>  include/configs/clearfog.h             |   6 +-
>  include/configs/controlcenterdc.h      |   8 +-
>  include/configs/db-88f6720.h           |   3 -
>  include/configs/db-88f6820-amc.h       |   5 -
>  include/configs/db-88f6820-gp.h        |   6 -
>  include/configs/db-mv784mp-gp.h        |   3 -
>  include/configs/ds414.h                |   5 -
>  include/configs/helios4.h              |   6 +-
>  include/configs/theadorable.h          |   3 -
>  include/configs/turris_omnia.h         |   6 -
>  include/configs/x530.h                 |   3 -
>  scripts/config_whitelist.txt           |   1 -
>  tools/Makefile                         |   8 -
>  tools/kwbimage.c                       | 339 +++++++++++++++++--------
>  tools/kwbimage.h                       |  30 ++-
>  tools/kwboot.c                         |  14 +-
>  23 files changed, 296 insertions(+), 285 deletions(-)
>  delete mode 100644 board/gdsys/a38x/spl.c
>
> --
> 2.31.1
>