Hi Detlev, On Fri, Aug 13, 2010 at 14:00:21, Detlev Zundel wrote:
> > 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. We will most likely end up using this aspect of ATAG_REVISION as further revisions of the EVM appear. > The maximum speed of a CPU is orthogonal for > me, i.e. there can still be a fast and a slow rev 1 CPU Note that we are not taking about CPU being fast or slow at a given point of time. We are talking about whether the cpu on the board can support a given rate. It means that the silicon has been characterized (and probably priced) differently. So you can actually treat it as a different CPU revision. In fact, all of these speed graded parts are sold separately with different datasheets. Please see the 375 and 456 AM1x parts: http://focus.ti.com/paramsearch/docs/parametricsearch.tsp?family=dsp§ionId=2&tabId=2641&familyId=1877¶mCriteria=no and the 300 MHz OMAP-L1x parts: http://focus.ti.com/paramsearch/docs/parametricsearch.tsp?family=dsp§ionId=2&tabId=2231&familyId=1621¶mCriteria=no Moreover, CPU revision only occupies bits 0-3 of the ATAG_REVISION. As you mention the rest of the bits can be used to mark other changes in the EVMs (bug fixes etc). > but still we > cannot enable cache. Do you see my point? No, let me know if the above explanation clarifies the topic for you. > I hope I explained my point better this time. Yes, but I am still unconvinced ATAG_REVISION is not suitable for this purpose. > > >> > 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. Sure, I can include this in the commit text and in the README I promised. > >> > + > >> > +/* > >> > + * 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 > >> > >> The 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. Why would dropping "bit[0-3]" make anything clear? As I mentioned above other bits can be used in future to determine other aspects of board revision. May be I can add that information. Is the following more clear? /* * 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 * bit[4-31] Reserved (unused for now) */ > >> 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 see. Let me toss between this and specifying the speed in HZ and leaving the variable as is. I guess you would be OK both ways? Thanks, Sekhar _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
