Dear Aneesh V, In message <4dd11511.1060...@ti.com> you wrote: > > >> + const char *omap4_rev = NULL; > >> + switch (omap4_revision()) { > >> + case OMAP4430_ES1_0: > >> + omap4_rev = "OMAP4430 ES1.0"; > >> + break; > >> + case OMAP4430_ES2_0: > >> + omap4_rev = "OMAP4430 ES2.0"; > >> + break; > >> + case OMAP4430_ES2_1: > >> + omap4_rev = "OMAP4430 ES2.1"; > >> + break; > >> + case OMAP4430_ES2_2: > >> + omap4_rev = "OMAP4430 ES2.2"; > >> + break; > > > > Such code does not really scale well. Can this not be improved? I > > think we just had similar discussions for i.MX5x - please check what > > the result of these was. > > Are you referring to this one? > http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/98522 > > If so, it may not work for us: > > 1. Please note that the above function is just for getting the string > not for the revision itself. To get the revision we have > omap4_revision().
Well, when you already have such a funxction, then why cannot it be made to return useful values that are well-suited for formatting? Instead of #define OMAP4430_ES1_0 1 #define OMAP4430_ES2_0 2 #define OMAP4430_ES2_1 3 #define OMAP4430_ES2_2 4 you could use #define OMAP4430_ES1_0 10 #define OMAP4430_ES2_0 20 #define OMAP4430_ES2_1 21 #define OMAP4430_ES2_2 22 And then use a plain sprintf(omap4_rev, "OMAP4430 ES%d.%d", rev/10, rev%10); or similar. > 2. In our case we do not have a 1:1 mapping between the > revisions(monotonically increasing numbers) we need in the U-Boot and > the value obtained from the ID_CODE register. So, a translation is > inevitable. This is not needed. See above. Any form of table driven approach would be suitable, too. > 3. We need increasing numbers for subsequent revisions so that we can > have something like: Should be no problem, see above. Just define your number scheme. Instead of decimal packing you could also adapt something like the major/minor numbers for devices, and use the existing macros to extract the parts. There are tons of existing solutions for this type of problem, just pick one that fits. > >> + default: > >> + omap4_rev = "OMAP4 - Unknown Rev"; > >> + break; > > > > Please also output what the unknown revision was - this saves at least > > one debug round if you ever run into this case. > > This function should be in sync with omap4_revision() function(unless > there is a bug). So, the rev will be OMAP4430_SILICON_ID_INVALID in > that case. In this case omap4_revision() should print the value it is reading. Whenever you are reading "unexpected? numbers the first thing you will do during debugging is to check _what_ exactly you are reawding - so you can help and safe one step by just printing thei information which you have ready in your hands. > I shall print out the ARM revision and OMAP revision registers when I > get into OMAP4430_SILICON_ID_INVALID situation in omap4_revision() Thanks. 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: w...@denx.de "We have the right to survive!" "Not be killing others." -- Deela and Kirk, "Wink of An Eye", stardate 5710.5 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot