* Grygorii Strashko <grygorii.strashko@xxxxxx> [190904 11:39]:
> On 03/09/2019 21:29, Tony Lindgren wrote:
> > 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.
You are right, good catch. Maybe this in addition to the clocks is
why some dra7 boards fail to enable cpsw depending on the bootloader?
> > @@ -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."
OK makes sense now thanks. I've dropped that change and added a comment
there. Updated patch below again to test.
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);
@@ -1660,7 +1662,7 @@ static int sysc_init_module(struct sysc *ddata)
goto err_main_clocks;
}
- if (!ddata->legacy_mode && manage_clocks) {
+ if (!ddata->legacy_mode) {
error = sysc_enable_module(ddata->dev);
if (error)
goto err_main_clocks;
@@ -1677,6 +1679,7 @@ static int sysc_init_module(struct sysc *ddata)
if (manage_clocks)
sysc_disable_main_clocks(ddata);
err_opt_clocks:
+ /* No re-enable of clockdomain autoidle to prevent module autoidle */
if (manage_clocks) {
sysc_disable_opt_clocks(ddata);
sysc_clkdm_allow_idle(ddata);
@@ -2357,6 +2360,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 +2470,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
