Re: [Patch 2/6] MIPS: Add HARDWARE_WATCHPOINTS definitions andsupport code.

From: Geert Uytterhoeven
Date: Thu Sep 11 2008 - 03:27:31 EST


On Wed, 10 Sep 2008, David Daney wrote:

Given

> + case 4:
> + write_c0_watchlo3(watches->watchlo[3]);
> + /* Write 1 to the I, R, and W bits to clear them, and
> + 1 to G so all ASIDs are trapped. */
> + write_c0_watchhi3(0x40000007 | watches->watchhi[3]);
> + case 3:
> + write_c0_watchlo2(watches->watchlo[2]);
> + write_c0_watchhi2(0x40000007 | watches->watchhi[2]);
> + case 2:
> + write_c0_watchlo1(watches->watchlo[1]);
> + write_c0_watchhi1(0x40000007 | watches->watchhi[1]);
> + case 1:
> + write_c0_watchlo0(watches->watchlo[0]);
> + write_c0_watchhi0(0x40000007 | watches->watchhi[0]);

and

> + case 4:
> + watches->watchhi[3] = (read_c0_watchhi3() & 0x0fff);
> + case 3:
> + watches->watchhi[2] = (read_c0_watchhi2() & 0x0fff);
> + case 2:
> + watches->watchhi[1] = (read_c0_watchhi1() & 0x0fff);
> + case 1:
> + watches->watchhi[0] = (read_c0_watchhi0() & 0x0fff);

and

> + case 8:
> + write_c0_watchlo7(0);
> + case 7:
> + write_c0_watchlo6(0);
> + case 6:
> + write_c0_watchlo5(0);
> + case 5:
> + write_c0_watchlo4(0);
> + case 4:
> + write_c0_watchlo3(0);
> + case 3:
> + write_c0_watchlo2(0);
> + case 2:
> + write_c0_watchlo1(0);
> + case 1:
> + write_c0_watchlo0(0);

do the same for each registers, perhaps it makes sense to create
read_c0_watchhi(), write_c0_watchlo(), and write_c0_watchhi() macros
that take the watchdog register index as a parameter? Then the above can
be turned in simple loops.

> + write_c0_watchlo0(7);
> + t = read_c0_watchlo0();
> + write_c0_watchlo0(0);
> + c->watch_reg_masks[0] = t & 7;
> +
> + /* Write the mask bits and read them back to determine which
> + * can be used. */
> + c->watch_reg_count = 1;
> + c->watch_reg_use_cnt = 1;
> + t = read_c0_watchhi0();
> + write_c0_watchhi0(t | 0xff8);
> + t = read_c0_watchhi0();
> + c->watch_reg_masks[0] |= (t & 0xff8);
> + if ((t & 0x80000000) == 0)
> + return;
> +
> + write_c0_watchlo1(7);
> + t = read_c0_watchlo1();
> + write_c0_watchlo1(0);
> + c->watch_reg_masks[1] = t & 7;
> +
> + c->watch_reg_count = 2;
> + c->watch_reg_use_cnt = 2;
> + t = read_c0_watchhi1();
> + write_c0_watchhi1(t | 0xff8);
> + t = read_c0_watchhi1();
> + c->watch_reg_masks[1] |= (t & 0xff8);
> + if ((t & 0x80000000) == 0)
> + return;
> +
> + write_c0_watchlo2(7);
> + t = read_c0_watchlo2();
> + write_c0_watchlo2(0);
> + c->watch_reg_masks[2] = t & 7;
> +
> + c->watch_reg_count = 3;
> + c->watch_reg_use_cnt = 3;
> + t = read_c0_watchhi2();
> + write_c0_watchhi2(t | 0xff8);
> + t = read_c0_watchhi2();
> + c->watch_reg_masks[2] |= (t & 0xff8);
> + if ((t & 0x80000000) == 0)
> + return;
> +
> + write_c0_watchlo3(7);
> + t = read_c0_watchlo3();
> + write_c0_watchlo3(0);
> + c->watch_reg_masks[3] = t & 7;
> +
> + c->watch_reg_count = 4;
> + c->watch_reg_use_cnt = 4;
> + t = read_c0_watchhi3();
> + write_c0_watchhi3(t | 0xff8);
> + t = read_c0_watchhi3();
> + c->watch_reg_masks[3] |= (t & 0xff8);
> + if ((t & 0x80000000) == 0)
> + return;

Same here

> + /* We use at most 4, but probe and report up to 8. */
> + c->watch_reg_count = 5;
> + t = read_c0_watchhi4();
> + if ((t & 0x80000000) == 0)
> + return;
> +
> + c->watch_reg_count = 6;
> + t = read_c0_watchhi5();
> + if ((t & 0x80000000) == 0)
> + return;
> +
> + c->watch_reg_count = 7;
> + t = read_c0_watchhi6();
> + if ((t & 0x80000000) == 0)
> + return;
> +
> + c->watch_reg_count = 8;

and here

BTW, no check for read_c0_watchhi7()?

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/