Geert Uytterhoeven wrote: > 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); [...] > 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. I thought that too when I first started looking at it, but the {read,write}_c0_watchhi{0,1,2,3,4,5,6,7} macros expand to a single machine instruction. The bit pattern of the instruction is determined at compile time, so you would need something like the switch statement somewhere. Explicitly showing it in the code seemed as good as hiding the complexity in some macro or access function. [...] >> + 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()? > The current patch uses a maximum of four register sets, since we are only reporting the number of sets, we don't care about the characteristics of watchhi[7] and thus don't need to read it. Thanks, David Daney