[U-Boot] [PATCH 1/3] cfi_flash: use specific length types for cword
Stefan Roese
sr at denx.de
Wed Oct 21 12:24:08 CEST 2015
Hi Ryan,
On 21.10.2015 11:54, Ryan Harkin wrote:
> cword.l is an unsigned long int, but it is intended to be 32 bits long.
>
> Unfortunately, it's 64-bits long on a 64-bit system, meaning that a
> 64-bit system cannot use 32-bit wide CFI flash parts.
>
> Similar problems also exist with the other cword sizes.
>
> Also, this patch introduced compiler warnings:
>
> drivers/mtd/cfi_flash.c: In function 'flash_write_cmd':
> drivers/mtd/cfi_flash.c:348:3: warning: format '%lx' expects argument of
> type 'long unsigned int', but argument 4 has type '__u32' [-Wformat=]
> debug ("fwc addr %p cmd %x %8.8lx 32bit x %d bit\n", addr,
> ^
> drivers/mtd/cfi_flash.c: In function 'flash_isequal':
> drivers/mtd/cfi_flash.c:404:3: warning: format '%lx' expects argument of
> type 'long unsigned int', but argument 3 has type '__u32' [-Wformat=]
> debug ("is= %8.8x %8.8lx\n", flash_read32(addr), cword.l);
> ^
>
> I masked these warnings in this patch, however, I am quite sure this is
> the wrong approach.
Why do you think this is the wrong approach? Changing from "long" to
"u32" seems correct to me.
> Signed-off-by: Ryan Harkin <ryan.harkin at linaro.org>
> ---
> drivers/mtd/cfi_flash.c | 4 ++--
> include/mtd/cfi_flash.h | 8 ++++----
> 2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c
> index 50983b8..cee85a9 100644
> --- a/drivers/mtd/cfi_flash.c
> +++ b/drivers/mtd/cfi_flash.c
> @@ -345,7 +345,7 @@ void flash_write_cmd (flash_info_t * info, flash_sect_t sect,
> flash_write16(cword.w, addr);
> break;
> case FLASH_CFI_32BIT:
> - debug ("fwc addr %p cmd %x %8.8lx 32bit x %d bit\n", addr,
> + debug ("fwc addr %p cmd %x %8.8x 32bit x %d bit\n", addr,
> cmd, cword.l,
> info->chipwidth << CFI_FLASH_SHIFT_WIDTH);
> flash_write32(cword.l, addr);
> @@ -401,7 +401,7 @@ static int flash_isequal (flash_info_t * info, flash_sect_t sect,
> retval = (flash_read16(addr) == cword.w);
> break;
> case FLASH_CFI_32BIT:
> - debug ("is= %8.8x %8.8lx\n", flash_read32(addr), cword.l);
> + debug ("is= %8.8x %8.8x\n", flash_read32(addr), cword.l);
> retval = (flash_read32(addr) == cword.l);
> break;
> case FLASH_CFI_64BIT:
> diff --git a/include/mtd/cfi_flash.h b/include/mtd/cfi_flash.h
> index 048b477..cd30619 100644
> --- a/include/mtd/cfi_flash.h
> +++ b/include/mtd/cfi_flash.h
> @@ -105,10 +105,10 @@
> #define NUM_ERASE_REGIONS 4 /* max. number of erase regions */
>
> typedef union {
> - unsigned char c;
> - unsigned short w;
> - unsigned long l;
> - unsigned long long ll;
> + __u8 c;
> + __u16 w;
> + __u32 l;
> + __u64 ll;
> } cfiword_t;
Please use the "uX" types and not the "__uX" types here. Those are
already widely used in this header.
Thanks,
Stefan
More information about the U-Boot
mailing list