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

[Tom Rini]

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).

> > [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!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170503/ef6715a1/attachment.sig>