On 03/09/2019 21:29, Tony Lindgren wrote:
* Tony Lindgren <tony@xxxxxxxxxxx> [190903 17:01]:
* Grygorii Strashko <grygorii.strashko@xxxxxx> [190903 15:58]:
This might be not right thing to do as in probe this operation is delayed
sysc_probe()
pm_runtime_enable(ddata->dev);
error = pm_runtime_get_sync(ddata->dev);
^^^ above is the first PM runtime get call, so it might be bette to keep module active
Oh OK, yeah that needs some more thinking.
^^^ by the way, CPSW will fail here with "ti,no-idle" removed
if (ddata->cfg.quirks & (SYSC_QUIRK_NO_IDLE_ON_INIT |
SYSC_QUIRK_NO_RESET_ON_INIT)) {
schedule_delayed_work(&ddata->idle_work, 3000);
^^ and check and double disable module in first PM runtime suspend
} else {
Hmm yeah that might work.
Actually we can just decrement the clocks in ti_sysc_idle()
since we have it already available.
+ sysc_clkdm_allow_idle(ddata);
No. if SYSC_QUIRK_NO_IDLE is set we can't do above
Well we should pair here since we call it unconditionally. The
calls for sysc_clkdm_deny_idle() and sysc_clkdm_allow_idle()
are only needed when enabling or disabling the clocks.
I've kept the sysc_clkdm_allow_idle() change to keep things
paired locally. That is assuming you don't come up with good
reasons to not do it :)
Updated patch below to play with.
Regards,
Tony
8< ---------------------
diff --git a/drivers/bus/ti-sysc.c b/drivers/bus/ti-sysc.c
--- a/drivers/bus/ti-sysc.c
+++ b/drivers/bus/ti-sysc.c
@@ -1632,17 +1632,19 @@ static int sysc_init_module(struct sysc *ddata)
if (error)
return error;
- if (manage_clocks) {
- sysc_clkdm_deny_idle(ddata);
+ sysc_clkdm_deny_idle(ddata);
- error = sysc_enable_opt_clocks(ddata);
- if (error)
- return error;
+ /*
+ * Always enable clocks. The bootloader may or may not have enabled
+ * the related clocks.
+ */
+ error = sysc_enable_opt_clocks(ddata);
+ if (error)
+ return error;
- error = sysc_enable_main_clocks(ddata);
- if (error)
- goto err_opt_clocks;
- }
+ error = sysc_enable_main_clocks(ddata);
+ if (error)
+ goto err_opt_clocks;
if (!(ddata->cfg.quirks & SYSC_QUIRK_NO_RESET_ON_INIT)) {
error = sysc_rstctrl_reset_deassert(ddata, true);
- if (!ddata->legacy_mode && manage_clocks) {
+ if (!ddata->legacy_mode) {
error = sysc_enable_module(ddata->dev);
if (error)
goto err_main_clocks;
Module should also enabled here.
@@ -1677,10 +1679,10 @@ static int sysc_init_module(struct sysc *ddata)
if (manage_clocks)
sysc_disable_main_clocks(ddata);
err_opt_clocks:
- if (manage_clocks) {
+ if (manage_clocks)
sysc_disable_opt_clocks(ddata);
- sysc_clkdm_allow_idle(ddata);
- }
+
+ sysc_clkdm_allow_idle(ddata);
clkdm doesn't have counters while clock do, so if module is in HW_AUTO
and clkdm in HW_AUTO - the module can go IDLE between this point and ti_sysc_idle() call.
Errate i877 required
CM_GMAC_CLKSTCTRL[1:0] CLKTRCTRL = 0x2:SW_WKUP.
to be set at boot time and never changed.
and
"In addition to programming SW_WKUP(0x2) on CM_GMAC_CLKSTCTRL, SW should
also program modulemode field as ENABLED(0x2) on CM_GMAC_GMAC_CLKCTRL
register."
return error;
}
@@ -2357,6 +2359,28 @@ static void ti_sysc_idle(struct work_struct *work)
ddata = container_of(work, struct sysc, idle_work.work);
+ /*
+ * One time decrement of clock usage counts if left on from init.
+ * Note that we disable opt clocks unconditionally in this case
+ * as they are enabled unconditionally during init without
+ * considering sysc_opt_clks_needed() at that point.
+ */
+ if (ddata->cfg.quirks & (SYSC_QUIRK_NO_IDLE |
+ SYSC_QUIRK_NO_IDLE_ON_INIT)) {
+ sysc_clkdm_deny_idle(ddata);
+ sysc_disable_main_clocks(ddata);
+ sysc_disable_opt_clocks(ddata);
+ sysc_clkdm_allow_idle(ddata);
+ }
+
+ /* Keep permanent PM runtime usage count for SYSC_QUIRK_NO_IDLE */
+ if (ddata->cfg.quirks & SYSC_QUIRK_NO_IDLE)
+ return;
+
+ /*
+ * Decrement PM runtime usage count for SYSC_QUIRK_NO_IDLE_ON_INIT
+ * and SYSC_QUIRK_NO_RESET_ON_INIT
+ */
if (pm_runtime_active(ddata->dev))
pm_runtime_put_sync(ddata->dev);
}
@@ -2445,7 +2469,8 @@ static int sysc_probe(struct platform_device *pdev)
INIT_DELAYED_WORK(&ddata->idle_work, ti_sysc_idle);
/* At least earlycon won't survive without deferred idle */
- if (ddata->cfg.quirks & (SYSC_QUIRK_NO_IDLE_ON_INIT |
+ if (ddata->cfg.quirks & (SYSC_QUIRK_NO_IDLE |
+ SYSC_QUIRK_NO_IDLE_ON_INIT |
SYSC_QUIRK_NO_RESET_ON_INIT)) {
schedule_delayed_work(&ddata->idle_work, 3000);
} else {
--
Best regards,
grygorii
