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