CoreSight

coresight@lists.linaro.org

4 participants
2500 discussions

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

2 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

2 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

2 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

2 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

2 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

2 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

2 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

2 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

2 months, 1 week

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

by Suzuki K Poulose

On 29/07/2025 12:31, Mark Brown wrote: > On Mon, Jul 28, 2025 at 05:45:04PM +0100, Mark Brown wrote: >> On Thu, Jul 24, 2025 at 04:22:34PM +0100, Leo Yan wrote: >> >> Previously we would return NULL for any error (which isn't super great >> for deferred probe but never mind). >> >>> + pclk = devm_clk_get_enabled(dev, "apb_pclk"); >>> + if (IS_ERR(pclk)) >>> + pclk = devm_clk_get_enabled(dev, "apb"); >> >> ... >> >>> return pclk; >>> } >> >> Now we pass errors back to the caller, making missing clocks fatal. > > Thinking about this some more I think for compatiblity these clocks need > to be treated as optional - that's what the original code was > effectively doing, and I can imagine this isn't the only SoC which has > (hopefully) always on clocks and didn't wire things up in DT. You're right. The static components (funnels, replicators) don't have APB programming interface and hence no clocks. That said, may be the "is amba device" check could be used to enforce the presence of a clock. I will let Leo sort this out Suzuki

2 months, 1 week

← Newer
1
...
6
7
8
9
10
11
12
...
250
Older →

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

CoreSight