On Mon, Jan 18, 2010 at 23:35,  <[email protected]> wrote:
> Modified: trunk/arch/blackfin/include/asm/context.S (8178 => 8179)
>
> @@ -73,6 +75,7 @@
>  #else
>       cli r0;
>  #endif
> +     TRACE_IRQS_OFF

this call site doesnt need to save any registers before calling a C
function as all registers have been saved onto the stack

> @@ -289,6 +292,7 @@
>       p0.l = _bfin_irq_flags;
>       r0 = [p0];
>  #endif
> +     TRACE_IRQS_ON
>       sti r0;
>
>       sp += 4;        /* Skip Reserved */

no need to save any registers here as all of them get restored from the stack

> Modified: trunk/arch/blackfin/include/asm/irqflags.h (8178 => 8179)
>
> +#ifdef CONFIG_TRACE_IRQFLAGS
> +
> +.macro bfin_trace_hardirqs_on
> +     [--sp] = rets;
> +     [--sp] = (r7:0, p5:0);
> +     sp += -12;
> +     call _trace_hardirqs_on;
> +     sp += 12;
> +     (r7:0, p5:0) = [sp++];
> +     rets = [sp++];
> +.endm
> +
> +.macro bfin_trace_hardirqs_off
> +     [--sp] = rets;
> +     [--sp] = (r7:0, p5:0);
> +     sp += -12;
> +     call _trace_hardirqs_off;
> +     sp += 12;
> +     (r7:0, p5:0) = [sp++];
> +     rets = [sp++];
> +.endm

this seems pretty heavy handed.  are you sure you need to save rets ?
any called C function should be saving/restoring it itself as part of
the ABI.  it might be better to have the call site worry about
registers that need saving since none of the ones here need any regs
saved ...

> +#  define TRACE_IRQS_ON      bfin_trace_hardirqs_on
> +#  define TRACE_IRQS_OFF     bfin_trace_hardirqs_off
> +#else
> +#  define TRACE_IRQS_ON
> +#  define TRACE_IRQS_OFF
>  #endif

this indirection really isnt needed.  have the assembly macro expand
into nothing:
.macro bfin_trace_hardirqs_off
#ifdef TRACE_IRQS_ON
....
#endif
.endm

then always call bfin_trace_hardirqs_{on,off}

> Modified: trunk/arch/blackfin/mach-common/entry.S (8178 => 8179)
>
> @@ -966,6 +966,7 @@
>  #else
>       cli r0;
>  #endif
> +     TRACE_IRQS_OFF
>       [--sp] = RETI;
>       SP += 4;
>       rts;

i dont think any regs need saving here

> @@ -997,6 +998,7 @@
>       p0.h = _bfin_irq_flags;
>       r0 = [p0];
>  #endif
> +     TRACE_IRQS_ON
>       sti r0;
>
>       /* finish the userspace "atomic" functions for it */

nor here

> Modified: trunk/arch/blackfin/mach-common/interrupt.S (8178 => 8179)
>
> @@ -88,6 +88,7 @@
>  #else
>       cli r1;
>  #endif
> +     TRACE_IRQS_OFF
>       [--sp] = RETI;  /* orig_pc */
>       /* Clear all L registers.  */
>       r1 = 0 (x);

all state was just saved, so no need to save any regs here
-mike
_______________________________________________
Linux-kernel-commits mailing list
[email protected]
https://blackfin.uclinux.org/mailman/listinfo/linux-kernel-commits

Reply via email to