[Cbe-oss-dev] [PATCH] OProfile: fix cbe pm signal routing problem

[Bob Nelson]

Yamamoto-san,

On Friday 10 August 2007 03:13:29 am Takashi02 Yamamoto wrote:
> This patch fixes the cbe pm signal routing problem.

Is this a documented problem?  Where?  I haven't seen anything mentioned 
previously.
> 
> We can confirm the fix by the following oprofile event parameter.
> "opcontrol --event=cache_hit:10000:0x42"

Why are you setting the unit mask?  This is not something you should be doing 
in general, you should use the default values for the unit mask.
> 
> Signed-off-by: Takashi Yamamoto <Takashi02_Yamamoto at hq.scei.sony.co.jp>
> 
> Index: linux-2.6.22.1/arch/powerpc/oprofile/op_model_cell.c
> ===================================================================
> --- linux-2.6.22.1.orig/arch/powerpc/oprofile/op_model_cell.c
> +++ linux-2.6.22.1/arch/powerpc/oprofile/op_model_cell.c
> @@ -291,13 +291,13 @@ static void set_pm_event(u32 ctr, int ev
>         for (i = 0; i < NUM_TRACE_BUS_WORDS; i++) {
>                 if (bus_word & (1 << i)) {
>                         pm_regs.debug_bus_control |=
> -                           (bus_type << (31 - (2 * i) + 1));
> +                           (bus_type << (30 - (2 * i)));
> 
>                         for (j = 0; j < NUM_INPUT_BUS_WORDS; j++) {
>                                 if (input_bus[j] == 0xff) {
>                                         input_bus[j] = i;
>                                         pm_regs.group_control |=
> -                                           (i << (31 - i));
> +                                           (input_bus[j] << (30 - (2 * 
> j)));
I think your change may work for your particular testcase setting the unit 
mask, but it looks to me like it is going to break almost everything else.  I 
am going to take a more thorough look at the module but I can't see how this 
doesn't change/break some case(s) other than your test above.
>                                         break;
>                                 }
>                         }

Bob Nelson