On Wed, Jul 22, 2020 at 06:20:28PM +0100, Suzuki K Poulose wrote:
We are about to introduce support for sysreg access to ETMv4.4+
component. Since there are generic routines that access the
registers (e.g, CS_LOCK/UNLOCK , claim/disclaim operations, timeout)
and in order to preserve the logic of these operations at a single place
we introduce an abstraction layer for the accesses to a given device.
This will also be helpful in consolidating the sysfs.attribute helpers,
that we define per driver.
Please drop the last sentence, it doesn't add to the current patch.
diff --git a/drivers/hwtracing/coresight/coresight.c b/drivers/hwtracing/coresight/coresight.c
index e9c90f2de34a..38e9c03ab754 100644
--- a/drivers/hwtracing/coresight/coresight.c
+++ b/drivers/hwtracing/coresight/coresight.c
@@ -1387,6 +1387,54 @@ static int __init coresight_init(void)
}
* coresight_release_platform_data: Release references to the devices connected
* to the output port of this device.
@@ -1451,6 +1499,7 @@ struct coresight_device *coresight_register(struct coresight_desc *desc)
csdev->type = desc->type;
csdev->subtype = desc->subtype;
csdev->ops = desc->ops;
+ csdev->access = desc->access;
csdev->orphan = false;
csdev->dev.type = &coresight_dev_type[desc->type];
diff --git a/include/linux/coresight.h b/include/linux/coresight.h
index 58fffdecdbfd..81ac708689f8 100644
--- a/include/linux/coresight.h
+++ b/include/linux/coresight.h
@@ -7,6 +7,7 @@
#define _LINUX_CORESIGHT_H
#include <linux/device.h>
+#include <linux/io.h>
#include <linux/perf_event.h>
#include <linux/sched.h>
@@ -114,6 +115,32 @@ struct coresight_platform_data {
struct coresight_connection *conns;
};
+/**
+ * struct csdev_access - Abstraction of a CoreSight device access.
+ *
+ * @no_iomem : True if the device doesn't have iomem access.
+ * @base : When no_iomem == false, base address of the component
+ * @read : Read from the given "offset" of the given instance.
+ * @write : Write "val" to the given "offset".
+ */
+struct csdev_access {
+ bool no_iomem;
I find the no_iomen to be difficult to understand, especially when prefixed with
'!'. Using "has_iomem" would be a lot more intuitive and would avoid extra
mental gymnastics.
+u32 coresight_relaxed_read32(struct coresight_device *csdev, u32 offset);
+u32 coresight_read32(struct coresight_device *csdev, u32 offset);
+void coresight_write32(struct coresight_device *csdev, u32 val, u32 offset);
+void coresight_relaxed_write32(struct coresight_device *csdev,
+ u32 val,
+ u32 offset);
+
Why are the 64 bit version outside of the #ifdef and the 32 bit within?
+static inline u64 coresight_read64(struct coresight_device *csdev, u32 offset)
+{
+ WARN_ON_ONCE(1);
Not sure about the motivation behind using WARN_ON_ONCE(), and only in the read
functions. I would simply return 0 here. After all if CONFIG_CORESIGHT is not
defined they won't make it very far.
+ return 0;
+}
+
+static inline void coresight_relaxed_write64(struct coresight_device *csdev,
+ u64 val,
+ u32 offset)
+{
+}
+
+static inline void coresight_write64(struct coresight_device *csdev, u64 val, u32 offset)
+{
+}
+
#endif
I will likely come back to this patch once I have reviewed the rest of the set.