public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
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

  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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox