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 <[email protected]>
---
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..c1bb93c7c1de 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,
+ u32 lo_offset, u32 hi_offset)
+{
+ u64 val;
+
+ if (likely(csa->io_mem)) {
+ val = readl_relaxed(csa->base + lo_offset);
+ val |= (hi_offset == -1) ? 0 :
+ (u64)readl_relaxed(csa->base + hi_offset) << 32;
+ return val;
+ }
+
+ val = csa->read(lo_offset, true, false);
+ val |= (hi_offset == -1) ? 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
Hi James,
On Mon, Jul 25, 2022 at 03:52:19PM +0100, James Clark 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 <[email protected]>
> ---
> 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..c1bb93c7c1de 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,
> + u32 lo_offset, u32 hi_offset)
Parameters lo_offset and hi_offset are s32 in coresight_read_reg_pair()...
> +{
> + u64 val;
> +
> + if (likely(csa->io_mem)) {
> + val = readl_relaxed(csa->base + lo_offset);
> + val |= (hi_offset == -1) ? 0 :
> + (u64)readl_relaxed(csa->base + hi_offset) << 32;
> + return val;
> + }
> +
> + val = csa->read(lo_offset, true, false);
> + val |= (hi_offset == -1) ? 0 :
... and hi_offset can't take on a negative value.
> + (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
>
On 22/08/2022 18:37, Mathieu Poirier wrote:
> Hi James,
>
> On Mon, Jul 25, 2022 at 03:52:19PM +0100, James Clark 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 <[email protected]>
>> ---
>> 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..c1bb93c7c1de 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,
>> + u32 lo_offset, u32 hi_offset)
>
> Parameters lo_offset and hi_offset are s32 in coresight_read_reg_pair()...
Hi Mathieu,
I probably should have mentioned this in the commit message. You're
right that the previous version used signed values, but the csdev
accessors in include/linux/coresight.h all use u32 and I had to add a
new one in there for 'csdev_access_relaxed_read_pair()' which would have
looked very out of place if it was the only one to used signed values.
Because of this I also changed the 'not set' test for hi_offset from '<
-1' to '== -1' which would also work with unsigned values. So although
it looks different, it is still working the same way as before.
I can think of some possible options to make it better:
* Have csdev_access_relaxed_read_pair() be the only csdev_access_
function to take signed values
* Keep the unsigned type but change the unset value of -1 to be
UINT_MAX
* Split the accessors into ones that are 64 bit pairs, and ones that
are a single read. It's always known when it's defined whether it's
a 'pair' or not, so technically this if statement with the 'not set'
value isn't actually needed, you just use a different accessor type
I was tempted to do the 3rd one during the refactor, but I wanted to
keep it more like the original than not. I'm not a fan of the first
option, I think that would be confusing to read the code and would look
like a mistake. So I'm more in favour of 2 or 3. What are your thoughts?
>
>> +{
>> + u64 val;
>> +
>> + if (likely(csa->io_mem)) {
>> + val = readl_relaxed(csa->base + lo_offset);
>> + val |= (hi_offset == -1) ? 0 :
>> + (u64)readl_relaxed(csa->base + hi_offset) << 32;
>> + return val;
>> + }
>> +
>> + val = csa->read(lo_offset, true, false);
>> + val |= (hi_offset == -1) ? 0 :
>
> ... and hi_offset can't take on a negative value.
This is just shorthand for UINT_MAX. I could change it to be more
explicit (option 2 above)?
>
>> + (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
>>
[...]
> >>
> >> 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..c1bb93c7c1de 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,
> >> + u32 lo_offset, u32 hi_offset)
> >
> > Parameters lo_offset and hi_offset are s32 in coresight_read_reg_pair()...
>
> Hi Mathieu,
>
> I probably should have mentioned this in the commit message. You're
> right that the previous version used signed values, but the csdev
> accessors in include/linux/coresight.h all use u32 and I had to add a
> new one in there for 'csdev_access_relaxed_read_pair()' which would have
> looked very out of place if it was the only one to used signed values.
>
> Because of this I also changed the 'not set' test for hi_offset from '<
> -1' to '== -1' which would also work with unsigned values. So although
> it looks different, it is still working the same way as before.
>
> I can think of some possible options to make it better:
>
> * Have csdev_access_relaxed_read_pair() be the only csdev_access_
> function to take signed values
That part is not a big deal for me.
>
> * Keep the unsigned type but change the unset value of -1 to be
> UINT_MAX
I find this counterintuitive and error prone. And sparse will likely yell
at you profusely.
>
> * Split the accessors into ones that are 64 bit pairs, and ones that
> are a single read. It's always known when it's defined whether it's
> a 'pair' or not, so technically this if statement with the 'not set'
> value isn't actually needed, you just use a different accessor type
>
That would work.
> I was tempted to do the 3rd one during the refactor, but I wanted to
> keep it more like the original than not. I'm not a fan of the first
> option, I think that would be confusing to read the code and would look
> like a mistake. So I'm more in favour of 2 or 3. What are your thoughts?
Let's meet in the middle and go with option 3.
Thanks,
Mathieu
>
> >
> >> +{
> >> + u64 val;
> >> +
> >> + if (likely(csa->io_mem)) {
> >> + val = readl_relaxed(csa->base + lo_offset);
> >> + val |= (hi_offset == -1) ? 0 :
> >> + (u64)readl_relaxed(csa->base + hi_offset) << 32;
> >> + return val;
> >> + }
> >> +
> >> + val = csa->read(lo_offset, true, false);
> >> + val |= (hi_offset == -1) ? 0 :
> >
> > ... and hi_offset can't take on a negative value.
>
> This is just shorthand for UINT_MAX. I could change it to be more
> explicit (option 2 above)?
>
> >
> >> + (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
> >>