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
