[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ffb2fdb8-7aa3-4e2b-aec7-07dc7fe26f7e@quicinc.com>
Date: Wed, 26 Feb 2025 15:07:50 +0800
From: Jie Gan <quic_jiegan@...cinc.com>
To: Suzuki K Poulose <suzuki.poulose@....com>,
Mike Leach
<mike.leach@...aro.org>,
James Clark <james.clark@...aro.org>,
"Alexander
Shishkin" <alexander.shishkin@...ux.intel.com>,
Maxime Coquelin
<mcoquelin.stm32@...il.com>,
Alexandre Torgue <alexandre.torgue@...s.st.com>,
Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>,
Bjorn Andersson <andersson@...nel.org>,
Konrad Dybcio <konradybcio@...nel.org>
CC: Tingwei Zhang <quic_tingweiz@...cinc.com>,
Jinlong Mao
<quic_jinlmao@...cinc.com>, <coresight@...ts.linaro.org>,
<linux-arm-kernel@...ts.infradead.org>, <linux-kernel@...r.kernel.org>,
<devicetree@...r.kernel.org>, <linux-arm-msm@...r.kernel.org>,
<linux-stm32@...md-mailman.stormreply.com>
Subject: Re: [PATCH v14 06/10] Coresight: Change to read the trace ID from
coresight_path
On 2/26/2025 12:13 PM, Jie Gan wrote:
> The source device can directly read the trace ID from the coresight_path
> which result in etm_read_alloc_trace_id and etm4_read_alloc_trace_id being
> deleted.
>
> Signed-off-by: Jie Gan <quic_jiegan@...cinc.com>
Correct tags:
Co-developed-by: James Clark <james.clark@...aro.org>
Signed-off-by: James Clark <james.clark@...aro.org>
Signed-off-by: Jie Gan <quic_jiegan@...cinc.com>
> ---
> drivers/hwtracing/coresight/coresight-dummy.c | 2 +-
> .../hwtracing/coresight/coresight-etm-perf.c | 8 +--
> drivers/hwtracing/coresight/coresight-etm.h | 1 -
> .../coresight/coresight-etm3x-core.c | 54 +++----------------
> .../coresight/coresight-etm4x-core.c | 54 +++----------------
> drivers/hwtracing/coresight/coresight-etm4x.h | 1 -
> drivers/hwtracing/coresight/coresight-stm.c | 2 +-
> drivers/hwtracing/coresight/coresight-sysfs.c | 7 +--
> drivers/hwtracing/coresight/coresight-tpdm.c | 2 +-
> include/linux/coresight.h | 2 +-
> 10 files changed, 25 insertions(+), 108 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-dummy.c b/drivers/hwtracing/coresight/coresight-dummy.c
> index b5692ba358c1..aaa92b5081e3 100644
> --- a/drivers/hwtracing/coresight/coresight-dummy.c
> +++ b/drivers/hwtracing/coresight/coresight-dummy.c
> @@ -24,7 +24,7 @@ DEFINE_CORESIGHT_DEVLIST(sink_devs, "dummy_sink");
>
> static int dummy_source_enable(struct coresight_device *csdev,
> struct perf_event *event, enum cs_mode mode,
> - __maybe_unused struct coresight_trace_id_map *id_map)
> + __maybe_unused struct coresight_path *path)
> {
> if (!coresight_take_mode(csdev, mode))
> return -EBUSY;
> diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c
> index 134290ab622e..300305d67a1d 100644
> --- a/drivers/hwtracing/coresight/coresight-etm-perf.c
> +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c
> @@ -461,7 +461,6 @@ static void etm_event_start(struct perf_event *event, int flags)
> struct coresight_device *sink, *csdev = per_cpu(csdev_src, cpu);
> struct coresight_path *path;
> u64 hw_id;
> - u8 trace_id;
>
> if (!csdev)
> goto fail;
> @@ -504,8 +503,7 @@ static void etm_event_start(struct perf_event *event, int flags)
> goto fail_end_stop;
>
> /* Finally enable the tracer */
> - if (source_ops(csdev)->enable(csdev, event, CS_MODE_PERF,
> - &sink->perf_sink_id_map))
> + if (source_ops(csdev)->enable(csdev, event, CS_MODE_PERF, path))
> goto fail_disable_path;
>
> /*
> @@ -515,13 +513,11 @@ static void etm_event_start(struct perf_event *event, int flags)
> if (!cpumask_test_cpu(cpu, &event_data->aux_hwid_done)) {
> cpumask_set_cpu(cpu, &event_data->aux_hwid_done);
>
> - trace_id = coresight_trace_id_read_cpu_id_map(cpu, &sink->perf_sink_id_map);
> -
> hw_id = FIELD_PREP(CS_AUX_HW_ID_MAJOR_VERSION_MASK,
> CS_AUX_HW_ID_MAJOR_VERSION);
> hw_id |= FIELD_PREP(CS_AUX_HW_ID_MINOR_VERSION_MASK,
> CS_AUX_HW_ID_MINOR_VERSION);
> - hw_id |= FIELD_PREP(CS_AUX_HW_ID_TRACE_ID_MASK, trace_id);
> + hw_id |= FIELD_PREP(CS_AUX_HW_ID_TRACE_ID_MASK, path->trace_id);
> hw_id |= FIELD_PREP(CS_AUX_HW_ID_SINK_ID_MASK, coresight_get_sink_id(sink));
>
> perf_report_aux_output_id(event, hw_id);
> diff --git a/drivers/hwtracing/coresight/coresight-etm.h b/drivers/hwtracing/coresight/coresight-etm.h
> index e02c3ea972c9..171f1384f7c0 100644
> --- a/drivers/hwtracing/coresight/coresight-etm.h
> +++ b/drivers/hwtracing/coresight/coresight-etm.h
> @@ -284,6 +284,5 @@ extern const struct attribute_group *coresight_etm_groups[];
> void etm_set_default(struct etm_config *config);
> void etm_config_trace_mode(struct etm_config *config);
> struct etm_config *get_etm_config(struct etm_drvdata *drvdata);
> -int etm_read_alloc_trace_id(struct etm_drvdata *drvdata);
> void etm_release_trace_id(struct etm_drvdata *drvdata);
> #endif
> diff --git a/drivers/hwtracing/coresight/coresight-etm3x-core.c b/drivers/hwtracing/coresight/coresight-etm3x-core.c
> index c1dda4bc4a2f..8927bfaf3af2 100644
> --- a/drivers/hwtracing/coresight/coresight-etm3x-core.c
> +++ b/drivers/hwtracing/coresight/coresight-etm3x-core.c
> @@ -455,26 +455,6 @@ static int etm_cpu_id(struct coresight_device *csdev)
> return drvdata->cpu;
> }
>
> -int etm_read_alloc_trace_id(struct etm_drvdata *drvdata)
> -{
> - int trace_id;
> -
> - /*
> - * This will allocate a trace ID to the cpu,
> - * or return the one currently allocated.
> - *
> - * trace id function has its own lock
> - */
> - trace_id = coresight_trace_id_get_cpu_id(drvdata->cpu);
> - if (IS_VALID_CS_TRACE_ID(trace_id))
> - drvdata->traceid = (u8)trace_id;
> - else
> - dev_err(&drvdata->csdev->dev,
> - "Failed to allocate trace ID for %s on CPU%d\n",
> - dev_name(&drvdata->csdev->dev), drvdata->cpu);
> - return trace_id;
> -}
> -
> void etm_release_trace_id(struct etm_drvdata *drvdata)
> {
> coresight_trace_id_put_cpu_id(drvdata->cpu);
> @@ -482,38 +462,22 @@ void etm_release_trace_id(struct etm_drvdata *drvdata)
>
> static int etm_enable_perf(struct coresight_device *csdev,
> struct perf_event *event,
> - struct coresight_trace_id_map *id_map)
> + struct coresight_path *path)
> {
> struct etm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
> - int trace_id;
>
> if (WARN_ON_ONCE(drvdata->cpu != smp_processor_id()))
> return -EINVAL;
>
> /* Configure the tracer based on the session's specifics */
> etm_parse_event_config(drvdata, event);
> -
> - /*
> - * perf allocates cpu ids as part of _setup_aux() - device needs to use
> - * the allocated ID. This reads the current version without allocation.
> - *
> - * This does not use the trace id lock to prevent lock_dep issues
> - * with perf locks - we know the ID cannot change until perf shuts down
> - * the session
> - */
> - trace_id = coresight_trace_id_read_cpu_id_map(drvdata->cpu, id_map);
> - if (!IS_VALID_CS_TRACE_ID(trace_id)) {
> - dev_err(&drvdata->csdev->dev, "Failed to set trace ID for %s on CPU%d\n",
> - dev_name(&drvdata->csdev->dev), drvdata->cpu);
> - return -EINVAL;
> - }
> - drvdata->traceid = (u8)trace_id;
> + drvdata->traceid = path->trace_id;
>
> /* And enable it */
> return etm_enable_hw(drvdata);
> }
>
> -static int etm_enable_sysfs(struct coresight_device *csdev)
> +static int etm_enable_sysfs(struct coresight_device *csdev, struct coresight_path *path)
> {
> struct etm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
> struct etm_enable_arg arg = { };
> @@ -521,10 +485,7 @@ static int etm_enable_sysfs(struct coresight_device *csdev)
>
> spin_lock(&drvdata->spinlock);
>
> - /* sysfs needs to allocate and set a trace ID */
> - ret = etm_read_alloc_trace_id(drvdata);
> - if (ret < 0)
> - goto unlock_enable_sysfs;
> + drvdata->traceid = path->trace_id;
>
> /*
> * Configure the ETM only if the CPU is online. If it isn't online
> @@ -545,7 +506,6 @@ static int etm_enable_sysfs(struct coresight_device *csdev)
> if (ret)
> etm_release_trace_id(drvdata);
>
> -unlock_enable_sysfs:
> spin_unlock(&drvdata->spinlock);
>
> if (!ret)
> @@ -554,7 +514,7 @@ static int etm_enable_sysfs(struct coresight_device *csdev)
> }
>
> static int etm_enable(struct coresight_device *csdev, struct perf_event *event,
> - enum cs_mode mode, struct coresight_trace_id_map *id_map)
> + enum cs_mode mode, struct coresight_path *path)
> {
> int ret;
> struct etm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
> @@ -566,10 +526,10 @@ static int etm_enable(struct coresight_device *csdev, struct perf_event *event,
>
> switch (mode) {
> case CS_MODE_SYSFS:
> - ret = etm_enable_sysfs(csdev);
> + ret = etm_enable_sysfs(csdev, path);
> break;
> case CS_MODE_PERF:
> - ret = etm_enable_perf(csdev, event, id_map);
> + ret = etm_enable_perf(csdev, event, path);
> break;
> default:
> ret = -EINVAL;
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> index cfd116b87460..1ed957f5df61 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> @@ -233,25 +233,6 @@ static int etm4_cpu_id(struct coresight_device *csdev)
> return drvdata->cpu;
> }
>
> -int etm4_read_alloc_trace_id(struct etmv4_drvdata *drvdata)
> -{
> - int trace_id;
> -
> - /*
> - * This will allocate a trace ID to the cpu,
> - * or return the one currently allocated.
> - * The trace id function has its own lock
> - */
> - trace_id = coresight_trace_id_get_cpu_id(drvdata->cpu);
> - if (IS_VALID_CS_TRACE_ID(trace_id))
> - drvdata->trcid = (u8)trace_id;
> - else
> - dev_err(&drvdata->csdev->dev,
> - "Failed to allocate trace ID for %s on CPU%d\n",
> - dev_name(&drvdata->csdev->dev), drvdata->cpu);
> - return trace_id;
> -}
> -
> void etm4_release_trace_id(struct etmv4_drvdata *drvdata)
> {
> coresight_trace_id_put_cpu_id(drvdata->cpu);
> @@ -788,9 +769,9 @@ static int etm4_parse_event_config(struct coresight_device *csdev,
>
> static int etm4_enable_perf(struct coresight_device *csdev,
> struct perf_event *event,
> - struct coresight_trace_id_map *id_map)
> + struct coresight_path *path)
> {
> - int ret = 0, trace_id;
> + int ret = 0;
> struct etmv4_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
>
> if (WARN_ON_ONCE(drvdata->cpu != smp_processor_id())) {
> @@ -803,22 +784,7 @@ static int etm4_enable_perf(struct coresight_device *csdev,
> if (ret)
> goto out;
>
> - /*
> - * perf allocates cpu ids as part of _setup_aux() - device needs to use
> - * the allocated ID. This reads the current version without allocation.
> - *
> - * This does not use the trace id lock to prevent lock_dep issues
> - * with perf locks - we know the ID cannot change until perf shuts down
> - * the session
> - */
> - trace_id = coresight_trace_id_read_cpu_id_map(drvdata->cpu, id_map);
> - if (!IS_VALID_CS_TRACE_ID(trace_id)) {
> - dev_err(&drvdata->csdev->dev, "Failed to set trace ID for %s on CPU%d\n",
> - dev_name(&drvdata->csdev->dev), drvdata->cpu);
> - ret = -EINVAL;
> - goto out;
> - }
> - drvdata->trcid = (u8)trace_id;
> + drvdata->trcid = path->trace_id;
>
> /* And enable it */
> ret = etm4_enable_hw(drvdata);
> @@ -827,7 +793,7 @@ static int etm4_enable_perf(struct coresight_device *csdev,
> return ret;
> }
>
> -static int etm4_enable_sysfs(struct coresight_device *csdev)
> +static int etm4_enable_sysfs(struct coresight_device *csdev, struct coresight_path *path)
> {
> struct etmv4_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
> struct etm4_enable_arg arg = { };
> @@ -844,10 +810,7 @@ static int etm4_enable_sysfs(struct coresight_device *csdev)
>
> spin_lock(&drvdata->spinlock);
>
> - /* sysfs needs to read and allocate a trace ID */
> - ret = etm4_read_alloc_trace_id(drvdata);
> - if (ret < 0)
> - goto unlock_sysfs_enable;
> + drvdata->trcid = path->trace_id;
>
> /*
> * Executing etm4_enable_hw on the cpu whose ETM is being enabled
> @@ -864,7 +827,6 @@ static int etm4_enable_sysfs(struct coresight_device *csdev)
> if (ret)
> etm4_release_trace_id(drvdata);
>
> -unlock_sysfs_enable:
> spin_unlock(&drvdata->spinlock);
>
> if (!ret)
> @@ -873,7 +835,7 @@ static int etm4_enable_sysfs(struct coresight_device *csdev)
> }
>
> static int etm4_enable(struct coresight_device *csdev, struct perf_event *event,
> - enum cs_mode mode, struct coresight_trace_id_map *id_map)
> + enum cs_mode mode, struct coresight_path *path)
> {
> int ret;
>
> @@ -884,10 +846,10 @@ static int etm4_enable(struct coresight_device *csdev, struct perf_event *event,
>
> switch (mode) {
> case CS_MODE_SYSFS:
> - ret = etm4_enable_sysfs(csdev);
> + ret = etm4_enable_sysfs(csdev, path);
> break;
> case CS_MODE_PERF:
> - ret = etm4_enable_perf(csdev, event, id_map);
> + ret = etm4_enable_perf(csdev, event, path);
> break;
> default:
> ret = -EINVAL;
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h
> index 1119762b5cec..2b92de17b5a2 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x.h
> +++ b/drivers/hwtracing/coresight/coresight-etm4x.h
> @@ -1066,6 +1066,5 @@ static inline bool etm4x_is_ete(struct etmv4_drvdata *drvdata)
> return drvdata->arch >= ETM_ARCH_ETE;
> }
>
> -int etm4_read_alloc_trace_id(struct etmv4_drvdata *drvdata);
> void etm4_release_trace_id(struct etmv4_drvdata *drvdata);
> #endif
> diff --git a/drivers/hwtracing/coresight/coresight-stm.c b/drivers/hwtracing/coresight/coresight-stm.c
> index aca25b5e3be2..26f9339f38b9 100644
> --- a/drivers/hwtracing/coresight/coresight-stm.c
> +++ b/drivers/hwtracing/coresight/coresight-stm.c
> @@ -195,7 +195,7 @@ static void stm_enable_hw(struct stm_drvdata *drvdata)
>
> static int stm_enable(struct coresight_device *csdev, struct perf_event *event,
> enum cs_mode mode,
> - __maybe_unused struct coresight_trace_id_map *trace_id)
> + __maybe_unused struct coresight_path *path)
> {
> struct stm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
>
> diff --git a/drivers/hwtracing/coresight/coresight-sysfs.c b/drivers/hwtracing/coresight/coresight-sysfs.c
> index d03751bf3d8a..3ac5b52413a6 100644
> --- a/drivers/hwtracing/coresight/coresight-sysfs.c
> +++ b/drivers/hwtracing/coresight/coresight-sysfs.c
> @@ -53,7 +53,8 @@ ssize_t coresight_simple_show32(struct device *_dev,
> EXPORT_SYMBOL_GPL(coresight_simple_show32);
>
> static int coresight_enable_source_sysfs(struct coresight_device *csdev,
> - enum cs_mode mode, void *data)
> + enum cs_mode mode,
> + struct coresight_path *path)
> {
> int ret;
>
> @@ -64,7 +65,7 @@ static int coresight_enable_source_sysfs(struct coresight_device *csdev,
> */
> lockdep_assert_held(&coresight_mutex);
> if (coresight_get_mode(csdev) != CS_MODE_SYSFS) {
> - ret = source_ops(csdev)->enable(csdev, data, mode, NULL);
> + ret = source_ops(csdev)->enable(csdev, NULL, mode, path);
> if (ret)
> return ret;
> }
> @@ -217,7 +218,7 @@ int coresight_enable_sysfs(struct coresight_device *csdev)
> if (ret)
> goto err_path;
>
> - ret = coresight_enable_source_sysfs(csdev, CS_MODE_SYSFS, NULL);
> + ret = coresight_enable_source_sysfs(csdev, CS_MODE_SYSFS, path);
> if (ret)
> goto err_source;
>
> diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c b/drivers/hwtracing/coresight/coresight-tpdm.c
> index c38f9701665e..fec49a537658 100644
> --- a/drivers/hwtracing/coresight/coresight-tpdm.c
> +++ b/drivers/hwtracing/coresight/coresight-tpdm.c
> @@ -440,7 +440,7 @@ static void __tpdm_enable(struct tpdm_drvdata *drvdata)
>
> static int tpdm_enable(struct coresight_device *csdev, struct perf_event *event,
> enum cs_mode mode,
> - __maybe_unused struct coresight_trace_id_map *id_map)
> + __maybe_unused struct coresight_path *path)
> {
> struct tpdm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
>
> diff --git a/include/linux/coresight.h b/include/linux/coresight.h
> index 00404ba19352..8f76e7c45b38 100644
> --- a/include/linux/coresight.h
> +++ b/include/linux/coresight.h
> @@ -400,7 +400,7 @@ struct coresight_ops_link {
> struct coresight_ops_source {
> int (*cpu_id)(struct coresight_device *csdev);
> int (*enable)(struct coresight_device *csdev, struct perf_event *event,
> - enum cs_mode mode, struct coresight_trace_id_map *id_map);
> + enum cs_mode mode, struct coresight_path *path);
> void (*disable)(struct coresight_device *csdev,
> struct perf_event *event);
> };
Powered by blists - more mailing lists