Hi 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.
Do I understand you correctly that you _misuses_ an atag defined to carry the revision of a CPU to carry the maximum allowed clock frequency?
The EVM can be populated with devices of different speed grades and this patch treats those as different revisions of the EVM. Why would this be treated as a misuse of ATAG_REVISION?
A revision for me is attached to certain bugs/problems which we may need to work around in software. Think about something like "we can enable caching only on rev2 CPUs". For all I know, the ATAG_REVISION tag seems to capture this aspect. The maximum speed of a CPU is orthogonal for me, i.e. there can still be a fast and a slow rev 1 CPU but still we cannot enable cache. Do you see my point? If you want to express a maximum speed, then use an ATAG which supports such a usage. Reading the name ATAG_REVISION in code I would _never_ think that this transports the maximum clock frequency.
Is this really a good idea? I can easily imagine different CPU revisions with different maximum clock frequencies. How will you handle that?
I don't think I understood you. This patch _is_ meant to handle different revisions of DA850 EVMs populated with DA850 devices of differing speed grades.
I hope I explained my point better this time.
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.
I do not understand how the UBL gets to set the _U-Boot_ environment variable "maxspeed". Can you please explain how this is done?
UBL does not (cannot) set the maxspeed variable. The user of U-Boot is expected to set it based on what he sees on the packaging. Alternately he can also change the U-Boot configuration file and re-build U-Boot. UBL will setup the board to work at the common frequency of 300 MHz.
I see, so please write in the documentation that the user is supposed to set this variable correctly for his chip. I did not read this from the text.
Signed-off-by: Sekhar Nori nsekhar@ti.com
v2: removed unnecessary logical OR while constructing revision value
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 eeb456c..0eb3608 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 MHzThe description says it returns "bit[0-3]" which may seem that those canstants are encoded by a single bit each, whereas the code uses integer values. Change either the comment or the code.
[0-3] usually indicates the range the range 0 to 3. Do you have suggestions on how else this might be documented?
As I said, "bit[0-3]" denotes the 4 bits 0-3, i.e. a range from 0-15. (In this context the numbers below would actually translate into individual bits.) Just drop this "bit[0-3]" and it is clear.
Moreover I do not like that you call the variable "maxpseed" but interpret the value in kHz. Maybe the variable should be called "maxspeed_khz"?
I am hoping the documentation promised in the response above will help clarify its usage. I wanted to keep the variable name short.
Shortness is a worthwhile goal but clearness even more so. Those 4 characters would prevent anyone from ever looking into the documentation on deciding what unit the value is in. Personally I believe this is worth it. I still remember old times when the Linux kernel for PowerPC switched from its interpretation of clock frequencies from hz to mhz and the many support questions it generated....
Cheers Detlev