Re: [PATCH 2/2] hw/openrisc: Fixed undercounting of TTCR in continuous mode

[Stafford Horne]

On Thu, Dec 19, 2024 at 08:08:14PM +0000, Joel Holdsworth wrote:
> > > > > +        /* 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.
> >
> > I think you're reading too much into it -- I just think it's a bug which 
> > isn't particularly noticeable because the clock is only off by 1ns.
> 
> Richard is correct. It should be:
> 
> or1k_timer->ttcr_offset += -(cpu->env.ttmr & TTMR_TP);
> 
> Stafford: sorry for not being responsive. I've been very busy lately, and 
> it's been several months since I touched anything OpenRISC-related. Are you 
> able to push this the rest of the way through the acceptance process? I had 
> understood that you were looking for a more elaborate overhaul of the 
> OpenRISC timer design which I haven't had time to work on. But if the patch 
> can go forward in its current form, I think the improvement is worth having 
> even it doesn't address other design issues.

Hi Joel,

Yes, I was able to sort out the issues and get everything tested and fixed up.
The patch is now committed upstream in QEMU for the 9.2.0 release.  I also
thought its better to have this upstream in its current form rather than wait
for further improvements.

Thanks for your help.  I look forward to you getting some free cycles and
helping more with OpenRISC when you get any chance.

I was also busy last year until November and didn't have much time to work on
this.

-Stafford