Hi,

On 12/10/2023 16:56, Athira Rajeev wrote:


On 05-Oct-2023, at 3:06 PM, Suzuki K Poulose <suzuki.poul...@arm.com> wrote:

On 05/10/2023 06:02, Namhyung Kim wrote:
On Thu, Sep 28, 2023 at 9:11 PM Athira Rajeev
<atraj...@linux.vnet.ibm.com> wrote:


...

Thanks for the fix.

Nothing to do with this patch, but I am wondering if the original patch
is over engineered and may not be future proof.

e.g.,

cs_etm_dev_name() {
+ cs_etm_path=$(find  /sys/bus/event_source/devices/cs_etm/ -name cpu* -print 
-quit)

Right there you got the device name and we can easily deduce the name of
the "ETM" node.

e.g,:
etm=$(basename $(readlink cs_etm_path) | sed "s/[0-9]\+$//")

And practically, nobody prevents an ETE mixed with an ETM on a "hybrid"
system (hopefully, no one builds it ;-))

Also, instead of hardcoding "ete" and "etm" prefixes from the arch part,
we should simply use the cpu nodes from :

/sys/bus/event_source/devices/cs_etm/

e.g.,

arm_cs_etm_traverse_path_test() {
# Iterate for every ETM device
for c in /sys/bus/event_source/devices/cs_etm/cpu*; do
# Read the link to be on the safer side
dev=`readlink $c`

# Find the ETM device belonging to which CPU
cpu=`cat $dev/cpu`

# Use depth-first search (DFS) to iterate outputs
arm_cs_iterate_devices $dev $cpu
done;
}



You'd better add Coresight folks on this.
Maybe this file was missing in the MAINTAINERS file.

And the original author of the commit, that introduced the issue too.

Suzuki

Hi All,
Thanks for the discussion and feedbacks.

This patch fixes the shellcheck warning introduced in function "cs_etm_dev_name". But with the 
changes that Suzuki suggested, we won't need the function "cs_etm_dev_name" since the code will use 
"/sys/bus/event_source/devices/cs_etm/" .  In that case, can I drop this patch for now from this 
series ?


Yes, please. James will send out the proposed patch

Suzuki


Reply via email to