[media] em28xx: fix analog streaming with USB bulk transfers
authorFrank Schaefer <fschaefer.oss@googlemail.com>
Thu, 7 Feb 2013 16:32:46 +0000 (13:32 -0300)
committerMauro Carvalho Chehab <mchehab@redhat.com>
Mon, 11 Feb 2013 20:17:44 +0000 (18:17 -0200)
With the conversion to videobuf2, some unnecessary calls of
em28xx_set_alternate() have been removed. It is now called at analog streaming
start only.
This has unveiled a bug that causes USB bulk transfers to fail with all urbs
having status -EVOERFLOW.
The reason is, that for bulk transfers usb_set_interface() needs to be called
even if the previous alt setting was the same (side note: bulk transfers seem
to work only with alt=0).
While it seems to be NOT necessary for isoc transfers, it's reasonable to just
call usb_set_interface() unconditionally in em28xx_set_alternate().
Also add a comment that explains the issue to prevent regressions in the future.

Cc: stable@vger.kernel.org # for 3.8
Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
drivers/media/usb/em28xx/em28xx-core.c

index ee00f9e2342093c5a11f9af9a02f38095f31a6a3..aaedd11791f22fd4e1f7abc3fd2c3ef84d66123f 100644 (file)
@@ -813,12 +813,12 @@ int em28xx_resolution_set(struct em28xx *dev)
 /* Set USB alternate setting for analog video */
 int em28xx_set_alternate(struct em28xx *dev)
 {
-       int errCode, prev_alt = dev->alt;
+       int errCode;
        int i;
        unsigned int min_pkt_size = dev->width * 2 + 4;
 
        /* NOTE: for isoc transfers, only alt settings > 0 are allowed
-                for bulk transfers, use alt=0 as default value */
+                bulk transfers seem to work only with alt=0 ! */
        dev->alt = 0;
        if ((alt > 0) && (alt < dev->num_alt)) {
                em28xx_coredbg("alternate forced to %d\n", dev->alt);
@@ -849,25 +849,26 @@ int em28xx_set_alternate(struct em28xx *dev)
        }
 
 set_alt:
-       if (dev->alt != prev_alt) {
-               if (dev->analog_xfer_bulk) {
-                       dev->max_pkt_size = 512; /* USB 2.0 spec */
-                       dev->packet_multiplier = EM28XX_BULK_PACKET_MULTIPLIER;
-               } else { /* isoc */
-                       em28xx_coredbg("minimum isoc packet size: %u (alt=%d)\n",
-                                      min_pkt_size, dev->alt);
-                       dev->max_pkt_size =
-                                         dev->alt_max_pkt_size_isoc[dev->alt];
-                       dev->packet_multiplier = EM28XX_NUM_ISOC_PACKETS;
-               }
-               em28xx_coredbg("setting alternate %d with wMaxPacketSize=%u\n",
-                              dev->alt, dev->max_pkt_size);
-               errCode = usb_set_interface(dev->udev, 0, dev->alt);
-               if (errCode < 0) {
-                       em28xx_errdev("cannot change alternate number to %d (error=%i)\n",
-                                       dev->alt, errCode);
-                       return errCode;
-               }
+       /* NOTE: for bulk transfers, we need to call usb_set_interface()
+        * even if the previous settings were the same. Otherwise streaming
+        * fails with all urbs having status = -EOVERFLOW ! */
+       if (dev->analog_xfer_bulk) {
+               dev->max_pkt_size = 512; /* USB 2.0 spec */
+               dev->packet_multiplier = EM28XX_BULK_PACKET_MULTIPLIER;
+       } else { /* isoc */
+               em28xx_coredbg("minimum isoc packet size: %u (alt=%d)\n",
+                              min_pkt_size, dev->alt);
+               dev->max_pkt_size =
+                                 dev->alt_max_pkt_size_isoc[dev->alt];
+               dev->packet_multiplier = EM28XX_NUM_ISOC_PACKETS;
+       }
+       em28xx_coredbg("setting alternate %d with wMaxPacketSize=%u\n",
+                      dev->alt, dev->max_pkt_size);
+       errCode = usb_set_interface(dev->udev, 0, dev->alt);
+       if (errCode < 0) {
+               em28xx_errdev("cannot change alternate number to %d (error=%i)\n",
+                             dev->alt, errCode);
+               return errCode;
        }
        return 0;
 }