Hi Nishanth,
On Thu, Aug 12, 2010 at 11:13:10, Nishanth Menon wrote:
On 08/11/2010 10:37 AM, Nori, Sekhar wrote:
Hi Nishanth,
On Wed, Aug 11, 2010 at 09:33:29, Nishanth Menon wrote:
On 08/10/2010 06:39 AM, Sekhar Nori wrote:
diff --git a/board/davinci/da8xxevm/da850evm.c b/board/davinci/da8xxevm/da850evm.c index 959b2c6..6a6d4fb 100644 --- a/board/davinci/da8xxevm/da850evm.c +++ b/board/davinci/da8xxevm/da850evm.c @@ -70,6 +70,44 @@ static const struct lpsc_resource lpsc[] = { { DAVINCI_LPSC_GPIO }, };
+#ifndef CONFIG_DA850_EVM_MAX_SPEED +#define CONFIG_DA850_EVM_MAX_SPEED 300000 +#endif
+/*
- get_board_rev() - setup to pass kernel board revision information
- Returns:
- bit[0-3] Maximum speed supported by the DA850/OMAP-L138/AM18x part
0 - 300 MHz
1 - 372 MHz
2 - 408 MHz
3 - 456 MHz- */
+u32 get_board_rev(void) +{
- char *s;
- u32 maxspeed = CONFIG_DA850_EVM_MAX_SPEED;
- u32 rev = 0;
- s = getenv("maxspeed");
- if (s)
maxspeed = simple_strtoul(s, NULL, 10);- switch (maxspeed) {
- case 456000:
rev |= 3;break;- case 408000:
rev |= 2;break;- case 372000:
rev |= 1;
[...]
break;- }
- return rev;
IMHO, the logic could be simplified?
option 1: u8 rev=0; s = simple_strtoul(s, NULL, 10);
aarrg.. emailing with eyes half shut mistake should have been: s = getenv("maxspeed");
if (s) { switch (simple_strtoul(s, NULL, 10)) { case 456000: rev = 3; break; case 408000: rev = 2; break; case 372000: rev = 1; break; } }
Not sure how this simplifies the logic. I'd argue multiple strtoul calls are actually better avoided. How does it handle the case where max speed is to be set using board configuration?
my bad, the above should explain..
I still don't see how this handles the case when maxspeed env variable is not set. The above code will just default to rev = 0 in that case.
We haven't gotten rid of any constructs either so not sure what the simplification here is.
Thanks, Sekhar