On 1/7/19 12:02 PM, Oliver Neukum wrote: > On Do, 2019-01-03 at 02:10 +0100, Marek Vasut wrote: >> The information whether the SMSC95xx is in_pm or not can be derived from >> the usbdev->suspend_count. First thing called in smsc95xx_suspend() is >> usbnet_suspend(), which increments the usbdev->suspend_count and since >> then the driver only calls _nopm() functions and functions with in_pm >> set to 1. The smsc95xx_resume() does the inverse, it calls functions >> with _nopm() suffix and with in_pm set to 1 until usbnet_resume(), which >> sets the usbdev->suspend_count to 0. > > Hi, Hello Oliver, > unfortunately I am forced to disagree in the strongest terms. > > The logic behind this patch is wrong. The "in_pm" parameter > exists because some function need to be called in the code paths > needed to implement power management. Under those circumstances > they must not take a pm reference to keep the device awake, lest > the driver deadlock. > (For example __smsc95xx_read_reg) > > However from other code paths precisely that reference must be taken > in order to make sure that the driver do not try to communicate with > a suspended device. > If you check the suspend counter, you will omit the necessary getting > of a reference precisely when it is needed. The driver will never > dedalock, but you will cause numerous races. > > I must suggest to simply drop this part and redo the series. OK, let's drop the whole series. -- Best regards, Marek Vasut
