* [PATCH v10 0/7] Provide support for Trigger Generation Unit
@ 2026-01-09 2:11 Songwei Chai
2026-01-09 2:11 ` [PATCH v10 1/7] dt-bindings: arm: Add support for Qualcomm TGU trace Songwei Chai
` (7 more replies)
0 siblings, 8 replies; 29+ messages in thread
From: Songwei Chai @ 2026-01-09 2:11 UTC (permalink / raw)
To: andersson, alexander.shishkin, mike.leach, suzuki.poulose,
james.clark, krzk+dt, conor+dt
Cc: Songwei Chai, linux-kernel, linux-arm-kernel, linux-arm-msm,
coresight, devicetree, gregkh
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@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 V9: https://lore.kernel.org/all/20251219065902.2296896-1-songwei.chai@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@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@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@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@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@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@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@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@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 | 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 | 709 ++++++++++++++++++
drivers/hwtracing/qcom/tgu.h | 272 +++++++
8 files changed, 1148 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.34.1
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v10 1/7] dt-bindings: arm: Add support for Qualcomm TGU trace
2026-01-09 2:11 [PATCH v10 0/7] Provide support for Trigger Generation Unit Songwei Chai
@ 2026-01-09 2:11 ` Songwei Chai
2026-01-09 2:11 ` [PATCH v10 2/7] qcom-tgu: Add TGU driver Songwei Chai
` (6 subsequent siblings)
7 siblings, 0 replies; 29+ messages in thread
From: Songwei Chai @ 2026-01-09 2:11 UTC (permalink / raw)
To: andersson, alexander.shishkin, mike.leach, suzuki.poulose,
james.clark, krzk+dt, conor+dt
Cc: Songwei Chai, linux-kernel, linux-arm-kernel, linux-arm-msm,
coresight, devicetree, gregkh, Rob Herring
The Trigger Generation Unit (TGU) is designed to detect patterns or
sequences within a specific region of the System on Chip (SoC). Once
configured and activated, it monitors sense inputs and can detect a
pre-programmed state or sequence across clock cycles, subsequently
producing a trigger.
TGU configuration space
offset table
x-------------------------x
| |
| |
| | Step configuration
| | space layout
| coresight management | x-------------x
| registers | |---> | |
| | | | reserve |
| | | | |
|-------------------------| | |-------------|
| | | | priority[3] |
| step[7] |<-- | |-------------|
|-------------------------| | | | priority[2] |
| | | | |-------------|
| ... | |Steps region | | priority[1] |
| | | | |-------------|
|-------------------------| | | | priority[0] |
| |<-- | |-------------|
| step[0] |--------------------> | |
|-------------------------| | condition |
| | | |
| control and status | x-------------x
| space | | |
x-------------------------x |Timer/Counter|
| |
x-------------x
TGU Configuration in Hardware
The TGU provides a step region for user configuration, similar
to a flow chart. Each step region consists of three register clusters:
1.Priority Region: Sets the required signals with priority.
2.Condition Region: Defines specific requirements (e.g., signal A
reaches three times) and the subsequent action once the requirement is
met.
3.Timer/Counter (Optional): Provides timing or counting functionality.
Add a new tgu.yaml file to describe the bindings required to
define the TGU in the device trees.
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
Signed-off-by: Songwei Chai <songwei.chai@oss.qualcomm.com>
---
.../devicetree/bindings/arm/qcom,tgu.yaml | 92 +++++++++++++++++++
1 file changed, 92 insertions(+)
create mode 100644 Documentation/devicetree/bindings/arm/qcom,tgu.yaml
diff --git a/Documentation/devicetree/bindings/arm/qcom,tgu.yaml b/Documentation/devicetree/bindings/arm/qcom,tgu.yaml
new file mode 100644
index 000000000000..5b6a58ebe691
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/qcom,tgu.yaml
@@ -0,0 +1,92 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+# Copyright (c) 2025 Qualcomm Innovation Center, Inc. All rights reserved.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/arm/qcom,tgu.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Trigger Generation Unit - TGU
+
+description: |
+ The Trigger Generation Unit (TGU) is a Data Engine which can be utilized
+ to sense a plurality of signals and create a trigger into the CTI or
+ generate interrupts to processors. The TGU is like the trigger circuit
+ of a Logic Analyzer. The corresponding trigger logic can be realized by
+ configuring the conditions for each step after sensing the signal.
+ Once setup and enabled, it will observe sense inputs and based upon
+ the activity of those inputs, even over clock cycles, may detect a
+ preprogrammed state/sequence and then produce a trigger or interrupt.
+
+ The primary use case of the TGU is to detect patterns or sequences on a
+ given set of signals within some region to identify the issue in time
+ once there is abnormal behavior in the subsystem.
+
+maintainers:
+ - Mao Jinlong <jinlong.mao@oss.qualcomm.com>
+ - Songwei Chai <songwei.chai@oss.qualcomm.com>
+
+# Need a custom select here or 'arm,primecell' will match on lots of nodes
+select:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - qcom,tgu
+ required:
+ - compatible
+
+properties:
+ compatible:
+ items:
+ - const: qcom,tgu
+ - const: arm,primecell
+
+ reg:
+ maxItems: 1
+
+ clocks:
+ maxItems: 1
+
+ clock-names:
+ items:
+ - const: apb_pclk
+
+ in-ports:
+ $ref: /schemas/graph.yaml#/properties/ports
+ additionalProperties: false
+
+ properties:
+ port:
+ description:
+ The port mechanism here ensures the relationship between TGU and
+ TPDM, as TPDM is one of the inputs for TGU. It will allow TGU to
+ function as TPDM's helper and enable TGU when the connected
+ TPDM is enabled.
+ $ref: /schemas/graph.yaml#/properties/port
+
+required:
+ - compatible
+ - reg
+ - clocks
+ - clock-names
+
+additionalProperties: false
+
+examples:
+ - |
+ tgu@10b0e000 {
+ compatible = "qcom,tgu", "arm,primecell";
+ reg = <0x10b0e000 0x1000>;
+
+ clocks = <&aoss_qmp>;
+ clock-names = "apb_pclk";
+
+ in-ports {
+ port {
+ tgu_in_tpdm_swao: endpoint{
+ remote-endpoint = <&tpdm_swao_out_tgu>;
+ };
+ };
+ };
+ };
+...
--
2.34.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v10 2/7] qcom-tgu: Add TGU driver
2026-01-09 2:11 [PATCH v10 0/7] Provide support for Trigger Generation Unit Songwei Chai
2026-01-09 2:11 ` [PATCH v10 1/7] dt-bindings: arm: Add support for Qualcomm TGU trace Songwei Chai
@ 2026-01-09 2:11 ` Songwei Chai
2026-01-09 10:27 ` Suzuki K Poulose
` (2 more replies)
2026-01-09 2:11 ` [PATCH v10 3/7] qcom-tgu: Add signal priority support Songwei Chai
` (5 subsequent siblings)
7 siblings, 3 replies; 29+ messages in thread
From: Songwei Chai @ 2026-01-09 2:11 UTC (permalink / raw)
To: andersson, alexander.shishkin, mike.leach, suzuki.poulose,
james.clark, krzk+dt, conor+dt
Cc: Songwei Chai, linux-kernel, linux-arm-kernel, linux-arm-msm,
coresight, devicetree, gregkh
Add driver to support device TGU (Trigger Generation Unit).
TGU is a Data Engine which can be utilized to sense a plurality of
signals and create a trigger into the CTI or generate interrupts to
processors. Add probe/enable/disable functions for tgu.
Signed-off-by: Songwei Chai <songwei.chai@oss.qualcomm.com>
---
.../ABI/testing/sysfs-bus-amba-devices-tgu | 9 +
drivers/Makefile | 1 +
drivers/hwtracing/Kconfig | 2 +
drivers/hwtracing/qcom/Kconfig | 18 ++
drivers/hwtracing/qcom/Makefile | 3 +
drivers/hwtracing/qcom/tgu.c | 176 ++++++++++++++++++
drivers/hwtracing/qcom/tgu.h | 51 +++++
7 files changed, 260 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-bus-amba-devices-tgu
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
diff --git a/Documentation/ABI/testing/sysfs-bus-amba-devices-tgu b/Documentation/ABI/testing/sysfs-bus-amba-devices-tgu
new file mode 100644
index 000000000000..56ec3f5ab5d6
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-amba-devices-tgu
@@ -0,0 +1,9 @@
+What: /sys/bus/amba/devices/<tgu-name>/enable_tgu
+Date: January 2026
+KernelVersion 6.19
+Contact: Jinlong Mao <jinlong.mao@oss.qualcomm.com>, Songwei Chai <songwei.chai@oss.qualcomm.com>
+Description:
+ (RW) Set/Get the enable/disable status of TGU
+ Accepts only one of the 2 values - 0 or 1.
+ 0 : disable TGU.
+ 1 : enable TGU.
diff --git a/drivers/Makefile b/drivers/Makefile
index ccc05f1eae3e..9608a3debb1f 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -177,6 +177,7 @@ obj-$(CONFIG_RAS) += ras/
obj-$(CONFIG_USB4) += thunderbolt/
obj-$(CONFIG_CORESIGHT) += hwtracing/coresight/
obj-y += hwtracing/intel_th/
+obj-y += hwtracing/qcom/
obj-$(CONFIG_STM) += hwtracing/stm/
obj-$(CONFIG_HISI_PTT) += hwtracing/ptt/
obj-y += android/
diff --git a/drivers/hwtracing/Kconfig b/drivers/hwtracing/Kconfig
index 911ee977103c..8a640218eed8 100644
--- a/drivers/hwtracing/Kconfig
+++ b/drivers/hwtracing/Kconfig
@@ -7,4 +7,6 @@ source "drivers/hwtracing/intel_th/Kconfig"
source "drivers/hwtracing/ptt/Kconfig"
+source "drivers/hwtracing/qcom/Kconfig"
+
endmenu
diff --git a/drivers/hwtracing/qcom/Kconfig b/drivers/hwtracing/qcom/Kconfig
new file mode 100644
index 000000000000..d6f6d4b0f28e
--- /dev/null
+++ b/drivers/hwtracing/qcom/Kconfig
@@ -0,0 +1,18 @@
+# SPDX-License-Identifier: GPL-2.0-only
+#
+# QCOM specific hwtracing drivers
+#
+menu "Qualcomm specific hwtracing drivers"
+
+config QCOM_TGU
+ tristate "QCOM Trigger Generation Unit driver"
+ help
+ This driver provides support for Trigger Generation Unit that is
+ used to detect patterns or sequences on a given set of signals.
+ TGU is used to monitor a particular bus within a given region to
+ detect illegal transaction sequences or slave responses. It is also
+ used to monitor a data stream to detect protocol violations and to
+ provide a trigger point for centering data around a specific event
+ within the trace data buffer.
+
+endmenu
diff --git a/drivers/hwtracing/qcom/Makefile b/drivers/hwtracing/qcom/Makefile
new file mode 100644
index 000000000000..5a0a868c1ea0
--- /dev/null
+++ b/drivers/hwtracing/qcom/Makefile
@@ -0,0 +1,3 @@
+# SPDX-License-Identifier: GPL-2.0
+
+obj-$(CONFIG_QCOM_TGU) += tgu.o
diff --git a/drivers/hwtracing/qcom/tgu.c b/drivers/hwtracing/qcom/tgu.c
new file mode 100644
index 000000000000..c5b2b384e6ae
--- /dev/null
+++ b/drivers/hwtracing/qcom/tgu.c
@@ -0,0 +1,176 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
+ */
+
+#include <linux/amba/bus.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/pm_runtime.h>
+
+#include "tgu.h"
+
+static void tgu_write_all_hw_regs(struct tgu_drvdata *drvdata)
+{
+ TGU_UNLOCK(drvdata->base);
+ /* Enable TGU to program the triggers */
+ writel(1, drvdata->base + TGU_CONTROL);
+ TGU_LOCK(drvdata->base);
+}
+
+static int tgu_enable(struct device *dev)
+{
+ struct tgu_drvdata *drvdata = dev_get_drvdata(dev);
+
+ guard(spinlock)(&drvdata->lock);
+ if (drvdata->enable)
+ return -EBUSY;
+
+ tgu_write_all_hw_regs(drvdata);
+ drvdata->enable = true;
+
+ return 0;
+}
+
+static void tgu_disable(struct device *dev)
+{
+ struct tgu_drvdata *drvdata = dev_get_drvdata(dev);
+
+ guard(spinlock)(&drvdata->lock);
+ if (drvdata->enable) {
+ TGU_UNLOCK(drvdata->base);
+ writel(0, drvdata->base + TGU_CONTROL);
+ TGU_LOCK(drvdata->base);
+
+ drvdata->enable = false;
+ }
+}
+
+static ssize_t enable_tgu_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct tgu_drvdata *drvdata = dev_get_drvdata(dev);
+ bool enabled;
+
+ guard(spinlock)(&drvdata->lock);
+ enabled = drvdata->enable;
+
+ return sysfs_emit(buf, "%d\n", enabled ? 1 : 0);
+}
+
+/* enable_tgu_store - Configure Trace and Gating Unit (TGU) triggers. */
+static ssize_t enable_tgu_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf,
+ size_t size)
+{
+ unsigned long val;
+ int ret = 0;
+
+ ret = kstrtoul(buf, 0, &val);
+ if (ret)
+ return ret;
+
+ if (val) {
+ ret = pm_runtime_resume_and_get(dev);
+ if (ret)
+ return ret;
+ ret = tgu_enable(dev);
+ if (ret) {
+ pm_runtime_put(dev);
+ return ret;
+ }
+ } else {
+ tgu_disable(dev);
+ pm_runtime_put(dev);
+ }
+
+ return size;
+}
+static DEVICE_ATTR_RW(enable_tgu);
+
+static struct attribute *tgu_common_attrs[] = {
+ &dev_attr_enable_tgu.attr,
+ NULL,
+};
+
+static const struct attribute_group tgu_common_grp = {
+ .attrs = tgu_common_attrs,
+ NULL,
+};
+
+static const struct attribute_group *tgu_attr_groups[] = {
+ &tgu_common_grp,
+ NULL,
+};
+
+static int tgu_probe(struct amba_device *adev, const struct amba_id *id)
+{
+ struct device *dev = &adev->dev;
+ struct tgu_drvdata *drvdata;
+ int ret;
+
+ drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
+ if (!drvdata)
+ return -ENOMEM;
+
+ drvdata->dev = &adev->dev;
+ dev_set_drvdata(dev, drvdata);
+
+ drvdata->base = devm_ioremap_resource(dev, &adev->res);
+ if (IS_ERR(drvdata->base))
+ return PTR_ERR(drvdata->base);
+
+ spin_lock_init(&drvdata->lock);
+
+ ret = sysfs_create_groups(&dev->kobj, tgu_attr_groups);
+ if (ret) {
+ dev_err(dev, "failed to create sysfs groups: %d\n", ret);
+ return ret;
+ }
+
+ drvdata->enable = false;
+
+ pm_runtime_put(&adev->dev);
+ return 0;
+}
+
+static void tgu_remove(struct amba_device *adev)
+{
+ struct device *dev = &adev->dev;
+
+ sysfs_remove_groups(&dev->kobj, tgu_attr_groups);
+
+ tgu_disable(dev);
+}
+
+static const struct amba_id tgu_ids[] = {
+ {
+ .id = 0x000f0e00,
+ .mask = 0x000fffff,
+ },
+ { 0, 0, NULL },
+};
+
+MODULE_DEVICE_TABLE(amba, tgu_ids);
+
+static struct amba_driver tgu_driver = {
+ .drv = {
+ .name = "qcom-tgu",
+ .suppress_bind_attrs = true,
+ },
+ .probe = tgu_probe,
+ .remove = tgu_remove,
+ .id_table = tgu_ids,
+};
+
+module_amba_driver(tgu_driver);
+
+MODULE_AUTHOR("Songwei Chai <songwei.chai@oss.qualcomm.com>");
+MODULE_AUTHOR("Jinlong Mao <jinlong.mao@oss.qualcomm.com>");
+MODULE_DESCRIPTION("Qualcomm Trigger Generation Unit driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/hwtracing/qcom/tgu.h b/drivers/hwtracing/qcom/tgu.h
new file mode 100644
index 000000000000..b11cfb28261d
--- /dev/null
+++ b/drivers/hwtracing/qcom/tgu.h
@@ -0,0 +1,51 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
+ */
+
+#ifndef _QCOM_TGU_H
+#define _QCOM_TGU_H
+
+/* Register addresses */
+#define TGU_CONTROL 0x0000
+#define TGU_LAR 0xfb0
+#define TGU_UNLOCK_OFFSET 0xc5acce55
+
+static inline void TGU_LOCK(void __iomem *addr)
+{
+ do {
+ /* Wait for things to settle */
+ mb();
+ writel_relaxed(0x0, addr + TGU_LAR);
+ } while (0);
+}
+
+static inline void TGU_UNLOCK(void __iomem *addr)
+{
+ do {
+ writel_relaxed(TGU_UNLOCK_OFFSET, addr + TGU_LAR);
+ /* Make sure everyone has seen this */
+ mb();
+ } while (0);
+}
+
+/**
+ * struct tgu_drvdata - Data structure for a TGU (Trigger Generator Unit)
+ * @base: Memory-mapped base address of the TGU device
+ * @dev: Pointer to the associated device structure
+ * @lock: Spinlock for handling concurrent access
+ * @enable: Flag indicating whether the TGU device is enabled
+ *
+ * This structure defines the data associated with a TGU device,
+ * including its base address, device pointers, clock, spinlock for
+ * synchronization, trigger data pointers, maximum limits for various
+ * trigger-related parameters, and enable status.
+ */
+struct tgu_drvdata {
+ void __iomem *base;
+ struct device *dev;
+ spinlock_t lock;
+ bool enable;
+};
+
+#endif
--
2.34.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v10 3/7] qcom-tgu: Add signal priority support
2026-01-09 2:11 [PATCH v10 0/7] Provide support for Trigger Generation Unit Songwei Chai
2026-01-09 2:11 ` [PATCH v10 1/7] dt-bindings: arm: Add support for Qualcomm TGU trace Songwei Chai
2026-01-09 2:11 ` [PATCH v10 2/7] qcom-tgu: Add TGU driver Songwei Chai
@ 2026-01-09 2:11 ` Songwei Chai
2026-01-13 11:09 ` Konrad Dybcio
2026-01-09 2:11 ` [PATCH v10 4/7] qcom-tgu: Add TGU decode support Songwei Chai
` (4 subsequent siblings)
7 siblings, 1 reply; 29+ messages in thread
From: Songwei Chai @ 2026-01-09 2:11 UTC (permalink / raw)
To: andersson, alexander.shishkin, mike.leach, suzuki.poulose,
james.clark, krzk+dt, conor+dt
Cc: Songwei Chai, linux-kernel, linux-arm-kernel, linux-arm-msm,
coresight, devicetree, gregkh
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.
Signed-off-by: Songwei Chai <songwei.chai@oss.qualcomm.com>
---
.../ABI/testing/sysfs-bus-amba-devices-tgu | 7 +
drivers/hwtracing/qcom/tgu.c | 160 ++++++++++++++++++
drivers/hwtracing/qcom/tgu.h | 113 +++++++++++++
3 files changed, 280 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-bus-amba-devices-tgu b/Documentation/ABI/testing/sysfs-bus-amba-devices-tgu
index 56ec3f5ab5d6..ec630e6ff2ee 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: January 2026
+KernelVersion 6.19
+Contact: Jinlong Mao <jinlong.mao@oss.qualcomm.com>, Songwei Chai <songwei.chai@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 c5b2b384e6ae..f8870766d624 100644
--- a/drivers/hwtracing/qcom/tgu.c
+++ b/drivers/hwtracing/qcom/tgu.c
@@ -14,14 +14,120 @@
#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->max_step) * (drvdata->max_reg) +
+ step_index * (drvdata->max_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);
+
+ 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;
+
+ ret = kstrtoul(buf, 0, &val);
+ if (ret)
+ return ret;
+
+ guard(spinlock)(&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);
+ int ret = SYSFS_GROUP_INVISIBLE;
+
+ if (tgu_attr->step_index < drvdata->max_step) {
+ ret = (tgu_attr->reg_num < drvdata->max_reg) ?
+ attr->mode : 0;
+ }
+ return ret;
+}
+
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->max_step; i++) {
+ for (j = 0; j < MAX_PRIORITY; j++) {
+ for (k = 0; k < drvdata->max_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);
+ if (((num_sense_input * NUMBER_BITS_EACH_SIGNAL) % LENGTH_REGISTER) == 0)
+ num_reg = (num_sense_input * NUMBER_BITS_EACH_SIGNAL) / LENGTH_REGISTER;
+ else
+ num_reg = ((num_sense_input * NUMBER_BITS_EACH_SIGNAL) / LENGTH_REGISTER) + 1;
+ drvdata->max_reg = num_reg;
+
+}
+
+static void tgu_set_steps(struct tgu_drvdata *drvdata)
+{
+ u32 devid;
+
+ devid = readl(drvdata->base + TGU_DEVID);
+
+ drvdata->max_step = TGU_DEVID_STEPS(devid);
+}
+
static int tgu_enable(struct device *dev)
{
struct tgu_drvdata *drvdata = dev_get_drvdata(dev);
@@ -105,6 +211,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,
};
@@ -112,6 +250,8 @@ static int tgu_probe(struct amba_device *adev, const struct amba_id *id)
{
struct device *dev = &adev->dev;
struct tgu_drvdata *drvdata;
+ size_t priority_size;
+ unsigned int *priority;
int ret;
drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
@@ -127,12 +267,32 @@ static int tgu_probe(struct amba_device *adev, const struct amba_id *id)
spin_lock_init(&drvdata->lock);
+ tgu_set_reg_number(drvdata);
+ tgu_set_steps(drvdata);
+
ret = sysfs_create_groups(&dev->kobj, tgu_attr_groups);
if (ret) {
dev_err(dev, "failed to create sysfs groups: %d\n", ret);
return ret;
}
+ drvdata->value_table =
+ devm_kzalloc(dev, sizeof(*drvdata->value_table), GFP_KERNEL);
+ if (!drvdata->value_table)
+ return -ENOMEM;
+
+ priority_size = MAX_PRIORITY * drvdata->max_reg *
+ drvdata->max_step *
+ sizeof(*(drvdata->value_table->priority));
+
+ priority = devm_kzalloc(dev, priority_size, GFP_KERNEL);
+
+ if (!priority)
+ return -ENOMEM;
+
+ drvdata->value_table->priority = priority;
+
+
drvdata->enable = false;
pm_runtime_put(&adev->dev);
diff --git a/drivers/hwtracing/qcom/tgu.h b/drivers/hwtracing/qcom/tgu.h
index b11cfb28261d..f54cea01e427 100644
--- a/drivers/hwtracing/qcom/tgu.h
+++ b/drivers/hwtracing/qcom/tgu.h
@@ -10,6 +10,113 @@
#define TGU_CONTROL 0x0000
#define TGU_LAR 0xfb0
#define TGU_UNLOCK_OFFSET 0xc5acce55
+#define TGU_DEVID 0xfc8
+
+#define BMVAL(val, lsb, msb) ((val & GENMASK(msb, lsb)) >> lsb)
+#define TGU_DEVID_SENSE_INPUT(devid_val) ((int) BMVAL(devid_val, 10, 17))
+#define TGU_DEVID_STEPS(devid_val) ((int)BMVAL(devid_val, 3, 6))
+#define NUMBER_BITS_EACH_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$
+ * | registe | |---> |prioroty[3] |$
+ * | | | |-------------|base+n*0x1D8+0x194$
+ * | | | |prioroty[2] |$
+ * |-------------------------| | |-------------|base+n*0x1D8+0x134$
+ * | | | |prioroty[1] |$
+ * | step[7] | | |-------------|base+n*0x1D8+0xD4$
+ * |-------------------------|->base+0x40+7*0x1D8 | |prioroty[0] |$
+ * | | | |-------------|base+n*0x1D8+0x74$
+ * | ... | | | condition |$
+ * | | | | select |$
+ * |-------------------------|->base+0x40+1*0x1D8 | |-------------|base+n*0x1D8+0x60$
+ * | | | | 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$
+ *
+ */
+#define STEP_OFFSET 0x1D8
+#define PRIORITY_START_OFFSET 0x0074
+#define PRIORITY_OFFSET 0x60
+#define REG_OFFSET 0x4
+
+/* 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 \
+ })
+
+enum 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;
+ u32 reg_num;
+};
+
+struct value_table {
+ unsigned int *priority;
+};
static inline void TGU_LOCK(void __iomem *addr)
{
@@ -35,6 +142,9 @@ static inline void TGU_UNLOCK(void __iomem *addr)
* @dev: Pointer to the associated device structure
* @lock: Spinlock for handling concurrent access
* @enable: Flag indicating whether the TGU device is enabled
+ * @value_table: Store given value based on relevant parameters.
+ * @max_reg: Maximum number of registers
+ * @max_step: Maximum step size
*
* This structure defines the data associated with a TGU device,
* including its base address, device pointers, clock, spinlock for
@@ -46,6 +156,9 @@ struct tgu_drvdata {
struct device *dev;
spinlock_t lock;
bool enable;
+ struct value_table *value_table;
+ int max_reg;
+ int max_step;
};
#endif
--
2.34.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v10 4/7] qcom-tgu: Add TGU decode support
2026-01-09 2:11 [PATCH v10 0/7] Provide support for Trigger Generation Unit Songwei Chai
` (2 preceding siblings ...)
2026-01-09 2:11 ` [PATCH v10 3/7] qcom-tgu: Add signal priority support Songwei Chai
@ 2026-01-09 2:11 ` Songwei Chai
2026-01-13 11:13 ` Konrad Dybcio
2026-01-09 2:11 ` [PATCH v10 5/7] qcom-tgu: Add support to configure next action Songwei Chai
` (3 subsequent siblings)
7 siblings, 1 reply; 29+ messages in thread
From: Songwei Chai @ 2026-01-09 2:11 UTC (permalink / raw)
To: andersson, alexander.shishkin, mike.leach, suzuki.poulose,
james.clark, krzk+dt, conor+dt
Cc: Songwei Chai, linux-kernel, linux-arm-kernel, linux-arm-msm,
coresight, devicetree, gregkh
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.
Signed-off-by: Songwei Chai <songwei.chai@oss.qualcomm.com>
---
.../ABI/testing/sysfs-bus-amba-devices-tgu | 7 +
drivers/hwtracing/qcom/tgu.c | 162 ++++++++++++++++--
drivers/hwtracing/qcom/tgu.h | 27 +++
3 files changed, 177 insertions(+), 19 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-bus-amba-devices-tgu b/Documentation/ABI/testing/sysfs-bus-amba-devices-tgu
index ec630e6ff2ee..4efa36a11d8e 100644
--- a/Documentation/ABI/testing/sysfs-bus-amba-devices-tgu
+++ b/Documentation/ABI/testing/sysfs-bus-amba-devices-tgu
@@ -14,3 +14,10 @@ KernelVersion 6.19
Contact: Jinlong Mao <jinlong.mao@oss.qualcomm.com>, Songwei Chai <songwei.chai@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: January 2026
+KernelVersion 6.19
+Contact: Jinlong Mao <jinlong.mao@oss.qualcomm.com>, Songwei Chai <songwei.chai@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 f8870766d624..39301d35ee9d 100644
--- a/drivers/hwtracing/qcom/tgu.c
+++ b/drivers/hwtracing/qcom/tgu.c
@@ -18,8 +18,36 @@ static int calculate_array_location(struct tgu_drvdata *drvdata,
int step_index, int operation_index,
int reg_index)
{
- return operation_index * (drvdata->max_step) * (drvdata->max_reg) +
- step_index * (drvdata->max_reg) + reg_index;
+ int ret = -EINVAL;
+
+ switch (operation_index) {
+ case TGU_PRIORITY0:
+ case TGU_PRIORITY1:
+ case TGU_PRIORITY2:
+ case TGU_PRIORITY3:
+ ret = operation_index * (drvdata->max_step) *
+ (drvdata->max_reg) +
+ step_index * (drvdata->max_reg) + reg_index;
+ break;
+ case TGU_CONDITION_DECODE:
+ ret = step_index * (drvdata->max_condition_decode) +
+ reg_index;
+ break;
+ default:
+ break;
+ }
+ return ret;
+}
+
+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, "%s - Fail\n", __func__);
+
+ return result;
}
static ssize_t tgu_dataset_show(struct device *dev,
@@ -30,12 +58,26 @@ 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);
-
- return sysfs_emit(buf, "0x%x\n",
- drvdata->value_table->priority[index]);
+ index = check_array_location(drvdata, tgu_attr->step_index,
+ tgu_attr->operation_index, tgu_attr->reg_num);
+
+ if (index == -EINVAL)
+ return 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,
@@ -47,18 +89,37 @@ static ssize_t tgu_dataset_store(struct device *dev,
container_of(attr, struct tgu_attribute, attr);
unsigned long val;
int index;
+ int ret;
ret = kstrtoul(buf, 0, &val);
if (ret)
return ret;
guard(spinlock)(&tgu_drvdata->lock);
- index = calculate_array_location(tgu_drvdata, tgu_attr->step_index,
+ index = check_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;
+ if (index == -EINVAL)
+ return -EINVAL;
+
+ 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 ret;
}
static umode_t tgu_node_visible(struct kobject *kobject,
@@ -74,13 +135,27 @@ static umode_t tgu_node_visible(struct kobject *kobject,
int ret = SYSFS_GROUP_INVISIBLE;
if (tgu_attr->step_index < drvdata->max_step) {
- ret = (tgu_attr->reg_num < drvdata->max_reg) ?
- attr->mode : 0;
+ switch (tgu_attr->operation_index) {
+ case TGU_PRIORITY0:
+ case TGU_PRIORITY1:
+ case TGU_PRIORITY2:
+ case TGU_PRIORITY3:
+ ret = (tgu_attr->reg_num < drvdata->max_reg) ?
+ attr->mode : 0;
+ break;
+ case TGU_CONDITION_DECODE:
+ ret = (tgu_attr->reg_num <
+ drvdata->max_condition_decode) ?
+ attr->mode : 0;
+ break;
+ default:
+ break;
+ }
}
return ret;
}
-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;
@@ -88,8 +163,10 @@ static void tgu_write_all_hw_regs(struct tgu_drvdata *drvdata)
for (i = 0; i < drvdata->max_step; i++) {
for (j = 0; j < MAX_PRIORITY; j++) {
for (k = 0; k < drvdata->max_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 +
@@ -97,9 +174,23 @@ static void tgu_write_all_hw_regs(struct tgu_drvdata *drvdata)
}
}
}
+
+ for (i = 0; i < drvdata->max_step; i++) {
+ for (j = 0; j < drvdata->max_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)
@@ -128,18 +219,32 @@ static void tgu_set_steps(struct tgu_drvdata *drvdata)
drvdata->max_step = TGU_DEVID_STEPS(devid);
}
+static void tgu_set_conditions(struct tgu_drvdata *drvdata)
+{
+ u32 devid;
+
+ devid = readl(drvdata->base + TGU_DEVID);
+ drvdata->max_condition_decode = TGU_DEVID_CONDITIONS(devid);
+}
+
static int tgu_enable(struct device *dev)
{
struct tgu_drvdata *drvdata = dev_get_drvdata(dev);
+ int ret = 0;
guard(spinlock)(&drvdata->lock);
if (drvdata->enable)
return -EBUSY;
- tgu_write_all_hw_regs(drvdata);
+ ret = tgu_write_all_hw_regs(drvdata);
+
+ if (ret == -EINVAL)
+ goto exit;
+
drvdata->enable = true;
- return 0;
+exit:
+ return ret;
}
static void tgu_disable(struct device *dev)
@@ -243,6 +348,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,
};
@@ -250,8 +363,8 @@ static int tgu_probe(struct amba_device *adev, const struct amba_id *id)
{
struct device *dev = &adev->dev;
struct tgu_drvdata *drvdata;
- size_t priority_size;
- unsigned int *priority;
+ size_t priority_size, condition_size;
+ unsigned int *priority, *condition;
int ret;
drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
@@ -269,6 +382,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);
ret = sysfs_create_groups(&dev->kobj, tgu_attr_groups);
if (ret) {
@@ -292,6 +406,16 @@ static int tgu_probe(struct amba_device *adev, const struct amba_id *id)
drvdata->value_table->priority = priority;
+ condition_size = drvdata->max_condition_decode *
+ drvdata->max_step *
+ sizeof(*(drvdata->value_table->condition_decode));
+
+ condition = devm_kzalloc(dev, condition_size, GFP_KERNEL);
+
+ if (!condition)
+ return -ENOMEM;
+
+ drvdata->value_table->condition_decode = condition;
drvdata->enable = false;
diff --git a/drivers/hwtracing/qcom/tgu.h b/drivers/hwtracing/qcom/tgu.h
index f54cea01e427..0d058663aca5 100644
--- a/drivers/hwtracing/qcom/tgu.h
+++ b/drivers/hwtracing/qcom/tgu.h
@@ -15,6 +15,7 @@
#define BMVAL(val, lsb, msb) ((val & GENMASK(msb, lsb)) >> lsb)
#define TGU_DEVID_SENSE_INPUT(devid_val) ((int) BMVAL(devid_val, 10, 17))
#define TGU_DEVID_STEPS(devid_val) ((int)BMVAL(devid_val, 3, 6))
+#define TGU_DEVID_CONDITIONS(devid_val) ((int)BMVAL(devid_val, 0, 2))
#define NUMBER_BITS_EACH_SIGNAL 4
#define LENGTH_REGISTER 32
@@ -48,6 +49,7 @@
*/
#define STEP_OFFSET 0x1D8
#define PRIORITY_START_OFFSET 0x0074
+#define CONDITION_DECODE_OFFSET 0x0050
#define PRIORITY_OFFSET 0x60
#define REG_OFFSET 0x4
@@ -56,6 +58,9 @@
(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), \
@@ -68,6 +73,9 @@
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), \
STEP_PRIORITY(step_index, 1, priority), \
@@ -90,6 +98,14 @@
NULL \
}
+#define STEP_DECODE_LIST(n) \
+ {STEP_DECODE(n, 0), \
+ 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),\
@@ -97,11 +113,19 @@
.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 */
@@ -116,6 +140,7 @@ struct tgu_attribute {
struct value_table {
unsigned int *priority;
+ unsigned int *condition_decode;
};
static inline void TGU_LOCK(void __iomem *addr)
@@ -145,6 +170,7 @@ static inline void TGU_UNLOCK(void __iomem *addr)
* @value_table: Store given value based on relevant parameters.
* @max_reg: Maximum number of registers
* @max_step: Maximum step size
+ * @max_condition_decode: Maximum number of condition_decode
*
* This structure defines the data associated with a TGU device,
* including its base address, device pointers, clock, spinlock for
@@ -159,6 +185,7 @@ struct tgu_drvdata {
struct value_table *value_table;
int max_reg;
int max_step;
+ int max_condition_decode;
};
#endif
--
2.34.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v10 5/7] qcom-tgu: Add support to configure next action
2026-01-09 2:11 [PATCH v10 0/7] Provide support for Trigger Generation Unit Songwei Chai
` (3 preceding siblings ...)
2026-01-09 2:11 ` [PATCH v10 4/7] qcom-tgu: Add TGU decode support Songwei Chai
@ 2026-01-09 2:11 ` Songwei Chai
2026-01-13 11:15 ` Konrad Dybcio
2026-01-09 2:11 ` [PATCH v10 6/7] qcom-tgu: Add timer/counter functionality for TGU Songwei Chai
` (2 subsequent siblings)
7 siblings, 1 reply; 29+ messages in thread
From: Songwei Chai @ 2026-01-09 2:11 UTC (permalink / raw)
To: andersson, alexander.shishkin, mike.leach, suzuki.poulose,
james.clark, krzk+dt, conor+dt
Cc: Songwei Chai, linux-kernel, linux-arm-kernel, linux-arm-msm,
coresight, devicetree, gregkh
Add "select" node for each step to determine if another step is taken,
trigger(s) are generated, counters/timers incremented/decremented, etc.
Signed-off-by: Songwei Chai <songwei.chai@oss.qualcomm.com>
---
.../ABI/testing/sysfs-bus-amba-devices-tgu | 7 +++
drivers/hwtracing/qcom/tgu.c | 57 ++++++++++++++++++-
drivers/hwtracing/qcom/tgu.h | 27 +++++++++
3 files changed, 89 insertions(+), 2 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-bus-amba-devices-tgu b/Documentation/ABI/testing/sysfs-bus-amba-devices-tgu
index 4efa36a11d8e..fffe65d3c0db 100644
--- a/Documentation/ABI/testing/sysfs-bus-amba-devices-tgu
+++ b/Documentation/ABI/testing/sysfs-bus-amba-devices-tgu
@@ -21,3 +21,10 @@ KernelVersion 6.19
Contact: Jinlong Mao <jinlong.mao@oss.qualcomm.com>, Songwei Chai <songwei.chai@oss.qualcomm.com>
Description:
(RW) Set/Get the decode mode with specific step for TGU.
+
+What: /sys/bus/amba/devices/<tgu-name>/step[0:7]_condition_select/reg[0:3]
+Date: January 2026
+KernelVersion 6.19
+Contact: Jinlong Mao <jinlong.mao@oss.qualcomm.com>, Songwei Chai <songwei.chai@oss.qualcomm.com>
+Description:
+ (RW) Set/Get the next action with specific step for TGU.
diff --git a/drivers/hwtracing/qcom/tgu.c b/drivers/hwtracing/qcom/tgu.c
index 39301d35ee9d..e1a1f0f423ba 100644
--- a/drivers/hwtracing/qcom/tgu.c
+++ b/drivers/hwtracing/qcom/tgu.c
@@ -33,6 +33,10 @@ static int calculate_array_location(struct tgu_drvdata *drvdata,
ret = step_index * (drvdata->max_condition_decode) +
reg_index;
break;
+ case TGU_CONDITION_SELECT:
+ ret = step_index * (drvdata->max_condition_select) +
+ reg_index;
+ break;
default:
break;
}
@@ -74,6 +78,9 @@ static ssize_t tgu_dataset_show(struct device *dev,
case TGU_CONDITION_DECODE:
return sysfs_emit(buf, "0x%x\n",
drvdata->value_table->condition_decode[index]);
+ case TGU_CONDITION_SELECT:
+ return sysfs_emit(buf, "0x%x\n",
+ drvdata->value_table->condition_select[index]);
default:
break;
}
@@ -115,6 +122,10 @@ static ssize_t tgu_dataset_store(struct device *dev,
tgu_drvdata->value_table->condition_decode[index] = val;
ret = size;
break;
+ case TGU_CONDITION_SELECT:
+ tgu_drvdata->value_table->condition_select[index] = val;
+ ret = size;
+ break;
default:
ret = -EINVAL;
break;
@@ -148,6 +159,15 @@ static umode_t tgu_node_visible(struct kobject *kobject,
drvdata->max_condition_decode) ?
attr->mode : 0;
break;
+ case TGU_CONDITION_SELECT:
+ /* 'default' register is at the end of 'select' region */
+ if (tgu_attr->reg_num ==
+ drvdata->max_condition_select - 1)
+ attr->name = "default";
+ ret = (tgu_attr->reg_num <
+ drvdata->max_condition_select) ?
+ attr->mode : 0;
+ break;
default:
break;
}
@@ -186,6 +206,19 @@ static ssize_t tgu_write_all_hw_regs(struct tgu_drvdata *drvdata)
drvdata->base + CONDITION_DECODE_STEP(i, j));
}
}
+
+ for (i = 0; i < drvdata->max_step; i++) {
+ for (j = 0; j < drvdata->max_condition_select; j++) {
+ index = check_array_location(drvdata, i,
+ TGU_CONDITION_SELECT, j);
+
+ if (index == -EINVAL)
+ goto exit;
+
+ writel(drvdata->value_table->condition_select[index],
+ drvdata->base + CONDITION_SELECT_STEP(i, j));
+ }
+ }
/* Enable TGU to program the triggers */
writel(1, drvdata->base + TGU_CONTROL);
exit:
@@ -225,6 +258,8 @@ static void tgu_set_conditions(struct tgu_drvdata *drvdata)
devid = readl(drvdata->base + TGU_DEVID);
drvdata->max_condition_decode = TGU_DEVID_CONDITIONS(devid);
+ /* select region has an additional 'default' register */
+ drvdata->max_condition_select = TGU_DEVID_CONDITIONS(devid) + 1;
}
static int tgu_enable(struct device *dev)
@@ -356,6 +391,14 @@ static const struct attribute_group *tgu_attr_groups[] = {
CONDITION_DECODE_ATTRIBUTE_GROUP_INIT(5),
CONDITION_DECODE_ATTRIBUTE_GROUP_INIT(6),
CONDITION_DECODE_ATTRIBUTE_GROUP_INIT(7),
+ CONDITION_SELECT_ATTRIBUTE_GROUP_INIT(0),
+ CONDITION_SELECT_ATTRIBUTE_GROUP_INIT(1),
+ CONDITION_SELECT_ATTRIBUTE_GROUP_INIT(2),
+ CONDITION_SELECT_ATTRIBUTE_GROUP_INIT(3),
+ CONDITION_SELECT_ATTRIBUTE_GROUP_INIT(4),
+ CONDITION_SELECT_ATTRIBUTE_GROUP_INIT(5),
+ CONDITION_SELECT_ATTRIBUTE_GROUP_INIT(6),
+ CONDITION_SELECT_ATTRIBUTE_GROUP_INIT(7),
NULL,
};
@@ -363,8 +406,8 @@ static int tgu_probe(struct amba_device *adev, const struct amba_id *id)
{
struct device *dev = &adev->dev;
struct tgu_drvdata *drvdata;
- size_t priority_size, condition_size;
- unsigned int *priority, *condition;
+ size_t priority_size, condition_size, select_size;
+ unsigned int *priority, *condition, *select;
int ret;
drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
@@ -417,6 +460,16 @@ static int tgu_probe(struct amba_device *adev, const struct amba_id *id)
drvdata->value_table->condition_decode = condition;
+ select_size = drvdata->max_condition_select * drvdata->max_step *
+ sizeof(*(drvdata->value_table->condition_select));
+
+ select = devm_kzalloc(dev, select_size, GFP_KERNEL);
+
+ if (!select)
+ return -ENOMEM;
+
+ drvdata->value_table->condition_select = select;
+
drvdata->enable = false;
pm_runtime_put(&adev->dev);
diff --git a/drivers/hwtracing/qcom/tgu.h b/drivers/hwtracing/qcom/tgu.h
index 0d058663aca5..8c92e88d7e2c 100644
--- a/drivers/hwtracing/qcom/tgu.h
+++ b/drivers/hwtracing/qcom/tgu.h
@@ -50,6 +50,7 @@
#define STEP_OFFSET 0x1D8
#define PRIORITY_START_OFFSET 0x0074
#define CONDITION_DECODE_OFFSET 0x0050
+#define CONDITION_SELECT_OFFSET 0x0060
#define PRIORITY_OFFSET 0x60
#define REG_OFFSET 0x4
@@ -61,6 +62,9 @@
#define CONDITION_DECODE_STEP(step, decode) \
(CONDITION_DECODE_OFFSET + REG_OFFSET * decode + STEP_OFFSET * step)
+#define CONDITION_SELECT_STEP(step, select) \
+ (CONDITION_SELECT_OFFSET + REG_OFFSET * select + 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), \
@@ -76,6 +80,9 @@
#define STEP_DECODE(step_index, reg_num) \
tgu_dataset_rw(reg##reg_num, step_index, TGU_CONDITION_DECODE, reg_num)
+#define STEP_SELECT(step_index, reg_num) \
+ tgu_dataset_rw(reg##reg_num, step_index, TGU_CONDITION_SELECT, reg_num)
+
#define STEP_PRIORITY_LIST(step_index, priority) \
{STEP_PRIORITY(step_index, 0, priority), \
STEP_PRIORITY(step_index, 1, priority), \
@@ -106,6 +113,15 @@
NULL \
}
+#define STEP_SELECT_LIST(n) \
+ {STEP_SELECT(n, 0), \
+ STEP_SELECT(n, 1), \
+ STEP_SELECT(n, 2), \
+ STEP_SELECT(n, 3), \
+ STEP_SELECT(n, 4), \
+ NULL \
+ }
+
#define PRIORITY_ATTRIBUTE_GROUP_INIT(step, priority)\
(&(const struct attribute_group){\
.attrs = (struct attribute*[])STEP_PRIORITY_LIST(step, priority),\
@@ -120,12 +136,20 @@
.name = "step" #step "_condition_decode" \
})
+#define CONDITION_SELECT_ATTRIBUTE_GROUP_INIT(step)\
+ (&(const struct attribute_group){\
+ .attrs = (struct attribute*[])STEP_SELECT_LIST(step),\
+ .is_visible = tgu_node_visible,\
+ .name = "step" #step "_condition_select" \
+ })
+
enum operation_index {
TGU_PRIORITY0,
TGU_PRIORITY1,
TGU_PRIORITY2,
TGU_PRIORITY3,
TGU_CONDITION_DECODE,
+ TGU_CONDITION_SELECT,
};
/* Maximum priority that TGU supports */
@@ -141,6 +165,7 @@ struct tgu_attribute {
struct value_table {
unsigned int *priority;
unsigned int *condition_decode;
+ unsigned int *condition_select;
};
static inline void TGU_LOCK(void __iomem *addr)
@@ -171,6 +196,7 @@ static inline void TGU_UNLOCK(void __iomem *addr)
* @max_reg: Maximum number of registers
* @max_step: Maximum step size
* @max_condition_decode: Maximum number of condition_decode
+ * @max_condition_select: Maximum number of condition_select
*
* This structure defines the data associated with a TGU device,
* including its base address, device pointers, clock, spinlock for
@@ -186,6 +212,7 @@ struct tgu_drvdata {
int max_reg;
int max_step;
int max_condition_decode;
+ int max_condition_select;
};
#endif
--
2.34.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v10 6/7] qcom-tgu: Add timer/counter functionality for TGU
2026-01-09 2:11 [PATCH v10 0/7] Provide support for Trigger Generation Unit Songwei Chai
` (4 preceding siblings ...)
2026-01-09 2:11 ` [PATCH v10 5/7] qcom-tgu: Add support to configure next action Songwei Chai
@ 2026-01-09 2:11 ` Songwei Chai
2026-01-13 11:19 ` Konrad Dybcio
2026-01-09 2:11 ` [PATCH v10 7/7] qcom-tgu: Add reset node to initialize Songwei Chai
2026-01-13 1:53 ` [PATCH v10 0/7] Provide support for Trigger Generation Unit Songwei Chai
7 siblings, 1 reply; 29+ messages in thread
From: Songwei Chai @ 2026-01-09 2:11 UTC (permalink / raw)
To: andersson, alexander.shishkin, mike.leach, suzuki.poulose,
james.clark, krzk+dt, conor+dt
Cc: Songwei Chai, linux-kernel, linux-arm-kernel, linux-arm-msm,
coresight, devicetree, gregkh
Add counter and timer node for each step which could be
programed if they are to be utilized in trigger event/sequence.
Signed-off-by: Songwei Chai <songwei.chai@oss.qualcomm.com>
---
.../ABI/testing/sysfs-bus-amba-devices-tgu | 14 ++
drivers/hwtracing/qcom/tgu.c | 126 +++++++++++++++++-
drivers/hwtracing/qcom/tgu.h | 54 ++++++++
3 files changed, 192 insertions(+), 2 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-bus-amba-devices-tgu b/Documentation/ABI/testing/sysfs-bus-amba-devices-tgu
index fffe65d3c0db..61b5a08bdee1 100644
--- a/Documentation/ABI/testing/sysfs-bus-amba-devices-tgu
+++ b/Documentation/ABI/testing/sysfs-bus-amba-devices-tgu
@@ -28,3 +28,17 @@ KernelVersion 6.19
Contact: Jinlong Mao <jinlong.mao@oss.qualcomm.com>, Songwei Chai <songwei.chai@oss.qualcomm.com>
Description:
(RW) Set/Get the next action with specific step for TGU.
+
+What: /sys/bus/amba/devices/<tgu-name>/step[0:7]_timer/reg[0:1]
+Date: January 2026
+KernelVersion 6.19
+Contact: Jinlong Mao <jinlong.mao@oss.qualcomm.com>, Songwei Chai <songwei.chai@oss.qualcomm.com>
+Description:
+ (RW) Set/Get the timer value with specific step for TGU.
+
+What: /sys/bus/amba/devices/<tgu-name>/step[0:7]_counter/reg[0:1]
+Date: January 2026
+KernelVersion 6.19
+Contact: Jinlong Mao <jinlong.mao@oss.qualcomm.com>, Songwei Chai <songwei.chai@oss.qualcomm.com>
+Description:
+ (RW) Set/Get the counter value with specific step for TGU.
diff --git a/drivers/hwtracing/qcom/tgu.c b/drivers/hwtracing/qcom/tgu.c
index e1a1f0f423ba..a3d9c3c4e28a 100644
--- a/drivers/hwtracing/qcom/tgu.c
+++ b/drivers/hwtracing/qcom/tgu.c
@@ -37,6 +37,12 @@ static int calculate_array_location(struct tgu_drvdata *drvdata,
ret = step_index * (drvdata->max_condition_select) +
reg_index;
break;
+ case TGU_COUNTER:
+ ret = step_index * (drvdata->max_counter) + reg_index;
+ break;
+ case TGU_TIMER:
+ ret = step_index * (drvdata->max_timer) + reg_index;
+ break;
default:
break;
}
@@ -81,6 +87,12 @@ static ssize_t tgu_dataset_show(struct device *dev,
case TGU_CONDITION_SELECT:
return sysfs_emit(buf, "0x%x\n",
drvdata->value_table->condition_select[index]);
+ case TGU_TIMER:
+ return sysfs_emit(buf, "0x%x\n",
+ drvdata->value_table->timer[index]);
+ case TGU_COUNTER:
+ return sysfs_emit(buf, "0x%x\n",
+ drvdata->value_table->counter[index]);
default:
break;
}
@@ -126,6 +138,14 @@ static ssize_t tgu_dataset_store(struct device *dev,
tgu_drvdata->value_table->condition_select[index] = val;
ret = size;
break;
+ case TGU_TIMER:
+ tgu_drvdata->value_table->timer[index] = val;
+ ret = size;
+ break;
+ case TGU_COUNTER:
+ tgu_drvdata->value_table->counter[index] = val;
+ ret = size;
+ break;
default:
ret = -EINVAL;
break;
@@ -168,6 +188,22 @@ static umode_t tgu_node_visible(struct kobject *kobject,
drvdata->max_condition_select) ?
attr->mode : 0;
break;
+ case TGU_COUNTER:
+ if (drvdata->max_counter == 0)
+ ret = SYSFS_GROUP_INVISIBLE;
+ else
+ ret = (tgu_attr->reg_num <
+ drvdata->max_counter) ?
+ attr->mode : 0;
+ break;
+ case TGU_TIMER:
+ if (drvdata->max_timer == 0)
+ ret = SYSFS_GROUP_INVISIBLE;
+ else
+ ret = (tgu_attr->reg_num <
+ drvdata->max_timer) ?
+ attr->mode : 0;
+ break;
default:
break;
}
@@ -219,6 +255,30 @@ static ssize_t tgu_write_all_hw_regs(struct tgu_drvdata *drvdata)
drvdata->base + CONDITION_SELECT_STEP(i, j));
}
}
+
+ for (i = 0; i < drvdata->max_step; i++) {
+ for (j = 0; j < drvdata->max_timer; j++) {
+ index = check_array_location(drvdata, i, TGU_TIMER, j);
+
+ if (index == -EINVAL)
+ goto exit;
+
+ writel(drvdata->value_table->timer[index],
+ drvdata->base + TIMER_COMPARE_STEP(i, j));
+ }
+ }
+
+ for (i = 0; i < drvdata->max_step; i++) {
+ for (j = 0; j < drvdata->max_counter; j++) {
+ index = check_array_location(drvdata, i, TGU_COUNTER, j);
+
+ if (index == -EINVAL)
+ goto exit;
+
+ writel(drvdata->value_table->counter[index],
+ drvdata->base + COUNTER_COMPARE_STEP(i, j));
+ }
+ }
/* Enable TGU to program the triggers */
writel(1, drvdata->base + TGU_CONTROL);
exit:
@@ -262,6 +322,31 @@ static void tgu_set_conditions(struct tgu_drvdata *drvdata)
drvdata->max_condition_select = TGU_DEVID_CONDITIONS(devid) + 1;
}
+static void tgu_set_timer_counter(struct tgu_drvdata *drvdata)
+{
+ int num_timers, num_counters;
+ u32 devid2;
+
+ devid2 = readl(drvdata->base + CORESIGHT_DEVID2);
+
+ if (TGU_DEVID2_TIMER0(devid2) && TGU_DEVID2_TIMER1(devid2))
+ num_timers = 2;
+ else if (TGU_DEVID2_TIMER0(devid2) || TGU_DEVID2_TIMER1(devid2))
+ num_timers = 1;
+ else
+ num_timers = 0;
+
+ if (TGU_DEVID2_COUNTER0(devid2) && TGU_DEVID2_COUNTER1(devid2))
+ num_counters = 2;
+ else if (TGU_DEVID2_COUNTER0(devid2) || TGU_DEVID2_COUNTER1(devid2))
+ num_counters = 1;
+ else
+ num_counters = 0;
+
+ drvdata->max_timer = num_timers;
+ drvdata->max_counter = num_counters;
+}
+
static int tgu_enable(struct device *dev)
{
struct tgu_drvdata *drvdata = dev_get_drvdata(dev);
@@ -399,6 +484,22 @@ static const struct attribute_group *tgu_attr_groups[] = {
CONDITION_SELECT_ATTRIBUTE_GROUP_INIT(5),
CONDITION_SELECT_ATTRIBUTE_GROUP_INIT(6),
CONDITION_SELECT_ATTRIBUTE_GROUP_INIT(7),
+ TIMER_ATTRIBUTE_GROUP_INIT(0),
+ TIMER_ATTRIBUTE_GROUP_INIT(1),
+ TIMER_ATTRIBUTE_GROUP_INIT(2),
+ TIMER_ATTRIBUTE_GROUP_INIT(3),
+ TIMER_ATTRIBUTE_GROUP_INIT(4),
+ TIMER_ATTRIBUTE_GROUP_INIT(5),
+ TIMER_ATTRIBUTE_GROUP_INIT(6),
+ TIMER_ATTRIBUTE_GROUP_INIT(7),
+ COUNTER_ATTRIBUTE_GROUP_INIT(0),
+ COUNTER_ATTRIBUTE_GROUP_INIT(1),
+ COUNTER_ATTRIBUTE_GROUP_INIT(2),
+ COUNTER_ATTRIBUTE_GROUP_INIT(3),
+ COUNTER_ATTRIBUTE_GROUP_INIT(4),
+ COUNTER_ATTRIBUTE_GROUP_INIT(5),
+ COUNTER_ATTRIBUTE_GROUP_INIT(6),
+ COUNTER_ATTRIBUTE_GROUP_INIT(7),
NULL,
};
@@ -406,8 +507,8 @@ static int tgu_probe(struct amba_device *adev, const struct amba_id *id)
{
struct device *dev = &adev->dev;
struct tgu_drvdata *drvdata;
- size_t priority_size, condition_size, select_size;
- unsigned int *priority, *condition, *select;
+ size_t priority_size, condition_size, select_size, timer_size, counter_size;
+ unsigned int *priority, *condition, *select, *timer, *counter;
int ret;
drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
@@ -426,6 +527,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);
+ tgu_set_timer_counter(drvdata);
ret = sysfs_create_groups(&dev->kobj, tgu_attr_groups);
if (ret) {
@@ -470,6 +572,26 @@ static int tgu_probe(struct amba_device *adev, const struct amba_id *id)
drvdata->value_table->condition_select = select;
+ timer_size = drvdata->max_step * drvdata->max_timer *
+ sizeof(*(drvdata->value_table->timer));
+
+ timer = devm_kzalloc(dev, timer_size, GFP_KERNEL);
+
+ if (!timer)
+ return -ENOMEM;
+
+ drvdata->value_table->timer = timer;
+
+ counter_size = drvdata->max_step * drvdata->max_counter *
+ sizeof(*(drvdata->value_table->counter));
+
+ counter = devm_kzalloc(dev, counter_size, GFP_KERNEL);
+
+ if (!counter)
+ return -ENOMEM;
+
+ drvdata->value_table->counter = counter;
+
drvdata->enable = false;
pm_runtime_put(&adev->dev);
diff --git a/drivers/hwtracing/qcom/tgu.h b/drivers/hwtracing/qcom/tgu.h
index 8c92e88d7e2c..94708750b02d 100644
--- a/drivers/hwtracing/qcom/tgu.h
+++ b/drivers/hwtracing/qcom/tgu.h
@@ -11,11 +11,17 @@
#define TGU_LAR 0xfb0
#define TGU_UNLOCK_OFFSET 0xc5acce55
#define TGU_DEVID 0xfc8
+#define CORESIGHT_DEVID2 0xfc0
#define BMVAL(val, lsb, msb) ((val & GENMASK(msb, lsb)) >> lsb)
#define TGU_DEVID_SENSE_INPUT(devid_val) ((int) BMVAL(devid_val, 10, 17))
#define TGU_DEVID_STEPS(devid_val) ((int)BMVAL(devid_val, 3, 6))
#define TGU_DEVID_CONDITIONS(devid_val) ((int)BMVAL(devid_val, 0, 2))
+#define TGU_DEVID2_TIMER0(devid_val) ((int)BMVAL(devid_val, 18, 23))
+#define TGU_DEVID2_TIMER1(devid_val) ((int)BMVAL(devid_val, 13, 17))
+#define TGU_DEVID2_COUNTER0(devid_val) ((int)BMVAL(devid_val, 6, 11))
+#define TGU_DEVID2_COUNTER1(devid_val) ((int)BMVAL(devid_val, 0, 5))
+
#define NUMBER_BITS_EACH_SIGNAL 4
#define LENGTH_REGISTER 32
@@ -51,6 +57,8 @@
#define PRIORITY_START_OFFSET 0x0074
#define CONDITION_DECODE_OFFSET 0x0050
#define CONDITION_SELECT_OFFSET 0x0060
+#define TIMER_START_OFFSET 0x0040
+#define COUNTER_START_OFFSET 0x0048
#define PRIORITY_OFFSET 0x60
#define REG_OFFSET 0x4
@@ -62,6 +70,12 @@
#define CONDITION_DECODE_STEP(step, decode) \
(CONDITION_DECODE_OFFSET + REG_OFFSET * decode + STEP_OFFSET * step)
+#define TIMER_COMPARE_STEP(step, timer) \
+ (TIMER_START_OFFSET + REG_OFFSET * timer + STEP_OFFSET * step)
+
+#define COUNTER_COMPARE_STEP(step, counter) \
+ (COUNTER_START_OFFSET + REG_OFFSET * counter + STEP_OFFSET * step)
+
#define CONDITION_SELECT_STEP(step, select) \
(CONDITION_SELECT_OFFSET + REG_OFFSET * select + STEP_OFFSET * step)
@@ -83,6 +97,12 @@
#define STEP_SELECT(step_index, reg_num) \
tgu_dataset_rw(reg##reg_num, step_index, TGU_CONDITION_SELECT, reg_num)
+#define STEP_TIMER(step_index, reg_num) \
+ tgu_dataset_rw(reg##reg_num, step_index, TGU_TIMER, reg_num)
+
+#define STEP_COUNTER(step_index, reg_num) \
+ tgu_dataset_rw(reg##reg_num, step_index, TGU_COUNTER, reg_num)
+
#define STEP_PRIORITY_LIST(step_index, priority) \
{STEP_PRIORITY(step_index, 0, priority), \
STEP_PRIORITY(step_index, 1, priority), \
@@ -122,6 +142,18 @@
NULL \
}
+#define STEP_TIMER_LIST(n) \
+ {STEP_TIMER(n, 0), \
+ STEP_TIMER(n, 1), \
+ NULL \
+ }
+
+#define STEP_COUNTER_LIST(n) \
+ {STEP_COUNTER(n, 0), \
+ STEP_COUNTER(n, 1), \
+ NULL \
+ }
+
#define PRIORITY_ATTRIBUTE_GROUP_INIT(step, priority)\
(&(const struct attribute_group){\
.attrs = (struct attribute*[])STEP_PRIORITY_LIST(step, priority),\
@@ -143,6 +175,20 @@
.name = "step" #step "_condition_select" \
})
+#define TIMER_ATTRIBUTE_GROUP_INIT(step)\
+ (&(const struct attribute_group){\
+ .attrs = (struct attribute*[])STEP_TIMER_LIST(step),\
+ .is_visible = tgu_node_visible,\
+ .name = "step" #step "_timer" \
+ })
+
+#define COUNTER_ATTRIBUTE_GROUP_INIT(step)\
+ (&(const struct attribute_group){\
+ .attrs = (struct attribute*[])STEP_COUNTER_LIST(step),\
+ .is_visible = tgu_node_visible,\
+ .name = "step" #step "_counter" \
+ })
+
enum operation_index {
TGU_PRIORITY0,
TGU_PRIORITY1,
@@ -150,6 +196,8 @@ enum operation_index {
TGU_PRIORITY3,
TGU_CONDITION_DECODE,
TGU_CONDITION_SELECT,
+ TGU_TIMER,
+ TGU_COUNTER
};
/* Maximum priority that TGU supports */
@@ -166,6 +214,8 @@ struct value_table {
unsigned int *priority;
unsigned int *condition_decode;
unsigned int *condition_select;
+ unsigned int *timer;
+ unsigned int *counter;
};
static inline void TGU_LOCK(void __iomem *addr)
@@ -197,6 +247,8 @@ static inline void TGU_UNLOCK(void __iomem *addr)
* @max_step: Maximum step size
* @max_condition_decode: Maximum number of condition_decode
* @max_condition_select: Maximum number of condition_select
+ * @max_timer: Maximum number of timers
+ * @max_counter: Maximum number of counters
*
* This structure defines the data associated with a TGU device,
* including its base address, device pointers, clock, spinlock for
@@ -213,6 +265,8 @@ struct tgu_drvdata {
int max_step;
int max_condition_decode;
int max_condition_select;
+ int max_timer;
+ int max_counter;
};
#endif
--
2.34.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v10 7/7] qcom-tgu: Add reset node to initialize
2026-01-09 2:11 [PATCH v10 0/7] Provide support for Trigger Generation Unit Songwei Chai
` (5 preceding siblings ...)
2026-01-09 2:11 ` [PATCH v10 6/7] qcom-tgu: Add timer/counter functionality for TGU Songwei Chai
@ 2026-01-09 2:11 ` Songwei Chai
2026-01-13 11:22 ` Konrad Dybcio
2026-01-13 1:53 ` [PATCH v10 0/7] Provide support for Trigger Generation Unit Songwei Chai
7 siblings, 1 reply; 29+ messages in thread
From: Songwei Chai @ 2026-01-09 2:11 UTC (permalink / raw)
To: andersson, alexander.shishkin, mike.leach, suzuki.poulose,
james.clark, krzk+dt, conor+dt
Cc: Songwei Chai, linux-kernel, linux-arm-kernel, linux-arm-msm,
coresight, devicetree, gregkh
Add reset node to initialize the value of
priority/condition_decode/condition_select/timer/counter nodes.
Signed-off-by: Songwei Chai <songwei.chai@oss.qualcomm.com>
---
.../ABI/testing/sysfs-bus-amba-devices-tgu | 7 ++
drivers/hwtracing/qcom/tgu.c | 74 +++++++++++++++++++
2 files changed, 81 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-bus-amba-devices-tgu b/Documentation/ABI/testing/sysfs-bus-amba-devices-tgu
index 61b5a08bdee1..fa2618b31ab9 100644
--- a/Documentation/ABI/testing/sysfs-bus-amba-devices-tgu
+++ b/Documentation/ABI/testing/sysfs-bus-amba-devices-tgu
@@ -42,3 +42,10 @@ KernelVersion 6.19
Contact: Jinlong Mao <jinlong.mao@oss.qualcomm.com>, Songwei Chai <songwei.chai@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: January 2026
+KernelVersion 6.19
+Contact: Jinlong Mao <jinlong.mao@oss.qualcomm.com>, Songwei Chai <songwei.chai@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 a3d9c3c4e28a..0a45ff78858b 100644
--- a/drivers/hwtracing/qcom/tgu.c
+++ b/drivers/hwtracing/qcom/tgu.c
@@ -424,8 +424,82 @@ static ssize_t enable_tgu_store(struct device *dev,
}
static DEVICE_ATTR_RW(enable_tgu);
+/* reset_tgu_store - Reset Trace and Gating Unit (TGU) configuration. */
+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);
+ unsigned long value;
+ int i, j, ret;
+
+ if (kstrtoul(buf, 0, &value) || value == 0)
+ return -EINVAL;
+
+ if (!drvdata->enable) {
+ ret = pm_runtime_get_sync(drvdata->dev);
+ if (ret < 0) {
+ pm_runtime_put(drvdata->dev);
+ return ret;
+ }
+ }
+
+ guard(spinlock)(&drvdata->lock);
+ TGU_UNLOCK(drvdata->base);
+
+ writel(0, drvdata->base + TGU_CONTROL);
+
+ TGU_LOCK(drvdata->base);
+
+ if (drvdata->value_table->priority)
+ memset(drvdata->value_table->priority, 0,
+ MAX_PRIORITY * drvdata->max_step *
+ drvdata->max_reg * sizeof(unsigned int));
+
+ if (drvdata->value_table->condition_decode)
+ memset(drvdata->value_table->condition_decode, 0,
+ drvdata->max_condition_decode * drvdata->max_step *
+ sizeof(unsigned int));
+
+ /* Initialize all condition registers to NOT(value=0x1000000) */
+ for (i = 0; i < drvdata->max_step; i++) {
+ for (j = 0; j < drvdata->max_condition_decode; j++) {
+ drvdata->value_table
+ ->condition_decode[calculate_array_location(
+ drvdata, i, TGU_CONDITION_DECODE, j)] =
+ 0x1000000;
+ }
+ }
+
+ if (drvdata->value_table->condition_select)
+ memset(drvdata->value_table->condition_select, 0,
+ drvdata->max_condition_select * drvdata->max_step *
+ sizeof(unsigned int));
+
+ if (drvdata->value_table->timer)
+ memset(drvdata->value_table->timer, 0,
+ (drvdata->max_step) *
+ (drvdata->max_timer) *
+ sizeof(unsigned int));
+
+ if (drvdata->value_table->counter)
+ memset(drvdata->value_table->counter, 0,
+ (drvdata->max_step) *
+ (drvdata->max_counter) *
+ sizeof(unsigned int));
+
+ dev_dbg(dev, "Qualcomm-TGU reset complete\n");
+
+ drvdata->enable = false;
+ pm_runtime_put(drvdata->dev);
+
+ 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,
};
--
2.34.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH v10 2/7] qcom-tgu: Add TGU driver
2026-01-09 2:11 ` [PATCH v10 2/7] qcom-tgu: Add TGU driver Songwei Chai
@ 2026-01-09 10:27 ` Suzuki K Poulose
2026-01-12 1:43 ` Songwei Chai
2026-01-09 11:28 ` Jie Gan
2026-01-13 10:33 ` Konrad Dybcio
2 siblings, 1 reply; 29+ messages in thread
From: Suzuki K Poulose @ 2026-01-09 10:27 UTC (permalink / raw)
To: Songwei Chai, andersson, alexander.shishkin, mike.leach,
james.clark, krzk+dt, conor+dt
Cc: linux-kernel, linux-arm-kernel, linux-arm-msm, coresight,
devicetree, gregkh
On 09/01/2026 02:11, Songwei Chai wrote:
> Add driver to support device TGU (Trigger Generation Unit).
> TGU is a Data Engine which can be utilized to sense a plurality of
> signals and create a trigger into the CTI or generate interrupts to
> processors. Add probe/enable/disable functions for tgu.
>
> Signed-off-by: Songwei Chai <songwei.chai@oss.qualcomm.com>
> ---
> .../ABI/testing/sysfs-bus-amba-devices-tgu | 9 +
> drivers/Makefile | 1 +
> drivers/hwtracing/Kconfig | 2 +
> drivers/hwtracing/qcom/Kconfig | 18 ++
> drivers/hwtracing/qcom/Makefile | 3 +
> drivers/hwtracing/qcom/tgu.c | 176 ++++++++++++++++++
> drivers/hwtracing/qcom/tgu.h | 51 +++++
> 7 files changed, 260 insertions(+)
> create mode 100644 Documentation/ABI/testing/sysfs-bus-amba-devices-tgu
> 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
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-amba-devices-tgu b/Documentation/ABI/testing/sysfs-bus-amba-devices-tgu
> new file mode 100644
> index 000000000000..56ec3f5ab5d6
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-bus-amba-devices-tgu
> @@ -0,0 +1,9 @@
> +What: /sys/bus/amba/devices/<tgu-name>/enable_tgu
> +Date: January 2026
> +KernelVersion 6.19
It's a bit too late for v6.19, this should rather be 6.20/7.0
Suzuki
> +Contact: Jinlong Mao <jinlong.mao@oss.qualcomm.com>, Songwei Chai <songwei.chai@oss.qualcomm.com>
> +Description:
> + (RW) Set/Get the enable/disable status of TGU
> + Accepts only one of the 2 values - 0 or 1.
> + 0 : disable TGU.
> + 1 : enable TGU.
> diff --git a/drivers/Makefile b/drivers/Makefile
> index ccc05f1eae3e..9608a3debb1f 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -177,6 +177,7 @@ obj-$(CONFIG_RAS) += ras/
> obj-$(CONFIG_USB4) += thunderbolt/
> obj-$(CONFIG_CORESIGHT) += hwtracing/coresight/
> obj-y += hwtracing/intel_th/
> +obj-y += hwtracing/qcom/
> obj-$(CONFIG_STM) += hwtracing/stm/
> obj-$(CONFIG_HISI_PTT) += hwtracing/ptt/
> obj-y += android/
> diff --git a/drivers/hwtracing/Kconfig b/drivers/hwtracing/Kconfig
> index 911ee977103c..8a640218eed8 100644
> --- a/drivers/hwtracing/Kconfig
> +++ b/drivers/hwtracing/Kconfig
> @@ -7,4 +7,6 @@ source "drivers/hwtracing/intel_th/Kconfig"
>
> source "drivers/hwtracing/ptt/Kconfig"
>
> +source "drivers/hwtracing/qcom/Kconfig"
> +
> endmenu
> diff --git a/drivers/hwtracing/qcom/Kconfig b/drivers/hwtracing/qcom/Kconfig
> new file mode 100644
> index 000000000000..d6f6d4b0f28e
> --- /dev/null
> +++ b/drivers/hwtracing/qcom/Kconfig
> @@ -0,0 +1,18 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +#
> +# QCOM specific hwtracing drivers
> +#
> +menu "Qualcomm specific hwtracing drivers"
> +
> +config QCOM_TGU
> + tristate "QCOM Trigger Generation Unit driver"
> + help
> + This driver provides support for Trigger Generation Unit that is
> + used to detect patterns or sequences on a given set of signals.
> + TGU is used to monitor a particular bus within a given region to
> + detect illegal transaction sequences or slave responses. It is also
> + used to monitor a data stream to detect protocol violations and to
> + provide a trigger point for centering data around a specific event
> + within the trace data buffer.
> +
> +endmenu
> diff --git a/drivers/hwtracing/qcom/Makefile b/drivers/hwtracing/qcom/Makefile
> new file mode 100644
> index 000000000000..5a0a868c1ea0
> --- /dev/null
> +++ b/drivers/hwtracing/qcom/Makefile
> @@ -0,0 +1,3 @@
> +# SPDX-License-Identifier: GPL-2.0
> +
> +obj-$(CONFIG_QCOM_TGU) += tgu.o
> diff --git a/drivers/hwtracing/qcom/tgu.c b/drivers/hwtracing/qcom/tgu.c
> new file mode 100644
> index 000000000000..c5b2b384e6ae
> --- /dev/null
> +++ b/drivers/hwtracing/qcom/tgu.c
> @@ -0,0 +1,176 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> + */
> +
> +#include <linux/amba/bus.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/pm_runtime.h>
> +
> +#include "tgu.h"
> +
> +static void tgu_write_all_hw_regs(struct tgu_drvdata *drvdata)
> +{
> + TGU_UNLOCK(drvdata->base);
> + /* Enable TGU to program the triggers */
> + writel(1, drvdata->base + TGU_CONTROL);
> + TGU_LOCK(drvdata->base);
> +}
> +
> +static int tgu_enable(struct device *dev)
> +{
> + struct tgu_drvdata *drvdata = dev_get_drvdata(dev);
> +
> + guard(spinlock)(&drvdata->lock);
> + if (drvdata->enable)
> + return -EBUSY;
> +
> + tgu_write_all_hw_regs(drvdata);
> + drvdata->enable = true;
> +
> + return 0;
> +}
> +
> +static void tgu_disable(struct device *dev)
> +{
> + struct tgu_drvdata *drvdata = dev_get_drvdata(dev);
> +
> + guard(spinlock)(&drvdata->lock);
> + if (drvdata->enable) {
> + TGU_UNLOCK(drvdata->base);
> + writel(0, drvdata->base + TGU_CONTROL);
> + TGU_LOCK(drvdata->base);
> +
> + drvdata->enable = false;
> + }
> +}
> +
> +static ssize_t enable_tgu_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct tgu_drvdata *drvdata = dev_get_drvdata(dev);
> + bool enabled;
> +
> + guard(spinlock)(&drvdata->lock);
> + enabled = drvdata->enable;
> +
> + return sysfs_emit(buf, "%d\n", enabled ? 1 : 0);
> +}
> +
> +/* enable_tgu_store - Configure Trace and Gating Unit (TGU) triggers. */
> +static ssize_t enable_tgu_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf,
> + size_t size)
> +{
> + unsigned long val;
> + int ret = 0;
> +
> + ret = kstrtoul(buf, 0, &val);
> + if (ret)
> + return ret;
> +
> + if (val) {
> + ret = pm_runtime_resume_and_get(dev);
> + if (ret)
> + return ret;
> + ret = tgu_enable(dev);
> + if (ret) {
> + pm_runtime_put(dev);
> + return ret;
> + }
> + } else {
> + tgu_disable(dev);
> + pm_runtime_put(dev);
> + }
> +
> + return size;
> +}
> +static DEVICE_ATTR_RW(enable_tgu);
> +
> +static struct attribute *tgu_common_attrs[] = {
> + &dev_attr_enable_tgu.attr,
> + NULL,
> +};
> +
> +static const struct attribute_group tgu_common_grp = {
> + .attrs = tgu_common_attrs,
> + NULL,
> +};
> +
> +static const struct attribute_group *tgu_attr_groups[] = {
> + &tgu_common_grp,
> + NULL,
> +};
> +
> +static int tgu_probe(struct amba_device *adev, const struct amba_id *id)
> +{
> + struct device *dev = &adev->dev;
> + struct tgu_drvdata *drvdata;
> + int ret;
> +
> + drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
> + if (!drvdata)
> + return -ENOMEM;
> +
> + drvdata->dev = &adev->dev;
> + dev_set_drvdata(dev, drvdata);
> +
> + drvdata->base = devm_ioremap_resource(dev, &adev->res);
> + if (IS_ERR(drvdata->base))
> + return PTR_ERR(drvdata->base);
> +
> + spin_lock_init(&drvdata->lock);
> +
> + ret = sysfs_create_groups(&dev->kobj, tgu_attr_groups);
> + if (ret) {
> + dev_err(dev, "failed to create sysfs groups: %d\n", ret);
> + return ret;
> + }
> +
> + drvdata->enable = false;
> +
> + pm_runtime_put(&adev->dev);
> + return 0;
> +}
> +
> +static void tgu_remove(struct amba_device *adev)
> +{
> + struct device *dev = &adev->dev;
> +
> + sysfs_remove_groups(&dev->kobj, tgu_attr_groups);
> +
> + tgu_disable(dev);
> +}
> +
> +static const struct amba_id tgu_ids[] = {
> + {
> + .id = 0x000f0e00,
> + .mask = 0x000fffff,
> + },
> + { 0, 0, NULL },
> +};
> +
> +MODULE_DEVICE_TABLE(amba, tgu_ids);
> +
> +static struct amba_driver tgu_driver = {
> + .drv = {
> + .name = "qcom-tgu",
> + .suppress_bind_attrs = true,
> + },
> + .probe = tgu_probe,
> + .remove = tgu_remove,
> + .id_table = tgu_ids,
> +};
> +
> +module_amba_driver(tgu_driver);
> +
> +MODULE_AUTHOR("Songwei Chai <songwei.chai@oss.qualcomm.com>");
> +MODULE_AUTHOR("Jinlong Mao <jinlong.mao@oss.qualcomm.com>");
> +MODULE_DESCRIPTION("Qualcomm Trigger Generation Unit driver");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/hwtracing/qcom/tgu.h b/drivers/hwtracing/qcom/tgu.h
> new file mode 100644
> index 000000000000..b11cfb28261d
> --- /dev/null
> +++ b/drivers/hwtracing/qcom/tgu.h
> @@ -0,0 +1,51 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> + */
> +
> +#ifndef _QCOM_TGU_H
> +#define _QCOM_TGU_H
> +
> +/* Register addresses */
> +#define TGU_CONTROL 0x0000
> +#define TGU_LAR 0xfb0
> +#define TGU_UNLOCK_OFFSET 0xc5acce55
> +
> +static inline void TGU_LOCK(void __iomem *addr)
> +{
> + do {
> + /* Wait for things to settle */
> + mb();
> + writel_relaxed(0x0, addr + TGU_LAR);
> + } while (0);
> +}
> +
> +static inline void TGU_UNLOCK(void __iomem *addr)
> +{
> + do {
> + writel_relaxed(TGU_UNLOCK_OFFSET, addr + TGU_LAR);
> + /* Make sure everyone has seen this */
> + mb();
> + } while (0);
> +}
> +
> +/**
> + * struct tgu_drvdata - Data structure for a TGU (Trigger Generator Unit)
> + * @base: Memory-mapped base address of the TGU device
> + * @dev: Pointer to the associated device structure
> + * @lock: Spinlock for handling concurrent access
> + * @enable: Flag indicating whether the TGU device is enabled
> + *
> + * This structure defines the data associated with a TGU device,
> + * including its base address, device pointers, clock, spinlock for
> + * synchronization, trigger data pointers, maximum limits for various
> + * trigger-related parameters, and enable status.
> + */
> +struct tgu_drvdata {
> + void __iomem *base;
> + struct device *dev;
> + spinlock_t lock;
> + bool enable;
> +};
> +
> +#endif
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v10 2/7] qcom-tgu: Add TGU driver
2026-01-09 2:11 ` [PATCH v10 2/7] qcom-tgu: Add TGU driver Songwei Chai
2026-01-09 10:27 ` Suzuki K Poulose
@ 2026-01-09 11:28 ` Jie Gan
2026-01-12 1:41 ` Songwei Chai
2026-01-13 10:33 ` Konrad Dybcio
2 siblings, 1 reply; 29+ messages in thread
From: Jie Gan @ 2026-01-09 11:28 UTC (permalink / raw)
To: Songwei Chai, andersson, alexander.shishkin, mike.leach,
suzuki.poulose, james.clark, krzk+dt, conor+dt
Cc: linux-kernel, linux-arm-kernel, linux-arm-msm, coresight,
devicetree, gregkh
On 1/9/2026 10:11 AM, Songwei Chai wrote:
> Add driver to support device TGU (Trigger Generation Unit).
> TGU is a Data Engine which can be utilized to sense a plurality of
> signals and create a trigger into the CTI or generate interrupts to
> processors. Add probe/enable/disable functions for tgu.
>
> Signed-off-by: Songwei Chai <songwei.chai@oss.qualcomm.com>
> ---
> .../ABI/testing/sysfs-bus-amba-devices-tgu | 9 +
> drivers/Makefile | 1 +
> drivers/hwtracing/Kconfig | 2 +
> drivers/hwtracing/qcom/Kconfig | 18 ++
> drivers/hwtracing/qcom/Makefile | 3 +
> drivers/hwtracing/qcom/tgu.c | 176 ++++++++++++++++++
> drivers/hwtracing/qcom/tgu.h | 51 +++++
drivers/hwtracing/qcom is a new dir, I suppose this patch series will go
through QCOM tree? If Yes, I think it's better to update the MAINTAINER
file to add QCOM maintainers for maintaining this dir. Otherwise the
get_maintainer script can not obtain proper reviewer&maintainer for
reviewing.
Thanks,
Jie
> 7 files changed, 260 insertions(+)
> create mode 100644 Documentation/ABI/testing/sysfs-bus-amba-devices-tgu
> 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
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-amba-devices-tgu b/Documentation/ABI/testing/sysfs-bus-amba-devices-tgu
> new file mode 100644
> index 000000000000..56ec3f5ab5d6
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-bus-amba-devices-tgu
> @@ -0,0 +1,9 @@
> +What: /sys/bus/amba/devices/<tgu-name>/enable_tgu
> +Date: January 2026
> +KernelVersion 6.19
> +Contact: Jinlong Mao <jinlong.mao@oss.qualcomm.com>, Songwei Chai <songwei.chai@oss.qualcomm.com>
> +Description:
> + (RW) Set/Get the enable/disable status of TGU
> + Accepts only one of the 2 values - 0 or 1.
> + 0 : disable TGU.
> + 1 : enable TGU.
> diff --git a/drivers/Makefile b/drivers/Makefile
> index ccc05f1eae3e..9608a3debb1f 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -177,6 +177,7 @@ obj-$(CONFIG_RAS) += ras/
> obj-$(CONFIG_USB4) += thunderbolt/
> obj-$(CONFIG_CORESIGHT) += hwtracing/coresight/
> obj-y += hwtracing/intel_th/
> +obj-y += hwtracing/qcom/
> obj-$(CONFIG_STM) += hwtracing/stm/
> obj-$(CONFIG_HISI_PTT) += hwtracing/ptt/
> obj-y += android/
> diff --git a/drivers/hwtracing/Kconfig b/drivers/hwtracing/Kconfig
> index 911ee977103c..8a640218eed8 100644
> --- a/drivers/hwtracing/Kconfig
> +++ b/drivers/hwtracing/Kconfig
> @@ -7,4 +7,6 @@ source "drivers/hwtracing/intel_th/Kconfig"
>
> source "drivers/hwtracing/ptt/Kconfig"
>
> +source "drivers/hwtracing/qcom/Kconfig"
> +
> endmenu
> diff --git a/drivers/hwtracing/qcom/Kconfig b/drivers/hwtracing/qcom/Kconfig
> new file mode 100644
> index 000000000000..d6f6d4b0f28e
> --- /dev/null
> +++ b/drivers/hwtracing/qcom/Kconfig
> @@ -0,0 +1,18 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +#
> +# QCOM specific hwtracing drivers
> +#
> +menu "Qualcomm specific hwtracing drivers"
> +
> +config QCOM_TGU
> + tristate "QCOM Trigger Generation Unit driver"
> + help
> + This driver provides support for Trigger Generation Unit that is
> + used to detect patterns or sequences on a given set of signals.
> + TGU is used to monitor a particular bus within a given region to
> + detect illegal transaction sequences or slave responses. It is also
> + used to monitor a data stream to detect protocol violations and to
> + provide a trigger point for centering data around a specific event
> + within the trace data buffer.
> +
> +endmenu
> diff --git a/drivers/hwtracing/qcom/Makefile b/drivers/hwtracing/qcom/Makefile
> new file mode 100644
> index 000000000000..5a0a868c1ea0
> --- /dev/null
> +++ b/drivers/hwtracing/qcom/Makefile
> @@ -0,0 +1,3 @@
> +# SPDX-License-Identifier: GPL-2.0
> +
> +obj-$(CONFIG_QCOM_TGU) += tgu.o
> diff --git a/drivers/hwtracing/qcom/tgu.c b/drivers/hwtracing/qcom/tgu.c
> new file mode 100644
> index 000000000000..c5b2b384e6ae
> --- /dev/null
> +++ b/drivers/hwtracing/qcom/tgu.c
> @@ -0,0 +1,176 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> + */
> +
> +#include <linux/amba/bus.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/pm_runtime.h>
> +
> +#include "tgu.h"
> +
> +static void tgu_write_all_hw_regs(struct tgu_drvdata *drvdata)
> +{
> + TGU_UNLOCK(drvdata->base);
> + /* Enable TGU to program the triggers */
> + writel(1, drvdata->base + TGU_CONTROL);
> + TGU_LOCK(drvdata->base);
> +}
> +
> +static int tgu_enable(struct device *dev)
> +{
> + struct tgu_drvdata *drvdata = dev_get_drvdata(dev);
> +
> + guard(spinlock)(&drvdata->lock);
> + if (drvdata->enable)
> + return -EBUSY;
> +
> + tgu_write_all_hw_regs(drvdata);
> + drvdata->enable = true;
> +
> + return 0;
> +}
> +
> +static void tgu_disable(struct device *dev)
> +{
> + struct tgu_drvdata *drvdata = dev_get_drvdata(dev);
> +
> + guard(spinlock)(&drvdata->lock);
> + if (drvdata->enable) {
> + TGU_UNLOCK(drvdata->base);
> + writel(0, drvdata->base + TGU_CONTROL);
> + TGU_LOCK(drvdata->base);
> +
> + drvdata->enable = false;
> + }
> +}
> +
> +static ssize_t enable_tgu_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct tgu_drvdata *drvdata = dev_get_drvdata(dev);
> + bool enabled;
> +
> + guard(spinlock)(&drvdata->lock);
> + enabled = drvdata->enable;
> +
> + return sysfs_emit(buf, "%d\n", enabled ? 1 : 0);
> +}
> +
> +/* enable_tgu_store - Configure Trace and Gating Unit (TGU) triggers. */
> +static ssize_t enable_tgu_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf,
> + size_t size)
> +{
> + unsigned long val;
> + int ret = 0;
> +
> + ret = kstrtoul(buf, 0, &val);
> + if (ret)
> + return ret;
> +
> + if (val) {
> + ret = pm_runtime_resume_and_get(dev);
> + if (ret)
> + return ret;
> + ret = tgu_enable(dev);
> + if (ret) {
> + pm_runtime_put(dev);
> + return ret;
> + }
> + } else {
> + tgu_disable(dev);
> + pm_runtime_put(dev);
> + }
> +
> + return size;
> +}
> +static DEVICE_ATTR_RW(enable_tgu);
> +
> +static struct attribute *tgu_common_attrs[] = {
> + &dev_attr_enable_tgu.attr,
> + NULL,
> +};
> +
> +static const struct attribute_group tgu_common_grp = {
> + .attrs = tgu_common_attrs,
> + NULL,
> +};
> +
> +static const struct attribute_group *tgu_attr_groups[] = {
> + &tgu_common_grp,
> + NULL,
> +};
> +
> +static int tgu_probe(struct amba_device *adev, const struct amba_id *id)
> +{
> + struct device *dev = &adev->dev;
> + struct tgu_drvdata *drvdata;
> + int ret;
> +
> + drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
> + if (!drvdata)
> + return -ENOMEM;
> +
> + drvdata->dev = &adev->dev;
> + dev_set_drvdata(dev, drvdata);
> +
> + drvdata->base = devm_ioremap_resource(dev, &adev->res);
> + if (IS_ERR(drvdata->base))
> + return PTR_ERR(drvdata->base);
> +
> + spin_lock_init(&drvdata->lock);
> +
> + ret = sysfs_create_groups(&dev->kobj, tgu_attr_groups);
> + if (ret) {
> + dev_err(dev, "failed to create sysfs groups: %d\n", ret);
> + return ret;
> + }
> +
> + drvdata->enable = false;
> +
> + pm_runtime_put(&adev->dev);
> + return 0;
> +}
> +
> +static void tgu_remove(struct amba_device *adev)
> +{
> + struct device *dev = &adev->dev;
> +
> + sysfs_remove_groups(&dev->kobj, tgu_attr_groups);
> +
> + tgu_disable(dev);
> +}
> +
> +static const struct amba_id tgu_ids[] = {
> + {
> + .id = 0x000f0e00,
> + .mask = 0x000fffff,
> + },
> + { 0, 0, NULL },
> +};
> +
> +MODULE_DEVICE_TABLE(amba, tgu_ids);
> +
> +static struct amba_driver tgu_driver = {
> + .drv = {
> + .name = "qcom-tgu",
> + .suppress_bind_attrs = true,
> + },
> + .probe = tgu_probe,
> + .remove = tgu_remove,
> + .id_table = tgu_ids,
> +};
> +
> +module_amba_driver(tgu_driver);
> +
> +MODULE_AUTHOR("Songwei Chai <songwei.chai@oss.qualcomm.com>");
> +MODULE_AUTHOR("Jinlong Mao <jinlong.mao@oss.qualcomm.com>");
> +MODULE_DESCRIPTION("Qualcomm Trigger Generation Unit driver");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/hwtracing/qcom/tgu.h b/drivers/hwtracing/qcom/tgu.h
> new file mode 100644
> index 000000000000..b11cfb28261d
> --- /dev/null
> +++ b/drivers/hwtracing/qcom/tgu.h
> @@ -0,0 +1,51 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> + */
> +
> +#ifndef _QCOM_TGU_H
> +#define _QCOM_TGU_H
> +
> +/* Register addresses */
> +#define TGU_CONTROL 0x0000
> +#define TGU_LAR 0xfb0
> +#define TGU_UNLOCK_OFFSET 0xc5acce55
> +
> +static inline void TGU_LOCK(void __iomem *addr)
> +{
> + do {
> + /* Wait for things to settle */
> + mb();
> + writel_relaxed(0x0, addr + TGU_LAR);
> + } while (0);
> +}
> +
> +static inline void TGU_UNLOCK(void __iomem *addr)
> +{
> + do {
> + writel_relaxed(TGU_UNLOCK_OFFSET, addr + TGU_LAR);
> + /* Make sure everyone has seen this */
> + mb();
> + } while (0);
> +}
> +
> +/**
> + * struct tgu_drvdata - Data structure for a TGU (Trigger Generator Unit)
> + * @base: Memory-mapped base address of the TGU device
> + * @dev: Pointer to the associated device structure
> + * @lock: Spinlock for handling concurrent access
> + * @enable: Flag indicating whether the TGU device is enabled
> + *
> + * This structure defines the data associated with a TGU device,
> + * including its base address, device pointers, clock, spinlock for
> + * synchronization, trigger data pointers, maximum limits for various
> + * trigger-related parameters, and enable status.
> + */
> +struct tgu_drvdata {
> + void __iomem *base;
> + struct device *dev;
> + spinlock_t lock;
> + bool enable;
> +};
> +
> +#endif
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v10 2/7] qcom-tgu: Add TGU driver
2026-01-09 11:28 ` Jie Gan
@ 2026-01-12 1:41 ` Songwei Chai
0 siblings, 0 replies; 29+ messages in thread
From: Songwei Chai @ 2026-01-12 1:41 UTC (permalink / raw)
To: Jie Gan, andersson, alexander.shishkin, mike.leach,
suzuki.poulose, james.clark, krzk+dt, conor+dt
Cc: linux-kernel, linux-arm-kernel, linux-arm-msm, coresight,
devicetree, gregkh
On 1/9/2026 7:28 PM, Jie Gan wrote:
>
>
> On 1/9/2026 10:11 AM, Songwei Chai wrote:
>> Add driver to support device TGU (Trigger Generation Unit).
>> TGU is a Data Engine which can be utilized to sense a plurality of
>> signals and create a trigger into the CTI or generate interrupts to
>> processors. Add probe/enable/disable functions for tgu.
>>
>> Signed-off-by: Songwei Chai <songwei.chai@oss.qualcomm.com>
>> ---
>> .../ABI/testing/sysfs-bus-amba-devices-tgu | 9 +
>> drivers/Makefile | 1 +
>> drivers/hwtracing/Kconfig | 2 +
>> drivers/hwtracing/qcom/Kconfig | 18 ++
>> drivers/hwtracing/qcom/Makefile | 3 +
>> drivers/hwtracing/qcom/tgu.c | 176 ++++++++++++++++++
>> drivers/hwtracing/qcom/tgu.h | 51 +++++
>
> drivers/hwtracing/qcom is a new dir, I suppose this patch series will go
> through QCOM tree? If Yes, I think it's better to update the MAINTAINER
> file to add QCOM maintainers for maintaining this dir. Otherwise the
> get_maintainer script can not obtain proper reviewer&maintainer for
> reviewing.
thanks Jie, will update further.>
> Thanks,
> Jie
>
>> 7 files changed, 260 insertions(+)
>> create mode 100644 Documentation/ABI/testing/sysfs-bus-amba-devices-tgu
>> 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
>>
>> diff --git a/Documentation/ABI/testing/sysfs-bus-amba-devices-tgu b/
>> Documentation/ABI/testing/sysfs-bus-amba-devices-tgu
>> new file mode 100644
>> index 000000000000..56ec3f5ab5d6
>> --- /dev/null
>> +++ b/Documentation/ABI/testing/sysfs-bus-amba-devices-tgu
>> @@ -0,0 +1,9 @@
>> +What: /sys/bus/amba/devices/<tgu-name>/enable_tgu
>> +Date: January 2026
>> +KernelVersion 6.19
>> +Contact: Jinlong Mao <jinlong.mao@oss.qualcomm.com>, Songwei Chai
>> <songwei.chai@oss.qualcomm.com>
>> +Description:
>> + (RW) Set/Get the enable/disable status of TGU
>> + Accepts only one of the 2 values - 0 or 1.
>> + 0 : disable TGU.
>> + 1 : enable TGU.
>> diff --git a/drivers/Makefile b/drivers/Makefile
>> index ccc05f1eae3e..9608a3debb1f 100644
>> --- a/drivers/Makefile
>> +++ b/drivers/Makefile
>> @@ -177,6 +177,7 @@ obj-$(CONFIG_RAS) += ras/
>> obj-$(CONFIG_USB4) += thunderbolt/
>> obj-$(CONFIG_CORESIGHT) += hwtracing/coresight/
>> obj-y += hwtracing/intel_th/
>> +obj-y += hwtracing/qcom/
>> obj-$(CONFIG_STM) += hwtracing/stm/
>> obj-$(CONFIG_HISI_PTT) += hwtracing/ptt/
>> obj-y += android/
>> diff --git a/drivers/hwtracing/Kconfig b/drivers/hwtracing/Kconfig
>> index 911ee977103c..8a640218eed8 100644
>> --- a/drivers/hwtracing/Kconfig
>> +++ b/drivers/hwtracing/Kconfig
>> @@ -7,4 +7,6 @@ source "drivers/hwtracing/intel_th/Kconfig"
>> source "drivers/hwtracing/ptt/Kconfig"
>> +source "drivers/hwtracing/qcom/Kconfig"
>> +
>> endmenu
>> diff --git a/drivers/hwtracing/qcom/Kconfig b/drivers/hwtracing/qcom/
>> Kconfig
>> new file mode 100644
>> index 000000000000..d6f6d4b0f28e
>> --- /dev/null
>> +++ b/drivers/hwtracing/qcom/Kconfig
>> @@ -0,0 +1,18 @@
>> +# SPDX-License-Identifier: GPL-2.0-only
>> +#
>> +# QCOM specific hwtracing drivers
>> +#
>> +menu "Qualcomm specific hwtracing drivers"
>> +
>> +config QCOM_TGU
>> + tristate "QCOM Trigger Generation Unit driver"
>> + help
>> + This driver provides support for Trigger Generation Unit that is
>> + used to detect patterns or sequences on a given set of signals.
>> + TGU is used to monitor a particular bus within a given region to
>> + detect illegal transaction sequences or slave responses. It is
>> also
>> + used to monitor a data stream to detect protocol violations and to
>> + provide a trigger point for centering data around a specific event
>> + within the trace data buffer.
>> +
>> +endmenu
>> diff --git a/drivers/hwtracing/qcom/Makefile b/drivers/hwtracing/qcom/
>> Makefile
>> new file mode 100644
>> index 000000000000..5a0a868c1ea0
>> --- /dev/null
>> +++ b/drivers/hwtracing/qcom/Makefile
>> @@ -0,0 +1,3 @@
>> +# SPDX-License-Identifier: GPL-2.0
>> +
>> +obj-$(CONFIG_QCOM_TGU) += tgu.o
>> diff --git a/drivers/hwtracing/qcom/tgu.c b/drivers/hwtracing/qcom/tgu.c
>> new file mode 100644
>> index 000000000000..c5b2b384e6ae
>> --- /dev/null
>> +++ b/drivers/hwtracing/qcom/tgu.c
>> @@ -0,0 +1,176 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
>> + */
>> +
>> +#include <linux/amba/bus.h>
>> +#include <linux/device.h>
>> +#include <linux/err.h>
>> +#include <linux/io.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/pm_runtime.h>
>> +
>> +#include "tgu.h"
>> +
>> +static void tgu_write_all_hw_regs(struct tgu_drvdata *drvdata)
>> +{
>> + TGU_UNLOCK(drvdata->base);
>> + /* Enable TGU to program the triggers */
>> + writel(1, drvdata->base + TGU_CONTROL);
>> + TGU_LOCK(drvdata->base);
>> +}
>> +
>> +static int tgu_enable(struct device *dev)
>> +{
>> + struct tgu_drvdata *drvdata = dev_get_drvdata(dev);
>> +
>> + guard(spinlock)(&drvdata->lock);
>> + if (drvdata->enable)
>> + return -EBUSY;
>> +
>> + tgu_write_all_hw_regs(drvdata);
>> + drvdata->enable = true;
>> +
>> + return 0;
>> +}
>> +
>> +static void tgu_disable(struct device *dev)
>> +{
>> + struct tgu_drvdata *drvdata = dev_get_drvdata(dev);
>> +
>> + guard(spinlock)(&drvdata->lock);
>> + if (drvdata->enable) {
>> + TGU_UNLOCK(drvdata->base);
>> + writel(0, drvdata->base + TGU_CONTROL);
>> + TGU_LOCK(drvdata->base);
>> +
>> + drvdata->enable = false;
>> + }
>> +}
>> +
>> +static ssize_t enable_tgu_show(struct device *dev,
>> + struct device_attribute *attr, char *buf)
>> +{
>> + struct tgu_drvdata *drvdata = dev_get_drvdata(dev);
>> + bool enabled;
>> +
>> + guard(spinlock)(&drvdata->lock);
>> + enabled = drvdata->enable;
>> +
>> + return sysfs_emit(buf, "%d\n", enabled ? 1 : 0);
>> +}
>> +
>> +/* enable_tgu_store - Configure Trace and Gating Unit (TGU) triggers. */
>> +static ssize_t enable_tgu_store(struct device *dev,
>> + struct device_attribute *attr,
>> + const char *buf,
>> + size_t size)
>> +{
>> + unsigned long val;
>> + int ret = 0;
>> +
>> + ret = kstrtoul(buf, 0, &val);
>> + if (ret)
>> + return ret;
>> +
>> + if (val) {
>> + ret = pm_runtime_resume_and_get(dev);
>> + if (ret)
>> + return ret;
>> + ret = tgu_enable(dev);
>> + if (ret) {
>> + pm_runtime_put(dev);
>> + return ret;
>> + }
>> + } else {
>> + tgu_disable(dev);
>> + pm_runtime_put(dev);
>> + }
>> +
>> + return size;
>> +}
>> +static DEVICE_ATTR_RW(enable_tgu);
>> +
>> +static struct attribute *tgu_common_attrs[] = {
>> + &dev_attr_enable_tgu.attr,
>> + NULL,
>> +};
>> +
>> +static const struct attribute_group tgu_common_grp = {
>> + .attrs = tgu_common_attrs,
>> + NULL,
>> +};
>> +
>> +static const struct attribute_group *tgu_attr_groups[] = {
>> + &tgu_common_grp,
>> + NULL,
>> +};
>> +
>> +static int tgu_probe(struct amba_device *adev, const struct amba_id *id)
>> +{
>> + struct device *dev = &adev->dev;
>> + struct tgu_drvdata *drvdata;
>> + int ret;
>> +
>> + drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
>> + if (!drvdata)
>> + return -ENOMEM;
>> +
>> + drvdata->dev = &adev->dev;
>> + dev_set_drvdata(dev, drvdata);
>> +
>> + drvdata->base = devm_ioremap_resource(dev, &adev->res);
>> + if (IS_ERR(drvdata->base))
>> + return PTR_ERR(drvdata->base);
>> +
>> + spin_lock_init(&drvdata->lock);
>> +
>> + ret = sysfs_create_groups(&dev->kobj, tgu_attr_groups);
>> + if (ret) {
>> + dev_err(dev, "failed to create sysfs groups: %d\n", ret);
>> + return ret;
>> + }
>> +
>> + drvdata->enable = false;
>> +
>> + pm_runtime_put(&adev->dev);
>> + return 0;
>> +}
>> +
>> +static void tgu_remove(struct amba_device *adev)
>> +{
>> + struct device *dev = &adev->dev;
>> +
>> + sysfs_remove_groups(&dev->kobj, tgu_attr_groups);
>> +
>> + tgu_disable(dev);
>> +}
>> +
>> +static const struct amba_id tgu_ids[] = {
>> + {
>> + .id = 0x000f0e00,
>> + .mask = 0x000fffff,
>> + },
>> + { 0, 0, NULL },
>> +};
>> +
>> +MODULE_DEVICE_TABLE(amba, tgu_ids);
>> +
>> +static struct amba_driver tgu_driver = {
>> + .drv = {
>> + .name = "qcom-tgu",
>> + .suppress_bind_attrs = true,
>> + },
>> + .probe = tgu_probe,
>> + .remove = tgu_remove,
>> + .id_table = tgu_ids,
>> +};
>> +
>> +module_amba_driver(tgu_driver);
>> +
>> +MODULE_AUTHOR("Songwei Chai <songwei.chai@oss.qualcomm.com>");
>> +MODULE_AUTHOR("Jinlong Mao <jinlong.mao@oss.qualcomm.com>");
>> +MODULE_DESCRIPTION("Qualcomm Trigger Generation Unit driver");
>> +MODULE_LICENSE("GPL");
>> diff --git a/drivers/hwtracing/qcom/tgu.h b/drivers/hwtracing/qcom/tgu.h
>> new file mode 100644
>> index 000000000000..b11cfb28261d
>> --- /dev/null
>> +++ b/drivers/hwtracing/qcom/tgu.h
>> @@ -0,0 +1,51 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/*
>> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
>> + */
>> +
>> +#ifndef _QCOM_TGU_H
>> +#define _QCOM_TGU_H
>> +
>> +/* Register addresses */
>> +#define TGU_CONTROL 0x0000
>> +#define TGU_LAR 0xfb0
>> +#define TGU_UNLOCK_OFFSET 0xc5acce55
>> +
>> +static inline void TGU_LOCK(void __iomem *addr)
>> +{
>> + do {
>> + /* Wait for things to settle */
>> + mb();
>> + writel_relaxed(0x0, addr + TGU_LAR);
>> + } while (0);
>> +}
>> +
>> +static inline void TGU_UNLOCK(void __iomem *addr)
>> +{
>> + do {
>> + writel_relaxed(TGU_UNLOCK_OFFSET, addr + TGU_LAR);
>> + /* Make sure everyone has seen this */
>> + mb();
>> + } while (0);
>> +}
>> +
>> +/**
>> + * struct tgu_drvdata - Data structure for a TGU (Trigger Generator
>> Unit)
>> + * @base: Memory-mapped base address of the TGU device
>> + * @dev: Pointer to the associated device structure
>> + * @lock: Spinlock for handling concurrent access
>> + * @enable: Flag indicating whether the TGU device is enabled
>> + *
>> + * This structure defines the data associated with a TGU device,
>> + * including its base address, device pointers, clock, spinlock for
>> + * synchronization, trigger data pointers, maximum limits for various
>> + * trigger-related parameters, and enable status.
>> + */
>> +struct tgu_drvdata {
>> + void __iomem *base;
>> + struct device *dev;
>> + spinlock_t lock;
>> + bool enable;
>> +};
>> +
>> +#endif
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v10 2/7] qcom-tgu: Add TGU driver
2026-01-09 10:27 ` Suzuki K Poulose
@ 2026-01-12 1:43 ` Songwei Chai
0 siblings, 0 replies; 29+ messages in thread
From: Songwei Chai @ 2026-01-12 1:43 UTC (permalink / raw)
To: Suzuki K Poulose, andersson, alexander.shishkin, mike.leach,
james.clark, krzk+dt, conor+dt
Cc: linux-kernel, linux-arm-kernel, linux-arm-msm, coresight,
devicetree, gregkh
On 1/9/2026 6:27 PM, Suzuki K Poulose wrote:
> On 09/01/2026 02:11, Songwei Chai wrote:
>> Add driver to support device TGU (Trigger Generation Unit).
>> TGU is a Data Engine which can be utilized to sense a plurality of
>> signals and create a trigger into the CTI or generate interrupts to
>> processors. Add probe/enable/disable functions for tgu.
>>
>> Signed-off-by: Songwei Chai <songwei.chai@oss.qualcomm.com>
>> ---
>> .../ABI/testing/sysfs-bus-amba-devices-tgu | 9 +
>> drivers/Makefile | 1 +
>> drivers/hwtracing/Kconfig | 2 +
>> drivers/hwtracing/qcom/Kconfig | 18 ++
>> drivers/hwtracing/qcom/Makefile | 3 +
>> drivers/hwtracing/qcom/tgu.c | 176 ++++++++++++++++++
>> drivers/hwtracing/qcom/tgu.h | 51 +++++
>> 7 files changed, 260 insertions(+)
>> create mode 100644 Documentation/ABI/testing/sysfs-bus-amba-devices-tgu
>> 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
>>
>> diff --git a/Documentation/ABI/testing/sysfs-bus-amba-devices-tgu b/
>> Documentation/ABI/testing/sysfs-bus-amba-devices-tgu
>> new file mode 100644
>> index 000000000000..56ec3f5ab5d6
>> --- /dev/null
>> +++ b/Documentation/ABI/testing/sysfs-bus-amba-devices-tgu
>> @@ -0,0 +1,9 @@
>> +What: /sys/bus/amba/devices/<tgu-name>/enable_tgu
>> +Date: January 2026
>> +KernelVersion 6.19
>
> It's a bit too late for v6.19, this should rather be 6.20/7.0
Sure, Suzuki. Thanks for comment.>
> Suzuki
>
>
>> +Contact: Jinlong Mao <jinlong.mao@oss.qualcomm.com>, Songwei Chai
>> <songwei.chai@oss.qualcomm.com>
>> +Description:
>> + (RW) Set/Get the enable/disable status of TGU
>> + Accepts only one of the 2 values - 0 or 1.
>> + 0 : disable TGU.
>> + 1 : enable TGU.
>> diff --git a/drivers/Makefile b/drivers/Makefile
>> index ccc05f1eae3e..9608a3debb1f 100644
>> --- a/drivers/Makefile
>> +++ b/drivers/Makefile
>> @@ -177,6 +177,7 @@ obj-$(CONFIG_RAS) += ras/
>> obj-$(CONFIG_USB4) += thunderbolt/
>> obj-$(CONFIG_CORESIGHT) += hwtracing/coresight/
>> obj-y += hwtracing/intel_th/
>> +obj-y += hwtracing/qcom/
>> obj-$(CONFIG_STM) += hwtracing/stm/
>> obj-$(CONFIG_HISI_PTT) += hwtracing/ptt/
>> obj-y += android/
>> diff --git a/drivers/hwtracing/Kconfig b/drivers/hwtracing/Kconfig
>> index 911ee977103c..8a640218eed8 100644
>> --- a/drivers/hwtracing/Kconfig
>> +++ b/drivers/hwtracing/Kconfig
>> @@ -7,4 +7,6 @@ source "drivers/hwtracing/intel_th/Kconfig"
>> source "drivers/hwtracing/ptt/Kconfig"
>> +source "drivers/hwtracing/qcom/Kconfig"
>> +
>> endmenu
>> diff --git a/drivers/hwtracing/qcom/Kconfig b/drivers/hwtracing/qcom/
>> Kconfig
>> new file mode 100644
>> index 000000000000..d6f6d4b0f28e
>> --- /dev/null
>> +++ b/drivers/hwtracing/qcom/Kconfig
>> @@ -0,0 +1,18 @@
>> +# SPDX-License-Identifier: GPL-2.0-only
>> +#
>> +# QCOM specific hwtracing drivers
>> +#
>> +menu "Qualcomm specific hwtracing drivers"
>> +
>> +config QCOM_TGU
>> + tristate "QCOM Trigger Generation Unit driver"
>> + help
>> + This driver provides support for Trigger Generation Unit that is
>> + used to detect patterns or sequences on a given set of signals.
>> + TGU is used to monitor a particular bus within a given region to
>> + detect illegal transaction sequences or slave responses. It is
>> also
>> + used to monitor a data stream to detect protocol violations and to
>> + provide a trigger point for centering data around a specific event
>> + within the trace data buffer.
>> +
>> +endmenu
>> diff --git a/drivers/hwtracing/qcom/Makefile b/drivers/hwtracing/qcom/
>> Makefile
>> new file mode 100644
>> index 000000000000..5a0a868c1ea0
>> --- /dev/null
>> +++ b/drivers/hwtracing/qcom/Makefile
>> @@ -0,0 +1,3 @@
>> +# SPDX-License-Identifier: GPL-2.0
>> +
>> +obj-$(CONFIG_QCOM_TGU) += tgu.o
>> diff --git a/drivers/hwtracing/qcom/tgu.c b/drivers/hwtracing/qcom/tgu.c
>> new file mode 100644
>> index 000000000000..c5b2b384e6ae
>> --- /dev/null
>> +++ b/drivers/hwtracing/qcom/tgu.c
>> @@ -0,0 +1,176 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
>> + */
>> +
>> +#include <linux/amba/bus.h>
>> +#include <linux/device.h>
>> +#include <linux/err.h>
>> +#include <linux/io.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/pm_runtime.h>
>> +
>> +#include "tgu.h"
>> +
>> +static void tgu_write_all_hw_regs(struct tgu_drvdata *drvdata)
>> +{
>> + TGU_UNLOCK(drvdata->base);
>> + /* Enable TGU to program the triggers */
>> + writel(1, drvdata->base + TGU_CONTROL);
>> + TGU_LOCK(drvdata->base);
>> +}
>> +
>> +static int tgu_enable(struct device *dev)
>> +{
>> + struct tgu_drvdata *drvdata = dev_get_drvdata(dev);
>> +
>> + guard(spinlock)(&drvdata->lock);
>> + if (drvdata->enable)
>> + return -EBUSY;
>> +
>> + tgu_write_all_hw_regs(drvdata);
>> + drvdata->enable = true;
>> +
>> + return 0;
>> +}
>> +
>> +static void tgu_disable(struct device *dev)
>> +{
>> + struct tgu_drvdata *drvdata = dev_get_drvdata(dev);
>> +
>> + guard(spinlock)(&drvdata->lock);
>> + if (drvdata->enable) {
>> + TGU_UNLOCK(drvdata->base);
>> + writel(0, drvdata->base + TGU_CONTROL);
>> + TGU_LOCK(drvdata->base);
>> +
>> + drvdata->enable = false;
>> + }
>> +}
>> +
>> +static ssize_t enable_tgu_show(struct device *dev,
>> + struct device_attribute *attr, char *buf)
>> +{
>> + struct tgu_drvdata *drvdata = dev_get_drvdata(dev);
>> + bool enabled;
>> +
>> + guard(spinlock)(&drvdata->lock);
>> + enabled = drvdata->enable;
>> +
>> + return sysfs_emit(buf, "%d\n", enabled ? 1 : 0);
>> +}
>> +
>> +/* enable_tgu_store - Configure Trace and Gating Unit (TGU) triggers. */
>> +static ssize_t enable_tgu_store(struct device *dev,
>> + struct device_attribute *attr,
>> + const char *buf,
>> + size_t size)
>> +{
>> + unsigned long val;
>> + int ret = 0;
>> +
>> + ret = kstrtoul(buf, 0, &val);
>> + if (ret)
>> + return ret;
>> +
>> + if (val) {
>> + ret = pm_runtime_resume_and_get(dev);
>> + if (ret)
>> + return ret;
>> + ret = tgu_enable(dev);
>> + if (ret) {
>> + pm_runtime_put(dev);
>> + return ret;
>> + }
>> + } else {
>> + tgu_disable(dev);
>> + pm_runtime_put(dev);
>> + }
>> +
>> + return size;
>> +}
>> +static DEVICE_ATTR_RW(enable_tgu);
>> +
>> +static struct attribute *tgu_common_attrs[] = {
>> + &dev_attr_enable_tgu.attr,
>> + NULL,
>> +};
>> +
>> +static const struct attribute_group tgu_common_grp = {
>> + .attrs = tgu_common_attrs,
>> + NULL,
>> +};
>> +
>> +static const struct attribute_group *tgu_attr_groups[] = {
>> + &tgu_common_grp,
>> + NULL,
>> +};
>> +
>> +static int tgu_probe(struct amba_device *adev, const struct amba_id *id)
>> +{
>> + struct device *dev = &adev->dev;
>> + struct tgu_drvdata *drvdata;
>> + int ret;
>> +
>> + drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
>> + if (!drvdata)
>> + return -ENOMEM;
>> +
>> + drvdata->dev = &adev->dev;
>> + dev_set_drvdata(dev, drvdata);
>> +
>> + drvdata->base = devm_ioremap_resource(dev, &adev->res);
>> + if (IS_ERR(drvdata->base))
>> + return PTR_ERR(drvdata->base);
>> +
>> + spin_lock_init(&drvdata->lock);
>> +
>> + ret = sysfs_create_groups(&dev->kobj, tgu_attr_groups);
>> + if (ret) {
>> + dev_err(dev, "failed to create sysfs groups: %d\n", ret);
>> + return ret;
>> + }
>> +
>> + drvdata->enable = false;
>> +
>> + pm_runtime_put(&adev->dev);
>> + return 0;
>> +}
>> +
>> +static void tgu_remove(struct amba_device *adev)
>> +{
>> + struct device *dev = &adev->dev;
>> +
>> + sysfs_remove_groups(&dev->kobj, tgu_attr_groups);
>> +
>> + tgu_disable(dev);
>> +}
>> +
>> +static const struct amba_id tgu_ids[] = {
>> + {
>> + .id = 0x000f0e00,
>> + .mask = 0x000fffff,
>> + },
>> + { 0, 0, NULL },
>> +};
>> +
>> +MODULE_DEVICE_TABLE(amba, tgu_ids);
>> +
>> +static struct amba_driver tgu_driver = {
>> + .drv = {
>> + .name = "qcom-tgu",
>> + .suppress_bind_attrs = true,
>> + },
>> + .probe = tgu_probe,
>> + .remove = tgu_remove,
>> + .id_table = tgu_ids,
>> +};
>> +
>> +module_amba_driver(tgu_driver);
>> +
>> +MODULE_AUTHOR("Songwei Chai <songwei.chai@oss.qualcomm.com>");
>> +MODULE_AUTHOR("Jinlong Mao <jinlong.mao@oss.qualcomm.com>");
>> +MODULE_DESCRIPTION("Qualcomm Trigger Generation Unit driver");
>> +MODULE_LICENSE("GPL");
>> diff --git a/drivers/hwtracing/qcom/tgu.h b/drivers/hwtracing/qcom/tgu.h
>> new file mode 100644
>> index 000000000000..b11cfb28261d
>> --- /dev/null
>> +++ b/drivers/hwtracing/qcom/tgu.h
>> @@ -0,0 +1,51 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/*
>> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
>> + */
>> +
>> +#ifndef _QCOM_TGU_H
>> +#define _QCOM_TGU_H
>> +
>> +/* Register addresses */
>> +#define TGU_CONTROL 0x0000
>> +#define TGU_LAR 0xfb0
>> +#define TGU_UNLOCK_OFFSET 0xc5acce55
>> +
>> +static inline void TGU_LOCK(void __iomem *addr)
>> +{
>> + do {
>> + /* Wait for things to settle */
>> + mb();
>> + writel_relaxed(0x0, addr + TGU_LAR);
>> + } while (0);
>> +}
>> +
>> +static inline void TGU_UNLOCK(void __iomem *addr)
>> +{
>> + do {
>> + writel_relaxed(TGU_UNLOCK_OFFSET, addr + TGU_LAR);
>> + /* Make sure everyone has seen this */
>> + mb();
>> + } while (0);
>> +}
>> +
>> +/**
>> + * struct tgu_drvdata - Data structure for a TGU (Trigger Generator
>> Unit)
>> + * @base: Memory-mapped base address of the TGU device
>> + * @dev: Pointer to the associated device structure
>> + * @lock: Spinlock for handling concurrent access
>> + * @enable: Flag indicating whether the TGU device is enabled
>> + *
>> + * This structure defines the data associated with a TGU device,
>> + * including its base address, device pointers, clock, spinlock for
>> + * synchronization, trigger data pointers, maximum limits for various
>> + * trigger-related parameters, and enable status.
>> + */
>> +struct tgu_drvdata {
>> + void __iomem *base;
>> + struct device *dev;
>> + spinlock_t lock;
>> + bool enable;
>> +};
>> +
>> +#endif
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v10 0/7] Provide support for Trigger Generation Unit
2026-01-09 2:11 [PATCH v10 0/7] Provide support for Trigger Generation Unit Songwei Chai
` (6 preceding siblings ...)
2026-01-09 2:11 ` [PATCH v10 7/7] qcom-tgu: Add reset node to initialize Songwei Chai
@ 2026-01-13 1:53 ` Songwei Chai
7 siblings, 0 replies; 29+ messages in thread
From: Songwei Chai @ 2026-01-13 1:53 UTC (permalink / raw)
To: andersson, alexander.shishkin, mike.leach, suzuki.poulose,
james.clark, krzk+dt, conor+dt
Cc: linux-kernel, linux-arm-kernel, linux-arm-msm, coresight,
devicetree, gregkh
Hi Bjorn,
Currently the TGU driver code has been relocated to
'drivers/hwtracing/qcom' and fully decoupled from the Coresight subsystem.
Could you kindly review the current implementation and share your
comments from maintainer's perspective? Your feedback will help us
identify any issues and accelerate the upstream progress.
Thank you for your time and support.
BRs,
Songwei
On 1/9/2026 10:11 AM, 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-kcaj1qO4ZcGnA@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 V9: https://lore.kernel.org/all/20251219065902.2296896-1-songwei.chai@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@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@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@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@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@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@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@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@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 | 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 | 709 ++++++++++++++++++
> drivers/hwtracing/qcom/tgu.h | 272 +++++++
> 8 files changed, 1148 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
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v10 2/7] qcom-tgu: Add TGU driver
2026-01-09 2:11 ` [PATCH v10 2/7] qcom-tgu: Add TGU driver Songwei Chai
2026-01-09 10:27 ` Suzuki K Poulose
2026-01-09 11:28 ` Jie Gan
@ 2026-01-13 10:33 ` Konrad Dybcio
2026-01-27 2:13 ` Songwei Chai
2 siblings, 1 reply; 29+ messages in thread
From: Konrad Dybcio @ 2026-01-13 10:33 UTC (permalink / raw)
To: Songwei Chai, andersson, alexander.shishkin, mike.leach,
suzuki.poulose, james.clark, krzk+dt, conor+dt
Cc: linux-kernel, linux-arm-kernel, linux-arm-msm, coresight,
devicetree, gregkh
On 1/9/26 3:11 AM, Songwei Chai wrote:
> Add driver to support device TGU (Trigger Generation Unit).
> TGU is a Data Engine which can be utilized to sense a plurality of
> signals and create a trigger into the CTI or generate interrupts to
> processors. Add probe/enable/disable functions for tgu.
>
> Signed-off-by: Songwei Chai <songwei.chai@oss.qualcomm.com>
> ---
[...]
> +static void tgu_disable(struct device *dev)
> +{
> + struct tgu_drvdata *drvdata = dev_get_drvdata(dev);
> +
> + guard(spinlock)(&drvdata->lock);
> + if (drvdata->enable) {
if (!drvdata->enabled)
return
> + TGU_UNLOCK(drvdata->base);
> + writel(0, drvdata->base + TGU_CONTROL);
> + TGU_LOCK(drvdata->base);
> +
> + drvdata->enable = false;
> + }
> +}
> +
> +static ssize_t enable_tgu_show(struct device *dev,
'enabled', in all references to this, please
> + struct device_attribute *attr, char *buf)
> +{
> + struct tgu_drvdata *drvdata = dev_get_drvdata(dev);
> + bool enabled;
> +
> + guard(spinlock)(&drvdata->lock);
> + enabled = drvdata->enable;
> +
> + return sysfs_emit(buf, "%d\n", enabled ? 1 : 0);
!!enabled
> +}
> +
> +/* enable_tgu_store - Configure Trace and Gating Unit (TGU) triggers. */
> +static ssize_t enable_tgu_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf,
> + size_t size)
> +{
> + unsigned long val;
> + int ret = 0;
Drop the initialization, you override it immediately a line below
[...]
> +static int tgu_probe(struct amba_device *adev, const struct amba_id *id)
> +{
> + struct device *dev = &adev->dev;
> + struct tgu_drvdata *drvdata;
> + int ret;
> +
> + drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
> + if (!drvdata)
> + return -ENOMEM;
> +
> + drvdata->dev = &adev->dev;
> + dev_set_drvdata(dev, drvdata);
> +
> + drvdata->base = devm_ioremap_resource(dev, &adev->res);
> + if (IS_ERR(drvdata->base))
> + return PTR_ERR(drvdata->base);
> +
> + spin_lock_init(&drvdata->lock);
> +
> + ret = sysfs_create_groups(&dev->kobj, tgu_attr_groups);
> + if (ret) {
> + dev_err(dev, "failed to create sysfs groups: %d\n", ret);
> + return ret;
> + }
> +
> + drvdata->enable = false;
> +
> + pm_runtime_put(&adev->dev);
> + return 0;
Please keep a consistent \n above return as you did in all other places
throughout this file
[...]
> +/* Register addresses */
> +#define TGU_CONTROL 0x0000
> +#define TGU_LAR 0xfb0
> +#define TGU_UNLOCK_OFFSET 0xc5acce55
Please align the values (it's easier with tabs)
> +
> +static inline void TGU_LOCK(void __iomem *addr)
> +{
> + do {
> + /* Wait for things to settle */
> + mb();
What are we waiting for here?
> + writel_relaxed(0x0, addr + TGU_LAR);
If you do a prompt TGU_LOCK()-TGU_UNLOCK() the writes may arrive in
the order opposite to what you want, I'd say this shouldn't be _relaxed()
and we should probably have a readback here to make sure the effect has
taken place immediately
> + } while (0);
> +}
> +
> +static inline void TGU_UNLOCK(void __iomem *addr)
> +{
> + do {
> + writel_relaxed(TGU_UNLOCK_OFFSET, addr + TGU_LAR);
> + /* Make sure everyone has seen this */
> + mb();
I believe this should be a readback instead
> + } while (0);
> +}
> +
> +/**
> + * struct tgu_drvdata - Data structure for a TGU (Trigger Generator Unit)
> + * @base: Memory-mapped base address of the TGU device
> + * @dev: Pointer to the associated device structure
> + * @lock: Spinlock for handling concurrent access
Concurrent access to what? (presumably the TGU?)
> + * @enable: Flag indicating whether the TGU device is enabled
> + *
> + * This structure defines the data associated with a TGU device,
> + * including its base address, device pointers, clock, spinlock for
> + * synchronization, trigger data pointers, maximum limits for various
> + * trigger-related parameters, and enable status.
> + */
> +struct tgu_drvdata {
> + void __iomem *base;
> + struct device *dev;
> + spinlock_t lock;
> + bool enable;
"enabled", please
Konrad
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v10 3/7] qcom-tgu: Add signal priority support
2026-01-09 2:11 ` [PATCH v10 3/7] qcom-tgu: Add signal priority support Songwei Chai
@ 2026-01-13 11:09 ` Konrad Dybcio
2026-01-27 2:23 ` Songwei Chai
0 siblings, 1 reply; 29+ messages in thread
From: Konrad Dybcio @ 2026-01-13 11:09 UTC (permalink / raw)
To: Songwei Chai, andersson, alexander.shishkin, mike.leach,
suzuki.poulose, james.clark, krzk+dt, conor+dt
Cc: linux-kernel, linux-arm-kernel, linux-arm-msm, coresight,
devicetree, gregkh
On 1/9/26 3:11 AM, 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.
>
> Signed-off-by: Songwei Chai <songwei.chai@oss.qualcomm.com>
> ---
[...]
> +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;
> +
> + ret = kstrtoul(buf, 0, &val);
> + if (ret)
> + return ret;
> +
> + guard(spinlock)(&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;
Style: some functions have a \n before return, some don't. The former
is preferred
> +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);
> + int ret = SYSFS_GROUP_INVISIBLE;
> +
> + if (tgu_attr->step_index < drvdata->max_step) {
> + ret = (tgu_attr->reg_num < drvdata->max_reg) ?
> + attr->mode : 0;
> + }
> + return ret;
This is very convoluted
How about:
if (tgu_attr->step_index >= drvdata->max_step)
return 0;
if (tgu_attr->reg_num >= drvdata->max_reg)
return 0;
return attr->mode;
?
[...]
> +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);
> + if (((num_sense_input * NUMBER_BITS_EACH_SIGNAL) % LENGTH_REGISTER) == 0)
> + num_reg = (num_sense_input * NUMBER_BITS_EACH_SIGNAL) / LENGTH_REGISTER;
> + else
> + num_reg = ((num_sense_input * NUMBER_BITS_EACH_SIGNAL) / LENGTH_REGISTER) + 1;
num_reg = base case
if (num_sense_input * NUMBER_BITS_EACH_SIGNAL) % LENGTH_REGISTER)
num_reg++;
?
[...]
> @@ -112,6 +250,8 @@ static int tgu_probe(struct amba_device *adev, const struct amba_id *id)
> {
> struct device *dev = &adev->dev;
> struct tgu_drvdata *drvdata;
> + size_t priority_size;
> + unsigned int *priority;
reverse-Christmas-tree would be nice
> int ret;
>
> drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
> @@ -127,12 +267,32 @@ static int tgu_probe(struct amba_device *adev, const struct amba_id *id)
>
> spin_lock_init(&drvdata->lock);
>
> + tgu_set_reg_number(drvdata);
> + tgu_set_steps(drvdata);
> +
> ret = sysfs_create_groups(&dev->kobj, tgu_attr_groups);
> if (ret) {
> dev_err(dev, "failed to create sysfs groups: %d\n", ret);
> return ret;
> }
>
> + drvdata->value_table =
> + devm_kzalloc(dev, sizeof(*drvdata->value_table), GFP_KERNEL);
> + if (!drvdata->value_table)
> + return -ENOMEM;
> +
> + priority_size = MAX_PRIORITY * drvdata->max_reg *
> + drvdata->max_step *
> + sizeof(*(drvdata->value_table->priority));
> +
> + priority = devm_kzalloc(dev, priority_size, GFP_KERNEL);
> +
> + if (!priority)
stray \n above
[...]
> +#define BMVAL(val, lsb, msb) ((val & GENMASK(msb, lsb)) >> lsb)
> +#define TGU_DEVID_SENSE_INPUT(devid_val) ((int) BMVAL(devid_val, 10, 17))
> +#define TGU_DEVID_STEPS(devid_val) ((int)BMVAL(devid_val, 3, 6))
> +#define NUMBER_BITS_EACH_SIGNAL 4
"TGU_BITS_PER_SIGNAL"
Konrad
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v10 4/7] qcom-tgu: Add TGU decode support
2026-01-09 2:11 ` [PATCH v10 4/7] qcom-tgu: Add TGU decode support Songwei Chai
@ 2026-01-13 11:13 ` Konrad Dybcio
2026-01-27 2:34 ` Songwei Chai
0 siblings, 1 reply; 29+ messages in thread
From: Konrad Dybcio @ 2026-01-13 11:13 UTC (permalink / raw)
To: Songwei Chai, andersson, alexander.shishkin, mike.leach,
suzuki.poulose, james.clark, krzk+dt, conor+dt
Cc: linux-kernel, linux-arm-kernel, linux-arm-msm, coresight,
devicetree, gregkh
On 1/9/26 3:11 AM, Songwei Chai wrote:
> 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.
>
> Signed-off-by: Songwei Chai <songwei.chai@oss.qualcomm.com>
> ---
[...]
> @@ -18,8 +18,36 @@ static int calculate_array_location(struct tgu_drvdata *drvdata,
> int step_index, int operation_index,
> int reg_index)
> {
> - return operation_index * (drvdata->max_step) * (drvdata->max_reg) +
> - step_index * (drvdata->max_reg) + reg_index;
I think this type of calculations could use a wrapper
> + int ret = -EINVAL;
> +
> + switch (operation_index) {
> + case TGU_PRIORITY0:
> + case TGU_PRIORITY1:
> + case TGU_PRIORITY2:
> + case TGU_PRIORITY3:
> + ret = operation_index * (drvdata->max_step) *
> + (drvdata->max_reg) +
> + step_index * (drvdata->max_reg) + reg_index;
> + break;
> + case TGU_CONDITION_DECODE:
> + ret = step_index * (drvdata->max_condition_decode) +
> + reg_index;
> + break;
> + default:
> + break;
> + }
> + return ret;
The only thing your switch statement is assign a value to ret and break
out. Change that to a direct return, and drop 'ret' altogether
> +}
> +
> +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, "%s - Fail\n", __func__);
Avoid __func__.
The error message is very non-descriptive
[...]
> static int tgu_enable(struct device *dev)
> {
> struct tgu_drvdata *drvdata = dev_get_drvdata(dev);
> + int ret = 0;
>
> guard(spinlock)(&drvdata->lock);
> if (drvdata->enable)
> return -EBUSY;
>
> - tgu_write_all_hw_regs(drvdata);
> + ret = tgu_write_all_hw_regs(drvdata);
> +
> + if (ret == -EINVAL)
stray \n
> + goto exit;
> +
> drvdata->enable = true;
>
> - return 0;
> +exit:
> + return ret;
ret = tgu_write_all_hw_regs(drvdata);
if (!ret)
drvdata->enable = true;
return ret
Konrad
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v10 5/7] qcom-tgu: Add support to configure next action
2026-01-09 2:11 ` [PATCH v10 5/7] qcom-tgu: Add support to configure next action Songwei Chai
@ 2026-01-13 11:15 ` Konrad Dybcio
2026-01-27 2:43 ` Songwei Chai
0 siblings, 1 reply; 29+ messages in thread
From: Konrad Dybcio @ 2026-01-13 11:15 UTC (permalink / raw)
To: Songwei Chai, andersson, alexander.shishkin, mike.leach,
suzuki.poulose, james.clark, krzk+dt, conor+dt
Cc: linux-kernel, linux-arm-kernel, linux-arm-msm, coresight,
devicetree, gregkh
On 1/9/26 3:11 AM, Songwei Chai wrote:
> Add "select" node for each step to determine if another step is taken,
> trigger(s) are generated, counters/timers incremented/decremented, etc.
>
> Signed-off-by: Songwei Chai <songwei.chai@oss.qualcomm.com>
> ---
[...]
> + case TGU_CONDITION_SELECT:
> + /* 'default' register is at the end of 'select' region */
> + if (tgu_attr->reg_num ==
> + drvdata->max_condition_select - 1)
> + attr->name = "default";
> + ret = (tgu_attr->reg_num <
> + drvdata->max_condition_select) ?
> + attr->mode : 0;
similarly to my previous comments
[...]
> + for (i = 0; i < drvdata->max_step; i++) {
> + for (j = 0; j < drvdata->max_condition_select; j++) {
> + index = check_array_location(drvdata, i,
> + TGU_CONDITION_SELECT, j);
> +
> + if (index == -EINVAL)
stray \n
> + goto exit;
> +
> + writel(drvdata->value_table->condition_select[index],
> + drvdata->base + CONDITION_SELECT_STEP(i, j));
> + }
> + }
> /* Enable TGU to program the triggers */
> writel(1, drvdata->base + TGU_CONTROL);
> exit:
> @@ -225,6 +258,8 @@ static void tgu_set_conditions(struct tgu_drvdata *drvdata)
>
> devid = readl(drvdata->base + TGU_DEVID);
> drvdata->max_condition_decode = TGU_DEVID_CONDITIONS(devid);
> + /* select region has an additional 'default' register */
> + drvdata->max_condition_select = TGU_DEVID_CONDITIONS(devid) + 1;
> }
>
> static int tgu_enable(struct device *dev)
> @@ -356,6 +391,14 @@ static const struct attribute_group *tgu_attr_groups[] = {
> CONDITION_DECODE_ATTRIBUTE_GROUP_INIT(5),
> CONDITION_DECODE_ATTRIBUTE_GROUP_INIT(6),
> CONDITION_DECODE_ATTRIBUTE_GROUP_INIT(7),
> + CONDITION_SELECT_ATTRIBUTE_GROUP_INIT(0),
> + CONDITION_SELECT_ATTRIBUTE_GROUP_INIT(1),
> + CONDITION_SELECT_ATTRIBUTE_GROUP_INIT(2),
> + CONDITION_SELECT_ATTRIBUTE_GROUP_INIT(3),
> + CONDITION_SELECT_ATTRIBUTE_GROUP_INIT(4),
> + CONDITION_SELECT_ATTRIBUTE_GROUP_INIT(5),
> + CONDITION_SELECT_ATTRIBUTE_GROUP_INIT(6),
> + CONDITION_SELECT_ATTRIBUTE_GROUP_INIT(7),
> NULL,
> };
>
> @@ -363,8 +406,8 @@ static int tgu_probe(struct amba_device *adev, const struct amba_id *id)
> {
> struct device *dev = &adev->dev;
> struct tgu_drvdata *drvdata;
> - size_t priority_size, condition_size;
> - unsigned int *priority, *condition;
> + size_t priority_size, condition_size, select_size;
> + unsigned int *priority, *condition, *select;
> int ret;
>
> drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
> @@ -417,6 +460,16 @@ static int tgu_probe(struct amba_device *adev, const struct amba_id *id)
>
> drvdata->value_table->condition_decode = condition;
>
> + select_size = drvdata->max_condition_select * drvdata->max_step *
> + sizeof(*(drvdata->value_table->condition_select));
> +
> + select = devm_kzalloc(dev, select_size, GFP_KERNEL);
> +
> + if (!select)
stray \n
> + return -ENOMEM;
> +
> + drvdata->value_table->condition_select = select;
I don't see a need for an intemediate variable here
[...]
> * @max_condition_decode: Maximum number of condition_decode
> + * @max_condition_select: Maximum number of condition_select
Maximum value, perhaps? You haven't explained the feature very well
so I'm not sure what this is supposed to reflect
Konrad
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v10 6/7] qcom-tgu: Add timer/counter functionality for TGU
2026-01-09 2:11 ` [PATCH v10 6/7] qcom-tgu: Add timer/counter functionality for TGU Songwei Chai
@ 2026-01-13 11:19 ` Konrad Dybcio
2026-01-27 3:02 ` Songwei Chai
0 siblings, 1 reply; 29+ messages in thread
From: Konrad Dybcio @ 2026-01-13 11:19 UTC (permalink / raw)
To: Songwei Chai, andersson, alexander.shishkin, mike.leach,
suzuki.poulose, james.clark, krzk+dt, conor+dt
Cc: linux-kernel, linux-arm-kernel, linux-arm-msm, coresight,
devicetree, gregkh
On 1/9/26 3:11 AM, Songwei Chai wrote:
> Add counter and timer node for each step which could be
> programed if they are to be utilized in trigger event/sequence.
>
> Signed-off-by: Songwei Chai <songwei.chai@oss.qualcomm.com>
> ---
[...]
> +static void tgu_set_timer_counter(struct tgu_drvdata *drvdata)
> +{
> + int num_timers, num_counters;
> + u32 devid2;
> +
> + devid2 = readl(drvdata->base + CORESIGHT_DEVID2);
> +
> + if (TGU_DEVID2_TIMER0(devid2) && TGU_DEVID2_TIMER1(devid2))
> + num_timers = 2;
> + else if (TGU_DEVID2_TIMER0(devid2) || TGU_DEVID2_TIMER1(devid2))
> + num_timers = 1;
> + else
> + num_timers = 0;
> +
> + if (TGU_DEVID2_COUNTER0(devid2) && TGU_DEVID2_COUNTER1(devid2))
> + num_counters = 2;
> + else if (TGU_DEVID2_COUNTER0(devid2) || TGU_DEVID2_COUNTER1(devid2))
> + num_counters = 1;
> + else
> + num_counters = 0;
> +
> + drvdata->max_timer = num_timers;
> + drvdata->max_counter = num_counters;
int num_timers = 0, num_counters = 0
if (TGU_DEVID2_TIMER0(devid2))
num_timers++
if (TGU_DEVID2_TIMER1(devid2))
num_timers++
etc.
unless you want to guard against a case where TIMER0 reports as absent
and TIMER1 as present and you consider that invalid (I don't know)
[...]
> + timer_size = drvdata->max_step * drvdata->max_timer *
> + sizeof(*(drvdata->value_table->timer));
> +
> + timer = devm_kzalloc(dev, timer_size, GFP_KERNEL);
> +
> + if (!timer)
stray \n
> + return -ENOMEM;
> +
> + drvdata->value_table->timer = timer;
> +
> + counter_size = drvdata->max_step * drvdata->max_counter *
> + sizeof(*(drvdata->value_table->counter));
> +
> + counter = devm_kzalloc(dev, counter_size, GFP_KERNEL);
devm_kcalloc, perhaps?
> +
> + if (!counter)
stray \n
> + return -ENOMEM;
> +
> + drvdata->value_table->counter = counter;
> +
> drvdata->enable = false;
>
> pm_runtime_put(&adev->dev);
> diff --git a/drivers/hwtracing/qcom/tgu.h b/drivers/hwtracing/qcom/tgu.h
> index 8c92e88d7e2c..94708750b02d 100644
> --- a/drivers/hwtracing/qcom/tgu.h
> +++ b/drivers/hwtracing/qcom/tgu.h
> @@ -11,11 +11,17 @@
> #define TGU_LAR 0xfb0
> #define TGU_UNLOCK_OFFSET 0xc5acce55
> #define TGU_DEVID 0xfc8
> +#define CORESIGHT_DEVID2 0xfc0
>
> #define BMVAL(val, lsb, msb) ((val & GENMASK(msb, lsb)) >> lsb)
This is NIH FIELD_GET()
[...]
> static inline void TGU_LOCK(void __iomem *addr)
> @@ -197,6 +247,8 @@ static inline void TGU_UNLOCK(void __iomem *addr)
> * @max_step: Maximum step size
> * @max_condition_decode: Maximum number of condition_decode
> * @max_condition_select: Maximum number of condition_select
> + * @max_timer: Maximum number of timers
> + * @max_counter: Maximum number of counters
> *
> * This structure defines the data associated with a TGU device,
> * including its base address, device pointers, clock, spinlock for
> @@ -213,6 +265,8 @@ struct tgu_drvdata {
> int max_step;
> int max_condition_decode;
> int max_condition_select;
> + int max_timer;
> + int max_counter;
num_timers, num_counters definitely fits better here
Konrad
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v10 7/7] qcom-tgu: Add reset node to initialize
2026-01-09 2:11 ` [PATCH v10 7/7] qcom-tgu: Add reset node to initialize Songwei Chai
@ 2026-01-13 11:22 ` Konrad Dybcio
2026-01-27 2:50 ` Songwei Chai
0 siblings, 1 reply; 29+ messages in thread
From: Konrad Dybcio @ 2026-01-13 11:22 UTC (permalink / raw)
To: Songwei Chai, andersson, alexander.shishkin, mike.leach,
suzuki.poulose, james.clark, krzk+dt, conor+dt
Cc: linux-kernel, linux-arm-kernel, linux-arm-msm, coresight,
devicetree, gregkh
On 1/9/26 3:11 AM, 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@oss.qualcomm.com>
> ---
[...]
> +/* reset_tgu_store - Reset Trace and Gating Unit (TGU) configuration. */
> +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);
> + unsigned long value;
> + int i, j, ret;
> +
> + if (kstrtoul(buf, 0, &value) || value == 0)
> + return -EINVAL;
Your documentation blurb promises that only 1 is accepted, but this is not
the case. I think the previous additions had a similar flaw
> +
> + if (!drvdata->enable) {
I think this check needs to be made under a lock, otherwise something else
may pull the plug inbetween
> + ret = pm_runtime_get_sync(drvdata->dev);
> + if (ret < 0) {
> + pm_runtime_put(drvdata->dev);
> + return ret;
> + }
> + }
> +
> + guard(spinlock)(&drvdata->lock);
> + TGU_UNLOCK(drvdata->base);
> +
> + writel(0, drvdata->base + TGU_CONTROL);
> +
> + TGU_LOCK(drvdata->base);
This is tgu_disable()
> +
> + if (drvdata->value_table->priority)
> + memset(drvdata->value_table->priority, 0,
> + MAX_PRIORITY * drvdata->max_step *
> + drvdata->max_reg * sizeof(unsigned int));
> +
> + if (drvdata->value_table->condition_decode)
> + memset(drvdata->value_table->condition_decode, 0,
> + drvdata->max_condition_decode * drvdata->max_step *
> + sizeof(unsigned int));
> +
> + /* Initialize all condition registers to NOT(value=0x1000000) */
One \t too much
> + for (i = 0; i < drvdata->max_step; i++) {
> + for (j = 0; j < drvdata->max_condition_decode; j++) {
> + drvdata->value_table
> + ->condition_decode[calculate_array_location(
> + drvdata, i, TGU_CONDITION_DECODE, j)] =
> + 0x1000000;
This is unreadable, take a pointer to condition_decode[]
> + }
> + }
> +
> + if (drvdata->value_table->condition_select)
> + memset(drvdata->value_table->condition_select, 0,
> + drvdata->max_condition_select * drvdata->max_step *
> + sizeof(unsigned int));
> +
> + if (drvdata->value_table->timer)
> + memset(drvdata->value_table->timer, 0,
> + (drvdata->max_step) *
> + (drvdata->max_timer) *
> + sizeof(unsigned int));
> +
> + if (drvdata->value_table->counter)
> + memset(drvdata->value_table->counter, 0,
> + (drvdata->max_step) *
> + (drvdata->max_counter) *
> + sizeof(unsigned int));
This is similarly difficult to read with almost random indentation
Konrad
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v10 2/7] qcom-tgu: Add TGU driver
2026-01-13 10:33 ` Konrad Dybcio
@ 2026-01-27 2:13 ` Songwei Chai
2026-01-27 10:35 ` Konrad Dybcio
0 siblings, 1 reply; 29+ messages in thread
From: Songwei Chai @ 2026-01-27 2:13 UTC (permalink / raw)
To: Konrad Dybcio, andersson, alexander.shishkin, mike.leach,
suzuki.poulose, james.clark, krzk+dt, conor+dt
Cc: linux-kernel, linux-arm-kernel, linux-arm-msm, coresight,
devicetree, gregkh
Hi Konrad,
Sorry for the late reply.
On 1/13/2026 6:33 PM, Konrad Dybcio wrote:
> On 1/9/26 3:11 AM, Songwei Chai wrote:
>> Add driver to support device TGU (Trigger Generation Unit).
>> TGU is a Data Engine which can be utilized to sense a plurality of
>> signals and create a trigger into the CTI or generate interrupts to
>> processors. Add probe/enable/disable functions for tgu.
>>
>> Signed-off-by: Songwei Chai <songwei.chai@oss.qualcomm.com>
>> ---
>
> [...]
>
>> +static void tgu_disable(struct device *dev)
>> +{
>> + struct tgu_drvdata *drvdata = dev_get_drvdata(dev);
>> +
>> + guard(spinlock)(&drvdata->lock);
>> + if (drvdata->enable) {
>
> if (!drvdata->enabled)
> return
>
Marked, will update.
>
>> + TGU_UNLOCK(drvdata->base);
>> + writel(0, drvdata->base + TGU_CONTROL);
>> + TGU_LOCK(drvdata->base);
>> +
>> + drvdata->enable = false;
>> + }
>> +}
>> +
>> +static ssize_t enable_tgu_show(struct device *dev,
>
> 'enabled', in all references to this, please
sure.
>
>> + struct device_attribute *attr, char *buf)
>> +{
>> + struct tgu_drvdata *drvdata = dev_get_drvdata(dev);
>> + bool enabled;
>> +
>> + guard(spinlock)(&drvdata->lock);
>> + enabled = drvdata->enable;
>> +
>> + return sysfs_emit(buf, "%d\n", enabled ? 1 : 0);
>
> !!enabled
Marked, will update.
>
>
>> +}
>> +
>> +/* enable_tgu_store - Configure Trace and Gating Unit (TGU) triggers. */
>> +static ssize_t enable_tgu_store(struct device *dev,
>> + struct device_attribute *attr,
>> + const char *buf,
>> + size_t size)
>> +{
>> + unsigned long val;
>> + int ret = 0;
>
> Drop the initialization, you override it immediately a line below
Sure.
>
> [...]
>
>> +static int tgu_probe(struct amba_device *adev, const struct amba_id *id)
>> +{
>> + struct device *dev = &adev->dev;
>> + struct tgu_drvdata *drvdata;
>> + int ret;
>> +
>> + drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
>> + if (!drvdata)
>> + return -ENOMEM;
>> +
>> + drvdata->dev = &adev->dev;
>> + dev_set_drvdata(dev, drvdata);
>> +
>> + drvdata->base = devm_ioremap_resource(dev, &adev->res);
>> + if (IS_ERR(drvdata->base))
>> + return PTR_ERR(drvdata->base);
>> +
>> + spin_lock_init(&drvdata->lock);
>> +
>> + ret = sysfs_create_groups(&dev->kobj, tgu_attr_groups);
>> + if (ret) {
>> + dev_err(dev, "failed to create sysfs groups: %d\n", ret);
>> + return ret;
>> + }
>> +
>> + drvdata->enable = false;
>> +
>> + pm_runtime_put(&adev->dev);
>> + return 0;
>
> Please keep a consistent \n above return as you did in all other places
> throughout this file
Sure.
>
> [...]
>
>> +/* Register addresses */
>> +#define TGU_CONTROL 0x0000
>> +#define TGU_LAR 0xfb0
>> +#define TGU_UNLOCK_OFFSET 0xc5acce55
>
> Please align the values (it's easier with tabs)
>
Sure.
>> +
>> +static inline void TGU_LOCK(void __iomem *addr)
>> +{
>> + do {
>> + /* Wait for things to settle */
>> + mb();
>
> What are we waiting for here?
>
>> + writel_relaxed(0x0, addr + TGU_LAR);
>
> If you do a prompt TGU_LOCK()-TGU_UNLOCK() the writes may arrive in
> the order opposite to what you want, I'd say this shouldn't be _relaxed()
> and we should probably have a readback here to make sure the effect has
> taken place immediately
>
>> + } while (0);
>> +}
>> +
>> +static inline void TGU_UNLOCK(void __iomem *addr)
>> +{
>> + do {
>> + writel_relaxed(TGU_UNLOCK_OFFSET, addr + TGU_LAR);
>> + /* Make sure everyone has seen this */
>> + mb();
>
> I believe this should be a readback instead
>
>> + } while (0);
>> +}
This lock/unlock sequence is intentionally modelled after the existing
CoreSight CS_LOCK/CS_UNLOCK helpers, which have been in mainline for a
long time and are widely used on ARM systems.
The barriers here are meant to provide CPU-side ordering guarantees
around the LAR access rather than to wait for the hardware lock/unlock
to complete. In particular, the intent is to prevent configuration
accesses from being reordered across the lock/unlock boundary, matching
the CoreSight programming model.
I agree that the comments may be misleading in that regard, and I can
update them to clarify the ordering intent.
If you still prefer a stricter write + readback sequence here, I’m also
happy to switch to that for additional conservatism.
>> +
>> +/**
>> + * struct tgu_drvdata - Data structure for a TGU (Trigger Generator Unit)
>> + * @base: Memory-mapped base address of the TGU device
>> + * @dev: Pointer to the associated device structure
>> + * @lock: Spinlock for handling concurrent access
>
> Concurrent access to what? (presumably the TGU?)
Concurrent access to private data, such as drvdata->enabled here.
I will update the comment to avoid the confusion.
>
>> + * @enable: Flag indicating whether the TGU device is enabled
>> + *
>> + * This structure defines the data associated with a TGU device,
>> + * including its base address, device pointers, clock, spinlock for
>> + * synchronization, trigger data pointers, maximum limits for various
>> + * trigger-related parameters, and enable status.
>> + */
>> +struct tgu_drvdata {
>> + void __iomem *base;
>> + struct device *dev;
>> + spinlock_t lock;
>> + bool enable;
>
> "enabled", please
sure.
>
> Konrad
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v10 3/7] qcom-tgu: Add signal priority support
2026-01-13 11:09 ` Konrad Dybcio
@ 2026-01-27 2:23 ` Songwei Chai
2026-01-27 10:30 ` Konrad Dybcio
0 siblings, 1 reply; 29+ messages in thread
From: Songwei Chai @ 2026-01-27 2:23 UTC (permalink / raw)
To: Konrad Dybcio, andersson, alexander.shishkin, mike.leach,
suzuki.poulose, james.clark, krzk+dt, conor+dt
Cc: linux-kernel, linux-arm-kernel, linux-arm-msm, coresight,
devicetree, gregkh
On 1/13/2026 7:09 PM, Konrad Dybcio wrote:
> On 1/9/26 3:11 AM, 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.
>>
>> Signed-off-by: Songwei Chai <songwei.chai@oss.qualcomm.com>
>> ---
>
> [...]
>
>> +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;
>> +
>> + ret = kstrtoul(buf, 0, &val);
>> + if (ret)
>> + return ret;
>> +
>> + guard(spinlock)(&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;
>
> Style: some functions have a \n before return, some don't. The former
> is preferred
Sure, Konrad.
I will update and pay more attention next time.
>
>> +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);
>> + int ret = SYSFS_GROUP_INVISIBLE;
>> +
>> + if (tgu_attr->step_index < drvdata->max_step) {
>> + ret = (tgu_attr->reg_num < drvdata->max_reg) ?
>> + attr->mode : 0;
>> + }
>> + return ret;
>
> This is very convoluted
>
> How about:
>
> if (tgu_attr->step_index >= drvdata->max_step)
> return 0;
>
> if (tgu_attr->reg_num >= drvdata->max_reg)
> return 0;
>
> return attr->mode;
>
> ?
>
> [...]
>
I agree that the expanded form is clearer step-by-step, but I
intentionally kept the current version as it keeps the bounds checks
localized and avoids multiple early returns in this simple case.
I find the conditional expression still reasonably readable here, but
I’m happy to revisit this if you feel strongly about the style.
>> +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);
>> + if (((num_sense_input * NUMBER_BITS_EACH_SIGNAL) % LENGTH_REGISTER) == 0)
>> + num_reg = (num_sense_input * NUMBER_BITS_EACH_SIGNAL) / LENGTH_REGISTER;
>> + else
>> + num_reg = ((num_sense_input * NUMBER_BITS_EACH_SIGNAL) / LENGTH_REGISTER) + 1;
>
> num_reg = base case
>
> if (num_sense_input * NUMBER_BITS_EACH_SIGNAL) % LENGTH_REGISTER)
> num_reg++;
>
> ?
>
Marked, will update.
> [...]
>
>> @@ -112,6 +250,8 @@ static int tgu_probe(struct amba_device *adev, const struct amba_id *id)
>> {
>> struct device *dev = &adev->dev;
>> struct tgu_drvdata *drvdata;
>> + size_t priority_size;
>> + unsigned int *priority;
>
> reverse-Christmas-tree would be nice
>
>
Marked, will update.
>> int ret;
>>
>> drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
>> @@ -127,12 +267,32 @@ static int tgu_probe(struct amba_device *adev, const struct amba_id *id)
>>
>> spin_lock_init(&drvdata->lock);
>>
>> + tgu_set_reg_number(drvdata);
>> + tgu_set_steps(drvdata);
>> +
>> ret = sysfs_create_groups(&dev->kobj, tgu_attr_groups);
>> if (ret) {
>> dev_err(dev, "failed to create sysfs groups: %d\n", ret);
>> return ret;
>> }
>>
>> + drvdata->value_table =
>> + devm_kzalloc(dev, sizeof(*drvdata->value_table), GFP_KERNEL);
>> + if (!drvdata->value_table)
>> + return -ENOMEM;
>> +
>> + priority_size = MAX_PRIORITY * drvdata->max_reg *
>> + drvdata->max_step *
>> + sizeof(*(drvdata->value_table->priority));
>> +
>> + priority = devm_kzalloc(dev, priority_size, GFP_KERNEL);
>> +
>> + if (!priority)
>
> stray \n above
Marked, will update.
>
> [...]
>
>> +#define BMVAL(val, lsb, msb) ((val & GENMASK(msb, lsb)) >> lsb)
>> +#define TGU_DEVID_SENSE_INPUT(devid_val) ((int) BMVAL(devid_val, 10, 17))
>> +#define TGU_DEVID_STEPS(devid_val) ((int)BMVAL(devid_val, 3, 6))
>> +#define NUMBER_BITS_EACH_SIGNAL 4
>
> "TGU_BITS_PER_SIGNAL"
Marked, will update.
>
> Konrad
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v10 4/7] qcom-tgu: Add TGU decode support
2026-01-13 11:13 ` Konrad Dybcio
@ 2026-01-27 2:34 ` Songwei Chai
2026-01-27 10:31 ` Konrad Dybcio
0 siblings, 1 reply; 29+ messages in thread
From: Songwei Chai @ 2026-01-27 2:34 UTC (permalink / raw)
To: Konrad Dybcio, andersson, alexander.shishkin, mike.leach,
suzuki.poulose, james.clark, krzk+dt, conor+dt
Cc: linux-kernel, linux-arm-kernel, linux-arm-msm, coresight,
devicetree, gregkh
On 1/13/2026 7:13 PM, Konrad Dybcio wrote:
> On 1/9/26 3:11 AM, Songwei Chai wrote:
>> 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.
>>
>> Signed-off-by: Songwei Chai <songwei.chai@oss.qualcomm.com>
>> ---
>
> [...]
>
>> @@ -18,8 +18,36 @@ static int calculate_array_location(struct tgu_drvdata *drvdata,
>> int step_index, int operation_index,
>> int reg_index)
>> {
>> - return operation_index * (drvdata->max_step) * (drvdata->max_reg) +
>> - step_index * (drvdata->max_reg) + reg_index;
>
> I think this type of calculations could use a wrapper
Agree, this calculation is already wrapped in the calculate_array_location.
>
>> + int ret = -EINVAL;
>> +
>> + switch (operation_index) {
>> + case TGU_PRIORITY0:
>> + case TGU_PRIORITY1:
>> + case TGU_PRIORITY2:
>> + case TGU_PRIORITY3:
>> + ret = operation_index * (drvdata->max_step) *
>> + (drvdata->max_reg) +
>> + step_index * (drvdata->max_reg) + reg_index;
>> + break;
>> + case TGU_CONDITION_DECODE:
>> + ret = step_index * (drvdata->max_condition_decode) +
>> + reg_index;
>> + break;
>> + default:
>> + break;
>> + }
>> + return ret;
>
> The only thing your switch statement is assign a value to ret and break
> out. Change that to a direct return, and drop 'ret' altogether
>
I kept a single return intentionally so the function has a single exit
point. This makes it easier to extend with common post-processing or
debug logic later if needed.
That said, I’m fine switching to direct returns if you prefer the
simpler style here.
>
>> +}
>> +
>> +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, "%s - Fail\n", __func__);
>
> Avoid __func__.
>
> The error message is very non-descriptive
Marked.Will update.
>
> [...]
>
>> static int tgu_enable(struct device *dev)
>> {
>> struct tgu_drvdata *drvdata = dev_get_drvdata(dev);
>> + int ret = 0;
>>
>> guard(spinlock)(&drvdata->lock);
>> if (drvdata->enable)
>> return -EBUSY;
>>
>> - tgu_write_all_hw_regs(drvdata);
>> + ret = tgu_write_all_hw_regs(drvdata);
>> +
>> + if (ret == -EINVAL)
>
> stray \n
Sure.
>> + goto exit;
>> +
>> drvdata->enable = true;
>>
>> - return 0;
>> +exit:
>> + return ret;
>
> ret = tgu_write_all_hw_regs(drvdata);
> if (!ret)
> drvdata->enable = true;
>
> return ret
Will update.
>
> Konrad
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v10 5/7] qcom-tgu: Add support to configure next action
2026-01-13 11:15 ` Konrad Dybcio
@ 2026-01-27 2:43 ` Songwei Chai
2026-01-27 10:32 ` Konrad Dybcio
0 siblings, 1 reply; 29+ messages in thread
From: Songwei Chai @ 2026-01-27 2:43 UTC (permalink / raw)
To: Konrad Dybcio, andersson, alexander.shishkin, mike.leach,
suzuki.poulose, james.clark, krzk+dt, conor+dt
Cc: linux-kernel, linux-arm-kernel, linux-arm-msm, coresight,
devicetree, gregkh
On 1/13/2026 7:15 PM, Konrad Dybcio wrote:
> On 1/9/26 3:11 AM, Songwei Chai wrote:
>> Add "select" node for each step to determine if another step is taken,
>> trigger(s) are generated, counters/timers incremented/decremented, etc.
>>
>> Signed-off-by: Songwei Chai <songwei.chai@oss.qualcomm.com>
>> ---
>
> [...]
>
>> + case TGU_CONDITION_SELECT:
>> + /* 'default' register is at the end of 'select' region */
>> + if (tgu_attr->reg_num ==
>> + drvdata->max_condition_select - 1)
>> + attr->name = "default";
>> + ret = (tgu_attr->reg_num <
>> + drvdata->max_condition_select) ?
>> + attr->mode : 0;
>
> similarly to my previous comments
>
> [...]
>
>> + for (i = 0; i < drvdata->max_step; i++) {
>> + for (j = 0; j < drvdata->max_condition_select; j++) {
>> + index = check_array_location(drvdata, i,
>> + TGU_CONDITION_SELECT, j);
>> +
>> + if (index == -EINVAL)
>
> stray \n
>> + goto exit;
>> +
>> + writel(drvdata->value_table->condition_select[index],
>> + drvdata->base + CONDITION_SELECT_STEP(i, j));
>> + }
>> + }
>> /* Enable TGU to program the triggers */
>> writel(1, drvdata->base + TGU_CONTROL);
>> exit:
>> @@ -225,6 +258,8 @@ static void tgu_set_conditions(struct tgu_drvdata *drvdata)
>>
>> devid = readl(drvdata->base + TGU_DEVID);
>> drvdata->max_condition_decode = TGU_DEVID_CONDITIONS(devid);
>> + /* select region has an additional 'default' register */
>> + drvdata->max_condition_select = TGU_DEVID_CONDITIONS(devid) + 1;
>> }
>>
>> static int tgu_enable(struct device *dev)
>> @@ -356,6 +391,14 @@ static const struct attribute_group *tgu_attr_groups[] = {
>> CONDITION_DECODE_ATTRIBUTE_GROUP_INIT(5),
>> CONDITION_DECODE_ATTRIBUTE_GROUP_INIT(6),
>> CONDITION_DECODE_ATTRIBUTE_GROUP_INIT(7),
>> + CONDITION_SELECT_ATTRIBUTE_GROUP_INIT(0),
>> + CONDITION_SELECT_ATTRIBUTE_GROUP_INIT(1),
>> + CONDITION_SELECT_ATTRIBUTE_GROUP_INIT(2),
>> + CONDITION_SELECT_ATTRIBUTE_GROUP_INIT(3),
>> + CONDITION_SELECT_ATTRIBUTE_GROUP_INIT(4),
>> + CONDITION_SELECT_ATTRIBUTE_GROUP_INIT(5),
>> + CONDITION_SELECT_ATTRIBUTE_GROUP_INIT(6),
>> + CONDITION_SELECT_ATTRIBUTE_GROUP_INIT(7),
>> NULL,
>> };
>>
>> @@ -363,8 +406,8 @@ static int tgu_probe(struct amba_device *adev, const struct amba_id *id)
>> {
>> struct device *dev = &adev->dev;
>> struct tgu_drvdata *drvdata;
>> - size_t priority_size, condition_size;
>> - unsigned int *priority, *condition;
>> + size_t priority_size, condition_size, select_size;
>> + unsigned int *priority, *condition, *select;
>> int ret;
>>
>> drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
>> @@ -417,6 +460,16 @@ static int tgu_probe(struct amba_device *adev, const struct amba_id *id)
>>
>> drvdata->value_table->condition_decode = condition;
>>
>> + select_size = drvdata->max_condition_select * drvdata->max_step *
>> + sizeof(*(drvdata->value_table->condition_select));
>> +
>> + select = devm_kzalloc(dev, select_size, GFP_KERNEL);
>> +
>> + if (!select)
>
> stray \n
Will improve this based on the comments above.
>
>> + return -ENOMEM;
>> +
>> + drvdata->value_table->condition_select = select;
>
> I don't see a need for an intemediate variable here
This was done intentionally, following the earlier suggestion in v9 to
introduce named intermediate variables for better readability when
dealing with allocations.
I’m happy to inline the allocation if you prefer the simpler form here.
>
> [...]
>
>> * @max_condition_decode: Maximum number of condition_decode
>> + * @max_condition_select: Maximum number of condition_select
>
> Maximum value, perhaps? You haven't explained the feature very well
> so I'm not sure what this is supposed to reflect
Will add "Maximum value" description in cover letter.
>
> Konrad
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v10 7/7] qcom-tgu: Add reset node to initialize
2026-01-13 11:22 ` Konrad Dybcio
@ 2026-01-27 2:50 ` Songwei Chai
0 siblings, 0 replies; 29+ messages in thread
From: Songwei Chai @ 2026-01-27 2:50 UTC (permalink / raw)
To: Konrad Dybcio, andersson, alexander.shishkin, mike.leach,
suzuki.poulose, james.clark, krzk+dt, conor+dt
Cc: linux-kernel, linux-arm-kernel, linux-arm-msm, coresight,
devicetree, gregkh
On 1/13/2026 7:22 PM, Konrad Dybcio wrote:
> On 1/9/26 3:11 AM, 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@oss.qualcomm.com>
>> ---
>
> [...]
>
>> +/* reset_tgu_store - Reset Trace and Gating Unit (TGU) configuration. */
>> +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);
>> + unsigned long value;
>> + int i, j, ret;
>> +
>> + if (kstrtoul(buf, 0, &value) || value == 0)
>> + return -EINVAL;
>
> Your documentation blurb promises that only 1 is accepted, but this is not
> the case. I think the previous additions had a similar flaw
I’ll fix this to only accept 1 and review the previous additions
for similar issues.
>
>> +
>> + if (!drvdata->enable) {
>
> I think this check needs to be made under a lock, otherwise something else
> may pull the plug inbetween
Will move "guard(spinlock)(&drvdata->lock);" before "drvdata->enable" check.
>
>> + ret = pm_runtime_get_sync(drvdata->dev);
>> + if (ret < 0) {
>> + pm_runtime_put(drvdata->dev);
>> + return ret;
>> + }
>> + }
>> +
>> + guard(spinlock)(&drvdata->lock);
>> + TGU_UNLOCK(drvdata->base);
>> +
>> + writel(0, drvdata->base + TGU_CONTROL);
>> +
>> + TGU_LOCK(drvdata->base);
>
> This is tgu_disable()
will use tgu_disable instead.
>
>> +
>> + if (drvdata->value_table->priority)
>> + memset(drvdata->value_table->priority, 0,
>> + MAX_PRIORITY * drvdata->max_step *
>> + drvdata->max_reg * sizeof(unsigned int));
>> +
>> + if (drvdata->value_table->condition_decode)
>> + memset(drvdata->value_table->condition_decode, 0,
>> + drvdata->max_condition_decode * drvdata->max_step *
>> + sizeof(unsigned int));
>> +
>> + /* Initialize all condition registers to NOT(value=0x1000000) */
>
> One \t too much
will update.
>
>> + for (i = 0; i < drvdata->max_step; i++) {
>> + for (j = 0; j < drvdata->max_condition_decode; j++) {
>> + drvdata->value_table
>> + ->condition_decode[calculate_array_location(
>> + drvdata, i, TGU_CONDITION_DECODE, j)] =
>> + 0x1000000;
>
> This is unreadable, take a pointer to condition_decode[]
sure.
>
>> + }
>> + }
>> +
>> + if (drvdata->value_table->condition_select)
>> + memset(drvdata->value_table->condition_select, 0,
>> + drvdata->max_condition_select * drvdata->max_step *
>> + sizeof(unsigned int));
>> +
>> + if (drvdata->value_table->timer)
>> + memset(drvdata->value_table->timer, 0,
>> + (drvdata->max_step) *
>> + (drvdata->max_timer) *
>> + sizeof(unsigned int));
>> +
>> + if (drvdata->value_table->counter)
>> + memset(drvdata->value_table->counter, 0,
>> + (drvdata->max_step) *
>> + (drvdata->max_counter) *
>> + sizeof(unsigned int));
>
> This is similarly difficult to read with almost random indentation
>
I agree, the indentation hurts readability. I’ll rework this to make the
expression clearer.
> Konrad
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v10 6/7] qcom-tgu: Add timer/counter functionality for TGU
2026-01-13 11:19 ` Konrad Dybcio
@ 2026-01-27 3:02 ` Songwei Chai
0 siblings, 0 replies; 29+ messages in thread
From: Songwei Chai @ 2026-01-27 3:02 UTC (permalink / raw)
To: Konrad Dybcio, andersson, alexander.shishkin, mike.leach,
suzuki.poulose, james.clark, krzk+dt, conor+dt
Cc: linux-kernel, linux-arm-kernel, linux-arm-msm, coresight,
devicetree, gregkh
On 1/13/2026 7:19 PM, Konrad Dybcio wrote:
> On 1/9/26 3:11 AM, Songwei Chai wrote:
>> Add counter and timer node for each step which could be
>> programed if they are to be utilized in trigger event/sequence.
>>
>> Signed-off-by: Songwei Chai <songwei.chai@oss.qualcomm.com>
>> ---
>
> [...]
>
>> +static void tgu_set_timer_counter(struct tgu_drvdata *drvdata)
>> +{
>> + int num_timers, num_counters;
>> + u32 devid2;
>> +
>> + devid2 = readl(drvdata->base + CORESIGHT_DEVID2);
>> +
>> + if (TGU_DEVID2_TIMER0(devid2) && TGU_DEVID2_TIMER1(devid2))
>> + num_timers = 2;
>> + else if (TGU_DEVID2_TIMER0(devid2) || TGU_DEVID2_TIMER1(devid2))
>> + num_timers = 1;
>> + else
>> + num_timers = 0;
>> +
>> + if (TGU_DEVID2_COUNTER0(devid2) && TGU_DEVID2_COUNTER1(devid2))
>> + num_counters = 2;
>> + else if (TGU_DEVID2_COUNTER0(devid2) || TGU_DEVID2_COUNTER1(devid2))
>> + num_counters = 1;
>> + else
>> + num_counters = 0;
>> +
>> + drvdata->max_timer = num_timers;
>> + drvdata->max_counter = num_counters;
>
> int num_timers = 0, num_counters = 0
>
> if (TGU_DEVID2_TIMER0(devid2))
> num_timers++
>
> if (TGU_DEVID2_TIMER1(devid2))
> num_timers++
>
> etc.
>
> unless you want to guard against a case where TIMER0 reports as absent
> and TIMER1 as present and you consider that invalid (I don't know)
Based on the current documentation and the hardware we have encountered
so far, this case - "TIMER1 present, TIMER0 absent" does not occur.
>
> [...]
>
>> + timer_size = drvdata->max_step * drvdata->max_timer *
>> + sizeof(*(drvdata->value_table->timer));
>> +
>> + timer = devm_kzalloc(dev, timer_size, GFP_KERNEL);
>> +
>> + if (!timer)
>
> stray \n
sure.
>
>> + return -ENOMEM;
>> +
>> + drvdata->value_table->timer = timer;
>> +
>> + counter_size = drvdata->max_step * drvdata->max_counter *
>> + sizeof(*(drvdata->value_table->counter));
>> +
>> + counter = devm_kzalloc(dev, counter_size, GFP_KERNEL);
>
> devm_kcalloc, perhaps?
Agreed. Using devm_kcalloc() makes the intent clearer and safer here
>
>> +
>> + if (!counter)
>
> stray \n
sure.
>
>> + return -ENOMEM;
>> +
>> + drvdata->value_table->counter = counter;
>> +
>> drvdata->enable = false;
>>
>> pm_runtime_put(&adev->dev);
>> diff --git a/drivers/hwtracing/qcom/tgu.h b/drivers/hwtracing/qcom/tgu.h
>> index 8c92e88d7e2c..94708750b02d 100644
>> --- a/drivers/hwtracing/qcom/tgu.h
>> +++ b/drivers/hwtracing/qcom/tgu.h
>> @@ -11,11 +11,17 @@
>> #define TGU_LAR 0xfb0
>> #define TGU_UNLOCK_OFFSET 0xc5acce55
>> #define TGU_DEVID 0xfc8
>> +#define CORESIGHT_DEVID2 0xfc0
>>
>> #define BMVAL(val, lsb, msb) ((val & GENMASK(msb, lsb)) >> lsb)
>
> This is NIH FIELD_GET()
>
ok, will try to use "FIELD_GET".
> [...]
>
>> static inline void TGU_LOCK(void __iomem *addr)
>> @@ -197,6 +247,8 @@ static inline void TGU_UNLOCK(void __iomem *addr)
>> * @max_step: Maximum step size
>> * @max_condition_decode: Maximum number of condition_decode
>> * @max_condition_select: Maximum number of condition_select
>> + * @max_timer: Maximum number of timers
>> + * @max_counter: Maximum number of counters
>> *
>> * This structure defines the data associated with a TGU device,
>> * including its base address, device pointers, clock, spinlock for
>> @@ -213,6 +265,8 @@ struct tgu_drvdata {
>> int max_step;
>> int max_condition_decode;
>> int max_condition_select;
>> + int max_timer;
>> + int max_counter;
>
> num_timers, num_counters definitely fits better here
>
uhh.. yeah.
> Konrad
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v10 3/7] qcom-tgu: Add signal priority support
2026-01-27 2:23 ` Songwei Chai
@ 2026-01-27 10:30 ` Konrad Dybcio
0 siblings, 0 replies; 29+ messages in thread
From: Konrad Dybcio @ 2026-01-27 10:30 UTC (permalink / raw)
To: Songwei Chai, andersson, alexander.shishkin, mike.leach,
suzuki.poulose, james.clark, krzk+dt, conor+dt
Cc: linux-kernel, linux-arm-kernel, linux-arm-msm, coresight,
devicetree, gregkh
On 1/27/26 3:23 AM, Songwei Chai wrote:
>
>
> On 1/13/2026 7:09 PM, Konrad Dybcio wrote:
>> On 1/9/26 3:11 AM, 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.
>>>
>>> Signed-off-by: Songwei Chai <songwei.chai@oss.qualcomm.com>
>>> ---
[...]
>>> + if (tgu_attr->step_index < drvdata->max_step) {
>>> + ret = (tgu_attr->reg_num < drvdata->max_reg) ?
>>> + attr->mode : 0;
>>> + }
>>> + return ret;
>>
>> This is very convoluted
>>
>> How about:
>>
>> if (tgu_attr->step_index >= drvdata->max_step)
>> return 0;
>>
>> if (tgu_attr->reg_num >= drvdata->max_reg)
>> return 0;
>>
>> return attr->mode;
>>
>> ?
>>
>> [...]
>>
> I agree that the expanded form is clearer step-by-step, but I intentionally kept the current version as it keeps the bounds checks localized and avoids multiple early returns in this simple case.
Multiple early returns are not an anti-pattern to avoid
Konrad
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v10 4/7] qcom-tgu: Add TGU decode support
2026-01-27 2:34 ` Songwei Chai
@ 2026-01-27 10:31 ` Konrad Dybcio
0 siblings, 0 replies; 29+ messages in thread
From: Konrad Dybcio @ 2026-01-27 10:31 UTC (permalink / raw)
To: Songwei Chai, andersson, alexander.shishkin, mike.leach,
suzuki.poulose, james.clark, krzk+dt, conor+dt
Cc: linux-kernel, linux-arm-kernel, linux-arm-msm, coresight,
devicetree, gregkh
On 1/27/26 3:34 AM, Songwei Chai wrote:
>
>
> On 1/13/2026 7:13 PM, Konrad Dybcio wrote:
>> On 1/9/26 3:11 AM, Songwei Chai wrote:
>>> 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.
>>>
>>> Signed-off-by: Songwei Chai <songwei.chai@oss.qualcomm.com>
>>> ---
[...]
>>> + switch (operation_index) {
>>> + case TGU_PRIORITY0:
>>> + case TGU_PRIORITY1:
>>> + case TGU_PRIORITY2:
>>> + case TGU_PRIORITY3:
>>> + ret = operation_index * (drvdata->max_step) *
>>> + (drvdata->max_reg) +
>>> + step_index * (drvdata->max_reg) + reg_index;
>>> + break;
>>> + case TGU_CONDITION_DECODE:
>>> + ret = step_index * (drvdata->max_condition_decode) +
>>> + reg_index;
>>> + break;
>>> + default:
>>> + break;
>>> + }
>>> + return ret;
>>
>> The only thing your switch statement is assign a value to ret and break
>> out. Change that to a direct return, and drop 'ret' altogether
>>
>
> I kept a single return intentionally so the function has a single exit
> point. This makes it easier to extend with common post-processing or
> debug logic later if needed.
>
> That said, I’m fine switching to direct returns if you prefer the simpler style here.
It's up to you. Do you expect you'll need more debugging or extending
of this function in near future?
Konrad
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v10 5/7] qcom-tgu: Add support to configure next action
2026-01-27 2:43 ` Songwei Chai
@ 2026-01-27 10:32 ` Konrad Dybcio
0 siblings, 0 replies; 29+ messages in thread
From: Konrad Dybcio @ 2026-01-27 10:32 UTC (permalink / raw)
To: Songwei Chai, andersson, alexander.shishkin, mike.leach,
suzuki.poulose, james.clark, krzk+dt, conor+dt
Cc: linux-kernel, linux-arm-kernel, linux-arm-msm, coresight,
devicetree, gregkh
On 1/27/26 3:43 AM, Songwei Chai wrote:
>
>
> On 1/13/2026 7:15 PM, Konrad Dybcio wrote:
>> On 1/9/26 3:11 AM, Songwei Chai wrote:
>>> Add "select" node for each step to determine if another step is taken,
>>> trigger(s) are generated, counters/timers incremented/decremented, etc.
>>>
>>> Signed-off-by: Songwei Chai <songwei.chai@oss.qualcomm.com>
>>> ---
[...]
>>> + select = devm_kzalloc(dev, select_size, GFP_KERNEL);
>>> +
>>> + if (!select)
>>
>> stray \n
> Will improve this based on the comments above.
>>
>>> + return -ENOMEM;
>>> +
>>> + drvdata->value_table->condition_select = select;
>>
>> I don't see a need for an intemediate variable here
>
> This was done intentionally, following the earlier suggestion in v9 to
> introduce named intermediate variables for better readability when dealing with allocations.
>
> I’m happy to inline the allocation if you prefer the simpler form here.
I don't mind that much
Konrad
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v10 2/7] qcom-tgu: Add TGU driver
2026-01-27 2:13 ` Songwei Chai
@ 2026-01-27 10:35 ` Konrad Dybcio
0 siblings, 0 replies; 29+ messages in thread
From: Konrad Dybcio @ 2026-01-27 10:35 UTC (permalink / raw)
To: Songwei Chai, andersson, alexander.shishkin, mike.leach,
suzuki.poulose, james.clark, krzk+dt, conor+dt
Cc: linux-kernel, linux-arm-kernel, linux-arm-msm, coresight,
devicetree, gregkh
On 1/27/26 3:13 AM, Songwei Chai wrote:
> Hi Konrad,
>
> Sorry for the late reply.
>
> On 1/13/2026 6:33 PM, Konrad Dybcio wrote:
>> On 1/9/26 3:11 AM, Songwei Chai wrote:
>>> Add driver to support device TGU (Trigger Generation Unit).
>>> TGU is a Data Engine which can be utilized to sense a plurality of
>>> signals and create a trigger into the CTI or generate interrupts to
>>> processors. Add probe/enable/disable functions for tgu.
>>>
>>> Signed-off-by: Songwei Chai <songwei.chai@oss.qualcomm.com>
>>> ---
[...]
>>> +static inline void TGU_LOCK(void __iomem *addr)
>>> +{
>>> + do {
>>> + /* Wait for things to settle */
>>> + mb();
>>
>> What are we waiting for here?
>>
>>> + writel_relaxed(0x0, addr + TGU_LAR);
>>
>> If you do a prompt TGU_LOCK()-TGU_UNLOCK() the writes may arrive in
>> the order opposite to what you want, I'd say this shouldn't be _relaxed()
>> and we should probably have a readback here to make sure the effect has
>> taken place immediately
>>
>>> + } while (0);
>>> +}
>>> +
>>> +static inline void TGU_UNLOCK(void __iomem *addr)
>>> +{
>>> + do {
>>> + writel_relaxed(TGU_UNLOCK_OFFSET, addr + TGU_LAR);
>>> + /* Make sure everyone has seen this */
>>> + mb();
>>
>> I believe this should be a readback instead
>>
>>> + } while (0);
>>> +}
> This lock/unlock sequence is intentionally modelled after the existing CoreSight CS_LOCK/CS_UNLOCK helpers, which have been in mainline for a
> long time and are widely used on ARM systems.
>
> The barriers here are meant to provide CPU-side ordering guarantees
> around the LAR access rather than to wait for the hardware lock/unlock
> to complete. In particular, the intent is to prevent configuration
> accesses from being reordered across the lock/unlock boundary, matching
> the CoreSight programming model.
>
> I agree that the comments may be misleading in that regard, and I can
> update them to clarify the ordering intent.
>
> If you still prefer a stricter write + readback sequence here, I’m also
> happy to switch to that for additional conservatism.
If the hardware doesn't mind potentially receiving commands in the
locked state (i.e. they're not dropped), then this seems fine
Otherwise, I think this may end up to random misconfigurations
Konrad
^ permalink raw reply [flat|nested] 29+ messages in thread
end of thread, other threads:[~2026-01-27 10:35 UTC | newest]
Thread overview: 29+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-01-09 2:11 [PATCH v10 0/7] Provide support for Trigger Generation Unit Songwei Chai
2026-01-09 2:11 ` [PATCH v10 1/7] dt-bindings: arm: Add support for Qualcomm TGU trace Songwei Chai
2026-01-09 2:11 ` [PATCH v10 2/7] qcom-tgu: Add TGU driver Songwei Chai
2026-01-09 10:27 ` Suzuki K Poulose
2026-01-12 1:43 ` Songwei Chai
2026-01-09 11:28 ` Jie Gan
2026-01-12 1:41 ` Songwei Chai
2026-01-13 10:33 ` Konrad Dybcio
2026-01-27 2:13 ` Songwei Chai
2026-01-27 10:35 ` Konrad Dybcio
2026-01-09 2:11 ` [PATCH v10 3/7] qcom-tgu: Add signal priority support Songwei Chai
2026-01-13 11:09 ` Konrad Dybcio
2026-01-27 2:23 ` Songwei Chai
2026-01-27 10:30 ` Konrad Dybcio
2026-01-09 2:11 ` [PATCH v10 4/7] qcom-tgu: Add TGU decode support Songwei Chai
2026-01-13 11:13 ` Konrad Dybcio
2026-01-27 2:34 ` Songwei Chai
2026-01-27 10:31 ` Konrad Dybcio
2026-01-09 2:11 ` [PATCH v10 5/7] qcom-tgu: Add support to configure next action Songwei Chai
2026-01-13 11:15 ` Konrad Dybcio
2026-01-27 2:43 ` Songwei Chai
2026-01-27 10:32 ` Konrad Dybcio
2026-01-09 2:11 ` [PATCH v10 6/7] qcom-tgu: Add timer/counter functionality for TGU Songwei Chai
2026-01-13 11:19 ` Konrad Dybcio
2026-01-27 3:02 ` Songwei Chai
2026-01-09 2:11 ` [PATCH v10 7/7] qcom-tgu: Add reset node to initialize Songwei Chai
2026-01-13 11:22 ` Konrad Dybcio
2026-01-27 2:50 ` Songwei Chai
2026-01-13 1:53 ` [PATCH v10 0/7] Provide support for Trigger Generation Unit Songwei Chai
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox