* 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);
@@ -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);
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 {
--
2.23.0
