my apologies for yet more pedantic nitpickery, this time related to how to properly move/locate macro definitions in header files. i notice this bit in include/mpc8xx.h (note the two tests of "#if 0"):

/*----------------------------------------------------------------------- * TBSCR - Time Base Status and Control Register 11-26 */ #define TBSCR_TBIRQ7 0x8000 /* Time Base Interrupt Request 7 */ #define TBSCR_TBIRQ6 0x4000 /* Time Base Interrupt Request 6 */ #define TBSCR_TBIRQ5 0x2000 /* Time Base Interrupt Request 5 */ #define TBSCR_TBIRQ4 0x1000 /* Time Base Interrupt Request 4 */ #define TBSCR_TBIRQ3 0x0800 /* Time Base Interrupt Request 3 */ #define TBSCR_TBIRQ2 0x0400 /* Time Base Interrupt Request 2 */ #define TBSCR_TBIRQ1 0x0200 /* Time Base Interrupt Request 1 */ #define TBSCR_TBIRQ0 0x0100 /* Time Base Interrupt Request 0 */ #if 0 /* already in asm/8xx_immap.h */ #define TBSCR_REFA 0x0080 /* Reference Interrupt Status A */ #define TBSCR_REFB 0x0040 /* Reference Interrupt Status B */ #define TBSCR_REFAE 0x0008 /* Second Interrupt Enable A */ #define TBSCR_REFBE 0x0004 /* Second Interrupt Enable B */ #define TBSCR_TBF 0x0002 /* Time Base Freeze */ #define TBSCR_TBE 0x0001 /* Time Base Enable */ #endif

/*----------------------------------------------------------------------- * PISCR - Periodic Interrupt Status and Control Register 11-31 */ #undef PISCR_PIRQ /* TBD */ #define PISCR_PITF 0x0002 /* Periodic Interrupt Timer Freeze */ #if 0 /* already in asm/8xx_immap.h */ #define PISCR_PS 0x0080 /* Periodic interrupt Status */ #define PISCR_PIE 0x0004 /* Periodic Interrupt Enable */ #define PISCR_PTE 0x0001 /* Periodic Timer Enable */ #endif

sure enough, the parts that are "if 0"ed out are indeed in arch/powerpc/include/asm/8xx_immap.h:

#define TBSCR_TBIRQ_MASK ((ushort)0xff00) #define TBSCR_REFA ((ushort)0x0080) #define TBSCR_REFB ((ushort)0x0040) #define TBSCR_REFAE ((ushort)0x0008) #define TBSCR_REFBE ((ushort)0x0004) #define TBSCR_TBF ((ushort)0x0002) #define TBSCR_TBE ((ushort)0x0001)

... snip ...

#define PISCR_PIRQ_MASK ((ushort)0xff00) #define PISCR_PS ((ushort)0x0080) #define PISCR_PIE ((ushort)0x0004) #define PISCR_PTF ((ushort)0x0002) #define PISCR_PTE ((ushort)0x0001)

but this brings up the question, where should content like that properly be defined? it seems awkward to break the bit flag definitions for a single register over two header files (especially when they're not defined syntactically identically). is there a historical reason that that content is split over two header files?

oh, and about the above, there is only one reference in the entire tree to PISCR_PIRQ (where it's being undef'ed):

include/mpc8xx.h:#undef PISCR_PIRQ /* TBD */

so i'm guessing that's superfluous (perhaps the old form for defining PISCR_PIRQ_MASK?). i'd submit a patch to get rid of it, but i'll hold off if i can do a larger cleanup that covers that as well in one shot.

thoughts?

rday

p.s. hang on ... this bit in arch/powerpc/include/asm/8xx_immap.h:

#define TBSCR_TBIRQ_MASK ((ushort)0xff00) #define TBSCR_REFA ((ushort)0x0080) #define TBSCR_REFB ((ushort)0x0040) #define TBSCR_REFAE ((ushort)0x0008) #define TBSCR_REFBE ((ushort)0x0004) #define TBSCR_TBF ((ushort)0x0002) #define TBSCR_TBE ((ushort)0x0001)

#define RTCSC_RTCIRQ_MASK ((ushort)0xff00) #define RTCSC_SEC ((ushort)0x0080) #define RTCSC_ALR ((ushort)0x0040) #define RTCSC_38K ((ushort)0x0010) #define RTCSC_SIE ((ushort)0x0008) #define RTCSC_ALE ((ushort)0x0004) #define RTCSC_RTF ((ushort)0x0002) #define RTCSC_RTE ((ushort)0x0001)

#define PISCR_PIRQ_MASK ((ushort)0xff00) #define PISCR_PS ((ushort)0x0080) #define PISCR_PIE ((ushort)0x0004) #define PISCR_PTF ((ushort)0x0002) #define PISCR_PTE ((ushort)0x0001)

i just noticed that none of the *_MASK macros are referenced anywhere in the entire source tree:

$ grep -r TBSCR_TBIRQ_MASK * arch/powerpc/include/asm/8xx_immap.h:#define TBSCR_TBIRQ_MASK ((ushort)0xff00) $ grep -r RTCSC_RTCIRQ_MASK * arch/powerpc/include/asm/8xx_immap.h:#define RTCSC_RTCIRQ_MASK ((ushort)0xff00) $ grep -r PISCR_PIRQ_MASK * arch/powerpc/include/asm/8xx_immap.h:#define PISCR_PIRQ_MASK ((ushort)0xff00) arch/powerpc/include/asm/immap_8260.h:#define PISCR_PIRQ_MASK ((ushort)0xff00) $

so what are they used for?