From 4a089668ef22c295ed4997289cc48446c849249c Mon Sep 17 00:00:00 2001 From: Mauro Carvalho Chehab Date: Sat, 3 Mar 2018 13:11:11 -0500 Subject: [PATCH] media: em28xx-cards: rework the em28xx probing code There is a complex loop there with identifies the em28xx endpoints. It has lots of identations inside, and big names, making harder to understand. Simplify it by moving the main logic into a static function. While here, rename "interface" var to "intf". Signed-off-by: Mauro Carvalho Chehab --- drivers/media/usb/em28xx/em28xx-cards.c | 283 +++++++++++++++++--------------- 1 file changed, 149 insertions(+), 134 deletions(-) diff --git a/drivers/media/usb/em28xx/em28xx-cards.c b/drivers/media/usb/em28xx/em28xx-cards.c index e592cd7cd3a4..6e8247849c4f 100644 --- a/drivers/media/usb/em28xx/em28xx-cards.c +++ b/drivers/media/usb/em28xx/em28xx-cards.c @@ -3358,13 +3358,13 @@ EXPORT_SYMBOL_GPL(em28xx_free_device); * allocates and inits the device structs, registers i2c bus and v4l device */ static int em28xx_init_dev(struct em28xx *dev, struct usb_device *udev, - struct usb_interface *interface, + struct usb_interface *intf, int minor) { int retval; const char *chip_name = NULL; - dev->intf = interface; + dev->intf = intf; mutex_init(&dev->ctrl_urb_lock); spin_lock_init(&dev->slock); @@ -3538,11 +3538,115 @@ static int em28xx_duplicate_dev(struct em28xx *dev) /* high bandwidth multiplier, as encoded in highspeed endpoint descriptors */ #define hb_mult(wMaxPacketSize) (1 + (((wMaxPacketSize) >> 11) & 0x03)) +static void em28xx_check_usb_descriptor(struct em28xx *dev, + struct usb_device *udev, + struct usb_interface *intf, + int alt, int ep, + bool *has_vendor_audio, + bool *has_video, + bool *has_dvb) +{ + const struct usb_endpoint_descriptor *e; + int sizedescr, size; + + /* + * NOTE: + * + * Old logic with support for isoc transfers only was: + * 0x82 isoc => analog + * 0x83 isoc => audio + * 0x84 isoc => digital + * + * New logic with support for bulk transfers + * 0x82 isoc => analog + * 0x82 bulk => analog + * 0x83 isoc* => audio + * 0x84 isoc => digital + * 0x84 bulk => analog or digital** + * 0x85 isoc => digital TS2 + * 0x85 bulk => digital TS2 + * (*: audio should always be isoc) + * (**: analog, if ep 0x82 is isoc, otherwise digital) + * + * The new logic preserves backwards compatibility and + * reflects the endpoint configurations we have seen + * so far. But there might be devices for which this + * logic is not sufficient... + */ + + e = &intf->altsetting[alt].endpoint[ep].desc; + + if (!usb_endpoint_dir_in(e)) + return; + + sizedescr = le16_to_cpu(e->wMaxPacketSize); + size = sizedescr & 0x7ff; + + if (udev->speed == USB_SPEED_HIGH) + size = size * hb_mult(sizedescr); + + /* Only inspect input endpoints */ + + switch (e->bEndpointAddress) { + case 0x82: + *has_video = true; + if (usb_endpoint_xfer_isoc(e)) { + dev->analog_ep_isoc = e->bEndpointAddress; + dev->alt_max_pkt_size_isoc[alt] = size; + } else if (usb_endpoint_xfer_bulk(e)) { + dev->analog_ep_bulk = e->bEndpointAddress; + } + return; + case 0x83: + if (usb_endpoint_xfer_isoc(e)) + *has_vendor_audio = true; + else + dev_err(&intf->dev, + "error: skipping audio endpoint 0x83, because it uses bulk transfers !\n"); + return; + case 0x84: + if (*has_video && (usb_endpoint_xfer_bulk(e))) { + dev->analog_ep_bulk = e->bEndpointAddress; + } else { + if (usb_endpoint_xfer_isoc(e)) { + if (size > dev->dvb_max_pkt_size_isoc) { + /* + * 2) some manufacturers (e.g. Terratec) + * disable endpoints by setting + * wMaxPacketSize to 0 bytes for all + * alt settings. So far, we've seen + * this for DVB isoc endpoints only. + */ + *has_dvb = true; + dev->dvb_ep_isoc = e->bEndpointAddress; + dev->dvb_max_pkt_size_isoc = size; + dev->dvb_alt_isoc = alt; + } + } else { + *has_dvb = true; + dev->dvb_ep_bulk = e->bEndpointAddress; + } + } + return; + case 0x85: + if (usb_endpoint_xfer_isoc(e)) { + if (size > dev->dvb_max_pkt_size_isoc_ts2) { + dev->dvb_ep_isoc_ts2 = e->bEndpointAddress; + dev->dvb_max_pkt_size_isoc_ts2 = size; + dev->dvb_alt_isoc = alt; + } + } else { + dev->dvb_ep_bulk_ts2 = e->bEndpointAddress; + } + return; + } +} + /* * em28xx_usb_probe() * checks for supported devices */ -static int em28xx_usb_probe(struct usb_interface *interface, +static int em28xx_usb_probe(struct usb_interface *intf, const struct usb_device_id *id) { struct usb_device *udev; @@ -3550,17 +3654,17 @@ static int em28xx_usb_probe(struct usb_interface *interface, int retval; bool has_vendor_audio = false, has_video = false, has_dvb = false; int i, nr, try_bulk; - const int ifnum = interface->altsetting[0].desc.bInterfaceNumber; + const int ifnum = intf->altsetting[0].desc.bInterfaceNumber; char *speed; - udev = usb_get_dev(interface_to_usbdev(interface)); + udev = usb_get_dev(interface_to_usbdev(intf)); /* Check to see next free device and mark as used */ do { nr = find_first_zero_bit(em28xx_devused, EM28XX_MAXBOARDS); if (nr >= EM28XX_MAXBOARDS) { /* No free device slots */ - dev_err(&interface->dev, + dev_err(&intf->dev, "Driver supports up to %i em28xx boards.\n", EM28XX_MAXBOARDS); retval = -ENOMEM; @@ -3569,13 +3673,13 @@ static int em28xx_usb_probe(struct usb_interface *interface, } while (test_and_set_bit(nr, em28xx_devused)); /* Don't register audio interfaces */ - if (interface->altsetting[0].desc.bInterfaceClass == USB_CLASS_AUDIO) { - dev_err(&interface->dev, + if (intf->altsetting[0].desc.bInterfaceClass == USB_CLASS_AUDIO) { + dev_err(&intf->dev, "audio device (%04x:%04x): interface %i, class %i\n", le16_to_cpu(udev->descriptor.idVendor), le16_to_cpu(udev->descriptor.idProduct), ifnum, - interface->altsetting[0].desc.bInterfaceClass); + intf->altsetting[0].desc.bInterfaceClass); retval = -ENODEV; goto err; @@ -3589,9 +3693,9 @@ static int em28xx_usb_probe(struct usb_interface *interface, } /* compute alternate max packet sizes */ - dev->alt_max_pkt_size_isoc = - kmalloc(sizeof(dev->alt_max_pkt_size_isoc[0]) * - interface->num_altsetting, GFP_KERNEL); + dev->alt_max_pkt_size_isoc = kcalloc(intf->num_altsetting, + sizeof(dev->alt_max_pkt_size_isoc[0]), + GFP_KERNEL); if (!dev->alt_max_pkt_size_isoc) { kfree(dev); retval = -ENOMEM; @@ -3599,106 +3703,17 @@ static int em28xx_usb_probe(struct usb_interface *interface, } /* Get endpoints */ - for (i = 0; i < interface->num_altsetting; i++) { + for (i = 0; i < intf->num_altsetting; i++) { int ep; for (ep = 0; - ep < interface->altsetting[i].desc.bNumEndpoints; - ep++) { - const struct usb_endpoint_descriptor *e; - int sizedescr, size; - - e = &interface->altsetting[i].endpoint[ep].desc; - - sizedescr = le16_to_cpu(e->wMaxPacketSize); - size = sizedescr & 0x7ff; - - if (udev->speed == USB_SPEED_HIGH) - size = size * hb_mult(sizedescr); - - if (usb_endpoint_dir_in(e)) { - switch (e->bEndpointAddress) { - case 0x82: - has_video = true; - if (usb_endpoint_xfer_isoc(e)) { - dev->analog_ep_isoc = - e->bEndpointAddress; - dev->alt_max_pkt_size_isoc[i] = size; - } else if (usb_endpoint_xfer_bulk(e)) { - dev->analog_ep_bulk = - e->bEndpointAddress; - } - break; - case 0x83: - if (usb_endpoint_xfer_isoc(e)) { - has_vendor_audio = true; - } else { - dev_err(&interface->dev, - "error: skipping audio endpoint 0x83, because it uses bulk transfers !\n"); - } - break; - case 0x84: - if (has_video && - (usb_endpoint_xfer_bulk(e))) { - dev->analog_ep_bulk = - e->bEndpointAddress; - } else { - if (usb_endpoint_xfer_isoc(e)) { - if (size > dev->dvb_max_pkt_size_isoc) { - has_dvb = true; /* see NOTE (~) */ - dev->dvb_ep_isoc = e->bEndpointAddress; - dev->dvb_max_pkt_size_isoc = size; - dev->dvb_alt_isoc = i; - } - } else { - has_dvb = true; - dev->dvb_ep_bulk = e->bEndpointAddress; - } - } - break; - case 0x85: - if (usb_endpoint_xfer_isoc(e)) { - if (size > dev->dvb_max_pkt_size_isoc_ts2) { - dev->dvb_ep_isoc_ts2 = e->bEndpointAddress; - dev->dvb_max_pkt_size_isoc_ts2 = size; - dev->dvb_alt_isoc = i; - } - } else { - dev->dvb_ep_bulk_ts2 = e->bEndpointAddress; - } - break; - } - } - /* - * NOTE: - * Old logic with support for isoc transfers only was: - * 0x82 isoc => analog - * 0x83 isoc => audio - * 0x84 isoc => digital - * - * New logic with support for bulk transfers - * 0x82 isoc => analog - * 0x82 bulk => analog - * 0x83 isoc* => audio - * 0x84 isoc => digital - * 0x84 bulk => analog or digital** - * 0x85 isoc => digital TS2 - * 0x85 bulk => digital TS2 - * (*: audio should always be isoc) - * (**: analog, if ep 0x82 is isoc, otherwise digital) - * - * The new logic preserves backwards compatibility and - * reflects the endpoint configurations we have seen - * so far. But there might be devices for which this - * logic is not sufficient... - */ - /* - * NOTE (~): some manufacturers (e.g. Terratec) disable - * endpoints by setting wMaxPacketSize to 0 bytes for - * all alt settings. So far, we've seen this for - * DVB isoc endpoints only. - */ - } + ep < intf->altsetting[i].desc.bNumEndpoints; + ep++) + em28xx_check_usb_descriptor(dev, udev, intf, + i, ep, + &has_vendor_audio, + &has_video, + &has_dvb); } if (!(has_vendor_audio || has_video || has_dvb)) { @@ -3721,7 +3736,7 @@ static int em28xx_usb_probe(struct usb_interface *interface, speed = "unknown"; } - dev_err(&interface->dev, + dev_err(&intf->dev, "New device %s %s @ %s Mbps (%04x:%04x, interface %d, class %d)\n", udev->manufacturer ? udev->manufacturer : "", udev->product ? udev->product : "", @@ -3729,7 +3744,7 @@ static int em28xx_usb_probe(struct usb_interface *interface, le16_to_cpu(udev->descriptor.idVendor), le16_to_cpu(udev->descriptor.idProduct), ifnum, - interface->altsetting->desc.bInterfaceNumber); + intf->altsetting->desc.bInterfaceNumber); /* * Make sure we have 480 Mbps of bandwidth, otherwise things like @@ -3737,8 +3752,8 @@ static int em28xx_usb_probe(struct usb_interface *interface, * not enough even for most Digital TV streams. */ if (udev->speed != USB_SPEED_HIGH && disable_usb_speed_check == 0) { - dev_err(&interface->dev, "Device initialization failed.\n"); - dev_err(&interface->dev, + dev_err(&intf->dev, "Device initialization failed.\n"); + dev_err(&intf->dev, "Device must be connected to a high-speed USB 2.0 port.\n"); retval = -ENODEV; goto err_free; @@ -3756,17 +3771,17 @@ static int em28xx_usb_probe(struct usb_interface *interface, dev->dev_next = NULL; if (has_vendor_audio) { - dev_err(&interface->dev, + dev_err(&intf->dev, "Audio interface %i found (Vendor Class)\n", ifnum); dev->usb_audio_type = EM28XX_USB_AUDIO_VENDOR; } - /* Checks if audio is provided by a USB Audio Class interface */ + /* Checks if audio is provided by a USB Audio Class intf */ for (i = 0; i < udev->config->desc.bNumInterfaces; i++) { struct usb_interface *uif = udev->config->interface[i]; if (uif->altsetting[0].desc.bInterfaceClass == USB_CLASS_AUDIO) { if (has_vendor_audio) - dev_err(&interface->dev, + dev_err(&intf->dev, "em28xx: device seems to have vendor AND usb audio class interfaces !\n" "\t\tThe vendor interface will be ignored. Please contact the developers \n"); dev->usb_audio_type = EM28XX_USB_AUDIO_CLASS; @@ -3775,27 +3790,27 @@ static int em28xx_usb_probe(struct usb_interface *interface, } if (has_video) - dev_err(&interface->dev, "Video interface %i found:%s%s\n", + dev_err(&intf->dev, "Video interface %i found:%s%s\n", ifnum, dev->analog_ep_bulk ? " bulk" : "", dev->analog_ep_isoc ? " isoc" : ""); if (has_dvb) - dev_err(&interface->dev, "DVB interface %i found:%s%s\n", + dev_err(&intf->dev, "DVB interface %i found:%s%s\n", ifnum, dev->dvb_ep_bulk ? " bulk" : "", dev->dvb_ep_isoc ? " isoc" : ""); - dev->num_alt = interface->num_altsetting; + dev->num_alt = intf->num_altsetting; if ((unsigned int)card[nr] < em28xx_bcount) dev->model = card[nr]; - /* save our data pointer in this interface device */ - usb_set_intfdata(interface, dev); + /* save our data pointer in this intf device */ + usb_set_intfdata(intf, dev); /* allocate device struct and check if the device is a webcam */ mutex_init(&dev->lock); - retval = em28xx_init_dev(dev, udev, interface, nr); + retval = em28xx_init_dev(dev, udev, intf, nr); if (retval) goto err_free; @@ -3812,7 +3827,7 @@ static int em28xx_usb_probe(struct usb_interface *interface, if (has_video && dev->board.decoder == EM28XX_NODECODER && dev->em28xx_sensor == EM28XX_NOSENSOR) { - dev_err(&interface->dev, + dev_err(&intf->dev, "Currently, V4L2 is not supported on this model\n"); has_video = false; dev->has_video = false; @@ -3822,13 +3837,13 @@ static int em28xx_usb_probe(struct usb_interface *interface, if (has_video) { if (!dev->analog_ep_isoc || (try_bulk && dev->analog_ep_bulk)) dev->analog_xfer_bulk = 1; - dev_err(&interface->dev, "analog set to %s mode.\n", + dev_err(&intf->dev, "analog set to %s mode.\n", dev->analog_xfer_bulk ? "bulk" : "isoc"); } if (has_dvb) { if (!dev->dvb_ep_isoc || (try_bulk && dev->dvb_ep_bulk)) dev->dvb_xfer_bulk = 1; - dev_err(&interface->dev, "dvb set to %s mode.\n", + dev_err(&intf->dev, "dvb set to %s mode.\n", dev->dvb_xfer_bulk ? "bulk" : "isoc"); } @@ -3842,8 +3857,8 @@ static int em28xx_usb_probe(struct usb_interface *interface, dev->dev_next->model = id->driver_info; mutex_init(&dev->dev_next->lock); - retval = em28xx_init_dev(dev->dev_next, udev, interface, - dev->dev_next->devno); + retval = em28xx_init_dev(dev->dev_next, udev, intf, + dev->dev_next->devno); if (retval) goto err_free; @@ -3923,12 +3938,12 @@ err_no_slot: * called when the device gets disconnected * video device will be unregistered on v4l2_close in case it is still open */ -static void em28xx_usb_disconnect(struct usb_interface *interface) +static void em28xx_usb_disconnect(struct usb_interface *intf) { struct em28xx *dev; - dev = usb_get_intfdata(interface); - usb_set_intfdata(interface, NULL); + dev = usb_get_intfdata(intf); + usb_set_intfdata(intf, NULL); if (!dev) return; @@ -3959,23 +3974,23 @@ static void em28xx_usb_disconnect(struct usb_interface *interface) kref_put(&dev->ref, em28xx_free_device); } -static int em28xx_usb_suspend(struct usb_interface *interface, +static int em28xx_usb_suspend(struct usb_interface *intf, pm_message_t message) { struct em28xx *dev; - dev = usb_get_intfdata(interface); + dev = usb_get_intfdata(intf); if (!dev) return 0; em28xx_suspend_extension(dev); return 0; } -static int em28xx_usb_resume(struct usb_interface *interface) +static int em28xx_usb_resume(struct usb_interface *intf) { struct em28xx *dev; - dev = usb_get_intfdata(interface); + dev = usb_get_intfdata(intf); if (!dev) return 0; em28xx_resume_extension(dev); -- 2.11.0