Create and insert or remove the etr_buf_node to/from the etr_buf_list.
Signed-off-by: Jie Gan <jie.gan@oss.qualcomm.com>
---
.../hwtracing/coresight/coresight-tmc-etr.c | 65 +++++++++++++++++++
drivers/hwtracing/coresight/coresight-tmc.h | 2 +
2 files changed, 67 insertions(+)
diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
index b07fcdb3fe1a..e8ecb3e087ab 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
@@ -1909,6 +1909,71 @@ const struct coresight_ops tmc_etr_cs_ops = {
.panic_ops = &tmc_etr_sync_ops,
};
+/**
+ * tmc_clean_etr_buf_list - clean the etr_buf_list.
+ * @drvdata: driver data of the TMC device.
+ *
+ * Remove the allocated node from the list and free the extra buffer.
+ */
+void tmc_clean_etr_buf_list(struct tmc_drvdata *drvdata)
+{
+ struct etr_buf_node *nd, *next;
+
+ list_for_each_entry_safe(nd, next, &drvdata->etr_buf_list, node) {
+ if (nd->sysfs_buf == drvdata->sysfs_buf) {
+ list_del(&nd->node);
+ kfree(nd);
+ } else {
+ /* Free allocated buffers which are not utilized by ETR */
+ list_del(&nd->node);
+ tmc_free_etr_buf(nd->sysfs_buf);
+ nd->sysfs_buf = NULL;
+ kfree(nd);
+ }
+ }
+}
+EXPORT_SYMBOL_GPL(tmc_clean_etr_buf_list);
+
+/**
+ * tmc_create_etr_buf_node - create a node to store the alloc_buf and
+ * insert the node to the etr_buf_list. Create a new buffer if the
+ * alloc_buf is NULL.
+ * @drvdata: driver data of the TMC device.
+ * @alloc_buf: the buffer that is inserted to the list.
+ *
+ * Return 0 upon success and return the error number if fail.
+ */
+int tmc_create_etr_buf_node(struct tmc_drvdata *drvdata, struct etr_buf *alloc_buf)
+{
+ struct etr_buf_node *sysfs_buf_node;
+ struct etr_buf *sysfs_buf;
+
+ if (!alloc_buf) {
+ sysfs_buf = tmc_alloc_etr_buf(drvdata, drvdata->size, 0, cpu_to_node(0), NULL);
+ if (IS_ERR(sysfs_buf))
+ return PTR_ERR(sysfs_buf);
+ } else
+ sysfs_buf = alloc_buf;
+
+ sysfs_buf_node = kzalloc(sizeof(struct etr_buf_node), GFP_KERNEL);
+ if (IS_ERR(sysfs_buf_node)) {
+ if (!alloc_buf)
+ tmc_free_etr_buf(sysfs_buf);
+ return PTR_ERR(sysfs_buf_node);
+ }
+
+ sysfs_buf_node->sysfs_buf = sysfs_buf;
+ sysfs_buf_node->reading = false;
+ if (!alloc_buf)
+ sysfs_buf_node->is_free = true;
+ else
+ sysfs_buf_node->is_free = false;
+ list_add(&sysfs_buf_node->node, &drvdata->etr_buf_list);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(tmc_create_etr_buf_node);
+
int tmc_read_prepare_etr(struct tmc_drvdata *drvdata)
{
int ret = 0;
diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h
index 52ee5f8efe8c..3cb8ba9f88f5 100644
--- a/drivers/hwtracing/coresight/coresight-tmc.h
+++ b/drivers/hwtracing/coresight/coresight-tmc.h
@@ -461,5 +461,7 @@ void tmc_etr_remove_catu_ops(void);
struct etr_buf *tmc_etr_get_buffer(struct coresight_device *csdev,
enum cs_mode mode, void *data);
extern const struct attribute_group coresight_etr_group;
+void tmc_clean_etr_buf_list(struct tmc_drvdata *drvdata);
+int tmc_create_etr_buf_node(struct tmc_drvdata *drvdata, struct etr_buf *alloc_buf);
#endif
--
2.34.1Hi,
On Fri, 25 Jul 2025 at 11:08, Jie Gan <jie.gan@oss.qualcomm.com> wrote:
>
> Create and insert or remove the etr_buf_node to/from the etr_buf_list.
>
> Signed-off-by: Jie Gan <jie.gan@oss.qualcomm.com>
> ---
> .../hwtracing/coresight/coresight-tmc-etr.c | 65 +++++++++++++++++++
> drivers/hwtracing/coresight/coresight-tmc.h | 2 +
> 2 files changed, 67 insertions(+)
>
> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> index b07fcdb3fe1a..e8ecb3e087ab 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> @@ -1909,6 +1909,71 @@ const struct coresight_ops tmc_etr_cs_ops = {
> .panic_ops = &tmc_etr_sync_ops,
> };
>
> +/**
> + * tmc_clean_etr_buf_list - clean the etr_buf_list.
> + * @drvdata: driver data of the TMC device.
> + *
> + * Remove the allocated node from the list and free the extra buffer.
> + */
> +void tmc_clean_etr_buf_list(struct tmc_drvdata *drvdata)
> +{
> + struct etr_buf_node *nd, *next;
> +
> + list_for_each_entry_safe(nd, next, &drvdata->etr_buf_list, node) {
> + if (nd->sysfs_buf == drvdata->sysfs_buf) {
> + list_del(&nd->node);
> + kfree(nd);
> + } else {
> + /* Free allocated buffers which are not utilized by ETR */
> + list_del(&nd->node);
> + tmc_free_etr_buf(nd->sysfs_buf);
> + nd->sysfs_buf = NULL;
> + kfree(nd);
> + }
> + }
> +}
> +EXPORT_SYMBOL_GPL(tmc_clean_etr_buf_list);
> +
> +/**
> + * tmc_create_etr_buf_node - create a node to store the alloc_buf and
> + * insert the node to the etr_buf_list. Create a new buffer if the
> + * alloc_buf is NULL.
> + * @drvdata: driver data of the TMC device.
> + * @alloc_buf: the buffer that is inserted to the list.
> + *
> + * Return 0 upon success and return the error number if fail.
> + */
> +int tmc_create_etr_buf_node(struct tmc_drvdata *drvdata, struct etr_buf *alloc_buf)
This list handle function pair look a little asymmetric. Is it not
possible to change this to tmc_create_etr_buf_list(struct tmc_drvdata
*drvdata, int num_nodes)
so that one function creates all the nodes, and another destroys them.
In the logic that decides between using multi buffer or single buffer
you can then use a construct such as:
if (single_buffer)
drvdata->sysfs_buf = <alloc sysfs buffer>
else {
tmc_create_etr_buf_list(drvdata, 2);
<switch in avail buffer to drvdata->sysfs_buf
}
> +{
> + struct etr_buf_node *sysfs_buf_node;
> + struct etr_buf *sysfs_buf;
> +
> + if (!alloc_buf) {
> + sysfs_buf = tmc_alloc_etr_buf(drvdata, drvdata->size, 0, cpu_to_node(0), NULL);
> + if (IS_ERR(sysfs_buf))
> + return PTR_ERR(sysfs_buf);
> + } else
> + sysfs_buf = alloc_buf;
> +
> + sysfs_buf_node = kzalloc(sizeof(struct etr_buf_node), GFP_KERNEL);
> + if (IS_ERR(sysfs_buf_node)) {
> + if (!alloc_buf)
> + tmc_free_etr_buf(sysfs_buf);
> + return PTR_ERR(sysfs_buf_node);
> + }
> +
> + sysfs_buf_node->sysfs_buf = sysfs_buf;
> + sysfs_buf_node->reading = false;
> + if (!alloc_buf)
> + sysfs_buf_node->is_free = true;
> + else
> + sysfs_buf_node->is_free = false;
> + list_add(&sysfs_buf_node->node, &drvdata->etr_buf_list);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(tmc_create_etr_buf_node);
> +
> int tmc_read_prepare_etr(struct tmc_drvdata *drvdata)
> {
> int ret = 0;
> diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h
> index 52ee5f8efe8c..3cb8ba9f88f5 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc.h
> +++ b/drivers/hwtracing/coresight/coresight-tmc.h
> @@ -461,5 +461,7 @@ void tmc_etr_remove_catu_ops(void);
> struct etr_buf *tmc_etr_get_buffer(struct coresight_device *csdev,
> enum cs_mode mode, void *data);
> extern const struct attribute_group coresight_etr_group;
> +void tmc_clean_etr_buf_list(struct tmc_drvdata *drvdata);
> +int tmc_create_etr_buf_node(struct tmc_drvdata *drvdata, struct etr_buf *alloc_buf);
>
> #endif
> --
> 2.34.1
>
Regards
Mike
--
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK
On 8/5/2025 6:27 PM, Mike Leach wrote:
> Hi,
>
> On Fri, 25 Jul 2025 at 11:08, Jie Gan <jie.gan@oss.qualcomm.com> wrote:
>>
>> Create and insert or remove the etr_buf_node to/from the etr_buf_list.
>>
>> Signed-off-by: Jie Gan <jie.gan@oss.qualcomm.com>
>> ---
>> .../hwtracing/coresight/coresight-tmc-etr.c | 65 +++++++++++++++++++
>> drivers/hwtracing/coresight/coresight-tmc.h | 2 +
>> 2 files changed, 67 insertions(+)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
>> index b07fcdb3fe1a..e8ecb3e087ab 100644
>> --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
>> +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
>> @@ -1909,6 +1909,71 @@ const struct coresight_ops tmc_etr_cs_ops = {
>> .panic_ops = &tmc_etr_sync_ops,
>> };
>>
>> +/**
>> + * tmc_clean_etr_buf_list - clean the etr_buf_list.
>> + * @drvdata: driver data of the TMC device.
>> + *
>> + * Remove the allocated node from the list and free the extra buffer.
>> + */
>> +void tmc_clean_etr_buf_list(struct tmc_drvdata *drvdata)
>> +{
>> + struct etr_buf_node *nd, *next;
>> +
>> + list_for_each_entry_safe(nd, next, &drvdata->etr_buf_list, node) {
>> + if (nd->sysfs_buf == drvdata->sysfs_buf) {
>> + list_del(&nd->node);
>> + kfree(nd);
>> + } else {
>> + /* Free allocated buffers which are not utilized by ETR */
>> + list_del(&nd->node);
>> + tmc_free_etr_buf(nd->sysfs_buf);
>> + nd->sysfs_buf = NULL;
>> + kfree(nd);
>> + }
>> + }
>> +}
>> +EXPORT_SYMBOL_GPL(tmc_clean_etr_buf_list);
>> +
>> +/**
>> + * tmc_create_etr_buf_node - create a node to store the alloc_buf and
>> + * insert the node to the etr_buf_list. Create a new buffer if the
>> + * alloc_buf is NULL.
>> + * @drvdata: driver data of the TMC device.
>> + * @alloc_buf: the buffer that is inserted to the list.
>> + *
>> + * Return 0 upon success and return the error number if fail.
>> + */
>> +int tmc_create_etr_buf_node(struct tmc_drvdata *drvdata, struct etr_buf *alloc_buf)
>
> This list handle function pair look a little asymmetric. Is it not
> possible to change this to tmc_create_etr_buf_list(struct tmc_drvdata
> *drvdata, int num_nodes)
> so that one function creates all the nodes, and another destroys them.
>
> In the logic that decides between using multi buffer or single buffer
> you can then use a construct such as:
>
> if (single_buffer)
> drvdata->sysfs_buf = <alloc sysfs buffer>
> else {
> tmc_create_etr_buf_list(drvdata, 2);
> <switch in avail buffer to drvdata->sysfs_buf
> }
The lsit handle function pair definitely looks a little bit asymmetric.
I will consider the suggestion in next version.
Thanks,
Jie
>
>> +{
>> + struct etr_buf_node *sysfs_buf_node;
>> + struct etr_buf *sysfs_buf;
>> +
>> + if (!alloc_buf) {
>> + sysfs_buf = tmc_alloc_etr_buf(drvdata, drvdata->size, 0, cpu_to_node(0), NULL);
>> + if (IS_ERR(sysfs_buf))
>> + return PTR_ERR(sysfs_buf);
>> + } else
>> + sysfs_buf = alloc_buf;
>> +
>> + sysfs_buf_node = kzalloc(sizeof(struct etr_buf_node), GFP_KERNEL);
>> + if (IS_ERR(sysfs_buf_node)) {
>> + if (!alloc_buf)
>> + tmc_free_etr_buf(sysfs_buf);
>> + return PTR_ERR(sysfs_buf_node);
>> + }
>> +
>> + sysfs_buf_node->sysfs_buf = sysfs_buf;
>> + sysfs_buf_node->reading = false;
>> + if (!alloc_buf)
>> + sysfs_buf_node->is_free = true;
>> + else
>> + sysfs_buf_node->is_free = false;
>> + list_add(&sysfs_buf_node->node, &drvdata->etr_buf_list);
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(tmc_create_etr_buf_node);
>> +
>> int tmc_read_prepare_etr(struct tmc_drvdata *drvdata)
>> {
>> int ret = 0;
>> diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h
>> index 52ee5f8efe8c..3cb8ba9f88f5 100644
>> --- a/drivers/hwtracing/coresight/coresight-tmc.h
>> +++ b/drivers/hwtracing/coresight/coresight-tmc.h
>> @@ -461,5 +461,7 @@ void tmc_etr_remove_catu_ops(void);
>> struct etr_buf *tmc_etr_get_buffer(struct coresight_device *csdev,
>> enum cs_mode mode, void *data);
>> extern const struct attribute_group coresight_etr_group;
>> +void tmc_clean_etr_buf_list(struct tmc_drvdata *drvdata);
>> +int tmc_create_etr_buf_node(struct tmc_drvdata *drvdata, struct etr_buf *alloc_buf);
>>
>> #endif
>> --
>> 2.34.1
>>
>
> Regards
>
> Mike
© 2016 - 2025 Red Hat, Inc.