Sergei, Thanks for your review! > I suggest to also remove these 3 lines from the heading comment in this > file -- they're not true anyway: > > * This file gets included from lowlevel asm headers too, to provide > * wrapped versions of the local_irq_*() APIs, based on the > * raw_local_irq_*() macros from the lowlevel headers.
Good point. > > +#ifdef CONFIG_TRACE_IRQFLAGS > > +#define TRACE_DISABLE_INTS bl .powerpc_trace_hardirqs_off > > +#else > > +#define TRACE_DISABLE_INTS > > +#endif > > + > > Erm, weren't those supposed to be in <asm-powerpc/irqflags.h>? I guess they could be there. I was mucking my way through ;) > The following code seemed over-engineered: Yeah, perfectly correct. See my last mail to this thread from this morning. > Again, could have been more compact... unless trace_hardirqs_*() calls > need to be in certain order WRT writes to PACASOFTIRQEN -- if so, in the 1st > case that I've pointed out the order was not identical (probably wrong?)... Yeah, there is an ordering requirement and it was wrong in that version of the patch. > > +/* > > + * crappy helper for irq-trace > > + */ > > + > > +#include <asm/ppc_asm.h> > > +#include <asm/asm-offsets.h> > > + > > +#define STACKSPACE GPR0 + 16*8 > > I guess this should be 16*4 for PPC32. > Didn't you forget to add GPR0 to the offsets here? > You certainly did. I bet you're clobbering GPR11/12 (if I didn't > miscount)... All correct. That might even be why it didn't work on 32-bit for me and then I gave up. In any case, I completely rewrote this file. johannes
signature.asc
Description: This is a digitally signed message part
_______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
