Re: [PATCH v10 3/8] coresight: tmc: Introduce sysfs_read_ops to wrap sysfs read operations

On 22/01/2026 02:08, Jie Gan wrote:

Introduce sysfs_read_ops as a wrapper, wrap sysfs read operations, for reading trace data from the TMC buffer.

Reviewed-by: Mike Leach mike.leach@linaro.org Signed-off-by: Jie Gan jie.gan@oss.qualcomm.com

---

drivers/hwtracing/coresight/coresight-tmc-core.c | 50 +++++++++++------------- drivers/hwtracing/coresight/coresight-tmc.h | 17 ++++++++ 2 files changed, 40 insertions(+), 27 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c b/drivers/hwtracing/coresight/coresight-tmc-core.c index 1ea255ffa67c..8b0397a77e57 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-core.c +++ b/drivers/hwtracing/coresight/coresight-tmc-core.c @@ -232,17 +232,10 @@ static int tmc_read_prepare(struct tmc_drvdata *drvdata) { int ret = 0;

• switch (drvdata->config_type) {
• case TMC_CONFIG_TYPE_ETB:
• case TMC_CONFIG_TYPE_ETF:

• ret = tmc_read_prepare_etb(drvdata);
  

• break;
  

• case TMC_CONFIG_TYPE_ETR:

• ret = tmc_read_prepare_etr(drvdata);
  

• break;
  

• default:

• if (drvdata->sysfs_ops)

• ret = drvdata->sysfs_ops->read_prepare(drvdata);
  

• else ret = -EINVAL;

• }

if (!ret) dev_dbg(&drvdata->csdev->dev, "TMC read start\n"); @@ -254,17 +247,10 @@ static int tmc_read_unprepare(struct tmc_drvdata *drvdata) { int ret = 0;

• switch (drvdata->config_type) {
• case TMC_CONFIG_TYPE_ETB:
• case TMC_CONFIG_TYPE_ETF:

• ret = tmc_read_unprepare_etb(drvdata);
  

• break;
  

• case TMC_CONFIG_TYPE_ETR:

• ret = tmc_read_unprepare_etr(drvdata);
  

• break;
  

• default:

• if (drvdata->sysfs_ops)

• ret = drvdata->sysfs_ops->read_unprepare(drvdata);
  

• else ret = -EINVAL;

• }

if (!ret) dev_dbg(&drvdata->csdev->dev, "TMC read end\n"); @@ -291,13 +277,8 @@ static int tmc_open(struct inode *inode, struct file *file) static ssize_t tmc_get_sysfs_trace(struct tmc_drvdata *drvdata, loff_t pos, size_t len, char **bufpp) {

• switch (drvdata->config_type) {
• case TMC_CONFIG_TYPE_ETB:
• case TMC_CONFIG_TYPE_ETF:

• return tmc_etb_get_sysfs_trace(drvdata, pos, len, bufpp);
  

• case TMC_CONFIG_TYPE_ETR:

• return tmc_etr_get_sysfs_trace(drvdata, pos, len, bufpp);
  

• }

• if (drvdata->sysfs_ops)

• return drvdata->sysfs_ops->get_trace_data(drvdata, pos, len, bufpp);
  

minor nit: Please could we bail out in tmc_open() if the drvdata->sysfs_ops is not set. That way, we don't have to sprinkle it everywhere. And also, we don't expect to see this case anyways.

return -EINVAL; } @@ -769,6 +750,18 @@ static void register_crash_dev_interface(struct tmc_drvdata *drvdata, "Valid crash tracedata found\n"); } +static const struct sysfs_read_ops tmc_etb_sysfs_read_ops = {

minor nit: please could we rename the struct type to :

struct tmc_sysfs_ops

and then use

etb_sysfs_ops = {} etr_sysfs_ops = {}

Rest looks fine to me

Suzuki

• .read_prepare = tmc_read_prepare_etb,
• .read_unprepare = tmc_read_unprepare_etb,
• .get_trace_data = tmc_etb_get_sysfs_trace,

+};

+static const struct sysfs_read_ops tmc_etr_sysfs_read_ops = {

• .read_prepare = tmc_read_prepare_etr,
• .read_unprepare = tmc_read_unprepare_etr,
• .get_trace_data = tmc_etr_get_sysfs_trace,

+};

• static int __tmc_probe(struct device *dev, struct resource *res) { int ret = 0;

@@ -828,6 +821,7 @@ static int __tmc_probe(struct device *dev, struct resource *res) desc.subtype.sink_subtype = CORESIGHT_DEV_SUBTYPE_SINK_BUFFER; desc.ops = &tmc_etb_cs_ops; dev_list = &etb_devs;

• drvdata->sysfs_ops = &tmc_etb_sysfs_read_ops;
  

  break; case TMC_CONFIG_TYPE_ETR: desc.groups = coresight_etr_groups;

@@ -841,6 +835,7 @@ static int __tmc_probe(struct device *dev, struct resource *res) mutex_init(&drvdata->idr_mutex); dev_list = &etr_devs; INIT_LIST_HEAD(&drvdata->etr_buf_list);

• drvdata->sysfs_ops = &tmc_etr_sysfs_read_ops;
  

  break; case TMC_CONFIG_TYPE_ETF: desc.groups = coresight_etf_groups;

@@ -849,6 +844,7 @@ static int __tmc_probe(struct device *dev, struct resource *res) desc.subtype.link_subtype = CORESIGHT_DEV_SUBTYPE_LINK_FIFO; desc.ops = &tmc_etf_cs_ops; dev_list = &etf_devs;

• drvdata->sysfs_ops = &tmc_etb_sysfs_read_ops;
  

  break; default: pr_err("%s: Unsupported TMC config\n", desc.name);

diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h index 9b3c4e6f0a5e..c9a82ff6cd00 100644 --- a/drivers/hwtracing/coresight/coresight-tmc.h +++ b/drivers/hwtracing/coresight/coresight-tmc.h @@ -221,6 +221,8 @@ struct etr_buf_node { struct list_head node; }; +struct sysfs_read_ops;

• /**
  • struct tmc_drvdata - specifics associated to an TMC component
  • @atclk: optional clock for the core parts of the TMC.

@@ -258,6 +260,7 @@ struct etr_buf_node {

•  Used by ETR/ETF.
  

• @etr_buf_list: List that is used to manage allocated etr_buf.
• @reading_node: Available buffer_node for byte-cntr reading.

• • @sysfs_ops: Read operations for sysfs mode.
  */ struct tmc_drvdata { struct clk *atclk;

@@ -290,6 +293,20 @@ struct tmc_drvdata { struct tmc_resrv_buf crash_mdata; struct list_head etr_buf_list; struct etr_buf_node *reading_node;

• const struct sysfs_read_ops *sysfs_ops;

+};

+/**

• • struct sysfs_read_ops - read operations for TMC and its helper devices
• • @read_prepare: prepare operation.
• • @read_unprepare: unprepare operation.
• • @get_trace_data: read operation.
• */

+struct sysfs_read_ops {

• int (*read_prepare)(struct tmc_drvdata *drvdata);
• int (*read_unprepare)(struct tmc_drvdata *drvdata);
• ssize_t (*get_trace_data)(struct tmc_drvdata *drvdata, loff_t pos,

• 		  size_t len, char **bufpp);
  

  };

struct etr_buf_operations {