On 16/05/2025 5:07 pm, Leo Yan wrote:
If a trace unit has been enabled, a CPU suspend operation misses to disable the trace unit. After the CPU resumes, it also does not follow the general flow for re-enabling the trace unit. As a result, this may lead to a wrong state machine for the ETM.
This commit stops the trace unit during the CPU suspend flow and re-enables the trace unit in the resume flow.
Fixes: f188b5e76aae ("coresight: etm4x: Save/restore state across CPU low power states") Signed-off-by: Leo Yan leo.yan@arm.com
drivers/hwtracing/coresight/coresight-etm4x-core.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index 6a5898355a83..b12d59b89a49 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -1820,6 +1820,11 @@ static int __etm4_cpu_save(struct etmv4_drvdata *drvdata) state = drvdata->save_state; state->trcprgctlr = etm4x_read32(csa, TRCPRGCTLR);
- /* Stop the trace unit if it is enabled */
- if (state->trcprgctlr)
etm4_cpu_save() looks at coresight_get_mode() to determine enabled state. Seems a bit inconsistent to look directly at the enable bit right after checking coresight_get_mode().
etm4_disable_trace_unit(drvdata);
Section 3.4.1 in ARM IHI 0064H.b doesn't say that anything extra needs to be done here, and it implies that restoring to an enabled state should work. It might make sense to mention why it diverges from the published sequence. Or get the sequence updated? I suppose that document doesn't include ETE and I see there is one extra register missing from __etm4_cpu_save() which is written to in etm4_disable_trace_unit().
Plus we end up doing extra things like two waits on TRCSTATR.PMSTABLE.
if (drvdata->nr_pe) state->trcprocselr = etm4x_read32(csa, TRCPROCSELR); state->trcconfigr = etm4x_read32(csa, TRCCONFIGR); @@ -1951,7 +1956,6 @@ static void __etm4_cpu_restore(struct etmv4_drvdata *drvdata) etm4_cs_unlock(drvdata, csa); etm4x_relaxed_write32(csa, state->trcclaimset, TRCCLAIMSET);
- etm4x_relaxed_write32(csa, state->trcprgctlr, TRCPRGCTLR); if (drvdata->nr_pe) etm4x_relaxed_write32(csa, state->trcprocselr, TRCPROCSELR); etm4x_relaxed_write32(csa, state->trcconfigr, TRCCONFIGR);
@@ -2035,6 +2039,15 @@ static void __etm4_cpu_restore(struct etmv4_drvdata *drvdata) /* Unlock the OS lock to re-enable trace and external debug access */ etm4_os_unlock(drvdata);
- /*
* Re-enable the trace unit if it was enabled before suspend.* Put it after etm4_os_unlock() as unlocking the OS lock is the* prerequisite for enabling the trace unit.*/- if (state->trcprgctlr)
etm4_enable_trace_unit(drvdata);- etm4_cs_lock(drvdata, csa); }