[PATCH v2 0/5] coresight: Reduce duplicated sysfs accessors

[PATCH v2 0/5] coresight: Reduce duplicated sysfs accessors

[James Clark]

Changes since v1:

  * Keep existing signed offset value types until the very last commit
    and then remove them all at once

  * Also split out usages of read_pair() and read32() into separate
    functions in the last commit 

  * Whitespace fixes

  * Replaced any touched scnprintf() with sysfs_emit()

======

The intent of this change is to reduce the large number identical of
functions created by macros for sysfs accessors. It's possible to re-use
the same function but pass in the register to access as an argument.
This reduces the size of the coresight modules folder by 244KB.

The first two patches are refactors to simplify and remove some dead
code, and the second two are the changes to use a shared function.

Testing
=======

No changes in any of the outputs:

  cat /sys/bus/coresight/devices/*/mgmt/* > before.txt
  cat /sys/bus/coresight/devices/*/mgmt/* > after.txt
  diff before.txt after.txt

With the following modules loaded:

  ls /sys/bus/coresight/devices/
  etm0  etm2  funnel0  funnel2  replicator0  tmc_etf0  tmc_etf2  tpiu0
  etm1  etm3  funnel1  funnel3  stm0         tmc_etf1  tmc_etr0


James Clark (5):
  coresight: Remove unused function parameter
  coresight: Simplify sysfs accessors by using csdev_access abstraction
  coresight: Re-use same function for similar sysfs register accessors
  coresight: cti-sysfs: Re-use same functions for similar sysfs register
    accessors
  coresight: Make new csdev_access offsets unsigned

 drivers/hwtracing/coresight/coresight-catu.c  |  27 +--
 drivers/hwtracing/coresight/coresight-catu.h  |   8 +-
 drivers/hwtracing/coresight/coresight-core.c  |  28 +++
 .../hwtracing/coresight/coresight-cti-sysfs.c | 213 +++++++-----------
 drivers/hwtracing/coresight/coresight-etb10.c |  28 +--
 .../coresight/coresight-etm3x-sysfs.c         |  34 +--
 drivers/hwtracing/coresight/coresight-priv.h  |  74 +++---
 .../coresight/coresight-replicator.c          |  10 +-
 drivers/hwtracing/coresight/coresight-stm.c   |  40 +---
 .../hwtracing/coresight/coresight-tmc-core.c  |  48 ++--
 drivers/hwtracing/coresight/coresight-tmc.h   |   4 +-
 include/linux/coresight.h                     |  23 ++
 12 files changed, 227 insertions(+), 310 deletions(-)

-- 
2.28.0

[PATCH v2 1/5] coresight: Remove unused function parameter

[James Clark]

The ability to use a custom function in this sysfs show function isn't
used so remove it.

No functional changes.

Signed-off-by: James Clark <james.clark@arm.com>
---
 drivers/hwtracing/coresight/coresight-priv.h | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h
index ff1dd2092ac5..f2458b794ef3 100644
--- a/drivers/hwtracing/coresight/coresight-priv.h
+++ b/drivers/hwtracing/coresight/coresight-priv.h
@@ -40,31 +40,23 @@
 #define ETM_MODE_EXCL_KERN	BIT(30)
 #define ETM_MODE_EXCL_USER	BIT(31)
 
-typedef u32 (*coresight_read_fn)(const struct device *, u32 offset);
-#define __coresight_simple_func(type, func, name, lo_off, hi_off)	\
+#define __coresight_simple_show(type, name, lo_off, hi_off)		\
 static ssize_t name##_show(struct device *_dev,				\
 			   struct device_attribute *attr, char *buf)	\
 {									\
 	type *drvdata = dev_get_drvdata(_dev->parent);			\
-	coresight_read_fn fn = func;					\
 	u64 val;							\
 	pm_runtime_get_sync(_dev->parent);				\
-	if (fn)								\
-		val = (u64)fn(_dev->parent, lo_off);			\
-	else								\
-		val = coresight_read_reg_pair(drvdata->base,		\
-						 lo_off, hi_off);	\
+	val = coresight_read_reg_pair(drvdata->base, lo_off, hi_off);	\
 	pm_runtime_put_sync(_dev->parent);				\
 	return scnprintf(buf, PAGE_SIZE, "0x%llx\n", val);		\
 }									\
 static DEVICE_ATTR_RO(name)
 
-#define coresight_simple_func(type, func, name, offset)			\
-	__coresight_simple_func(type, func, name, offset, -1)
 #define coresight_simple_reg32(type, name, offset)			\
-	__coresight_simple_func(type, NULL, name, offset, -1)
+	__coresight_simple_show(type, name, offset, -1)
 #define coresight_simple_reg64(type, name, lo_off, hi_off)		\
-	__coresight_simple_func(type, NULL, name, lo_off, hi_off)
+	__coresight_simple_show(type, name, lo_off, hi_off)
 
 extern const u32 coresight_barrier_pkt[4];
 #define CORESIGHT_BARRIER_PKT_SIZE (sizeof(coresight_barrier_pkt))
-- 
2.28.0

[PATCH v2 2/5] coresight: Simplify sysfs accessors by using csdev_access abstraction

[James Clark]

The coresight_device struct is available in the sysfs accessor, and this
contains a csdev_access struct which can be used to access registers.
Use this instead of passing in the type of each drvdata so that a common
function can be shared between all the cs drivers.

No functional changes.

Signed-off-by: James Clark <james.clark@arm.com>
---
 drivers/hwtracing/coresight/coresight-catu.c  | 18 +++++-----
 drivers/hwtracing/coresight/coresight-etb10.c | 19 +++++------
 .../coresight/coresight-etm3x-sysfs.c         | 23 ++++++-------
 drivers/hwtracing/coresight/coresight-priv.h  | 14 ++++----
 .../coresight/coresight-replicator.c          |  7 ++--
 drivers/hwtracing/coresight/coresight-stm.c   | 27 +++++++--------
 .../hwtracing/coresight/coresight-tmc-core.c  | 33 ++++++++-----------
 include/linux/coresight.h                     | 18 ++++++++++
 8 files changed, 79 insertions(+), 80 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-catu.c b/drivers/hwtracing/coresight/coresight-catu.c
index e0740c6dbd54..9d89c4054046 100644
--- a/drivers/hwtracing/coresight/coresight-catu.c
+++ b/drivers/hwtracing/coresight/coresight-catu.c
@@ -365,16 +365,14 @@ static const struct etr_buf_operations etr_catu_buf_ops = {
 	.get_data = catu_get_data_etr_buf,
 };
 
-coresight_simple_reg32(struct catu_drvdata, devid, CORESIGHT_DEVID);
-coresight_simple_reg32(struct catu_drvdata, control, CATU_CONTROL);
-coresight_simple_reg32(struct catu_drvdata, status, CATU_STATUS);
-coresight_simple_reg32(struct catu_drvdata, mode, CATU_MODE);
-coresight_simple_reg32(struct catu_drvdata, axictrl, CATU_AXICTRL);
-coresight_simple_reg32(struct catu_drvdata, irqen, CATU_IRQEN);
-coresight_simple_reg64(struct catu_drvdata, sladdr,
-		       CATU_SLADDRLO, CATU_SLADDRHI);
-coresight_simple_reg64(struct catu_drvdata, inaddr,
-		       CATU_INADDRLO, CATU_INADDRHI);
+coresight_simple_reg32(devid, CORESIGHT_DEVID);
+coresight_simple_reg32(control, CATU_CONTROL);
+coresight_simple_reg32(status, CATU_STATUS);
+coresight_simple_reg32(mode, CATU_MODE);
+coresight_simple_reg32(axictrl, CATU_AXICTRL);
+coresight_simple_reg32(irqen, CATU_IRQEN);
+coresight_simple_reg64(sladdr, CATU_SLADDRLO, CATU_SLADDRHI);
+coresight_simple_reg64(inaddr, CATU_INADDRLO, CATU_INADDRHI);
 
 static struct attribute *catu_mgmt_attrs[] = {
 	&dev_attr_devid.attr,
diff --git a/drivers/hwtracing/coresight/coresight-etb10.c b/drivers/hwtracing/coresight/coresight-etb10.c
index efa39820acec..405bb3355cb1 100644
--- a/drivers/hwtracing/coresight/coresight-etb10.c
+++ b/drivers/hwtracing/coresight/coresight-etb10.c
@@ -655,17 +655,14 @@ static const struct file_operations etb_fops = {
 	.llseek		= no_llseek,
 };
 
-#define coresight_etb10_reg(name, offset)		\
-	coresight_simple_reg32(struct etb_drvdata, name, offset)
-
-coresight_etb10_reg(rdp, ETB_RAM_DEPTH_REG);
-coresight_etb10_reg(sts, ETB_STATUS_REG);
-coresight_etb10_reg(rrp, ETB_RAM_READ_POINTER);
-coresight_etb10_reg(rwp, ETB_RAM_WRITE_POINTER);
-coresight_etb10_reg(trg, ETB_TRG);
-coresight_etb10_reg(ctl, ETB_CTL_REG);
-coresight_etb10_reg(ffsr, ETB_FFSR);
-coresight_etb10_reg(ffcr, ETB_FFCR);
+coresight_simple_reg32(rdp, ETB_RAM_DEPTH_REG);
+coresight_simple_reg32(sts, ETB_STATUS_REG);
+coresight_simple_reg32(rrp, ETB_RAM_READ_POINTER);
+coresight_simple_reg32(rwp, ETB_RAM_WRITE_POINTER);
+coresight_simple_reg32(trg, ETB_TRG);
+coresight_simple_reg32(ctl, ETB_CTL_REG);
+coresight_simple_reg32(ffsr, ETB_FFSR);
+coresight_simple_reg32(ffcr, ETB_FFCR);
 
 static struct attribute *coresight_etb_mgmt_attrs[] = {
 	&dev_attr_rdp.attr,
diff --git a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
index 68fcbf4ce7a8..12f8e8176c7e 100644
--- a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
+++ b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
@@ -1252,19 +1252,16 @@ static struct attribute *coresight_etm_attrs[] = {
 	NULL,
 };
 
-#define coresight_etm3x_reg(name, offset)			\
-	coresight_simple_reg32(struct etm_drvdata, name, offset)
-
-coresight_etm3x_reg(etmccr, ETMCCR);
-coresight_etm3x_reg(etmccer, ETMCCER);
-coresight_etm3x_reg(etmscr, ETMSCR);
-coresight_etm3x_reg(etmidr, ETMIDR);
-coresight_etm3x_reg(etmcr, ETMCR);
-coresight_etm3x_reg(etmtraceidr, ETMTRACEIDR);
-coresight_etm3x_reg(etmteevr, ETMTEEVR);
-coresight_etm3x_reg(etmtssvr, ETMTSSCR);
-coresight_etm3x_reg(etmtecr1, ETMTECR1);
-coresight_etm3x_reg(etmtecr2, ETMTECR2);
+coresight_simple_reg32(etmccr, ETMCCR);
+coresight_simple_reg32(etmccer, ETMCCER);
+coresight_simple_reg32(etmscr, ETMSCR);
+coresight_simple_reg32(etmidr, ETMIDR);
+coresight_simple_reg32(etmcr, ETMCR);
+coresight_simple_reg32(etmtraceidr, ETMTRACEIDR);
+coresight_simple_reg32(etmteevr, ETMTEEVR);
+coresight_simple_reg32(etmtssvr, ETMTSSCR);
+coresight_simple_reg32(etmtecr1, ETMTECR1);
+coresight_simple_reg32(etmtecr2, ETMTECR2);
 
 static struct attribute *coresight_etm_mgmt_attrs[] = {
 	&dev_attr_etmccr.attr,
diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h
index f2458b794ef3..cf8ae768106e 100644
--- a/drivers/hwtracing/coresight/coresight-priv.h
+++ b/drivers/hwtracing/coresight/coresight-priv.h
@@ -40,23 +40,23 @@
 #define ETM_MODE_EXCL_KERN	BIT(30)
 #define ETM_MODE_EXCL_USER	BIT(31)
 
-#define __coresight_simple_show(type, name, lo_off, hi_off)		\
+#define __coresight_simple_show(name, lo_off, hi_off)		\
 static ssize_t name##_show(struct device *_dev,				\
 			   struct device_attribute *attr, char *buf)	\
 {									\
-	type *drvdata = dev_get_drvdata(_dev->parent);			\
+	struct coresight_device *csdev = container_of(_dev, struct coresight_device, dev); \
 	u64 val;							\
 	pm_runtime_get_sync(_dev->parent);				\
-	val = coresight_read_reg_pair(drvdata->base, lo_off, hi_off);	\
+	val = csdev_access_relaxed_read_pair(&csdev->access, lo_off, hi_off);	\
 	pm_runtime_put_sync(_dev->parent);				\
 	return scnprintf(buf, PAGE_SIZE, "0x%llx\n", val);		\
 }									\
 static DEVICE_ATTR_RO(name)
 
-#define coresight_simple_reg32(type, name, offset)			\
-	__coresight_simple_show(type, name, offset, -1)
-#define coresight_simple_reg64(type, name, lo_off, hi_off)		\
-	__coresight_simple_show(type, name, lo_off, hi_off)
+#define coresight_simple_reg32(name, offset)			\
+	__coresight_simple_show(name, offset, -1)
+#define coresight_simple_reg64(name, lo_off, hi_off)		\
+	__coresight_simple_show(name, lo_off, hi_off)
 
 extern const u32 coresight_barrier_pkt[4];
 #define CORESIGHT_BARRIER_PKT_SIZE (sizeof(coresight_barrier_pkt))
diff --git a/drivers/hwtracing/coresight/coresight-replicator.c b/drivers/hwtracing/coresight/coresight-replicator.c
index b86acbc74cf0..7cffcbb2ec42 100644
--- a/drivers/hwtracing/coresight/coresight-replicator.c
+++ b/drivers/hwtracing/coresight/coresight-replicator.c
@@ -196,11 +196,8 @@ static const struct coresight_ops replicator_cs_ops = {
 	.link_ops	= &replicator_link_ops,
 };
 
-#define coresight_replicator_reg(name, offset) \
-	coresight_simple_reg32(struct replicator_drvdata, name, offset)
-
-coresight_replicator_reg(idfilter0, REPLICATOR_IDFILTER0);
-coresight_replicator_reg(idfilter1, REPLICATOR_IDFILTER1);
+coresight_simple_reg32(idfilter0, REPLICATOR_IDFILTER0);
+coresight_simple_reg32(idfilter1, REPLICATOR_IDFILTER1);
 
 static struct attribute *replicator_mgmt_attrs[] = {
 	&dev_attr_idfilter0.attr,
diff --git a/drivers/hwtracing/coresight/coresight-stm.c b/drivers/hwtracing/coresight/coresight-stm.c
index bb14a3a8a921..4a31905604fe 100644
--- a/drivers/hwtracing/coresight/coresight-stm.c
+++ b/drivers/hwtracing/coresight/coresight-stm.c
@@ -634,21 +634,18 @@ static ssize_t traceid_store(struct device *dev,
 }
 static DEVICE_ATTR_RW(traceid);
 
-#define coresight_stm_reg(name, offset)	\
-	coresight_simple_reg32(struct stm_drvdata, name, offset)
-
-coresight_stm_reg(tcsr, STMTCSR);
-coresight_stm_reg(tsfreqr, STMTSFREQR);
-coresight_stm_reg(syncr, STMSYNCR);
-coresight_stm_reg(sper, STMSPER);
-coresight_stm_reg(spter, STMSPTER);
-coresight_stm_reg(privmaskr, STMPRIVMASKR);
-coresight_stm_reg(spscr, STMSPSCR);
-coresight_stm_reg(spmscr, STMSPMSCR);
-coresight_stm_reg(spfeat1r, STMSPFEAT1R);
-coresight_stm_reg(spfeat2r, STMSPFEAT2R);
-coresight_stm_reg(spfeat3r, STMSPFEAT3R);
-coresight_stm_reg(devid, CORESIGHT_DEVID);
+coresight_simple_reg32(tcsr, STMTCSR);
+coresight_simple_reg32(tsfreqr, STMTSFREQR);
+coresight_simple_reg32(syncr, STMSYNCR);
+coresight_simple_reg32(sper, STMSPER);
+coresight_simple_reg32(spter, STMSPTER);
+coresight_simple_reg32(privmaskr, STMPRIVMASKR);
+coresight_simple_reg32(spscr, STMSPSCR);
+coresight_simple_reg32(spmscr, STMSPMSCR);
+coresight_simple_reg32(spfeat1r, STMSPFEAT1R);
+coresight_simple_reg32(spfeat2r, STMSPFEAT2R);
+coresight_simple_reg32(spfeat3r, STMSPFEAT3R);
+coresight_simple_reg32(devid, CORESIGHT_DEVID);
 
 static struct attribute *coresight_stm_attrs[] = {
 	&dev_attr_hwevent_enable.attr,
diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c b/drivers/hwtracing/coresight/coresight-tmc-core.c
index d0276af82494..781d213526b7 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-core.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-core.c
@@ -251,25 +251,20 @@ static enum tmc_mem_intf_width tmc_get_memwidth(u32 devid)
 	return memwidth;
 }
 
-#define coresight_tmc_reg(name, offset)			\
-	coresight_simple_reg32(struct tmc_drvdata, name, offset)
-#define coresight_tmc_reg64(name, lo_off, hi_off)	\
-	coresight_simple_reg64(struct tmc_drvdata, name, lo_off, hi_off)
-
-coresight_tmc_reg(rsz, TMC_RSZ);
-coresight_tmc_reg(sts, TMC_STS);
-coresight_tmc_reg(trg, TMC_TRG);
-coresight_tmc_reg(ctl, TMC_CTL);
-coresight_tmc_reg(ffsr, TMC_FFSR);
-coresight_tmc_reg(ffcr, TMC_FFCR);
-coresight_tmc_reg(mode, TMC_MODE);
-coresight_tmc_reg(pscr, TMC_PSCR);
-coresight_tmc_reg(axictl, TMC_AXICTL);
-coresight_tmc_reg(authstatus, TMC_AUTHSTATUS);
-coresight_tmc_reg(devid, CORESIGHT_DEVID);
-coresight_tmc_reg64(rrp, TMC_RRP, TMC_RRPHI);
-coresight_tmc_reg64(rwp, TMC_RWP, TMC_RWPHI);
-coresight_tmc_reg64(dba, TMC_DBALO, TMC_DBAHI);
+coresight_simple_reg32(rsz, TMC_RSZ);
+coresight_simple_reg32(sts, TMC_STS);
+coresight_simple_reg32(trg, TMC_TRG);
+coresight_simple_reg32(ctl, TMC_CTL);
+coresight_simple_reg32(ffsr, TMC_FFSR);
+coresight_simple_reg32(ffcr, TMC_FFCR);
+coresight_simple_reg32(mode, TMC_MODE);
+coresight_simple_reg32(pscr, TMC_PSCR);
+coresight_simple_reg32(axictl, TMC_AXICTL);
+coresight_simple_reg32(authstatus, TMC_AUTHSTATUS);
+coresight_simple_reg32(devid, CORESIGHT_DEVID);
+coresight_simple_reg64(rrp, TMC_RRP, TMC_RRPHI);
+coresight_simple_reg64(rwp, TMC_RWP, TMC_RWPHI);
+coresight_simple_reg64(dba, TMC_DBALO, TMC_DBAHI);
 
 static struct attribute *coresight_tmc_mgmt_attrs[] = {
 	&dev_attr_rsz.attr,
diff --git a/include/linux/coresight.h b/include/linux/coresight.h
index 9f445f09fcfe..a47dd1f62216 100644
--- a/include/linux/coresight.h
+++ b/include/linux/coresight.h
@@ -372,6 +372,24 @@ static inline u32 csdev_access_relaxed_read32(struct csdev_access *csa,
 	return csa->read(offset, true, false);
 }
 
+static inline u64 csdev_access_relaxed_read_pair(struct csdev_access *csa,
+						 s32 lo_offset, s32 hi_offset)
+{
+	u64 val;
+
+	if (likely(csa->io_mem)) {
+		val = readl_relaxed(csa->base + lo_offset);
+		val |= (hi_offset < 0) ? 0 :
+		       (u64)readl_relaxed(csa->base + hi_offset) << 32;
+		return val;
+	}
+
+	val = csa->read(lo_offset, true, false);
+	val |= (hi_offset < 0) ? 0 :
+	       (u64)csa->read(hi_offset, true, false) << 32;
+	return val;
+}
+
 static inline u32 csdev_access_read32(struct csdev_access *csa, u32 offset)
 {
 	if (likely(csa->io_mem))
-- 
2.28.0

[PATCH v2 3/5] coresight: Re-use same function for similar sysfs register accessors

[James Clark]

Currently each accessor macro creates an identical function which wastes
space in the text area and pollutes the ftrace function names. Change it
so that the same function is used, but the register to access is passed
in as parameter rather than baked into each function.

Signed-off-by: James Clark <james.clark@arm.com>
---
 drivers/hwtracing/coresight/coresight-catu.c  | 25 ++++-------
 drivers/hwtracing/coresight/coresight-core.c  | 14 ++++++
 drivers/hwtracing/coresight/coresight-etb10.c | 25 ++++-------
 .../coresight/coresight-etm3x-sysfs.c         | 31 +++++--------
 drivers/hwtracing/coresight/coresight-priv.h  | 40 +++++++++--------
 .../coresight/coresight-replicator.c          |  7 +--
 drivers/hwtracing/coresight/coresight-stm.c   | 37 ++++++----------
 .../hwtracing/coresight/coresight-tmc-core.c  | 43 ++++++-------------
 8 files changed, 91 insertions(+), 131 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-catu.c b/drivers/hwtracing/coresight/coresight-catu.c
index 9d89c4054046..bc90a03f478f 100644
--- a/drivers/hwtracing/coresight/coresight-catu.c
+++ b/drivers/hwtracing/coresight/coresight-catu.c
@@ -365,24 +365,15 @@ static const struct etr_buf_operations etr_catu_buf_ops = {
 	.get_data = catu_get_data_etr_buf,
 };
 
-coresight_simple_reg32(devid, CORESIGHT_DEVID);
-coresight_simple_reg32(control, CATU_CONTROL);
-coresight_simple_reg32(status, CATU_STATUS);
-coresight_simple_reg32(mode, CATU_MODE);
-coresight_simple_reg32(axictrl, CATU_AXICTRL);
-coresight_simple_reg32(irqen, CATU_IRQEN);
-coresight_simple_reg64(sladdr, CATU_SLADDRLO, CATU_SLADDRHI);
-coresight_simple_reg64(inaddr, CATU_INADDRLO, CATU_INADDRHI);
-
 static struct attribute *catu_mgmt_attrs[] = {
-	&dev_attr_devid.attr,
-	&dev_attr_control.attr,
-	&dev_attr_status.attr,
-	&dev_attr_mode.attr,
-	&dev_attr_axictrl.attr,
-	&dev_attr_irqen.attr,
-	&dev_attr_sladdr.attr,
-	&dev_attr_inaddr.attr,
+	coresight_simple_reg32(devid, CORESIGHT_DEVID),
+	coresight_simple_reg32(control, CATU_CONTROL),
+	coresight_simple_reg32(status, CATU_STATUS),
+	coresight_simple_reg32(mode, CATU_MODE),
+	coresight_simple_reg32(axictrl, CATU_AXICTRL),
+	coresight_simple_reg32(irqen, CATU_IRQEN),
+	coresight_simple_reg64(sladdr, CATU_SLADDRLO, CATU_SLADDRHI),
+	coresight_simple_reg64(inaddr, CATU_INADDRLO, CATU_INADDRHI),
 	NULL,
 };
 
diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
index 1edfec1e9d18..c63b2167a69f 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -60,6 +60,20 @@ EXPORT_SYMBOL_GPL(coresight_barrier_pkt);
 
 static const struct cti_assoc_op *cti_assoc_ops;
 
+ssize_t coresight_simple_show(struct device *_dev,
+			      struct device_attribute *attr, char *buf)
+{
+	struct coresight_device *csdev = container_of(_dev, struct coresight_device, dev);
+	struct cs_pair_attribute *cs_attr = container_of(attr, struct cs_pair_attribute, attr);
+	u64 val;
+
+	pm_runtime_get_sync(_dev->parent);
+	val = csdev_access_relaxed_read_pair(&csdev->access, cs_attr->lo_off, cs_attr->hi_off);
+	pm_runtime_put_sync(_dev->parent);
+	return sysfs_emit(buf, "0x%llx\n", val);
+}
+EXPORT_SYMBOL_GPL(coresight_simple_show);
+
 void coresight_set_cti_ops(const struct cti_assoc_op *cti_op)
 {
 	cti_assoc_ops = cti_op;
diff --git a/drivers/hwtracing/coresight/coresight-etb10.c b/drivers/hwtracing/coresight/coresight-etb10.c
index 405bb3355cb1..8aa6e4f83e42 100644
--- a/drivers/hwtracing/coresight/coresight-etb10.c
+++ b/drivers/hwtracing/coresight/coresight-etb10.c
@@ -655,24 +655,15 @@ static const struct file_operations etb_fops = {
 	.llseek		= no_llseek,
 };
 
-coresight_simple_reg32(rdp, ETB_RAM_DEPTH_REG);
-coresight_simple_reg32(sts, ETB_STATUS_REG);
-coresight_simple_reg32(rrp, ETB_RAM_READ_POINTER);
-coresight_simple_reg32(rwp, ETB_RAM_WRITE_POINTER);
-coresight_simple_reg32(trg, ETB_TRG);
-coresight_simple_reg32(ctl, ETB_CTL_REG);
-coresight_simple_reg32(ffsr, ETB_FFSR);
-coresight_simple_reg32(ffcr, ETB_FFCR);
-
 static struct attribute *coresight_etb_mgmt_attrs[] = {
-	&dev_attr_rdp.attr,
-	&dev_attr_sts.attr,
-	&dev_attr_rrp.attr,
-	&dev_attr_rwp.attr,
-	&dev_attr_trg.attr,
-	&dev_attr_ctl.attr,
-	&dev_attr_ffsr.attr,
-	&dev_attr_ffcr.attr,
+	coresight_simple_reg32(rdp, ETB_RAM_DEPTH_REG),
+	coresight_simple_reg32(sts, ETB_STATUS_REG),
+	coresight_simple_reg32(rrp, ETB_RAM_READ_POINTER),
+	coresight_simple_reg32(rwp, ETB_RAM_WRITE_POINTER),
+	coresight_simple_reg32(trg, ETB_TRG),
+	coresight_simple_reg32(ctl, ETB_CTL_REG),
+	coresight_simple_reg32(ffsr, ETB_FFSR),
+	coresight_simple_reg32(ffcr, ETB_FFCR),
 	NULL,
 };
 
diff --git a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
index 12f8e8176c7e..fd81eca3ec18 100644
--- a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
+++ b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
@@ -1252,28 +1252,17 @@ static struct attribute *coresight_etm_attrs[] = {
 	NULL,
 };
 
-coresight_simple_reg32(etmccr, ETMCCR);
-coresight_simple_reg32(etmccer, ETMCCER);
-coresight_simple_reg32(etmscr, ETMSCR);
-coresight_simple_reg32(etmidr, ETMIDR);
-coresight_simple_reg32(etmcr, ETMCR);
-coresight_simple_reg32(etmtraceidr, ETMTRACEIDR);
-coresight_simple_reg32(etmteevr, ETMTEEVR);
-coresight_simple_reg32(etmtssvr, ETMTSSCR);
-coresight_simple_reg32(etmtecr1, ETMTECR1);
-coresight_simple_reg32(etmtecr2, ETMTECR2);
-
 static struct attribute *coresight_etm_mgmt_attrs[] = {
-	&dev_attr_etmccr.attr,
-	&dev_attr_etmccer.attr,
-	&dev_attr_etmscr.attr,
-	&dev_attr_etmidr.attr,
-	&dev_attr_etmcr.attr,
-	&dev_attr_etmtraceidr.attr,
-	&dev_attr_etmteevr.attr,
-	&dev_attr_etmtssvr.attr,
-	&dev_attr_etmtecr1.attr,
-	&dev_attr_etmtecr2.attr,
+	coresight_simple_reg32(etmccr, ETMCCR),
+	coresight_simple_reg32(etmccer, ETMCCER),
+	coresight_simple_reg32(etmscr, ETMSCR),
+	coresight_simple_reg32(etmidr, ETMIDR),
+	coresight_simple_reg32(etmcr, ETMCR),
+	coresight_simple_reg32(etmtraceidr, ETMTRACEIDR),
+	coresight_simple_reg32(etmteevr, ETMTEEVR),
+	coresight_simple_reg32(etmtssvr, ETMTSSCR),
+	coresight_simple_reg32(etmtecr1, ETMTECR1),
+	coresight_simple_reg32(etmtecr2, ETMTECR2),
 	NULL,
 };
 
diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h
index cf8ae768106e..07b392bfdbcd 100644
--- a/drivers/hwtracing/coresight/coresight-priv.h
+++ b/drivers/hwtracing/coresight/coresight-priv.h
@@ -39,24 +39,30 @@
 
 #define ETM_MODE_EXCL_KERN	BIT(30)
 #define ETM_MODE_EXCL_USER	BIT(31)
+struct cs_pair_attribute {
+	struct device_attribute attr;
+	s32 lo_off;
+	s32 hi_off;
+};
 
-#define __coresight_simple_show(name, lo_off, hi_off)		\
-static ssize_t name##_show(struct device *_dev,				\
-			   struct device_attribute *attr, char *buf)	\
-{									\
-	struct coresight_device *csdev = container_of(_dev, struct coresight_device, dev); \
-	u64 val;							\
-	pm_runtime_get_sync(_dev->parent);				\
-	val = csdev_access_relaxed_read_pair(&csdev->access, lo_off, hi_off);	\
-	pm_runtime_put_sync(_dev->parent);				\
-	return scnprintf(buf, PAGE_SIZE, "0x%llx\n", val);		\
-}									\
-static DEVICE_ATTR_RO(name)
-
-#define coresight_simple_reg32(name, offset)			\
-	__coresight_simple_show(name, offset, -1)
-#define coresight_simple_reg64(name, lo_off, hi_off)		\
-	__coresight_simple_show(name, lo_off, hi_off)
+extern ssize_t coresight_simple_show(struct device *_dev,
+				     struct device_attribute *attr, char *buf);
+
+#define coresight_simple_reg32(name, offset)				\
+	(&((struct cs_pair_attribute[]) {				\
+	   {								\
+		__ATTR(name, 0444, coresight_simple_show, NULL),	\
+		offset, -1						\
+	   }								\
+	})[0].attr.attr)
+
+#define coresight_simple_reg64(name, lo_off, hi_off)			\
+	(&((struct cs_pair_attribute[]) {				\
+	   {								\
+		__ATTR(name, 0444, coresight_simple_show, NULL),	\
+		lo_off, hi_off						\
+	   }								\
+	})[0].attr.attr)
 
 extern const u32 coresight_barrier_pkt[4];
 #define CORESIGHT_BARRIER_PKT_SIZE (sizeof(coresight_barrier_pkt))
diff --git a/drivers/hwtracing/coresight/coresight-replicator.c b/drivers/hwtracing/coresight/coresight-replicator.c
index 7cffcbb2ec42..4dd50546d7e4 100644
--- a/drivers/hwtracing/coresight/coresight-replicator.c
+++ b/drivers/hwtracing/coresight/coresight-replicator.c
@@ -196,12 +196,9 @@ static const struct coresight_ops replicator_cs_ops = {
 	.link_ops	= &replicator_link_ops,
 };
 
-coresight_simple_reg32(idfilter0, REPLICATOR_IDFILTER0);
-coresight_simple_reg32(idfilter1, REPLICATOR_IDFILTER1);
-
 static struct attribute *replicator_mgmt_attrs[] = {
-	&dev_attr_idfilter0.attr,
-	&dev_attr_idfilter1.attr,
+	coresight_simple_reg32(idfilter0, REPLICATOR_IDFILTER0),
+	coresight_simple_reg32(idfilter1, REPLICATOR_IDFILTER1),
 	NULL,
 };
 
diff --git a/drivers/hwtracing/coresight/coresight-stm.c b/drivers/hwtracing/coresight/coresight-stm.c
index 4a31905604fe..463f449cfb79 100644
--- a/drivers/hwtracing/coresight/coresight-stm.c
+++ b/drivers/hwtracing/coresight/coresight-stm.c
@@ -634,19 +634,6 @@ static ssize_t traceid_store(struct device *dev,
 }
 static DEVICE_ATTR_RW(traceid);
 
-coresight_simple_reg32(tcsr, STMTCSR);
-coresight_simple_reg32(tsfreqr, STMTSFREQR);
-coresight_simple_reg32(syncr, STMSYNCR);
-coresight_simple_reg32(sper, STMSPER);
-coresight_simple_reg32(spter, STMSPTER);
-coresight_simple_reg32(privmaskr, STMPRIVMASKR);
-coresight_simple_reg32(spscr, STMSPSCR);
-coresight_simple_reg32(spmscr, STMSPMSCR);
-coresight_simple_reg32(spfeat1r, STMSPFEAT1R);
-coresight_simple_reg32(spfeat2r, STMSPFEAT2R);
-coresight_simple_reg32(spfeat3r, STMSPFEAT3R);
-coresight_simple_reg32(devid, CORESIGHT_DEVID);
-
 static struct attribute *coresight_stm_attrs[] = {
 	&dev_attr_hwevent_enable.attr,
 	&dev_attr_hwevent_select.attr,
@@ -657,18 +644,18 @@ static struct attribute *coresight_stm_attrs[] = {
 };
 
 static struct attribute *coresight_stm_mgmt_attrs[] = {
-	&dev_attr_tcsr.attr,
-	&dev_attr_tsfreqr.attr,
-	&dev_attr_syncr.attr,
-	&dev_attr_sper.attr,
-	&dev_attr_spter.attr,
-	&dev_attr_privmaskr.attr,
-	&dev_attr_spscr.attr,
-	&dev_attr_spmscr.attr,
-	&dev_attr_spfeat1r.attr,
-	&dev_attr_spfeat2r.attr,
-	&dev_attr_spfeat3r.attr,
-	&dev_attr_devid.attr,
+	coresight_simple_reg32(tcsr, STMTCSR),
+	coresight_simple_reg32(tsfreqr, STMTSFREQR),
+	coresight_simple_reg32(syncr, STMSYNCR),
+	coresight_simple_reg32(sper, STMSPER),
+	coresight_simple_reg32(spter, STMSPTER),
+	coresight_simple_reg32(privmaskr, STMPRIVMASKR),
+	coresight_simple_reg32(spscr, STMSPSCR),
+	coresight_simple_reg32(spmscr, STMSPMSCR),
+	coresight_simple_reg32(spfeat1r, STMSPFEAT1R),
+	coresight_simple_reg32(spfeat2r, STMSPFEAT2R),
+	coresight_simple_reg32(spfeat3r, STMSPFEAT3R),
+	coresight_simple_reg32(devid, CORESIGHT_DEVID),
 	NULL,
 };
 
diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c b/drivers/hwtracing/coresight/coresight-tmc-core.c
index 781d213526b7..07abf28ad725 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-core.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-core.c
@@ -251,36 +251,21 @@ static enum tmc_mem_intf_width tmc_get_memwidth(u32 devid)
 	return memwidth;
 }
 
-coresight_simple_reg32(rsz, TMC_RSZ);
-coresight_simple_reg32(sts, TMC_STS);
-coresight_simple_reg32(trg, TMC_TRG);
-coresight_simple_reg32(ctl, TMC_CTL);
-coresight_simple_reg32(ffsr, TMC_FFSR);
-coresight_simple_reg32(ffcr, TMC_FFCR);
-coresight_simple_reg32(mode, TMC_MODE);
-coresight_simple_reg32(pscr, TMC_PSCR);
-coresight_simple_reg32(axictl, TMC_AXICTL);
-coresight_simple_reg32(authstatus, TMC_AUTHSTATUS);
-coresight_simple_reg32(devid, CORESIGHT_DEVID);
-coresight_simple_reg64(rrp, TMC_RRP, TMC_RRPHI);
-coresight_simple_reg64(rwp, TMC_RWP, TMC_RWPHI);
-coresight_simple_reg64(dba, TMC_DBALO, TMC_DBAHI);
-
 static struct attribute *coresight_tmc_mgmt_attrs[] = {
-	&dev_attr_rsz.attr,
-	&dev_attr_sts.attr,
-	&dev_attr_rrp.attr,
-	&dev_attr_rwp.attr,
-	&dev_attr_trg.attr,
-	&dev_attr_ctl.attr,
-	&dev_attr_ffsr.attr,
-	&dev_attr_ffcr.attr,
-	&dev_attr_mode.attr,
-	&dev_attr_pscr.attr,
-	&dev_attr_devid.attr,
-	&dev_attr_dba.attr,
-	&dev_attr_axictl.attr,
-	&dev_attr_authstatus.attr,
+	coresight_simple_reg32(rsz, TMC_RSZ),
+	coresight_simple_reg32(sts, TMC_STS),
+	coresight_simple_reg64(rrp, TMC_RRP, TMC_RRPHI),
+	coresight_simple_reg64(rwp, TMC_RWP, TMC_RWPHI),
+	coresight_simple_reg32(trg, TMC_TRG),
+	coresight_simple_reg32(ctl, TMC_CTL),
+	coresight_simple_reg32(ffsr, TMC_FFSR),
+	coresight_simple_reg32(ffcr, TMC_FFCR),
+	coresight_simple_reg32(mode, TMC_MODE),
+	coresight_simple_reg32(pscr, TMC_PSCR),
+	coresight_simple_reg32(devid, CORESIGHT_DEVID),
+	coresight_simple_reg64(dba, TMC_DBALO, TMC_DBAHI),
+	coresight_simple_reg32(axictl, TMC_AXICTL),
+	coresight_simple_reg32(authstatus, TMC_AUTHSTATUS),
 	NULL,
 };
 
-- 
2.28.0

[PATCH v2 4/5] coresight: cti-sysfs: Re-use same functions for similar sysfs register accessors

[James Clark]

Currently each accessor macro creates an identical function which wastes
space in the text area and pollutes the ftrace function name list.
Change it so that the same function is used, but the register to access
is passed in as parameter rather than baked into each function.

Note that only the single accessor is used here and not
csdev_access_relaxed_read_pair() like in the previous commit, so
so a single unsigned offset value is stored instead.

Signed-off-by: James Clark <james.clark@arm.com>
---
 .../hwtracing/coresight/coresight-cti-sysfs.c | 213 +++++++-----------
 drivers/hwtracing/coresight/coresight-priv.h  |   5 +
 2 files changed, 91 insertions(+), 127 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-cti-sysfs.c b/drivers/hwtracing/coresight/coresight-cti-sysfs.c
index 7ff7e7780bbf..478b8d38b744 100644
--- a/drivers/hwtracing/coresight/coresight-cti-sysfs.c
+++ b/drivers/hwtracing/coresight/coresight-cti-sysfs.c
@@ -163,48 +163,82 @@ static struct attribute *coresight_cti_attrs[] = {
 
 /* register based attributes */
 
-/* macro to access RO registers with power check only (no enable check). */
-#define coresight_cti_reg(name, offset)			\
-static ssize_t name##_show(struct device *dev,				\
-			   struct device_attribute *attr, char *buf)	\
-{									\
-	struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);	\
-	u32 val = 0;							\
-	pm_runtime_get_sync(dev->parent);				\
-	spin_lock(&drvdata->spinlock);					\
-	if (drvdata->config.hw_powered)					\
-		val = readl_relaxed(drvdata->base + offset);		\
-	spin_unlock(&drvdata->spinlock);				\
-	pm_runtime_put_sync(dev->parent);				\
-	return sprintf(buf, "0x%x\n", val);				\
-}									\
-static DEVICE_ATTR_RO(name)
+/* Read registers with power check only (no enable check). */
+static ssize_t coresight_cti_reg_show(struct device *dev,
+			   struct device_attribute *attr, char *buf)
+{
+	struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);
+	struct cs_off_attribute *cti_attr = container_of(attr, struct cs_off_attribute, attr);
+	u32 val = 0;
 
-/* coresight management registers */
-coresight_cti_reg(devaff0, CTIDEVAFF0);
-coresight_cti_reg(devaff1, CTIDEVAFF1);
-coresight_cti_reg(authstatus, CORESIGHT_AUTHSTATUS);
-coresight_cti_reg(devarch, CORESIGHT_DEVARCH);
-coresight_cti_reg(devid, CORESIGHT_DEVID);
-coresight_cti_reg(devtype, CORESIGHT_DEVTYPE);
-coresight_cti_reg(pidr0, CORESIGHT_PERIPHIDR0);
-coresight_cti_reg(pidr1, CORESIGHT_PERIPHIDR1);
-coresight_cti_reg(pidr2, CORESIGHT_PERIPHIDR2);
-coresight_cti_reg(pidr3, CORESIGHT_PERIPHIDR3);
-coresight_cti_reg(pidr4, CORESIGHT_PERIPHIDR4);
+	pm_runtime_get_sync(dev->parent);
+	spin_lock(&drvdata->spinlock);
+	if (drvdata->config.hw_powered)
+		val = readl_relaxed(drvdata->base + cti_attr->off);
+	spin_unlock(&drvdata->spinlock);
+	pm_runtime_put_sync(dev->parent);
+	return sysfs_emit(buf, "0x%x\n", val);
+}
+
+/* Write registers with power check only (no enable check). */
+static ssize_t coresight_cti_reg_store(struct device *dev,
+				       struct device_attribute *attr,
+				       const char *buf, size_t size)
+{
+	struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);
+	struct cs_off_attribute *cti_attr = container_of(attr, struct cs_off_attribute, attr);
+	unsigned long val = 0;
+
+	if (kstrtoul(buf, 0, &val))
+		return -EINVAL;
 
+	pm_runtime_get_sync(dev->parent);
+	spin_lock(&drvdata->spinlock);
+	if (drvdata->config.hw_powered)
+		cti_write_single_reg(drvdata, cti_attr->off, val);
+	spin_unlock(&drvdata->spinlock);
+	pm_runtime_put_sync(dev->parent);
+	return size;
+}
+
+#define coresight_cti_reg(name, offset)					\
+	(&((struct cs_off_attribute[]) {				\
+	   {								\
+		__ATTR(name, 0444, coresight_cti_reg_show, NULL),	\
+		offset							\
+	   }								\
+	})[0].attr.attr)
+
+#define coresight_cti_reg_rw(name, offset)				\
+	(&((struct cs_off_attribute[]) {				\
+	   {								\
+		__ATTR(name, 0644, coresight_cti_reg_show,		\
+		       coresight_cti_reg_store),			\
+		offset							\
+	   }								\
+	})[0].attr.attr)
+
+#define coresight_cti_reg_wo(name, offset)				\
+	(&((struct cs_off_attribute[]) {				\
+	   {								\
+		__ATTR(name, 0200, NULL, coresight_cti_reg_store),	\
+		offset							\
+	   }								\
+	})[0].attr.attr)
+
+/* coresight management registers */
 static struct attribute *coresight_cti_mgmt_attrs[] = {
-	&dev_attr_devaff0.attr,
-	&dev_attr_devaff1.attr,
-	&dev_attr_authstatus.attr,
-	&dev_attr_devarch.attr,
-	&dev_attr_devid.attr,
-	&dev_attr_devtype.attr,
-	&dev_attr_pidr0.attr,
-	&dev_attr_pidr1.attr,
-	&dev_attr_pidr2.attr,
-	&dev_attr_pidr3.attr,
-	&dev_attr_pidr4.attr,
+	coresight_cti_reg(devaff0, CTIDEVAFF0),
+	coresight_cti_reg(devaff1, CTIDEVAFF1),
+	coresight_cti_reg(authstatus, CORESIGHT_AUTHSTATUS),
+	coresight_cti_reg(devarch, CORESIGHT_DEVARCH),
+	coresight_cti_reg(devid, CORESIGHT_DEVID),
+	coresight_cti_reg(devtype, CORESIGHT_DEVTYPE),
+	coresight_cti_reg(pidr0, CORESIGHT_PERIPHIDR0),
+	coresight_cti_reg(pidr1, CORESIGHT_PERIPHIDR1),
+	coresight_cti_reg(pidr2, CORESIGHT_PERIPHIDR2),
+	coresight_cti_reg(pidr3, CORESIGHT_PERIPHIDR3),
+	coresight_cti_reg(pidr4, CORESIGHT_PERIPHIDR4),
 	NULL,
 };
 
@@ -454,86 +488,11 @@ static ssize_t apppulse_store(struct device *dev,
 }
 static DEVICE_ATTR_WO(apppulse);
 
-coresight_cti_reg(triginstatus, CTITRIGINSTATUS);
-coresight_cti_reg(trigoutstatus, CTITRIGOUTSTATUS);
-coresight_cti_reg(chinstatus, CTICHINSTATUS);
-coresight_cti_reg(choutstatus, CTICHOUTSTATUS);
-
 /*
  * Define CONFIG_CORESIGHT_CTI_INTEGRATION_REGS to enable the access to the
  * integration control registers. Normally only used to investigate connection
  * data.
  */
-#ifdef CONFIG_CORESIGHT_CTI_INTEGRATION_REGS
-
-/* macro to access RW registers with power check only (no enable check). */
-#define coresight_cti_reg_rw(name, offset)				\
-static ssize_t name##_show(struct device *dev,				\
-			   struct device_attribute *attr, char *buf)	\
-{									\
-	struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);	\
-	u32 val = 0;							\
-	pm_runtime_get_sync(dev->parent);				\
-	spin_lock(&drvdata->spinlock);					\
-	if (drvdata->config.hw_powered)					\
-		val = readl_relaxed(drvdata->base + offset);		\
-	spin_unlock(&drvdata->spinlock);				\
-	pm_runtime_put_sync(dev->parent);				\
-	return sprintf(buf, "0x%x\n", val);				\
-}									\
-									\
-static ssize_t name##_store(struct device *dev,				\
-			    struct device_attribute *attr,		\
-			    const char *buf, size_t size)		\
-{									\
-	struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);	\
-	unsigned long val = 0;						\
-	if (kstrtoul(buf, 0, &val))					\
-		return -EINVAL;						\
-									\
-	pm_runtime_get_sync(dev->parent);				\
-	spin_lock(&drvdata->spinlock);					\
-	if (drvdata->config.hw_powered)					\
-		cti_write_single_reg(drvdata, offset, val);		\
-	spin_unlock(&drvdata->spinlock);				\
-	pm_runtime_put_sync(dev->parent);				\
-	return size;							\
-}									\
-static DEVICE_ATTR_RW(name)
-
-/* macro to access WO registers with power check only (no enable check). */
-#define coresight_cti_reg_wo(name, offset)				\
-static ssize_t name##_store(struct device *dev,				\
-			    struct device_attribute *attr,		\
-			    const char *buf, size_t size)		\
-{									\
-	struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);	\
-	unsigned long val = 0;						\
-	if (kstrtoul(buf, 0, &val))					\
-		return -EINVAL;						\
-									\
-	pm_runtime_get_sync(dev->parent);				\
-	spin_lock(&drvdata->spinlock);					\
-	if (drvdata->config.hw_powered)					\
-		cti_write_single_reg(drvdata, offset, val);		\
-	spin_unlock(&drvdata->spinlock);				\
-	pm_runtime_put_sync(dev->parent);				\
-	return size;							\
-}									\
-static DEVICE_ATTR_WO(name)
-
-coresight_cti_reg_rw(itchout, ITCHOUT);
-coresight_cti_reg_rw(ittrigout, ITTRIGOUT);
-coresight_cti_reg_rw(itctrl, CORESIGHT_ITCTRL);
-coresight_cti_reg_wo(itchinack, ITCHINACK);
-coresight_cti_reg_wo(ittriginack, ITTRIGINACK);
-coresight_cti_reg(ittrigin, ITTRIGIN);
-coresight_cti_reg(itchin, ITCHIN);
-coresight_cti_reg(itchoutack, ITCHOUTACK);
-coresight_cti_reg(ittrigoutack, ITTRIGOUTACK);
-
-#endif /* CORESIGHT_CTI_INTEGRATION_REGS */
-
 static struct attribute *coresight_cti_regs_attrs[] = {
 	&dev_attr_inout_sel.attr,
 	&dev_attr_inen.attr,
@@ -544,20 +503,20 @@ static struct attribute *coresight_cti_regs_attrs[] = {
 	&dev_attr_appset.attr,
 	&dev_attr_appclear.attr,
 	&dev_attr_apppulse.attr,
-	&dev_attr_triginstatus.attr,
-	&dev_attr_trigoutstatus.attr,
-	&dev_attr_chinstatus.attr,
-	&dev_attr_choutstatus.attr,
+	coresight_cti_reg(triginstatus, CTITRIGINSTATUS),
+	coresight_cti_reg(trigoutstatus, CTITRIGOUTSTATUS),
+	coresight_cti_reg(chinstatus, CTICHINSTATUS),
+	coresight_cti_reg(choutstatus, CTICHOUTSTATUS),
 #ifdef CONFIG_CORESIGHT_CTI_INTEGRATION_REGS
-	&dev_attr_itctrl.attr,
-	&dev_attr_ittrigin.attr,
-	&dev_attr_itchin.attr,
-	&dev_attr_ittrigout.attr,
-	&dev_attr_itchout.attr,
-	&dev_attr_itchoutack.attr,
-	&dev_attr_ittrigoutack.attr,
-	&dev_attr_ittriginack.attr,
-	&dev_attr_itchinack.attr,
+	coresight_cti_reg_rw(itctrl, CORESIGHT_ITCTRL),
+	coresight_cti_reg(ittrigin, ITTRIGIN),
+	coresight_cti_reg(itchin, ITCHIN),
+	coresight_cti_reg_rw(ittrigout, ITTRIGOUT),
+	coresight_cti_reg_rw(itchout, ITCHOUT),
+	coresight_cti_reg(itchoutack, ITCHOUTACK),
+	coresight_cti_reg(ittrigoutack, ITTRIGOUTACK),
+	coresight_cti_reg_wo(ittriginack, ITTRIGINACK),
+	coresight_cti_reg_wo(itchinack, ITCHINACK),
 #endif
 	NULL,
 };
diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h
index 07b392bfdbcd..c211979deca5 100644
--- a/drivers/hwtracing/coresight/coresight-priv.h
+++ b/drivers/hwtracing/coresight/coresight-priv.h
@@ -45,6 +45,11 @@ struct cs_pair_attribute {
 	s32 hi_off;
 };
 
+struct cs_off_attribute {
+	struct device_attribute attr;
+	u32 off;
+};
+
 extern ssize_t coresight_simple_show(struct device *_dev,
 				     struct device_attribute *attr, char *buf);
 
-- 
2.28.0

[PATCH v2 5/5] coresight: Make new csdev_access offsets unsigned

[James Clark]

New csdev_access functions were added as part of the previous
refactor. In order to make them more consistent with the
existing ones, change any signed offset types to be unsigned.

Now that they are unsigned, stop using hi_off = -1 to signify
a single 32bit access. Instead just call the existing 32bit
accessors. This is also applied to other parts of the codebase,
and the coresight_{read,write}_reg_pair() functions can be
deleted.

Signed-off-by: James Clark <james.clark@arm.com>
---
 drivers/hwtracing/coresight/coresight-catu.h |  8 ++---
 drivers/hwtracing/coresight/coresight-core.c | 18 ++++++++--
 drivers/hwtracing/coresight/coresight-priv.h | 35 +++++---------------
 drivers/hwtracing/coresight/coresight-tmc.h  |  4 +--
 include/linux/coresight.h                    | 27 +++++++++------
 5 files changed, 47 insertions(+), 45 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-catu.h b/drivers/hwtracing/coresight/coresight-catu.h
index 6160c2d75a56..442e034bbfba 100644
--- a/drivers/hwtracing/coresight/coresight-catu.h
+++ b/drivers/hwtracing/coresight/coresight-catu.h
@@ -70,24 +70,24 @@ struct catu_drvdata {
 static inline u32							\
 catu_read_##name(struct catu_drvdata *drvdata)				\
 {									\
-	return coresight_read_reg_pair(drvdata->base, offset, -1);	\
+	return csdev_access_relaxed_read32(&drvdata->csdev->access, offset); \
 }									\
 static inline void							\
 catu_write_##name(struct catu_drvdata *drvdata, u32 val)		\
 {									\
-	coresight_write_reg_pair(drvdata->base, val, offset, -1);	\
+	csdev_access_relaxed_write32(&drvdata->csdev->access, val, offset); \
 }
 
 #define CATU_REG_PAIR(name, lo_off, hi_off)				\
 static inline u64							\
 catu_read_##name(struct catu_drvdata *drvdata)				\
 {									\
-	return coresight_read_reg_pair(drvdata->base, lo_off, hi_off);	\
+	return csdev_access_relaxed_read_pair(&drvdata->csdev->access, lo_off, hi_off); \
 }									\
 static inline void							\
 catu_write_##name(struct catu_drvdata *drvdata, u64 val)		\
 {									\
-	coresight_write_reg_pair(drvdata->base, val, lo_off, hi_off);	\
+	csdev_access_relaxed_write_pair(&drvdata->csdev->access, val, lo_off, hi_off); \
 }
 
 CATU_REG32(control, CATU_CONTROL);
diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
index c63b2167a69f..d5dbc67bacb4 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -60,7 +60,7 @@ EXPORT_SYMBOL_GPL(coresight_barrier_pkt);
 
 static const struct cti_assoc_op *cti_assoc_ops;
 
-ssize_t coresight_simple_show(struct device *_dev,
+ssize_t coresight_simple_show_pair(struct device *_dev,
 			      struct device_attribute *attr, char *buf)
 {
 	struct coresight_device *csdev = container_of(_dev, struct coresight_device, dev);
@@ -72,7 +72,21 @@ ssize_t coresight_simple_show(struct device *_dev,
 	pm_runtime_put_sync(_dev->parent);
 	return sysfs_emit(buf, "0x%llx\n", val);
 }
-EXPORT_SYMBOL_GPL(coresight_simple_show);
+EXPORT_SYMBOL_GPL(coresight_simple_show_pair);
+
+ssize_t coresight_simple_show32(struct device *_dev,
+			      struct device_attribute *attr, char *buf)
+{
+	struct coresight_device *csdev = container_of(_dev, struct coresight_device, dev);
+	struct cs_off_attribute *cs_attr = container_of(attr, struct cs_off_attribute, attr);
+	u64 val;
+
+	pm_runtime_get_sync(_dev->parent);
+	val = csdev_access_relaxed_read32(&csdev->access, cs_attr->off);
+	pm_runtime_put_sync(_dev->parent);
+	return sysfs_emit(buf, "0x%llx\n", val);
+}
+EXPORT_SYMBOL_GPL(coresight_simple_show32);
 
 void coresight_set_cti_ops(const struct cti_assoc_op *cti_op)
 {
diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h
index c211979deca5..595ce5862056 100644
--- a/drivers/hwtracing/coresight/coresight-priv.h
+++ b/drivers/hwtracing/coresight/coresight-priv.h
@@ -41,8 +41,8 @@
 #define ETM_MODE_EXCL_USER	BIT(31)
 struct cs_pair_attribute {
 	struct device_attribute attr;
-	s32 lo_off;
-	s32 hi_off;
+	u32 lo_off;
+	u32 hi_off;
 };
 
 struct cs_off_attribute {
@@ -50,21 +50,23 @@ struct cs_off_attribute {
 	u32 off;
 };
 
-extern ssize_t coresight_simple_show(struct device *_dev,
+extern ssize_t coresight_simple_show32(struct device *_dev,
+				     struct device_attribute *attr, char *buf);
+extern ssize_t coresight_simple_show_pair(struct device *_dev,
 				     struct device_attribute *attr, char *buf);
 
 #define coresight_simple_reg32(name, offset)				\
-	(&((struct cs_pair_attribute[]) {				\
+	(&((struct cs_off_attribute[]) {				\
 	   {								\
-		__ATTR(name, 0444, coresight_simple_show, NULL),	\
-		offset, -1						\
+		__ATTR(name, 0444, coresight_simple_show32, NULL),	\
+		offset							\
 	   }								\
 	})[0].attr.attr)
 
 #define coresight_simple_reg64(name, lo_off, hi_off)			\
 	(&((struct cs_pair_attribute[]) {				\
 	   {								\
-		__ATTR(name, 0444, coresight_simple_show, NULL),	\
+		__ATTR(name, 0444, coresight_simple_show_pair, NULL),	\
 		lo_off, hi_off						\
 	   }								\
 	})[0].attr.attr)
@@ -130,25 +132,6 @@ static inline void CS_UNLOCK(void __iomem *addr)
 	} while (0);
 }
 
-static inline u64
-coresight_read_reg_pair(void __iomem *addr, s32 lo_offset, s32 hi_offset)
-{
-	u64 val;
-
-	val = readl_relaxed(addr + lo_offset);
-	val |= (hi_offset < 0) ? 0 :
-	       (u64)readl_relaxed(addr + hi_offset) << 32;
-	return val;
-}
-
-static inline void coresight_write_reg_pair(void __iomem *addr, u64 val,
-						 s32 lo_offset, s32 hi_offset)
-{
-	writel_relaxed((u32)val, addr + lo_offset);
-	if (hi_offset >= 0)
-		writel_relaxed((u32)(val >> 32), addr + hi_offset);
-}
-
 void coresight_disable_path(struct list_head *path);
 int coresight_enable_path(struct list_head *path, u32 mode, void *sink_data);
 struct coresight_device *coresight_get_sink(struct list_head *path);
diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h
index 6bec20a392b3..66959557cf39 100644
--- a/drivers/hwtracing/coresight/coresight-tmc.h
+++ b/drivers/hwtracing/coresight/coresight-tmc.h
@@ -282,12 +282,12 @@ ssize_t tmc_etr_get_sysfs_trace(struct tmc_drvdata *drvdata,
 static inline u64							\
 tmc_read_##name(struct tmc_drvdata *drvdata)				\
 {									\
-	return coresight_read_reg_pair(drvdata->base, lo_off, hi_off);	\
+	return csdev_access_relaxed_read_pair(&drvdata->csdev->access, lo_off, hi_off); \
 }									\
 static inline void							\
 tmc_write_##name(struct tmc_drvdata *drvdata, u64 val)			\
 {									\
-	coresight_write_reg_pair(drvdata->base, val, lo_off, hi_off);	\
+	csdev_access_relaxed_write_pair(&drvdata->csdev->access, val, lo_off, hi_off); \
 }
 
 TMC_REG_PAIR(rrp, TMC_RRP, TMC_RRPHI)
diff --git a/include/linux/coresight.h b/include/linux/coresight.h
index a47dd1f62216..1554021231f9 100644
--- a/include/linux/coresight.h
+++ b/include/linux/coresight.h
@@ -373,21 +373,26 @@ static inline u32 csdev_access_relaxed_read32(struct csdev_access *csa,
 }
 
 static inline u64 csdev_access_relaxed_read_pair(struct csdev_access *csa,
-						 s32 lo_offset, s32 hi_offset)
+						 u32 lo_offset, u32 hi_offset)
 {
-	u64 val;
-
 	if (likely(csa->io_mem)) {
-		val = readl_relaxed(csa->base + lo_offset);
-		val |= (hi_offset < 0) ? 0 :
-		       (u64)readl_relaxed(csa->base + hi_offset) << 32;
-		return val;
+		return readl_relaxed(csa->base + lo_offset) |
+			((u64)readl_relaxed(csa->base + hi_offset) << 32);
 	}
 
-	val = csa->read(lo_offset, true, false);
-	val |= (hi_offset < 0) ? 0 :
-	       (u64)csa->read(hi_offset, true, false) << 32;
-	return val;
+	return csa->read(lo_offset, true, false) | (csa->read(hi_offset, true, false) << 32);
+}
+
+static inline void csdev_access_relaxed_write_pair(struct csdev_access *csa, u64 val,
+						   u32 lo_offset, u32 hi_offset)
+{
+	if (likely(csa->io_mem)) {
+		writel_relaxed((u32)val, csa->base + lo_offset);
+		writel_relaxed((u32)(val >> 32), csa->base + hi_offset);
+	} else {
+		csa->write((u32)val, lo_offset, true, false);
+		csa->write((u32)(val >> 32), hi_offset, true, false);
+	}
 }
 
 static inline u32 csdev_access_read32(struct csdev_access *csa, u32 offset)
-- 
2.28.0

Re: [PATCH v2 1/5] coresight: Remove unused function parameter

[Mike Leach]

On Tue, 30 Aug 2022 at 18:26, James Clark <james.clark@arm.com> wrote:
>
> The ability to use a custom function in this sysfs show function isn't
> used so remove it.
>
> No functional changes.
>
> Signed-off-by: James Clark <james.clark@arm.com>
> ---
>  drivers/hwtracing/coresight/coresight-priv.h | 16 ++++------------
>  1 file changed, 4 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h
> index ff1dd2092ac5..f2458b794ef3 100644
> --- a/drivers/hwtracing/coresight/coresight-priv.h
> +++ b/drivers/hwtracing/coresight/coresight-priv.h
> @@ -40,31 +40,23 @@
>  #define ETM_MODE_EXCL_KERN     BIT(30)
>  #define ETM_MODE_EXCL_USER     BIT(31)
>
> -typedef u32 (*coresight_read_fn)(const struct device *, u32 offset);
> -#define __coresight_simple_func(type, func, name, lo_off, hi_off)      \
> +#define __coresight_simple_show(type, name, lo_off, hi_off)            \
>  static ssize_t name##_show(struct device *_dev,                                \
>                            struct device_attribute *attr, char *buf)    \
>  {                                                                      \
>         type *drvdata = dev_get_drvdata(_dev->parent);                  \
> -       coresight_read_fn fn = func;                                    \
>         u64 val;                                                        \
>         pm_runtime_get_sync(_dev->parent);                              \
> -       if (fn)                                                         \
> -               val = (u64)fn(_dev->parent, lo_off);                    \
> -       else                                                            \
> -               val = coresight_read_reg_pair(drvdata->base,            \
> -                                                lo_off, hi_off);       \
> +       val = coresight_read_reg_pair(drvdata->base, lo_off, hi_off);   \
>         pm_runtime_put_sync(_dev->parent);                              \
>         return scnprintf(buf, PAGE_SIZE, "0x%llx\n", val);              \
>  }                                                                      \
>  static DEVICE_ATTR_RO(name)
>
> -#define coresight_simple_func(type, func, name, offset)                        \
> -       __coresight_simple_func(type, func, name, offset, -1)
>  #define coresight_simple_reg32(type, name, offset)                     \
> -       __coresight_simple_func(type, NULL, name, offset, -1)
> +       __coresight_simple_show(type, name, offset, -1)
>  #define coresight_simple_reg64(type, name, lo_off, hi_off)             \
> -       __coresight_simple_func(type, NULL, name, lo_off, hi_off)
> +       __coresight_simple_show(type, name, lo_off, hi_off)
>
>  extern const u32 coresight_barrier_pkt[4];
>  #define CORESIGHT_BARRIER_PKT_SIZE (sizeof(coresight_barrier_pkt))
> --
> 2.28.0
>

Reviewed-by: Mike Leach <mike.leach@linaro.org>

-- 
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK

Re: [PATCH v2 2/5] coresight: Simplify sysfs accessors by using csdev_access abstraction

[Mike Leach]

On Tue, 30 Aug 2022 at 18:26, James Clark <james.clark@arm.com> wrote:
>
> The coresight_device struct is available in the sysfs accessor, and this
> contains a csdev_access struct which can be used to access registers.
> Use this instead of passing in the type of each drvdata so that a common
> function can be shared between all the cs drivers.
>
> No functional changes.
>
> Signed-off-by: James Clark <james.clark@arm.com>
> ---
>  drivers/hwtracing/coresight/coresight-catu.c  | 18 +++++-----
>  drivers/hwtracing/coresight/coresight-etb10.c | 19 +++++------
>  .../coresight/coresight-etm3x-sysfs.c         | 23 ++++++-------
>  drivers/hwtracing/coresight/coresight-priv.h  | 14 ++++----
>  .../coresight/coresight-replicator.c          |  7 ++--
>  drivers/hwtracing/coresight/coresight-stm.c   | 27 +++++++--------
>  .../hwtracing/coresight/coresight-tmc-core.c  | 33 ++++++++-----------
>  include/linux/coresight.h                     | 18 ++++++++++
>  8 files changed, 79 insertions(+), 80 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-catu.c b/drivers/hwtracing/coresight/coresight-catu.c
> index e0740c6dbd54..9d89c4054046 100644
> --- a/drivers/hwtracing/coresight/coresight-catu.c
> +++ b/drivers/hwtracing/coresight/coresight-catu.c
> @@ -365,16 +365,14 @@ static const struct etr_buf_operations etr_catu_buf_ops = {
>         .get_data = catu_get_data_etr_buf,
>  };
>
> -coresight_simple_reg32(struct catu_drvdata, devid, CORESIGHT_DEVID);
> -coresight_simple_reg32(struct catu_drvdata, control, CATU_CONTROL);
> -coresight_simple_reg32(struct catu_drvdata, status, CATU_STATUS);
> -coresight_simple_reg32(struct catu_drvdata, mode, CATU_MODE);
> -coresight_simple_reg32(struct catu_drvdata, axictrl, CATU_AXICTRL);
> -coresight_simple_reg32(struct catu_drvdata, irqen, CATU_IRQEN);
> -coresight_simple_reg64(struct catu_drvdata, sladdr,
> -                      CATU_SLADDRLO, CATU_SLADDRHI);
> -coresight_simple_reg64(struct catu_drvdata, inaddr,
> -                      CATU_INADDRLO, CATU_INADDRHI);
> +coresight_simple_reg32(devid, CORESIGHT_DEVID);
> +coresight_simple_reg32(control, CATU_CONTROL);
> +coresight_simple_reg32(status, CATU_STATUS);
> +coresight_simple_reg32(mode, CATU_MODE);
> +coresight_simple_reg32(axictrl, CATU_AXICTRL);
> +coresight_simple_reg32(irqen, CATU_IRQEN);
> +coresight_simple_reg64(sladdr, CATU_SLADDRLO, CATU_SLADDRHI);
> +coresight_simple_reg64(inaddr, CATU_INADDRLO, CATU_INADDRHI);
>
>  static struct attribute *catu_mgmt_attrs[] = {
>         &dev_attr_devid.attr,
> diff --git a/drivers/hwtracing/coresight/coresight-etb10.c b/drivers/hwtracing/coresight/coresight-etb10.c
> index efa39820acec..405bb3355cb1 100644
> --- a/drivers/hwtracing/coresight/coresight-etb10.c
> +++ b/drivers/hwtracing/coresight/coresight-etb10.c
> @@ -655,17 +655,14 @@ static const struct file_operations etb_fops = {
>         .llseek         = no_llseek,
>  };
>
> -#define coresight_etb10_reg(name, offset)              \
> -       coresight_simple_reg32(struct etb_drvdata, name, offset)
> -
> -coresight_etb10_reg(rdp, ETB_RAM_DEPTH_REG);
> -coresight_etb10_reg(sts, ETB_STATUS_REG);
> -coresight_etb10_reg(rrp, ETB_RAM_READ_POINTER);
> -coresight_etb10_reg(rwp, ETB_RAM_WRITE_POINTER);
> -coresight_etb10_reg(trg, ETB_TRG);
> -coresight_etb10_reg(ctl, ETB_CTL_REG);
> -coresight_etb10_reg(ffsr, ETB_FFSR);
> -coresight_etb10_reg(ffcr, ETB_FFCR);
> +coresight_simple_reg32(rdp, ETB_RAM_DEPTH_REG);
> +coresight_simple_reg32(sts, ETB_STATUS_REG);
> +coresight_simple_reg32(rrp, ETB_RAM_READ_POINTER);
> +coresight_simple_reg32(rwp, ETB_RAM_WRITE_POINTER);
> +coresight_simple_reg32(trg, ETB_TRG);
> +coresight_simple_reg32(ctl, ETB_CTL_REG);
> +coresight_simple_reg32(ffsr, ETB_FFSR);
> +coresight_simple_reg32(ffcr, ETB_FFCR);
>
>  static struct attribute *coresight_etb_mgmt_attrs[] = {
>         &dev_attr_rdp.attr,
> diff --git a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
> index 68fcbf4ce7a8..12f8e8176c7e 100644
> --- a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
> +++ b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
> @@ -1252,19 +1252,16 @@ static struct attribute *coresight_etm_attrs[] = {
>         NULL,
>  };
>
> -#define coresight_etm3x_reg(name, offset)                      \
> -       coresight_simple_reg32(struct etm_drvdata, name, offset)
> -
> -coresight_etm3x_reg(etmccr, ETMCCR);
> -coresight_etm3x_reg(etmccer, ETMCCER);
> -coresight_etm3x_reg(etmscr, ETMSCR);
> -coresight_etm3x_reg(etmidr, ETMIDR);
> -coresight_etm3x_reg(etmcr, ETMCR);
> -coresight_etm3x_reg(etmtraceidr, ETMTRACEIDR);
> -coresight_etm3x_reg(etmteevr, ETMTEEVR);
> -coresight_etm3x_reg(etmtssvr, ETMTSSCR);
> -coresight_etm3x_reg(etmtecr1, ETMTECR1);
> -coresight_etm3x_reg(etmtecr2, ETMTECR2);
> +coresight_simple_reg32(etmccr, ETMCCR);
> +coresight_simple_reg32(etmccer, ETMCCER);
> +coresight_simple_reg32(etmscr, ETMSCR);
> +coresight_simple_reg32(etmidr, ETMIDR);
> +coresight_simple_reg32(etmcr, ETMCR);
> +coresight_simple_reg32(etmtraceidr, ETMTRACEIDR);
> +coresight_simple_reg32(etmteevr, ETMTEEVR);
> +coresight_simple_reg32(etmtssvr, ETMTSSCR);
> +coresight_simple_reg32(etmtecr1, ETMTECR1);
> +coresight_simple_reg32(etmtecr2, ETMTECR2);
>
>  static struct attribute *coresight_etm_mgmt_attrs[] = {
>         &dev_attr_etmccr.attr,
> diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h
> index f2458b794ef3..cf8ae768106e 100644
> --- a/drivers/hwtracing/coresight/coresight-priv.h
> +++ b/drivers/hwtracing/coresight/coresight-priv.h
> @@ -40,23 +40,23 @@
>  #define ETM_MODE_EXCL_KERN     BIT(30)
>  #define ETM_MODE_EXCL_USER     BIT(31)
>
> -#define __coresight_simple_show(type, name, lo_off, hi_off)            \
> +#define __coresight_simple_show(name, lo_off, hi_off)          \
>  static ssize_t name##_show(struct device *_dev,                                \
>                            struct device_attribute *attr, char *buf)    \
>  {                                                                      \
> -       type *drvdata = dev_get_drvdata(_dev->parent);                  \
> +       struct coresight_device *csdev = container_of(_dev, struct coresight_device, dev); \
>         u64 val;                                                        \
>         pm_runtime_get_sync(_dev->parent);                              \
> -       val = coresight_read_reg_pair(drvdata->base, lo_off, hi_off);   \
> +       val = csdev_access_relaxed_read_pair(&csdev->access, lo_off, hi_off);   \
>         pm_runtime_put_sync(_dev->parent);                              \
>         return scnprintf(buf, PAGE_SIZE, "0x%llx\n", val);              \
>  }                                                                      \
>  static DEVICE_ATTR_RO(name)
>
> -#define coresight_simple_reg32(type, name, offset)                     \
> -       __coresight_simple_show(type, name, offset, -1)
> -#define coresight_simple_reg64(type, name, lo_off, hi_off)             \
> -       __coresight_simple_show(type, name, lo_off, hi_off)
> +#define coresight_simple_reg32(name, offset)                   \
> +       __coresight_simple_show(name, offset, -1)
> +#define coresight_simple_reg64(name, lo_off, hi_off)           \
> +       __coresight_simple_show(name, lo_off, hi_off)
>
>  extern const u32 coresight_barrier_pkt[4];
>  #define CORESIGHT_BARRIER_PKT_SIZE (sizeof(coresight_barrier_pkt))
> diff --git a/drivers/hwtracing/coresight/coresight-replicator.c b/drivers/hwtracing/coresight/coresight-replicator.c
> index b86acbc74cf0..7cffcbb2ec42 100644
> --- a/drivers/hwtracing/coresight/coresight-replicator.c
> +++ b/drivers/hwtracing/coresight/coresight-replicator.c
> @@ -196,11 +196,8 @@ static const struct coresight_ops replicator_cs_ops = {
>         .link_ops       = &replicator_link_ops,
>  };
>
> -#define coresight_replicator_reg(name, offset) \
> -       coresight_simple_reg32(struct replicator_drvdata, name, offset)
> -
> -coresight_replicator_reg(idfilter0, REPLICATOR_IDFILTER0);
> -coresight_replicator_reg(idfilter1, REPLICATOR_IDFILTER1);
> +coresight_simple_reg32(idfilter0, REPLICATOR_IDFILTER0);
> +coresight_simple_reg32(idfilter1, REPLICATOR_IDFILTER1);
>
>  static struct attribute *replicator_mgmt_attrs[] = {
>         &dev_attr_idfilter0.attr,
> diff --git a/drivers/hwtracing/coresight/coresight-stm.c b/drivers/hwtracing/coresight/coresight-stm.c
> index bb14a3a8a921..4a31905604fe 100644
> --- a/drivers/hwtracing/coresight/coresight-stm.c
> +++ b/drivers/hwtracing/coresight/coresight-stm.c
> @@ -634,21 +634,18 @@ static ssize_t traceid_store(struct device *dev,
>  }
>  static DEVICE_ATTR_RW(traceid);
>
> -#define coresight_stm_reg(name, offset)        \
> -       coresight_simple_reg32(struct stm_drvdata, name, offset)
> -
> -coresight_stm_reg(tcsr, STMTCSR);
> -coresight_stm_reg(tsfreqr, STMTSFREQR);
> -coresight_stm_reg(syncr, STMSYNCR);
> -coresight_stm_reg(sper, STMSPER);
> -coresight_stm_reg(spter, STMSPTER);
> -coresight_stm_reg(privmaskr, STMPRIVMASKR);
> -coresight_stm_reg(spscr, STMSPSCR);
> -coresight_stm_reg(spmscr, STMSPMSCR);
> -coresight_stm_reg(spfeat1r, STMSPFEAT1R);
> -coresight_stm_reg(spfeat2r, STMSPFEAT2R);
> -coresight_stm_reg(spfeat3r, STMSPFEAT3R);
> -coresight_stm_reg(devid, CORESIGHT_DEVID);
> +coresight_simple_reg32(tcsr, STMTCSR);
> +coresight_simple_reg32(tsfreqr, STMTSFREQR);
> +coresight_simple_reg32(syncr, STMSYNCR);
> +coresight_simple_reg32(sper, STMSPER);
> +coresight_simple_reg32(spter, STMSPTER);
> +coresight_simple_reg32(privmaskr, STMPRIVMASKR);
> +coresight_simple_reg32(spscr, STMSPSCR);
> +coresight_simple_reg32(spmscr, STMSPMSCR);
> +coresight_simple_reg32(spfeat1r, STMSPFEAT1R);
> +coresight_simple_reg32(spfeat2r, STMSPFEAT2R);
> +coresight_simple_reg32(spfeat3r, STMSPFEAT3R);
> +coresight_simple_reg32(devid, CORESIGHT_DEVID);
>
>  static struct attribute *coresight_stm_attrs[] = {
>         &dev_attr_hwevent_enable.attr,
> diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c b/drivers/hwtracing/coresight/coresight-tmc-core.c
> index d0276af82494..781d213526b7 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc-core.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc-core.c
> @@ -251,25 +251,20 @@ static enum tmc_mem_intf_width tmc_get_memwidth(u32 devid)
>         return memwidth;
>  }
>
> -#define coresight_tmc_reg(name, offset)                        \
> -       coresight_simple_reg32(struct tmc_drvdata, name, offset)
> -#define coresight_tmc_reg64(name, lo_off, hi_off)      \
> -       coresight_simple_reg64(struct tmc_drvdata, name, lo_off, hi_off)
> -
> -coresight_tmc_reg(rsz, TMC_RSZ);
> -coresight_tmc_reg(sts, TMC_STS);
> -coresight_tmc_reg(trg, TMC_TRG);
> -coresight_tmc_reg(ctl, TMC_CTL);
> -coresight_tmc_reg(ffsr, TMC_FFSR);
> -coresight_tmc_reg(ffcr, TMC_FFCR);
> -coresight_tmc_reg(mode, TMC_MODE);
> -coresight_tmc_reg(pscr, TMC_PSCR);
> -coresight_tmc_reg(axictl, TMC_AXICTL);
> -coresight_tmc_reg(authstatus, TMC_AUTHSTATUS);
> -coresight_tmc_reg(devid, CORESIGHT_DEVID);
> -coresight_tmc_reg64(rrp, TMC_RRP, TMC_RRPHI);
> -coresight_tmc_reg64(rwp, TMC_RWP, TMC_RWPHI);
> -coresight_tmc_reg64(dba, TMC_DBALO, TMC_DBAHI);
> +coresight_simple_reg32(rsz, TMC_RSZ);
> +coresight_simple_reg32(sts, TMC_STS);
> +coresight_simple_reg32(trg, TMC_TRG);
> +coresight_simple_reg32(ctl, TMC_CTL);
> +coresight_simple_reg32(ffsr, TMC_FFSR);
> +coresight_simple_reg32(ffcr, TMC_FFCR);
> +coresight_simple_reg32(mode, TMC_MODE);
> +coresight_simple_reg32(pscr, TMC_PSCR);
> +coresight_simple_reg32(axictl, TMC_AXICTL);
> +coresight_simple_reg32(authstatus, TMC_AUTHSTATUS);
> +coresight_simple_reg32(devid, CORESIGHT_DEVID);
> +coresight_simple_reg64(rrp, TMC_RRP, TMC_RRPHI);
> +coresight_simple_reg64(rwp, TMC_RWP, TMC_RWPHI);
> +coresight_simple_reg64(dba, TMC_DBALO, TMC_DBAHI);
>
>  static struct attribute *coresight_tmc_mgmt_attrs[] = {
>         &dev_attr_rsz.attr,
> diff --git a/include/linux/coresight.h b/include/linux/coresight.h
> index 9f445f09fcfe..a47dd1f62216 100644
> --- a/include/linux/coresight.h
> +++ b/include/linux/coresight.h
> @@ -372,6 +372,24 @@ static inline u32 csdev_access_relaxed_read32(struct csdev_access *csa,
>         return csa->read(offset, true, false);
>  }
>
> +static inline u64 csdev_access_relaxed_read_pair(struct csdev_access *csa,
> +                                                s32 lo_offset, s32 hi_offset)
> +{
> +       u64 val;
> +
> +       if (likely(csa->io_mem)) {
> +               val = readl_relaxed(csa->base + lo_offset);
> +               val |= (hi_offset < 0) ? 0 :
> +                      (u64)readl_relaxed(csa->base + hi_offset) << 32;
> +               return val;
> +       }
> +
> +       val = csa->read(lo_offset, true, false);
> +       val |= (hi_offset < 0) ? 0 :
> +              (u64)csa->read(hi_offset, true, false) << 32;
> +       return val;
> +}
> +
>  static inline u32 csdev_access_read32(struct csdev_access *csa, u32 offset)
>  {
>         if (likely(csa->io_mem))
> --
> 2.28.0
>

Reviewed-by: Mike Leach <mike.leach@linaro.org>
-- 
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK

Re: [PATCH v2 3/5] coresight: Re-use same function for similar sysfs register accessors

[Mike Leach]

On Tue, 30 Aug 2022 at 18:26, James Clark <james.clark@arm.com> wrote:
>
> Currently each accessor macro creates an identical function which wastes
> space in the text area and pollutes the ftrace function names. Change it
> so that the same function is used, but the register to access is passed
> in as parameter rather than baked into each function.
>
> Signed-off-by: James Clark <james.clark@arm.com>
> ---
>  drivers/hwtracing/coresight/coresight-catu.c  | 25 ++++-------
>  drivers/hwtracing/coresight/coresight-core.c  | 14 ++++++
>  drivers/hwtracing/coresight/coresight-etb10.c | 25 ++++-------
>  .../coresight/coresight-etm3x-sysfs.c         | 31 +++++--------
>  drivers/hwtracing/coresight/coresight-priv.h  | 40 +++++++++--------
>  .../coresight/coresight-replicator.c          |  7 +--
>  drivers/hwtracing/coresight/coresight-stm.c   | 37 ++++++----------
>  .../hwtracing/coresight/coresight-tmc-core.c  | 43 ++++++-------------
>  8 files changed, 91 insertions(+), 131 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-catu.c b/drivers/hwtracing/coresight/coresight-catu.c
> index 9d89c4054046..bc90a03f478f 100644
> --- a/drivers/hwtracing/coresight/coresight-catu.c
> +++ b/drivers/hwtracing/coresight/coresight-catu.c
> @@ -365,24 +365,15 @@ static const struct etr_buf_operations etr_catu_buf_ops = {
>         .get_data = catu_get_data_etr_buf,
>  };
>
> -coresight_simple_reg32(devid, CORESIGHT_DEVID);
> -coresight_simple_reg32(control, CATU_CONTROL);
> -coresight_simple_reg32(status, CATU_STATUS);
> -coresight_simple_reg32(mode, CATU_MODE);
> -coresight_simple_reg32(axictrl, CATU_AXICTRL);
> -coresight_simple_reg32(irqen, CATU_IRQEN);
> -coresight_simple_reg64(sladdr, CATU_SLADDRLO, CATU_SLADDRHI);
> -coresight_simple_reg64(inaddr, CATU_INADDRLO, CATU_INADDRHI);
> -
>  static struct attribute *catu_mgmt_attrs[] = {
> -       &dev_attr_devid.attr,
> -       &dev_attr_control.attr,
> -       &dev_attr_status.attr,
> -       &dev_attr_mode.attr,
> -       &dev_attr_axictrl.attr,
> -       &dev_attr_irqen.attr,
> -       &dev_attr_sladdr.attr,
> -       &dev_attr_inaddr.attr,
> +       coresight_simple_reg32(devid, CORESIGHT_DEVID),
> +       coresight_simple_reg32(control, CATU_CONTROL),
> +       coresight_simple_reg32(status, CATU_STATUS),
> +       coresight_simple_reg32(mode, CATU_MODE),
> +       coresight_simple_reg32(axictrl, CATU_AXICTRL),
> +       coresight_simple_reg32(irqen, CATU_IRQEN),
> +       coresight_simple_reg64(sladdr, CATU_SLADDRLO, CATU_SLADDRHI),
> +       coresight_simple_reg64(inaddr, CATU_INADDRLO, CATU_INADDRHI),
>         NULL,
>  };
>
> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
> index 1edfec1e9d18..c63b2167a69f 100644
> --- a/drivers/hwtracing/coresight/coresight-core.c
> +++ b/drivers/hwtracing/coresight/coresight-core.c
> @@ -60,6 +60,20 @@ EXPORT_SYMBOL_GPL(coresight_barrier_pkt);
>
>  static const struct cti_assoc_op *cti_assoc_ops;
>
> +ssize_t coresight_simple_show(struct device *_dev,
> +                             struct device_attribute *attr, char *buf)
> +{
> +       struct coresight_device *csdev = container_of(_dev, struct coresight_device, dev);
> +       struct cs_pair_attribute *cs_attr = container_of(attr, struct cs_pair_attribute, attr);
> +       u64 val;
> +
> +       pm_runtime_get_sync(_dev->parent);
> +       val = csdev_access_relaxed_read_pair(&csdev->access, cs_attr->lo_off, cs_attr->hi_off);
> +       pm_runtime_put_sync(_dev->parent);
> +       return sysfs_emit(buf, "0x%llx\n", val);
> +}
> +EXPORT_SYMBOL_GPL(coresight_simple_show);
> +
>  void coresight_set_cti_ops(const struct cti_assoc_op *cti_op)
>  {
>         cti_assoc_ops = cti_op;
> diff --git a/drivers/hwtracing/coresight/coresight-etb10.c b/drivers/hwtracing/coresight/coresight-etb10.c
> index 405bb3355cb1..8aa6e4f83e42 100644
> --- a/drivers/hwtracing/coresight/coresight-etb10.c
> +++ b/drivers/hwtracing/coresight/coresight-etb10.c
> @@ -655,24 +655,15 @@ static const struct file_operations etb_fops = {
>         .llseek         = no_llseek,
>  };
>
> -coresight_simple_reg32(rdp, ETB_RAM_DEPTH_REG);
> -coresight_simple_reg32(sts, ETB_STATUS_REG);
> -coresight_simple_reg32(rrp, ETB_RAM_READ_POINTER);
> -coresight_simple_reg32(rwp, ETB_RAM_WRITE_POINTER);
> -coresight_simple_reg32(trg, ETB_TRG);
> -coresight_simple_reg32(ctl, ETB_CTL_REG);
> -coresight_simple_reg32(ffsr, ETB_FFSR);
> -coresight_simple_reg32(ffcr, ETB_FFCR);
> -
>  static struct attribute *coresight_etb_mgmt_attrs[] = {
> -       &dev_attr_rdp.attr,
> -       &dev_attr_sts.attr,
> -       &dev_attr_rrp.attr,
> -       &dev_attr_rwp.attr,
> -       &dev_attr_trg.attr,
> -       &dev_attr_ctl.attr,
> -       &dev_attr_ffsr.attr,
> -       &dev_attr_ffcr.attr,
> +       coresight_simple_reg32(rdp, ETB_RAM_DEPTH_REG),
> +       coresight_simple_reg32(sts, ETB_STATUS_REG),
> +       coresight_simple_reg32(rrp, ETB_RAM_READ_POINTER),
> +       coresight_simple_reg32(rwp, ETB_RAM_WRITE_POINTER),
> +       coresight_simple_reg32(trg, ETB_TRG),
> +       coresight_simple_reg32(ctl, ETB_CTL_REG),
> +       coresight_simple_reg32(ffsr, ETB_FFSR),
> +       coresight_simple_reg32(ffcr, ETB_FFCR),
>         NULL,
>  };
>
> diff --git a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
> index 12f8e8176c7e..fd81eca3ec18 100644
> --- a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
> +++ b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
> @@ -1252,28 +1252,17 @@ static struct attribute *coresight_etm_attrs[] = {
>         NULL,
>  };
>
> -coresight_simple_reg32(etmccr, ETMCCR);
> -coresight_simple_reg32(etmccer, ETMCCER);
> -coresight_simple_reg32(etmscr, ETMSCR);
> -coresight_simple_reg32(etmidr, ETMIDR);
> -coresight_simple_reg32(etmcr, ETMCR);
> -coresight_simple_reg32(etmtraceidr, ETMTRACEIDR);
> -coresight_simple_reg32(etmteevr, ETMTEEVR);
> -coresight_simple_reg32(etmtssvr, ETMTSSCR);
> -coresight_simple_reg32(etmtecr1, ETMTECR1);
> -coresight_simple_reg32(etmtecr2, ETMTECR2);
> -
>  static struct attribute *coresight_etm_mgmt_attrs[] = {
> -       &dev_attr_etmccr.attr,
> -       &dev_attr_etmccer.attr,
> -       &dev_attr_etmscr.attr,
> -       &dev_attr_etmidr.attr,
> -       &dev_attr_etmcr.attr,
> -       &dev_attr_etmtraceidr.attr,
> -       &dev_attr_etmteevr.attr,
> -       &dev_attr_etmtssvr.attr,
> -       &dev_attr_etmtecr1.attr,
> -       &dev_attr_etmtecr2.attr,
> +       coresight_simple_reg32(etmccr, ETMCCR),
> +       coresight_simple_reg32(etmccer, ETMCCER),
> +       coresight_simple_reg32(etmscr, ETMSCR),
> +       coresight_simple_reg32(etmidr, ETMIDR),
> +       coresight_simple_reg32(etmcr, ETMCR),
> +       coresight_simple_reg32(etmtraceidr, ETMTRACEIDR),
> +       coresight_simple_reg32(etmteevr, ETMTEEVR),
> +       coresight_simple_reg32(etmtssvr, ETMTSSCR),
> +       coresight_simple_reg32(etmtecr1, ETMTECR1),
> +       coresight_simple_reg32(etmtecr2, ETMTECR2),
>         NULL,
>  };
>
> diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h
> index cf8ae768106e..07b392bfdbcd 100644
> --- a/drivers/hwtracing/coresight/coresight-priv.h
> +++ b/drivers/hwtracing/coresight/coresight-priv.h
> @@ -39,24 +39,30 @@
>
>  #define ETM_MODE_EXCL_KERN     BIT(30)
>  #define ETM_MODE_EXCL_USER     BIT(31)
> +struct cs_pair_attribute {
> +       struct device_attribute attr;
> +       s32 lo_off;
> +       s32 hi_off;
> +};
>
> -#define __coresight_simple_show(name, lo_off, hi_off)          \
> -static ssize_t name##_show(struct device *_dev,                                \
> -                          struct device_attribute *attr, char *buf)    \
> -{                                                                      \
> -       struct coresight_device *csdev = container_of(_dev, struct coresight_device, dev); \
> -       u64 val;                                                        \
> -       pm_runtime_get_sync(_dev->parent);                              \
> -       val = csdev_access_relaxed_read_pair(&csdev->access, lo_off, hi_off);   \
> -       pm_runtime_put_sync(_dev->parent);                              \
> -       return scnprintf(buf, PAGE_SIZE, "0x%llx\n", val);              \
> -}                                                                      \
> -static DEVICE_ATTR_RO(name)
> -
> -#define coresight_simple_reg32(name, offset)                   \
> -       __coresight_simple_show(name, offset, -1)
> -#define coresight_simple_reg64(name, lo_off, hi_off)           \
> -       __coresight_simple_show(name, lo_off, hi_off)
> +extern ssize_t coresight_simple_show(struct device *_dev,
> +                                    struct device_attribute *attr, char *buf);
> +
> +#define coresight_simple_reg32(name, offset)                           \
> +       (&((struct cs_pair_attribute[]) {                               \
> +          {                                                            \
> +               __ATTR(name, 0444, coresight_simple_show, NULL),        \
> +               offset, -1                                              \
> +          }                                                            \
> +       })[0].attr.attr)
> +
> +#define coresight_simple_reg64(name, lo_off, hi_off)                   \
> +       (&((struct cs_pair_attribute[]) {                               \
> +          {                                                            \
> +               __ATTR(name, 0444, coresight_simple_show, NULL),        \
> +               lo_off, hi_off                                          \
> +          }                                                            \
> +       })[0].attr.attr)
>
>  extern const u32 coresight_barrier_pkt[4];
>  #define CORESIGHT_BARRIER_PKT_SIZE (sizeof(coresight_barrier_pkt))
> diff --git a/drivers/hwtracing/coresight/coresight-replicator.c b/drivers/hwtracing/coresight/coresight-replicator.c
> index 7cffcbb2ec42..4dd50546d7e4 100644
> --- a/drivers/hwtracing/coresight/coresight-replicator.c
> +++ b/drivers/hwtracing/coresight/coresight-replicator.c
> @@ -196,12 +196,9 @@ static const struct coresight_ops replicator_cs_ops = {
>         .link_ops       = &replicator_link_ops,
>  };
>
> -coresight_simple_reg32(idfilter0, REPLICATOR_IDFILTER0);
> -coresight_simple_reg32(idfilter1, REPLICATOR_IDFILTER1);
> -
>  static struct attribute *replicator_mgmt_attrs[] = {
> -       &dev_attr_idfilter0.attr,
> -       &dev_attr_idfilter1.attr,
> +       coresight_simple_reg32(idfilter0, REPLICATOR_IDFILTER0),
> +       coresight_simple_reg32(idfilter1, REPLICATOR_IDFILTER1),
>         NULL,
>  };
>
> diff --git a/drivers/hwtracing/coresight/coresight-stm.c b/drivers/hwtracing/coresight/coresight-stm.c
> index 4a31905604fe..463f449cfb79 100644
> --- a/drivers/hwtracing/coresight/coresight-stm.c
> +++ b/drivers/hwtracing/coresight/coresight-stm.c
> @@ -634,19 +634,6 @@ static ssize_t traceid_store(struct device *dev,
>  }
>  static DEVICE_ATTR_RW(traceid);
>
> -coresight_simple_reg32(tcsr, STMTCSR);
> -coresight_simple_reg32(tsfreqr, STMTSFREQR);
> -coresight_simple_reg32(syncr, STMSYNCR);
> -coresight_simple_reg32(sper, STMSPER);
> -coresight_simple_reg32(spter, STMSPTER);
> -coresight_simple_reg32(privmaskr, STMPRIVMASKR);
> -coresight_simple_reg32(spscr, STMSPSCR);
> -coresight_simple_reg32(spmscr, STMSPMSCR);
> -coresight_simple_reg32(spfeat1r, STMSPFEAT1R);
> -coresight_simple_reg32(spfeat2r, STMSPFEAT2R);
> -coresight_simple_reg32(spfeat3r, STMSPFEAT3R);
> -coresight_simple_reg32(devid, CORESIGHT_DEVID);
> -
>  static struct attribute *coresight_stm_attrs[] = {
>         &dev_attr_hwevent_enable.attr,
>         &dev_attr_hwevent_select.attr,
> @@ -657,18 +644,18 @@ static struct attribute *coresight_stm_attrs[] = {
>  };
>
>  static struct attribute *coresight_stm_mgmt_attrs[] = {
> -       &dev_attr_tcsr.attr,
> -       &dev_attr_tsfreqr.attr,
> -       &dev_attr_syncr.attr,
> -       &dev_attr_sper.attr,
> -       &dev_attr_spter.attr,
> -       &dev_attr_privmaskr.attr,
> -       &dev_attr_spscr.attr,
> -       &dev_attr_spmscr.attr,
> -       &dev_attr_spfeat1r.attr,
> -       &dev_attr_spfeat2r.attr,
> -       &dev_attr_spfeat3r.attr,
> -       &dev_attr_devid.attr,
> +       coresight_simple_reg32(tcsr, STMTCSR),
> +       coresight_simple_reg32(tsfreqr, STMTSFREQR),
> +       coresight_simple_reg32(syncr, STMSYNCR),
> +       coresight_simple_reg32(sper, STMSPER),
> +       coresight_simple_reg32(spter, STMSPTER),
> +       coresight_simple_reg32(privmaskr, STMPRIVMASKR),
> +       coresight_simple_reg32(spscr, STMSPSCR),
> +       coresight_simple_reg32(spmscr, STMSPMSCR),
> +       coresight_simple_reg32(spfeat1r, STMSPFEAT1R),
> +       coresight_simple_reg32(spfeat2r, STMSPFEAT2R),
> +       coresight_simple_reg32(spfeat3r, STMSPFEAT3R),
> +       coresight_simple_reg32(devid, CORESIGHT_DEVID),
>         NULL,
>  };
>
> diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c b/drivers/hwtracing/coresight/coresight-tmc-core.c
> index 781d213526b7..07abf28ad725 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc-core.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc-core.c
> @@ -251,36 +251,21 @@ static enum tmc_mem_intf_width tmc_get_memwidth(u32 devid)
>         return memwidth;
>  }
>
> -coresight_simple_reg32(rsz, TMC_RSZ);
> -coresight_simple_reg32(sts, TMC_STS);
> -coresight_simple_reg32(trg, TMC_TRG);
> -coresight_simple_reg32(ctl, TMC_CTL);
> -coresight_simple_reg32(ffsr, TMC_FFSR);
> -coresight_simple_reg32(ffcr, TMC_FFCR);
> -coresight_simple_reg32(mode, TMC_MODE);
> -coresight_simple_reg32(pscr, TMC_PSCR);
> -coresight_simple_reg32(axictl, TMC_AXICTL);
> -coresight_simple_reg32(authstatus, TMC_AUTHSTATUS);
> -coresight_simple_reg32(devid, CORESIGHT_DEVID);
> -coresight_simple_reg64(rrp, TMC_RRP, TMC_RRPHI);
> -coresight_simple_reg64(rwp, TMC_RWP, TMC_RWPHI);
> -coresight_simple_reg64(dba, TMC_DBALO, TMC_DBAHI);
> -
>  static struct attribute *coresight_tmc_mgmt_attrs[] = {
> -       &dev_attr_rsz.attr,
> -       &dev_attr_sts.attr,
> -       &dev_attr_rrp.attr,
> -       &dev_attr_rwp.attr,
> -       &dev_attr_trg.attr,
> -       &dev_attr_ctl.attr,
> -       &dev_attr_ffsr.attr,
> -       &dev_attr_ffcr.attr,
> -       &dev_attr_mode.attr,
> -       &dev_attr_pscr.attr,
> -       &dev_attr_devid.attr,
> -       &dev_attr_dba.attr,
> -       &dev_attr_axictl.attr,
> -       &dev_attr_authstatus.attr,
> +       coresight_simple_reg32(rsz, TMC_RSZ),
> +       coresight_simple_reg32(sts, TMC_STS),
> +       coresight_simple_reg64(rrp, TMC_RRP, TMC_RRPHI),
> +       coresight_simple_reg64(rwp, TMC_RWP, TMC_RWPHI),
> +       coresight_simple_reg32(trg, TMC_TRG),
> +       coresight_simple_reg32(ctl, TMC_CTL),
> +       coresight_simple_reg32(ffsr, TMC_FFSR),
> +       coresight_simple_reg32(ffcr, TMC_FFCR),
> +       coresight_simple_reg32(mode, TMC_MODE),
> +       coresight_simple_reg32(pscr, TMC_PSCR),
> +       coresight_simple_reg32(devid, CORESIGHT_DEVID),
> +       coresight_simple_reg64(dba, TMC_DBALO, TMC_DBAHI),
> +       coresight_simple_reg32(axictl, TMC_AXICTL),
> +       coresight_simple_reg32(authstatus, TMC_AUTHSTATUS),
>         NULL,
>  };
>
> --
> 2.28.0
>

Reviewed-by: Mike Leach <mike.leach@linaro.org>

-- 
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK

Re: [PATCH v2 4/5] coresight: cti-sysfs: Re-use same functions for similar sysfs register accessors

[Mike Leach]

On Tue, 30 Aug 2022 at 18:26, James Clark <james.clark@arm.com> wrote:
>
> Currently each accessor macro creates an identical function which wastes
> space in the text area and pollutes the ftrace function name list.
> Change it so that the same function is used, but the register to access
> is passed in as parameter rather than baked into each function.
>
> Note that only the single accessor is used here and not
> csdev_access_relaxed_read_pair() like in the previous commit, so
> so a single unsigned offset value is stored instead.
>
> Signed-off-by: James Clark <james.clark@arm.com>
> ---
>  .../hwtracing/coresight/coresight-cti-sysfs.c | 213 +++++++-----------
>  drivers/hwtracing/coresight/coresight-priv.h  |   5 +
>  2 files changed, 91 insertions(+), 127 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-cti-sysfs.c b/drivers/hwtracing/coresight/coresight-cti-sysfs.c
> index 7ff7e7780bbf..478b8d38b744 100644
> --- a/drivers/hwtracing/coresight/coresight-cti-sysfs.c
> +++ b/drivers/hwtracing/coresight/coresight-cti-sysfs.c
> @@ -163,48 +163,82 @@ static struct attribute *coresight_cti_attrs[] = {
>
>  /* register based attributes */
>
> -/* macro to access RO registers with power check only (no enable check). */
> -#define coresight_cti_reg(name, offset)                        \
> -static ssize_t name##_show(struct device *dev,                         \
> -                          struct device_attribute *attr, char *buf)    \
> -{                                                                      \
> -       struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);     \
> -       u32 val = 0;                                                    \
> -       pm_runtime_get_sync(dev->parent);                               \
> -       spin_lock(&drvdata->spinlock);                                  \
> -       if (drvdata->config.hw_powered)                                 \
> -               val = readl_relaxed(drvdata->base + offset);            \
> -       spin_unlock(&drvdata->spinlock);                                \
> -       pm_runtime_put_sync(dev->parent);                               \
> -       return sprintf(buf, "0x%x\n", val);                             \
> -}                                                                      \
> -static DEVICE_ATTR_RO(name)
> +/* Read registers with power check only (no enable check). */
> +static ssize_t coresight_cti_reg_show(struct device *dev,
> +                          struct device_attribute *attr, char *buf)
> +{
> +       struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);
> +       struct cs_off_attribute *cti_attr = container_of(attr, struct cs_off_attribute, attr);
> +       u32 val = 0;
>
> -/* coresight management registers */
> -coresight_cti_reg(devaff0, CTIDEVAFF0);
> -coresight_cti_reg(devaff1, CTIDEVAFF1);
> -coresight_cti_reg(authstatus, CORESIGHT_AUTHSTATUS);
> -coresight_cti_reg(devarch, CORESIGHT_DEVARCH);
> -coresight_cti_reg(devid, CORESIGHT_DEVID);
> -coresight_cti_reg(devtype, CORESIGHT_DEVTYPE);
> -coresight_cti_reg(pidr0, CORESIGHT_PERIPHIDR0);
> -coresight_cti_reg(pidr1, CORESIGHT_PERIPHIDR1);
> -coresight_cti_reg(pidr2, CORESIGHT_PERIPHIDR2);
> -coresight_cti_reg(pidr3, CORESIGHT_PERIPHIDR3);
> -coresight_cti_reg(pidr4, CORESIGHT_PERIPHIDR4);
> +       pm_runtime_get_sync(dev->parent);
> +       spin_lock(&drvdata->spinlock);
> +       if (drvdata->config.hw_powered)
> +               val = readl_relaxed(drvdata->base + cti_attr->off);
> +       spin_unlock(&drvdata->spinlock);
> +       pm_runtime_put_sync(dev->parent);
> +       return sysfs_emit(buf, "0x%x\n", val);
> +}
> +
> +/* Write registers with power check only (no enable check). */
> +static ssize_t coresight_cti_reg_store(struct device *dev,
> +                                      struct device_attribute *attr,
> +                                      const char *buf, size_t size)
> +{
> +       struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);
> +       struct cs_off_attribute *cti_attr = container_of(attr, struct cs_off_attribute, attr);
> +       unsigned long val = 0;
> +
> +       if (kstrtoul(buf, 0, &val))
> +               return -EINVAL;
>
> +       pm_runtime_get_sync(dev->parent);
> +       spin_lock(&drvdata->spinlock);
> +       if (drvdata->config.hw_powered)
> +               cti_write_single_reg(drvdata, cti_attr->off, val);
> +       spin_unlock(&drvdata->spinlock);
> +       pm_runtime_put_sync(dev->parent);
> +       return size;
> +}
> +
> +#define coresight_cti_reg(name, offset)                                        \
> +       (&((struct cs_off_attribute[]) {                                \
> +          {                                                            \
> +               __ATTR(name, 0444, coresight_cti_reg_show, NULL),       \
> +               offset                                                  \
> +          }                                                            \
> +       })[0].attr.attr)
> +
> +#define coresight_cti_reg_rw(name, offset)                             \
> +       (&((struct cs_off_attribute[]) {                                \
> +          {                                                            \
> +               __ATTR(name, 0644, coresight_cti_reg_show,              \
> +                      coresight_cti_reg_store),                        \
> +               offset                                                  \
> +          }                                                            \
> +       })[0].attr.attr)
> +
> +#define coresight_cti_reg_wo(name, offset)                             \
> +       (&((struct cs_off_attribute[]) {                                \
> +          {                                                            \
> +               __ATTR(name, 0200, NULL, coresight_cti_reg_store),      \
> +               offset                                                  \
> +          }                                                            \
> +       })[0].attr.attr)
> +
> +/* coresight management registers */
>  static struct attribute *coresight_cti_mgmt_attrs[] = {
> -       &dev_attr_devaff0.attr,
> -       &dev_attr_devaff1.attr,
> -       &dev_attr_authstatus.attr,
> -       &dev_attr_devarch.attr,
> -       &dev_attr_devid.attr,
> -       &dev_attr_devtype.attr,
> -       &dev_attr_pidr0.attr,
> -       &dev_attr_pidr1.attr,
> -       &dev_attr_pidr2.attr,
> -       &dev_attr_pidr3.attr,
> -       &dev_attr_pidr4.attr,
> +       coresight_cti_reg(devaff0, CTIDEVAFF0),
> +       coresight_cti_reg(devaff1, CTIDEVAFF1),
> +       coresight_cti_reg(authstatus, CORESIGHT_AUTHSTATUS),
> +       coresight_cti_reg(devarch, CORESIGHT_DEVARCH),
> +       coresight_cti_reg(devid, CORESIGHT_DEVID),
> +       coresight_cti_reg(devtype, CORESIGHT_DEVTYPE),
> +       coresight_cti_reg(pidr0, CORESIGHT_PERIPHIDR0),
> +       coresight_cti_reg(pidr1, CORESIGHT_PERIPHIDR1),
> +       coresight_cti_reg(pidr2, CORESIGHT_PERIPHIDR2),
> +       coresight_cti_reg(pidr3, CORESIGHT_PERIPHIDR3),
> +       coresight_cti_reg(pidr4, CORESIGHT_PERIPHIDR4),
>         NULL,
>  };
>
> @@ -454,86 +488,11 @@ static ssize_t apppulse_store(struct device *dev,
>  }
>  static DEVICE_ATTR_WO(apppulse);
>
> -coresight_cti_reg(triginstatus, CTITRIGINSTATUS);
> -coresight_cti_reg(trigoutstatus, CTITRIGOUTSTATUS);
> -coresight_cti_reg(chinstatus, CTICHINSTATUS);
> -coresight_cti_reg(choutstatus, CTICHOUTSTATUS);
> -
>  /*
>   * Define CONFIG_CORESIGHT_CTI_INTEGRATION_REGS to enable the access to the
>   * integration control registers. Normally only used to investigate connection
>   * data.
>   */
> -#ifdef CONFIG_CORESIGHT_CTI_INTEGRATION_REGS
> -
> -/* macro to access RW registers with power check only (no enable check). */
> -#define coresight_cti_reg_rw(name, offset)                             \
> -static ssize_t name##_show(struct device *dev,                         \
> -                          struct device_attribute *attr, char *buf)    \
> -{                                                                      \
> -       struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);     \
> -       u32 val = 0;                                                    \
> -       pm_runtime_get_sync(dev->parent);                               \
> -       spin_lock(&drvdata->spinlock);                                  \
> -       if (drvdata->config.hw_powered)                                 \
> -               val = readl_relaxed(drvdata->base + offset);            \
> -       spin_unlock(&drvdata->spinlock);                                \
> -       pm_runtime_put_sync(dev->parent);                               \
> -       return sprintf(buf, "0x%x\n", val);                             \
> -}                                                                      \
> -                                                                       \
> -static ssize_t name##_store(struct device *dev,                                \
> -                           struct device_attribute *attr,              \
> -                           const char *buf, size_t size)               \
> -{                                                                      \
> -       struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);     \
> -       unsigned long val = 0;                                          \
> -       if (kstrtoul(buf, 0, &val))                                     \
> -               return -EINVAL;                                         \
> -                                                                       \
> -       pm_runtime_get_sync(dev->parent);                               \
> -       spin_lock(&drvdata->spinlock);                                  \
> -       if (drvdata->config.hw_powered)                                 \
> -               cti_write_single_reg(drvdata, offset, val);             \
> -       spin_unlock(&drvdata->spinlock);                                \
> -       pm_runtime_put_sync(dev->parent);                               \
> -       return size;                                                    \
> -}                                                                      \
> -static DEVICE_ATTR_RW(name)
> -
> -/* macro to access WO registers with power check only (no enable check). */
> -#define coresight_cti_reg_wo(name, offset)                             \
> -static ssize_t name##_store(struct device *dev,                                \
> -                           struct device_attribute *attr,              \
> -                           const char *buf, size_t size)               \
> -{                                                                      \
> -       struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);     \
> -       unsigned long val = 0;                                          \
> -       if (kstrtoul(buf, 0, &val))                                     \
> -               return -EINVAL;                                         \
> -                                                                       \
> -       pm_runtime_get_sync(dev->parent);                               \
> -       spin_lock(&drvdata->spinlock);                                  \
> -       if (drvdata->config.hw_powered)                                 \
> -               cti_write_single_reg(drvdata, offset, val);             \
> -       spin_unlock(&drvdata->spinlock);                                \
> -       pm_runtime_put_sync(dev->parent);                               \
> -       return size;                                                    \
> -}                                                                      \
> -static DEVICE_ATTR_WO(name)
> -
> -coresight_cti_reg_rw(itchout, ITCHOUT);
> -coresight_cti_reg_rw(ittrigout, ITTRIGOUT);
> -coresight_cti_reg_rw(itctrl, CORESIGHT_ITCTRL);
> -coresight_cti_reg_wo(itchinack, ITCHINACK);
> -coresight_cti_reg_wo(ittriginack, ITTRIGINACK);
> -coresight_cti_reg(ittrigin, ITTRIGIN);
> -coresight_cti_reg(itchin, ITCHIN);
> -coresight_cti_reg(itchoutack, ITCHOUTACK);
> -coresight_cti_reg(ittrigoutack, ITTRIGOUTACK);
> -
> -#endif /* CORESIGHT_CTI_INTEGRATION_REGS */
> -
>  static struct attribute *coresight_cti_regs_attrs[] = {
>         &dev_attr_inout_sel.attr,
>         &dev_attr_inen.attr,
> @@ -544,20 +503,20 @@ static struct attribute *coresight_cti_regs_attrs[] = {
>         &dev_attr_appset.attr,
>         &dev_attr_appclear.attr,
>         &dev_attr_apppulse.attr,
> -       &dev_attr_triginstatus.attr,
> -       &dev_attr_trigoutstatus.attr,
> -       &dev_attr_chinstatus.attr,
> -       &dev_attr_choutstatus.attr,
> +       coresight_cti_reg(triginstatus, CTITRIGINSTATUS),
> +       coresight_cti_reg(trigoutstatus, CTITRIGOUTSTATUS),
> +       coresight_cti_reg(chinstatus, CTICHINSTATUS),
> +       coresight_cti_reg(choutstatus, CTICHOUTSTATUS),
>  #ifdef CONFIG_CORESIGHT_CTI_INTEGRATION_REGS
> -       &dev_attr_itctrl.attr,
> -       &dev_attr_ittrigin.attr,
> -       &dev_attr_itchin.attr,
> -       &dev_attr_ittrigout.attr,
> -       &dev_attr_itchout.attr,
> -       &dev_attr_itchoutack.attr,
> -       &dev_attr_ittrigoutack.attr,
> -       &dev_attr_ittriginack.attr,
> -       &dev_attr_itchinack.attr,
> +       coresight_cti_reg_rw(itctrl, CORESIGHT_ITCTRL),
> +       coresight_cti_reg(ittrigin, ITTRIGIN),
> +       coresight_cti_reg(itchin, ITCHIN),
> +       coresight_cti_reg_rw(ittrigout, ITTRIGOUT),
> +       coresight_cti_reg_rw(itchout, ITCHOUT),
> +       coresight_cti_reg(itchoutack, ITCHOUTACK),
> +       coresight_cti_reg(ittrigoutack, ITTRIGOUTACK),
> +       coresight_cti_reg_wo(ittriginack, ITTRIGINACK),
> +       coresight_cti_reg_wo(itchinack, ITCHINACK),
>  #endif
>         NULL,
>  };
> diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h
> index 07b392bfdbcd..c211979deca5 100644
> --- a/drivers/hwtracing/coresight/coresight-priv.h
> +++ b/drivers/hwtracing/coresight/coresight-priv.h
> @@ -45,6 +45,11 @@ struct cs_pair_attribute {
>         s32 hi_off;
>  };
>
> +struct cs_off_attribute {
> +       struct device_attribute attr;
> +       u32 off;
> +};
> +
>  extern ssize_t coresight_simple_show(struct device *_dev,
>                                      struct device_attribute *attr, char *buf);
>
> --
> 2.28.0
>
Reviewed-by: Mike Leach <mike.leach@linaro.org>


-- 
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK

Re: [PATCH v2 5/5] coresight: Make new csdev_access offsets unsigned

[Mike Leach]

On Tue, 30 Aug 2022 at 18:26, James Clark <james.clark@arm.com> wrote:
>
> New csdev_access functions were added as part of the previous
> refactor. In order to make them more consistent with the
> existing ones, change any signed offset types to be unsigned.
>
> Now that they are unsigned, stop using hi_off = -1 to signify
> a single 32bit access. Instead just call the existing 32bit
> accessors. This is also applied to other parts of the codebase,
> and the coresight_{read,write}_reg_pair() functions can be
> deleted.
>
> Signed-off-by: James Clark <james.clark@arm.com>
> ---
>  drivers/hwtracing/coresight/coresight-catu.h |  8 ++---
>  drivers/hwtracing/coresight/coresight-core.c | 18 ++++++++--
>  drivers/hwtracing/coresight/coresight-priv.h | 35 +++++---------------
>  drivers/hwtracing/coresight/coresight-tmc.h  |  4 +--
>  include/linux/coresight.h                    | 27 +++++++++------
>  5 files changed, 47 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-catu.h b/drivers/hwtracing/coresight/coresight-catu.h
> index 6160c2d75a56..442e034bbfba 100644
> --- a/drivers/hwtracing/coresight/coresight-catu.h
> +++ b/drivers/hwtracing/coresight/coresight-catu.h
> @@ -70,24 +70,24 @@ struct catu_drvdata {
>  static inline u32                                                      \
>  catu_read_##name(struct catu_drvdata *drvdata)                         \
>  {                                                                      \
> -       return coresight_read_reg_pair(drvdata->base, offset, -1);      \
> +       return csdev_access_relaxed_read32(&drvdata->csdev->access, offset); \
>  }                                                                      \
>  static inline void                                                     \
>  catu_write_##name(struct catu_drvdata *drvdata, u32 val)               \
>  {                                                                      \
> -       coresight_write_reg_pair(drvdata->base, val, offset, -1);       \
> +       csdev_access_relaxed_write32(&drvdata->csdev->access, val, offset); \
>  }
>
>  #define CATU_REG_PAIR(name, lo_off, hi_off)                            \
>  static inline u64                                                      \
>  catu_read_##name(struct catu_drvdata *drvdata)                         \
>  {                                                                      \
> -       return coresight_read_reg_pair(drvdata->base, lo_off, hi_off);  \
> +       return csdev_access_relaxed_read_pair(&drvdata->csdev->access, lo_off, hi_off); \
>  }                                                                      \
>  static inline void                                                     \
>  catu_write_##name(struct catu_drvdata *drvdata, u64 val)               \
>  {                                                                      \
> -       coresight_write_reg_pair(drvdata->base, val, lo_off, hi_off);   \
> +       csdev_access_relaxed_write_pair(&drvdata->csdev->access, val, lo_off, hi_off); \
>  }
>
>  CATU_REG32(control, CATU_CONTROL);
> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
> index c63b2167a69f..d5dbc67bacb4 100644
> --- a/drivers/hwtracing/coresight/coresight-core.c
> +++ b/drivers/hwtracing/coresight/coresight-core.c
> @@ -60,7 +60,7 @@ EXPORT_SYMBOL_GPL(coresight_barrier_pkt);
>
>  static const struct cti_assoc_op *cti_assoc_ops;
>
> -ssize_t coresight_simple_show(struct device *_dev,
> +ssize_t coresight_simple_show_pair(struct device *_dev,
>                               struct device_attribute *attr, char *buf)
>  {
>         struct coresight_device *csdev = container_of(_dev, struct coresight_device, dev);
> @@ -72,7 +72,21 @@ ssize_t coresight_simple_show(struct device *_dev,
>         pm_runtime_put_sync(_dev->parent);
>         return sysfs_emit(buf, "0x%llx\n", val);
>  }
> -EXPORT_SYMBOL_GPL(coresight_simple_show);
> +EXPORT_SYMBOL_GPL(coresight_simple_show_pair);
> +
> +ssize_t coresight_simple_show32(struct device *_dev,
> +                             struct device_attribute *attr, char *buf)
> +{
> +       struct coresight_device *csdev = container_of(_dev, struct coresight_device, dev);
> +       struct cs_off_attribute *cs_attr = container_of(attr, struct cs_off_attribute, attr);
> +       u64 val;
> +
> +       pm_runtime_get_sync(_dev->parent);
> +       val = csdev_access_relaxed_read32(&csdev->access, cs_attr->off);
> +       pm_runtime_put_sync(_dev->parent);
> +       return sysfs_emit(buf, "0x%llx\n", val);
> +}
> +EXPORT_SYMBOL_GPL(coresight_simple_show32);
>
>  void coresight_set_cti_ops(const struct cti_assoc_op *cti_op)
>  {
> diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h
> index c211979deca5..595ce5862056 100644
> --- a/drivers/hwtracing/coresight/coresight-priv.h
> +++ b/drivers/hwtracing/coresight/coresight-priv.h
> @@ -41,8 +41,8 @@
>  #define ETM_MODE_EXCL_USER     BIT(31)
>  struct cs_pair_attribute {
>         struct device_attribute attr;
> -       s32 lo_off;
> -       s32 hi_off;
> +       u32 lo_off;
> +       u32 hi_off;
>  };
>
>  struct cs_off_attribute {
> @@ -50,21 +50,23 @@ struct cs_off_attribute {
>         u32 off;
>  };
>
> -extern ssize_t coresight_simple_show(struct device *_dev,
> +extern ssize_t coresight_simple_show32(struct device *_dev,
> +                                    struct device_attribute *attr, char *buf);
> +extern ssize_t coresight_simple_show_pair(struct device *_dev,
>                                      struct device_attribute *attr, char *buf);
>
>  #define coresight_simple_reg32(name, offset)                           \
> -       (&((struct cs_pair_attribute[]) {                               \
> +       (&((struct cs_off_attribute[]) {                                \
>            {                                                            \
> -               __ATTR(name, 0444, coresight_simple_show, NULL),        \
> -               offset, -1                                              \
> +               __ATTR(name, 0444, coresight_simple_show32, NULL),      \
> +               offset                                                  \
>            }                                                            \
>         })[0].attr.attr)
>
>  #define coresight_simple_reg64(name, lo_off, hi_off)                   \
>         (&((struct cs_pair_attribute[]) {                               \
>            {                                                            \
> -               __ATTR(name, 0444, coresight_simple_show, NULL),        \
> +               __ATTR(name, 0444, coresight_simple_show_pair, NULL),   \
>                 lo_off, hi_off                                          \
>            }                                                            \
>         })[0].attr.attr)
> @@ -130,25 +132,6 @@ static inline void CS_UNLOCK(void __iomem *addr)
>         } while (0);
>  }
>
> -static inline u64
> -coresight_read_reg_pair(void __iomem *addr, s32 lo_offset, s32 hi_offset)
> -{
> -       u64 val;
> -
> -       val = readl_relaxed(addr + lo_offset);
> -       val |= (hi_offset < 0) ? 0 :
> -              (u64)readl_relaxed(addr + hi_offset) << 32;
> -       return val;
> -}
> -
> -static inline void coresight_write_reg_pair(void __iomem *addr, u64 val,
> -                                                s32 lo_offset, s32 hi_offset)
> -{
> -       writel_relaxed((u32)val, addr + lo_offset);
> -       if (hi_offset >= 0)
> -               writel_relaxed((u32)(val >> 32), addr + hi_offset);
> -}
> -
>  void coresight_disable_path(struct list_head *path);
>  int coresight_enable_path(struct list_head *path, u32 mode, void *sink_data);
>  struct coresight_device *coresight_get_sink(struct list_head *path);
> diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h
> index 6bec20a392b3..66959557cf39 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc.h
> +++ b/drivers/hwtracing/coresight/coresight-tmc.h
> @@ -282,12 +282,12 @@ ssize_t tmc_etr_get_sysfs_trace(struct tmc_drvdata *drvdata,
>  static inline u64                                                      \
>  tmc_read_##name(struct tmc_drvdata *drvdata)                           \
>  {                                                                      \
> -       return coresight_read_reg_pair(drvdata->base, lo_off, hi_off);  \
> +       return csdev_access_relaxed_read_pair(&drvdata->csdev->access, lo_off, hi_off); \
>  }                                                                      \
>  static inline void                                                     \
>  tmc_write_##name(struct tmc_drvdata *drvdata, u64 val)                 \
>  {                                                                      \
> -       coresight_write_reg_pair(drvdata->base, val, lo_off, hi_off);   \
> +       csdev_access_relaxed_write_pair(&drvdata->csdev->access, val, lo_off, hi_off); \
>  }
>
>  TMC_REG_PAIR(rrp, TMC_RRP, TMC_RRPHI)
> diff --git a/include/linux/coresight.h b/include/linux/coresight.h
> index a47dd1f62216..1554021231f9 100644
> --- a/include/linux/coresight.h
> +++ b/include/linux/coresight.h
> @@ -373,21 +373,26 @@ static inline u32 csdev_access_relaxed_read32(struct csdev_access *csa,
>  }
>
>  static inline u64 csdev_access_relaxed_read_pair(struct csdev_access *csa,
> -                                                s32 lo_offset, s32 hi_offset)
> +                                                u32 lo_offset, u32 hi_offset)
>  {
> -       u64 val;
> -
>         if (likely(csa->io_mem)) {
> -               val = readl_relaxed(csa->base + lo_offset);
> -               val |= (hi_offset < 0) ? 0 :
> -                      (u64)readl_relaxed(csa->base + hi_offset) << 32;
> -               return val;
> +               return readl_relaxed(csa->base + lo_offset) |
> +                       ((u64)readl_relaxed(csa->base + hi_offset) << 32);
>         }
>
> -       val = csa->read(lo_offset, true, false);
> -       val |= (hi_offset < 0) ? 0 :
> -              (u64)csa->read(hi_offset, true, false) << 32;
> -       return val;
> +       return csa->read(lo_offset, true, false) | (csa->read(hi_offset, true, false) << 32);
> +}
> +
> +static inline void csdev_access_relaxed_write_pair(struct csdev_access *csa, u64 val,
> +                                                  u32 lo_offset, u32 hi_offset)
> +{
> +       if (likely(csa->io_mem)) {
> +               writel_relaxed((u32)val, csa->base + lo_offset);
> +               writel_relaxed((u32)(val >> 32), csa->base + hi_offset);
> +       } else {
> +               csa->write((u32)val, lo_offset, true, false);
> +               csa->write((u32)(val >> 32), hi_offset, true, false);
> +       }
>  }
>
>  static inline u32 csdev_access_read32(struct csdev_access *csa, u32 offset)
> --
> 2.28.0
>

Reviewed-by: Mike Leach <mike.leach@linaro.org>
-- 
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK

Re: [PATCH v2 0/5] coresight: Reduce duplicated sysfs accessors

[Mathieu Poirier]

I have applied this set.

Thanks,
Mathieu

On Tue, Aug 30, 2022 at 06:26:08PM +0100, James Clark wrote:
> Changes since v1:
> 
>   * Keep existing signed offset value types until the very last commit
>     and then remove them all at once
> 
>   * Also split out usages of read_pair() and read32() into separate
>     functions in the last commit 
> 
>   * Whitespace fixes
> 
>   * Replaced any touched scnprintf() with sysfs_emit()
> 
> ======
> 
> The intent of this change is to reduce the large number identical of
> functions created by macros for sysfs accessors. It's possible to re-use
> the same function but pass in the register to access as an argument.
> This reduces the size of the coresight modules folder by 244KB.
> 
> The first two patches are refactors to simplify and remove some dead
> code, and the second two are the changes to use a shared function.
> 
> Testing
> =======
> 
> No changes in any of the outputs:
> 
>   cat /sys/bus/coresight/devices/*/mgmt/* > before.txt
>   cat /sys/bus/coresight/devices/*/mgmt/* > after.txt
>   diff before.txt after.txt
> 
> With the following modules loaded:
> 
>   ls /sys/bus/coresight/devices/
>   etm0  etm2  funnel0  funnel2  replicator0  tmc_etf0  tmc_etf2  tpiu0
>   etm1  etm3  funnel1  funnel3  stm0         tmc_etf1  tmc_etr0
> 
> 
> James Clark (5):
>   coresight: Remove unused function parameter
>   coresight: Simplify sysfs accessors by using csdev_access abstraction
>   coresight: Re-use same function for similar sysfs register accessors
>   coresight: cti-sysfs: Re-use same functions for similar sysfs register
>     accessors
>   coresight: Make new csdev_access offsets unsigned
> 
>  drivers/hwtracing/coresight/coresight-catu.c  |  27 +--
>  drivers/hwtracing/coresight/coresight-catu.h  |   8 +-
>  drivers/hwtracing/coresight/coresight-core.c  |  28 +++
>  .../hwtracing/coresight/coresight-cti-sysfs.c | 213 +++++++-----------
>  drivers/hwtracing/coresight/coresight-etb10.c |  28 +--
>  .../coresight/coresight-etm3x-sysfs.c         |  34 +--
>  drivers/hwtracing/coresight/coresight-priv.h  |  74 +++---
>  .../coresight/coresight-replicator.c          |  10 +-
>  drivers/hwtracing/coresight/coresight-stm.c   |  40 +---
>  .../hwtracing/coresight/coresight-tmc-core.c  |  48 ++--
>  drivers/hwtracing/coresight/coresight-tmc.h   |   4 +-
>  include/linux/coresight.h                     |  23 ++
>  12 files changed, 227 insertions(+), 310 deletions(-)
> 
> -- 
> 2.28.0
>