Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove usages of raw register manipulation outside of PAC crates #1194

Closed
MabezDev opened this issue Feb 21, 2024 · 8 comments
Closed

Remove usages of raw register manipulation outside of PAC crates #1194

MabezDev opened this issue Feb 21, 2024 · 8 comments
Assignees
Labels
status:in-progress This task is currently being worked on
Milestone

Comments

@MabezDev
Copy link
Member

MabezDev commented Feb 21, 2024

We currently have duplicates of these reg functions where the svd's don't have enough info, and we have to resort back to raw register writes. E.g: https://github.com/search?q=repo%3Aesp-rs%2Fesp-hal%20reg_set&type=code.

We should cleanup and modularise their usage/definitions across our chip line-up.

cc: @playfulFence who initially noticed the issue.

@bjoernQ
Copy link
Contributor

bjoernQ commented Feb 21, 2024

Maybe we could even avoid those functions and use something like https://crates.io/crates/tock-registers

But I think such code originates from 1:1 translation of esp-idf code - but once such code is known working, we could improve it

@jessebraham
Copy link
Member

Maybe I'm missing something, but why are we going to all this effort instead of just fixing the PACs?

@bjoernQ
Copy link
Contributor

bjoernQ commented Feb 21, 2024

Maybe I'm missing something, but why are we going to all this effort instead of just fixing the PACs?

That would certainly be the best solution for registers! I somehow thought of I2C_BBPLL which we also deal with in those places

@MabezDev MabezDev changed the title Refactor reg_X_bit functions Remove usages of raw register manipulation outside of PAC crates Feb 21, 2024
@MabezDev
Copy link
Member Author

Good idea! Let's do that instead, I've updated the title.

@jessebraham
Copy link
Member

I don't think I will be able to do this any time soon unfortunately, but if somebody else wants to tackle it I can at least explain what needs to be done to them, so just lemme know. If nobody has tackled this in a month or two when I wrap up my currently projects then I will take care of it at that point.

@playfulFence
Copy link
Contributor

Just to have some part of an investigation here:
Most of the cases of manipulations with raw register addresses are related to regi2c_* functions, which has a tracking issue here #1740. Which means that this issue highly depends on it. At this point, I think, I will temporarily switch to analyzing regi2c_* functions.

@playfulFence playfulFence added status:in-progress This task is currently being worked on and removed status:long-term This task will be around a while labels Jul 16, 2024
@playfulFence
Copy link
Contributor

Work has been done on this issue (see in PRs above). At the moment, most operations with raw register addresses are performed in regi2c_* functions. Due to the complexity and black-box nature of this part, it was decided to postpone this work, so I consider this issue closed.
I will keep it open until the end of this day and, if there are no hard opinions, I will close it as successfully completed

@MabezDev
Copy link
Member Author

We can track the regi2c stuff in #1740, so I think we can close this now.

@github-project-automation github-project-automation bot moved this from Todo to Done in esp-rs Aug 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status:in-progress This task is currently being worked on
Projects
Archived in project
Development

No branches or pull requests

5 participants