Re: [Intel-wired-lan] [PATCH] e1000e: Undo e1000e_pm_freeze if __e1000_shutdown fails

On Mon, Feb 20, 2017 at 12:43:11PM +0000, Ruinskiy, Dima wrote:

quoted

-----Original Message-----

quoted

Fixes: 2800209994f8 ("e1000e: Refactor PM flows")
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99847
Signed-off-by: Chris Wilson <redacted>
Cc: Tvrtko Ursulin <redacted>
Cc: Jeff Kirsher <redacted>
Cc: Dave Ertman <redacted>
Cc: Bruce Allan <redacted>
Cc: intel-wired-lan@lists.osuosl.org
Cc: netdev@vger.kernel.org
---
drivers/net/ethernet/intel/e1000e/netdev.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c

b/drivers/net/ethernet/intel/e1000e/netdev.c
index eccf1da9356b..429a5210230d 100644

--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c

@@ -6615,12 +6615,19 @@ static int e1000e_pm_thaw(struct device *dev)

static int e1000e_pm_suspend(struct device *dev)  {
	struct pci_dev *pdev = to_pci_dev(dev);
+	int rc;

	e1000e_flush_lpic(pdev);

	e1000e_pm_freeze(dev);

-	return __e1000_shutdown(pdev, false);
+	rc = __e1000_shutdown(pdev, false);
+	if (rc) {
+		e1000e_pm_thaw(dev);
+		return rc;
+	}
+
+	return 0;
}

static int e1000e_pm_resume(struct device *dev)
--

Looks reasonable. However, can't you get the same result with fewer code lines?
-	return __e1000_shutdown(pdev, false);
+	rc = __e1000_shutdown(pdev, false);
+	if (rc)
+		e1000e_pm_thaw(dev);
+
+	return rc;

You are welcome to use whatever style is consistent with the rest of the
driver. :)
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre