[v10] Provide support for Trigger Generation Unit

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

Posted by Songwei Chai 1 month ago

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

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

Posted by Konrad Dybcio 3 weeks, 6 days ago

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

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

Posted by Songwei Chai 1 week, 6 days ago


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

[PATCH v10 1/7] dt-bindings: arm: Add support for Qualcomm TGU trace
[PATCH v10 2/7] qcom-tgu: Add TGU driver
[PATCH v10 3/7] qcom-tgu: Add signal priority support
[PATCH v10 4/7] qcom-tgu: Add TGU decode support
[PATCH v10 5/7] qcom-tgu: Add support to configure next action
[PATCH v10 6/7] qcom-tgu: Add timer/counter functionality for TGU
[PATCH v10 7/7] qcom-tgu: Add reset node to initialize