Hi Suzuki,

I have starte to review this - comments will be scattered over a few days.

On Wed, Jul 22, 2020 at 06:20:27PM +0100, Suzuki K Poulose wrote:

Skip cpu save/restore before the coresight device is registered.

Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Mike Leach mike.leach@linaro.org Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com

---

drivers/hwtracing/coresight/coresight-etm4x.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c b/drivers/hwtracing/coresight/coresight-etm4x.c index 6d7d2169bfb2..cb83fb77ded6 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x.c +++ b/drivers/hwtracing/coresight/coresight-etm4x.c @@ -1135,7 +1135,13 @@ static int etm4_cpu_save(struct etmv4_drvdata *drvdata) { int i, ret = 0; struct etmv4_save_state *state;

• struct device *etm_dev = &drvdata->csdev->dev;

• struct coresight_device *csdev = drvdata->csdev;
• struct device *etm_dev;
• if (WARN_ON(!csdev))

• return -ENODEV;
  

• etm_dev = &csdev->dev;

/* * As recommended by 3.4.1 ("The procedure when powering down the PE") @@ -1261,6 +1267,10 @@ static void etm4_cpu_restore(struct etmv4_drvdata *drvdata) { int i; struct etmv4_save_state *state = drvdata->save_state;

• struct coresight_device *csdev = drvdata->csdev;
• if (WARN_ON(!csdev))

• return;
  

Restore and save operations are only called from etm4_cpu_pm_notify() where the check for a valid drvdata->csdev is already done.

CS_UNLOCK(drvdata->base); @@ -1368,6 +1378,10 @@ static int etm4_cpu_pm_notify(struct notifier_block *nb, unsigned long cmd, drvdata = etmdrvdata[cpu];

• /* If we have not registered the device there is nothing to do */
• if (!drvdata->csdev)

• return NOTIFY_OK;
  

Can you describe the scenario you've seen this happening in? Probably best to add it to the changelog.

• if (!drvdata->save_state) return NOTIFY_OK;

2.24.1

On 07/29/2020 08:56 PM, Mathieu Poirier wrote:

On Wed, Jul 22, 2020 at 06:20:28PM +0100, Suzuki K Poulose wrote:

...

We are about to introduce support for sysreg access to ETMv4.4+ component. Since there are generic routines that access the registers (e.g, CS_LOCK/UNLOCK , claim/disclaim operations, timeout) and in order to preserve the logic of these operations at a single place we introduce an abstraction layer for the accesses to a given device. This will also be helpful in consolidating the sysfs.attribute helpers, that we define per driver.

Please drop the last sentence, it doesn't add to the current patch.

Sure.

...

diff --git a/drivers/hwtracing/coresight/coresight.c b/drivers/hwtracing/coresight/coresight.c index e9c90f2de34a..38e9c03ab754 100644 --- a/drivers/hwtracing/coresight/coresight.c +++ b/drivers/hwtracing/coresight/coresight.c @@ -1387,6 +1387,54 @@ static int __init coresight_init(void) }

...

...

• coresight_release_platform_data: Release references to the devices connected
• to the output port of this device.

@@ -1451,6 +1499,7 @@ struct coresight_device *coresight_register(struct coresight_desc *desc) csdev->type = desc->type; csdev->subtype = desc->subtype; csdev->ops = desc->ops;

• csdev->access = desc->access; csdev->orphan = false;

csdev->dev.type = &coresight_dev_type[desc->type]; diff --git a/include/linux/coresight.h b/include/linux/coresight.h index 58fffdecdbfd..81ac708689f8 100644 --- a/include/linux/coresight.h +++ b/include/linux/coresight.h @@ -7,6 +7,7 @@ #define _LINUX_CORESIGHT_H #include <linux/device.h> +#include <linux/io.h> #include <linux/perf_event.h> #include <linux/sched.h> @@ -114,6 +115,32 @@ struct coresight_platform_data { struct coresight_connection *conns; }; +/**

• • struct csdev_access - Abstraction of a CoreSight device access.
• • @no_iomem : True if the device doesn't have iomem access.
• • @base : When no_iomem == false, base address of the component
• • @read : Read from the given "offset" of the given instance.
• • @write : Write "val" to the given "offset".
• */

+struct csdev_access {

• bool no_iomem;

I find the no_iomen to be difficult to understand, especially when prefixed with '!'. Using "has_iomem" would be a lot more intuitive and would avoid extra mental gymnastics.

I agree. That was a bit of laziness in part, to limit the changes to the existing drivers, where almost everyone, except the ETM would need to simply use the MMIO approach. So, in order to keep those changes to minimum, i.e, simply initialize the base, I used the inverted logic. I will fix it.

...

+u32 coresight_relaxed_read32(struct coresight_device *csdev, u32 offset); +u32 coresight_read32(struct coresight_device *csdev, u32 offset); +void coresight_write32(struct coresight_device *csdev, u32 val, u32 offset); +void coresight_relaxed_write32(struct coresight_device *csdev,

• 	       u32 val,
  

• 	       u32 offset);
  

Why are the 64 bit version outside of the #ifdef and the 32 bit within?

Mistake ;-). I will address all of the comments above in my next version.

...

...

+static inline u64 coresight_read64(struct coresight_device *csdev, u32 offset) +{

• WARN_ON_ONCE(1);

Not sure about the motivation behind using WARN_ON_ONCE(), and only in the read functions. I would simply return 0 here. After all if CONFIG_CORESIGHT is not defined they won't make it very far.

If someone is reading the values, they might do something with the value, i.e, make a decision when they shouldn't. This is just to prevent such cases.

...

• return 0;

+}

+static inline void coresight_relaxed_write64(struct coresight_device *csdev,

• 			     u64 val,
  

• 			     u32 offset)
  

+{ +}

+static inline void coresight_write64(struct coresight_device *csdev, u64 val, u32 offset) +{ +}

• #endif

I will likely come back to this patch once I have reviewed the rest of the set.

Sure. I am looking for thoughts on the proposed API (not ABI) changes, as they are quite significant and invasive changes in the code, without much functionality changes.

Thank you for the review !

Suzuki

On Wed, Jul 22, 2020 at 06:20:29PM +0100, Suzuki K Poulose wrote:

TPIU driver access the device before the coresight device is registered. In other words, before the drvdata->csdev is valid. Thus, we need to make sure that the csdev_access is valid for both the invocations. Switch to using the csdev_access directly instead of relying on availability of drvdata->csdev.

I'm not sure all of the above is needed and based on the wording I could easily see this patch being selected for stable backport, which would be a mistak.

The gist of this patch is that we are moving to the access abstraction and the changelog should reflect that.

Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Mike Leach mike.leach@linaro.org Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com

---

drivers/hwtracing/coresight/coresight-tpiu.c | 30 +++++++++----------- 1 file changed, 13 insertions(+), 17 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-tpiu.c b/drivers/hwtracing/coresight/coresight-tpiu.c index 7ef7649f48ad..84ff4bf5d3b8 100644 --- a/drivers/hwtracing/coresight/coresight-tpiu.c +++ b/drivers/hwtracing/coresight/coresight-tpiu.c @@ -60,49 +60,45 @@ struct tpiu_drvdata { struct coresight_device *csdev; }; -static void tpiu_enable_hw(struct tpiu_drvdata *drvdata) +static void tpiu_enable_hw(struct csdev_access *csa) {

• CS_UNLOCK(drvdata->base);

• CS_UNLOCK(csa->base);

/* TODO: fill this up */

• CS_LOCK(drvdata->base);

• CS_LOCK(csa->base);

} static int tpiu_enable(struct coresight_device *csdev, u32 mode, void *__unused) {

• struct tpiu_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
• tpiu_enable_hw(drvdata);

• tpiu_enable_hw(&csdev->access); atomic_inc(csdev->refcnt); dev_dbg(&csdev->dev, "TPIU enabled\n"); return 0;

} -static void tpiu_disable_hw(struct tpiu_drvdata *drvdata) +static void tpiu_disable_hw(struct csdev_access *csa) {

• CS_UNLOCK(drvdata->base);

• CS_UNLOCK(csa->base);

/* Clear formatter and stop on flush */

• writel_relaxed(FFCR_STOP_FI, drvdata->base + TPIU_FFCR);

• csdev_access_relaxed_write32(csa, FFCR_STOP_FI, TPIU_FFCR); /* Generate manual flush */

• writel_relaxed(FFCR_STOP_FI | FFCR_FON_MAN, drvdata->base + TPIU_FFCR);

• csdev_access_relaxed_write32(csa, FFCR_STOP_FI | FFCR_FON_MAN, TPIU_FFCR); /* Wait for flush to complete */

• coresight_timeout(drvdata->base, TPIU_FFCR, FFCR_FON_MAN_BIT, 0);

• coresight_timeout(csa->base, TPIU_FFCR, FFCR_FON_MAN_BIT, 0); /* Wait for formatter to stop */

• coresight_timeout(drvdata->base, TPIU_FFSR, FFSR_FT_STOPPED_BIT, 1);

• coresight_timeout(csa->base, TPIU_FFSR, FFSR_FT_STOPPED_BIT, 1);

• CS_LOCK(drvdata->base);

• CS_LOCK(csa->base);

} static int tpiu_disable(struct coresight_device *csdev) {

• struct tpiu_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
• if (atomic_dec_return(csdev->refcnt)) return -EBUSY;

• tpiu_disable_hw(drvdata);

• tpiu_disable_hw(&csdev->access);

dev_dbg(&csdev->dev, "TPIU disabled\n"); return 0; @@ -152,7 +148,7 @@ static int tpiu_probe(struct amba_device *adev, const struct amba_id *id) desc.access.base = base;

Any reason for introducing the above in patch 02? I would have done that as part of this patch... Also part of this patch I would remove drvdata::base since it is no longer needed.

I'm out of time for today - I will continue tomorrow.

Regards, Mathieu

/* Disable tpiu to support older devices */

• tpiu_disable_hw(drvdata);

• tpiu_disable_hw(&desc.access);

pdata = coresight_get_platform_data(dev); if (IS_ERR(pdata)) -- 2.24.1

On 07/30/2020 06:31 PM, Mathieu Poirier wrote:

On Wed, Jul 22, 2020 at 06:20:30PM +0100, Suzuki K Poulose wrote:

...

etm4_init_arch_data is called early during the device probe, even before the coresight_device is registered. Since we are about to replace the direct access via abstraction layer, we need a way to pass in the csdev_access for the given device. Towards this free up the argument, which is already available via etmdrvdata[smp_processor_id()].

Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Mike Leach mike.leach@linaro.org Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com

---

drivers/hwtracing/coresight/coresight-etm4x.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c b/drivers/hwtracing/coresight/coresight-etm4x.c index 7bb74c659c4f..67deb4a4e618 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x.c +++ b/drivers/hwtracing/coresight/coresight-etm4x.c @@ -614,7 +614,8 @@ static const struct coresight_ops etm4_cs_ops = { .source_ops = &etm4_source_ops, }; -static void etm4_init_arch_data(void *info)

+static void etm4_init_arch_data(void *__unused) { u32 etmidr0; u32 etmidr1; @@ -622,8 +623,14 @@ static void etm4_init_arch_data(void *info) u32 etmidr3; u32 etmidr4; u32 etmidr5;

• struct etmv4_drvdata *drvdata = info;
• int i;

• struct etmv4_drvdata *drvdata;
• int i, cpu;
• cpu = raw_smp_processor_id();

Can you provide details on the motivation to use the raw_ version over the regular one? As far as I can see in linux/smp.h there is no difference between them unless DEBUB_PREEMPT is enabled. Even then the debug version won't complain since the task is CPU affined.

Right, it is partly my misunderstanding. debug_smp_processor_id() is to detect cases where smp_processor_id() is called in pre-emptible contexts. This is not the case here. So it is fine to use the smp_processor_id(). I will switch to that in the next version.

Thanks Suzuki

On Wed, Jul 22, 2020 at 06:20:31PM +0100, Suzuki K Poulose wrote:

Convert the generic routines to use the new access abstraction layer gradually, starting with coresigth_timeout.

Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Mike Leach mike.leach@linaro.org Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com

---

drivers/hwtracing/coresight/coresight-catu.c | 2 +- drivers/hwtracing/coresight/coresight-etb10.c | 5 +++-- drivers/hwtracing/coresight/coresight-etm4x.c | 12 +++++++----- drivers/hwtracing/coresight/coresight-stm.c | 2 +- drivers/hwtracing/coresight/coresight-tmc.c | 9 ++++++--- drivers/hwtracing/coresight/coresight-tpiu.c | 4 ++-- drivers/hwtracing/coresight/coresight.c | 15 +++++++++------ include/linux/coresight.h | 17 +++++++++++++---- 8 files changed, 42 insertions(+), 24 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-catu.c b/drivers/hwtracing/coresight/coresight-catu.c index 6299ff7b8a14..30f037d4549d 100644 --- a/drivers/hwtracing/coresight/coresight-catu.c +++ b/drivers/hwtracing/coresight/coresight-catu.c @@ -401,7 +401,7 @@ static const struct attribute_group *catu_groups[] = { static inline int catu_wait_for_ready(struct catu_drvdata *drvdata) {

• return coresight_timeout(drvdata->base,

• return coresight_timeout(&drvdata->csdev->access, CATU_STATUS, CATU_STATUS_READY, 1);

} diff --git a/drivers/hwtracing/coresight/coresight-etb10.c b/drivers/hwtracing/coresight/coresight-etb10.c index 0f2735e15119..507f63c51be5 100644 --- a/drivers/hwtracing/coresight/coresight-etb10.c +++ b/drivers/hwtracing/coresight/coresight-etb10.c @@ -251,6 +251,7 @@ static void __etb_disable_hw(struct etb_drvdata *drvdata) { u32 ffcr; struct device *dev = &drvdata->csdev->dev;

• struct coresight_device *csdev = drvdata->csdev;

CS_UNLOCK(drvdata->base); @@ -262,7 +263,7 @@ static void __etb_disable_hw(struct etb_drvdata *drvdata) ffcr |= ETB_FFCR_FON_MAN; writel_relaxed(ffcr, drvdata->base + ETB_FFCR);

• if (coresight_timeout(drvdata->base, ETB_FFCR, ETB_FFCR_BIT, 0)) {

• if (coresight_timeout(&csdev->access, ETB_FFCR, ETB_FFCR_BIT, 0)) { dev_err(dev, "timeout while waiting for completion of Manual Flush\n"); }

@@ -270,7 +271,7 @@ static void __etb_disable_hw(struct etb_drvdata *drvdata) /* disable trace capture */ writel_relaxed(0x0, drvdata->base + ETB_CTL_REG);

• if (coresight_timeout(drvdata->base, ETB_FFSR, ETB_FFSR_BIT, 1)) {

• if (coresight_timeout(&csdev->access, ETB_FFSR, ETB_FFSR_BIT, 1)) { dev_err(dev, "timeout while waiting for Formatter to Stop\n"); }

diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c b/drivers/hwtracing/coresight/coresight-etm4x.c index 67deb4a4e618..d78d79940fc9 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x.c +++ b/drivers/hwtracing/coresight/coresight-etm4x.c @@ -109,6 +109,7 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata) int i, rc; struct etmv4_config *config = &drvdata->config; struct device *etm_dev = &drvdata->csdev->dev;

• struct coresight_device *csdev = drvdata->csdev;

CS_UNLOCK(drvdata->base); @@ -122,7 +123,7 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata) writel_relaxed(0, drvdata->base + TRCPRGCTLR); /* wait for TRCSTATR.IDLE to go up */

• if (coresight_timeout(drvdata->base, TRCSTATR, TRCSTATR_IDLE_BIT, 1))

• if (coresight_timeout(&csdev->access, TRCSTATR, TRCSTATR_IDLE_BIT, 1)) dev_err(etm_dev, "timeout while waiting for Idle Trace Status\n");

@@ -209,7 +210,7 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata) writel_relaxed(1, drvdata->base + TRCPRGCTLR); /* wait for TRCSTATR.IDLE to go back down to '0' */

• if (coresight_timeout(drvdata->base, TRCSTATR, TRCSTATR_IDLE_BIT, 0))

• if (coresight_timeout(&csdev->access, TRCSTATR, TRCSTATR_IDLE_BIT, 0)) dev_err(etm_dev, "timeout while waiting for Idle Trace Status\n");

@@ -474,6 +475,7 @@ static void etm4_disable_hw(void *info) struct etmv4_drvdata *drvdata = info; struct etmv4_config *config = &drvdata->config; struct device *etm_dev = &drvdata->csdev->dev;

• struct coresight_device *csdev = drvdata->csdev; int i;

CS_UNLOCK(drvdata->base); @@ -500,7 +502,7 @@ static void etm4_disable_hw(void *info) writel_relaxed(control, drvdata->base + TRCPRGCTLR); /* wait for TRCSTATR.PMSTABLE to go to '1' */

• if (coresight_timeout(drvdata->base, TRCSTATR,

• if (coresight_timeout(&csdev->access, TRCSTATR, TRCSTATR_PMSTABLE_BIT, 1)) dev_err(etm_dev, "timeout while waiting for PM stable Trace Status\n");

@@ -1163,7 +1165,7 @@ static int etm4_cpu_save(struct etmv4_drvdata *drvdata) etm4_os_lock(drvdata); /* wait for TRCSTATR.PMSTABLE to go up */

• if (coresight_timeout(drvdata->base, TRCSTATR,

• if (coresight_timeout(&csdev->access, TRCSTATR, TRCSTATR_PMSTABLE_BIT, 1)) { dev_err(etm_dev, "timeout while waiting for PM Stable Status\n");

@@ -1247,7 +1249,7 @@ static int etm4_cpu_save(struct etmv4_drvdata *drvdata) state->trcpdcr = readl(drvdata->base + TRCPDCR); /* wait for TRCSTATR.IDLE to go up */

• if (coresight_timeout(drvdata->base, TRCSTATR, TRCSTATR_IDLE_BIT, 1)) {

• if (coresight_timeout(&csdev->access, TRCSTATR, TRCSTATR_IDLE_BIT, 1)) { dev_err(etm_dev, "timeout while waiting for Idle Trace Status\n"); etm4_os_unlock(drvdata);

diff --git a/drivers/hwtracing/coresight/coresight-stm.c b/drivers/hwtracing/coresight/coresight-stm.c index c8509cc78512..b704000c96b8 100644 --- a/drivers/hwtracing/coresight/coresight-stm.c +++ b/drivers/hwtracing/coresight/coresight-stm.c @@ -270,7 +270,7 @@ static void stm_disable(struct coresight_device *csdev, spin_unlock(&drvdata->spinlock); /* Wait until the engine has completely stopped */

• coresight_timeout(drvdata->base, STMTCSR, STMTCSR_BUSY_BIT, 0);
  

• coresight_timeout(&csdev->access, STMTCSR, STMTCSR_BUSY_BIT, 0);
  

pm_runtime_put(csdev->dev.parent); diff --git a/drivers/hwtracing/coresight/coresight-tmc.c b/drivers/hwtracing/coresight/coresight-tmc.c index b49795ad6861..6036eb73dce8 100644 --- a/drivers/hwtracing/coresight/coresight-tmc.c +++ b/drivers/hwtracing/coresight/coresight-tmc.c @@ -33,16 +33,19 @@ DEFINE_CORESIGHT_DEVLIST(etr_devs, "tmc_etr"); void tmc_wait_for_tmcready(struct tmc_drvdata *drvdata) {

• struct coresight_device *csdev = drvdata->csdev;
• /* Ensure formatter, unformatter and hardware fifo are empty */

• if (coresight_timeout(drvdata->base,

• if (coresight_timeout(&csdev->access, TMC_STS, TMC_STS_TMCREADY_BIT, 1)) {

• dev_err(&drvdata->csdev->dev,
  

• dev_err(&csdev->dev,
  "timeout while waiting for TMC to be Ready\n");
  

  }

} void tmc_flush_and_stop(struct tmc_drvdata *drvdata) {

• struct coresight_device *csdev = drvdata->csdev; u32 ffcr;

ffcr = readl_relaxed(drvdata->base + TMC_FFCR); @@ -51,7 +54,7 @@ void tmc_flush_and_stop(struct tmc_drvdata *drvdata) ffcr |= BIT(TMC_FFCR_FLUSHMAN_BIT); writel_relaxed(ffcr, drvdata->base + TMC_FFCR); /* Ensure flush completes */

• if (coresight_timeout(drvdata->base,

• if (coresight_timeout(&csdev->access, TMC_FFCR, TMC_FFCR_FLUSHMAN_BIT, 0)) { dev_err(&drvdata->csdev->dev, "timeout while waiting for completion of Manual Flush\n");

diff --git a/drivers/hwtracing/coresight/coresight-tpiu.c b/drivers/hwtracing/coresight/coresight-tpiu.c index 84ff4bf5d3b8..282d80e97265 100644 --- a/drivers/hwtracing/coresight/coresight-tpiu.c +++ b/drivers/hwtracing/coresight/coresight-tpiu.c @@ -86,9 +86,9 @@ static void tpiu_disable_hw(struct csdev_access *csa) /* Generate manual flush */ csdev_access_relaxed_write32(csa, FFCR_STOP_FI | FFCR_FON_MAN, TPIU_FFCR); /* Wait for flush to complete */

• coresight_timeout(csa->base, TPIU_FFCR, FFCR_FON_MAN_BIT, 0);

• coresight_timeout(csa, TPIU_FFCR, FFCR_FON_MAN_BIT, 0); /* Wait for formatter to stop */

• coresight_timeout(csa->base, TPIU_FFSR, FFSR_FT_STOPPED_BIT, 1);

• coresight_timeout(csa, TPIU_FFSR, FFSR_FT_STOPPED_BIT, 1);

CS_LOCK(csa->base); } diff --git a/drivers/hwtracing/coresight/coresight.c b/drivers/hwtracing/coresight/coresight.c index 38e9c03ab754..21e7615fcbc8 100644 --- a/drivers/hwtracing/coresight/coresight.c +++ b/drivers/hwtracing/coresight/coresight.c @@ -1338,23 +1338,26 @@ static void coresight_remove_conns(struct coresight_device *csdev) } /**

• • coresight_timeout - loop until a bit has changed to a specific state.
• • @addr: base address of the area of interest.
• • @offset: address of a register, starting from @addr.

• • coresight_timeout - loop until a bit has changed to a specific register

• • 	state.
    

• • @csa: coresight device access for the device
• • @offset: Offset of the register from the base of the device.
  • @position: the position of the bit of interest.
  • @value: the value the bit should have.
  • Return: 0 as soon as the bit has taken the desired state or -EAGAIN if
  • TIMEOUT_US has elapsed, which ever happens first.
  */

-int coresight_timeout(void __iomem *addr, u32 offset, int position, int value) +int coresight_timeout(struct csdev_access *csa,

•       u32 offset,
  

•       int position,
  

•       int value)
  

{ int i; u32 val; for (i = TIMEOUT_US; i > 0; i--) {

• val = __raw_readl(addr + offset);
  

• val = csdev_access_read32(csa, offset);
  

Here we are moving to a memory barrier access, which is probably not a bad thing.

/* waiting on the bit to go from 0 to 1 */
if (value) {
	if (val & BIT(position))

diff --git a/include/linux/coresight.h b/include/linux/coresight.h index 81ac708689f8..2989306f6041 100644 --- a/include/linux/coresight.h +++ b/include/linux/coresight.h @@ -433,8 +433,10 @@ coresight_register(struct coresight_desc *desc); extern void coresight_unregister(struct coresight_device *csdev); extern int coresight_enable(struct coresight_device *csdev); extern void coresight_disable(struct coresight_device *csdev); -extern int coresight_timeout(void __iomem *addr, u32 offset,

• 	     int position, int value);
  

+extern int coresight_timeout(struct csdev_access *csa,

• 	     u32 offset,
  

• 	     int position,
  

• 	     int value);
  

extern int coresight_claim_device(void __iomem *base); extern int coresight_claim_device_unlocked(void __iomem *base); @@ -460,8 +462,15 @@ static inline void coresight_unregister(struct coresight_device *csdev) {} static inline int coresight_enable(struct coresight_device *csdev) { return -ENOSYS; } static inline void coresight_disable(struct coresight_device *csdev) {} -static inline int coresight_timeout(void __iomem *addr, u32 offset,

• 		     int position, int value) { return 1; }
  

+static inline int coresight_timeout(struct csdev_access *csa,

• 		    u32 offset,
  

• 		    int position,
  

• 		    int value)
  

+{

• return 1;

+}

Reviewed-by: Mathieu Poirier mathieu.poirier@linaro.org

static inline int coresight_claim_device_unlocked(void __iomem *base) { return -EINVAL; -- 2.24.1

On Wed, Jul 22, 2020 at 06:20:32PM +0100, Suzuki K Poulose wrote:

Convert the CoreSight CLAIM set/clear, LOCK/UNLOCK operations to use the coresight device access abstraction.

Mostly a mechanical change.

Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Mike Leach mike.leach@linaro.org Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com

---

drivers/hwtracing/coresight/coresight-catu.c | 14 ++-- .../hwtracing/coresight/coresight-cpu-debug.c | 26 ++++++-- .../hwtracing/coresight/coresight-cti-sysfs.c | 4 +- drivers/hwtracing/coresight/coresight-cti.c | 30 +++++---- drivers/hwtracing/coresight/coresight-etb10.c | 20 +++--- .../coresight/coresight-etm3x-sysfs.c | 8 +-- drivers/hwtracing/coresight/coresight-etm3x.c | 44 ++++++++----- drivers/hwtracing/coresight/coresight-etm4x.c | 44 ++++++++----- .../hwtracing/coresight/coresight-funnel.c | 18 ++--- drivers/hwtracing/coresight/coresight-priv.h | 9 +-- .../coresight/coresight-replicator.c | 27 +++++--- drivers/hwtracing/coresight/coresight-stm.c | 46 ++++++++----- .../hwtracing/coresight/coresight-tmc-etf.c | 36 ++++++---- .../hwtracing/coresight/coresight-tmc-etr.c | 19 +++--- drivers/hwtracing/coresight/coresight-tpiu.c | 9 +-- drivers/hwtracing/coresight/coresight.c | 66 +++++++++++-------- include/linux/coresight.h | 16 ++--- 17 files changed, 266 insertions(+), 170 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-catu.c b/drivers/hwtracing/coresight/coresight-catu.c index 30f037d4549d..2918d0a4fe2e 100644 --- a/drivers/hwtracing/coresight/coresight-catu.c +++ b/drivers/hwtracing/coresight/coresight-catu.c @@ -411,6 +411,7 @@ static int catu_enable_hw(struct catu_drvdata *drvdata, void *data) u32 control, mode; struct etr_buf *etr_buf = data; struct device *dev = &drvdata->csdev->dev;

• struct coresight_device *csdev = drvdata->csdev;

if (catu_wait_for_ready(drvdata)) dev_warn(dev, "Timeout while waiting for READY\n"); @@ -421,7 +422,7 @@ static int catu_enable_hw(struct catu_drvdata *drvdata, void *data) return -EBUSY; }

• rc = coresight_claim_device_unlocked(drvdata->base);

• rc = coresight_claim_device_unlocked(csdev); if (rc) return rc;

@@ -455,9 +456,9 @@ static int catu_enable(struct coresight_device *csdev, void *data) int rc; struct catu_drvdata *catu_drvdata = csdev_to_catu_drvdata(csdev);

• CS_UNLOCK(catu_drvdata->base);

• CS_UNLOCK(&csdev->access); rc = catu_enable_hw(catu_drvdata, data);

• CS_LOCK(catu_drvdata->base);

• CS_LOCK(&csdev->access); return rc;

} @@ -465,9 +466,10 @@ static int catu_disable_hw(struct catu_drvdata *drvdata) { int rc = 0; struct device *dev = &drvdata->csdev->dev;

• struct coresight_device *csdev = drvdata->csdev;

catu_write_control(drvdata, 0);

• coresight_disclaim_device_unlocked(drvdata->base);

• coresight_disclaim_device_unlocked(csdev); if (catu_wait_for_ready(drvdata)) { dev_info(dev, "Timeout while waiting for READY\n"); rc = -EAGAIN;

@@ -482,9 +484,9 @@ static int catu_disable(struct coresight_device *csdev, void *__unused) int rc; struct catu_drvdata *catu_drvdata = csdev_to_catu_drvdata(csdev);

• CS_UNLOCK(catu_drvdata->base);

• CS_UNLOCK(&csdev->access); rc = catu_disable_hw(catu_drvdata);

• CS_LOCK(catu_drvdata->base);

• CS_LOCK(&csdev->access); return rc;

} diff --git a/drivers/hwtracing/coresight/coresight-cpu-debug.c b/drivers/hwtracing/coresight/coresight-cpu-debug.c index 96544b348c27..efbf98c1056b 100644 --- a/drivers/hwtracing/coresight/coresight-cpu-debug.c +++ b/drivers/hwtracing/coresight/coresight-cpu-debug.c @@ -108,6 +108,24 @@ static bool debug_enable; module_param_named(enable, debug_enable, bool, 0600); MODULE_PARM_DESC(enable, "Control to enable coresight CPU debug functionality"); +static inline void debug_lock(struct debug_drvdata *drvdata) +{

• struct csdev_acces dummy = {

• .base	= drvdata->base,
  

• };
• CS_LOCK(&dummy);

+}

+static void debug_unlock(struct debug_drvdata *drvdata) +{

• struct csdev_acces dummy = {

• .base	= drvdata->base,
  

• };
• CS_UNLOCK(&dummy);

+}

static void debug_os_unlock(struct debug_drvdata *drvdata) { /* Unlocks the debug registers */ @@ -191,7 +209,7 @@ static void debug_read_regs(struct debug_drvdata *drvdata) { u32 save_edprcr;

• CS_UNLOCK(drvdata->base);

• debug_unlock(drvdata);

/* Unlock os lock */ debug_os_unlock(drvdata); @@ -238,7 +256,7 @@ static void debug_read_regs(struct debug_drvdata *drvdata) /* Restore EDPRCR register */ writel_relaxed(save_edprcr, drvdata->base + EDPRCR);

• CS_LOCK(drvdata->base);

• debug_lock(drvdata);

} #ifdef CONFIG_64BIT @@ -326,13 +344,13 @@ static void debug_init_arch_data(void *info) u32 mode, pcsr_offset; u32 eddevid, eddevid1;

• CS_UNLOCK(drvdata->base);

• debug_unlock(drvdata);

/* Read device info */ eddevid = readl_relaxed(drvdata->base + EDDEVID); eddevid1 = readl_relaxed(drvdata->base + EDDEVID1);

• CS_LOCK(drvdata->base);

• debug_lock(drvdata);

/* Parse implementation feature */ mode = eddevid & EDDEVID_PCSAMPLE_MODE; diff --git a/drivers/hwtracing/coresight/coresight-cti-sysfs.c b/drivers/hwtracing/coresight/coresight-cti-sysfs.c index 392757f3a019..5897ede39222 100644 --- a/drivers/hwtracing/coresight/coresight-cti-sysfs.c +++ b/drivers/hwtracing/coresight/coresight-cti-sysfs.c @@ -223,11 +223,11 @@ static ssize_t cti_reg32_show(struct device *dev, char *buf, spin_lock(&drvdata->spinlock); if ((reg_offset >= 0) && cti_active(config)) {

• CS_UNLOCK(drvdata->base);
  

• CS_UNLOCK(&drvdata->csdev->access);
  

  val = readl_relaxed(drvdata->base + reg_offset); if (pcached_val) *pcached_val = val;

• CS_LOCK(drvdata->base);
  

• CS_LOCK(&drvdata->csdev->access);
  

  } else if (pcached_val) { val = *pcached_val; }

diff --git a/drivers/hwtracing/coresight/coresight-cti.c b/drivers/hwtracing/coresight/coresight-cti.c index c810ea3ba155..bdc75aa7d71c 100644 --- a/drivers/hwtracing/coresight/coresight-cti.c +++ b/drivers/hwtracing/coresight/coresight-cti.c @@ -60,10 +60,11 @@ DEFINE_CORESIGHT_DEVLIST(cti_sys_devs, "cti_sys"); /* write set of regs to hardware - call with spinlock claimed */ void cti_write_all_hw_regs(struct cti_drvdata *drvdata) {

• struct coresight_device *csdev = drvdata->csdev; struct cti_config *config = &drvdata->config; int i;

• CS_UNLOCK(drvdata->base);

• CS_UNLOCK(&csdev->access);

/* disable CTI before writing registers */ writel_relaxed(0, drvdata->base + CTICONTROL); @@ -83,7 +84,7 @@ void cti_write_all_hw_regs(struct cti_drvdata *drvdata) /* re-enable CTI */ writel_relaxed(1, drvdata->base + CTICONTROL);

• CS_LOCK(drvdata->base);

• CS_LOCK(&csdev->access);

} static void cti_enable_hw_smp_call(void *info) @@ -108,7 +109,7 @@ static int cti_enable_hw(struct cti_drvdata *drvdata) goto cti_state_unchanged; /* claim the device */

• rc = coresight_claim_device(drvdata->base);

• rc = coresight_claim_device(drvdata->csdev); if (rc) goto cti_err_not_enabled;

@@ -152,7 +153,7 @@ static void cti_cpuhp_enable_hw(struct cti_drvdata *drvdata) goto cti_hp_not_enabled; /* try to claim the device */

• if (coresight_claim_device(drvdata->base))

• if (coresight_claim_device(drvdata->csdev)) goto cti_hp_not_enabled;

cti_write_all_hw_regs(drvdata); @@ -171,6 +172,7 @@ static int cti_disable_hw(struct cti_drvdata *drvdata) { struct cti_config *config = &drvdata->config; struct device *dev = &drvdata->csdev->dev;

• struct coresight_device *csdev = drvdata->csdev;

spin_lock(&drvdata->spinlock); @@ -182,14 +184,14 @@ static int cti_disable_hw(struct cti_drvdata *drvdata) if (!config->hw_enabled || !config->hw_powered) goto cti_not_disabled;

• CS_UNLOCK(drvdata->base);

• CS_UNLOCK(&csdev->access);

/* disable CTI */ writel_relaxed(0, drvdata->base + CTICONTROL); config->hw_enabled = false;

• coresight_disclaim_device_unlocked(drvdata->base);
• CS_LOCK(drvdata->base);

• coresight_disclaim_device_unlocked(csdev);
• CS_LOCK(&csdev->access); spin_unlock(&drvdata->spinlock); pm_runtime_put(dev); return 0;

@@ -202,9 +204,9 @@ static int cti_disable_hw(struct cti_drvdata *drvdata) void cti_write_single_reg(struct cti_drvdata *drvdata, int offset, u32 value) {

• CS_UNLOCK(drvdata->base);

• CS_UNLOCK(&drvdata->csdev->access); writel_relaxed(value, drvdata->base + offset);

• CS_LOCK(drvdata->base);

• CS_LOCK(&drvdata->csdev->access);

} void cti_write_intack(struct device *dev, u32 ackval) @@ -662,6 +664,7 @@ static int cti_cpu_pm_notify(struct notifier_block *nb, unsigned long cmd, void *v) { struct cti_drvdata *drvdata;

• struct coresight_device *csdev; unsigned int cpu = smp_processor_id(); int notify_res = NOTIFY_OK;

@@ -669,6 +672,7 @@ static int cti_cpu_pm_notify(struct notifier_block *nb, unsigned long cmd, return NOTIFY_OK; drvdata = cti_cpu_drvdata[cpu];

• csdev = drvdata->csdev;

if (WARN_ON_ONCE(drvdata->ctidev.cpu != cpu)) return NOTIFY_BAD; @@ -680,13 +684,13 @@ static int cti_cpu_pm_notify(struct notifier_block *nb, unsigned long cmd, /* CTI regs all static - we have a copy & nothing to save */ drvdata->config.hw_powered = false; if (drvdata->config.hw_enabled)

• 	coresight_disclaim_device(drvdata->base);
  

• 	coresight_disclaim_device(csdev);
  

  break;

case CPU_PM_ENTER_FAILED: drvdata->config.hw_powered = true; if (drvdata->config.hw_enabled) {

• 	if (coresight_claim_device(drvdata->base))
  

• 	if (coresight_claim_device(csdev))
  	drvdata->config.hw_enabled = false;
  

  } break;

@@ -699,7 +703,7 @@ static int cti_cpu_pm_notify(struct notifier_block *nb, unsigned long cmd, /* check enable reference count to enable HW */ if (atomic_read(&drvdata->config.enable_req_count)) { /* check we can claim the device as we re-power */

• 	if (coresight_claim_device(drvdata->base))
  

• 	if (coresight_claim_device(csdev))
  	goto cti_notify_exit;
  

drvdata->config.hw_enabled = true; @@ -742,7 +746,7 @@ static int cti_dying_cpu(unsigned int cpu) spin_lock(&drvdata->spinlock); drvdata->config.hw_powered = false;

• coresight_disclaim_device(drvdata->base);

• coresight_disclaim_device(drvdata->csdev); spin_unlock(&drvdata->spinlock); return 0;

} diff --git a/drivers/hwtracing/coresight/coresight-etb10.c b/drivers/hwtracing/coresight/coresight-etb10.c index 507f63c51be5..e375b3ee0bb5 100644 --- a/drivers/hwtracing/coresight/coresight-etb10.c +++ b/drivers/hwtracing/coresight/coresight-etb10.c @@ -107,7 +107,7 @@ static void __etb_enable_hw(struct etb_drvdata *drvdata) int i; u32 depth;

• CS_UNLOCK(drvdata->base);

• CS_UNLOCK(&drvdata->csdev->access);

depth = drvdata->buffer_depth; /* reset write RAM pointer address */ @@ -127,12 +127,12 @@ static void __etb_enable_hw(struct etb_drvdata *drvdata) /* ETB trace capture enable */ writel_relaxed(ETB_CTL_CAPT_EN, drvdata->base + ETB_CTL_REG);

• CS_LOCK(drvdata->base);

• CS_LOCK(&drvdata->csdev->access);

} static int etb_enable_hw(struct etb_drvdata *drvdata) {

• int rc = coresight_claim_device(drvdata->base);

• int rc = coresight_claim_device(drvdata->csdev);

if (rc) return rc; @@ -253,7 +253,7 @@ static void __etb_disable_hw(struct etb_drvdata *drvdata) struct device *dev = &drvdata->csdev->dev; struct coresight_device *csdev = drvdata->csdev;

• CS_UNLOCK(drvdata->base);

• CS_UNLOCK(&csdev->access);

ffcr = readl_relaxed(drvdata->base + ETB_FFCR); /* stop formatter when a stop has completed */ @@ -276,7 +276,7 @@ static void __etb_disable_hw(struct etb_drvdata *drvdata) "timeout while waiting for Formatter to Stop\n"); }

• CS_LOCK(drvdata->base);

• CS_LOCK(&csdev->access);

} static void etb_dump_hw(struct etb_drvdata *drvdata) @@ -289,7 +289,7 @@ static void etb_dump_hw(struct etb_drvdata *drvdata) u32 frame_off, frame_endoff; struct device *dev = &drvdata->csdev->dev;

• CS_UNLOCK(drvdata->base);

• CS_UNLOCK(&drvdata->csdev->access);

read_ptr = readl_relaxed(drvdata->base + ETB_RAM_READ_POINTER); write_ptr = readl_relaxed(drvdata->base + ETB_RAM_WRITE_POINTER); @@ -337,14 +337,14 @@ static void etb_dump_hw(struct etb_drvdata *drvdata) writel_relaxed(read_ptr, drvdata->base + ETB_RAM_READ_POINTER);

• CS_LOCK(drvdata->base);

• CS_LOCK(&drvdata->csdev->access);

} static void etb_disable_hw(struct etb_drvdata *drvdata) { __etb_disable_hw(drvdata); etb_dump_hw(drvdata);

• coresight_disclaim_device(drvdata->base);

• coresight_disclaim_device(drvdata->csdev);

} static int etb_disable(struct coresight_device *csdev) @@ -448,7 +448,7 @@ static unsigned long etb_update_buffer(struct coresight_device *csdev, goto out; __etb_disable_hw(drvdata);

• CS_UNLOCK(drvdata->base);

• CS_UNLOCK(&drvdata->csdev->access);

/* unit is in words, not bytes */ read_ptr = readl_relaxed(drvdata->base + ETB_RAM_READ_POINTER); @@ -563,7 +563,7 @@ static unsigned long etb_update_buffer(struct coresight_device *csdev, handle->head += to_read; __etb_enable_hw(drvdata);

• CS_LOCK(drvdata->base);

• CS_LOCK(&drvdata->csdev->access);

out: spin_unlock_irqrestore(&drvdata->spinlock, flags); diff --git a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c index e8c7649f123e..362f5da90523 100644 --- a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c +++ b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c @@ -50,11 +50,11 @@ static ssize_t etmsr_show(struct device *dev, pm_runtime_get_sync(dev->parent); spin_lock_irqsave(&drvdata->spinlock, flags);

• CS_UNLOCK(drvdata->base);

• CS_UNLOCK(&drvdata->csdev->access);

val = etm_readl(drvdata, ETMSR);

• CS_LOCK(drvdata->base);

• CS_LOCK(&drvdata->csdev->access); spin_unlock_irqrestore(&drvdata->spinlock, flags); pm_runtime_put(dev->parent);

@@ -948,9 +948,9 @@ static ssize_t seq_curr_state_show(struct device *dev, pm_runtime_get_sync(dev->parent); spin_lock_irqsave(&drvdata->spinlock, flags);

• CS_UNLOCK(drvdata->base);

• CS_UNLOCK(&drvdata->csdev->access); val = (etm_readl(drvdata, ETMSQR) & ETM_SQR_MASK);

• CS_LOCK(drvdata->base);

• CS_LOCK(&drvdata->csdev->access);

spin_unlock_irqrestore(&drvdata->spinlock, flags); pm_runtime_put(dev->parent); diff --git a/drivers/hwtracing/coresight/coresight-etm3x.c b/drivers/hwtracing/coresight/coresight-etm3x.c index 7ddcb7fcb2d6..d6df7bc97531 100644 --- a/drivers/hwtracing/coresight/coresight-etm3x.c +++ b/drivers/hwtracing/coresight/coresight-etm3x.c @@ -360,10 +360,11 @@ static int etm_enable_hw(struct etm_drvdata *drvdata) int i, rc; u32 etmcr; struct etm_config *config = &drvdata->config;

• struct coresight_device *csdev = drvdata->csdev;

• CS_UNLOCK(drvdata->base);

• CS_UNLOCK(&csdev->access);

• rc = coresight_claim_device_unlocked(drvdata->base);

• rc = coresight_claim_device_unlocked(csdev); if (rc) goto done;

@@ -423,9 +424,9 @@ static int etm_enable_hw(struct etm_drvdata *drvdata) etm_clr_prog(drvdata); done:

• CS_LOCK(drvdata->base);

• CS_LOCK(&csdev->access);

• dev_dbg(&drvdata->csdev->dev, "cpu: %d enable smp call done: %d\n",

• dev_dbg(&csdev->dev, "cpu: %d enable smp call done: %d\n", drvdata->cpu, rc); return rc;

} @@ -468,9 +469,9 @@ int etm_get_trace_id(struct etm_drvdata *drvdata) spin_lock_irqsave(&drvdata->spinlock, flags);

• CS_UNLOCK(drvdata->base);

• CS_UNLOCK(&drvdata->csdev->access); trace_id = (etm_readl(drvdata, ETMTRACEIDR) & ETM_TRACEID_MASK);

• CS_LOCK(drvdata->base);

• CS_LOCK(&drvdata->csdev->access);

spin_unlock_irqrestore(&drvdata->spinlock, flags); pm_runtime_put(etm_dev); @@ -568,8 +569,9 @@ static void etm_disable_hw(void *info) int i; struct etm_drvdata *drvdata = info; struct etm_config *config = &drvdata->config;

• struct coresight_device *csdev = drvdata->csdev;

• CS_UNLOCK(drvdata->base);

• CS_UNLOCK(&csdev->access); etm_set_prog(drvdata);

/* Read back sequencer and counters for post trace analysis */ @@ -579,22 +581,23 @@ static void etm_disable_hw(void *info) config->cntr_val[i] = etm_readl(drvdata, ETMCNTVRn(i)); etm_set_pwrdwn(drvdata);

• coresight_disclaim_device_unlocked(drvdata->base);

• coresight_disclaim_device_unlocked(csdev);

• CS_LOCK(drvdata->base);

• CS_LOCK(&csdev->access);

• dev_dbg(&drvdata->csdev->dev,

• dev_dbg(&csdev->dev, "cpu: %d disable smp call done\n", drvdata->cpu);

} static void etm_disable_perf(struct coresight_device *csdev) { struct etm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);

• struct coresight_device *csdev;

if (WARN_ON_ONCE(drvdata->cpu != smp_processor_id())) return;

• CS_UNLOCK(drvdata->base);

• CS_UNLOCK(&csdev->access);

/* Setting the prog bit disables tracing immediately */ etm_set_prog(drvdata); @@ -604,9 +607,9 @@ static void etm_disable_perf(struct coresight_device *csdev) * power down the tracer. */ etm_set_pwrdwn(drvdata);

• coresight_disclaim_device_unlocked(drvdata->base);

• coresight_disclaim_device_unlocked(csdev);

• CS_LOCK(drvdata->base);

• CS_LOCK(&csdev->access);

} static void etm_disable_sysfs(struct coresight_device *csdev) @@ -732,16 +735,25 @@ static bool etm_arch_supported(u8 arch) return true; } +/*

• • This function is called even before the csdev is registered.
• • Thus we cant rely on drvdata->csdev to be valid.
• */

static void etm_init_arch_data(void *info) { u32 etmidr; u32 etmccr;

• struct etm_drvdata *drvdata = info;

• int cpu;
• struct etm_drvdata *drvdata;
• struct csdev_access *csa = info;
• cpu = smp_processor_id();
• drvdata = etmdrvdata[cpu];

/* Make sure all registers are accessible */ etm_os_unlock(drvdata);

• CS_UNLOCK(drvdata->base);

• CS_UNLOCK(&csa);

/* First dummy read */ (void)etm_readl(drvdata, ETMPDSR); @@ -774,7 +786,7 @@ static void etm_init_arch_data(void *info) etm_set_pwrdwn(drvdata); etm_clr_pwrup(drvdata);

• CS_LOCK(drvdata->base);

• CS_LOCK(csa);

} static void etm_init_trace_id(struct etm_drvdata *drvdata) diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c b/drivers/hwtracing/coresight/coresight-etm4x.c index d78d79940fc9..ce32d4ebc8b9 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x.c +++ b/drivers/hwtracing/coresight/coresight-etm4x.c @@ -108,14 +108,15 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata) { int i, rc; struct etmv4_config *config = &drvdata->config;

• struct device *etm_dev = &drvdata->csdev->dev; struct coresight_device *csdev = drvdata->csdev;

• struct device *etm_dev = &csdev->dev;
• struct csdev_access *csa = &csdev->access;

• CS_UNLOCK(drvdata->base);

• CS_UNLOCK(csa);

etm4_os_unlock(drvdata);

• rc = coresight_claim_device_unlocked(drvdata->base);

• rc = coresight_claim_device_unlocked(csdev); if (rc) goto done;

@@ -222,7 +223,7 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata) isb(); done:

• CS_LOCK(drvdata->base);

• CS_LOCK(csa);

dev_dbg(etm_dev, "cpu: %d enable smp call done: %d\n", drvdata->cpu, rc); @@ -478,7 +479,7 @@ static void etm4_disable_hw(void *info) struct coresight_device *csdev = drvdata->csdev; int i;

• CS_UNLOCK(drvdata->base);

• CS_UNLOCK(&csdev->access);

if (!drvdata->skip_power_up) { /* power can be removed from the trace unit now */ @@ -519,11 +520,11 @@ static void etm4_disable_hw(void *info) readl_relaxed(drvdata->base + TRCCNTVRn(i)); }

• coresight_disclaim_device_unlocked(drvdata->base);

• coresight_disclaim_device_unlocked(csdev);

• CS_LOCK(drvdata->base);

• CS_LOCK(&csdev->access);

• dev_dbg(&drvdata->csdev->dev,

• dev_dbg(etm_dev,

Why is this change needed?

"cpu: %d disable smp call done\n", drvdata->cpu);

} @@ -616,8 +617,12 @@ static const struct coresight_ops etm4_cs_ops = { .source_ops = &etm4_source_ops, };

-static void etm4_init_arch_data(void *__unused) +/*

• • This is called before coresight device is registered. Thus
• • we must know the access method. drvdata is already available
• • from per-cpu area.
• */

+static void etm4_init_arch_data(void *info) { u32 etmidr0; u32 etmidr1; @@ -625,6 +630,7 @@ static void etm4_init_arch_data(void *__unused) u32 etmidr3; u32 etmidr4; u32 etmidr5;

• struct csdev_access *csa = info; struct etmv4_drvdata *drvdata; int i, cpu;

@@ -637,7 +643,7 @@ static void etm4_init_arch_data(void *__unused) /* Make sure all registers are accessible */ etm4_os_unlock(drvdata);

• CS_UNLOCK(drvdata->base);

• CS_UNLOCK(csa);

/* find all capabilities of the tracing unit */ etmidr0 = readl_relaxed(drvdata->base + TRCIDR0); @@ -791,7 +797,7 @@ static void etm4_init_arch_data(void *__unused) drvdata->nrseqstate = BMVAL(etmidr5, 25, 27); /* NUMCNTR, bits[30:28] number of counters available for tracing */ drvdata->nr_cntr = BMVAL(etmidr5, 28, 30);

• CS_LOCK(drvdata->base);

• CS_LOCK(csa);

} static void etm4_set_default_config(struct etmv4_config *config) @@ -1146,11 +1152,13 @@ static int etm4_cpu_save(struct etmv4_drvdata *drvdata) struct etmv4_save_state *state; struct coresight_device *csdev = drvdata->csdev; struct device *etm_dev;

• struct csdev_access *csa;

if (WARN_ON(!csdev)) return -ENODEV; etm_dev = &csdev->dev;

• csa = &csdev->access;

/* * As recommended by 3.4.1 ("The procedure when powering down the PE") @@ -1159,7 +1167,7 @@ static int etm4_cpu_save(struct etmv4_drvdata *drvdata) dsb(sy); isb();

• CS_UNLOCK(drvdata->base);

• CS_UNLOCK(csa);

/* Lock the OS lock to disable trace and external debugger access */ etm4_os_lock(drvdata); @@ -1268,7 +1276,7 @@ static int etm4_cpu_save(struct etmv4_drvdata *drvdata) drvdata->base + TRCPDCR); out:

• CS_LOCK(drvdata->base);

• CS_LOCK(csa); return ret;

} @@ -1277,11 +1285,13 @@ static void etm4_cpu_restore(struct etmv4_drvdata *drvdata) int i; struct etmv4_save_state *state = drvdata->save_state; struct coresight_device *csdev = drvdata->csdev;

• struct csdev_access *csa;

if (WARN_ON(!csdev)) return;

• CS_UNLOCK(drvdata->base);

• csa = &csdev->access;
• CS_UNLOCK(csa);

writel_relaxed(state->trcclaimset, drvdata->base + TRCCLAIMSET); @@ -1373,7 +1383,7 @@ static void etm4_cpu_restore(struct etmv4_drvdata *drvdata) /* Unlock the OS lock to re-enable trace and external debug access */ etm4_os_unlock(drvdata);

• CS_LOCK(drvdata->base);

• CS_LOCK(csa);

} static int etm4_cpu_pm_notify(struct notifier_block *nb, unsigned long cmd, @@ -1526,7 +1536,7 @@ static int etm4_probe(struct amba_device *adev, const struct amba_id *id) etmdrvdata[drvdata->cpu] = drvdata; if (smp_call_function_single(drvdata->cpu,

• 		etm4_init_arch_data,  NULL, 1))
  

• 		etm4_init_arch_data,  &desc.access, 1))
  

  dev_err(dev, "ETM arch init failed\n");

ret = etm4_pm_setup_cpuslocked(); diff --git a/drivers/hwtracing/coresight/coresight-funnel.c b/drivers/hwtracing/coresight/coresight-funnel.c index 67fc3e3b77d8..d61ffbfe0a5c 100644 --- a/drivers/hwtracing/coresight/coresight-funnel.c +++ b/drivers/hwtracing/coresight/coresight-funnel.c @@ -52,13 +52,14 @@ static int dynamic_funnel_enable_hw(struct funnel_drvdata *drvdata, int port) { u32 functl; int rc = 0;

• struct coresight_device *csdev = drvdata->csdev;

• CS_UNLOCK(drvdata->base);

• CS_UNLOCK(&csdev->access);

functl = readl_relaxed(drvdata->base + FUNNEL_FUNCTL); /* Claim the device only when we enable the first slave */ if (!(functl & FUNNEL_ENSx_MASK)) {

• rc = coresight_claim_device_unlocked(drvdata->base);
  

• rc = coresight_claim_device_unlocked(csdev);
  

  if (rc) goto done; }

@@ -69,7 +70,7 @@ static int dynamic_funnel_enable_hw(struct funnel_drvdata *drvdata, int port) writel_relaxed(functl, drvdata->base + FUNNEL_FUNCTL); writel_relaxed(drvdata->priority, drvdata->base + FUNNEL_PRICTL); done:

• CS_LOCK(drvdata->base);

• CS_LOCK(&csdev->access); return rc;

} @@ -101,8 +102,9 @@ static void dynamic_funnel_disable_hw(struct funnel_drvdata *drvdata, int inport) { u32 functl;

• struct coresight_device *csdev = drvdata->csdev;

Sometimes a csdev variable is declared, sometimes not as in get_funnel_ctrl_hw() below and this makes it hard to review all these changes. Please select a heuristic and keep with it. I prefer this version but not dead set on it.

Also please split in two, on for CS_LOCK/UNLOCK() and another one for the claim tag functions.

• CS_UNLOCK(drvdata->base);

• CS_UNLOCK(&csdev->access);

functl = readl_relaxed(drvdata->base + FUNNEL_FUNCTL); functl &= ~(1 << inport); @@ -110,9 +112,9 @@ static void dynamic_funnel_disable_hw(struct funnel_drvdata *drvdata, /* Disclaim the device if none of the slaves are now active */ if (!(functl & FUNNEL_ENSx_MASK))

• coresight_disclaim_device_unlocked(drvdata->base);
  

• coresight_disclaim_device_unlocked(csdev);
  

• CS_LOCK(drvdata->base);

• CS_LOCK(&csdev->access);

} static void funnel_disable(struct coresight_device *csdev, int inport, @@ -173,9 +175,9 @@ static u32 get_funnel_ctrl_hw(struct funnel_drvdata *drvdata) { u32 functl;

• CS_UNLOCK(drvdata->base);

• CS_UNLOCK(&drvdata->csdev->access); functl = readl_relaxed(drvdata->base + FUNNEL_FUNCTL);

• CS_LOCK(drvdata->base);

• CS_LOCK(&drvdata->csdev->access);

return functl; } diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h index f2dc625ea585..a87ce11a0ea6 100644 --- a/drivers/hwtracing/coresight/coresight-priv.h +++ b/drivers/hwtracing/coresight/coresight-priv.h @@ -108,19 +108,20 @@ static inline void coresight_insert_barrier_packet(void *buf) } -static inline void CS_LOCK(void __iomem *addr) +static inline void CS_LOCK(struct csdev_access *csa) { do { /* Wait for things to settle */ mb();

• writel_relaxed(0x0, addr + CORESIGHT_LAR);
  

• csdev_access_relaxed_write32(csa, 0x0, CORESIGHT_LAR);
  

  } while (0);

} -static inline void CS_UNLOCK(void __iomem *addr) +static inline void CS_UNLOCK(struct csdev_access *csa) { do {

• writel_relaxed(CORESIGHT_UNLOCK, addr + CORESIGHT_LAR);
  

• csdev_access_relaxed_write32(csa, CORESIGHT_UNLOCK,
  

• 			  CORESIGHT_LAR);
  

  /* Make sure everyone has seen this */ mb(); } while (0);

diff --git a/drivers/hwtracing/coresight/coresight-replicator.c b/drivers/hwtracing/coresight/coresight-replicator.c index 65704ada20a5..4c04d819b1a6 100644 --- a/drivers/hwtracing/coresight/coresight-replicator.c +++ b/drivers/hwtracing/coresight/coresight-replicator.c @@ -45,15 +45,20 @@ struct replicator_drvdata { static void dynamic_replicator_reset(struct replicator_drvdata *drvdata) {

• CS_UNLOCK(drvdata->base);

• struct coresight_device *csdev = drvdata->csdev;

• if (!coresight_claim_device_unlocked(drvdata->base)) {

• if (WARN_ON(!csdev))

• return;
  

• CS_UNLOCK(&csdev->access);
• if (!coresight_claim_device_unlocked(csdev)) { writel_relaxed(0xff, drvdata->base + REPLICATOR_IDFILTER0); writel_relaxed(0xff, drvdata->base + REPLICATOR_IDFILTER1);

• coresight_disclaim_device_unlocked(drvdata->base);
  

• coresight_disclaim_device_unlocked(csdev);
  

  }

• CS_LOCK(drvdata->base);

• CS_LOCK(&csdev->access);

} /* @@ -70,8 +75,9 @@ static int dynamic_replicator_enable(struct replicator_drvdata *drvdata, { int rc = 0; u32 id0val, id1val;

• struct coresight_device *csdev = drvdata->csdev;

• CS_UNLOCK(drvdata->base);

• CS_UNLOCK(&csdev->access);

id0val = readl_relaxed(drvdata->base + REPLICATOR_IDFILTER0); id1val = readl_relaxed(drvdata->base + REPLICATOR_IDFILTER1); @@ -84,7 +90,7 @@ static int dynamic_replicator_enable(struct replicator_drvdata *drvdata, id0val = id1val = 0xff; if (id0val == 0xff && id1val == 0xff)

• rc = coresight_claim_device_unlocked(drvdata->base);
  

• rc = coresight_claim_device_unlocked(csdev);
  

if (!rc) { switch (outport) { @@ -106,7 +112,7 @@ static int dynamic_replicator_enable(struct replicator_drvdata *drvdata, writel_relaxed(id1val, drvdata->base + REPLICATOR_IDFILTER1); }

• CS_LOCK(drvdata->base);

• CS_LOCK(&csdev->access);

return rc; } @@ -140,6 +146,7 @@ static void dynamic_replicator_disable(struct replicator_drvdata *drvdata, int inport, int outport) { u32 reg;

• struct coresight_device *csdev = drvdata->csdev;

switch (outport) { case 0: @@ -153,15 +160,15 @@ static void dynamic_replicator_disable(struct replicator_drvdata *drvdata, return; }

• CS_UNLOCK(drvdata->base);

• CS_UNLOCK(&csdev->access);

/* disable the flow of ATB data through port */ writel_relaxed(0xff, drvdata->base + reg); if ((readl_relaxed(drvdata->base + REPLICATOR_IDFILTER0) == 0xff) && (readl_relaxed(drvdata->base + REPLICATOR_IDFILTER1) == 0xff))

• coresight_disclaim_device_unlocked(drvdata->base);
  

• CS_LOCK(drvdata->base);

• coresight_disclaim_device_unlocked(csdev);
  

• CS_LOCK(&csdev->access);

} static void replicator_disable(struct coresight_device *csdev, int inport, diff --git a/drivers/hwtracing/coresight/coresight-stm.c b/drivers/hwtracing/coresight/coresight-stm.c index b704000c96b8..112221dd6dd3 100644 --- a/drivers/hwtracing/coresight/coresight-stm.c +++ b/drivers/hwtracing/coresight/coresight-stm.c @@ -148,7 +148,9 @@ struct stm_drvdata { static void stm_hwevent_enable_hw(struct stm_drvdata *drvdata) {

• CS_UNLOCK(drvdata->base);

• struct coresight_device *csdev = drvdata->csdev;
• CS_UNLOCK(&csdev->access);

writel_relaxed(drvdata->stmhebsr, drvdata->base + STMHEBSR); writel_relaxed(drvdata->stmheter, drvdata->base + STMHETER); @@ -157,29 +159,33 @@ static void stm_hwevent_enable_hw(struct stm_drvdata *drvdata) 0x04, /* Error detection on event tracing */ drvdata->base + STMHEMCR);

• CS_LOCK(drvdata->base);

• CS_LOCK(&csdev->access);

} static void stm_port_enable_hw(struct stm_drvdata *drvdata) {

• CS_UNLOCK(drvdata->base);

• struct coresight_device *csdev = drvdata->csdev;
• CS_UNLOCK(&csdev->access); /* ATB trigger enable on direct writes to TRIG locations */ writel_relaxed(0x10, drvdata->base + STMSPTRIGCSR); writel_relaxed(drvdata->stmspscr, drvdata->base + STMSPSCR); writel_relaxed(drvdata->stmsper, drvdata->base + STMSPER);

• CS_LOCK(drvdata->base);

• CS_LOCK(&csdev->access);

} static void stm_enable_hw(struct stm_drvdata *drvdata) {

• struct coresight_device *csdev = drvdata->csdev;
• if (drvdata->stmheer) stm_hwevent_enable_hw(drvdata);

stm_port_enable_hw(drvdata);

• CS_UNLOCK(drvdata->base);

• CS_UNLOCK(&csdev->access);

/* 4096 byte between synchronisation packets */ writel_relaxed(0xFFF, drvdata->base + STMSYNCR); @@ -188,7 +194,7 @@ static void stm_enable_hw(struct stm_drvdata *drvdata) 0x01), /* global STM enable */ drvdata->base + STMTCSR);

• CS_LOCK(drvdata->base);

• CS_LOCK(&csdev->access);

} static int stm_enable(struct coresight_device *csdev, @@ -218,36 +224,41 @@ static int stm_enable(struct coresight_device *csdev, static void stm_hwevent_disable_hw(struct stm_drvdata *drvdata) {

• CS_UNLOCK(drvdata->base);

• struct coresight_device *csdev = drvdata->csdev;
• CS_UNLOCK(&csdev->access);

writel_relaxed(0x0, drvdata->base + STMHEMCR); writel_relaxed(0x0, drvdata->base + STMHEER); writel_relaxed(0x0, drvdata->base + STMHETER);

• CS_LOCK(drvdata->base);

• CS_LOCK(&csdev->access);

} static void stm_port_disable_hw(struct stm_drvdata *drvdata) {

• CS_UNLOCK(drvdata->base);

• struct coresight_device *csdev = drvdata->csdev;
• CS_UNLOCK(&csdev->access);

writel_relaxed(0x0, drvdata->base + STMSPER); writel_relaxed(0x0, drvdata->base + STMSPTRIGCSR);

• CS_LOCK(drvdata->base);

• CS_LOCK(&csdev->access);

} static void stm_disable_hw(struct stm_drvdata *drvdata) { u32 val;

• struct coresight_device *csdev = drvdata->csdev;

• CS_UNLOCK(drvdata->base);

• CS_UNLOCK(&csdev->access);

val = readl_relaxed(drvdata->base + STMTCSR); val &= ~0x1; /* clear global STM enable [0] */ writel_relaxed(val, drvdata->base + STMTCSR);

• CS_LOCK(drvdata->base);

• CS_LOCK(&csdev->access);

stm_port_disable_hw(drvdata); if (drvdata->stmheer) @@ -534,6 +545,8 @@ static ssize_t port_select_store(struct device *dev, const char *buf, size_t size) { struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent);

• struct coresight_device *csdev = drvdata->csdev;
• unsigned long val, stmsper; int ret = 0;

@@ -545,13 +558,13 @@ static ssize_t port_select_store(struct device *dev, drvdata->stmspscr = val; if (local_read(&drvdata->mode)) {

• CS_UNLOCK(drvdata->base);
  

• CS_UNLOCK(&csdev->access);
  

  /* Process as per ARM's TRM recommendation */ stmsper = readl_relaxed(drvdata->base + STMSPER); writel_relaxed(0x0, drvdata->base + STMSPER); writel_relaxed(drvdata->stmspscr, drvdata->base + STMSPSCR); writel_relaxed(stmsper, drvdata->base + STMSPER);

• CS_LOCK(drvdata->base);
  

• CS_LOCK(&csdev->access);
  

  } spin_unlock(&drvdata->spinlock);

@@ -581,6 +594,7 @@ static ssize_t port_enable_store(struct device *dev, const char *buf, size_t size) { struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent);

• struct coresight_device *csdev = drvdata->csdev; unsigned long val; int ret = 0;

@@ -592,9 +606,9 @@ static ssize_t port_enable_store(struct device *dev, drvdata->stmsper = val; if (local_read(&drvdata->mode)) {

• CS_UNLOCK(drvdata->base);
  

• CS_UNLOCK(&csdev->access);
  

  writel_relaxed(drvdata->stmsper, drvdata->base + STMSPER);

• CS_LOCK(drvdata->base);
  

• CS_LOCK(&csdev->access);
  

  } spin_unlock(&drvdata->spinlock);

diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/drivers/hwtracing/coresight/coresight-tmc-etf.c index 6375504ba8b0..00c82a6f5f4a 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-etf.c +++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c @@ -18,7 +18,9 @@ static int tmc_set_etf_buffer(struct coresight_device *csdev, static void __tmc_etb_enable_hw(struct tmc_drvdata *drvdata) {

• CS_UNLOCK(drvdata->base);

• struct coresight_device *csdev = drvdata->csdev;
• CS_UNLOCK(&csdev->access);

/* Wait for TMCSReady bit to be set */ tmc_wait_for_tmcready(drvdata); @@ -32,12 +34,12 @@ static void __tmc_etb_enable_hw(struct tmc_drvdata *drvdata) writel_relaxed(drvdata->trigger_cntr, drvdata->base + TMC_TRG); tmc_enable_hw(drvdata);

• CS_LOCK(drvdata->base);

• CS_LOCK(&csdev->access);

} static int tmc_etb_enable_hw(struct tmc_drvdata *drvdata) {

• int rc = coresight_claim_device(drvdata->base);

• int rc = coresight_claim_device(drvdata->csdev);

if (rc) return rc; @@ -71,7 +73,9 @@ static void tmc_etb_dump_hw(struct tmc_drvdata *drvdata) static void __tmc_etb_disable_hw(struct tmc_drvdata *drvdata) {

• CS_UNLOCK(drvdata->base);

• struct coresight_device *csdev = drvdata->csdev;
• CS_UNLOCK(&csdev->access);

tmc_flush_and_stop(drvdata); /* @@ -82,18 +86,20 @@ static void __tmc_etb_disable_hw(struct tmc_drvdata *drvdata) tmc_etb_dump_hw(drvdata); tmc_disable_hw(drvdata);

• CS_LOCK(drvdata->base);

• CS_LOCK(&csdev->access);

} static void tmc_etb_disable_hw(struct tmc_drvdata *drvdata) { __tmc_etb_disable_hw(drvdata);

• coresight_disclaim_device(drvdata->base);

• coresight_disclaim_device(drvdata->csdev);

} static void __tmc_etf_enable_hw(struct tmc_drvdata *drvdata) {

• CS_UNLOCK(drvdata->base);

• struct coresight_device *csdev = drvdata->csdev;
• CS_UNLOCK(&csdev->access);

/* Wait for TMCSReady bit to be set */ tmc_wait_for_tmcready(drvdata); @@ -104,12 +110,12 @@ static void __tmc_etf_enable_hw(struct tmc_drvdata *drvdata) writel_relaxed(0x0, drvdata->base + TMC_BUFWM); tmc_enable_hw(drvdata);

• CS_LOCK(drvdata->base);

• CS_LOCK(&csdev->access);

} static int tmc_etf_enable_hw(struct tmc_drvdata *drvdata) {

• int rc = coresight_claim_device(drvdata->base);

• int rc = coresight_claim_device(drvdata->csdev);

if (rc) return rc; @@ -120,12 +126,14 @@ static int tmc_etf_enable_hw(struct tmc_drvdata *drvdata) static void tmc_etf_disable_hw(struct tmc_drvdata *drvdata) {

• CS_UNLOCK(drvdata->base);

• struct coresight_device *csdev = drvdata->csdev;
• CS_UNLOCK(&csdev->access);

tmc_flush_and_stop(drvdata); tmc_disable_hw(drvdata);

• coresight_disclaim_device_unlocked(drvdata->base);
• CS_LOCK(drvdata->base);

• coresight_disclaim_device_unlocked(csdev);
• CS_LOCK(&csdev->access);

} /* @@ -464,7 +472,7 @@ static unsigned long tmc_update_etf_buffer(struct coresight_device *csdev, if (atomic_read(csdev->refcnt) != 1) goto out;

• CS_UNLOCK(drvdata->base);

• CS_UNLOCK(&csdev->access);

tmc_flush_and_stop(drvdata); @@ -549,7 +557,7 @@ static unsigned long tmc_update_etf_buffer(struct coresight_device *csdev, if (buf->snapshot) handle->head += to_read;

• CS_LOCK(drvdata->base);

• CS_LOCK(&csdev->access);

out: spin_unlock_irqrestore(&drvdata->spinlock, flags); diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c index b29c2db94d96..a9ce3be44e2c 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c @@ -952,8 +952,9 @@ static void __tmc_etr_enable_hw(struct tmc_drvdata *drvdata) { u32 axictl, sts; struct etr_buf *etr_buf = drvdata->etr_buf;

• struct coresight_device *csdev = drvdata->csdev;

• CS_UNLOCK(drvdata->base);

• CS_UNLOCK(&csdev->access);

/* Wait for TMCSReady bit to be set */ tmc_wait_for_tmcready(drvdata); @@ -995,7 +996,7 @@ static void __tmc_etr_enable_hw(struct tmc_drvdata *drvdata) writel_relaxed(drvdata->trigger_cntr, drvdata->base + TMC_TRG); tmc_enable_hw(drvdata);

• CS_LOCK(drvdata->base);

• CS_LOCK(&csdev->access);

} static int tmc_etr_enable_hw(struct tmc_drvdata *drvdata, @@ -1021,7 +1022,7 @@ static int tmc_etr_enable_hw(struct tmc_drvdata *drvdata, rc = tmc_etr_enable_catu(drvdata, etr_buf); if (rc) return rc;

• rc = coresight_claim_device(drvdata->base);

• rc = coresight_claim_device(drvdata->csdev); if (!rc) { drvdata->etr_buf = etr_buf; __tmc_etr_enable_hw(drvdata);

@@ -1094,7 +1095,9 @@ static void tmc_etr_sync_sysfs_buf(struct tmc_drvdata *drvdata) static void __tmc_etr_disable_hw(struct tmc_drvdata *drvdata) {

• CS_UNLOCK(drvdata->base);

• struct coresight_device *csdev = drvdata->csdev;
• CS_UNLOCK(&csdev->access);

tmc_flush_and_stop(drvdata); /* @@ -1106,7 +1109,7 @@ static void __tmc_etr_disable_hw(struct tmc_drvdata *drvdata) tmc_disable_hw(drvdata);

• CS_LOCK(drvdata->base);

• CS_LOCK(&csdev->access);

} @@ -1115,7 +1118,7 @@ void tmc_etr_disable_hw(struct tmc_drvdata *drvdata) __tmc_etr_disable_hw(drvdata); /* Disable CATU device if this ETR is connected to one */ tmc_etr_disable_catu(drvdata);

• coresight_disclaim_device(drvdata->base);

• coresight_disclaim_device(drvdata->csdev); /* Reset the ETR buf used by hardware */ drvdata->etr_buf = NULL;

} @@ -1497,12 +1500,12 @@ tmc_update_etr_buffer(struct coresight_device *csdev, goto out; }

• CS_UNLOCK(drvdata->base);

• CS_UNLOCK(&csdev->access);

tmc_flush_and_stop(drvdata); tmc_sync_etr_buf(drvdata);

• CS_LOCK(drvdata->base);

• CS_LOCK(&csdev->access); spin_unlock_irqrestore(&drvdata->spinlock, flags);

lost = etr_buf->full; diff --git a/drivers/hwtracing/coresight/coresight-tpiu.c b/drivers/hwtracing/coresight/coresight-tpiu.c index 282d80e97265..8586c015d37a 100644 --- a/drivers/hwtracing/coresight/coresight-tpiu.c +++ b/drivers/hwtracing/coresight/coresight-tpiu.c @@ -62,11 +62,11 @@ struct tpiu_drvdata { static void tpiu_enable_hw(struct csdev_access *csa) {

• CS_UNLOCK(csa->base);

• CS_UNLOCK(csa);

/* TODO: fill this up */

• CS_LOCK(csa->base);

• CS_LOCK(csa);

} static int tpiu_enable(struct coresight_device *csdev, u32 mode, void *__unused) @@ -79,7 +79,8 @@ static int tpiu_enable(struct coresight_device *csdev, u32 mode, void *__unused) static void tpiu_disable_hw(struct csdev_access *csa) {

• CS_UNLOCK(csa->base);

• CS_UNLOCK(csa);

/* Clear formatter and stop on flush */ csdev_access_relaxed_write32(csa, FFCR_STOP_FI, TPIU_FFCR); @@ -90,7 +91,7 @@ static void tpiu_disable_hw(struct csdev_access *csa) /* Wait for formatter to stop */ coresight_timeout(csa, TPIU_FFSR, FFSR_FT_STOPPED_BIT, 1);

• CS_LOCK(csa->base);

• CS_LOCK(csa);

} static int tpiu_disable(struct coresight_device *csdev) diff --git a/drivers/hwtracing/coresight/coresight.c b/drivers/hwtracing/coresight/coresight.c index 21e7615fcbc8..c1b4355d2211 100644 --- a/drivers/hwtracing/coresight/coresight.c +++ b/drivers/hwtracing/coresight/coresight.c @@ -130,30 +130,32 @@ static int coresight_find_link_outport(struct coresight_device *csdev, return -ENODEV; } -static inline u32 coresight_read_claim_tags(void __iomem *base) +static inline u32 coresight_read_claim_tags(struct coresight_device *csdev) {

• return readl_relaxed(base + CORESIGHT_CLAIMCLR);

• return coresight_relaxed_read32(csdev, CORESIGHT_CLAIMCLR);

} -static inline bool coresight_is_claimed_self_hosted(void __iomem *base) +static inline bool coresight_is_claimed_self_hosted(struct coresight_device *csdev) {

• return coresight_read_claim_tags(base) == CORESIGHT_CLAIM_SELF_HOSTED;

• return coresight_read_claim_tags(csdev) == CORESIGHT_CLAIM_SELF_HOSTED;

} -static inline bool coresight_is_claimed_any(void __iomem *base) +static inline bool coresight_is_claimed_any(struct coresight_device *csdev) {

• return coresight_read_claim_tags(base) != 0;

• return coresight_read_claim_tags(csdev) != 0;

} -static inline void coresight_set_claim_tags(void __iomem *base) +static inline void coresight_set_claim_tags(struct coresight_device *csdev) {

• writel_relaxed(CORESIGHT_CLAIM_SELF_HOSTED, base + CORESIGHT_CLAIMSET);

• coresight_relaxed_write32(csdev, CORESIGHT_CLAIM_SELF_HOSTED,

• 		  CORESIGHT_CLAIMSET);
  

  isb();

} -static inline void coresight_clear_claim_tags(void __iomem *base) +static inline void coresight_clear_claim_tags(struct coresight_device *csdev) {

• writel_relaxed(CORESIGHT_CLAIM_SELF_HOSTED, base + CORESIGHT_CLAIMCLR);

• coresight_relaxed_write32(csdev, CORESIGHT_CLAIM_SELF_HOSTED,

• 		  CORESIGHT_CLAIMCLR);
  

  isb();

} @@ -167,26 +169,32 @@ static inline void coresight_clear_claim_tags(void __iomem *base)

• Called with CS_UNLOCKed for the component.
• Returns : 0 on success

*/ -int coresight_claim_device_unlocked(void __iomem *base) +int coresight_claim_device_unlocked(struct coresight_device *csdev) {

• if (coresight_is_claimed_any(base))

• if (WARN_ON(!csdev))

• return -EINVAL;
  

• if (coresight_is_claimed_any(csdev)) return -EBUSY;

• coresight_set_claim_tags(base);
• if (coresight_is_claimed_self_hosted(base))

• coresight_set_claim_tags(csdev);
• if (coresight_is_claimed_self_hosted(csdev)) return 0; /* There was a race setting the tags, clean up and fail */

• coresight_clear_claim_tags(base);

• coresight_clear_claim_tags(csdev); return -EBUSY;

} -int coresight_claim_device(void __iomem *base) +int coresight_claim_device(struct coresight_device *csdev) { int rc;

• CS_UNLOCK(base);
• rc = coresight_claim_device_unlocked(base);
• CS_LOCK(base);

• if (WARN_ON(!csdev))

• return -EINVAL;
  

• CS_UNLOCK(&csdev->access);
• rc = coresight_claim_device_unlocked(csdev);
• CS_LOCK(&csdev->access);

return rc; } @@ -195,11 +203,14 @@ int coresight_claim_device(void __iomem *base)

• coresight_disclaim_device_unlocked : Clear the claim tags for the device.
• Called with CS_UNLOCKed for the component.

*/ -void coresight_disclaim_device_unlocked(void __iomem *base) +void coresight_disclaim_device_unlocked(struct coresight_device *csdev) {

• if (coresight_is_claimed_self_hosted(base))

• coresight_clear_claim_tags(base);
  

• if (WARN_ON(!csdev))

• return;
  

• if (coresight_is_claimed_self_hosted(csdev))

• coresight_clear_claim_tags(csdev);
  

  else /*
  • The external agent may have not honoured our claim

@@ -209,11 +220,14 @@ void coresight_disclaim_device_unlocked(void __iomem *base) WARN_ON_ONCE(1); } -void coresight_disclaim_device(void __iomem *base) +void coresight_disclaim_device(struct coresight_device *csdev) {

• CS_UNLOCK(base);
• coresight_disclaim_device_unlocked(base);
• CS_LOCK(base);

• if (WARN_ON(!csdev))

• return;
  

• CS_UNLOCK(&csdev->access);
• coresight_disclaim_device_unlocked(csdev);
• CS_LOCK(&csdev->access);

} /* enable or disable an associated CTI device of the supplied CS device */ diff --git a/include/linux/coresight.h b/include/linux/coresight.h index 2989306f6041..c0cccdc8f9be 100644 --- a/include/linux/coresight.h +++ b/include/linux/coresight.h @@ -438,11 +438,11 @@ extern int coresight_timeout(struct csdev_access *csa, int position, int value); -extern int coresight_claim_device(void __iomem *base); -extern int coresight_claim_device_unlocked(void __iomem *base); +extern int coresight_claim_device(struct coresight_device *csdev); +extern int coresight_claim_device_unlocked(struct coresight_device *csdev); -extern void coresight_disclaim_device(void __iomem *base); -extern void coresight_disclaim_device_unlocked(void __iomem *base); +extern void coresight_disclaim_device(struct coresight_device *csdev); +extern void coresight_disclaim_device_unlocked(struct coresight_device *csdev); extern char *coresight_alloc_device_name(struct coresight_dev_list *devs, struct device *dev); @@ -471,18 +471,18 @@ static inline int coresight_timeout(struct csdev_access *csa, return 1; } -static inline int coresight_claim_device_unlocked(void __iomem *base) +static inline int coresight_claim_device_unlocked(struct coresight_device *csdev) { return -EINVAL; } -static inline int coresight_claim_device(void __iomem *base) +static inline int coresight_claim_device(struct coresight_device *csdev) { return -EINVAL; } -static inline void coresight_disclaim_device(void __iomem *base) {} -static inline void coresight_disclaim_device_unlocked(void __iomem *base) {} +static inline void coresight_disclaim_device(struct coresight_device *csdev) {} +static inline void coresight_disclaim_device_unlocked(struct coresight_device *csdev) {} static inline bool coresight_loses_context_with_cpu(struct device *dev) { -- 2.24.1

On 07/30/2020 09:20 PM, Mathieu Poirier wrote:

On Wed, Jul 22, 2020 at 06:20:34PM +0100, Suzuki K Poulose wrote:

...

Convert all register accesses from etm4x driver to use a wrapper to allow switching the access at runtime with little overhead.

co-developed by sed tool ;-), mostly equivalent to :

s/readl(_relaxed)?(drvdata->base + (.*))/etm4x_\1_read32(csdev, \2) s/writel(_relaxed)?((.*), drvdata->base + (.*))/etm4x_\1_write32(csdev, \2, \3)

We don't want to replace them with the csdev_access_* to avoid a function call for every register access for system register access.

Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Mike Leach mike.leach@linaro.org Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com

---

.../coresight/coresight-etm4x-sysfs.c | 9 +- drivers/hwtracing/coresight/coresight-etm4x.c | 334 +++++++++--------- drivers/hwtracing/coresight/coresight-etm4x.h | 24 ++ 3 files changed, 189 insertions(+), 178 deletions(-)

Agreed to all the comments.

...

diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h index b8283e1d6d88..2b51d03ab6d7 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x.h +++ b/drivers/hwtracing/coresight/coresight-etm4x.h @@ -120,6 +120,30 @@ #define TRCCIDR2 0xFF8 #define TRCCIDR3 0xFFC +#define etm4x_relaxed_read32(csa, offset) \

• readl_relaxed((csa)->base + (offset))

+#define etm4x_read32(csa, offset) \

• readl((csa)->base + (offset))

+#define etm4x_relaxed_write32(csa, val, offset) \

• writel_relaxed((val), (csa)->base + (offset))

+#define etm4x_write32(csa, val, offset) \

• writel((val), (csa)->base + (offset))

+#define etm4x_relaxed_read64(csa, offset) \

• readq_relaxed((csa)->base + (offset))

+#define etm4x_read64(csa, offset) \

• readq((csa)->base + (offset))

+#define etm4x_relaxed_write64(csa, val, offset) \

• writeq_relaxed((val), (csa)->base + (offset))

+#define etm4x_write64(csa, val, offset) \

• writeq((val), (csa)->base + (offset))

Since I haven't gone through the rest of the patches I'll assume you want to enhance the above to pick an access type at some point in the future.

Yes, they are plumbed in with the introduction of system instruction support. We don't want to the overhead of a function call for each register access. Also, we would like to avoid jumping through the large switch..cases for a compile time constant offset. Hence this macro.

Cheers Suzuki

On Wed, Jul 22, 2020 at 06:20:35PM +0100, Suzuki K Poulose wrote:

ETMv4.4 architecture defines the system instructions for accessing ETM via register accesses. Add basic support for accessing a given register via system instructions.

Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Mike Leach mike.leach@linaro.org Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com

---

drivers/hwtracing/coresight/coresight-etm4x.c | 39 ++ drivers/hwtracing/coresight/coresight-etm4x.h | 379 ++++++++++++++++-- 2 files changed, 394 insertions(+), 24 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c b/drivers/hwtracing/coresight/coresight-etm4x.c index b901c91d3ee4..e37aababa4c9 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x.c +++ b/drivers/hwtracing/coresight/coresight-etm4x.c @@ -57,6 +57,45 @@ static int etm4_set_event_filters(struct etmv4_drvdata *drvdata, static enum cpuhp_state hp_online; +u64 etm4x_sysreg_read(struct csdev_access *csa,

•       u32 offset,
  

•       bool _relaxed,
  

•       bool _64bit)
  

+{

• u64 res = 0;
• if (!_relaxed)

• mb();	/* Imitate the !relaxed I/O helpers */
  

• switch (offset) {
• ETM4x_READ_CASES(res)
• default :

• WARN_ONCE(1, "etm4x: trying to read unsupported register @%x\n",
  

• 	 offset);
  

• }
• return res;

+}

+void etm4x_sysreg_write(struct csdev_access *csa,

• 	u64 val,
  

• 	u32 offset,
  

• 	bool _relaxed,
  

• 	bool _64bit)
  

+{

• if (!_relaxed)

• mb();	/* Imitate the !relaxed I/O helpers */
  

• if (!_64bit)

• val &= GENMASK(31, 0);
  

• switch (offset) {
• ETM4x_WRITE_CASES(val)
• default :

• WARN_ONCE(1, "etm4x: trying to write to unsupported register @%x\n",
  

• 	offset);
  

• }

+}

static void etm4_os_unlock_csa(struct etmv4_drvdata *drvdata, struct csdev_access *csa) { /* Writing 0 to TRCOSLAR unlocks the trace registers */ diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h index 2b51d03ab6d7..f5d708206339 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x.h +++ b/drivers/hwtracing/coresight/coresight-etm4x.h @@ -44,13 +44,13 @@ #define TRCVDSACCTLR 0x0A4 #define TRCVDARCCTLR 0x0A8 /* Derived resources registers */ -#define TRCSEQEVRn(n) (0x100 + (n * 4)) +#define TRCSEQEVRn(n) (0x100 + (n * 4)) /* n = 0-2 */ #define TRCSEQRSTEVR 0x118 #define TRCSEQSTR 0x11C #define TRCEXTINSELR 0x120 -#define TRCCNTRLDVRn(n) (0x140 + (n * 4)) -#define TRCCNTCTLRn(n) (0x150 + (n * 4)) -#define TRCCNTVRn(n) (0x160 + (n * 4)) +#define TRCCNTRLDVRn(n) (0x140 + (n * 4)) /* n = 0-3 */ +#define TRCCNTCTLRn(n) (0x150 + (n * 4)) /* n = 0-3 */ +#define TRCCNTVRn(n) (0x160 + (n * 4)) /* n = 0-3 */ /* ID registers */ #define TRCIDR8 0x180 #define TRCIDR9 0x184 @@ -59,7 +59,7 @@ #define TRCIDR12 0x190 #define TRCIDR13 0x194 #define TRCIMSPEC0 0x1C0 -#define TRCIMSPECn(n) (0x1C0 + (n * 4)) +#define TRCIMSPECn(n) (0x1C0 + (n * 4)) /* n = 1-7 */ #define TRCIDR0 0x1E0 #define TRCIDR1 0x1E4 #define TRCIDR2 0x1E8 @@ -68,9 +68,12 @@ #define TRCIDR5 0x1F4 #define TRCIDR6 0x1F8 #define TRCIDR7 0x1FC -/* Resource selection registers */ +/*

• • Resource selection registers, n = 2-31.
• • First pair (regs 0, 1) is always present and is reserved.
• */

#define TRCRSCTLRn(n) (0x200 + (n * 4)) -/* Single-shot comparator registers */ +/* Single-shot comparator registers, n = 0-7 */ #define TRCSSCCRn(n) (0x280 + (n * 4)) #define TRCSSCSRn(n) (0x2A0 + (n * 4)) #define TRCSSPCICRn(n) (0x2C0 + (n * 4)) @@ -80,11 +83,13 @@ #define TRCPDCR 0x310 #define TRCPDSR 0x314 /* Trace registers (0x318-0xEFC) */ -/* Comparator registers */ +/* Address Comparator registers n = 0-15 */ #define TRCACVRn(n) (0x400 + (n * 8)) #define TRCACATRn(n) (0x480 + (n * 8)) +/* Data Value Comparator Value registers, n = 0-7 */ #define TRCDVCVRn(n) (0x500 + (n * 16)) #define TRCDVCMRn(n) (0x580 + (n * 16)) +/* ContextID/Virtual ContextID comparators, n = 0-7 */

Extra documentation is good but it has to be in a separate patch.

#define TRCCIDCVRn(n) (0x600 + (n * 8)) #define TRCVMIDCVRn(n) (0x640 + (n * 8)) #define TRCCIDCCTLR0 0x680 @@ -120,29 +125,345 @@ #define TRCCIDR2 0xFF8 #define TRCCIDR3 0xFFC -#define etm4x_relaxed_read32(csa, offset) \

• readl_relaxed((csa)->base + (offset))

+/*

• • System instructions to access ETM registers.
• • See ETMv4.4 spec ARM IHI0064F section 4.3.6 System instructions
• */

+#define ETM4x_OFFSET_TO_REG(x) ((x) >> 2)

+#if defined(CONFIG_ARM64)

+#define ETM4x_CRn(n) (((n) >> 7) & 0x7) +#define ETM4x_Op2(n) (((n) >> 4) & 0x7) +#define ETM4x_CRm(n) ((n) & 0xf)

+#include <asm/sysreg.h> +#define ETM4x_REG_NUM_TO_SYSREG(n) \

• sys_reg(2, 1, ETM4x_CRn(n), ETM4x_CRm(n), ETM4x_Op2(n))

+#define READ_ETM4x_REG(reg) \

• read_sysreg_s(ETM4x_REG_NUM_TO_SYSREG((reg)))

+#define WRITE_ETM4x_REG(val, reg) \

• write_sysreg_s(val, ETM4x_REG_NUM_TO_SYSREG((reg)))

-#define etm4x_read32(csa, offset) \

• readl((csa)->base + (offset))

+#define read_etm4x_sysreg_const_offset(offset) \

• READ_ETM4x_REG(ETM4x_OFFSET_TO_REG(offset))

-#define etm4x_relaxed_write32(csa, val, offset) \

• writel_relaxed((val), (csa)->base + (offset))

+#define write_etm4x_sysreg_const_offset(val, offset) \

• WRITE_ETM4x_REG(val, ETM4x_OFFSET_TO_REG(offset))

-#define etm4x_write32(csa, val, offset) \

• writel((val), (csa)->base + (offset))

+#elif defined(CONFIG_ARM) -#define etm4x_relaxed_read64(csa, offset) \

• readq_relaxed((csa)->base + (offset))

+#include <asm/hardware/cp14.h> -#define etm4x_read64(csa, offset) \

• readq((csa)->base + (offset))

+#define etm4x_cp14_CRn(n) __stringify(ETM4x_CRn(n)) +#define etm4x_cp14_CRm(n) __stringify(ETM4x_CRm(n)) +#define etm4x_cp14_Op2(n) __stringify(ETM4x_Op2(n)) -#define etm4x_relaxed_write64(csa, val, offset) \

• writeq_relaxed((val), (csa)->base + (offset))

+#define READ_ETM4x_REG(reg) \

• MRC14(1, ETM4x_CRn(reg), ETM4x_CRm(reg), ETM4x_Op2(reg))

+#define read_etm4x_sysreg_const_offset(offset) \

• READ_ETM4x_REG(ETM4x_OFFSET_TO_REG(offset))

+#define WRITE_ETM4x_REG(val, reg) \

• MCR14((val), 1, ETM4x_CRn((reg)), ETM4x_CRm((reg)), ETM4x_Op2((reg)))

+#define write_etm4x_sysreg_const_offset(val, offset) \

• WRITE_ETM4x_REG(val, ETM4x_OFFSET_TO_REG(offset))

+#endif

+#define CASE_READ(res, x) \

• case (x): { (res) = read_etm4x_sysreg_const_offset((x)); break; }

+#define CASE_WRITE(val, x) \

• case (x): { write_etm4x_sysreg_const_offset((val), (x)); break; }

+#define CASE_LIST(op, val) \

• CASE_##op((val), TRCPRGCTLR) \
• CASE_##op((val), TRCPROCSELR) \
• CASE_##op((val), TRCSTATR) \
• CASE_##op((val), TRCCONFIGR) \
• CASE_##op((val), TRCAUXCTLR) \
• CASE_##op((val), TRCEVENTCTL0R) \
• CASE_##op((val), TRCEVENTCTL1R) \
• CASE_##op((val), TRCSTALLCTLR) \
• CASE_##op((val), TRCTSCTLR) \
• CASE_##op((val), TRCSYNCPR) \
• CASE_##op((val), TRCCCCTLR) \
• CASE_##op((val), TRCBBCTLR) \
• CASE_##op((val), TRCTRACEIDR) \
• CASE_##op((val), TRCQCTLR) \
• CASE_##op((val), TRCVICTLR) \
• CASE_##op((val), TRCVIIECTLR) \
• CASE_##op((val), TRCVISSCTLR) \
• CASE_##op((val), TRCVIPCSSCTLR) \
• CASE_##op((val), TRCVDCTLR) \
• CASE_##op((val), TRCVDSACCTLR) \
• CASE_##op((val), TRCVDARCCTLR) \
• CASE_##op((val), TRCSEQEVRn(0)) \
• CASE_##op((val), TRCSEQEVRn(1)) \
• CASE_##op((val), TRCSEQEVRn(2)) \
• CASE_##op((val), TRCSEQRSTEVR) \
• CASE_##op((val), TRCSEQSTR) \
• CASE_##op((val), TRCEXTINSELR) \
• CASE_##op((val), TRCCNTRLDVRn(0)) \
• CASE_##op((val), TRCCNTRLDVRn(1)) \
• CASE_##op((val), TRCCNTRLDVRn(2)) \
• CASE_##op((val), TRCCNTRLDVRn(3)) \
• CASE_##op((val), TRCCNTCTLRn(0)) \
• CASE_##op((val), TRCCNTCTLRn(1)) \
• CASE_##op((val), TRCCNTCTLRn(2)) \
• CASE_##op((val), TRCCNTCTLRn(3)) \
• CASE_##op((val), TRCCNTVRn(0)) \
• CASE_##op((val), TRCCNTVRn(1)) \
• CASE_##op((val), TRCCNTVRn(2)) \
• CASE_##op((val), TRCCNTVRn(3)) \
• CASE_##op((val), TRCIDR8) \
• CASE_##op((val), TRCIDR9) \
• CASE_##op((val), TRCIDR10) \
• CASE_##op((val), TRCIDR11) \
• CASE_##op((val), TRCIDR12) \
• CASE_##op((val), TRCIDR13) \
• CASE_##op((val), TRCIMSPECn(0)) \
• CASE_##op((val), TRCIMSPECn(1)) \
• CASE_##op((val), TRCIMSPECn(2)) \
• CASE_##op((val), TRCIMSPECn(3)) \
• CASE_##op((val), TRCIMSPECn(4)) \
• CASE_##op((val), TRCIMSPECn(5)) \
• CASE_##op((val), TRCIMSPECn(6)) \
• CASE_##op((val), TRCIMSPECn(7)) \
• CASE_##op((val), TRCIDR0) \
• CASE_##op((val), TRCIDR1) \
• CASE_##op((val), TRCIDR2) \
• CASE_##op((val), TRCIDR3) \
• CASE_##op((val), TRCIDR4) \
• CASE_##op((val), TRCIDR5) \
• CASE_##op((val), TRCIDR6) \
• CASE_##op((val), TRCIDR7) \
• CASE_##op((val), TRCRSCTLRn(2)) \
• CASE_##op((val), TRCRSCTLRn(3)) \
• CASE_##op((val), TRCRSCTLRn(4)) \
• CASE_##op((val), TRCRSCTLRn(5)) \
• CASE_##op((val), TRCRSCTLRn(6)) \
• CASE_##op((val), TRCRSCTLRn(7)) \
• CASE_##op((val), TRCRSCTLRn(8)) \
• CASE_##op((val), TRCRSCTLRn(9)) \
• CASE_##op((val), TRCRSCTLRn(10)) \
• CASE_##op((val), TRCRSCTLRn(11)) \
• CASE_##op((val), TRCRSCTLRn(12)) \
• CASE_##op((val), TRCRSCTLRn(13)) \
• CASE_##op((val), TRCRSCTLRn(14)) \
• CASE_##op((val), TRCRSCTLRn(15)) \
• CASE_##op((val), TRCRSCTLRn(16)) \
• CASE_##op((val), TRCRSCTLRn(17)) \
• CASE_##op((val), TRCRSCTLRn(18)) \
• CASE_##op((val), TRCRSCTLRn(19)) \
• CASE_##op((val), TRCRSCTLRn(20)) \
• CASE_##op((val), TRCRSCTLRn(21)) \
• CASE_##op((val), TRCRSCTLRn(22)) \
• CASE_##op((val), TRCRSCTLRn(23)) \
• CASE_##op((val), TRCRSCTLRn(24)) \
• CASE_##op((val), TRCRSCTLRn(25)) \
• CASE_##op((val), TRCRSCTLRn(26)) \
• CASE_##op((val), TRCRSCTLRn(27)) \
• CASE_##op((val), TRCRSCTLRn(28)) \
• CASE_##op((val), TRCRSCTLRn(29)) \
• CASE_##op((val), TRCRSCTLRn(30)) \
• CASE_##op((val), TRCRSCTLRn(31)) \
• CASE_##op((val), TRCSSCCRn(0)) \
• CASE_##op((val), TRCSSCCRn(1)) \
• CASE_##op((val), TRCSSCCRn(2)) \
• CASE_##op((val), TRCSSCCRn(3)) \
• CASE_##op((val), TRCSSCCRn(4)) \
• CASE_##op((val), TRCSSCCRn(5)) \
• CASE_##op((val), TRCSSCCRn(6)) \
• CASE_##op((val), TRCSSCCRn(7)) \
• CASE_##op((val), TRCSSCSRn(0)) \
• CASE_##op((val), TRCSSCSRn(1)) \
• CASE_##op((val), TRCSSCSRn(2)) \
• CASE_##op((val), TRCSSCSRn(3)) \
• CASE_##op((val), TRCSSCSRn(4)) \
• CASE_##op((val), TRCSSCSRn(5)) \
• CASE_##op((val), TRCSSCSRn(6)) \
• CASE_##op((val), TRCSSCSRn(7)) \
• CASE_##op((val), TRCSSPCICRn(0)) \
• CASE_##op((val), TRCSSPCICRn(1)) \
• CASE_##op((val), TRCSSPCICRn(2)) \
• CASE_##op((val), TRCSSPCICRn(3)) \
• CASE_##op((val), TRCSSPCICRn(4)) \
• CASE_##op((val), TRCSSPCICRn(5)) \
• CASE_##op((val), TRCSSPCICRn(6)) \
• CASE_##op((val), TRCSSPCICRn(7)) \
• CASE_##op((val), TRCOSLAR) \
• CASE_##op((val), TRCOSLSR) \
• CASE_##op((val), TRCPDCR) \
• CASE_##op((val), TRCPDSR) \
• CASE_##op((val), TRCACVRn(0)) \
• CASE_##op((val), TRCACVRn(1)) \
• CASE_##op((val), TRCACVRn(2)) \
• CASE_##op((val), TRCACVRn(3)) \
• CASE_##op((val), TRCACVRn(4)) \
• CASE_##op((val), TRCACVRn(5)) \
• CASE_##op((val), TRCACVRn(6)) \
• CASE_##op((val), TRCACVRn(7)) \
• CASE_##op((val), TRCACVRn(8)) \
• CASE_##op((val), TRCACVRn(9)) \
• CASE_##op((val), TRCACVRn(10)) \
• CASE_##op((val), TRCACVRn(11)) \
• CASE_##op((val), TRCACVRn(12)) \
• CASE_##op((val), TRCACVRn(13)) \
• CASE_##op((val), TRCACVRn(14)) \
• CASE_##op((val), TRCACVRn(15)) \
• CASE_##op((val), TRCACATRn(0)) \
• CASE_##op((val), TRCACATRn(1)) \
• CASE_##op((val), TRCACATRn(2)) \
• CASE_##op((val), TRCACATRn(3)) \
• CASE_##op((val), TRCACATRn(4)) \
• CASE_##op((val), TRCACATRn(5)) \
• CASE_##op((val), TRCACATRn(6)) \
• CASE_##op((val), TRCACATRn(7)) \
• CASE_##op((val), TRCACATRn(8)) \
• CASE_##op((val), TRCACATRn(9)) \
• CASE_##op((val), TRCACATRn(10)) \
• CASE_##op((val), TRCACATRn(11)) \
• CASE_##op((val), TRCACATRn(12)) \
• CASE_##op((val), TRCACATRn(13)) \
• CASE_##op((val), TRCACATRn(14)) \
• CASE_##op((val), TRCACATRn(15)) \
• CASE_##op((val), TRCDVCVRn(0)) \
• CASE_##op((val), TRCDVCVRn(1)) \
• CASE_##op((val), TRCDVCVRn(2)) \
• CASE_##op((val), TRCDVCVRn(3)) \
• CASE_##op((val), TRCDVCVRn(4)) \
• CASE_##op((val), TRCDVCVRn(5)) \
• CASE_##op((val), TRCDVCVRn(6)) \
• CASE_##op((val), TRCDVCVRn(7)) \
• CASE_##op((val), TRCDVCMRn(0)) \
• CASE_##op((val), TRCDVCMRn(1)) \
• CASE_##op((val), TRCDVCMRn(2)) \
• CASE_##op((val), TRCDVCMRn(3)) \
• CASE_##op((val), TRCDVCMRn(4)) \
• CASE_##op((val), TRCDVCMRn(5)) \
• CASE_##op((val), TRCDVCMRn(6)) \
• CASE_##op((val), TRCDVCMRn(7)) \
• CASE_##op((val), TRCCIDCVRn(0)) \
• CASE_##op((val), TRCCIDCVRn(1)) \
• CASE_##op((val), TRCCIDCVRn(2)) \
• CASE_##op((val), TRCCIDCVRn(3)) \
• CASE_##op((val), TRCCIDCVRn(4)) \
• CASE_##op((val), TRCCIDCVRn(5)) \
• CASE_##op((val), TRCCIDCVRn(6)) \
• CASE_##op((val), TRCCIDCVRn(7)) \
• CASE_##op((val), TRCVMIDCVRn(0)) \
• CASE_##op((val), TRCVMIDCVRn(1)) \
• CASE_##op((val), TRCVMIDCVRn(2)) \
• CASE_##op((val), TRCVMIDCVRn(3)) \
• CASE_##op((val), TRCVMIDCVRn(4)) \
• CASE_##op((val), TRCVMIDCVRn(5)) \
• CASE_##op((val), TRCVMIDCVRn(6)) \
• CASE_##op((val), TRCVMIDCVRn(7)) \
• CASE_##op((val), TRCCIDCCTLR0) \
• CASE_##op((val), TRCCIDCCTLR1) \
• CASE_##op((val), TRCVMIDCCTLR0) \
• CASE_##op((val), TRCVMIDCCTLR1) \
• CASE_##op((val), TRCITCTRL) \
• CASE_##op((val), TRCCLAIMSET) \
• CASE_##op((val), TRCCLAIMCLR) \
• CASE_##op((val), TRCDEVAFF0) \
• CASE_##op((val), TRCDEVAFF1) \
• CASE_##op((val), TRCLAR) \
• CASE_##op((val), TRCLSR) \
• CASE_##op((val), TRCAUTHSTATUS) \
• CASE_##op((val), TRCDEVARCH) \
• CASE_##op((val), TRCDEVID) \
• CASE_##op((val), TRCDEVTYPE) \
• CASE_##op((val), TRCPIDR4) \
• CASE_##op((val), TRCPIDR5) \
• CASE_##op((val), TRCPIDR6) \
• CASE_##op((val), TRCPIDR7) \
• CASE_##op((val), TRCPIDR0) \
• CASE_##op((val), TRCPIDR1) \
• CASE_##op((val), TRCPIDR2) \
• CASE_##op((val), TRCPIDR3)

+#define ETM4x_READ_CASES(res) CASE_LIST(READ, (res)) +#define ETM4x_WRITE_CASES(val) CASE_LIST(WRITE, (val))

+#define __read_etm4x_sysreg_offset(csa, offset, _64bit) \

• ({ \

• u64 __val;							\
  

• 								\
  

• if (__builtin_constant_p((offset)))				\
  

• 	__val = read_etm4x_sysreg_const_offset((offset));	\
  

• else								\
  

• 	__val = etm4x_sysreg_read((csa), (offset),		\
  

• 				  true, _64bit);		\
  

• __val;								\
  

• })
  

+#define __write_etm4x_sysreg_offset(csa, val, offset, _64bit) \

• do { \

• if (__builtin_constant_p((offset)))			\
  

• 	write_etm4x_sysreg_const_offset((val),		\
  

• 					(offset));	\
  

• else							\
  

• 	etm4x_sysreg_write((csa), (val), (offset),	\
  

• 				true, _64bit);		\
  

• } while (0)

+#define etm4x_relaxed_read32(csa, offset) \

• (u32)(!(csa)->no_iomem ? \

•  readl_relaxed((csa)->base + (offset)) :		\
  

•  __read_etm4x_sysreg_offset((csa), (offset), false))
  

+#define etm4x_relaxed_read64(csa, offset) \

• (u64)(!(csa)->no_iomem ? \

•  readq_relaxed((csa)->base + (offset)) :		\
  

•  __read_etm4x_sysreg_offset((csa), (offset), true))
  

+#define etm4x_read32(csa, offset) \

• ({ \

• mb(); /* enforce order */				\
  

• etm4x_relaxed_read32((csa), (offset));			\
  

• })
  

+#define etm4x_read64(csa, offset) \

• ({ \

• mb(); /* enforce order */				\
  

• etm4x_relaxed_read64((csa), (offset));			\
  

• })
  

+#define etm4x_relaxed_write32(csa, val, offset) \

• do { \

• if (!(csa)->no_iomem)					\
  

• 	writel_relaxed((val), (csa)->base + (offset));	\
  

• else							\
  

• 	__write_etm4x_sysreg_offset((csa), (val),	\
  

• 				    (offset), false);	\
  

• } while (0)

+#define etm4x_relaxed_write64(csa, val, offset) \

• do { \

• if (!(csa)->no_iomem)					\
  

• 	writeq_relaxed((val), (csa)->base + (offset));	\
  

• else							\
  

• 	__write_etm4x_sysreg_offset((csa), (val),	\
  

• 				    (offset), true);	\
  

• } while (0)

+#define etm4x_write32(csa, val, offset) \

• do { \

• mb(); /* enforce order */				\
  

• etm4x_relaxed_write32((csa), (val), (offset));		\
  

• } while (0)

+#define etm4x_write64(csa, val, offset) \

• do { \

• mb(); /* enforce order */				\
  

• etm4x_relaxed_write64((csa), (val), (offset));		\
  

• } while (0)

-#define etm4x_write64(csa, val, offset) \

• writeq((val), (csa)->base + (offset))

/* ETMv4 resources */ #define ETM_MAX_NR_PE 8 @@ -501,4 +822,14 @@ enum etm_addr_ctxtype { extern const struct attribute_group *coresight_etmv4_groups[]; void etm4_config_trace_mode(struct etmv4_config *config);

+u64 etm4x_sysreg_read(struct csdev_access *csa,

•       u32 offset,
  

•       bool _relaxed,
  

•       bool _64bit);
  

+void etm4x_sysreg_write(struct csdev_access *csa,

• 	u64 val,
  

• 	u32 offset,
  

• 	bool _relaxed,
  

• 	bool _64bit);
  

#endif

2.24.1

On Wed, Jul 22, 2020 at 06:20:40PM +0100, Suzuki K Poulose wrote:

Document the bindings for ETMv4.4 and later with only system register access.

Cc: Rob Herring robh+dt@kernel.org Cc: devicetree@vger.kernel.org Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Mike Leach mike.leach@linaro.org Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com

---

Documentation/devicetree/bindings/arm/coresight.txt | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/arm/coresight.txt b/Documentation/devicetree/bindings/arm/coresight.txt index d711676b4a51..cfe47bdda728 100644 --- a/Documentation/devicetree/bindings/arm/coresight.txt +++ b/Documentation/devicetree/bindings/arm/coresight.txt @@ -34,9 +34,13 @@ its hardware characteristcs. Program Flow Trace Macrocell: "arm,coresight-etm3x", "arm,primecell";

• - Embedded Trace Macrocell (version 4.x):
  

• - Embedded Trace Macrocell (version 4.x), with memory mapped access.
  "arm,coresight-etm4x", "arm,primecell";
  

• - Embedded Trace Macrocell (version 4.4 and later) with system
  

•   register access only.
  

• 	"arm,coresight-etm-v4.4";
  

I would rather call this "arm,coresight-etm-v4.4+" so that the binding's semantic is still relevant when dealing with ETM v4.5 and onward.

Thanks, Mathieu

• • Coresight programmable Replicator : "arm,coresight-dynamic-replicator", "arm,primecell";

2.24.1

On Thu, Jul 30, 2020 at 05:38:37PM +0100, Suzuki K Poulose wrote:

On 07/29/2020 06:20 PM, Mathieu Poirier wrote:

...

On Wed, Jul 22, 2020 at 06:20:40PM +0100, Suzuki K Poulose wrote:

...

Document the bindings for ETMv4.4 and later with only system register access.

Cc: Rob Herring robh+dt@kernel.org Cc: devicetree@vger.kernel.org Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Mike Leach mike.leach@linaro.org Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com

---

Documentation/devicetree/bindings/arm/coresight.txt | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/arm/coresight.txt b/Documentation/devicetree/bindings/arm/coresight.txt index d711676b4a51..cfe47bdda728 100644 --- a/Documentation/devicetree/bindings/arm/coresight.txt +++ b/Documentation/devicetree/bindings/arm/coresight.txt @@ -34,9 +34,13 @@ its hardware characteristcs. Program Flow Trace Macrocell: "arm,coresight-etm3x", "arm,primecell";

• - Embedded Trace Macrocell (version 4.x):
  

• - Embedded Trace Macrocell (version 4.x), with memory mapped access.
  "arm,coresight-etm4x", "arm,primecell";
  

• - Embedded Trace Macrocell (version 4.4 and later) with system
  

•   register access only.
  

• 	"arm,coresight-etm-v4.4";
  

I would rather call this "arm,coresight-etm-v4.4+" so that the binding's semantic is still relevant when dealing with ETM v4.5 and onward.

AFAIUC, "compatible" stands for something that is compatible with v4.4. All v4.4+ versions that are compatible with v4.4 are covered here.

Your position is valid - let's to with what you had.

Having said that I am fine with "arm,coresight-etm-v4.4+" , if it is fine by the DT conventions.

Cheers Suzuki