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

NatFeats USB: Transfer size of bulk transfers not returned to Atari side #95

Open
czietz opened this issue Aug 8, 2022 · 3 comments
Open

Comments

@czietz
Copy link

czietz commented Aug 8, 2022

The NatFeats call USBHOST_SUBMIT_BULK_MSG returns the return value of libusb_bulk_transfer() but not the number of bytes transferred:

r = libusb_bulk_transfer(devh[dev_idx], endpoint, tempbuff, len, &transferred, timeout < 1000 ? 1000 : timeout);
D(bug("USBHost: return: %d len: %d transferred: %d", r, len, transferred));
return r;
}

However, the FreeMiNT driver uses this return value to populate dev->act_len, the actual transfer length:
https://github.com/freemint/freemint/blob/f42d8b22ebc68a60377eebfc7d7f60ccf77c823c/sys/usb/src.km/ucd/aranym/aranym-hcd.c#L225-L244

As the libusb documentation explains, the return value can be (among others)

0 on success (and populates transferred)
LIBUSB_ERROR_TIMEOUT if the transfer timed out (and populates transferred)

... which means that dev->act_len is filled incorrectly.

Imho, the NatFeats call should return transferred in these two cases and and a (negative) error in all other cases to the FreeMiNT driver.

@DavidGZ
Copy link
Collaborator

DavidGZ commented Aug 8, 2022

@czietz you're right, this must be fixed, probably the confusion came from that libusb_control_transfer() function actually returns the number of bytes transferred on success.

Thanks for spotting this.

@th-otto
Copy link
Member

th-otto commented Aug 9, 2022

Hm, but when libusb always populates *transferred, then returning a single value from the call might not be good enough.

@DavidGZ
Copy link
Collaborator

DavidGZ commented Dec 27, 2023

This has been fixed with 88f4713

Thanks @czietz for pointing to this bug.

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

3 participants