ARM: Add Cross Trigger Interface driver

The Cross Trigger Interface (CTI) helpers in cti.h include definitions for the Coresight Lock Access Register (LAR) and Lock Status Register (LSR). These registers are already defined in coresight.h and so rather than having multiple definitions, just use the definitions from coresight.h.

Add the following coresight macros ...
- coresight_unlock() --> Unlocks coresight module
- coresight_lock() --> Locks coresight module

Use the above macros for ETB, ETM and CTI. The do-while(0) statement has been removed from the macro as it is not a multi-line macro.

Signed-off-by Jon Hunter <[email protected]>

---
 arch/arm/include/asm/cti.h                |   16 +++-------------
 arch/arm/include/asm/hardware/coresight.h |   16 ++++++++--------
 2 files changed, 11 insertions(+), 21 deletions(-)

arch/arm/include/asm/hardware/coresight.h +4 -5

index 7ecd793..dcd0e66 100644
@@ -141,17 +141,17 @@
 #define ETBFF_TRIGEVT		BIT(9)
 #define ETBFF_TRIGFL		BIT(10)
 
-#define etb_writel(t, v, x) \
-	(__raw_writel((v), (t)->etb_regs + (x)))
+#define etb_writel(t, v, x) (__raw_writel((v), (t)->etb_regs + (x)))
 #define etb_readl(t, x) (__raw_readl((t)->etb_regs + (x)))
 
-#define etm_lock(t) do { etm_writel((t), 0, CSMR_LOCKACCESS); } while (0)
-#define etm_unlock(t) \
-	do { etm_writel((t), UNLOCK_MAGIC, CSMR_LOCKACCESS); } while (0)
+#define etb_lock(t) coresight_lock((t)->etb_regs)
+#define etb_unlock(t) coresight_unlock((t)->etb_regs)
+#define etm_lock(t) coresight_lock((t)->etm_regs)

Signature · 1 line

1.7.10.4

Hi Jon,

Quoted 18 lines · Jon Hunter

The Cross Trigger Interface (CTI) helpers in cti.h include definitions for the Coresight Lock Access Register (LAR) and Lock Status Register (LSR). These registers are already defined in coresight.h and so rather than having multiple definitions, just use the definitions from coresight.h.

Add the following coresight macros ...
- coresight_unlock() --> Unlocks coresight module
- coresight_lock() --> Locks coresight module

Use the above macros for ETB, ETM and CTI. The do-while(0) statement has been removed from the macro as it is not a multi-line macro.

Signed-off-by Jon Hunter <[email protected]>

---
 arch/arm/include/asm/cti.h                |   16 +++-------------
 arch/arm/include/asm/hardware/coresight.h |   16 ++++++++--------
 2 files changed, 11 insertions(+), 21 deletions(-)

[...]

Quoted 28 lines

arch/arm/include/asm/hardware/coresight.h +8 -8

index 7ecd793..dcd0e66 100644
@@ -141,17 +141,17 @@
 #define ETBFF_TRIGEVT		BIT(9)
 #define ETBFF_TRIGFL		BIT(10)
 
-#define etb_writel(t, v, x) \
-	(__raw_writel((v), (t)->etb_regs + (x)))
+#define etb_writel(t, v, x) (__raw_writel((v), (t)->etb_regs + (x)))
 #define etb_readl(t, x) (__raw_readl((t)->etb_regs + (x)))
 
-#define etm_lock(t) do { etm_writel((t), 0, CSMR_LOCKACCESS); } while (0)
-#define etm_unlock(t) \
-	do { etm_writel((t), UNLOCK_MAGIC, CSMR_LOCKACCESS); } while (0)
+#define etb_lock(t) coresight_lock((t)->etb_regs)
+#define etb_unlock(t) coresight_unlock((t)->etb_regs)
+#define etm_lock(t) coresight_lock((t)->etm_regs)
+#define etm_unlock(t) coresight_unlock((t)->etm_regs)
 
-#define etb_lock(t) do { etb_writel((t), 0, CSMR_LOCKACCESS); } while (0)
-#define etb_unlock(t) \
-	do { etb_writel((t), UNLOCK_MAGIC, CSMR_LOCKACCESS); } while (0)
+#define coresight_lock(base) (__raw_writel(0, base + CSMR_LOCKACCESS))
+#define coresight_unlock(base) \
+	(__raw_writel(UNLOCK_MAGIC, base + CSMR_LOCKACCESS))
 

arch/arm/include/asm/hardware/coresight.h line 156

Quoted 1 line

arch/arm/include/asm/hardware/coresight.h +0 -0

 #endif /* __ASM_HARDWARE_CORESIGHT_H */

How about we take this opportunity to divorce the more general coresight bits from the etb specific parts, like you've done for the cti. We could move the ETB stuff out into asm/etb.h (although it might be nice to keep all the coresight device headers in one place... answers on a postcard) and just have the architected coresight functionality in this header.

Quoted 52 lines · Will Deacon

Hi Jon,

Quoted 18 lines · Jon Hunter

The Cross Trigger Interface (CTI) helpers in cti.h include definitions for the Coresight Lock Access Register (LAR) and Lock Status Register (LSR). These registers are already defined in coresight.h and so rather than having multiple definitions, just use the definitions from coresight.h.

Add the following coresight macros ...
- coresight_unlock() --> Unlocks coresight module
- coresight_lock() --> Locks coresight module

Use the above macros for ETB, ETM and CTI. The do-while(0) statement has been removed from the macro as it is not a multi-line macro.

Signed-off-by Jon Hunter <[email protected]>

---
 arch/arm/include/asm/cti.h                |   16 +++-------------
 arch/arm/include/asm/hardware/coresight.h |   16 ++++++++--------
 2 files changed, 11 insertions(+), 21 deletions(-)

[...]

Quoted 28 lines

arch/arm/include/asm/hardware/coresight.h +8 -8

index 7ecd793..dcd0e66 100644
@@ -141,17 +141,17 @@
 #define ETBFF_TRIGEVT               BIT(9)
 #define ETBFF_TRIGFL                BIT(10)
 
-#define etb_writel(t, v, x) \
-    (__raw_writel((v), (t)->etb_regs + (x)))
+#define etb_writel(t, v, x) (__raw_writel((v), (t)->etb_regs + (x)))
 #define etb_readl(t, x) (__raw_readl((t)->etb_regs + (x)))
 
-#define etm_lock(t) do { etm_writel((t), 0, CSMR_LOCKACCESS); } while (0)
-#define etm_unlock(t) \
-    do { etm_writel((t), UNLOCK_MAGIC, CSMR_LOCKACCESS); } while (0)
+#define etb_lock(t) coresight_lock((t)->etb_regs)
+#define etb_unlock(t) coresight_unlock((t)->etb_regs)
+#define etm_lock(t) coresight_lock((t)->etm_regs)
+#define etm_unlock(t) coresight_unlock((t)->etm_regs)
 
-#define etb_lock(t) do { etb_writel((t), 0, CSMR_LOCKACCESS); } while (0)
-#define etb_unlock(t) \
-    do { etb_writel((t), UNLOCK_MAGIC, CSMR_LOCKACCESS); } while (0)
+#define coresight_lock(base) (__raw_writel(0, base + CSMR_LOCKACCESS))
+#define coresight_unlock(base) \
+    (__raw_writel(UNLOCK_MAGIC, base + CSMR_LOCKACCESS))
 

Quoted 7 lines

arch/arm/include/asm/hardware/coresight.h line 156

Quoted 1 line

arch/arm/include/asm/hardware/coresight.h +0 -0

 #endif /* __ASM_HARDWARE_CORESIGHT_H */

How about we take this opportunity to divorce the more general coresight bits from the etb specific parts, like you've done for the cti. We could move the ETB stuff out into asm/etb.h (although it might be nice to keep all the coresight device headers in one place... answers on a postcard) and just have the architected coresight functionality in this header.

Yes I have been wondering about that too. Long term it would be good to find a home in the drivers directory for all these coresight devices too. For now, we could extract the etb/etm parts from coresight.h into etb.h and etm.h, respectively.

Cheers
Jon
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Adds a device-tree binding for the ARM Cross Trigger Interface (CTI). The ARM Cross Trigger Interface provides a way to route events between processor modules. For example, on OMAP4430 we use the CTI module to route PMU events to the GIC interrupt module.

Signed-off-by Jon Hunter <[email protected]>

---
 Documentation/devicetree/bindings/arm/cti.txt |   32 +++++++++++++++++++++++++
 1 file changed, 32 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/cti.txt

Documentation/devicetree/bindings/arm/cti.txt new file +32 -0

new file mode 100644
index 0000000..4a0e2d3
@@ -0,0 +1,32 @@
+* ARM Cross Trigger Interface (CTI)
+
+The ARM Cross Trigger Interface provides a way to route events between
+processor modules. For example, debug events from one processor can be
+broadcasted to other processors. The events that can be routed between
+processors are specific to the device.
+
+Required properties:
+
+- compatible:			Should be "arm,primecell".
+- interrupts:			Interrupt associated with CTI module.
+- reg:				Contains timer register address range (base
+				address and length).
+- arm,cti-name:			A unique name for the CTI module, that will be
+				used when requesting the CTI module instance.
+
+
+Optional properties:
+
+- arm-primecell-periphid:	Primecell peripheral ID associated with CTI
+				module.
+
+
+Example:
+
+cti0: cti at 0x54148000 {
+	compatible = "arm,primecell";
+	interrupts = <0 1 0x4>;
+	reg = <0x54148000 0x1000>;
+	arm,cti-name = "cti0";
+	arm,primecell-periphid = <0x003bb906>;
+};

Quoted 4 lines · Jon Hunter

Adds a device-tree binding for the ARM Cross Trigger Interface (CTI). The ARM Cross Trigger Interface provides a way to route events between processor modules. For example, on OMAP4430 we use the CTI module to route PMU events to the GIC interrupt module.

Do you need to describe the PMU-CTI-GIC connection in DT?

Quoted 6 lines · Rob Herring

Quoted 4 lines · Jon Hunter

Adds a device-tree binding for the ARM Cross Trigger Interface (CTI). The ARM Cross Trigger Interface provides a way to route events between processor modules. For example, on OMAP4430 we use the CTI module to route PMU events to the GIC interrupt module.

Quoted 1 line

Do you need to describe the PMU-CTI-GIC connection in DT?

We definitely could. This is achieved by mapping a trigger-input to a trigger-output. So we could list the trigger outputs and inputs in the binding. For omap4430 we would have ...

arm,cti-trigin = <0 1 2 3 4 5 6>;

arm,cti-trigin-names = "dbgack", "pmuirq", "ptmextout0",

			"ptmextout1", "commtx", "commrx",
			"ptmtrigger";
arm,cti-trigout = <0 1 2 3 4 6 7>;
arm,cti-trigout-names = "edbgreq", "ptmextin0", "ptmextin1",
			"ptmextin2", "ptmextin3","mpuirq",
			"dbgrestart";

So to map the PMU to GIC, we would map the "pmuirq" trigger input to the "mpuirq" trigger output. Then we could setup the mapping by name instead of index.

Adds a device-tree binding for the ARM Cross Trigger Interface (CTI). The ARM Cross Trigger Interface provides a way to route events between processor modules. For example, on OMAP4430 we use the CTI module to route PMU events to the GIC interrupt module.

Signed-off-by Jon Hunter <[email protected]>

---
 Documentation/devicetree/bindings/arm/cti.txt |   32 +++++++++++++++++++++++++
 1 file changed, 32 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/cti.txt

Documentation/devicetree/bindings/arm/cti.txt new file +11 -0

new file mode 100644
index 0000000..4a0e2d3
@@ -0,0 +1,32 @@
+* ARM Cross Trigger Interface (CTI)
+
+The ARM Cross Trigger Interface provides a way to route events between
+processor modules. For example, debug events from one processor can be
+broadcasted to other processors. The events that can be routed between
+processors are specific to the device.
+
+Required properties:
+
+- compatible:			Should be "arm,primecell".
+- interrupts:			Interrupt associated with CTI module.

Quoted 28 lines · Jon Hunter

Adds a device-tree binding for the ARM Cross Trigger Interface (CTI). The ARM Cross Trigger Interface provides a way to route events between processor modules. For example, on OMAP4430 we use the CTI module to route PMU events to the GIC interrupt module.

Signed-off-by Jon Hunter <[email protected]>

---
 Documentation/devicetree/bindings/arm/cti.txt |   32 +++++++++++++++++++++++++
 1 file changed, 32 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/cti.txt

Documentation/devicetree/bindings/arm/cti.txt new file +11 -0

new file mode 100644
index 0000000..4a0e2d3
@@ -0,0 +1,32 @@
+* ARM Cross Trigger Interface (CTI)
+
+The ARM Cross Trigger Interface provides a way to route events between
+processor modules. For example, debug events from one processor can be
+broadcasted to other processors. The events that can be routed between
+processors are specific to the device.
+
+Required properties:
+
+- compatible:			Should be "arm,primecell".
+- interrupts:			Interrupt associated with CTI module.

Documentation/devicetree/bindings/arm/cti.txt line 21

Quoted 10 lines

Documentation/devicetree/bindings/arm/cti.txt new file +10 -0

+- reg:				Contains timer register address range (base
+				address and length).
+- arm,cti-name:			A unique name for the CTI module, that will be
+				used when requesting the CTI module instance.
+
+
+Optional properties:
+
+- arm-primecell-periphid:	Primecell peripheral ID associated with CTI
+				module.

For multi-cluster systems, I wouldn't be surprised to see multiple CTI instances, each with different CPU affinities. Can we include an affinity property following Mark's proposed binding?

http://lists.infradead.org/pipermail/linux-arm-kernel/2012-December/137290.html

Quoted 44 lines · Will Deacon

Quoted 28 lines · Jon Hunter

Adds a device-tree binding for the ARM Cross Trigger Interface (CTI). The ARM Cross Trigger Interface provides a way to route events between processor modules. For example, on OMAP4430 we use the CTI module to route PMU events to the GIC interrupt module.

Signed-off-by Jon Hunter <[email protected]>

---
 Documentation/devicetree/bindings/arm/cti.txt |   32 +++++++++++++++++++++++++
 1 file changed, 32 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/cti.txt

Documentation/devicetree/bindings/arm/cti.txt new file +11 -0

new file mode 100644
index 0000000..4a0e2d3
@@ -0,0 +1,32 @@
+* ARM Cross Trigger Interface (CTI)
+
+The ARM Cross Trigger Interface provides a way to route events between
+processor modules. For example, debug events from one processor can be
+broadcasted to other processors. The events that can be routed between
+processors are specific to the device.
+
+Required properties:
+
+- compatible:			Should be "arm,primecell".
+- interrupts:			Interrupt associated with CTI module.

Documentation/devicetree/bindings/arm/cti.txt line 21

Quoted 10 lines

Documentation/devicetree/bindings/arm/cti.txt new file +10 -0

+- reg:				Contains timer register address range (base
+				address and length).
+- arm,cti-name:			A unique name for the CTI module, that will be
+				used when requesting the CTI module instance.
+
+
+Optional properties:
+
+- arm-primecell-periphid:	Primecell peripheral ID associated with CTI
+				module.

For multi-cluster systems, I wouldn't be surprised to see multiple CTI instances, each with different CPU affinities. Can we include an affinity property following Mark's proposed binding?

Quoted 1 line

http://lists.infradead.org/pipermail/linux-arm-kernel/2012-December/137290.html

Yes I can take a look. Would something like that be applicable to pmu as well or is that unlikely to have different affinities? I am just wondering if there is something that we should implement in general for the various primecell components.

Cheers
Jon

Quoted 48 lines · Jon Hunter

Quoted 45 lines · Will Deacon

Quoted 28 lines · Jon Hunter

Adds a device-tree binding for the ARM Cross Trigger Interface (CTI). The ARM Cross Trigger Interface provides a way to route events between processor modules. For example, on OMAP4430 we use the CTI module to route PMU events to the GIC interrupt module.

Signed-off-by Jon Hunter <[email protected]>

---
 Documentation/devicetree/bindings/arm/cti.txt |   32 +++++++++++++++++++++++++
 1 file changed, 32 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/cti.txt

Documentation/devicetree/bindings/arm/cti.txt new file +11 -0

new file mode 100644
index 0000000..4a0e2d3
@@ -0,0 +1,32 @@
+* ARM Cross Trigger Interface (CTI)
+
+The ARM Cross Trigger Interface provides a way to route events between
+processor modules. For example, debug events from one processor can be
+broadcasted to other processors. The events that can be routed between
+processors are specific to the device.
+
+Required properties:
+
+- compatible:			Should be "arm,primecell".
+- interrupts:			Interrupt associated with CTI module.

Documentation/devicetree/bindings/arm/cti.txt line 21

Quoted 10 lines

Documentation/devicetree/bindings/arm/cti.txt new file +10 -0

+- reg:				Contains timer register address range (base
+				address and length).
+- arm,cti-name:			A unique name for the CTI module, that will be
+				used when requesting the CTI module instance.
+
+
+Optional properties:
+
+- arm-primecell-periphid:	Primecell peripheral ID associated with CTI
+				module.

For multi-cluster systems, I wouldn't be surprised to see multiple CTI instances, each with different CPU affinities. Can we include an affinity property following Mark's proposed binding?

http://lists.infradead.org/pipermail/linux-arm-kernel/2012-December/137290.html

Quoted 4 lines

Yes I can take a look. Would something like that be applicable to pmu as well or is that unlikely to have different affinities? I am just wondering if there is something that we should implement in general for the various primecell components.

Do you mean for describing the PMU's affinity to the perf subsystem or its wiring to the CTI?

It's certainly applicable for the former; I've been working on a series to enable support for the PMUs in both clusters in a A15x2 A7x3 coretile using the binding, and I intend to post a series shortly. I'm not sure about the latter, as I don't have much of an understanding about the CTI.

I'm not sure how many other components have affinity concerns, but the intention is for the binding to be reusable.

Convert the Cross Trigger Interface (CTI) helpers in cti.h into a AMBA bus driver so that we can use device-tree to look-up the hardware specific information such as base address and interrupt number during the device probe. This also add APIs to request, cti_get() and release, cti_put(), a CTI module so that drivers can allocate a module at runtime.

Currently, the driver only supports looking-up the CTI hardware information via device-tree, however, the driver could be extended to support non-device-tree configurations if needed for a particular architecture.

The CTI driver only currently supports CTI modules that have a single CPU interrupt, however, could be extended in the future to support more interrupts if a device requires this.

include/linux/amba/cti.h new file +21 -0

+/**
+ * struct cti - Cross Trigger Interface (CTI) struct
+ *
+ * @node:      Connects CTI instance to list of CTI instances
+ * @dev:       Pointer to device structure
+ * @base:      Mapped virtual address of the CTI module
+ * @name:      Name associated with CTI instance
+ * @irq:       Interrupt associated with CTI instance
+ * @trig_out:  Trigger output associated with interrupt (@irq)
+ * @reserved:  Used to indicate if CTI instance has been allocated
+ * @enabled:   Used to indicate if CTI instance has been enabled
+ */
+struct cti {
+       struct list_head node;
+       struct device *dev;
+       void __iomem *base;
+       const char *name;
+       int irq;
+       int trig_out;
+       bool reserved;
+       bool enabled;

Quoted 7 lines · Jon Hunter

Convert the Cross Trigger Interface (CTI) helpers in cti.h into a AMBA bus driver so that we can use device-tree to look-up the hardware specific information such as base address and interrupt number during the device probe. This also add APIs to request, cti_get() and release, cti_put(), a CTI module so that drivers can allocate a module at runtime.

Quoted 8 lines

Currently, the driver only supports looking-up the CTI hardware information via device-tree, however, the driver could be extended to support non-device-tree configurations if needed for a particular architecture.

The CTI driver only currently supports CTI modules that have a single CPU interrupt, however, could be extended in the future to support more interrupts if a device requires this.

Aha, so elaborating on my earlier comments, we basically want to do the same thing for ETB I reckon. This does raise the question about namespaces though...

Quoted 28 lines

include/linux/amba/cti.h new file +28 -0

+/**
+ * struct cti - Cross Trigger Interface (CTI) struct
+ *
+ * @node:      Connects CTI instance to list of CTI instances
+ * @dev:       Pointer to device structure
+ * @base:      Mapped virtual address of the CTI module
+ * @name:      Name associated with CTI instance
+ * @irq:       Interrupt associated with CTI instance
+ * @trig_out:  Trigger output associated with interrupt (@irq)
+ * @reserved:  Used to indicate if CTI instance has been allocated
+ * @enabled:   Used to indicate if CTI instance has been enabled
+ */
+struct cti {
+       struct list_head node;
+       struct device *dev;
+       void __iomem *base;
+       const char *name;
+       int irq;
+       int trig_out;
+       bool reserved;
+       bool enabled;
+};
+
+#ifdef CONFIG_ARM_AMBA_CTI
+
+int cti_map_trigger(struct cti *cti, int trig_in, int trig_out, int chan);
+int cti_enable(struct cti *cti);
+int cti_disable(struct cti *cti);

include/linux/amba/cti.h line 48

Quoted 3 lines

include/linux/amba/cti.h new file +3 -0

+int cti_irq_ack(struct cti *cti);
+struct cti *cti_get(const char *name);
+void cti_put(struct cti *cti);

I wonder whether we should stick these all into a struct and have a general way to see which coresight devices we have and then retrieve their ops structures (so things like perf can walk a virtual coresight bus containing initialised devices).

It might also help if we decide to describe the
plumbing in the device tree, like Rob suggested.

drivers/amba/cti.c new file +6 -0

+
+/**
+ * cti_irq_ack - acknowledges the CTI trigger output
+ * @cti: CTI instance
+ *
+ * Acknowledges the CTI trigger output by writting to the appropriate

drivers/amba/cti.c new file +19 -0

+	v |= BIT(cti->trig_out);
+	cti_writel(v, cti, CTIINTACK);
+
+	return 0;
+}
+
+
+static int cti_probe(struct amba_device *dev, const struct amba_id *id)
+{
+	struct cti *cti;
+	struct device_node *np = dev->dev.of_node;
+	int rc;
+
+	if (!np) {
+		dev_err(&dev->dev, "device-tree not found!\n");
+		return -ENODEV;
+	}
+
+	cti = devm_kzalloc(&dev->dev, sizeof(struct cti), GFP_KERNEL);

drivers/amba/cti.c new file +29 -0

+
+	if (!dev->irq[0]) {
+		dev_err(&dev->dev, "no CTI interrupt found!\n");
+		return -ENODEV;
+	}
+
+	cti->irq = dev->irq[0];
+	cti->base = of_iomap(np, 0);
+	if (!cti->base) {
+		dev_err(&dev->dev, "unable to map CTI registers!\n");
+		return -ENOMEM;
+	}
+
+	cti->dev = &dev->dev;
+	amba_set_drvdata(dev, cti);
+	list_add_tail(&cti->node, &cti_list);
+
+	/*
+	 * AMBA bus driver has already enabled RPM and incremented
+	 * use-count, so now we can safely decrement the use-count
+	 * and allow the CTI driver to manage RPM for the device.
+	 */
+	pm_runtime_put(&dev->dev);
+
+	dev_info(&dev->dev, "ARM CTI driver");
+
+	return 0;
+}
+

Quoted 11 lines · Jon Hunter

drivers/amba/cti.c new file +11 -0

+
+/**
+ * cti_irq_ack - acknowledges the CTI trigger output
+ * @cti: CTI instance
+ *
+ * Acknowledges the CTI trigger output by writting to the appropriate
+ * bit in the CTI interrupt acknowledge register.
+ */
+int cti_irq_ack(struct cti *cti)
+{
+	u32 v;

drivers/amba/cti.c line 153

Quoted 5 lines

drivers/amba/cti.c new file +5 -0

+
+	if (!cti || !cti->enabled)
+		return -EINVAL;
+
+	v = cti_readl(cti, CTIINTACK);

Just curious if CTIINTACK is a read-write register? This is a read-only for us.

Quoted 12 lines · Pratik Patel

Quoted 11 lines · Jon Hunter

drivers/amba/cti.c new file +11 -0

+
+/**
+ * cti_irq_ack - acknowledges the CTI trigger output
+ * @cti: CTI instance
+ *
+ * Acknowledges the CTI trigger output by writting to the appropriate
+ * bit in the CTI interrupt acknowledge register.
+ */
+int cti_irq_ack(struct cti *cti)
+{
+	u32 v;

Quoted 9 lines

drivers/amba/cti.c line 153

Quoted 5 lines

drivers/amba/cti.c new file +5 -0

+
+	if (!cti || !cti->enabled)
+		return -EINVAL;
+
+	v = cti_readl(cti, CTIINTACK);

Just curious if CTIINTACK is a read-write register? This is a read-only for us.

Mistyped - its a write-only for us

Quoted 6 lines · Jon Hunter

drivers/amba/cti.c new file +6 -0

+
+/**
+ * cti_irq_ack - acknowledges the CTI trigger output
+ * @cti: CTI instance
+ *
+ * Acknowledges the CTI trigger output by writting to the appropriate

drivers/amba/cti.c line 153

Quoted 10 lines

drivers/amba/cti.c new file +10 -0

+ * bit in the CTI interrupt acknowledge register.
+ */
+int cti_irq_ack(struct cti *cti)
+{
+	u32 v;
+
+	if (!cti || !cti->enabled)
+		return -EINVAL;
+
+	v = cti_readl(cti, CTIINTACK);

Just curious if CTIINTACK is a read-write register? This is a read-only for us.

Quoted 24 lines

drivers/amba/cti.c new file +24 -0

+	v |= BIT(cti->trig_out);
+	cti_writel(v, cti, CTIINTACK);
+
+	return 0;
+}
+
+
+static int cti_probe(struct amba_device *dev, const struct amba_id *id)
+{
+	struct cti *cti;
+	struct device_node *np = dev->dev.of_node;
+	int rc;
+
+	if (!np) {
+		dev_err(&dev->dev, "device-tree not found!\n");
+		return -ENODEV;
+	}
+
+	cti = devm_kzalloc(&dev->dev, sizeof(struct cti), GFP_KERNEL);
+	if (!cti) {
+		dev_err(&dev->dev, "memory allocation failed!\n");
+		return -ENOMEM;
+	}
+

drivers/amba/cti.c line 234

Quoted 5 lines

drivers/amba/cti.c new file +5 -0

+	rc = of_property_read_string_index(np, "arm,cti-name", 0, &cti->name);
+	if (rc) {
+		dev_err(&dev->dev, "no name found for CTI!\n");
+		return rc;
+	}

Shouldn't the CTI driver have some kind of clock management that it does for itself?

Quoted 45 lines · Pratik Patel

Quoted 6 lines · Jon Hunter

drivers/amba/cti.c new file +6 -0

+
+/**
+ * cti_irq_ack - acknowledges the CTI trigger output
+ * @cti: CTI instance
+ *
+ * Acknowledges the CTI trigger output by writting to the appropriate

drivers/amba/cti.c line 153

Quoted 10 lines

drivers/amba/cti.c new file +10 -0

+ * bit in the CTI interrupt acknowledge register.
+ */
+int cti_irq_ack(struct cti *cti)
+{
+	u32 v;
+
+	if (!cti || !cti->enabled)
+		return -EINVAL;
+
+	v = cti_readl(cti, CTIINTACK);

Just curious if CTIINTACK is a read-write register? This is a read-only for us.

Quoted 24 lines

drivers/amba/cti.c new file +24 -0

+	v |= BIT(cti->trig_out);
+	cti_writel(v, cti, CTIINTACK);
+
+	return 0;
+}
+
+
+static int cti_probe(struct amba_device *dev, const struct amba_id *id)
+{
+	struct cti *cti;
+	struct device_node *np = dev->dev.of_node;
+	int rc;
+
+	if (!np) {
+		dev_err(&dev->dev, "device-tree not found!\n");
+		return -ENODEV;
+	}
+
+	cti = devm_kzalloc(&dev->dev, sizeof(struct cti), GFP_KERNEL);
+	if (!cti) {
+		dev_err(&dev->dev, "memory allocation failed!\n");
+		return -ENOMEM;
+	}
+

Quoted 8 lines

drivers/amba/cti.c line 234

Quoted 5 lines

drivers/amba/cti.c new file +5 -0

+	rc = of_property_read_string_index(np, "arm,cti-name", 0, &cti->name);
+	if (rc) {
+		dev_err(&dev->dev, "no name found for CTI!\n");
+		return rc;
+	}

Shouldn't the CTI driver have some kind of clock management that it does for itself?

It does by using runtime PM. If you look at the cti_get/put functions, you will see calls to pm_runtime_get/put. These calls will enable the AMBA apb-clock. If you need to enable additional clocks then you could register pm runtime resume/idle call backs to do this.