[RFC] 4xx hardware watchpoint support
Luis Machado
luisgpm at linux.vnet.ibm.com
Thu Jul 24 00:42:02 EST 2008
> Some comment, first the above negate conditional
> looks rather ugly, I'd rather do a
>
> #if defined(CONFIG_4xx) || defined(CONFIG_BOOKE)
> dbcr0 case
> #else
> dabr case
> #endif
Yes, it makes sense. I'll switch it around.
> second I wonder why we have the notify_die only for one case, that seems
> rather odd. Looking further the notify_die is even more odd because
> DIE_DABR_MATCH doesn't actually appear anywhere else in the kernel.
> I'd suggest simply removing it.
DIE_DABR_MATCH doesn't appear anywhere else because there is only a
single function responsible for handling the DABR/DAC events on powerPC
with this modification. It would make sense to call this to both the
DAC/DABR cases though (i.e. taking it out of the #ifdef), what do you
think?
> Can you redo this in the normal Linux comment style, ala:
>
> /*
> * For processors using DABR (i.e. 970), the bottom 3 bits are flags.
> * It was assumed, on previous implementations, that 3 bits were
> * passed together with the data address, fitting the design of the
> * DABR register, as follows:
> *
> * bit 0: Read flag
> * bit 1: Write flag
> * bit 2: Breakpoint translation
> *
> * Thus, we use them here as so.
> */
>
> and similar in few other places.
Will do, thanks for reviewing this one.
Regards,
Luis
More information about the Linuxppc-dev
mailing list