From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tom Date: Sun, 15 Nov 2009 14:08:11 -0600 Subject: [U-Boot] [PATCH] omap3: allow slection of NAND GPMC settings based on board config In-Reply-To: <782515bb0911151141u7d0e88c0s43c986d1b3fc1b63@mail.gmail.com> References: <1257926592-14782-3-git-send-email-mike@compulab.co.il> <4AFFC84D.9050903@compulab.co.il> <4B002DF0.4050800@windriver.com> <782515bb0911151141u7d0e88c0s43c986d1b3fc1b63@mail.gmail.com> Message-ID: <4B005FAB.9030209@windriver.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Nishanth Menon wrote: > On Sun, Nov 15, 2009 at 10:36 AM, Tom wrote: >> Mike Rapoport wrote: >>> Any comments on this? >>> >> This is a good idea but.. >> >> These defines are used in the TI codebase but >> it doesn't look like the samsung nand is used here. >> >> As a test I removed the samsung defines >> and reran MAKEALL arm. >> No failures. >> Also grepping the code shows that it isn't used. >> >> Do you plan on using the samsung nand ? > SDP3430 uses Samsung NAND. > >> If not, I think a better patch would be to remove the smnand defines. > NAK. > >> Also I think the nand and onnand initialization could be generalized. You >> may want to look at that. >> >> Thanks >> Tom >> >>> Mike Rapoport wrote: >>>> There are several presets for GPMC registers defined in >>>> include/asm-arm/arch-omap3/mem.h. Allow selection between SMNAND and >>>> M_NAND presets based on OMAP34XX_GPMC_NAND_ defines >>>> >>>> Signed-off-by: Mike Rapoport >>>> --- >>>> cpu/arm_cortexa8/omap3/mem.c | 25 ++++++++++++++++--------- >>>> 1 files changed, 16 insertions(+), 9 deletions(-) >>>> >>>> diff --git a/cpu/arm_cortexa8/omap3/mem.c b/cpu/arm_cortexa8/omap3/mem.c >>>> index 8b8cd6d..2bd7e1c 100644 >>>> --- a/cpu/arm_cortexa8/omap3/mem.c >>>> +++ b/cpu/arm_cortexa8/omap3/mem.c >>>> @@ -44,14 +44,21 @@ volatile unsigned int boot_flash_env_addr; >>>> struct gpmc *gpmc_cfg; >>>> >>>> #if defined(CONFIG_CMD_NAND) >>>> -static const u32 gpmc_m_nand[GPMC_MAX_REG] = { >>>> - M_NAND_GPMC_CONFIG1, >>>> - M_NAND_GPMC_CONFIG2, >>>> - M_NAND_GPMC_CONFIG3, >>>> - M_NAND_GPMC_CONFIG4, >>>> - M_NAND_GPMC_CONFIG5, >>>> - M_NAND_GPMC_CONFIG6, 0 >>>> -}; >>>> +#define GPMC_NAND(PART) \ >>>> + static const u32 gpmc_nand[GPMC_MAX_REG] = { \ >>>> + PART##_GPMC_CONFIG1, \ >>>> + PART##_GPMC_CONFIG2, \ >>>> + PART##_GPMC_CONFIG3, \ >>>> + PART##_GPMC_CONFIG4, \ >>>> + PART##_GPMC_CONFIG5, \ >>>> + PART##_GPMC_CONFIG6, 0 \ > > This is completely unnecessary implementation IMHO, where required, > the board files can register > their own NAND device timings on a need basis. > NOTE: timing values change based on L3 clk -> e..g 3630 means that ALL > the above timing values > are invalid as L3 is at 200Mhz! > You are correct! Not that this is a surprise :P Agreeing with Nishanth.. Tom >>>> + }; >>>> + >>>> +#ifdef OMAP34XX_GPMC_NAND_SMNAND >>>> +GPMC_NAND(SMNAND) >>>> +#else >>>> +GPMC_NAND(M_NAND) >>>> +#endif >>>> >>>> #if defined(CONFIG_ENV_IS_IN_NAND) >>>> #define GPMC_CS 0 >>>> @@ -246,7 +253,7 @@ void gpmc_init(void) >>>> sdelay(1000); >>>> >>>> #if defined(CONFIG_CMD_NAND) /* CS 0 */ >>>> - gpmc_config = gpmc_m_nand; >>>> + gpmc_config = gpmc_nand; >>>> >>>> base = PISMO1_NAND_BASE; >>>> size = PISMO1_NAND_SIZE; > > IMHO, NAK. > Regards, > Nishanth Menon