[U-Boot] [PATCH 03/12] da850: add NOR boot mode support

[Netagunte, Nagabhushana]

Hi Denk,

Thanks for comments. My response is in-lined.

Regards,
Nag

On Tue, Aug 02, 2011 at 21:42:28, Wolfgang Denk wrote:
> Dear nagabhushana.netagunte at ti.com,
> 
> In message <1312299792-16415-4-git-send-email-nagabhushana.netagunte at ti.com> you wrote:
> > From: Nagabhushana Netagunte <nagabhushana.netagunte at ti.com>
> > 
> > Add an option to use NOR boot mode in configuration file and 
> > correspanding pin-mux support in board file.
> > 
> > Signed-off-by: Sudhakar Rajashekhara <sudhakar.raj at ti.com>
> > Signed-off-by: Nagabhushana Netagunte <nagabhushana.netagunte at ti.com>
> ...
> >  #define CONFIG_DRIVER_TI_EMAC
> >  #define CONFIG_USE_SPIFLASH
> > +#undef CONFIG_USE_NAND
> > +#undef CONFIG_USE_NOR
> 
> Please do not undef what is not defined anyway.  If you want to add comments to the user, then use C comments for this purpose.
> 

Will do that.

> Also, please note that none of the CONFIG_ options listed here (CONFIG_DRIVER_TI_EMAC, CONFIG_USE_SPIFLASH, CONFIG_USE_NAND, or
> CONFIG_USE_NOR) are documented.  Please add appropriate documentation to the README file.
> 

Will add a new patch to add appropriate documentation.

> >  /*
> >   * SoC Configuration
> > @@ -129,6 +131,23 @@
> >  #define CONFIG_NET_MULTI
> >  #endif
> >  
> > +#ifdef CONFIG_USE_NOR
> > +#define CONFIG_ENV_IS_IN_FLASH
> > +#undef CONFIG_SYS_NO_FLASH
> 
> Please do not undef what is not defined anyway.  Please fix globally.
> 
> > +#define CONFIG_SYS_FLASH_SECT_SZ	(128 << 10) /* 128KB */
> > +#define CONFIG_ENV_OFFSET		(CONFIG_SYS_FLASH_SECT_SZ * 3)
> > +#define CONFIG_ENV_SIZE			(128 << 10)
> 
> Are you absolutely sure that you need 128 KiB of environment data?
> Keep in mind that such a big environment will _considerably_ slow down booting - and in all practical situations I have seen so far the actual environment size was in the order of a few KiB only - I don;t even remember any board with more than 10 KiB.
> 

I agree with you that 128 KiB is huge. It is 128KiB because sector size for
NOR flash is 128KiB. So, decision was to reserve one complete sector for ENV
Data.

> Best regards,
> 
> Wolfgang Denk
> 
> -- 
> DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
> A list is only as strong as its weakest link.            -- Don Knuth
>