From mboxrd@z Thu Jan  1 00:00:00 1970
From: Albert ARIBAUD <albert.u.boot@aribaud.net>
Date: Sat, 16 Apr 2011 09:26:39 +0200
Subject: [U-Boot] [PATCH 2/3] cpu9260: update board support
In-Reply-To: <1301848555-14404-2-git-send-email-eric@eukrea.com>
References: <1301848555-14404-1-git-send-email-eric@eukrea.com>
	<1301848555-14404-2-git-send-email-eric@eukrea.com>
Message-ID: <4DA944AF.3020103@aribaud.net>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Le 03/04/2011 18:35, Eric B?nard a ?crit :

> diff --git a/board/eukrea/cpu9260/cpu9260.c b/board/eukrea/cpu9260/cpu9260.c
> index 61b6c33..9ec48a0 100644
> --- a/board/eukrea/cpu9260/cpu9260.c
> +++ b/board/eukrea/cpu9260/cpu9260.c

> @@ -188,26 +175,16 @@ int board_init(void)
>
>   int dram_init(void)
>   {
> -	gd->bd->bi_dram[0].start = PHYS_SDRAM;
> -	if (get_ram_size((long *) PHYS_SDRAM, PHYS_SDRAM_SIZE) !=
> -	    PHYS_SDRAM_SIZE)
> -		return -1;
> -
> -	gd->bd->bi_dram[0].size = PHYS_SDRAM_SIZE;
> +	gd->ram_size = get_ram_size((volatile long *)CONFIG_SYS_SDRAM_BASE,
> +			CONFIG_SYS_SDRAM_SIZE);

Checkpatch warns about the volatile here.

I know the get_ram_size() prototype calls for the volatile attribute, 
but what is the rationale here for this? get_ram_size() just needs the 
RAM base address *value*; if it requires volatile accesses to it, it can 
arrange for these inside its definition. Besides, throughout the code 
base there are 19 instances of get_ram_size() callw where the argument 
is cast to volatile, against 130 where it is not.

Wolfgang et al.: how about removing the 'volatile' qualifier from the 
get_ram_size() prototype?

Eric: if your patch does not cause a warning without the volatile in the 
call, can you update and repost it as V2?

> diff --git a/include/configs/cpu9260.h b/include/configs/cpu9260.h
> index d239423..a8ada2d 100644
> --- a/include/configs/cpu9260.h
> +++ b/include/configs/cpu9260.h

> -#define CONFIG_SYS_NAND_READY_PIN		AT91_PIN_PC13
> -#define CONFIG_SYS_NAND_ENABLE_PIN		AT91_PIN_PC14
> +#define CONFIG_SYS_NAND_READY_PIN		AT91_PIO_PORTC, 13
> +#define CONFIG_SYS_NAND_ENABLE_PIN		AT91_PIO_PORTC, 14

> -#define CONFIG_RED_LED				AT91_PIN_PC11
> -#define CONFIG_GREEN_LED			AT91_PIN_PC12
> -#define CONFIG_YELLOW_LED			AT91_PIN_PC7
> -#define CONFIG_BLUE_LED				AT91_PIN_PC9
> +#define CONFIG_RED_LED				AT91_PIO_PORTC, 11
> +#define CONFIG_GREEN_LED			AT91_PIO_PORTC, 12
> +#define CONFIG_YELLOW_LED			AT91_PIO_PORTC, 7
> +#define CONFIG_BLUE_LED				AT91_PIO_PORTC, 9

Checkpatch considers these errors. This is again a case where we'd want 
it to ignore it... or reconsider this type of macro, which intends to 
expand to several function arguments.

For now I'll ignore these 6 checkpatch errors.

Amicalement,
-- 
Albert.