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

[Lokesh Vutla <lokeshvutla@ti.com>]

On Wednesday 03 May 2017 09:19 PM, Tom Rini wrote:
> On Wed, May 03, 2017 at 09:10:35PM +0530, Lokesh Vutla wrote:
>>
>>
>> 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.

As you said, looks like lot of other users are using it. Ill convert to
Kconfig symbol.

>>
>>>
>>>>> [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?
> 
> Well, part of the big open question here I have is, what does and
> doesn't exist (and was shipped) for PG 1.0?  My recollection at the time
> was that we didn't care about ZCZ vs ZCE packages as it basically came
> down to Beaglebone White (and some lead customers) that got ZCZ PG 1.0
> and everything else got PG2.x (with PG2.0 also being selective and PG2.1
> being the general release).  I really don't want to start handicapping

Okay. Then Ill use 720MHz for PG1.0 and repost the series.

Thanks and regards,
Lokesh