queue-5.0/i2c-designware-do-not-allow-i2c_dw_xfer-calls-while-.patch

   1 From 1d2047a553d2710034b44b520410ed59ca03b57c Mon Sep 17 00:00:00 2001
   2 From: Hans de Goede <hdegoede@redhat.com>
   3 Date: Fri, 22 Feb 2019 14:08:40 +0100
   4 Subject: i2c: designware: Do not allow i2c_dw_xfer() calls while suspended
   5
   6 [ Upstream commit 2751541555382dfa7661bcfaac3ee0fac49f505d ]
   7
   8 On most Intel Bay- and Cherry-Trail systems the PMIC is connected over I2C
   9 and the PMIC is accessed through various means by the _PS0 and _PS3 ACPI
  10 methods (power on / off methods) of various devices.
  11
  12 This leads to suspend/resume ordering problems where a device may be
  13 resumed and get its _PS0 method executed before the I2C controller is
  14 resumed. On Cherry Trail this leads to errors like these:
  15
  16      i2c_designware 808622C1:06: controller timed out
  17      ACPI Error: AE_ERROR, Returned by Handler for [UserDefinedRegion]
  18      ACPI Error: Method parse/execution failed \_SB.P18W._ON, AE_ERROR
  19      video LNXVIDEO:00: Failed to change power state to D0
  20
  21 But on Bay Trail this caused I2C reads to seem to succeed, but they end
  22 up returning wrong data, which ends up getting written back by the typical
  23 read-modify-write cycle done to turn on various power-resources.
  24
  25 Debugging the problems caused by this silent data corruption is quite
  26 nasty. This commit adds a check which disallows i2c_dw_xfer() calls to
  27 happen until the controller's resume method has completed.
  28
  29 Which turns the silent data corruption into getting these errors in
  30 dmesg instead:
  31
  32     i2c_designware 80860F41:04: Error i2c_dw_xfer call while suspended
  33     ACPI Error: AE_ERROR, Returned by Handler for [UserDefinedRegion]
  34     ACPI Error: Method parse/execution failed \_SB.PCI0.GFX0._PS0, AE_ERROR
  35
  36 Which is much better.
  37
  38 Note the above errors are an example of issues which this patch will
  39 help to debug, the actual fix requires fixing the suspend order and
  40 this has been fixed by a different commit.
  41
  42 Note the setting / clearing of the suspended flag in the suspend / resume
  43 methods is NOT protected by i2c_lock_bus(). This is intentional as these
  44 methods get called from i2c_dw_xfer() (through pm_runtime_get/put) a nd
  45 i2c_dw_xfer() is called with the i2c_bus_lock held, so otherwise we would
  46 deadlock. This means that there is a theoretical race between a non runtime
  47 suspend and the suspended check in i2c_dw_xfer(), this is not a problem
  48 since normally we should not hit the race and this check is primarily a
  49 debugging tool so hitting the check if there are suspend/resume ordering
  50 problems does not need to be 100% reliable.
  51
  52 Signed-off-by: Hans de Goede <hdegoede@redhat.com>
  53 Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
  54 Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
  55 Signed-off-by: Sasha Levin <sashal@kernel.org>
  56 ---
  57  drivers/i2c/busses/i2c-designware-core.h    | 2 ++
  58  drivers/i2c/busses/i2c-designware-master.c  | 6 ++++++
  59  drivers/i2c/busses/i2c-designware-pcidrv.c  | 7 ++++++-
  60  drivers/i2c/busses/i2c-designware-platdrv.c | 3 +++
  61  4 files changed, 17 insertions(+), 1 deletion(-)
  62
  63 diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
  64 index b4a0b2b99a78..6b4ef1d38fb2 100644
  65 --- a/drivers/i2c/busses/i2c-designware-core.h
  66 +++ b/drivers/i2c/busses/i2c-designware-core.h
  67 @@ -215,6 +215,7 @@
  68   * @disable_int: function to disable all interrupts
  69   * @init: function to initialize the I2C hardware
  70   * @mode: operation mode - DW_IC_MASTER or DW_IC_SLAVE
  71 + * @suspended: set to true if the controller is suspended
  72   *
  73   * HCNT and LCNT parameters can be used if the platform knows more accurate
  74   * values than the one computed based only on the input clock frequency.
  75 @@ -270,6 +271,7 @@ struct dw_i2c_dev {
  76         int                     (*set_sda_hold_time)(struct dw_i2c_dev *dev);
  77         int                     mode;
  78         struct i2c_bus_recovery_info rinfo;
  79 +       bool                    suspended;
  80  };
  81
  82  #define ACCESS_SWAP            0x00000001
  83 diff --git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c
  84 index 8d1bc44d2530..bb8e3f149979 100644
  85 --- a/drivers/i2c/busses/i2c-designware-master.c
  86 +++ b/drivers/i2c/busses/i2c-designware-master.c
  87 @@ -426,6 +426,12 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
  88
  89         pm_runtime_get_sync(dev->dev);
  90
  91 +       if (dev->suspended) {
  92 +               dev_err(dev->dev, "Error %s call while suspended\n", __func__);
  93 +               ret = -ESHUTDOWN;
  94 +               goto done_nolock;
  95 +       }
  96 +
  97         reinit_completion(&dev->cmd_complete);
  98         dev->msgs = msgs;
  99         dev->msgs_num = num;
 100 diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c b/drivers/i2c/busses/i2c-designware-pcidrv.c
 101 index d50f80487214..76810deb2de6 100644
 102 --- a/drivers/i2c/busses/i2c-designware-pcidrv.c
 103 +++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
 104 @@ -176,6 +176,7 @@ static int i2c_dw_pci_suspend(struct device *dev)
 105         struct pci_dev *pdev = to_pci_dev(dev);
 106         struct dw_i2c_dev *i_dev = pci_get_drvdata(pdev);
 107
 108 +       i_dev->suspended = true;
 109         i_dev->disable(i_dev);
 110
 111         return 0;
 112 @@ -185,8 +186,12 @@ static int i2c_dw_pci_resume(struct device *dev)
 113  {
 114         struct pci_dev *pdev = to_pci_dev(dev);
 115         struct dw_i2c_dev *i_dev = pci_get_drvdata(pdev);
 116 +       int ret;
 117
 118 -       return i_dev->init(i_dev);
 119 +       ret = i_dev->init(i_dev);
 120 +       i_dev->suspended = false;
 121 +
 122 +       return ret;
 123  }
 124  #endif
 125
 126 diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
 127 index 9eaac3be1f63..ead5e7de3e4d 100644
 128 --- a/drivers/i2c/busses/i2c-designware-platdrv.c
 129 +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
 130 @@ -454,6 +454,8 @@ static int dw_i2c_plat_suspend(struct device *dev)
 131  {
 132         struct dw_i2c_dev *i_dev = dev_get_drvdata(dev);
 133
 134 +       i_dev->suspended = true;
 135 +
 136         if (i_dev->shared_with_punit)
 137                 return 0;
 138
 139 @@ -471,6 +473,7 @@ static int dw_i2c_plat_resume(struct device *dev)
 140                 i2c_dw_prepare_clk(i_dev, true);
 141
 142         i_dev->init(i_dev);
 143 +       i_dev->suspended = false;
 144
 145         return 0;
 146  }
 147 --
 148 2.19.1
 149