search

cpldcpu / u-wire

Smallest possible USB compliant device with V-USB
67 stars 14 forks source link

possible bugs in asm code #3

Closed nerdralph closed 8 months ago

nerdralph commented 3 years ago

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: https://github.com/ArminJo/micronucleus-firmware/issues/11

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

nerdralph commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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.