CoreSight

coresight@lists.linaro.org

6 participants
2571 discussions

Re: [PATCH v4 05/10] coresight: tmc: Introduce tmc_read_ops to wrap read operations

by Mike Leach

On Wed, 6 Aug 2025 at 07:30, Jie Gan <jie.gan(a)oss.qualcomm.com> wrote: > > > > On 8/5/2025 6:55 PM, Mike Leach wrote: > > Hi > > > > On Fri, 25 Jul 2025 at 11:08, Jie Gan <jie.gan(a)oss.qualcomm.com> wrote: > >> > >> Introduce tmc_read_ops as a wrapper, wrap read operations, for reading > >> trace data from the TMC buffer. > >> > >> Signed-off-by: Jie Gan <jie.gan(a)oss.qualcomm.com> > >> --- > >> .../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 4d249af93097..f668047c5df4 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->read_ops) > >> + ret = drvdata->read_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->read_ops) > >> + ret = drvdata->read_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->read_ops) > >> + return drvdata->read_ops->get_trace_data(drvdata, pos, len, bufpp); > >> > >> return -EINVAL; > >> } > >> @@ -769,6 +750,18 @@ static void register_crash_dev_interface(struct tmc_drvdata *drvdata, > >> "Valid crash tracedata found\n"); > >> } > >> > >> +static const struct tmc_read_ops tmc_etb_read_ops = { > >> + .read_prepare = tmc_read_prepare_etb, > >> + .read_unprepare = tmc_read_unprepare_etb, > >> + .get_trace_data = tmc_etb_get_sysfs_trace, > >> +}; > >> + > >> +static const struct tmc_read_ops tmc_etr_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; > >> @@ -818,6 +811,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->read_ops = &tmc_etb_read_ops; > >> break; > >> case TMC_CONFIG_TYPE_ETR: > >> desc.groups = coresight_etr_groups; > >> @@ -831,6 +825,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->read_ops = &tmc_etr_read_ops; > >> break; > >> case TMC_CONFIG_TYPE_ETF: > >> desc.groups = coresight_etf_groups; > >> @@ -839,6 +834,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->read_ops = &tmc_etb_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 3cb8ba9f88f5..2ad8e288c94b 100644 > >> --- a/drivers/hwtracing/coresight/coresight-tmc.h > >> +++ b/drivers/hwtracing/coresight/coresight-tmc.h > >> @@ -223,6 +223,8 @@ struct etr_buf_node { > >> struct list_head node; > >> }; > >> > >> +struct tmc_read_ops; > >> + > > > > declare the entire structure here rather than later. > > There is an issue of declare the entire structure here because the > function pointer needs use the struct tmc_drvdata which is not defined > at this point and it will cause a compile error. > Understood. Mike > Thanks, > Jie > > > > >> /** > >> * struct tmc_drvdata - specifics associated to an TMC component > >> * @pclk: APB clock if present, otherwise NULL > >> @@ -259,6 +261,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 for byte-cntr reading. > >> + * @tmc_read_ops: Read operations for TMC device. > >> */ > >> struct tmc_drvdata { > >> struct clk *pclk; > >> @@ -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 tmc_read_ops *read_ops; > > > > probably should be named sysfs_read_ops to be consistent with the > > perf/sysfs differentiation within the rest of the structure > > > >> +}; > >> + > >> +/** > >> + * struct tmc_read_ops - read operations for TMC and its helper devices > >> + * @read_prepare: prepare operation. > >> + * @read_unprepare: unprepare operation. > >> + * @get_trace_data: read operation. > >> + */ > >> +struct tmc_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 { > >> -- > >> 2.34.1 > >> > > > > with the above changes:- > > > > Reviewed-by: Mike Leach <mike.leach(a)linaro.org> > > > -- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK

4 months, 1 week

Re: [PATCH v4 08/10] coresight: add a new function in helper_ops

by Mike Leach

Hi, On Fri, 25 Jul 2025 at 11:08, Jie Gan <jie.gan(a)oss.qualcomm.com> wrote: > > Add a new function to identifiy whether the byte-cntr function is > enabled or not in helper_ops. > > The byte-cntr's read_ops is expected if the byte-cntr is enabled when > the user try to read trace data via sysfs node. > > Signed-off-by: Jie Gan <jie.gan(a)oss.qualcomm.com> > --- > .../hwtracing/coresight/coresight-ctcu-core.c | 35 +++++++++++++++++++ > include/linux/coresight.h | 3 ++ > 2 files changed, 38 insertions(+) > > diff --git a/drivers/hwtracing/coresight/coresight-ctcu-core.c b/drivers/hwtracing/coresight/coresight-ctcu-core.c > index 8fc08e42187e..dec911980939 100644 > --- a/drivers/hwtracing/coresight/coresight-ctcu-core.c > +++ b/drivers/hwtracing/coresight/coresight-ctcu-core.c > @@ -234,9 +234,44 @@ static int ctcu_disable(struct coresight_device *csdev, void *data) > return ctcu_set_etr_traceid(csdev, path, false); > } > > +static bool ctcu_qcom_byte_cntr_in_use(struct coresight_device *csdev, > + void **data) > +{ > + struct ctcu_byte_cntr *byte_cntr_data; > + struct coresight_device *helper; > + struct ctcu_drvdata *drvdata; > + int port; > + > + if (!csdev) > + return false; > + > + helper = coresight_get_helper(csdev, CORESIGHT_DEV_SUBTYPE_HELPER_CTCU); > + if (!helper) > + return false; > + > + port = coresight_get_in_port_dest(csdev, helper); > + if (port < 0) > + return false; > + > + drvdata = dev_get_drvdata(helper->dev.parent); > + /* Something wrong when initialize byte_cntr_read_ops */ > + if (!drvdata->byte_cntr_read_ops) > + return false; > + > + byte_cntr_data = &drvdata->byte_cntr_data[port]; > + /* Return the pointer of the ctcu_drvdata if byte-cntr has enabled */ > + if (byte_cntr_data && byte_cntr_data->thresh_val) { > + *data = (void *)drvdata->byte_cntr_read_ops; > + return true; > + } > + > + return false; > +} > + > static const struct coresight_ops_helper ctcu_helper_ops = { > .enable = ctcu_enable, > .disable = ctcu_disable, > + .qcom_byte_cntr_in_use = ctcu_qcom_byte_cntr_in_use, These ops structures are for generic functions used throughout the devices. Do not put this function here. This is a specific ctcu helper. Make it external to the file and declare it in coresight-ctcu.h Mike > }; > > static const struct coresight_ops ctcu_ops = { > diff --git a/include/linux/coresight.h b/include/linux/coresight.h > index 4ac65c68bbf4..b5f052854b08 100644 > --- a/include/linux/coresight.h > +++ b/include/linux/coresight.h > @@ -419,11 +419,14 @@ struct coresight_ops_source { > * > * @enable : Enable the device > * @disable : Disable the device > + * @qcom_byte_cntr_in_use: check whether the byte-cntr is enabled. > */ > struct coresight_ops_helper { > int (*enable)(struct coresight_device *csdev, enum cs_mode mode, > void *data); > int (*disable)(struct coresight_device *csdev, void *data); > + bool (*qcom_byte_cntr_in_use)(struct coresight_device *csdev, > + void **data); > }; > > > -- > 2.34.1 > -- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK

4 months, 1 week

Re: [PATCH v4 08/10] coresight: add a new function in helper_ops

by Mike Leach

Hi, On Fri, 25 Jul 2025 at 11:08, Jie Gan <jie.gan(a)oss.qualcomm.com> wrote: > > Add a new function to identifiy whether the byte-cntr function is > enabled or not in helper_ops. > > The byte-cntr's read_ops is expected if the byte-cntr is enabled when > the user try to read trace data via sysfs node. > > Signed-off-by: Jie Gan <jie.gan(a)oss.qualcomm.com> > --- > .../hwtracing/coresight/coresight-ctcu-core.c | 35 +++++++++++++++++++ > include/linux/coresight.h | 3 ++ > 2 files changed, 38 insertions(+) > > diff --git a/drivers/hwtracing/coresight/coresight-ctcu-core.c b/drivers/hwtracing/coresight/coresight-ctcu-core.c > index 8fc08e42187e..dec911980939 100644 > --- a/drivers/hwtracing/coresight/coresight-ctcu-core.c > +++ b/drivers/hwtracing/coresight/coresight-ctcu-core.c > @@ -234,9 +234,44 @@ static int ctcu_disable(struct coresight_device *csdev, void *data) > return ctcu_set_etr_traceid(csdev, path, false); > } > > +static bool ctcu_qcom_byte_cntr_in_use(struct coresight_device *csdev, > + void **data) > +{ > + struct ctcu_byte_cntr *byte_cntr_data; > + struct coresight_device *helper; > + struct ctcu_drvdata *drvdata; > + int port; > + > + if (!csdev) > + return false; > + > + helper = coresight_get_helper(csdev, CORESIGHT_DEV_SUBTYPE_HELPER_CTCU); > + if (!helper) > + return false; > + > + port = coresight_get_in_port_dest(csdev, helper); > + if (port < 0) > + return false; > + > + drvdata = dev_get_drvdata(helper->dev.parent); > + /* Something wrong when initialize byte_cntr_read_ops */ > + if (!drvdata->byte_cntr_read_ops) > + return false; > + > + byte_cntr_data = &drvdata->byte_cntr_data[port]; > + /* Return the pointer of the ctcu_drvdata if byte-cntr has enabled */ > + if (byte_cntr_data && byte_cntr_data->thresh_val) { > + *data = (void *)drvdata->byte_cntr_read_ops; > + return true; > + } > + > + return false; > +} > + > static const struct coresight_ops_helper ctcu_helper_ops = { > .enable = ctcu_enable, > .disable = ctcu_disable, > + .qcom_byte_cntr_in_use = ctcu_qcom_byte_cntr_in_use, > }; > > static const struct coresight_ops ctcu_ops = { > diff --git a/include/linux/coresight.h b/include/linux/coresight.h > index 4ac65c68bbf4..b5f052854b08 100644 > --- a/include/linux/coresight.h > +++ b/include/linux/coresight.h > @@ -419,11 +419,14 @@ struct coresight_ops_source { > * > * @enable : Enable the device > * @disable : Disable the device > + * @qcom_byte_cntr_in_use: check whether the byte-cntr is enabled. > */ > struct coresight_ops_helper { > int (*enable)(struct coresight_device *csdev, enum cs_mode mode, > void *data); > int (*disable)(struct coresight_device *csdev, void *data); > + bool (*qcom_byte_cntr_in_use)(struct coresight_device *csdev, > + void **data); > }; > > > -- > 2.34.1 > -- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK

4 months, 1 week

Re: [PATCH v4 05/10] coresight: tmc: Introduce tmc_read_ops to wrap read operations

by Mike Leach

Hi On Fri, 25 Jul 2025 at 11:08, Jie Gan <jie.gan(a)oss.qualcomm.com> wrote: > > Introduce tmc_read_ops as a wrapper, wrap read operations, for reading > trace data from the TMC buffer. > > Signed-off-by: Jie Gan <jie.gan(a)oss.qualcomm.com> > --- > .../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 4d249af93097..f668047c5df4 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->read_ops) > + ret = drvdata->read_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->read_ops) > + ret = drvdata->read_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->read_ops) > + return drvdata->read_ops->get_trace_data(drvdata, pos, len, bufpp); > > return -EINVAL; > } > @@ -769,6 +750,18 @@ static void register_crash_dev_interface(struct tmc_drvdata *drvdata, > "Valid crash tracedata found\n"); > } > > +static const struct tmc_read_ops tmc_etb_read_ops = { > + .read_prepare = tmc_read_prepare_etb, > + .read_unprepare = tmc_read_unprepare_etb, > + .get_trace_data = tmc_etb_get_sysfs_trace, > +}; > + > +static const struct tmc_read_ops tmc_etr_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; > @@ -818,6 +811,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->read_ops = &tmc_etb_read_ops; > break; > case TMC_CONFIG_TYPE_ETR: > desc.groups = coresight_etr_groups; > @@ -831,6 +825,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->read_ops = &tmc_etr_read_ops; > break; > case TMC_CONFIG_TYPE_ETF: > desc.groups = coresight_etf_groups; > @@ -839,6 +834,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->read_ops = &tmc_etb_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 3cb8ba9f88f5..2ad8e288c94b 100644 > --- a/drivers/hwtracing/coresight/coresight-tmc.h > +++ b/drivers/hwtracing/coresight/coresight-tmc.h > @@ -223,6 +223,8 @@ struct etr_buf_node { > struct list_head node; > }; > > +struct tmc_read_ops; > + declare the entire structure here rather than later. > /** > * struct tmc_drvdata - specifics associated to an TMC component > * @pclk: APB clock if present, otherwise NULL > @@ -259,6 +261,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 for byte-cntr reading. > + * @tmc_read_ops: Read operations for TMC device. > */ > struct tmc_drvdata { > struct clk *pclk; > @@ -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 tmc_read_ops *read_ops; probably should be named sysfs_read_ops to be consistent with the perf/sysfs differentiation within the rest of the structure > +}; > + > +/** > + * struct tmc_read_ops - read operations for TMC and its helper devices > + * @read_prepare: prepare operation. > + * @read_unprepare: unprepare operation. > + * @get_trace_data: read operation. > + */ > +struct tmc_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 { > -- > 2.34.1 > with the above changes:- Reviewed-by: Mike Leach <mike.leach(a)linaro.org> -- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK

4 months, 1 week

Re: [PATCH v4 04/10] coresight: tmc: add create/delete functions for etr_buf_node

by Mike Leach

Hi, On Fri, 25 Jul 2025 at 11:08, Jie Gan <jie.gan(a)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(a)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

4 months, 1 week

Re: [PATCH v4 03/10] coresight: tmc: add etr_buf_list to store allocated etr_buf

by Mike Leach

Hi On Fri, 25 Jul 2025 at 11:08, Jie Gan <jie.gan(a)oss.qualcomm.com> wrote: > > Add a list to store allocated etr_buf. > > The byte-cntr functionality requires two etr_buf to receive trace data. > The active etr_buf collects the trace data from source device, while the > byte-cntr reading function accesses the deactivated etr_buf after is > has been filled and synced, transferring data to the userspace. > > Signed-off-by: Jie Gan <jie.gan(a)oss.qualcomm.com> > --- > .../hwtracing/coresight/coresight-tmc-core.c | 1 + > drivers/hwtracing/coresight/coresight-tmc.h | 19 +++++++++++++++++++ > 2 files changed, 20 insertions(+) > > diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c b/drivers/hwtracing/coresight/coresight-tmc-core.c > index be964656be93..4d249af93097 100644 > --- a/drivers/hwtracing/coresight/coresight-tmc-core.c > +++ b/drivers/hwtracing/coresight/coresight-tmc-core.c > @@ -830,6 +830,7 @@ static int __tmc_probe(struct device *dev, struct resource *res) > idr_init(&drvdata->idr); > mutex_init(&drvdata->idr_mutex); > dev_list = &etr_devs; > + INIT_LIST_HEAD(&drvdata->etr_buf_list); > break; > case TMC_CONFIG_TYPE_ETF: > desc.groups = coresight_etf_groups; > diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h > index 6541a27a018e..52ee5f8efe8c 100644 > --- a/drivers/hwtracing/coresight/coresight-tmc.h > +++ b/drivers/hwtracing/coresight/coresight-tmc.h > @@ -208,6 +208,21 @@ struct tmc_resrv_buf { > s64 len; > }; > > +/** > + * @sysfs_buf: Allocated sysfs_buf. > + * @is_free: Indicates whether the buffer is free to choose. > + * @reading: Indicates whether the buffer is reading. > + * @pos: Position of the buffer. > + * @node: Node in etr_buf_list. > + */ > +struct etr_buf_node { > + struct etr_buf *sysfs_buf; > + bool is_free; > + bool reading; > + loff_t pos; > + struct list_head node; > +}; > + > /** > * struct tmc_drvdata - specifics associated to an TMC component > * @pclk: APB clock if present, otherwise NULL > @@ -242,6 +257,8 @@ struct tmc_resrv_buf { > * (after crash) by default. > * @crash_mdata: Reserved memory for storing tmc crash metadata. > * Used by ETR/ETF. > + * @etr_buf_list: List that is used to manage allocated etr_buf. > + * @reading_node: Available buffer for byte-cntr reading. > */ > struct tmc_drvdata { > struct clk *pclk; > @@ -271,6 +288,8 @@ struct tmc_drvdata { > struct etr_buf *perf_buf; > struct tmc_resrv_buf resrv_buf; > struct tmc_resrv_buf crash_mdata; > + struct list_head etr_buf_list; > + struct etr_buf_node *reading_node; Potential simplification:- do you need both reading_node here and reading in the etr_buf_node? reading_node handles the logic for which buffer is being read, while is_free handles the empty/full logic - reading seems unneeded? > }; > > struct etr_buf_operations { > -- > 2.34.1 > regards Mike -- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK

4 months, 1 week

Re: [PATCH v4 02/10] coresight: core: add a new API to retrieve the helper device

by Mike Leach

On Fri, 25 Jul 2025 at 11:08, Jie Gan <jie.gan(a)oss.qualcomm.com> wrote: > > Retrieving the helper device of the specific coresight device based on > its helper_subtype because a single coresight device may has multiple types > of the helper devices. > > Signed-off-by: Jie Gan <jie.gan(a)oss.qualcomm.com> > --- > drivers/hwtracing/coresight/coresight-core.c | 35 ++++++++++++++++++++ > drivers/hwtracing/coresight/coresight-priv.h | 2 ++ > 2 files changed, 37 insertions(+) > > diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c > index 042c4fa39e55..018b1119c48a 100644 > --- a/drivers/hwtracing/coresight/coresight-core.c > +++ b/drivers/hwtracing/coresight/coresight-core.c > @@ -580,6 +580,41 @@ struct coresight_device *coresight_get_sink(struct coresight_path *path) > } > EXPORT_SYMBOL_GPL(coresight_get_sink); > > +/** > + * coresight_get_helper: find the helper device of the assigned csdev. > + * > + * @csdev: The csdev the helper device is conntected to. > + * @type: helper_subtype of the expected helper device. > + * > + * Retrieve the helper device for the specific csdev based on its > + * helper_subtype. > + * > + * Return: the helper's csdev upon success or NULL for fail. > + */ > +struct coresight_device *coresight_get_helper(struct coresight_device *csdev, > + int type) > +{ > + int i; > + struct coresight_device *helper; > + > + /* protect the connections */ > + mutex_lock(&coresight_mutex); > + for (i = 0; i < csdev->pdata->nr_outconns; ++i) { > + helper = csdev->pdata->out_conns[i]->dest_dev; > + if (!helper || !coresight_is_helper(helper)) > + continue; > + > + if (helper->subtype.helper_subtype == type) { > + mutex_unlock(&coresight_mutex); > + return helper; > + } > + } > + mutex_unlock(&coresight_mutex); > + > + return NULL; > +} > +EXPORT_SYMBOL_GPL(coresight_get_helper); > + > /** > * coresight_get_in_port_dest: get the in-port number of the dest device > * that is connected to the src device. > diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h > index e51b22b8ebde..f80122827934 100644 > --- a/drivers/hwtracing/coresight/coresight-priv.h > +++ b/drivers/hwtracing/coresight/coresight-priv.h > @@ -158,6 +158,8 @@ void coresight_path_assign_trace_id(struct coresight_path *path, > enum cs_mode mode); > int coresight_get_in_port_dest(struct coresight_device *src, > struct coresight_device *dest); > +struct coresight_device *coresight_get_helper(struct coresight_device *csdev, > + int type); > > #if IS_ENABLED(CONFIG_CORESIGHT_SOURCE_ETM3X) > int etm_readl_cp14(u32 off, unsigned int *val); > -- > 2.34.1 > Reviewed-by: Mike Leach <mike.leach(a)linaro.org> -- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK

4 months, 1 week

Re: [PATCH v4 01/10] coresight: core: Refactoring ctcu_get_active_port and make it generic

by Mike Leach

On Fri, 25 Jul 2025 at 11:08, Jie Gan <jie.gan(a)oss.qualcomm.com> wrote: > > Remove ctcu_get_active_port from CTCU module and add it to the core > framework. > > The port number is crucial for the CTCU device to identify which ETR > it serves. With the port number we can correctly get required parameters > of the CTCU device in TMC module. > > Signed-off-by: Jie Gan <jie.gan(a)oss.qualcomm.com> > --- > drivers/hwtracing/coresight/coresight-core.c | 24 +++++++++++++++++++ > .../hwtracing/coresight/coresight-ctcu-core.c | 19 +-------------- > drivers/hwtracing/coresight/coresight-priv.h | 2 ++ > 3 files changed, 27 insertions(+), 18 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c > index 1accd7cbd54b..042c4fa39e55 100644 > --- a/drivers/hwtracing/coresight/coresight-core.c > +++ b/drivers/hwtracing/coresight/coresight-core.c > @@ -580,6 +580,30 @@ struct coresight_device *coresight_get_sink(struct coresight_path *path) > } > EXPORT_SYMBOL_GPL(coresight_get_sink); > > +/** > + * coresight_get_in_port_dest: get the in-port number of the dest device > + * that is connected to the src device. > + * > + * @src: csdev of the source device. > + * @dest: csdev of the destination device. > + * > + * Return: port number upon success or -EINVAL for fail. > + */ > +int coresight_get_in_port_dest(struct coresight_device *src, > + struct coresight_device *dest) > +{ > + struct coresight_platform_data *pdata = dest->pdata; > + int i; > + > + for (i = 0; i < pdata->nr_inconns; ++i) { > + if (pdata->in_conns[i]->src_dev == src) > + return pdata->in_conns[i]->dest_port; > + } > + > + return -EINVAL; > +} > +EXPORT_SYMBOL_GPL(coresight_get_in_port_dest); > + > u32 coresight_get_sink_id(struct coresight_device *csdev) > { > if (!csdev->ea) > diff --git a/drivers/hwtracing/coresight/coresight-ctcu-core.c b/drivers/hwtracing/coresight/coresight-ctcu-core.c > index c6bafc96db96..3bdedf041390 100644 > --- a/drivers/hwtracing/coresight/coresight-ctcu-core.c > +++ b/drivers/hwtracing/coresight/coresight-ctcu-core.c > @@ -118,23 +118,6 @@ static int __ctcu_set_etr_traceid(struct coresight_device *csdev, u8 traceid, in > return 0; > } > > -/* > - * Searching the sink device from helper's view in case there are multiple helper devices > - * connected to the sink device. > - */ > -static int ctcu_get_active_port(struct coresight_device *sink, struct coresight_device *helper) > -{ > - struct coresight_platform_data *pdata = helper->pdata; > - int i; > - > - for (i = 0; i < pdata->nr_inconns; ++i) { > - if (pdata->in_conns[i]->src_dev == sink) > - return pdata->in_conns[i]->dest_port; > - } > - > - return -EINVAL; > -} > - > static int ctcu_set_etr_traceid(struct coresight_device *csdev, struct coresight_path *path, > bool enable) > { > @@ -147,7 +130,7 @@ static int ctcu_set_etr_traceid(struct coresight_device *csdev, struct coresight > return -EINVAL; > } > > - port_num = ctcu_get_active_port(sink, csdev); > + port_num = coresight_get_in_port_dest(sink, csdev); > if (port_num < 0) > return -EINVAL; > > diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h > index 33e22b1ba043..e51b22b8ebde 100644 > --- a/drivers/hwtracing/coresight/coresight-priv.h > +++ b/drivers/hwtracing/coresight/coresight-priv.h > @@ -156,6 +156,8 @@ void coresight_remove_links(struct coresight_device *orig, > u32 coresight_get_sink_id(struct coresight_device *csdev); > void coresight_path_assign_trace_id(struct coresight_path *path, > enum cs_mode mode); > +int coresight_get_in_port_dest(struct coresight_device *src, > + struct coresight_device *dest); > > #if IS_ENABLED(CONFIG_CORESIGHT_SOURCE_ETM3X) > int etm_readl_cp14(u32 off, unsigned int *val); > -- > 2.34.1 > Reviewed by: Mike Leach <mike.leach(a)linaro.org> -- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK

4 months, 1 week

Re: [PATCH v2 0/2] Enable CTCU device for QCS8300

by Mike Leach

Hi Jie, On Tue, 5 Aug 2025 at 05:11, Jie Gan <jie.gan(a)oss.qualcomm.com> wrote: > > > > On 7/28/2025 9:08 AM, Jie Gan wrote: > > > > > > On 7/15/2025 8:41 AM, Jie Gan wrote: > >> > >> > >> On 6/24/2025 5:59 PM, Jie Gan wrote: > >>> Enable CTCU device for QCS8300 platform. Add a fallback mechnasim in > >>> binding to utilize > >>> the compitable of the SA8775p platform becuase the CTCU for QCS8300 > >>> shares same > >>> configurations as SA8775p platform. > >> > >> Gentle ping. > > > > Gentle ping. > > Gentle ping. > Hi Coresight maintainers, > > Can you please help to review the patch? > > Thanks, > Jie > > > > > Thanks, > > Jie > > > >> > >> Hi Suzuki, Mike, James, Rob > >> > >> Can you plz help to review the patch from Coresight view? > >> > >> Thanks, > >> Jie > >> > >>> > >>> Changes in V2: > >>> 1. Add Krzysztof's R-B tag for dt-binding patch. > >>> 2. Add Konrad's Acked-by tag for dt patch. > >>> 3. Rebased on tag next-20250623. > >>> 4. Missed email addresses for coresight's maintainers in V1, loop them. > >>> Link to V1 - https://lore.kernel.org/all/20250327024943.3502313-1- > >>> jie.gan(a)oss.qualcomm.com/ > >>> > >>> Jie Gan (2): > >>> dt-bindings: arm: add CTCU device for QCS8300 > >>> arm64: dts: qcom: qcs8300: Add CTCU and ETR nodes > >>> > >>> .../bindings/arm/qcom,coresight-ctcu.yaml | 9 +- > >>> arch/arm64/boot/dts/qcom/qcs8300.dtsi | 153 ++++++++++++++++++ > >>> 2 files changed, 160 insertions(+), 2 deletions(-) > >>> > >> > >> > > > You need to send a new patch addressing the comments made by Krzysztof.. Regards Mike -- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK

4 months, 1 week

Re: [PATCH v5 04/10] coresight: Appropriately disable programming clocks

by Leo Yan

On Wed, Jul 30, 2025 at 12:09:42PM +0100, Mark Brown wrote: > On Wed, Jul 30, 2025 at 12:01:25PM +0100, Suzuki K Poulose wrote: > > > I would recommend using that and don't force the use of apb_clk/apb > > for AMBA devices. If the firmware doesn't specify a clock, but does > > specify the CoreSight components, it knows it better. > > And perhaps more to the point if a currently working system suddenly > starts requiring additional clocks in it's binding that's an ABI break. Yes, the change should not break any platforms if the DT binding is passed correctly. I will update with devm_clk_get_optional_enabled(). Just for the record, I was a bit concerned that the driver might not report a missing clock after switching to the optional clock API. After discussed with Rob and Suzuki, I understand this should not be a problem. Any missing clock issue can be caught by the DT schema static checker, or a system hang during the development phase would remind developers to bind clocks properly. Thanks, Leo

4 months, 2 weeks

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

CoreSight