Re: [PATCH] i2c: designware: Add disable runtime pm quirk

[AceLan Kao]

Hi Jarkko,

Sorry, I lost track of this thread and didn't understand what you want
me to try.
I'm willing to try it if you can explain it more.

My colleague comes out another solution for this issue
   https://lkml.org/lkml/2019/7/5/17
and it explains why it takes up to 100ms to wake up.
This solution is more aggressive to zero the d3cold_delay and it looks
like the delay is not necessary.

Anyway, we have two different proposed solutions now, my proposed
solution is specific to the listed platforms, but we may have to
extent the list(platforms which uses the old firmware),
the other proposed solution from my colleague is generic and apply to
all platforms which loads intel-lpss-pci driver, it may lead to
unexpected regressions which we don't see now.
Please give some suggestions, thanks.

Best regards,
AceLan Kao.

Jarkko Nikula <jarkko.nikula@xxxxxxxxxxxxxxx> æ 2019å6æ26æ éä äå2:27åéï
>
> On 6/26/19 5:32 AM, AceLan Kao wrote:
> > Adding I2C_HID_QUIRK_NO_RUNTIME_PM quirk doesn't help on this issue.
> > Actually, Goodix touchpad already has that PM quirk in the list for other issue.
> >          { I2C_VENDOR_ID_GOODIX, I2C_DEVICE_ID_GOODIX_01F0,
> >                 I2C_HID_QUIRK_NO_RUNTIME_PM },
> > I also modify the code as you suggested, but no luck.
> >
> Yeah, I realized it won't help as the i2c-hid device is anyway powered
> on constantly when the device is open by the pm_runtime_get_sync() call
> in i2c_hid_open().
>
> > It's not Goodix takes time to wakeup, it's designware I2C controller.
> > Designware doesn't do anything wrong here, it's Goodix set the interrupt timeout
> > that leads to the issue, so we have to prevent designware from runtime
> > suspended.
> > But only on that bus where Goodix is connected and open by user space.
> What I mean something like below:
>
> diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c
> b/drivers/hid/i2c-hid/i2c-hid-core.c
> index 90164fed08d3..bbeaa39ddc23 100644
> --- a/drivers/hid/i2c-hid/i2c-hid-core.c
> +++ b/drivers/hid/i2c-hid/i2c-hid-core.c
> @@ -795,6 +795,9 @@ static int i2c_hid_open(struct hid_device *hid)
>         struct i2c_hid *ihid = i2c_get_clientdata(client);
>         int ret = 0;
>
> +       /* some quirk test here */
> +       pm_runtime_get_sync(&client->adapter->dev);
> +
>         ret = pm_runtime_get_sync(&client->dev);
>         if (ret < 0)
>                 return ret;
> @@ -812,6 +815,9 @@ static void i2c_hid_close(struct hid_device *hid)
>
>         /* Save some power */
>         pm_runtime_put(&client->dev);
> +
> +       /* some quirk test here */
> +       pm_runtime_put(&client->adapter->dev);
>   }
>
>   static int i2c_hid_power(struct hid_device *hid, int lvl)
>
> --
> Jarkko