Bug 11767

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.
> 

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
 };
 

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.

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

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.

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

(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.

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...

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

I had a mistake in my patch file. One bracket wasnt't copied. Well, it
seems to work well, now. Thanks for help :-)

This patch works for me (without it, speedtch was unusable with 2.6.27).

The patch was just sent to Linus for 2.6.28, so this bug report can be closed.