search

Rahix / avr-device

Register access crate for AVR microcontrollers
Apache License 2.0
168 stars 67 forks source link

Add support for attiny828 #126

Closed SnakeOilSalesman closed 1 year ago

SnakeOilSalesman commented 1 year ago

Adding support for ATtiny828.

Few notes on this:

  1. Original atdf file had no GPIO signals and pinout sections. As I understand, pins can be constructed in avr-hal even without those sections, but my gut feeling tells me it's better to have them.
  2. Previous versions of atdf file had register named EEAR, which actually, according to the datasheet, is EEARL. Latest atdf have both - EEAR and EEARL, but if we keep both of them, avr-hal is not able to construct EEPROM. So I decdide to remove EEAR variant.
SnakeOilSalesman commented 1 year ago

@Rahix, hey! May I ask you kindly to take a look into this PR?

SnakeOilSalesman commented 1 year ago

@Rahix, is there anything wrong with this PR? It's been more than 2 month since I opened it and you usually react in 2 days. I also prepared changes for avr-hal project, but they blocked by this PR.

Rahix commented 1 year ago

It's been more than 2 month since I opened it and you usually react in 2 days.

Hey! So sorry for leaving you waiting for so long :( I'm having trouble finding time for this project so things are moving quite a bit more slowly than they should. In any case, thanks for persisting and nagging me about it, this is the correct thing to do :+1:

Anyway, from your PR description and commit messages I understand that the ATDF in this PR was modified from the version that Microchip provides? We usually take a different route for dealing with missing/wrong information in these files: The ATDF should be the original version exactly and any modifications are done later on using the patching framework. This has a number of advantages:

So please revert to the original ATDF source file. The EEAR/EEARL modification can be done via the patch yaml - if you need help with this, let me know.

Your other modification, the signals & pinout sections, aren't necessary, I'd say.

SnakeOilSalesman commented 1 year ago

Hey!

Thanks for reaching out! And sorry for pushing.

I read your comments, thanks for pointing it all out. I'll take another round and will try to do all that stuff with original ATDF file. Will come back soon.

SnakeOilSalesman commented 1 year ago

@Rahix, hey!

Pardon me for a long delay with the update on this one. I revert ATDF file to the original condition and remove EEAR register via patch file.

Please, let me know, if I missed something.