Re: [U-Boot] [PATCH V4] ARM: mxs: tools: Add mkimage support for MXS bootstream

Hi Marek,

On 25/08/2013 18:33, Marek Vasut wrote:

...

...

•  MODE string_mode
  

•  - Restart the CPU and start booting from device specified by the
  

• "string_mode" argument. The "string_mode" differs for each CPU
• and can be:

•     i.MX23, string_mode = USB/I2C/SPI1_FLASH/SPI2_FLASH/NAND_BCH
  

•                           JTAG/SPI3_EEPROM/SD_SSP0/SD_SSP1
  

•     i.MX28, string_mode = USB/I2C/SPI2_FLASH/SPI3_FLASH/NAND_BCH
  

•                           JTAG/SPI2_EEPROM/SD_SSP0/SD_SSP1
  

Is this equivalent to the "bmode" command on i.MX5/6 ?

BMODE command? How does that one work?

It sets for i.MX (MX5/MX6) from which source the BootROM must read the image. We can say it is a soft way to overwrite the boot pin configuration without having to write into fuses, useful to test different boot conditions.

...

...

• Compression Types

diff --git a/tools/Makefile b/tools/Makefile index 33fad6b..bbae5a2 100644 --- a/tools/Makefile +++ b/tools/Makefile @@ -83,6 +83,7 @@ NOPED_OBJ_FILES-y += aisimage.o

NOPED_OBJ_FILES-y += kwbimage.o NOPED_OBJ_FILES-y += pblimage.o NOPED_OBJ_FILES-y += imximage.o

+NOPED_OBJ_FILES-y += mxsimage.o

Can you reorder the list ?

How? It's out of order already.

I know - it would be nice if the list was in order again, at least for the components of mkimage (NOPED_OBJ_FILES-).

...

...

• • CRC32
• */

+static uint32_t crc32(uint8_t *data, uint32_t len) +{

• const uint32_t poly = 0x04c11db7;

Comparing this polinomial with the one in lib/crc32.c, they are identical. The crc32 function you define here should give the same result as our old crc32 (global) in lib/crc32.c. Then, can you drop this one and use the already implemented function ?

As I do not want to diverge more than necessary, this can be done in subsequent patch.

However this adds a duplication in the code, and using the global crc should be quite straightforward.

[...]

...

...

• /*

• * Append the command to the last section.
  

• */
  

• if (!sctx->cmd_head) {

• sctx->cmd_head = cctx;
  

• sctx->cmd_tail = cctx;
  

• } else {

• sctx->cmd_tail->cmd = cctx;
  

• sctx->cmd_tail = cctx;
  

• }

This append stuff can be factorized at least with a macro - it is repeated again and again in all commands.

This can (read : shall) be even replaced by include/linked_lists.h , but not in this patch.

Ok, that's fine.

Best regards, Stefano Babic