[U-Boot] [PATCH][RESEND]: Fix for U-Boot build failure with CONFIG_SYS_NO_FLASH defined for qemu-mips.

Wolfgang Denk
2010-03-20T22:58:51Z

Dear Himanshu Chauhan,

I apologize for the long delay (I could try and claim I was waiting
for anybody else to comment first, but it seems noodu feels addessed).

In message <4B2C7961.3070008@symmetricore.com> you wrote:
> U-Boot hangs with qemu-system-mips with ##unknown flash error.
> Disabling flash using CONFIG_SYS_NO_FLASH breaks the build.
> This patch fixes the issue. Don't know if its okay.
>
> Signed-off-by: Himanshu Chauhan <himanshu@symmetricore.com>
>
> diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c
> index efd6aec..5bd3af0 100644
> --- a/common/cmd_bootm.c
> +++ b/common/cmd_bootm.c
> @@ -76,7 +76,7 @@ extern void bz_internal_error(int);
> static int image_info (unsigned long addr);
> #endif
>
> -#if defined(CONFIG_CMD_IMLS)
> +#if !defined(CONFIG_SYS_NO_FLASH) && defined(CONFIG_CMD_IMLS)

I don't like this approach - CONFIG_CMD_IMLS and CONFIG_SYS_NO_FLASH
should not be combined like that. Assume somebody wants to extend the
functionality of the "imls" command to check for images in NAND flash
(or other similar storage devices) as well.

Disabling NOR flash on a system should be just a matter of system
configuration. If you have no NOR flash on your board (so you select
CONFIG_SYS_NO_FLASH), then why do you select CONFIG_CMD_IMLS which
obviously attempts to access NOR flash?

> diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c
> index 24eb33f..06c7271 100644
> --- a/drivers/mtd/cfi_flash.c
> +++ b/drivers/mtd/cfi_flash.c
> @@ -41,6 +41,7 @@
> #include <environment.h>
> #include <mtd/cfi_flash.h>
>
> +#ifndef CONFIG_SYS_NO_FLASH

Same here. Why do you enable the CFI driver at all when you don't have
NOR flash on your system?

> --- a/include/configs/qemu-mips.h
> +++ b/include/configs/qemu-mips.h
> :q
> @@ -142,6 +142,7 @@
> #define CONFIG_SYS_INIT_SP_OFFSET 0x400000
>
> /* We boot from this flash, selected with dip switch */
> +#define CONFIG_SYS_NO_FLASH
> #define CONFIG_SYS_FLASH_BASE 0xbfc00000
> #define CONFIG_SYS_MAX_FLASH_BANKS 1
> #define CONFIG_SYS_MAX_FLASH_SECT 128
> @@ -149,7 +150,8 @@
> #define CONFIG_FLASH_CFI_DRIVER 1
> #define CONFIG_SYS_FLASH_USE_BUFFER_WRITE 1
>
> -#define CONFIG_ENV_IS_IN_FLASH 1
> +#define CONFIG_ENV_IS_IN_FLASH 0
> +#define CONFIG_ENV_IS_NOWHERE
> #define CONFIG_ENV_ADDR (CONFIG_SYS_FLASH_BASE + CONFIG_SYS_MONITOR_LEN)

Here you should disable NOR related features - like the CFI driver or
the imls command.

Sorry, but I reject this patch.

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
PROGRAM - n. A magic spell cast over a computer allowing it to turn
one's input into error messages.
v. tr. - To engage in a pastime similar to banging one's head against
a wall, but with fewer opportunities for reward.