Skip to content

Conversation

@amaanq
Copy link
Member

@amaanq amaanq commented Apr 10, 2025

This PR updates the AVR code in the staging branch - it's effectively my way of saying "I've reviewed the code", but there were a lot of changes to be made, so I took it upon myself to make those changes, since #2021 is quite old and I didn't want to burden the author with dozens of comments :^).

The main highlights are:

  • Cleaned up main AVR header, there's no need for special constants on the enums, as well as having the CPU enum be a combo of board x cpu.
  • Removing the "Object" class stuff that upstream QEMU has
  • Removing unnecessary code that isn't used
  • Removed "unicorn helper" for macros - if that were to be added, it'd make more sense to do it in a PR that updates every CPU's code
  • Cleaned up the test
@wtdcode
Copy link
Member

wtdcode commented Apr 11, 2025

the history is missing and seems hard to review?

@wtdcode
Copy link
Member

wtdcode commented Apr 11, 2025

Also the author is quite active, let me ping him @glennsec

Sorry for the long stalling and we are working go get it into 2.2.0 that would be released in next 2-3 months. Any ideas or comments for this?

@amaanq
Copy link
Member Author

amaanq commented Apr 11, 2025

the history is missing and seems hard to review?

I based it off the staging branch - I could have it target #2021, so it's directly stacked off of that instead if that makes it easier to review. If splitting up the commits would help with reviewing too I can do that as well 🙂

@wtdcode
Copy link
Member

wtdcode commented Apr 11, 2025

the history is missing and seems hard to review?

I based it off the staging branch - I could have it target #2021, so it's directly stacked off of that instead if that makes it easier to review. If splitting up the commits would help with reviewing too I can do that as well 🙂

It's okay to not splitting for me. Just asking in case the author would like a cleaner history. Anyway nice work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants