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 (4):
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
drivers/hwtracing/coresight/coresight-catu.c | 27 +--
drivers/hwtracing/coresight/coresight-core.c | 14 ++
.../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 | 48 ++--
.../coresight/coresight-replicator.c | 10 +-
drivers/hwtracing/coresight/coresight-stm.c | 40 +---
.../hwtracing/coresight/coresight-tmc-core.c | 48 ++--
include/linux/coresight.h | 18 ++
10 files changed, 196 insertions(+), 284 deletions(-)
--
2.28.0
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.
Signed-off-by: James Clark <[email protected]>
---
.../hwtracing/coresight/coresight-cti-sysfs.c | 213 +++++++-----------
1 file changed, 86 insertions(+), 127 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-cti-sysfs.c b/drivers/hwtracing/coresight/coresight-cti-sysfs.c
index 7ff7e7780bbf..95148338f3b6 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_ext_attribute *cti_attr = container_of(attr, struct cs_ext_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->lo_off);
+ spin_unlock(&drvdata->spinlock);
+ pm_runtime_put_sync(dev->parent);
+ return sprintf(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_ext_attribute *cti_attr = container_of(attr, struct cs_ext_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->lo_off, val);
+ spin_unlock(&drvdata->spinlock);
+ pm_runtime_put_sync(dev->parent);
+ return size;
+}
+
+#define coresight_cti_reg(name, offset) \
+ (&((struct cs_ext_attribute[]) { \
+ { \
+ __ATTR(name, 0444, coresight_cti_reg_show, NULL), \
+ offset, -1 \
+ } \
+ })[0].attr.attr)
+
+#define coresight_cti_reg_rw(name, offset) \
+ (&((struct cs_ext_attribute[]) { \
+ { \
+ __ATTR(name, 0644, coresight_cti_reg_show, \
+ coresight_cti_reg_store), \
+ offset, -1 \
+ } \
+ })[0].attr.attr)
+
+#define coresight_cti_reg_wo(name, offset) \
+ (&((struct cs_ext_attribute[]) { \
+ { \
+ __ATTR(name, 0200, NULL, coresight_cti_reg_store), \
+ offset, -1 \
+ } \
+ })[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,
};
--
2.28.0
On Mon, Jul 25, 2022 at 03:52:17PM +0100, James Clark wrote:
> 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 (4):
> 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
>
> drivers/hwtracing/coresight/coresight-catu.c | 27 +--
> drivers/hwtracing/coresight/coresight-core.c | 14 ++
> .../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 | 48 ++--
> .../coresight/coresight-replicator.c | 10 +-
> drivers/hwtracing/coresight/coresight-stm.c | 40 +---
> .../hwtracing/coresight/coresight-tmc-core.c | 48 ++--
> include/linux/coresight.h | 18 ++
> 10 files changed, 196 insertions(+), 284 deletions(-)
I am done reviewing this set. It would be nice if you could respin this as
quickly as possible so that I could apply it to next early in the cycle.
Thanks,
Mathieu
>
> --
> 2.28.0
>