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

From: Michael Ellerman <[email-masked]>
Date: Tue, 14 Nov 2006 14:44:40 +1100

On Fri, 2006-11-10 at 12:01 -0800, Geoff Levand wrote:
> This adds a platform specific spu management abstraction and the coresponding
> routines to support the IBM Cell Blade.  It also removes the hypervisor only
> resources that were included in struct spu.
> 
> Three new platform specific routines are introduced, spu_enumerate_spus(),
> spu_create_spu() and spu_destroy_spu().  The underlining design uses a new
> type, struct spu_management_ops, to hold function pointers that the platform
> setup code is expected to initialize to instances appropriate to that platform.
> 
> For the IBM Cell Blade support, I put the hypervisor only resources that were
> in struct spu into a platform specific data structure struct spu_pdata.
> 
> 
> Signed-off-by: Geoff Levand <[email-masked]>
> 
> ---
> 
> Michael,
> 
> Unfortunately, for your xmon spu support, your DUMP_FIELD is setup in such a
> way that it is not easy to change to use from inside spu_priv1_mmio.c, so I
> left of_dump_pdata_fields() empty.  We'll need to work on something usable
> there, or make some other way to abstract those platform specific spu
> variables.

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
 
        DUMP_FIELD(spu, "0x%p", priv2);
 }

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