Re: [PATCH v10 7/7] qcom-tgu: Add reset node to initialize
From: Konrad Dybcio <hidden>
Date: 2026-01-13 11:22:34
Also in:
linux-arm-msm, linux-devicetree, lkml
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 <redacted> ---
[...]
quoted hunk ↗ jump to hunk
+/* 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
quoted hunk ↗ jump to hunk
+ 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()
quoted hunk ↗ jump to hunk
+ + 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
quoted hunk ↗ jump to hunk
+ 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[]
quoted hunk ↗ jump to hunk
+ } + } + + 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