ch32v: Add get_freqs, USART hal, I2C hal #743
Conversation
| switch (sws) { | ||
| 0 => { | ||
| // HSI used as system clock | ||
| sysclk = HSI_VALUE; |
There was a problem hiding this comment.
should use cpu.cpu_frequency?
There was a problem hiding this comment.
Let's hold off on standardizing globals like that. Maybe have a scoped global for the clock HAL. So something like hal.clocks.get_sysclk_hz()
| }, | ||
| 1 => { | ||
| // HSE used as system clock | ||
| sysclk = HSE_VALUE; |
There was a problem hiding this comment.
should use board.cpu_frequency, but only if some flag indicates that there is an external clock?
| const regs = i2c.get_regs(); | ||
| const star1 = regs.STAR1.read(); | ||
|
|
||
| if (star1.AF == 1) { |
There was a problem hiding this comment.
Could more than one error be set? Do we want to clear all of them?
There was a problem hiding this comment.
It's possible if they're sticky. I would provide an option to the caller, assume you want the below behaviour by default -- this one destroys less information, but they get the option of clearing all of them when one error is fetched.
| microzig.board.init(); | ||
|
|
||
| // Enable peripheral clocks for USART2 and GPIOA | ||
| RCC.APB2PCENR.modify(.{ |
There was a problem hiding this comment.
should usart apply take the pins and do this stuff as well?
There was a problem hiding this comment.
Stuff like the clock of another peripheral shouldn't be the responsibility of the USART HAL.
There was a problem hiding this comment.
I have to test, but last few commits try to tuck it away in the hal, and the hal makes calls to the clock hal
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.
| microzig.board.init(); | ||
|
|
||
| // Enable peripheral clocks for USART2 and GPIOA | ||
| RCC.APB2PCENR.modify(.{ |
There was a problem hiding this comment.
Stuff like the clock of another peripheral shouldn't be the responsibility of the USART HAL.
| switch (sws) { | ||
| 0 => { | ||
| // HSI used as system clock | ||
| sysclk = HSI_VALUE; |
There was a problem hiding this comment.
Let's hold off on standardizing globals like that. Maybe have a scoped global for the clock HAL. So something like hal.clocks.get_sysclk_hz()
| const regs = i2c.get_regs(); | ||
| const star1 = regs.STAR1.read(); | ||
|
|
||
| if (star1.AF == 1) { |
There was a problem hiding this comment.
It's possible if they're sticky. I would provide an option to the caller, assume you want the below behaviour by default -- this one destroys less information, but they get the option of clearing all of them when one error is fetched.
|
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 |
A lot of this functionality was informed by https://github.com/robinjanssens/WCH-Toolchain