Re: [Simulavr-devel] Timer(s)

[Theodore A. Roth]

On Mon, 20 Oct 2003, Keith Gudger wrote:

>
>
> On Fri, 17 Oct 2003, Theodore A. Roth wrote:
>
> >
> > Keith,
> >
> > Attached is your ports and usb additions reduced to diffs. I really would
> > rather see diffs than entire files. I just spent an hour and a half
> > generating these diffs then spent 15 minutes reviewing them. I'd really
> > the diffs without cvs. Let know if you need help with that.
>
> Yes, I need help with how to get the nice diff files you use.

Well, I let cvs do most of the grunt work. Basically, I generate a
diff like this:

  1. Bring the source up to date with 'cvs update'.
  2. Edit the source, compile, test, etc.
  3. Do another 'cvs update' to merge in any one's changes that where
     checked in while I was working on my changes. May need to fix
     some conflicts.
  4. Generate the patch with 'cvs diff -u > foo.diff'. (Actually I
     have a simple wrapper script called cpatch which filters the cvs
     diff output and automates generating a ChangeLog entry template.)
  5. Manually inspect the diff file with vim. (vim colorizes the diff
     making it easier to read.)
  6. Fill in the ChangeLog entry. This is important so I can look at
     the patch in 3 months and remember what the heck it does.
  7. If the patch looks good, I post/commit/submit it as appropriate.

When I create new files, then I need to do a cvs add of the new files
before I generate the diff or they will be missing from the diff. Of
course that doesn't work if you don't have cvs write access. In that
case, you need to do this this for each new file to add the new files:

  $ diff -u /dev/null newfile >> foo.diff

Be sure to run the diff for the new files at from the same directory
that you issued the cvs diff command from or else the patch won't
apply cleanly.

Without cvs, things are a bit more difficult. The easiest thing to do
is have two or more copies of the source tree. The trees should be
side by side in the same dir and each should have a unique name. Keep
on tree pristine and use one for working with. Once you are ready to
make a patch, you use the diff program as such:

  $ diff -urN src_tree.orig src_tree.new > foo.diff

But now foo.diff will also contain things that are not relevant
(generated files like Makefile.in and Makefile, etc. There's two ways
to deal with this: either use the -x or -X diff options, or manually
edit the diff file.

Well, that should get you started. Let me know if there are gaps that
need filled in.

> BTW, I am doing most of this work by dialup to an Atmel Redhat
> machine.  I can only get through their firewall when I'm physically
> at the SJ site.

Yuck.

>
> >
> > Now, as for your changes...
> >
> > The port E and F additions are not going to work with the mega128 (and
> > similar) devices. This is a design flaw on my part. I need to figure out to
> > get make VDevs handle discontinuous IO addresses. Other than that, you
> > changes look good. I'll try to do some basic regression tests and most
> > likely commit it with a comment saying not to use it with anything but the
> > usb devices.
> >
> > The usb additions look good too, but I think the irq vector tables are
> > wrong. For example:
> >
> > +/* supports 326 */
> > +static IntVectTable vtab_at43usb326 = {
> > +    .RESET =        { "IRQ_RESET",          0x00,    0x00 },
> > +    .INT0  =        { "IRQ_INT0",           0x02,    0x00 },
> > +    .TIMER0_OVF   = { "IRQ_TIMER0_OVF",     0x0e,    0x00 },
> > +    .ANA_COMP     = { "IRQ_USB",            0x18,    0x00 }
> > +};
> >
> > Shouldn't the addresses be sequential? Some thing like this:
> >
> > +/* supports 326 */
> > +static IntVectTable vtab_at43usb326 = {
> > +    .RESET =        { "IRQ_RESET",          0x00,    0x00 },
> > +    .INT0  =        { "IRQ_INT0",           0x02,    0x00 },
> > +    .TIMER0_OVF   = { "IRQ_TIMER0_OVF",     0x04,    0x00 },
> > +    .ANA_COMP     = { "IRQ_USB",            0x06,    0x00 }
> > +};
> >
> > The other tables you added show similar holes.
> >
> > Ted Roth
> >
>
> The tables I added are correct.  There should be holes in the interrupt
> tables - that's the we we made the USB devices.

Ok. I thought that might be the case. There should be comment noting
that then so no one goes and "cleans up" the usb vector tables in the
future.

> That's one of the nicer things about your new interrupt structure -
> I can simply include only the interrupts that the devices have!
> Thanks.

That was the intent of the change: let the compiler NULL out the
unused vector entries and let the programmer focus on getting the
vector right with a minimum of brain drain. ;-)

I'll commit your patches in a bit after adding a note about the holes
in the vector table.

Ted Roth