Message ID | 1312442059-23935-17-git-send-email-thierry.reding@avionic-design.de (mailing list archive) |
---|---|
State | Rejected, archived |
Headers |
Return-path: <linux-media-owner@vger.kernel.org> Envelope-to: mchehab@infradead.org Delivery-date: Thu, 04 Aug 2011 07:15:05 +0000 Received: from casper.infradead.org [85.118.1.10] by localhost.localdomain with IMAP (fetchmail-6.3.17) for <mchehab@localhost> (single-drop); Thu, 04 Aug 2011 08:52:39 -0300 (BRT) Received: from vger.kernel.org ([209.132.180.67]) by casper.infradead.org with esmtp (Exim 4.76 #1 (Red Hat Linux)) id 1Qos93-0002rl-BJ; Thu, 04 Aug 2011 07:15:05 +0000 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752171Ab1HDHOr (ORCPT <rfc822; kmpark@infradead.org> + 1 other); Thu, 4 Aug 2011 03:14:47 -0400 Received: from moutng.kundenserver.de ([212.227.126.186]:52529 "EHLO moutng.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752050Ab1HDHO3 (ORCPT <rfc822; linux-media@vger.kernel.org>); Thu, 4 Aug 2011 03:14:29 -0400 Received: from localhost (p548E06E4.dip0.t-ipconnect.de [84.142.6.228]) by mrelayeu.kundenserver.de (node=mrbap4) with ESMTP (Nemesis) id 0LwZNH-1RRDY31TZP-018IQi; Thu, 04 Aug 2011 09:14:28 +0200 From: Thierry Reding <thierry.reding@avionic-design.de> To: linux-media@vger.kernel.org Subject: [PATCH 16/21] [staging] tm6000: Select interface on first open. Date: Thu, 4 Aug 2011 09:14:14 +0200 Message-Id: <1312442059-23935-17-git-send-email-thierry.reding@avionic-design.de> X-Mailer: git-send-email 1.7.6 In-Reply-To: <1312442059-23935-1-git-send-email-thierry.reding@avionic-design.de> References: <1312442059-23935-1-git-send-email-thierry.reding@avionic-design.de> X-Provags-ID: V02:K0:Dzv1KSk8B+TUFf+5rCPEFGejyhcPkSDRU6slE22EoKv 0EYBrOIO9y7JfOKc9Q9uvSerMVx6Qdtyn2AEgG7NmgkPDrHa72 8B64Qq1gywX4JaAaIbDLwCbMbDpPxKKflgjJ8Sd4xwp+IlMrz8 R4cOFiZ5oqGdXqFuPG/UUE9SzSdjFNtW2T0MceT4p7UqcCVMm9 b6FR4hPSSxZ66uX4qraE+XBKc/RoWZaYG0NE9mSydN6AM0E003 mx6K3fEKgL0+Z Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: <linux-media.vger.kernel.org> X-Mailing-List: linux-media@vger.kernel.org |
Commit Message
Thierry Reding
Aug. 4, 2011, 7:14 a.m. UTC
Instead of selecting the default interface setting when preparing isochronous transfers, select it on the first call to open() to make sure it is available earlier. --- drivers/staging/tm6000/tm6000-video.c | 17 ++++++++++++----- 1 files changed, 12 insertions(+), 5 deletions(-)
Comments
Em 04-08-2011 04:14, Thierry Reding escreveu: > Instead of selecting the default interface setting when preparing > isochronous transfers, select it on the first call to open() to make > sure it is available earlier. Hmm... I fail to see what this is needed earlier. The ISOC endpont is used only when the device is streaming. Did you get any bug related to it? If so, please describe it better. > --- > drivers/staging/tm6000/tm6000-video.c | 17 ++++++++++++----- > 1 files changed, 12 insertions(+), 5 deletions(-) > > diff --git a/drivers/staging/tm6000/tm6000-video.c b/drivers/staging/tm6000/tm6000-video.c > index 70fc19e..b59a0da 100644 > --- a/drivers/staging/tm6000/tm6000-video.c > +++ b/drivers/staging/tm6000/tm6000-video.c > @@ -595,11 +595,6 @@ static int tm6000_prepare_isoc(struct tm6000_core *dev) > tm6000_uninit_isoc(dev); > /* Stop interrupt USB pipe */ > tm6000_ir_int_stop(dev); > - > - usb_set_interface(dev->udev, > - dev->isoc_in.bInterfaceNumber, > - dev->isoc_in.bAlternateSetting); > - > /* Start interrupt USB pipe */ > tm6000_ir_int_start(dev); > > @@ -1484,6 +1479,18 @@ static int tm6000_open(struct file *file) > break; > } > > + if (dev->users == 0) { > + int err = usb_set_interface(dev->udev, > + dev->isoc_in.bInterfaceNumber, > + dev->isoc_in.bAlternateSetting); > + if (err < 0) { > + dev_err(&vdev->dev, "failed to select interface %d, " > + "alt. setting %d\n", > + dev->isoc_in.bInterfaceNumber, > + dev->isoc_in.bAlternateSetting); > + } > + } > + > /* If more than one user, mutex should be added */ > dev->users++; > -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
* Mauro Carvalho Chehab wrote: > Em 04-08-2011 04:14, Thierry Reding escreveu: > > Instead of selecting the default interface setting when preparing > > isochronous transfers, select it on the first call to open() to make > > sure it is available earlier. > > Hmm... I fail to see what this is needed earlier. The ISOC endpont is used > only when the device is streaming. > > Did you get any bug related to it? If so, please describe it better. I'm not sure whether this really fixes a bug, but it seems a little wrong to me to selecting the interface so late in the process when in fact the device is already being configured before (video standard, audio mode, firmware upload, ...). Thinking about it, this may actually be part of the fix for the "device hangs sometimes for inexplicable reasons" bug that this whole patch series seems to fix. Thierry
Em 01-09-2011 02:19, Thierry Reding escreveu: > * Mauro Carvalho Chehab wrote: >> Em 04-08-2011 04:14, Thierry Reding escreveu: >>> Instead of selecting the default interface setting when preparing >>> isochronous transfers, select it on the first call to open() to make >>> sure it is available earlier. >> >> Hmm... I fail to see what this is needed earlier. The ISOC endpont is used >> only when the device is streaming. >> >> Did you get any bug related to it? If so, please describe it better. > > I'm not sure whether this really fixes a bug, but it seems a little wrong to > me to selecting the interface so late in the process when in fact the device > is already being configured before (video standard, audio mode, firmware > upload, ...). Some applications may open the device just to change the controls. All other drivers only set alternates/interfaces when the streaming is requested, as alternates/interfaces are needed only there. > Thinking about it, this may actually be part of the fix for the "device hangs > sometimes for inexplicable reasons" bug that this whole patch series seems to > fix. It is unlikely, except if the firmware inside the chip is broken (unfortunately, we have serious reasons to believe that the internal firmware on this chipset has serious bugs). I prefer to not apply this patch, except if we have a good reason for that, as otherwise this driver will behave different than the others. Regards, Mauro. > > Thierry -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
* Mauro Carvalho Chehab wrote: > Em 01-09-2011 02:19, Thierry Reding escreveu: > > * Mauro Carvalho Chehab wrote: > >> Em 04-08-2011 04:14, Thierry Reding escreveu: > >>> Instead of selecting the default interface setting when preparing > >>> isochronous transfers, select it on the first call to open() to make > >>> sure it is available earlier. > >> > >> Hmm... I fail to see what this is needed earlier. The ISOC endpont is used > >> only when the device is streaming. > >> > >> Did you get any bug related to it? If so, please describe it better. > > > > I'm not sure whether this really fixes a bug, but it seems a little wrong to > > me to selecting the interface so late in the process when in fact the device > > is already being configured before (video standard, audio mode, firmware > > upload, ...). > > Some applications may open the device just to change the controls. All other drivers > only set alternates/interfaces when the streaming is requested, as alternates/interfaces > are needed only there. Okay, I didn't know that it was only necessary for streaming. > > Thinking about it, this may actually be part of the fix for the "device hangs > > sometimes for inexplicable reasons" bug that this whole patch series seems to > > fix. > > It is unlikely, except if the firmware inside the chip is broken (unfortunately, > we have serious reasons to believe that the internal firmware on this chipset has > serious bugs). Indeed! =) > I prefer to not apply this patch, except if we have a good reason for that, > as otherwise this driver will behave different than the others. Okay, it's your call. Unfortunately I no longer have the hardware available to test if this is really related to the bug. I'll have to check again when I have the hardware. Thierry
diff --git a/drivers/staging/tm6000/tm6000-video.c b/drivers/staging/tm6000/tm6000-video.c index 70fc19e..b59a0da 100644 --- a/drivers/staging/tm6000/tm6000-video.c +++ b/drivers/staging/tm6000/tm6000-video.c @@ -595,11 +595,6 @@ static int tm6000_prepare_isoc(struct tm6000_core *dev) tm6000_uninit_isoc(dev); /* Stop interrupt USB pipe */ tm6000_ir_int_stop(dev); - - usb_set_interface(dev->udev, - dev->isoc_in.bInterfaceNumber, - dev->isoc_in.bAlternateSetting); - /* Start interrupt USB pipe */ tm6000_ir_int_start(dev); @@ -1484,6 +1479,18 @@ static int tm6000_open(struct file *file) break; } + if (dev->users == 0) { + int err = usb_set_interface(dev->udev, + dev->isoc_in.bInterfaceNumber, + dev->isoc_in.bAlternateSetting); + if (err < 0) { + dev_err(&vdev->dev, "failed to select interface %d, " + "alt. setting %d\n", + dev->isoc_in.bInterfaceNumber, + dev->isoc_in.bAlternateSetting); + } + } + /* If more than one user, mutex should be added */ dev->users++;