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

Question about ack to PC side using read zero length packet #84

Open
seago66 opened this issue Sep 27, 2022 · 3 comments
Open

Question about ack to PC side using read zero length packet #84

seago66 opened this issue Sep 27, 2022 · 3 comments

Comments

@seago66
Copy link

seago66 commented Sep 27, 2022

Hello,

Is this a bug for below code that should use write but not read ?
// Just ACK
status = read (ctx->usb_device, &status, 0);

Thanks a lot!

@marcone
Copy link
Contributor

marcone commented Nov 10, 2024

@jfdelnero I was wondering about this too. In change 1e6936a I removed the read() for the MTP_REQ_CANCEL case because it was causing a hang, however I found that changing that 0-length read to a 0-length write also works. I am unfamiliar with the actual behavior of the USB driver though, so don't know if doing an "ACK" really requires doing a read, or if that was perhaps a copy/paste error and it was supposed to be a write all along.

@jfdelnero
Copy link
Member

0 byte read is generally used to ack. for example :

In http://www.linux-usb.org/gadget/usb.c

Line 1429 (and some others)

		/* ... ack (a write would stall) */
		status = read (fd, &status, 0);
		if (status)
			perror ("ack SET_CONFIGURATION");
		return;

@marcone
Copy link
Contributor

marcone commented Nov 11, 2024

OK, though given that there's no further explanation or documentation of the reason for the stall, I can't help but wonder if it's just because of a driver quirk in a 20 year old USB driver (that example code was written over 20 years ago).

In any case, that read in the cancel case was definitely causing the hang that caused the Windows playback problems. It does seem to be device-specific though (as I mentioned in another comment it worked OK both with and without the read on a Rock Pi 4C+), so if it turns out to be necessary on some platforms then it should be put back with a compile-time or run-time option to skip the call.

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