linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH/RFC v2 0/4] ARM: hw_breakpoint: Avoid undef instruction exceptions on wake-up
@ 2014-09-30 12:26 Geert Uytterhoeven
  2014-09-30 12:26 ` [PATCH/RFC v2 1/4] ARM: hw_breakpoint: Add arm_dbg_regs_available flag Geert Uytterhoeven
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Geert Uytterhoeven @ 2014-09-30 12:26 UTC (permalink / raw)
  To: Russell King, Will Deacon, Mathieu Poirier, Simon Horman, Magnus Damm
  Cc: linux-arm-kernel, linux-sh, linux-kernel, Geert Uytterhoeven

	Hi all,

If power area D4, which contains the Coresight-ETM hardware block, is
powered down on R-Mobile A1 (r8a7740), the kernel crashes when
suspending from s2ram with:

    Internal error: Oops - undefined instruction: 0 [#1] ARM

This happens because dbg_cpu_pm_notify() calls reset_ctrl_regs(), which
can't access the debug registers as the debug module is powered down.

As suggested by Russell King, track whether the ETM block is powered down
to fix this.

The availability of the debug registers depends on the platform and its
state.  Hence provide a mechanism for platform code to indicate that the
debug registers are available or not, using a boolean flag that
defaults to true.

This is an alternative solution for "[PATCH] ARM: hw_breakpoint: Trap undef
instruction exceptions on wake-up" (https://lkml.org/lkml/2014/9/17/190).

Thanks for your comments!

Geert Uytterhoeven (4):
  [RFC] ARM: hw_breakpoint: Add arm_dbg_regs_available flag
  [RFC] ARM: shmobile: r8a7740 legacy: Sync arm_dbg_regs_available with
    D4 PM domain
  [RFC] ARM: shmobile: R-Mobile: Sync arm_dbg_regs_available with D4 PM
    domain
  [RFC] ARM: shmobile: r8a7740 dtsi: Add minimal device node for
    Coresight-ETM

 arch/arm/boot/dts/r8a7740.dtsi       |  5 +++++
 arch/arm/include/asm/hw_breakpoint.h |  2 ++
 arch/arm/kernel/hw_breakpoint.c      |  7 +++++++
 arch/arm/mach-shmobile/pm-r8a7740.c  | 19 +++++++++++++++++++
 arch/arm/mach-shmobile/pm-rmobile.c  | 35 ++++++++++++++++++++++++++++++++---
 5 files changed, 65 insertions(+), 3 deletions(-)

-- 
1.9.1

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH/RFC v2 1/4] ARM: hw_breakpoint: Add arm_dbg_regs_available flag
  2014-09-30 12:26 [PATCH/RFC v2 0/4] ARM: hw_breakpoint: Avoid undef instruction exceptions on wake-up Geert Uytterhoeven
@ 2014-09-30 12:26 ` Geert Uytterhoeven
  2014-09-30 16:03   ` Will Deacon
  2014-09-30 12:26 ` [PATCH/RFC v2 2/4] ARM: shmobile: r8a7740 legacy: Sync arm_dbg_regs_available with D4 PM domain Geert Uytterhoeven
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Geert Uytterhoeven @ 2014-09-30 12:26 UTC (permalink / raw)
  To: Russell King, Will Deacon, Mathieu Poirier, Simon Horman, Magnus Damm
  Cc: linux-arm-kernel, linux-sh, linux-kernel, Geert Uytterhoeven

If power area D4, which contains the Coresight-ETM hardware block, is
powered down on R-Mobile A1 (r8a7740), the kernel crashes when
suspending from s2ram with:

    Internal error: Oops - undefined instruction: 0 [#1] ARM

This happens because dbg_cpu_pm_notify() calls reset_ctrl_regs(), which
can't access the debug registers as the debug module is powered down.

As suggested by Russell King, track whether the ETM block is powered down
to fix this.

The availability of the debug registers depends on the platform and its
state.  Hence provide a mechanism for platform code to indicate that the
debug registers are available or not, using a boolean flag that defaults
to true.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v2:
  - New

 arch/arm/include/asm/hw_breakpoint.h | 2 ++
 arch/arm/kernel/hw_breakpoint.c      | 7 +++++++
 2 files changed, 9 insertions(+)

diff --git a/arch/arm/include/asm/hw_breakpoint.h b/arch/arm/include/asm/hw_breakpoint.h
index 8e427c7b44257d2d..51ffdae41bfe3754 100644
--- a/arch/arm/include/asm/hw_breakpoint.h
+++ b/arch/arm/include/asm/hw_breakpoint.h
@@ -110,6 +110,8 @@ static inline void decode_ctrl_reg(u32 reg,
 	asm volatile("mcr p14, 0, %0, " #N "," #M ", " #OP2 : : "r" (VAL));\
 } while (0)
 
+extern bool arm_dbg_regs_available;
+
 struct notifier_block;
 struct perf_event;
 struct pmu;
diff --git a/arch/arm/kernel/hw_breakpoint.c b/arch/arm/kernel/hw_breakpoint.c
index b5b452f90f761bd2..96193c0165fbe5ca 100644
--- a/arch/arm/kernel/hw_breakpoint.c
+++ b/arch/arm/kernel/hw_breakpoint.c
@@ -38,6 +38,8 @@
 #include <asm/traps.h>
 #include <asm/hardware/coresight.h>
 
+bool arm_dbg_regs_available = true;
+
 /* Breakpoint currently in use for each BRP. */
 static DEFINE_PER_CPU(struct perf_event *, bp_on_reg[ARM_MAX_BRP]);
 
@@ -931,6 +933,11 @@ static void reset_ctrl_regs(void *unused)
 	int i, raw_num_brps, err = 0, cpu = smp_processor_id();
 	u32 val;
 
+	if (!arm_dbg_regs_available) {
+		pr_warn_once("Debug registers are not available\n");
+		return;
+	}
+
 	/*
 	 * v7 debug contains save and restore registers so that debug state
 	 * can be maintained across low-power modes without leaving the debug
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH/RFC v2 2/4] ARM: shmobile: r8a7740 legacy: Sync arm_dbg_regs_available with D4 PM domain
  2014-09-30 12:26 [PATCH/RFC v2 0/4] ARM: hw_breakpoint: Avoid undef instruction exceptions on wake-up Geert Uytterhoeven
  2014-09-30 12:26 ` [PATCH/RFC v2 1/4] ARM: hw_breakpoint: Add arm_dbg_regs_available flag Geert Uytterhoeven
@ 2014-09-30 12:26 ` Geert Uytterhoeven
  2014-09-30 12:26 ` [PATCH/RFC v2 3/4] ARM: shmobile: R-Mobile: " Geert Uytterhoeven
  2014-09-30 12:26 ` [PATCH/RFC v2 4/4] ARM: shmobile: r8a7740 dtsi: Add minimal device node for Coresight-ETM Geert Uytterhoeven
  3 siblings, 0 replies; 9+ messages in thread
From: Geert Uytterhoeven @ 2014-09-30 12:26 UTC (permalink / raw)
  To: Russell King, Will Deacon, Mathieu Poirier, Simon Horman, Magnus Damm
  Cc: linux-arm-kernel, linux-sh, linux-kernel, Geert Uytterhoeven

If power area D4, which contains the Coresight-ETM hardware block, is
powered down on R-Mobile A1 (r8a7740), the kernel crashes when
suspending from s2ram with:

    Internal error: Oops - undefined instruction: 0 [#1] ARM

This happens because dbg_cpu_pm_notify() calls reset_ctrl_regs(), which
can't access the debug registers as the debug module is powered down.

Keep the arm_dbg_regs_available flag in sync with the state of the D4 PM
domain to fix this.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
This is a fix on top of series "[PATCH v2 00/11] ARM: shmobile:
r8a7740/armadillo800eva legacy PM domain support"
(http://www.spinics.net/lists/arm-kernel/msg365435.html), to be folded
into "[PATCH v2 08/11] ARM: shmobile: r8a7740: Add D4 pm domain
support" (http://www.spinics.net/lists/arm-kernel/msg365437.html).

v2:
  - New

---
 arch/arm/mach-shmobile/pm-r8a7740.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/arch/arm/mach-shmobile/pm-r8a7740.c b/arch/arm/mach-shmobile/pm-r8a7740.c
index f92e64fd91de5631..b4a52f4067a87105 100644
--- a/arch/arm/mach-shmobile/pm-r8a7740.c
+++ b/arch/arm/mach-shmobile/pm-r8a7740.c
@@ -12,6 +12,8 @@
 #include <linux/io.h>
 #include <linux/suspend.h>
 
+#include <asm/hw_breakpoint.h>
+
 #include "common.h"
 #include "pm-rmobile.h"
 
@@ -36,6 +38,21 @@ static int r8a7740_pd_a3sp_suspend(void)
 	return console_suspend_enabled ? 0 : -EBUSY;
 }
 
+/*
+ * The D4 domain contains the Coresight-ETM hardware block.
+ * The debug registers cannot be accessed while this domain is powered down.
+ */
+static int r8a7740_pd_d4_suspend(void)
+{
+	arm_dbg_regs_available = false;
+	return 0;
+}
+
+static void r8a7740_pd_d4_resume(void)
+{
+	arm_dbg_regs_available = true;
+}
+
 static struct rmobile_pm_domain r8a7740_pm_domains[] = {
 	{
 		.genpd.name	= "A4LC",
@@ -49,6 +66,8 @@ static struct rmobile_pm_domain r8a7740_pm_domains[] = {
 		.genpd.name	= "D4",
 		.base		= SYSC_BASE,
 		.bit_shift	= 3,
+		.suspend	= r8a7740_pd_d4_suspend,
+		.resume		= r8a7740_pd_d4_resume,
 	}, {
 		.genpd.name	= "A4R",
 		.base		= SYSC_BASE,
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH/RFC v2 3/4] ARM: shmobile: R-Mobile: Sync arm_dbg_regs_available with D4 PM domain
  2014-09-30 12:26 [PATCH/RFC v2 0/4] ARM: hw_breakpoint: Avoid undef instruction exceptions on wake-up Geert Uytterhoeven
  2014-09-30 12:26 ` [PATCH/RFC v2 1/4] ARM: hw_breakpoint: Add arm_dbg_regs_available flag Geert Uytterhoeven
  2014-09-30 12:26 ` [PATCH/RFC v2 2/4] ARM: shmobile: r8a7740 legacy: Sync arm_dbg_regs_available with D4 PM domain Geert Uytterhoeven
@ 2014-09-30 12:26 ` Geert Uytterhoeven
  2014-09-30 12:26 ` [PATCH/RFC v2 4/4] ARM: shmobile: r8a7740 dtsi: Add minimal device node for Coresight-ETM Geert Uytterhoeven
  3 siblings, 0 replies; 9+ messages in thread
From: Geert Uytterhoeven @ 2014-09-30 12:26 UTC (permalink / raw)
  To: Russell King, Will Deacon, Mathieu Poirier, Simon Horman, Magnus Damm
  Cc: linux-arm-kernel, linux-sh, linux-kernel, Geert Uytterhoeven

If power area D4, which contains the Coresight-ETM hardware block, is
powered down on R-Mobile A1 (r8a7740), the kernel crashes when
suspending from s2ram with:

    Internal error: Oops - undefined instruction: 0 [#1] ARM

This happens because dbg_cpu_pm_notify() calls reset_ctrl_regs(), which
can't access the debug registers as the debug module is powered down.

Keep the arm_dbg_regs_available flag in sync with the state of the PM
domain that contains a device node that is compatible with
"arm,coresight-etm3x".

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
This is a fix on top of series "[PATCH 00/13] ARM: shmobile: R-Mobile:
DT PM domain support" (https://lkml.org/lkml/2014/9/25/452), to be
folded into "[PATCH v3 09/13] ARM: shmobile: R-Mobile: Add DT support
for PM domains" (https://lkml.org/lkml/2014/9/25/448).

v2:
  - New

---
 arch/arm/mach-shmobile/pm-rmobile.c | 35 ++++++++++++++++++++++++++++++++---
 1 file changed, 32 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-shmobile/pm-rmobile.c b/arch/arm/mach-shmobile/pm-rmobile.c
index b1aa0e8541feaea4..80be5848adc8f15c 100644
--- a/arch/arm/mach-shmobile/pm-rmobile.c
+++ b/arch/arm/mach-shmobile/pm-rmobile.c
@@ -21,7 +21,10 @@
 #include <linux/pm.h>
 #include <linux/pm_clock.h>
 #include <linux/slab.h>
+
+#include <asm/hw_breakpoint.h>
 #include <asm/io.h>
+
 #include "pm-rmobile.h"
 
 /* SYSC */
@@ -198,20 +201,36 @@ static int rmobile_pd_suspend_console(void)
 	return console_suspend_enabled ? 0 : -EBUSY;
 }
 
+/*
+ * This domain contains the Coresight-ETM hardware block.
+ * The debug registers cannot be accessed while this domain is powered down.
+ */
+static int rmobile_pd_suspend_debug(void)
+{
+	arm_dbg_regs_available = false;
+	return 0;
+}
+
+static void rmobile_pd_resume_debug(void)
+{
+	arm_dbg_regs_available = true;
+}
+
 #define MAX_NUM_CPU_PDS		8
 
 static unsigned int num_cpu_pds __initdata;
 static struct device_node *cpu_pds[MAX_NUM_CPU_PDS] __initdata;
 static struct device_node *console_pd __initdata;
+static struct device_node *debug_pd __initdata;
 
 static void __init get_special_pds(void)
 {
-	struct device_node *cpu, *pd;
+	struct device_node *np, *pd;
 	unsigned int i;
 
 	/* PM domains containing CPUs */
-	for_each_node_by_type(cpu, "cpu") {
-		pd = of_parse_phandle(cpu, "power-domains", 0);
+	for_each_node_by_type(np, "cpu") {
+		pd = of_parse_phandle(np, "power-domains", 0);
 		if (!pd)
 			continue;
 
@@ -236,6 +255,11 @@ static void __init get_special_pds(void)
 	/* PM domain containing console */
 	if (of_stdout)
 		console_pd = of_parse_phandle(of_stdout, "power-domains", 0);
+
+	/* PM domain containing Coresight-ETM */
+	np = of_find_compatible_node(NULL, NULL, "arm,coresight-etm3x");
+	if (np)
+		debug_pd = of_parse_phandle(np, "power-domains", 0);
 }
 
 static void __init put_special_pds(void)
@@ -245,6 +269,7 @@ static void __init put_special_pds(void)
 	for (i = 0; i < num_cpu_pds; i++)
 		of_node_put(cpu_pds[i]);
 	of_node_put(console_pd);
+	of_node_put(debug_pd);
 }
 
 static bool __init pd_contains_cpu(const struct device_node *pd)
@@ -271,6 +296,10 @@ static void __init rmobile_setup_pm_domain(struct device_node *np,
 		pr_debug("PM domain %s contains serial console\n", name);
 		pd->gov = &pm_domain_always_on_gov;
 		pd->suspend = rmobile_pd_suspend_console;
+	} else if (np == debug_pd) {
+		pr_debug("PM domain %s contains Coresight-ETM\n", name);
+		pd->suspend = rmobile_pd_suspend_debug;
+		pd->resume = rmobile_pd_resume_debug;
 	}
 
 	rmobile_init_pm_domain(pd);
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH/RFC v2 4/4] ARM: shmobile: r8a7740 dtsi: Add minimal device node for Coresight-ETM
  2014-09-30 12:26 [PATCH/RFC v2 0/4] ARM: hw_breakpoint: Avoid undef instruction exceptions on wake-up Geert Uytterhoeven
                   ` (2 preceding siblings ...)
  2014-09-30 12:26 ` [PATCH/RFC v2 3/4] ARM: shmobile: R-Mobile: " Geert Uytterhoeven
@ 2014-09-30 12:26 ` Geert Uytterhoeven
  3 siblings, 0 replies; 9+ messages in thread
From: Geert Uytterhoeven @ 2014-09-30 12:26 UTC (permalink / raw)
  To: Russell King, Will Deacon, Mathieu Poirier, Simon Horman, Magnus Damm
  Cc: linux-arm-kernel, linux-sh, linux-kernel, Geert Uytterhoeven, devicetree

If power area D4, which contains the Coresight-ETM hardware block, is
powered down on R-Mobile A1 (r8a7740), the kernel crashes when
suspending from s2ram with:

    Internal error: Oops - undefined instruction: 0 [#1] ARM

This happens because dbg_cpu_pm_notify() calls reset_ctrl_regs(), which
can't access the debug registers as the debug module is powered down.

Add a minimal device node for the Coresight-ETM hardware block, and hook
it up to the D4 PM domain, so the R-Mobile System Controller driver can
keep the arm_dbg_regs_available flag in sync with the state of the D4 PM
domain.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: devicetree@vger.kernel.org
---
This is a fix on top of series "[PATCH 00/13] ARM: shmobile: R-Mobile:
DT PM domain support" (https://lkml.org/lkml/2014/9/25/452), to be
folded into "[PATCH v3 10/13] ARM: shmobile: r8a7740 dtsi: Add PM domain
support" (https://lkml.org/lkml/2014/9/25/450).

v2:
  - New

---
 arch/arm/boot/dts/r8a7740.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/boot/dts/r8a7740.dtsi b/arch/arm/boot/dts/r8a7740.dtsi
index 0fb93df46af4ce08..2ae08fc99be678c3 100644
--- a/arch/arm/boot/dts/r8a7740.dtsi
+++ b/arch/arm/boot/dts/r8a7740.dtsi
@@ -44,6 +44,11 @@
 		interrupts = <0 83 IRQ_TYPE_LEVEL_HIGH>;
 	};
 
+	ptm {
+		compatible = "arm,coresight-etm3x";
+		power-domains = <&pd_d4>;
+	};
+
 	cmt1: timer@e6138000 {
 		compatible = "renesas,cmt-48-r8a7740", "renesas,cmt-48";
 		reg = <0xe6138000 0x170>;
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH/RFC v2 1/4] ARM: hw_breakpoint: Add arm_dbg_regs_available flag
  2014-09-30 12:26 ` [PATCH/RFC v2 1/4] ARM: hw_breakpoint: Add arm_dbg_regs_available flag Geert Uytterhoeven
@ 2014-09-30 16:03   ` Will Deacon
  2014-10-01  7:41     ` Geert Uytterhoeven
  0 siblings, 1 reply; 9+ messages in thread
From: Will Deacon @ 2014-09-30 16:03 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Russell King, Mathieu Poirier, Simon Horman, Magnus Damm,
	linux-arm-kernel, linux-sh, linux-kernel

Hi Geert,

On Tue, Sep 30, 2014 at 01:26:24PM +0100, Geert Uytterhoeven wrote:
> If power area D4, which contains the Coresight-ETM hardware block, is
> powered down on R-Mobile A1 (r8a7740), the kernel crashes when
> suspending from s2ram with:
> 
>     Internal error: Oops - undefined instruction: 0 [#1] ARM
> 
> This happens because dbg_cpu_pm_notify() calls reset_ctrl_regs(), which
> can't access the debug registers as the debug module is powered down.
> 
> As suggested by Russell King, track whether the ETM block is powered down
> to fix this.
> 
> The availability of the debug registers depends on the platform and its
> state.  Hence provide a mechanism for platform code to indicate that the
> debug registers are available or not, using a boolean flag that defaults
> to true.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> v2:
>   - New
> 
>  arch/arm/include/asm/hw_breakpoint.h | 2 ++
>  arch/arm/kernel/hw_breakpoint.c      | 7 +++++++
>  2 files changed, 9 insertions(+)
> 
> diff --git a/arch/arm/include/asm/hw_breakpoint.h b/arch/arm/include/asm/hw_breakpoint.h
> index 8e427c7b44257d2d..51ffdae41bfe3754 100644
> --- a/arch/arm/include/asm/hw_breakpoint.h
> +++ b/arch/arm/include/asm/hw_breakpoint.h
> @@ -110,6 +110,8 @@ static inline void decode_ctrl_reg(u32 reg,
>  	asm volatile("mcr p14, 0, %0, " #N "," #M ", " #OP2 : : "r" (VAL));\
>  } while (0)
>  
> +extern bool arm_dbg_regs_available;
> +
>  struct notifier_block;
>  struct perf_event;
>  struct pmu;
> diff --git a/arch/arm/kernel/hw_breakpoint.c b/arch/arm/kernel/hw_breakpoint.c
> index b5b452f90f761bd2..96193c0165fbe5ca 100644
> --- a/arch/arm/kernel/hw_breakpoint.c
> +++ b/arch/arm/kernel/hw_breakpoint.c
> @@ -38,6 +38,8 @@
>  #include <asm/traps.h>
>  #include <asm/hardware/coresight.h>
>  
> +bool arm_dbg_regs_available = true;
> +
>  /* Breakpoint currently in use for each BRP. */
>  static DEFINE_PER_CPU(struct perf_event *, bp_on_reg[ARM_MAX_BRP]);
>  
> @@ -931,6 +933,11 @@ static void reset_ctrl_regs(void *unused)
>  	int i, raw_num_brps, err = 0, cpu = smp_processor_id();
>  	u32 val;
>  
> +	if (!arm_dbg_regs_available) {
> +		pr_warn_once("Debug registers are not available\n");
> +		return;
> +	}

Whilst I guess this solves your problem, it doesn't feel like a scalable fix
for something that can/will assumedly happen elsewhere in an SoC (e.g. PMU
registers in perf). I'd much rather have a generic abstraction for power
domains, which subsystems such as hw_breakpoint can attempt to take a
reference on when they want to access registers in that domain.

Will

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH/RFC v2 1/4] ARM: hw_breakpoint: Add arm_dbg_regs_available flag
  2014-09-30 16:03   ` Will Deacon
@ 2014-10-01  7:41     ` Geert Uytterhoeven
  2014-10-01 14:38       ` Mathieu Poirier
  0 siblings, 1 reply; 9+ messages in thread
From: Geert Uytterhoeven @ 2014-10-01  7:41 UTC (permalink / raw)
  To: Will Deacon
  Cc: Geert Uytterhoeven, Russell King, Mathieu Poirier, Simon Horman,
	Magnus Damm, linux-arm-kernel, linux-sh, linux-kernel

Hi Will,

On Tue, Sep 30, 2014 at 6:03 PM, Will Deacon <will.deacon@arm.com> wrote:
> On Tue, Sep 30, 2014 at 01:26:24PM +0100, Geert Uytterhoeven wrote:
>> If power area D4, which contains the Coresight-ETM hardware block, is
>> powered down on R-Mobile A1 (r8a7740), the kernel crashes when
>> suspending from s2ram with:
>>
>>     Internal error: Oops - undefined instruction: 0 [#1] ARM
>>
>> This happens because dbg_cpu_pm_notify() calls reset_ctrl_regs(), which
>> can't access the debug registers as the debug module is powered down.
>>
>> As suggested by Russell King, track whether the ETM block is powered down
>> to fix this.
>>
>> The availability of the debug registers depends on the platform and its
>> state.  Hence provide a mechanism for platform code to indicate that the
>> debug registers are available or not, using a boolean flag that defaults
>> to true.

> Whilst I guess this solves your problem, it doesn't feel like a scalable fix
> for something that can/will assumedly happen elsewhere in an SoC (e.g. PMU
> registers in perf). I'd much rather have a generic abstraction for power
> domains, which subsystems such as hw_breakpoint can attempt to take a
> reference on when they want to access registers in that domain.

I agree this is a bit hackish, and not a long-term solution.

As soon as the ARM debug/perf subsystem starts using devices instantiated
from DT, implementing PM runtime support, this will work out-of-the-box.
Until then, we cannot have proper support for the D4 PM domain on R-Mobile
SoCs, without hacks like this (or like never powering down the D4 PM domain
--- perhaps that's the way to go).

I believe Mathieu is working on the former, but so far without converting
hw_breakpoint.c nor adding PM runtime support?

Thanks for your understanding!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH/RFC v2 1/4] ARM: hw_breakpoint: Add arm_dbg_regs_available flag
  2014-10-01  7:41     ` Geert Uytterhoeven
@ 2014-10-01 14:38       ` Mathieu Poirier
  2014-10-01 20:26         ` Geert Uytterhoeven
  0 siblings, 1 reply; 9+ messages in thread
From: Mathieu Poirier @ 2014-10-01 14:38 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Will Deacon, Geert Uytterhoeven, Russell King, Simon Horman,
	Magnus Damm, linux-arm-kernel, linux-sh, linux-kernel

On 1 October 2014 01:41, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> Hi Will,
>
> On Tue, Sep 30, 2014 at 6:03 PM, Will Deacon <will.deacon@arm.com> wrote:
>> On Tue, Sep 30, 2014 at 01:26:24PM +0100, Geert Uytterhoeven wrote:
>>> If power area D4, which contains the Coresight-ETM hardware block, is
>>> powered down on R-Mobile A1 (r8a7740), the kernel crashes when
>>> suspending from s2ram with:
>>>
>>>     Internal error: Oops - undefined instruction: 0 [#1] ARM
>>>
>>> This happens because dbg_cpu_pm_notify() calls reset_ctrl_regs(), which
>>> can't access the debug registers as the debug module is powered down.
>>>
>>> As suggested by Russell King, track whether the ETM block is powered down
>>> to fix this.
>>>
>>> The availability of the debug registers depends on the platform and its
>>> state.  Hence provide a mechanism for platform code to indicate that the
>>> debug registers are available or not, using a boolean flag that defaults
>>> to true.

I'm afraid the usage of the handle "arm,coresight-etm3x" is arbitrary
here as what the code isn't related to an embedded trace module, but
rather to the power domain that trace module happens to be in.  If we
are to take that solution a handle name relate to the debug power
domain is likely to be more appropriate.

>
>> Whilst I guess this solves your problem, it doesn't feel like a scalable fix
>> for something that can/will assumedly happen elsewhere in an SoC (e.g. PMU
>> registers in perf). I'd much rather have a generic abstraction for power
>> domains, which subsystems such as hw_breakpoint can attempt to take a
>> reference on when they want to access registers in that domain.
>
> I agree this is a bit hackish, and not a long-term solution.
>
> As soon as the ARM debug/perf subsystem starts using devices instantiated
> from DT, implementing PM runtime support, this will work out-of-the-box.
> Until then, we cannot have proper support for the D4 PM domain on R-Mobile
> SoCs, without hacks like this (or like never powering down the D4 PM domain
> --- perhaps that's the way to go).
>
> I believe Mathieu is working on the former, but so far without converting
> hw_breakpoint.c nor adding PM runtime support?

Humm... At this time the coresight framework doesn't deal with PM
runtime support.  It is something that's been on the list of things to
do for a while now but I never had the time to get to it.  On the flip
side I currently have two boards on my desk that need to interact with
different debug domains to work properly and as such, the feature is
bound to appear at some point.

>
> Thanks for your understanding!
>
> Gr{oetje,eeting}s,
>
>                         Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH/RFC v2 1/4] ARM: hw_breakpoint: Add arm_dbg_regs_available flag
  2014-10-01 14:38       ` Mathieu Poirier
@ 2014-10-01 20:26         ` Geert Uytterhoeven
  0 siblings, 0 replies; 9+ messages in thread
From: Geert Uytterhoeven @ 2014-10-01 20:26 UTC (permalink / raw)
  To: Mathieu Poirier
  Cc: Will Deacon, Geert Uytterhoeven, Russell King, Simon Horman,
	Magnus Damm, linux-arm-kernel, linux-sh, linux-kernel

Hi Mathieu,

On Wed, Oct 1, 2014 at 4:38 PM, Mathieu Poirier
<mathieu.poirier@linaro.org> wrote:
> On 1 October 2014 01:41, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>> On Tue, Sep 30, 2014 at 6:03 PM, Will Deacon <will.deacon@arm.com> wrote:
>>> On Tue, Sep 30, 2014 at 01:26:24PM +0100, Geert Uytterhoeven wrote:
>>>> If power area D4, which contains the Coresight-ETM hardware block, is
>>>> powered down on R-Mobile A1 (r8a7740), the kernel crashes when
>>>> suspending from s2ram with:
>>>>
>>>>     Internal error: Oops - undefined instruction: 0 [#1] ARM
>>>>
>>>> This happens because dbg_cpu_pm_notify() calls reset_ctrl_regs(), which
>>>> can't access the debug registers as the debug module is powered down.
>>>>
>>>> As suggested by Russell King, track whether the ETM block is powered down
>>>> to fix this.
>>>>
>>>> The availability of the debug registers depends on the platform and its
>>>> state.  Hence provide a mechanism for platform code to indicate that the
>>>> debug registers are available or not, using a boolean flag that defaults
>>>> to true.
>
> I'm afraid the usage of the handle "arm,coresight-etm3x" is arbitrary
> here as what the code isn't related to an embedded trace module, but
> rather to the power domain that trace module happens to be in.  If we
> are to take that solution a handle name relate to the debug power
> domain is likely to be more appropriate.

W.r.t. PM domains, the datasheet only mentions "Coresight-ETM" being
in the D4 power area. That's why I used the "arm,coresight-etm3x" handle.
It's still to be discovered what else is hidden there...

>>> Whilst I guess this solves your problem, it doesn't feel like a scalable fix
>>> for something that can/will assumedly happen elsewhere in an SoC (e.g. PMU
>>> registers in perf). I'd much rather have a generic abstraction for power
>>> domains, which subsystems such as hw_breakpoint can attempt to take a
>>> reference on when they want to access registers in that domain.
>>
>> I agree this is a bit hackish, and not a long-term solution.
>>
>> As soon as the ARM debug/perf subsystem starts using devices instantiated
>> from DT, implementing PM runtime support, this will work out-of-the-box.
>> Until then, we cannot have proper support for the D4 PM domain on R-Mobile
>> SoCs, without hacks like this (or like never powering down the D4 PM domain
>> --- perhaps that's the way to go).
>>
>> I believe Mathieu is working on the former, but so far without converting
>> hw_breakpoint.c nor adding PM runtime support?
>
> Humm... At this time the coresight framework doesn't deal with PM
> runtime support.  It is something that's been on the list of things to
> do for a while now but I never had the time to get to it.  On the flip
> side I currently have two boards on my desk that need to interact with
> different debug domains to work properly and as such, the feature is
> bound to appear at some point.

OK. We'll see...

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2014-10-01 20:26 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-30 12:26 [PATCH/RFC v2 0/4] ARM: hw_breakpoint: Avoid undef instruction exceptions on wake-up Geert Uytterhoeven
2014-09-30 12:26 ` [PATCH/RFC v2 1/4] ARM: hw_breakpoint: Add arm_dbg_regs_available flag Geert Uytterhoeven
2014-09-30 16:03   ` Will Deacon
2014-10-01  7:41     ` Geert Uytterhoeven
2014-10-01 14:38       ` Mathieu Poirier
2014-10-01 20:26         ` Geert Uytterhoeven
2014-09-30 12:26 ` [PATCH/RFC v2 2/4] ARM: shmobile: r8a7740 legacy: Sync arm_dbg_regs_available with D4 PM domain Geert Uytterhoeven
2014-09-30 12:26 ` [PATCH/RFC v2 3/4] ARM: shmobile: R-Mobile: " Geert Uytterhoeven
2014-09-30 12:26 ` [PATCH/RFC v2 4/4] ARM: shmobile: r8a7740 dtsi: Add minimal device node for Coresight-ETM Geert Uytterhoeven

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).