On 11/2/20 9:46 PM, Mathieu Poirier wrote:

Hi Suzuki,

On Wed, Oct 28, 2020 at 10:09:25PM +0000, Suzuki K Poulose wrote:

...

TRCSSPCICR<n> is present only if all of the following are true: TRCIDR4.NUMSSCC > n. TRCIDR4.NUMPC > 0b0000 . TRCSSCSR<n>.PC == 0b1

Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com

drivers/hwtracing/coresight/coresight-etm4x-core.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index d78a37b6592c..0310eac9dc16 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -175,8 +175,9 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata) drvdata->base + TRCSSCCRn(i)); writel_relaxed(config->ss_status[i], drvdata->base + TRCSSCSRn(i));

• writel_relaxed(config->ss_pe_cmp[i],
  

• 	       drvdata->base + TRCSSPCICRn(i));
  

• if (drvdata->nr_pe)
  

Aren't you missing to check the value of the PC bit in TRCSSCSRn?

             /*
              * TRCSSCSRn:PC, bit[3]: Indidate support for single-shot PE
              * comparator input.
              */
             if (drvdata->nr_pe && (config->ss_status[i] & BIT(3)))

You're right. Thank for catching this. I will update the series and drop the patches 1-5.

I have picked up patches 1 to 5 and added a "Cc:stable" to paches 2, 4 and 5. More comments to come tomorrow.

Thanks ! Suzuki

Hi Suzuki,

On Wed, Oct 28, 2020 at 10:09:26PM +0000, 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.

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 | 1 + drivers/hwtracing/coresight/coresight-core.c | 49 +++++ .../hwtracing/coresight/coresight-cti-core.c | 1 + drivers/hwtracing/coresight/coresight-etb10.c | 1 + .../coresight/coresight-etm3x-core.c | 1 + .../coresight/coresight-etm4x-core.c | 1 + .../hwtracing/coresight/coresight-funnel.c | 1 + .../coresight/coresight-replicator.c | 1 + drivers/hwtracing/coresight/coresight-stm.c | 1 + .../hwtracing/coresight/coresight-tmc-core.c | 1 + drivers/hwtracing/coresight/coresight-tpiu.c | 1 + include/linux/coresight.h | 197 ++++++++++++++++++ 12 files changed, 256 insertions(+)

diff --git a/drivers/hwtracing/coresight/coresight-catu.c b/drivers/hwtracing/coresight/coresight-catu.c index 99430f6cf5a5..5baf29510f1b 100644 --- a/drivers/hwtracing/coresight/coresight-catu.c +++ b/drivers/hwtracing/coresight/coresight-catu.c @@ -551,6 +551,7 @@ static int catu_probe(struct amba_device *adev, const struct amba_id *id) dev->platform_data = pdata; drvdata->base = base;

• catu_desc.access = CSDEV_ACCESS_IOMEM(base);

Ok for those

catu_desc.pdata = pdata; catu_desc.dev = dev; catu_desc.groups = catu_groups; diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c index cc9e8025c533..e96deaca8cab 100644 --- a/drivers/hwtracing/coresight/coresight-core.c +++ b/drivers/hwtracing/coresight/coresight-core.c @@ -1452,6 +1452,54 @@ int coresight_timeout(void __iomem *addr, u32 offset, int position, int value) } EXPORT_SYMBOL_GPL(coresight_timeout); +u32 coresight_relaxed_read32(struct coresight_device *csdev, u32 offset) +{

• return csdev_access_relaxed_read32(&csdev->access, offset);

This really doesn't give us much other than another jump. I would give function csdev_access_relaxed_read32() a coresight_device argument instead of a csdev_access and rename it to coresight_relaxed_read32(). The same for the other access functions.

+}

+u32 coresight_read32(struct coresight_device *csdev, u32 offset) +{

• return csdev_access_read32(&csdev->access, offset);

+}

+void coresight_relaxed_write32(struct coresight_device *csdev,

• 	       u32 val,
  

• 	       u32 offset)
  

+{

Extra new line

• csdev_access_relaxed_write32(&csdev->access, val, offset);

+}

+void coresight_write32(struct coresight_device *csdev, u32 val, u32 offset) +{

• csdev_access_write32(&csdev->access, val, offset);

+}

+u64 coresight_relaxed_read64(struct coresight_device *csdev, u32 offset) +{

• return csdev_access_relaxed_read64(&csdev->access, offset);

+}

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

• return csdev_access_read64(&csdev->access, offset);

+}

+void coresight_relaxed_write64(struct coresight_device *csdev,

• 	       u64 val,
  

• 	       u32 offset)
  

+{

Extra new line

• csdev_access_relaxed_write64(&csdev->access, val, offset);

+}

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

• csdev_access_write64(&csdev->access, val, offset);

+}

/*

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

@@ -1516,6 +1564,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/drivers/hwtracing/coresight/coresight-cti-core.c b/drivers/hwtracing/coresight/coresight-cti-core.c index d28eae93e55c..3bb0de97d66e 100644 --- a/drivers/hwtracing/coresight/coresight-cti-core.c +++ b/drivers/hwtracing/coresight/coresight-cti-core.c @@ -870,6 +870,7 @@ static int cti_probe(struct amba_device *adev, const struct amba_id *id) return PTR_ERR(base); drvdata->base = base;

• cti_desc.access = CSDEV_ACCESS_IOMEM(base);

dev_set_drvdata(dev, drvdata); diff --git a/drivers/hwtracing/coresight/coresight-etb10.c b/drivers/hwtracing/coresight/coresight-etb10.c index 248cc82c838e..b91633c6c9b4 100644 --- a/drivers/hwtracing/coresight/coresight-etb10.c +++ b/drivers/hwtracing/coresight/coresight-etb10.c @@ -755,6 +755,7 @@ static int etb_probe(struct amba_device *adev, const struct amba_id *id) return PTR_ERR(base); drvdata->base = base;

• desc.access = CSDEV_ACCESS_IOMEM(base);

spin_lock_init(&drvdata->spinlock); diff --git a/drivers/hwtracing/coresight/coresight-etm3x-core.c b/drivers/hwtracing/coresight/coresight-etm3x-core.c index 47f610b1c2b1..36c5b0ae1b43 100644 --- a/drivers/hwtracing/coresight/coresight-etm3x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm3x-core.c @@ -839,6 +839,7 @@ static int etm_probe(struct amba_device *adev, const struct amba_id *id) return PTR_ERR(base); drvdata->base = base;

• desc.access = CSDEV_ACCESS_IOMEM(base);

spin_lock_init(&drvdata->spinlock); diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index 0310eac9dc16..c5cb93f1b80c 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -1512,6 +1512,7 @@ static int etm4_probe(struct amba_device *adev, const struct amba_id *id) return PTR_ERR(base); drvdata->base = base;

• desc.access = CSDEV_ACCESS_IOMEM(base);

spin_lock_init(&drvdata->spinlock); diff --git a/drivers/hwtracing/coresight/coresight-funnel.c b/drivers/hwtracing/coresight/coresight-funnel.c index af40814ce560..f77466aea26f 100644 --- a/drivers/hwtracing/coresight/coresight-funnel.c +++ b/drivers/hwtracing/coresight/coresight-funnel.c @@ -242,6 +242,7 @@ static int funnel_probe(struct device *dev, struct resource *res) } drvdata->base = base; desc.groups = coresight_funnel_groups;

• desc.access = CSDEV_ACCESS_IOMEM(base);
  

  }

dev_set_drvdata(dev, drvdata); diff --git a/drivers/hwtracing/coresight/coresight-replicator.c b/drivers/hwtracing/coresight/coresight-replicator.c index 62afdde0e5ea..fcf25740116c 100644 --- a/drivers/hwtracing/coresight/coresight-replicator.c +++ b/drivers/hwtracing/coresight/coresight-replicator.c @@ -254,6 +254,7 @@ static int replicator_probe(struct device *dev, struct resource *res) } drvdata->base = base; desc.groups = replicator_groups;

• desc.access = CSDEV_ACCESS_IOMEM(base);
  

  }

if (fwnode_property_present(dev_fwnode(dev), diff --git a/drivers/hwtracing/coresight/coresight-stm.c b/drivers/hwtracing/coresight/coresight-stm.c index b0ad912651a9..315500b7763f 100644 --- a/drivers/hwtracing/coresight/coresight-stm.c +++ b/drivers/hwtracing/coresight/coresight-stm.c @@ -884,6 +884,7 @@ static int stm_probe(struct amba_device *adev, const struct amba_id *id) if (IS_ERR(base)) return PTR_ERR(base); drvdata->base = base;

• desc.access = CSDEV_ACCESS_IOMEM(base);

ret = stm_get_stimulus_area(dev, &ch_res); if (ret) diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c b/drivers/hwtracing/coresight/coresight-tmc-core.c index 5653e0945c74..8fd640d41e1b 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-core.c +++ b/drivers/hwtracing/coresight/coresight-tmc-core.c @@ -456,6 +456,7 @@ static int tmc_probe(struct amba_device *adev, const struct amba_id *id) } drvdata->base = base;

• desc.access = CSDEV_ACCESS_IOMEM(base);

spin_lock_init(&drvdata->spinlock); diff --git a/drivers/hwtracing/coresight/coresight-tpiu.c b/drivers/hwtracing/coresight/coresight-tpiu.c index 566c57e03596..dfa3b91d0281 100644 --- a/drivers/hwtracing/coresight/coresight-tpiu.c +++ b/drivers/hwtracing/coresight/coresight-tpiu.c @@ -149,6 +149,7 @@ static int tpiu_probe(struct amba_device *adev, const struct amba_id *id) return PTR_ERR(base); drvdata->base = base;

• desc.access = CSDEV_ACCESS_IOMEM(base);

/* Disable tpiu to support older devices */ tpiu_disable_hw(drvdata); diff --git a/include/linux/coresight.h b/include/linux/coresight.h index 7d3c87e5b97c..5a34c2f2debc 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,38 @@ struct coresight_platform_data { struct coresight_connection *conns; }; +/**

• • struct csdev_access - Abstraction of a CoreSight device access.
• • @io_mem : True if the device has memory mapped I/O
• • @base : When io_mem == true, 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 io_mem;
• union {

• void __iomem *base;
  

• struct {
  

• 	u64 (*read)(struct csdev_access *csa,
  

• 		    u32 offset,
  

• 		    bool relaxed,
  

• 		    bool _64bit);
  

• 	void (*write)(struct csdev_access *csa,
  

• 		      u64 val,
  

• 		      u32 offset,
  

• 		      bool relaxed,
  

• 		      bool _64bit);
  

• };
  

• };

+};

+#define CSDEV_ACCESS_IOMEM(_addr) \

• ((struct csdev_access) { \

• .io_mem		= true,		\
  

• .base		= (_addr),	\
  

• })

/**

• struct coresight_desc - description of a component required from drivers
• @type: as defined by @coresight_dev_type.

@@ -125,6 +158,7 @@ struct coresight_platform_data {

• @groups: operations specific to this component. These will end up

• in the component's sysfs sub-directory.
  

• @name: name for the coresight device, also shown under sysfs.

• • @access: Describe access to the device
  */

struct coresight_desc { enum coresight_dev_type type; @@ -134,6 +168,7 @@ struct coresight_desc { struct device *dev; const struct attribute_group **groups; const char *name;

• struct csdev_access access;

}; /** @@ -186,6 +221,7 @@ struct coresight_sysfs_link {

• @def_sink: cached reference to default sink found for this device.
• @ect_dev: Associated cross trigger device. Not part of the trace data

• path or connections.
  

• • @access: Device i/o access abstraction for this device.
  • @nr_links: number of sysfs links created to other components from this

  • device. These will appear in the "connections" group.
    

  • @has_conns_grp: Have added a "connections" group for sysfs links.

@@ -205,6 +241,7 @@ struct coresight_device { struct coresight_device *def_sink; /* cross trigger handling */ struct coresight_device *ect_dev;

• struct csdev_access access;

Access should be part of the generic section of the structure rather than under CTI.

/* sysfs links between components */ int nr_links; bool has_conns_grp; @@ -326,6 +363,107 @@ struct coresight_ops { }; #if IS_ENABLED(CONFIG_CORESIGHT)

+static inline u32 csdev_access_relaxed_read32(struct csdev_access *csa,

• 			      u32 offset)
  

+{

• if (likely(csa->io_mem))

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

• return csa->read(csa, offset, true, false);

+}

+static inline u32 csdev_access_read32(struct csdev_access *csa, u32 offset) +{

• if (likely(csa->io_mem))

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

• return csa->read(csa, offset, false, false);

+}

+static inline void csdev_access_relaxed_write32(struct csdev_access *csa,

• 				u32 val,
  

No need for a new line.

• 				u32 offset)
  

+{

• if (likely(csa->io_mem))

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

• else

• csa->write(csa, val, offset, true, false);
  

+}

+static inline void csdev_access_write32(struct csdev_access *csa, u32 val, u32 offset) +{

• if (likely(csa->io_mem))

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

• else

• csa->write(csa, val, offset, false, false);
  

+}

+#ifdef CONFIG_64BIT

+static inline u64 csdev_access_relaxed_read64(struct csdev_access *csa,

• 			   u32 offset)
  

Indentation

+{

• if (likely(csa->io_mem))

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

• return csa->read(csa, offset, true, true);

+}

+static inline u64 csdev_access_read64(struct csdev_access *csa, u32 offset) +{

• if (likely(csa->io_mem))

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

• return csa->read(csa, offset, false, true);

+}

+static inline void csdev_access_relaxed_write64(struct csdev_access *csa,

• 				u64 val,
  

No need for a new line.

• 				u32 offset)
  

+{

• if (likely(csa->io_mem))

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

• else

• csa->write(csa, val, offset, true, true);
  

+}

+static inline void csdev_access_write64(struct csdev_access *csa, u64 val, u32 offset) +{

• if (likely(csa->io_mem))

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

• else

• csa->write(csa, val, offset, false, true);
  

+}

+#else

#else /* CONFIG_64BIT */

+static inline u64 csdev_access_relaxed_read64(struct csdev_access *csa,

• 			   u32 offset)
  

+{

• WARN_ON(1);
• return 0;

+}

+static inline u64 csdev_access_read64(struct csdev_access *csa, u32 offset) +{

• WARN_ON(1);
• return 0;

+}

+static inline void csdev_access_relaxed_write64(struct csdev_access *csa,

• 				u64 val,
  

No need for a new line.

• 				u32 offset)
  

+{

• WARN_ON(1);

+}

+static inline void csdev_access_write64(struct csdev_access *csa, u64 val, u32 offset) +{

• WARN_ON(1);

+} +#endif

#endif /* CONFIG_64BIT */

extern struct coresight_device * coresight_register(struct coresight_desc *desc); extern void coresight_unregister(struct coresight_device *csdev); @@ -343,6 +481,20 @@ extern char *coresight_alloc_device_name(struct coresight_dev_list *devs, struct device *dev); extern bool coresight_loses_context_with_cpu(struct device *dev);

+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);
  

No need for an new line

+u64 coresight_relaxed_read64(struct coresight_device *csdev, u32 offset); +u64 coresight_read64(struct coresight_device *csdev, u32 offset); +void coresight_relaxed_write64(struct coresight_device *csdev,

• 	       u64 val, u32 offset);
  

+void coresight_write64(struct coresight_device *csdev, u64 val, u32 offset);

#else static inline struct coresight_device * coresight_register(struct coresight_desc *desc) { return NULL; } @@ -369,6 +521,51 @@ static inline bool coresight_loses_context_with_cpu(struct device *dev) { return false; }

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

• WARN_ON_ONCE(1);
• return 0;

+}

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

• WARN_ON_ONCE(1);
• return 0;

+}

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

+static inline void coresight_relaxed_write32(struct coresight_device *csdev,

• 			      u32 val, u32 offset);
  

Indentation

+{ +}

+static inline u64 coresight_relaxed_read64(struct coresight_device *csdev,

• 			   u32 offset)
  

+{

• WARN_ON_ONCE(1);
• return 0;

+}

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

• WARN_ON_ONCE(1);
• 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

#endif /* IS_ENABLED(CONFIG_CORESIGHT) */

Otherwise it is quite confusing.

extern int coresight_get_cpu(struct device *dev); -- 2.24.1

On 11/3/20 5:25 PM, Mathieu Poirier wrote:

On Tue, Nov 03, 2020 at 10:14:17AM -0700, Mathieu Poirier wrote:

...

Hi Suzuki,

On Wed, Oct 28, 2020 at 10:09:26PM +0000, 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.

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 | 1 + drivers/hwtracing/coresight/coresight-core.c | 49 +++++ .../hwtracing/coresight/coresight-cti-core.c | 1 + drivers/hwtracing/coresight/coresight-etb10.c | 1 + .../coresight/coresight-etm3x-core.c | 1 + .../coresight/coresight-etm4x-core.c | 1 + .../hwtracing/coresight/coresight-funnel.c | 1 + .../coresight/coresight-replicator.c | 1 + drivers/hwtracing/coresight/coresight-stm.c | 1 + .../hwtracing/coresight/coresight-tmc-core.c | 1 + drivers/hwtracing/coresight/coresight-tpiu.c | 1 + include/linux/coresight.h | 197 ++++++++++++++++++ 12 files changed, 256 insertions(+)

diff --git a/drivers/hwtracing/coresight/coresight-catu.c b/drivers/hwtracing/coresight/coresight-catu.c index 99430f6cf5a5..5baf29510f1b 100644 --- a/drivers/hwtracing/coresight/coresight-catu.c +++ b/drivers/hwtracing/coresight/coresight-catu.c @@ -551,6 +551,7 @@ static int catu_probe(struct amba_device *adev, const struct amba_id *id) dev->platform_data = pdata; drvdata->base = base;

• catu_desc.access = CSDEV_ACCESS_IOMEM(base);

Ok for those

...

catu_desc.pdata = pdata; catu_desc.dev = dev; catu_desc.groups = catu_groups; diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c index cc9e8025c533..e96deaca8cab 100644 --- a/drivers/hwtracing/coresight/coresight-core.c +++ b/drivers/hwtracing/coresight/coresight-core.c @@ -1452,6 +1452,54 @@ int coresight_timeout(void __iomem *addr, u32 offset, int position, int value) } EXPORT_SYMBOL_GPL(coresight_timeout); +u32 coresight_relaxed_read32(struct coresight_device *csdev, u32 offset) +{

• return csdev_access_relaxed_read32(&csdev->access, offset);

This really doesn't give us much other than another jump. I would give function csdev_access_relaxed_read32() a coresight_device argument instead of a csdev_access and rename it to coresight_relaxed_read32(). The same for the other access functions.

Ignore the above, TPIU just gave me the logic behind what you did.

Thanks Mathieu, will address your comments in the next version.

Suzuki

On 11/9/20 9:00 PM, Mathieu Poirier wrote:

On Wed, Oct 28, 2020 at 10:09:26PM +0000, 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.

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 | 1 + drivers/hwtracing/coresight/coresight-core.c | 49 +++++ .../hwtracing/coresight/coresight-cti-core.c | 1 + drivers/hwtracing/coresight/coresight-etb10.c | 1 + .../coresight/coresight-etm3x-core.c | 1 + .../coresight/coresight-etm4x-core.c | 1 + .../hwtracing/coresight/coresight-funnel.c | 1 + .../coresight/coresight-replicator.c | 1 + drivers/hwtracing/coresight/coresight-stm.c | 1 + .../hwtracing/coresight/coresight-tmc-core.c | 1 + drivers/hwtracing/coresight/coresight-tpiu.c | 1 + include/linux/coresight.h | 197 ++++++++++++++++++ 12 files changed, 256 insertions(+)

diff --git a/drivers/hwtracing/coresight/coresight-catu.c b/drivers/hwtracing/coresight/coresight-catu.c index 99430f6cf5a5..5baf29510f1b 100644 --- a/drivers/hwtracing/coresight/coresight-catu.c +++ b/drivers/hwtracing/coresight/coresight-catu.c @@ -551,6 +551,7 @@ static int catu_probe(struct amba_device *adev, const struct amba_id *id) dev->platform_data = pdata; drvdata->base = base;

• catu_desc.access = CSDEV_ACCESS_IOMEM(base); catu_desc.pdata = pdata; catu_desc.dev = dev; catu_desc.groups = catu_groups;

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c index cc9e8025c533..e96deaca8cab 100644 --- a/drivers/hwtracing/coresight/coresight-core.c +++ b/drivers/hwtracing/coresight/coresight-core.c @@ -1452,6 +1452,54 @@ int coresight_timeout(void __iomem *addr, u32 offset, int position, int value) } EXPORT_SYMBOL_GPL(coresight_timeout); +u32 coresight_relaxed_read32(struct coresight_device *csdev, u32 offset) +{

• return csdev_access_relaxed_read32(&csdev->access, offset);

+}

+u32 coresight_read32(struct coresight_device *csdev, u32 offset) +{

• return csdev_access_read32(&csdev->access, offset);

+}

+void coresight_relaxed_write32(struct coresight_device *csdev,

• 	       u32 val,
  

• 	       u32 offset)
  

+{

• csdev_access_relaxed_write32(&csdev->access, val, offset);

+}

+void coresight_write32(struct coresight_device *csdev, u32 val, u32 offset) +{

• csdev_access_write32(&csdev->access, val, offset);

+}

+u64 coresight_relaxed_read64(struct coresight_device *csdev, u32 offset) +{

• return csdev_access_relaxed_read64(&csdev->access, offset);

+}

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

• return csdev_access_read64(&csdev->access, offset);

+}

+void coresight_relaxed_write64(struct coresight_device *csdev,

• 	       u64 val,
  

• 	       u32 offset)
  

+{

• csdev_access_relaxed_write64(&csdev->access, val, offset);

+}

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

• csdev_access_write64(&csdev->access, val, offset);

+}

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

@@ -1516,6 +1564,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/drivers/hwtracing/coresight/coresight-cti-core.c b/drivers/hwtracing/coresight/coresight-cti-core.c index d28eae93e55c..3bb0de97d66e 100644 --- a/drivers/hwtracing/coresight/coresight-cti-core.c +++ b/drivers/hwtracing/coresight/coresight-cti-core.c @@ -870,6 +870,7 @@ static int cti_probe(struct amba_device *adev, const struct amba_id *id) return PTR_ERR(base); drvdata->base = base;

• cti_desc.access = CSDEV_ACCESS_IOMEM(base);

dev_set_drvdata(dev, drvdata); diff --git a/drivers/hwtracing/coresight/coresight-etb10.c b/drivers/hwtracing/coresight/coresight-etb10.c index 248cc82c838e..b91633c6c9b4 100644 --- a/drivers/hwtracing/coresight/coresight-etb10.c +++ b/drivers/hwtracing/coresight/coresight-etb10.c @@ -755,6 +755,7 @@ static int etb_probe(struct amba_device *adev, const struct amba_id *id) return PTR_ERR(base); drvdata->base = base;

• desc.access = CSDEV_ACCESS_IOMEM(base);

spin_lock_init(&drvdata->spinlock); diff --git a/drivers/hwtracing/coresight/coresight-etm3x-core.c b/drivers/hwtracing/coresight/coresight-etm3x-core.c index 47f610b1c2b1..36c5b0ae1b43 100644 --- a/drivers/hwtracing/coresight/coresight-etm3x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm3x-core.c @@ -839,6 +839,7 @@ static int etm_probe(struct amba_device *adev, const struct amba_id *id) return PTR_ERR(base); drvdata->base = base;

• desc.access = CSDEV_ACCESS_IOMEM(base);

spin_lock_init(&drvdata->spinlock); diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index 0310eac9dc16..c5cb93f1b80c 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -1512,6 +1512,7 @@ static int etm4_probe(struct amba_device *adev, const struct amba_id *id) return PTR_ERR(base); drvdata->base = base;

• desc.access = CSDEV_ACCESS_IOMEM(base);

spin_lock_init(&drvdata->spinlock); diff --git a/drivers/hwtracing/coresight/coresight-funnel.c b/drivers/hwtracing/coresight/coresight-funnel.c index af40814ce560..f77466aea26f 100644 --- a/drivers/hwtracing/coresight/coresight-funnel.c +++ b/drivers/hwtracing/coresight/coresight-funnel.c @@ -242,6 +242,7 @@ static int funnel_probe(struct device *dev, struct resource *res) } drvdata->base = base; desc.groups = coresight_funnel_groups;

• desc.access = CSDEV_ACCESS_IOMEM(base);
  

  }

dev_set_drvdata(dev, drvdata); diff --git a/drivers/hwtracing/coresight/coresight-replicator.c b/drivers/hwtracing/coresight/coresight-replicator.c index 62afdde0e5ea..fcf25740116c 100644 --- a/drivers/hwtracing/coresight/coresight-replicator.c +++ b/drivers/hwtracing/coresight/coresight-replicator.c @@ -254,6 +254,7 @@ static int replicator_probe(struct device *dev, struct resource *res) } drvdata->base = base; desc.groups = replicator_groups;

• desc.access = CSDEV_ACCESS_IOMEM(base);
  

  }

if (fwnode_property_present(dev_fwnode(dev), diff --git a/drivers/hwtracing/coresight/coresight-stm.c b/drivers/hwtracing/coresight/coresight-stm.c index b0ad912651a9..315500b7763f 100644 --- a/drivers/hwtracing/coresight/coresight-stm.c +++ b/drivers/hwtracing/coresight/coresight-stm.c @@ -884,6 +884,7 @@ static int stm_probe(struct amba_device *adev, const struct amba_id *id) if (IS_ERR(base)) return PTR_ERR(base); drvdata->base = base;

• desc.access = CSDEV_ACCESS_IOMEM(base);

ret = stm_get_stimulus_area(dev, &ch_res); if (ret) diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c b/drivers/hwtracing/coresight/coresight-tmc-core.c index 5653e0945c74..8fd640d41e1b 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-core.c +++ b/drivers/hwtracing/coresight/coresight-tmc-core.c @@ -456,6 +456,7 @@ static int tmc_probe(struct amba_device *adev, const struct amba_id *id) } drvdata->base = base;

• desc.access = CSDEV_ACCESS_IOMEM(base);

spin_lock_init(&drvdata->spinlock); diff --git a/drivers/hwtracing/coresight/coresight-tpiu.c b/drivers/hwtracing/coresight/coresight-tpiu.c index 566c57e03596..dfa3b91d0281 100644 --- a/drivers/hwtracing/coresight/coresight-tpiu.c +++ b/drivers/hwtracing/coresight/coresight-tpiu.c @@ -149,6 +149,7 @@ static int tpiu_probe(struct amba_device *adev, const struct amba_id *id) return PTR_ERR(base); drvdata->base = base;

• desc.access = CSDEV_ACCESS_IOMEM(base);

/* Disable tpiu to support older devices */ tpiu_disable_hw(drvdata); diff --git a/include/linux/coresight.h b/include/linux/coresight.h index 7d3c87e5b97c..5a34c2f2debc 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,38 @@ struct coresight_platform_data { struct coresight_connection *conns; }; +/**

• • struct csdev_access - Abstraction of a CoreSight device access.
• • @io_mem : True if the device has memory mapped I/O
• • @base : When io_mem == true, 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 io_mem;
• union {

• void __iomem *base;
  

• struct {
  

• 	u64 (*read)(struct csdev_access *csa,
  

• 		    u32 offset,
  

• 		    bool relaxed,
  

• 		    bool _64bit);
  

• 	void (*write)(struct csdev_access *csa,
  

• 		      u64 val,
  

• 		      u32 offset,
  

• 		      bool relaxed,
  

• 		      bool _64bit);
  

What is the strategy behing passing a struct csdev_acces to (*read) and (*write)? I kept looking for it while reviewing this set but couldn't find anything.

Right now it is unused. But, when I designed it, it was supposed to also clean up the ETM3 accesses, where some of them are iomapped and some are CP14 access. So, we could use the csa to get the "base" for iomap access and also use the sysreg access. This can be another series. Didn't want to bloat up this series which is already a mammoth to review.

Also, the drivers could use the "csa" to do the following : - Get to a container struct for more information to do appropriate actions. - Extend the csa by adding a private field (this was part of one of the drafts, but later dropped to lack of immediate uses).

I could add in a comment if that helps.

Cheers Suzki

Thanks Mathieu

...

• };
  

• };

+};

+#define CSDEV_ACCESS_IOMEM(_addr) \

• ((struct csdev_access) { \

• .io_mem		= true,		\
  

• .base		= (_addr),	\
  

• })
• /**
  • struct coresight_desc - description of a component required from drivers
  • @type: as defined by @coresight_dev_type.

@@ -125,6 +158,7 @@ struct coresight_platform_data {

• @groups: operations specific to this component. These will end up

• in the component's sysfs sub-directory.
  

• @name: name for the coresight device, also shown under sysfs.

• • @access: Describe access to the device
  */ struct coresight_desc { enum coresight_dev_type type;

@@ -134,6 +168,7 @@ struct coresight_desc { struct device *dev; const struct attribute_group **groups; const char *name;

• struct csdev_access access; };

/** @@ -186,6 +221,7 @@ struct coresight_sysfs_link {

• @def_sink: cached reference to default sink found for this device.
• @ect_dev: Associated cross trigger device. Not part of the trace data

• path or connections.
  

• • @access: Device i/o access abstraction for this device.
  • @nr_links: number of sysfs links created to other components from this

  • device. These will appear in the "connections" group.
    

  • @has_conns_grp: Have added a "connections" group for sysfs links.

@@ -205,6 +241,7 @@ struct coresight_device { struct coresight_device *def_sink; /* cross trigger handling */ struct coresight_device *ect_dev;

• struct csdev_access access; /* sysfs links between components */ int nr_links; bool has_conns_grp;

@@ -326,6 +363,107 @@ struct coresight_ops { }; #if IS_ENABLED(CONFIG_CORESIGHT)

+static inline u32 csdev_access_relaxed_read32(struct csdev_access *csa,

• 			      u32 offset)
  

+{

• if (likely(csa->io_mem))

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

• return csa->read(csa, offset, true, false);

+}

+static inline u32 csdev_access_read32(struct csdev_access *csa, u32 offset) +{

• if (likely(csa->io_mem))

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

• return csa->read(csa, offset, false, false);

+}

+static inline void csdev_access_relaxed_write32(struct csdev_access *csa,

• 				u32 val,
  

• 				u32 offset)
  

+{

• if (likely(csa->io_mem))

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

• else

• csa->write(csa, val, offset, true, false);
  

+}

+static inline void csdev_access_write32(struct csdev_access *csa, u32 val, u32 offset) +{

• if (likely(csa->io_mem))

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

• else

• csa->write(csa, val, offset, false, false);
  

+}

+#ifdef CONFIG_64BIT

+static inline u64 csdev_access_relaxed_read64(struct csdev_access *csa,

• 			   u32 offset)
  

+{

• if (likely(csa->io_mem))

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

• return csa->read(csa, offset, true, true);

+}

+static inline u64 csdev_access_read64(struct csdev_access *csa, u32 offset) +{

• if (likely(csa->io_mem))

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

• return csa->read(csa, offset, false, true);

+}

+static inline void csdev_access_relaxed_write64(struct csdev_access *csa,

• 				u64 val,
  

• 				u32 offset)
  

+{

• if (likely(csa->io_mem))

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

• else

• csa->write(csa, val, offset, true, true);
  

+}

+static inline void csdev_access_write64(struct csdev_access *csa, u64 val, u32 offset) +{

• if (likely(csa->io_mem))

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

• else

• csa->write(csa, val, offset, false, true);
  

+}

+#else

+static inline u64 csdev_access_relaxed_read64(struct csdev_access *csa,

• 			   u32 offset)
  

+{

• WARN_ON(1);
• return 0;

+}

+static inline u64 csdev_access_read64(struct csdev_access *csa, u32 offset) +{

• WARN_ON(1);
• return 0;

+}

+static inline void csdev_access_relaxed_write64(struct csdev_access *csa,

• 				u64 val,
  

• 				u32 offset)
  

+{

• WARN_ON(1);

+}

+static inline void csdev_access_write64(struct csdev_access *csa, u64 val, u32 offset) +{

• WARN_ON(1);

+} +#endif

• extern struct coresight_device * coresight_register(struct coresight_desc *desc); extern void coresight_unregister(struct coresight_device *csdev);

@@ -343,6 +481,20 @@ extern char *coresight_alloc_device_name(struct coresight_dev_list *devs, struct device *dev); extern bool coresight_loses_context_with_cpu(struct device *dev);

+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);
  

+u64 coresight_relaxed_read64(struct coresight_device *csdev, u32 offset); +u64 coresight_read64(struct coresight_device *csdev, u32 offset); +void coresight_relaxed_write64(struct coresight_device *csdev,

• 	       u64 val, u32 offset);
  

+void coresight_write64(struct coresight_device *csdev, u64 val, u32 offset);

• #else static inline struct coresight_device * coresight_register(struct coresight_desc *desc) { return NULL; }

@@ -369,6 +521,51 @@ static inline bool coresight_loses_context_with_cpu(struct device *dev) { return false; }

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

• WARN_ON_ONCE(1);
• return 0;

+}

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

• WARN_ON_ONCE(1);
• return 0;

+}

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

+static inline void coresight_relaxed_write32(struct coresight_device *csdev,

• 			      u32 val, u32 offset);
  

+{ +}

+static inline u64 coresight_relaxed_read64(struct coresight_device *csdev,

• 			   u32 offset)
  

+{

• WARN_ON_ONCE(1);
• return 0;

+}

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

• WARN_ON_ONCE(1);
• 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

extern int coresight_get_cpu(struct device *dev); -- 2.24.1

On 11/3/20 6:03 PM, Mathieu Poirier wrote:

On Wed, Oct 28, 2020 at 10:09:28PM +0000, Suzuki K Poulose wrote:

...

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

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

---

...

diff --git a/drivers/hwtracing/coresight/coresight-catu.c b/drivers/hwtracing/coresight/coresight-catu.c index 5baf29510f1b..34c74b05c542 100644 --- a/drivers/hwtracing/coresight/coresight-catu.c +++ b/drivers/hwtracing/coresight/coresight-catu.c @@ -401,8 +401,9 @@ static const struct attribute_group *catu_groups[] = { static inline int catu_wait_for_ready(struct catu_drvdata *drvdata) {

• return coresight_timeout(drvdata->base,

• 		 CATU_STATUS, CATU_STATUS_READY, 1);
  

• struct csdev_access *csa = &drvdata->csdev->access;
• return coresight_timeout(csa, CATU_STATUS, CATU_STATUS_READY, 1); }

static int catu_enable_hw(struct catu_drvdata *drvdata, void *data) diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c index e96deaca8cab..42ba989a6b5e 100644 --- a/drivers/hwtracing/coresight/coresight-core.c +++ b/drivers/hwtracing/coresight/coresight-core.c @@ -1412,23 +1412,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)
  

There is no need for stacking, please maximise the 80 characters. The function stubs in coresight.h should also be revised.

...

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

• val = __raw_readl(addr + offset);
  

• val = csdev_access_read32(csa, offset);
  

I vaguely remember commenting on this, or perhaps it was on some othe patch you wrote... Anyways, I think it is a good thing to go from an unordered access to an ordered access for the timeout function.

Yes, you did mention it in the v1.

With the above:

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

Thanks, will address the comments.

Suzuki

On Wed, Oct 28, 2020 at 10:09:29PM +0000, Suzuki K Poulose wrote:

Convert the generic CLAIM tag management APIs to use the device access layer abstraction.

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 | 6 +- drivers/hwtracing/coresight/coresight-core.c | 66 +++++++++++-------- .../hwtracing/coresight/coresight-cti-core.c | 17 +++-- drivers/hwtracing/coresight/coresight-etb10.c | 4 +- .../coresight/coresight-etm3x-core.c | 8 ++- .../coresight/coresight-etm4x-core.c | 4 +- .../hwtracing/coresight/coresight-funnel.c | 6 +- .../coresight/coresight-replicator.c | 16 +++-- .../hwtracing/coresight/coresight-tmc-etf.c | 10 +-- .../hwtracing/coresight/coresight-tmc-etr.c | 4 +- include/linux/coresight.h | 16 ++--- 11 files changed, 95 insertions(+), 62 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-catu.c b/drivers/hwtracing/coresight/coresight-catu.c index 34c74b05c542..438063523463 100644 --- a/drivers/hwtracing/coresight/coresight-catu.c +++ b/drivers/hwtracing/coresight/coresight-catu.c @@ -412,6 +412,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"); @@ -422,7 +423,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;

@@ -466,9 +467,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;

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c index 42ba989a6b5e..4e7dd5941046 100644 --- a/drivers/hwtracing/coresight/coresight-core.c +++ b/drivers/hwtracing/coresight/coresight-core.c @@ -145,30 +145,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 csdev_access_relaxed_read32(&csdev->access, 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);

• csdev_access_relaxed_write32(&csdev->access, CORESIGHT_CLAIM_SELF_HOSTED,

• 		  CORESIGHT_CLAIMSET);
  

Indentation

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);

• csdev_access_relaxed_write32(&csdev->access, CORESIGHT_CLAIM_SELF_HOSTED,

• 		  CORESIGHT_CLAIMCLR);
  

Indentation

isb(); } @@ -182,27 +184,33 @@ 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;

} EXPORT_SYMBOL_GPL(coresight_claim_device_unlocked); -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.base);
• rc = coresight_claim_device_unlocked(csdev);
• CS_LOCK(csdev->access.base);

return rc; } @@ -212,11 +220,14 @@ EXPORT_SYMBOL_GPL(coresight_claim_device);

• 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

@@ -227,11 +238,14 @@ void coresight_disclaim_device_unlocked(void __iomem *base) } EXPORT_SYMBOL_GPL(coresight_disclaim_device_unlocked); -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.base);
• coresight_disclaim_device_unlocked(csdev);
• CS_LOCK(csdev->access.base);

} EXPORT_SYMBOL_GPL(coresight_disclaim_device); diff --git a/drivers/hwtracing/coresight/coresight-cti-core.c b/drivers/hwtracing/coresight/coresight-cti-core.c index 3bb0de97d66e..5651d7f61a70 100644 --- a/drivers/hwtracing/coresight/coresight-cti-core.c +++ b/drivers/hwtracing/coresight/coresight-cti-core.c @@ -102,7 +102,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;

@@ -136,7 +136,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); @@ -154,6 +154,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); @@ -171,7 +172,7 @@ static int cti_disable_hw(struct cti_drvdata *drvdata) writel_relaxed(0, drvdata->base + CTICONTROL); config->hw_enabled = false;

• coresight_disclaim_device_unlocked(drvdata->base);

• coresight_disclaim_device_unlocked(csdev); CS_LOCK(drvdata->base); spin_unlock(&drvdata->spinlock); pm_runtime_put(dev);

@@ -655,6 +656,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;

@@ -662,6 +664,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; @@ -673,13 +676,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;

@@ -692,7 +695,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; @@ -736,7 +739,7 @@ static int cti_dying_cpu(unsigned int cpu) spin_lock(&drvdata->spinlock); drvdata->config.hw_powered = false; if (drvdata->config.hw_enabled)

• 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 f994a22876d9..0a416883e012 100644 --- a/drivers/hwtracing/coresight/coresight-etb10.c +++ b/drivers/hwtracing/coresight/coresight-etb10.c @@ -132,7 +132,7 @@ static void __etb_enable_hw(struct etb_drvdata *drvdata) 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; @@ -344,7 +344,7 @@ 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) diff --git a/drivers/hwtracing/coresight/coresight-etm3x-core.c b/drivers/hwtracing/coresight/coresight-etm3x-core.c index 36c5b0ae1b43..70d6d809cb47 100644 --- a/drivers/hwtracing/coresight/coresight-etm3x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm3x-core.c @@ -358,10 +358,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);

• rc = coresight_claim_device_unlocked(drvdata->base);

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

@@ -566,6 +567,7 @@ 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); etm_set_prog(drvdata); @@ -577,7 +579,7 @@ 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); @@ -602,7 +604,7 @@ 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); } diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index 2744aadaa5ec..4f1bcf98fefe 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -115,7 +115,7 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata) etm4_os_unlock(drvdata);

• rc = coresight_claim_device_unlocked(drvdata->base);

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

@@ -523,7 +523,7 @@ 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); diff --git a/drivers/hwtracing/coresight/coresight-funnel.c b/drivers/hwtracing/coresight/coresight-funnel.c index f77466aea26f..2f8675c527bf 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); 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; }

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

• struct coresight_device *csdev = drvdata->csdev;

CS_UNLOCK(drvdata->base); @@ -110,7 +112,7 @@ 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); } diff --git a/drivers/hwtracing/coresight/coresight-replicator.c b/drivers/hwtracing/coresight/coresight-replicator.c index fcf25740116c..e35d79e74e30 100644 --- a/drivers/hwtracing/coresight/coresight-replicator.c +++ b/drivers/hwtracing/coresight/coresight-replicator.c @@ -45,12 +45,18 @@ struct replicator_drvdata { static void dynamic_replicator_reset(struct replicator_drvdata *drvdata) {

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

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

• if (WARN_ON(!csdev))

• return;
  

I don't see a need for this check. Function replicator_reset() is called from probe() where the validity of drvdata->csdev is checked just before.

• CS_UNLOCK(drvdata->base);
• 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); @@ -70,6 +76,7 @@ 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); @@ -84,7 +91,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) { @@ -140,6 +147,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: @@ -160,7 +168,7 @@ static void dynamic_replicator_disable(struct replicator_drvdata *drvdata, if ((readl_relaxed(drvdata->base + REPLICATOR_IDFILTER0) == 0xff) && (readl_relaxed(drvdata->base + REPLICATOR_IDFILTER1) == 0xff))

• coresight_disclaim_device_unlocked(drvdata->base);
  

• coresight_disclaim_device_unlocked(csdev);
  

  CS_LOCK(drvdata->base);

} diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/drivers/hwtracing/coresight/coresight-tmc-etf.c index 44402d413ebb..3a5605eda68f 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-etf.c +++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c @@ -37,7 +37,7 @@ static void __tmc_etb_enable_hw(struct tmc_drvdata *drvdata) 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; @@ -88,7 +88,7 @@ static void __tmc_etb_disable_hw(struct tmc_drvdata *drvdata) 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) @@ -109,7 +109,7 @@ static void __tmc_etf_enable_hw(struct tmc_drvdata *drvdata) 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,11 +120,13 @@ static int tmc_etf_enable_hw(struct tmc_drvdata *drvdata) static void tmc_etf_disable_hw(struct tmc_drvdata *drvdata) {

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

tmc_flush_and_stop(drvdata); tmc_disable_hw(drvdata);

• coresight_disclaim_device_unlocked(drvdata->base);

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

} diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c index 714f9e867e5f..a8b1c49761c7 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c @@ -1038,7 +1038,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);

@@ -1132,7 +1132,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;

} diff --git a/include/linux/coresight.h b/include/linux/coresight.h index cefc8fe239e6..c2d0a2aea45f 100644 --- a/include/linux/coresight.h +++ b/include/linux/coresight.h @@ -474,11 +474,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); @@ -513,18 +513,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 Wed, Oct 28, 2020 at 10:09:31PM +0000, 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. This is a prepartory step to add system register access later where the support is available.

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-core.c | 326 +++++++++--------- .../coresight/coresight-etm4x-sysfs.c | 9 +- drivers/hwtracing/coresight/coresight-etm4x.h | 24 ++ 3 files changed, 187 insertions(+), 172 deletions(-)

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

diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index 4f1bcf98fefe..4af7d45dfe63 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -56,18 +56,28 @@ static u64 etm4_get_access_type(struct etmv4_config *config); static enum cpuhp_state hp_online; -static void etm4_os_unlock(struct etmv4_drvdata *drvdata) +static void etm4_os_unlock_csa(struct etmv4_drvdata *drvdata, struct csdev_access *csa) { /* Writing 0 to TRCOSLAR unlocks the trace registers */

• writel_relaxed(0x0, drvdata->base + TRCOSLAR);

• etm4x_relaxed_write32(csa, 0x0, TRCOSLAR); drvdata->os_unlock = true; isb();

} +static void etm4_os_unlock(struct etmv4_drvdata *drvdata) +{

• if (!WARN_ON(!drvdata->csdev))

• etm4_os_unlock_csa(drvdata, &drvdata->csdev->access);
  

+}

static void etm4_os_lock(struct etmv4_drvdata *drvdata) {

• if (WARN_ON(!drvdata->csdev))

• return;
  

• /* Writing 0x1 to TRCOSLAR locks the trace registers */

• writel_relaxed(0x1, drvdata->base + TRCOSLAR);

• etm4x_relaxed_write32(&drvdata->csdev->access, 0x1, TRCOSLAR); drvdata->os_unlock = false; isb();

} @@ -120,45 +130,39 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata) goto done; /* Disable the trace unit before programming trace registers */

• writel_relaxed(0, drvdata->base + TRCPRGCTLR);

• etm4x_relaxed_write32(csa, 0, TRCPRGCTLR);

/* wait for TRCSTATR.IDLE to go up */ if (coresight_timeout(csa, TRCSTATR, TRCSTATR_IDLE_BIT, 1)) dev_err(etm_dev, "timeout while waiting for Idle Trace Status\n"); if (drvdata->nr_pe)

• writel_relaxed(config->pe_sel, drvdata->base + TRCPROCSELR);
  

• writel_relaxed(config->cfg, drvdata->base + TRCCONFIGR);

• etm4x_relaxed_write32(csa, config->pe_sel, TRCPROCSELR);
  

• etm4x_relaxed_write32(csa, config->cfg, TRCCONFIGR); /* nothing specific implemented */

• writel_relaxed(0x0, drvdata->base + TRCAUXCTLR);
• writel_relaxed(config->eventctrl0, drvdata->base + TRCEVENTCTL0R);
• writel_relaxed(config->eventctrl1, drvdata->base + TRCEVENTCTL1R);
• writel_relaxed(config->stall_ctrl, drvdata->base + TRCSTALLCTLR);
• writel_relaxed(config->ts_ctrl, drvdata->base + TRCTSCTLR);
• writel_relaxed(config->syncfreq, drvdata->base + TRCSYNCPR);
• writel_relaxed(config->ccctlr, drvdata->base + TRCCCCTLR);
• writel_relaxed(config->bb_ctrl, drvdata->base + TRCBBCTLR);
• writel_relaxed(drvdata->trcid, drvdata->base + TRCTRACEIDR);
• writel_relaxed(config->vinst_ctrl, drvdata->base + TRCVICTLR);
• writel_relaxed(config->viiectlr, drvdata->base + TRCVIIECTLR);
• writel_relaxed(config->vissctlr,

•        drvdata->base + TRCVISSCTLR);
  

• etm4x_relaxed_write32(csa, 0x0, TRCAUXCTLR);
• etm4x_relaxed_write32(csa, config->eventctrl0, TRCEVENTCTL0R);
• etm4x_relaxed_write32(csa, config->eventctrl1, TRCEVENTCTL1R);
• etm4x_relaxed_write32(csa, config->stall_ctrl, TRCSTALLCTLR);
• etm4x_relaxed_write32(csa, config->ts_ctrl, TRCTSCTLR);
• etm4x_relaxed_write32(csa, config->syncfreq, TRCSYNCPR);
• etm4x_relaxed_write32(csa, config->ccctlr, TRCCCCTLR);
• etm4x_relaxed_write32(csa, config->bb_ctrl, TRCBBCTLR);
• etm4x_relaxed_write32(csa, drvdata->trcid, TRCTRACEIDR);
• etm4x_relaxed_write32(csa, config->vinst_ctrl, TRCVICTLR);
• etm4x_relaxed_write32(csa, config->viiectlr, TRCVIIECTLR);
• etm4x_relaxed_write32(csa, config->vissctlr, TRCVISSCTLR); if (drvdata->nr_pe_cmp)

• writel_relaxed(config->vipcssctlr,
  

• 	       drvdata->base + TRCVIPCSSCTLR);
  

• etm4x_relaxed_write32(csa, config->vipcssctlr, TRCVIPCSSCTLR);
  

  for (i = 0; i < drvdata->nrseqstate - 1; i++)

• writel_relaxed(config->seq_ctrl[i],
  

• 	       drvdata->base + TRCSEQEVRn(i));
  

• writel_relaxed(config->seq_rst, drvdata->base + TRCSEQRSTEVR);
• writel_relaxed(config->seq_state, drvdata->base + TRCSEQSTR);
• writel_relaxed(config->ext_inp, drvdata->base + TRCEXTINSELR);

• etm4x_relaxed_write32(csa, config->seq_ctrl[i], TRCSEQEVRn(i));
  

• etm4x_relaxed_write32(csa, config->seq_rst, TRCSEQRSTEVR);
• etm4x_relaxed_write32(csa, config->seq_state, TRCSEQSTR);
• etm4x_relaxed_write32(csa, config->ext_inp, TRCEXTINSELR); for (i = 0; i < drvdata->nr_cntr; i++) {

• writel_relaxed(config->cntrldvr[i],
  

• 	       drvdata->base + TRCCNTRLDVRn(i));
  

• writel_relaxed(config->cntr_ctrl[i],
  

• 	       drvdata->base + TRCCNTCTLRn(i));
  

• writel_relaxed(config->cntr_val[i],
  

• 	       drvdata->base + TRCCNTVRn(i));
  

• etm4x_relaxed_write32(csa, config->cntrldvr[i], TRCCNTRLDVRn(i));
  

• etm4x_relaxed_write32(csa, config->cntr_ctrl[i], TRCCNTCTLRn(i));
  

• etm4x_relaxed_write32(csa, config->cntr_val[i], TRCCNTVRn(i));
  

  }

/* @@ -166,52 +170,45 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata) * such start at 2. */ for (i = 2; i < drvdata->nr_resource * 2; i++)

• writel_relaxed(config->res_ctrl[i],
  

• 	       drvdata->base + TRCRSCTLRn(i));
  

• etm4x_relaxed_write32(csa, config->res_ctrl[i], TRCRSCTLRn(i));
  

for (i = 0; i < drvdata->nr_ss_cmp; i++) { /* always clear status bit on restart if using single-shot */ if (config->ss_ctrl[i] || config->ss_pe_cmp[i]) config->ss_status[i] &= ~BIT(31);

• writel_relaxed(config->ss_ctrl[i],
  

• 	       drvdata->base + TRCSSCCRn(i));
  

• writel_relaxed(config->ss_status[i],
  

• 	       drvdata->base + TRCSSCSRn(i));
  

• etm4x_relaxed_write32(csa, config->ss_ctrl[i], TRCSSCCRn(i));
  

• etm4x_relaxed_write32(csa, config->ss_status[i], TRCSSCSRn(i));
  

  if (drvdata->nr_pe)

• 	writel_relaxed(config->ss_pe_cmp[i],
  

• 		       drvdata->base + TRCSSPCICRn(i));
  

• 	etm4x_relaxed_write32(csa, config->ss_pe_cmp[i], TRCSSPCICRn(i));
  

  } for (i = 0; i < drvdata->nr_addr_cmp; i++) {

• writeq_relaxed(config->addr_val[i],
  

• 	       drvdata->base + TRCACVRn(i));
  

• writeq_relaxed(config->addr_acc[i],
  

• 	       drvdata->base + TRCACATRn(i));
  

• etm4x_relaxed_write64(csa, config->addr_val[i], TRCACVRn(i));
  

• etm4x_relaxed_write64(csa, config->addr_acc[i], TRCACATRn(i));
  

  } for (i = 0; i < drvdata->numcidc; i++)

• writeq_relaxed(config->ctxid_pid[i],
  

• 	       drvdata->base + TRCCIDCVRn(i));
  

• writel_relaxed(config->ctxid_mask0, drvdata->base + TRCCIDCCTLR0);

• etm4x_relaxed_write64(csa, config->ctxid_pid[i], TRCCIDCVRn(i));
  

• etm4x_relaxed_write32(csa, config->ctxid_mask0, TRCCIDCCTLR0); if (drvdata->numcidc > 4)

• writel_relaxed(config->ctxid_mask1, drvdata->base + TRCCIDCCTLR1);
  

• etm4x_relaxed_write32(csa, config->ctxid_mask1, TRCCIDCCTLR1);
  

for (i = 0; i < drvdata->numvmidc; i++)

• writeq_relaxed(config->vmid_val[i],
  

• 	       drvdata->base + TRCVMIDCVRn(i));
  

• writel_relaxed(config->vmid_mask0, drvdata->base + TRCVMIDCCTLR0);

• etm4x_relaxed_write64(csa, config->vmid_val[i], TRCVMIDCVRn(i));
  

• etm4x_relaxed_write32(csa, config->vmid_mask0, TRCVMIDCCTLR0); if (drvdata->numvmidc > 4)

• writel_relaxed(config->vmid_mask1, drvdata->base + TRCVMIDCCTLR1);
  

• etm4x_relaxed_write32(csa, config->vmid_mask1, TRCVMIDCCTLR1);
  

if (!drvdata->skip_power_up) {

• u32 trcpdcr = etm4x_relaxed_read32(csa, TRCPDCR);
  

• /*
  • Request to keep the trace unit powered and also
  • emulation of powerdown
  */

• writel_relaxed(readl_relaxed(drvdata->base + TRCPDCR) |
  

• 	       TRCPDCR_PU, drvdata->base + TRCPDCR);
  

• etm4x_relaxed_write32(csa, trcpdcr | TRCPDCR_PU, TRCPDCR);
  

  }

/* Enable the trace unit */

• writel_relaxed(1, drvdata->base + TRCPRGCTLR);

• etm4x_relaxed_write32(csa, 1, TRCPRGCTLR);

/* wait for TRCSTATR.IDLE to go back down to '0' */ if (coresight_timeout(csa, TRCSTATR, TRCSTATR_IDLE_BIT, 0)) @@ -487,12 +484,12 @@ static void etm4_disable_hw(void *info) if (!drvdata->skip_power_up) { /* power can be removed from the trace unit now */

• control = readl_relaxed(drvdata->base + TRCPDCR);
  

• control = etm4x_relaxed_read32(csa, TRCPDCR);
  

  control &= ~TRCPDCR_PU;

• writel_relaxed(control, drvdata->base + TRCPDCR);
  

• etm4x_relaxed_write32(csa, control, TRCPDCR);
  

  }

• control = readl_relaxed(drvdata->base + TRCPRGCTLR);

• control = etm4x_relaxed_read32(csa, TRCPRGCTLR);

/* EN, bit[0] Trace unit enable bit */ control &= ~0x1; @@ -504,7 +501,7 @@ static void etm4_disable_hw(void *info) */ dsb(sy); isb();

• writel_relaxed(control, drvdata->base + TRCPRGCTLR);

• etm4x_relaxed_write32(csa, control, TRCPRGCTLR);

/* wait for TRCSTATR.PMSTABLE to go to '1' */ if (coresight_timeout(csa, TRCSTATR, TRCSTATR_PMSTABLE_BIT, 1)) @@ -514,13 +511,13 @@ static void etm4_disable_hw(void *info) /* read the status of the single shot comparators */ for (i = 0; i < drvdata->nr_ss_cmp; i++) { config->ss_status[i] =

• 	readl_relaxed(drvdata->base + TRCSSCSRn(i));
  

• 	etm4x_relaxed_read32(csa, TRCSSCSRn(i));
  

  }

/* read back the current counter values */ for (i = 0; i < drvdata->nr_cntr; i++) { config->cntr_val[i] =

• 	readl_relaxed(drvdata->base + TRCCNTVRn(i));
  

• 	etm4x_relaxed_read32(csa, TRCCNTVRn(i));
  

  }

coresight_disclaim_device_unlocked(csdev); @@ -549,7 +546,7 @@ static int etm4_disable_perf(struct coresight_device *csdev, * scheduled again. Configuration of the start/stop logic happens in * function etm4_set_event_filters(). */

• control = readl_relaxed(drvdata->base + TRCVICTLR);

• control = etm4x_relaxed_read32(&csdev->access, TRCVICTLR); /* TRCVICTLR::SSSTATUS, bit[9] */ filters->ssstatus = (control & BIT(9));

@@ -629,15 +626,17 @@ static void etm4_init_arch_data(void *info) u32 etmidr4; u32 etmidr5; struct etmv4_drvdata *drvdata = info;

• struct csdev_access tmp_csa = CSDEV_ACCESS_IOMEM(drvdata->base);
• struct csdev_access *csa = &tmp_csa; int i;

/* Make sure all registers are accessible */

• etm4_os_unlock(drvdata);

• etm4_os_unlock_csa(drvdata, csa);

CS_UNLOCK(drvdata->base); /* find all capabilities of the tracing unit */

• etmidr0 = readl_relaxed(drvdata->base + TRCIDR0);

• etmidr0 = etm4x_relaxed_read32(csa, TRCIDR0);

/* INSTP0, bits[2:1] P0 tracing support field */ if (BMVAL(etmidr0, 1, 1) && BMVAL(etmidr0, 2, 2)) @@ -677,7 +676,7 @@ static void etm4_init_arch_data(void *info) drvdata->ts_size = BMVAL(etmidr0, 24, 28); /* base architecture of trace unit */

• etmidr1 = readl_relaxed(drvdata->base + TRCIDR1);

• etmidr1 = etm4x_relaxed_read32(csa, TRCIDR1); /*
  • TRCARCHMIN, bits[7:4] architecture the minor version number
  • TRCARCHMAJ, bits[11:8] architecture major versin number

@@ -686,7 +685,7 @@ static void etm4_init_arch_data(void *info) drvdata->config.arch = drvdata->arch; /* maximum size of resources */

• etmidr2 = readl_relaxed(drvdata->base + TRCIDR2);

• etmidr2 = etm4x_relaxed_read32(csa, TRCIDR2); /* CIDSIZE, bits[9:5] Indicates the Context ID size */ drvdata->ctxid_size = BMVAL(etmidr2, 5, 9); /* VMIDSIZE, bits[14:10] Indicates the VMID size */

@@ -694,7 +693,7 @@ static void etm4_init_arch_data(void *info) /* CCSIZE, bits[28:25] size of the cycle counter in bits minus 12 */ drvdata->ccsize = BMVAL(etmidr2, 25, 28);

• etmidr3 = readl_relaxed(drvdata->base + TRCIDR3);

• etmidr3 = etm4x_relaxed_read32(csa, TRCIDR3); /* CCITMIN, bits[11:0] minimum threshold value that can be programmed */ drvdata->ccitmin = BMVAL(etmidr3, 0, 11); /* EXLEVEL_S, bits[19:16] Secure state instruction tracing */

@@ -744,7 +743,7 @@ static void etm4_init_arch_data(void *info) drvdata->nooverflow = false; /* number of resources trace unit supports */

• etmidr4 = readl_relaxed(drvdata->base + TRCIDR4);

• etmidr4 = etm4x_relaxed_read32(csa, TRCIDR4); /* NUMACPAIRS, bits[0:3] number of addr comparator pairs for tracing */ drvdata->nr_addr_cmp = BMVAL(etmidr4, 0, 3); /* NUMPC, bits[15:12] number of PE comparator inputs for tracing */

@@ -770,14 +769,14 @@ static void etm4_init_arch_data(void *info) drvdata->nr_ss_cmp = BMVAL(etmidr4, 20, 23); for (i = 0; i < drvdata->nr_ss_cmp; i++) { drvdata->config.ss_status[i] =

• 	readl_relaxed(drvdata->base + TRCSSCSRn(i));
  

• 	etm4x_relaxed_read32(csa, TRCSSCSRn(i));
  

  } /* NUMCIDC, bits[27:24] number of Context ID comparators for tracing */ drvdata->numcidc = BMVAL(etmidr4, 24, 27); /* NUMVMIDC, bits[31:28] number of VMID comparators for tracing */ drvdata->numvmidc = BMVAL(etmidr4, 28, 31);

• etmidr5 = readl_relaxed(drvdata->base + TRCIDR5);

• etmidr5 = etm4x_relaxed_read32(csa, TRCIDR5); /* NUMEXTIN, bits[8:0] number of external inputs implemented */ drvdata->nr_ext_inp = BMVAL(etmidr5, 0, 8); /* TRACEIDSIZE, bits[21:16] indicates the trace ID width */

@@ -1196,56 +1195,56 @@ static int etm4_cpu_save(struct etmv4_drvdata *drvdata) state = drvdata->save_state;

• state->trcprgctlr = readl(drvdata->base + TRCPRGCTLR);

• state->trcprgctlr = etm4x_read32(csa, TRCPRGCTLR); if (drvdata->nr_pe)

• state->trcprocselr = readl(drvdata->base + TRCPROCSELR);
  

• state->trcconfigr = readl(drvdata->base + TRCCONFIGR);
• state->trcauxctlr = readl(drvdata->base + TRCAUXCTLR);
• state->trceventctl0r = readl(drvdata->base + TRCEVENTCTL0R);
• state->trceventctl1r = readl(drvdata->base + TRCEVENTCTL1R);
• state->trcstallctlr = readl(drvdata->base + TRCSTALLCTLR);
• state->trctsctlr = readl(drvdata->base + TRCTSCTLR);
• state->trcsyncpr = readl(drvdata->base + TRCSYNCPR);
• state->trcccctlr = readl(drvdata->base + TRCCCCTLR);
• state->trcbbctlr = readl(drvdata->base + TRCBBCTLR);
• state->trctraceidr = readl(drvdata->base + TRCTRACEIDR);
• state->trcqctlr = readl(drvdata->base + TRCQCTLR);
• state->trcvictlr = readl(drvdata->base + TRCVICTLR);
• state->trcviiectlr = readl(drvdata->base + TRCVIIECTLR);
• state->trcvissctlr = readl(drvdata->base + TRCVISSCTLR);

• state->trcprocselr = etm4x_read32(csa, TRCPROCSELR);
  

• state->trcconfigr = etm4x_read32(csa, TRCCONFIGR);
• state->trcauxctlr = etm4x_read32(csa, TRCAUXCTLR);
• state->trceventctl0r = etm4x_read32(csa, TRCEVENTCTL0R);
• state->trceventctl1r = etm4x_read32(csa, TRCEVENTCTL1R);
• state->trcstallctlr = etm4x_read32(csa, TRCSTALLCTLR);
• state->trctsctlr = etm4x_read32(csa, TRCTSCTLR);
• state->trcsyncpr = etm4x_read32(csa, TRCSYNCPR);
• state->trcccctlr = etm4x_read32(csa, TRCCCCTLR);
• state->trcbbctlr = etm4x_read32(csa, TRCBBCTLR);
• state->trctraceidr = etm4x_read32(csa, TRCTRACEIDR);
• state->trcqctlr = etm4x_read32(csa, TRCQCTLR);
• state->trcvictlr = etm4x_read32(csa, TRCVICTLR);
• state->trcviiectlr = etm4x_read32(csa, TRCVIIECTLR);
• state->trcvissctlr = etm4x_read32(csa, TRCVISSCTLR); if (drvdata->nr_pe_cmp)

• state->trcvipcssctlr = readl(drvdata->base + TRCVIPCSSCTLR);
  

• state->trcvdctlr = readl(drvdata->base + TRCVDCTLR);
• state->trcvdsacctlr = readl(drvdata->base + TRCVDSACCTLR);
• state->trcvdarcctlr = readl(drvdata->base + TRCVDARCCTLR);

• state->trcvipcssctlr = etm4x_read32(csa, TRCVIPCSSCTLR);
  

• state->trcvdctlr = etm4x_read32(csa, TRCVDCTLR);
• state->trcvdsacctlr = etm4x_read32(csa, TRCVDSACCTLR);
• state->trcvdarcctlr = etm4x_read32(csa, TRCVDARCCTLR);

for (i = 0; i < drvdata->nrseqstate - 1; i++)

• state->trcseqevr[i] = readl(drvdata->base + TRCSEQEVRn(i));
  

• state->trcseqevr[i] = etm4x_read32(csa, TRCSEQEVRn(i));
  

• state->trcseqrstevr = readl(drvdata->base + TRCSEQRSTEVR);
• state->trcseqstr = readl(drvdata->base + TRCSEQSTR);
• state->trcextinselr = readl(drvdata->base + TRCEXTINSELR);

• state->trcseqrstevr = etm4x_read32(csa, TRCSEQRSTEVR);
• state->trcseqstr = etm4x_read32(csa, TRCSEQSTR);
• state->trcextinselr = etm4x_read32(csa, TRCEXTINSELR);

for (i = 0; i < drvdata->nr_cntr; i++) {

• state->trccntrldvr[i] = readl(drvdata->base + TRCCNTRLDVRn(i));
  

• state->trccntctlr[i] = readl(drvdata->base + TRCCNTCTLRn(i));
  

• state->trccntvr[i] = readl(drvdata->base + TRCCNTVRn(i));
  

• state->trccntrldvr[i] = etm4x_read32(csa, TRCCNTRLDVRn(i));
  

• state->trccntctlr[i] = etm4x_read32(csa, TRCCNTCTLRn(i));
  

• state->trccntvr[i] = etm4x_read32(csa, TRCCNTVRn(i));
  

  }

for (i = 0; i < drvdata->nr_resource * 2; i++)

• state->trcrsctlr[i] = readl(drvdata->base + TRCRSCTLRn(i));
  

• state->trcrsctlr[i] = etm4x_read32(csa, TRCRSCTLRn(i));
  

for (i = 0; i < drvdata->nr_ss_cmp; i++) {

• state->trcssccr[i] = readl(drvdata->base + TRCSSCCRn(i));
  

• state->trcsscsr[i] = readl(drvdata->base + TRCSSCSRn(i));
  

• state->trcssccr[i] = etm4x_read32(csa, TRCSSCCRn(i));
  

• state->trcsscsr[i] = etm4x_read32(csa, TRCSSCSRn(i));
  

  if (drvdata->nr_pe)

• 	state->trcsspcicr[i] = readl(drvdata->base + TRCSSPCICRn(i));
  

• 	state->trcsspcicr[i] = etm4x_read32(csa, TRCSSPCICRn(i));
  

  }

for (i = 0; i < drvdata->nr_addr_cmp * 2; i++) {

• state->trcacvr[i] = readq(drvdata->base + TRCACVRn(i));
  

• state->trcacatr[i] = readq(drvdata->base + TRCACATRn(i));
  

• state->trcacvr[i] = etm4x_read64(csa, TRCACVRn(i));
  

• state->trcacatr[i] = etm4x_read64(csa, TRCACATRn(i));
  

  }

/* @@ -1256,22 +1255,22 @@ static int etm4_cpu_save(struct etmv4_drvdata *drvdata) */ for (i = 0; i < drvdata->numcidc; i++)

• state->trccidcvr[i] = readq(drvdata->base + TRCCIDCVRn(i));
  

• state->trccidcvr[i] = etm4x_read64(csa, TRCCIDCVRn(i));
  

for (i = 0; i < drvdata->numvmidc; i++)

• state->trcvmidcvr[i] = readq(drvdata->base + TRCVMIDCVRn(i));
  

• state->trcvmidcvr[i] = etm4x_read64(csa, TRCVMIDCVRn(i));
  

• state->trccidcctlr0 = readl(drvdata->base + TRCCIDCCTLR0);

• state->trccidcctlr0 = etm4x_read32(csa, TRCCIDCCTLR0); if (drvdata->numcidc > 4)

• state->trccidcctlr1 = readl(drvdata->base + TRCCIDCCTLR1);
  

• state->trccidcctlr1 = etm4x_read32(csa, TRCCIDCCTLR1);
  

• state->trcvmidcctlr0 = readl(drvdata->base + TRCVMIDCCTLR0);

• state->trcvmidcctlr0 = etm4x_read32(csa, TRCVMIDCCTLR0); if (drvdata->numvmidc > 4)

• state->trcvmidcctlr1 = readl(drvdata->base + TRCVMIDCCTLR1);
  

• state->trcvmidcctlr0 = etm4x_read32(csa, TRCVMIDCCTLR1);
  

• state->trcclaimset = readl(drvdata->base + TRCCLAIMCLR);

• state->trcclaimset = etm4x_read32(csa, TRCCLAIMCLR);

• state->trcpdcr = readl(drvdata->base + TRCPDCR);

• state->trcpdcr = etm4x_read32(csa, TRCPDCR);

/* wait for TRCSTATR.IDLE to go up */ if (coresight_timeout(csa, TRCSTATR, TRCSTATR_IDLE_BIT, 1)) { @@ -1289,8 +1288,7 @@ static int etm4_cpu_save(struct etmv4_drvdata *drvdata) * potentially save power on systems that respect the TRCPDCR_PU * despite requesting software to save/restore state. */

• writel_relaxed((state->trcpdcr & ~TRCPDCR_PU),

• 	drvdata->base + TRCPDCR);
  

• etm4x_relaxed_write32(csa, (state->trcpdcr & ~TRCPDCR_PU), TRCPDCR);

out: CS_LOCK(drvdata->base); @@ -1301,92 +1299,82 @@ static void etm4_cpu_restore(struct etmv4_drvdata *drvdata) { int i; struct etmv4_save_state *state = drvdata->save_state;

• struct csdev_access tmp_csa = CSDEV_ACCESS_IOMEM(drvdata->base);
• struct csdev_access *csa = &tmp_csa;

CS_UNLOCK(drvdata->base);

• writel_relaxed(state->trcclaimset, drvdata->base + TRCCLAIMSET);

• etm4x_relaxed_write32(csa, state->trcclaimset, TRCCLAIMSET);

• writel_relaxed(state->trcprgctlr, drvdata->base + TRCPRGCTLR);

• etm4x_relaxed_write32(csa, state->trcprgctlr, TRCPRGCTLR); if (drvdata->nr_pe)

• writel_relaxed(state->trcprocselr, drvdata->base + TRCPROCSELR);
  

• writel_relaxed(state->trcconfigr, drvdata->base + TRCCONFIGR);
• writel_relaxed(state->trcauxctlr, drvdata->base + TRCAUXCTLR);
• writel_relaxed(state->trceventctl0r, drvdata->base + TRCEVENTCTL0R);
• writel_relaxed(state->trceventctl1r, drvdata->base + TRCEVENTCTL1R);
• writel_relaxed(state->trcstallctlr, drvdata->base + TRCSTALLCTLR);
• writel_relaxed(state->trctsctlr, drvdata->base + TRCTSCTLR);
• writel_relaxed(state->trcsyncpr, drvdata->base + TRCSYNCPR);
• writel_relaxed(state->trcccctlr, drvdata->base + TRCCCCTLR);
• writel_relaxed(state->trcbbctlr, drvdata->base + TRCBBCTLR);
• writel_relaxed(state->trctraceidr, drvdata->base + TRCTRACEIDR);
• writel_relaxed(state->trcqctlr, drvdata->base + TRCQCTLR);
• writel_relaxed(state->trcvictlr, drvdata->base + TRCVICTLR);
• writel_relaxed(state->trcviiectlr, drvdata->base + TRCVIIECTLR);
• writel_relaxed(state->trcvissctlr, drvdata->base + TRCVISSCTLR);

• etm4x_relaxed_write32(csa, state->trcprocselr, TRCPROCSELR);
  

• etm4x_relaxed_write32(csa, state->trcconfigr, TRCCONFIGR);
• etm4x_relaxed_write32(csa, state->trcauxctlr, TRCAUXCTLR);
• etm4x_relaxed_write32(csa, state->trceventctl0r, TRCEVENTCTL0R);
• etm4x_relaxed_write32(csa, state->trceventctl1r, TRCEVENTCTL1R);
• etm4x_relaxed_write32(csa, state->trcstallctlr, TRCSTALLCTLR);
• etm4x_relaxed_write32(csa, state->trctsctlr, TRCTSCTLR);
• etm4x_relaxed_write32(csa, state->trcsyncpr, TRCSYNCPR);
• etm4x_relaxed_write32(csa, state->trcccctlr, TRCCCCTLR);
• etm4x_relaxed_write32(csa, state->trcbbctlr, TRCBBCTLR);
• etm4x_relaxed_write32(csa, state->trctraceidr, TRCTRACEIDR);
• etm4x_relaxed_write32(csa, state->trcqctlr, TRCQCTLR);
• etm4x_relaxed_write32(csa, state->trcvictlr, TRCVICTLR);
• etm4x_relaxed_write32(csa, state->trcviiectlr, TRCVIIECTLR);
• etm4x_relaxed_write32(csa, state->trcvissctlr, TRCVISSCTLR); if (drvdata->nr_pe_cmp)

• writel_relaxed(state->trcvipcssctlr, drvdata->base + TRCVIPCSSCTLR);
  

• writel_relaxed(state->trcvdctlr, drvdata->base + TRCVDCTLR);
• writel_relaxed(state->trcvdsacctlr, drvdata->base + TRCVDSACCTLR);
• writel_relaxed(state->trcvdarcctlr, drvdata->base + TRCVDARCCTLR);

• etm4x_relaxed_write32(csa, state->trcvipcssctlr, TRCVIPCSSCTLR);
  

• etm4x_relaxed_write32(csa, state->trcvdctlr, TRCVDCTLR);
• etm4x_relaxed_write32(csa, state->trcvdsacctlr, TRCVDSACCTLR);
• etm4x_relaxed_write32(csa, state->trcvdarcctlr, TRCVDARCCTLR);

for (i = 0; i < drvdata->nrseqstate - 1; i++)

• writel_relaxed(state->trcseqevr[i],
  

• 	       drvdata->base + TRCSEQEVRn(i));
  

• etm4x_relaxed_write32(csa, state->trcseqevr[i], TRCSEQEVRn(i));
  

• writel_relaxed(state->trcseqrstevr, drvdata->base + TRCSEQRSTEVR);
• writel_relaxed(state->trcseqstr, drvdata->base + TRCSEQSTR);
• writel_relaxed(state->trcextinselr, drvdata->base + TRCEXTINSELR);

• etm4x_relaxed_write32(csa, state->trcseqrstevr, TRCSEQRSTEVR);
• etm4x_relaxed_write32(csa, state->trcseqstr, TRCSEQSTR);
• etm4x_relaxed_write32(csa, state->trcextinselr, TRCEXTINSELR);

for (i = 0; i < drvdata->nr_cntr; i++) {

• writel_relaxed(state->trccntrldvr[i],
  

• 	       drvdata->base + TRCCNTRLDVRn(i));
  

• writel_relaxed(state->trccntctlr[i],
  

• 	       drvdata->base + TRCCNTCTLRn(i));
  

• writel_relaxed(state->trccntvr[i],
  

• 	       drvdata->base + TRCCNTVRn(i));
  

• etm4x_relaxed_write32(csa, state->trccntrldvr[i], TRCCNTRLDVRn(i));
  

• etm4x_relaxed_write32(csa, state->trccntctlr[i], TRCCNTCTLRn(i));
  

• etm4x_relaxed_write32(csa, state->trccntvr[i], TRCCNTVRn(i));
  

  }

for (i = 0; i < drvdata->nr_resource * 2; i++)

• writel_relaxed(state->trcrsctlr[i],
  

• 	       drvdata->base + TRCRSCTLRn(i));
  

• etm4x_relaxed_write32(csa, state->trcrsctlr[i], TRCRSCTLRn(i));
  

for (i = 0; i < drvdata->nr_ss_cmp; i++) {

• writel_relaxed(state->trcssccr[i],
  

• 	       drvdata->base + TRCSSCCRn(i));
  

• writel_relaxed(state->trcsscsr[i],
  

• 	       drvdata->base + TRCSSCSRn(i));
  

• etm4x_relaxed_write32(csa, state->trcssccr[i], TRCSSCCRn(i));
  

• etm4x_relaxed_write32(csa, state->trcsscsr[i], TRCSSCSRn(i));
  

  if (drvdata->nr_pe)

• 	writel_relaxed(state->trcsspcicr[i],
  

• 		       drvdata->base + TRCSSPCICRn(i));
  

• 	etm4x_relaxed_write32(csa, state->trcsspcicr[i], TRCSSPCICRn(i));
  

  }

for (i = 0; i < drvdata->nr_addr_cmp * 2; i++) {

• writeq_relaxed(state->trcacvr[i],
  

• 	       drvdata->base + TRCACVRn(i));
  

• writeq_relaxed(state->trcacatr[i],
  

• 	       drvdata->base + TRCACATRn(i));
  

• etm4x_relaxed_write64(csa, state->trcacvr[i], TRCACVRn(i));
  

• etm4x_relaxed_write64(csa, state->trcacatr[i], TRCACATRn(i));
  

  }

for (i = 0; i < drvdata->numcidc; i++)

• writeq_relaxed(state->trccidcvr[i],
  

• 	       drvdata->base + TRCCIDCVRn(i));
  

• etm4x_relaxed_write64(csa, state->trccidcvr[i], TRCCIDCVRn(i));
  

for (i = 0; i < drvdata->numvmidc; i++)

• writeq_relaxed(state->trcvmidcvr[i],
  

• 	       drvdata->base + TRCVMIDCVRn(i));
  

• etm4x_relaxed_write64(csa, state->trcvmidcvr[i], TRCVMIDCVRn(i));
  

• writel_relaxed(state->trccidcctlr0, drvdata->base + TRCCIDCCTLR0);

• etm4x_relaxed_write32(csa, state->trccidcctlr0, TRCCIDCCTLR0); if (drvdata->numcidc > 4)

• writel_relaxed(state->trccidcctlr1, drvdata->base + TRCCIDCCTLR1);
  

• etm4x_relaxed_write32(csa, state->trccidcctlr1, TRCCIDCCTLR1);
  

• writel_relaxed(state->trcvmidcctlr0, drvdata->base + TRCVMIDCCTLR0);

• etm4x_relaxed_write32(csa, state->trcvmidcctlr0, TRCVMIDCCTLR0); if (drvdata->numvmidc > 4)

• writel_relaxed(state->trcvmidcctlr1, drvdata->base + TRCVMIDCCTLR1);
  

• etm4x_relaxed_write32(csa, state->trcvmidcctlr0, TRCVMIDCCTLR1);
  

• writel_relaxed(state->trcclaimset, drvdata->base + TRCCLAIMSET);

• etm4x_relaxed_write32(csa, state->trcclaimset, TRCCLAIMSET);

• writel_relaxed(state->trcpdcr, drvdata->base + TRCPDCR);

• etm4x_relaxed_write32(csa, state->trcpdcr, TRCPDCR);

drvdata->state_needs_restore = false; diff --git a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c index c4781d4e5886..fce9df16bfb5 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c @@ -2319,7 +2319,8 @@ static struct attribute *coresight_etmv4_attrs[] = { }; struct etmv4_reg {

• void __iomem *addr;

• struct coresight_device *csdev;
• u32 offset; u32 data;

}; @@ -2327,7 +2328,7 @@ static void do_smp_cross_read(void *data) { struct etmv4_reg *reg = data;

• reg->data = readl_relaxed(reg->addr);

• reg->data = etm4x_relaxed_read32(&reg->csdev->access, reg->offset);

} static u32 etmv4_cross_read(const struct device *dev, u32 offset) @@ -2335,7 +2336,9 @@ static u32 etmv4_cross_read(const struct device *dev, u32 offset) struct etmv4_drvdata *drvdata = dev_get_drvdata(dev); struct etmv4_reg reg;

• reg.addr = drvdata->base + offset;

• reg.offset = offset;
• reg.csdev = drvdata->csdev;
• /*
  • smp cross call ensures the CPU will be powered up before
  • accessing the ETMv4 trace core registers

diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h index eefc7371c6c4..14e0f29db6b3 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))

/* ETMv4 resources */ #define ETM_MAX_NR_PE 8

#define ETMv4_MAX_CNTR 4

2.24.1

On Wed, Oct 28, 2020 at 10:09:32PM +0000, Suzuki K Poulose wrote:

As we are about define a switch..case table for individual register access by offset for implementing the system instruction support, document the possible set of registers for each group to make it easier to co-relate.

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.h | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-)

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

I'm done for today and will continue tomorrow.

Thanks, Mathieu

diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h index 14e0f29db6b3..510828c73db6 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 */ #define TRCCIDCVRn(n) (0x600 + (n * 8)) #define TRCVMIDCVRn(n) (0x640 + (n * 8))

#define TRCCIDCCTLR0 0x680

2.24.1

On Wed, Oct 28, 2020 at 10:09:33PM +0000, 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

---

.../coresight/coresight-etm4x-core.c | 39 ++ drivers/hwtracing/coresight/coresight-etm4x.h | 348 ++++++++++++++++-- 2 files changed, 365 insertions(+), 22 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index 4af7d45dfe63..90b80982c615 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -56,6 +56,45 @@ static u64 etm4_get_access_type(struct etmv4_config *config); static enum cpuhp_state hp_online; +u64 etm4x_sysreg_read(struct csdev_access *csa,

•       u32 offset,
  

•       bool _relaxed,
  

•       bool _64bit)
  

Please fix the stacking.

+{

• u64 res = 0;
• switch (offset) {
• ETM4x_READ_CASES(res)
• default :

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

• 	 offset);
  

• }
• if (!_relaxed)

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

• return res;

+}

+void etm4x_sysreg_write(struct csdev_access *csa,

• 	u64 val,
  

• 	u32 offset,
  

• 	bool _relaxed,
  

• 	bool _64bit)
  

Here too.

+{

• if (!_relaxed)

• __iowmb();	/* 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 510828c73db6..5cf71b30a652 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x.h +++ b/drivers/hwtracing/coresight/coresight-etm4x.h @@ -125,29 +125,323 @@ #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))

+/*

• • 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)

+#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 read_etm4x_sysreg_const_offset(offset) \

• READ_ETM4x_REG(ETM4x_OFFSET_TO_REG(offset))

+#define write_etm4x_sysreg_const_offset(val, offset) \

• WRITE_ETM4x_REG(val, ETM4x_OFFSET_TO_REG(offset))

+#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)))				\
  

Neat trick - I wonder how you stumbled on that one.

• 	__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)->io_mem ? \

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

•  read_etm4x_sysreg_offset((csa), (offset), false)))
  

Please add an extra new line - otherwise it is very hard to read.

+#define etm4x_relaxed_read64(csa, offset) \

• ((u64)((csa)->io_mem ? \

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

•  read_etm4x_sysreg_offset((csa), (offset), true)))
  

Here too.

+#define etm4x_read32(csa, offset) \

• ({ \

• u32 __val = etm4x_relaxed_read32((csa), (offset));	\
  

• __iormb(__val);						\
  

• __val;							\
  

• })
  

+#define etm4x_read64(csa, offset) \

• ({ \

• u64 __val = etm4x_relaxed_read64((csa), (offset));	\
  

• __iormb(__val);						\
  

• __val;							\
  

• })
  

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

• do { \

• if ((csa)->io_mem)					\
  

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

• else							\
  

• 	write_etm4x_sysreg_offset((csa), (val),	\
  

• 				    (offset), false);	\
  

Why using an if/else statement and above the '?' condition marker? I would really like a uniform approach. Otherwise the reader keeps looking for something subtle when there isn't.

• } while (0)

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

• do { \

• if ((csa)->io_mem)					\
  

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

• else							\
  

• 	write_etm4x_sysreg_offset((csa), (val),	\
  

• 				    (offset), true);	\
  

• } while (0)

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

• do { \

• __iowmb();						\
  

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

• } while (0)

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

• do { \

• __iowmb();						\
  

• 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 @@ -512,4 +806,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);
  

With the above:

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

This patch holds together well. I commend you on rendering something that is quite complex into a manageable implementation. That being said it will impact Mike's complex configuration patchset (or Mike's complex configuration patchset will have an impact on this).

#endif

2.24.1

On Wed, Oct 28, 2020 at 10:09:35PM +0000, Suzuki K Poulose wrote:

The Software lock is not implemented for system instructions based accesses. So, skip the lock register access in such cases.

Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com

.../coresight/coresight-etm4x-core.c | 40 ++++++++++++------- 1 file changed, 25 insertions(+), 15 deletions(-)

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

diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index a5c914b16e59..a12d58a04c5d 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -121,6 +121,21 @@ static void etm4_os_lock(struct etmv4_drvdata *drvdata) isb(); } +static void etm4_cs_lock(struct etmv4_drvdata *drvdata,

• 	 struct csdev_access *csa)
  

+{

• /* Software Lock is only accessible via memory mapped interface */
• if (csa->io_mem)

• CS_LOCK(csa->base);
  

+}

+static void etm4_cs_unlock(struct etmv4_drvdata *drvdata,

• 	 struct csdev_access *csa)
  

+{

• if (csa->io_mem)

• CS_UNLOCK(csa->base);
  

+}

static bool etm4_arch_supported(u8 arch) { /* Mask out the minor version number */ @@ -160,8 +175,7 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata) struct device *etm_dev = &csdev->dev; struct csdev_access *csa = &csdev->access;

• CS_UNLOCK(drvdata->base);

• etm4_cs_unlock(drvdata, csa); etm4_os_unlock(drvdata);

rc = coresight_claim_device_unlocked(csdev); @@ -262,7 +276,7 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata) isb(); done:

• CS_LOCK(drvdata->base);

• etm4_cs_lock(drvdata, csa);

dev_dbg(etm_dev, "cpu: %d enable smp call done: %d\n", drvdata->cpu, rc); @@ -519,7 +533,7 @@ static void etm4_disable_hw(void *info) struct csdev_access *csa = &csdev->access; int i;

• CS_UNLOCK(drvdata->base);

• etm4_cs_unlock(drvdata, csa);

if (!drvdata->skip_power_up) { /* power can be removed from the trace unit now */ @@ -560,8 +574,7 @@ static void etm4_disable_hw(void *info) } coresight_disclaim_device_unlocked(csdev);

• CS_LOCK(drvdata->base);

• etm4_cs_lock(drvdata, csa);

dev_dbg(&drvdata->csdev->dev, "cpu: %d disable smp call done\n", drvdata->cpu); @@ -671,8 +684,7 @@ static void etm4_init_arch_data(void *info) /* Make sure all registers are accessible */ etm4_os_unlock_csa(drvdata, csa);

• CS_UNLOCK(drvdata->base);

• etm4_cs_unlock(drvdata, csa);

/* find all capabilities of the tracing unit */ etmidr0 = etm4x_relaxed_read32(csa, TRCIDR0); @@ -837,7 +849,7 @@ static void etm4_init_arch_data(void *info) 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);

• etm4_cs_lock(drvdata, csa);

} /* Set ELx trace filter access in the TRCVICTLR register */ @@ -1218,8 +1230,7 @@ static int etm4_cpu_save(struct etmv4_drvdata *drvdata) dsb(sy); isb();

• CS_UNLOCK(drvdata->base);

• etm4_cs_unlock(drvdata, csa); /* Lock the OS lock to disable trace and external debugger access */ etm4_os_lock(drvdata);

@@ -1330,7 +1341,7 @@ static int etm4_cpu_save(struct etmv4_drvdata *drvdata) etm4x_relaxed_write32(csa, (state->trcpdcr & ~TRCPDCR_PU), TRCPDCR); out:

• CS_LOCK(drvdata->base);

• etm4_cs_lock(drvdata, csa); return ret;

} @@ -1341,8 +1352,7 @@ static void etm4_cpu_restore(struct etmv4_drvdata *drvdata) struct csdev_access tmp_csa = CSDEV_ACCESS_IOMEM(drvdata->base); struct csdev_access *csa = &tmp_csa;

• CS_UNLOCK(drvdata->base);

• etm4_cs_unlock(drvdata, csa); etm4x_relaxed_write32(csa, state->trcclaimset, TRCCLAIMSET);

etm4x_relaxed_write32(csa, state->trcprgctlr, TRCPRGCTLR); @@ -1426,7 +1436,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);

• etm4_cs_lock(drvdata, csa);

} static int etm4_cpu_pm_notify(struct notifier_block *nb, unsigned long cmd, -- 2.24.1

On Wed, Oct 28, 2020 at 10:09:36PM +0000, Suzuki K Poulose wrote:

We rely on the ETM architecture version to decide whether Secure EL2 is available on the CPU for excluding the level for address comparators and viewinst main control register. We must instead use the TRCDIDR3.EXLEVEL_S field to detect the supported levels.

Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com

drivers/hwtracing/coresight/coresight-etm4x-core.c | 13 +++---------- drivers/hwtracing/coresight/coresight-etm4x.h | 6 ++++-- 2 files changed, 7 insertions(+), 12 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index a12d58a04c5d..6e3f9cb7de3f 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -733,7 +733,6 @@ static void etm4_init_arch_data(void *info) * TRCARCHMAJ, bits[11:8] architecture major versin number */ drvdata->arch = BMVAL(etmidr1, 4, 11);

• drvdata->config.arch = drvdata->arch;

/* maximum size of resources */ etmidr2 = etm4x_relaxed_read32(csa, TRCIDR2); @@ -749,6 +748,7 @@ static void etm4_init_arch_data(void *info) drvdata->ccitmin = BMVAL(etmidr3, 0, 11); /* EXLEVEL_S, bits[19:16] Secure state instruction tracing */ drvdata->s_ex_level = BMVAL(etmidr3, 16, 19);

• drvdata->config.s_ex_level = drvdata->s_ex_level; /* EXLEVEL_NS, bits[23:20] Non-secure state instruction tracing */ drvdata->ns_ex_level = BMVAL(etmidr3, 20, 23);

@@ -920,16 +920,9 @@ static u64 etm4_get_ns_access_type(struct etmv4_config *config) static u64 etm4_get_access_type(struct etmv4_config *config) { u64 access_type = etm4_get_ns_access_type(config);

• u64 s_hyp = (config->arch & 0x0f) >= 0x4 ? ETM_EXLEVEL_S_HYP : 0;

• /*

• * EXLEVEL_S, bits[11:8], don't trace anything happening
  

• * in secure state.
  

• */
  

• access_type |= (ETM_EXLEVEL_S_APP |

• 	ETM_EXLEVEL_S_OS	|
  

• 	s_hyp			|
  

• 	ETM_EXLEVEL_S_MON);
  

• /* All supported secure ELs are excluded */
• access_type |= (u64)config->s_ex_level << TRCACATR_EXLEVEL_SHIFT;

return access_type; } diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h index e7f6b7b16fb7..2ac4ecb0af61 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x.h +++ b/drivers/hwtracing/coresight/coresight-etm4x.h @@ -546,6 +546,8 @@ /* PowerDown Control Register bits */ #define TRCPDCR_PU BIT(3) +#define TRCACATR_EXLEVEL_SHIFT 8

/* secure state access levels - TRCACATRn */ #define ETM_EXLEVEL_S_APP BIT(8) #define ETM_EXLEVEL_S_OS BIT(9) @@ -615,7 +617,7 @@

• @vmid_mask0: VM ID comparator mask for comparator 0-3.
• @vmid_mask1: VM ID comparator mask for comparator 4-7.
• @ext_inp: External input selection.

• • @arch: ETM architecture version (for arch dependent config).

• • @s_ex_level: Secure ELs where tracing is supported.
  */

struct etmv4_config { u32 mode; @@ -659,7 +661,7 @@ struct etmv4_config { u32 vmid_mask0; u32 vmid_mask1; u32 ext_inp;

• u8 arch;

• u8 s_ex_level;

Instead of making s_ex_level redundant I suggest to pass a struct etmv4_drvdata to etm4_get_access_type().

More comments to come tomorrow.

Thanks, Mathieu

}; /** -- 2.24.1

On Mon, Nov 09, 2020 at 09:40:54AM +0000, Suzuki K Poulose wrote:

On 11/5/20 9:55 PM, Mathieu Poirier wrote:

...

On Wed, Oct 28, 2020 at 10:09:36PM +0000, Suzuki K Poulose wrote:

...

We rely on the ETM architecture version to decide whether Secure EL2 is available on the CPU for excluding the level for address comparators and viewinst main control register. We must instead use the TRCDIDR3.EXLEVEL_S field to detect the supported levels.

Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com

drivers/hwtracing/coresight/coresight-etm4x-core.c | 13 +++---------- drivers/hwtracing/coresight/coresight-etm4x.h | 6 ++++-- 2 files changed, 7 insertions(+), 12 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index a12d58a04c5d..6e3f9cb7de3f 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -733,7 +733,6 @@ static void etm4_init_arch_data(void *info) * TRCARCHMAJ, bits[11:8] architecture major versin number */ drvdata->arch = BMVAL(etmidr1, 4, 11);

• drvdata->config.arch = drvdata->arch; /* maximum size of resources */ etmidr2 = etm4x_relaxed_read32(csa, TRCIDR2);

@@ -749,6 +748,7 @@ static void etm4_init_arch_data(void *info) drvdata->ccitmin = BMVAL(etmidr3, 0, 11); /* EXLEVEL_S, bits[19:16] Secure state instruction tracing */ drvdata->s_ex_level = BMVAL(etmidr3, 16, 19);

• drvdata->config.s_ex_level = drvdata->s_ex_level; /* EXLEVEL_NS, bits[23:20] Non-secure state instruction tracing */ drvdata->ns_ex_level = BMVAL(etmidr3, 20, 23);

@@ -920,16 +920,9 @@ static u64 etm4_get_ns_access_type(struct etmv4_config *config) static u64 etm4_get_access_type(struct etmv4_config *config) { u64 access_type = etm4_get_ns_access_type(config);

• u64 s_hyp = (config->arch & 0x0f) >= 0x4 ? ETM_EXLEVEL_S_HYP : 0;
• /*

• * EXLEVEL_S, bits[11:8], don't trace anything happening
  

• * in secure state.
  

• */
  

• access_type |= (ETM_EXLEVEL_S_APP |

• 	ETM_EXLEVEL_S_OS	|
  

• 	s_hyp			|
  

• 	ETM_EXLEVEL_S_MON);
  

• /* All supported secure ELs are excluded */
• access_type |= (u64)config->s_ex_level << TRCACATR_EXLEVEL_SHIFT; return access_type; }

diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h index e7f6b7b16fb7..2ac4ecb0af61 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x.h +++ b/drivers/hwtracing/coresight/coresight-etm4x.h @@ -546,6 +546,8 @@ /* PowerDown Control Register bits */ #define TRCPDCR_PU BIT(3) +#define TRCACATR_EXLEVEL_SHIFT 8

• /* secure state access levels - TRCACATRn */ #define ETM_EXLEVEL_S_APP BIT(8) #define ETM_EXLEVEL_S_OS BIT(9)

@@ -615,7 +617,7 @@

• @vmid_mask0: VM ID comparator mask for comparator 0-3.
• @vmid_mask1: VM ID comparator mask for comparator 4-7.
• @ext_inp: External input selection.

• • @arch: ETM architecture version (for arch dependent config).

• • @s_ex_level: Secure ELs where tracing is supported.
  */ struct etmv4_config { u32 mode;

@@ -659,7 +661,7 @@ struct etmv4_config { u32 vmid_mask0; u32 vmid_mask1; u32 ext_inp;

• u8 arch;

• u8 s_ex_level;

Instead of making s_ex_level redundant I suggest to pass a struct etmv4_drvdata to etm4_get_access_type().

I had given thought about that. But this is called from different places, e.g, etm4_set_comparator_filter() where the drvdata is not available. Thus, making that change is quite invasive, going down to the call stack. Morever, the config->arch was already redundant (also cached in drvdata), mainly due to this reason. We simply replace the arch field with the actual supported mask of the ELs for precise programming.