[PATCH RFC u-boot-mvebu 0/6] arm: mvebu: Fix boot mode detection
Pali Rohár
pali at kernel.org
Thu Mar 23 20:19:55 CET 2023
On Thursday 23 March 2023 19:33:27 Pali Rohár wrote:
> On Thursday 23 March 2023 11:01:22 Martin Rowe wrote:
> > On Wed, 22 Mar 2023 at 18:09, Pali Rohár <pali at kernel.org> wrote:
> > >
> > > On Wednesday 22 March 2023 11:14:42 Martin Rowe wrote:
> > > > On Tue, 21 Mar 2023 at 17:26, Pali Rohár <pali at kernel.org> wrote:
> > > >
> > > > > On Tuesday 21 March 2023 08:34:24 Martin Rowe wrote:
> > > > > > On Mon, 20 Mar 2023 at 21:33, Pali Rohár <pali at kernel.org> wrote:
> > > > > >
> > > > > > > On Monday 20 March 2023 18:45:01 Pali Rohár wrote:
> > > > > > > > On Monday 20 March 2023 12:01:03 Martin Rowe wrote:
> > > > > > > > > On Sun, 19 Mar 2023 at 18:20, Pali Rohár <pali at kernel.org> wrote:
> > > > > > > > >
> > > > > > > > > > On Sunday 19 March 2023 17:47:57 Pali Rohár wrote:
> > > > > > > > > > > On Sunday 19 March 2023 03:30:33 Martin Rowe wrote:
> > > > > > > > > > > > On Sun, 5 Mar 2023 at 11:55, Pali Rohár <pali at kernel.org>
> > > > > wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > > On Sunday 05 March 2023 04:21:42 Martin Rowe wrote:
> > > > > > > > > > > > > > On Sat, 4 Mar 2023 at 10:51, Pali Rohár <pali at kernel.org
> > > > > >
> > > > > > > wrote:
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > > Improve code for checking strapping pins which
> > > > > specifies
> > > > > > > boot
> > > > > > > > > > mode
> > > > > > > > > > > > > source.
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > Martin, could you test if Clearfog can be still
> > > > > configured
> > > > > > > into
> > > > > > > > > > UART
> > > > > > > > > > > > > > > booting mode via HW switches and if it still works
> > > > > > > correctly?
> > > > > > > > > > First
> > > > > > > > > > > > > > > patch is reverting UART related commit for Clearfog
> > > > > which I
> > > > > > > > > > think it
> > > > > > > > > > > > > not
> > > > > > > > > > > > > > > needed anymore.
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > On Clearfog the logic in the CONFIG_ARMADA_38X ifdef
> > > > > before
> > > > > > > the
> > > > > > > > > > switch
> > > > > > > > > > > > > that
> > > > > > > > > > > > > > you refactored in cpu.c/get_boot_device is all that gets
> > > > > > > > > > processed. It
> > > > > > > > > > > > > > decides there is an error and returns BOOT_DEVICE_UART,
> > > > > > > probably
> > > > > > > > > > because
> > > > > > > > > > > > > of
> > > > > > > > > > > > > > the invalid boot workaround for broken UART selection
> > > > > that
> > > > > > > you
> > > > > > > > > > > > > identified.
> > > > > > > > > > > > >
> > > > > > > > > > > > > Ok, so I figured out correctly how this invalid mode works.
> > > > > > > > > > > > >
> > > > > > > > > > > > > > UART only works if I use the clearfog_spi_defconfig or
> > > > > if I
> > > > > > > select
> > > > > > > > > > > > > > CONFIG_MVEBU_SPL_BOOT_DEVICE_UART=y. It does not work
> > > > > with
> > > > > > > the MMC
> > > > > > > > > > or
> > > > > > > > > > > > > SATA
> > > > > > > > > > > > > > defconfigs. I get the same result without this patch
> > > > > series
> > > > > > > > > > applied,
> > > > > > > > > > > > > though.
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > The failed cases have the same output (other than kwboot
> > > > > > > header
> > > > > > > > > > patching
> > > > > > > > > > > > > > output) until after sending boot image data is complete.
> > > > > The
> > > > > > > > > > output stops
> > > > > > > > > > > > > > after:
> > > > > > > > > > > > > > ================================
> > > > > > > > > > > > > > 98 %
> > > > > > > > > >
> > > > > [.................................................................
> > > > > > > > > > > > > > ]
> > > > > > > > > > > > > > Done
> > > > > > > > > > > > > > Finishing transfer
> > > > > > > > > > > > > > [Type Ctrl-\ + c to quit]
> > > > > > > > > > > > > > ================================
> > > > > > > > > > > > >
> > > > > > > > > > > > > This is very strange because
> > > > > CONFIG_MVEBU_SPL_BOOT_DEVICE_UART
> > > > > > > just
> > > > > > > > > > > > > instruct mkimage what to put into kwbimage header.
> > > > > > > > > > > > >
> > > > > > > > > > > > > If I'm looking at the output correctly then SPL was
> > > > > booted, it
> > > > > > > > > > correctly
> > > > > > > > > > > > > trained DDR RAM, returned back to bootrom, kwboot continued
> > > > > > > sending
> > > > > > > > > > main
> > > > > > > > > > > > > u-boot and bootrom confirmed that transfer of both SPL and
> > > > > main
> > > > > > > > > > u-boot
> > > > > > > > > > > > > is complete. But then there is no output from main u-boot.
> > > > > > > > > > > > >
> > > > > > > > > > > > > > It looks like an unrelated issue with kwboot.c, which I
> > > > > was
> > > > > > > sure
> > > > > > > > > > was
> > > > > > > > > > > > > > working after the last patches but I can no longer
> > > > > reproduce
> > > > > > > a
> > > > > > > > > > successful
> > > > > > > > > > > > > > boot.
> > > > > > > > > > > > >
> > > > > > > > > > > > > Can you check that you are using _both_ mkimage and kwboot
> > > > > from
> > > > > > > > > > version
> > > > > > > > > > > > > with applying _all_ my patches recently sent to ML? Because
> > > > > > > both
> > > > > > > > > > mkimage
> > > > > > > > > > > > > and kwboot have fixes for SATA and SDIO images.
> > > > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > > > > > I tested using the latest next branch which has those
> > > > > changes in
> > > > > > > it.
> > > > > > > > > > Steps:
> > > > > > > > > > > > - Set UART boot mode on device
> > > > > > > > > > > > - make clean
> > > > > > > > > > > > - make clearfog_defconfig
> > > > > > > > > > > > - make
> > > > > > > > > > > > - ./tools/kwboot -b u-boot-with-spl.kwb -t /dev/ttyUSB0
> > > > > > > > > > > >
> > > > > > > > > > > > For me it looks like that either mkimage generated incorrect
> > > > > > > image size
> > > > > > > > > > > > > for SATA or SDIO image. Or kwboot incorrectly parsed that
> > > > > > > image size
> > > > > > > > > > > > > from kwbimage header and sent smaller image.
> > > > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > > > > > <MMC output>
> > > > > > > > > > > > ./tools/kwboot -b u-boot-with-spl.kwb -t /dev/ttyUSB0
> > > > > > > > > > > > kwboot version 2023.04-rc4-00339-gcefd0449d6
> > > > > > > > > > > > Detected kwbimage v1 with SDIO boot signature
> > > > > > > > > > > > Patching image boot signature to UART
> > > > > > > > > > > > Aligning image header to Xmodem block size
> > > > > > > > > > > > Sending boot message. Please reboot the target...\
> > > > > > > > > > > > Sending boot image header (113408 bytes)...
> > > > > > > > > > > > 0 %
> > > > > > > > > > > >
> > > > > > > > > >
> > > > > > >
> > > > > [......................................................................]
> > > > > > > > > > > > <snip>
> > > > > > > > > > > > 94 % [..............................................
> > > > > > > > > > > > ]
> > > > > > > > > > > > Done
> > > > > > > > > > > >
> > > > > > > > > > > > U-Boot SPL 2023.04-rc4-00339-gcefd0449d6 (Mar 19 2023 -
> > > > > 12:57:31
> > > > > > > +1000)
> > > > > > > > > > > > High speed PHY - Version: 2.0
> > > > > > > > > > > > EEPROM TLV detection failed: Using static config for Clearfog
> > > > > > > Pro.
> > > > > > > > > > > > Detected Device ID 6828
> > > > > > > > > > > > board SerDes lanes topology details:
> > > > > > > > > > > > | Lane # | Speed | Type |
> > > > > > > > > > > > --------------------------------
> > > > > > > > > > > > | 0 | 3 | SATA0 |
> > > > > > > > > > > > | 1 | 0 | SGMII1 |
> > > > > > > > > > > > | 2 | 5 | PCIe1 |
> > > > > > > > > > > > | 3 | 5 | USB3 HOST1 |
> > > > > > > > > > > > | 4 | 5 | PCIe2 |
> > > > > > > > > > > > | 5 | 0 | SGMII2 |
> > > > > > > > > > > > --------------------------------
> > > > > > > > > > > > High speed PHY - Ended Successfully
> > > > > > > > > > > > mv_ddr: 14.0.0
> > > > > > > > > > > > DDR3 Training Sequence - Switching XBAR Window to FastPath
> > > > > Window
> > > > > > > > > > > > mv_ddr: completed successfully
> > > > > > > > > > > > Trying to boot from BOOTROM
> > > > > > > > > > > > Returning to BootROM (return address 0xffff05c4)...
> > > > > > > > > > > >
> > > > > > > > > > > > Sending boot image data (474564 bytes)...
> > > > > > > > > > > > 0 %
> > > > > > > > > > > >
> > > > > > > > > >
> > > > > > >
> > > > > [......................................................................]
> > > > > > > > > > > > <snip>
> > > > > > > > > > > > 98 %
> > > > > > > > > >
> > > > > [....................................................................
> > > > > > > > > > > > ]
> > > > > > > > > > > > Done
> > > > > > > > > > > > Finishing transfer
> > > > > > > > > > > > [Type Ctrl-\ + c to quit]
> > > > > > > > > > > > </MMC output>
> > > > > > > > > > > >
> > > > > > > > > > > > <MMC sizes>
> > > > > > > > > > > > du -b u-boot*
> > > > > > > > > > > > 4996828 u-boot
> > > > > > > > > > > > 474304 u-boot.bin
> > > > > > > > > > > > 15155 u-boot.cfg
> > > > > > > > > > > > 19496 u-boot.dtb
> > > > > > > > > > > > 474304 u-boot-dtb.bin
> > > > > > > > > > > > 474368 u-boot-dtb.img
> > > > > > > > > > > > 474368 u-boot.img
> > > > > > > > > > > > 1721 u-boot.lds
> > > > > > > > > > > > 1069982 u-boot.map
> > > > > > > > > > > > 454808 u-boot-nodtb.bin
> > > > > > > > > > > > 1307712 u-boot.srec
> > > > > > > > > > > > 198841 u-boot.sym
> > > > > > > > > > > > 588288 u-boot-with-spl.kwb
> > > > > > > > > > > > </MMC sizes>
> > > > > > > > > > > >
> > > > > > > > > > > > If I make menuconfig and set the boot mode to UART before
> > > > > making
> > > > > > > I get:
> > > > > > > > > > > >
> > > > > > > > > > > > <UART output>
> > > > > > > > > > > > ./tools/kwboot -b u-boot-with-spl.kwb -t /dev/ttyUSB0
> > > > > > > > > > > > kwboot version 2023.04-rc4-00339-gcefd0449d6
> > > > > > > > > > > > Detected kwbimage v1 with UART boot signature
> > > > > > > > > > > > Aligning image header to Xmodem block size
> > > > > > > > > > > > Sending boot message. Please reboot the target...\
> > > > > > > > > > > > Sending boot image header (113408 bytes)...
> > > > > > > > > > > > 0 %
> > > > > > > > > > > >
> > > > > > > > > >
> > > > > > >
> > > > > [......................................................................]
> > > > > > > > > > > > <snip>
> > > > > > > > > > > > 94 % [..............................................
> > > > > > > > > > > > ]
> > > > > > > > > > > > Done
> > > > > > > > > > > >
> > > > > > > > > > > > U-Boot SPL 2023.04-rc4-00339-gcefd0449d6 (Mar 19 2023 -
> > > > > 13:20:48
> > > > > > > +1000)
> > > > > > > > > > > > High speed PHY - Version: 2.0
> > > > > > > > > > > > EEPROM TLV detection failed: Using static config for Clearfog
> > > > > > > Pro.
> > > > > > > > > > > > Detected Device ID 6828
> > > > > > > > > > > > board SerDes lanes topology details:
> > > > > > > > > > > > | Lane # | Speed | Type |
> > > > > > > > > > > > --------------------------------
> > > > > > > > > > > > | 0 | 3 | SATA0 |
> > > > > > > > > > > > | 1 | 0 | SGMII1 |
> > > > > > > > > > > > | 2 | 5 | PCIe1 |
> > > > > > > > > > > > | 3 | 5 | USB3 HOST1 |
> > > > > > > > > > > > | 4 | 5 | PCIe2 |
> > > > > > > > > > > > | 5 | 0 | SGMII2 |
> > > > > > > > > > > > --------------------------------
> > > > > > > > > > > > High speed PHY - Ended Successfully
> > > > > > > > > > > > mv_ddr: 14.0.0
> > > > > > > > > > > > DDR3 Training Sequence - Switching XBAR Window to FastPath
> > > > > Window
> > > > > > > > > > > > mv_ddr: completed successfully
> > > > > > > > > > > > Trying to boot from BOOTROM
> > > > > > > > > > > > Returning to BootROM (return address 0xffff05c4)...
> > > > > > > > > > > >
> > > > > > > > > > > > Sending boot image data (474404 bytes)...
> > > > > > > > > > > > 0 %
> > > > > > > > > > > >
> > > > > > > > > >
> > > > > > >
> > > > > [......................................................................]
> > > > > > > > > > > > <snip>
> > > > > > > > > > > > 98 %
> > > > > > > > > >
> > > > > [...................................................................
> > > > > > > > > > > > ]
> > > > > > > > > > > > Done
> > > > > > > > > > > > Finishing transfer
> > > > > > > > > > > > [Type Ctrl-\ + c to quit]
> > > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > > > > > U-Boot 2023.04-rc4-00339-gcefd0449d6 (Mar 19 2023 - 13:20:48
> > > > > > > +1000)
> > > > > > > > > > > > </UART output>
> > > > > > > > > > > >
> > > > > > > > > > > > <UART sizes>
> > > > > > > > > > > > du -b u-boot*
> > > > > > > > > > > > 4997204 u-boot
> > > > > > > > > > > > 474400 u-boot.bin
> > > > > > > > > > > > 15103 u-boot.cfg
> > > > > > > > > > > > 19496 u-boot.dtb
> > > > > > > > > > > > 474400 u-boot-dtb.bin
> > > > > > > > > > > > 474464 u-boot-dtb.img
> > > > > > > > > > > > 474464 u-boot.img
> > > > > > > > > > > > 1721 u-boot.lds
> > > > > > > > > > > > 1070068 u-boot.map
> > > > > > > > > > > > 454904 u-boot-nodtb.bin
> > > > > > > > > > > > 1307988 u-boot.srec
> > > > > > > > > > > > 198886 u-boot.sym
> > > > > > > > > > > > 587904 u-boot-with-spl.kwb
> > > > > > > > > > > > </UART sizes>
> > > > > > > > > > > >
> > > > > > > > > > > > The difference is very small somewhere if that is the cause.
> > > > > Let
> > > > > > > me
> > > > > > > > > > know if
> > > > > > > > > > > > there's other data I can get to help with this one.
> > > > > > > > > > >
> > > > > > > > > > > Difference should be only in the kwbimage header plus some
> > > > > > > aligning. It
> > > > > > > > > > > just proves what I wrote before "mkimage generated incorrect
> > > > > image
> > > > > > > size
> > > > > > > > > > > for SDIO image. Or kwboot incorrectly parsed that image size
> > > > > > > > > > > from kwbimage header and sent smaller image.".
> > > > > > > > > > >
> > > > > > > > > > > I will try to find a bug in mkimage or kwboot tool. Or you can
> > > > > try
> > > > > > > it
> > > > > > > > > > > too. As mmc booting is working fine from mmc I have felling
> > > > > that
> > > > > > > bug is
> > > > > > > > > > > in kwboot code which parses mmc images.
> > > > > > > > > >
> > > > > > > > > > Ok, I think I found 3 bugs, all are UART related (not mmc). Could
> > > > > > > you try
> > > > > > > > > > these changes?
> > > > > > > > > >
> > > > > > > > >
> > > > > > > > > Dedicated UART still works, patching an MMC for UART with kwboot
> > > > > still
> > > > > > > > > hangs after finishing transfer; no change.
> > > > > > > >
> > > > > > > > Bah :-( So there is still another bug. I will look at the code
> > > > > again...
> > > > > > >
> > > > > > > I do not see anything wrong there :-(
> > > > > > >
> > > > > > > Could you try to normally build mmc image and then run following
> > > > > > > commands to manually generate uart image u-boot-with-spl.uart.kwb?
> > > > > > >
> > > > > > > sed 's/^BOOT_FROM.*/BOOT_FROM uart/'
> > > > > ./arch/arm/mach-mvebu/kwbimage.cfg
> > > > > > > > kwbimage.uart.cfg
> > > > > > > ./tools/mkimage -n kwbimage.uart.cfg -T kwbimage -a 0x00800000 -e
> > > > > > > 0x00800000 -d u-boot.bin u-boot-with-spl.uart.kwb
> > > > > > >
> > > > > > > I would like to know if this UART image is bootable or not.
> > > > > > >
> > > > > >
> > > > > > It is bootable. Here is the output from the normal mkimage:
> > > > > > ./tools/mkimage -n ./arch/arm/mach-mvebu/kwbimage.cfg -T kwbimage -a
> > > > > > 0x00800000 -e 0x00800000 -d u-boot.bin u-boot-with-spl.kwb
> > > > > > Image Type: MVEBU Boot from sdio Image
> > > > > > Image version:1
> > > > > > BIN Img Size: 113416 Bytes = 110.76 KiB = 0.11 MiB
> > > > > > BIN Img Offs: 48 Bytes = 0.05 KiB = 0.00 MiB
> > > > > > Data Size: 475072 Bytes = 463.94 KiB = 0.45 MiB
> > > > > > Data Offset: 113664 Bytes = 111.00 KiB = 0.11 MiB
> > > > > > Load Address: 00800000
> > > > > > Entry Point: 00800000
> > > > > >
> > > > > > Here is the output from your custom command:
> > > > > > ./tools/mkimage -n kwbimage.uart.cfg -T kwbimage -a 0x00800000 -e
> > > > > > 0x00800000 -d u-boot.bin u-boot-with-spl.uart.kwb
> > > > > > Image Type: MVEBU Boot from uart Image
> > > > > > Image version:1
> > > > > > BIN Img Size: 113416 Bytes = 110.76 KiB = 0.11 MiB
> > > > > > BIN Img Offs: 48 Bytes = 0.05 KiB = 0.00 MiB
> > > > > > Data Size: 475072 Bytes = 463.94 KiB = 0.45 MiB
> > > > > > Data Offset: 113536 Bytes = 110.88 KiB = 0.11 MiB
> > > > > > Load Address: 00800000
> > > > > > Entry Point: 00800000
> > > > >
> > > > > Obviously I have this output as I compiled it many times. There is
> > > > > nothing suspicious. Data offset is different just be cause mmc image is
> > > > > aligned to 512 byte long sector size and uart image is aligned to 128
> > > > > byte log xmodem block size. Hence data offset needs to be multiply of
> > > > > 128 or 512 based on the image type
> > > > >
> > > > > > The difference is 128 bytes for the data offset. When I run kwboot I get:
> > > > > > Sending boot image data (475204 bytes)... [for the normal mkimage which
> > > > > > doesn't work]
> > > > > > Sending boot image data (475076 bytes)... [for the custom mkimage which
> > > > > > does work]
> > > > >
> > > > > But this is suspicious. Data image size printed by kwboot is data size
> > > > > printed by mkimage plus 4 bytes (which is checksum). It is correct for
> > > > > custom uart image, but not for mmc image converted to uart image by
> > > > > kwboot.
> > > > >
> > > > > And now I see where can be an issue. In kwbimage v1 header is stored
> > > > > length of the header itself (it can be variable length) and also offset
> > > > > where the data part of the image starts. As I pointed in one of the
> > > > > patch chunk sent previously, bootrom reads kwbimage header by loading
> > > > > number of xmodem blocks equals to header size divided by 128 (xmodem
> > > > > block size) rounded down. So patch chunk manually increased header size
> > > > > to the next multiply of 128 bytes. And after bootrom read kwbimage
> > > > > header it immediately starts reading data part of the image - and
> > > > > completely ignores the fact that in header is stored offset to the data
> > > > > part. In case we have different alignment, it is possible that between
> > > > > header and data part is a gap, which is multiply of the 128 bytes. E.g.
> > > > > original header size was 300 bytes, alignment was 512 bytes; new
> > > > > alignment is 128 bytes, which effectively decrease size of the header
> > > > > from 512 bytes to just 384 bytes; with creating a gap of 128 bytes
> > > > > between header and data part.
> > > > >
> > > > > Could you try following change if it fixes?
> > > > >
> > > >
> > > > Success! I tested kwboot patching of MMC, SATA and SPI images and all boot
> > > > successfully with all 4 patches applied.
> > >
> > > Perfect! Could you also try to send without kwboot patch image with that
> > > off-by-one offset which I described in other? It would be nice to have
> > > some proof that this is really the issue here.
> >
> > One byte in either direction without the patch to remove the gap does
> > not boot, just hangs after "Finishing transfer".
>
> Ok, then probably I did not fully understood that disassembled code.
Hm.. have you recalculated also header checksum? If not then failure is
expected. Because the first byte of the data image is moved to the
position of the last byte of the header (where is the filler) and so it
breaks header checksum. If you send me working mmc kwbimage I can try to
convert it into "shifted off-by-one" uart kwbimage.
> > The patch moves the
> > data by 256 bytes with the same files, so it seems like there's
> > another effect involved.
>
> This movement is correct. It is changing alignment from 512 bytes to
> just 128 bytes. So if in the last 512 byte long block are only 200 bytes
> then then 200 bytes in 128 byte long alignment needs only 256 bytes and
> so gap between 256th byte and 512th byte needs to be removed.
>
> > > I will prepare another kwboot and kwbimage patches which finally should
> > > solve these issues.
> > >
> > > > UART images work with all 4 patches applied as well, but also work with
> > > > just the two patches at the end of kwboot.c kwboot_img_patch applied.
> > >
> > > Could you try to send UART image also via "sx" utility? Just to be sure
> > > that UART images generated by mkimage are valid for xmodem transfer
> > > without need to do any kwboot on-the-fly patching.
> >
> > ./tools/kwboot -b /dev/ttyUSB0
> > sx -b u-boot-with-spl.kwb < /dev/ttyUSB0 > /dev/ttyUSB0
> > Sending u-boot-with-spl.kwb, 4626 blocks: Give your local XMODEM
> > receive command now.
> > Xmodem sectors/kbytes sent: 898/112kRetry 0: Got 0d for sector ACK
> > Retry 0: NAK on sector
> > Retry 0: Got 6c for sector ACK
> > Retry 0: NAK on sector
> > Retry 0: Got 49 for sector ACK
> > Retry 0: NAK on sector
> > Retry 0: Got 49 for sector ACK
> > Retry 0: NAK on sector
> > Bytes Sent: 592128 BPS:7679
> >
> > Transfer complete
> > ./tools/kwboot -t /dev/ttyUSB0
> > kwboot version 2023.04-rc4-00342-g7e562609bb-dirty
> > [Type Ctrl-\ + c to quit]
> >
> > ...and nothing; no u-boot prompt, system doesn't boot, it just hangs
> > like when kwboot fails.
>
> Ah... I guess sx is too smart is trying to resubmit xmodem block when it
> receives unknown reply (which is the SPL output). If you look into
> kwboot.c source code you can find description of A38x BootROM bug which
> cause that resubmit/retry is broken and must not be used _after_ sending
> header.
>
> Could you try instead to use kwboot from U-Boot v2021.07 for booting
> UART kwbimage? I just want to be sure that UART image is correctly
> generated by mkimage and I hope that this old kwboot is not smart enough
> for on-the-fly "patching".
>
> Anyway, I think everything. I will prepare patches and send them to ML.
>
> > I added a write function after kwboot.c kwboot_img_patch gets called
> > and compared the output. The files are identical until the very end
> > where kwboot trims some zeros. I tried sending both the patched and
> > unpatched file and got the same hanging with both. Given the patched
> > file works with kwboot, I suspect the remaining magic is happening in
> > the kwboot_xmodem function.
>
> Ok.
>
> > I've never seen sx work on clearfog. I assumed the SPL serial output
> > corrupted the transfer, so I've always used kwboot. Maybe it's a quirk
> > of this board, but either way I don't think the result of this test is
> > meaningful.
>
> I thought that I saw sx working on A385 in the past. But maybe too long
> SPL output makes it broken.
>
> > > > diff --git a/tools/kwboot.c b/tools/kwboot.c
> > > > > index a118a26d282c..272128db3946 100644
> > > > > --- a/tools/kwboot.c
> > > > > +++ b/tools/kwboot.c
> > > > > @@ -2181,6 +2181,16 @@ kwboot_img_patch(void *img, size_t *size, int
> > > > > baudrate)
> > > > > }
> > > > > }
> > > > >
> > > > > + /* Remove the gap between header and data part by moving data part
> > > > > */
> > > > > + if (!is_secure && hdrsz != le32_to_cpu(hdr->srcaddr)) {
> > > > > + kwboot_printv("Removing gap between image header and
> > > > > data\n");
> > > > > + memmove(img + hdrsz, img + le32_to_cpu(hdr->srcaddr),
> > > > > le32_to_cpu(hdr->blocksize));
> > > > > + hdr->srcaddr = cpu_to_le32(hdrsz);
> > > > > + } else if (le32_to_cpu(hdr->srcaddr) % KWBOOT_XM_BLKSZ) {
> > > > > + fprintf(stderr, "Cannot align image with secure header\n");
> > > > > + goto err;
> > > > > + }
> > > > > +
> > > > > hdr->checksum = kwboot_hdr_csum8(hdr) - csum;
> > > > >
> > > > > *size = le32_to_cpu(hdr->srcaddr) + le32_to_cpu(hdr->blocksize);
> > > > >
> > > > > > That 128 extra carries across. I'll poke around a bit more, but it seems
> > > > > > like a good lead so I wanted to share.
> > > > > >
> > > > > >
> > > > > > > Also it would be interesting to enable CONFIG_DEBUG_UART_ANNOUNCE
> > > > > option
> > > > > > > and check if you see early announce message on UART.
> > > > > > >
> > > > > >
> > > > > > That's enabled for all the clearfog builds already.
> > > > > >
> > > > > >
> > > > > > > > Meanwhile could you do following tests?
> > > > > > > >
> > > > > > > > 1) The one which you done with patched kwboot and kwbimage, but send
> > > > > > > > output (sizes and aligning information from kwboot is useful there).
> > > > > > > >
> > > > > > > > 2) Use kwboot only for sending magic packet (-b without image) and
> > > > > then
> > > > > > > > use "sx" program for transferring kwb image over UART (instead of
> > > > > > > > kwboot). "sx" should work only with dedicated UART image type.
> > > > > > > >
> > > > > > > > These commands would do it (replace ttyX by correct UART device)
> > > > > > > >
> > > > > > > > $ ./tools/kwboot -b /dev/ttyX
> > > > > > > > $ sh -c 'exec 0<>/dev/ttyX 1>&0; sx u-boot-with-spl.kwb'
> > > > > > > >
> > > > > > > > >
> > > > > > > > > > diff --git a/tools/kwbimage.c b/tools/kwbimage.c
> > > > > > > > > > index 309657a5637b..177084adf825 100644
> > > > > > > > > > --- a/tools/kwbimage.c
> > > > > > > > > > +++ b/tools/kwbimage.c
> > > > > > > > > > @@ -1231,6 +1231,16 @@ static size_t image_headersz_v1(int
> > > > > *hasext)
> > > > > > > > > > if (count > 0)
> > > > > > > > > > headersz += sizeof(struct register_set_hdr_v1) +
> > > > > 8 *
> > > > > > > count
> > > > > > > > > > + 4;
> > > > > > > > > >
> > > > > > > > > > + /*
> > > > > > > > > > + * For all images except UART, headersz stored in header
> > > > > > > itself
> > > > > > > > > > should
> > > > > > > > > > + * contains header size without padding. For UART image
> > > > > > > BootROM
> > > > > > > > > > rounds
> > > > > > > > > > + * down headersz to multiply of 128 bytes. Therefore
> > > > > align
> > > > > > > UART
> > > > > > > > > > headersz
> > > > > > > > > > + * to multiply of 128 bytes to ensure that remaining UART
> > > > > > > header
> > > > > > > > > > bytes
> > > > > > > > > > + * are not ignored by BootROM.
> > > > > > > > > > + */
> > > > > > > > > > + if (image_get_bootfrom() == IBR_HDR_UART_ID)
> > > > > > > > > > + headersz = ALIGN(headersz, 128);
> > > > > > > > > > +
> > > > > > > > > > return headersz;
> > > > > > > > > > }
> > > > > > > > > >
> > > > > > > > > > diff --git a/tools/kwboot.c b/tools/kwboot.c
> > > > > > > > > > index 1844d7291fe0..b40800c108fc 100644
> > > > > > > > > > --- a/tools/kwboot.c
> > > > > > > > > > +++ b/tools/kwboot.c
> > > > > > > > > > @@ -2079,6 +2079,8 @@ kwboot_img_patch(void *img, size_t *size,
> > > > > int
> > > > > > > > > > baudrate)
> > > > > > > > > > goto err;
> > > > > > > > > > }
> > > > > > > > > > kwboot_img_grow_data_right(img, size,
> > > > > > > sizeof(uint32_t));
> > > > > > > > > > + /* Update the 32-bit data checksum */
> > > > > > > > > > + *kwboot_img_csum32_ptr(img) =
> > > > > kwboot_img_csum32(img);
> > > > > > > > > > }
> > > > > > > > > >
> > > > > > > > > > if (!kwboot_img_has_ddr_init(img) &&
> > > > > > > > > > @@ -2168,6 +2170,17 @@ kwboot_img_patch(void *img, size_t *size,
> > > > > int
> > > > > > > > > > baudrate)
> > > > > > > > > >
> > > > > > > > > > kwboot_printv("Aligning image header to Xmodem
> > > > > block
> > > > > > > > > > size\n");
> > > > > > > > > > kwboot_img_grow_hdr(img, size, grow);
> > > > > > > > > > +
> > > > > > > > > > + /*
> > > > > > > > > > + * kwbimage v1 contains header size field and for
> > > > > > > UART
> > > > > > > > > > type it
> > > > > > > > > > + * must be set to the aligned xmodem header size
> > > > > > > because
> > > > > > > > > > BootROM
> > > > > > > > > > + * rounds header size down to xmodem block size.
> > > > > > > > > > + */
> > > > > > > > > > + if (kwbimage_version(img) == 1) {
> > > > > > > > > > + hdrsz += grow;
> > > > > > > > > > + hdr->headersz_msb = hdrsz >> 16;
> > > > > > > > > > + hdr->headersz_lsb = cpu_to_le16(hdrsz &
> > > > > > > 0xffff);
> > > > > > > > > > + }
> > > > > > > > > > }
> > > > > > > > > >
> > > > > > > > > > hdr->checksum = kwboot_hdr_csum8(hdr) - csum;
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > > > >
> > > > > > > > > > > > > > > Also could you check if SATA booting is still working
> > > > > > > correctly?
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > SATA works correctly.
> > > > > > > > > > > > >
> > > > > > > > > > > > > Perfect!
> > > > > > > > > > > > >
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > > Tony, should address problems with SPI booting when it
> > > > > is
> > > > > > > > > > configured to
> > > > > > > > > > > > > > > different configuration. In fourth commit I added all
> > > > > > > possible
> > > > > > > > > > boot
> > > > > > > > > > > > > mode
> > > > > > > > > > > > > > > strapping pin configurations which are recognized by
> > > > > A385
> > > > > > > > > > bootrom (and
> > > > > > > > > > > > > > > not the only one described in the HW spec, which is
> > > > > > > incomplete).
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > Stefan, do you have some AXP board with SATA boot
> > > > > source?
> > > > > > > > > > Because I'm
> > > > > > > > > > > > > > > adding it for completeness in the last sixth patch.
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > Pali Rohár (6):
> > > > > > > > > > > > > > > arm: mvebu: Remove A38x BOOT_FROM_UART_ALT 0x3f
> > > > > constant
> > > > > > > > > > > > > > > arm: mvebu: Remove A38x BOOT_FROM_SATA 0x22 constant
> > > > > > > > > > > > > > > arm: mvebu: Convert BOOT_FROM_* constants to function
> > > > > > > macros
> > > > > > > > > > > > > > > arm: mvebu: Define all options for A38x BOOT_FROM_*
> > > > > > > macros
> > > > > > > > > > > > > > > arm: mvebu: Define all BOOTROM_ERR_MODE_* macros
> > > > > > > > > > > > > > > arm: mvebu: Define all options for AXP BOOT_FROM_*
> > > > > macros
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > arch/arm/mach-mvebu/cpu.c | 20
> > > > > ++++++-------
> > > > > > > > > > > > > > > arch/arm/mach-mvebu/include/mach/soc.h | 41
> > > > > > > > > > ++++++++++++++++----------
> > > > > > > > > > > > > > > 2 files changed, 35 insertions(+), 26 deletions(-)
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > --
> > > > > > > > > > > > > > > 2.20.1
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > >
> > > > > > > > > > > > >
> > > > > > > > > >
> > > > > > >
> > > > >
More information about the U-Boot
mailing list