Re: [U-Boot-Users] [PATCH] cfi_flash: fix flash on BE machines with CFG_WRITE_SWAPPED_DATA

In message 20080715211240.GA16786@www.tglx.de you wrote:

That command needs to be in little endian format on BE machines with CFG_WRITE_SWAPPED_DATA. Without this patch, the command 0xf0 gets saved on stack as 0x00 00 00 f0 and 0x00 gets written into

On stack?

@@ -306,16 +306,21 @@ static void flash_make_cmd(flash_info_t *info, u32 cmd, void *cmdbuf) int i; int cword_offset; int cp_offset;

• int cmd_le; uchar val; uchar *cp = (uchar *) cmdbuf;

• cmd_le = cpu_to_le32(cmd);

This bloats code size for cases where it is not needed.

You seem to be focussing one one specific system configuration only, but there are 4 possible situations that need to be handled:

- BE without CFG_WRITE_SWAPPED_DATA - BE with CFG_WRITE_SWAPPED_DATA - LE without CFG_WRITE_SWAPPED_DATA - LE with CFG_WRITE_SWAPPED_DATA

I don't think you handle all for of them correctly.

for (i = info->portwidth; i > 0; i--){ cword_offset = (info->portwidth-i)%info->chipwidth; #if defined(__LITTLE_ENDIAN) || defined(CFG_WRITE_SWAPPED_DATA) cp_offset = info->portwidth - i;

• val = *((uchar*)&cmd + cword_offset);
  

• val = *((uchar*)&cmd_le + cword_offset);
  

#else cp_offset = i - 1;

• /* we rely here on the fact, that cmd _has_ to be in BE
  

•  * order because we are not __LITTLE_ENDIAN
  

•  */
  

  val = *((uchar*)&cmd + sizeof(u32) - cword_offset - 1);

Hm... we are BE, because we are not LE.

Sounds kind of obvious to me?

[Nitpick: incorrect multiline comment style]

Best regards,

Wolfgang Denk