From: Stefan Roese <sr@denx.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 1/3] cfi_flash: use specific length types for cword
Date: Wed, 21 Oct 2015 12:24:08 +0200 [thread overview]
Message-ID: <562767C8.1000702@denx.de> (raw)
In-Reply-To: <1445421247-15013-2-git-send-email-ryan.harkin@linaro.org>
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@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
next prev parent reply other threads:[~2015-10-21 10:24 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-10-21 9:54 [U-Boot] [PATCH 0/3] vexpress64 flash support Ryan Harkin
2015-10-21 9:54 ` [U-Boot] [PATCH 1/3] cfi_flash: use specific length types for cword Ryan Harkin
2015-10-21 10:24 ` Stefan Roese [this message]
2015-10-21 11:34 ` Ryan Harkin
2015-10-21 12:40 ` Stefan Roese
2015-10-21 12:49 ` Ryan Harkin
2015-10-21 13:48 ` Ryan Harkin
2015-10-21 14:00 ` Stefan Roese
2015-10-21 9:54 ` [U-Boot] [PATCH 2/3] vexpress64: remove #error Ryan Harkin
2015-10-27 11:36 ` Linus Walleij
2015-10-27 12:29 ` Ryan Harkin
2015-11-11 16:11 ` Ryan Harkin
2015-11-17 13:36 ` Linus Walleij
2015-10-21 9:54 ` [U-Boot] [PATCH 3/3] vexpress64: store env in flash Ryan Harkin
2015-10-27 11:49 ` Linus Walleij
2015-10-27 12:49 ` Ryan Harkin
2015-10-27 21:39 ` Linus Walleij
2015-10-28 15:07 ` Ryan Harkin
2015-10-28 15:10 ` Linus Walleij
2015-10-28 16:06 ` Ryan Harkin
2015-10-28 21:43 ` Linus Walleij
2015-10-28 22:07 ` Tom Rini
2015-10-29 9:17 ` Ryan Harkin
2015-10-29 14:50 ` Tom Rini
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=562767C8.1000702@denx.de \
--to=sr@denx.de \
--cc=u-boot@lists.denx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.