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 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
    >

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