* [U-Boot] [RFC] cfi_flash and complex address mapping
@ 2009-03-25 16:20 thomas.langer at infineon.com
2009-03-26 10:03 ` Stefan Roese
0 siblings, 1 reply; 4+ messages in thread
From: thomas.langer at infineon.com @ 2009-03-25 16:20 UTC (permalink / raw)
To: u-boot
Hi,
I'm in the process of preparing some code for a new board and want to use the
generic cfi flash driver. The problem is EBU (External Bus Unit) of the chip,
which is internally 32bit.
The access to 8/16bit values are always mapped by the EBU, but in a little
endian way, which is bad on a big endian system: On an interface configured
for 16bit, the following conversion from internal address to bus-address is done:
CPU > external bus
0x00000000 > 0x00000002
0x00000002 > 0x00000000
0x00000004 > 0x00000006
0x00000006 > 0x00000004
If I implement my own flash_read%/flash_write% to do the address mapping, the
data is also swapped. In this functions I cannot decide if the access is for
the flash control or the data to be programmed.
My current solution is the following patch and these definitions in the board
config file (for a 16bit flash):
#define FLASH_FIXUP_ADDR_8(addr) ((void*)((ulong)(addr)^2))
#define FLASH_FIXUP_ADDR_16(addr) ((void*)((ulong)(addr)^2))
My question now is: Would such a change be accepted for the mainline u-boot?
Or does someone has a better idea?
Thanks for your comments,
Thomas
PS: The patch is against u-boot-2009.01
--- a/drivers/mtd/cfi_flash.c
+++ b/drivers/mtd/cfi_flash.c
@@ -176,6 +176,23 @@ flash_info_t flash_info[CFI_MAX_FLASH_BA
#define CONFIG_SYS_FLASH_CFI_WIDTH FLASH_CFI_8BIT
#endif
+/*
+ * Check if address fixup macros are defined, define defaults otherwise
+ */
+#ifndef FLASH_FIXUP_ADDR_8
+#define FLASH_FIXUP_ADDR_8(addr) (addr)
+#endif
+#ifndef FLASH_FIXUP_ADDR_16
+#define FLASH_FIXUP_ADDR_16(addr) (addr)
+#endif
+#ifndef FLASH_FIXUP_ADDR_32
+#define FLASH_FIXUP_ADDR_32(addr) (addr)
+#endif
+#ifndef FLASH_FIXUP_ADDR_64
+#define FLASH_FIXUP_ADDR_64(addr) (addr)
+#endif
+
+
/* CFI standard query structure */
struct cfi_qry {
u8 qry[3];
@@ -392,9 +409,9 @@ static inline uchar flash_read_uchar (fl
cp = flash_map (info, 0, offset);
#if defined(__LITTLE_ENDIAN) || defined(CONFIG_SYS_WRITE_SWAPPED_DATA)
- retval = flash_read8(cp);
+ retval = flash_read8(FLASH_FIXUP_ADDR_8(cp));
#else
- retval = flash_read8(cp + info->portwidth - 1);
+ retval = flash_read8(FLASH_FIXUP_ADDR_8(cp) + info->portwidth - 1);
#endif
flash_unmap (info, 0, offset, cp);
return retval;
@@ -408,7 +425,7 @@ static inline ushort flash_read_word (fl
ushort *addr, retval;
addr = flash_map (info, 0, offset);
- retval = flash_read16 (addr);
+ retval = flash_read16 (FLASH_FIXUP_ADDR_16(addr));
flash_unmap (info, 0, offset, addr);
return retval;
}
@@ -433,19 +450,28 @@ static ulong flash_read_long (flash_info
debug ("long addr is at %p info->portwidth = %d\n", addr,
info->portwidth);
for (x = 0; x < 4 * info->portwidth; x++) {
- debug ("addr[%x] = 0x%x\n", x, flash_read8(addr + x));
+ debug ("addr[%x] = 0x%x\n", x,
+ flash_read8(FLASH_FIXUP_ADDR_32(addr) + x));
}
#endif
#if defined(__LITTLE_ENDIAN) || defined(CONFIG_SYS_WRITE_SWAPPED_DATA)
- retval = ((flash_read8(addr) << 16) |
- (flash_read8(addr + info->portwidth) << 24) |
- (flash_read8(addr + 2 * info->portwidth)) |
- (flash_read8(addr + 3 * info->portwidth) << 8));
+ retval = ((flash_read8(FLASH_FIXUP_ADDR_8
+ (addr) << 16) |
+ (flash_read8(FLASH_FIXUP_ADDR_8
+ (addr + info->portwidth)) << 24) |
+ (flash_read8(FLASH_FIXUP_ADDR_8
+ (addr + 2 * info->portwidth))) |
+ (flash_read8(FLASH_FIXUP_ADDR_8
+ (addr + 3 * info->portwidth)) << 8));
#else
- retval = ((flash_read8(addr + 2 * info->portwidth - 1) << 24) |
- (flash_read8(addr + info->portwidth - 1) << 16) |
- (flash_read8(addr + 4 * info->portwidth - 1) << 8) |
- (flash_read8(addr + 3 * info->portwidth - 1)));
+ retval = ((flash_read8(FLASH_FIXUP_ADDR_8
+ (addr + 2 * info->portwidth - 1)) << 24) |
+ (flash_read8(FLASH_FIXUP_ADDR_8
+ (addr + info->portwidth - 1)) << 16) |
+ (flash_read8(FLASH_FIXUP_ADDR_8
+ (addr + 4 * info->portwidth - 1)) << 8) |
+ (flash_read8(FLASH_FIXUP_ADDR_8
+ (addr + 3 * info->portwidth - 1))));
#endif
flash_unmap(info, sect, offset, addr);
@@ -466,21 +492,22 @@ static void flash_write_cmd (flash_info_
flash_make_cmd (info, cmd, &cword);
switch (info->portwidth) {
case FLASH_CFI_8BIT:
- debug ("fwc addr %p cmd %x %x 8bit x %d bit\n", addr, cmd,
- cword.c, info->chipwidth << CFI_FLASH_SHIFT_WIDTH);
- flash_write8(cword.c, addr);
+ debug ("fwc addr %p cmd %x %x 8bit x %d bit\n",
+ FLASH_FIXUP_ADDR_8(addr), cmd, cword.c,
+ info->chipwidth << CFI_FLASH_SHIFT_WIDTH);
+ flash_write8(cword.c, FLASH_FIXUP_ADDR_8(addr));
break;
case FLASH_CFI_16BIT:
- debug ("fwc addr %p cmd %x %4.4x 16bit x %d bit\n", addr,
- cmd, cword.w,
+ debug ("fwc addr %p cmd %x %4.4x 16bit x %d bit\n",
+ FLASH_FIXUP_ADDR_16(addr), cmd, cword.w,
info->chipwidth << CFI_FLASH_SHIFT_WIDTH);
- flash_write16(cword.w, addr);
+ flash_write16(cword.w, FLASH_FIXUP_ADDR_16(addr));
break;
case FLASH_CFI_32BIT:
- debug ("fwc addr %p cmd %x %8.8lx 32bit x %d bit\n", addr,
- cmd, cword.l,
+ debug ("fwc addr %p cmd %x %8.8lx 32bit x %d bit\n",
+ FLASH_FIXUP_ADDR_32(addr), cmd, cword.l,
info->chipwidth << CFI_FLASH_SHIFT_WIDTH);
- flash_write32(cword.l, addr);
+ flash_write32(cword.l, FLASH_FIXUP_ADDR_32(addr));
break;
case FLASH_CFI_64BIT:
#ifdef DEBUG
@@ -490,11 +517,11 @@ static void flash_write_cmd (flash_info_
print_longlong (str, cword.ll);
debug ("fwrite addr %p cmd %x %s 64 bit x %d bit\n",
- addr, cmd, str,
+ FLASH_FIXUP_ADDR_64(addr), cmd, str,
info->chipwidth << CFI_FLASH_SHIFT_WIDTH);
}
#endif
- flash_write64(cword.ll, addr);
+ flash_write64(cword.ll, FLASH_FIXUP_ADDR_64(addr));
break;
}
@@ -525,16 +552,19 @@ static int flash_isequal (flash_info_t *
debug ("is= cmd %x(%c) addr %p ", cmd, cmd, addr);
switch (info->portwidth) {
case FLASH_CFI_8BIT:
- debug ("is= %x %x\n", flash_read8(addr), cword.c);
- retval = (flash_read8(addr) == cword.c);
+ debug ("is= %x %x\n",
+ flash_read8(FLASH_FIXUP_ADDR_8(addr)), cword.c);
+ retval = (flash_read8(FLASH_FIXUP_ADDR_8(addr)) == cword.c);
break;
case FLASH_CFI_16BIT:
- debug ("is= %4.4x %4.4x\n", flash_read16(addr), cword.w);
- retval = (flash_read16(addr) == cword.w);
+ debug ("is= %4.4x %4.4x\n",
+ flash_read16(FLASH_FIXUP_ADDR_16(addr)), cword.w);
+ retval = (flash_read16(FLASH_FIXUP_ADDR_16(addr)) == cword.w);
break;
case FLASH_CFI_32BIT:
- debug ("is= %8.8x %8.8lx\n", flash_read32(addr), cword.l);
- retval = (flash_read32(addr) == cword.l);
+ debug ("is= %8.8x %8.8lx\n",
+ flash_read32(FLASH_FIXUP_ADDR_32(addr)), cword.l);
+ retval = (flash_read32(FLASH_FIXUP_ADDR_32(addr)) == cword.l);
break;
case FLASH_CFI_64BIT:
#ifdef DEBUG
@@ -542,12 +572,13 @@ static int flash_isequal (flash_info_t *
char str1[20];
char str2[20];
- print_longlong (str1, flash_read64(addr));
+ print_longlong (str1, flash_read64(FLASH_FIXUP_ADDR_64
+ (addr)));
print_longlong (str2, cword.ll);
debug ("is= %s %s\n", str1, str2);
}
#endif
- retval = (flash_read64(addr) == cword.ll);
+ retval = (flash_read64(FLASH_FIXUP_ADDR_64(addr)) == cword.ll);
break;
default:
retval = 0;
@@ -571,16 +602,20 @@ static int flash_isset (flash_info_t * i
flash_make_cmd (info, cmd, &cword);
switch (info->portwidth) {
case FLASH_CFI_8BIT:
- retval = ((flash_read8(addr) & cword.c) == cword.c);
+ retval = ((flash_read8(FLASH_FIXUP_ADDR_8(addr))
+ & cword.c) == cword.c);
break;
case FLASH_CFI_16BIT:
- retval = ((flash_read16(addr) & cword.w) == cword.w);
+ retval = ((flash_read16(FLASH_FIXUP_ADDR_16(addr))
+ & cword.w) == cword.w);
break;
case FLASH_CFI_32BIT:
- retval = ((flash_read32(addr) & cword.l) == cword.l);
+ retval = ((flash_read32(FLASH_FIXUP_ADDR_32(addr))
+ & cword.l) == cword.l);
break;
case FLASH_CFI_64BIT:
- retval = ((flash_read64(addr) & cword.ll) == cword.ll);
+ retval = ((flash_read64(FLASH_FIXUP_ADDR_64(addr))
+ & cword.ll) == cword.ll);
break;
default:
retval = 0;
@@ -604,17 +639,22 @@ static int flash_toggle (flash_info_t *
flash_make_cmd (info, cmd, &cword);
switch (info->portwidth) {
case FLASH_CFI_8BIT:
- retval = flash_read8(addr) != flash_read8(addr);
+ retval = flash_read8(FLASH_FIXUP_ADDR_8(addr)) !=
+ flash_read8(FLASH_FIXUP_ADDR_8(addr));
break;
case FLASH_CFI_16BIT:
- retval = flash_read16(addr) != flash_read16(addr);
+ retval = flash_read16(FLASH_FIXUP_ADDR_16(addr)) !=
+ flash_read16(FLASH_FIXUP_ADDR_16(addr));
break;
case FLASH_CFI_32BIT:
- retval = flash_read32(addr) != flash_read32(addr);
+ retval = flash_read32(FLASH_FIXUP_ADDR_32(addr)) !=
+ flash_read32(FLASH_FIXUP_ADDR_32(addr));
break;
case FLASH_CFI_64BIT:
- retval = ( (flash_read32( addr ) != flash_read32( addr )) ||
- (flash_read32(addr+4) != flash_read32(addr+4)) );
+ retval = ( (flash_read32(FLASH_FIXUP_ADDR_64( addr )) !=
+ flash_read32(FLASH_FIXUP_ADDR_64( addr ))) ||
+ (flash_read32(FLASH_FIXUP_ADDR_64(addr+4)) !=
+ flash_read32(FLASH_FIXUP_ADDR_64(addr+4))) );
break;
default:
retval = 0;
^ permalink raw reply [flat|nested] 4+ messages in thread
* [U-Boot] [RFC] cfi_flash and complex address mapping
2009-03-25 16:20 [U-Boot] [RFC] cfi_flash and complex address mapping thomas.langer at infineon.com
@ 2009-03-26 10:03 ` Stefan Roese
2009-03-26 14:25 ` Wolfgang Denk
0 siblings, 1 reply; 4+ messages in thread
From: Stefan Roese @ 2009-03-26 10:03 UTC (permalink / raw)
To: u-boot
On Wednesday 25 March 2009, thomas.langer at infineon.com wrote:
> I'm in the process of preparing some code for a new board and want to use
> the generic cfi flash driver. The problem is EBU (External Bus Unit) of the
> chip, which is internally 32bit.
Which SoC is this btw?
> The access to 8/16bit values are always mapped by the EBU, but in a little
> endian way, which is bad on a big endian system: On an interface configured
> for 16bit, the following conversion from internal address to bus-address is
> done: CPU > external bus
> 0x00000000 > 0x00000002
> 0x00000002 > 0x00000000
> 0x00000004 > 0x00000006
> 0x00000006 > 0x00000004
Arrgh!
> If I implement my own flash_read%/flash_write% to do the address mapping,
> the data is also swapped. In this functions I cannot decide if the access
> is for the flash control or the data to be programmed.
Yes, too bad that this doesn't work for you.
> My current solution is the following patch and these definitions in the
> board config file (for a 16bit flash):
>
> #define FLASH_FIXUP_ADDR_8(addr) ((void*)((ulong)(addr)^2))
> #define FLASH_FIXUP_ADDR_16(addr) ((void*)((ulong)(addr)^2))
>
> My question now is: Would such a change be accepted for the mainline
> u-boot? Or does someone has a better idea?
Yes, I think this could be accepted. The overall impact on the driver is not
too big. Let's see if others have some complaints about it. If not, I'll
accept you patch after you changed some minor details.
Please find some more comments below.
> Thanks for your comments,
> Thomas
>
> PS: The patch is against u-boot-2009.01
You should add your Signed-off-by line to this commit text. Best you generate
and send your patches using the git tools.
>
> --- a/drivers/mtd/cfi_flash.c
> +++ b/drivers/mtd/cfi_flash.c
> @@ -176,6 +176,23 @@ flash_info_t flash_info[CFI_MAX_FLASH_BA
> #define CONFIG_SYS_FLASH_CFI_WIDTH FLASH_CFI_8BIT
> #endif
>
> +/*
> + * Check if address fixup macros are defined, define defaults otherwise
> + */
> +#ifndef FLASH_FIXUP_ADDR_8
> +#define FLASH_FIXUP_ADDR_8(addr) (addr)
> +#endif
> +#ifndef FLASH_FIXUP_ADDR_16
> +#define FLASH_FIXUP_ADDR_16(addr) (addr)
> +#endif
> +#ifndef FLASH_FIXUP_ADDR_32
> +#define FLASH_FIXUP_ADDR_32(addr) (addr)
> +#endif
> +#ifndef FLASH_FIXUP_ADDR_64
> +#define FLASH_FIXUP_ADDR_64(addr) (addr)
> +#endif
> +
> +
> /* CFI standard query structure */
> struct cfi_qry {
> u8 qry[3];
> @@ -392,9 +409,9 @@ static inline uchar flash_read_uchar (fl
>
> cp = flash_map (info, 0, offset);
> #if defined(__LITTLE_ENDIAN) || defined(CONFIG_SYS_WRITE_SWAPPED_DATA)
> - retval = flash_read8(cp);
> + retval = flash_read8(FLASH_FIXUP_ADDR_8(cp));
> #else
> - retval = flash_read8(cp + info->portwidth - 1);
> + retval = flash_read8(FLASH_FIXUP_ADDR_8(cp) + info->portwidth - 1);
> #endif
> flash_unmap (info, 0, offset, cp);
> return retval;
> @@ -408,7 +425,7 @@ static inline ushort flash_read_word (fl
> ushort *addr, retval;
>
> addr = flash_map (info, 0, offset);
> - retval = flash_read16 (addr);
> + retval = flash_read16 (FLASH_FIXUP_ADDR_16(addr));
> flash_unmap (info, 0, offset, addr);
> return retval;
> }
> @@ -433,19 +450,28 @@ static ulong flash_read_long (flash_info
> debug ("long addr is at %p info->portwidth = %d\n", addr,
> info->portwidth);
> for (x = 0; x < 4 * info->portwidth; x++) {
> - debug ("addr[%x] = 0x%x\n", x, flash_read8(addr + x));
> + debug ("addr[%x] = 0x%x\n", x,
> + flash_read8(FLASH_FIXUP_ADDR_32(addr) + x));
> }
> #endif
> #if defined(__LITTLE_ENDIAN) || defined(CONFIG_SYS_WRITE_SWAPPED_DATA)
> - retval = ((flash_read8(addr) << 16) |
> - (flash_read8(addr + info->portwidth) << 24) |
> - (flash_read8(addr + 2 * info->portwidth)) |
> - (flash_read8(addr + 3 * info->portwidth) << 8));
> + retval = ((flash_read8(FLASH_FIXUP_ADDR_8
> + (addr) << 16) |
I don't like this line split above and below, where the parameter for
FLASH_FIXUP_ADDR_8 is moved into the 2nd line. But I understand that you did
this because of the line length restrictions, correct? Perhaps we should
introduce a local variable for info->portwidth (e.g. pw) and the statement
will still fit into one line? Or at least the parameter for FLASH_FIXUP...
> + (flash_read8(FLASH_FIXUP_ADDR_8
> + (addr + info->portwidth)) << 24) |
> + (flash_read8(FLASH_FIXUP_ADDR_8
> + (addr + 2 * info->portwidth))) |
> + (flash_read8(FLASH_FIXUP_ADDR_8
> + (addr + 3 * info->portwidth)) << 8));
> #else
> - retval = ((flash_read8(addr + 2 * info->portwidth - 1) << 24) |
> - (flash_read8(addr + info->portwidth - 1) << 16) |
> - (flash_read8(addr + 4 * info->portwidth - 1) << 8) |
> - (flash_read8(addr + 3 * info->portwidth - 1)));
> + retval = ((flash_read8(FLASH_FIXUP_ADDR_8
> + (addr + 2 * info->portwidth - 1)) << 24) |
> + (flash_read8(FLASH_FIXUP_ADDR_8
> + (addr + info->portwidth - 1)) << 16) |
> + (flash_read8(FLASH_FIXUP_ADDR_8
> + (addr + 4 * info->portwidth - 1)) << 8) |
> + (flash_read8(FLASH_FIXUP_ADDR_8
> + (addr + 3 * info->portwidth - 1))));
> #endif
> flash_unmap(info, sect, offset, addr);
>
> @@ -466,21 +492,22 @@ static void flash_write_cmd (flash_info_
> flash_make_cmd (info, cmd, &cword);
> switch (info->portwidth) {
> case FLASH_CFI_8BIT:
> - debug ("fwc addr %p cmd %x %x 8bit x %d bit\n", addr, cmd,
> - cword.c, info->chipwidth << CFI_FLASH_SHIFT_WIDTH);
> - flash_write8(cword.c, addr);
> + debug ("fwc addr %p cmd %x %x 8bit x %d bit\n",
> + FLASH_FIXUP_ADDR_8(addr), cmd, cword.c,
> + info->chipwidth << CFI_FLASH_SHIFT_WIDTH);
> + flash_write8(cword.c, FLASH_FIXUP_ADDR_8(addr));
> break;
> case FLASH_CFI_16BIT:
> - debug ("fwc addr %p cmd %x %4.4x 16bit x %d bit\n", addr,
> - cmd, cword.w,
> + debug ("fwc addr %p cmd %x %4.4x 16bit x %d bit\n",
> + FLASH_FIXUP_ADDR_16(addr), cmd, cword.w,
> info->chipwidth << CFI_FLASH_SHIFT_WIDTH);
> - flash_write16(cword.w, addr);
> + flash_write16(cword.w, FLASH_FIXUP_ADDR_16(addr));
> break;
> case FLASH_CFI_32BIT:
> - debug ("fwc addr %p cmd %x %8.8lx 32bit x %d bit\n", addr,
> - cmd, cword.l,
> + debug ("fwc addr %p cmd %x %8.8lx 32bit x %d bit\n",
> + FLASH_FIXUP_ADDR_32(addr), cmd, cword.l,
> info->chipwidth << CFI_FLASH_SHIFT_WIDTH);
> - flash_write32(cword.l, addr);
> + flash_write32(cword.l, FLASH_FIXUP_ADDR_32(addr));
> break;
> case FLASH_CFI_64BIT:
> #ifdef DEBUG
> @@ -490,11 +517,11 @@ static void flash_write_cmd (flash_info_
> print_longlong (str, cword.ll);
>
> debug ("fwrite addr %p cmd %x %s 64 bit x %d bit\n",
> - addr, cmd, str,
> + FLASH_FIXUP_ADDR_64(addr), cmd, str,
> info->chipwidth << CFI_FLASH_SHIFT_WIDTH);
> }
> #endif
> - flash_write64(cword.ll, addr);
> + flash_write64(cword.ll, FLASH_FIXUP_ADDR_64(addr));
> break;
> }
>
> @@ -525,16 +552,19 @@ static int flash_isequal (flash_info_t *
> debug ("is= cmd %x(%c) addr %p ", cmd, cmd, addr);
> switch (info->portwidth) {
> case FLASH_CFI_8BIT:
> - debug ("is= %x %x\n", flash_read8(addr), cword.c);
> - retval = (flash_read8(addr) == cword.c);
> + debug ("is= %x %x\n",
> + flash_read8(FLASH_FIXUP_ADDR_8(addr)), cword.c);
> + retval = (flash_read8(FLASH_FIXUP_ADDR_8(addr)) == cword.c);
> break;
> case FLASH_CFI_16BIT:
> - debug ("is= %4.4x %4.4x\n", flash_read16(addr), cword.w);
> - retval = (flash_read16(addr) == cword.w);
> + debug ("is= %4.4x %4.4x\n",
> + flash_read16(FLASH_FIXUP_ADDR_16(addr)), cword.w);
> + retval = (flash_read16(FLASH_FIXUP_ADDR_16(addr)) == cword.w);
> break;
> case FLASH_CFI_32BIT:
> - debug ("is= %8.8x %8.8lx\n", flash_read32(addr), cword.l);
> - retval = (flash_read32(addr) == cword.l);
> + debug ("is= %8.8x %8.8lx\n",
> + flash_read32(FLASH_FIXUP_ADDR_32(addr)), cword.l);
> + retval = (flash_read32(FLASH_FIXUP_ADDR_32(addr)) == cword.l);
> break;
> case FLASH_CFI_64BIT:
> #ifdef DEBUG
> @@ -542,12 +572,13 @@ static int flash_isequal (flash_info_t *
> char str1[20];
> char str2[20];
>
> - print_longlong (str1, flash_read64(addr));
> + print_longlong (str1, flash_read64(FLASH_FIXUP_ADDR_64
> + (addr)));
> print_longlong (str2, cword.ll);
> debug ("is= %s %s\n", str1, str2);
> }
> #endif
> - retval = (flash_read64(addr) == cword.ll);
> + retval = (flash_read64(FLASH_FIXUP_ADDR_64(addr)) == cword.ll);
> break;
> default:
> retval = 0;
> @@ -571,16 +602,20 @@ static int flash_isset (flash_info_t * i
> flash_make_cmd (info, cmd, &cword);
> switch (info->portwidth) {
> case FLASH_CFI_8BIT:
> - retval = ((flash_read8(addr) & cword.c) == cword.c);
> + retval = ((flash_read8(FLASH_FIXUP_ADDR_8(addr))
> + & cword.c) == cword.c);
> break;
> case FLASH_CFI_16BIT:
> - retval = ((flash_read16(addr) & cword.w) == cword.w);
> + retval = ((flash_read16(FLASH_FIXUP_ADDR_16(addr))
> + & cword.w) == cword.w);
> break;
> case FLASH_CFI_32BIT:
> - retval = ((flash_read32(addr) & cword.l) == cword.l);
> + retval = ((flash_read32(FLASH_FIXUP_ADDR_32(addr))
> + & cword.l) == cword.l);
> break;
> case FLASH_CFI_64BIT:
> - retval = ((flash_read64(addr) & cword.ll) == cword.ll);
> + retval = ((flash_read64(FLASH_FIXUP_ADDR_64(addr))
> + & cword.ll) == cword.ll);
> break;
> default:
> retval = 0;
> @@ -604,17 +639,22 @@ static int flash_toggle (flash_info_t *
> flash_make_cmd (info, cmd, &cword);
> switch (info->portwidth) {
> case FLASH_CFI_8BIT:
> - retval = flash_read8(addr) != flash_read8(addr);
> + retval = flash_read8(FLASH_FIXUP_ADDR_8(addr)) !=
> + flash_read8(FLASH_FIXUP_ADDR_8(addr));
> break;
> case FLASH_CFI_16BIT:
> - retval = flash_read16(addr) != flash_read16(addr);
> + retval = flash_read16(FLASH_FIXUP_ADDR_16(addr)) !=
> + flash_read16(FLASH_FIXUP_ADDR_16(addr));
> break;
> case FLASH_CFI_32BIT:
> - retval = flash_read32(addr) != flash_read32(addr);
> + retval = flash_read32(FLASH_FIXUP_ADDR_32(addr)) !=
> + flash_read32(FLASH_FIXUP_ADDR_32(addr));
> break;
> case FLASH_CFI_64BIT:
> - retval = ( (flash_read32( addr ) != flash_read32( addr )) ||
> - (flash_read32(addr+4) != flash_read32(addr+4)) );
> + retval = ( (flash_read32(FLASH_FIXUP_ADDR_64( addr )) !=
> + flash_read32(FLASH_FIXUP_ADDR_64( addr ))) ||
> + (flash_read32(FLASH_FIXUP_ADDR_64(addr+4)) !=
> + flash_read32(FLASH_FIXUP_ADDR_64(addr+4))) );
Please remove the spaces before the ")" and after the "(". They don't make
sense now anymore.
And please CC me on CFI related patches. This makes it easier for me not to
miss anything.
Thanks.
Best regards,
Stefan
=====================================================================
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office at denx.de
=====================================================================
^ permalink raw reply [flat|nested] 4+ messages in thread
* [U-Boot] [RFC] cfi_flash and complex address mapping
2009-03-26 10:03 ` Stefan Roese
@ 2009-03-26 14:25 ` Wolfgang Denk
2009-03-26 14:39 ` thomas.langer at infineon.com
0 siblings, 1 reply; 4+ messages in thread
From: Wolfgang Denk @ 2009-03-26 14:25 UTC (permalink / raw)
To: u-boot
Dear Stefan Roese,
In message <200903261103.28452.sr@denx.de> you wrote:
>
> > #define FLASH_FIXUP_ADDR_8(addr) ((void*)((ulong)(addr)^2))
> > #define FLASH_FIXUP_ADDR_16(addr) ((void*)((ulong)(addr)^2))
...
> Yes, I think this could be accepted. The overall impact on the driver is not
> too big. Let's see if others have some complaints about it. If not, I'll
> accept you patch after you changed some minor details.
I think that should be an inline function instead of a macro.
Best regards,
Wolfgang Denk
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Live long and prosper.
-- Spock, "Amok Time", stardate 3372.7
^ permalink raw reply [flat|nested] 4+ messages in thread
* [U-Boot] [RFC] cfi_flash and complex address mapping
2009-03-26 14:25 ` Wolfgang Denk
@ 2009-03-26 14:39 ` thomas.langer at infineon.com
0 siblings, 0 replies; 4+ messages in thread
From: thomas.langer at infineon.com @ 2009-03-26 14:39 UTC (permalink / raw)
To: u-boot
> In message <200903261103.28452.sr@denx.de> you wrote:
> >
> > > #define FLASH_FIXUP_ADDR_8(addr) ((void*)((ulong)(addr)^2))
> > > #define FLASH_FIXUP_ADDR_16(addr) ((void*)((ulong)(addr)^2))
> ...
> > Yes, I think this could be accepted. The overall impact on
> the driver is not
> > too big. Let's see if others have some complaints about it.
> If not, I'll
> > accept you patch after you changed some minor details.
>
> I think that should be an inline function instead of a macro.
>
> Best regards,
>
> Wolfgang Denk
Thanks for the comments. I will take it into account before
sending the complete patch series for the board(s).
But I have to do some more cleanups before I can do that.
I just wanted to check if this is the right way to go.
Best regards,
Thomas Langer
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2009-03-26 14:39 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-03-25 16:20 [U-Boot] [RFC] cfi_flash and complex address mapping thomas.langer at infineon.com
2009-03-26 10:03 ` Stefan Roese
2009-03-26 14:25 ` Wolfgang Denk
2009-03-26 14:39 ` thomas.langer at infineon.com
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox