Hi Mike,
On Mon, 7 Sep 2020 at 10:52, Mike Leach mike.leach@linaro.org wrote:
Hi,
On Thu, 3 Sep 2020 at 18:42, Mathieu Poirier mathieu.poirier@linaro.org wrote:
On Wed, Sep 02, 2020 at 06:37:13PM +0800, Jonathan Zhou wrote:
The TRCSEQEVR(3) is reserved, using '@nrseqstate - 1' instead to avoid accessing the reserved register.
Fixes: f188b5e76aae ("coresight: etm4x: Save/restore state across CPU low power states") Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Suzuki K Poulose suzuki.poulose@arm.com Cc: Mike Leach mike.leach@linaro.org Cc: Shaokun Zhang zhangshaokun@hisilicon.com Cc: lizixian@hisilicon.com
Signed-off-by: Jonathan Zhou jonathan.zhouwen@huawei.com
drivers/hwtracing/coresight/coresight-etm4x.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c b/drivers/hwtracing/coresight/coresight-etm4x.c index 96425e818fc2..44e44c817bf8 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x.c +++ b/drivers/hwtracing/coresight/coresight-etm4x.c @@ -1183,7 +1183,7 @@ static int etm4_cpu_save(struct etmv4_drvdata *drvdata) state->trcvdsacctlr = readl(drvdata->base + TRCVDSACCTLR); state->trcvdarcctlr = readl(drvdata->base + TRCVDARCCTLR);
for (i = 0; i < drvdata->nrseqstate; i++)
for (i = 0; i < drvdata->nrseqstate - 1; i++) state->trcseqevr[i] = readl(drvdata->base + TRCSEQEVRn(i));
The section 3.4.3 "Guidelines for trace unit registers to be saved and restored" of the ETM4 Architecture Specification (ARM IHI0064F ID042818) is clear on the fact that registers TRCSEQEVR0-3 have to be taken into account when saving the
I think this is a typo in the TRM. I'll ping the docs people in ARM.
Did you get an answer from the document people? Is this really a typographical problem?
Thanks, Mathieu
trace unit state.
Looking @ the register descriptions for TRCSEQEVRn (7.3.63) in the above document n=0-2. The number of states is set by TRCIDR5.NUMSEQSTATE (7.3.35). This can take the value 0 or 4. If 4 then there are 3 TRCSEQEVR(n) registers - 0 to 2 - one for each state transition.
Thus this patch is correct in using nrseqstate - 1.
Regards
Mike
Thanks, Mathieu
state->trcseqrstevr = readl(drvdata->base + TRCSEQRSTEVR);
@@ -1288,7 +1288,7 @@ static void etm4_cpu_restore(struct etmv4_drvdata *drvdata) writel_relaxed(state->trcvdsacctlr, drvdata->base + TRCVDSACCTLR); writel_relaxed(state->trcvdarcctlr, drvdata->base + TRCVDARCCTLR);
for (i = 0; i < drvdata->nrseqstate; i++)
for (i = 0; i < drvdata->nrseqstate - 1; i++) writel_relaxed(state->trcseqevr[i], drvdata->base + TRCSEQEVRn(i));
-- 1.9.1
-- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK