Re: [Simulavr-devel] Timers Diff Patch

[Theodore A. Roth]

On Tue, 28 Oct 2003, Keith Gudger wrote:

> Ted:
> 
> Attached is the diff for the files that add Timer1.  I did the diff while 
> logged into CVS.  I'm sorry to report that I can't find the change log 
> entries to edit, so I did not do that.
> 
> Here is a synopsis of what I did.  Starting with Hermann's patch, I added 
> only what I needed to get Timer 1.  I tried to use as much of his patch as 
> possible, so that it would be extensible to Timer 3, but I did have to make 
> some compromises.  I also implemented the TEMP register, so that Timer 1's 
> behavior matches the data sheet.  I've tested it and the timer, COMPA and 
> COMPB all work as I expect them to.
> 
> Let me know if there are any problems.  I will work next on adding an ADC 
> and adding a USB module so that I can simulate USB stuff.  If I can get the 
> USB module in, this would make Simulavr + avr-gdb the *only* way to 
> simulate these parts.

Keith,

I'm still going over your patch. 

I noticed you (or maybe Hermann?) used uint for the 16 bit timer tccr
register and uint for ocr in struct _OCReg16. I'd rather see the uintN_t
types used instead of uint. The output compare register should be uint16_t.
I'm wondering if it would be better to have tccr be a 3 element array of
uint8_t instead of using uint32_t since that correlates better with the data
sheet and removes the messiness of bit shifts and such.

In any case where we are simulating an AVR register, I think it's best to 
use the uintN_t type that matches the registers size.

A few other nit-picky requests. Please put spaces around operators. For 
example, don't do this:

  timer->tccr = (timer->tccr&0xFF00)|val;

but do this instead:

  timer->tccr = (timer->tccr & 0xFF00) | val;

I find that the second form is much easier to read.

I would also like to start keeping lines under 80 chars in length. I know 
most of the code I've written doesn't do that, but I've changed my ways 
since I wrote that. Once I get caught up on patches, and just before the 
next release, I'm going to perform a global reformat with the indent program 
so as to be mostly gnu standards compliant.

Other than that, your patch is looking good so far.

Ted Roth