Re: [PATCH v2 3/4] coresight: etm3x: Deal with CLAIM tag before and after accessing HW

[Suzuki K Poulose]

On 11/09/2018 05:59 PM, Mathieu Poirier wrote:

On Thu, 8 Nov 2018 at 10:13, Suzuki K Poulose <suzuki.poul...@arm.com> wrote:

Hi,

On 07/11/2018 23:08, Mathieu Poirier wrote:

This patch moves access to the CLAIM tag so that no modification to the HW
happens before and after the CLAIM operation has been carried.

Signed-off-by: Mathieu Poirier <mathieu.poir...@linaro.org>
---
   drivers/hwtracing/coresight/coresight-etm3x.c | 16 ++++++++--------
   1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm3x.c 
b/drivers/hwtracing/coresight/coresight-etm3x.c
index fd5c4cca7db5..4f638d81a66a 100644
--- a/drivers/hwtracing/coresight/coresight-etm3x.c
+++ b/drivers/hwtracing/coresight/coresight-etm3x.c
@@ -363,15 +363,16 @@ static int etm_enable_hw(struct etm_drvdata *drvdata)

       CS_UNLOCK(drvdata->base);

+     rc = coresight_claim_device_unlocked(drvdata->base);
+     if (rc)
+             goto done;
+
       /* Turn engine on */
       etm_clr_pwrdwn(drvdata);
       /* Apply power to trace registers */
       etm_set_pwrup(drvdata);
       /* Make sure all registers are accessible */
       etm_os_unlock(drvdata);
-     rc = coresight_claim_device_unlocked(drvdata->base);
-     if (rc)
-             goto done;

       etm_set_prog(drvdata);

@@ -422,12 +423,11 @@ static int etm_enable_hw(struct etm_drvdata *drvdata)
       etm_clr_prog(drvdata);

   done:
-     if (rc)
-             etm_set_pwrdwn(drvdata);
       CS_LOCK(drvdata->base);

-     dev_dbg(drvdata->dev, "cpu: %d enable smp call done: %d\n",
-             drvdata->cpu, rc);
+     if (!rc)
+             dev_dbg(drvdata->dev, "cpu: %d enable smp call done: %d\n",
+                     drvdata->cpu, rc);

Isn't it good to report the failure case too ? Anyway it is dev_dbg and
will be a useful info when we debug issues. Otherwise,

Simply removing the "if (!rc)" will do the trick.  Can I do the
modification and add your tag or you prefer to see another revision?

I am fine with that change folded in, no need to respin it.

Reviewed-by: Suzuki K Poulose <suzuki.poul...@arm.com>

Cheers
Suzuki

       return rc;
   }

@@ -577,9 +577,9 @@ static void etm_disable_hw(void *info)
       for (i = 0; i < drvdata->nr_cntr; i++)
               config->cntr_val[i] = etm_readl(drvdata, ETMCNTVRn(i));

+     etm_set_pwrdwn(drvdata);
       coresight_disclaim_device_unlocked(drvdata->base);

-     etm_set_pwrdwn(drvdata);
       CS_LOCK(drvdata->base);

       dev_dbg(drvdata->dev, "cpu: %d disable smp call done\n", drvdata->cpu);