On Sat, Nov 23, 2024 at 07:39:57AM -0600, Richard Henderson wrote: > On 11/23/24 04:38, Stafford Horne wrote: > > + or1k_timer->ttcr = or1k_timer->ttcr_offset + > > + (now - or1k_timer->clk_offset + TIMER_PERIOD - 1) / TIMER_PERIOD; > > Better using DIV_ROUND_UP.
Sure, I can change it to that. > > + /* Zero the count by applying a negative offset to the counter */ > > + or1k_timer->ttcr_offset += UINT32_MAX - (cpu->env.ttmr & TTMR_TP); > > Since UINT32_MAX is -1 in this context, this appears to be off-by-one. > I think -(ttmr & mask) alone is correct. Thanks, I did send a mail to Joel asking about this bit. He didn't respond for 2 weeks to I just sent the patch as is as it appears to work. As I understand, yes UINT32_MAX is just -1. But why the -1? I guess it's because after ttcr_offset is updated we call cpu_openrisc_timer_update() which calls cpu_openrisc_count_update() to update ttcr. Since a few _ns would have passed and we are rounding up it will update ttcr to 0. But maybe I am reading too much into it. If you think that makes sense I could add a comment as such, also I would prefer to change to UINT32_MAX to -1. -Stafford
