[PATCH 1/1] UBUNTU: SAUCE: i2c: designware: Add disable runtime pm quirk

[Kai-Heng Feng]

at 14:08, AceLan Kao <acelan.kao at canonical.com> wrote:

> It's still necessary to prevent it from calling pm_runtime_get/put  
> explicitly,
> The "put" function still make designware enter runtime suspended, so
> but "get" can't wake it up,

That means there’s an imbalanced pm helper usage, so the pm count reaches 0  
at some point.
But it’s another bug though, so I think your approach is a good compromise.

Kai-Heng

> and then it leads to strange issue that it complains it can't wake up
> the dw and the i2c command times out.
>
> Kai-Heng Feng <kai.heng.feng at canonical.com> 於 2019年6月20日 週四 下午1:04寫道：
>> at 10:38, AceLan Kao <acelan.kao at canonical.com> wrote:
>>
>>> BugLink: https://bugs.launchpad.net/bugs/1833484
>>>
>>> Dell machines come with goodix touchpad IC suffer from the double click
>>> issue if the Designware I2C adapter enters runtime suspend.
>>>
>>> It's because the goodix re-assert the interrupt if host doesn't read the
>>> data within 100ms and desiginware takes a longer time to wake up from
>>> runtime suspend. In the case, it got a second interrupt during
>>> resuming, so it thinks it's a double click.
>>>
>>> There is no simple way to fix this, it's a firmware issue and goodix
>>> agrees to fix this in their firmware on next release, but this issue is
>>> still affects the machines that don't come with an updated firmware. So,
>>> add a quirk to mark those machines and avoid the designware to enter
>>> runtime suspend.
>>>
>>> Signed-off-by: AceLan Kao <acelan.kao at canonical.com>
>>> ---
>>>  drivers/i2c/busses/i2c-designware-master.c | 30 ++++++++++++++++++++--
>>>  1 file changed, 28 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/i2c/busses/i2c-designware-master.c
>>> b/drivers/i2c/busses/i2c-designware-master.c
>>> index bb8e3f149979..7acb015f0470 100644
>>> --- a/drivers/i2c/busses/i2c-designware-master.c
>>> +++ b/drivers/i2c/busses/i2c-designware-master.c
>>> @@ -9,6 +9,7 @@
>>>   * Copyright (C) 2009 Provigent Ltd.
>>>   */
>>>  #include <linux/delay.h>
>>> +#include <linux/dmi.h>
>>>  #include <linux/err.h>
>>>  #include <linux/errno.h>
>>>  #include <linux/export.h>
>>> @@ -22,6 +23,25 @@
>>>
>>>  #include "i2c-designware-core.h"
>>>
>>> +static int no_runtime_pm;
>>> +static const struct dmi_system_id i2c_dw_no_runtime_pm[] = {
>>> +     {
>>> +             .ident = "Dell Vostro 5390",
>>> +             .matches = {
>>> +                     DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
>>> +                     DMI_MATCH(DMI_PRODUCT_NAME, "Vostro 5390"),
>>> +             },
>>> +     },
>>> +     {
>>> +             .ident = "Dell Vostro 5391",
>>> +             .matches = {
>>> +                     DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
>>> +                     DMI_MATCH(DMI_PRODUCT_NAME, "Vostro 5391"),
>>> +             },
>>> +     },
>>> +     { }
>>> +};
>>> +
>>>  static void i2c_dw_configure_fifo_master(struct dw_i2c_dev *dev)
>>>  {
>>>       /* Configure Tx/Rx FIFO threshold levels */
>>> @@ -424,7 +444,8 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg
>>> msgs[], int num)
>>>
>>>       dev_dbg(dev->dev, "%s: msgs: %d\n", __func__, num);
>>>
>>> -     pm_runtime_get_sync(dev->dev);
>>> +     if (!no_runtime_pm)
>>> +             pm_runtime_get_sync(dev->dev);
>>>
>>>       if (dev->suspended) {
>>>               dev_err(dev->dev, "Error %s call while suspended\n", __func__);
>>> @@ -502,7 +523,8 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg
>>> msgs[], int num)
>>>
>>>  done_nolock:
>>>       pm_runtime_mark_last_busy(dev->dev);
>>> -     pm_runtime_put_autosuspend(dev->dev);
>>> +     if (!no_runtime_pm)
>>> +             pm_runtime_put_autosuspend(dev->dev);
>>>
>>>       return ret;
>>>  }
>>> @@ -734,6 +756,10 @@ int i2c_dw_probe(struct dw_i2c_dev *dev)
>>>       if (ret)
>>>               return ret;
>>>
>>> +     no_runtime_pm = dmi_check_system(i2c_dw_no_runtime_pm);
>>> +     if (no_runtime_pm)
>>> +             __pm_runtime_disable(dev->dev, true);
>>> +
>>
>> I think using pm helpers in i2c_dw_xfer() is unnecessary if we do this in
>> i2c_dw_probe():
>>
>> --- a/drivers/i2c/busses/i2c-designware-master.c
>> +++ b/drivers/i2c/busses/i2c-designware-master.c
>> @@ -743,7 +743,9 @@ int i2c_dw_probe(struct dw_i2c_dev *dev)
>>          ret = i2c_add_numbered_adapter(adap);
>>          if (ret)
>>                  dev_err(dev->dev, "failure adding adapter: %d\n", ret);
>> -       pm_runtime_put_noidle(dev->dev);
>> +
>> +       if (!no_runtime_pm)
>> +               pm_runtime_put_noidle(dev->dev);
>>
>>          return ret;
>>   }
>>
>>
>>>      /*
>>>        * Increment PM usage count during adapter registration in order to
>>>        * avoid possible spurious runtime suspend when adapter device is
>>> --
>>> 2.17.1
>>>
>>>
>>> --
>>> kernel-team mailing list
>>> kernel-team at lists.ubuntu.com
>>> https://lists.ubuntu.com/mailman/listinfo/kernel-team