lkml.org 
[lkml]   [2020]   [May]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCHv2 3/4] coresight: replicator: Reset replicator if context is lost
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 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().
>

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
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member
of Code Aurora Forum, hosted by The Linux Foundation

\
 
 \ /
  Last update: 2020-05-22 19:56    [W:0.071 / U:3.572 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site