| Message ID | 20190412102738.12679-1-leo.yan@linaro.org |
|---|---|
| Headers | show |
| Series | CoreSight: Support static funnel with DT binding consolidation | expand |
On 04/12/2019 11:27 AM, Leo Yan wrote: > CoreSight uses below bindings for replicator: > > Dynamic replicator, aka. configurable replicator: > "arm,coresight-dynamic-replicator", "arm,primecell"; > > Static replicator, aka. non-configurable replicator: > "arm,coresight-replicator"; > > The compatible string "arm,coresight-replicator" is not an explicit > naming to express the replicator is 'static'. To unify the naming > convention, this patch introduces a new compatible string > "arm,coresight-static-replicator" for the static replicator; the > compatible string "arm,coresight-replicator" is kept for backward > compatibility, but tag it as obsolete and suggest to use the new > compatible string. > > As result CoreSight replicator have below bindings: > > Dynamic replicator: > "arm,coresight-dynamic-replicator", "arm,primecell"; > > Static replicator: > "arm,coresight-static-replicator"; > "arm,coresight-replicator"; (obsolete) > > Signed-off-by: Leo Yan <leo.yan@linaro.org> > --- > Documentation/devicetree/bindings/arm/coresight.txt | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/Documentation/devicetree/bindings/arm/coresight.txt b/Documentation/devicetree/bindings/arm/coresight.txt > index f8aff65ab921..d02d160fa8ac 100644 > --- a/Documentation/devicetree/bindings/arm/coresight.txt > +++ b/Documentation/devicetree/bindings/arm/coresight.txt > @@ -69,7 +69,10 @@ its hardware characteristcs. > > * compatible: Currently supported value is (note the absence of the > AMBA markee): > - - "arm,coresight-replicator" > + - Coresight Non-configurable Replicator: > + "arm,coresight-static-replicator"; > + "arm,coresight-replicator"; (OBSOLETE. For backward > + compatibility and will be removed) > > * port or ports: see "Graph bindings for Coresight" below. > > @@ -169,7 +172,7 @@ Example: > /* non-configurable replicators don't show up on the > * AMBA bus. As such no need to add "arm,primecell". > */ > - compatible = "arm,coresight-replicator"; > + compatible = "arm,coresight-static-replicator"; Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
On 04/12/2019 11:27 AM, Leo Yan wrote: > This patch adds a device id for the new static replicator compatible > string; it changes the driver name from "coresight-replicator" to > "coresight-static-replicator" as well. > > This patch also gives warning when use the replicator obsolete DT > binding. > > Signed-off-by: Leo Yan <leo.yan@linaro.org> > --- > drivers/hwtracing/coresight/coresight-replicator.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/hwtracing/coresight/coresight-replicator.c b/drivers/hwtracing/coresight/coresight-replicator.c > index 4e0da85efd2d..8c9ce74498e1 100644 > --- a/drivers/hwtracing/coresight/coresight-replicator.c > +++ b/drivers/hwtracing/coresight/coresight-replicator.c > @@ -189,6 +189,9 @@ static int replicator_probe(struct device *dev, struct resource *res) > dev->platform_data = pdata; > } > > + if (of_device_is_compatible(np, "arm,coresight-replicator")) > + pr_warn_once("Uses OBSOLETE CoreSight replicator binding\n"); > + > drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL); > if (!drvdata) > return -ENOMEM; > @@ -285,13 +288,14 @@ static const struct dev_pm_ops replicator_dev_pm_ops = { > > static const struct of_device_id static_replicator_match[] = { > {.compatible = "arm,coresight-replicator"}, > + {.compatible = "arm,coresight-static-replicator"}, > {} > }; > > static struct platform_driver static_replicator_driver = { > .probe = static_replicator_probe, > .driver = { > - .name = "coresight-replicator", > + .name = "coresight-static-replicator", > .of_match_table = static_replicator_match, > .pm = &replicator_dev_pm_ops, > .suppress_bind_attrs = true, > Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
On Fri, Apr 12, 2019 at 11:40:13AM +0100, Suzuki K Poulose wrote: > On 04/12/2019 11:27 AM, Leo Yan wrote: > > CoreSight uses below bindings for replicator: > > > > Dynamic replicator, aka. configurable replicator: > > "arm,coresight-dynamic-replicator", "arm,primecell"; > > > > Static replicator, aka. non-configurable replicator: > > "arm,coresight-replicator"; > > > > The compatible string "arm,coresight-replicator" is not an explicit > > naming to express the replicator is 'static'. To unify the naming > > convention, this patch introduces a new compatible string > > "arm,coresight-static-replicator" for the static replicator; the > > compatible string "arm,coresight-replicator" is kept for backward > > compatibility, but tag it as obsolete and suggest to use the new > > compatible string. > > > > As result CoreSight replicator have below bindings: > > > > Dynamic replicator: > > "arm,coresight-dynamic-replicator", "arm,primecell"; > > > > Static replicator: > > "arm,coresight-static-replicator"; > > "arm,coresight-replicator"; (obsolete) > > > > Signed-off-by: Leo Yan <leo.yan@linaro.org> > > --- > > Documentation/devicetree/bindings/arm/coresight.txt | 7 +++++-- > > 1 file changed, 5 insertions(+), 2 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/arm/coresight.txt b/Documentation/devicetree/bindings/arm/coresight.txt > > index f8aff65ab921..d02d160fa8ac 100644 > > --- a/Documentation/devicetree/bindings/arm/coresight.txt > > +++ b/Documentation/devicetree/bindings/arm/coresight.txt > > @@ -69,7 +69,10 @@ its hardware characteristcs. > > * compatible: Currently supported value is (note the absence of the > > AMBA markee): > > - - "arm,coresight-replicator" > > + - Coresight Non-configurable Replicator: > > + "arm,coresight-static-replicator"; > > + "arm,coresight-replicator"; (OBSOLETE. For backward > > + compatibility and will be removed) > > * port or ports: see "Graph bindings for Coresight" below. > > @@ -169,7 +172,7 @@ Example: > > /* non-configurable replicators don't show up on the > > * AMBA bus. As such no need to add "arm,primecell". > > */ > > - compatible = "arm,coresight-replicator"; > > + compatible = "arm,coresight-static-replicator"; > > Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com> Thanks a lot for quick reviewing, Suzuki.
Hi Leo, On Fri, 12 Apr 2019 at 04:28, Leo Yan <leo.yan@linaro.org> wrote: > > CoreSight uses below bindings for replicator: > > Dynamic replicator, aka. configurable replicator: > "arm,coresight-dynamic-replicator", "arm,primecell"; > > Static replicator, aka. non-configurable replicator: > "arm,coresight-replicator"; > > The compatible string "arm,coresight-replicator" is not an explicit > naming to express the replicator is 'static'. To unify the naming > convention, this patch introduces a new compatible string > "arm,coresight-static-replicator" for the static replicator; the > compatible string "arm,coresight-replicator" is kept for backward > compatibility, but tag it as obsolete and suggest to use the new > compatible string. > > As result CoreSight replicator have below bindings: > > Dynamic replicator: > "arm,coresight-dynamic-replicator", "arm,primecell"; > > Static replicator: > "arm,coresight-static-replicator"; > "arm,coresight-replicator"; (obsolete) > > Signed-off-by: Leo Yan <leo.yan@linaro.org> > --- > Documentation/devicetree/bindings/arm/coresight.txt | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/Documentation/devicetree/bindings/arm/coresight.txt b/Documentation/devicetree/bindings/arm/coresight.txt > index f8aff65ab921..d02d160fa8ac 100644 > --- a/Documentation/devicetree/bindings/arm/coresight.txt > +++ b/Documentation/devicetree/bindings/arm/coresight.txt > @@ -69,7 +69,10 @@ its hardware characteristcs. > > * compatible: Currently supported value is (note the absence of the > AMBA markee): > - - "arm,coresight-replicator" > + - Coresight Non-configurable Replicator: > + "arm,coresight-static-replicator"; > + "arm,coresight-replicator"; (OBSOLETE. For backward > + compatibility and will be removed) > > * port or ports: see "Graph bindings for Coresight" below. > > @@ -169,7 +172,7 @@ Example: > /* non-configurable replicators don't show up on the > * AMBA bus. As such no need to add "arm,primecell". > */ > - compatible = "arm,coresight-replicator"; > + compatible = "arm,coresight-static-replicator"; > > out-ports { > #address-cells = <1>; > -- > 2.17.1 Since this is a binding patch it needs to be sent on its own. >
On Tue, Apr 16, 2019 at 02:18:40PM -0600, Mathieu Poirier wrote: > Hi Leo, > > On Fri, 12 Apr 2019 at 04:28, Leo Yan <leo.yan@linaro.org> wrote: > > > > CoreSight uses below bindings for replicator: > > > > Dynamic replicator, aka. configurable replicator: > > "arm,coresight-dynamic-replicator", "arm,primecell"; > > > > Static replicator, aka. non-configurable replicator: > > "arm,coresight-replicator"; > > > > The compatible string "arm,coresight-replicator" is not an explicit > > naming to express the replicator is 'static'. To unify the naming > > convention, this patch introduces a new compatible string > > "arm,coresight-static-replicator" for the static replicator; the > > compatible string "arm,coresight-replicator" is kept for backward > > compatibility, but tag it as obsolete and suggest to use the new > > compatible string. > > > > As result CoreSight replicator have below bindings: > > > > Dynamic replicator: > > "arm,coresight-dynamic-replicator", "arm,primecell"; > > > > Static replicator: > > "arm,coresight-static-replicator"; > > "arm,coresight-replicator"; (obsolete) > > > > Signed-off-by: Leo Yan <leo.yan@linaro.org> > > --- > > Documentation/devicetree/bindings/arm/coresight.txt | 7 +++++-- > > 1 file changed, 5 insertions(+), 2 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/arm/coresight.txt b/Documentation/devicetree/bindings/arm/coresight.txt > > index f8aff65ab921..d02d160fa8ac 100644 > > --- a/Documentation/devicetree/bindings/arm/coresight.txt > > +++ b/Documentation/devicetree/bindings/arm/coresight.txt > > @@ -69,7 +69,10 @@ its hardware characteristcs. > > > > * compatible: Currently supported value is (note the absence of the > > AMBA markee): > > - - "arm,coresight-replicator" > > + - Coresight Non-configurable Replicator: > > + "arm,coresight-static-replicator"; > > + "arm,coresight-replicator"; (OBSOLETE. For backward > > + compatibility and will be removed) > > > > * port or ports: see "Graph bindings for Coresight" below. > > > > @@ -169,7 +172,7 @@ Example: > > /* non-configurable replicators don't show up on the > > * AMBA bus. As such no need to add "arm,primecell". > > */ > > - compatible = "arm,coresight-replicator"; > > + compatible = "arm,coresight-static-replicator"; > > > > out-ports { > > #address-cells = <1>; > > -- > > 2.17.1 > > Since this is a binding patch it needs to be sent on its own. Thanks for reminding, Mathieu. Since this is the second time you remind me to send DT binding related patches separately, so I may misunderstand your meaning and want to get clarification to avoid making the same mistake for many times. Before I remembered in one patch set we need to organise patches with sending document patch (or document changing patch) ahead and then followed by the corresponding code change patch. So this can give the reviewers more clear context; and this also can present the merging dependency between document change patches and the code change patches. This is the rule I followed in this patch set and I sent to CoreSight and DT maintainers (and mailing lists) together. Please let me know what you think about this? And also welcome Rob/Mark's suggestions. Thanks, Leo Yan
On Wed, Apr 17, 2019 at 07:57:50AM -0600, Mathieu Poirier wrote: > On Tue, 16 Apr 2019 at 18:10, Leo Yan <leo.yan@linaro.org> wrote: > > > > On Tue, Apr 16, 2019 at 02:18:40PM -0600, Mathieu Poirier wrote: > > > Hi Leo, > > > > > > On Fri, 12 Apr 2019 at 04:28, Leo Yan <leo.yan@linaro.org> wrote: > > > > > > > > CoreSight uses below bindings for replicator: > > > > > > > > Dynamic replicator, aka. configurable replicator: > > > > "arm,coresight-dynamic-replicator", "arm,primecell"; > > > > > > > > Static replicator, aka. non-configurable replicator: > > > > "arm,coresight-replicator"; > > > > > > > > The compatible string "arm,coresight-replicator" is not an explicit > > > > naming to express the replicator is 'static'. To unify the naming > > > > convention, this patch introduces a new compatible string > > > > "arm,coresight-static-replicator" for the static replicator; the > > > > compatible string "arm,coresight-replicator" is kept for backward > > > > compatibility, but tag it as obsolete and suggest to use the new > > > > compatible string. > > > > > > > > As result CoreSight replicator have below bindings: > > > > > > > > Dynamic replicator: > > > > "arm,coresight-dynamic-replicator", "arm,primecell"; > > > > > > > > Static replicator: > > > > "arm,coresight-static-replicator"; > > > > "arm,coresight-replicator"; (obsolete) > > > > > > > > Signed-off-by: Leo Yan <leo.yan@linaro.org> > > > > --- > > > > Documentation/devicetree/bindings/arm/coresight.txt | 7 +++++-- > > > > 1 file changed, 5 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/Documentation/devicetree/bindings/arm/coresight.txt b/Documentation/devicetree/bindings/arm/coresight.txt > > > > index f8aff65ab921..d02d160fa8ac 100644 > > > > --- a/Documentation/devicetree/bindings/arm/coresight.txt > > > > +++ b/Documentation/devicetree/bindings/arm/coresight.txt > > > > @@ -69,7 +69,10 @@ its hardware characteristcs. > > > > > > > > * compatible: Currently supported value is (note the absence of the > > > > AMBA markee): > > > > - - "arm,coresight-replicator" > > > > + - Coresight Non-configurable Replicator: > > > > + "arm,coresight-static-replicator"; > > > > + "arm,coresight-replicator"; (OBSOLETE. For backward > > > > + compatibility and will be removed) > > > > > > > > * port or ports: see "Graph bindings for Coresight" below. > > > > > > > > @@ -169,7 +172,7 @@ Example: > > > > /* non-configurable replicators don't show up on the > > > > * AMBA bus. As such no need to add "arm,primecell". > > > > */ > > > > - compatible = "arm,coresight-replicator"; > > > > + compatible = "arm,coresight-static-replicator"; > > > > > > > > out-ports { > > > > #address-cells = <1>; > > > > -- > > > > 2.17.1 > > > > > > Since this is a binding patch it needs to be sent on its own. > > > > Thanks for reminding, Mathieu. > > > > Since this is the second time you remind me to send DT binding related > > patches separately, so I may misunderstand your meaning and want to get > > clarification to avoid making the same mistake for many times. > > > > Before I remembered in one patch set we need to organise patches with > > sending document patch (or document changing patch) ahead and then > > followed by the corresponding code change patch. So this can give the > > reviewers more clear context; and this also can present the merging > > dependency between document change patches and the code change patches. > > > > This is the rule I followed in this patch set and I sent to CoreSight > > and DT maintainers (and mailing lists) together. > > > > Please let me know what you think about this? And also welcome > > Rob/Mark's suggestions. > > https://www.kernel.org/doc/Documentation/devicetree/bindings/submitting-patches.txt I'm not clear on what you are asking for. Binding patches should be separate patch, but can and should still be in a series if there's relevant code changes. Rob
On Fri, 12 Apr 2019 18:27:35 +0800, Leo Yan wrote: > CoreSight uses below bindings for replicator: > > Dynamic replicator, aka. configurable replicator: > "arm,coresight-dynamic-replicator", "arm,primecell"; > > Static replicator, aka. non-configurable replicator: > "arm,coresight-replicator"; > > The compatible string "arm,coresight-replicator" is not an explicit > naming to express the replicator is 'static'. To unify the naming > convention, this patch introduces a new compatible string > "arm,coresight-static-replicator" for the static replicator; the > compatible string "arm,coresight-replicator" is kept for backward > compatibility, but tag it as obsolete and suggest to use the new > compatible string. > > As result CoreSight replicator have below bindings: > > Dynamic replicator: > "arm,coresight-dynamic-replicator", "arm,primecell"; > > Static replicator: > "arm,coresight-static-replicator"; > "arm,coresight-replicator"; (obsolete) > > Signed-off-by: Leo Yan <leo.yan@linaro.org> > --- > Documentation/devicetree/bindings/arm/coresight.txt | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > Reviewed-by: Rob Herring <robh@kernel.org>
On Fri, 12 Apr 2019 at 04:27, Leo Yan <leo.yan@linaro.org> wrote: > > Since before there have no platforms use static funnel in mainline > kernel (though maybe some in-house SoC has used it but didn't upstream > for mainline kernel yet so we don't be aware for it), when enable > CoreSight DT binding for hikey960, we found the SoC uses the static > funnel in the link path and but it's not supported in CoreSight funnel > driver. > > When support static funnel it's found the DT binding compatible string > doesn't follow the same fashion between replicator and funnel; so the > first two patches are firstly to refine the replicator DT binding and > correspondingly, change the replicator driver to support new DT binding > compatible. > > Patch 0003 is to update DT documentation to support static funnel (we > call it as non-configurable funnel in documentation) and unify the > compatible string with the same fashion for replicator and funnel. > > Patch 0004 is to support the static funnel in the CoreSight funnel > driver. > > The consolidated DT binding compatible strings are as below: > > Dynamic replicator: > "arm,coresight-dynamic-replicator", "arm,primecell"; > > Static replicator: > "arm,coresight-static-replicator"; > "arm,coresight-replicator"; (obsolete) > > Static funnel: > "arm,coresight-static-funnel"; > > Dynamic funnel: > "arm,coresight-dynamic-funnel", "arm,primecell"; > "arm,coresight-funnel", "arm,primecell"; (obsolete) > > This patch set has been rebased on CoreSight next branch [1] with > latest commit d5d246a56af0 ("coresight: Merge the static and dynamic > replicator drivers") and tested on Hikey960/Hikey with perf commands: > > Hikey960: > > # perf record -e cs_etm/@ec033000.etr/ --per-thread ./main > # perf report --tui > > Hikey: > > # perf record -e cs_etm/@f6404000.etr/ --per-thread uname > # perf report --tui > > [1] https://git.linaro.org/kernel/coresight.git/log/?h=next > > == Changes for v5 == > * Marked the the obsolete compatible strings to be more abvious > (Suzuki). > * Used pr_warn_once() to print out warning if DT binding uses > the obsolete compatible string (Suzuki). > > == Changes for v4 == > * Consolidate the DT binding compatible strings for replicator and > funnel (Mathieu). > * Changed replicator driver to support new compatible string. > > > Leo Yan (4): > dt-bindings: arm: coresight: Add new compatible for static replicator > coresight: replicator: Add new device id for static replicator > dt-bindings: arm: coresight: Unify funnel DT binding > coresight: funnel: Support static funnel > > .../devicetree/bindings/arm/coresight.txt | 60 +++++++-- > .../hwtracing/coresight/coresight-funnel.c | 116 ++++++++++++++---- > .../coresight/coresight-replicator.c | 6 +- > 3 files changed, 147 insertions(+), 35 deletions(-) I have applied this set. Thanks, Mathieu > > -- > 2.17.1 >
On Fri, Apr 26, 2019 at 10:26:24AM -0600, Mathieu Poirier wrote: [...] > > Leo Yan (4): > > dt-bindings: arm: coresight: Add new compatible for static replicator > > coresight: replicator: Add new device id for static replicator > > dt-bindings: arm: coresight: Unify funnel DT binding > > coresight: funnel: Support static funnel > > > > .../devicetree/bindings/arm/coresight.txt | 60 +++++++-- > > .../hwtracing/coresight/coresight-funnel.c | 116 ++++++++++++++---- > > .../coresight/coresight-replicator.c | 6 +- > > 3 files changed, 147 insertions(+), 35 deletions(-) > > I have applied this set. Thanks, Mathieu. Also thanks Rob and you for clarification DT binding.
Since before there have no platforms use static funnel in mainline kernel (though maybe some in-house SoC has used it but didn't upstream for mainline kernel yet so we don't be aware for it), when enable CoreSight DT binding for hikey960, we found the SoC uses the static funnel in the link path and but it's not supported in CoreSight funnel driver. When support static funnel it's found the DT binding compatible string doesn't follow the same fashion between replicator and funnel; so the first two patches are firstly to refine the replicator DT binding and correspondingly, change the replicator driver to support new DT binding compatible. Patch 0003 is to update DT documentation to support static funnel (we call it as non-configurable funnel in documentation) and unify the compatible string with the same fashion for replicator and funnel. Patch 0004 is to support the static funnel in the CoreSight funnel driver. The consolidated DT binding compatible strings are as below: Dynamic replicator: "arm,coresight-dynamic-replicator", "arm,primecell"; Static replicator: "arm,coresight-static-replicator"; "arm,coresight-replicator"; (obsolete) Static funnel: "arm,coresight-static-funnel"; Dynamic funnel: "arm,coresight-dynamic-funnel", "arm,primecell"; "arm,coresight-funnel", "arm,primecell"; (obsolete) This patch set has been rebased on CoreSight next branch [1] with latest commit d5d246a56af0 ("coresight: Merge the static and dynamic replicator drivers") and tested on Hikey960/Hikey with perf commands: Hikey960: # perf record -e cs_etm/@ec033000.etr/ --per-thread ./main # perf report --tui Hikey: # perf record -e cs_etm/@f6404000.etr/ --per-thread uname # perf report --tui [1] https://git.linaro.org/kernel/coresight.git/log/?h=next == Changes for v5 == * Marked the the obsolete compatible strings to be more abvious (Suzuki). * Used pr_warn_once() to print out warning if DT binding uses the obsolete compatible string (Suzuki). == Changes for v4 == * Consolidate the DT binding compatible strings for replicator and funnel (Mathieu). * Changed replicator driver to support new compatible string. Leo Yan (4): dt-bindings: arm: coresight: Add new compatible for static replicator coresight: replicator: Add new device id for static replicator dt-bindings: arm: coresight: Unify funnel DT binding coresight: funnel: Support static funnel .../devicetree/bindings/arm/coresight.txt | 60 +++++++-- .../hwtracing/coresight/coresight-funnel.c | 116 ++++++++++++++---- .../coresight/coresight-replicator.c | 6 +- 3 files changed, 147 insertions(+), 35 deletions(-) -- 2.17.1