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

[Lokesh Vutla]

On Wednesday 03 May 2017 08:53 PM, Tom Rini wrote:
> On Wed, May 03, 2017 at 07:36:32PM +0530, Lokesh Vutla wrote:
>>
>>
>> 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?
> 
> If you can get rid of it, do so, but please make sure to check on how
> the other am335x boards are making use of it (ie the Siemens stuff).

okay Ill check it.

> 
>>> [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.
> 
> My concern is that PG1.0 stuff was previously being clocked at 720MHz,
> but now will be down to 500MHz.  I'm not sure if in these cases anything
> else (ie Linux) touches this and would be kicking it back up.  It'll
> also slow down boot there a bit.  Thanks!

Till now for PG 1.0 we are clocking at 720MHz(OPP_NITRO for ZCZ package)
and configuring voltages at 1.13V(OPP_100) which is wrong(Especially for
ZCE package 500MHz is the maximum supported value.) I tried to make a
common value for everything.

Do you want me to run at 720MHz then increase the voltage accordingly
and handle ZCE package separately?

Thanks and regards,
Lokesh