| Submitter | xdrudis |
|---|---|
| Date | 2011-02-26 01:39:39 |
| Message ID | <20110226013939.GB5330@ideafix.casa.ct> |
| Download | mbox | patch |
| Permalink | /patch/2705/ |
| State | New |
| Headers | show |
Comments
On 02/26/2011 03:39 AM, xdrudis wrote: > This patch tries to fix compilation when you select EXPERT in make menuconfig. > HT Frequencies are multiples of 200MHz AFAIK, so there are no 300MHz and 500MHz. I'm not sure why the build breaks, and why this fixes it, but I don't think this is the right solution. Someone wiser should comment on this. Alex
On Sat, Feb 26, 2011 at 03:53:43AM +0200, Alex G. wrote: > On 02/26/2011 03:39 AM, xdrudis wrote: > > This patch tries to fix compilation when you select EXPERT in make menuconfig. > > > HT Frequencies are multiples of 200MHz AFAIK, so there are no 300MHz and > 500MHz. I'm not sure why the build breaks, and why this fixes it, but I > don't think this is the right solution. > Someone wiser should comment on this. > Oh! You may well be right. All others are multiples of 200 MHz . Then we should maybe remove 2 #elif in the following code. But I wonder whether the break in the progression of values indicates that all values from there on should be changed too. I have no idea what this does, other than the Kconfig help: choice prompt "HyperTransport frequency" default LIMIT_HT_SPEED_AUTO help This option sets the maximum permissible HyperTransport link frequency. Use of this option will only limit the autodetected HT frequency. It will not (and cannot) increase the frequency beyond the autodetected limits. This is primarily used to work around poorly designed or laid out HT traces on certain motherboards. The code where it is used is in src/northbridge/amd/amdht/h3finit.c:1330 #if CONFIG_EXPERT && CONFIG_LIMIT_HT_SPEED_200 cbPCBFreqLimit = 0x0001; #elif CONFIG_EXPERT && CONFIG_LIMIT_HT_SPEED_300 cbPCBFreqLimit = 0x0003; #elif CONFIG_EXPERT && CONFIG_LIMIT_HT_SPEED_400 cbPCBFreqLimit = 0x0007; #elif CONFIG_EXPERT && CONFIG_LIMIT_HT_SPEED_500 cbPCBFreqLimit = 0x000F; #elif CONFIG_EXPERT && CONFIG_LIMIT_HT_SPEED_600 cbPCBFreqLimit = 0x001F; #elif CONFIG_EXPERT && CONFIG_LIMIT_HT_SPEED_800 cbPCBFreqLimit = 0x003F; #elif CONFIG_EXPERT && CONFIG_LIMIT_HT_SPEED_1000 cbPCBFreqLimit = 0x007F; #elif CONFIG_EXPERT && CONFIG_LIMIT_HT_SPEED_1200 cbPCBFreqLimit = 0x00FF; #elif CONFIG_EXPERT && CONFIG_LIMIT_HT_SPEED_1400 cbPCBFreqLimit = 0x01FF; #elif CONFIG_EXPERT && CONFIG_LIMIT_HT_SPEED_1600 cbPCBFreqLimit = 0x03FF; #elif CONFIG_EXPERT && CONFIG_LIMIT_HT_SPEED_1800 cbPCBFreqLimit = 0x07FF; #elif CONFIG_EXPERT && CONFIG_LIMIT_HT_SPEED_2000 cbPCBFreqLimit = 0x0FFF; #elif CONFIG_EXPERT && CONFIG_LIMIT_HT_SPEED_2200 cbPCBFreqLimit = 0x1FFF; #elif CONFIG_EXPERT && CONFIG_LIMIT_HT_SPEED_2400 cbPCBFreqLimit = 0x3FFF; #elif CONFIG_EXPERT && CONFIG_LIMIT_HT_SPEED_2600 cbPCBFreqLimit = 0x7FFF; #else cbPCBFreqLimit = 0xFFFF; // Maximum allowed by autoconfiguration #endif
xdrudis wrote: > > HT Frequencies are multiples of 200MHz AFAIK, so there are no > > 300MHz and 500MHz. .. > Oh! You may well be right. All others are multiples of 200 MHz . > > Then we should maybe remove 2 #elif in the following code. But I > wonder whether the break in the progression of values indicates > that all values from there on should be changed too. A safer change might be: > #elif CONFIG_EXPERT && CONFIG_LIMIT_HT_SPEED_300 > #elif CONFIG_EXPERT && CONFIG_LIMIT_HT_SPEED_500 #elif CONFIG_EXPERT && defined(CONFIG_LIMIT_HT_SPEED_300) && CONFIG_LIMIT_HT_SPEED_300 #elif CONFIG_EXPERT && defined(CONFIG_LIMIT_HT_SPEED_500) && CONFIG_LIMIT_HT_SPEED_500 I would ack that if it builds. //Peter
On Sat, Feb 26, 2011 at 07:17:46PM +0100, Peter Stuge wrote: > xdrudis wrote: > > > HT Frequencies are multiples of 200MHz AFAIK, so there are no > > > 300MHz and 500MHz. > .. > > Oh! You may well be right. All others are multiples of 200 MHz . > > > > Then we should maybe remove 2 #elif in the following code. But I > > wonder whether the break in the progression of values indicates > > that all values from there on should be changed too. > > A safer change might be: > > > #elif CONFIG_EXPERT && CONFIG_LIMIT_HT_SPEED_300 > > #elif CONFIG_EXPERT && CONFIG_LIMIT_HT_SPEED_500 > > #elif CONFIG_EXPERT && defined(CONFIG_LIMIT_HT_SPEED_300) && CONFIG_LIMIT_HT_SPEED_300 > #elif CONFIG_EXPERT && defined(CONFIG_LIMIT_HT_SPEED_500) && CONFIG_LIMIT_HT_SPEED_500 > > > I would ack that if it builds. > I'd rather wait to see if someone who understands the code knows whether 300 and 500 make sense (then we define them as in my patch) or they don't (then we eliminate the #elif and maybe modify some values around there). What you propose just defers the real question to Kconfig. Besides, without my patch, I don't think CONFIG_LIMIT_HT_SPEED_300 and CONFIG_LIMIT_HT_SPEED_500 are defined anyware, so you're solution is equivalent to eliminaing the #elif.
The 500MHz and 300MHz are valid HT frequencies
Table 59. Link Frequency Bit Field Encoding
[FreqExt, Link Frequency] Transmitter Clock Frequency
Encoding (MHz)
0_0000 200 (default)
0_0001 300
0_0010 400
0_0011 500
0_0100 600
0_0101 800
0_0110 1000
0_0111 1200*
0_1000 1400*
0_1001 1600*
0_1010 1800
0_1011 2000
0_1100 2200
0_1101 2400
0_1110 2600
0_1111 Vendor-Specific
1_0000 Reserved
1_0001 2800
1_0010 3000
1_0011 3200
1_0100 to 1_1111 Reserved
* These frequencies were defined in revision 2.00 of the specification but are
redefined electrically in revision 3.00. The presence of a Gen3
capability block indicates that a device implements the Gen3 electrical
behavior for
these frequencies. Gen1 and Gen3 devices are not interoperable at these
frequencies.
Check http://www.hypertransport.org/docs/twgdocs/HTC20051222-0046-0035.pdf
For details
Thanks,
Rudolf
xdrudis wrote: > This patch tries to fix compilation when you select EXPERT in > make menuconfig. .. > Signed-off-by: Xavi Drudis Ferran <xdrudis@tinet.cat> Acked-by: Peter Stuge <peter@stuge.se> r6386
Patch
If I select Expert mode in make menuconfig I couldn't compile because it complained of 2 missing configuration constants. I hope this is the right solution, but haven't really checked that the related code in src/northbridge/amd/amdht/h3finit.c is correct. Signed-off-by: Xavi Drudis Ferran <xdrudis@tinet.cat> --- --- coreboot-disupducode/src/northbridge/amd/Kconfig 2011-02-26 00:01:01.000000000 +0100 +++ coreboot-expert/src/northbridge/amd/Kconfig 2011-02-26 03:02:15.000000000 +0100 @@ -24,8 +24,12 @@ config LIMIT_HT_SPEED_200 bool "Limit HT frequency to 200MHz" +config LIMIT_HT_SPEED_300 + bool "Limit HT frequency to 300MHz" config LIMIT_HT_SPEED_400 bool "Limit HT frequency to 400MHz" +config LIMIT_HT_SPEED_500 + bool "Limit HT frequency to 500MHz" config LIMIT_HT_SPEED_600 bool "Limit HT frequency to 600MHz" config LIMIT_HT_SPEED_800