[PATCH net] net: qmi_wwan: call subdriver with control intf only

[PATCH net] net: qmi_wwan: call subdriver with control intf only

[Bjørn Mork]

This fixes a hang on suspend due to calling wdm_suspend on
the unregistered data interface. The hang should have been
a NULL pointer reference had it not been for a logic error
in the cdc_wdm code.

  commit 230718bd net: qmi_wwan: bind to both control and data interface

changed qmi_wwan to use cdc_wdm as a subdriver for devices with
a two-interface QMI/wwan function.  The commit failed to update
qmi_wwan_suspend and qmi_wwan_resume, which were written to handle
either a single combined interface function, or no subdriver at all.

The result was that we called into the subdriver both when the
control interface was suspended and when the data interface was
suspended.  Calling the subdriver suspend function with an
unregistered interface is not supported and will make the
subdriver bug out.

Signed-off-by: Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org>
---
 drivers/net/usb/qmi_wwan.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index adfab3f..9d1679f 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -297,7 +297,7 @@ static int qmi_wwan_suspend(struct usb_interface *intf, pm_message_t message)
 	if (ret < 0)
 		goto err;
 
-	if (info->subdriver && info->subdriver->suspend)
+	if (intf == info->control && info->subdriver && info->subdriver->suspend)
 		ret = info->subdriver->suspend(intf, message);
 	if (ret < 0)
 		usbnet_resume(intf);
@@ -311,7 +311,7 @@ static int qmi_wwan_resume(struct usb_interface *intf)
 	struct qmi_wwan_state *info = (void *)&dev->data;
 	int ret = 0;
 
-	if (info->subdriver && info->subdriver->resume)
+	if (intf == info->control && info->subdriver && info->subdriver->resume)
 		ret = info->subdriver->resume(intf);
 	if (ret < 0)
 		goto err;
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH net v2] net: qmi_wwan: call subdriver with control intf only

[Bjørn Mork]

This fixes a hang on suspend due to calling wdm_suspend on
the unregistered data interface. The hang should have been
a NULL pointer reference had it not been for a logic error
in the cdc_wdm code.

  commit 230718bd net: qmi_wwan: bind to both control and data interface

changed qmi_wwan to use cdc_wdm as a subdriver for devices with
a two-interface QMI/wwan function.  The commit failed to update
qmi_wwan_suspend and qmi_wwan_resume, which were written to handle
either a single combined interface function, or no subdriver at all.

The result was that we called into the subdriver both when the
control interface was suspended and when the data interface was
suspended.  Calling the subdriver suspend function with an
unregistered interface is not supported and will make the
subdriver bug out.

Signed-off-by: Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org>
---
v2: fix resume error path as well


 drivers/net/usb/qmi_wwan.c |    7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index 17b8f3e..b1ba68f 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -297,7 +297,7 @@ static int qmi_wwan_suspend(struct usb_interface *intf, pm_message_t message)
 	if (ret < 0)
 		goto err;
 
-	if (info->subdriver && info->subdriver->suspend)
+	if (intf == info->control && info->subdriver && info->subdriver->suspend)
 		ret = info->subdriver->suspend(intf, message);
 	if (ret < 0)
 		usbnet_resume(intf);
@@ -310,13 +310,14 @@ static int qmi_wwan_resume(struct usb_interface *intf)
 	struct usbnet *dev = usb_get_intfdata(intf);
 	struct qmi_wwan_state *info = (void *)&dev->data;
 	int ret = 0;
+	bool callsub = (intf == info->control && info->subdriver && info->subdriver->resume);
 
-	if (info->subdriver && info->subdriver->resume)
+	if (callsub)
 		ret = info->subdriver->resume(intf);
 	if (ret < 0)
 		goto err;
 	ret = usbnet_resume(intf);
-	if (ret < 0 && info->subdriver && info->subdriver->resume && info->subdriver->suspend)
+	if (ret < 0 && callsub && info->subdriver->suspend)
 		info->subdriver->suspend(intf, PMSG_SUSPEND);
 err:
 	return ret;
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH net v2] net: qmi_wwan: call subdriver with control intf only

[David Miller]

From: Bjørn Mork <bjorn@mork.no>
Date: Thu, 13 Sep 2012 08:44:35 +0200

> This fixes a hang on suspend due to calling wdm_suspend on
> the unregistered data interface. The hang should have been
> a NULL pointer reference had it not been for a logic error
> in the cdc_wdm code.
> 
>   commit 230718bd net: qmi_wwan: bind to both control and data interface
> 
> changed qmi_wwan to use cdc_wdm as a subdriver for devices with
> a two-interface QMI/wwan function.  The commit failed to update
> qmi_wwan_suspend and qmi_wwan_resume, which were written to handle
> either a single combined interface function, or no subdriver at all.
> 
> The result was that we called into the subdriver both when the
> control interface was suspended and when the data interface was
> suspended.  Calling the subdriver suspend function with an
> unregistered interface is not supported and will make the
> subdriver bug out.
> 
> Signed-off-by: Bjørn Mork <bjorn@mork.no>

Applied, thanks.

Re: [PATCH net] net: qmi_wwan: call subdriver with control intf only

[David Miller]

From: Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org>
Date: Mon, 10 Sep 2012 22:15:50 +0200

> This fixes a hang on suspend due to calling wdm_suspend on
> the unregistered data interface. The hang should have been
> a NULL pointer reference had it not been for a logic error
> in the cdc_wdm code.
> 
>   commit 230718bd net: qmi_wwan: bind to both control and data interface
> 
> changed qmi_wwan to use cdc_wdm as a subdriver for devices with
> a two-interface QMI/wwan function.  The commit failed to update
> qmi_wwan_suspend and qmi_wwan_resume, which were written to handle
> either a single combined interface function, or no subdriver at all.
> 
> The result was that we called into the subdriver both when the
> control interface was suspended and when the data interface was
> suspended.  Calling the subdriver suspend function with an
> unregistered interface is not supported and will make the
> subdriver bug out.
> 
> Signed-off-by: Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org>

Applied, thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html