On Thursday 27 September 2018 05:01 AM, Tony Lindgren wrote:
> * Grygorii Strashko <grygorii.strashko@xxxxxx> [180926 21:41]:
>> On 09/26/2018 11:23 AM, Tony Lindgren wrote:
>>> +static int __maybe_unused omap_i2c_suspend(struct device *dev)
>>> +{
>>> + struct omap_i2c_dev *ddata = dev_get_drvdata(dev);
>>> + int error;
>>> +
>>> + /* Is device still enabled because of autosuspend? */
>>> + if (ddata->is_suspended)
>>> + return 0;
>>
>> Sry, but you can't do this. There is no sync between suspend and PM runtime.
>>
>>
>>> +
>>> + /* Paired with call in omap_i2c_resume() */
>>> + error = pm_runtime_put_sync_suspend(dev);
>>
>> This is nop!!! More over, in general you can't predict how many times pm_runtime_get was called and
>> what's the current value of usage_count.
>
> Hmm yeah that's a good point.
>
>> To make things work the pm_runtime_force_xx() have to be used, or
>> like with omap_device, platform/bus code have to handle device state
>> at suspend_no_irq stage.
>
> OK. So looks like the other i2c bus drivers have already solved it
> with simply SET_NOIRQ_SYSTEM_SLEEP_PM_OPS which is along the lines
> you're suggesting.
>
> The following works for me, does it look OK to you?
This works pretty well with DS0 on am437x-gp-evm. Tested rtc wakeup,
wakeup using uart and touch. All of the work well.
https://pastebin.ubuntu.com/p/RJhJJKhJHX/
Also tested on beaglebone black DS0 works fine. Although there seems to
be an additional warning which was not seen before:
"l4-wkup-clkctrl:0008:0: failed to disable"
log: https://pastebin.ubuntu.com/p/Ns2WRdVkXS/
Also the above warning is present without/with this patch on [0].
No such warnings on am4 though.
[0]
https://git.kernel.org/pub/scm/linux/kernel/git/tmlind/linux-omap.git/log/?h=omap-for-v4.20/dt-ti-sysc-tmp-testing
Regards,
Keerthy
>
> Regards,
>
> Tony
>
> 8< ---------------------
> diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
> --- a/drivers/i2c/busses/i2c-omap.c
> +++ b/drivers/i2c/busses/i2c-omap.c
> @@ -1498,8 +1498,7 @@ static int omap_i2c_remove(struct platform_device *pdev)
> return 0;
> }
>
> -#ifdef CONFIG_PM
> -static int omap_i2c_runtime_suspend(struct device *dev)
> +static int __maybe_unused omap_i2c_runtime_suspend(struct device *dev)
> {
> struct omap_i2c_dev *omap = dev_get_drvdata(dev);
>
> @@ -1525,7 +1524,7 @@ static int omap_i2c_runtime_suspend(struct device *dev)
> return 0;
> }
>
> -static int omap_i2c_runtime_resume(struct device *dev)
> +static int __maybe_unused omap_i2c_runtime_resume(struct device *dev)
> {
> struct omap_i2c_dev *omap = dev_get_drvdata(dev);
>
> @@ -1540,20 +1539,18 @@ static int omap_i2c_runtime_resume(struct device *dev)
> }
>
> static const struct dev_pm_ops omap_i2c_pm_ops = {
> + SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
> + pm_runtime_force_resume)
> SET_RUNTIME_PM_OPS(omap_i2c_runtime_suspend,
> omap_i2c_runtime_resume, NULL)
> };
> -#define OMAP_I2C_PM_OPS (&omap_i2c_pm_ops)
> -#else
> -#define OMAP_I2C_PM_OPS NULL
> -#endif /* CONFIG_PM */
>
> static struct platform_driver omap_i2c_driver = {
> .probe = omap_i2c_probe,
> .remove = omap_i2c_remove,
> .driver = {
> .name = "omap_i2c",
> - .pm = OMAP_I2C_PM_OPS,
> + .pm = &omap_i2c_pm_ops,
> .of_match_table = of_match_ptr(omap_i2c_of_match),
> },
> };
>
