Re: [PATCH v5 01/12] MIPS/Oprofile: extract PMU defines/helper functions for sharing

[Deng-Cheng Zhu <dengcheng.zhu@gmail.com>]

Hi, David


Thanks for your comments! I'm replying to you in the patch order. If my
comments are wrong or are bad ideas, please point out.


2010/5/28 David Daney <david.s.daney@gmail.com>:
> Why predicate the entire contents of the file?
>
> In any event, if you keep it, it shold probably be something like:
>
>    #if defined(CONFIG_CPU_MIPSR1) || defined(CONFIG_CPU_MIPSR2)
[DC]:
1) This line is not for the "entire" contents of the file, other CPUs
LOONGSON2 and RM9000 are here.
2) The CONFIG_CPU* came from the Makefile of oprofile. If other CPUs are
newly supported, we can add into the #if #elif.
3) The perf counter helper functions are special to mipsxx/loongson2/rm9000
espcially the reset_counter() will be implemented respectively. Although
they will be moved to perf_event_$cpu.c when Oprofile uses Perf-events as
backend, they are currently shared by Oprofile and Perf-events to reduce
code copying.


> Some or all of that should probably go in asm/mipsregs.h
[DC]: Now that we create pmu.h and these #define's are originally in
op_model_$cpu.c not in mipsregs.h, how about keeping them here? BTW, after
making Oprofile use Perf-events as backend (patches 8~12 do this), pmu.h
will only contain register definitions (no static function definitions),
then we can consider deleting pmu.h and moving its contents to mipsregs.h,
is it OK?


> Are 0 and 1 really the only conceivable values?
[DC]: This is also from Oprofile. If we use:
#define vpe_id()       (cpu_has_mipsmt_pertccounters ? \
                       0 : cpu_data[smp_processor_id()].vpe_id)
The possible return value is supposed to be 0 or 1.


Deng-Cheng