Hi Mathieu,
On 2020-05-22 23:10, Mathieu Poirier wrote:
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.
Mike was ok with suggested-by, so I will go with that.
.../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.
Sure will add that.
};
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 areremoved,
* 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().
Sure will add in next version.
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".
Yes I understand it is quite ugly, but AFAIK we do not have any SoCs already released and coming in near future with such limitations in funnels or other components. So I will stick to the replicator specific property.
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 for reviewing these patches.
Thanks, Sai