On most Intel Bay- and Cherry-Trail systems the PMIC is connected over I2C
and the PMIC is accessed through various means by the _PS0 and _PS3 ACPI
methods (power on / off methods) of various devices.
This leads to suspend/resume ordering problems where a device may be
resumed and get its _PS0 method executed before the I2C controller is
resumed. On Cherry Trail this leads to errors like these:
i2c_designware 808622C1:06: controller timed out
ACPI Error: AE_ERROR, Returned by Handler for [UserDefinedRegion]
ACPI Error: Method parse/execution failed \_SB.P18W._ON, AE_ERROR
video LNXVIDEO:00: Failed to change power state to D0
But on Bay Trail this caused I2C reads to seem to succeed, but they end
up returning wrong data, which ends up getting written back by the typical
read-modify-write cycle done to turn on various power-resources.
Debugging the problems caused by this silent data corruption is quite
nasty. This commit adds a check which disallows i2c_dw_xfer() calls to
happen until the controller's resume method has completed.
Which turns the silent data corruption into getting these errors in
dmesg instead:
i2c_designware 80860F41:04: Error i2c_dw_xfer call while suspended
ACPI Error: AE_ERROR, Returned by Handler for [UserDefinedRegion]
ACPI Error: Method parse/execution failed \_SB.PCI0.GFX0._PS0, AE_ERROR
Which is much better.
Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
---
drivers/i2c/busses/i2c-designware-core.h | 2 ++
drivers/i2c/busses/i2c-designware-master.c | 6 ++++++
drivers/i2c/busses/i2c-designware-pcidrv.c | 7 ++++++-
drivers/i2c/busses/i2c-designware-platdrv.c | 3 +++
4 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index 1b5a6d47f73d..31e0c84ab4e7 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -215,6 +215,7 @@
* @disable_int: function to disable all interrupts
* @init: function to initialize the I2C hardware
* @mode: operation mode - DW_IC_MASTER or DW_IC_SLAVE
+ * @suspended: set to true if the controller is suspended
*
* HCNT and LCNT parameters can be used if the platform knows more accurate
* values than the one computed based only on the input clock frequency.
@@ -268,6 +269,7 @@ struct dw_i2c_dev {
int (*init)(struct dw_i2c_dev *dev);
int mode;
struct i2c_bus_recovery_info rinfo;
+ bool suspended;
};
#define ACCESS_SWAP 0x00000001
diff --git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c
index 2a630ac35ba2..77a1ab33a3d4 100644
--- a/drivers/i2c/busses/i2c-designware-master.c
+++ b/drivers/i2c/busses/i2c-designware-master.c
@@ -424,6 +424,12 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
pm_runtime_get_sync(dev->dev);
+ if (dev->suspended) {
+ dev_err(dev->dev, "Error %s call while suspended\n", __func__);
+ ret = -EIO;
+ goto done_nolock;
+ }
+
reinit_completion(&dev->cmd_complete);
dev->msgs = msgs;
dev->msgs_num = num;
diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c b/drivers/i2c/busses/i2c-designware-pcidrv.c
index d50f80487214..76810deb2de6 100644
--- a/drivers/i2c/busses/i2c-designware-pcidrv.c
+++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
@@ -176,6 +176,7 @@ static int i2c_dw_pci_suspend(struct device *dev)
struct pci_dev *pdev = to_pci_dev(dev);
struct dw_i2c_dev *i_dev = pci_get_drvdata(pdev);
+ i_dev->suspended = true;
i_dev->disable(i_dev);
return 0;
@@ -185,8 +186,12 @@ static int i2c_dw_pci_resume(struct device *dev)
{
struct pci_dev *pdev = to_pci_dev(dev);
struct dw_i2c_dev *i_dev = pci_get_drvdata(pdev);
+ int ret;
- return i_dev->init(i_dev);
+ ret = i_dev->init(i_dev);
+ i_dev->suspended = false;
+
+ return ret;
}
#endif
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 19833f9aaefe..d4604bea78ad 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -414,6 +414,8 @@ static int dw_i2c_plat_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;
@@ -431,6 +433,7 @@ static int dw_i2c_plat_resume(struct device *dev)
i2c_dw_prepare_clk(i_dev, true);
i_dev->init(i_dev);
+ i_dev->suspended = false;
return 0;
}
--
2.19.0.rc1
