Re: [Patch V3 2/2] usbtest: Add interrupt EP testcases

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 5/23/2014 8:14 PM, Daniele Forsi wrote:
2014-05-23 8:32 GMT+02:00 Amit Virdi:

@@ -124,6 +130,9 @@ get_endpoints(struct usbtest_dev *dev, struct usb_interface *intf)
                         switch (usb_endpoint_type(&e->desc)) {
                         case USB_ENDPOINT_XFER_BULK:
                                 break;
+                       case USB_ENDPOINT_XFER_INT:
+                               if (dev->info->intr)
+                                       goto try_intr;
                         case USB_ENDPOINT_XFER_ISOC:
                                 if (dev->info->iso)
                                         goto try_iso;
@@ -139,6 +148,15 @@ get_endpoints(struct usbtest_dev *dev, struct usb_interface *intf)

I don't think you really mean to fall through to case
USB_ENDPOINT_XFER_ISOC if the test is false, but the logic of that

I do mean to fall through to check whether the EP is ISOC type if it isn't INTERRUPT.

for-loop is becoming harder to follow

in pseudo code, the switch statement is like this?
case USB_ENDPOINT_XFER_BULK:
   set in or out;
   break;
case USB_ENDPOINT_XFER_INT:
   set int_in or int_out;
   break;
case USB_ENDPOINT_XFER_ISOC:
   set iso_in or iso_out;
   break;
default:
  do nothing;

it would be easier to follow even if it adds and indentation level


I do agree that the goto statements don't look visually pleasing here, but the approach you suggested would end up in upto 7 levels of indentation and that would make the code look even more ugly (see below sample code snippet)

---
for (tmp = 0; tmp < intf->num_altsetting; tmp++) {
			unsigned ep;
.
.
.

	for (ep = 0; ep < alt->desc.bNumEndpoints; ep++) {
		struct usb_host_endpoint	*e;
		e = alt->endpoint + ep;
		switch (usb_endpoint_type(&e->desc)) {
.
.
.

                  case USB_ENDPOINT_XFER_INT:
                         if (dev->info->intr) {
                                 if usb_endpoint_dir_in(&e->desc) {
                                         if (!int_in)
                                                 int_in = e;
                                 }
                                 else {
                                         if (!int_out)
                                                 int_out = e;
                                 }
                         }
                         break;
.
.
.
.

---
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux