TI: DaVinci DA850 EVM: support passing device speed grade information to kernel
PATCH 1/1 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.
---
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(-)
board/davinci/da8xxevm/da850evm.c +38 -0
@@ -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
include/configs/da850evm.h +1 -0
@@ -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"
Signature · 1 line
1.6.2.4
PATCH 1/1 TI: DaVinci DA850 EVM: support passing device speed grade information to kernel
Quoted 53 lines · Sekhar Nori
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<[email protected]>---
Applies to latest of U-Boot mainline masterboard/davinci/da8xxevm/da850evm.c | 38 +++++++++++++++++++++++++++++++++++++ include/configs/da850evm.h | 1 + 2 files changed, 39 insertions(+), 0 deletions(-)board/davinci/da8xxevm/da850evm.c +22
-0@@ -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 );
board/davinci/da8xxevm/da850evm.c static const struct lpsc_resource lpsc[] = { · line 104
Quoted 10 lines
board/davinci/da8xxevm/da850evm.c +10
-0+ +switch (maxspeed) { +case 456000 : + rev |=3 ; +break ; +case 408000 : + rev |=2 ; +break ; +case 372000 : + rev |=1 ;
wondering if the |= makes any sense...
board/davinci/da8xxevm/da850evm.c static const struct lpsc_resource lpsc[] = { · line 108
Quoted 4 lines
board/davinci/da8xxevm/da850evm.c +4
-0+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;
Quoted 17 lines
board/davinci/da8xxevm/da850evm.c +2
-0+} +int board_init (void ) {#ifndef CONFIG_USE_IRQ include/configs/da850evm.h +1
-0@@ -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"
Nori
PATCH 1/1 TI: DaVinci DA850 EVM: support passing device speed grade information to kernel
Hi Nishanth,
Quoted 35 lines · Nishanth Menon
Quoted 33 lines · Sekhar Nori
board/davinci/da8xxevm/da850evm.c +25
-0@@ -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 :
Quoted 8 lines
board/davinci/da8xxevm/da850evm.c static const struct lpsc_resource lpsc[] = { · line 104
Quoted 7 lines
board/davinci/da8xxevm/da850evm.c +7
-0+ 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.
Quoted 10 lines
board/davinci/da8xxevm/da850evm.c static const struct lpsc_resource lpsc[] = { · line 108
Quoted 4 lines
board/davinci/da8xxevm/da850evm.c +4
-0+break ; + } + +return rev;IMHO, the logic could be simplified?
option 1:
u8 rev=0;
s =simple_strtoul(s, NULL, 10);
Quoted 13 lines
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?
Quoted 4 lines
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
PATCH 1/1 TI: DaVinci DA850 EVM: support passing device speed grade information to kernel
Quoted 47 lines · Sekhar
Hi Nishanth,
Quoted 35 lines · Nishanth Menon
Quoted 33 lines · Sekhar Nori
board/davinci/da8xxevm/da850evm.c +25
-0@@ -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 :Quoted 8 lines
board/davinci/da8xxevm/da850evm.c static const struct lpsc_resource lpsc[] = { · line 104
Quoted 7 lines
board/davinci/da8xxevm/da850evm.c +7
-0+ rev |=3 ; +break ; +case 408000 : + rev |=2 ; +break ; +case 372000 : + rev |=1 ;wondering if the |= makes any sense...
Quoted 3 lines
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.
Quoted 11 lines
Quoted 11 lines
board/davinci/da8xxevm/da850evm.c static const struct lpsc_resource lpsc[] = { · line 108
Quoted 5 lines
board/davinci/da8xxevm/da850evm.c +4
-0+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");
Quoted 14 lines
Quoted 5 lines
if (s) { switch (simple_strtoul(s, NULL, 10)) { case 456000: rev = 3; break;Quoted 8 lines
case 408000: rev = 2; break; case 372000: rev = 1; break; } }
Quoted 4 lines
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..
Quoted 8 lines
Quoted 4 lines
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
Nori
PATCH 1/1 TI: DaVinci DA850 EVM: support passing device speed grade information to kernel
Hi Nishanth,
Quoted 46 lines · Nishanth Menon
Quoted 45 lines · Sekhar
Hi Nishanth,
Quoted 42 lines · Nishanth Menon
Quoted 40 lines · Sekhar Nori
board/davinci/da8xxevm/da850evm.c +32
-0@@ -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 ;
[...]
Quoted 28 lines
Quoted 11 lines
Quoted 11 lines
board/davinci/da8xxevm/da850evm.c static const struct lpsc_resource lpsc[] = { · line 108
Quoted 5 lines
board/davinci/da8xxevm/da850evm.c +4
-0+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");Quoted 14 lines
Quoted 13 lines
if (s) { switch (simple_strtoul(s, NULL, 10)) { case 456000: rev = 3; break; case 408000: rev = 2; break; case 372000: rev = 1; break; } }
Quoted 5 lines
Quoted 4 lines
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
PATCH 1/1 TI: DaVinci DA850 EVM: support passing device speed grade information to kernel
Quoted 81 lines · Sekhar
Hi Nishanth,
Quoted 46 lines · Nishanth Menon
Quoted 45 lines · Sekhar
Hi Nishanth,
Quoted 42 lines · Nishanth Menon
Quoted 40 lines · Sekhar Nori
board/davinci/da8xxevm/da850evm.c +32
-0@@ -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 ;[...]
Quoted 28 lines
Quoted 11 lines
Quoted 11 lines
board/davinci/da8xxevm/da850evm.c static const struct lpsc_resource lpsc[] = { · line 108
Quoted 5 lines
board/davinci/da8xxevm/da850evm.c +4
-0+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");Quoted 14 lines
Quoted 13 lines
if (s) { switch (simple_strtoul(s, NULL, 10)) { case 456000: rev = 3; break; case 408000: rev = 2; break; case 372000: rev = 1; break; } }
Quoted 8 lines
Quoted 5 lines
Quoted 4 lines
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).
Quoted 3 lines
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