On Thu, May 6, 2010 at 12:36 AM, Hiremath, Vaibhav hvaibhav@ti.com wrote:
-----Original Message----- From: Wolfgang Denk [mailto:wd@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 hvaibhav@ti.com,
In message 1272034546-26041-2-git-send-email-hvaibhav@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