[U-Boot] [PATCH] ARM DaVinci: Adding CONFIG options for NAND ALE and CLE

[Paulraj, Sandeep]

Dave,

Please see inline.

> -----Original Message-----
> From: David Brownell [mailto:david-b at pacbell.net]
> Sent: Tuesday, April 28, 2009 5:51 PM
> To: Paulraj, Sandeep; u-boot at lists.denx.de
> Cc: davinci-linux-open-source at linux.davincidsp.com
> Subject: Re: [PATCH] ARM DaVinci: Adding CONFIG options for NAND ALE and
> CLE
> 
> On Tuesday 28 April 2009, s-paulraj at ti.com wrote:
> > The CLE and ALE for DaVinci DM644x is not the same as DM646x. This patch
> > adds 2 CONFIG_ options to the config.h header files to all the DM6446
> based
> > boards. These values are then used by the driver.
> 
> I had been thinking that the davinci_nand driver should
> probably change to let board_nand_init() really become a
> board-specific function...
> 
> It would call a driver routine to init the callback functions
> and set up private data so that for example the 2GByte NAND
> could be properly treated as a single device with two chips,
> instead of two separate devices.  (As Linux does; I'm just
> assuming that mechanism works right in U-Boot.)
[Sandeep] At the U-Boot level do we really need this mechanism?
> 
> Other private data could include CLE/ALE masks, if needed.
[Sandeep] This is the way we do it in the kernel. Again does U-boot really need this much sophistication.
> I may well have missed something, but I thought that the
> ROM on all chips used the same mask values when booting
> from NAND flash... but that non-boot chips might end up
> using different values, if they wanted.  (Burst memory
> access would work better if toggling low address bits
> didn't affect ALE/CLE, etc.)
> 
> 
> Alternatively ... how about just letting those values be
> defined in the headers if the boot-from-NAND defaults
> shouldn't be used?
> 
> That is, the u-boot davinci_nand.c code could
> 
> 	#ifndef CONFIG_SYS_DAVINCI_NAND_CLE
> 	... #define it to the default 0x10 ...
> 	#endif
[Sandeep] It is my personal opinion that we should try to avoid such #ifdefs in the driver. I would just define all these in the config.h file. Let me know your opinion. We will need such a #ifndef for the CLE as well so I would just put both the values in the config.h rather than have 2 #ifndefs in the NAND driver
> 
> I think the convention for these things (toplevel README) is
> that since they're hardware-specific, CONFIG_SYS_* is the
> prefix not CONFIG_* for those symbols.
[Sandeep] will do
> 
> 
> Related: MASK_ALE should not be 0x0a by default, but 0x08;
> right? 
[Sandeep] That is what I use in the internal LSP releases
 That's what newer docs say; I think the old stuff
> was just a bug.  In Linux, 0x08 works just fine.
[Sandeep] Yes that is correct. I was going to send an e-mail tomorrow to see if anyone had any objections to me changing this value
> 
> And for that matter, include/asm-arm/arch-davinci/nand_defs.h
> is starting to look almost un-necessary after those cleanup
> patches I sent.  :)
[Sandeep] yes I noticed that.
> 
> 
> > This has been tested on the
> > DM644x, DM355, DM357 and DM365. Support for the latter 3 will be
> > added soon.
[Sandeep] Its ready, it just a matter of getting the patches to apply over other patches in a form acceptable to everybody.
> 
> I'm glad to see TI's latest U-Boot work becoming public...
[Sandeep] I don't know why version 1.3.4 has not become public yet but it will be soon.
> 
> However, this won't apply on top of the cleanup patches I just
> sent, which remove the NAND_CE0{ALE,CLE,DATA} symbols in favor
> of the relevant nand_chip pointer.  As you know, that change
> is important for support of the EVM boards which include the
> socketed 2 GByte NAND chips, with multiple chip select lines.
[Sandeep] yes and I will send an updated patch tomorrow.
> 
> - Dave
>