Hi Suzuki, Mike,
On 2020-05-06 13:05, Sai Prakash Ranjan wrote:
[...]
OK - sorry I read your statement saying that replicator1 was 0 after
the reset in probe(), rather than look at the logs.
From the logs it is working at the time probe() occurs, but by the
time we come to enable the replicator later, something has reset
these
registers / hardware outside the control of the replicator driver.
Yes, I will try to get some more information from the firmware side if
there is anything messing up.
This turned out to be a clock/pm issue. To confirm, I just marked clk
as critical
so that it won't be gated and I saw the replicator1(swao_replicator)
registers
intact after probe. Also alternatively, I tried to comment out
disabling pclk
to check if there is something wrong in amba pm and this keeps the
registers
intact as well.
@@ -288,7 +295,7 @@ static int amba_probe(struct device *dev)
pm_runtime_set_suspended(dev);
pm_runtime_put_noidle(dev);
- amba_put_disable_pclk(pcdev);
+ //amba_put_disable_pclk(pcdev);
dev_pm_domain_detach(dev, true);
} while (0);
I checked with the debug team and there is a limitation with
the replicator(swao_replicator) in the AOSS group where it
loses the idfilter register context when the clock is disabled.
This is not just in SC7180 SoC but also reported on some latest
upcoming QCOM SoCs as well and will need to be taken care in
order to enable coresight on these chipsets.
Here's what's happening - After the replicator is initialized,
the clock is disabled in amba_pm_runtime_suspend() as a part of
pm runtime workqueue with the assumption that there will be no
loss of context after the replicator is initialized. But it doesn't
hold good with the replicators with these unfortunate limitation
and the idfilter register context is lost.
[ 5.889406] amba_pm_runtime_suspend devname=6b06000.replicator ret=0
[ 5.914516] Workqueue: pm pm_runtime_work
[ 5.918648] Call trace:
[ 5.921185] dump_backtrace+0x0/0x1d0
[ 5.924958] show_stack+0x2c/0x38
[ 5.928382] dump_stack+0xc0/0x104
[ 5.931896] amba_pm_runtime_suspend+0xd8/0xe0
[ 5.936469] __rpm_callback+0xe0/0x140
[ 5.940332] rpm_callback+0x38/0x98
[ 5.943926] rpm_suspend+0xec/0x618
[ 5.947522] rpm_idle+0x5c/0x3f8
[ 5.950851] pm_runtime_work+0xa8/0xc0
[ 5.954718] process_one_work+0x1f8/0x4c0
[ 5.958848] worker_thread+0x50/0x468
[ 5.962623] kthread+0x12c/0x158
[ 5.965957] ret_from_fork+0x10/0x1c
This is a platform/SoC specific replicator issue, so we can either
introduce some DT property for replicators to identify which replicator
has this limitation, check in replicator_enable() and reset the
registers
or have something like below diff to check the idfilter registers in
replicator_enable() and then reset with clear comment specifying it’s
the
hardware limitation on some QCOM SoCs. Please let me know your thoughts
on
this?
diff --git a/drivers/hwtracing/coresight/coresight-replicator.c
b/drivers/hwtracing/coresight/coresight-replicator.c
index e7dc1c31d20d..a9c039c944eb 100644
--- a/drivers/hwtracing/coresight/coresight-replicator.c
+++ b/drivers/hwtracing/coresight/coresight-replicator.c
@@ -68,6 +68,17 @@ static int dynamic_replicator_enable(struct
replicator_drvdata *drvdata,
int rc = 0;
u32 reg;
+ /*
+ * On some QCOM SoCs with replicators in Always-On domain,
disabling
+ * clock will result in replicator losing its context. Currently
+ * as a part of pm_runtime workqueue, amba_pm_runtime_suspend
disables
+ * clock assuming the context is not lost which is not true for
cases
+ * with hardware limitations as the above.
+ */
+ if ((readl_relaxed(drvdata->base + REPLICATOR_IDFILTER0) == 0)
&&
+ (readl_relaxed(drvdata->base + REPLICATOR_IDFILTER1) == 0))
+ dynamic_replicator_reset(drvdata);
+
switch (outport) {
case 0:
reg = REPLICATOR_IDFILTER0;
Thanks,
Sai
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member
of Code Aurora Forum, hosted by The Linux Foundation
