Re: [PATCH v1 2/2] dt-bindings: arm: add property for coresight component name

Re: [PATCH v1 2/2] dt-bindings: arm: add property for coresight component name

[taozha]

On 2021-04-16 19:23, Alexander Shishkin wrote:
> Tao Zhang <taozha@codeaurora.org> writes:
> 
>> Add property "coresight-name" for coresight component name. This
>> allows coresight driver to read device name from device entries.
>> 
>> Signed-off-by: Tao Zhang <taozha@codeaurora.org>
>> ---
>>  Documentation/devicetree/bindings/arm/coresight.txt | 2 ++
>>  1 file changed, 2 insertions(+)
>> 
>> diff --git a/Documentation/devicetree/bindings/arm/coresight.txt 
>> b/Documentation/devicetree/bindings/arm/coresight.txt
>> index d711676..0e980ce 100644
>> --- a/Documentation/devicetree/bindings/arm/coresight.txt
>> +++ b/Documentation/devicetree/bindings/arm/coresight.txt
>> @@ -103,6 +103,8 @@ its hardware characteristcs.
>>  	  powers down the coresight component also powers down and loses its
>>  	  context. This property is currently only used for the ETM 4.x 
>> driver.
>> 
>> +	* coresight-name: the name of the coresight devices.
> 
> Which devices? Also, is it a common practice to extend device tree
> definitions based on arbitrary driver needs, or should there be some
> sort of a discussion first?
> 
> Regards,
> --
> Alex
Through the device tree entries, we can define their own name for any 
coresight device. This design is mainly used to facilitate the unified 
naming of coresight devgies across targets. e.g, without this patch, we 
can only see from sysFS there are multiple funnels, but we cannot know 
which funnel it is based on their names from sysFS. After applying this 
patch, we can directly know what device it is by observing the device 
name in sysFS. And the common scripts can be developed, since applying 
this patch, the same coresight device can have the same name across 
targets. Each developer or vendor can define the name of each coresight 
device according to their preferences and products.

Tao

Re: [PATCH v1 2/2] dt-bindings: arm: add property for coresight component name

[Mike Leach]

Hi

On Fri, 16 Apr 2021 at 15:16, <taozha@codeaurora.org> wrote:
>
> On 2021-04-16 19:23, Alexander Shishkin wrote:
> > Tao Zhang <taozha@codeaurora.org> writes:
> >
> >> Add property "coresight-name" for coresight component name. This
> >> allows coresight driver to read device name from device entries.
> >>
> >> Signed-off-by: Tao Zhang <taozha@codeaurora.org>
> >> ---
> >>  Documentation/devicetree/bindings/arm/coresight.txt | 2 ++
> >>  1 file changed, 2 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/arm/coresight.txt
> >> b/Documentation/devicetree/bindings/arm/coresight.txt
> >> index d711676..0e980ce 100644
> >> --- a/Documentation/devicetree/bindings/arm/coresight.txt
> >> +++ b/Documentation/devicetree/bindings/arm/coresight.txt
> >> @@ -103,6 +103,8 @@ its hardware characteristcs.
> >>        powers down the coresight component also powers down and loses its
> >>        context. This property is currently only used for the ETM 4.x
> >> driver.
> >>
> >> +    * coresight-name: the name of the coresight devices.
> >
> > Which devices? Also, is it a common practice to extend device tree
> > definitions based on arbitrary driver needs, or should there be some
> > sort of a discussion first?
> >
> > Regards,
> > --
> > Alex
> Through the device tree entries, we can define their own name for any
> coresight device. This design is mainly used to facilitate the unified
> naming of coresight devgies across targets. e.g, without this patch, we
> can only see from sysFS there are multiple funnels, but we cannot know
> which funnel it is based on their names from sysFS. After applying this
> patch, we can directly know what device it is by observing the device
> name in sysFS. And the common scripts can be developed, since applying
> this patch, the same coresight device can have the same name across
> targets. Each developer or vendor can define the name of each coresight
> device according to their preferences and products.
>
> Tao

1) I am concerned that this will break the existing protocol which
associates a fixed device type name + number with each device - i.e.
etm0, funnel1 etc.
This naming convention allows for generic common scripts to be developed - see:
./tools/perf/tests/shel/test_arm_coresight.sh
This relies on the device type prefixes to iterate across all devices
in a system - and uses the connections links that are present in each
of the devices to determine the topology.
Replacing these with arbitrary names will break existing scripts.

2) Using the current system it is entirely possible to determine which
specific device a given name relates to.
e.g. ls -al /sys/bus/coresight/devices/

lrwxrwxrwx 1 root root 0 Apr 14 19:02 cti_cpu0 ->
../../../devices/platform/soc/858000.cti/cti_cpu0
lrwxrwxrwx 1 root root 0 Apr 14 19:02 cti_cpu1 ->
../../../devices/platform/soc/859000.cti/cti_cpu1
lrwxrwxrwx 1 root root 0 Apr 14 19:02 cti_cpu2 ->
../../../devices/platform/soc/85a000.cti/cti_cpu2
lrwxrwxrwx 1 root root 0 Apr 14 19:02 cti_cpu3 ->
../../../devices/platform/soc/85b000.cti/cti_cpu3
lrwxrwxrwx 1 root root 0 Apr 14 19:02 cti_sys0 ->
../../../devices/platform/soc/810000.cti/cti_sys0
lrwxrwxrwx 1 root root 0 Apr 14 19:02 cti_sys1 ->
../../../devices/platform/soc/811000.cti/cti_sys1
lrwxrwxrwx 1 root root 0 Apr 14 19:02 etm0 ->
../../../devices/platform/soc/85c000.etm/etm0
lrwxrwxrwx 1 root root 0 Apr 14 19:02 etm1 ->
../../../devices/platform/soc/85d000.etm/etm1
lrwxrwxrwx 1 root root 0 Apr 14 19:02 etm2 ->
../../../devices/platform/soc/85e000.etm/etm2
lrwxrwxrwx 1 root root 0 Apr 14 19:02 etm3 ->
../../../devices/platform/soc/85f000.etm/etm3
lrwxrwxrwx 1 root root 0 Apr 16 14:17 funnel0 ->
../../../devices/platform/soc/821000.funnel/funnel0
lrwxrwxrwx 1 root root 0 Apr 16 14:17 funnel1 ->
../../../devices/platform/soc/841000.funnel/funnel1
lrwxrwxrwx 1 root root 0 Apr 16 14:17 replicator0 ->
../../../devices/platform/soc/824000.replicator/replicator0
lrwxrwxrwx 1 root root 0 Apr 16 14:17 tmc_etf0 ->
../../../devices/platform/soc/825000.etf/tmc_etf0
lrwxrwxrwx 1 root root 0 Apr 16 14:17 tmc_etr0 ->
../../../devices/platform/soc/826000.etr/tmc_etr0
lrwxrwxrwx 1 root root 0 Apr 16 14:17 tpiu0 ->
../../../devices/platform/soc/820000.tpiu/tpiu

Further topology can be determined using the connections sub-directory
in each device:-

ls -al  /sys/bus/coresight/devices/etm0/connections/out\:0
lrwxrwxrwx 1 root root 0 Apr 16 14:18
/sys/bus/coresight/devices/etm0/connections/out:0 ->
../../../841000.funnel/funnel1

Using this information it is possible to iterate across the entire
topology of any coresight system.

3) If there is some scripting requirement that cannot be solved with
the information available above - then it would be better to add this
name as an alias rather than a direct replacement.
Therefore any coresight device could have an alias_name entry, that
could be interrogated by a script and used as required. This avoids
breaking any existing scripts using the established naming convention.

4) Any devicetree attribute should follow the <owner>,<attribute>
naming convention. e.g. arm,some_attribute.
I agree with Alex that it may not be normal practice to add in
attributes in these circumstances - this does not appear to relate to
a specific hardware feature or limitation. You may wish to discuss
this with the device tree maintainers.

Thanks and Regards

Mike

-- 
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK

Re: [PATCH v1 2/2] dt-bindings: arm: add property for coresight component name

[taozha]

On 2021-04-16 22:47, Mike Leach wrote:
> Hi
> 
> On Fri, 16 Apr 2021 at 15:16, <taozha@codeaurora.org> wrote:
>> 
>> On 2021-04-16 19:23, Alexander Shishkin wrote:
>> > Tao Zhang <taozha@codeaurora.org> writes:
>> >
>> >> Add property "coresight-name" for coresight component name. This
>> >> allows coresight driver to read device name from device entries.
>> >>
>> >> Signed-off-by: Tao Zhang <taozha@codeaurora.org>
>> >> ---
>> >>  Documentation/devicetree/bindings/arm/coresight.txt | 2 ++
>> >>  1 file changed, 2 insertions(+)
>> >>
>> >> diff --git a/Documentation/devicetree/bindings/arm/coresight.txt
>> >> b/Documentation/devicetree/bindings/arm/coresight.txt
>> >> index d711676..0e980ce 100644
>> >> --- a/Documentation/devicetree/bindings/arm/coresight.txt
>> >> +++ b/Documentation/devicetree/bindings/arm/coresight.txt
>> >> @@ -103,6 +103,8 @@ its hardware characteristcs.
>> >>        powers down the coresight component also powers down and loses its
>> >>        context. This property is currently only used for the ETM 4.x
>> >> driver.
>> >>
>> >> +    * coresight-name: the name of the coresight devices.
>> >
>> > Which devices? Also, is it a common practice to extend device tree
>> > definitions based on arbitrary driver needs, or should there be some
>> > sort of a discussion first?
>> >
>> > Regards,
>> > --
>> > Alex
>> Through the device tree entries, we can define their own name for any
>> coresight device. This design is mainly used to facilitate the unified
>> naming of coresight devgies across targets. e.g, without this patch, 
>> we
>> can only see from sysFS there are multiple funnels, but we cannot know
>> which funnel it is based on their names from sysFS. After applying 
>> this
>> patch, we can directly know what device it is by observing the device
>> name in sysFS. And the common scripts can be developed, since applying
>> this patch, the same coresight device can have the same name across
>> targets. Each developer or vendor can define the name of each 
>> coresight
>> device according to their preferences and products.
>> 
>> Tao
> 
> 1) I am concerned that this will break the existing protocol which
> associates a fixed device type name + number with each device - i.e.
> etm0, funnel1 etc.
> This naming convention allows for generic common scripts to be 
> developed - see:
> ./tools/perf/tests/shel/test_arm_coresight.sh
> This relies on the device type prefixes to iterate across all devices
> in a system - and uses the connections links that are present in each
> of the devices to determine the topology.
> Replacing these with arbitrary names will break existing scripts.
> 
Yes, agree with you. The patch should not break the existing protocol.
> 2) Using the current system it is entirely possible to determine which
> specific device a given name relates to.
> e.g. ls -al /sys/bus/coresight/devices/
> 
> lrwxrwxrwx 1 root root 0 Apr 14 19:02 cti_cpu0 ->
> ../../../devices/platform/soc/858000.cti/cti_cpu0
> lrwxrwxrwx 1 root root 0 Apr 14 19:02 cti_cpu1 ->
> ../../../devices/platform/soc/859000.cti/cti_cpu1
> lrwxrwxrwx 1 root root 0 Apr 14 19:02 cti_cpu2 ->
> ../../../devices/platform/soc/85a000.cti/cti_cpu2
> lrwxrwxrwx 1 root root 0 Apr 14 19:02 cti_cpu3 ->
> ../../../devices/platform/soc/85b000.cti/cti_cpu3
> lrwxrwxrwx 1 root root 0 Apr 14 19:02 cti_sys0 ->
> ../../../devices/platform/soc/810000.cti/cti_sys0
> lrwxrwxrwx 1 root root 0 Apr 14 19:02 cti_sys1 ->
> ../../../devices/platform/soc/811000.cti/cti_sys1
> lrwxrwxrwx 1 root root 0 Apr 14 19:02 etm0 ->
> ../../../devices/platform/soc/85c000.etm/etm0
> lrwxrwxrwx 1 root root 0 Apr 14 19:02 etm1 ->
> ../../../devices/platform/soc/85d000.etm/etm1
> lrwxrwxrwx 1 root root 0 Apr 14 19:02 etm2 ->
> ../../../devices/platform/soc/85e000.etm/etm2
> lrwxrwxrwx 1 root root 0 Apr 14 19:02 etm3 ->
> ../../../devices/platform/soc/85f000.etm/etm3
> lrwxrwxrwx 1 root root 0 Apr 16 14:17 funnel0 ->
> ../../../devices/platform/soc/821000.funnel/funnel0
> lrwxrwxrwx 1 root root 0 Apr 16 14:17 funnel1 ->
> ../../../devices/platform/soc/841000.funnel/funnel1
> lrwxrwxrwx 1 root root 0 Apr 16 14:17 replicator0 ->
> ../../../devices/platform/soc/824000.replicator/replicator0
> lrwxrwxrwx 1 root root 0 Apr 16 14:17 tmc_etf0 ->
> ../../../devices/platform/soc/825000.etf/tmc_etf0
> lrwxrwxrwx 1 root root 0 Apr 16 14:17 tmc_etr0 ->
> ../../../devices/platform/soc/826000.etr/tmc_etr0
> lrwxrwxrwx 1 root root 0 Apr 16 14:17 tpiu0 ->
> ../../../devices/platform/soc/820000.tpiu/tpiu
> 
> Further topology can be determined using the connections sub-directory
> in each device:-
> 
> ls -al  /sys/bus/coresight/devices/etm0/connections/out\:0
> lrwxrwxrwx 1 root root 0 Apr 16 14:18
> /sys/bus/coresight/devices/etm0/connections/out:0 ->
> ../../../841000.funnel/funnel1
> 
> Using this information it is possible to iterate across the entire
> topology of any coresight system.
> 
Yes, these information can iterate across the entire topology of any 
coresight
system. But it cannot quickly identify specific coresight devices across 
targets.
e.g. If there are a lot of cti on one target, we need to identify the 
cti by its
register address. The same function cti may has different register 
address across
targets. This patch allow the same function coresight device has the 
same name,
and it could be operated by scripts easily.
> 3) If there is some scripting requirement that cannot be solved with
> the information available above - then it would be better to add this
> name as an alias rather than a direct replacement.
> Therefore any coresight device could have an alias_name entry, that
> could be interrogated by a script and used as required. This avoids
> breaking any existing scripts using the established naming convention.
> 
Agree with you. Can I use sysfs_create_link to create a kernel link 
between
alias name and coresight device kobject? At the same time, the original
coresight device naming method remains unchanged. Thus, the existing 
protocol
will not be broken.
Or is there any better suggeston on how to add alias name for coresight 
device?
> 4) Any devicetree attribute should follow the <owner>,<attribute>
> naming convention. e.g. arm,some_attribute.
> I agree with Alex that it may not be normal practice to add in
> attributes in these circumstances - this does not appear to relate to
> a specific hardware feature or limitation. You may wish to discuss
> this with the device tree maintainers.
> 
> Thanks and Regards
> 
> Mike
Sure. I will update this according to your suggestion on patch v2.

Best.
Tao

[PATCH v1 0/2] coresight: Add support for device names

[Tao Zhang]

This series add support for coresight device name. In this way,
nodes with specific names can be generated under sysfs, not just
names with prefixes and index numbers. The coresight device can
be quickly identified by the coresight names of sysfs nodes. This
also allows using same names for CoreSight devices across different
targets. This makes it easy to develop common scripts, which can
be run across targets. Meanwhile, the script can use the same
device name to control the same coresight device.

This series patches base on coresight-next repo
http://git.linaro.org/kernel/coresight.git/log/?h=next

Tao Zhang (2):
  coresight: Add support for device names
  dt-bindings: arm: add property for coresight component name

 Documentation/devicetree/bindings/arm/coresight.txt | 2 ++
 drivers/hwtracing/coresight/coresight-core.c        | 6 ++++++
 2 files changed, 8 insertions(+)

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v1 2/2] dt-bindings: arm: add property for coresight component name

[Tao Zhang]

Add property "coresight-name" for coresight component name. This
allows coresight driver to read device name from device entries.

Signed-off-by: Tao Zhang <taozha@codeaurora.org>
---
 Documentation/devicetree/bindings/arm/coresight.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/coresight.txt b/Documentation/devicetree/bindings/arm/coresight.txt
index d711676..0e980ce 100644
--- a/Documentation/devicetree/bindings/arm/coresight.txt
+++ b/Documentation/devicetree/bindings/arm/coresight.txt
@@ -103,6 +103,8 @@ its hardware characteristcs.
 	  powers down the coresight component also powers down and loses its
 	  context. This property is currently only used for the ETM 4.x driver.
 
+	* coresight-name: the name of the coresight devices.
+
 * Optional properties for ETM/PTMs:
 
 	* arm,cp14: must be present if the system accesses ETM/PTM management
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH 0/2] *** SUBJECT HERE ***

[Tao Zhang]

*** BLURB HERE ***

Tao Zhang (2):
  coresight: Add support for device names
  dt-bindings: arm: add property for coresight component name

 Documentation/devicetree/bindings/arm/coresight.txt | 2 ++
 drivers/hwtracing/coresight/coresight-core.c        | 6 ++++++
 2 files changed, 8 insertions(+)

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v1 2/2] dt-bindings: arm: add property for coresight component name

[Tao Zhang]

Add property "coresight-name" for coresight component name. This
allows coresight driver to read device name from device entries.

Signed-off-by: Tao Zhang <taozha@codeaurora.org>
---
 Documentation/devicetree/bindings/arm/coresight.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/coresight.txt b/Documentation/devicetree/bindings/arm/coresight.txt
index d711676..0e980ce 100644
--- a/Documentation/devicetree/bindings/arm/coresight.txt
+++ b/Documentation/devicetree/bindings/arm/coresight.txt
@@ -103,6 +103,8 @@ its hardware characteristcs.
 	  powers down the coresight component also powers down and loses its
 	  context. This property is currently only used for the ETM 4.x driver.
 
+	* coresight-name: the name of the coresight devices.
+
 * Optional properties for ETM/PTMs:
 
 	* arm,cp14: must be present if the system accesses ETM/PTM management
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v1 2/2] dt-bindings: arm: add property for coresight component name

[Alexander Shishkin]

Tao Zhang <taozha@codeaurora.org> writes:

> Add property "coresight-name" for coresight component name. This
> allows coresight driver to read device name from device entries.
>
> Signed-off-by: Tao Zhang <taozha@codeaurora.org>
> ---
>  Documentation/devicetree/bindings/arm/coresight.txt | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/arm/coresight.txt b/Documentation/devicetree/bindings/arm/coresight.txt
> index d711676..0e980ce 100644
> --- a/Documentation/devicetree/bindings/arm/coresight.txt
> +++ b/Documentation/devicetree/bindings/arm/coresight.txt
> @@ -103,6 +103,8 @@ its hardware characteristcs.
>  	  powers down the coresight component also powers down and loses its
>  	  context. This property is currently only used for the ETM 4.x driver.
>  
> +	* coresight-name: the name of the coresight devices.

Which devices? Also, is it a common practice to extend device tree
definitions based on arbitrary driver needs, or should there be some
sort of a discussion first?

Regards,
--
Alex