Hi Azuma-san,
On Thu August 30 2007 6:21 am, takaki.azuma wrote:
> It is a main body of pfm_cell_reset_signals() and
> pfm_cell_activate_signals(). (in the new file: perfmon_cell_signals.c)
> Moreover, this patch includes pfm_cell_pmc_check() too.
>
> -- Takaki Azuma
>
> Signed-off-by: Takaki Azuma <[EMAIL PROTECTED]>
> Signed-off-by: Takayuki Uchikawa <[EMAIL PROTECTED]>
>
> Index: linux-2.6/arch/powerpc/perfmon/perfmon_cell_signals.c
> ===================================================================
> --- linux-2.6.git/arch/powerpc/perfmon/perfmon_cell_signals.c
> +++ linux-2.6/arch/powerpc/perfmon/perfmon_cell_signals.c
> @@ -0,0 +1,270 @@
> +/*
> + * This file contains the Cell performance monitor signal setting
> + * used by perfmon_cell.c.
> + *
> + * (C) Copyright 2007 TOSHIBA CORPORATION
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of version 2 of the GNU General Public
> + * License as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
> + * 02110, USA
> + */
> +#ifndef __PERFMON_CELL_SIGNALS_C__
> +#define __PERFMON_CELL_SIGNALS_C__
The "#ifndef/#define" is normally only used in header files.
> +#include <linux/module.h>
> +#include <linux/perfmon.h>
> +#include <asm/cell-pmu.h>
> +#include <asm/io.h>
> +#include <asm/spu.h>
> +#include <asm/machdep.h>
> +#include "../platforms/cell/cbe_regs.h"
> +
> +#define HID1_RESET_MASK (~0x00000001fffffffful)
> +#define PPU_IU1_WORD0_HID1_EN_MASK (~0x00000001f0c0802cul)
> +#define PPU_IU1_WORD0_HID1_EN_WORD ( 0x00000001f0400000ul)
> +#define PPU_IU1_WORD1_HID1_EN_MASK (~0x000000010fc08023ul)
> +#define PPU_IU1_WORD1_HID1_EN_WORD ( 0x000000010f400001ul)
> +#define PPU_XU_WORD0_HID1_EN_MASK (~0x00000001f038402cul)
> +#define PPU_XU_WORD0_HID1_EN_WORD ( 0x00000001f0080008ul)
> +#define PPU_XU_WORD1_HID1_EN_MASK (~0x000000010f074023ul)
> +#define PPU_XU_WORD1_HID1_EN_WORD ( 0x000000010f030008ul)
> +struct cell_rtas_arg {
> + u16 cpu;
> + u16 sub_unit;
> + s16 signal_group;
> + u8 bus_word;
> + u8 bit;
> +};
> +#define GET_BUS_WORD(x) ((u8)(((x) >> 48) & 0x0ff))
> +#define GET_SIGNAL_GROUP_NUMBER(x) ((s16)(((x) & 0x0ffff) / 100))
If there are structures and/or macros that perfmon_cell.c and
perfmon_cell_signals.c both need to know about, they should be moved to the
perfmon_cell_signals.h file instead of duplicating the structure definition
in multiple places.
> +enum {
> + BUS_WORD_0 = 1,
> + BUS_WORD_1 = 2,
> + BUS_WORD_2 = 4,
> + BUS_WORD_3 = 8,
> +};
> +
> +enum {
> + SIG_GROUP_NONE = 0,
> +
> + /* 2. PowerPC Processor Unit (PPU) Signal Selection */
> + SIG_GROUP_PPU_IU1 = 21,
> + SIG_GROUP_PPU_XU = 23,
> +};
> +
> +#define rmw_spr(spr_id, a_mask, o_mask) \
> + do { \
> + u64 value = mfspr(spr_id); \
> + value &= (u64)(a_mask); \
> + value |= (u64)(o_mask); \
> + mtspr(spr_id, value); \
> + } while (0)
> +
> +#define rmwb_mmio_reg64(mem, bitN, bit) \
> + rmw_mmio_reg64(mem, ~((bit) << (63 - (bitN))), ((bit) << (63 - (bitN))))
> +
> +#define rmw_mmio_reg64(mem, a_mask, o_mask) \
> + do { \
> + u64 value = in_be64(&mem); \
> + value &= ((u64)(a_mask)); \
> + value |= ((u64)(o_mask)); \
> + out_be64(&mem, value); \
> + } while (0)
rmwb_mmio_reg64 should be defined after rmw_mmio_reg64 so it's clear that
there's a dependency.
> +#define passthrough_enable(cpu) passthrough(cpu, 1ul)
> +#define passthrough_disable(cpu) passthrough(cpu, 0ul)
Likewise for these two macros.
> +static inline int passthrough(u32 cpu, u64 bit)
> +{
> + struct cbe_ppe_priv_regs __iomem * ppe_priv_regs;
> + struct cbe_pmd_regs __iomem * pmd_regs;
> + struct cbe_mic_tm_regs __iomem * mic_tm_regs;
> +
> + ppe_priv_regs = cbe_get_cpu_ppe_priv_regs(cpu);
> + pmd_regs = cbe_get_cpu_pmd_regs(cpu);
> + mic_tm_regs = cbe_get_cpu_mic_tm_regs(cpu);
> +
> + if (!ppe_priv_regs || !pmd_regs || !mic_tm_regs) {
> + PFM_DBG("Not found cbe_regs_map -> %ph,%ph,%ph",
> + ppe_priv_regs, pmd_regs, mic_tm_regs);
> + return -EINVAL;
> + }
> +
> + rmwb_mmio_reg64(ppe_priv_regs->L2_debug1, 61, bit);
> + rmwb_mmio_reg64(ppe_priv_regs->ciu_dr1, 5, bit);
> + rmwb_mmio_reg64(pmd_regs->on_ramp_trace, 39, bit);
> + rmwb_mmio_reg64(mic_tm_regs->MBL_debug, 20, bit);
> +
> + return 0;
> +}
> +
> +static inline int reset_signal_registers(u32 cpu)
> +{
The "cpu" argument is not used. Also, this routine can be "void" since it
always returns 0.
> + rmw_spr(SPRN_HID1, HID1_RESET_MASK, 0ul);
> +
> + return 0;
> +}
> +
> +/*
> + * pfm_cell_reset_signals
> + */
> +int pfm_cell_reset_signals(u32 cpu)
> +{
> + passthrough_disable(cpu);
> + reset_signal_registers(cpu);
> + return 0;
Likewise, this routine can be "void" if it always returns 0.
> +}
> +
> +static int ppu_selection(struct cell_rtas_arg * signal)
> +{
> + u64 hid1_enable_word = 0ul;
> + u64 hid1_enable_mask = 0ul;
> +
> + BUG_ON(!signal);
The way that ppu_selection() is being called, there's no way that "signal"
will be NULL.
> +
> + switch (signal->signal_group) {
> + default:
> + PFM_DBG("Not implemented signal_group(%d), now",
> + signal->signal_group);
> + return -EINVAL;
I would recommend putting "default" sections at the end of the "switch"
blocks.
> +
> + case SIG_GROUP_PPU_IU1: /* 2.1 PPU Instruction Unit - Group 1 */
> + switch (signal->bus_word) {
> + default:
> + PFM_DBG("Invalid bus_word(%xh)", signal->bus_word);
> + return -EINVAL;
> + case BUS_WORD_0:
> + hid1_enable_mask = PPU_IU1_WORD0_HID1_EN_MASK;
> + hid1_enable_word = PPU_IU1_WORD0_HID1_EN_WORD;
> + break;
> + case BUS_WORD_1:
> + hid1_enable_mask = PPU_IU1_WORD1_HID1_EN_MASK;
> + hid1_enable_word = PPU_IU1_WORD1_HID1_EN_WORD;
> + break;
> + }
> + break;
> +
> + case SIG_GROUP_PPU_XU: /* 2.3 PPU Execution Unit */
> + switch (signal->bus_word) {
> + default:
> + PFM_DBG("Invalid bus_word(%xh)", signal->bus_word);
> + return -EINVAL;
> + case BUS_WORD_0:
> + hid1_enable_mask = PPU_XU_WORD0_HID1_EN_MASK;
> + hid1_enable_word = PPU_XU_WORD0_HID1_EN_WORD;
> + break;
> + case BUS_WORD_1:
> + hid1_enable_mask = PPU_XU_WORD1_HID1_EN_MASK;
> + hid1_enable_word = PPU_XU_WORD1_HID1_EN_WORD;
> + break;
> + }
> + break;
> + }
> +
> + rmw_spr(SPRN_HID1, hid1_enable_mask, hid1_enable_word);
> +
> + return 0;
> +}
> +
> +/*
> + * pfm_cell_activate_signals
> + */
> +int pfm_cell_activate_signals(struct cell_rtas_arg * signals, int
> num_signals) +{
> + int err = 0;
> + int i;
> +
> + BUG_ON((0 < num_signals) && !signals);
Don't BUG_ON() in this situation. The routine needs treat this as an error and
return a proper error code.
> +
> + for (i = 0; i < num_signals; i++) {
> +
> + switch (signals[i].signal_group) {
> + default:
> + PFM_DBG("Not implemented signal_group(%d), now",
> + signals[i].signal_group);
> + return -EINVAL;
> +
> + /* 2. PowerPC Processor Unit (PPU) Signal Selection */
> + case SIG_GROUP_PPU_IU1:
> + case SIG_GROUP_PPU_XU:
> + err = ppu_selection(&signals[i]);
> + if (err != 0)
> + return err;
> + break;
> + }
> +
> + passthrough_enable(signals[i].cpu);
> +
> + break;
> + }
> +
> + return err;
> +}
> +
> +/*
> + * pfm_cell_pmc_check
> + */
> +int pfm_cell_pmc_check(struct pfm_context * ctx,
> + struct pfm_event_set * set,
> + struct pfarg_pmc * req)
> +{
> + u64 cnum;
> + u64 reg_num;
cnum and reg_num don't need to be a u64. u16 should be fine.
> + s16 signal_group;
> + u8 bus_word;
> +
> + BUG_ON(!ctx || !set || !req);
Checking "ctx", "set", and "req" shouldn't be necessary. The Perfmon core has
already validated those pointers.
> +
> + reg_num = req->reg_num;
> + bus_word = GET_BUS_WORD(req->reg_value);
> + signal_group = GET_SIGNAL_GROUP_NUMBER(req->reg_value);
> +
> + BUG_ON((reg_num < NR_CTRS) || ((NR_CTRS * 2) <= reg_num));
Treat this as an error and return a proper error code.
> +
> + switch (signal_group) {
> + default:
> + PFM_DBG("Not implemented signal_group(%d), now", signal_group);
> + return -EINVAL;
> +
> + case SIG_GROUP_PPU_IU1:
> + case SIG_GROUP_PPU_XU:
> + if ((bus_word != 0) && (bus_word != 1)) {
> + PFM_DBG("invalid bus word. (%x,%d)",
> + bus_word, signal_group);
> + return -EINVAL;
> + }
> + break;
> + }
> +
> + for (cnum = NR_CTRS; cnum < (NR_CTRS * 2); cnum++) {
> + if (!test_bit(cnum, cast_ulp(set->used_pmcs)))
> + continue;
> + if (bus_word != GET_BUS_WORD(set->pmcs[cnum]))
> + continue;
> + if (signal_group == GET_SIGNAL_GROUP_NUMBER(set->pmcs[cnum]))
> + continue;
> +
> + PFM_DBG("impossible combination. (%lu,%u,%d) (%lu,%u,%d)",
> + reg_num, bus_word, signal_group, cnum,
> + GET_BUS_WORD(set->pmcs[cnum]),
> + GET_SIGNAL_GROUP_NUMBER(set->pmcs[cnum]));
> + return -EBUSY;
> + }
> +
> + return 0;
> +}
> +
> +#endif /* __PERFMON_CELL_SIGNALS_C__ */
> Index: linux-2.6/arch/powerpc/perfmon/perfmon_cell_signals.h
> ===================================================================
> --- linux-2.6.git/arch/powerpc/perfmon/perfmon_cell_signals.h
> +++ linux-2.6/arch/powerpc/perfmon/perfmon_cell_signals.h
> @@ -0,0 +1,31 @@
> +/*
> + * This file contains the Cell performance monitor signal setting
> + * used by perfmon_cell.h.
> + *
> + * (C) Copyright 2007 TOSHIBA CORPORATION
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of version 2 of the GNU General Public
> + * License as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
> + * 02110, USA
> + */
> +#ifndef __PERFMON_CELL_SIGNALS_H__
> +#define __PERFMON_CELL_SIGNALS_H__
> +
> +extern int pfm_cell_reset_signals(u32 cpu);
> +extern int pfm_cell_activate_signals(struct cell_rtas_arg * signals,
> + int num_signals);
> +extern int pfm_cell_pmc_check(struct pfm_context * ctx,
> + struct pfm_event_set * set,
> + struct pfarg_pmc * req);
> +
> +#endif /* __PERFMON_CELL_SIGNALS_H__ */
Thanks,
--
Kevin Corry
[EMAIL PROTECTED]
http://www.ibm.com/linux/
_______________________________________________
perfmon mailing list
[email protected]
http://www.hpl.hp.com/hosted/linux/mail-archives/perfmon/
