| Message ID | d93e28fc80227f9a385130a766a24f8f39a1dcf0.1561037262.git.saiprakash.ranjan@codeaurora.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | coresight: Set affinity to invalid for missing CPU phandle | expand |
On 20/06/2019 14:45, Sai Prakash Ranjan wrote: > Currently the coresight etm and cpu-debug drivers > assume the affinity to CPU0 returned by coresight > platform and continue the probe in case of missing > CPU phandle. This is not true and leads to crash > in some cases, so abort the probe in case of missing > CPU phandle. > > Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org> > --- > drivers/hwtracing/coresight/coresight-cpu-debug.c | 3 +++ > drivers/hwtracing/coresight/coresight-etm3x.c | 3 +++ > drivers/hwtracing/coresight/coresight-etm4x.c | 3 +++ > 3 files changed, 9 insertions(+) > > diff --git a/drivers/hwtracing/coresight/coresight-cpu-debug.c b/drivers/hwtracing/coresight/coresight-cpu-debug.c > index 07a1367c733f..43f32fa71ff9 100644 > --- a/drivers/hwtracing/coresight/coresight-cpu-debug.c > +++ b/drivers/hwtracing/coresight/coresight-cpu-debug.c > @@ -579,6 +579,9 @@ static int debug_probe(struct amba_device *adev, const struct amba_id *id) > return -ENOMEM; > > drvdata->cpu = coresight_get_cpu(dev); > + if (drvdata->cpu == -ENODEV) > + return -ENODEV; if (drvdata->cpu < 0) return drvdata->cpu; Same everywhere below ? Also, I would like to hear Mathieu's thoughts on this change. If he's OK with it: Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com> with the change above.
On 6/20/2019 7:28 PM, Suzuki K Poulose wrote: > > > On 20/06/2019 14:45, Sai Prakash Ranjan wrote: >> Currently the coresight etm and cpu-debug drivers >> assume the affinity to CPU0 returned by coresight >> platform and continue the probe in case of missing >> CPU phandle. This is not true and leads to crash >> in some cases, so abort the probe in case of missing >> CPU phandle. >> >> Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org> >> --- >> drivers/hwtracing/coresight/coresight-cpu-debug.c | 3 +++ >> drivers/hwtracing/coresight/coresight-etm3x.c | 3 +++ >> drivers/hwtracing/coresight/coresight-etm4x.c | 3 +++ >> 3 files changed, 9 insertions(+) >> >> diff --git a/drivers/hwtracing/coresight/coresight-cpu-debug.c >> b/drivers/hwtracing/coresight/coresight-cpu-debug.c >> index 07a1367c733f..43f32fa71ff9 100644 >> --- a/drivers/hwtracing/coresight/coresight-cpu-debug.c >> +++ b/drivers/hwtracing/coresight/coresight-cpu-debug.c >> @@ -579,6 +579,9 @@ static int debug_probe(struct amba_device *adev, >> const struct amba_id *id) >> return -ENOMEM; >> drvdata->cpu = coresight_get_cpu(dev); >> + if (drvdata->cpu == -ENODEV) >> + return -ENODEV; > > if (drvdata->cpu < 0) > return drvdata->cpu; > > Same everywhere below ? > > Also, I would like to hear Mathieu's thoughts on this change. If he's OK > with it: > > Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com> with the change > above. > > Thanks, I will make the change and repost. -Sai
On 20/06/2019 15:55, Sai Prakash Ranjan wrote: > On 6/20/2019 7:28 PM, Suzuki K Poulose wrote: >> >> >> On 20/06/2019 14:45, Sai Prakash Ranjan wrote: >>> Currently the coresight etm and cpu-debug drivers >>> assume the affinity to CPU0 returned by coresight >>> platform and continue the probe in case of missing >>> CPU phandle. This is not true and leads to crash >>> in some cases, so abort the probe in case of missing >>> CPU phandle. >>> >>> Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org> >>> --- >>> drivers/hwtracing/coresight/coresight-cpu-debug.c | 3 +++ >>> drivers/hwtracing/coresight/coresight-etm3x.c | 3 +++ >>> drivers/hwtracing/coresight/coresight-etm4x.c | 3 +++ >>> 3 files changed, 9 insertions(+) >>> >>> diff --git a/drivers/hwtracing/coresight/coresight-cpu-debug.c >>> b/drivers/hwtracing/coresight/coresight-cpu-debug.c >>> index 07a1367c733f..43f32fa71ff9 100644 >>> --- a/drivers/hwtracing/coresight/coresight-cpu-debug.c >>> +++ b/drivers/hwtracing/coresight/coresight-cpu-debug.c >>> @@ -579,6 +579,9 @@ static int debug_probe(struct amba_device *adev, >>> const struct amba_id *id) >>> return -ENOMEM; >>> drvdata->cpu = coresight_get_cpu(dev); >>> + if (drvdata->cpu == -ENODEV) >>> + return -ENODEV; >> >> if (drvdata->cpu < 0) >> return drvdata->cpu; >> >> Same everywhere below ? >> >> Also, I would like to hear Mathieu's thoughts on this change. If he's OK >> with it: >> >> Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com> with the change >> above. >> >> > > Thanks, I will make the change and repost. Please wait for Mathieu's thoughts on it. And in general I would wait for feedback from the people in a version, before posting another one, to reduce the number of respins. Cheers Suzuki
On 6/20/2019 8:53 PM, Suzuki K Poulose wrote: > > > > Please wait for Mathieu's thoughts on it. And in general I would wait > for feedback from the people in a version, before posting another one, > to reduce the number of respins. > Mathieu already said he was OK in the other thread, but I will wait for some more feedbacks. Thanks, Sai
On Thu, Jun 20, 2019 at 07:15:47PM +0530, Sai Prakash Ranjan wrote: > Currently the coresight etm and cpu-debug drivers > assume the affinity to CPU0 returned by coresight > platform and continue the probe in case of missing > CPU phandle. This is not true and leads to crash > in some cases, so abort the probe in case of missing > CPU phandle. > > Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org> > --- > drivers/hwtracing/coresight/coresight-cpu-debug.c | 3 +++ > drivers/hwtracing/coresight/coresight-etm3x.c | 3 +++ > drivers/hwtracing/coresight/coresight-etm4x.c | 3 +++ > 3 files changed, 9 insertions(+) > > diff --git a/drivers/hwtracing/coresight/coresight-cpu-debug.c b/drivers/hwtracing/coresight/coresight-cpu-debug.c > index 07a1367c733f..43f32fa71ff9 100644 > --- a/drivers/hwtracing/coresight/coresight-cpu-debug.c > +++ b/drivers/hwtracing/coresight/coresight-cpu-debug.c > @@ -579,6 +579,9 @@ static int debug_probe(struct amba_device *adev, const struct amba_id *id) > return -ENOMEM; > > drvdata->cpu = coresight_get_cpu(dev); > + if (drvdata->cpu == -ENODEV) > + return -ENODEV; As Suzuki pointed out, simply return the error message conveyed by coresight_get_cpu(). Also please merge both patches together to avoid bisect nightmare. Thank you for the contribution, Mathieu > + > 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-etm3x.c b/drivers/hwtracing/coresight/coresight-etm3x.c > index 225c2982e4fe..882e2751746c 100644 > --- a/drivers/hwtracing/coresight/coresight-etm3x.c > +++ b/drivers/hwtracing/coresight/coresight-etm3x.c > @@ -816,6 +816,9 @@ static int etm_probe(struct amba_device *adev, const struct amba_id *id) > } > > drvdata->cpu = coresight_get_cpu(dev); > + if (drvdata->cpu == -ENODEV) > + return -ENODEV; > + > desc.name = devm_kasprintf(dev, GFP_KERNEL, "etm%d", drvdata->cpu); > if (!desc.name) > return -ENOMEM; > diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c b/drivers/hwtracing/coresight/coresight-etm4x.c > index 7fe266194ab5..97d71dbbeb19 100644 > --- a/drivers/hwtracing/coresight/coresight-etm4x.c > +++ b/drivers/hwtracing/coresight/coresight-etm4x.c > @@ -1101,6 +1101,9 @@ static int etm4_probe(struct amba_device *adev, const struct amba_id *id) > spin_lock_init(&drvdata->spinlock); > > drvdata->cpu = coresight_get_cpu(dev); > + if (drvdata->cpu == -ENODEV) > + return -ENODEV; > + > desc.name = devm_kasprintf(dev, GFP_KERNEL, "etm%d", drvdata->cpu); > if (!desc.name) > return -ENOMEM; > -- > QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member > of Code Aurora Forum, hosted by The Linux Foundation >
Hi Mathieu, Thanks for the review comments. On 6/20/2019 11:13 PM, Mathieu Poirier wrote: > On Thu, Jun 20, 2019 at 07:15:47PM +0530, Sai Prakash Ranjan wrote: >> diff --git a/drivers/hwtracing/coresight/coresight-cpu-debug.c b/drivers/hwtracing/coresight/coresight-cpu-debug.c >> index 07a1367c733f..43f32fa71ff9 100644 >> --- a/drivers/hwtracing/coresight/coresight-cpu-debug.c >> +++ b/drivers/hwtracing/coresight/coresight-cpu-debug.c >> @@ -579,6 +579,9 @@ static int debug_probe(struct amba_device *adev, const struct amba_id *id) >> return -ENOMEM; >> >> drvdata->cpu = coresight_get_cpu(dev); >> + if (drvdata->cpu == -ENODEV) >> + return -ENODEV; > > As Suzuki pointed out, simply return the error message conveyed by > coresight_get_cpu(). > > Also please merge both patches together to avoid bisect nightmare. Sure, will do it.
diff --git a/drivers/hwtracing/coresight/coresight-cpu-debug.c b/drivers/hwtracing/coresight/coresight-cpu-debug.c index 07a1367c733f..43f32fa71ff9 100644 --- a/drivers/hwtracing/coresight/coresight-cpu-debug.c +++ b/drivers/hwtracing/coresight/coresight-cpu-debug.c @@ -579,6 +579,9 @@ static int debug_probe(struct amba_device *adev, const struct amba_id *id) return -ENOMEM; drvdata->cpu = coresight_get_cpu(dev); + 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-etm3x.c b/drivers/hwtracing/coresight/coresight-etm3x.c index 225c2982e4fe..882e2751746c 100644 --- a/drivers/hwtracing/coresight/coresight-etm3x.c +++ b/drivers/hwtracing/coresight/coresight-etm3x.c @@ -816,6 +816,9 @@ static int etm_probe(struct amba_device *adev, const struct amba_id *id) } drvdata->cpu = coresight_get_cpu(dev); + if (drvdata->cpu == -ENODEV) + return -ENODEV; + desc.name = devm_kasprintf(dev, GFP_KERNEL, "etm%d", drvdata->cpu); if (!desc.name) return -ENOMEM; diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c b/drivers/hwtracing/coresight/coresight-etm4x.c index 7fe266194ab5..97d71dbbeb19 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x.c +++ b/drivers/hwtracing/coresight/coresight-etm4x.c @@ -1101,6 +1101,9 @@ static int etm4_probe(struct amba_device *adev, const struct amba_id *id) spin_lock_init(&drvdata->spinlock); drvdata->cpu = coresight_get_cpu(dev); + if (drvdata->cpu == -ENODEV) + return -ENODEV; + desc.name = devm_kasprintf(dev, GFP_KERNEL, "etm%d", drvdata->cpu); if (!desc.name) return -ENOMEM;
Currently the coresight etm and cpu-debug drivers assume the affinity to CPU0 returned by coresight platform and continue the probe in case of missing CPU phandle. This is not true and leads to crash in some cases, so abort the probe in case of missing CPU phandle. Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org> --- drivers/hwtracing/coresight/coresight-cpu-debug.c | 3 +++ drivers/hwtracing/coresight/coresight-etm3x.c | 3 +++ drivers/hwtracing/coresight/coresight-etm4x.c | 3 +++ 3 files changed, 9 insertions(+)