RE: [BUG] usb_dev_resume returns -113 due to work items queued by usb on pm_wq is not executed before suspending.

2014-08-19 Thread Du, ChangbinX 
 Commit d6236f6d1d88 prevents the roothub resuming work item from being
 queued.
 
  4) We force system suspending process aborted (by modifying code) just
 after the usb
Device suspend callback is invoked.
  5) Then PM core will call resume callback of the usb device to recovery. On
 this step
usb_dev_resume() invokes and it return -113 error. Because the
 can_submit flag of
root hub is 0. Root hub resuming work item is still pending.
 
 I still think that commit has already fixed your problem.
 
 Alan Stern

Yes, thanks. Issue didn't reproduce after applying that patch.
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [BUG] usb_dev_resume returns -113 due to work items queued by usb on pm_wq is not executed before suspending.

2014-08-18 Thread Du, ChangbinX 
 On Fri, 15 Aug 2014, Du, ChangbinX wrote:
  If my analysis is correct, could you share your ideas for this issue?
 
 Hasn't this already been fixed?  See commit d6236f6d1d88 (xhci: Fix runtime
 suspended xhci from blocking system suspend).
 
 Alan Stern

Hi, Stern,
These are two different issues. Commit d6236f6d1d88 fixes issue that xhci 
runtime
pm logical blocks system suspend. But the issue we are encountering is usb 
device
resuming fail and lead to device reset.

This issue can be reproduced by below steps:
1) Plug a usb device to usb3 host, and make sure this device can enter runtime
  suspend state.
2) Wait HCD entering runtime suspend state.( xhci_suspend() will be called)
3) Make system starts system suspending. On this step, PM core freeze threads 
and
  resume hcd  the usb device to runtime active state(xhci_resume() will be 
called, 
  and roothub resuming work item is queued). Then calls the suspend callbacks 
of devices'.
4) We force system suspending process aborted (by modifying code) just after 
the usb
  Device suspend callback is invoked.
5) Then PM core will call resume callback of the usb device to recovery. On 
this step
  usb_dev_resume() invokes and it return -113 error. Because the can_submit 
flag of
  root hub is 0. Root hub resuming work item is still pending.

The sequence is:
xhci_suspend()  ---  host runtime 
suspend
PM: Syncing filesystems ... done.
PM: Preparing system for mem sleep
Freezing user space processes
xhci_resume()   --- resume to runtime 
active, work item queued , 
   but 
the roothub is not resumed(can_submit=0).
PM: Entering mem sleep
... --- 
invoke device suspend callbacks
1-1:1.0: device suspending
...
PM: Some devices failed to suspend  --- suspending process aborted 
and starts recovery
--- 
call resuming callbacks for suspended devices
1-1:1.0: usb_dev_resume() - usb_resume - 
  usb_resume_both()-generic_resume()-
  usb_port_resume()-hub_port_status()-
  usb_control_msg()-usb_submit_urb()-
  usb_hcd_link_urb_to_ep()

int usb_hcd_link_urb_to_ep(struct usb_hcd *hcd, struct urb *urb)
{
...
if (unlikely(!urb-dev-can_submit)) {
rc = -EHOSTUNREACH;   // -113
goto done;
}
...
}

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

[BUG] usb_dev_resume returns -113 due to work items queued by usb on pm_wq is not executed before suspending.

2014-08-14 Thread Du, ChangbinX 
Hi, All,
As described in runtime_pm.txt for pm_wq:
The power management work queue pm_wq in which bus types and device
drivers can put their PM-related work items. It is strongly recommended that
pm_wq be used for queuing all work items related to runtime PM, because
this allows them to be synchronized with system-wide power transitions
(suspend to RAM, hibernation and resume from system sleep states).

Per my understanding, all runtime PM related works items queued on pm_wq
should be completed before suspend to RAM. So to ensure device state is
runtime active before suspending. Having checked the pm code, I found this is
not true for work items queued by drivers.

Now usb driver has used this pm_wq to run a work item that resumes root hub
(see function xhci_resume()-usb_hcd_resume_root_hub()). But sometimes
this work is not completed before usb device suspend. That is to say root hub
device may still in runtime suspend state before suspending. And this can result
in problem. One case is that as below error log shows,

[  108.046248] PM: Entering mem sleep
[  108.050487] Suspending console(s) (use no_console_suspend to debug)
[  108.426510] active wakeup source: event5-576
[  108.426529] PM: Some devices failed to suspend
[  108.426887] dpm_run_callback(): usb_dev_resume+0x0/0x20 returns -113
[  108.426918] PM: Device 1-2 failed to resume async: error -113
[  108.428299] PM: resume of devices complete after 1.755 msecs

The usb_dev_resume() return error -113, which mean host is in suspend state
when resuming a device. The scenario is:
1) Just before system suspending, pm core will run hcd runtime resume
  routine if host is in runtime suspend state.
2) Hcd runtime resume function xhci_resume() returns, and roothub resume
  worker was queued by usb_hcd_resume_root_hub().
3) system suspend continue going before roothub resume worker starts executing.
  Thus host is still in runtime suspend state.
4) One event make suspending process aborted before hcd suspended. Then pm
  core will call resume routines for just suspended device. But when resuming
  a usb device it find the host is in suspended. Then return error -113.

If my analysis is correct, could you share your ideas for this issue?

Regards and Thanks!
Du, Changbin

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

RE: [PATCH] usb/core: fix NULL pointer dereference in recursively_mark_NOTATTACHED

2014-01-07 Thread Du, ChangbinX 
  Changbin, after looking more closely I realized there was a second
  aspect to this race: recursively_mark_NOTATTACHED uses hub-ports[i]
  while hub_disconnect removes the port devices.  You ought to be able
  to cause an oops by inserting a delay just after the loop where
  usb_hub_remove_port_device is called.
 
  The updated patch below should fix both problems.  Can you test it?
 
  Alan Stern
 
 
 Ok, I'll test it today or tomorrow. Please wait my response.

Alan, I cannot cause a panic after inserting a delay just after 
usb_hub_remove_port_device is called, even move the delay after 
kfree(hub-ports). recursively_mark_NOTATTACHED will not access 
hub-ports[i] since maxchild has been set to 0.

Alan, I think your last patch can fix this issue. 
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH] usb/core: fix NULL pointer dereference in recursively_mark_NOTATTACHED

2014-01-02 Thread Du, ChangbinX 
 On Thu, 26 Dec 2013, Du, ChangbinX wrote:
 
  I can reproduce issue by adding a delay just after
  usb_set_intfdata(intf, NULL)  (echo -1  bConfigurationValue to trigger
 hub_dissconnect())without your patch.
 
  After patch applied, cannot reproduce and didn't found any other issue.
 Patch works well.
 
  Alan, need I update patch to v2 or you will do it?
 
 Changbin, after looking more closely I realized there was a second aspect to
 this race: recursively_mark_NOTATTACHED uses hub-ports[i] while
 hub_disconnect removes the port devices.  You ought to be able to cause
 an oops by inserting a delay just after the loop where
 usb_hub_remove_port_device is called.
 
 The updated patch below should fix both problems.  Can you test it?
 
 Alan Stern
 

Ok, I'll test it today or tomorrow. Please wait my response.
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH] usb/core: fix NULL pointer dereference in recursively_mark_NOTATTACHED

2013-12-25 Thread Du, ChangbinX 
 On Tue, 24 Dec 2013, Alan Stern wrote:
 I think this will fix it.  Take a close look and do some careful testing.
 
 Alan Stern
 
 Index: usb-3.13/drivers/usb/core/hub.c
 ==
 =
 --- usb-3.13.orig/drivers/usb/core/hub.c
 +++ usb-3.13/drivers/usb/core/hub.c
 @@ -1622,11 +1622,14 @@ static void hub_disconnect(struct usb_in
   hub-error = 0;
   hub_quiesce(hub, HUB_DISCONNECT);
 
 - usb_set_intfdata (intf, NULL);
 -
   for (i = 0; i  hdev-maxchild; i++)
   usb_hub_remove_port_device(hub, i + 1);
 +
 + /* Avoid races with recursively_mark_NOTATTACHED() */
 + spin_lock_irq(device_state_lock);
   hub-hdev-maxchild = 0;
 + usb_set_intfdata(intf, NULL);
 + spin_unlock_irq(device_state_lock);
 
   if (hub-hdev-speed == USB_SPEED_HIGH)
   highspeed_hubs--;
 

Sorry for late response. Agree with you. I will test your patch carefully and 
let you know the result. Thanks!

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

RE: [PATCH] usb/core: fix NULL pointer dereference in recursively_mark_NOTATTACHED

2013-12-24 Thread Du, ChangbinX 
 From: Alan Stern [mailto:st...@rowland.harvard.edu]
 Sent: Monday, December 23, 2013 11:13 PM
 To: Du, ChangbinX
 Cc: gre...@linuxfoundation.org; sarah.a.sh...@linux.intel.com; Lan, Tianyu;
 burzalod...@gmail.com; linux-usb@vger.kernel.org;
 linux-ker...@vger.kernel.org
 Subject: Re: [PATCH] usb/core: fix NULL pointer dereference in
 recursively_mark_NOTATTACHED
 
 On Mon, 23 Dec 2013, Du, ChangbinX wrote:
 
  usb_hub_to_struct_hub() can return NULL if the hub without active
  configuration. So the result must be checked.
 
  BUG: unable to handle kernel NULL pointer dereference at 015c

 How did you manage to trigger this BUG?  If hub is NULL then
 udev-maxchild should be 0.  See the code in hub_disconnect().

 Alan Stern

Hello, Alan. The hub also should be null if actconfig is null. You can see it 
in function
usb_hub_to_struct_hub().
udev-maxchild will be set to 0 in hub_disconnect(). But before that,
recursively_mark_NOTATTACHED may be called when calling usb_disconnect(). So 
this issue
will happen when usb_disconnect a hub that not have a configuration yet.
It happened once here when unplugging otg cable from DUT(will cause hcd 
removed) with
tiers of hub and devices. But it's not easy to reproduce it.
This is my analysis, how do you think?

Du, Changbin
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] usb/core: fix NULL pointer dereference in recursively_mark_NOTATTACHED

2013-12-23 Thread Du, ChangbinX 
From ee48b3736d437af79d4fe858cdf64f241c76c339 Mon Sep 17 00:00:00 2001
From: Du, Changbin changbinx...@intel.com
Date: Wed, 18 Dec 2013 16:47:21 +0800
Subject: [PATCH] usb/core: fix NULL pointer dereference in
 recursively_mark_NOTATTACHED

usb_hub_to_struct_hub() can return NULL if the hub without active
configuration. So the result must be checked.

BUG: unable to handle kernel NULL pointer dereference at 015c
IP: [c16d5fb0] recursively_mark_NOTATTACHED+0x20/0x60
Call Trace:
 [c16d5fc4] recursively_mark_NOTATTACHED+0x34/0x60
 [c16d5fc4] recursively_mark_NOTATTACHED+0x34/0x60
 [c16d5fc4] recursively_mark_NOTATTACHED+0x34/0x60
 [c16d5fc4] recursively_mark_NOTATTACHED+0x34/0x60
 [c16d6082] usb_set_device_state+0x92/0x120
 [c16d862b] usb_disconnect+0x2b/0x1a0
 [c16dd4c0] usb_remove_hcd+0xb0/0x160
 [c19ca846] ? _raw_spin_unlock_irqrestore+0x26/0x50
 [c1704efc] ehci_mid_remove+0x1c/0x30
 [c1704f26] ehci_mid_stop_host+0x16/0x30
 [c16f7698] penwell_otg_work+0xd28/0x3520
 [c19c945b] ? __schedule+0x39b/0x7f0
 [c19cdb9d] ? sub_preempt_count+0x3d/0x50
 [c125e97d] process_one_work+0x11d/0x3d0
 [c19c7f4d] ? mutex_unlock+0xd/0x10
 [c125e0e5] ? manage_workers.isra.24+0x1b5/0x270
 [c125f009] worker_thread+0xf9/0x320
 [c19ca846] ? _raw_spin_unlock_irqrestore+0x26/0x50
 [c125ef10] ? rescuer_thread+0x2b0/0x2b0
 [c1264ac4] kthread+0x94/0xa0
 [c19d0f77] ret_from_kernel_thread+0x1b/0x28
 [c1264a30] ? kthread_create_on_node+0xc0/0xc0

Signed-off-by: Du, Changbin changbinx...@intel.com
---
 drivers/usb/core/hub.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index bd9dc35..01a1fe6 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -1870,9 +1870,11 @@ static void recursively_mark_NOTATTACHED(struct 
usb_device *udev)
struct usb_hub *hub = usb_hub_to_struct_hub(udev);
int i;
 
-   for (i = 0; i  udev-maxchild; ++i) {
-   if (hub-ports[i]-child)
-   recursively_mark_NOTATTACHED(hub-ports[i]-child);
+   if (hub) {
+   for (i = 0; i  udev-maxchild; ++i) {
+   if (hub-ports[i]-child)
+   
recursively_mark_NOTATTACHED(hub-ports[i]-child);
+   }
}
if (udev-state == USB_STATE_SUSPENDED)
udev-active_duration -= jiffies;
-- 
1.8.3.2

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

[PATCH] usb/gadget: should use u16 type variable to store MaxPower

2013-12-17 Thread Du, ChangbinX 
From 7e827a0d300e084f74c65122baa5e3193f9a7f18 Mon Sep 17 00:00:00 2001
From: Du, Changbin changbinx...@intel.com
Date: Mon, 16 Dec 2013 20:32:13 +0800
Subject: [PATCH] usb/gadget: should use u16 type variable to store MaxPower

The MaxPower field is of u16 type. So using u8 type variable can break
data (high byte lost).

Signed-off-by: Du, Changbin changbinx...@intel.com
---
 drivers/usb/gadget/composite.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index 2018ba1..8a34173 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -1714,7 +1714,7 @@ composite_resume(struct usb_gadget *gadget)
 {
struct usb_composite_dev*cdev = get_gadget_data(gadget);
struct usb_function *f;
-   u8  maxpower;
+   u16 maxpower;
 
/* REVISIT:  should we have config level
 * suspend/resume callbacks?
-- 
1.8.3.2
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH] net/cdc_ncm: fix null pointer panic at usbnet_link_change

2013-10-30 Thread Du, ChangbinX 
 From: Bjørn Mork [mailto:bj...@mork.no]
 Sent: Tuesday, October 29, 2013 4:41 PM
 To: Du, ChangbinX
 Cc: oli...@neukum.org; linux-usb@vger.kernel.org; net...@vger.kernel.org;
 linux-ker...@vger.kernel.org
 Subject: Re: [PATCH] net/cdc_ncm: fix null pointer panic at usbnet_link_change
 
 Du, ChangbinX changbinx...@intel.com writes:
 
  From: Du, Changbin changbinx...@intel.com
 
  In cdc_ncm_bind() function, it call cdc_ncm_bind_common() to setup usb.
  But cdc_ncm_bind_common() may meet error and cause usbnet_disconnect()
  be called which calls free_netdev(net).
 
 I am sure you are right, but I really don't see how that can happen.
 
 AFAICS, we ensure that the intfdata is set to NULL before calling
 usb_driver_release_interface() in the error cleanup in
 cdc_ncm_bind_common():
 
 
 error2:
   usb_set_intfdata(ctx-control, NULL);
   usb_set_intfdata(ctx-data, NULL);
   if (ctx-data != ctx-control)
   usb_driver_release_interface(driver, ctx-data);
 error:
   cdc_ncm_free((struct cdc_ncm_ctx *)dev-data[0]);
   dev-data[0] = 0;
   dev_info(dev-udev-dev, bind() failure\n);
   return -ENODEV;
 
 
 Thus we hit the test in disconnect, and usbnet_disconnect() is never
 called:
 
 static void cdc_ncm_disconnect(struct usb_interface *intf)
 {
   struct usbnet *dev = usb_get_intfdata(intf);
 
   if (dev == NULL)
   return; /* already disconnected */
 
   usbnet_disconnect(intf);
 }
 
 So we should really be OK, but we are not  I would appreciate it if
 anyone took the time to feed me why, with a very small spoon...
 

Yes, you are right. It should not happen if cdc_ncm_disconnect is not called. 
But this really happened.
It produced on kernel 3.10. Here is the full panic message for you to refer. I 
will get a method try to 
reproduce it.

[   15.635727] BUG: Bad page state in process khubd  pfn:31994
[   15.641989] page:f3ad9280 count:0 mapcount:0 mapping:0020 index:0x0
[   15.649384] page flags: 0x4000()
[   15.670096] BUG: unable to handle kernel NULL pointer dereference at 0078
[   15.678078] IP: [c24b7f5e] usbnet_link_change+0x1e/0x80
[   15.684120] *pde =  
[   15.687339] Oops:  [#1] SMP 
[   15.690953] Modules linked in: atmel_mxt_ts vxd392 videobuf_vmalloc 
videobuf_core bcm_bt_lpm bcm432)
[   15.702658] CPU: 0 PID: 573 Comm: khubd Tainted: GB   W  O 3.10.1+ #1
[   15.710241] task: f27e8f30 ti: f2084000 task.ti: f2084000
[   15.716270] EIP: 0060:[c24b7f5e] EFLAGS: 00010246 CPU: 0
[   15.722396] EIP is at usbnet_link_change+0x1e/0x80
[   15.727747] EAX: f18524c0 EBX:  ECX: f18524c0 EDX: 
[   15.734746] ESI: f18524c0 EDI:  EBP: f2085b7c ESP: f2085b70
[   15.741746] DS: 007b ES: 007b FS: 00d8 GS:  SS: 0068
[   15.747775] CR0: 8005003b CR2: 0078 CR3: 02c3b000 CR4: 001007d0
[   15.754774] DR0:  DR1:  DR2:  DR3: 
[   15.761774] DR6: 0ff0 DR7: 0400
[   15.766054] Stack:
[   15.768295]   f18524c0 c2a03818 f2085b8c c24bc69a f1852000 f1f52e00 
f2085be0
[   15.776991]  c24b8d32  0001  c2167f2c f2085bb4 c2821253 
f2085bec
[   15.785687]  0246 0246 f18d8088 f1f52e1c f1fce464 f1fce400 f18524c0 
c28a06e0
[   15.794376] Call Trace:
[   15.797109]  [c24bc69a] cdc_ncm_bind+0x3a/0x50
[   15.802267]  [c24b8d32] usbnet_probe+0x282/0x7d0
[   15.807621]  [c2167f2c] ? sysfs_new_dirent+0x6c/0x100
[   15.813460]  [c2821253] ? mutex_lock+0x13/0x40
[   15.818618]  [c24bb278] cdc_ncm_probe+0x8/0x10
[   15.823777]  [c24e0ef7] usb_probe_interface+0x187/0x2c0
[   15.829811]  [c23caa8a] ? driver_sysfs_add+0x6a/0x90
[   15.835550]  [c23cb290] ? __driver_attach+0x90/0x90
[   15.841192]  [c23caf14] driver_probe_device+0x74/0x360
[   15.847127]  [c24e07b1] ? usb_match_id+0x41/0x60
[   15.852479]  [c24e081e] ? usb_device_match+0x4e/0x90
[   15.858219]  [c23cb290] ? __driver_attach+0x90/0x90
[   15.863861]  [c23cb2c9] __device_attach+0x39/0x50
[   15.869311]  [c23c93f4] bus_for_each_drv+0x34/0x70
[   15.874856]  [c23cae2b] device_attach+0x7b/0x90
[   15.880109]  [c23cb290] ? __driver_attach+0x90/0x90
[   15.885752]  [c23ca38f] bus_probe_device+0x6f/0x90
[   15.891298]  [c23c8a08] device_add+0x558/0x630
[   15.896457]  [c24e4821] ? usb_create_ep_devs+0x71/0xd0
[   15.902391]  [c24dd0db] ? create_intf_ep_devs+0x4b/0x70
[   15.908422]  [c24df2bf] usb_set_configuration+0x4bf/0x800
[   15.914648]  [c23cb290] ? __driver_attach+0x90/0x90
[   15.920292]  [c24e809b] generic_probe+0x2b/0x90
[   15.925546]  [c24e105c] usb_probe_device+0x2c/0x70
[   15.931091]  [c23caf14] driver_probe_device+0x74/0x360
[   15.943345]  [c2272102] ? kobject_uevent_env+0x182/0x620
[   15.949473]  [c2272102] ? kobject_uevent_env+0x182/0x620
[   15.955601]  [c23cb290] ? __driver_attach+0x90/0x90
[   15.961242]  [c23cb2c9] __device_attach+0x39/0x50
[   15.966692]  [c23c93f4] bus_for_each_drv+0x34/0x70
[   15.972238]  [c23cae2b] device_attach+0x7b/0x90
[   15.977492

[PATCH] net/cdc_ncm: fix null pointer panic at usbnet_link_change

2013-10-28 Thread Du, ChangbinX 
From: Du, Changbin changbinx...@intel.com

In cdc_ncm_bind() function, it call cdc_ncm_bind_common() to setup usb.
But cdc_ncm_bind_common() may meet error and cause usbnet_disconnect()
be called which calls free_netdev(net). Thus usbnet structure(alloced
with net_device structure) will be freed,too.
So we cannot call usbnet_link_change() if cdc_ncm_bind_common() return
error.

BUG: unable to handle kernel NULL pointer dereference at 0078
EIP is at usbnet_link_change+0x1e/0x80
Call Trace:
 [c24bc69a] cdc_ncm_bind+0x3a/0x50
 [c24b8d32] usbnet_probe+0x282/0x7d0
 [c2167f2c] ? sysfs_new_dirent+0x6c/0x100
 [c2821253] ? mutex_lock+0x13/0x40
 [c24bb278] cdc_ncm_probe+0x8/0x10
 [c24e0ef7] usb_probe_interface+0x187/0x2c0
 [c23caa8a] ? driver_sysfs_add+0x6a/0x90
 [c23cb290] ? __driver_attach+0x90/0x90
 [c23caf14] driver_probe_device+0x74/0x360
 [c24e07b1] ? usb_match_id+0x41/0x60
 [c24e081e] ? usb_device_match+0x4e/0x90
 [c23cb290] ? __driver_attach+0x90/0x90
 [c23cb2c9] __device_attach+0x39/0x50
 [c23c93f4] bus_for_each_drv+0x34/0x70
 [c23cae2b] device_attach+0x7b/0x90
 [c23cb290] ? __driver_attach+0x90/0x90
 [c23ca38f] bus_probe_device+0x6f/0x90
 [c23c8a08] device_add+0x558/0x630
 [c24e4821] ? usb_create_ep_devs+0x71/0xd0
 [c24dd0db] ? create_intf_ep_devs+0x4b/0x70
 [c24df2bf] usb_set_configuration+0x4bf/0x800
 [c23cb290] ? __driver_attach+0x90/0x90
 [c24e809b] generic_probe+0x2b/0x90
 [c24e105c] usb_probe_device+0x2c/0x70
 [c23caf14] driver_probe_device+0x74/0x360

Signed-off-by: Du, Changbin changbinx...@intel.com
---
 drivers/net/usb/cdc_ncm.c | 17 +
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 43afde8..af37ecf 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -603,14 +603,15 @@ static int cdc_ncm_bind(struct usbnet *dev, struct 
usb_interface *intf)
 
/* NCM data altsetting is always 1 */
ret = cdc_ncm_bind_common(dev, intf, 1);
-
-   /*
-* We should get an event when network connection is connected or
-* disconnected. Set network connection in disconnected state
-* (carrier is OFF) during attach, so the IP network stack does not
-* start IPv6 negotiation and more.
-*/
-   usbnet_link_change(dev, 0, 0);
+   if (!ret) {
+   /*
+* We should get an event when network connection is connected
+* or disconnected. Set network connection in disconnected
+* state (carrier is OFF) during attach, so the IP network stack
+* does not start IPv6 negotiation and more.
+*/
+   usbnet_link_change(dev, 0, 0);
+   }
return ret;
 }
 
-- 
1.8.3.2
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: About TRB_TO_EP_INDEX() macro using

2013-08-06 Thread Du, ChangbinX 
 On Mon, Aug 05, 2013 at 08:07:25AM +, Du, ChangbinX wrote:
  Recently when I check xHCI code, find that some functions try to get EP 
  index
  from a Command Completion Event TRB via TRB_TO_EP_INDEX() macro.
 
  This is totally wrong. The macro definition is:
 
  #define TRB_TO_EP_INDEX(p)  p)  (0x1f  16))  16) - 
  1)
 
  TRB_TO_EP_INDEX only applies to Transfer Event TRB. But for Command
  Completion Event TRB, there is no Endpoint ID field. So, we cannot directly
  get EP index from these TRBs, but we can find it by the TRB Pointer.
 
  Here list two functions for you to check:
  handle_stopped_endpoint()
  handle_reset_ep_completion()
 
 Care to send a patch showing how you would change this if it is
 incorrect?
 
 thanks,
 
 greg k-h
 --

Hello, Greg k-h. I am not very sure about this issue. If this is true, kernel 
will panic when 
invoking above functions. I want someone help to confirm if I miss something. 
If it's really 
a bug, I will work out a patch to fix it.
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

About TRB_TO_EP_INDEX() macro using

2013-08-05 Thread Du, ChangbinX 
Recently when I check xHCI code, find that some functions try to get EP index 
from a Command Completion Event TRB via TRB_TO_EP_INDEX() macro.

This is totally wrong. The macro definition is:

#define TRB_TO_EP_INDEX(p)  p)  (0x1f  16))  16) - 
1)

TRB_TO_EP_INDEX only applies to Transfer Event TRB. But for Command 
Completion Event TRB, there is no Endpoint ID field. So, we cannot directly 
get EP index from these TRBs, but we can find it by the TRB Pointer.

Here list two functions for you to check:
handle_stopped_endpoint()
handle_reset_ep_completion()

Regards  Thanks!
Changbin


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