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

Fix CDC FIFO used getting out of sync #711

Merged
merged 1 commit into from
Aug 26, 2021

Conversation

Daft-Freak
Copy link
Collaborator

... by deleting it.

I found a .blit that would reliably fail to flash (99% bug). Adding any
extra logging to the USB code caused it to work, as did writing extra
data at the end. Then I noticed that the read index was sometimes behind
the write index by several entries, but "used" was 0.

I think this is what was happening:

  • FIFO write is called from the USB interupt
  • write increments "used", read decrements it
  • those inc/decs are actually "load, add, store"
  • interrupt can happen in the middle of that
  • write increments from interrupt in the middle of the decrement in read
  • "used" is now too low

So, I deleted the used variable and checked the read/write index instead
... and that .blit now works reliably.

This hopefully fixes the "99% bug"... I've tested this with my usual "flash everything" script (86 .blits this time).

... by deleting it.

I found a .blit that would reliably fail to flash (99% bug). Adding any
extra logging to the USB code caused it to work, as did writing extra
data at the end. Then I noticed that the read index was sometimes behind
the write index by several entries, but "used" was 0.

I think this is what was happening:

- FIFO write is called from the USB interupt
- write increments "used", read decrements it
- those inc/decs are actually "load, add, store"
- interrupt can happen in the middle of that
- write increments from interrupt in the middle of the decrement in read
- "used" is now too low

So, I deleted the used variable and checked the read/write index instead
... and that .blit now works reliably.
@Gadgetoid
Copy link
Contributor

I think it might work!

image

@Gadgetoid Gadgetoid merged commit c2c1207 into 32blit:master Aug 26, 2021
@Gadgetoid
Copy link
Contributor

Thank you. RIP 99% hangs, nobody will miss you 😆

@Daft-Freak Daft-Freak deleted the fix-flashing branch August 26, 2021 14:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants