On 09/25/2018 12:55 PM, Tony Lindgren wrote:
* Tony Lindgren <tony@xxxxxxxxxxx> [180925 14:44]:
* Keerthy <j-keerthy@xxxxxx> [180925 05:18]:
I tried Branch [1] above. AM335x-boneblack is doing all good with ds0.
AM437x-gp the following is happening:
https://pastebin.ubuntu.com/p/fmZNYgJ9Sp/
There seems to be a timeout due to which resume is not immediate i see
1-2s delay and i see i2c timeout.
[ 402.247271] omap_i2c 4802a000.i2c: controller timed out
[ 402.247441] pixcir_ts 1-005c: pixcir_int_enable: can't read reg 0x34
: -110
[ 402.247486] pixcir_ts 1-005c: Failed to disable interrupt generation:
-110
[ 402.247522] pixcir_ts 1-005c: Failed to stop
[ 402.247607] dpm_run_callback(): pixcir_i2c_ts_resume+0x0/0x98
[pixcir_i2c_ts] returns -110
so pixcir_i2c_ts_resume is retuning -110 which is timeout error code.
I did some touch screen testing after DS0 resume seems to be working.
Everything is in pastebin link provided above.
You might want to try it with "echo 1 > /sys/power/pm_print_times"
and add some dbg print in _od_resume_noirq()/_od_suspend_noirq()
Thanks for testing. Hmm no idea what might be going on with
the pixcir. I have edt-ft5x06 on the am437x-sk-evm and that
behaves just fine for suspend and resume. Both are on i2c1,
pixcir calls pixcir_start/stop from suspend and resume while
edt-ft5x06 does not reconfigure anything.
I'll try read some edt-ft5x06 register values on suspend
and resume, let's see if that makes it reproducable here.
So after adding a call to edt_ft5x06_ts_readwrite to
edt_ft5x06_ts_resume I see -110 errors too. Looks like we
have i2c-omap not idled for suspend because of autoidle if
an i2c client reconfigures it's registers for suspend.
Care to test with the following patch applied? If that works
for you, I'll send a proper patch for that.
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
@@ -208,6 +208,8 @@ struct omap_i2c_dev {
* if set, should be trsh+1
*/
u32 rev;
+ unsigned int is_suspended:1;
+ unsigned int needs_resume:1;
unsigned b_hw:1; /* bad h/w fixes */
unsigned bb_valid:1; /* true when BB-bit reflects
* the I2C bus state
@@ -1498,8 +1500,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);
@@ -1521,11 +1522,12 @@ static int omap_i2c_runtime_suspend(struct device *dev)
}
pinctrl_pm_select_sleep_state(dev);
+ omap->is_suspended = true;
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);
@@ -1535,6 +1537,47 @@ static int omap_i2c_runtime_resume(struct device *dev)
return 0;
__omap_i2c_init(omap);
+ omap->is_suspended = false;
+
+ return 0;
+}
+
+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) {
+ error = pm_runtime_put_sync_suspend(dev);
sry, this make no sense - put is nop, because of
pm_runtime_get_noresume() in device_prepare()
+ if (error < 0) {
+ dev_err(dev, "%s failed: %i\n", __func__, error);
+
+ return error;
+ }
+ ddata->needs_resume = true;
+ }
+
+ return 0;
+}
+
+static int __maybe_unused omap_i2c_resume(struct device *dev)
+{
+ struct omap_i2c_dev *ddata = dev_get_drvdata(dev);
+ int error;
+
+ if (ddata->needs_resume) {
+ error = pm_runtime_get_sync(dev);
+ if (error < 0) {
+ dev_err(dev, "%s failed: %i\n", __func__, error);
+ pm_runtime_put_noidle(dev);
+
+ return error;
+ }
+ pm_runtime_mark_last_busy(dev);
+ pm_runtime_put_autosuspend(dev);
+ ddata->needs_resume = false;
+ }
return 0;
}
@@ -1542,18 +1585,15 @@ static int omap_i2c_runtime_resume(struct device *dev)
static const struct dev_pm_ops omap_i2c_pm_ops = {
SET_RUNTIME_PM_OPS(omap_i2c_runtime_suspend,
omap_i2c_runtime_resume, NULL)
+ SET_SYSTEM_SLEEP_PM_OPS(omap_i2c_suspend, omap_i2c_resume)
};
-#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),
},
};
--
regards,
-grygorii
