From mboxrd@z Thu Jan  1 00:00:00 1970
From: Amit Virdi <amit.virdi@st.com>
Date: Mon, 12 Mar 2012 18:00:01 +0530
Subject: [U-Boot] [PATCH 07/25] SPEAr: Add basic arch related support
 for SPEAr SoCs
In-Reply-To: <201203071449.42587.sr@denx.de>
References: <1331121854-20494-1-git-send-email-amit.virdi@st.com>
	<1331121854-20494-8-git-send-email-amit.virdi@st.com>
	<201203071449.42587.sr@denx.de>
Message-ID: <4F5DEC49.4070905@st.com>
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

Hello Stefan,

>> + */
>> +
>> +#include<common.h>
>> +#include<asm/io.h>
>> +#include<asm/arch/hardware.h>
>> +#include<asm/arch/spr_misc.h>
>> +
>> +#ifdef CONFIG_ARCH_CPU_INIT
>
> Why not remove this #ifdef here and make compile it in unconditionally? It is
> needed for each SPEAr board, right?
>

Yes, we don't need this flag.

>> +int arch_cpu_init(void)
>> +{
>> +	struct misc_regs *const misc_p =
>> +	    (struct misc_regs *)CONFIG_SPEAR_MISCBASE;
>> +	u32 periph1_clken;
>> +
>> +	periph1_clken = readl(&misc_p->periph1_clken);
>> +
>> +#if defined(CONFIG_SPEAR3XX)
>> +	periph1_clken |= MISC_GPT2ENB;
>> +#elif defined(CONFIG_SPEAR600)
>> +	periph1_clken |= MISC_GPT3ENB;
>> +#endif
>> +
>> +#if defined(CONFIG_PL011_SERIAL)
>> +	periph1_clken |= MISC_UART0ENB;
>> +#endif
>> +#if defined(CONFIG_DESIGNWARE_ETH)
>> +	periph1_clken |= MISC_ETHENB;
>> +#endif
>> +#if defined(CONFIG_DW_UDC)
>> +	periph1_clken |= MISC_USBDENB;
>> +#endif
>> +#if defined(CONFIG_DW_I2C)
>> +	periph1_clken |= MISC_I2CENB;
>> +#endif
>> +#if defined(CONFIG_ST_SMI)
>> +	periph1_clken |= MISC_SMIENB;
>> +#endif
>> +#if defined(CONFIG_NAND_FSMC)
>> +	periph1_clken |= MISC_FSMCENB;
>> +#endif
>> +
>> +	writel(periph1_clken,&misc_p->periph1_clken);
>> +	return 0;
>> +}
>> +#endif
>> +
>> +#ifdef CONFIG_DISPLAY_CPUINFO
>
> Again, I would remove this #ifdef here. Make it unconditionally for all SPEAr
> boards. We have enough #ifdef's already. :)
>

Yeah. In V2, I would not use these flags.

>> +int print_cpuinfo(void)
>> +{
>> +#ifdef CONFIG_SPEAR300
>> +	printf("CPU:   SPEAr300\n");
>> +#elif defined(CONFIG_SPEAR310)
>> +	printf("CPU:   SPEAr310\n");
>> +#elif defined(CONFIG_SPEAR320)
>> +	printf("CPU:   SPEAr320\n");
>> +#elif defined(CONFIG_SPEAR600)
>> +	printf("CPU:   SPEAr600\n");
>> +#else
>> +#error CPU not supported in spear platform
>> +#endif
>> +	return 0;
>> +}
>> +#endif
>> diff --git a/arch/arm/include/asm/arch-spear/spr_misc.h
>> b/arch/arm/include/asm/arch-spear/spr_misc.h index 8b96d9b..b10c726 100644
>> --- a/arch/arm/include/asm/arch-spear/spr_misc.h
>> +++ b/arch/arm/include/asm/arch-spear/spr_misc.h
>> @@ -126,5 +126,12 @@ struct misc_regs {
>>
>>   /* PERIPH1_CLKEN, PERIPH1_RST value */
>>   #define MISC_USBDENB			0x01000000
>> +#define MISC_ETHENB			0x00800000
>> +#define MISC_SMIENB			0x00200000
>> +#define MISC_GPT3ENB			0x00010000
>> +#define MISC_GPT2ENB			0x00000800
>> +#define MISC_FSMCENB			0x00000200
>> +#define MISC_I2CENB			0x00000080
>> +#define MISC_UART0ENB			0x00000008
>>
>>   #endif
>> diff --git a/include/configs/spear-common.h
>> b/include/configs/spear-common.h index 3f52442..5540630 100644
>> --- a/include/configs/spear-common.h
>> +++ b/include/configs/spear-common.h
>> @@ -182,6 +182,8 @@
>>   #define CONFIG_ENV_SIZE				0x02000
>>
>>   /* Miscellaneous configurable options */
>> +#define CONFIG_ARCH_CPU_INIT
>> +#define CONFIG_DISPLAY_CPUINFO
>
> Other than my comment above:
>
> Acked-by: Stefan Roese<sr@denx.de>
>

Ok, I would change the code and add yours Acked-by.

Regards
Amit Virdi