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

[Michael Ellerman]

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.

cheers

-- 
Michael Ellerman
OzLabs, IBM Australia Development Lab

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
URL: <http://lists.ozlabs.org/pipermail/linuxppc-dev/attachments/20061115/d5cb243c/attachment.pgp>