Hi Sai,
On Tue, May 19, 2020 at 07:06:02PM +0530, Sai Prakash Ranjan wrote:
On some QCOM SoCs, replicators in Always-On domain loses its context as soon as the clock is disabled. Currently as a part of pm_runtime workqueue, clock is disabled after the replicator is initialized by amba_pm_runtime_suspend assuming that context is not lost which is not true for replicators with such limitations. So add a new property "qcom,replicator-loses-context" to identify such replicators and reset them.
Suggested-by: Mike Leach mike.leach@linaro.org Signed-off-by: Sai Prakash Ranjan saiprakash.ranjan@codeaurora.org
Added Mike's suggested by for parts other than the DT property. Perhaps I should add Co-developed-by Mike since the full skeletal was given by Mike. I can add that if required on the next version.
I will let Mike decide what he wants to do - I'm fine either way.
.../coresight/coresight-replicator.c | 53 +++++++++++++------ 1 file changed, 36 insertions(+), 17 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-replicator.c b/drivers/hwtracing/coresight/coresight-replicator.c index c619b456f55a..ba66160c8140 100644 --- a/drivers/hwtracing/coresight/coresight-replicator.c +++ b/drivers/hwtracing/coresight/coresight-replicator.c @@ -38,6 +38,7 @@ struct replicator_drvdata { struct clk *atclk; struct coresight_device *csdev; spinlock_t spinlock;
- bool check_idfilter_val;
Please add documentation for the new field, the same way other fields are documented.
}; static void dynamic_replicator_reset(struct replicator_drvdata *drvdata) @@ -66,29 +67,43 @@ static int dynamic_replicator_enable(struct replicator_drvdata *drvdata, int inport, int outport) { int rc = 0;
- u32 reg;
- switch (outport) {
- case 0:
reg = REPLICATOR_IDFILTER0;
break;
- case 1:
reg = REPLICATOR_IDFILTER1;
break;
- default:
WARN_ON(1);
return -EINVAL;
- }
- u32 id0val, id1val;
CS_UNLOCK(drvdata->base);
- if ((readl_relaxed(drvdata->base + REPLICATOR_IDFILTER0) == 0xff) &&
(readl_relaxed(drvdata->base + REPLICATOR_IDFILTER1) == 0xff))
- id0val = readl_relaxed(drvdata->base + REPLICATOR_IDFILTER0);
- id1val = readl_relaxed(drvdata->base + REPLICATOR_IDFILTER1);
- /*
* Some replicator designs lose context when AMBA clocks are removed,
* so have a check for this.
*/
- if (drvdata->check_idfilter_val && id0val == 0x0 && id1val == 0x0)
id0val = id1val = 0xff;
- if (id0val == 0xff && id1val == 0xff) rc = coresight_claim_device_unlocked(drvdata->base);
- if (!rc) {
switch (outport) {
case 0:
id0val = 0x0;
break;
case 1:
id1val = 0x0;
break;
default:
WARN_ON(1);
rc = -EINVAL;
}
- }
- /* Ensure that the outport is enabled. */
- if (!rc)
writel_relaxed(0x00, drvdata->base + reg);
- if (!rc) {
writel_relaxed(id0val, drvdata->base + REPLICATOR_IDFILTER0);
writel_relaxed(id1val, drvdata->base + REPLICATOR_IDFILTER1);
- }
- CS_LOCK(drvdata->base);
return rc; @@ -239,6 +254,10 @@ static int replicator_probe(struct device *dev, struct resource *res) desc.groups = replicator_groups; }
- if (fwnode_property_present(dev_fwnode(dev),
"qcom,replicator-loses-context"))
drvdata->check_idfilter_val = true;
The header <linux/property.h> needs to be added for function fwnode_property_present().
What is the clock situation with other QC components like funnels? Have they also been designed the same way? If so the binding should probably be "qcom,component-loses-context", otherwise what you have suggested will work just fine. My goal here is to avoid having "qcom,replicator-loses-context" and "qcom,funnel-loses-context".
Lastly, I have applied patch 1 and 2 of this set to my tree so no need to resend them again with the next revision.
Thanks, Mathieu
dev_set_drvdata(dev, drvdata); pdata = coresight_get_platform_data(dev); -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation