From: James Clark <james.clark@linaro.org>
To: Jie Gan <quic_jiegan@quicinc.com>
Cc: quic_tingweiz@quicinc.com, quic_jinlmao@quicinc.com,
coresight@lists.linaro.org, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
linux-arm-msm@vger.kernel.org,
linux-stm32@st-md-mailman.stormreply.com, suzuki.poulose@arm.com,
mike.leach@linaro.org, alexander.shishkin@linux.intel.com,
mcoquelin.stm32@gmail.com, alexandre.torgue@foss.st.com,
robh@kernel.org, krzk+dt@kernel.org, conor+dt@kernel.org,
andersson@kernel.org, konradybcio@kernel.org
Subject: Re: [PATCH 1/3] coresight: Don't save handle in path
Date: Thu, 6 Feb 2025 14:34:04 +0000 [thread overview]
Message-ID: <7c78ca62-bb7e-4b95-9590-ad21f1c6f171@linaro.org> (raw)
In-Reply-To: <85bdaa4d-3805-4ae3-917e-7258a131741c@quicinc.com>
On 06/02/2025 3:02 am, Jie Gan wrote:
>
>
> On 2/1/2025 12:36 AM, James Clark wrote:
>> Signed-off-by: James Clark <james.clark@linaro.org>
>> ---
>> drivers/hwtracing/coresight/coresight-core.c | 10 +++++-----
>> drivers/hwtracing/coresight/coresight-dummy.c | 2 +-
>> drivers/hwtracing/coresight/coresight-etb10.c | 8 +++-----
>> drivers/hwtracing/coresight/coresight-etm-perf.c | 3 +--
>> drivers/hwtracing/coresight/coresight-priv.h | 4 ++--
>> drivers/hwtracing/coresight/coresight-sysfs.c | 2 +-
>> drivers/hwtracing/coresight/coresight-tmc-etf.c | 9 ++++-----
>> drivers/hwtracing/coresight/coresight-tmc-etr.c | 13 +++++--------
>> drivers/hwtracing/coresight/coresight-tmc.h | 2 +-
>> drivers/hwtracing/coresight/coresight-tpiu.c | 2 +-
>> drivers/hwtracing/coresight/coresight-trbe.c | 4 +---
>> drivers/hwtracing/coresight/ultrasoc-smb.c | 8 +++-----
>> include/linux/coresight.h | 2 +-
>> 13 files changed, 29 insertions(+), 40 deletions(-)
>>
>
> Hi James
>
> I removed the handle from coresight_path and placed these codes in a
> separate patch. However, I believe this change is not related to the
> CTCU driver or coresight_path. Therefore, I suggest we submit it
> independently.
>
> Thanks,
> Jie
>
>
Yeah if you've removed the handle from the path then we can do the other
void* changes separately. Makes sense.
>> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/
>> hwtracing/coresight/coresight-core.c
>> index 11d5d5320b1a..253ef02fde12 100644
>> --- a/drivers/hwtracing/coresight/coresight-core.c
>> +++ b/drivers/hwtracing/coresight/coresight-core.c
>> @@ -272,9 +272,9 @@ void coresight_add_helper(struct coresight_device
>> *csdev,
>> EXPORT_SYMBOL_GPL(coresight_add_helper);
>> static int coresight_enable_sink(struct coresight_device *csdev,
>> - enum cs_mode mode, void *data)
>> + enum cs_mode mode, struct perf_output_handle *handle)
>> {
>> - return sink_ops(csdev)->enable(csdev, mode, data);
>> + return sink_ops(csdev)->enable(csdev, mode, handle);
>> }
>> static void coresight_disable_sink(struct coresight_device *csdev)
>> @@ -448,7 +448,8 @@ static int coresight_enable_helpers(struct
>> coresight_device *csdev,
>> return 0;
>> }
>> -int coresight_enable_path(struct coresight_path *cs_path, enum
>> cs_mode mode)
>> +int coresight_enable_path(struct coresight_path *cs_path, enum
>> cs_mode mode,
>> + struct perf_output_handle *handle)
>> {
>> int ret = 0;
>> u32 type;
>> @@ -479,7 +480,7 @@ int coresight_enable_path(struct coresight_path
>> *cs_path, enum cs_mode mode)
>> switch (type) {
>> case CORESIGHT_DEV_TYPE_SINK:
>> - ret = coresight_enable_sink(csdev, mode, cs_path);
>> + ret = coresight_enable_sink(csdev, mode, handle);
>> /*
>> * Sink is the first component turned on. If we
>> * failed to enable the sink, there are no components
>> @@ -807,7 +808,6 @@ void coresight_release_path(struct coresight_path
>> *cs_path)
>> kfree(nd);
>> }
>> - cs_path->handle = NULL;
>> kfree(cs_path->path);
>> kfree(cs_path);
>> }
>> diff --git a/drivers/hwtracing/coresight/coresight-dummy.c b/drivers/
>> hwtracing/coresight/coresight-dummy.c
>> index dfcf24e9c49a..6f86d33efef4 100644
>> --- a/drivers/hwtracing/coresight/coresight-dummy.c
>> +++ b/drivers/hwtracing/coresight/coresight-dummy.c
>> @@ -54,7 +54,7 @@ static int dummy_source_trace_id(struct
>> coresight_device *csdev, enum cs_mode mo
>> }
>> static int dummy_sink_enable(struct coresight_device *csdev, enum
>> cs_mode mode,
>> - void *data)
>> + struct perf_output_handle *handle)
>> {
>> dev_dbg(csdev->dev.parent, "Dummy sink enabled\n");
>> diff --git a/drivers/hwtracing/coresight/coresight-etb10.c b/drivers/
>> hwtracing/coresight/coresight-etb10.c
>> index 05430d8931d1..e373b0f590bf 100644
>> --- a/drivers/hwtracing/coresight/coresight-etb10.c
>> +++ b/drivers/hwtracing/coresight/coresight-etb10.c
>> @@ -167,14 +167,12 @@ static int etb_enable_sysfs(struct
>> coresight_device *csdev)
>> return ret;
>> }
>> -static int etb_enable_perf(struct coresight_device *csdev, void *data)
>> +static int etb_enable_perf(struct coresight_device *csdev, struct
>> perf_output_handle *handle)
>> {
>> int ret = 0;
>> pid_t pid;
>> unsigned long flags;
>> struct etb_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
>> - struct coresight_path *cs_path = (struct coresight_path *)data;
>> - struct perf_output_handle *handle = cs_path->handle;
>> struct cs_buffers *buf = etm_perf_sink_config(handle);
>> spin_lock_irqsave(&drvdata->spinlock, flags);
>> @@ -225,7 +223,7 @@ static int etb_enable_perf(struct coresight_device
>> *csdev, void *data)
>> }
>> static int etb_enable(struct coresight_device *csdev, enum cs_mode
>> mode,
>> - void *data)
>> + struct perf_output_handle *handle)
>> {
>> int ret;
>> @@ -234,7 +232,7 @@ static int etb_enable(struct coresight_device
>> *csdev, enum cs_mode mode,
>> ret = etb_enable_sysfs(csdev);
>> break;
>> case CS_MODE_PERF:
>> - ret = etb_enable_perf(csdev, data);
>> + ret = etb_enable_perf(csdev, handle);
>> break;
>> default:
>> ret = -EINVAL;
>> diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/
>> drivers/hwtracing/coresight/coresight-etm-perf.c
>> index b6765abb0a26..0fad9968c2c0 100644
>> --- a/drivers/hwtracing/coresight/coresight-etm-perf.c
>> +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c
>> @@ -501,9 +501,8 @@ static void etm_event_start(struct perf_event
>> *event, int flags)
>> if (WARN_ON_ONCE(!sink))
>> goto fail_end_stop;
>> - cs_path->handle = handle;
>> /* Nothing will happen without a path */
>> - if (coresight_enable_path(cs_path, CS_MODE_PERF))
>> + if (coresight_enable_path(cs_path, CS_MODE_PERF, handle))
>> goto fail_end_stop;
>> /* Finally enable the tracer */
>> diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/
>> hwtracing/coresight/coresight-priv.h
>> index 8e02a222b9f8..7bd43304f461 100644
>> --- a/drivers/hwtracing/coresight/coresight-priv.h
>> +++ b/drivers/hwtracing/coresight/coresight-priv.h
>> @@ -112,7 +112,6 @@ struct cs_buffers {
>> * @trace_id: trace_id of the whole path.
>> */
>> struct coresight_path {
>> - struct perf_output_handle *handle;
>> struct list_head *path;
>> u8 trace_id;
>> };
>> @@ -142,7 +141,8 @@ static inline void CS_UNLOCK(void __iomem *addr)
>> }
>> void coresight_disable_path(struct coresight_path *cs_path);
>> -int coresight_enable_path(struct coresight_path *cs_path, enum
>> cs_mode mode);
>> +int coresight_enable_path(struct coresight_path *cs_path, enum
>> cs_mode mode,
>> + struct perf_output_handle *handle);
>> struct coresight_device *coresight_get_sink(struct list_head *path);
>> struct coresight_device *coresight_get_sink_by_id(u32 id);
>> struct coresight_device *
>> diff --git a/drivers/hwtracing/coresight/coresight-sysfs.c b/drivers/
>> hwtracing/coresight/coresight-sysfs.c
>> index 04e76cc1bfdf..f9a6b838726c 100644
>> --- a/drivers/hwtracing/coresight/coresight-sysfs.c
>> +++ b/drivers/hwtracing/coresight/coresight-sysfs.c
>> @@ -209,7 +209,7 @@ int coresight_enable_sysfs(struct coresight_device
>> *csdev)
>> goto out;
>> }
>> - ret = coresight_enable_path(cs_path, CS_MODE_SYSFS);
>> + ret = coresight_enable_path(cs_path, CS_MODE_SYSFS, NULL);
>> if (ret)
>> goto err_path;
>> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/
>> drivers/hwtracing/coresight/coresight-tmc-etf.c
>> index e6b07f917556..fdf1c2511d67 100644
>> --- a/drivers/hwtracing/coresight/coresight-tmc-etf.c
>> +++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c
>> @@ -244,14 +244,13 @@ static int tmc_enable_etf_sink_sysfs(struct
>> coresight_device *csdev)
>> return ret;
>> }
>> -static int tmc_enable_etf_sink_perf(struct coresight_device *csdev,
>> void *data)
>> +static int tmc_enable_etf_sink_perf(struct coresight_device *csdev,
>> + struct perf_output_handle *handle)
>> {
>> int ret = 0;
>> pid_t pid;
>> unsigned long flags;
>> struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
>> - struct coresight_path *cs_path= (struct coresight_path *)data;
>> - struct perf_output_handle *handle = cs_path->handle;
>> struct cs_buffers *buf = etm_perf_sink_config(handle);
>> spin_lock_irqsave(&drvdata->spinlock, flags);
>> @@ -303,7 +302,7 @@ static int tmc_enable_etf_sink_perf(struct
>> coresight_device *csdev, void *data)
>> }
>> static int tmc_enable_etf_sink(struct coresight_device *csdev,
>> - enum cs_mode mode, void *data)
>> + enum cs_mode mode, struct perf_output_handle *handle)
>> {
>> int ret;
>> @@ -312,7 +311,7 @@ static int tmc_enable_etf_sink(struct
>> coresight_device *csdev,
>> ret = tmc_enable_etf_sink_sysfs(csdev);
>> break;
>> case CS_MODE_PERF:
>> - ret = tmc_enable_etf_sink_perf(csdev, data);
>> + ret = tmc_enable_etf_sink_perf(csdev, handle);
>> break;
>> /* We shouldn't be here */
>> default:
>> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/
>> drivers/hwtracing/coresight/coresight-tmc-etr.c
>> index 82a872882e24..2d0bd06bab2a 100644
>> --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
>> +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
>> @@ -1252,10 +1252,8 @@ static int tmc_enable_etr_sink_sysfs(struct
>> coresight_device *csdev)
>> }
>> struct etr_buf *tmc_etr_get_buffer(struct coresight_device *csdev,
>> - enum cs_mode mode, void *data)
>> + enum cs_mode mode, struct perf_output_handle *handle)
>> {
>> - struct coresight_path *cs_path = (struct coresight_path *)data;
>> - struct perf_output_handle *handle = cs_path->handle;
>> struct etr_perf_buffer *etr_perf;
>> switch (mode) {
>> @@ -1643,14 +1641,13 @@ tmc_update_etr_buffer(struct coresight_device
>> *csdev,
>> return size;
>> }
>> -static int tmc_enable_etr_sink_perf(struct coresight_device *csdev,
>> void *data)
>> +static int tmc_enable_etr_sink_perf(struct coresight_device *csdev,
>> + struct perf_output_handle *handle)
>> {
>> int rc = 0;
>> pid_t pid;
>> unsigned long flags;
>> struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
>> - struct coresight_path *cs_path = (struct coresight_path *)data;
>> - struct perf_output_handle *handle = cs_path->handle;
>> struct etr_perf_buffer *etr_perf = etm_perf_sink_config(handle);
>> spin_lock_irqsave(&drvdata->spinlock, flags);
>> @@ -1698,13 +1695,13 @@ static int tmc_enable_etr_sink_perf(struct
>> coresight_device *csdev, void *data)
>> }
>> static int tmc_enable_etr_sink(struct coresight_device *csdev,
>> - enum cs_mode mode, void *data)
>> + enum cs_mode mode, struct perf_output_handle *handle)
>> {
>> switch (mode) {
>> case CS_MODE_SYSFS:
>> return tmc_enable_etr_sink_sysfs(csdev);
>> case CS_MODE_PERF:
>> - return tmc_enable_etr_sink_perf(csdev, data);
>> + return tmc_enable_etr_sink_perf(csdev, handle);
>> default:
>> return -EINVAL;
>> }
>> diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/
>> hwtracing/coresight/coresight-tmc.h
>> index 2671926be62a..e991afd43742 100644
>> --- a/drivers/hwtracing/coresight/coresight-tmc.h
>> +++ b/drivers/hwtracing/coresight/coresight-tmc.h
>> @@ -336,7 +336,7 @@ struct coresight_device
>> *tmc_etr_get_catu_device(struct tmc_drvdata *drvdata);
>> void tmc_etr_set_catu_ops(const struct etr_buf_operations *catu);
>> void tmc_etr_remove_catu_ops(void);
>> struct etr_buf *tmc_etr_get_buffer(struct coresight_device *csdev,
>> - enum cs_mode mode, void *data);
>> + enum cs_mode mode, struct perf_output_handle
>> *handle);
>> extern const struct attribute_group coresight_etr_group;
>> #endif
>> diff --git a/drivers/hwtracing/coresight/coresight-tpiu.c b/drivers/
>> hwtracing/coresight/coresight-tpiu.c
>> index 97ef36f03ec2..ccf463ac7bf5 100644
>> --- a/drivers/hwtracing/coresight/coresight-tpiu.c
>> +++ b/drivers/hwtracing/coresight/coresight-tpiu.c
>> @@ -75,7 +75,7 @@ static void tpiu_enable_hw(struct csdev_access *csa)
>> }
>> static int tpiu_enable(struct coresight_device *csdev, enum cs_mode
>> mode,
>> - void *__unused)
>> + struct perf_output_handle *__unused)
>> {
>> struct tpiu_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
>> diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/
>> hwtracing/coresight/coresight-trbe.c
>> index 5005efd88a66..997d5976d2d2 100644
>> --- a/drivers/hwtracing/coresight/coresight-trbe.c
>> +++ b/drivers/hwtracing/coresight/coresight-trbe.c
>> @@ -1009,12 +1009,10 @@ static int __arm_trbe_enable(struct trbe_buf
>> *buf,
>> }
>> static int arm_trbe_enable(struct coresight_device *csdev, enum
>> cs_mode mode,
>> - void *data)
>> + struct perf_output_handle *handle)
>> {
>> struct trbe_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
>> struct trbe_cpudata *cpudata = dev_get_drvdata(&csdev->dev);
>> - struct coresight_path *cs_path = (struct coresight_path *)data;
>> - struct perf_output_handle *handle = cs_path->handle;
>> struct trbe_buf *buf = etm_perf_sink_config(handle);
>> WARN_ON(cpudata->cpu != smp_processor_id());
>> diff --git a/drivers/hwtracing/coresight/ultrasoc-smb.c b/drivers/
>> hwtracing/coresight/ultrasoc-smb.c
>> index 9be88394b3bb..1574b5067206 100644
>> --- a/drivers/hwtracing/coresight/ultrasoc-smb.c
>> +++ b/drivers/hwtracing/coresight/ultrasoc-smb.c
>> @@ -213,11 +213,9 @@ static void smb_enable_sysfs(struct
>> coresight_device *csdev)
>> coresight_set_mode(csdev, CS_MODE_SYSFS);
>> }
>> -static int smb_enable_perf(struct coresight_device *csdev, void *data)
>> +static int smb_enable_perf(struct coresight_device *csdev, struct
>> perf_output_handle *handle)
>> {
>> struct smb_drv_data *drvdata = dev_get_drvdata(csdev->dev.parent);
>> - struct coresight_path *cs_path = (struct coresight_path *)data;
>> - struct perf_output_handle *handle = cs_path->handle;
>> struct cs_buffers *buf = etm_perf_sink_config(handle);
>> pid_t pid;
>> @@ -241,7 +239,7 @@ static int smb_enable_perf(struct coresight_device
>> *csdev, void *data)
>> }
>> static int smb_enable(struct coresight_device *csdev, enum cs_mode
>> mode,
>> - void *data)
>> + struct perf_output_handle *handle)
>> {
>> struct smb_drv_data *drvdata = dev_get_drvdata(csdev->dev.parent);
>> int ret = 0;
>> @@ -262,7 +260,7 @@ static int smb_enable(struct coresight_device
>> *csdev, enum cs_mode mode,
>> smb_enable_sysfs(csdev);
>> break;
>> case CS_MODE_PERF:
>> - ret = smb_enable_perf(csdev, data);
>> + ret = smb_enable_perf(csdev, handle);
>> break;
>> default:
>> ret = -EINVAL;
>> diff --git a/include/linux/coresight.h b/include/linux/coresight.h
>> index 87f9baa7fefe..a859fc00eef9 100644
>> --- a/include/linux/coresight.h
>> +++ b/include/linux/coresight.h
>> @@ -353,7 +353,7 @@ enum cs_mode {
>> */
>> struct coresight_ops_sink {
>> int (*enable)(struct coresight_device *csdev, enum cs_mode mode,
>> - void *data);
>> + struct perf_output_handle *handle);
>> int (*disable)(struct coresight_device *csdev);
>> void *(*alloc_buffer)(struct coresight_device *csdev,
>> struct perf_event *event, void **pages,
>
next prev parent reply other threads:[~2025-02-06 14:34 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-01-24 7:25 [PATCH v9 0/6] Coresight: Add Coresight TMC Control Unit driver Jie Gan
2025-01-24 7:25 ` [PATCH v9 1/6] Coresight: Add support for new APB clock name Jie Gan
2025-01-24 7:25 ` [PATCH v9 2/6] Coresight: Add trace_id function to retrieving the trace ID Jie Gan
2025-01-24 7:25 ` [PATCH v9 3/6] Coresight: Introduce a new struct coresight_path Jie Gan
2025-01-28 11:54 ` James Clark
2025-01-29 0:57 ` Jie Gan
2025-01-31 16:36 ` [PATCH 0/3] coresight: Alloc trace ID after building the path James Clark
2025-01-31 16:36 ` [PATCH 1/3] coresight: Don't save handle in path James Clark
2025-02-06 3:02 ` Jie Gan
2025-02-06 14:34 ` James Clark [this message]
2025-01-31 16:36 ` [PATCH 2/3] coresight: Export coresight_get_sink() James Clark
2025-01-31 16:36 ` [PATCH 3/3] coresight: Alloc trace ID after building the path James Clark
2025-02-05 4:13 ` [PATCH 0/3] " Jie Gan
2025-02-05 7:44 ` Jie Gan
2025-01-31 16:43 ` [PATCH v9 3/6] Coresight: Introduce a new struct coresight_path James Clark
2025-02-05 4:09 ` Jie Gan
2025-01-24 7:25 ` [PATCH v9 4/6] dt-bindings: arm: Add Coresight TMC Control Unit hardware Jie Gan
2025-01-24 7:25 ` [PATCH v9 5/6] Coresight: Add Coresight TMC Control Unit driver Jie Gan
2025-01-28 11:55 ` James Clark
2025-01-29 0:46 ` Jie Gan
2025-01-29 10:35 ` James Clark
2025-01-29 13:02 ` Jie Gan
2025-01-29 14:07 ` James Clark
2025-01-24 7:25 ` [PATCH v9 6/6] arm64: dts: qcom: sa8775p: Add CTCU and ETR nodes Jie Gan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7c78ca62-bb7e-4b95-9590-ad21f1c6f171@linaro.org \
--to=james.clark@linaro.org \
--cc=alexander.shishkin@linux.intel.com \
--cc=alexandre.torgue@foss.st.com \
--cc=andersson@kernel.org \
--cc=conor+dt@kernel.org \
--cc=coresight@lists.linaro.org \
--cc=devicetree@vger.kernel.org \
--cc=konradybcio@kernel.org \
--cc=krzk+dt@kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-stm32@st-md-mailman.stormreply.com \
--cc=mcoquelin.stm32@gmail.com \
--cc=mike.leach@linaro.org \
--cc=quic_jiegan@quicinc.com \
--cc=quic_jinlmao@quicinc.com \
--cc=quic_tingweiz@quicinc.com \
--cc=robh@kernel.org \
--cc=suzuki.poulose@arm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®