Re: [PATCH v17 07/10] qcom-tgu: Add support to configure next action

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [High] qcom-tgu: swapped operation_index and step_index in
  tgu_dataset_rw()
--

commit 39b03844fdda185e25e4736b4873ab4173c403fb
Author: Songwei Chai <songwei.chai@oss.qualcomm.com>

qcom-tgu: Add support to configure next action

This commit adds per-step condition_select sysfs attribute groups,
including numbered select registers and a default next-action register,
and flushes their staged values to the TGU hardware when enabled.

> diff --git a/drivers/hwtracing/qcom/tgu.c b/drivers/hwtracing/qcom/tgu.c
> index 72b98a96d7f20..6e66752a7b8dd 100644
> --- a/drivers/hwtracing/qcom/tgu.c
> +++ b/drivers/hwtracing/qcom/tgu.c
[ ... ]
> @@ -409,6 +473,29 @@ static const struct attribute_group tgu_common_grp = {
>  		.name = "step" #step "_condition_decode" \
>  	})
>  
> +#define STEP_SELECT(step_index, reg_num) \
> +	tgu_dataset_rw(reg##reg_num, step_index, TGU_CONDITION_SELECT, reg_num)
> +
> +#define STEP_SELECT_DEFAULT(step_index) \
> +	tgu_dataset_rw(default, step_index, TGU_CONDITION_SELECT, \
> +			TGU_SELECT_DEFAULT_REG)

[Severity: High]
Does STEP_SELECT() initialize tgu_attribute with operation_index and
step_index swapped?

struct tgu_attribute declares operation_index before step_index, whereas
tgu_dataset_rw() uses positional initialization with step_index before the
operation enum:

drivers/hwtracing/qcom/tgu.c:
struct tgu_attribute {
	struct device_attribute attr;
	enum operation_index operation_index;
	u8 step_index;
	u8 reg_num;
};
...
#define tgu_dataset_rw(name, step_index, priority, reg_num)              \
	(&((struct tgu_attribute[]){ {                                   \
		__ATTR(name, 0644, tgu_dataset_show, tgu_dataset_store), \
		step_index,                                              \
		priority,                                                \
		reg_num,                                                 \
	} })[0].attr.attr)

Every step[0:7]_condition_select attribute ends up with operation_index =
0..7 and step_index = TGU_CONDITION_SELECT (5).  In tgu_node_visible(),
this hides all condition_select groups when drvdata->num_step <= 5.  When
drvdata->num_step > 5, the default attributes and step[6:7]_condition_select
remain hidden, accesses to step[0:4]_condition_select/reg[0:3] read and
write drvdata->priority or drvdata->condition_decode for step 5, and
step5_priority[0:3] and step5_condition_decode (which have
operation_index = 5) are misrouted to TGU_CONDITION_SELECT for steps 0..4.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113024935.3173715-1-songwei.chai@oss.qualcomm.com?part=7