[U-Boot] [PATCH 2/3] SPEAr: Configure FSMC driver for NAND interface

[Amit Virdi]

Hello Stefan,

On 2/27/2012 3:32 PM, Stefan Roese wrote:
> Hi Amit,
>
> please find a few comments below.
>
> On Monday 27 February 2012 10:38:23 Amit Virdi wrote:
>> From: Vipin KUMAR<vipin.kumar at st.com>
>>
>> Since FSMC is a standard IP and it supports different memory interfaces, it
>> is supported independent of spear platform and spear is configured to use
>> that driver for interfacing with the NAND device
>>
>> Signed-off-by: Vipin Kumar<vipin.kumar at st.com>
>> Signed-off-by: Amit Virdi<amit.virdi at st.com>
>> ---
>>   arch/arm/include/asm/arch-spear/hardware.h |    8 ++++----
>>   board/spear/spear300/spear300.c            |    7 ++++---
>>   board/spear/spear310/spear310.c            |    7 ++++---
>>   board/spear/spear320/spear320.c            |    7 ++++---
>>   board/spear/spear600/spear600.c            |    7 ++++---
>>   include/configs/spear-common.h             |    2 +-
>>   include/configs/spear3xx.h                 |    4 ++++
>>   include/configs/spear6xx.h                 |    3 +++
>>   8 files changed, 28 insertions(+), 17 deletions(-)
>>
>> diff --git a/arch/arm/include/asm/arch-spear/hardware.h
>> b/arch/arm/include/asm/arch-spear/hardware.h index 818f36c..a6517b2 100644
>> --- a/arch/arm/include/asm/arch-spear/hardware.h
>> +++ b/arch/arm/include/asm/arch-spear/hardware.h
>> @@ -37,15 +37,15 @@
>>
>>   #if defined(CONFIG_SPEAR600)
>>   #define CONFIG_SYS_I2C_BASE			(0xD0200000)
>> -#define CONFIG_SPEAR_FSMCBASE			(0xD1800000)
>> +#define CONFIG_SYS_FSMC_BASE			(0xD1800000)
>
> Please remove the parentheses here as they are not needed. Should be done to
> all those defines, perhaps in a cosmetic cleanup patch at some time as well:
>

Yes. I'll be sending a separate cleanup commit.

> +#define CONFIG_SYS_FSMC_BASE			0xD1800000
>
>>   #elif defined(CONFIG_SPEAR300)

<snip>

>> diff --git a/include/configs/spear-common.h
>> b/include/configs/spear-common.h index 516b78e..c37305f 100644
>> --- a/include/configs/spear-common.h
>> +++ b/include/configs/spear-common.h
>> @@ -90,7 +90,7 @@
>>   /* NAND FLASH Configuration */
>>   #define CONFIG_MTD_DEVICE
>>   #define CONFIG_MTD_PARTITIONS
>> -#define CONFIG_NAND_SPEAR			1
>> +#define CONFIG_NAND_FSMC
>>   #define CONFIG_SYS_MAX_NAND_DEVICE		1
>>   #define CONFIG_MTD_NAND_VERIFY_WRITE
>
> I suggest that you remove this last define. Most likely it was added for
> debugging purpose only. It slows down the speed and it brakes UBI support.
>

Ok.

>> diff --git a/include/configs/spear3xx.h b/include/configs/spear3xx.h
>> index bd5d111..5bdd874 100644
>> --- a/include/configs/spear3xx.h
>> +++ b/include/configs/spear3xx.h
>> @@ -117,6 +117,10 @@
>>
>>   #endif
>>
>> +/* NAND flash configuration */
>> +#define CONFIG_SYS_FSMC_NAND_SP
>> +#define CONFIG_SYS_FSMC_NAND_8BIT
>> +
>>   #if defined(CONFIG_SPEAR300)
>>   #define CONFIG_SYS_NAND_BASE			0x80000000
>>
>> diff --git a/include/configs/spear6xx.h b/include/configs/spear6xx.h
>> index 8de7ebd..1e06c72 100644
>> --- a/include/configs/spear6xx.h
>> +++ b/include/configs/spear6xx.h
>> @@ -38,6 +38,9 @@
>>   #define CONFIG_PL01x_PORTS			{ (void
> *)CONFIG_SYS_SERIAL0, \
>>   						(void
> *)CONFIG_SYS_SERIAL1 }
>>
>> +/* NAND flash configuration */
>> +#define CONFIG_SYS_FSMC_NAND_SP
>> +#define CONFIG_SYS_FSMC_NAND_8BIT
>
> You also need the following define for this to work with the latest NAND
> subsystem:
>
> #define CONFIG_MTD_ECC_SOFT
>
> Not sure about SPEAr3xx. Most likely this needs it as well.
>

Ok. I'll verify the image after the making the changes you suggested and 
then get back.

Thanks
Amit Virdi