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

Yingchao Deng (Consultant)
2026-04-28T02:18:27Z

On 4/28/2026 2:15 AM, Leo Yan wrote:
> 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
Will update.

Thanks,
Yingchao