kwboot: Marvell Dove UART booting

Pali Rohár pali at kernel.org
Tue Feb 15 20:11:15 CET 2022


On Monday 14 February 2022 14:00:19 Tony Dinh wrote:
> Hi Pali,
> 
> On Mon, Feb 14, 2022 at 1:58 AM Pali Rohár <pali at kernel.org> wrote:
> >
> > On Sunday 13 February 2022 17:10:26 Tony Dinh wrote:
> > > Hi Pali,
> > >
> > > Please see below.
> > >
> > > On Sun, Feb 13, 2022 at 4:21 PM Pali Rohár <pali at kernel.org> wrote:
> > > >
> > > > On Sunday 13 February 2022 16:08:51 Tony Dinh wrote:
> > > > > Hi Pali,
> > > > >
> > > > > On Sun, Feb 13, 2022 at 3:42 PM Pali Rohár <pali at kernel.org> wrote:
> > > > > >
> > > > > > On Sunday 13 February 2022 15:24:46 Tony Dinh wrote:
> > > > > > > Hi Pali,
> > > > > > >
> > > > > > > Looks different, but the BootROM did not start the image. Please see
> > > > > > > the log below.
> > > > > > >
> > > > > > > On Sun, Feb 13, 2022 at 2:48 PM Pali Rohár <pali at kernel.org> wrote:
> > > > > > > >
> > > > > > > > On Sunday 13 February 2022 14:45:07 Tony Dinh wrote:
> > > > > > > > > Hi Pali,
> > > > > > > > >
> > > > > > > > > Some compile errors, please see below.
> > > > > > > > >
> > > > > > > > > On Sun, Feb 13, 2022 at 8:16 AM Pali Rohár <pali at kernel.org> wrote:
> > > > > > > > > >
> > > > > > > > > > On Wednesday 22 December 2021 20:08:56 Tony Dinh wrote:
> > > > > > > > > > > *** Run kwboot
> > > > > > > > > > >
> > > > > > > > > > > # kwboot -t -p -B 115200 /dev/ttyUSB0 -D /localdisk/mtd0.t5335z
> > > > > > > > > > > Patching image boot signature to UART
> > > > > > > > > > > Aligning image header to Xmodem block size
> > > > > > > > > > > Waiting 2s and flushing tty
> > > > > > > > > > > Sending boot image header (512 bytes)...
> > > > > > > > > > >  25 % [....                                                                  ]
> > > > > > > > > > > Done
> > > > > > > > > > > Sending boot image data (607664 bytes)...
> > > > > > > > > > >   0 % [......................................................................]
> > > > > > > > > > >   1 % [......................................................................]
> > > > > > > > > > >   2 % [......................................................................]
> > > > > > > > > > > <snip>
> > > > > > > > > > >  95 % [......................................................................]
> > > > > > > > > > >  97 % [......................................................................]
> > > > > > > > > > >  98 % [..........................................................            ]
> > > > > > > > > > > Done
> > > > > > > > > > > Finishing transfer
> > > > > > > > > > > [Type Ctrl-\ + c to quit]
> > > > > > > > > > >
> > > > > > > > > > > *** Hung here! BootROM did not execute the image payload.
> > > > > > > > > > > ***
> > > > > > > > > > > *** The file mtd0.t5335z is a dd dump from the SPI flash mtd0 with
> > > > > > > > > > > *** this command:
> > > > > > > > > > > ***     # dd if=/dev/mtd0 of=mtd0.t5335z bs=768k conv=sync
> > > > > > > > > > >
> > > > > > > > > > > <End log>
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > - Pali's observation:
> > > > > > > > > > >
> > > > > > > > > > > It looks like Dove uses kwbimage v0 format with extensions, at
> > > > > > > > > > > least according to Function Spec. See 'Binary Code Extension' and
> > > > > > > > > > > 'Header Extension'. Currently kwboot and kwbimage supports v0 image only
> > > > > > > > > > > with one extension.
> > > > > > > > > >
> > > > > > > > > > I quickly looked at it. Could you try following patch?
> > > > > > > > > >
> > > > > > > > > > diff --git a/tools/kwbimage.h b/tools/kwbimage.h
> > > > > > > > > > index 74e5d87a4fef..15e83ececc76 100644
> > > > > > > > > > --- a/tools/kwbimage.h
> > > > > > > > > > +++ b/tools/kwbimage.h
> > > > > > > > > > @@ -61,14 +64,46 @@ struct ext_hdr_v0_reg {
> > > > > > > > > >         uint32_t rdata;
> > > > > > > > > >  } __packed;
> > > > > > > > > >
> > > > > > > > > > -#define EXT_HDR_V0_REG_COUNT ((0x1dc - 0x20) / sizeof(struct ext_hdr_v0_reg))
> > > > > > > > > > -
> > > > > > > > > > +/* Structure of the extension header, version 0 (Kirkwood, Dove) */
> > > > > > > > > >  struct ext_hdr_v0 {
> > > > > > > > > > -       uint32_t              offset;
> > > > > > > > > > -       uint8_t               reserved[0x20 - sizeof(uint32_t)];
> > > > > > > > > > -       struct ext_hdr_v0_reg rcfg[EXT_HDR_V0_REG_COUNT];
> > > > > > > > > > -       uint8_t               reserved2[7];
> > > > > > > > > > -       uint8_t               checksum;
> > > > > > > > > > +       /*
> > > > > > > > > > +        * Beware that extension header offsets specified in 88AP510 Functional
> > > > > > > > > > +        * Specifications are relative to the start of the main header, not to
> > > > > > > > > > +        * the start of the extension header itself.
> > > > > > > > > > +        */
> > > > > > > > > > +       uint32_t offset;                /* 0x0-0x3     */
> > > > > > > > > > +       uint8_t  rsvd1[8];              /* 0x4-0xB     */
> > > > > > > > > > +       uint32_t ddrinitdelay;          /* 0xC-0xF     */
> > > > > > > > > > +       uint32_t match_addr;            /* 0x10-0x13   */
> > > > > > > > > > +       uint32_t match_mask;            /* 0x14-0x17   */
> > > > > > > > > > +       uint32_t match_value;           /* 0x18-0x1B   */
> > > > > > > > > > +       uint8_t  ddrwritetype;          /* 0x1C        */
> > > > > > > > > > +       uint8_t  ddrresetmpp;           /* 0x1D        */
> > > > > > > > > > +       uint8_t  ddrclkenmpp;           /* 0x1E        */
> > > > > > > > > > +       uint8_t  ddrmppdelay;           /* 0x1F        */
> > > > > > > > > > +       struct ext_hdr_v0_reg rcfg[55]; /* 0x20-0x1D7  */
> > > > > > > > > > +       uint8_t  rsvd2[7];              /* 0x1D8-0x1DE */
> > > > > > > > > > +       uint8_t  checksum;              /* 0x1DF       */
> > > > > > > > > > +} __packed;
> > > > > > > > > > +
> > > > > > > > > > +/* Structure of the binary code header, version 0 (Dove) */
> > > > > > > > > > +struct binext_hdr_v0 {
> > > > > > > > > > +       uint32_t match_addr;            /* 0x00-0x03  */
> > > > > > > > > > +       uint32_t match_mask;            /* 0x04-0x07  */
> > > > > > > > > > +       uint32_t match_value;           /* 0x08-0x0B  */
> > > > > > > > > > +       uint32_t offset;                /* 0x0C-0x0F  */
> > > > > > > > > > +       uint32_t destaddr;              /* 0x10-0x13  */
> > > > > > > > > > +       uint32_t size;                  /* 0x14-0x17  */
> > > > > > > > > > +       uint32_t execaddr;              /* 0x18-0x1B  */
> > > > > > > > > > +       uint32_t param1;                /* 0x1C-0x1F  */
> > > > > > > > > > +       uint32_t param2;                /* 0x20-0x23  */
> > > > > > > > > > +       uint32_t param3;                /* 0x24-0x27  */
> > > > > > > > > > +       uint32_t param4;                /* 0x28-0x2B  */
> > > > > > > > > > +       uint8_t  params;                /* 0x2C       */
> > > > > > > > > > +       uint8_t  rsvd1;                 /* 0x2D       */
> > > > > > > > > > +       uint8_t  rsvd2;                 /* 0x2E       */
> > > > > > > > > > +       uint8_t  checksum;              /* 0x2F       */
> > > > > > > > > > +       uint8_t  code[2000];            /* 0x30-0x7FF */
> > > > > > > > > >  } __packed;
> > > > > > > > > >
> > > > > > > > > >  /* Structure of the main header, version 1 (Armada 370/XP/375/38x/39x) */
> > > > > > > > > > @@ -213,8 +248,20 @@ static inline size_t kwbheader_size(const void *header)
> > > > > > > > > >         if (kwbimage_version(header) == 0) {
> > > > > > > > > >                 const struct main_hdr_v0 *hdr = header;
> > > > > > > > > >
> > > > > > > > > > +               /*
> > > > > > > > > > +                * First extension header starts immediately after the main
> > > > > > > > > > +                * header without any padding. Between extension headers is
> > > > > > > > > > +                * 0x20 byte padding. There is no padding after the last
> > > > > > > > > > +                * extension header. First binary code header starts immediately
> > > > > > > > > > +                * after the last extension header (or immediately after the
> > > > > > > > > > +                * main header if there is no extension header) without any
> > > > > > > > > > +                * padding. There is no padding between binary code headers and
> > > > > > > > > > +                * neither after the last binary code header.
> > > > > > > > > > +                */
> > > > > > > > > >                 return sizeof(*hdr) +
> > > > > > > > > > -                      hdr->ext ? sizeof(struct ext_hdr_v0) : 0;
> > > > > > > > > > +                      hdr->ext * sizeof(struct ext_hdr_v0) +
> > > > > > > > > > +                      ((hdr->ext > 1) ? (hdr->ext * 0x20) : 0) +
> > > > > > > > > > +                      hdr->binext * sizeof(struct binext_hdr_v0);
> > > > > > > > > >         } else {
> > > > > > > > > >                 const struct main_hdr_v1 *hdr = header;
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > It fixes kwbheader_size() function to returns correct size of the image
> > > > > > > > > > header (with all v0 extensions), so it could help kwboot to convert
> > > > > > > > > > image with non-UART sign to UART version and send it over UART.
> > > > > > > > >
> > > > > > > > > Applied the patch, and make tools gave this error:
> > > > > > > > >
> > > > > > > > > tools/kwbimage.h:255:13: error: ‘const struct main_hdr_v0’ has no
> > > > > > > > > member named ‘binext’
> > > > > > > > >  255 |          hdr->binext * sizeof(struct binext_hdr_v0);
> > > > > > > > >
> > > > > > > > > Thanks,
> > > > > > > > > Tony
> > > > > > > >
> > > > > > > > Ah, I have not sent whole patch...
> > > > > > > >
> > > > > > > > In tools/kwbimage.h for struct main_hdr_v0 { ... } replace
> > > > > > > >
> > > > > > > >   uint16_t rsvd2;
> > > > > > > >
> > > > > > > > by
> > > > > > > >
> > > > > > > >   uint8_t  rsvd2;
> > > > > > > >   uint8_t  binext;
> > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > > > Fixed that, and then I ran it the same way before. In the serial
> > > > > > > console, tell BootROM to set the boot device to UART, and then exit,
> > > > > > > run kwboot.
> > > > > > >
> > > > > > > <BEGIN log>
> > > > > > >
> > > > > > > Bootstrap 2.33>x 0x0E
> > > > > > >
> > > > > > > Terminating...
> > > > > > > Thanks for using picocom
> > > > > > >
> > > > > > > ./kwboot-2022/kwboot.dove -t -p -B 115200 /dev/ttyUSB0 -D ./t5335z/mtd0.t5335z
> > > > > > > kwboot version 2022.04-rc1-00360-g162c22bfbc-dirty
> > > > > > > Patching image boot signature to UART
> > > > > > > Sending boot image header (3072 bytes)...
> > > > > > >   4 % [........................                                              ]
> > > > > > > Done
> > > > > > > Sending boot image data (605104 bytes)...
> > > > > > >   0 % [......................................................................]
> > > > > > > <snip>
> > > > > > >  97 % [......................................................................]
> > > > > > >  99 % [......................................                                ]
> > > > > > > Done
> > > > > > > Finishing transfer
> > > > > > > [Type Ctrl-\ + c to quit]
> > > > > > > <END log>
> > > > > > >
> > > > > > > It hung here. And I have to recycle power to boot again.
> > > > > > >
> > > > > > > Observation: in the previous run with unpatched kwboot, the header
> > > > > > > size was 512 bytes, and the image was 607664 bytes. So this time
> > > > > > > kwboot sends the extension header, too. Not sure if 3072 bytes is
> > > > > > > correct.
> > > > > >
> > > > > > If I parsed that image (file mtd0.t5335z) correctly it has:
> > > > > > * main header  (0x20)
> > > > > > * ext header   (0x1E0)
> > > > > > * 0x20 padding (0x20)
> > > > > > * ext header   (0x1E0)
> > > > > > * bin header   (0x800)
> > > > > > * image data   (0x93bb0)
> > > > > >
> > > > > > So it is: 3072 bytes for headers + 605104 bytes for image data, which
> > > > > > now matches.
> > > > > >
> > > > > > Plus in main header is stored that image data starts at offset 0xc00
> > > > > > which is 3072 bytes.
> > > > > >
> > > > > > Now kwboot showed just that is patching UART signature and no more
> > > > > > alignment, so it should send headers and data without adding any
> > > > > > additional padding or alignment.
> > > > > >
> > > > > > If it still does not work, I do not have an idea...
> > > > > >
> > > > > > It would be required to take U-Boot source code for that board and build
> > > > > > UART image from sources. Maybe UART version needs to be compiled
> > > > > > differently, like it was in past also for Armada builds.
> > > > >
> > > > > To double check I dumped the images.
> > > > >
> > > > > ./kwboot-2022/dumpimage -T kwbimage -p -1  ./t5335z/mtd0.t5335z -o
> > > > > /tmp/mtd0.t5335z.cfg
> > > > > -rw-r--r--  1 root root   1155 Feb 13 15:58 mtd0.t5335z.cfg
> > > >
> > > > This cfg file is incomplete as kwbimage does not read those new
> > > > structures.
> > >
> > > I got your other patch for kwbimage.c compiled in dumpimage. So the
> > > cfg seems OK.
> >
> > No, it is not. There are completely missing information
> > from struct ext_hdr_v0 and struct binext_hdr_v0.
> 
> Got it.
> 
> > > # cat /tmp/mtd0.t5335z.cfg
> > >
> > > BOOT_FROM spi
> > > #SRC_ADDRESS 0xc00
> > > #BLOCK_SIZE 0x93bb0
> > > #DEST_ADDRESS 0x00600000
> > > #EXEC_ADDRESS 0x00690000
> > > DATA 0xd0020104 0x00000000
> > > DATA 0xd0800020 0x00022530
> > > DATA 0xd0800030 0x00022330
> > > DATA 0xd0800050 0x911a00c3
> > > DATA 0xd0800060 0x74780504
> > > DATA 0xd0800190 0xc2005554
> > > DATA 0xd08001c0 0x3694da09
> > > DATA 0xd0800650 0x00130131
> > > DATA 0xd0800660 0x84040200
> > > DATA 0xd0800080 0x00000000
> > > DATA 0xd0800090 0x00080000
> > > DATA 0xd08000f0 0xc0000000
> > > DATA 0xd08001a0 0x20c0c009
> > > DATA 0xd0800280 0x010e0202
> > > DATA 0xd0800760 0x00000201
> > > DATA 0xd0800770 0x0100000a
> > > DATA 0xd0800140 0x20004044
> > > DATA 0xd08001d0 0x177c2779
> > > DATA 0xd08001e0 0x07700330
> > > DATA 0xd08001f0 0x00000033
> > > DATA 0xd0800200 0x0011311c
> > > DATA 0xd0800210 0x00300000
> > > DATA 0xd0800240 0x80000000
> > > DATA 0xd0800510 0x010e0101
> > > DATA 0xd0800230 0x2028006a
> > > DATA 0xd0800e10 0x00280062
> > > DATA 0xd0800e20 0x00280062
> > > DATA 0xd0800e30 0x00280062
> > > DATA 0xd0800100 0x000e0001
> > > DATA 0xd0800110 0x000e0000
> > > DATA 0xd0800120 0x00000001
> > > DATA 0xd00d040c 0x00000008
> > > DATA 0xd0010604 0x00004215
> > > DATA 0xd0020104 0x00000000
> > > DATA 0xd0020104 0x00000000
> > > DATA 0xd0020104 0x00000000
> > > DATA 0xd0020104 0x00000000
> > > DATA 0xd0020104 0x00000000
> > > DATA 0x0 0x0
> > > #RSVD2 0x100r
> >
> > Plus see this incomplete RSVD2.
> 
> Indeed. This is the missing info that I've been looking for with other
> Kirkwood boards, too. I can't wait to see the patch :)

Now I sent it, it is there:
https://patchwork.ozlabs.org/project/uboot/list/?series=286325&state=*

> Thanks,
> Tony
> 
> 
> >
> > >
> > > > There is also bin header for '-p 1' but again kwbimage cannot extract it
> > > > yet.
> > > >
> > > > (Btw, I have WIP patches for this stuff, if you are interesting I can send it)
> > >
> > > Sure, if you want early testing while you're working on it, please send.
> >
> > Ok, later I can prepare patches for you.
> >
> > > Thanks,
> > > Tony
> > >
> > > > > ./kwboot-2022/dumpimage -T kwbimage -p 0   ./t5335z/mtd0.t5335z -o
> > > > > /tmp/mtd0.t5335z.0.img
> > > > > -rw-------  1 root root 605100 Feb 13 15:58 mtd0.t5335z.0.img
> > > > >
> > > > > ./kwboot-2022/mkimage  -l  ./t5335z/mtd0.t5335z
> > > > > Image Type:   MVEBU Boot from spi Image
> > > > > Image version:0
> > > > > Data Size:    605100 Bytes = 590.92 KiB = 0.58 MiB
> > > > > Load Address: 00600000
> > > > > Entry Point:  00690000
> > > > >
> > > > > So it is checked out. The image is 605100 with 4 bytes checksum =
> > > > > 605104. Did I read it correctly? i.e. the dump does not include the
> > > > > checksum?
> > > >
> > > > Yes!
> > > >
> > > > > Seems I got to get to work on that stock UART image!

Do you have some _vendor_ bootable UART image?

Also look into Functional Specifications, chapter 5.7.1 Boot from UART.
There is written: only one extended header is supported. But your SPI
image have two extended headers and one binary header. So maybe you
would have to strip one extended header...


More information about the U-Boot mailing list