From c57813b8b288dcb5e158993cfdff31a60bc24955 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 23 Feb 2022 14:48:38 +0100 Subject: [PATCH] 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 Reviewed-by: Andy Shevchenko Acked-by: Jarkko Nikula Signed-off-by: Wolfram Sang --- drivers/i2c/busses/i2c-designware-pcidrv.c | 31 +++++++++++++++++---- drivers/i2c/busses/i2c-designware-platdrv.c | 31 +++++++++++++++++---- 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 2782dddfb087c..a736a2aaa66e3 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 9973ac894a519..f0703286d93b7 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) -- 2.30.2