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

possible bugs in asm code #3

Closed
nerdralph opened this issue Aug 24, 2020 · 4 comments
Closed

possible bugs in asm code #3

nerdralph opened this issue Aug 24, 2020 · 4 comments

Comments

@nerdralph
Copy link

Is it just by chance that ZL isn't close to 255 when the function is called? It could cause unexpected errors by syncing to the wrong JK transition.
https://github.com/cpldcpu/u-wire/blob/master/firmware/usbdrv/usbdrvasm12.inc#L59

It looks like Armin copied the same code:
ArminJo/micronucleus-firmware#11

I also noticed that other than for the t10, x1/x2 use r16/r17 which are supposed to be callee-saved.

@nerdralph
Copy link
Author

An easy solution to the r16/r17 usage would be to always use r23 & r24, not just for the t10.
https://github.com/cpldcpu/u-wire/blob/master/firmware/usbdrv/usbdrvasm.S#L23

@cpldcpu
Copy link
Owner

cpldcpu commented Aug 24, 2020

Indeed, that looks like an oversight. I am not sure about the impact though, because even if the first test falls through, the loop can still synchronize with the remainder of SYNC. It's only critical if the invocation of the handler is delayed so that there are only 2 sync cycles left.

Regarding the registers: Yes, certainly.

@nerdralph
Copy link
Author

Good point about having 3 JK transitions to sync to. You've obviously kept your v-usb knowledge fresh despite writing u-wire 6 years ago.

While we are on the topic of the sync, did you notice the rjmp waitForK is odd-cycle aligned?
https://github.com/cpldcpu/u-wire/blob/master/firmware/usbdrv/usbdrvasm12.inc#L87
And with the 16.5Mhz version, where there is an odd number of cycles (11) per bit, the rjmp is even-cycle aligned.
https://github.com/micronucleus/micronucleus/blob/master/firmware/usbdrv/usbdrvasm165.inc#L96

Each waitForK pass is accurate to within 2 cycles, so 2 passes shifted 1 cycle will make the timing accurate within 1 cycle. For the 16Mhz version at 10 2/3 cycles per bit. it should be accurate to within 2/3rds of a cycle, compensating for the 1/3rd cycle jitter in the bit sampling window of the code that follows.

@cpldcpu
Copy link
Owner

cpldcpu commented Aug 24, 2020

Well, I spent enough time debugging PHY-level issues with V-USB :) It's a fickle system.

I have not looked into the receiver loops in detail, but I recall that the 16.5 Mhz one was the most stable one. That's is probably the reason why.

Regarding registers in u-Wire: Keep in mind that the tinyavr core (ATtiny10) only has 16 registers. Therefore AVR-GCC uses a different register mapping than on normal AVR and a lot of registers in V-USB had to be moved around to avoid issues.

@cpldcpu cpldcpu closed this as completed Nov 18, 2023
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

No branches or pull requests

2 participants