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>

diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c b/drivers/i2c/busses/i2c-designware-pcidrv.c
index 2782dddfb087c66d5d8df2ed49ceecb0e96bf0da..a736a2aaa66e34184d085dd9f04a9d036de3cacb 100644 (file)
--- 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 9973ac894a5194753689f6d42fd99f30808a1cc5..f0703286d93b78026e6df9e2b4d0115a3555c702 100644 (file)
--- 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)