Add attiny841 and attiny861

[jaxter184]

Preliminary pull request adding attiny841 and attiny861 chips to go with Rahix/avr-hal#105.

[Rahix]

Thanks for your PR! I've got some minor comments, overall the changeset looks good!

[jaxter184]

I still need to figure out the timer stuff, and I probably want to look through the patches a little more closely to make sure that I'm doing it correctly

[jaxter184]

I think I've just about figured out the timer stuff, with one caveat: In patch/timer/dev, there are a 2 different 16-bit timer patches. The primary difference that I could distinguish is that one describes that OCix is set/cleared at TOP rather than BOTTOM. However, the datasheet register descriptions are the same between the devices that use 16bit-tiny84-tc1.yaml (attiny84 table 14-3) and the devices that use 16bit.yaml (for example: atmega2560 table 20-3), namely that both say to set and clear at BOTTOM. Am I reading these wrong? Is there an error? Should I be looking at a different part of the datasheet for this information?

[Rahix]

I think I've just about figured out the timer stuff, with one caveat: In patch/timer/dev, there are a 2 different 16-bit timer patches. The primary difference that I could distinguish is that one describes that OCix is set/cleared at TOP rather than BOTTOM. However, the datasheet register descriptions are the same between the devices that use 16bit-tiny84-tc1.yaml (attiny84 table 14-3) and the devices that use 16bit.yaml (for example: atmega2560 table 20-3), namely that both say to set and clear at BOTTOM. Am I reading these wrong? Is there an error? Should I be looking at a different part of the datasheet for this information?

Looks like this slipped through and you're right, they are set/cleared at BOTTOM. The other difference between those files is the TIFRn register. Maybe that doesn't exist on the ATtiny version?

[Rahix]

Similar change to above should work here as well I think.

[jaxter184]

When I added this change, it broke the AC patch for atmega328pb, which I think is because of an error in the svd file for that chip. According to the svd file, there's a register called ACSRA with no fields (in addition to the ACSR register with the ACIS field), but the ACSRA field doesn't show up at all in the datasheet as far as I can tell. For now, I made the changes to the common AC patch, and replaced the common patch in atmega328pb.yaml with one that works. I'm not sure this is the best solution though.

[jaxter184]

I think I've just about figured out the timer stuff, with one caveat: In patch/timer/dev, there are a 2 different 16-bit timer patches. The primary difference that I could distinguish is that one describes that OCix is set/cleared at TOP rather than BOTTOM. However, the datasheet register descriptions are the same between the devices that use 16bit-tiny84-tc1.yaml (attiny84 table 14-3) and the devices that use 16bit.yaml (for example: atmega2560 table 20-3), namely that both say to set and clear at BOTTOM. Am I reading these wrong? Is there an error? Should I be looking at a different part of the datasheet for this information?

Looks like this slipped through and you're right, they are set/cleared at BOTTOM. The other difference between those files is the TIFRn register. Maybe that doesn't exist on the ATtiny version?

It looks like TIFRn does exist in the ATtiny84. Should I maybe consolidate it into one file? Is it bad practice to put multiple unrelated changes in a single pull request?

[Rahix]

It looks like TIFRn does exist in the ATtiny84. Should I maybe consolidate it into one file?

If possible, that would of course be the best!

Is it bad practice to put multiple unrelated changes in a single pull request?

I guess this is highly project specific. Here's my take: I think it is very important that every "atomic" change goes into a separate commit. I.e. every change that makes "sense" on its own and can stand without the following changes being needed should be separated.

The reason for this is that this means the git history is a clean record of what was done with the project. If I find a bug in a few years, I can just git blame and find out what was changed and ideally also why (if a proper commit message was written). Similarly, if something stops working at some point I can find the exact change that introduced a regression using git bisect.

If multiple changes happen in a single commit or if an "atomic" change is split out over multiple commits, this is of course not going to work effectively.

Now, to archieve this clean separation, there are two ways:

• If contributors are willing to take care of this, they can rebase their changeset to split it into such clean, isolated commits (and document each one with a good commit message). This would be super helpful for me, because I can then just rebase the commit series and all is good.
• If contributors send a PR with more or less random commits or non-descriptive commit messages, I'll have to squash the series and write a proper message explaining all changes. This of course means that potentially multiple changes are merged into a single commit ...

I see that you've already followed the first route which is really nice (thanks a lot!). So I have no problem if you add more changes into this series. A minor nag if you want to make it even more perfect: You could reorder the commits slightly so that each one could possibly compile (right now the ATmega328PB fix is after the changes to the AC common patch). But that's really a minor detail.

---

On a side note: There is currently a merge conflict. Please rebase on master to fix :)

[jaxter184]

I've rebased the commits, and assuming my testing procedure was correct, they are all makeable.

[Rahix]

Looks really good and thanks for taking the time to properly split out the commits!

[Rahix]

Thanks for your work in this PR!