xairy / raw-gadget

USB Raw Gadget — a low-level interface for the Linux USB Gadget subsystem
333 stars 35 forks source link

RPi 4 test result #18

Closed AristoChen closed 2 years ago

AristoChen commented 2 years ago

Hi

I tested raw-gadget with RPi 4, and when the test case 13(bulk, ep halt set/clear) was executed, there will be an error, sometimes it is ioctl(USB_RAW_IOCTL_EP_WRITE): Operation now in progress, and sometimes it is ioctl(USB_RAW_IOCTL_EP_READ): Operation now in progress

seems that there is something interrupting the down_interruptible() function in line 94 of raw-gadget.c, which cause it return -EINTR

then I edited the gadget.c like this, simply just retry if error occured.

diff --git a/tests/gadget.c b/tests/gadget.c
index 449cbba..779eb50 100644
--- a/tests/gadget.c
+++ b/tests/gadget.c
@@ -170,18 +170,20 @@ int usb_raw_ep_enable(int fd, struct usb_endpoint_descriptor *desc) {

 int usb_raw_ep_read(int fd, struct usb_raw_ep_io *io) {
        int rv = ioctl(fd, USB_RAW_IOCTL_EP_READ, io);
-       if (rv < 0) {
+       while (rv < 0) {
                perror("ioctl(USB_RAW_IOCTL_EP_READ)");
-               exit(EXIT_FAILURE);
+               rv = ioctl(fd, USB_RAW_IOCTL_EP_READ, io);
+               //exit(EXIT_FAILURE);
        }
        return rv;
 }

 int usb_raw_ep_write(int fd, struct usb_raw_ep_io *io) {
        int rv = ioctl(fd, USB_RAW_IOCTL_EP_WRITE, io);
-       if (rv < 0) {
+       while (rv < 0) {
                perror("ioctl(USB_RAW_IOCTL_EP_WRITE)");
-               exit(EXIT_FAILURE);
+               rv = ioctl(fd, USB_RAW_IOCTL_EP_WRITE, io);
+               //exit(EXIT_FAILURE);
        }
        return rv;
 }

RPi 4 is able to pass the run_test.py after this modification

The question is: should I just add the test result for RPi 4 that tested with the above modification? or should I also modify the ioctl mechanism in gadget.c like the above?

xairy commented 2 years ago

Could you try this:

int usb_raw_ep_read(int fd, struct usb_raw_ep_io *io) {
    int rv = ioctl(fd, USB_RAW_IOCTL_EP_READ, io);
        if (rv == -EINPROGRESS) {
                // Ignore failures caused by the test that halts endpoints.
                return rv;
        }
    if (rv < 0) {
        perror("ioctl(USB_RAW_IOCTL_EP_READ)");
        exit(EXIT_FAILURE);
    }
    return rv;
}

And the same change for usb_raw_ep_write().

AristoChen commented 2 years ago

Hi,

Seems that rv will always be -1 if error occured, so I use this one and it works

if (errno == EINPROGRESS) {
    // Ignore failures caused by the test that halts endpoints.
    return rv;
}

If this looks good to you, then I will send a PR for RPi 4 testing result with this code change

xairy commented 2 years ago

Ah, errno. Yes, this looks good if placed inside the if (rv < 0) clause. Thanks!

xairy commented 2 years ago

FTR, if you end up testing even more different hardware, there's a patch from Jahn Horn, which might be required.

AristoChen commented 2 years ago

Ah, errno. Yes, this looks good if placed inside the if (rv < 0) clause. Thanks!

I just send a PR #19 , let me know if I did anything wrong

FTR, if you end up testing even more different hardware, there's a patch from Jahn Horn, which might be required.

Ok, noted