On Mon, May 18, 2009 at 01:35:32PM +1000, Benjamin Herrenschmidt wrote: > On Thu, 2009-05-14 at 19:13 +0530, K.Prasad wrote: > > plain text document attachment (ppc64_prepare_code_01) > > Prepare the PowerPC code for HW Breakpoint infrastructure patches by > > including > > relevant constant definitions and function declarations. > > Hi ! > > Some comments below... > > > +#define HBP_NUM 1 /* Number of physical HW breakpoint registers */ > > Can you use a more verbose constant ? reg.h is included everywhere so > the risk of collision is high. >
This constant is used by the generic HW Breakpoint code in kernel/hw_breakpoint.c too and renaming it here will require changes there too, while I don't see any existing name-space clashes. Instead the definition can be moved into arch/powerpc/include/asm/hw_breakpoint.h and in arch/power/include/asm/processor.h. What do you think? > > #define DABR_TRANSLATION (1UL << 2) > > #define DABR_DATA_WRITE (1UL << 1) > > #define DABR_DATA_READ (1UL << 0) > > +#define DABR_DATA_RW (3UL << 0) > > Do you really need that ? It's just DABR_DATA_WRITE | DABR_DATA_READ :-) > Yes, it looks silly. I will remove it. The HW_BREAKPOINT_RW defined in arch/powerpc/include/asm/hw_breakpoint.h will instead be defined as: #define HW_BREAKPOINT_RW (DABR_DATA_READ | DABR_DATA_WRITE) > Cheers, > Ben. > Thanks, K.Prasad _______________________________________________ Linuxppc-dev mailing list [email protected] https://ozlabs.org/mailman/listinfo/linuxppc-dev
