[U-Boot] [RESEND:PATCH-V4] OMAP3EVM: Added NAND support
From: Vaibhav Hiremath hvaibhav@ti.com
The EVMS have been shipping with NAND (instead of OneNAND) as default. So, this patch sets NAND as default.
To choose OneNAND, define CMD_ONENAND instead of CMD_NAND in the config file omap3_evm.h,
Changes from V3 :- - Added undef statement for CMD_ONENAND.
Signed-off-by: Vaibhav Hiremath hvaibhav@ti.com --- include/configs/omap3_evm.h | 9 ++++++++- 1 files changed, 8 insertions(+), 1 deletions(-)
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 */ +#define CONFIG_CMD_NAND /* NAND support */ #define CONFIG_CMD_DHCP #define CONFIG_CMD_PING
@@ -306,7 +307,13 @@ #define CONFIG_SYS_MONITOR_BASE CONFIG_SYS_FLASH_BASE #define CONFIG_SYS_ONENAND_BASE ONENAND_MAP
+#if defined(CONFIG_CMD_NAND) +#define CONFIG_NAND_OMAP_GPMC +#define GPMC_NAND_ECC_LP_x16_LAYOUT 1 +#define CONFIG_ENV_IS_IN_NAND +#elif defined(CONFIG_CMD_ONENAND) #define CONFIG_ENV_IS_IN_ONENAND 1 +#endif #define ONENAND_ENV_OFFSET 0x260000 /* environment starts here */ #define SMNAND_ENV_OFFSET 0x260000 /* environment starts here */
-- 1.6.2.4
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.
Best regards,
Wolfgang Denk
-----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.
Thanks, Vaibhav
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@denx.de What is tolerance? -- it is the consequence of humanity. We are all formed of frailty and error; let us pardon reciprocally each other's folly -- that is the first law of nature. - Voltaire
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
Dear Nishanth Menon,
In message u2h782515bb1005060340o3b366389mfdb71c240cbc7f90@mail.gmail.com you wrote:
On Thu, May 6, 2010 at 12:36 AM, Hiremath, Vaibhav hvaibhav@ti.com wrote:
...
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.
Well, of course the user _has_ to dig into the code and check if the feature is supported, because there is no information what the "#undef" or the comment means - it can mean anything:
- disabled here and left in so you can easily re-add it if you like - disabled because known to be unsupported or broken - disabled because untested - ...
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.
Please provide one working configuration, or several config options, but don't try to add kind of configuration menues using "#define" / "#undef" lists. These are useless and confusing at best.
Best regards,
Wolfgang Denk
On Thu, May 6, 2010 at 5:50 AM, Wolfgang Denk wd@denx.de wrote:
Dear Nishanth Menon,
In message u2h782515bb1005060340o3b366389mfdb71c240cbc7f90@mail.gmail.com you wrote:
On Thu, May 6, 2010 at 12:36 AM, Hiremath, Vaibhav hvaibhav@ti.com wrote:
...
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.
Well, of course the user _has_ to dig into the code and check if the feature is supported, because there is no information what the "#undef" or the comment means - it can mean anything:
- disabled here and left in so you can easily re-add it if you like
- disabled because known to be unsupported or broken
- disabled because untested
- ...
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.
Please provide one working configuration, or several config options, but don't try to add kind of configuration menues using "#define" / "#undef" lists. These are useless and confusing at best.
your points are valid, two options as I see here: a) build configurations - nand_boot, nor_boot etc.. b) IMHO, the better solution would be to allow the same u-boot to boot from all devices (nand, nor, onenand, mmc) without needing a rebuild. but the neccessity for that is to have environment variable which can be moved around.. TI has an implementation based on an ancient u-boot which actually does this, it will be interesting to know if there are others who may be interested in a similar feature in mainline u-boot.
Regards, Nishanth Menon
Dear Nishanth Menon,
In message j2h782515bb1005060354tc4574d80wb4a1aa3a2b40bc89@mail.gmail.com you wrote:
your points are valid, two options as I see here: a) build configurations - nand_boot, nor_boot etc.. b) IMHO, the better solution would be to allow the same u-boot to boot from all devices (nand, nor, onenand, mmc) without needing a rebuild.
b) is clearly the better and more user-friendly solution.
but the neccessity for that is to have environment variable which can be moved around.. ...
Hm... really? Assuming that all the storage devices are always present, you can decide to use one standard location for the environment independent of where the U-Boot image gets loaded from (that may actually be considered an advantage by some).
... TI has an implementation based on an ancient u-bootwhich actually does this, it will be interesting to know if there are others who may be interested in a similar feature in mainline u-boot.
Sure - please post a patch (as RFC) so we can see what you have in mind.
Best regards,
Wolfgang Denk
On Thu, May 6, 2010 at 6:03 AM, Wolfgang Denk wd@denx.de wrote:
Dear Nishanth Menon,
In message j2h782515bb1005060354tc4574d80wb4a1aa3a2b40bc89@mail.gmail.com you wrote:
your points are valid, two options as I see here: a) build configurations - nand_boot, nor_boot etc.. b) IMHO, the better solution would be to allow the same u-boot to boot from all devices (nand, nor, onenand, mmc) without needing a rebuild.
b) is clearly the better and more user-friendly solution.
but the neccessity for that is to have environment variable which can be moved around.. ...
Hm... really? Assuming that all the storage devices are always present, you can decide to use one standard location for the environment independent of where the U-Boot image gets loaded from (that may actually be considered an advantage by some).
it is like http://dev.omapzoom.org/?p=bootloader/u-boot.git;a=blob;f=board/omap3430sdp/...
Infact in some of the settings, one of the chips might even not be mapped :(..
yes, one option could have been to use a single flash for all env (in fact some platforms like (if i recollect beagle booting off MMC uses NAND for env) - but is not a nice idea if the usecase is to use that entire flash for filesystem.. so the safe option in general has been to expect the environment to stick to the memory u-boot booted off from..
... TI has an implementation based on an ancient u-boot which actually does this, it will be interesting to know if there are others who may be interested in a similar feature in mainline u-boot.
Sure - please post a patch (as RFC) so we can see what you have in mind.
Yep.. waiting for my next free cycle ;) i would have pointed as an RFC to the git repo history, unfortunately the transition from clearcase to git was done as a single blob :(
Regards, Nishanth Menon
Dear Nishanth Menon,
In message o2q782515bb1005060411u9e53fd21idbfcf95cd5e1ea59@mail.gmail.com you wrote:
Yep.. waiting for my next free cycle ;) i would have pointed as an RFC to the git repo history, unfortunately the transition from clearcase to git was done as a single blob :(
Heh. This matches my experience - each and every contact with ClearCase has always been a clear case of PITA :-(
Best regards,
Wolfgang Denk
-----Original Message----- From: Wolfgang Denk [mailto:wd@denx.de] Sent: Thursday, May 06, 2010 4:20 PM To: Nishanth Menon Cc: Hiremath, Vaibhav; u-boot@lists.denx.de Subject: Re: [U-Boot] [RESEND:PATCH-V4] OMAP3EVM: Added NAND support
Dear Nishanth Menon,
In message u2h782515bb1005060340o3b366389mfdb71c240cbc7f90@mail.gmail.com you wrote:
On Thu, May 6, 2010 at 12:36 AM, Hiremath, Vaibhav hvaibhav@ti.com
wrote: ...
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.
Well, of course the user _has_ to dig into the code and check if the feature is supported, because there is no information what the "#undef" or the comment means - it can mean anything:
- disabled here and left in so you can easily re-add it if you like
- disabled because known to be unsupported or broken
- disabled because untested
- ...
[Hiremath, Vaibhav] Ok. Agreed. I will remove undef line and submit the patch.
Thanks, Vaibhav
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.
Please provide one working configuration, or several config options, but don't try to add kind of configuration menues using "#define" / "#undef" lists. These are useless and confusing at best.
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@denx.de Motto of the Electrical Engineer: Working computer hardware is a lot like an erect penis: it stays up as long as you don't fuck with it.
Dear "Hiremath, Vaibhav",
In message 19F8576C6E063C45BE387C64729E7394044E351B9F@dbde02.ent.ti.com you wrote:
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.
Hm... what makes you think we could assume that commented out code is actually working and can be enabled as we like? This is a highly speculative assumption, and probably more often wrong than right.
Best regards,
Wolfgang Denk
participants (4)
-
Hiremath, Vaibhav -
hvaibhav＠ti.com
-
Nishanth Menon -
Wolfgang Denk