Re: [PATCH v5 06/12] MIPS: add support for hardware performance events (mipsxx)

[David Daney <david.s.daney@gmail.com>]

On 05/27/2010 06:03 AM, Deng-Cheng Zhu wrote:
> This patch adds the mipsxx Perf-event support based on the skeleton.
> Generic hardware events and cache events are now fully implemented for
> the 24K/34K/74K/1004K cores. To support other cores in mipsxx (such as
> R10000/SB1), the generic hardware event tables and cache event tables
> need to be filled out. To support other CPUs which have different PMU
> than mipsxx, such as RM9000 and LOONGSON2, the additional files
> perf_event_$cpu.c need to be created.
>
> To test the functionality of Perf-event, you may want to compile the tool
> "perf" for your MIPS platform. You can refer to the following URL:
> http://www.linux-mips.org/archives/linux-mips/2010-04/msg00158.html
>
> Please note: Before that patch is accepted, you can choose a "specific"
> rmb() which is suitable for your platform -- an example is provided in
> the description of that patch.
>
> You also need to customize the CFLAGS and LDFLAGS in tools/perf/Makefile
> for your libs, includes, etc.
>
> In case you encounter the boot failure in SMVP kernel on multi-threading
> CPUs, you may take a look at:
> http://www.linux-mips.org/git?p=linux-mti.git;a=commitdiff;h=5460815027d802697b879644c74f0e8365254020
>
> Signed-off-by: Deng-Cheng Zhu<dengcheng.zhu@gmail.com>
> ---
>   arch/mips/kernel/perf_event.c        |    2 +
>   arch/mips/kernel/perf_event_mipsxx.c |  719 ++++++++++++++++++++++++++++++++++
>   2 files changed, 721 insertions(+), 0 deletions(-)
>   create mode 100644 arch/mips/kernel/perf_event_mipsxx.c
>

General comments:

Can you separate the code that reads and writes the performance counter 
registers from the definitions of the various counters themselves?

Also take into account that the counter registers may be either 32 or 64 
bits wide.  The interfaces you are defining should take that into 
account even if the specific implementations only work with 32-bit 
registers.

Thanks,
David Daney