[U-Boot] [PATCH] TI: DaVinci DA850 EVM: support passing device speed grade information to kernel
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; + break; + } + + return rev; +} + 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"
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 Norinsekhar@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"
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;wondering if the |= makes any sense...
Yeah, I put it in there in case additional fields pop-up in board revision. It doesn't make a lot of sense now so I will remove it.
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; } }
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?
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..
Right now there is no known plan to add more speeds so I will stick with the switch. You are right, option of using a lookup deserves a look-in if there were a lot more speed steps.
Thanks, Sekhar
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;wondering if the |= makes any sense...
Yeah, I put it in there in case additional fields pop-up in board revision. It doesn't make a lot of sense now so I will remove it.
thx.
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..
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..
Right now there is no known plan to add more speeds so I will stick with the switch. You are right, option of using a lookup deserves a look-in if there were a lot more speed steps.
ok. Regards, Nishanth Menon
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
On 08/12/2010 01:14 AM, Nori, Sekhar wrote:
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.
it does the same here as well (default rev = 0). and the fact that compared to v1, |= was avoided (this is part of v2 of the patch).
We haven't gotten rid of any constructs either so not sure what the simplification here is.
got rid of a var maxspeed - but I doubt it will have much benefit, please consider my objection withdrawn.. just to point to the note that the var has not much of a lifetime or a function beyond providing data to the switch..
Regards, Nishanth Mnon
participants (3)
-
Nishanth Menon -
Nori, Sekhar -
Sekhar Nori