Subject: Re: [patch 2/2] Cell: Wrap master run control bit

From: Geoff Levand <[email-masked]>
Date: Thu, 06 Sep 2007 18:31:20 -0700

Arnd Bergmann wrote:
> On Tuesday 04 September 2007, Geoff Levand wrote:
>> 
>> From: Masato Noguchi <[email-masked]>
>> 
>> Add platform specific SPU run control routines.
>> 
>> The current spufs_run_spu() implementation uses the SPU master
>> run control bit (MFC_SR1[S]) to control SPE execution, but the
>> PS3 hypervisor does not support the use of this feature.  This
>> change adds run control wrapper routines.  The  bare metal
>> routines use the master run control bit, and the PS3 specific
>> routines use the priv2 run control register.
>> 
>> Signed-off-by: Masato Noguchi <[email-masked]>
>> Signed-off-by: Geoff Levand <[email-masked]>
> 
> Hmm, ok. Since I don't think we'll get to do the right solution
> (unmapping the ps registers on the ps3) that soon, doing this
> patch is at least better than the current situation where we
> end up with an oops every time we leave spu_run.
> 
> It would probably be good to mention the remaining problem
> with the current approach in the changelog and as a comment
> in the ps3 specific functions.

Ok, I did that.

>> @@ -178,6 +179,8 @@ struct spu_management_ops {
>>  	int (*enumerate_spus)(int (*fn)(void *data));
>>  	int (*create_spu)(struct spu *spu, void *data);
>>  	int (*destroy_spu)(struct spu *spu);
>> +	int (*enable_spu)(struct spu_context *ctx);
>> +	int (*disable_spu)(struct spu_context *ctx);
>>  	int (*init_affinity)(void);
>>  };
> 
> Also, I think you should make the return type of the callback
> 'void' since the result is not used anywhere.

Noguchi-san was hesitant to do this.  I also thought lets leave it
as is until we consider the unmapping support, as maybe a return
value might make sense.  Jeremy, if you think it better to return
void, please make the update.

-Geoff