On Tue, Jul 22, 2008 at 10:47:58PM -0300, Luis Machado wrote:
> +void do_dabr(struct pt_regs *regs, unsigned long address,
> + unsigned long error_code)
> +{
> + siginfo_t info;
> +
> +#if !(defined(CONFIG_4xx) || defined(CONFIG_BOOKE))
> + if (notify_die(DIE_DABR_MATCH, "dabr_match", regs, error_code,
> + 11, SIGSEGV) == NOTIFY_STOP)
> + return;
> +
> + if (debugger_dabr_match(regs))
> + return;
> +
> + /* Clear the DAC and struct entries. One shot trigger. */
> +#else /* (defined(CONFIG_4xx) || defined(CONFIG_BOOKE)) */
> + mtspr(SPRN_DBCR0, mfspr(SPRN_DBCR0) & ~(DBSR_DAC1R | DBSR_DAC1W
> + | DBCR0_IDM));
> +#endif
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
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.
> + /* 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. */
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.
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
