On Thu, May 6, 2010 at 12:36 AM, Hiremath, Vaibhav <hvaib...@ti.com> wrote: > >> -----Original Message----- >> From: Wolfgang Denk [mailto:w...@denx.de] >> Sent: Thursday, May 06, 2010 1:31 AM >> To: Hiremath, Vaibhav >> Cc: u-boot@lists.denx.de >> Subject: Re: [U-Boot] [RESEND:PATCH-V4] OMAP3EVM: Added NAND support >> >> Dear hvaib...@ti.com, >> >> In message <1272034546-26041-2-git-send-email-hvaib...@ti.com> you wrote: >> > >> > diff --git a/include/configs/omap3_evm.h b/include/configs/omap3_evm.h >> > index 0d99f7d..1d31731 100644 >> > --- a/include/configs/omap3_evm.h >> > +++ b/include/configs/omap3_evm.h >> > @@ -151,7 +151,8 @@ >> > >> > #define CONFIG_CMD_I2C /* I2C serial bus support */ >> > #define CONFIG_CMD_MMC /* MMC support */ >> > -#define CONFIG_CMD_ONENAND /* ONENAND support */ >> > +#undef CONFIG_CMD_ONENAND /* ONENAND support */ >> >> Please do not #undef what is not #define'd anyway. >> > [Hiremath, Vaibhav] This was the initial comment received from Nishanth Menon > on very first path, and that's where I added > undef line. > > The initial patch was something - > >> -#define CONFIG_CMD_ONENAND /* ONENAND support */ >> +/*#define CONFIG_CMD_ONENAND*/ /* ONENAND support */ >> +#define CONFIG_CMD_NAND /* NAND support */ > > I do agree that we don't have to undef here, but agreed to Nishant's comment > only because from user point of view, if user would > like to enable ONENAND support then for him it's easy he just have to comment > NAND line and make change this #define. He > doesn't have to dig inside code to find out whether ONENAND is supported or > not. my 2cents as a background: platforms such as SDP platforms have three flash devices - nand, onenand and nor - these are development platforms and are meant to bootup from any of these devices based on which ever dip switch is set. having a #undef is more elegant than /* */ and easier to use from a developer perspective.
Regards, Nishanth Menon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot