Re: [PATCH 02/19] usbnet: smsc95xx: Stop propagation of in_pm

On 1/7/19 12:02 PM, Oliver Neukum wrote:

On Do, 2019-01-03 at 02:10 +0100, Marek Vasut wrote:

quoted

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