* [PATCH 0/2] coresight: Set affinity to invalid for missing CPU phandle @ 2019-06-20 13:45 Sai Prakash Ranjan 2019-06-20 13:45 ` [PATCH 1/2] " Sai Prakash Ranjan 2019-06-20 13:45 ` [PATCH 2/2] coresight: Abort probe " Sai Prakash Ranjan 0 siblings, 2 replies; 13+ messages in thread From: Sai Prakash Ranjan @ 2019-06-20 13:45 UTC (permalink / raw) To: Mathieu Poirier, Suzuki K Poulose, Leo Yan, Alexander Shishkin, Andy Gross, David Brown, Mark Rutland Cc: Rajendra Nayak, Vivek Gautam, Sibi Sankar, linux-arm-kernel, linux-kernel, linux-arm-msm, Sai Prakash Ranjan In case of missing CPU phandle, the affinity is set default to CPU0 which is not a correct assumption and leads to crashes in few cases. Fix this by returning -ENODEV in coresight platform and abort the probe in coresight etm and cpu-debug drivers. Sai Prakash Ranjan (2): coresight: Set affinity to invalid for missing CPU phandle coresight: Abort probe for missing CPU phandle drivers/hwtracing/coresight/coresight-cpu-debug.c | 3 +++ drivers/hwtracing/coresight/coresight-etm3x.c | 3 +++ drivers/hwtracing/coresight/coresight-etm4x.c | 3 +++ drivers/hwtracing/coresight/coresight-platform.c | 10 ++++++---- 4 files changed, 15 insertions(+), 4 deletions(-) -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 1/2] coresight: Set affinity to invalid for missing CPU phandle 2019-06-20 13:45 [PATCH 0/2] coresight: Set affinity to invalid for missing CPU phandle Sai Prakash Ranjan @ 2019-06-20 13:45 ` Sai Prakash Ranjan 2019-06-20 13:55 ` Suzuki K Poulose 2019-06-20 17:39 ` Mathieu Poirier 2019-06-20 13:45 ` [PATCH 2/2] coresight: Abort probe " Sai Prakash Ranjan 1 sibling, 2 replies; 13+ messages in thread From: Sai Prakash Ranjan @ 2019-06-20 13:45 UTC (permalink / raw) To: Mathieu Poirier, Suzuki K Poulose, Leo Yan, Alexander Shishkin, Andy Gross, David Brown, Mark Rutland Cc: Rajendra Nayak, Vivek Gautam, Sibi Sankar, linux-arm-kernel, linux-kernel, linux-arm-msm, Sai Prakash Ranjan Affinity defaults to CPU0 in case of missing CPU phandle and this leads to crashes in some cases because of such wrong assumption. Fix this by returning -ENODEV in coresight platform for such cases and then handle it in the coresight drivers. Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org> --- drivers/hwtracing/coresight/coresight-platform.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/hwtracing/coresight/coresight-platform.c b/drivers/hwtracing/coresight/coresight-platform.c index 3c5ceda8db24..b1ea60c210e1 100644 --- a/drivers/hwtracing/coresight/coresight-platform.c +++ b/drivers/hwtracing/coresight/coresight-platform.c @@ -160,15 +160,17 @@ static int of_coresight_get_cpu(struct device *dev) if (!dev->of_node) return 0; + dn = of_parse_phandle(dev->of_node, "cpu", 0); - /* Affinity defaults to CPU0 */ + + /* Affinity defaults to invalid if no cpu nodes are found*/ 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; + return cpu; } /* -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] coresight: Set affinity to invalid for missing CPU phandle 2019-06-20 13:45 ` [PATCH 1/2] " Sai Prakash Ranjan @ 2019-06-20 13:55 ` Suzuki K Poulose 2019-06-20 14:54 ` Sai Prakash Ranjan 2019-06-20 17:39 ` Mathieu Poirier 1 sibling, 1 reply; 13+ messages in thread From: Suzuki K Poulose @ 2019-06-20 13:55 UTC (permalink / raw) To: saiprakash.ranjan, mathieu.poirier, leo.yan, alexander.shishkin, andy.gross, david.brown, mark.rutland Cc: rnayak, vivek.gautam, sibis, linux-arm-kernel, linux-kernel, linux-arm-msm Sai, Thanks for the patch. Please could you change the subject to : "coresight: Do not default to CPU0 for missing CPU phandle" On 20/06/2019 14:45, Sai Prakash Ranjan wrote: > Affinity defaults to CPU0 in case of missing CPU phandle > and this leads to crashes in some cases because of such > wrong assumption. Fix this by returning -ENODEV in Thats not the right justification. Causing crashes is due to bad DT/firmware. I would be happy with something like : "Coresight platform support assumes that a missing \"cpu\" phandle defaults to CPU0. This could be problematic and unnecessarily binds components to CPU0, where they may not be. Let us make the DT binding rules a bit stricter by not defaulting to CPU0 for missing "cpu" affinity information." Also, you must 1) update the devicetree/bindings document to reflect the same. 2) update the drivers to take appropriate action on the missing CPU where they are expected (e.g, CPU-debug, etm*), to prevent breaking a bisect. > coresight platform for such cases and then handle it > in the coresight drivers. > > Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org> > --- > drivers/hwtracing/coresight/coresight-platform.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-platform.c b/drivers/hwtracing/coresight/coresight-platform.c > index 3c5ceda8db24..b1ea60c210e1 100644 > --- a/drivers/hwtracing/coresight/coresight-platform.c > +++ b/drivers/hwtracing/coresight/coresight-platform.c > @@ -160,15 +160,17 @@ static int of_coresight_get_cpu(struct device *dev) > > if (!dev->of_node) > return 0; > + > dn = of_parse_phandle(dev->of_node, "cpu", 0); > - /* Affinity defaults to CPU0 */ > + > + /* Affinity defaults to invalid if no cpu nodes are found*/ The code is self explanatory here. You could drop the comment. > 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; > + return cpu; > } > Suzuki ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] coresight: Set affinity to invalid for missing CPU phandle 2019-06-20 13:55 ` Suzuki K Poulose @ 2019-06-20 14:54 ` Sai Prakash Ranjan 0 siblings, 0 replies; 13+ messages in thread From: Sai Prakash Ranjan @ 2019-06-20 14:54 UTC (permalink / raw) To: Suzuki K Poulose, mathieu.poirier, leo.yan, alexander.shishkin, andy.gross, david.brown, mark.rutland Cc: rnayak, vivek.gautam, sibis, linux-arm-kernel, linux-kernel, linux-arm-msm Hi Suzuki, Thanks for the review. On 6/20/2019 7:25 PM, Suzuki K Poulose wrote: > > Sai, > > Thanks for the patch. Please could you change the subject to : > > "coresight: Do not default to CPU0 for missing CPU phandle" > Sure. > On 20/06/2019 14:45, Sai Prakash Ranjan wrote: >> Affinity defaults to CPU0 in case of missing CPU phandle >> and this leads to crashes in some cases because of such >> wrong assumption. Fix this by returning -ENODEV in > > Thats not the right justification. Causing crashes is due to > bad DT/firmware. I would be happy with something like : > > "Coresight platform support assumes that a missing \"cpu\" phandle > defaults to CPU0. This could be problematic and unnecessarily binds > components to CPU0, where they may not be. Let us make the DT binding > rules a bit stricter by not defaulting to CPU0 for missing "cpu" > affinity information." > > Also, you must > > 1) update the devicetree/bindings document to reflect the same. > 2) update the drivers to take appropriate action on the missing CPU > where they are expected (e.g, CPU-debug, etm*), to prevent > breaking a bisect. > > Sure will do it and repost. >> coresight platform for such cases and then handle it >> in the coresight drivers. >> >> Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org> >> --- >> drivers/hwtracing/coresight/coresight-platform.c | 10 ++++++---- >> 1 file changed, 6 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/hwtracing/coresight/coresight-platform.c >> b/drivers/hwtracing/coresight/coresight-platform.c >> index 3c5ceda8db24..b1ea60c210e1 100644 >> --- a/drivers/hwtracing/coresight/coresight-platform.c >> +++ b/drivers/hwtracing/coresight/coresight-platform.c >> @@ -160,15 +160,17 @@ static int of_coresight_get_cpu(struct device *dev) >> if (!dev->of_node) >> return 0; >> + >> dn = of_parse_phandle(dev->of_node, "cpu", 0); >> - /* Affinity defaults to CPU0 */ >> + >> + /* Affinity defaults to invalid if no cpu nodes are found*/ > > The code is self explanatory here. You could drop the comment. > Sure. >> 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; >> + return cpu; >> } > > Suzuki -Sai -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] coresight: Set affinity to invalid for missing CPU phandle 2019-06-20 13:45 ` [PATCH 1/2] " Sai Prakash Ranjan 2019-06-20 13:55 ` Suzuki K Poulose @ 2019-06-20 17:39 ` Mathieu Poirier 2019-06-20 18:05 ` Sai Prakash Ranjan 1 sibling, 1 reply; 13+ messages in thread From: Mathieu Poirier @ 2019-06-20 17:39 UTC (permalink / raw) To: Sai Prakash Ranjan Cc: Suzuki K Poulose, Leo Yan, Alexander Shishkin, Andy Gross, David Brown, Mark Rutland, Rajendra Nayak, Vivek Gautam, Sibi Sankar, linux-arm-kernel, linux-kernel, linux-arm-msm Hi Sai, On Thu, Jun 20, 2019 at 07:15:46PM +0530, Sai Prakash Ranjan wrote: > Affinity defaults to CPU0 in case of missing CPU phandle > and this leads to crashes in some cases because of such > wrong assumption. Fix this by returning -ENODEV in > coresight platform for such cases and then handle it > in the coresight drivers. > > Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org> > --- > drivers/hwtracing/coresight/coresight-platform.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-platform.c b/drivers/hwtracing/coresight/coresight-platform.c > index 3c5ceda8db24..b1ea60c210e1 100644 > --- a/drivers/hwtracing/coresight/coresight-platform.c > +++ b/drivers/hwtracing/coresight/coresight-platform.c > @@ -160,15 +160,17 @@ static int of_coresight_get_cpu(struct device *dev) > > if (!dev->of_node) > return 0; An error should be returned if the above condition is true. > + Spurious newline > dn = of_parse_phandle(dev->of_node, "cpu", 0); > - /* Affinity defaults to CPU0 */ > + > + /* Affinity defaults to invalid if no cpu nodes are found*/ > 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; > + return cpu; > } > > /* > -- > QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member > of Code Aurora Forum, hosted by The Linux Foundation > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] coresight: Set affinity to invalid for missing CPU phandle 2019-06-20 17:39 ` Mathieu Poirier @ 2019-06-20 18:05 ` Sai Prakash Ranjan 0 siblings, 0 replies; 13+ messages in thread From: Sai Prakash Ranjan @ 2019-06-20 18:05 UTC (permalink / raw) To: Mathieu Poirier Cc: Suzuki K Poulose, Leo Yan, Alexander Shishkin, Andy Gross, David Brown, Mark Rutland, Rajendra Nayak, Vivek Gautam, Sibi Sankar, linux-arm-kernel, linux-kernel, linux-arm-msm Hi Mathieu, Thanks for the review comments. On 6/20/2019 11:09 PM, Mathieu Poirier wrote: > Hi Sai, > > On Thu, Jun 20, 2019 at 07:15:46PM +0530, Sai Prakash Ranjan wrote: >> Affinity defaults to CPU0 in case of missing CPU phandle >> and this leads to crashes in some cases because of such >> wrong assumption. Fix this by returning -ENODEV in >> coresight platform for such cases and then handle it >> in the coresight drivers. >> >> Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org> >> --- >> drivers/hwtracing/coresight/coresight-platform.c | 10 ++++++---- >> 1 file changed, 6 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/hwtracing/coresight/coresight-platform.c b/drivers/hwtracing/coresight/coresight-platform.c >> index 3c5ceda8db24..b1ea60c210e1 100644 >> --- a/drivers/hwtracing/coresight/coresight-platform.c >> +++ b/drivers/hwtracing/coresight/coresight-platform.c >> @@ -160,15 +160,17 @@ static int of_coresight_get_cpu(struct device *dev) >> >> if (!dev->of_node) >> return 0; > > An error should be returned if the above condition is true. > Will do it, thanks. >> + > > Spurious newline > This was on purpose, the code looks much cleaner. -Sai -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/2] coresight: Abort probe for missing CPU phandle 2019-06-20 13:45 [PATCH 0/2] coresight: Set affinity to invalid for missing CPU phandle Sai Prakash Ranjan 2019-06-20 13:45 ` [PATCH 1/2] " Sai Prakash Ranjan @ 2019-06-20 13:45 ` Sai Prakash Ranjan 2019-06-20 13:58 ` Suzuki K Poulose 2019-06-20 17:43 ` Mathieu Poirier 1 sibling, 2 replies; 13+ messages in thread From: Sai Prakash Ranjan @ 2019-06-20 13:45 UTC (permalink / raw) To: Mathieu Poirier, Suzuki K Poulose, Leo Yan, Alexander Shishkin, Andy Gross, David Brown, Mark Rutland Cc: Rajendra Nayak, Vivek Gautam, Sibi Sankar, linux-arm-kernel, linux-kernel, linux-arm-msm, Sai Prakash Ranjan 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 (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 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] coresight: Abort probe for missing CPU phandle 2019-06-20 13:45 ` [PATCH 2/2] coresight: Abort probe " Sai Prakash Ranjan @ 2019-06-20 13:58 ` Suzuki K Poulose 2019-06-20 14:55 ` Sai Prakash Ranjan 2019-06-20 17:43 ` Mathieu Poirier 1 sibling, 1 reply; 13+ messages in thread From: Suzuki K Poulose @ 2019-06-20 13:58 UTC (permalink / raw) To: saiprakash.ranjan, mathieu.poirier, leo.yan, alexander.shishkin, david.brown, mark.rutland Cc: rnayak, vivek.gautam, sibis, linux-arm-kernel, linux-kernel, linux-arm-msm 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. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] coresight: Abort probe for missing CPU phandle 2019-06-20 13:58 ` Suzuki K Poulose @ 2019-06-20 14:55 ` Sai Prakash Ranjan 2019-06-20 15:23 ` Suzuki K Poulose 0 siblings, 1 reply; 13+ messages in thread From: Sai Prakash Ranjan @ 2019-06-20 14:55 UTC (permalink / raw) To: Suzuki K Poulose, mathieu.poirier, leo.yan, alexander.shishkin, david.brown, mark.rutland Cc: rnayak, vivek.gautam, sibis, linux-arm-kernel, linux-kernel, linux-arm-msm 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 -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] coresight: Abort probe for missing CPU phandle 2019-06-20 14:55 ` Sai Prakash Ranjan @ 2019-06-20 15:23 ` Suzuki K Poulose 2019-06-20 16:10 ` Sai Prakash Ranjan 0 siblings, 1 reply; 13+ messages in thread From: Suzuki K Poulose @ 2019-06-20 15:23 UTC (permalink / raw) To: saiprakash.ranjan, mathieu.poirier, leo.yan, alexander.shishkin, david.brown, mark.rutland Cc: rnayak, vivek.gautam, sibis, linux-arm-kernel, linux-kernel, linux-arm-msm 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] coresight: Abort probe for missing CPU phandle 2019-06-20 15:23 ` Suzuki K Poulose @ 2019-06-20 16:10 ` Sai Prakash Ranjan 0 siblings, 0 replies; 13+ messages in thread From: Sai Prakash Ranjan @ 2019-06-20 16:10 UTC (permalink / raw) To: Suzuki K Poulose, mathieu.poirier, leo.yan, alexander.shishkin, david.brown, mark.rutland Cc: rnayak, vivek.gautam, sibis, linux-arm-kernel, linux-kernel, linux-arm-msm 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 -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] coresight: Abort probe for missing CPU phandle 2019-06-20 13:45 ` [PATCH 2/2] coresight: Abort probe " Sai Prakash Ranjan 2019-06-20 13:58 ` Suzuki K Poulose @ 2019-06-20 17:43 ` Mathieu Poirier 2019-06-20 18:07 ` Sai Prakash Ranjan 1 sibling, 1 reply; 13+ messages in thread From: Mathieu Poirier @ 2019-06-20 17:43 UTC (permalink / raw) To: Sai Prakash Ranjan Cc: Suzuki K Poulose, Leo Yan, Alexander Shishkin, Andy Gross, David Brown, Mark Rutland, Rajendra Nayak, Vivek Gautam, Sibi Sankar, linux-arm-kernel, linux-kernel, linux-arm-msm 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 > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] coresight: Abort probe for missing CPU phandle 2019-06-20 17:43 ` Mathieu Poirier @ 2019-06-20 18:07 ` Sai Prakash Ranjan 0 siblings, 0 replies; 13+ messages in thread From: Sai Prakash Ranjan @ 2019-06-20 18:07 UTC (permalink / raw) To: Mathieu Poirier Cc: Suzuki K Poulose, Leo Yan, Alexander Shishkin, Andy Gross, David Brown, Mark Rutland, Rajendra Nayak, Vivek Gautam, Sibi Sankar, linux-arm-kernel, linux-kernel, linux-arm-msm 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. -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2019-06-20 18:26 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-06-20 13:45 [PATCH 0/2] coresight: Set affinity to invalid for missing CPU phandle Sai Prakash Ranjan 2019-06-20 13:45 ` [PATCH 1/2] " Sai Prakash Ranjan 2019-06-20 13:55 ` Suzuki K Poulose 2019-06-20 14:54 ` Sai Prakash Ranjan 2019-06-20 17:39 ` Mathieu Poirier 2019-06-20 18:05 ` Sai Prakash Ranjan 2019-06-20 13:45 ` [PATCH 2/2] coresight: Abort probe " Sai Prakash Ranjan 2019-06-20 13:58 ` Suzuki K Poulose 2019-06-20 14:55 ` Sai Prakash Ranjan 2019-06-20 15:23 ` Suzuki K Poulose 2019-06-20 16:10 ` Sai Prakash Ranjan 2019-06-20 17:43 ` Mathieu Poirier 2019-06-20 18:07 ` Sai Prakash Ranjan
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).