Skip to content
  • Hans de Goede's avatar
    i2c: designware: Do not allow i2c_dw_xfer() calls while suspended · 27515415
    Hans de Goede authored
    
    
    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.
    
    Note the above errors are an example of issues which this patch will
    help to debug, the actual fix requires fixing the suspend order and
    this has been fixed by a different commit.
    
    Note the setting / clearing of the suspended flag in the suspend / resume
    methods is NOT protected by i2c_lock_bus(). This is intentional as these
    methods get called from i2c_dw_xfer() (through pm_runtime_get/put) a nd
    i2c_dw_xfer() is called with the i2c_bus_lock held, so otherwise we would
    deadlock. This means that there is a theoretical race between a non runtime
    suspend and the suspended check in i2c_dw_xfer(), this is not a problem
    since normally we should not hit the race and this check is primarily a
    debugging tool so hitting the check if there are suspend/resume ordering
    problems does not need to be 100% reliable.
    
    Signed-off-by: default avatarHans de Goede <hdegoede@redhat.com>
    Reviewed-by: default avatarAndy Shevchenko <andriy.shevchenko@linux.intel.com>
    Signed-off-by: default avatarWolfram Sang <wsa@the-dreams.de>
    27515415