RFC: performance monitor/oprofile support for e500

[Andy Fleming]

Well, it looks like I messed up the patch, and included some stuff I'm 
working on for generalizing the PHY interface.  Strange, because I 
thought I had hand-inspected that patch...  Ah, I see: I copied the 
wrong patch, which was done against linux-2.6, rather than my internal 
tree.  Please ignore the patch, since much of that code is obsolete.

Anyway, I will respond to Kumar's comments, and have attached the 
CORRECT patch:

On Oct 31, 2004, at 23:43, Kumar Gala wrote:

> Andy,
>
> Some feedback:
>
> * Any reason to get ride of the 604 code now,w/o replacing it with 
> something?

Well, it doesn't really do much, and I was told that no one was using 
it.  If people want, I can add it back.

> * Why have you introduce two CPU features (CPU_FTR_CAN_USE_PMON_INTR & 
> CPU_FTR_FSL_BOOKE_PMON) you dont use ?

Ah, this is for the near future, when I implement support for 74xx 
processors.  The non-Freescale-Book-E parts use a different perfmon 
scheme, and I wanted a way to distinguish.  It may not be useful, 
though, I agree, since I am using CONFIG_FSL_BOOKE to do this.  
CAN_USE_PMON_INTR is important, though, to detect the errata which 
could send the processor into never-neverland.  Eventually, some 74xx 
processors will have this bit set, while some will not.

> * In the PerformanceMonitor can't you use regs->nip for the same 
> purpose of regs->sia?

Hmmm.... somehow I missed this when I was looking for a way to get the 
sampled address.  I will change this if no one thinks of a good reason 
to do otherwise.

> * Does arch/ppc/kernel/perfmon.c really need all the headers you are 
> including?

Probably not.  Does anyone know an easy way to determine which headers 
are needed?

> * Does perfmon.c make more sense as perfmon_fsl_booke.c

Well, I thought about that, but I figured that it was ok to put all of 
the code into one file, and use #ifdef to distinguish.  I'm willing to 
divide it into two (one for FSL booke, and one for classic), though.

> * op_ppc32_setup, I'd prefer the saving & restoring of the perf_irq 
> like arch/ppc64/oprofile/common.c does

Well, does this really make sense?  If one driver grabs the interrupt, 
and a subsequent driver tries to grab it, do we want the second driver 
to win, but nicely restore the handler later, or do we want the second 
driver to fail, and give the user the chance to disable the other 
driver?  I definitely won't use the method in ppc64, since that isn't 
thread-safe if more than one driver wants the perfmon interrupt.

> * a number of issues related to SMP

I admit, I haven't carefully worked this out, as there aren't any SMP 
e500 systems out there.

> * any reason get_kernel is a function?

Nope.  Just legacy from having copied the ppc64 code.

> * Do ctr_read & ctr_write really belong in op_impl.h, especially since 
> different implementations will be needed for classic ppc.

Good point.  I will move these to op_model_e500.c

>
>
> I sent a note to Paul M. about putting num_counters into cpu_specs 
> table.

That would be good.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: oprofile-kernel-patch2
Type: application/octet-stream
Size: 31038 bytes
Desc: not available
Url : http://ozlabs.org/pipermail/linuxppc-embedded/attachments/20041101/0d01e17a/attachment.obj