| Message ID | 92a33fa58c77206b338220427e92dabbd1d197f7.1561054498.git.saiprakash.ranjan@codeaurora.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | coresight: Do not default to CPU0 for missing CPU phandle | expand |
Hi Sai, On 06/20/2019 07:31 PM, Sai Prakash Ranjan wrote: > 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 in coresight etm and cpu-debug drivers, abort the probe > for such cases. > > Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org> > Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com> Please drop this tag for now. > --- > Documentation/devicetree/bindings/arm/coresight.txt | 2 +- > 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 +++++----- > 5 files changed, 15 insertions(+), 6 deletions(-) > > diff --git a/Documentation/devicetree/bindings/arm/coresight.txt b/Documentation/devicetree/bindings/arm/coresight.txt > index 8a88ddebc1a2..c4659ba9457d 100644 > --- a/Documentation/devicetree/bindings/arm/coresight.txt > +++ b/Documentation/devicetree/bindings/arm/coresight.txt > @@ -88,7 +88,7 @@ its hardware characteristcs. > registers via co-processor 14. > > * cpu: the cpu phandle this ETM/PTM is affined to. When omitted the > - source is considered to belong to CPU0. > + affinity is set to invalid. > Please move this from the "Optional properties". It is not "Optional" anymore with this change. Please make sure it is evident that this is mandatory. Also please fix the bindings document for cpu-debug.txt. > * Optional property for TMC: > > diff --git a/drivers/hwtracing/coresight/coresight-platform.c b/drivers/hwtracing/coresight/coresight-platform.c > index 3c5ceda8db24..8b03fa573684 100644 > --- a/drivers/hwtracing/coresight/coresight-platform.c > +++ b/drivers/hwtracing/coresight/coresight-platform.c > @@ -159,16 +159,16 @@ static int of_coresight_get_cpu(struct device *dev) > struct device_node *dn; > > if (!dev->of_node) > - return 0; > + return -ENODEV; > + > dn = of_parse_phandle(dev->of_node, "cpu", 0); > - /* Affinity defaults to CPU0 */ > if (!dn) > - return 0; > + return -ENODEV; > + > cpu = of_cpu_node_to_id(dn); > of_node_put(dn); > Please fix the acpi_coresight_get_cpu() for ACPI. Rest looks fine to me. Suzuki
Hello Suzuki, On 6/21/2019 3:18 PM, Suzuki K Poulose wrote: > Hi Sai, > > > On 06/20/2019 07:31 PM, Sai Prakash Ranjan wrote: >> 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 in coresight etm and cpu-debug drivers, abort the probe >> for such cases. >> >> Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org> >> Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com> > > Please drop this tag for now. > Ok will drop this. >> --- >> Documentation/devicetree/bindings/arm/coresight.txt | 2 +- >> 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 +++++----- >> 5 files changed, 15 insertions(+), 6 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/arm/coresight.txt >> b/Documentation/devicetree/bindings/arm/coresight.txt >> index 8a88ddebc1a2..c4659ba9457d 100644 >> --- a/Documentation/devicetree/bindings/arm/coresight.txt >> +++ b/Documentation/devicetree/bindings/arm/coresight.txt >> @@ -88,7 +88,7 @@ its hardware characteristcs. >> registers via co-processor 14. >> * cpu: the cpu phandle this ETM/PTM is affined to. When omitted the >> - source is considered to belong to CPU0. >> + affinity is set to invalid. > > Please move this from the "Optional properties". It is not "Optional" > anymore with this change. Please make sure it is evident that this > is mandatory. Also please fix the bindings document for cpu-debug.txt. > > >> * Optional property for TMC: > >> diff --git a/drivers/hwtracing/coresight/coresight-platform.c >> b/drivers/hwtracing/coresight/coresight-platform.c >> index 3c5ceda8db24..8b03fa573684 100644 >> --- a/drivers/hwtracing/coresight/coresight-platform.c >> +++ b/drivers/hwtracing/coresight/coresight-platform.c >> @@ -159,16 +159,16 @@ static int of_coresight_get_cpu(struct device *dev) >> struct device_node *dn; >> if (!dev->of_node) >> - return 0; >> + return -ENODEV; >> + >> dn = of_parse_phandle(dev->of_node, "cpu", 0); >> - /* Affinity defaults to CPU0 */ >> if (!dn) >> - return 0; >> + return -ENODEV; >> + >> cpu = of_cpu_node_to_id(dn); >> of_node_put(dn); > > Please fix the acpi_coresight_get_cpu() for ACPI. > Ok will do it. Thanks again for the review comments. -Sai
diff --git a/Documentation/devicetree/bindings/arm/coresight.txt b/Documentation/devicetree/bindings/arm/coresight.txt index 8a88ddebc1a2..c4659ba9457d 100644 --- a/Documentation/devicetree/bindings/arm/coresight.txt +++ b/Documentation/devicetree/bindings/arm/coresight.txt @@ -88,7 +88,7 @@ its hardware characteristcs. registers via co-processor 14. * cpu: the cpu phandle this ETM/PTM is affined to. When omitted the - source is considered to belong to CPU0. + affinity is set to invalid. * Optional property for TMC: diff --git a/drivers/hwtracing/coresight/coresight-cpu-debug.c b/drivers/hwtracing/coresight/coresight-cpu-debug.c index 07a1367c733f..58bfd6319f65 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 < 0) + return drvdata->cpu; + 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..e2cb6873c3f2 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 < 0) + return drvdata->cpu; + 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..7bcac8896fc1 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 < 0) + return drvdata->cpu; + desc.name = devm_kasprintf(dev, GFP_KERNEL, "etm%d", drvdata->cpu); if (!desc.name) return -ENOMEM; diff --git a/drivers/hwtracing/coresight/coresight-platform.c b/drivers/hwtracing/coresight/coresight-platform.c index 3c5ceda8db24..8b03fa573684 100644 --- a/drivers/hwtracing/coresight/coresight-platform.c +++ b/drivers/hwtracing/coresight/coresight-platform.c @@ -159,16 +159,16 @@ static int of_coresight_get_cpu(struct device *dev) struct device_node *dn; if (!dev->of_node) - return 0; + return -ENODEV; + dn = of_parse_phandle(dev->of_node, "cpu", 0); - /* Affinity defaults to CPU0 */ 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; } /*