Re: [PATCH v10 5/7] qcom-tgu: Add support to configure next action
From: Songwei Chai
Date: Mon Jan 26 2026 - 22:07:14 EST
On 1/13/2026 7:15 PM, Konrad Dybcio wrote:
On 1/9/26 3:11 AM, Songwei Chai wrote:Will improve this based on the comments above.
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@xxxxxxxxxxxxxxxx>
---
[...]
+ 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
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.
Will add "Maximum value" description in cover letter.
[...]
* @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