Patchwork [Natty,SRU,1/1] usbnet/cdc_ncm: add missing .reset_resume hook

login
register
mail settings
Submitter Leann Ogasawara
Date June 12, 2011, 11:25 p.m.
Message ID <1307921137.2573.18.camel@hp-mini>
Download mbox | patch
Permalink /patch/100122/
State New
Headers show

Pull-request

git://kernel.ubuntu.com/ogasawara/ubuntu-natty.git lp793892

Comments

Leann Ogasawara - June 12, 2011, 11:25 p.m.
BugLink: http://bugs.launchpad.net/bugs/793892

SRU Justification:

Impact: The cdc_ncm module, which provides support for the Ericsson
F5521gw Mobile Broadband Modem, unregisters the device after suspend
because the .reset_resume hook in the driver is not assigned.

Fix: Upstream commit 85e3c65fa3a1d0542c18151

Test Case: Without the fix, after resume from suspend you'll notice
messages similar to the following in dmesg output:

cdc_ncm 2-1.4:1.6: no reset_resume for driver cdc_ncm?
cdc_ncm 2-1.4:1.7: no reset_resume for driver cdc_ncm?
cdc_ncm 2-1.4:1.6: usb0: unregister 'cdc_ncm' usb-0000:00:1d.0-1.4, CDC NCM

The patch author, who is also the LP bug reporter, submitted this to
upstream 2.6.38.y but was unfortunately told there will be no further
2.6.38.y releases:  

http://marc.info/?l=linux-usb&m=130737305305181&w=2

I've built a test kernel with the patch applied and have confirmation it
resolves the issue.  Please consider for Natty SRU.  Note the only
reason it is not a clean cherry-pick is due to the DRIVER_VERSION
string.

Thanks,
Leann

The following changes since commit f3fd91d6053b0fc7d37815d2830495ffe5c6b06e:
  Tim Gardner (1):
        UBUNTU: Start new release

are available in the git repository at:

  git://kernel.ubuntu.com/ogasawara/ubuntu-natty.git lp793892

Stefan Metzmacher (1):
      usbnet/cdc_ncm: add missing .reset_resume hook

 drivers/net/usb/cdc_ncm.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

From 9a5ebc5a7ace79a8683f6ac0d10d154d21e73b4d Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze@samba.org>
Date: Wed, 1 Jun 2011 02:01:41 +0000
Subject: [PATCH] usbnet/cdc_ncm: add missing .reset_resume hook

BugLink: http://bugs.launchpad.net/bugs/793892

This avoids messages like this after suspend:

   cdc_ncm 2-1.4:1.6: no reset_resume for driver cdc_ncm?
   cdc_ncm 2-1.4:1.7: no reset_resume for driver cdc_ncm?
   cdc_ncm 2-1.4:1.6: usb0: unregister 'cdc_ncm' usb-0000:00:1d.0-1.4, CDC NCM

This is important for the Ericsson F5521gw GSM/UMTS modem.
Otherwise modemmanager looses the fact that the cdc_ncm and cdc_acm devices
belong together.

The cdc_ether module does the same.

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Signed-off-by: David S. Miller <davem@davemloft.net>

(backport from commit 85e3c65fa3a1d0542c18151 upstream)
Signed-off-by: Leann Ogasawara <leann.ogasawara@canonical.com>
---
 drivers/net/usb/cdc_ncm.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
Brad Figg - June 13, 2011, 9:27 a.m.
On 06/12/2011 04:25 PM, Leann Ogasawara wrote:
> BugLink: http://bugs.launchpad.net/bugs/793892
>
> SRU Justification:
>
> Impact: The cdc_ncm module, which provides support for the Ericsson
> F5521gw Mobile Broadband Modem, unregisters the device after suspend
> because the .reset_resume hook in the driver is not assigned.
>
> Fix: Upstream commit 85e3c65fa3a1d0542c18151
>
> Test Case: Without the fix, after resume from suspend you'll notice
> messages similar to the following in dmesg output:
>
> cdc_ncm 2-1.4:1.6: no reset_resume for driver cdc_ncm?
> cdc_ncm 2-1.4:1.7: no reset_resume for driver cdc_ncm?
> cdc_ncm 2-1.4:1.6: usb0: unregister 'cdc_ncm' usb-0000:00:1d.0-1.4, CDC NCM
>
> The patch author, who is also the LP bug reporter, submitted this to
> upstream 2.6.38.y but was unfortunately told there will be no further
> 2.6.38.y releases:
>
> http://marc.info/?l=linux-usb&m=130737305305181&w=2
>
> I've built a test kernel with the patch applied and have confirmation it
> resolves the issue.  Please consider for Natty SRU.  Note the only
> reason it is not a clean cherry-pick is due to the DRIVER_VERSION
> string.
>
> Thanks,
> Leann
>
> The following changes since commit f3fd91d6053b0fc7d37815d2830495ffe5c6b06e:
>    Tim Gardner (1):
>          UBUNTU: Start new release
>
> are available in the git repository at:
>
>    git://kernel.ubuntu.com/ogasawara/ubuntu-natty.git lp793892
>
> Stefan Metzmacher (1):
>        usbnet/cdc_ncm: add missing .reset_resume hook
>
>   drivers/net/usb/cdc_ncm.c |    3 ++-
>   1 files changed, 2 insertions(+), 1 deletions(-)
>
>  From 9a5ebc5a7ace79a8683f6ac0d10d154d21e73b4d Mon Sep 17 00:00:00 2001
> From: Stefan Metzmacher<metze@samba.org>
> Date: Wed, 1 Jun 2011 02:01:41 +0000
> Subject: [PATCH] usbnet/cdc_ncm: add missing .reset_resume hook
>
> BugLink: http://bugs.launchpad.net/bugs/793892
>
> This avoids messages like this after suspend:
>
>     cdc_ncm 2-1.4:1.6: no reset_resume for driver cdc_ncm?
>     cdc_ncm 2-1.4:1.7: no reset_resume for driver cdc_ncm?
>     cdc_ncm 2-1.4:1.6: usb0: unregister 'cdc_ncm' usb-0000:00:1d.0-1.4, CDC NCM
>
> This is important for the Ericsson F5521gw GSM/UMTS modem.
> Otherwise modemmanager looses the fact that the cdc_ncm and cdc_acm devices
> belong together.
>
> The cdc_ether module does the same.
>
> Signed-off-by: Stefan Metzmacher<metze@samba.org>
> Signed-off-by: David S. Miller<davem@davemloft.net>
>
> (backport from commit 85e3c65fa3a1d0542c18151 upstream)
> Signed-off-by: Leann Ogasawara<leann.ogasawara@canonical.com>
> ---
>   drivers/net/usb/cdc_ncm.c |    3 ++-
>   1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
> index 7113168..9d78fe6 100644
> --- a/drivers/net/usb/cdc_ncm.c
> +++ b/drivers/net/usb/cdc_ncm.c
> @@ -54,7 +54,7 @@
>   #include<linux/usb/usbnet.h>
>   #include<linux/usb/cdc.h>
>
> -#define	DRIVER_VERSION				"7-Feb-2011"
> +#define	DRIVER_VERSION				"01-June-2011"
>
>   /* CDC NCM subclass 3.2.1 */
>   #define USB_CDC_NCM_NDP16_LENGTH_MIN		0x10
> @@ -1254,6 +1254,7 @@ static struct usb_driver cdc_ncm_driver = {
>   	.disconnect = cdc_ncm_disconnect,
>   	.suspend = usbnet_suspend,
>   	.resume = usbnet_resume,
> +	.reset_resume =	usbnet_resume,
>   	.supports_autosuspend = 1,
>   };
>

Acked-by: Brad Figg <brad.figg@canonical.com>
Andy Whitcroft - June 13, 2011, 11:09 a.m.
On Mon, Jun 13, 2011 at 02:27:47AM -0700, Brad Figg wrote:
> On 06/12/2011 04:25 PM, Leann Ogasawara wrote:
> >BugLink: http://bugs.launchpad.net/bugs/793892
> >
> >SRU Justification:
> >
> >Impact: The cdc_ncm module, which provides support for the Ericsson
> >F5521gw Mobile Broadband Modem, unregisters the device after suspend
> >because the .reset_resume hook in the driver is not assigned.
> >
> >Fix: Upstream commit 85e3c65fa3a1d0542c18151

This is a short SHA1, we really should make sure they are fully expanded
else tooling will miss them easily and they can become ambigious over
time.

> >
> >Test Case: Without the fix, after resume from suspend you'll notice
> >messages similar to the following in dmesg output:
> >
> >cdc_ncm 2-1.4:1.6: no reset_resume for driver cdc_ncm?
> >cdc_ncm 2-1.4:1.7: no reset_resume for driver cdc_ncm?
> >cdc_ncm 2-1.4:1.6: usb0: unregister 'cdc_ncm' usb-0000:00:1d.0-1.4, CDC NCM
> >
> >The patch author, who is also the LP bug reporter, submitted this to
> >upstream 2.6.38.y but was unfortunately told there will be no further
> >2.6.38.y releases:
> >
> >http://marc.info/?l=linux-usb&m=130737305305181&w=2
> >
> >I've built a test kernel with the patch applied and have confirmation it
> >resolves the issue.  Please consider for Natty SRU.  Note the only
> >reason it is not a clean cherry-pick is due to the DRIVER_VERSION
> >string.
> >
> >Thanks,
> >Leann
> >
> >The following changes since commit f3fd91d6053b0fc7d37815d2830495ffe5c6b06e:
> >   Tim Gardner (1):
> >         UBUNTU: Start new release
> >
> >are available in the git repository at:
> >
> >   git://kernel.ubuntu.com/ogasawara/ubuntu-natty.git lp793892
> >
> >Stefan Metzmacher (1):
> >       usbnet/cdc_ncm: add missing .reset_resume hook
> >
> >  drivers/net/usb/cdc_ncm.c |    3 ++-
> >  1 files changed, 2 insertions(+), 1 deletions(-)
> >
> > From 9a5ebc5a7ace79a8683f6ac0d10d154d21e73b4d Mon Sep 17 00:00:00 2001
> >From: Stefan Metzmacher<metze@samba.org>
> >Date: Wed, 1 Jun 2011 02:01:41 +0000
> >Subject: [PATCH] usbnet/cdc_ncm: add missing .reset_resume hook
> >
> >BugLink: http://bugs.launchpad.net/bugs/793892
> >
> >This avoids messages like this after suspend:
> >
> >    cdc_ncm 2-1.4:1.6: no reset_resume for driver cdc_ncm?
> >    cdc_ncm 2-1.4:1.7: no reset_resume for driver cdc_ncm?
> >    cdc_ncm 2-1.4:1.6: usb0: unregister 'cdc_ncm' usb-0000:00:1d.0-1.4, CDC NCM
> >
> >This is important for the Ericsson F5521gw GSM/UMTS modem.
> >Otherwise modemmanager looses the fact that the cdc_ncm and cdc_acm devices
> >belong together.
> >
> >The cdc_ether module does the same.
> >
> >Signed-off-by: Stefan Metzmacher<metze@samba.org>
> >Signed-off-by: David S. Miller<davem@davemloft.net>
> >
> >(backport from commit 85e3c65fa3a1d0542c18151 upstream)

Same here.

> >Signed-off-by: Leann Ogasawara<leann.ogasawara@canonical.com>
> >---
> >  drivers/net/usb/cdc_ncm.c |    3 ++-
> >  1 files changed, 2 insertions(+), 1 deletions(-)
> >
> >diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
> >index 7113168..9d78fe6 100644
> >--- a/drivers/net/usb/cdc_ncm.c
> >+++ b/drivers/net/usb/cdc_ncm.c
> >@@ -54,7 +54,7 @@
> >  #include<linux/usb/usbnet.h>
> >  #include<linux/usb/cdc.h>
> >
> >-#define	DRIVER_VERSION				"7-Feb-2011"
> >+#define	DRIVER_VERSION				"01-June-2011"
> >
> >  /* CDC NCM subclass 3.2.1 */
> >  #define USB_CDC_NCM_NDP16_LENGTH_MIN		0x10
> >@@ -1254,6 +1254,7 @@ static struct usb_driver cdc_ncm_driver = {
> >  	.disconnect = cdc_ncm_disconnect,
> >  	.suspend = usbnet_suspend,
> >  	.resume = usbnet_resume,
> >+	.reset_resume =	usbnet_resume,
> >  	.supports_autosuspend = 1,
> >  };
> >
> 
> Acked-by: Brad Figg <brad.figg@canonical.com>

This resume interface seems to exist, and this pattern is common in
other simple drivers.  Therefore:

Acked-by: Andy Whitcroft <apw@canonical.com>

-apw
Leann Ogasawara - June 13, 2011, 1:34 p.m.
Applied to Natty master-next but with a fixed up fully expanded sha1
noted in the commit message.  I've also fixed up the bug report to also
reference the fully expanded sha1 as well.

Thanks,
Leann

On Mon, 2011-06-13 at 12:09 +0100, Andy Whitcroft wrote:
> On Mon, Jun 13, 2011 at 02:27:47AM -0700, Brad Figg wrote:
> > On 06/12/2011 04:25 PM, Leann Ogasawara wrote:
> > >BugLink: http://bugs.launchpad.net/bugs/793892
> > >
> > >SRU Justification:
> > >
> > >Impact: The cdc_ncm module, which provides support for the Ericsson
> > >F5521gw Mobile Broadband Modem, unregisters the device after suspend
> > >because the .reset_resume hook in the driver is not assigned.
> > >
> > >Fix: Upstream commit 85e3c65fa3a1d0542c18151
> 
> This is a short SHA1, we really should make sure they are fully expanded
> else tooling will miss them easily and they can become ambigious over
> time.
> 
> > >
> > >Test Case: Without the fix, after resume from suspend you'll notice
> > >messages similar to the following in dmesg output:
> > >
> > >cdc_ncm 2-1.4:1.6: no reset_resume for driver cdc_ncm?
> > >cdc_ncm 2-1.4:1.7: no reset_resume for driver cdc_ncm?
> > >cdc_ncm 2-1.4:1.6: usb0: unregister 'cdc_ncm' usb-0000:00:1d.0-1.4, CDC NCM
> > >
> > >The patch author, who is also the LP bug reporter, submitted this to
> > >upstream 2.6.38.y but was unfortunately told there will be no further
> > >2.6.38.y releases:
> > >
> > >http://marc.info/?l=linux-usb&m=130737305305181&w=2
> > >
> > >I've built a test kernel with the patch applied and have confirmation it
> > >resolves the issue.  Please consider for Natty SRU.  Note the only
> > >reason it is not a clean cherry-pick is due to the DRIVER_VERSION
> > >string.
> > >
> > >Thanks,
> > >Leann
> > >
> > >The following changes since commit f3fd91d6053b0fc7d37815d2830495ffe5c6b06e:
> > >   Tim Gardner (1):
> > >         UBUNTU: Start new release
> > >
> > >are available in the git repository at:
> > >
> > >   git://kernel.ubuntu.com/ogasawara/ubuntu-natty.git lp793892
> > >
> > >Stefan Metzmacher (1):
> > >       usbnet/cdc_ncm: add missing .reset_resume hook
> > >
> > >  drivers/net/usb/cdc_ncm.c |    3 ++-
> > >  1 files changed, 2 insertions(+), 1 deletions(-)
> > >
> > > From 9a5ebc5a7ace79a8683f6ac0d10d154d21e73b4d Mon Sep 17 00:00:00 2001
> > >From: Stefan Metzmacher<metze@samba.org>
> > >Date: Wed, 1 Jun 2011 02:01:41 +0000
> > >Subject: [PATCH] usbnet/cdc_ncm: add missing .reset_resume hook
> > >
> > >BugLink: http://bugs.launchpad.net/bugs/793892
> > >
> > >This avoids messages like this after suspend:
> > >
> > >    cdc_ncm 2-1.4:1.6: no reset_resume for driver cdc_ncm?
> > >    cdc_ncm 2-1.4:1.7: no reset_resume for driver cdc_ncm?
> > >    cdc_ncm 2-1.4:1.6: usb0: unregister 'cdc_ncm' usb-0000:00:1d.0-1.4, CDC NCM
> > >
> > >This is important for the Ericsson F5521gw GSM/UMTS modem.
> > >Otherwise modemmanager looses the fact that the cdc_ncm and cdc_acm devices
> > >belong together.
> > >
> > >The cdc_ether module does the same.
> > >
> > >Signed-off-by: Stefan Metzmacher<metze@samba.org>
> > >Signed-off-by: David S. Miller<davem@davemloft.net>
> > >
> > >(backport from commit 85e3c65fa3a1d0542c18151 upstream)
> 
> Same here.
> 
> > >Signed-off-by: Leann Ogasawara<leann.ogasawara@canonical.com>
> > >---
> > >  drivers/net/usb/cdc_ncm.c |    3 ++-
> > >  1 files changed, 2 insertions(+), 1 deletions(-)
> > >
> > >diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
> > >index 7113168..9d78fe6 100644
> > >--- a/drivers/net/usb/cdc_ncm.c
> > >+++ b/drivers/net/usb/cdc_ncm.c
> > >@@ -54,7 +54,7 @@
> > >  #include<linux/usb/usbnet.h>
> > >  #include<linux/usb/cdc.h>
> > >
> > >-#define	DRIVER_VERSION				"7-Feb-2011"
> > >+#define	DRIVER_VERSION				"01-June-2011"
> > >
> > >  /* CDC NCM subclass 3.2.1 */
> > >  #define USB_CDC_NCM_NDP16_LENGTH_MIN		0x10
> > >@@ -1254,6 +1254,7 @@ static struct usb_driver cdc_ncm_driver = {
> > >  	.disconnect = cdc_ncm_disconnect,
> > >  	.suspend = usbnet_suspend,
> > >  	.resume = usbnet_resume,
> > >+	.reset_resume =	usbnet_resume,
> > >  	.supports_autosuspend = 1,
> > >  };
> > >
> > 
> > Acked-by: Brad Figg <brad.figg@canonical.com>
> 
> This resume interface seems to exist, and this pattern is common in
> other simple drivers.  Therefore:
> 
> Acked-by: Andy Whitcroft <apw@canonical.com>
> 
> -apw
>

Patch

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 7113168..9d78fe6 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -54,7 +54,7 @@ 
 #include <linux/usb/usbnet.h>
 #include <linux/usb/cdc.h>
 
-#define	DRIVER_VERSION				"7-Feb-2011"
+#define	DRIVER_VERSION				"01-June-2011"
 
 /* CDC NCM subclass 3.2.1 */
 #define USB_CDC_NCM_NDP16_LENGTH_MIN		0x10
@@ -1254,6 +1254,7 @@  static struct usb_driver cdc_ncm_driver = {
 	.disconnect = cdc_ncm_disconnect,
 	.suspend = usbnet_suspend,
 	.resume = usbnet_resume,
+	.reset_resume =	usbnet_resume,
 	.supports_autosuspend = 1,
 };