Ming Lei <ming.lei@xxxxxxxxxxxxx> writes: > Hi, > > This patch adds comments on interface driver suspend callback > to emphasize that the failure return value is ignored by > USB core in system sleep context, so do not try to recover > device for this case, otherwise the URB traffic scheduled > in recovery of failure path may cross system sleep, and may > cause problems. Well, an unexpected error did happen so problems are to be expected, yes. > Also fixes the USB serial, HID and several usbnet drivers > which may recover device in suspend failure path of system sleep. I believe all of these are wrong unless you have any real bug which is fixed by this. All these drivers suspend in multiple steps, where each step can fail. If a later step fails then they revert any previously successful step before returning the failure, thereby ensuring that the device/driver state when suspend returns is consistently either suspended or resumed. The error recovery they do in suspend is not about preventing suspend at all. It is about ensuring that that the driver and device is in a consistent state, which is "resumed" if suspend fails. Your patch set make the drivers return from suspend in some intermediate state, where the device and/or driver is neither suspended nor resumed. This is wrong. You still did not necessarily kill all URBs, but you killed some of them. What is resume() going to do then? I am going to NAK the cdc_mbim and qmi_wwan pacthes unless you can convince me that we need to add a "partly-suspended" state for the system suspend error case. In which case the patch will need to include the corresponding resume fix for the "partly-suspended" state. Bjørn -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
