Re: Coresight causes synchronous external abort on msm8916

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Suzuki,

On 6/20/2019 2:36 PM, Suzuki K Poulose wrote:


We are not yet there in the Coresight driver and we crash at AMBA bus layer
trying to read the PID of the CoreSight device. So I doubt if this is an
issue your patch trying to address. I still think this is a debug power domain
issue. More your patch below.

Yes, I suppose you are right. Just for testing, I had disabled psci
enable method for non boot cpus on msm8916 and it just crashed without
any traces. So, I thought maybe that could have been a reason for Stephan's crash as well.


like cpu affinity issue. Can you please try out this patch and let us
know?

In general I am for the patch, breaking the "assumption" that a missing CPU
phandle gives you the affinity of "0".


diff --git a/drivers/hwtracing/coresight/coresight-cpu-debug.c
b/drivers/hwtracing/coresight/coresight-cpu-debug.c
index e8819d750938..9acf9f190d42 100644
--- a/drivers/hwtracing/coresight/coresight-cpu-debug.c
+++ b/drivers/hwtracing/coresight/coresight-cpu-debug.c
@@ -579,7 +579,11 @@ static int debug_probe(struct amba_device *adev,
const struct amba_id *id)
       if (!drvdata)
           return -ENOMEM;

-    drvdata->cpu = np ? of_coresight_get_cpu(np) : 0;
+    drvdata->cpu = np ? of_coresight_get_cpu(np) : -ENODEV;


of_coresight_get_cpu() must be modified to return -ENODEV, rather than
defaulting to 0. This is something that is required by the CTI driver too.
And lets not bring up something and assume it belongs to CPU0.

+    if (drvdata->cpu == -ENODEV) {
+        return -ENODEV;
+    }
+
       if (per_cpu(debug_drvdata, drvdata->cpu)) {
           dev_err(dev, "CPU%d drvdata has already been initialized\n",
               drvdata->cpu);
diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c
b/drivers/hwtracing/coresight/coresight-etm4x.c
index 8bb0092c7ec2..660432acbac0 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x.c
@@ -1107,7 +1107,10 @@ static int etm4_probe(struct amba_device *adev,
const struct amba_id *id)

       spin_lock_init(&drvdata->spinlock);

-    drvdata->cpu = pdata ? pdata->cpu : 0;

I believe, we should simply abort when we don't have pdata. There is no point
in registering this ETM unless we know where this is connected to.


I did not understand this comment since I am returning with ENODEV here
and not registering this ETM.

+    drvdata->cpu = pdata ? pdata->cpu : -ENODEV;
+    if (drvdata->cpu == -ENODEV) {
+        return -ENODEV;
+       }


       cpus_read_lock();
       etmdrvdata[drvdata->cpu] = drvdata;
diff --git a/drivers/hwtracing/coresight/of_coresight.c
b/drivers/hwtracing/coresight/of_coresight.c
index 7045930fc958..8c1b90ba233c 100644
--- a/drivers/hwtracing/coresight/of_coresight.c
+++ b/drivers/hwtracing/coresight/of_coresight.c
@@ -153,14 +153,14 @@ int of_coresight_get_cpu(const struct device_node
*node)
       struct device_node *dn;

       dn = of_parse_phandle(node, "cpu", 0);
-    /* Affinity defaults to CPU0 */
+    /* Affinity defaults to invalid */
       if (!dn)
-        return 0;
+        return -ENODEV;
       cpu = of_cpu_node_to_id(dn);
       of_node_put(dn);

-    /* Affinity to CPU0 if no cpu nodes are found */
-    return (cpu < 0) ? 0 : cpu;
+    /* Affinity to invalid if no cpu nodes are found */
+    return (cpu < 0) ? -ENODEV : cpu;

     return cpu ?

If you split this into 3 different patches, I would be happy to Ack them.


Sure, I will ready the patches.

Thanks,
Sai

Mathieu,

What do you think ?


Cheers
Suzuki

--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux