STM32: Add never_reset reservation to RGBMatrix init #3441
Conversation
| @@ -36,6 +36,7 @@ extern void _PM_IRQ_HANDLER(void); | |||
| void *common_hal_rgbmatrix_timer_allocate() { | |||
| TIM_TypeDef * timer = stm_peripherals_find_timer(); | |||
| stm_peripherals_timer_reserve(timer); | |||
| stm_peripherals_timer_never_reset(timer); | |||
There was a problem hiding this comment.
with "never_reset", does stm_peripherals_timer_free in common_hal_rgbmatrix_timer_free() remain enough to fully deallocate/free/reset the timer?
There was a problem hiding this comment.
I'm sorry, can you rephrase? stm_peripherals_timer_free does undo never reset, is that your question? What do you mean by "remain"?
There was a problem hiding this comment.
I'm still not fully familiar/comfortable with the transitions among these states; the naming is a bit different in each port and for each type of managed resource and I'm never entirely confident the semantics are the same. (As a larger issue, it wouldn't hurt to document this for developers reference and to ensure consistency)
My question is whether the state transtion from "timer is in never_reset state" to "timer is in free state" by calling "timer_free" is a correct transition. I think you answered yes.
There was a problem hiding this comment.
Yes. I think the going concept here is that never_reset is actually a wholly separate option from reserved - it is possible to have an un-reserved pin that is not in never_reset. I suggested joining the two at one point, so that never_reset was strictly a subset of reserved, but I believe we decided against it (I don't fully remember the reason).
Conceptually, reserved is the idea that a pin/peripheral cannot be claimed twice within a single runtime. never_reset is the idea that resetting will not change the reserved status or the pin attributes of that pin/peripheral (basically, immunity status to the reset_all function). To turn off never_reset, you typically call the single-reset function (the not reset-all one) which completely resets all aspects of the pin/peripheral. We have some reset_ok functions which specifically turn off never_reset and nothing else, but they're rarely implemented and even more rarely used.
I'd definitely be down to document this or remove any inconsistencies that you have found. I think we could also revisit whether the two ideas should be combined more concretely in some way - basically, is there ever a scenario where you want something to be in never_reset without it actually being claimed?
When soft-reset on STM32, RGBMatrix currently gets stuck in
_PM_swapbuffer_maybe, as the timer IRQ it relies on to exit an infinite loop is disabled by the reset process. This PR adds timers used by the RGBMatrix constructor to the never_reset array so this does not happen. Tested on STM32F405 Feather Express.Also removes some timer debug messages that were accidentally left in from a previous PR.
Resolves #3440. Mentioned in #3051 but ultimately unrelated to the issue there, which seems to have just been a documentation mixup.