All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mathieu Poirier <mathieu.poirier@linaro.org>
To: James Clark <james.clark@arm.com>
Cc: suzuki.poulose@arm.com, coresight@lists.linaro.org,
	mike.leach@linaro.org, leo.yan@linaro.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v1 2/4] coresight: Simplify sysfs accessors by using csdev_access abstraction
Date: Mon, 22 Aug 2022 11:37:35 -0600	[thread overview]
Message-ID: <20220822173735.GC1583519@p14s> (raw)
In-Reply-To: <20220725145221.517776-3-james.clark@arm.com>

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 <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..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
> 

  reply	other threads:[~2022-08-22 17:37 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-25 14:52 [PATCH v1 0/4] coresight: Reduce duplicated sysfs accessors James Clark
2022-07-25 14:52 ` [PATCH v1 1/4] coresight: Remove unused function parameter James Clark
2022-07-25 15:07   ` German Gomez
2022-07-25 14:52 ` [PATCH v1 2/4] coresight: Simplify sysfs accessors by using csdev_access abstraction James Clark
2022-08-22 17:37   ` Mathieu Poirier [this message]
2022-08-23 12:28     ` James Clark
2022-08-25 16:54       ` Mathieu Poirier
2022-07-25 14:52 ` [PATCH v1 3/4] coresight: Re-use same function for similar sysfs register accessors James Clark
2022-08-22 17:43   ` Mathieu Poirier
2022-07-25 14:52 ` [PATCH v1 4/4] coresight: cti-sysfs: Re-use same functions " James Clark
2022-08-22 17:45 ` [PATCH v1 0/4] coresight: Reduce duplicated sysfs accessors Mathieu Poirier

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220822173735.GC1583519@p14s \
    --to=mathieu.poirier@linaro.org \
    --cc=coresight@lists.linaro.org \
    --cc=james.clark@arm.com \
    --cc=leo.yan@linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mike.leach@linaro.org \
    --cc=suzuki.poulose@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.