Re: [PATCH v8 2/4] coresight: cti: encode trigger register index in register offsets

Leo Yan
2026-04-27T17:48:09Z

On Sun, Apr 26, 2026 at 05:44:39PM +0800, Yingchao Deng wrote:
> Introduce a small encoding to carry the register index together with the
> base offset in a single u32, and use a common helper to compute the final
> MMIO address. This refactors register access to be based on the encoded
> (reg, nr) pair, reducing duplicated arithmetic and making it easier to
> support variants that bank or relocate trigger-indexed registers.
>
> Signed-off-by: Yingchao Deng <yingchao.deng@oss.qualcomm.com>
> ---
> drivers/hwtracing/coresight/coresight-cti-core.c | 31 +++++++++++++++--------
> drivers/hwtracing/coresight/coresight-cti-sysfs.c | 4 +--
> drivers/hwtracing/coresight/coresight-cti.h | 16 ++++++++++--
> 3 files changed, 36 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-cti-core.c b/drivers/hwtracing/coresight/coresight-cti-core.c
> index 4e7d12bd2d3e..c4cbeb64365b 100644
> --- a/drivers/hwtracing/coresight/coresight-cti-core.c
> +++ b/drivers/hwtracing/coresight/coresight-cti-core.c
> @@ -42,6 +42,14 @@ static DEFINE_MUTEX(ect_mutex);
> #define csdev_to_cti_drvdata(csdev) \
> dev_get_drvdata(csdev->dev.parent)
>
> +static void __iomem *cti_reg_addr(struct cti_drvdata *drvdata, int reg)
> +{
> + u32 offset = CTI_REG_CLR_NR(reg);
> + u32 nr = CTI_REG_GET_NR(reg);
> +
> + return drvdata->base + offset + sizeof(u32) * nr;
> +}

Could you try below change, which is more straightforward?

static void __iomem *__reg_addr(struct cti_drvdata *drvdata, int off,
int index)
{
return drvdata->base + offset + sizeof(u32) * index;
}

#define reg_addr(drvdata, off) \
__reg_addr((drvdata), (off), 0)

#define reg_index_addr(drvdata, off, i) \
__reg_addr((drvdata), (off), (i))

> +
> /* write set of regs to hardware - call with spinlock claimed */
> void cti_write_all_hw_regs(struct cti_drvdata *drvdata)
> {
> @@ -55,16 +63,17 @@ void cti_write_all_hw_regs(struct cti_drvdata *drvdata)
>
> /* write the CTI trigger registers */
> for (i = 0; i < config->nr_trig_max; i++) {
> - writel_relaxed(config->ctiinen[i], drvdata->base + CTIINEN(i));
> + writel_relaxed(config->ctiinen[i],
> + cti_reg_addr(drvdata, CTI_REG_SET_NR(CTIINEN, i)));

writel_relaxed(config->ctiinen[i],
reg_index_addr(drvdata, CTIINEN, i));

> writel_relaxed(config->ctiouten[i],
> - drvdata->base + CTIOUTEN(i));
> + cti_reg_addr(drvdata, CTI_REG_SET_NR(CTIOUTEN, i)));

writel_relaxed(config->ctiouten[i],
reg_index_addr(drvdata, CTIOUTEN, i));

[...]

> +/*
> + * Encode CTI register offset and register index in one u32:
> + * - bits[0:11] : base register offset (0x000 to 0xFFF)
> + * - bits[24:31] : register index (nr)
> + */
> +#define CTI_REG_NR_MASK GENMASK(31, 24)
> +#define CTI_REG_GET_NR(reg) FIELD_GET(CTI_REG_NR_MASK, (reg))
> +#define CTI_REG_SET_NR_CONST(reg, nr) ((reg) | FIELD_PREP_CONST(CTI_REG_NR_MASK, (nr)))
> +#define CTI_REG_SET_NR(reg, nr) ((reg) | FIELD_PREP(CTI_REG_NR_MASK, (nr)))
> +#define CTI_REG_CLR_NR(reg) ((reg) & (~CTI_REG_NR_MASK))

I know this might come from my suggestion, and it is also will be
heavily used in patch 04. We can have strightforward way to
implement this, please drop these macros.

I will reply in patch 04 separately. Sorry my review might cause
extra effort.

Thanks,
Leo