Re: SMTC support status in latest git head.

["Kevin D. Kissell" <kevink@paralogos.com>]

On 01/05/11 05:11, Anoop P A wrote:
> On Tue, 2011-01-04 at 10:33 -0800, Kevin D. Kissell wrote:
>> On 01/04/11 09:54, Anoop P A wrote:
>>> On Tue, 2011-01-04 at 09:21 -0800, Kevin D. Kissell wrote:
>>>> I'm trying to figure out a reason why your change below should help, and
>>>> offhand, modulo tool bugs, I don't see it.  I'm assuming that your diff
>>>> below is a diff relative to the pre-patch stackframe.h.   I wouldn't
>>> Yes patch created against stock code .
>>>
>>>> bless it as an alternative because it moves code and comments
>>>> unnecessarily - all you should really have to do is to move the
>>>>
>>>>
>>>>     190                 mfc0    v1, CP0_STATUS
>>>>     191                 LONG_S  $2, PT_R2(sp)
>>>>
>>>> to be just after the #endif /* CONFIG_MIPS_MT_SMTC */ at around line 201.
>>> Actually I just moved code under CONFIG_MIPS_MT_SMTC to previous block
>>> of code ( which store $0 ) . git diff did the rest on behalf of me :)
>>>
>>>> If moving the save of zero to PT_R0(sp) actually makes a difference,
>>>> it's evidence that you've got problems in your toolchain (or, heaven
>>>> forbid, your pipeline)!
>>> In previous version of patch usage of V0 was creating issue. I have
>>> verified this with previous version of code ( working code before
>>> David's instruction rearrangement patch.) .
>> Argh.  It's not very clearly commented, but it looks as if the system
>> call trap handler has an implicit assumption that v0 has never been
>> changed by SAVE_SOME, TRACE_IRQS_ON_RELOAD, or STI.  So yeah, moving the
>> code around to fix the v1 conflict ends up being better than using v0 -
>> otherwise, we'd need to add a LONG_L v0, PT_R2(sp) somewhere after the
>> LONG_S v0, PT_TCSTATUS(sp) of the original patch.
> Well, Here is the patch.
>
> diff --git a/arch/mips/include/asm/stackframe.h
> b/arch/mips/include/asm/stackframe.h
> index 58730c5..19418c4 100644
> --- a/arch/mips/include/asm/stackframe.h
> +++ b/arch/mips/include/asm/stackframe.h
> @@ -187,8 +187,6 @@
>   		 * need it to operate correctly
>   		 */
>   		LONG_S	$0, PT_R0(sp)
> -		mfc0	v1, CP0_STATUS
> -		LONG_S	$2, PT_R2(sp)
>   #ifdef CONFIG_MIPS_MT_SMTC
>   		/*
>   		 * Ideally, these instructions would be shuffled in
> @@ -199,6 +197,8 @@
>   		.set	mips0
>   		LONG_S	v1, PT_TCSTATUS(sp)
>   #endif /* CONFIG_MIPS_MT_SMTC */
> +		mfc0	v1, CP0_STATUS
> +		LONG_S	$2, PT_R2(sp)
>   		LONG_S	$4, PT_R4(sp)
>   		LONG_S	$5, PT_R5(sp)
>   		LONG_S	v1, PT_STATUS(sp)

That's exactly what I'd propose as the cleanest minimal fix.  I've got a 
version that also replaces the .set mips32 / .set mips0 with the .set 
push / .set pop paradigm, which I'd have used in the original code if 
I'd known at the time about that assembler directive.  I'm hoping to be 
able to test on a Malta/34K reference platform, and make sure there 
isn't breakage on that platform branch as well, before we commit to the 
repository.

Your msp_smtc.c file looks plausible on the face of it.  The 
init_secondary function has the quirk that it expects to execute on each 
"CPU" in numerical order, which is very likely but not guaranteed. It 
*ought* to be harmless in the rare case where it fails, but the 
assumption is worth a comment, IMHO.

At this point, there shouldn't be a whole lot of SMTC-specific mystery 
to get your timer running on the second VPE.  You know it's taking 
interrupts, because of the IPIs getting through, so in principle you 
just need to run the chain of enables from the clock peripheral itself 
through the CIC to the CPU core and the IM bits.

It would be really cool if we could get a stable repository branch that 
boots SMTC out-of-the-box on both Malta and the MSP platform.

             Regards,

             Kevin K.