Wait for 50ms after reset (fixes #7) by drfiemost · Pull Request #8 · libsidplayfp/exsid-driver

drfiemost · 2026-02-01T10:18:35Z

to avoid hangs.
Fix suggested by @yxkalle based on jsidplay2

@yxkalle

to avoid hangs. Fix suggested by @yxkalle based on jsidplay2

f00b4r0 · 2026-02-01T18:35:47Z

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.

f00b4r0 · 2026-02-11T10:40:59Z

-	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);


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.

drfiemost · 2026-02-18T19:24:49Z

Weird, I can't reproduce anymore... I'm positive I did see the hang both when it was reported and before submitting this PR.
My only suspect now is a kernel regression recently fixed.

f00b4r0 · 2026-02-18T21:19:29Z

No worries, that was an easy fix then :)
I couldn't make sense why this old code would suddenly break either.

drfiemost · 2026-02-01T10:18:35Z

to avoid hangs.
Fix suggested by @yxkalle based on jsidplay2

@yxkalle

to avoid hangs. Fix suggested by @yxkalle based on jsidplay2

f00b4r0 · 2026-02-01T18:35:47Z

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.

f00b4r0 · 2026-02-11T10:40:59Z

-	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);


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.

drfiemost · 2026-02-18T19:24:49Z

Weird, I can't reproduce anymore... I'm positive I did see the hang both when it was reported and before submitting this PR.
My only suspect now is a kernel regression recently fixed.

f00b4r0 · 2026-02-18T21:19:29Z

No worries, that was an easy fix then :)
I couldn't make sense why this old code would suddenly break either.

drfiemost · 2026-02-01T10:18:35Z

to avoid hangs.
Fix suggested by @yxkalle based on jsidplay2

@yxkalle

to avoid hangs. Fix suggested by @yxkalle based on jsidplay2

f00b4r0 · 2026-02-01T18:35:47Z

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.

f00b4r0 · 2026-02-11T10:40:59Z

-	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);


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.

drfiemost · 2026-02-18T19:24:49Z

Weird, I can't reproduce anymore... I'm positive I did see the hang both when it was reported and before submitting this PR.
My only suspect now is a kernel regression recently fixed.

f00b4r0 · 2026-02-18T21:19:29Z

No worries, that was an easy fix then :)
I couldn't make sense why this old code would suddenly break either.

drfiemost · 2026-02-01T10:18:35Z

to avoid hangs.
Fix suggested by @yxkalle based on jsidplay2

@yxkalle

to avoid hangs. Fix suggested by @yxkalle based on jsidplay2

f00b4r0 · 2026-02-01T18:35:47Z

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.

f00b4r0 · 2026-02-11T10:40:59Z

-	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);


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.

drfiemost · 2026-02-18T19:24:49Z

Weird, I can't reproduce anymore... I'm positive I did see the hang both when it was reported and before submitting this PR.
My only suspect now is a kernel regression recently fixed.

f00b4r0 · 2026-02-18T21:19:29Z

No worries, that was an easy fix then :)
I couldn't make sense why this old code would suddenly break either.

drfiemost · 2026-02-01T10:18:35Z

to avoid hangs.
Fix suggested by @yxkalle based on jsidplay2

@yxkalle

to avoid hangs. Fix suggested by @yxkalle based on jsidplay2

f00b4r0 · 2026-02-01T18:35:47Z

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.

f00b4r0 · 2026-02-11T10:40:59Z

-	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);


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.

drfiemost · 2026-02-18T19:24:49Z

Weird, I can't reproduce anymore... I'm positive I did see the hang both when it was reported and before submitting this PR.
My only suspect now is a kernel regression recently fixed.

f00b4r0 · 2026-02-18T21:19:29Z

No worries, that was an easy fix then :)
I couldn't make sense why this old code would suddenly break either.

drfiemost · 2026-02-01T10:18:35Z

to avoid hangs.
Fix suggested by @yxkalle based on jsidplay2

@yxkalle

to avoid hangs. Fix suggested by @yxkalle based on jsidplay2

f00b4r0 · 2026-02-01T18:35:47Z

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.

f00b4r0 · 2026-02-11T10:40:59Z

-	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);


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.

drfiemost · 2026-02-18T19:24:49Z

Weird, I can't reproduce anymore... I'm positive I did see the hang both when it was reported and before submitting this PR.
My only suspect now is a kernel regression recently fixed.

f00b4r0 · 2026-02-18T21:19:29Z

No worries, that was an easy fix then :)
I couldn't make sense why this old code would suddenly break either.

drfiemost · 2026-02-01T10:18:35Z

to avoid hangs.
Fix suggested by @yxkalle based on jsidplay2

@yxkalle

to avoid hangs. Fix suggested by @yxkalle based on jsidplay2

f00b4r0 · 2026-02-01T18:35:47Z

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.

f00b4r0 · 2026-02-11T10:40:59Z

-	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);


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.

drfiemost · 2026-02-18T19:24:49Z

Weird, I can't reproduce anymore... I'm positive I did see the hang both when it was reported and before submitting this PR.
My only suspect now is a kernel regression recently fixed.

f00b4r0 · 2026-02-18T21:19:29Z

No worries, that was an easy fix then :)
I couldn't make sense why this old code would suddenly break either.

drfiemost · 2026-02-01T10:18:35Z

to avoid hangs.
Fix suggested by @yxkalle based on jsidplay2

@yxkalle

to avoid hangs. Fix suggested by @yxkalle based on jsidplay2

f00b4r0 · 2026-02-01T18:35:47Z

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.

f00b4r0 · 2026-02-11T10:40:59Z

-	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);


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.

drfiemost · 2026-02-18T19:24:49Z

Weird, I can't reproduce anymore... I'm positive I did see the hang both when it was reported and before submitting this PR.
My only suspect now is a kernel regression recently fixed.

f00b4r0 · 2026-02-18T21:19:29Z

No worries, that was an easy fix then :)
I couldn't make sense why this old code would suddenly break either.

drfiemost requested a review from f00b4r0 February 1, 2026 10:18

drfiemost force-pushed the reset_hang_7 branch from 481e6da to 00aef38Compare February 1, 2026 12:17

Wait for 50ms after reset (fixes#7)
dece1e5
to avoid hangs. Fix suggested by @yxkalle based on jsidplay2

drfiemost force-pushed the reset_hang_7 branch from 00aef38 to dece1e5Compare February 1, 2026 12:18

f00b4r0 reviewed Feb 11, 2026
View reviewed changes

drfiemost closed this Feb 18, 2026

drfiemost requested a review from f00b4r0 February 1, 2026 10:18

drfiemost force-pushed the reset_hang_7 branch from 481e6da to 00aef38Compare February 1, 2026 12:17

Wait for 50ms after reset (fixes#7)
dece1e5
to avoid hangs. Fix suggested by @yxkalle based on jsidplay2

drfiemost force-pushed the reset_hang_7 branch from 00aef38 to dece1e5Compare February 1, 2026 12:18

f00b4r0 reviewed Feb 11, 2026
View reviewed changes

drfiemost closed this Feb 18, 2026

drfiemost requested a review from f00b4r0 February 1, 2026 10:18

drfiemost force-pushed the reset_hang_7 branch from 481e6da to 00aef38Compare February 1, 2026 12:17

Wait for 50ms after reset (fixes#7)
dece1e5
to avoid hangs. Fix suggested by @yxkalle based on jsidplay2

drfiemost force-pushed the reset_hang_7 branch from 00aef38 to dece1e5Compare February 1, 2026 12:18

f00b4r0 reviewed Feb 11, 2026
View reviewed changes

drfiemost closed this Feb 18, 2026

drfiemost requested a review from f00b4r0 February 1, 2026 10:18

drfiemost force-pushed the reset_hang_7 branch from 481e6da to 00aef38Compare February 1, 2026 12:17

Wait for 50ms after reset (fixes#7)
dece1e5
to avoid hangs. Fix suggested by @yxkalle based on jsidplay2

drfiemost force-pushed the reset_hang_7 branch from 00aef38 to dece1e5Compare February 1, 2026 12:18

f00b4r0 reviewed Feb 11, 2026
View reviewed changes

drfiemost closed this Feb 18, 2026

drfiemost requested a review from f00b4r0 February 1, 2026 10:18

drfiemost force-pushed the reset_hang_7 branch from 481e6da to 00aef38Compare February 1, 2026 12:17

Wait for 50ms after reset (fixes#7)
dece1e5
to avoid hangs. Fix suggested by @yxkalle based on jsidplay2

drfiemost force-pushed the reset_hang_7 branch from 00aef38 to dece1e5Compare February 1, 2026 12:18

f00b4r0 reviewed Feb 11, 2026
View reviewed changes

drfiemost closed this Feb 18, 2026

drfiemost requested a review from f00b4r0 February 1, 2026 10:18

drfiemost force-pushed the reset_hang_7 branch from 481e6da to 00aef38Compare February 1, 2026 12:17

Wait for 50ms after reset (fixes#7)
dece1e5
to avoid hangs. Fix suggested by @yxkalle based on jsidplay2

drfiemost force-pushed the reset_hang_7 branch from 00aef38 to dece1e5Compare February 1, 2026 12:18

f00b4r0 reviewed Feb 11, 2026
View reviewed changes

drfiemost closed this Feb 18, 2026

drfiemost requested a review from f00b4r0 February 1, 2026 10:18

drfiemost force-pushed the reset_hang_7 branch from 481e6da to 00aef38Compare February 1, 2026 12:17

Wait for 50ms after reset (fixes#7)
dece1e5
to avoid hangs. Fix suggested by @yxkalle based on jsidplay2

drfiemost force-pushed the reset_hang_7 branch from 00aef38 to dece1e5Compare February 1, 2026 12:18

f00b4r0 reviewed Feb 11, 2026
View reviewed changes

drfiemost closed this Feb 18, 2026

drfiemost requested a review from f00b4r0 February 1, 2026 10:18

drfiemost force-pushed the reset_hang_7 branch from 481e6da to 00aef38Compare February 1, 2026 12:17

Wait for 50ms after reset (fixes#7)
dece1e5
to avoid hangs. Fix suggested by @yxkalle based on jsidplay2

drfiemost force-pushed the reset_hang_7 branch from 00aef38 to dece1e5Compare February 1, 2026 12:18

f00b4r0 reviewed Feb 11, 2026
View reviewed changes

drfiemost closed this Feb 18, 2026

Uh oh!

Conversation

drfiemost commented Feb 1, 2026

Uh oh!

f00b4r0 commented Feb 1, 2026

Uh oh!

f00b4r0Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

drfiemost commented Feb 18, 2026

Uh oh!

f00b4r0 commented Feb 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

drfiemost commented Feb 1, 2026

Uh oh!

f00b4r0 commented Feb 1, 2026

Uh oh!

f00b4r0Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

drfiemost commented Feb 18, 2026

Uh oh!

f00b4r0 commented Feb 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

drfiemost commented Feb 1, 2026

Uh oh!

f00b4r0 commented Feb 1, 2026

Uh oh!

f00b4r0Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

drfiemost commented Feb 18, 2026

Uh oh!

f00b4r0 commented Feb 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

drfiemost commented Feb 1, 2026

Uh oh!

f00b4r0 commented Feb 1, 2026

Uh oh!

f00b4r0Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

drfiemost commented Feb 18, 2026

Uh oh!

f00b4r0 commented Feb 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects