From mboxrd@z Thu Jan  1 00:00:00 1970
From: Stefano Babic <sbabic@denx.de>
Date: Mon, 26 Aug 2013 09:14:53 +0200
Subject: [U-Boot] [PATCH V4] ARM: mxs: tools: Add mkimage support for
	MXS bootstream
In-Reply-To: <201308251833.27075.marex@denx.de>
References: <1376955905-6350-1-git-send-email-marex@denx.de>
	<52136BAD.6080903@denx.de> <201308251833.27075.marex@denx.de>
Message-ID: <521B006D.4040005@denx.de>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

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

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================