[PATCH v2 1/7] powerpc/85xx: re-enable timebase sync disabled by KEXEC patch

[Kumar Gala]

On Nov 22, 2011, at 3:29 AM, Li Yang-R58472 wrote:

>> Subject: Re: [PATCH v2 1/7] powerpc/85xx: re-enable timebase sync disabled
>> by KEXEC patch
>> 
>> On Fri, Nov 18, 2011 at 08:35:02AM -0600, Kumar Gala wrote:
>>> 
>>> On Nov 16, 2011, at 12:42 PM, Scott Wood wrote:
>>> 
>>>> On 11/16/2011 03:55 AM, Zhao Chenhui wrote:
>>>>> From: Li Yang <leoli at freescale.com>
>>>>> 
>>>>> The timebase sync is not only necessary when using KEXEC. It should
>> also
>>>>> be used by normal boot up and cpu hotplug. Remove the ifdef added by
>>>>> the KEXEC patch.
>>>> 
>>>> Again, no it should not be used by normal boot up (whether KEXEC
>> support
>>>> is enabled or not).  We should only do timebase sync when we actually
>>>> need to (when we've actually just reset a core), and we should do it
>> the
>>>> way U-Boot does rather than with smp-tbsync.c.
> 
> While looking into the timebase sync code in u-boot, I have a few questions.
> 
>        /* enable time base at the platform */
>        if (whoami)
>                devdisr |= MPC85xx_DEVDISR_TB1;
>        else
>                devdisr |= MPC85xx_DEVDISR_TB0;
>        out_be32(&gur->devdisr, devdisr);
> 
>        /* readback to sync write */
>        in_be32(&gur->devdisr);
> 
>        mtspr(SPRN_TBWU, 0);
>        mtspr(SPRN_TBWL, 0);
> 
> What are the TBWU/TBWL registers?  I can't find the definition of them in either e500 RM or booke RM.  Are they valid to be used?  What is the result of writing to them?  Aren't the SPR registers core specific?  How can we set the TB for the other cores?

TBWU/TBWL are SPR 284/285 (they might be called something a little different in the manual).

You can only set the TB on the core itself.  The way the u-boot code works is it sets TBL/TBU (TB) to 0 in release.S for secondary cores.  In the code you reference we are setting TB on the 'master' core to 0 since we turned TB on earlier for the master core and now are resetting it to 0 to sync all the cores.

> 
>        devdisr &= ~(MPC85xx_DEVDISR_TB0 | MPC85xx_DEVDISR_TB1);
>        out_be32(&gur->devdisr, devdisr);
> 
> Also in the UM, I read "Blocks disabled by DEVDISR must not be re-enabled without a hard reset."  Is it safe we enable it here?

Yes this is safe, TB isn't truly a block.  Manual should technically be updated to exclude TB.

- k