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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
