On 05-12-2011 05:21, Thierry Reding wrote:
* linuxtv@xxxxxxxxxxxxxxx wrote:
From: Stefan Ringel<linuxtv@xxxxxxxxxxxxxxx>
Signed-off-by: Stefan Ringel<linuxtv@xxxxxxxxxxxxxxx>
Your commit message needs more details. Why do you think this is a bugfix?
Also this commit seems to effectively revert (and then partially reimplement)
a patch that I posted some months ago.
Thierry,
I noticed this. I tested tm6000 with those changes with both the first gen
tm5600 devices I have and HVR900H and I didn't notice any bad thing with this
approach, and changing from one standard to another is now faster.
So, I decided to apply it (with the remaining patches I've made to
fix audio for PAL/M and NTSC/M).
I also noticed that TM6000_QUIRK_NO_USB_DELAY is not needed anymore
(still, Stefan's patches didn't remove it completely).
Could you please test if the problems you've solved with your approach
are still occurring?
Regards,
Mauro
---
drivers/media/video/tm6000/tm6000-core.c | 49 -----------------------------
drivers/media/video/tm6000/tm6000-video.c | 21 ++++++++++--
2 files changed, 17 insertions(+), 53 deletions(-)
diff --git a/drivers/media/video/tm6000/tm6000-core.c b/drivers/media/video/tm6000/tm6000-core.c
index c007e6d..920299e 100644
--- a/drivers/media/video/tm6000/tm6000-core.c
+++ b/drivers/media/video/tm6000/tm6000-core.c
@@ -599,55 +599,6 @@ int tm6000_init(struct tm6000_core *dev)
return rc;
}
-int tm6000_reset(struct tm6000_core *dev)
-{
- int pipe;
- int err;
-
- msleep(500);
-
- err = usb_set_interface(dev->udev, dev->isoc_in.bInterfaceNumber, 0);
- if (err< 0) {
- tm6000_err("failed to select interface %d, alt. setting 0\n",
- dev->isoc_in.bInterfaceNumber);
- return err;
- }
-
- err = usb_reset_configuration(dev->udev);
- if (err< 0) {
- tm6000_err("failed to reset configuration\n");
- return err;
- }
-
- if ((dev->quirks& TM6000_QUIRK_NO_USB_DELAY) == 0)
- msleep(5);
-
- /*
- * Not all devices have int_in defined
- */
- if (!dev->int_in.endp)
- return 0;
-
- err = usb_set_interface(dev->udev, dev->isoc_in.bInterfaceNumber, 2);
- if (err< 0) {
- tm6000_err("failed to select interface %d, alt. setting 2\n",
- dev->isoc_in.bInterfaceNumber);
- return err;
- }
-
- msleep(5);
-
- pipe = usb_rcvintpipe(dev->udev,
- dev->int_in.endp->desc.bEndpointAddress& USB_ENDPOINT_NUMBER_MASK);
-
- err = usb_clear_halt(dev->udev, pipe);
- if (err< 0) {
- tm6000_err("usb_clear_halt failed: %d\n", err);
- return err;
- }
-
- return 0;
-}
int tm6000_set_audio_bitrate(struct tm6000_core *dev, int bitrate)
{
diff --git a/drivers/media/video/tm6000/tm6000-video.c b/drivers/media/video/tm6000/tm6000-video.c
index 1e5ace0..4db3535 100644
--- a/drivers/media/video/tm6000/tm6000-video.c
+++ b/drivers/media/video/tm6000/tm6000-video.c
@@ -1609,12 +1609,25 @@ static int tm6000_release(struct file *file)
tm6000_uninit_isoc(dev);
+ /* Stop interrupt USB pipe */
+ tm6000_ir_int_stop(dev);
+
+ usb_reset_configuration(dev->udev);
+
+ if (&dev->int_in)
This check is wrong,&dev->int_in will always be true.
+ usb_set_interface(dev->udev,
+ dev->isoc_in.bInterfaceNumber,
+ 2);
+ else
+ usb_set_interface(dev->udev,
+ dev->isoc_in.bInterfaceNumber,
+ 0);
This would need better indentation.
+
+ /* Start interrupt USB pipe */
+ tm6000_ir_int_start(dev);
+
Why do you restart the IR interrupt pipe when the device is being released?
if (!fh->radio)
videobuf_mmap_free(&fh->vb_vidq);
-
- err = tm6000_reset(dev);
- if (err< 0)
- dev_err(&vdev->dev, "reset failed: %d\n", err);
}
kfree(fh);
I think this whole patch should be much shorter. Something along the lines
of:
@@ -1609,12 +1609,25 @@ static int tm6000_release(struct file *file)
tm6000_uninit_isoc(dev);
+ /* Stop interrupt USB pipe */
+ tm6000_ir_int_stop(dev);
+
if (!fh->radio)
videobuf_mmap_free(&fh->vb_vidq);
Thierry
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
