Re: [PATCH v8 4/4] coresight: cti: expose banked sysfs registers for Qualcomm extended CTI

[Leo Yan <leo.yan@arm.com>]

On Sun, Apr 26, 2026 at 05:44:41PM +0800, Yingchao Deng wrote:
> Qualcomm extended CTI implements banked trigger status and integration
> registers, where each bank covers 32 triggers. Multiple instances of
> these registers are required to expose the full trigger space.
> 
> Add static sysfs entries for the banked CTI registers and control their
> visibility based on the underlying hardware configuration. Numbered
> sysfs nodes are hidden on standard ARM CTIs, preserving the existing ABI.
> On Qualcomm CTIs, only banked registers backed by hardware are exposed,
> with the number of visible banks derived from nr_trig_max.
> 
> This ensures that userspace only sees registers that are actually
> implemented, while maintaining compatibility with existing CTI tooling.
> 
> Signed-off-by: Yingchao Deng <yingchao.deng@oss.qualcomm.com>
> ---
>  drivers/hwtracing/coresight/coresight-cti-sysfs.c | 58 +++++++++++++++++++++++
>  1 file changed, 58 insertions(+)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-cti-sysfs.c b/drivers/hwtracing/coresight/coresight-cti-sysfs.c
> index 8b70e7e38ea3..046757e4e9b6 100644
> --- a/drivers/hwtracing/coresight/coresight-cti-sysfs.c
> +++ b/drivers/hwtracing/coresight/coresight-cti-sysfs.c
> @@ -512,18 +512,36 @@ static struct attribute *coresight_cti_regs_attrs[] = {
>  	&dev_attr_appclear.attr,
>  	&dev_attr_apppulse.attr,
>  	coresight_cti_reg(triginstatus, CTITRIGINSTATUS),
> +	coresight_cti_reg(triginstatus1, CTI_REG_SET_NR_CONST(CTITRIGINSTATUS, 1)),

How about extend the cs_off_attribute struct:

  struct cs_off_attribute {
        struct device_attribute attr;
        u32 off;
        u32 index;
  };

  // by default, the index is 0
  #define coresight_cti_reg(name, offset)                               \
        (&((struct cs_off_attribute[]) {                                \
           {                                                            \
                __ATTR(name, 0444, coresight_cti_reg_show, NULL),       \
                offset                                                  \
                0                                                       \
           }                                                            \
  })[0].attr.attr)

  // For the register with index
  #define coresight_cti_reg_index(name, offset, index)                  \
        (&((struct cs_off_attribute[]) {                                \
           {                                                            \
                __ATTR(name, 0444, coresight_cti_reg_show, NULL),       \
                offset                                                  \
                index                                                   \
           }                                                            \
  })[0].attr.attr)

  coresight_cti_reg_index(triginstatus1, CTITRIGINSTATUS, 1),

> +	coresight_cti_reg(triginstatus2, CTI_REG_SET_NR_CONST(CTITRIGINSTATUS, 2)),
> +	coresight_cti_reg(triginstatus3, CTI_REG_SET_NR_CONST(CTITRIGINSTATUS, 3)),
>  	coresight_cti_reg(trigoutstatus, CTITRIGOUTSTATUS),
> +	coresight_cti_reg(trigoutstatus1, CTI_REG_SET_NR_CONST(CTITRIGOUTSTATUS, 1)),
> +	coresight_cti_reg(trigoutstatus2, CTI_REG_SET_NR_CONST(CTITRIGOUTSTATUS, 2)),
> +	coresight_cti_reg(trigoutstatus3, CTI_REG_SET_NR_CONST(CTITRIGOUTSTATUS, 3)),
>  	coresight_cti_reg(chinstatus, CTICHINSTATUS),
>  	coresight_cti_reg(choutstatus, CTICHOUTSTATUS),
>  #ifdef CONFIG_CORESIGHT_CTI_INTEGRATION_REGS
>  	coresight_cti_reg_rw(itctrl, CORESIGHT_ITCTRL),
>  	coresight_cti_reg(ittrigin, ITTRIGIN),
> +	coresight_cti_reg(ittrigin1, CTI_REG_SET_NR_CONST(ITTRIGIN, 1)),
> +	coresight_cti_reg(ittrigin2, CTI_REG_SET_NR_CONST(ITTRIGIN, 2)),
> +	coresight_cti_reg(ittrigin3, CTI_REG_SET_NR_CONST(ITTRIGIN, 3)),
>  	coresight_cti_reg(itchin, ITCHIN),
>  	coresight_cti_reg_rw(ittrigout, ITTRIGOUT),
> +	coresight_cti_reg_rw(ittrigout1, CTI_REG_SET_NR_CONST(ITTRIGOUT, 1)),
> +	coresight_cti_reg_rw(ittrigout2, CTI_REG_SET_NR_CONST(ITTRIGOUT, 2)),
> +	coresight_cti_reg_rw(ittrigout3, CTI_REG_SET_NR_CONST(ITTRIGOUT, 3)),
>  	coresight_cti_reg_rw(itchout, ITCHOUT),
>  	coresight_cti_reg(itchoutack, ITCHOUTACK),
>  	coresight_cti_reg(ittrigoutack, ITTRIGOUTACK),
> +	coresight_cti_reg(ittrigoutack1, CTI_REG_SET_NR_CONST(ITTRIGOUTACK, 1)),
> +	coresight_cti_reg(ittrigoutack2, CTI_REG_SET_NR_CONST(ITTRIGOUTACK, 2)),
> +	coresight_cti_reg(ittrigoutack3, CTI_REG_SET_NR_CONST(ITTRIGOUTACK, 3)),
>  	coresight_cti_reg_wo(ittriginack, ITTRIGINACK),
> +	coresight_cti_reg_wo(ittriginack1, CTI_REG_SET_NR_CONST(ITTRIGINACK, 1)),
> +	coresight_cti_reg_wo(ittriginack2, CTI_REG_SET_NR_CONST(ITTRIGINACK, 2)),
> +	coresight_cti_reg_wo(ittriginack3, CTI_REG_SET_NR_CONST(ITTRIGINACK, 3)),
>  	coresight_cti_reg_wo(itchinack, ITCHINACK),
>  #endif
>  	NULL,
> @@ -534,10 +552,50 @@ static umode_t coresight_cti_regs_is_visible(struct kobject *kobj,
>  {
>  	struct device *dev = kobj_to_dev(kobj);
>  	struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);
> +	static const char * const qcom_suffix_registers[] = {
> +		"triginstatus",
> +		"trigoutstatus",
> +#ifdef CONFIG_CORESIGHT_CTI_INTEGRATION_REGS
> +		"ittrigin",
> +		"ittrigout",
> +		"ittriginack",
> +		"ittrigoutack",
> +#endif
> +	};
> +	int i, nr, max_bank;
> +	size_t len;
>  
>  	if (attr == &dev_attr_asicctl.attr && !drvdata->config.asicctl_impl)
>  		return 0;
>  
> +	/*
> +	 * Banked regs are exposed as <qcom_suffix_registers><nr> (nr = 1..3).
> +	 * - Hide them on standard CTIs.
> +	 * - On QCOM CTIs, hide suffixes beyond the number of banks implied
> +	 *   by nr_trig_max (32 triggers per bank).
> +	 */
> +	for (i = 0; i < ARRAY_SIZE(qcom_suffix_registers); i++) {

This can be general for a register with index?  Like:

  for (i = 0; i < ARRAY_SIZE(registers_with_index); i++) {

> +		len = strlen(qcom_suffix_registers[i]);
> +
> +		if (strncmp(attr->name, qcom_suffix_registers[i], len))
> +			continue;
> +
> +		if (kstrtoint(attr->name + len, 10, &nr))
> +			continue;
> +
> +		if (!drvdata->is_qcom_cti)
> +			return 0;
> +
> +		if (nr < 1 || nr > 3)
> +			return 0;
> +
> +		max_bank = DIV_ROUND_UP(drvdata->config.nr_trig_max, 32) - 1;
> +		if (nr > max_bank)
> +			return 0;

Directly check the attr's index here?

  struct cs_off_attribute *cti_attr =
                container_of(attr, struct cs_off_attribute, attr);

  max_bank = DIV_ROUND_UP(drvdata->config.nr_trig_max, 32);
  if (cti_attr->index >= max_bank)
      return 0;

Thanks,
Leo