Bug 11767

Summary: Usbatm/speedtouch-driver bug
Product: Drivers Reporter: Arek Wiesner (phini)
Component: USBAssignee: Piotr Mikosik (mikosik)
Status: RESOLVED CODE_FIX    
Severity: normal CC: alan, frederic.coiffier
Priority: P1    
Hardware: All   
OS: Linux   
Kernel Version: 2.6.27 Subsystem:
Regression: Yes Bisected commit-id:

Description Arek Wiesner 2008-10-15 09:22:08 UTC
Latest working kernel version: 2.6.26
Distribution: Arch Linux
Hardware Environment: Speedtouch 330

Problem Description:
After updating to 2.6.27 kernel, I'm no longer able to boot my OS.
http://wklej.org/id/10663/

Steps to reproduce:
Install the 2.6.27 kernel
Boot the system.
It will fail at loading modules.
Comment 1 Anonymous Emailer 2008-10-15 10:23:38 UTC
Reply-To: akpm@linux-foundation.org


(switched to email.  Please respond via emailed reply-to-all, not via the
bugzilla web interface).

On Wed, 15 Oct 2008 09:22:09 -0700 (PDT) bugme-daemon@bugzilla.kernel.org wrote:

> http://bugzilla.kernel.org/show_bug.cgi?id=11767
> 
>            Summary: Usbatm/speedtouch-driver bug
>            Product: Drivers
>            Version: 2.5
>      KernelVersion: 2.6.27
>           Platform: All
>         OS/Version: Linux
>               Tree: Mainline
>             Status: NEW
>           Severity: normal
>           Priority: P1
>          Component: USB
>         AssignedTo: greg@kroah.com
>         ReportedBy: phini@phini.pol.pl
> 
> 
> Latest working kernel version: 2.6.26

USB ATM oopsed.  It is a regression.

> Distribution: Arch Linux
> Hardware Environment: Speedtouch 330
> 
> Problem Description:
> After updating to 2.6.27 kernel, I'm no longer able to boot my OS.
> http://wklej.org/id/10663/
> 
> Steps to reproduce:
> Install the 2.6.27 kernel
> Boot the system.
> It will fail at loading modules.
> 
Comment 2 Alan Stern 2008-10-15 12:37:57 UTC
On Wed, 15 Oct 2008, Andrew Morton wrote:

> > http://bugzilla.kernel.org/show_bug.cgi?id=11767
> > 
> >            Summary: Usbatm/speedtouch-driver bug

> > Problem Description:
> > After updating to 2.6.27 kernel, I'm no longer able to boot my OS.
> > http://wklej.org/id/10663/

The problem is that the speetch driver calls usb_reset_device() without
defining pre_reset and post_reset methods.  As a result, any interfaces
it claims during probe get disconnected.

Here's a minimal fix.  It isn't entirely correct, but at least it
should get the driver working as well as it did before 2.6.27.

Alan Stern


Index: usb-2.6/drivers/usb/atm/speedtch.c
===================================================================
--- usb-2.6.orig/drivers/usb/atm/speedtch.c
+++ usb-2.6/drivers/usb/atm/speedtch.c
@@ -722,6 +722,16 @@ static void speedtch_atm_stop(struct usb
 	flush_scheduled_work();
 }
 
+static int speedtch_pre_reset(struct usb_interface *intf)
+{
+	return 0;
+}
+
+static int speedtch_post_reset(struct usb_interface *intf)
+{
+	return 0;
+}
+
 
 /**********
 **  USB  **
@@ -740,6 +750,8 @@ static struct usb_driver speedtch_usb_dr
 	.name		= speedtch_driver_name,
 	.probe		= speedtch_usb_probe,
 	.disconnect	= usbatm_usb_disconnect,
+	.pre_reset	= speedtch_pre_reset,
+	.post_reset	= speedtch_post_reset,
 	.id_table	= speedtch_usb_ids
 };
 
Comment 3 Anonymous Emailer 2008-10-16 01:27:14 UTC
Reply-To: duncan.sands@free.fr

Hi Alan, thanks for looking into this.

> Here's a minimal fix.  It isn't entirely correct, but at least it
> should get the driver working as well as it did before 2.6.27.

Sounds good to me!

Best wishes,

Duncan.
Comment 4 Alan Stern 2008-10-16 06:42:17 UTC
On Thu, 16 Oct 2008, Duncan Sands wrote:

> Hi Alan, thanks for looking into this.
> 
> > Here's a minimal fix.  It isn't entirely correct, but at least it
> > should get the driver working as well as it did before 2.6.27.
> 
> Sounds good to me!

Duncan, do you want to improve this at all?  It seems reasonable that 
the driver would want to take some positive action when it is notified 
that the device is about to be reset.

But if you're happy with the patch as it is then I'll send it to Greg
for inclusion in 2.6.27.stable.

Alan Stern
Comment 5 Anonymous Emailer 2008-10-16 06:55:23 UTC
Reply-To: duncan.sands@free.fr

Hi Alan,

> Duncan, do you want to improve this at all?  It seems reasonable that 
> the driver would want to take some positive action when it is notified 
> that the device is about to be reset.

in general that is true, though in the case in question it reset itself,
so already knows what is going on.  (It's a pity that these modems require
prodding like this).  I'm not going to improve it because I'm not interested
in these modems any more (I still have one in a cupboard somewhere in case
of a crisis).

> But if you're happy with the patch as it is then I'll send it to Greg
> for inclusion in 2.6.27.stable.

Yes, thanks.

Best wishes,

Duncan.
Comment 6 Alan Stern 2008-10-16 09:57:36 UTC
On Thu, 16 Oct 2008, Duncan Sands wrote:

> > But if you're happy with the patch as it is then I'll send it to Greg
> > for inclusion in 2.6.27.stable.
> 
> Yes, thanks.

Arek, I don't want to submit the patch until you confirm that it fixes 
your bug.

Alan Stern
Comment 7 Arek Wiesner 2008-10-16 13:24:05 UTC
(In reply to comment #6)
> On Thu, 16 Oct 2008, Duncan Sands wrote:
> 
> > > But if you're happy with the patch as it is then I'll send it to Greg
> > > for inclusion in 2.6.27.stable.
> > 
> > Yes, thanks.
> 
> Arek, I don't want to submit the patch until you confirm that it fixes 
> your bug.
> 
> Alan Stern
> 

drivers/usb/atm/speedtch.c: In function ‘speedtch_atm_stop’:
drivers/usb/atm/speedtch.c:724: error: invalid storage class for function ‘speedtch_pre_reset’
drivers/usb/atm/speedtch.c:723: warning: ISO C90 forbids mixed declarations and code
drivers/usb/atm/speedtch.c:729: error: invalid storage class for function ‘speedtch_post_reset’

That's what happens when I want to compile kernel with that patch.
Comment 8 Arek Wiesner 2008-10-17 10:03:26 UTC
drivers/usb/atm/speedtch.c: In function ‘speedtch_atm_stop’:
drivers/usb/atm/speedtch.c:724: error: invalid storage class for
function ‘speedtch_pre_reset’
drivers/usb/atm/speedtch.c:723: warning: ISO C90 forbids mixed
declarations and code
drivers/usb/atm/speedtch.c:729: error: invalid storage class for
function ‘speedtch_post_reset’

That's what happens when I want to compile kernel with that patch. I'm
using GCC 4.2.3.

PS: Sorry for posting this twice. I forgot not to reply by bugzilla
interface...
Comment 9 Alan Stern 2008-10-17 12:12:56 UTC
On Fri, 17 Oct 2008, Arek Wiesner wrote:

> drivers/usb/atm/speedtch.c: In function ‘speedtch_atm_stop’:
> drivers/usb/atm/speedtch.c:724: error: invalid storage class for
> function ‘speedtch_pre_reset’
> drivers/usb/atm/speedtch.c:723: warning: ISO C90 forbids mixed
> declarations and code
> drivers/usb/atm/speedtch.c:729: error: invalid storage class for
> function ‘speedtch_post_reset’
> 
> That's what happens when I want to compile kernel with that patch. I'm
> using GCC 4.2.3.

I don't understand.  Are you sure the patch was applied correctly?  
This is what my file looks like, starting at line 720; yours should be
the same:

-------------------------------------------------------------------
	usb_free_urb(int_urb);

	flush_scheduled_work();
}

static int speedtch_pre_reset(struct usb_interface *intf)
{
	return 0;
}

static int speedtch_post_reset(struct usb_interface *intf)
{
	return 0;
}


/**********
**  USB  **
**********/
-------------------------------------------------------------------

Alan Stern
Comment 10 Arek Wiesner 2008-10-17 13:17:17 UTC
I had a mistake in my patch file. One bracket wasnt't copied. Well, it
seems to work well, now. Thanks for help :-)
Comment 11 Frédéric COIFFIER 2008-10-21 10:11:46 UTC
This patch works for me (without it, speedtch was unusable with 2.6.27).
Comment 12 Alan Stern 2008-10-22 11:06:59 UTC
The patch was just sent to Linus for 2.6.28, so this bug report can be closed.