[PATCH 6/16] cell: abstract spu management routines

[Geoff Levand]

Michael Ellerman wrote:
> On Tue, 2006-11-14 at 02:50 -0800, Geoff Levand wrote:
>> Arnd Bergmann wrote:
>> > On Tuesday 14 November 2006 04:44, Michael Ellerman wrote:
>> >> OK, back to the task at hand :) �For the moment I'd rather see you leave
>> >> out dump_data_fields(), as neither HV or baremetal implementations do
>> >> anything. Just put the offending xmon code inside an #ifdef
>> >> CONFIG_PPC_CELL_NATIVE. eg:
>> >> 
>> >> Index: cell/arch/powerpc/xmon/xmon.c
>> >> ===================================================================
>> >> --- cell.orig/arch/powerpc/xmon/xmon.c �2006-11-14 14:43:11.000000000 +1100
>> >> +++ cell/arch/powerpc/xmon/xmon.c � � � 2006-11-14 14:42:35.000000000 +1100
>> >> @@ -2807,12 +2807,11 @@ static void dump_spu_fields(struct spu *
>> >> � � � � � � � � � � � � in_be32(&spu->problem->spu_status_R));
>> >> � � � � DUMP_VALUE("0x%x", problem->spu_npc_RW,
>> >> � � � � � � � � � � � � in_be32(&spu->problem->spu_npc_RW));
>> >> +#ifdef CONFIG_PPC_CELL_NATIVE
>> >> � � � � DUMP_FIELD(spu, "0x%p", priv1);
>> >> -
>> >> - � � � if (spu->priv1) {
>> >> - � � � � � � � DUMP_VALUE("0x%lx", priv1->mfc_sr1_RW,
>> >> - � � � � � � � � � � � � � � � in_be64(&spu->priv1->mfc_sr1_RW));
>> >> - � � � }
>> >> + � � � DUMP_VALUE("0x%lx", priv1->mfc_sr1_RW,
>> >> + � � � � � � � � � � � in_be64(&spu->priv1->mfc_sr1_RW));
>> >> +#endif
>> > 
>> > Oops. Null pointer dereference.
>> > 
>> > You can't do this if you want to compile in both native and PS3PF
>> > support in a single kernel. I think your original code that prints
>> > priv1 and the sr1 only if priv1 exists is fine on both ways.
>> 
>> No, that won't work either since I moved priv1 to struct spu_pdata.
>> 
>> +struct spu_pdata {
>> +	int nid;
>> +	struct device_node *devnode;
>> +	struct spu_priv1 __iomem *priv1;
>> +};
> 
> OK, I wasn't very clear. I didn't mean that patch was the final solution
> - obviously it needs to take into account the movement of priv1 etc.
> 
>> The only way I see this working is to have a platform specific
>> routine to dump those spu_pdata variables.  The problem is that
>> only the spu platform code knows about the variables, and only the
>> xmon code knows how to do the dump.  That is why I suggested earlier
>> to re-work the xmon code to make a dump routine with global scope
>> that can be called from the various spu platform 
>> spu_dump_pdata_fields() routines.
>> 
>> Note that this is not specific to the native support, since I want
>> to hook my platform code into xmon also, and I need to somehow
>> dump my variables.  So the solution is not to add priv1 back into
>> struct spu.
> 
> I'm not that bothered, but I really don't think we want to spend too
> much effort engineering interfaces between xmon and the spu code - at
> the end of the day xmon is just a hackish debugging aid for _kernel
> developers_.
> 
> The simplest solution might just be to dump the address of the pdata
> pointer, and leave it up to the xmon user to dump that and interpret it
> as they see fit.

OK, that's a good solution for now.  I'll set it up that way in my patches.

-Geoff