remove HAL_Delay(1) for liquid-mod#117
Conversation
There was a problem hiding this comment.
Pull request overview
This PR removes a HAL_Delay(1) call from the flash-write busy-wait loop (presumably to avoid delays for the “liquid-mod” variant).
Changes:
- Removed
HAL_Delay(1)inside theflash_write()loop that waits for the flash chip to become ready.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| else | ||
| { | ||
| timeout_ctr++; | ||
| HAL_Delay( 1 ); | ||
| } |
There was a problem hiding this comment.
With HAL_Delay(1) removed, timeout_ctr++ no longer represents “1 ms elapsed”, so the timeout = num_bytes (documented as “1 ms/byte”) will be reached far too quickly and can cause premature FLASH_TIMEOUT. Consider basing the timeout on elapsed time (e.g., HAL_GetTick()/a deadline) so the loop remains delay-free without changing timeout semantics.
There was a problem hiding this comment.
@copilot apply changes based on this feedback
There was a problem hiding this comment.
Applied in commit 6c48016. Replaced timeout_ctr with a HAL_GetTick()-based elapsed-time check. The deadline is now computed as timeout_start = HAL_GetTick() at initialization, and the busy-wait checks (HAL_GetTick() - timeout_start) >= timeout — which preserves the original "1 ms/byte" semantics and is safe against uint32_t tick wraparound.
| @@ -820,7 +820,6 @@ for ( int i = 2; i < pflash_handle -> num_bytes; i += 2 ) | |||
| else | |||
| { | |||
| timeout_ctr++; | |||
There was a problem hiding this comment.
This while (flash_is_flash_busy()) loop is now a tight spin with no delay/yield, which can significantly increase CPU usage and potentially starve other time-sensitive work (e.g., USB handling) while waiting for the flash chip. If the delay must be removed, consider adding a lightweight yield/backoff mechanism appropriate for the runtime (RTOS delay/yield, or a bounded poll interval) while still meeting performance requirements.
| timeout_ctr++; | |
| timeout_ctr++; | |
| /* Add a small bounded backoff to avoid a tight busy-spin. */ | |
| for ( volatile uint32_t backoff_ctr = 0U; backoff_ctr < 64U; backoff_ctr++ ) | |
| { | |
| } |
…ation counter Agent-Logs-Url: https://github.com/SunDevilRocketry/mod/sessions/a9defb2b-8e1b-4a3a-b693-046530f214e4 Co-authored-by: niekky <80115619+niekky@users.noreply.github.com>
|
Recommend not merging this. Flash will no longer be in mod when v2.6.0 is released. You can make a dedicated liquids branch if you want it, otherwise please cherry pick this diff to driver. |
Description
A brief description of the changes in the PR
Issue Link
Please provide a link to the issue (e.g. "Closes #1").
Also, if this PR is one of multiple for this issue, link the parent if this is a child OR link
the children if this is the parent. Do not use "closes" keywords on child PRs, only use them on
the parent.
Testing
Attach any test artifacts here, if relevant.
Other
Leave any additional notes here
Reviewer Checklist
Standards
Error Handling
Memory
Performance