[U-Boot] [PATCH 1/3] arm: am33xx: Fix MPU opp selection

[Lokesh Vutla]

On Wednesday 03 May 2017 06:03 PM, Tom Rini wrote:
> On Tue, May 02, 2017 at 07:40:24PM +0530, Lokesh Vutla wrote:
>> Update MPU frequencies and voltages as per the latest
>> DM[1] dated: OCT 2011 Revised APRIL 2016, Section 5.4.
>> Below is the consolidated data:
>>
>> MPU values for PG 2.0 and later(Package ZCZ and ZCE):
>>
>>  -------------------------------------------------------
>> |	|	  ZCZ		|	  ZCE		|
>> |-------------------------------------------------------|
>> |	| VDD[V]   | ARM [MHz]	| VDD[V]   | ARM [MHz]  |
>> |-------|----------|------------|----------|------------|
>> | NITRO |  1.325   |   1000     |   NA     |    NA      |
>> |-------|----------|------------|----------|------------|
>> | TURBO |   1.26   |    800	|   NA     |    NA      |
>> |-------|----------|------------|----------|------------|
>> |OPP120 |   1.20   |    720     |   NA     |    NA      |
>> |-------|----------|------------|----------|------------|
>> |OPP100 |   1.10   |    600     |   1.10   |    600     |
>> |-------|----------|------------|----------|------------|
>> | OPP50 |   0.95   |    300     |   0.95   |    300     |
>>  -------------------------------------------------------
>>
>> There is no eFuse blown on PG1.0 Silicons due to which there is
>> no way to detect the maximum frequencies supported. So default
>> to OPP100 for which both frequency and voltages are common on both
>> the packages.
> 
> You say OPP100 here, but the code (and table) is for OPP50.  Which means
> a good bit of a speed cut.

The above table is for PG2.0 and later versions. For PG1.0 ARM runs at
500MHz in OPP100. Refer Table 5.3 and Table 5.5 in
DM(http://www.ti.com/lit/ds/symlink/am3356.pdf)

> 
> [snip]
>>  /* MAIN PLL Fdll = 550 MHz, by default */
>>  #ifndef CONFIG_SYS_MPUCLK
>> -#define CONFIG_SYS_MPUCLK	MPUPLL_M_550
>> +#define CONFIG_SYS_MPUCLK	MPUPLL_M_500
>>  #endif
> 
> Update the comment please.  Better yet, Kconfig migration (as this is
> only an am33xx thing).

Hmm.. The above value is used only for PG1.0 silicons and the value is
fixed per SoC. Do we need a Kconfig symbol for that?

> 
> [snip]
>> @@ -132,13 +132,21 @@ int am335x_get_efuse_mpu_max_freq(struct ctrl_dev *cdev)
>>  
>>  	sil_rev = readl(&cdev->deviceid) >> 28;
>>  
>> -	if (sil_rev == 1)
>> -		/* PG 2.0, efuse may not be set. */
>> -		return MPUPLL_M_800;
>> -	else if (sil_rev >= 2) {
>> +	if (sil_rev == 0) {
>> +		/* No efuse in PG 1.0. So use OPP100 */
>> +		return MPUPLL_M_500;
> 
> Isn't that OPP50?
> 
>> +	} else if (sil_rev >= 1) {
>>  		/* Check what the efuse says our max speed is. */
>> -		int efuse_arm_mpu_max_freq;
>> +		int efuse_arm_mpu_max_freq, package_type;
>>  		efuse_arm_mpu_max_freq = readl(&cdev->efuse_sma);
>> +		package_type = (efuse_arm_mpu_max_freq & PACKAGE_TYPE_MASK) >>
>> +				PACKAGE_TYPE_SHIFT;
>> +
>> +		/* PG 2.0, efuse may not be set. */
>> +		if (package_type == PACKAGE_TYPE_UNDEFINED || package_type ==
>> +		    PACKAGE_TYPE_RESERVED)
>> +			return MPUPLL_M_800;
>> +
>>  		switch ((efuse_arm_mpu_max_freq & DEVICE_ID_MASK)) {
>>  		case AM335X_ZCZ_1000:
>>  			return MPUPLL_M_1000;
>> @@ -155,14 +163,14 @@ int am335x_get_efuse_mpu_max_freq(struct ctrl_dev *cdev)
>>  		}
>>  	}
>>  
>> -	/* PG 1.0 or otherwise unknown, use the PG1.0 max */
>> -	return MPUPLL_M_720;
>> +	/* PG 1.0 or otherwise unknown, use the PG1.0 safe */
>> +	return MPUPLL_M_500;
> 
> Is the problem here new PG values getting unsafe values?  I'm concerned
> about slowing down PG1.0 stuff which is also in the wild, in numbers.
> 

No, I just wanted to return a value as it is a non-void function, may be
I should update the comment properly.

Thanks and regards,
Lokesh