CoreSight

coresight@lists.linaro.org

3 participants
2711 discussions

Re: [PATCH v6 0/5] Fix traceNoC probe issue on multiple QCOM platforms

by Suzuki K Poulose

On 09/11/2026 01:25, Jie Gan wrote: > > > On 7/10/2026 10:38 AM, Jie Gan wrote: >> The CoreSight TNOC (Trace Network-On-Chip) binding so far only allowed >> the >> two-string AMBA form "qcom,coresight-tnoc", "arm,primecell". That form >> forces the device onto the AMBA bus, where the driver must read the >> peripheral ID from the device registers during probe. On several QCOM >> platforms this AMBA peripheral-ID probing fails, so the traceNoC device >> never probes and its trace path is unavailable. >> >> This series drops the "arm,primecell" entry and turns "qcom,coresight- >> tnoc" >> into a standalone platform compatible. Without "arm,primecell" the device >> is created on the platform bus and probes through the platform driver by >> compatible string, bypassing the AMBA peripheral-ID read while >> remaining a >> fully functional Aggregator TNOC that allocates a system trace ID (ATID). >> >> The series is organized as: binding first, then the driver support for >> the >> platform-bus compatible, followed by the per-platform DT fixes that >> switch >> the affected nodes over to it. >> >> - Patch 1 (dt-bindings) drops "arm,primecell" and makes >> "qcom,coresight-tnoc" a single standalone compatible, and updates the >> example node accordingly. >> - Patch 2 (driver) adds "qcom,coresight-tnoc" to the platform driver's >> match table and renames the itnoc-specific names to generic tnoc names, >> since the platform driver now serves both the Interconnect and Aggregator >> TNOC. It also restricts the ATID-unsupported handling to >> "qcom,coresight-itnoc" only, so the Aggregator TNOC on the platform >> bus is >> no longer wrongly covered and correctly allocates a trace ID. >> - Patches 3-4 (kaanapali, sm8750) drop the "arm,primecell" entry on the >> traceNoC nodes and use the standalone "qcom,coresight-tnoc" compatible, >> fixing the probe failure on those platforms. >> - Patch 5 (glymur) switches the node from "qcom,coresight-itnoc" to >> "qcom,coresight-tnoc". This node is actually an Aggregator TNOC (its >> tn_ag_* endpoints show aggregation), so it should expose a system trace >> ID rather than being treated as an Interconnect TNOC. >> > > Gentle ping. Apologies for missing this. Please could you resend it based on v7.4-rc, as this doesn't apply cleanly on coresight/next Cheers Suzuki

21 minutes

Re: [PATCH] [v2] coresight: funnel: inport schedule by priority can not take work

by James Clark

On 27/10/2026 15:14, Chao Xu wrote: > The CoreSight SoC-600 TRM, section 10.11.2, PRIORITYCONTROL, a funnel > rejects priority register writes whenever any port is enabled. The > current sequence therefore prevent the configured priorities from > taking effect. Write the priority register before enabling the > first port. > > Signed-off-by: Chao Xu <xuchao066(a)163.com> > --- > drivers/hwtracing/coresight/coresight-funnel.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/hwtracing/coresight/coresight-funnel.c b/drivers/hwtracing/coresight/coresight-funnel.c > index 0abc11f0690c..500ff72a83c8 100644 > --- a/drivers/hwtracing/coresight/coresight-funnel.c > +++ b/drivers/hwtracing/coresight/coresight-funnel.c > @@ -62,13 +62,15 @@ static int dynamic_funnel_enable_hw(struct funnel_drvdata *drvdata, int port) > rc = coresight_claim_device_unlocked(csdev); > if (rc) > goto done; > + > + /* Update priorities while all ports are disabled */ > + writel_relaxed(drvdata->priority, drvdata->base + FUNNEL_PRICTL); > } > > functl &= ~FUNNEL_HOLDTIME_MASK; > functl |= FUNNEL_HOLDTIME; > functl |= (1 << port); > writel_relaxed(functl, drvdata->base + FUNNEL_FUNCTL); > - writel_relaxed(drvdata->priority, drvdata->base + FUNNEL_PRICTL); > done: > CS_LOCK(drvdata->base); > return rc; Reviewed-by: James Clark <james.clark(a)linaro.org>

29 minutes

Re: [PATCH] coresight: fix resource leaks on path build failure

by James Clark

On 09/11/2026 01:24, Jie Gan wrote: > > > On 5/13/2026 9:32 AM, Jie Gan wrote: >> Two related leaks when _coresight_build_path() encounters an error after >> coresight_grab_device() has already incremented the pm_runtime, module, >> and device references for a node: >> >> 1. In _coresight_build_path(), if kzalloc_obj() for the path node fails >> after coresight_grab_device() succeeds, coresight_drop_device() was >> never called, permanently leaking all three references. >> >> 2. In coresight_build_path(), on failure the partial path was freed with >> kfree(path) instead of coresight_release_path(path). kfree() only >> frees the coresight_path struct itself; it does not iterate path_list >> to call coresight_drop_device() and kfree() for each coresight_node >> already added by deeper recursive calls, leaking both the pm_runtime, >> module, and device references and the node memory for every element >> on the partial path. >> >> Fix both by adding coresight_drop_device() in the OOM unwind of >> _coresight_build_path(), and replacing kfree(path) with >> coresight_release_path(path) in coresight_build_path(). >> >> Fixes: 32b0707a4182 ("coresight: Add try_get_module() in >> coresight_grab_device()") >> Fixes: b3e94405941e ("coresight: associating path with session rather >> than tracer") >> Signed-off-by: Jie Gan <jie.gan(a)oss.qualcomm.com> > > Gentle ping. > >> --- >> drivers/hwtracing/coresight/coresight-core.c | 6 ++++-- >> 1 file changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/ >> hwtracing/coresight/coresight-core.c >> index 46f247f73cf6..c1354ea8e11d 100644 >> --- a/drivers/hwtracing/coresight/coresight-core.c >> +++ b/drivers/hwtracing/coresight/coresight-core.c >> @@ -825,8 +825,10 @@ static int _coresight_build_path(struct >> coresight_device *csdev, >> return ret; >> node = kzalloc_obj(struct coresight_node); >> - if (!node) >> + if (!node) { >> + coresight_drop_device(csdev); >> return -ENOMEM; >> + } >> node->csdev = csdev; >> list_add(&node->link, &path->path_list); >> @@ -851,7 +853,7 @@ struct coresight_path *coresight_build_path(struct >> coresight_device *source, >> rc = _coresight_build_path(source, source, sink, path); >> if (rc) { >> - kfree(path); >> + coresight_release_path(path); @Suzuki, there's a conflict here. This will need to be coresight_release_path_locked() if we also take "coresight: etm-perf: Serialize AUX topology access" from my Sashiko fixes. >> return ERR_PTR(rc); >> } >> >> --- >> base-commit: e98d21c170b01ddef366f023bbfcf6b31509fa83 >> change-id: 20260513-fix-memory-leak-issue-034b4a45265e >> >> Best regards, >

33 minutes

Re: [PATCH v16 0/7] Provide support for Trigger Generation Unit

by Suzuki K Poulose

Hi On 05/11/2026 12:19, Songwei.Chai wrote: > Hi Suzuki, > > I wanted to follow up and see if you might have some time to take a look > at this patch. Apologies for the delay in responding on this one. But I have now reviewed the series, please address the comments and repost them based on 7.4-rcX Cheers Suzuki > > We’d really appreciate any feedback you may have, as it would help us > determine the next steps and move this work forward. > Thanks for your time. > > Best regards, > Songwei > > On 7/30/2026 4:06 PM, Songwei Chai wrote: >> From: Songwei Chai <songchai(a)qti.qualcomm.com> >> >> 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-kcaj1qO4ZcGnA(a)mail.gmail.com/ >> >> 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: >> >> 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. >> >> 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/amba/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/amba/devices/<tgu-name>/ >> step0_priority0/reg0 >> - echo 0x11113233 > /sys/bus/amba/devices/<tgu-name>/ >> step0_priority1/reg0 >> >> 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/amba/devices/<tgu-name>/ >> step0_condition_decode/reg0 >> - echo 0x30 > /sys/bus/amba/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/amba/devices/<tgu-name>/ >> step0_condition_select/reg0 >> - echo 0x20008 > /sys/bus/amba/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/amba/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/amba/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/amba/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/amba/devices/<tgu-name>/step1_timer/reg0 >> >> 9. enale tgu >> - echo 1 > /sys/bus/amba/devices/<tgu-name>/enable_tgu >> --- >> Link to V15: https://lore.kernel.org/all/20260701055529.445819-1- >> songwei.chai(a)oss.qualcomm.com/ >> >> Changes in V16: >> - Fixed enable/disable race and PM refcount handling. >> - Fixed sysfs exposure before driver initialization. >> - Fixed priority allocation and access races. >> - Fixed uninitialized return value in condition decode. >> - Fixed sysfs attribute naming collision issues. >> - Added ABI documentation for the default node. >> --- >> Link to V14: https://lore.kernel.org/all/20260417073336.2712426-1- >> songwei.chai(a)oss.qualcomm.com/ >> >> Changes in V15: >> - Add a maintainer for "drivers/hwtracing/qcom" in MAINTAINERS >> --- >> Link to V13: https://lore.kernel.org/all/20260402092838.341295-1- >> songwei.chai(a)oss.qualcomm.com/ >> >> Changes in V14: >> - Fix some typos and formatting. >> --- >> Link to V12: https://lore.kernel.org/all/20260317032639.2393221-1- >> songwei.chai(a)oss.qualcomm.com/ >> >> Changes in V13: >> - add ":" after "KernelVersion" >> - add an enablement check in the enable function to avoid increasing >> the counter each time >> --- >> Link to V11: https://lore.kernel.org/all/ee1ca8e6-8e5f-47d8-8a24- >> f904ee2fc6d0(a)oss.qualcomm.com/ >> >> Changes in V12: >> - Remove the in-ports property from the bindings, as this device is >> decoupled from CoreSight. >> - Update kernel version and date. >> --- >> Link to V10: https://lore.kernel.org/ >> all/20c5406d-3e9f-4fdb-84ba-4cbe629c79b5(a)oss.qualcomm.com/ >> >> Changes in V11: >> - Change the names of members in drvdata: max_xxx -> num_xxx, enable - >> > enabled >> - Use "FIELD_GET" to replace "BMVAL" >> - Use devm_kcalloc to replace devm_kzalloc once create members of >> value_table >> - Keep a consistent \n above return >> - Keep reverse-Christmas-tree style >> - Add checks so that the enable and reset nodes only accept 0 or 1 >> --- >> Link to V9: https://lore.kernel.org/all/20251219065902.2296896-1- >> songwei.chai(a)oss.qualcomm.com/ >> >> Changes in V10: >> - Modified code formatting based on Jie's feedback to improve >> readability. >> - Applied inverse Christmas tree order to the variables. >> --- >> Link to V8: https://lore.kernel.org/all/20251203090055.2432719-1- >> songwei.chai(a)oss.qualcomm.com/ >> >> Changes in V9: >> - Decoupled the tgu driver from coresight header file and registered >> it as an amba device. >> - Retained Rob's reviewed-by tag on patch1/7 since the file remains >> unchanged. >> - Updated the sysfs node path in the Documentation directory. >> --- >> Link to V7: https://lore.kernel.org/all/20251104064043.88972-1- >> songwei.chai(a)oss.qualcomm.com/ >> >> 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(a)qti.qualcomm.com/ >> >> 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(a)quicinc.com/ >> >> 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.954066-1-quic_songchai(a)quicinc.com/ >> >> 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(a)quicinc.com/ >> >> 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(a)quicinc.com/ >> >> 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(a)quicinc.com/ >> >> 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 >> >> .../ABI/testing/sysfs-bus-amba-devices-tgu | 60 ++ >> .../devicetree/bindings/arm/qcom,tgu.yaml | 71 ++ >> MAINTAINERS | 11 + >> drivers/Makefile | 1 + >> drivers/hwtracing/Kconfig | 2 + >> drivers/hwtracing/qcom/Kconfig | 20 + >> drivers/hwtracing/qcom/Makefile | 3 + >> drivers/hwtracing/qcom/tgu.c | 782 ++++++++++++++++++ >> drivers/hwtracing/qcom/tgu.h | 318 +++++++ >> 9 files changed, 1268 insertions(+) >> create mode 100644 Documentation/ABI/testing/sysfs-bus-amba-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 >>

2 days, 20 hours

Re: [PATCH] Revert "perf/aux: Allocate non-contiguous AUX pages by default"

by Leo Yan

On Wed, Nov 04, 2026 at 12:59:13PM +0000, Will Deacon wrote: [...] > > Sorry for the late reply, and sorry that the commit message in > > 18049c8cff9c ("perf/aux: Allocate non-contiguous AUX pages by default") > > was imprecise about "exacerbating memory fragmentation". You and Dev are > > right that grouping same-lifetime pages into higher-order allocations > > does not itself worsen fragmentation, and the TLB reduction numbers for > > TRBE and SPE with large vmap() mappings make total sense given TRBE's > > tiny TLB. > > > > However, a straight revert will reintroduce a severe production issue we > > hit on Android devices, so I'd like to share what actually happened and > > suggest a way to get large mappings for SPE/TRBE without bringing back > > the regression. > > [...] > > Thanks, Yabin. This just hardens my stance [1] that the allocation decision > should be driven by userspace and not second-guessed by the kernel. So the > revert is _not_ the right approach, in my view. If a userspace interface is needed, what remaining reclaim or allocation policy should userspace control? I am not clear how userspace control would fix the PSI reclaim stalls described in Yabin's reply. Alternatively, we could optimize the reported reclaim cost in the kernel by clearing __GFP_RECLAIM for optional higher-order attempts, while preserving the existing policy at order 0 and for PERF_PMU_CAP_AUX_NO_SG PMUs. If testing confirms this works, combining this mitigation with a revert of 18049c8cff9c would be a reasonable way to restore higher-order allocations for SPE/TRBE without adding a userspace interface. Thanks, Leo

3 days, 1 hour

Re: [PATCH] Revert "perf/aux: Allocate non-contiguous AUX pages by default"

by Leo Yan

On Tue, Nov 03, 2026 at 12:46:24PM -0800, Yabin Cui wrote: ... > In field telemetry and local ftrace on Pixel devices (prior to > 18049c8cff9c, so __GFP_NORETRY was already in place), we observed heavy > rb_alloc_aux() -> __alloc_pages_direct_compact() stalls alongside > kcompactd0 pinned on big cores scanning order-9, and this accounted for > >20% of all perceptible app kills (via lmkd) across dogfood devices > until we stopped high-order AUX allocations. Thanks for the detailed explanation. My understanding is that repeated reclaim/compaction during AUX allocation raises memory PSI and can lead to lmkd killing applications. The problem to address is therefore the cost of obtaining higher-order pages under memory fragmentation. > Initially we worked around this in userspace (simpleperf) by setting > perf_event_attr::aux_watermark = 4096 so get_order(watermark) == 0. That > was fine for CoreSight ETR (which uses its own separate scatter-gather > buffer and only copies into AUX on flush/stop), but when enabling TRBE > (and SPE) it caused hardware maintenance interrupts every 4 KiB instead > of every 2 MiB. Also, in overwrite/snapshot mode rb_alloc_aux() ignores > aux_watermark and uses ilog2(nr_pages). That is why I sent 18049c8cff9c. Agreed. Reducing watermark to 4096 also increases maintenance interrupt frequency for SPE/TRBE. We should refine buffer allocation rather than using watermark as a workaround. > ### Possible solutions > > For PMUs without PERF_PMU_CAP_AUX_NO_SG (such as SPE, TRBE, and modern > Intel PT), higher-order AUX pages are an opportunistic optimization > rather than a functional requirement -- very similar to THP allocations > on fault (GFP_TRANSHUGE_LIGHT) or opportunistic high-order slabs in SLUB > (allocate_slab()). > > Instead of reverting to GFP_KERNEL reclaim/compaction for all high-order > attempts, could we do the following? > > 1. Make opportunistic high-order AUX allocations no-reclaim: > When !(event->pmu->capabilities & PERF_PMU_CAP_AUX_NO_SG) and > order > 0, clear __GFP_RECLAIM (and add __GFP_NOMEMALLOC, matching > SLUB's allocate_slab() and GFP_TRANSHUGE_LIGHT): > > gfp_t gfp = PERF_AUX_GFP; > > if (order && !has_no_sg) > gfp = (PERF_AUX_GFP | __GFP_NOMEMALLOC) & ~__GFP_RECLAIM; > > When order falls back to 0 (or when PERF_PMU_CAP_AUX_NO_SG is set), > keep PERF_AUX_GFP so single-page allocations still reclaim normally > under memory pressure. > > That way, when free order-9 / order-4 blocks exist, SPE and TRBE get > them immediately for PMD / contiguous-PTE vmap() mappings (and pages > stay grouped together); when memory is fragmented on a long-running > device, high-order probes fail fast on the freelist without waking > kcompactd/kswapd or triggering direct compaction and reclaim. This looks reasonable for me: try to obtain larger blocks cheaply, then then retain the PERF_AUX_GFP policy for order-0 allocations. I also agree with preserving the allocation policy for PERF_PMU_CAP_AUX_NO_SG case. Could you compare the memory PSI stall deltas before and after applying this option? I think measurements from your production devices would be convinced evidence for benefits. > 2. Clamp max_order in rb_alloc_aux() after fallback: > If rb_alloc_aux_page() requested `order` and had to fall back to a > smaller order, update `max_order = page_private(page)` in > rb_alloc_aux() so subsequent iterations for the rest of the buffer do > not keep re-probing higher orders that just failed. This also can avoid reclaim repeatedly, but the side effect is subsequent iterations would no longer try high order, which have no chance to benefit from large mapping. I would prefer to evaluate option 1 first. > (Optionally, if server/benchmark workloads ever want synchronous > compaction to form huge AUX mappings even on fragmented hosts, that could > be gated behind an opt-in attr/sysfs knob similar to THP defrag modes, > with light/no-reclaim as the default.) I will follow up with Will separately about the userspace interface. > Would this approach work for your SPE/TRBE use cases? Happy to help test > on our side. If option 1 works, we can confidently revert 18049c8cff9c, as it would restore higher-order allocations for TRBE/SPE without introducing the regression you mentioned. Thanks, Leo

3 days, 17 hours

Re: [PATCH v16 4/7] qcom-tgu: Add TGU decode support

by Suzuki K Poulose

On 30/07/2026 09:07, Songwei Chai wrote: > From: Songwei Chai <songchai(a)qti.qualcomm.com> > > Decoding is when all the potential pieces for creating a trigger > are brought together for a given step. Example - there may be a > counter keeping track of some occurrences and a priority-group that > is being used to detect a pattern on the sense inputs. These 2 > inputs to condition_decode must be programmed, for a given step, > to establish the condition for the trigger, or movement to another > steps. > > Reviewed-by: Jie Gan <jie.gan(a)oss.qualcomm.com> > Signed-off-by: Songwei Chai <songchai(a)qti.qualcomm.com> > --- > .../ABI/testing/sysfs-bus-amba-devices-tgu | 7 + > drivers/hwtracing/qcom/tgu.c | 182 ++++++++++++++++-- > drivers/hwtracing/qcom/tgu.h | 37 ++++ > 3 files changed, 206 insertions(+), 20 deletions(-) > > diff --git a/Documentation/ABI/testing/sysfs-bus-amba-devices-tgu b/Documentation/ABI/testing/sysfs-bus-amba-devices-tgu > index 7dd24314592d..705e75c776d4 100644 > --- a/Documentation/ABI/testing/sysfs-bus-amba-devices-tgu > +++ b/Documentation/ABI/testing/sysfs-bus-amba-devices-tgu > @@ -14,3 +14,10 @@ KernelVersion: 7.2 > Contact: Jinlong Mao <jinlong.mao(a)oss.qualcomm.com>, Songwei Chai <songwei.chai(a)oss.qualcomm.com> > Description: > (RW) Set/Get the sensed signal with specific step and priority for TGU. > + > +What: /sys/bus/amba/devices/<tgu-name>/step[0:7]_condition_decode/reg[0:3] > +Date: August 2026 > +KernelVersion: 7.2 7.5 for the next version , everywhere, please. > +Contact: Jinlong Mao <jinlong.mao(a)oss.qualcomm.com>, Songwei Chai <songwei.chai(a)oss.qualcomm.com> > +Description: > + (RW) Set/Get the decode mode with specific step for TGU. > diff --git a/drivers/hwtracing/qcom/tgu.c b/drivers/hwtracing/qcom/tgu.c > index 0e88b437f191..e83295217e7a 100644 > --- a/drivers/hwtracing/qcom/tgu.c > +++ b/drivers/hwtracing/qcom/tgu.c > @@ -19,8 +19,33 @@ static int calculate_array_location(struct tgu_drvdata *drvdata, > int step_index, int operation_index, > int reg_index) > { > - return operation_index * (drvdata->num_step) * (drvdata->num_reg) + > - step_index * (drvdata->num_reg) + reg_index; > + switch (operation_index) { > + case TGU_PRIORITY0: > + case TGU_PRIORITY1: > + case TGU_PRIORITY2: > + case TGU_PRIORITY3: > + return operation_index * (drvdata->num_step) * > + (drvdata->num_reg) + > + step_index * (drvdata->num_reg) + reg_index; > + case TGU_CONDITION_DECODE: > + return step_index * (drvdata->num_condition_decode) + > + reg_index; > + default: > + break; > + } > + > + return -EINVAL; > +} > + > +static int check_array_location(struct tgu_drvdata *drvdata, int step, > + int ops, int reg) > +{ > + int result = calculate_array_location(drvdata, step, ops, reg); > + > + if (result == -EINVAL) > + dev_err(drvdata->dev, "check array location - Fail\n"); > + > + return result; > } > > static ssize_t tgu_dataset_show(struct device *dev, > @@ -31,9 +56,11 @@ static ssize_t tgu_dataset_show(struct device *dev, > container_of(attr, struct tgu_attribute, attr); > int index; > > - index = calculate_array_location(drvdata, tgu_attr->step_index, > - tgu_attr->operation_index, > - tgu_attr->reg_num); > + index = check_array_location(drvdata, tgu_attr->step_index, > + tgu_attr->operation_index, tgu_attr->reg_num); > + > + if (index == -EINVAL) > + return index; > > /* > * Serialise against tgu_dataset_store() and tgu_enable(), which read > @@ -42,8 +69,20 @@ static ssize_t tgu_dataset_show(struct device *dev, > */ > guard(mutex)(&drvdata->lock); > > - return sysfs_emit(buf, "0x%x\n", > - drvdata->value_table->priority[index]); > + switch (tgu_attr->operation_index) { > + case TGU_PRIORITY0: > + case TGU_PRIORITY1: > + case TGU_PRIORITY2: > + case TGU_PRIORITY3: > + return sysfs_emit(buf, "0x%x\n", > + drvdata->value_table->priority[index]); > + case TGU_CONDITION_DECODE: > + return sysfs_emit(buf, "0x%x\n", > + drvdata->value_table->condition_decode[index]); > + default: > + break; > + } > + return -EINVAL; > } > > static ssize_t tgu_dataset_store(struct device *dev, > @@ -66,9 +105,27 @@ static ssize_t tgu_dataset_store(struct device *dev, > tgu_attr->operation_index, > tgu_attr->reg_num); > > - tgu_drvdata->value_table->priority[index] = val; > + if (index == -EINVAL) > + return index; > + > + switch (tgu_attr->operation_index) { > + case TGU_PRIORITY0: > + case TGU_PRIORITY1: > + case TGU_PRIORITY2: > + case TGU_PRIORITY3: > + tgu_drvdata->value_table->priority[index] = val; > + ret = size; > + break; > + case TGU_CONDITION_DECODE: > + tgu_drvdata->value_table->condition_decode[index] = val; > + ret = size; > + break; > + default: > + ret = -EINVAL; > + break; > + } > > - return size; > + return ret; > } > > static umode_t tgu_node_visible(struct kobject *kobject, > @@ -85,22 +142,41 @@ static umode_t tgu_node_visible(struct kobject *kobject, > if (tgu_attr->step_index >= drvdata->num_step) > return SYSFS_GROUP_INVISIBLE; > > - if (tgu_attr->reg_num >= drvdata->num_reg) > - return 0; > + switch (tgu_attr->operation_index) { > + case TGU_PRIORITY0: > + case TGU_PRIORITY1: > + case TGU_PRIORITY2: > + case TGU_PRIORITY3: > + if (!drvdata->num_reg) > + return SYSFS_GROUP_INVISIBLE; > + if (tgu_attr->reg_num < drvdata->num_reg) > + return attr->mode; > + break; > + case TGU_CONDITION_DECODE: > + if (!drvdata->num_condition_decode) > + return SYSFS_GROUP_INVISIBLE; > + if (tgu_attr->reg_num < drvdata->num_condition_decode) > + return attr->mode; > + break; > + default: > + break; > + } > > - return attr->mode; > + return 0; > } > > -static void tgu_write_all_hw_regs(struct tgu_drvdata *drvdata) > +static ssize_t tgu_write_all_hw_regs(struct tgu_drvdata *drvdata) > { > - int i, j, k, index; > + int i, j, k, index = 0; > > TGU_UNLOCK(drvdata->base); > for (i = 0; i < drvdata->num_step; i++) { > for (j = 0; j < MAX_PRIORITY; j++) { > for (k = 0; k < drvdata->num_reg; k++) { > - index = calculate_array_location( > + index = check_array_location( > drvdata, i, j, k); > + if (index == -EINVAL) > + goto exit; > > writel(drvdata->value_table->priority[index], > drvdata->base + > @@ -108,9 +184,23 @@ static void tgu_write_all_hw_regs(struct tgu_drvdata *drvdata) > } > } > } > + > + for (i = 0; i < drvdata->num_step; i++) { > + for (j = 0; j < drvdata->num_condition_decode; j++) { > + index = check_array_location(drvdata, i, > + TGU_CONDITION_DECODE, j); > + if (index == -EINVAL) > + goto exit; > + > + writel(drvdata->value_table->condition_decode[index], > + drvdata->base + CONDITION_DECODE_STEP(i, j)); > + } > + } > /* Enable TGU to program the triggers */ > writel(1, drvdata->base + TGU_CONTROL); > +exit: > TGU_LOCK(drvdata->base); > + return index >= 0 ? 0 : -EINVAL; > } > > static void tgu_set_reg_number(struct tgu_drvdata *drvdata) > @@ -153,10 +243,31 @@ static void tgu_set_steps(struct tgu_drvdata *drvdata) > } > } > > -static void tgu_enable(struct tgu_drvdata *drvdata) > +static int tgu_enable(struct tgu_drvdata *drvdata) > { > - tgu_write_all_hw_regs(drvdata); > + int ret; > + > + ret = tgu_write_all_hw_regs(drvdata); > + if (ret) > + return ret; > + > drvdata->enabled = true; > + return 0; > +} > + > +static void tgu_set_conditions(struct tgu_drvdata *drvdata) > +{ > + u32 devid; > + > + devid = readl(drvdata->base + TGU_DEVID); > + drvdata->num_condition_decode = TGU_DEVID_CONDITIONS(devid); > + > + if (drvdata->num_condition_decode > TGU_MAX_CONDITION_DECODE) { > + dev_warn(drvdata->dev, > + "DEVID reports %d condition decode regs, clamping to %d\n", > + drvdata->num_condition_decode, TGU_MAX_CONDITION_DECODE); > + drvdata->num_condition_decode = TGU_MAX_CONDITION_DECODE; > + } > } > > static void tgu_disable(struct tgu_drvdata *drvdata) > @@ -210,7 +321,11 @@ static ssize_t enable_tgu_store(struct device *dev, > if (ret) > return ret; > > - tgu_enable(drvdata); > + ret = tgu_enable(drvdata); > + if (ret) { > + pm_runtime_put(dev); > + return ret; > + } > } else { > if (!drvdata->enabled) > return -EINVAL; > @@ -266,6 +381,14 @@ static const struct attribute_group *tgu_attr_groups[] = { > PRIORITY_ATTRIBUTE_GROUP_INIT(7, 1), > PRIORITY_ATTRIBUTE_GROUP_INIT(7, 2), > PRIORITY_ATTRIBUTE_GROUP_INIT(7, 3), > + CONDITION_DECODE_ATTRIBUTE_GROUP_INIT(0), > + CONDITION_DECODE_ATTRIBUTE_GROUP_INIT(1), > + CONDITION_DECODE_ATTRIBUTE_GROUP_INIT(2), > + CONDITION_DECODE_ATTRIBUTE_GROUP_INIT(3), > + CONDITION_DECODE_ATTRIBUTE_GROUP_INIT(4), > + CONDITION_DECODE_ATTRIBUTE_GROUP_INIT(5), > + CONDITION_DECODE_ATTRIBUTE_GROUP_INIT(6), > + CONDITION_DECODE_ATTRIBUTE_GROUP_INIT(7), > NULL, > }; > > @@ -273,8 +396,9 @@ static int tgu_probe(struct amba_device *adev, const struct amba_id *id) > { > struct device *dev = &adev->dev; > struct tgu_drvdata *drvdata; > - unsigned int *priority; > - size_t priority_size; > + unsigned int *priority, *condition; > + size_t priority_size, condition_size; > + int i, j; > int ret; > > drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL); > @@ -294,6 +418,7 @@ static int tgu_probe(struct amba_device *adev, const struct amba_id *id) > > tgu_set_reg_number(drvdata); > tgu_set_steps(drvdata); > + tgu_set_conditions(drvdata); > > drvdata->value_table = > devm_kzalloc(dev, sizeof(*drvdata->value_table), GFP_KERNEL); > @@ -310,6 +435,23 @@ static int tgu_probe(struct amba_device *adev, const struct amba_id *id) > > drvdata->value_table->priority = priority; > > + condition_size = drvdata->num_condition_decode * drvdata->num_step; > + > + condition = devm_kcalloc(dev, condition_size, > + sizeof(*(drvdata->value_table->condition_decode)), > + GFP_KERNEL); > + if (!condition) > + return -ENOMEM; > + > + drvdata->value_table->condition_decode = condition; > + > + /* Initialize all condition decode registers to NOT */ > + for (i = 0; i < drvdata->num_step; i++) > + for (j = 0; j < drvdata->num_condition_decode; j++) > + condition[calculate_array_location( > + drvdata, i, TGU_CONDITION_DECODE, j)] = > + TGU_CONDITION_DECODE_NOT; > + > drvdata->enabled = false; > /* > * Create the sysfs groups only after the backing storage is fully > diff --git a/drivers/hwtracing/qcom/tgu.h b/drivers/hwtracing/qcom/tgu.h > index 7a3e1161c4cf..8b6a46995a5f 100644 > --- a/drivers/hwtracing/qcom/tgu.h > +++ b/drivers/hwtracing/qcom/tgu.h > @@ -19,6 +19,8 @@ > ((int)FIELD_GET(GENMASK(17, 10), devid_val)) > #define TGU_DEVID_STEPS(devid_val) \ > ((int)FIELD_GET(GENMASK(6, 3), devid_val)) > +#define TGU_DEVID_CONDITIONS(devid_val) \ > + ((int)FIELD_GET(GENMASK(2, 0), devid_val)) > #define TGU_BITS_PER_SIGNAL 4 > #define LENGTH_REGISTER 32 > > @@ -52,6 +54,7 @@ > */ > #define STEP_OFFSET 0x1D8 > #define PRIORITY_START_OFFSET 0x0074 > +#define CONDITION_DECODE_OFFSET 0x0050 > #define PRIORITY_OFFSET 0x60 > #define REG_OFFSET 0x4 > > @@ -63,6 +66,7 @@ > * > * TGU_MAX_STEPS static tables define step0..step7 > * TGU_MAX_PRIORITY_REGS STEP_PRIORITY_LIST defines reg0..reg17 > + * TGU_MAX_CONDITION_DECODE decode region 0x50..0x5F holds 4 u32 registers > * > * A device whose DEVID reports counts above these bounds is clamped, so writes > * never fall outside the ioremap()'d window or spill into an adjacent region. > @@ -70,12 +74,16 @@ > > #define TGU_MAX_STEPS 8 > #define TGU_MAX_PRIORITY_REGS 18 > +#define TGU_MAX_CONDITION_DECODE 4 > > /* Calculate compare step addresses */ > #define PRIORITY_REG_STEP(step, priority, reg)\ > (PRIORITY_START_OFFSET + PRIORITY_OFFSET * priority +\ > REG_OFFSET * reg + STEP_OFFSET * step) > > +#define CONDITION_DECODE_STEP(step, decode) \ > + (CONDITION_DECODE_OFFSET + REG_OFFSET * decode + STEP_OFFSET * step) > + > #define tgu_dataset_rw(name, step_index, type, reg_num) \ > (&((struct tgu_attribute[]){ { \ > __ATTR(name, 0644, tgu_dataset_show, tgu_dataset_store), \ > @@ -87,6 +95,8 @@ > #define STEP_PRIORITY(step_index, reg_num, priority) \ > tgu_dataset_rw(reg##reg_num, step_index, TGU_PRIORITY##priority, \ > reg_num) > +#define STEP_DECODE(step_index, reg_num) \ > + tgu_dataset_rw(reg##reg_num, step_index, TGU_CONDITION_DECODE, reg_num) > > #define STEP_PRIORITY_LIST(step_index, priority) \ > {STEP_PRIORITY(step_index, 0, priority), \ > @@ -110,6 +120,14 @@ > NULL \ > } > > +#define STEP_DECODE_LIST(n) \ > + {STEP_DECODE(n, 0), \ Please use a new line for all such defintions. > + STEP_DECODE(n, 1), \ > + STEP_DECODE(n, 2), \ > + STEP_DECODE(n, 3), \ > + NULL \ > + } > + > #define PRIORITY_ATTRIBUTE_GROUP_INIT(step, priority)\ > (&(const struct attribute_group){\ > .attrs = (struct attribute*[])STEP_PRIORITY_LIST(step, priority),\ > @@ -117,16 +135,32 @@ > .name = "step" #step "_priority" #priority \ > }) > > +#define CONDITION_DECODE_ATTRIBUTE_GROUP_INIT(step)\ > + (&(const struct attribute_group){\ > + .attrs = (struct attribute*[])STEP_DECODE_LIST(step),\ > + .is_visible = tgu_node_visible,\ > + .name = "step" #step "_condition_decode" \ > + }) > + > enum operation_index { > TGU_PRIORITY0, > TGU_PRIORITY1, > TGU_PRIORITY2, > TGU_PRIORITY3, > + TGU_CONDITION_DECODE, > }; > > /* Maximum priority that TGU supports */ > #define MAX_PRIORITY 4 > > +/* > + * Neutral ('NOT') value for the condition decode registers. This is the > + * driver's default so that, before userspace programs a real decode, an > + * enabled step does not spuriously match. Used both when seeding the shadow > + * table at probe and when clearing it via reset_tgu. > + */ > +#define TGU_CONDITION_DECODE_NOT 0x1000000 > + > struct tgu_attribute { > struct device_attribute attr; > u32 step_index; > @@ -136,6 +170,7 @@ struct tgu_attribute { > > struct value_table { > unsigned int *priority; > + unsigned int *condition_decode; > }; > > static inline void TGU_LOCK(void __iomem *addr) > @@ -166,6 +201,7 @@ static inline void TGU_UNLOCK(void __iomem *addr) > * @value_table: Store given value based on relevant parameters > * @num_reg: Maximum number of registers > * @num_step: Maximum step size > + * @num_condition_decode: Maximum number of condition_decode > * > * This structure defines the data associated with a TGU device, > * including its base address, device pointer, the lock serialising > @@ -179,6 +215,7 @@ struct tgu_drvdata { > struct value_table *value_table; > int num_reg; > int num_step; > + int num_condition_decode; > }; > > #endif

3 days, 19 hours

Re: [PATCH v4 5/7] perf: arm_pmuv3: Add pmuv3_event_implemented()

by Suzuki K Poulose

On 26/10/2026 05:48, Amir Ayupov wrote: > Drivers outside the PMU driver may need to know whether the local CPU's > PMU implements a common event before relying on it, for example a trace > unit driver checking that the PMU counts its external outputs as > TRCEXTOUTn. > > Add pmuv3_event_implemented() to the arm64 asm/arm_pmuv3.h, next to > read_pmceid0() and read_pmceid1(). It reads PMCEID0/1 on the calling CPU: > events 0x0000-0x003f are reported in bits [31:0], and the common events > from ARMV8_PMUV3_EXT_COMMON_EVENT_BASE in bits [63:32] from PMUv3p1. Move > ARMV8_PMUV3_EXT_COMMON_EVENT_BASE next to the event numbers in > linux/perf/arm_pmuv3.h so it is available without CONFIG_ARM_PMU. > > Suggested-by: James Clark <james.clark(a)linaro.org> > Assisted-by: Devmate:Opus-5.5 > Signed-off-by: Amir Ayupov <aaupov(a)fb.com> > --- > arch/arm64/include/asm/arm_pmuv3.h | 19 +++++++++++++++++++ > include/linux/perf/arm_pmu.h | 1 - > include/linux/perf/arm_pmuv3.h | 3 +++ > 3 files changed, 22 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/include/asm/arm_pmuv3.h b/arch/arm64/include/asm/arm_pmuv3.h > index 8a777dec8d88a..acd62c4f16743 100644 > --- a/arch/arm64/include/asm/arm_pmuv3.h > +++ b/arch/arm64/include/asm/arm_pmuv3.h > @@ -173,6 +173,25 @@ static inline bool pmuv3_implemented(int pmuver) > pmuver == ID_AA64DFR0_EL1_PMUVer_NI); > } > > +/* > + * Whether this CPU's PMU implements a common event, as reported by > + * PMCEID{0,1}: events 0x0000-0x003f in bits [31:0], and from PMUv3p1 events > + * 0x4000-0x403f in bits [63:32], which read as zero before PMUv3p1. > + * Must run on the CPU whose PMU is queried. > + */ > +static inline bool pmuv3_event_implemented(u16 event) > +{ > + bool ext = event >= ARMV8_PMUV3_EXT_COMMON_EVENT_BASE; > + u16 id = ext ? event - ARMV8_PMUV3_EXT_COMMON_EVENT_BASE : event; > + u64 pmceid; > + > + if (id >= 64 || !pmuv3_implemented(read_pmuver())) > + return false; > + > + pmceid = id < 32 ? :() : read_pmceid1(); > + return pmceid & BIT_ULL(id % 32 + (ext ? 32 : 0)); > +} > + > static inline bool is_pmuv3p4(int pmuver) > { > return pmuver >= ID_AA64DFR0_EL1_PMUVer_V3P4; > diff --git a/include/linux/perf/arm_pmu.h b/include/linux/perf/arm_pmu.h > index 02d2c7f45b527..792283d29d7b5 100644 > --- a/include/linux/perf/arm_pmu.h > +++ b/include/linux/perf/arm_pmu.h > @@ -124,7 +124,6 @@ struct arm_pmu { > u64 reg_brbidr; > #define ARMV8_PMUV3_MAX_COMMON_EVENTS 0x40 > DECLARE_BITMAP(pmceid_bitmap, ARMV8_PMUV3_MAX_COMMON_EVENTS); > -#define ARMV8_PMUV3_EXT_COMMON_EVENT_BASE 0x4000 > DECLARE_BITMAP(pmceid_ext_bitmap, ARMV8_PMUV3_MAX_COMMON_EVENTS); Could we not avoid this change ? Or why not move both of the definitions ? Also, I would need an ACK from the arm_pmu maintainers to accept this change via coresight tree. Cheers Suzuki > > /* Only to be used by ACPI probing code */ > diff --git a/include/linux/perf/arm_pmuv3.h b/include/linux/perf/arm_pmuv3.h > index d698efba28a27..c21f185a283d6 100644 > --- a/include/linux/perf/arm_pmuv3.h > +++ b/include/linux/perf/arm_pmuv3.h > @@ -78,6 +78,9 @@ > #define ARMV8_PMUV3_PERFCTR_STALL_SLOT_FRONTEND 0x003E > #define ARMV8_PMUV3_PERFCTR_STALL_SLOT 0x003F > > +/* Base of the common events reported in PMCEID{0,1}[63:32] from PMUv3p1 */ > +#define ARMV8_PMUV3_EXT_COMMON_EVENT_BASE 0x4000 > + > /* Statistical profiling extension microarchitectural events */ > #define ARMV8_SPE_PERFCTR_SAMPLE_POP 0x4000 > #define ARMV8_SPE_PERFCTR_SAMPLE_FEED 0x4001

3 days, 19 hours

Re: [PATCH v16 7/7] qcom-tgu: Add reset node to initialize

by Suzuki K Poulose

On 30/07/2026 09:07, Songwei Chai wrote: > Add reset node to initialize the value of > priority/condition_decode/condition_select/timer/counter nodes. > > Signed-off-by: Songwei Chai <songwei.chai(a)oss.qualcomm.com> > Reviewed-by: Jie Gan <jie.gan(a)oss.qualcomm.com> > --- > .../ABI/testing/sysfs-bus-amba-devices-tgu | 7 ++ > drivers/hwtracing/qcom/tgu.c | 84 +++++++++++++++++++ > 2 files changed, 91 insertions(+) > > diff --git a/Documentation/ABI/testing/sysfs-bus-amba-devices-tgu b/Documentation/ABI/testing/sysfs-bus-amba-devices-tgu > index a0cad2f16310..5c928a5fa033 100644 > --- a/Documentation/ABI/testing/sysfs-bus-amba-devices-tgu > +++ b/Documentation/ABI/testing/sysfs-bus-amba-devices-tgu > @@ -51,3 +51,10 @@ KernelVersion: 7.2 > Contact: Jinlong Mao <jinlong.mao(a)oss.qualcomm.com>, Songwei Chai <songwei.chai(a)oss.qualcomm.com> > Description: > (RW) Set/Get the counter value with specific step for TGU. > + > +What: /sys/bus/amba/devices/<tgu-name>/reset_tgu > +Date: August 2026 > +KernelVersion: 7.2 > +Contact: Jinlong Mao <jinlong.mao(a)oss.qualcomm.com>, Songwei Chai <songwei.chai(a)oss.qualcomm.com> > +Description: > + (Write) Write 1 to reset the dataset for TGU. > diff --git a/drivers/hwtracing/qcom/tgu.c b/drivers/hwtracing/qcom/tgu.c > index ea83273a628c..d65a68faa741 100644 > --- a/drivers/hwtracing/qcom/tgu.c > +++ b/drivers/hwtracing/qcom/tgu.c > @@ -228,6 +228,18 @@ static ssize_t tgu_write_all_hw_regs(struct tgu_drvdata *drvdata) > int i, j, k, index = 0; > > TGU_UNLOCK(drvdata->base); > + > + /* > + * Quiesce the detection engine before mutating any comparison input. > + * This function is also reached from reset_tgu_store() while the TGU is > + * already running (TGU_CONTROL == 1); rewriting the priority/condition/ > + * timer/counter registers register-by-register on a live engine could > + * feed a transient, half-updated match into CTI/interrupt consumers. > + * Programming with CONTROL == 0 and only re-enabling at the end keeps the > + * program-then-enable ordering for every caller. > + */ > + writel(0, drvdata->base + TGU_CONTROL); > + > for (i = 0; i < drvdata->num_step; i++) { > for (j = 0; j < MAX_PRIORITY; j++) { > for (k = 0; k < drvdata->num_reg; k++) { > @@ -459,8 +471,80 @@ static ssize_t enable_tgu_store(struct device *dev, > } > static DEVICE_ATTR_RW(enable_tgu); > > +static ssize_t reset_tgu_store(struct device *dev, > + struct device_attribute *attr, const char *buf, > + size_t size) > +{ > + struct tgu_drvdata *drvdata = dev_get_drvdata(dev); > + struct value_table *vt = drvdata->value_table; > + unsigned long value; > + int i, j, ret; > + > + if (kstrtoul(buf, 0, &value) || value != 1) > + return -EINVAL; > + > + /* > + * Hold the mutex across the whole sequence so it cannot race with > + * enable_tgu_store()/tgu_dataset_store(): the enabled check and the > + * hardware access stay consistent. No PM reference is taken or dropped > + * here - the shadow tables are plain memory, and the hardware is only > + * touched when the device is already resumed via the enable reference. > + */ > + guard(mutex)(&drvdata->lock); > + > + if (vt->priority) { > + size_t bytes = MAX_PRIORITY * drvdata->num_step * > + drvdata->num_reg * sizeof(unsigned int); Please could you add a macro to compute "number" of each array ? Something like : #define TGU_PRIORITY_SIZE(drvdata) (MAX_PRIORITY * (drvdata)->num_step * (drvdata)->num_reg) So that we use the same size calculation for allocation and reset ? You may scale it by sizeof(*vt->priority) > + memset(vt->priority, 0, bytes); > + } > + > + if (vt->condition_decode) { > + size_t bytes = drvdata->num_condition_decode * > + drvdata->num_step * sizeof(unsigned int); Same as above > + memset(vt->condition_decode, 0, bytes); > + > + /* Initialize all condition decode registers to NOT (0x1000000) */ then why do the memset above ? > + for (i = 0; i < drvdata->num_step; i++) > + for (j = 0; j < drvdata->num_condition_decode; j++) > + vt->condition_decode[calculate_array_location( > + drvdata, i, TGU_CONDITION_DECODE, j)] = > + TGU_CONDITION_DECODE_NOT; Then, we could simplify the above to: for (i = 0; i < TGU_CONDITION_DECODE(drvdata); i++) vt->condition_decode[i] = TGU_CONDITION_DECODE_NOT; Same for everything below. Suzuki > + } > + > + if (vt->condition_select) { > + size_t bytes = drvdata->num_condition_select * > + drvdata->num_step * sizeof(unsigned int); > + memset(vt->condition_select, 0, bytes); > + } > + > + if (vt->timer) { > + size_t bytes = drvdata->num_step * drvdata->num_timer * > + sizeof(unsigned int); > + memset(vt->timer, 0, bytes); > + } > + > + if (vt->counter) { > + size_t bytes = drvdata->num_step * drvdata->num_counter * > + sizeof(unsigned int); > + memset(vt->counter, 0, bytes); > + } > + > + /* If the TGU is running, apply the cleared configuration immediately. */ > + if (drvdata->enabled) { > + ret = tgu_write_all_hw_regs(drvdata); > + if (ret) > + return ret; > + } > + > + dev_dbg(dev, "Qualcomm-TGU reset complete\n"); > + > + return size; > +} > +static DEVICE_ATTR_WO(reset_tgu); > + > static struct attribute *tgu_common_attrs[] = { > &dev_attr_enable_tgu.attr, > + &dev_attr_reset_tgu.attr, > NULL, > }; >

3 days, 20 hours

Re: [PATCH v16 3/7] qcom-tgu: Add signal priority support

by Suzuki K Poulose

On 30/07/2026 09:07, Songwei Chai wrote: > Like circuit of a Logic analyzer, in TGU, the requirement could be > configured in each step and the trigger will be created once the > requirements are met. Add priority functionality here to sort the > signals into different priorities. The signal which is wanted could > be configured in each step's priority node, the larger number means > the higher priority and the signal with higher priority will be sensed > more preferentially. > > Reviewed-by: Jie Gan <jie.gan(a)oss.qualcomm.com> > Signed-off-by: Songwei Chai <songwei.chai(a)oss.qualcomm.com> > --- > .../ABI/testing/sysfs-bus-amba-devices-tgu | 7 + > drivers/hwtracing/qcom/tgu.c | 189 +++++++++++++++++- > drivers/hwtracing/qcom/tgu.h | 133 ++++++++++++ > 3 files changed, 328 insertions(+), 1 deletion(-) > > diff --git a/Documentation/ABI/testing/sysfs-bus-amba-devices-tgu b/Documentation/ABI/testing/sysfs-bus-amba-devices-tgu > index 36ab64f1e301..7dd24314592d 100644 > --- a/Documentation/ABI/testing/sysfs-bus-amba-devices-tgu > +++ b/Documentation/ABI/testing/sysfs-bus-amba-devices-tgu > @@ -7,3 +7,10 @@ Description: > Accepts only one of the 2 values - 0 or 1. > 0 : disable TGU. > 1 : enable TGU. > + > +What: /sys/bus/amba/devices/<tgu-name>/step[0:7]_priority[0:3]/reg[0:17] > +Date: August 2026 > +KernelVersion: 7.2 > +Contact: Jinlong Mao <jinlong.mao(a)oss.qualcomm.com>, Songwei Chai <songwei.chai(a)oss.qualcomm.com> > +Description: > + (RW) Set/Get the sensed signal with specific step and priority for TGU. > diff --git a/drivers/hwtracing/qcom/tgu.c b/drivers/hwtracing/qcom/tgu.c > index 9c7dd4a09f02..0e88b437f191 100644 > --- a/drivers/hwtracing/qcom/tgu.c > +++ b/drivers/hwtracing/qcom/tgu.c > @@ -15,14 +15,144 @@ > > #include "tgu.h" > > +static int calculate_array_location(struct tgu_drvdata *drvdata, > + int step_index, int operation_index, > + int reg_index) > +{ > + return operation_index * (drvdata->num_step) * (drvdata->num_reg) + > + step_index * (drvdata->num_reg) + reg_index; > +} > + > +static ssize_t tgu_dataset_show(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + struct tgu_drvdata *drvdata = dev_get_drvdata(dev); > + struct tgu_attribute *tgu_attr = > + container_of(attr, struct tgu_attribute, attr); > + int index; > + > + index = calculate_array_location(drvdata, tgu_attr->step_index, > + tgu_attr->operation_index, > + tgu_attr->reg_num); > + > + /* > + * Serialise against tgu_dataset_store() and tgu_enable(), which read > + * and write the same priority array, so the value is not observed > + * while being updated concurrently. > + */ > + guard(mutex)(&drvdata->lock); > + > + return sysfs_emit(buf, "0x%x\n", > + drvdata->value_table->priority[index]); > +} > + > +static ssize_t tgu_dataset_store(struct device *dev, > + struct device_attribute *attr, > + const char *buf, size_t size) > +{ > + struct tgu_drvdata *tgu_drvdata = dev_get_drvdata(dev); > + struct tgu_attribute *tgu_attr = > + container_of(attr, struct tgu_attribute, attr); > + unsigned long val; > + int index; > + int ret; > + > + ret = kstrtoul(buf, 0, &val); > + if (ret) > + return ret; > + > + guard(mutex)(&tgu_drvdata->lock); > + index = calculate_array_location(tgu_drvdata, tgu_attr->step_index, > + tgu_attr->operation_index, > + tgu_attr->reg_num); > + > + tgu_drvdata->value_table->priority[index] = val; > + > + return size; > +} > + > +static umode_t tgu_node_visible(struct kobject *kobject, > + struct attribute *attr, > + int n) > +{ > + struct device *dev = kobj_to_dev(kobject); > + struct tgu_drvdata *drvdata = dev_get_drvdata(dev); > + struct device_attribute *dev_attr = > + container_of(attr, struct device_attribute, attr); > + struct tgu_attribute *tgu_attr = > + container_of(dev_attr, struct tgu_attribute, attr); > + > + if (tgu_attr->step_index >= drvdata->num_step) > + return SYSFS_GROUP_INVISIBLE; > + > + if (tgu_attr->reg_num >= drvdata->num_reg) > + return 0; > + > + return attr->mode; > +} > + > static void tgu_write_all_hw_regs(struct tgu_drvdata *drvdata) > { > + int i, j, k, index; > + > TGU_UNLOCK(drvdata->base); > + for (i = 0; i < drvdata->num_step; i++) { > + for (j = 0; j < MAX_PRIORITY; j++) { > + for (k = 0; k < drvdata->num_reg; k++) { > + index = calculate_array_location( > + drvdata, i, j, k); > + > + writel(drvdata->value_table->priority[index], > + drvdata->base + > + PRIORITY_REG_STEP(i, j, k)); > + } > + } > + } > /* Enable TGU to program the triggers */ > writel(1, drvdata->base + TGU_CONTROL); > TGU_LOCK(drvdata->base); > } > > +static void tgu_set_reg_number(struct tgu_drvdata *drvdata) > +{ > + int num_sense_input; > + int num_reg; > + u32 devid; > + > + devid = readl(drvdata->base + TGU_DEVID); > + > + num_sense_input = TGU_DEVID_SENSE_INPUT(devid); > + num_reg = (num_sense_input * TGU_BITS_PER_SIGNAL) / LENGTH_REGISTER; > + > + if ((num_sense_input * TGU_BITS_PER_SIGNAL) % LENGTH_REGISTER) > + num_reg++; > + > + if (num_reg > TGU_MAX_PRIORITY_REGS) { > + dev_warn(drvdata->dev, > + "DEVID reports %d priority regs, clamping to %d\n", > + num_reg, TGU_MAX_PRIORITY_REGS); > + num_reg = TGU_MAX_PRIORITY_REGS; > + } > + > + drvdata->num_reg = num_reg; > +} > + > +static void tgu_set_steps(struct tgu_drvdata *drvdata) > +{ > + u32 devid; > + > + devid = readl(drvdata->base + TGU_DEVID); > + > + drvdata->num_step = TGU_DEVID_STEPS(devid); > + > + if (drvdata->num_step > TGU_MAX_STEPS) { > + dev_warn(drvdata->dev, > + "DEVID reports %d steps, clamping to %d\n", > + drvdata->num_step, TGU_MAX_STEPS); > + drvdata->num_step = TGU_MAX_STEPS; > + } > +} Why not combine setting the num_step and num_regs in one go ? You are reading the DEVID anyways, and such macro level functional split is useless. > + > static void tgu_enable(struct tgu_drvdata *drvdata) > { > tgu_write_all_hw_regs(drvdata); > @@ -104,6 +234,38 @@ static const struct attribute_group tgu_common_grp = { > > static const struct attribute_group *tgu_attr_groups[] = { > &tgu_common_grp, > + PRIORITY_ATTRIBUTE_GROUP_INIT(0, 0), > + PRIORITY_ATTRIBUTE_GROUP_INIT(0, 1), > + PRIORITY_ATTRIBUTE_GROUP_INIT(0, 2), > + PRIORITY_ATTRIBUTE_GROUP_INIT(0, 3), > + PRIORITY_ATTRIBUTE_GROUP_INIT(1, 0), > + PRIORITY_ATTRIBUTE_GROUP_INIT(1, 1), > + PRIORITY_ATTRIBUTE_GROUP_INIT(1, 2), > + PRIORITY_ATTRIBUTE_GROUP_INIT(1, 3), > + PRIORITY_ATTRIBUTE_GROUP_INIT(2, 0), > + PRIORITY_ATTRIBUTE_GROUP_INIT(2, 1), > + PRIORITY_ATTRIBUTE_GROUP_INIT(2, 2), > + PRIORITY_ATTRIBUTE_GROUP_INIT(2, 3), > + PRIORITY_ATTRIBUTE_GROUP_INIT(3, 0), > + PRIORITY_ATTRIBUTE_GROUP_INIT(3, 1), > + PRIORITY_ATTRIBUTE_GROUP_INIT(3, 2), > + PRIORITY_ATTRIBUTE_GROUP_INIT(3, 3), > + PRIORITY_ATTRIBUTE_GROUP_INIT(4, 0), > + PRIORITY_ATTRIBUTE_GROUP_INIT(4, 1), > + PRIORITY_ATTRIBUTE_GROUP_INIT(4, 2), > + PRIORITY_ATTRIBUTE_GROUP_INIT(4, 3), > + PRIORITY_ATTRIBUTE_GROUP_INIT(5, 0), > + PRIORITY_ATTRIBUTE_GROUP_INIT(5, 1), > + PRIORITY_ATTRIBUTE_GROUP_INIT(5, 2), > + PRIORITY_ATTRIBUTE_GROUP_INIT(5, 3), > + PRIORITY_ATTRIBUTE_GROUP_INIT(6, 0), > + PRIORITY_ATTRIBUTE_GROUP_INIT(6, 1), > + PRIORITY_ATTRIBUTE_GROUP_INIT(6, 2), > + PRIORITY_ATTRIBUTE_GROUP_INIT(6, 3), > + PRIORITY_ATTRIBUTE_GROUP_INIT(7, 0), > + PRIORITY_ATTRIBUTE_GROUP_INIT(7, 1), > + PRIORITY_ATTRIBUTE_GROUP_INIT(7, 2), > + PRIORITY_ATTRIBUTE_GROUP_INIT(7, 3), > NULL, > }; > > @@ -111,6 +273,8 @@ static int tgu_probe(struct amba_device *adev, const struct amba_id *id) > { > struct device *dev = &adev->dev; > struct tgu_drvdata *drvdata; > + unsigned int *priority; > + size_t priority_size; > int ret; > > drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL); > @@ -128,8 +292,31 @@ static int tgu_probe(struct amba_device *adev, const struct amba_id *id) > if (ret) > return ret; > > - drvdata->enabled = false; > + tgu_set_reg_number(drvdata); > + tgu_set_steps(drvdata); As mentioned above, combine both ? > + > + drvdata->value_table = > + devm_kzalloc(dev, sizeof(*drvdata->value_table), GFP_KERNEL); Could we not embed this in the drvdata without dynamic allocation ? > + if (!drvdata->value_table) > + return -ENOMEM; > > + priority_size = MAX_PRIORITY * drvdata->num_reg * drvdata->num_step; > + > + priority = devm_kcalloc(dev, priority_size, > + sizeof(*drvdata->value_table->priority), > + GFP_KERNEL); > + if (!priority) > + return -ENOMEM; > + > + drvdata->value_table->priority = priority; > + > + drvdata->enabled = false; > + /* > + * Create the sysfs groups only after the backing storage is fully > + * allocated, otherwise userspace could open a priority attribute and > + * have tgu_dataset_show() dereference an uninitialised value_table. > + * This also keeps the error paths above free of sysfs teardown. > + */ > ret = sysfs_create_groups(&dev->kobj, tgu_attr_groups); > if (ret) { > dev_err(dev, "failed to create sysfs groups: %d\n", ret); > diff --git a/drivers/hwtracing/qcom/tgu.h b/drivers/hwtracing/qcom/tgu.h > index ebba49402a20..7a3e1161c4cf 100644 > --- a/drivers/hwtracing/qcom/tgu.h > +++ b/drivers/hwtracing/qcom/tgu.h > @@ -6,10 +6,137 @@ > #ifndef _QCOM_TGU_H > #define _QCOM_TGU_H > > +#include <linux/bitfield.h> > +#include <linux/bits.h> > + > /* Register addresses */ > #define TGU_CONTROL 0x0000 > #define TGU_LAR 0xfb0 > #define TGU_UNLOCK_OFFSET 0xc5acce55 > +#define TGU_DEVID 0xfc8 > + > +#define TGU_DEVID_SENSE_INPUT(devid_val) \ > + ((int)FIELD_GET(GENMASK(17, 10), devid_val)) > +#define TGU_DEVID_STEPS(devid_val) \ > + ((int)FIELD_GET(GENMASK(6, 3), devid_val)) > +#define TGU_BITS_PER_SIGNAL 4 > +#define LENGTH_REGISTER 32 > + > +/* > + * TGU configuration space Step configuration > + * offset table space layout > + * x-------------------------x$ x-------------x$ > + * | |$ | |$ > + * | | | reserve |$ > + * | | | |$ > + * |coresight management | |-------------|base+n*0x1D8+0x1F4$ > + * | registers | |---> |priority[3] |$ > + * | | | |-------------|base+n*0x1D8+0x194$ > + * | | | |priority[2] |$ > + * |-------------------------| | |-------------|base+n*0x1D8+0x134$ > + * | | | |priority[1] |$ > + * | step[7] | | |-------------|base+n*0x1D8+0xD4$ > + * |-------------------------|->base+0x40+7*0x1D8 | |priority[0] |$ > + * | | | |-------------|base+n*0x1D8+0x74$ > + * | ... | | | condition |$ > + * | | | | select |$ > + * |-------------------------|->base+0x40+1*0x1D8 | |-------------|base+n*0x1D8+0x60$ minor nit: ^^^ s/base+0x40+1*0x1D8/step[0] + 0x1D8 and so on, for the above > + * | | | | condition |$ > + * | step[0] |--------------------> | decode |$ > + * |-------------------------|-> base+0x40 |-------------|base+n*0x1D8+0x50$ > + * | | | |$ > + * | Control and status space| |Timer/Counter|$ > + * | space | | |$ > + * x-------------------------x->base x-------------x base+n*0x1D8+0x40$ Also for each frame, you don't have to describe "base+n*..." that makes it unnecessarily crowded and complicated to read. > + * > + */ > +#define STEP_OFFSET 0x1D8 > +#define PRIORITY_START_OFFSET 0x0074 > +#define PRIORITY_OFFSET 0x60 From the diagram, it is CONDITION_SELECT ? Having two offsets named after PRIORITY is confusing. > +#define REG_OFFSET 0x4 > + > +/* > + * Upper bounds on the DEVID/DEVID2-derived per-region counts. The TGU register > + * layout is fixed by the offset macros above and mirrored by the static sysfs > + * attribute tables in tgu.c, so these are the largest counts the driver can > + * both address within the mapped window and expose to userspace: > + * > + * TGU_MAX_STEPS static tables define step0..step7 > + * TGU_MAX_PRIORITY_REGS STEP_PRIORITY_LIST defines reg0..reg17 What are these ? PRIORITY regs is 0-3 ? Do you mean each priority has 0-17 registers ? (See, the description using base+n*.. for the "step" frame is making this worse). You may move it down as a separate figure. From my maths: For pritority[0] -> 0xD4-0x74 = 0x60 => 0x18 => 24 registers! not 18! What am I missing ? > + * > + * A device whose DEVID reports counts above these bounds is clamped, so writes > + * never fall outside the ioremap()'d window or spill into an adjacent region. > + */ > + > +#define TGU_MAX_STEPS 8 > +#define TGU_MAX_PRIORITY_REGS 18 > + > +/* Calculate compare step addresses */ > +#define PRIORITY_REG_STEP(step, priority, reg)\ > + (PRIORITY_START_OFFSET + PRIORITY_OFFSET * priority +\ > + REG_OFFSET * reg + STEP_OFFSET * step) > + > +#define tgu_dataset_rw(name, step_index, type, reg_num) \ > + (&((struct tgu_attribute[]){ { \ > + __ATTR(name, 0644, tgu_dataset_show, tgu_dataset_store), \ > + step_index, \ > + type, \ > + reg_num, \ > + } })[0].attr.attr) > + > +#define STEP_PRIORITY(step_index, reg_num, priority) \ > + tgu_dataset_rw(reg##reg_num, step_index, TGU_PRIORITY##priority, \ > + reg_num) > + > +#define STEP_PRIORITY_LIST(step_index, priority) \ > + {STEP_PRIORITY(step_index, 0, priority), \ > + STEP_PRIORITY(step_index, 1, priority), \ > + STEP_PRIORITY(step_index, 2, priority), \ > + STEP_PRIORITY(step_index, 3, priority), \ > + STEP_PRIORITY(step_index, 4, priority), \ > + STEP_PRIORITY(step_index, 5, priority), \ > + STEP_PRIORITY(step_index, 6, priority), \ > + STEP_PRIORITY(step_index, 7, priority), \ > + STEP_PRIORITY(step_index, 8, priority), \ > + STEP_PRIORITY(step_index, 9, priority), \ > + STEP_PRIORITY(step_index, 10, priority), \ > + STEP_PRIORITY(step_index, 11, priority), \ > + STEP_PRIORITY(step_index, 12, priority), \ > + STEP_PRIORITY(step_index, 13, priority), \ > + STEP_PRIORITY(step_index, 14, priority), \ > + STEP_PRIORITY(step_index, 15, priority), \ > + STEP_PRIORITY(step_index, 16, priority), \ > + STEP_PRIORITY(step_index, 17, priority), \ > + NULL \ > + } > + > +#define PRIORITY_ATTRIBUTE_GROUP_INIT(step, priority)\ > + (&(const struct attribute_group){\ > + .attrs = (struct attribute*[])STEP_PRIORITY_LIST(step, priority),\ > + .is_visible = tgu_node_visible,\ > + .name = "step" #step "_priority" #priority \ > + }) Please could you move these macro magic to the .c file ? 1. Nobody else reuses them ? 2. It is really difficult to go back and forth to check what each macro means, given how complex it is. > + > +enum operation_index { why are we calling it operation_index ? > + TGU_PRIORITY0, > + TGU_PRIORITY1, > + TGU_PRIORITY2, > + TGU_PRIORITY3, > +}; > + > +/* Maximum priority that TGU supports */ > +#define MAX_PRIORITY 4 > + > +struct tgu_attribute { > + struct device_attribute attr; > + u32 step_index; > + enum operation_index operation_index; u8 priority ? > + u32 reg_num; u8 reg_num ? > +}; > + > +struct value_table { > + unsigned int *priority; > +}; > > static inline void TGU_LOCK(void __iomem *addr) > { > @@ -36,6 +163,9 @@ static inline void TGU_UNLOCK(void __iomem *addr) > * @lock: Mutex serialising enable/disable and the associated PM runtime > * reference counting against concurrent sysfs access > * @enabled: Flag indicating whether the TGU device is enabled > + * @value_table: Store given value based on relevant parameters > + * @num_reg: Maximum number of registers > + * @num_step: Maximum step size > * > * This structure defines the data associated with a TGU device, > * including its base address, device pointer, the lock serialising > @@ -46,6 +176,9 @@ struct tgu_drvdata { > struct device *dev; > struct mutex lock; > bool enabled; > + struct value_table *value_table; Why this ? why not struct value_table value_table ? Surely, we seem to be dynamically allocating the priorities etc from there. But do we need dynamic allocation for value_table field as such ? Suzuki > + int num_reg; > + int num_step; > }; > > #endif

3 days, 20 hours

Jump to page:

2026

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

CoreSight