[PATCH v2 0/3] coresight: enable debug module

[PATCH v2 0/3] coresight: enable debug module

[Leo Yan]

ARMv8 architecture reference manual (ARM DDI 0487A.k) Chapter H7 "The
Sample-based Profiling Extension" has description for sampling
registers, we can utilize these registers to check program counter
value with combined CPU exception level, secure state, etc. So this is
helpful for CPU lockup bugs, e.g. if one CPU has run into infinite loop
with IRQ disabled; the 'hang' CPU cannot switch context and handle any
interrupt, so it cannot handle SMP call for stack dump, etc.

This patch series is to enable coresight debug module with sample-based
registers and register call back notifier for PCSR register dumping
when panic happens, so we can see below dumping info for panic; and
this patch series has considered the conditions for access permission
for debug registers self, so this can avoid access debug registers when
CPU power domain is off; the driver also try to figure out the CPU is
in secure or non-secure state.

ARM external debug module:
CPU[0]:
 EDPRSR:  0000000b (Power:On DLK:Unlock)
 EDPCSR:  [<ffff00000808eb54>] handle_IPI+0xe4/0x150
 EDCIDSR: 00000000
 EDVIDSR: 90000000 (State:Non-secure Mode:EL1/0 Width:64bits VMID:0)
CPU[1]:
 EDPRSR:  0000000b (Power:On DLK:Unlock)
 EDPCSR:  [<ffff0000087a64c0>] debug_notifier_call+0x108/0x288
 EDCIDSR: 00000000
 EDVIDSR: 90000000 (State:Non-secure Mode:EL1/0 Width:64bits VMID:0)

[...]

This patch series has been verified on 96boards Hikey.

Changes from v1:
* According to Mike Leach suggestion, remove the binding for debug
  module clocks which have been directly provided by CPU clocks.
* According to Mathieu Poirier suggestion, add function
  of_coresight_get_cpu() and some minor refactors for debug module
  driver.

Changes from RFC:
* According to Mike Leach suggestion, added check for EDPRSR to avoid
  lockup; added supporting EDVIDSR and EDCIDSR registers;
* According to Mark Rutland and Mathieu Poirier suggestion, rewrote
  the documentation for DT binding;
* According to Mark and Mathieu suggestion, refined debug driver;


Leo Yan (3):
  coresight: bindings for debug module
  coresight: refactor with function of_coresight_get_cpu
  coresight: add support for debug module

 .../devicetree/bindings/arm/coresight-debug.txt    |  40 +++
 drivers/hwtracing/coresight/Kconfig                |  10 +
 drivers/hwtracing/coresight/Makefile               |   1 +
 drivers/hwtracing/coresight/coresight-debug.c      | 378 +++++++++++++++++++++
 drivers/hwtracing/coresight/of_coresight.c         |  35 +-
 include/linux/coresight.h                          |   2 +
 6 files changed, 454 insertions(+), 12 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/coresight-debug.txt
 create mode 100644 drivers/hwtracing/coresight/coresight-debug.c

-- 
2.7.4

[PATCH v2 1/3] coresight: bindings for debug module

[Leo Yan]

According to ARMv8 architecture reference manual (ARM DDI 0487A.k)
Chapter 'Part H: External debug', the CPU can integrate debug module
and it can support self-hosted debug and external debug. Especially
for supporting self-hosted debug, this means the program can access
the debug module from mmio region; and usually the mmio region is
integrated with coresight.

So add document for binding debug component, includes binding to APB
clock; and also need specify the CPU node which the debug module is
dedicated to specific CPU.

Suggested-by: Mike Leach <mike.leach@linaro.org>
Signed-off-by: Leo Yan <leo.yan@linaro.org>
---
 .../devicetree/bindings/arm/coresight-debug.txt    | 40 ++++++++++++++++++++++
 1 file changed, 40 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/coresight-debug.txt

diff --git a/Documentation/devicetree/bindings/arm/coresight-debug.txt b/Documentation/devicetree/bindings/arm/coresight-debug.txt
new file mode 100644
index 0000000..89820d5
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/coresight-debug.txt
@@ -0,0 +1,40 @@
+* CoreSight Debug Component:
+
+CoreSight debug component are compliant with the ARMv8 architecture reference
+manual (ARM DDI 0487A.k) Chapter 'Part H: External debug'. The external debug
+module is mainly used for two modes: self-hosted debug and external debug, and
+it can be accessed from mmio region from Coresight and eventually the debug
+module connects with CPU for debugging. And the debug module provides
+sample-based profiling extension, which can be used to sample CPU program
+counter, secure state and exception level, etc; usually every CPU has one
+dedicated debug module to be connected.
+
+Required properties:
+
+- compatible : should be
+	     * "arm,coresight-debug", "arm,primecell"; supplemented with
+	       "arm,primecell" as driver is using the AMBA bus interface.
+
+- reg : physical base address and length of the register set.
+
+- clocks : the clock associated to this component.
+
+- clock-names : the name of the clock referenced by the code. Since we are
+                using the AMBA framework, the name of the clock providing
+		the interconnect should be "apb_pclk" and the clock is
+		mandatory. The interface between the debug logic and the
+		processor core is clocked by the internal CPU clock, so it
+		is enabled with CPU clock by default.
+
+- cpu : the cpu phandle the debug module is affined to. When omitted
+	the source is considered to belong to CPU0.
+
+Example:
+
+	debug@f6590000 {
+		compatible = "arm,coresight-debug","arm,primecell";
+		reg = <0 0xf6590000 0 0x1000>;
+		clocks = <&sys_ctrl HI6220_DAPB_CLK>;
+		clock-names = "apb_pclk";
+		cpu = <&cpu0>;
+	};
-- 
2.7.4

[PATCH v2 2/3] coresight: refactor with function of_coresight_get_cpu

[Leo Yan]

This is refactor to add function of_coresight_get_cpu(), so it's used to
retrieve CPU id for coresight component. Finally can use it as a common
function for multiple places.

Suggested-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Signed-off-by: Leo Yan <leo.yan@linaro.org>
---
 drivers/hwtracing/coresight/of_coresight.c | 35 ++++++++++++++++++++----------
 include/linux/coresight.h                  |  2 ++
 2 files changed, 25 insertions(+), 12 deletions(-)

diff --git a/drivers/hwtracing/coresight/of_coresight.c b/drivers/hwtracing/coresight/of_coresight.c
index 629e031..2e7bbe2 100644
--- a/drivers/hwtracing/coresight/of_coresight.c
+++ b/drivers/hwtracing/coresight/of_coresight.c
@@ -101,14 +101,34 @@ static int of_coresight_alloc_memory(struct device *dev,
 	return 0;
 }
 
+int of_coresight_get_cpu(struct device_node *node)
+{
+	int cpu;
+	struct device_node *dn;
+
+	dn = of_parse_phandle(node, "cpu", 0);
+
+	/* Affinity defaults to CPU0 */
+	if (!dn)
+		return 0;
+
+	for_each_possible_cpu(cpu) {
+		if (dn == of_get_cpu_node(cpu, NULL))
+			break;
+	}
+	of_node_put(dn);
+
+	return cpu;
+}
+EXPORT_SYMBOL_GPL(of_coresight_get_cpu);
+
 struct coresight_platform_data *of_get_coresight_platform_data(
 				struct device *dev, struct device_node *node)
 {
-	int i = 0, ret = 0, cpu;
+	int i = 0, ret = 0;
 	struct coresight_platform_data *pdata;
 	struct of_endpoint endpoint, rendpoint;
 	struct device *rdev;
-	struct device_node *dn;
 	struct device_node *ep = NULL;
 	struct device_node *rparent = NULL;
 	struct device_node *rport = NULL;
@@ -175,16 +195,7 @@ struct coresight_platform_data *of_get_coresight_platform_data(
 		} while (ep);
 	}
 
-	/* Affinity defaults to CPU0 */
-	pdata->cpu = 0;
-	dn = of_parse_phandle(node, "cpu", 0);
-	for (cpu = 0; dn && cpu < nr_cpu_ids; cpu++) {
-		if (dn == of_get_cpu_node(cpu, NULL)) {
-			pdata->cpu = cpu;
-			break;
-		}
-	}
-	of_node_put(dn);
+	pdata->cpu = of_coresight_get_cpu(node);
 
 	return pdata;
 }
diff --git a/include/linux/coresight.h b/include/linux/coresight.h
index 2a5982c..6709561 100644
--- a/include/linux/coresight.h
+++ b/include/linux/coresight.h
@@ -263,11 +263,13 @@ static inline int coresight_timeout(void __iomem *addr, u32 offset,
 #endif
 
 #ifdef CONFIG_OF
+extern int of_coresight_get_cpu(struct device_node *node);
 extern struct coresight_platform_data *of_get_coresight_platform_data(
 				struct device *dev, struct device_node *node);
 #else
 static inline struct coresight_platform_data *of_get_coresight_platform_data(
 	struct device *dev, struct device_node *node) { return NULL; }
+static int of_coresight_get_cpu(struct device_node *node) { return 0; }
 #endif
 
 #ifdef CONFIG_PID_NS
-- 
2.7.4

[PATCH v2 3/3] coresight: add support for debug module

[Leo Yan]

Coresight includes debug module and usually the module connects with CPU
debug logic. ARMv8 architecture reference manual (ARM DDI 0487A.k) has
description for related info in "Part H: External Debug".

Chapter H7 "The Sample-based Profiling Extension" introduces several
sampling registers, e.g. we can check program counter value with
combined CPU exception level, secure state, etc. So this is helpful for
analysis CPU lockup scenarios, e.g. if one CPU has run into infinite
loop with IRQ disabled. In this case the CPU cannot switch context and
handle any interrupt (including IPIs), as the result it cannot handle
SMP call for stack dump.

This patch is to enable coresight debug module, so firstly this driver
is to bind apb clock for debug module and this is to ensure the debug
module can be accessed from program or external debugger. And the driver
uses sample-based registers for debug purpose, e.g. when system detects
the CPU lockup and trigger panic, the driver will dump program counter
and combined context registers (EDCIDSR, EDVIDSR); by parsing context
registers so can quickly get to know CPU secure state, exception level,
etc.

Some of the debug module registers are located in CPU power domain, so
in the driver it has checked the power state for CPU before accessing
registers within CPU power domain. For most safe way to use this driver,
it's suggested to disable CPU low power states, this can simply set
"nohlt" in kernel command line.

Signed-off-by: Leo Yan <leo.yan@linaro.org>
---
 drivers/hwtracing/coresight/Kconfig           |  10 +
 drivers/hwtracing/coresight/Makefile          |   1 +
 drivers/hwtracing/coresight/coresight-debug.c | 378 ++++++++++++++++++++++++++
 3 files changed, 389 insertions(+)
 create mode 100644 drivers/hwtracing/coresight/coresight-debug.c

diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig
index 130cb21..2c8883b 100644
--- a/drivers/hwtracing/coresight/Kconfig
+++ b/drivers/hwtracing/coresight/Kconfig
@@ -89,4 +89,14 @@ config CORESIGHT_STM
 	  logging useful software events or data coming from various entities
 	  in the system, possibly running different OSs
 
+config CORESIGHT_DEBUG
+	bool "CoreSight debug driver"
+	depends on ARM || ARM64
+	help
+	  This driver provides support for coresight debugging module. This
+	  is primarily used to dump sample-based profiling registers for
+	  panic. By disable CPU low power states (like "nohlt" in kernel
+	  command line), this is more safe and avoid lockup issue when
+	  access debug module registers.
+
 endif
diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile
index af480d9..d540d45 100644
--- a/drivers/hwtracing/coresight/Makefile
+++ b/drivers/hwtracing/coresight/Makefile
@@ -16,3 +16,4 @@ obj-$(CONFIG_CORESIGHT_SOURCE_ETM4X) += coresight-etm4x.o \
 					coresight-etm4x-sysfs.o
 obj-$(CONFIG_CORESIGHT_QCOM_REPLICATOR) += coresight-replicator-qcom.o
 obj-$(CONFIG_CORESIGHT_STM) += coresight-stm.o
+obj-$(CONFIG_CORESIGHT_DEBUG) += coresight-debug.o
diff --git a/drivers/hwtracing/coresight/coresight-debug.c b/drivers/hwtracing/coresight/coresight-debug.c
new file mode 100644
index 0000000..5214f8e
--- /dev/null
+++ b/drivers/hwtracing/coresight/coresight-debug.c
@@ -0,0 +1,378 @@
+/*
+ * Copyright (c) 2017 Linaro Limited. All rights reserved.
+ *
+ * Author: Leo Yan <leo.yan@linaro.org>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+#include <linux/amba/bus.h>
+#include <linux/coresight.h>
+#include <linux/cpu.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/moduleparam.h>
+#include <linux/slab.h>
+#include <linux/smp.h>
+#include <linux/types.h>
+#include <linux/uaccess.h>
+
+#include "coresight-priv.h"
+
+#define EDPCSR				0x0A0
+#define EDCIDSR				0x0A4
+#define EDVIDSR				0x0A8
+#define EDPCSR_HI			0x0AC
+#define EDOSLAR				0x300
+#define EDPRSR				0x314
+#define EDDEVID1			0xFC4
+#define EDDEVID				0xFC8
+
+#define EDPCSR_PROHIBITED		0xFFFFFFFF
+
+/* bits definition for EDPCSR */
+#ifndef CONFIG_64BIT
+#define EDPCSR_THUMB			BIT(0)
+#define EDPCSR_ARM_INST_MASK		GENMASK(31, 2)
+#define EDPCSR_THUMB_INST_MASK		GENMASK(31, 1)
+#endif
+
+/* bits definition for EDPRSR */
+#define EDPRSR_DLK			BIT(6)
+#define EDPRSR_PU			BIT(0)
+
+/* bits definition for EDVIDSR */
+#define EDVIDSR_NS			BIT(31)
+#define EDVIDSR_E2			BIT(30)
+#define EDVIDSR_E3			BIT(29)
+#define EDVIDSR_HV			BIT(28)
+#define EDVIDSR_VMID			GENMASK(7, 0)
+
+/* bits definition for EDDEVID1 */
+#define EDDEVID1_PCSR_OFFSET_MASK	GENMASK(3, 0)
+#define EDDEVID1_PCSR_OFFSET_INS_SET	(0x0)
+
+/* bits definition for EDDEVID */
+#define EDDEVID_PCSAMPLE_MODE		GENMASK(3, 0)
+#define EDDEVID_IMPL_EDPCSR_EDCIDSR	(0x2)
+#define EDDEVID_IMPL_FULL		(0x3)
+
+struct debug_drvdata {
+	void __iomem	*base;
+	struct device	*dev;
+	int		cpu;
+
+	bool		edpcsr_present;
+	bool		edvidsr_present;
+	bool		pc_has_offset;
+
+	u32		eddevid;
+	u32		eddevid1;
+
+	u32		edpcsr;
+	u32		edpcsr_hi;
+	u32		edprsr;
+	u32		edvidsr;
+	u32		edcidsr;
+};
+
+static DEFINE_PER_CPU(struct debug_drvdata *, debug_drvdata);
+
+static void debug_os_unlock(struct debug_drvdata *drvdata)
+{
+	/* Unlocks the debug registers */
+	writel_relaxed(0x0, drvdata->base + EDOSLAR);
+	wmb();
+}
+
+/*
+ * According to ARM DDI 0487A.k, before access external debug
+ * registers should firstly check the access permission; if any
+ * below condition has been met then cannot access debug
+ * registers to avoid lockup issue:
+ *
+ * - CPU power domain is powered off;
+ * - The OS Double Lock is locked;
+ *
+ * By checking EDPRSR can get to know if meet these conditions.
+ */
+static bool debug_access_permitted(struct debug_drvdata *drvdata)
+{
+	/* CPU is powered off */
+	if (!(drvdata->edprsr & EDPRSR_PU))
+		return false;
+
+	/* The OS Double Lock is locked */
+	if (drvdata->edprsr & EDPRSR_DLK)
+		return false;
+
+	return true;
+}
+
+static void debug_read_regs(struct debug_drvdata *drvdata)
+{
+	drvdata->edprsr = readl_relaxed(drvdata->base + EDPRSR);
+
+	if (!debug_access_permitted(drvdata))
+		return;
+
+	if (!drvdata->edpcsr_present)
+		return;
+
+	CS_UNLOCK(drvdata->base);
+
+	debug_os_unlock(drvdata);
+
+	drvdata->edpcsr = readl_relaxed(drvdata->base + EDPCSR);
+
+	/*
+	 * As described in ARM DDI 0487A.k, if the processing
+	 * element (PE) is in debug state, or sample-based
+	 * profiling is prohibited, EDPCSR reads as 0xFFFFFFFF;
+	 * EDCIDSR, EDVIDSR and EDPCSR_HI registers also become
+	 * UNKNOWN state. So directly bail out for this case.
+	 */
+	if (drvdata->edpcsr == EDPCSR_PROHIBITED) {
+		CS_LOCK(drvdata->base);
+		return;
+	}
+
+	/*
+	 * A read of the EDPCSR normally has the side-effect of
+	 * indirectly writing to EDCIDSR, EDVIDSR and EDPCSR_HI;
+	 * at this point it's safe to read value from them.
+	 */
+	drvdata->edcidsr = readl_relaxed(drvdata->base + EDCIDSR);
+#ifdef CONFIG_64BIT
+	drvdata->edpcsr_hi = readl_relaxed(drvdata->base + EDPCSR_HI);
+#endif
+
+	if (drvdata->edvidsr_present)
+		drvdata->edvidsr = readl_relaxed(drvdata->base + EDVIDSR);
+
+	CS_LOCK(drvdata->base);
+}
+
+#ifndef CONFIG_64BIT
+static bool debug_pc_has_offset(struct debug_drvdata *drvdata)
+{
+	u32 pcsr_offset;
+
+	pcsr_offset = drvdata->eddevid1 & EDDEVID1_PCSR_OFFSET_MASK;
+
+	return (pcsr_offset == EDDEVID1_PCSR_OFFSET_INS_SET);
+}
+
+static unsigned long debug_adjust_pc(struct debug_drvdata *drvdata,
+				     unsigned long pc)
+{
+	unsigned long arm_inst_offset = 0, thumb_inst_offset = 0;
+
+	if (debug_pc_has_offset(drvdata)) {
+		arm_inst_offset = 8;
+		thumb_inst_offset = 4;
+	}
+
+	/* Handle thumb instruction */
+	if (pc & EDPCSR_THUMB) {
+		pc = (pc & EDPCSR_THUMB_INST_MASK) - thumb_inst_offset;
+		return pc;
+	}
+
+	/*
+	 * Handle arm instruction offset, if the arm instruction
+	 * is not 4 byte alignment then it's possible the case
+	 * for implementation defined; keep original value for this
+	 * case and print info for notice.
+	 */
+	if (pc & BIT(1))
+		pr_emerg("Instruction offset is implementation defined\n");
+	else
+		pc = (pc & EDPCSR_ARM_INST_MASK) - arm_inst_offset;
+
+	return pc;
+}
+#endif
+
+static void debug_dump_regs(struct debug_drvdata *drvdata)
+{
+	unsigned long pc;
+
+	pr_emerg("\tEDPRSR:  %08x (Power:%s DLK:%s)\n", drvdata->edprsr,
+		 drvdata->edprsr & EDPRSR_PU ? "On" : "Off",
+		 drvdata->edprsr & EDPRSR_DLK ? "Lock" : "Unlock");
+
+	if (!debug_access_permitted(drvdata) || !drvdata->edpcsr_present) {
+		pr_emerg("No permission to access debug registers!\n");
+		return;
+	}
+
+	if (drvdata->edpcsr == EDPCSR_PROHIBITED) {
+		pr_emerg("CPU is in Debug state or profiling is prohibited!\n");
+		return;
+	}
+
+#ifdef CONFIG_64BIT
+	pc = (unsigned long)drvdata->edpcsr_hi << 32 |
+	     (unsigned long)drvdata->edpcsr;
+#else
+	pc = debug_adjust_pc(drvdata, (unsigned long)drvdata->edpcsr);
+#endif
+
+	pr_emerg("\tEDPCSR:  [<%p>] %pS\n", (void *)pc, (void *)pc);
+	pr_emerg("\tEDCIDSR: %08x\n", drvdata->edcidsr);
+
+	if (!drvdata->edvidsr_present)
+		return;
+
+	pr_emerg("\tEDVIDSR: %08x (State:%s Mode:%s Width:%s VMID:%x)\n",
+		 drvdata->edvidsr,
+		 drvdata->edvidsr & EDVIDSR_NS ? "Non-secure" : "Secure",
+		 drvdata->edvidsr & EDVIDSR_E3 ? "EL3" :
+			(drvdata->edvidsr & EDVIDSR_E2 ? "EL2" : "EL1/0"),
+		 drvdata->edvidsr & EDVIDSR_HV ? "64bits" : "32bits",
+		 drvdata->edvidsr & (u32)EDVIDSR_VMID);
+}
+
+/*
+ * Dump out information on panic.
+ */
+static int debug_notifier_call(struct notifier_block *self,
+			       unsigned long v, void *p)
+{
+	int cpu;
+
+	pr_emerg("ARM external debug module:\n");
+
+	for_each_possible_cpu(cpu) {
+
+		if (!per_cpu(debug_drvdata, cpu))
+			continue;
+
+		pr_emerg("CPU[%d]:\n", per_cpu(debug_drvdata, cpu)->cpu);
+
+		debug_read_regs(per_cpu(debug_drvdata, cpu));
+		debug_dump_regs(per_cpu(debug_drvdata, cpu));
+	}
+
+	return 0;
+}
+
+static struct notifier_block debug_notifier = {
+	.notifier_call = debug_notifier_call,
+};
+
+static void debug_init_arch_data(void *info)
+{
+	struct debug_drvdata *drvdata = info;
+	u32 mode;
+
+	CS_UNLOCK(drvdata->base);
+
+	debug_os_unlock(drvdata);
+
+	/* Read device info */
+	drvdata->eddevid  = readl_relaxed(drvdata->base + EDDEVID);
+	drvdata->eddevid1 = readl_relaxed(drvdata->base + EDDEVID1);
+
+	/* Parse implementation feature */
+	mode = drvdata->eddevid & EDDEVID_PCSAMPLE_MODE;
+	if (mode == EDDEVID_IMPL_FULL) {
+		drvdata->edpcsr_present  = true;
+		drvdata->edvidsr_present = true;
+	} else if (mode == EDDEVID_IMPL_EDPCSR_EDCIDSR) {
+		drvdata->edpcsr_present  = true;
+		drvdata->edvidsr_present = false;
+	} else {
+		drvdata->edpcsr_present  = false;
+		drvdata->edvidsr_present = false;
+	}
+
+	CS_LOCK(drvdata->base);
+}
+
+static int debug_probe(struct amba_device *adev, const struct amba_id *id)
+{
+	void __iomem *base;
+	struct device *dev = &adev->dev;
+	struct debug_drvdata *drvdata;
+	struct resource *res = &adev->res;
+	struct device_node *np = adev->dev.of_node;
+	char buf[32];
+	static int debug_count;
+
+	drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
+	if (!drvdata)
+		return -ENOMEM;
+
+	drvdata->cpu = np ? of_coresight_get_cpu(np) : 0;
+	drvdata->dev = &adev->dev;
+
+	dev_set_drvdata(dev, drvdata);
+
+	/* Validity for the resource is already checked by the AMBA core */
+	base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	drvdata->base = base;
+
+	get_online_cpus();
+	per_cpu(debug_drvdata, drvdata->cpu) = drvdata;
+
+	if (smp_call_function_single(drvdata->cpu,
+				debug_init_arch_data, drvdata, 1))
+		dev_err(dev, "Debug arch init failed\n");
+
+	put_online_cpus();
+
+	if (!debug_count++)
+		atomic_notifier_chain_register(&panic_notifier_list,
+					       &debug_notifier);
+
+	sprintf(buf, (char *)id->data, drvdata->cpu);
+	dev_info(dev, "%s initialized\n", buf);
+	return 0;
+}
+
+static struct amba_id debug_ids[] = {
+	{       /* Debug for Cortex-A53 */
+		.id	= 0x000bbd03,
+		.mask	= 0x000fffff,
+		.data   = "Coresight debug-CPU%d",
+	},
+	{       /* Debug for Cortex-A57 */
+		.id	= 0x000bbd07,
+		.mask	= 0x000fffff,
+		.data   = "Coresight debug-CPU%d",
+	},
+	{       /* Debug for Cortex-A72 */
+		.id	= 0x000bbd08,
+		.mask	= 0x000fffff,
+		.data   = "Coresight debug-CPU%d",
+	},
+	{ 0, 0 },
+};
+
+static struct amba_driver debug_driver = {
+	.drv = {
+		.name   = "coresight-debug",
+		.suppress_bind_attrs = true,
+	},
+	.probe		= debug_probe,
+	.id_table	= debug_ids,
+};
+builtin_amba_driver(debug_driver);
-- 
2.7.4

Re: [PATCH v2 1/3] coresight: bindings for debug module

[Mathieu Poirier]

On Tue, Feb 28, 2017 at 11:06:58PM +0800, Leo Yan wrote:
> According to ARMv8 architecture reference manual (ARM DDI 0487A.k)
> Chapter 'Part H: External debug', the CPU can integrate debug module
> and it can support self-hosted debug and external debug. Especially
> for supporting self-hosted debug, this means the program can access
> the debug module from mmio region; and usually the mmio region is
> integrated with coresight.
> 
> So add document for binding debug component, includes binding to APB
> clock; and also need specify the CPU node which the debug module is
> dedicated to specific CPU.
> 
> Suggested-by: Mike Leach <mike.leach@linaro.org>
> Signed-off-by: Leo Yan <leo.yan@linaro.org>
> ---
>  .../devicetree/bindings/arm/coresight-debug.txt    | 40 ++++++++++++++++++++++
>  1 file changed, 40 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/arm/coresight-debug.txt
> 
> diff --git a/Documentation/devicetree/bindings/arm/coresight-debug.txt b/Documentation/devicetree/bindings/arm/coresight-debug.txt
> new file mode 100644
> index 0000000..89820d5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/coresight-debug.txt
> @@ -0,0 +1,40 @@
> +* CoreSight Debug Component:
> +
> +CoreSight debug component are compliant with the ARMv8 architecture reference
> +manual (ARM DDI 0487A.k) Chapter 'Part H: External debug'. The external debug
> +module is mainly used for two modes: self-hosted debug and external debug, and
> +it can be accessed from mmio region from Coresight and eventually the debug
> +module connects with CPU for debugging. And the debug module provides
> +sample-based profiling extension, which can be used to sample CPU program
> +counter, secure state and exception level, etc; usually every CPU has one
> +dedicated debug module to be connected.
> +
> +Required properties:
> +
> +- compatible : should be
> +	     * "arm,coresight-debug", "arm,primecell"; supplemented with
> +	       "arm,primecell" as driver is using the AMBA bus interface.

             * "arm,coresight-debug"; supplemented with "arm,primecell" since
               this driver is using the AMBA bus interface.

> +
> +- reg : physical base address and length of the register set.
> +
> +- clocks : the clock associated to this component.
> +
> +- clock-names : the name of the clock referenced by the code. Since we are
> +                using the AMBA framework, the name of the clock providing
> +		the interconnect should be "apb_pclk" and the clock is
> +		mandatory. The interface between the debug logic and the
> +		processor core is clocked by the internal CPU clock, so it
> +		is enabled with CPU clock by default.
> +
> +- cpu : the cpu phandle the debug module is affined to. When omitted
> +	the source is considered to belong to CPU0.

s/source/module

With those change:

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

> +
> +Example:
> +
> +	debug@f6590000 {
> +		compatible = "arm,coresight-debug","arm,primecell";
> +		reg = <0 0xf6590000 0 0x1000>;
> +		clocks = <&sys_ctrl HI6220_DAPB_CLK>;
> +		clock-names = "apb_pclk";
> +		cpu = <&cpu0>;
> +	};
> -- 
> 2.7.4
> 

Re: [PATCH v2 2/3] coresight: refactor with function of_coresight_get_cpu

[Mathieu Poirier]

On Tue, Feb 28, 2017 at 11:06:59PM +0800, Leo Yan wrote:
> This is refactor to add function of_coresight_get_cpu(), so it's used to
> retrieve CPU id for coresight component. Finally can use it as a common
> function for multiple places.
> 
> Suggested-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> Signed-off-by: Leo Yan <leo.yan@linaro.org>
> ---
>  drivers/hwtracing/coresight/of_coresight.c | 35 ++++++++++++++++++++----------
>  include/linux/coresight.h                  |  2 ++
>  2 files changed, 25 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/of_coresight.c b/drivers/hwtracing/coresight/of_coresight.c
> index 629e031..2e7bbe2 100644
> --- a/drivers/hwtracing/coresight/of_coresight.c
> +++ b/drivers/hwtracing/coresight/of_coresight.c
> @@ -101,14 +101,34 @@ static int of_coresight_alloc_memory(struct device *dev,
>  	return 0;
>  }
>  
> +int of_coresight_get_cpu(struct device_node *node)
> +{
> +	int cpu;
> +	struct device_node *dn;
> +
> +	dn = of_parse_phandle(node, "cpu", 0);
> +
> +	/* Affinity defaults to CPU0 */
> +	if (!dn)
> +		return 0;
> +
> +	for_each_possible_cpu(cpu) {
> +		if (dn == of_get_cpu_node(cpu, NULL))
> +			break;
> +	}
> +	of_node_put(dn);

In the case where no CPU nodes are found this function will return the number
of the last CPU that was probed instead of '0'.

> +
> +	return cpu;
> +}
> +EXPORT_SYMBOL_GPL(of_coresight_get_cpu);

Any reason you exported this - the fact that is it declared in corsight.h should
be enough.

Thanks,
Mathieu

> +
>  struct coresight_platform_data *of_get_coresight_platform_data(
>  				struct device *dev, struct device_node *node)
>  {
> -	int i = 0, ret = 0, cpu;
> +	int i = 0, ret = 0;
>  	struct coresight_platform_data *pdata;
>  	struct of_endpoint endpoint, rendpoint;
>  	struct device *rdev;
> -	struct device_node *dn;
>  	struct device_node *ep = NULL;
>  	struct device_node *rparent = NULL;
>  	struct device_node *rport = NULL;
> @@ -175,16 +195,7 @@ struct coresight_platform_data *of_get_coresight_platform_data(
>  		} while (ep);
>  	}
>  
> -	/* Affinity defaults to CPU0 */
> -	pdata->cpu = 0;
> -	dn = of_parse_phandle(node, "cpu", 0);
> -	for (cpu = 0; dn && cpu < nr_cpu_ids; cpu++) {
> -		if (dn == of_get_cpu_node(cpu, NULL)) {
> -			pdata->cpu = cpu;
> -			break;
> -		}
> -	}
> -	of_node_put(dn);
> +	pdata->cpu = of_coresight_get_cpu(node);
>  
>  	return pdata;
>  }
> diff --git a/include/linux/coresight.h b/include/linux/coresight.h
> index 2a5982c..6709561 100644
> --- a/include/linux/coresight.h
> +++ b/include/linux/coresight.h
> @@ -263,11 +263,13 @@ static inline int coresight_timeout(void __iomem *addr, u32 offset,
>  #endif
>  
>  #ifdef CONFIG_OF
> +extern int of_coresight_get_cpu(struct device_node *node);
>  extern struct coresight_platform_data *of_get_coresight_platform_data(
>  				struct device *dev, struct device_node *node);
>  #else
>  static inline struct coresight_platform_data *of_get_coresight_platform_data(
>  	struct device *dev, struct device_node *node) { return NULL; }
> +static int of_coresight_get_cpu(struct device_node *node) { return 0; }
>  #endif
>  
>  #ifdef CONFIG_PID_NS
> -- 
> 2.7.4
> 

Re: [PATCH v2 3/3] coresight: add support for debug module

[Mathieu Poirier]

On Tue, Feb 28, 2017 at 11:07:00PM +0800, Leo Yan wrote:
> Coresight includes debug module and usually the module connects with CPU
> debug logic. ARMv8 architecture reference manual (ARM DDI 0487A.k) has
> description for related info in "Part H: External Debug".
> 
> Chapter H7 "The Sample-based Profiling Extension" introduces several
> sampling registers, e.g. we can check program counter value with
> combined CPU exception level, secure state, etc. So this is helpful for
> analysis CPU lockup scenarios, e.g. if one CPU has run into infinite
> loop with IRQ disabled. In this case the CPU cannot switch context and
> handle any interrupt (including IPIs), as the result it cannot handle
> SMP call for stack dump.
> 
> This patch is to enable coresight debug module, so firstly this driver
> is to bind apb clock for debug module and this is to ensure the debug
> module can be accessed from program or external debugger. And the driver
> uses sample-based registers for debug purpose, e.g. when system detects
> the CPU lockup and trigger panic, the driver will dump program counter
> and combined context registers (EDCIDSR, EDVIDSR); by parsing context
> registers so can quickly get to know CPU secure state, exception level,
> etc.
> 
> Some of the debug module registers are located in CPU power domain, so
> in the driver it has checked the power state for CPU before accessing
> registers within CPU power domain. For most safe way to use this driver,
> it's suggested to disable CPU low power states, this can simply set
> "nohlt" in kernel command line.
> 
> Signed-off-by: Leo Yan <leo.yan@linaro.org>
> ---
>  drivers/hwtracing/coresight/Kconfig           |  10 +
>  drivers/hwtracing/coresight/Makefile          |   1 +
>  drivers/hwtracing/coresight/coresight-debug.c | 378 ++++++++++++++++++++++++++
>  3 files changed, 389 insertions(+)
>  create mode 100644 drivers/hwtracing/coresight/coresight-debug.c
> 
> diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig
> index 130cb21..2c8883b 100644
> --- a/drivers/hwtracing/coresight/Kconfig
> +++ b/drivers/hwtracing/coresight/Kconfig
> @@ -89,4 +89,14 @@ config CORESIGHT_STM
>  	  logging useful software events or data coming from various entities
>  	  in the system, possibly running different OSs
>  
> +config CORESIGHT_DEBUG
> +	bool "CoreSight debug driver"
> +	depends on ARM || ARM64
> +	help
> +	  This driver provides support for coresight debugging module. This
> +	  is primarily used to dump sample-based profiling registers for
> +	  panic.

> +       By disable CPU low power states (like "nohlt" in kernel
> +	  command line), this is more safe and avoid lockup issue when
> +	  access debug module registers.

"To avoid lockups when accessing debug module registers, it is safer to disable
CPU low power states (like "nohlt" on the kernel command line) when using this
feature."

> +
>  endif
> diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile
> index af480d9..d540d45 100644
> --- a/drivers/hwtracing/coresight/Makefile
> +++ b/drivers/hwtracing/coresight/Makefile
> @@ -16,3 +16,4 @@ obj-$(CONFIG_CORESIGHT_SOURCE_ETM4X) += coresight-etm4x.o \
>  					coresight-etm4x-sysfs.o
>  obj-$(CONFIG_CORESIGHT_QCOM_REPLICATOR) += coresight-replicator-qcom.o
>  obj-$(CONFIG_CORESIGHT_STM) += coresight-stm.o
> +obj-$(CONFIG_CORESIGHT_DEBUG) += coresight-debug.o
> diff --git a/drivers/hwtracing/coresight/coresight-debug.c b/drivers/hwtracing/coresight/coresight-debug.c
> new file mode 100644
> index 0000000..5214f8e
> --- /dev/null
> +++ b/drivers/hwtracing/coresight/coresight-debug.c
> @@ -0,0 +1,378 @@
> +/*
> + * Copyright (c) 2017 Linaro Limited. All rights reserved.
> + *
> + * Author: Leo Yan <leo.yan@linaro.org>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published by
> + * the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program.  If not, see <http://www.gnu.org/licenses/>.
> + *
> + */
> +#include <linux/amba/bus.h>
> +#include <linux/coresight.h>
> +#include <linux/cpu.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/init.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/moduleparam.h>
> +#include <linux/slab.h>
> +#include <linux/smp.h>
> +#include <linux/types.h>
> +#include <linux/uaccess.h>
> +
> +#include "coresight-priv.h"
> +
> +#define EDPCSR				0x0A0
> +#define EDCIDSR				0x0A4
> +#define EDVIDSR				0x0A8
> +#define EDPCSR_HI			0x0AC
> +#define EDOSLAR				0x300
> +#define EDPRSR				0x314
> +#define EDDEVID1			0xFC4
> +#define EDDEVID				0xFC8
> +
> +#define EDPCSR_PROHIBITED		0xFFFFFFFF
> +
> +/* bits definition for EDPCSR */
> +#ifndef CONFIG_64BIT
> +#define EDPCSR_THUMB			BIT(0)
> +#define EDPCSR_ARM_INST_MASK		GENMASK(31, 2)
> +#define EDPCSR_THUMB_INST_MASK		GENMASK(31, 1)
> +#endif
> +
> +/* bits definition for EDPRSR */
> +#define EDPRSR_DLK			BIT(6)
> +#define EDPRSR_PU			BIT(0)
> +
> +/* bits definition for EDVIDSR */
> +#define EDVIDSR_NS			BIT(31)
> +#define EDVIDSR_E2			BIT(30)
> +#define EDVIDSR_E3			BIT(29)
> +#define EDVIDSR_HV			BIT(28)
> +#define EDVIDSR_VMID			GENMASK(7, 0)
> +
> +/* bits definition for EDDEVID1 */
> +#define EDDEVID1_PCSR_OFFSET_MASK	GENMASK(3, 0)
> +#define EDDEVID1_PCSR_OFFSET_INS_SET	(0x0)
> +
> +/* bits definition for EDDEVID */
> +#define EDDEVID_PCSAMPLE_MODE		GENMASK(3, 0)
> +#define EDDEVID_IMPL_EDPCSR_EDCIDSR	(0x2)
> +#define EDDEVID_IMPL_FULL		(0x3)
> +
> +struct debug_drvdata {
> +	void __iomem	*base;
> +	struct device	*dev;
> +	int		cpu;
> +
> +	bool		edpcsr_present;
> +	bool		edvidsr_present;
> +	bool		pc_has_offset;
> +
> +	u32		eddevid;
> +	u32		eddevid1;
> +
> +	u32		edpcsr;
> +	u32		edpcsr_hi;
> +	u32		edprsr;
> +	u32		edvidsr;
> +	u32		edcidsr;
> +};
> +
> +static DEFINE_PER_CPU(struct debug_drvdata *, debug_drvdata);
> +
> +static void debug_os_unlock(struct debug_drvdata *drvdata)
> +{
> +	/* Unlocks the debug registers */
> +	writel_relaxed(0x0, drvdata->base + EDOSLAR);
> +	wmb();
> +}
> +
> +/*
> + * According to ARM DDI 0487A.k, before access external debug
> + * registers should firstly check the access permission; if any
> + * below condition has been met then cannot access debug
> + * registers to avoid lockup issue:
> + *
> + * - CPU power domain is powered off;
> + * - The OS Double Lock is locked;
> + *
> + * By checking EDPRSR can get to know if meet these conditions.
> + */
> +static bool debug_access_permitted(struct debug_drvdata *drvdata)
> +{
> +	/* CPU is powered off */
> +	if (!(drvdata->edprsr & EDPRSR_PU))
> +		return false;
> +
> +	/* The OS Double Lock is locked */
> +	if (drvdata->edprsr & EDPRSR_DLK)
> +		return false;
> +
> +	return true;
> +}
> +
> +static void debug_read_regs(struct debug_drvdata *drvdata)
> +{
> +	drvdata->edprsr = readl_relaxed(drvdata->base + EDPRSR);
> +
> +	if (!debug_access_permitted(drvdata))
> +		return;
> +
> +	if (!drvdata->edpcsr_present)
> +		return;
> +
> +	CS_UNLOCK(drvdata->base);
> +
> +	debug_os_unlock(drvdata);
> +
> +	drvdata->edpcsr = readl_relaxed(drvdata->base + EDPCSR);
> +
> +	/*
> +	 * As described in ARM DDI 0487A.k, if the processing
> +	 * element (PE) is in debug state, or sample-based
> +	 * profiling is prohibited, EDPCSR reads as 0xFFFFFFFF;
> +	 * EDCIDSR, EDVIDSR and EDPCSR_HI registers also become
> +	 * UNKNOWN state. So directly bail out for this case.
> +	 */
> +	if (drvdata->edpcsr == EDPCSR_PROHIBITED) {
> +		CS_LOCK(drvdata->base);
> +		return;
> +	}
> +
> +	/*
> +	 * A read of the EDPCSR normally has the side-effect of
> +	 * indirectly writing to EDCIDSR, EDVIDSR and EDPCSR_HI;
> +	 * at this point it's safe to read value from them.
> +	 */
> +	drvdata->edcidsr = readl_relaxed(drvdata->base + EDCIDSR);
> +#ifdef CONFIG_64BIT
> +	drvdata->edpcsr_hi = readl_relaxed(drvdata->base + EDPCSR_HI);
> +#endif
> +
> +	if (drvdata->edvidsr_present)
> +		drvdata->edvidsr = readl_relaxed(drvdata->base + EDVIDSR);
> +
> +	CS_LOCK(drvdata->base);
> +}
> +
> +#ifndef CONFIG_64BIT
> +static bool debug_pc_has_offset(struct debug_drvdata *drvdata)
> +{
> +	u32 pcsr_offset;
> +
> +	pcsr_offset = drvdata->eddevid1 & EDDEVID1_PCSR_OFFSET_MASK;
> +
> +	return (pcsr_offset == EDDEVID1_PCSR_OFFSET_INS_SET);
> +}
> +
> +static unsigned long debug_adjust_pc(struct debug_drvdata *drvdata,
> +				     unsigned long pc)
> +{
> +	unsigned long arm_inst_offset = 0, thumb_inst_offset = 0;
> +
> +	if (debug_pc_has_offset(drvdata)) {
> +		arm_inst_offset = 8;
> +		thumb_inst_offset = 4;
> +	}
> +
> +	/* Handle thumb instruction */
> +	if (pc & EDPCSR_THUMB) {
> +		pc = (pc & EDPCSR_THUMB_INST_MASK) - thumb_inst_offset;
> +		return pc;
> +	}
> +
> +	/*
> +	 * Handle arm instruction offset, if the arm instruction
> +	 * is not 4 byte alignment then it's possible the case
> +	 * for implementation defined; keep original value for this
> +	 * case and print info for notice.
> +	 */
> +	if (pc & BIT(1))
> +		pr_emerg("Instruction offset is implementation defined\n");
> +	else
> +		pc = (pc & EDPCSR_ARM_INST_MASK) - arm_inst_offset;
> +
> +	return pc;
> +}
> +#endif
> +
> +static void debug_dump_regs(struct debug_drvdata *drvdata)
> +{
> +	unsigned long pc;
> +
> +	pr_emerg("\tEDPRSR:  %08x (Power:%s DLK:%s)\n", drvdata->edprsr,
> +		 drvdata->edprsr & EDPRSR_PU ? "On" : "Off",
> +		 drvdata->edprsr & EDPRSR_DLK ? "Lock" : "Unlock");
> +
> +	if (!debug_access_permitted(drvdata) || !drvdata->edpcsr_present) {
> +		pr_emerg("No permission to access debug registers!\n");
> +		return;
> +	}
> +
> +	if (drvdata->edpcsr == EDPCSR_PROHIBITED) {
> +		pr_emerg("CPU is in Debug state or profiling is prohibited!\n");
> +		return;
> +	}
> +
> +#ifdef CONFIG_64BIT
> +	pc = (unsigned long)drvdata->edpcsr_hi << 32 |
> +	     (unsigned long)drvdata->edpcsr;
> +#else
> +	pc = debug_adjust_pc(drvdata, (unsigned long)drvdata->edpcsr);
> +#endif
> +
> +	pr_emerg("\tEDPCSR:  [<%p>] %pS\n", (void *)pc, (void *)pc);
> +	pr_emerg("\tEDCIDSR: %08x\n", drvdata->edcidsr);
> +
> +	if (!drvdata->edvidsr_present)
> +		return;
> +
> +	pr_emerg("\tEDVIDSR: %08x (State:%s Mode:%s Width:%s VMID:%x)\n",
> +		 drvdata->edvidsr,
> +		 drvdata->edvidsr & EDVIDSR_NS ? "Non-secure" : "Secure",
> +		 drvdata->edvidsr & EDVIDSR_E3 ? "EL3" :
> +			(drvdata->edvidsr & EDVIDSR_E2 ? "EL2" : "EL1/0"),
> +		 drvdata->edvidsr & EDVIDSR_HV ? "64bits" : "32bits",
> +		 drvdata->edvidsr & (u32)EDVIDSR_VMID);
> +}
> +
> +/*
> + * Dump out information on panic.
> + */
> +static int debug_notifier_call(struct notifier_block *self,
> +			       unsigned long v, void *p)
> +{
> +	int cpu;
> +
> +	pr_emerg("ARM external debug module:\n");
> +
> +	for_each_possible_cpu(cpu) {
> +

Extra line - please remove.

> +		if (!per_cpu(debug_drvdata, cpu))
> +			continue;
> +
> +		pr_emerg("CPU[%d]:\n", per_cpu(debug_drvdata, cpu)->cpu);
> +
> +		debug_read_regs(per_cpu(debug_drvdata, cpu));
> +		debug_dump_regs(per_cpu(debug_drvdata, cpu));
> +	}
> +
> +	return 0;
> +}
> +
> +static struct notifier_block debug_notifier = {
> +	.notifier_call = debug_notifier_call,
> +};
> +
> +static void debug_init_arch_data(void *info)
> +{
> +	struct debug_drvdata *drvdata = info;
> +	u32 mode;
> +
> +	CS_UNLOCK(drvdata->base);
> +
> +	debug_os_unlock(drvdata);
> +
> +	/* Read device info */
> +	drvdata->eddevid  = readl_relaxed(drvdata->base + EDDEVID);
> +	drvdata->eddevid1 = readl_relaxed(drvdata->base + EDDEVID1);
> +
> +	/* Parse implementation feature */
> +	mode = drvdata->eddevid & EDDEVID_PCSAMPLE_MODE;
> +	if (mode == EDDEVID_IMPL_FULL) {
> +		drvdata->edpcsr_present  = true;
> +		drvdata->edvidsr_present = true;
> +	} else if (mode == EDDEVID_IMPL_EDPCSR_EDCIDSR) {
> +		drvdata->edpcsr_present  = true;
> +		drvdata->edvidsr_present = false;
> +	} else {
> +		drvdata->edpcsr_present  = false;
> +		drvdata->edvidsr_present = false;
> +	}
> +
> +	CS_LOCK(drvdata->base);
> +}
> +
> +static int debug_probe(struct amba_device *adev, const struct amba_id *id)
> +{
> +	void __iomem *base;
> +	struct device *dev = &adev->dev;
> +	struct debug_drvdata *drvdata;
> +	struct resource *res = &adev->res;
> +	struct device_node *np = adev->dev.of_node;
> +	char buf[32];
> +	static int debug_count;
> +
> +	drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
> +	if (!drvdata)
> +		return -ENOMEM;
> +
> +	drvdata->cpu = np ? of_coresight_get_cpu(np) : 0;
> +	drvdata->dev = &adev->dev;
> +
> +	dev_set_drvdata(dev, drvdata);
> +
> +	/* Validity for the resource is already checked by the AMBA core */
> +	base = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(base))
> +		return PTR_ERR(base);
> +
> +	drvdata->base = base;
> +
> +	get_online_cpus();
> +	per_cpu(debug_drvdata, drvdata->cpu) = drvdata;
> +
> +	if (smp_call_function_single(drvdata->cpu,
> +				debug_init_arch_data, drvdata, 1))
> +		dev_err(dev, "Debug arch init failed\n");
> +
> +	put_online_cpus();
> +
> +	if (!debug_count++)
> +		atomic_notifier_chain_register(&panic_notifier_list,
> +					       &debug_notifier);
> +
> +	sprintf(buf, (char *)id->data, drvdata->cpu);
> +	dev_info(dev, "%s initialized\n", buf);
> +	return 0;
> +}
> +
> +static struct amba_id debug_ids[] = {
> +	{       /* Debug for Cortex-A53 */
> +		.id	= 0x000bbd03,
> +		.mask	= 0x000fffff,
> +		.data   = "Coresight debug-CPU%d",
> +	},
> +	{       /* Debug for Cortex-A57 */
> +		.id	= 0x000bbd07,
> +		.mask	= 0x000fffff,
> +		.data   = "Coresight debug-CPU%d",
> +	},
> +	{       /* Debug for Cortex-A72 */
> +		.id	= 0x000bbd08,
> +		.mask	= 0x000fffff,
> +		.data   = "Coresight debug-CPU%d",
> +	},
> +	{ 0, 0 },
> +};

What board are you testing this on?  Based on the clock name in the DT example I
suppose it is HiKey.  If so would you mind sending the DT so that I can test
things on my side?

Thanks,
Mathieu

> +
> +static struct amba_driver debug_driver = {
> +	.drv = {
> +		.name   = "coresight-debug",
> +		.suppress_bind_attrs = true,
> +	},
> +	.probe		= debug_probe,
> +	.id_table	= debug_ids,
> +};
> +builtin_amba_driver(debug_driver);
> -- 
> 2.7.4
> 

Re: [PATCH v2 3/3] coresight: add support for debug module

[Leo Yan]

On Wed, Mar 01, 2017 at 10:35:56AM -0700, Mathieu Poirier wrote:
> On Tue, Feb 28, 2017 at 11:07:00PM +0800, Leo Yan wrote:
> > Coresight includes debug module and usually the module connects with CPU
> > debug logic. ARMv8 architecture reference manual (ARM DDI 0487A.k) has
> > description for related info in "Part H: External Debug".
> > 
> > Chapter H7 "The Sample-based Profiling Extension" introduces several
> > sampling registers, e.g. we can check program counter value with
> > combined CPU exception level, secure state, etc. So this is helpful for
> > analysis CPU lockup scenarios, e.g. if one CPU has run into infinite
> > loop with IRQ disabled. In this case the CPU cannot switch context and
> > handle any interrupt (including IPIs), as the result it cannot handle
> > SMP call for stack dump.
> > 
> > This patch is to enable coresight debug module, so firstly this driver
> > is to bind apb clock for debug module and this is to ensure the debug
> > module can be accessed from program or external debugger. And the driver
> > uses sample-based registers for debug purpose, e.g. when system detects
> > the CPU lockup and trigger panic, the driver will dump program counter
> > and combined context registers (EDCIDSR, EDVIDSR); by parsing context
> > registers so can quickly get to know CPU secure state, exception level,
> > etc.
> > 
> > Some of the debug module registers are located in CPU power domain, so
> > in the driver it has checked the power state for CPU before accessing
> > registers within CPU power domain. For most safe way to use this driver,
> > it's suggested to disable CPU low power states, this can simply set
> > "nohlt" in kernel command line.
> > 
> > Signed-off-by: Leo Yan <leo.yan@linaro.org>
> > ---
> >  drivers/hwtracing/coresight/Kconfig           |  10 +
> >  drivers/hwtracing/coresight/Makefile          |   1 +
> >  drivers/hwtracing/coresight/coresight-debug.c | 378 ++++++++++++++++++++++++++
> >  3 files changed, 389 insertions(+)
> >  create mode 100644 drivers/hwtracing/coresight/coresight-debug.c
> > 
> > diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig
> > index 130cb21..2c8883b 100644
> > --- a/drivers/hwtracing/coresight/Kconfig
> > +++ b/drivers/hwtracing/coresight/Kconfig
> > @@ -89,4 +89,14 @@ config CORESIGHT_STM
> >  	  logging useful software events or data coming from various entities
> >  	  in the system, possibly running different OSs
> >  
> > +config CORESIGHT_DEBUG
> > +	bool "CoreSight debug driver"
> > +	depends on ARM || ARM64
> > +	help
> > +	  This driver provides support for coresight debugging module. This
> > +	  is primarily used to dump sample-based profiling registers for
> > +	  panic.
> 
> > +       By disable CPU low power states (like "nohlt" in kernel
> > +	  command line), this is more safe and avoid lockup issue when
> > +	  access debug module registers.
> 
> "To avoid lockups when accessing debug module registers, it is safer to disable
> CPU low power states (like "nohlt" on the kernel command line) when using this
> feature."

Will fix.

> > +
> >  endif
> > diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile
> > index af480d9..d540d45 100644
> > --- a/drivers/hwtracing/coresight/Makefile
> > +++ b/drivers/hwtracing/coresight/Makefile
> > @@ -16,3 +16,4 @@ obj-$(CONFIG_CORESIGHT_SOURCE_ETM4X) += coresight-etm4x.o \
> >  					coresight-etm4x-sysfs.o
> >  obj-$(CONFIG_CORESIGHT_QCOM_REPLICATOR) += coresight-replicator-qcom.o
> >  obj-$(CONFIG_CORESIGHT_STM) += coresight-stm.o
> > +obj-$(CONFIG_CORESIGHT_DEBUG) += coresight-debug.o
> > diff --git a/drivers/hwtracing/coresight/coresight-debug.c b/drivers/hwtracing/coresight/coresight-debug.c
> > new file mode 100644
> > index 0000000..5214f8e
> > --- /dev/null
> > +++ b/drivers/hwtracing/coresight/coresight-debug.c
> > @@ -0,0 +1,378 @@
> > +/*
> > + * Copyright (c) 2017 Linaro Limited. All rights reserved.
> > + *
> > + * Author: Leo Yan <leo.yan@linaro.org>
> > + *
> > + * This program is free software; you can redistribute it and/or modify it
> > + * under the terms of the GNU General Public License version 2 as published by
> > + * the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope that it will be useful, but WITHOUT
> > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> > + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> > + * more details.
> > + *
> > + * You should have received a copy of the GNU General Public License along with
> > + * this program.  If not, see <http://www.gnu.org/licenses/>.
> > + *
> > + */
> > +#include <linux/amba/bus.h>
> > +#include <linux/coresight.h>
> > +#include <linux/cpu.h>
> > +#include <linux/device.h>
> > +#include <linux/err.h>
> > +#include <linux/init.h>
> > +#include <linux/io.h>
> > +#include <linux/kernel.h>
> > +#include <linux/moduleparam.h>
> > +#include <linux/slab.h>
> > +#include <linux/smp.h>
> > +#include <linux/types.h>
> > +#include <linux/uaccess.h>
> > +
> > +#include "coresight-priv.h"
> > +
> > +#define EDPCSR				0x0A0
> > +#define EDCIDSR				0x0A4
> > +#define EDVIDSR				0x0A8
> > +#define EDPCSR_HI			0x0AC
> > +#define EDOSLAR				0x300
> > +#define EDPRSR				0x314
> > +#define EDDEVID1			0xFC4
> > +#define EDDEVID				0xFC8
> > +
> > +#define EDPCSR_PROHIBITED		0xFFFFFFFF
> > +
> > +/* bits definition for EDPCSR */
> > +#ifndef CONFIG_64BIT
> > +#define EDPCSR_THUMB			BIT(0)
> > +#define EDPCSR_ARM_INST_MASK		GENMASK(31, 2)
> > +#define EDPCSR_THUMB_INST_MASK		GENMASK(31, 1)
> > +#endif
> > +
> > +/* bits definition for EDPRSR */
> > +#define EDPRSR_DLK			BIT(6)
> > +#define EDPRSR_PU			BIT(0)
> > +
> > +/* bits definition for EDVIDSR */
> > +#define EDVIDSR_NS			BIT(31)
> > +#define EDVIDSR_E2			BIT(30)
> > +#define EDVIDSR_E3			BIT(29)
> > +#define EDVIDSR_HV			BIT(28)
> > +#define EDVIDSR_VMID			GENMASK(7, 0)
> > +
> > +/* bits definition for EDDEVID1 */
> > +#define EDDEVID1_PCSR_OFFSET_MASK	GENMASK(3, 0)
> > +#define EDDEVID1_PCSR_OFFSET_INS_SET	(0x0)
> > +
> > +/* bits definition for EDDEVID */
> > +#define EDDEVID_PCSAMPLE_MODE		GENMASK(3, 0)
> > +#define EDDEVID_IMPL_EDPCSR_EDCIDSR	(0x2)
> > +#define EDDEVID_IMPL_FULL		(0x3)
> > +
> > +struct debug_drvdata {
> > +	void __iomem	*base;
> > +	struct device	*dev;
> > +	int		cpu;
> > +
> > +	bool		edpcsr_present;
> > +	bool		edvidsr_present;
> > +	bool		pc_has_offset;
> > +
> > +	u32		eddevid;
> > +	u32		eddevid1;
> > +
> > +	u32		edpcsr;
> > +	u32		edpcsr_hi;
> > +	u32		edprsr;
> > +	u32		edvidsr;
> > +	u32		edcidsr;
> > +};
> > +
> > +static DEFINE_PER_CPU(struct debug_drvdata *, debug_drvdata);
> > +
> > +static void debug_os_unlock(struct debug_drvdata *drvdata)
> > +{
> > +	/* Unlocks the debug registers */
> > +	writel_relaxed(0x0, drvdata->base + EDOSLAR);
> > +	wmb();
> > +}
> > +
> > +/*
> > + * According to ARM DDI 0487A.k, before access external debug
> > + * registers should firstly check the access permission; if any
> > + * below condition has been met then cannot access debug
> > + * registers to avoid lockup issue:
> > + *
> > + * - CPU power domain is powered off;
> > + * - The OS Double Lock is locked;
> > + *
> > + * By checking EDPRSR can get to know if meet these conditions.
> > + */
> > +static bool debug_access_permitted(struct debug_drvdata *drvdata)
> > +{
> > +	/* CPU is powered off */
> > +	if (!(drvdata->edprsr & EDPRSR_PU))
> > +		return false;
> > +
> > +	/* The OS Double Lock is locked */
> > +	if (drvdata->edprsr & EDPRSR_DLK)
> > +		return false;
> > +
> > +	return true;
> > +}
> > +
> > +static void debug_read_regs(struct debug_drvdata *drvdata)
> > +{
> > +	drvdata->edprsr = readl_relaxed(drvdata->base + EDPRSR);
> > +
> > +	if (!debug_access_permitted(drvdata))
> > +		return;
> > +
> > +	if (!drvdata->edpcsr_present)
> > +		return;
> > +
> > +	CS_UNLOCK(drvdata->base);
> > +
> > +	debug_os_unlock(drvdata);
> > +
> > +	drvdata->edpcsr = readl_relaxed(drvdata->base + EDPCSR);
> > +
> > +	/*
> > +	 * As described in ARM DDI 0487A.k, if the processing
> > +	 * element (PE) is in debug state, or sample-based
> > +	 * profiling is prohibited, EDPCSR reads as 0xFFFFFFFF;
> > +	 * EDCIDSR, EDVIDSR and EDPCSR_HI registers also become
> > +	 * UNKNOWN state. So directly bail out for this case.
> > +	 */
> > +	if (drvdata->edpcsr == EDPCSR_PROHIBITED) {
> > +		CS_LOCK(drvdata->base);
> > +		return;
> > +	}
> > +
> > +	/*
> > +	 * A read of the EDPCSR normally has the side-effect of
> > +	 * indirectly writing to EDCIDSR, EDVIDSR and EDPCSR_HI;
> > +	 * at this point it's safe to read value from them.
> > +	 */
> > +	drvdata->edcidsr = readl_relaxed(drvdata->base + EDCIDSR);
> > +#ifdef CONFIG_64BIT
> > +	drvdata->edpcsr_hi = readl_relaxed(drvdata->base + EDPCSR_HI);
> > +#endif
> > +
> > +	if (drvdata->edvidsr_present)
> > +		drvdata->edvidsr = readl_relaxed(drvdata->base + EDVIDSR);
> > +
> > +	CS_LOCK(drvdata->base);
> > +}
> > +
> > +#ifndef CONFIG_64BIT
> > +static bool debug_pc_has_offset(struct debug_drvdata *drvdata)
> > +{
> > +	u32 pcsr_offset;
> > +
> > +	pcsr_offset = drvdata->eddevid1 & EDDEVID1_PCSR_OFFSET_MASK;
> > +
> > +	return (pcsr_offset == EDDEVID1_PCSR_OFFSET_INS_SET);
> > +}
> > +
> > +static unsigned long debug_adjust_pc(struct debug_drvdata *drvdata,
> > +				     unsigned long pc)
> > +{
> > +	unsigned long arm_inst_offset = 0, thumb_inst_offset = 0;
> > +
> > +	if (debug_pc_has_offset(drvdata)) {
> > +		arm_inst_offset = 8;
> > +		thumb_inst_offset = 4;
> > +	}
> > +
> > +	/* Handle thumb instruction */
> > +	if (pc & EDPCSR_THUMB) {
> > +		pc = (pc & EDPCSR_THUMB_INST_MASK) - thumb_inst_offset;
> > +		return pc;
> > +	}
> > +
> > +	/*
> > +	 * Handle arm instruction offset, if the arm instruction
> > +	 * is not 4 byte alignment then it's possible the case
> > +	 * for implementation defined; keep original value for this
> > +	 * case and print info for notice.
> > +	 */
> > +	if (pc & BIT(1))
> > +		pr_emerg("Instruction offset is implementation defined\n");
> > +	else
> > +		pc = (pc & EDPCSR_ARM_INST_MASK) - arm_inst_offset;
> > +
> > +	return pc;
> > +}
> > +#endif
> > +
> > +static void debug_dump_regs(struct debug_drvdata *drvdata)
> > +{
> > +	unsigned long pc;
> > +
> > +	pr_emerg("\tEDPRSR:  %08x (Power:%s DLK:%s)\n", drvdata->edprsr,
> > +		 drvdata->edprsr & EDPRSR_PU ? "On" : "Off",
> > +		 drvdata->edprsr & EDPRSR_DLK ? "Lock" : "Unlock");
> > +
> > +	if (!debug_access_permitted(drvdata) || !drvdata->edpcsr_present) {
> > +		pr_emerg("No permission to access debug registers!\n");
> > +		return;
> > +	}
> > +
> > +	if (drvdata->edpcsr == EDPCSR_PROHIBITED) {
> > +		pr_emerg("CPU is in Debug state or profiling is prohibited!\n");
> > +		return;
> > +	}
> > +
> > +#ifdef CONFIG_64BIT
> > +	pc = (unsigned long)drvdata->edpcsr_hi << 32 |
> > +	     (unsigned long)drvdata->edpcsr;
> > +#else
> > +	pc = debug_adjust_pc(drvdata, (unsigned long)drvdata->edpcsr);
> > +#endif
> > +
> > +	pr_emerg("\tEDPCSR:  [<%p>] %pS\n", (void *)pc, (void *)pc);
> > +	pr_emerg("\tEDCIDSR: %08x\n", drvdata->edcidsr);
> > +
> > +	if (!drvdata->edvidsr_present)
> > +		return;
> > +
> > +	pr_emerg("\tEDVIDSR: %08x (State:%s Mode:%s Width:%s VMID:%x)\n",
> > +		 drvdata->edvidsr,
> > +		 drvdata->edvidsr & EDVIDSR_NS ? "Non-secure" : "Secure",
> > +		 drvdata->edvidsr & EDVIDSR_E3 ? "EL3" :
> > +			(drvdata->edvidsr & EDVIDSR_E2 ? "EL2" : "EL1/0"),
> > +		 drvdata->edvidsr & EDVIDSR_HV ? "64bits" : "32bits",
> > +		 drvdata->edvidsr & (u32)EDVIDSR_VMID);
> > +}
> > +
> > +/*
> > + * Dump out information on panic.
> > + */
> > +static int debug_notifier_call(struct notifier_block *self,
> > +			       unsigned long v, void *p)
> > +{
> > +	int cpu;
> > +
> > +	pr_emerg("ARM external debug module:\n");
> > +
> > +	for_each_possible_cpu(cpu) {
> > +
> 
> Extra line - please remove.

Will remove.

> > +		if (!per_cpu(debug_drvdata, cpu))
> > +			continue;
> > +
> > +		pr_emerg("CPU[%d]:\n", per_cpu(debug_drvdata, cpu)->cpu);
> > +
> > +		debug_read_regs(per_cpu(debug_drvdata, cpu));
> > +		debug_dump_regs(per_cpu(debug_drvdata, cpu));
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static struct notifier_block debug_notifier = {
> > +	.notifier_call = debug_notifier_call,
> > +};
> > +
> > +static void debug_init_arch_data(void *info)
> > +{
> > +	struct debug_drvdata *drvdata = info;
> > +	u32 mode;
> > +
> > +	CS_UNLOCK(drvdata->base);
> > +
> > +	debug_os_unlock(drvdata);
> > +
> > +	/* Read device info */
> > +	drvdata->eddevid  = readl_relaxed(drvdata->base + EDDEVID);
> > +	drvdata->eddevid1 = readl_relaxed(drvdata->base + EDDEVID1);
> > +
> > +	/* Parse implementation feature */
> > +	mode = drvdata->eddevid & EDDEVID_PCSAMPLE_MODE;
> > +	if (mode == EDDEVID_IMPL_FULL) {
> > +		drvdata->edpcsr_present  = true;
> > +		drvdata->edvidsr_present = true;
> > +	} else if (mode == EDDEVID_IMPL_EDPCSR_EDCIDSR) {
> > +		drvdata->edpcsr_present  = true;
> > +		drvdata->edvidsr_present = false;
> > +	} else {
> > +		drvdata->edpcsr_present  = false;
> > +		drvdata->edvidsr_present = false;
> > +	}
> > +
> > +	CS_LOCK(drvdata->base);
> > +}
> > +
> > +static int debug_probe(struct amba_device *adev, const struct amba_id *id)
> > +{
> > +	void __iomem *base;
> > +	struct device *dev = &adev->dev;
> > +	struct debug_drvdata *drvdata;
> > +	struct resource *res = &adev->res;
> > +	struct device_node *np = adev->dev.of_node;
> > +	char buf[32];
> > +	static int debug_count;
> > +
> > +	drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
> > +	if (!drvdata)
> > +		return -ENOMEM;
> > +
> > +	drvdata->cpu = np ? of_coresight_get_cpu(np) : 0;
> > +	drvdata->dev = &adev->dev;
> > +
> > +	dev_set_drvdata(dev, drvdata);
> > +
> > +	/* Validity for the resource is already checked by the AMBA core */
> > +	base = devm_ioremap_resource(dev, res);
> > +	if (IS_ERR(base))
> > +		return PTR_ERR(base);
> > +
> > +	drvdata->base = base;
> > +
> > +	get_online_cpus();
> > +	per_cpu(debug_drvdata, drvdata->cpu) = drvdata;
> > +
> > +	if (smp_call_function_single(drvdata->cpu,
> > +				debug_init_arch_data, drvdata, 1))
> > +		dev_err(dev, "Debug arch init failed\n");
> > +
> > +	put_online_cpus();
> > +
> > +	if (!debug_count++)
> > +		atomic_notifier_chain_register(&panic_notifier_list,
> > +					       &debug_notifier);
> > +
> > +	sprintf(buf, (char *)id->data, drvdata->cpu);
> > +	dev_info(dev, "%s initialized\n", buf);
> > +	return 0;
> > +}
> > +
> > +static struct amba_id debug_ids[] = {
> > +	{       /* Debug for Cortex-A53 */
> > +		.id	= 0x000bbd03,
> > +		.mask	= 0x000fffff,
> > +		.data   = "Coresight debug-CPU%d",
> > +	},
> > +	{       /* Debug for Cortex-A57 */
> > +		.id	= 0x000bbd07,
> > +		.mask	= 0x000fffff,
> > +		.data   = "Coresight debug-CPU%d",
> > +	},
> > +	{       /* Debug for Cortex-A72 */
> > +		.id	= 0x000bbd08,
> > +		.mask	= 0x000fffff,
> > +		.data   = "Coresight debug-CPU%d",
> > +	},
> > +	{ 0, 0 },
> > +};
> 
> What board are you testing this on?  Based on the clock name in the DT example I
> suppose it is HiKey.  If so would you mind sending the DT so that I can test
> things on my side?

Yes, I tested on Hikey. Will send a new version and include DT related
patches for your testing.

Thanks,
Leo Yan

> > +
> > +static struct amba_driver debug_driver = {
> > +	.drv = {
> > +		.name   = "coresight-debug",
> > +		.suppress_bind_attrs = true,
> > +	},
> > +	.probe		= debug_probe,
> > +	.id_table	= debug_ids,
> > +};
> > +builtin_amba_driver(debug_driver);
> > -- 
> > 2.7.4
> > 

Re: [PATCH v2 2/3] coresight: refactor with function of_coresight_get_cpu

[Leo Yan]

On Wed, Mar 01, 2017 at 09:02:33AM -0700, Mathieu Poirier wrote:
> On Tue, Feb 28, 2017 at 11:06:59PM +0800, Leo Yan wrote:
> > This is refactor to add function of_coresight_get_cpu(), so it's used to
> > retrieve CPU id for coresight component. Finally can use it as a common
> > function for multiple places.
> > 
> > Suggested-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> > Signed-off-by: Leo Yan <leo.yan@linaro.org>
> > ---
> >  drivers/hwtracing/coresight/of_coresight.c | 35 ++++++++++++++++++++----------
> >  include/linux/coresight.h                  |  2 ++
> >  2 files changed, 25 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/hwtracing/coresight/of_coresight.c b/drivers/hwtracing/coresight/of_coresight.c
> > index 629e031..2e7bbe2 100644
> > --- a/drivers/hwtracing/coresight/of_coresight.c
> > +++ b/drivers/hwtracing/coresight/of_coresight.c
> > @@ -101,14 +101,34 @@ static int of_coresight_alloc_memory(struct device *dev,
> >  	return 0;
> >  }
> >  
> > +int of_coresight_get_cpu(struct device_node *node)
> > +{
> > +	int cpu;
> > +	struct device_node *dn;
> > +
> > +	dn = of_parse_phandle(node, "cpu", 0);
> > +
> > +	/* Affinity defaults to CPU0 */
> > +	if (!dn)
> > +		return 0;
> > +
> > +	for_each_possible_cpu(cpu) {
> > +		if (dn == of_get_cpu_node(cpu, NULL))
> > +			break;
> > +	}
> > +	of_node_put(dn);
> 
> In the case where no CPU nodes are found this function will return the number
> of the last CPU that was probed instead of '0'.

Sure, will fix this.

> > +
> > +	return cpu;
> > +}
> > +EXPORT_SYMBOL_GPL(of_coresight_get_cpu);
> 
> Any reason you exported this - the fact that is it declared in corsight.h should
> be enough.

Will remove it.

Thanks,
Leo Yan

> > +
> >  struct coresight_platform_data *of_get_coresight_platform_data(
> >  				struct device *dev, struct device_node *node)
> >  {
> > -	int i = 0, ret = 0, cpu;
> > +	int i = 0, ret = 0;
> >  	struct coresight_platform_data *pdata;
> >  	struct of_endpoint endpoint, rendpoint;
> >  	struct device *rdev;
> > -	struct device_node *dn;
> >  	struct device_node *ep = NULL;
> >  	struct device_node *rparent = NULL;
> >  	struct device_node *rport = NULL;
> > @@ -175,16 +195,7 @@ struct coresight_platform_data *of_get_coresight_platform_data(
> >  		} while (ep);
> >  	}
> >  
> > -	/* Affinity defaults to CPU0 */
> > -	pdata->cpu = 0;
> > -	dn = of_parse_phandle(node, "cpu", 0);
> > -	for (cpu = 0; dn && cpu < nr_cpu_ids; cpu++) {
> > -		if (dn == of_get_cpu_node(cpu, NULL)) {
> > -			pdata->cpu = cpu;
> > -			break;
> > -		}
> > -	}
> > -	of_node_put(dn);
> > +	pdata->cpu = of_coresight_get_cpu(node);
> >  
> >  	return pdata;
> >  }
> > diff --git a/include/linux/coresight.h b/include/linux/coresight.h
> > index 2a5982c..6709561 100644
> > --- a/include/linux/coresight.h
> > +++ b/include/linux/coresight.h
> > @@ -263,11 +263,13 @@ static inline int coresight_timeout(void __iomem *addr, u32 offset,
> >  #endif
> >  
> >  #ifdef CONFIG_OF
> > +extern int of_coresight_get_cpu(struct device_node *node);
> >  extern struct coresight_platform_data *of_get_coresight_platform_data(
> >  				struct device *dev, struct device_node *node);
> >  #else
> >  static inline struct coresight_platform_data *of_get_coresight_platform_data(
> >  	struct device *dev, struct device_node *node) { return NULL; }
> > +static int of_coresight_get_cpu(struct device_node *node) { return 0; }
> >  #endif
> >  
> >  #ifdef CONFIG_PID_NS
> > -- 
> > 2.7.4
> > 

Re: [PATCH v2 1/3] coresight: bindings for debug module

[Leo Yan]

On Wed, Mar 01, 2017 at 08:45:50AM -0700, Mathieu Poirier wrote:
> On Tue, Feb 28, 2017 at 11:06:58PM +0800, Leo Yan wrote:
> > According to ARMv8 architecture reference manual (ARM DDI 0487A.k)
> > Chapter 'Part H: External debug', the CPU can integrate debug module
> > and it can support self-hosted debug and external debug. Especially
> > for supporting self-hosted debug, this means the program can access
> > the debug module from mmio region; and usually the mmio region is
> > integrated with coresight.
> > 
> > So add document for binding debug component, includes binding to APB
> > clock; and also need specify the CPU node which the debug module is
> > dedicated to specific CPU.
> > 
> > Suggested-by: Mike Leach <mike.leach@linaro.org>
> > Signed-off-by: Leo Yan <leo.yan@linaro.org>
> > ---
> >  .../devicetree/bindings/arm/coresight-debug.txt    | 40 ++++++++++++++++++++++
> >  1 file changed, 40 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/arm/coresight-debug.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/arm/coresight-debug.txt b/Documentation/devicetree/bindings/arm/coresight-debug.txt
> > new file mode 100644
> > index 0000000..89820d5
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/arm/coresight-debug.txt
> > @@ -0,0 +1,40 @@
> > +* CoreSight Debug Component:
> > +
> > +CoreSight debug component are compliant with the ARMv8 architecture reference
> > +manual (ARM DDI 0487A.k) Chapter 'Part H: External debug'. The external debug
> > +module is mainly used for two modes: self-hosted debug and external debug, and
> > +it can be accessed from mmio region from Coresight and eventually the debug
> > +module connects with CPU for debugging. And the debug module provides
> > +sample-based profiling extension, which can be used to sample CPU program
> > +counter, secure state and exception level, etc; usually every CPU has one
> > +dedicated debug module to be connected.
> > +
> > +Required properties:
> > +
> > +- compatible : should be
> > +	     * "arm,coresight-debug", "arm,primecell"; supplemented with
> > +	       "arm,primecell" as driver is using the AMBA bus interface.
> 
>              * "arm,coresight-debug"; supplemented with "arm,primecell" since
>                this driver is using the AMBA bus interface.
> 
> > +
> > +- reg : physical base address and length of the register set.
> > +
> > +- clocks : the clock associated to this component.
> > +
> > +- clock-names : the name of the clock referenced by the code. Since we are
> > +                using the AMBA framework, the name of the clock providing
> > +		the interconnect should be "apb_pclk" and the clock is
> > +		mandatory. The interface between the debug logic and the
> > +		processor core is clocked by the internal CPU clock, so it
> > +		is enabled with CPU clock by default.
> > +
> > +- cpu : the cpu phandle the debug module is affined to. When omitted
> > +	the source is considered to belong to CPU0.
> 
> s/source/module
> 
> With those change:
> 
> Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>

Thanks for reviewing :)

> > +
> > +Example:
> > +
> > +	debug@f6590000 {
> > +		compatible = "arm,coresight-debug","arm,primecell";
> > +		reg = <0 0xf6590000 0 0x1000>;
> > +		clocks = <&sys_ctrl HI6220_DAPB_CLK>;
> > +		clock-names = "apb_pclk";
> > +		cpu = <&cpu0>;
> > +	};
> > -- 
> > 2.7.4
> > 

Re: [PATCH v2 1/3] coresight: bindings for debug module

[Rob Herring]

On Tue, Feb 28, 2017 at 11:06:58PM +0800, Leo Yan wrote:
> According to ARMv8 architecture reference manual (ARM DDI 0487A.k)
> Chapter 'Part H: External debug', the CPU can integrate debug module
> and it can support self-hosted debug and external debug. Especially
> for supporting self-hosted debug, this means the program can access
> the debug module from mmio region; and usually the mmio region is
> integrated with coresight.
> 
> So add document for binding debug component, includes binding to APB
> clock; and also need specify the CPU node which the debug module is
> dedicated to specific CPU.
> 
> Suggested-by: Mike Leach <mike.leach@linaro.org>
> Signed-off-by: Leo Yan <leo.yan@linaro.org>
> ---
>  .../devicetree/bindings/arm/coresight-debug.txt    | 40 ++++++++++++++++++++++
>  1 file changed, 40 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/arm/coresight-debug.txt

Acked-by: Rob Herring <robh@kernel.org>