ch32v: Add I2C hal #743
ch32v: Add I2C hal #743
Conversation
mattnite
left a comment
mattnite
left a comment
There was a problem hiding this comment.
Besides weighing in, looks good to me, pull the trigger when you're ready.
Damn that multi-second delay when typing comments.
|
Let me fix this up. Don't merge yet please |
|
I bricked my board (doing something else). I have to wait for a new one to arrive to properly test everything |
Grazfather
force-pushed
the
wch_hals
branch
3 times, most recently
from
9a987ca to
6374220
Compare
Grazfather
changed the title
Grazfather
force-pushed
the
wch_hals
branch
3 times, most recently
from
b3b7bd3 to
ed4bb8a
Compare
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
|
||
| pub fn main() !void { | ||
| // Board brings up clocks and time | ||
| microzig.board.init(); |
There was a problem hiding this comment.
Maybe we should call this automatically in microzig_main as we do for hal.init?
There was a problem hiding this comment.
The nordic examples also follow this pattern of calling it explicitly. Would you propose that we add a extra case with the order of app -> board -> hal?, or should hal and board both have a chance to get called.
We do board-specific here because some boards would have different clock config based on whether they have an external clock, but I think that at the moment all of the existing boards are simply configured to use the internal oscillator.
There was a problem hiding this comment.
app -> board -> hal
exactly.
There was a problem hiding this comment.
OK. I can do that, but since it will affect other boards and I'd have to remove the explicit invocation in a bunch of examples I'll do it in another PR.
github-actions
bot
dismissed
their stale review
Updating with new lint results
A lot of this functionality was informed by https://github.com/robinjanssens/WCH-Toolchain