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

From: Oliver Neukum <[email-masked]>
Date: Mon, 07 Jan 2019 12:02:50 +0100

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,

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.

	Regards
		Oliver