Conversation
drfiemost
force-pushed
the
reset_hang_7
branch
from
481e6da to
00aef38
Compare
drfiemost
force-pushed
the
reset_hang_7
branch
from
00aef38 to
dece1e5
Compare
|
Thanks! This looks nice and compact but I'm confused as to why a 50ms delay would be necessary: the internal reset shouldn't take more than 50us, so a 1000x increase doesn't compute. I need to review the firmware code again and check the implications on exSID+ s as well. |
| xSoutb(xs, XS_AD_IOCTRS, 100); // this will stall | ||
| // This will stall and sleep for 50ms | ||
| // Reset needs some time to complete | ||
| xSoutb(xs, XS_AD_IOCTRS, 50000); |
There was a problem hiding this comment.
OK so I'm afraid I really can't make any sense of such a large delay. The internal delay routine executes in exactly 22 SID clock cycles, or 22µs. Nothing should require a 50ms delay.
; Reset IOCTL
; Trigger hardware SID reset
; Execution time: 1 SIDCLKs (+reset time)
; This will result in 1+20+1 SIDCLKs minimum total time (reset loop + rsync)
I can't reproduce this locally which doesn't help for debugging. Can you check if a more reasonable value works for you? Say start with a 10x increase on the current value, i.e. 1ms? That should be plenty enough already. If the delay length really helps then I wonder if we're dealing with a weird nanosleep() implementation that doesn't handle low values nicely?
I also do not understand how that fixes #7 since you mention there that the bug also occurs during init(), and reset() isn't called in init()? The provided backtrace doesn't involve reset() either. Can you elaborate? Thanks.
|
Weird, I can't reproduce anymore... I'm positive I did see the hang both when it was reported and before submitting this PR. |
|
No worries, that was an easy fix then :) |
2 participants
to avoid hangs.
Fix suggested by @yxkalle based on jsidplay2