CoreSight

coresight@lists.linaro.org

7 participants
2775 discussions

Re: [PATCH v6 3/9] coresight: tmc: add etr_buf_list to store allocated etr_buf

by Suzuki K Poulose

On 08/09/2025 03:01, Jie Gan 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> This patch alone doesn't make any sense. Please fold it with a proper user (i.e. later patches). I can't make any sense of how this will be used TBH without the following patches and on its own, it doesn't do much anyways. Suzuki > --- > drivers/hwtracing/coresight/coresight-tmc-core.c | 1 + > drivers/hwtracing/coresight/coresight-tmc.h | 17 +++++++++++++++++ > 2 files changed, 18 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..292e25d82b62 100644 > --- a/drivers/hwtracing/coresight/coresight-tmc.h > +++ b/drivers/hwtracing/coresight/coresight-tmc.h > @@ -208,6 +208,19 @@ struct tmc_resrv_buf { > s64 len; > }; > > +/** > + * @sysfs_buf: Allocated sysfs_buf. > + * @is_free: Indicates whether the buffer is free to choose. > + * @pos: Position of the buffer. > + * @node: Node in etr_buf_list. > + */ > +struct etr_buf_node { > + struct etr_buf *sysfs_buf; > + bool is_free; > + 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 +255,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 +286,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; > }; > > struct etr_buf_operations { >

6 months, 3 weeks

Re: [PATCH v6 1/9] coresight: core: Refactoring ctcu_get_active_port and make it generic

by Suzuki K Poulose

On 08/09/2025 03:01, Jie Gan 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. > > Reviewed-by: Mike Leach <mike.leach(a)linaro.org> > Signed-off-by: Jie Gan <jie.gan(a)oss.qualcomm.com> > --- > drivers/hwtracing/coresight/coresight-core.c | 24 +++++++++++++++++++++++ > drivers/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, Please could we make this more explicit : /* * Find the input port number at @csdev where a @remote * device is connected to. */ coresight_get_in_port(struct coresight_device *csdev, struct coresight_device *remote) { } Suzuki

6 months, 3 weeks

Re: [PATCH v6 2/9] coresight: core: add a new API to retrieve the helper device

by Suzuki K Poulose

On 08/09/2025 03:01, Jie Gan 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. > > Reviewed-by: Mike Leach <mike.leach(a)linaro.org> > 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) nit: enum coresight_dev_subtype_helper subtype Otherwise looks good to me Suzuki > +{ > + 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); >

6 months, 3 weeks

Re: [PATCH v8 0/7] Provides support for Trigger Generation Unit

by Suzuki K Poulose

Cc: Greg On 03/12/2025 09:00, Songwei Chai wrote: > We propose creating a new qcom directory under drivers/hwtracing > to host this TGU driver, as well as additional Qualcomm-specific > hwtracing drivers that we plan to submit in the coming months. > This structure will help organize vendor-specific implementations > and facilitate future development and maintenance. > > Feedback from the community on this proposal is highly appreciated. > > - Why we are proposing this: > > TGU has the ability to monitor signal conditions and trigger debug-related > actions, serving as a programmable hardware component that enhances system > trace and debug capabilities. Placing it under drivers/hwtracing aligns with > its function as a trace generation utility. > > We previously attempted to push this driver to drivers/hwtracing/coresight, > but did not receive support from the maintainers of the CoreSight subsystem. > The reason provided was: “This component is primarily a part of the > Qualcomm proprietary QPMDA subsystem, and is capable of operating > independently from the CoreSight hardware trace generation system.” > > Chat history : https://lore.kernel.org/all/CAJ9a7ViKxHThyZfFFDV_FkNRimk4uo1NrMtQ-kcaj1qO4Z… > > Given this, we have been considering whether it would be appropriate > to create a dedicated drivers/hwtracing/qcom directory for > Qualcomm-related hwtracing drivers. This would follow the precedent set > by Intel, which maintains its own directory at drivers/hwtracing/intel_th. > We believe this structure would significantly facilitate > future submissions of related Qualcomm drivers. > > - Maintenance of drivers/hwtracing/qcom: > Fine by, me. > Bjorn, who maintains linux-arm-msm, will be the maintainer of this > directory — we’ve discussed this with him and he’s aware that his task > list may grow accordingly. Additionally, Qualcomm engineers familiar with > the debug hardware — such as [Tingwei Zhang, Jinlong Mao, Songwei Chai], > will be available to review incoming patches and support ongoing > development. > > - Detail for TGU: > > This component can be utilized to sense a plurality of signals and > create a trigger into the CTI or generate interrupts to processors > once the input signal meets the conditions. We can treat the TGU’s > workflow as a flowsheet, it has some “steps” regions for customization. > In each step region, we can set the signals that we want with priority > in priority_group, set the conditions in each step via condition_decode, > and set the resultant action by condition_select. Meanwhile, > some TGUs (not all) also provide timer/counter functionality. > Based on the characteristics described above, we consider the TGU as a > helper in the CoreSight subsystem. Its master device is the TPDM, which > can transmit signals from other subsystems, and we reuse the existing > ports mechanism to link the TPDM to the connected TGU. Please remove the coresight_device and other dependencies. You may use generic bits, CS_LOCK/UNLOCK etc. But including coresight-priv.h is not something I would prefer. It brings in unnecessary dependencies between two subsystems and I don't see any reason for using -priv.h. It is named as it is, for a reason, coresight private definitions. > > Here is a detailed example to explain how to use the TGU: > > In this example, the TGU is configured to use 2 conditions, 2 steps, and > the timer. The goal is to look for one of two patterns which are generated > from TPDM, giving priority to one, and then generate a trigger once the > timer reaches a certain value. In other words, two conditions are used > for the first step to look for the two patterns, where the one with the > highest priority is used in the first condition. Then, in the second step, > the timer is enabled and set to be compared to the given value at each > clock cycle. These steps are better shown below. > > |-----------------| > | | > | TPDM | > | | > |-----------------| > | > | > --- --- --- --- --- --- --- --- --- --- --- --- --- --- --- --- ------ > | | | > | | |--------------------| | > | |---- ---> | | Go to next steps | | > | | | |--- ---> | Enable timer | | > | | v | | | | > | | |-----------------| | |--------------------| | > | | | | Yes | | | > | | | inputs==0xB | ----->| | <-------- | > | | | | | | No | | > | No | |-----------------| | v | | > | | | | |-----------------| | | > | | | | | | | | > | | | | | timer>=3 |-- | > | | v | | | | > | | |-----------------| | |-----------------| | > | | | | Yes | | | > | |--- | inputs==0xA | ----->| | Yes | > | | | | | > | |-----------------| v | > | |-----------------| | > | | | | > | | Trigger | | > | | | | > | |-----------------| | > | TGU | | > |--- --- --- --- --- --- --- --- --- --- --- --- --- --- |--- --- -- | > | > v > |-----------------| > |The controllers | > |which will use | > |triggers further | > |-----------------| > > steps: > 1. Reset TGU /*it will disable tgu and reset dataset*/ > - echo 1 > /sys/bus/coresight/devices/<tgu-name>/reset_tgu > > 2. Set the pattern match for priority0 to 0xA = 0b1010 and for > priority 1 to 0xB = 0b1011. > - echo 0x11113232 > /sys/bus/coresight/devices/<tgu-name>/step0_priority0/reg0 > - echo 0x11113233 > /sys/bus/coresight/devices/<tgu-name>/step0_priority1/reg0 Why do they need to be coresight devices and appear on the coresight bus if they are not coresight devices ? As I understand they are simply devices with some sysfs knobs. Otherwise, happy with the proposal Suzuki > > Note: > Bit distribution diagram for each priority register > |-------------------------------------------------------------------| > | Bits | Field Nam | Description | > |-------------------------------------------------------------------| > | | | 00 = bypass for OR output | > | 29:28 | SEL_BIT7_TYPE2 | 01 = bypass for AND output | > | | | 10 = sense input '0' is true| > | | | 11 = sense input '1' is true| > |-------------------------------------------------------------------| > | | | 00 = bypass for OR output | > | 25:24 | SEL_BIT6_TYPE2 | 01 = bypass for AND output | > | | | 10 = sense input '0' is true| > | | | 11 = sense input '1' is true| > |-------------------------------------------------------------------| > | | | 00 = bypass for OR output | > | 21:20 | SEL_BIT5_TYPE2 | 01 = bypass for AND output | > | | | 10 = sense input '0' is true| > | | | 11 = sense input '1' is true| > |-------------------------------------------------------------------| > | | | 00 = bypass for OR output | > | 17:16 | SEL_BIT4_TYPE2 | 01 = bypass for AND output | > | | | 10 = sense input '0' is true| > | | | 11 = sense input '1' is true| > |-------------------------------------------------------------------| > | | | 00 = bypass for OR output | > | 13:12 | SEL_BIT3_TYPE2 | 01 = bypass for AND output | > | | | 10 = sense input '0' is true| > | | | 11 = sense input '1' is true| > |-------------------------------------------------------------------| > | | | 00 = bypass for OR output | > | 9:8 | SEL_BIT2_TYPE2 | 01 = bypass for AND output | > | | | 10 = sense input '0' is true| > | | | 11 = sense input '1' is true| > |-------------------------------------------------------------------| > | | | 00 = bypass for OR output | > | 5:4 | SEL_BIT1_TYPE2 | 01 = bypass for AND output | > | | | 10 = sense input '0' is true| > | | | 11 = sense input '1' is true| > |-------------------------------------------------------------------| > | | | 00 = bypass for OR output | > | 1:0 | SEL_BIT0_TYPE2 | 01 = bypass for AND output | > | | | 10 = sense input '0' is true| > | | | 11 = sense input '1' is true| > |-------------------------------------------------------------------| > These bits are used to identify the signals we want to sense, with > a maximum signal number of 140. For example, to sense the signal > 0xA (binary 1010), we set the value of bits 0 to 13 to 3232, which > represents 1010. The remaining bits are set to 1, as we want to use > AND gate to summarize all the signals we want to sense here. For > rising or falling edge detection of any input to the priority, set > the remaining bits to 0 to use an OR gate. > > 3. look for the pattern for priority_i i=0,1. > - echo 0x3 > /sys/bus/coresight/devices/<tgu-name>/step0_condition_decode/reg0 > - echo 0x30 > /sys/bus/coresight/devices/<tgu-name>/step0_condition_decode/reg1 > > |-------------------------------------------------------------------------------| > | Bits | Field Nam | Description | > |-------------------------------------------------------------------------------| > | | |For each decoded condition, this | > | 24 | NOT |inverts the output. If the condition | > | | |decodes to true, and the NOT field | > | | |is '1', then the output is NOT true. | > |-------------------------------------------------------------------------------| > | | |When '1' the output from the associated| > | 21 | BC0_COMP_ACTIVE |comparator will be actively included in| > | | |the decoding of this particular | > | | |condition. | > |-------------------------------------------------------------------------------| > | | |When '1' the output from the associated| > | | |comparator will need to be 1 to affect | > | 20 | BC0_COMP_HIGH |the decoding of this condition. | > | | |Conversely, a '0' here requires a '0' | > | | |from the comparator | > |-------------------------------------------------------------------------------| > | | |When '1' the output from the associated| > | 17 | |comparator will be actively included in| > | | TC0_COMP_ACTIVE |the decoding of this particular | > | | |condition. | > |-------------------------------------------------------------------------------| > | | |When '1' the output from the associated| > | | |comparator will need to be 1 to affect | > | 16 | TC0_COMP_HIGH |the decoding of this particular | > | | |condition.Conversely, a 0 here | > | | |requires a '0' from the comparator | > |-------------------------------------------------------------------------------| > | | |When '1' the output from Priority_n | > | | |OR logic will be actively | > | 4n+3 | Priority_n_OR_ACTIVE|included in the decoding of | > | | (n=0,1,2,3) |this particular condition. | > | | | | > |-------------------------------------------------------------------------------| > | | |When '1' the output from Priority_n | > | | |will need to be '1' to affect the | > | 4n+2 | Priority_n_OR_HIGH |decoding of this particular | > | | (n=0,1,2,3) |condition. Conversely, a '0' here | > | | |requires a '0' from Priority_n OR logic| > |-------------------------------------------------------------------------------| > | | |When '1' the output from Priority_n | > | | |AND logic will be actively | > | 4n+1 |Priority_n_AND_ACTIVE|included in the decoding of this | > | | (n=0,1,2,3) |particular condition. | > | | | | > |-------------------------------------------------------------------------------| > | | |When '1' the output from Priority_n | > | | |AND logic will need to be '1' to | > | 4n | Priority_n_AND_HIGH |affect the decoding of this | > | | (n=0,1,2,3) |particular condition. Conversely, | > | | |a '0' here requires a '0' from | > | | |Priority_n AND logic. | > |-------------------------------------------------------------------------------| > Since we use `priority_0` and `priority_1` with an AND output in step 2, we set `0x3` > and `0x30` here to activate them. > > 4. Set NEXT_STEP = 1 and TC0_ENABLE = 1 so that when the conditions > are met then the next step will be step 1 and the timer will be enabled. > - echo 0x20008 > /sys/bus/coresight/devices/<tgu-name>/step0_condition_select/reg0 > - echo 0x20008 > /sys/bus/coresight/devices/<tgu-name>/step0_condition_select/reg1 > > |-----------------------------------------------------------------------------| > | Bits | Field Nam | Description | > |-----------------------------------------------------------------------------| > | | |This field defines the next step the | > | 18:17 | NEXT_STEP |TGU will 'goto' for the associated | > | | |Condition and Step. | > |-----------------------------------------------------------------------------| > | | |For each possible output trigger | > | 13 | TRIGGER |available, set a '1' if you want | > | | |the trigger to go active for the | > | | |associated condition and Step. | > |-----------------------------------------------------------------------------| > | | |This will cause BC0 to increment if the| > | 9 | BC0_INC |associated Condition is decoded for | > | | |this step. | > |-----------------------------------------------------------------------------| > | | |This will cause BC0 to decrement if the| > | 8 | BC0_DEC |associated Condition is decoded for | > | | |this step. | > |-----------------------------------------------------------------------------| > | | |This will clear BC0 count value to 0 if| > | 7 | BC0_CLEAR |the associated Condition is decoded | > | | |for this step. | > |-----------------------------------------------------------------------------| > | | |This will cause TC0 to increment until | > | 3 | TC0_ENABLE |paused or cleared if the associated | > | | |Condition is decoded for this step. | > |-----------------------------------------------------------------------------| > | | |This will cause TC0 to pause until | > | 2 | TC0_PAUSE |enabled if the associated Condition | > | | |is decoded for this step. | > |-----------------------------------------------------------------------------| > | | |This will clear TC0 count value to 0 | > | 1 | TC0_CLEAR |if the associated Condition is | > | | |decoded for this step. | > |-----------------------------------------------------------------------------| > | | |This will set the done signal to the | > | 0 | DONE |TGU FSM if the associated Condition | > | | |is decoded for this step. | > |-----------------------------------------------------------------------------| > Based on the distribution diagram, we set `0x20008` for `priority0` and `priority1` to > achieve "jump to step 1 and enable TC0" once the signal is sensed. > > 5. activate the timer comparison for this step. > - echo 0x30000 > /sys/bus/coresight/devices/<tgu-name>/step1_condition_decode/reg0 > > |-------------------------------------------------------------------------------| > | | |When '1' the output from the associated| > | 17 | |comparator will be actively included in| > | | TC0_COMP_ACTIVE |the decoding of this particular | > | | |condition. | > |-------------------------------------------------------------------------------| > | | |When '1' the output from the associated| > | | |comparator will need to be 1 to affect | > | 16 | TC0_COMP_HIGH |the decoding of this particular | > | | |condition.Conversely, a 0 here | > | | |requires a '0' from the comparator | > |-------------------------------------------------------------------------------| > Accroding to the decode distribution diagram , we give 0x30000 here to set 16th&17th bit > to enable timer comparison. > > 6. Set the NEXT_STEP = 0 and TC0_PAUSE = 1 and TC0_CLEAR = 1 once the timer > has reached the given value. > - echo 0x6 > /sys/bus/coresight/devices/<tgu-name>/step1_condition_select/reg0 > > 7. Enable Trigger 0 for TGU when the condition 0 is met in step1, > i.e. when the timer reaches 3. > - echo 0x2000 > /sys/bus/coresight/devices/<tgu-name>/step1_condition_select/default > > Note: > 1. 'default' register allows for establishing the resultant action for > the default condition > > 2. Trigger:For each possible output trigger available from > the Design document, there are three triggers: interrupts, CTI, > and Cross-TGU mapping.All three triggers can occur, but > the choice of which trigger to use depends on the user's > needs. > > 8. Compare the timer to 3 in step 1. > - echo 0x3 > /sys/bus/coresight/devices/<tgu-name>/step1_timer/reg0 > > 9. enale tgu > - echo 1 > /sys/bus/coresight/devices/<tgu-name>/enable_tgu > --- > Link to V7: https://lore.kernel.org/all/20251104064043.88972-1-songwei.chai@oss.qualcom… > > Changes in V8: > - Add "select" section in bindings. > - Update publish date in "sysfs-bus-coresight-devices-tgu". > --- > Link to V6: https://lore.kernel.org/all/20250709104114.22240-1-songchai@qti.qualcomm.co… > > Changes in V7: > - Move the TGU code location from 'drivers/hwtracing/coresight/' to 'drivers/hwtracing/qcom/'. > - Rename the spinlock used in the code from 'spinlock' to 'lock'. > - Perform the 'calculate_array_location' separately, instead of doing it within the function. > - Update the sender email address. > --- > Link to V5: https://lore.kernel.org/all/20250529081949.26493-1-quic_songchai@quicinc.co… > > Changes in V6: > - Replace spinlock with guard(spinlock) in tgu_enable. > - Remove redundant blank line. > - Update publish date and contact member's name in "sysfs-bus-coresight-devices-tgu". > --- > Link to V4: https://patchwork.kernel.org/project/linux-arm-msm/cover/20250423101054.954… > > Changes in V5: > - Update publish date and kernel_version in "sysfs-bus-coresight-devices-tgu" > --- > Link to V3: https://lore.kernel.org/all/20250227092640.2666894-1-quic_songchai@quicinc.… > > Changes in V4: > - Add changlog in coverletter. > - Correct 'year' in Copyright in patch1. > - Correct port mechansim description in patch1. > - Remove 'tgu-steps','tgu-regs','tgu-conditions','tgu-timer-counters' from dt-binding > and set them through reading DEVID register as per Mike's suggestion. > - Modify tgu_disable func to make it have single return point in patch2 as per > Mike's suggestion. > - Use sysfs_emit in enable_tgu_show func in ptach2. > - Remove redundant judgement in enable_tgu_store in patch2. > - Correct typo in description in patch3. > - Set default ret as SYSFS_GROUP_INVISIBLE, and returnret at end in pacth3 as > per Mike's suggestion. > - Remove tgu_dataset_ro definition in patch3 > - Use #define constants with explanations of what they are rather than > arbitrary magic numbers in patch3 and patch4. > - Check -EINVAL before using 'calculate_array_location()' in array in patch4. > - Add 'default' in 'tgu_dataset_show''s switch part in patch4. > - Document the value needed to initiate the reset in pacth7. > - Check "value" in 'reset_tgu_store' and bail out with an error code if 0 in patch7. > - Remove dev_dbg in 'reset_tgu_store' in patch7. > --- > Link to V2: https://lore.kernel.org/all/20241010073917.16023-1-quic_songchai@quicinc.co… > > Changes in V3: > - Correct typo and format in dt-binding in patch1 > - Rebase to the latest kernel version > --- > Link to V1: https://lore.kernel.org/all/20240830092311.14400-1-quic_songchai@quicinc.co… > > Changes in V2: > - Use real name instead of login name, > - Correct typo and format in dt-binding and code. > - Bring order in tgu_prob(declarations with and without assignments) as per > Krzysztof's suggestion. > - Add module device table in patch2. > - Set const for tgu_common_grp and tgu_ids in patch2. > - Initialize 'data' in tgu_ids to fix the warning in pacth2. > --- > > Songwei Chai (7): > dt-bindings: arm: Add support for Qualcomm TGU trace > qcom-tgu: Add TGU driver > qcom-tgu: Add signal priority support > qcom-tgu: Add TGU decode support > qcom-tgu: Add support to configure next action > qcom-tgu: Add timer/counter functionality for TGU > qcom-tgu: Add reset node to initialize > > .../testing/sysfs-bus-coresight-devices-tgu | 51 ++ > .../devicetree/bindings/arm/qcom,tgu.yaml | 92 +++ > drivers/Makefile | 1 + > drivers/hwtracing/Kconfig | 2 + > drivers/hwtracing/qcom/Kconfig | 18 + > drivers/hwtracing/qcom/Makefile | 3 + > drivers/hwtracing/qcom/tgu.c | 737 ++++++++++++++++++ > drivers/hwtracing/qcom/tgu.h | 252 ++++++ > 8 files changed, 1156 insertions(+) > create mode 100644 Documentation/ABI/testing/sysfs-bus-coresight-devices-tgu > create mode 100644 Documentation/devicetree/bindings/arm/qcom,tgu.yaml > create mode 100644 drivers/hwtracing/qcom/Kconfig > create mode 100644 drivers/hwtracing/qcom/Makefile > create mode 100644 drivers/hwtracing/qcom/tgu.c > create mode 100644 drivers/hwtracing/qcom/tgu.h >

6 months, 3 weeks

Re: [PATCH v6 6/9] dt-bindings: arm: add an interrupt property for Coresight CTCU

by Mike Leach

On Mon, 8 Sept 2025 at 03:02, Jie Gan <jie.gan(a)oss.qualcomm.com> wrote: > > Add an interrupt property to CTCU device. The interrupt will be triggered > when the data size in the ETR buffer exceeds the threshold of the > BYTECNTRVAL register. Programming a threshold in the BYTECNTRVAL register > of CTCU device will enable the interrupt. > > Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski(a)linaro.org> > Signed-off-by: Jie Gan <jie.gan(a)oss.qualcomm.com> > --- > .../devicetree/bindings/arm/qcom,coresight-ctcu.yaml | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) > > diff --git a/Documentation/devicetree/bindings/arm/qcom,coresight-ctcu.yaml b/Documentation/devicetree/bindings/arm/qcom,coresight-ctcu.yaml > index 843b52eaf872..ea05ad8f3dd3 100644 > --- a/Documentation/devicetree/bindings/arm/qcom,coresight-ctcu.yaml > +++ b/Documentation/devicetree/bindings/arm/qcom,coresight-ctcu.yaml > @@ -39,6 +39,16 @@ properties: > items: > - const: apb > > + interrupts: > + items: > + - description: Byte cntr interrupt for etr0 > + - description: Byte cntr interrupt for etr1 > + > + interrupt-names: > + items: > + - const: etr0 > + - const: etr1 > + > in-ports: > $ref: /schemas/graph.yaml#/properties/ports > > @@ -56,6 +66,8 @@ additionalProperties: false > > examples: > - | > + #include <dt-bindings/interrupt-controller/arm-gic.h> > + > ctcu@1001000 { > compatible = "qcom,sa8775p-ctcu"; > reg = <0x1001000 0x1000>; > @@ -63,6 +75,11 @@ examples: > clocks = <&aoss_qmp>; > clock-names = "apb"; > > + interrupts = <GIC_SPI 270 IRQ_TYPE_EDGE_RISING>, > + <GIC_SPI 262 IRQ_TYPE_EDGE_RISING>; > + interrupt-names = "etr0", > + "etr1"; > + > in-ports { > #address-cells = <1>; > #size-cells = <0>; > > -- > 2.34.1 > Not sure if you need me to review this purely DT hardware description update but... Reviewed-by: Mike Leach <mike.leach(a)linaro.org> -- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK

6 months, 3 weeks

Re: [PATCH v6 4/9] coresight: tmc: add create/clean functions for etr_buf_list

by Mike Leach

On Mon, 8 Sept 2025 at 03:02, 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> > --- > drivers/hwtracing/coresight/coresight-tmc-etr.c | 94 +++++++++++++++++++++++++ > drivers/hwtracing/coresight/coresight-tmc.h | 2 + > 2 files changed, 96 insertions(+) > > diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c > index b07fcdb3fe1a..ed15991b3217 100644 > --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c > +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c > @@ -1909,6 +1909,100 @@ 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) { > + if (coresight_get_mode(drvdata->csdev) == CS_MODE_DISABLED) { > + drvdata->sysfs_buf = NULL; > + tmc_free_etr_buf(nd->sysfs_buf); > + nd->sysfs_buf = NULL; > + } > + 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_list - create a list to manage the etr_buf_node. > + * @drvdata: driver data of the TMC device. > + * @num_nodes: number of nodes want to create with the list. > + * > + * Return 0 upon success and return the error number if fail. > + */ > +int tmc_create_etr_buf_list(struct tmc_drvdata *drvdata, int num_nodes) > +{ > + struct etr_buf_node *new_node; > + struct etr_buf *sysfs_buf; > + int i = 0, ret = 0; > + > + /* We dont need a list if there is only one node */ > + if (num_nodes < 2) > + return -EINVAL; > + > + /* We expect that sysfs_buf in drvdata has already been allocated. */ > + if (drvdata->sysfs_buf) { > + /* Directly insert the allocated sysfs_buf into the list first */ > + new_node = kzalloc(sizeof(struct etr_buf_node), GFP_KERNEL); > + if (IS_ERR(new_node)) > + return PTR_ERR(new_node); > + > + new_node->sysfs_buf = drvdata->sysfs_buf; > + new_node->is_free = false; > + list_add(&new_node->node, &drvdata->etr_buf_list); > + i++; > + } > + > + while (i < num_nodes) { > + new_node = kzalloc(sizeof(struct etr_buf_node), GFP_KERNEL); > + if (IS_ERR(new_node)) { > + ret = PTR_ERR(new_node); > + break; > + } > + > + sysfs_buf = tmc_alloc_etr_buf(drvdata, drvdata->size, 0, cpu_to_node(0), NULL); > + if (IS_ERR(sysfs_buf)) { > + kfree(new_node); > + ret = PTR_ERR(new_node); > + break; > + } > + > + /* We dont have a available sysfs_buf in drvdata, setup one */ > + if (!drvdata->sysfs_buf) { > + drvdata->sysfs_buf = sysfs_buf; > + new_node->is_free = false; > + } else > + new_node->is_free = true; > + > + new_node->sysfs_buf = sysfs_buf; > + list_add(&new_node->node, &drvdata->etr_buf_list); > + i++; > + } > + > + /* Clean the list if there is an error */ > + if (ret) > + tmc_clean_etr_buf_list(drvdata); > + > + return ret; > +} > +EXPORT_SYMBOL_GPL(tmc_create_etr_buf_list); > + > 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 292e25d82b62..ca0cba860d5f 100644 > --- a/drivers/hwtracing/coresight/coresight-tmc.h > +++ b/drivers/hwtracing/coresight/coresight-tmc.h > @@ -459,5 +459,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_list(struct tmc_drvdata *drvdata, int num_nodes); > > #endif > > -- > 2.34.1 > Reviewed-by: Mike Leach <mike.leach(a)linaro.org> -- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK

6 months, 3 weeks

Re: [PATCH v6 3/9] coresight: tmc: add etr_buf_list to store allocated etr_buf

by Mike Leach

On Mon, 8 Sept 2025 at 03:02, 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> > --- > drivers/hwtracing/coresight/coresight-tmc-core.c | 1 + > drivers/hwtracing/coresight/coresight-tmc.h | 17 +++++++++++++++++ > 2 files changed, 18 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..292e25d82b62 100644 > --- a/drivers/hwtracing/coresight/coresight-tmc.h > +++ b/drivers/hwtracing/coresight/coresight-tmc.h > @@ -208,6 +208,19 @@ struct tmc_resrv_buf { > s64 len; > }; > > +/** > + * @sysfs_buf: Allocated sysfs_buf. > + * @is_free: Indicates whether the buffer is free to choose. > + * @pos: Position of the buffer. > + * @node: Node in etr_buf_list. > + */ > +struct etr_buf_node { > + struct etr_buf *sysfs_buf; > + bool is_free; > + 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 +255,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 +286,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; > }; > > struct etr_buf_operations { > > -- > 2.34.1 > Reviewed-by: Mike Leach <mike.leach(a)linaro.org> -- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK

6 months, 3 weeks

Re: [PATCH v3] coresight: etm3x: Fix cntr_val_show() to match cntr_val_store() behavior

by James Clark

On 02/12/2025 8:26 am, Kuan-Wei Chiu wrote: > The cntr_val_show() function was intended to print the values of all > counters using a loop. However, due to a buffer overwrite issue with > sprintf(), it effectively only displayed the value of the last counter. > > The companion function, cntr_val_store(), allows users to modify a > specific counter selected by 'cntr_idx'. To maintain consistency > between read and write operations and to align with the ETM4x driver > behavior, modify cntr_val_show() to report only the value of the > currently selected counter. > > This change removes the loop and the "counter %d:" prefix, printing > only the hexadecimal value. It also adopts sysfs_emit() for standard > sysfs output formatting. > > Fixes: a939fc5a71ad ("coresight-etm: add CoreSight ETM/PTM driver") > Cc: stable(a)vger.kernel.org > Signed-off-by: Kuan-Wei Chiu <visitorckw(a)gmail.com> > --- > Build test only. > > Changes in v3: > - Switch format specifier to %#x to include the 0x prefix. > - Add Cc stable > > v2: https://lore.kernel.org/lkml/20251201095228.1905489-1-visitorckw@gmail.com/ > > .../hwtracing/coresight/coresight-etm3x-sysfs.c | 15 ++++----------- > 1 file changed, 4 insertions(+), 11 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c > index 762109307b86..b3c67e96a82a 100644 > --- a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c > +++ b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c > @@ -717,26 +717,19 @@ static DEVICE_ATTR_RW(cntr_rld_event); > static ssize_t cntr_val_show(struct device *dev, > struct device_attribute *attr, char *buf) > { > - int i, ret = 0; > u32 val; > struct etm_drvdata *drvdata = dev_get_drvdata(dev->parent); > struct etm_config *config = &drvdata->config; > > if (!coresight_get_mode(drvdata->csdev)) { > spin_lock(&drvdata->spinlock); > - for (i = 0; i < drvdata->nr_cntr; i++) > - ret += sprintf(buf, "counter %d: %x\n", > - i, config->cntr_val[i]); > + val = config->cntr_val[config->cntr_idx]; > spin_unlock(&drvdata->spinlock); > - return ret; > - } > - > - for (i = 0; i < drvdata->nr_cntr; i++) { > - val = etm_readl(drvdata, ETMCNTVRn(i)); > - ret += sprintf(buf, "counter %d: %x\n", i, val); > + } else { > + val = etm_readl(drvdata, ETMCNTVRn(config->cntr_idx)); > } > > - return ret; > + return sysfs_emit(buf, "%#x\n", val); > } > > static ssize_t cntr_val_store(struct device *dev, Reviewed-by: James Clark <james.clark(a)linaro.org>

6 months, 3 weeks

Re: [PATCH v2] coresight: etm3x: Fix cntr_val_show() to match cntr_val_store() behavior

by James Clark

On 01/12/2025 9:52 am, Kuan-Wei Chiu wrote: > The cntr_val_show() function was intended to print the values of all > counters using a loop. However, due to a buffer overwrite issue with > sprintf(), it effectively only displayed the value of the last counter. > > The companion function, cntr_val_store(), allows users to modify a > specific counter selected by 'cntr_idx'. To maintain consistency > between read and write operations and to align with the ETM4x driver > behavior, modify cntr_val_show() to report only the value of the > currently selected counter. > > This change removes the loop and the "counter %d:" prefix, printing > only the hexadecimal value. It also adopts sysfs_emit() for standard > sysfs output formatting. > > Fixes: a939fc5a71ad ("coresight-etm: add CoreSight ETM/PTM driver") > Signed-off-by: Kuan-Wei Chiu <visitorckw(a)gmail.com> > --- > Build test only. > > Changes in v2: > - Redesigned the fix to match cntr_val_store() logic (display only the > currently selected counter) instead of attempting to list all > counters. > - Removed the loop and the "counter %d:" prefix. > - Switched from sprintf() to sysfs_emit(). > - Refactored control flow to use a single return point. > > v1: https://lore.kernel.org/lkml/20251121002350.1166758-1-visitorckw@gmail.com/ > > .../hwtracing/coresight/coresight-etm3x-sysfs.c | 15 ++++----------- > 1 file changed, 4 insertions(+), 11 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c > index 762109307b86..a6a650331196 100644 > --- a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c > +++ b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c > @@ -717,26 +717,19 @@ static DEVICE_ATTR_RW(cntr_rld_event); > static ssize_t cntr_val_show(struct device *dev, > struct device_attribute *attr, char *buf) > { > - int i, ret = 0; > u32 val; > struct etm_drvdata *drvdata = dev_get_drvdata(dev->parent); > struct etm_config *config = &drvdata->config; > > if (!coresight_get_mode(drvdata->csdev)) { > spin_lock(&drvdata->spinlock); > - for (i = 0; i < drvdata->nr_cntr; i++) > - ret += sprintf(buf, "counter %d: %x\n", > - i, config->cntr_val[i]); > + val = config->cntr_val[config->cntr_idx]; > spin_unlock(&drvdata->spinlock); > - return ret; > - } > - > - for (i = 0; i < drvdata->nr_cntr; i++) { > - val = etm_readl(drvdata, ETMCNTVRn(i)); > - ret += sprintf(buf, "counter %d: %x\n", i, val); > + } else { > + val = etm_readl(drvdata, ETMCNTVRn(config->cntr_idx)); > } > > - return ret; > + return sysfs_emit(buf, "%x\n", val); Sorry I missed this on the previous version, but this should be %#x to add the "0x" prefix. All the other ones in this file have it. Otherwise you can't tell the difference between decimal and hex. > } > > static ssize_t cntr_val_store(struct device *dev,

6 months, 3 weeks

Re: [PATCH] coresight: etm3x: Fix buffer overwrite in cntr_val_show()

by Leo Yan

On Fri, Nov 21, 2025 at 12:23:50AM +0000, Kuan-Wei Chiu wrote: [...] > I noticed this issue while browsing the coresight code after attending > a technical talk on the subject. This code dates back to the initial > driver submission over 10 years ago, so I was surprised it hadn't been > caught earlier. Although I cannot perform runtime testing, the logic > error seems obvious to me, so I still decided to submit this patch. I have a question for maintainers. The ETMv4 architecture specification shows that ETMv4 was released as a non-confidential module in May 2013 (with the confidential release even a year earlier). So ETMv4 has been a public IP for more than 12+ years, and ETMv3 has been gradually retired since then. This fix can still be applied to older kernels, but seems to me that now might be an appropriate time to consider removing the ETMv3 driver from the mainline kernel? Thanks, Leo

6 months, 4 weeks

Jump to page:

2026

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

CoreSight