i2c: designware: Lock the adapter while setting the suspended flag

Lock the adapter while setting the suspended flag, to ensure that other
locked code always sees the change immediately, rather then possibly using
a stale value.

This involves splitting the suspend/resume callbacks into separate runtime
and normal suspend/resume calls. This is necessary because i2c_dw_xfer()
will get called by the i2c-core with the adapter locked and it in turn
calls the runtime-resume callback through pm_runtime_get_sync().

So the runtime versions of the suspend/resume callbacks cannot take
the adapter-lock. Note this patch simply makes the runtime suspend/resume
callbacks not deal with the suspended flag at all. During runtime the
pm_runtime_get_sync() from i2c_dw_xfer() will always ensure that the
adapter is resumed when necessary.

The suspended flag check is only necessary to check proper suspend/resume
ordering during normal suspend/resume which makes the pm_runtime_get_sync()
call a no-op.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Acked-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Signed-off-by: Wolfram Sang <wsa@kernel.org>

2 files changed, 52 insertions, 10 deletions

diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c b/drivers/i2c/busses/i2c-designware-pcidrv.c
index 2782dddfb087..a736a2aaa66e 100644
--- a/drivers/i2c/busses/i2c-designware-pcidrv.c
+++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
@@ -194,14 +194,30 @@ static struct dw_pci_controller dw_pci_controllers[] = {
 	},
 };
 
+static int __maybe_unused i2c_dw_pci_runtime_suspend(struct device *dev)
+{
+	struct dw_i2c_dev *i_dev = dev_get_drvdata(dev);
+
+	i_dev->disable(i_dev);
+	return 0;
+}
+
 static int __maybe_unused i2c_dw_pci_suspend(struct device *dev)
 {
 	struct dw_i2c_dev *i_dev = dev_get_drvdata(dev);
 
+	i2c_lock_bus(&i_dev->adapter, I2C_LOCK_ROOT_ADAPTER);
 	i_dev->suspended = true;
-	i_dev->disable(i_dev);
+	i2c_unlock_bus(&i_dev->adapter, I2C_LOCK_ROOT_ADAPTER);
 
-	return 0;
+	return i2c_dw_pci_runtime_suspend(dev);
+}
+
+static int __maybe_unused i2c_dw_pci_runtime_resume(struct device *dev)
+{
+	struct dw_i2c_dev *i_dev = dev_get_drvdata(dev);
+
+	return i_dev->init(i_dev);
 }
 
 static int __maybe_unused i2c_dw_pci_resume(struct device *dev)
@@ -209,14 +225,19 @@ static int __maybe_unused i2c_dw_pci_resume(struct device *dev)
 	struct dw_i2c_dev *i_dev = dev_get_drvdata(dev);
 	int ret;
 
-	ret = i_dev->init(i_dev);
+	ret = i2c_dw_pci_runtime_resume(dev);
+
+	i2c_lock_bus(&i_dev->adapter, I2C_LOCK_ROOT_ADAPTER);
 	i_dev->suspended = false;
+	i2c_unlock_bus(&i_dev->adapter, I2C_LOCK_ROOT_ADAPTER);
 
 	return ret;
 }
 
-static UNIVERSAL_DEV_PM_OPS(i2c_dw_pm_ops, i2c_dw_pci_suspend,
-			    i2c_dw_pci_resume, NULL);
+static const struct dev_pm_ops i2c_dw_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(i2c_dw_pci_suspend, i2c_dw_pci_resume)
+	SET_RUNTIME_PM_OPS(i2c_dw_pci_runtime_suspend, i2c_dw_pci_runtime_resume, NULL)
+};
 
 static int i2c_dw_pci_probe(struct pci_dev *pdev,
 			    const struct pci_device_id *id)
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 9973ac894a51..f0703286d93b 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -428,12 +428,10 @@ static void dw_i2c_plat_complete(struct device *dev)
 #endif
 
 #ifdef CONFIG_PM
-static int dw_i2c_plat_suspend(struct device *dev)
+static int dw_i2c_plat_runtime_suspend(struct device *dev)
 {
 	struct dw_i2c_dev *i_dev = dev_get_drvdata(dev);
 
-	i_dev->suspended = true;
-
 	if (i_dev->shared_with_punit)
 		return 0;
 
@@ -443,7 +441,18 @@ static int dw_i2c_plat_suspend(struct device *dev)
 	return 0;
 }
 
-static int dw_i2c_plat_resume(struct device *dev)
+static int dw_i2c_plat_suspend(struct device *dev)
+{
+	struct dw_i2c_dev *i_dev = dev_get_drvdata(dev);
+
+	i2c_lock_bus(&i_dev->adapter, I2C_LOCK_ROOT_ADAPTER);
+	i_dev->suspended = true;
+	i2c_unlock_bus(&i_dev->adapter, I2C_LOCK_ROOT_ADAPTER);
+
+	return dw_i2c_plat_runtime_suspend(dev);
+}
+
+static int dw_i2c_plat_runtime_resume(struct device *dev)
 {
 	struct dw_i2c_dev *i_dev = dev_get_drvdata(dev);
 
@@ -451,7 +460,19 @@ static int dw_i2c_plat_resume(struct device *dev)
 		i2c_dw_prepare_clk(i_dev, true);
 
 	i_dev->init(i_dev);
+
+	return 0;
+}
+
+static int dw_i2c_plat_resume(struct device *dev)
+{
+	struct dw_i2c_dev *i_dev = dev_get_drvdata(dev);
+
+	dw_i2c_plat_runtime_resume(dev);
+
+	i2c_lock_bus(&i_dev->adapter, I2C_LOCK_ROOT_ADAPTER);
 	i_dev->suspended = false;
+	i2c_unlock_bus(&i_dev->adapter, I2C_LOCK_ROOT_ADAPTER);
 
 	return 0;
 }
@@ -460,7 +481,7 @@ static const struct dev_pm_ops dw_i2c_dev_pm_ops = {
 	.prepare = dw_i2c_plat_prepare,
 	.complete = dw_i2c_plat_complete,
 	SET_LATE_SYSTEM_SLEEP_PM_OPS(dw_i2c_plat_suspend, dw_i2c_plat_resume)
-	SET_RUNTIME_PM_OPS(dw_i2c_plat_suspend, dw_i2c_plat_resume, NULL)
+	SET_RUNTIME_PM_OPS(dw_i2c_plat_runtime_suspend, dw_i2c_plat_runtime_resume, NULL)
 };
 
 #define DW_I2C_DEV_PMOPS (&dw_i2c_dev_pm_ops)