[U-Boot] [PATCH] TI: DaVinci DA850 EVM: support passing device speed grade information to kernel

[Nishanth Menon <menon.nishanth@gmail.com>]

On 08/10/2010 06:39 AM, Sekhar Nori wrote:
> The TI DA850/OMAP-L138/AM18x EVM can be populated with devices
> of different speed grades.
>
> The maximum speed the chip can support can only be determined from
> the label on the package (not software readable).
>
> Introduce a method to pass the speed grade information to kernel
> using ATAG_REVISION. The kernel uses this information to determine
> the maximum speed reachable using cpufreq.
>
> Note that U-Boot itself does not set the CPU rate. The CPU
> speed is setup by a primary bootloader ("UBL"). The rate setup
> by UBL could be different from the maximum speed grade of the
> device.
>
> Signed-off-by: Sekhar Nori<nsekhar@ti.com>
> ---
> Applies to latest of U-Boot mainline master
>
>   board/davinci/da8xxevm/da850evm.c |   38 +++++++++++++++++++++++++++++++++++++
>   include/configs/da850evm.h        |    1 +
>   2 files changed, 39 insertions(+), 0 deletions(-)
>
> 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;
wondering if the |= makes any sense...
> +		break;
> +	}
> +
> +	return rev;

IMHO, the logic could be simplified?

option 1:
u8 rev=0;
s = simple_strtoul(s, NULL, 10);
if (s) {
	switch (simple_strtoul(s, NULL, 10)) {
	case 456000:
		rev = 3;
		break;
	case 408000:
		rev = 2;
		break;
	case 372000:
		rev = 1;
		break;
	}
}

option 2:
if you think that the speeds could get added in the future, that switch 
is going to look pretty ugly.. use a lookup instead..

u32 speeds[]={372000,408000,456000};
int i;
u32 mspeed;
s=simple_strtoul(s, NULL, 10);
maxspeed = simple_strtoul(s, NULL, 10);
for (i=0;i<ARRAY_SIZE(speeds);i++)
	if (maxspeed == speeds[i])
		return i + 1;
return 0;
> +}
> +
>   int board_init(void)
>   {
>   #ifndef CONFIG_USE_IRQ
> diff --git a/include/configs/da850evm.h b/include/configs/da850evm.h
> index 357715d..3ea9032 100644
> --- a/include/configs/da850evm.h
> +++ b/include/configs/da850evm.h
> @@ -102,6 +102,7 @@
>    */
>   #define LINUX_BOOT_PARAM_ADDR	(CONFIG_SYS_MEMTEST_START + 0x100)
>   #define CONFIG_CMDLINE_TAG
> +#define CONFIG_REVISION_TAG
>   #define CONFIG_SETUP_MEMORY_TAGS
>   #define CONFIG_BOOTARGS		\
>   	"mem=32M console=ttyS2,115200n8 root=/dev/mtdblock2 rw noinitrd ip=dhcp"