All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ARM: imx: build v7_cpu_resume() unconditionally
@ 2020-01-16 14:18 ` Rouven Czerwinski
  0 siblings, 0 replies; 14+ messages in thread
From: Rouven Czerwinski @ 2020-01-16 14:18 UTC (permalink / raw)
  To: s.hauer, shawnguo
  Cc: kernel, linux-kernel, linux-arm-kernel, festevam, linux-imx,
	Ahmad Fatoum, Lucas Stach, Rouven Czerwinski

From: Ahmad Fatoum <a.fatoum@pengutronix.de>

This function is not only needed by the platform suspend code, but is also
reused as the CPU resume function when the ARM cores can be powered down
completely in deep idle, which is the case on i.MX6SX and i.MX6UL(L).

Providing the static inline stub whenever CONFIG_SUSPEND is disabled means
that those platforms will hang on resume from cpuidle if suspend is disabled.

So there are two problems:

  - The static inline stub masks the linker error
  - The function is not available where needed

Fix both by just building the function unconditionally, when
CONFIG_SOC_IMX6 is enabled. The actual code is three instructions long,
so it's arguably ok to just leave it in for all i.MX6 kernel configurations.

Fixes: 05136f0897b5 ("ARM: imx: support arm power off in cpuidle for i.mx6sx")
Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
Signed-off-by: Rouven Czerwinski <r.czerwinski@pengutronix.de>
---
 arch/arm/mach-imx/Makefile       |  2 ++
 arch/arm/mach-imx/common.h       |  4 ++--
 arch/arm/mach-imx/resume-imx6.S  | 24 ++++++++++++++++++++++++
 arch/arm/mach-imx/suspend-imx6.S | 14 --------------
 4 files changed, 28 insertions(+), 16 deletions(-)
 create mode 100644 arch/arm/mach-imx/resume-imx6.S

diff --git a/arch/arm/mach-imx/Makefile b/arch/arm/mach-imx/Makefile
index 35ff620537e6..03506ce46149 100644
--- a/arch/arm/mach-imx/Makefile
+++ b/arch/arm/mach-imx/Makefile
@@ -91,6 +91,8 @@ AFLAGS_suspend-imx6.o :=-Wa,-march=armv7-a
 obj-$(CONFIG_SOC_IMX6) += suspend-imx6.o
 obj-$(CONFIG_SOC_IMX53) += suspend-imx53.o
 endif
+AFLAGS_resume-imx6.o :=-Wa,-march=armv7-a
+obj-$(CONFIG_SOC_IMX6) += resume-imx6.o
 obj-$(CONFIG_SOC_IMX6) += pm-imx6.o
 
 obj-$(CONFIG_SOC_IMX1) += mach-imx1.o
diff --git a/arch/arm/mach-imx/common.h b/arch/arm/mach-imx/common.h
index 912aeceb4ff8..5aa5796cff0e 100644
--- a/arch/arm/mach-imx/common.h
+++ b/arch/arm/mach-imx/common.h
@@ -109,17 +109,17 @@ void imx_cpu_die(unsigned int cpu);
 int imx_cpu_kill(unsigned int cpu);
 
 #ifdef CONFIG_SUSPEND
-void v7_cpu_resume(void);
 void imx53_suspend(void __iomem *ocram_vbase);
 extern const u32 imx53_suspend_sz;
 void imx6_suspend(void __iomem *ocram_vbase);
 #else
-static inline void v7_cpu_resume(void) {}
 static inline void imx53_suspend(void __iomem *ocram_vbase) {}
 static const u32 imx53_suspend_sz;
 static inline void imx6_suspend(void __iomem *ocram_vbase) {}
 #endif
 
+void v7_cpu_resume(void);
+
 void imx6_pm_ccm_init(const char *ccm_compat);
 void imx6q_pm_init(void);
 void imx6dl_pm_init(void);
diff --git a/arch/arm/mach-imx/resume-imx6.S b/arch/arm/mach-imx/resume-imx6.S
new file mode 100644
index 000000000000..5bd1ba7ef15b
--- /dev/null
+++ b/arch/arm/mach-imx/resume-imx6.S
@@ -0,0 +1,24 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright 2014 Freescale Semiconductor, Inc.
+ */
+
+#include <linux/linkage.h>
+#include <asm/assembler.h>
+#include <asm/asm-offsets.h>
+#include <asm/hardware/cache-l2x0.h>
+#include "hardware.h"
+
+/*
+ * The following code must assume it is running from physical address
+ * where absolute virtual addresses to the data section have to be
+ * turned into relative ones.
+ */
+
+ENTRY(v7_cpu_resume)
+	bl	v7_invalidate_l1
+#ifdef CONFIG_CACHE_L2X0
+	bl	l2c310_early_resume
+#endif
+	b	cpu_resume
+ENDPROC(v7_cpu_resume)
diff --git a/arch/arm/mach-imx/suspend-imx6.S b/arch/arm/mach-imx/suspend-imx6.S
index 062391ff13da..1eabf2d2834b 100644
--- a/arch/arm/mach-imx/suspend-imx6.S
+++ b/arch/arm/mach-imx/suspend-imx6.S
@@ -327,17 +327,3 @@ resume:
 
 	ret	lr
 ENDPROC(imx6_suspend)
-
-/*
- * The following code must assume it is running from physical address
- * where absolute virtual addresses to the data section have to be
- * turned into relative ones.
- */
-
-ENTRY(v7_cpu_resume)
-	bl	v7_invalidate_l1
-#ifdef CONFIG_CACHE_L2X0
-	bl	l2c310_early_resume
-#endif
-	b	cpu_resume
-ENDPROC(v7_cpu_resume)
-- 
2.25.0


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

* [PATCH] ARM: imx: build v7_cpu_resume() unconditionally
@ 2020-01-16 14:18 ` Rouven Czerwinski
  0 siblings, 0 replies; 14+ messages in thread
From: Rouven Czerwinski @ 2020-01-16 14:18 UTC (permalink / raw)
  To: s.hauer, shawnguo
  Cc: Ahmad Fatoum, linux-kernel, linux-imx, kernel, Rouven Czerwinski,
	festevam, linux-arm-kernel, Lucas Stach

From: Ahmad Fatoum <a.fatoum@pengutronix.de>

This function is not only needed by the platform suspend code, but is also
reused as the CPU resume function when the ARM cores can be powered down
completely in deep idle, which is the case on i.MX6SX and i.MX6UL(L).

Providing the static inline stub whenever CONFIG_SUSPEND is disabled means
that those platforms will hang on resume from cpuidle if suspend is disabled.

So there are two problems:

  - The static inline stub masks the linker error
  - The function is not available where needed

Fix both by just building the function unconditionally, when
CONFIG_SOC_IMX6 is enabled. The actual code is three instructions long,
so it's arguably ok to just leave it in for all i.MX6 kernel configurations.

Fixes: 05136f0897b5 ("ARM: imx: support arm power off in cpuidle for i.mx6sx")
Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
Signed-off-by: Rouven Czerwinski <r.czerwinski@pengutronix.de>
---
 arch/arm/mach-imx/Makefile       |  2 ++
 arch/arm/mach-imx/common.h       |  4 ++--
 arch/arm/mach-imx/resume-imx6.S  | 24 ++++++++++++++++++++++++
 arch/arm/mach-imx/suspend-imx6.S | 14 --------------
 4 files changed, 28 insertions(+), 16 deletions(-)
 create mode 100644 arch/arm/mach-imx/resume-imx6.S

diff --git a/arch/arm/mach-imx/Makefile b/arch/arm/mach-imx/Makefile
index 35ff620537e6..03506ce46149 100644
--- a/arch/arm/mach-imx/Makefile
+++ b/arch/arm/mach-imx/Makefile
@@ -91,6 +91,8 @@ AFLAGS_suspend-imx6.o :=-Wa,-march=armv7-a
 obj-$(CONFIG_SOC_IMX6) += suspend-imx6.o
 obj-$(CONFIG_SOC_IMX53) += suspend-imx53.o
 endif
+AFLAGS_resume-imx6.o :=-Wa,-march=armv7-a
+obj-$(CONFIG_SOC_IMX6) += resume-imx6.o
 obj-$(CONFIG_SOC_IMX6) += pm-imx6.o
 
 obj-$(CONFIG_SOC_IMX1) += mach-imx1.o
diff --git a/arch/arm/mach-imx/common.h b/arch/arm/mach-imx/common.h
index 912aeceb4ff8..5aa5796cff0e 100644
--- a/arch/arm/mach-imx/common.h
+++ b/arch/arm/mach-imx/common.h
@@ -109,17 +109,17 @@ void imx_cpu_die(unsigned int cpu);
 int imx_cpu_kill(unsigned int cpu);
 
 #ifdef CONFIG_SUSPEND
-void v7_cpu_resume(void);
 void imx53_suspend(void __iomem *ocram_vbase);
 extern const u32 imx53_suspend_sz;
 void imx6_suspend(void __iomem *ocram_vbase);
 #else
-static inline void v7_cpu_resume(void) {}
 static inline void imx53_suspend(void __iomem *ocram_vbase) {}
 static const u32 imx53_suspend_sz;
 static inline void imx6_suspend(void __iomem *ocram_vbase) {}
 #endif
 
+void v7_cpu_resume(void);
+
 void imx6_pm_ccm_init(const char *ccm_compat);
 void imx6q_pm_init(void);
 void imx6dl_pm_init(void);
diff --git a/arch/arm/mach-imx/resume-imx6.S b/arch/arm/mach-imx/resume-imx6.S
new file mode 100644
index 000000000000..5bd1ba7ef15b
--- /dev/null
+++ b/arch/arm/mach-imx/resume-imx6.S
@@ -0,0 +1,24 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright 2014 Freescale Semiconductor, Inc.
+ */
+
+#include <linux/linkage.h>
+#include <asm/assembler.h>
+#include <asm/asm-offsets.h>
+#include <asm/hardware/cache-l2x0.h>
+#include "hardware.h"
+
+/*
+ * The following code must assume it is running from physical address
+ * where absolute virtual addresses to the data section have to be
+ * turned into relative ones.
+ */
+
+ENTRY(v7_cpu_resume)
+	bl	v7_invalidate_l1
+#ifdef CONFIG_CACHE_L2X0
+	bl	l2c310_early_resume
+#endif
+	b	cpu_resume
+ENDPROC(v7_cpu_resume)
diff --git a/arch/arm/mach-imx/suspend-imx6.S b/arch/arm/mach-imx/suspend-imx6.S
index 062391ff13da..1eabf2d2834b 100644
--- a/arch/arm/mach-imx/suspend-imx6.S
+++ b/arch/arm/mach-imx/suspend-imx6.S
@@ -327,17 +327,3 @@ resume:
 
 	ret	lr
 ENDPROC(imx6_suspend)
-
-/*
- * The following code must assume it is running from physical address
- * where absolute virtual addresses to the data section have to be
- * turned into relative ones.
- */
-
-ENTRY(v7_cpu_resume)
-	bl	v7_invalidate_l1
-#ifdef CONFIG_CACHE_L2X0
-	bl	l2c310_early_resume
-#endif
-	b	cpu_resume
-ENDPROC(v7_cpu_resume)
-- 
2.25.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH] ARM: imx: build v7_cpu_resume() unconditionally
  2020-01-16 14:18 ` Rouven Czerwinski
@ 2020-02-17  7:41   ` Rouven Czerwinski
  -1 siblings, 0 replies; 14+ messages in thread
From: Rouven Czerwinski @ 2020-02-17  7:41 UTC (permalink / raw)
  To: s.hauer, shawnguo
  Cc: Ahmad Fatoum, linux-kernel, linux-imx, kernel, festevam,
	linux-arm-kernel, Lucas Stach

Hi Shawn,

On Thu, 2020-01-16 at 15:18 +0100, Rouven Czerwinski wrote:
> From: Ahmad Fatoum <a.fatoum@pengutronix.de>
> 
> This function is not only needed by the platform suspend code, but is
> also
> reused as the CPU resume function when the ARM cores can be powered
> down
> completely in deep idle, which is the case on i.MX6SX and i.MX6UL(L).
> 
> Providing the static inline stub whenever CONFIG_SUSPEND is disabled
> means
> that those platforms will hang on resume from cpuidle if suspend is
> disabled.
> 
> So there are two problems:
> 
>   - The static inline stub masks the linker error
>   - The function is not available where needed
> 
> Fix both by just building the function unconditionally, when
> CONFIG_SOC_IMX6 is enabled. The actual code is three instructions
> long,
> so it's arguably ok to just leave it in for all i.MX6 kernel
> configurations.
> 
> Fixes: 05136f0897b5 ("ARM: imx: support arm power off in cpuidle for
> i.mx6sx")
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> Signed-off-by: Rouven Czerwinski <r.czerwinski@pengutronix.de>
> ---
>  arch/arm/mach-imx/Makefile       |  2 ++
>  arch/arm/mach-imx/common.h       |  4 ++--
>  arch/arm/mach-imx/resume-imx6.S  | 24 ++++++++++++++++++++++++
>  arch/arm/mach-imx/suspend-imx6.S | 14 --------------
>  4 files changed, 28 insertions(+), 16 deletions(-)
>  create mode 100644 arch/arm/mach-imx/resume-imx6.S

Gentle ping.

Can be found on patchwork: https://patchwork.kernel.org/patch/11337147/

Thanks,
Rouven Czerwinski


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

* Re: [PATCH] ARM: imx: build v7_cpu_resume() unconditionally
@ 2020-02-17  7:41   ` Rouven Czerwinski
  0 siblings, 0 replies; 14+ messages in thread
From: Rouven Czerwinski @ 2020-02-17  7:41 UTC (permalink / raw)
  To: s.hauer, shawnguo
  Cc: Ahmad Fatoum, linux-kernel, linux-imx, kernel, festevam,
	linux-arm-kernel, Lucas Stach

Hi Shawn,

On Thu, 2020-01-16 at 15:18 +0100, Rouven Czerwinski wrote:
> From: Ahmad Fatoum <a.fatoum@pengutronix.de>
> 
> This function is not only needed by the platform suspend code, but is
> also
> reused as the CPU resume function when the ARM cores can be powered
> down
> completely in deep idle, which is the case on i.MX6SX and i.MX6UL(L).
> 
> Providing the static inline stub whenever CONFIG_SUSPEND is disabled
> means
> that those platforms will hang on resume from cpuidle if suspend is
> disabled.
> 
> So there are two problems:
> 
>   - The static inline stub masks the linker error
>   - The function is not available where needed
> 
> Fix both by just building the function unconditionally, when
> CONFIG_SOC_IMX6 is enabled. The actual code is three instructions
> long,
> so it's arguably ok to just leave it in for all i.MX6 kernel
> configurations.
> 
> Fixes: 05136f0897b5 ("ARM: imx: support arm power off in cpuidle for
> i.mx6sx")
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> Signed-off-by: Rouven Czerwinski <r.czerwinski@pengutronix.de>
> ---
>  arch/arm/mach-imx/Makefile       |  2 ++
>  arch/arm/mach-imx/common.h       |  4 ++--
>  arch/arm/mach-imx/resume-imx6.S  | 24 ++++++++++++++++++++++++
>  arch/arm/mach-imx/suspend-imx6.S | 14 --------------
>  4 files changed, 28 insertions(+), 16 deletions(-)
>  create mode 100644 arch/arm/mach-imx/resume-imx6.S

Gentle ping.

Can be found on patchwork: https://patchwork.kernel.org/patch/11337147/

Thanks,
Rouven Czerwinski


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH] ARM: imx: build v7_cpu_resume() unconditionally
  2020-01-16 14:18 ` Rouven Czerwinski
@ 2020-02-17  7:52   ` Shawn Guo
  -1 siblings, 0 replies; 14+ messages in thread
From: Shawn Guo @ 2020-02-17  7:52 UTC (permalink / raw)
  To: Rouven Czerwinski
  Cc: s.hauer, Ahmad Fatoum, linux-kernel, linux-imx, kernel, festevam,
	linux-arm-kernel, Lucas Stach

On Thu, Jan 16, 2020 at 03:18:49PM +0100, Rouven Czerwinski wrote:
> From: Ahmad Fatoum <a.fatoum@pengutronix.de>
> 
> This function is not only needed by the platform suspend code, but is also
> reused as the CPU resume function when the ARM cores can be powered down
> completely in deep idle, which is the case on i.MX6SX and i.MX6UL(L).
> 
> Providing the static inline stub whenever CONFIG_SUSPEND is disabled means
> that those platforms will hang on resume from cpuidle if suspend is disabled.
> 
> So there are two problems:
> 
>   - The static inline stub masks the linker error
>   - The function is not available where needed
> 
> Fix both by just building the function unconditionally, when
> CONFIG_SOC_IMX6 is enabled. The actual code is three instructions long,
> so it's arguably ok to just leave it in for all i.MX6 kernel configurations.
> 
> Fixes: 05136f0897b5 ("ARM: imx: support arm power off in cpuidle for i.mx6sx")
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> Signed-off-by: Rouven Czerwinski <r.czerwinski@pengutronix.de>

Applied, thanks.

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

* Re: [PATCH] ARM: imx: build v7_cpu_resume() unconditionally
@ 2020-02-17  7:52   ` Shawn Guo
  0 siblings, 0 replies; 14+ messages in thread
From: Shawn Guo @ 2020-02-17  7:52 UTC (permalink / raw)
  To: Rouven Czerwinski
  Cc: Ahmad Fatoum, s.hauer, linux-kernel, linux-imx, kernel, festevam,
	linux-arm-kernel, Lucas Stach

On Thu, Jan 16, 2020 at 03:18:49PM +0100, Rouven Czerwinski wrote:
> From: Ahmad Fatoum <a.fatoum@pengutronix.de>
> 
> This function is not only needed by the platform suspend code, but is also
> reused as the CPU resume function when the ARM cores can be powered down
> completely in deep idle, which is the case on i.MX6SX and i.MX6UL(L).
> 
> Providing the static inline stub whenever CONFIG_SUSPEND is disabled means
> that those platforms will hang on resume from cpuidle if suspend is disabled.
> 
> So there are two problems:
> 
>   - The static inline stub masks the linker error
>   - The function is not available where needed
> 
> Fix both by just building the function unconditionally, when
> CONFIG_SOC_IMX6 is enabled. The actual code is three instructions long,
> so it's arguably ok to just leave it in for all i.MX6 kernel configurations.
> 
> Fixes: 05136f0897b5 ("ARM: imx: support arm power off in cpuidle for i.mx6sx")
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> Signed-off-by: Rouven Czerwinski <r.czerwinski@pengutronix.de>

Applied, thanks.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH] ARM: imx: build v7_cpu_resume() unconditionally
  2020-01-16 14:18 ` Rouven Czerwinski
@ 2020-03-22 18:50   ` Clemens Gruber
  -1 siblings, 0 replies; 14+ messages in thread
From: Clemens Gruber @ 2020-03-22 18:50 UTC (permalink / raw)
  To: Rouven Czerwinski
  Cc: s.hauer, shawnguo, Ahmad Fatoum, linux-kernel, linux-imx, kernel,
	festevam, linux-arm-kernel, Lucas Stach, stable,
	Greg Kroah-Hartman

Hi,

On Thu, Jan 16, 2020 at 03:18:49PM +0100, Rouven Czerwinski wrote:
> From: Ahmad Fatoum <a.fatoum@pengutronix.de>
> 
> This function is not only needed by the platform suspend code, but is also
> reused as the CPU resume function when the ARM cores can be powered down
> completely in deep idle, which is the case on i.MX6SX and i.MX6UL(L).
> 
> Providing the static inline stub whenever CONFIG_SUSPEND is disabled means
> that those platforms will hang on resume from cpuidle if suspend is disabled.
> 
> So there are two problems:
> 
>   - The static inline stub masks the linker error
>   - The function is not available where needed
> 
> Fix both by just building the function unconditionally, when
> CONFIG_SOC_IMX6 is enabled. The actual code is three instructions long,
> so it's arguably ok to just leave it in for all i.MX6 kernel configurations.
> 
> Fixes: 05136f0897b5 ("ARM: imx: support arm power off in cpuidle for i.mx6sx")
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> Signed-off-by: Rouven Czerwinski <r.czerwinski@pengutronix.de>
> ---
>  arch/arm/mach-imx/Makefile       |  2 ++
>  arch/arm/mach-imx/common.h       |  4 ++--
>  arch/arm/mach-imx/resume-imx6.S  | 24 ++++++++++++++++++++++++
>  arch/arm/mach-imx/suspend-imx6.S | 14 --------------
>  4 files changed, 28 insertions(+), 16 deletions(-)
>  create mode 100644 arch/arm/mach-imx/resume-imx6.S
> 
> diff --git a/arch/arm/mach-imx/Makefile b/arch/arm/mach-imx/Makefile
> index 35ff620537e6..03506ce46149 100644
> --- a/arch/arm/mach-imx/Makefile
> +++ b/arch/arm/mach-imx/Makefile
> @@ -91,6 +91,8 @@ AFLAGS_suspend-imx6.o :=-Wa,-march=armv7-a
>  obj-$(CONFIG_SOC_IMX6) += suspend-imx6.o
>  obj-$(CONFIG_SOC_IMX53) += suspend-imx53.o
>  endif
> +AFLAGS_resume-imx6.o :=-Wa,-march=armv7-a
> +obj-$(CONFIG_SOC_IMX6) += resume-imx6.o
>  obj-$(CONFIG_SOC_IMX6) += pm-imx6.o
>  
>  obj-$(CONFIG_SOC_IMX1) += mach-imx1.o
> diff --git a/arch/arm/mach-imx/common.h b/arch/arm/mach-imx/common.h
> index 912aeceb4ff8..5aa5796cff0e 100644
> --- a/arch/arm/mach-imx/common.h
> +++ b/arch/arm/mach-imx/common.h
> @@ -109,17 +109,17 @@ void imx_cpu_die(unsigned int cpu);
>  int imx_cpu_kill(unsigned int cpu);
>  
>  #ifdef CONFIG_SUSPEND
> -void v7_cpu_resume(void);
>  void imx53_suspend(void __iomem *ocram_vbase);
>  extern const u32 imx53_suspend_sz;
>  void imx6_suspend(void __iomem *ocram_vbase);
>  #else
> -static inline void v7_cpu_resume(void) {}
>  static inline void imx53_suspend(void __iomem *ocram_vbase) {}
>  static const u32 imx53_suspend_sz;
>  static inline void imx6_suspend(void __iomem *ocram_vbase) {}
>  #endif
>  
> +void v7_cpu_resume(void);
> +
>  void imx6_pm_ccm_init(const char *ccm_compat);
>  void imx6q_pm_init(void);
>  void imx6dl_pm_init(void);
> diff --git a/arch/arm/mach-imx/resume-imx6.S b/arch/arm/mach-imx/resume-imx6.S
> new file mode 100644
> index 000000000000..5bd1ba7ef15b
> --- /dev/null
> +++ b/arch/arm/mach-imx/resume-imx6.S
> @@ -0,0 +1,24 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright 2014 Freescale Semiconductor, Inc.
> + */
> +
> +#include <linux/linkage.h>
> +#include <asm/assembler.h>
> +#include <asm/asm-offsets.h>
> +#include <asm/hardware/cache-l2x0.h>
> +#include "hardware.h"
> +
> +/*
> + * The following code must assume it is running from physical address
> + * where absolute virtual addresses to the data section have to be
> + * turned into relative ones.
> + */
> +
> +ENTRY(v7_cpu_resume)
> +	bl	v7_invalidate_l1
> +#ifdef CONFIG_CACHE_L2X0
> +	bl	l2c310_early_resume
> +#endif
> +	b	cpu_resume
> +ENDPROC(v7_cpu_resume)
> diff --git a/arch/arm/mach-imx/suspend-imx6.S b/arch/arm/mach-imx/suspend-imx6.S
> index 062391ff13da..1eabf2d2834b 100644
> --- a/arch/arm/mach-imx/suspend-imx6.S
> +++ b/arch/arm/mach-imx/suspend-imx6.S
> @@ -327,17 +327,3 @@ resume:
>  
>  	ret	lr
>  ENDPROC(imx6_suspend)
> -
> -/*
> - * The following code must assume it is running from physical address
> - * where absolute virtual addresses to the data section have to be
> - * turned into relative ones.
> - */
> -
> -ENTRY(v7_cpu_resume)
> -	bl	v7_invalidate_l1
> -#ifdef CONFIG_CACHE_L2X0
> -	bl	l2c310_early_resume
> -#endif
> -	b	cpu_resume
> -ENDPROC(v7_cpu_resume)
> -- 
> 2.25.0

This patch broke the build for our i.MX6 kernel.

I am referring to commits 512a928aff in mainline and
7199cb65bb in linux-stable.

In our kernel, neither CONFIG_PM nor CONFIG_SUSPEND are set. Therefore,
ARM_CPU_SUSPEND is also unset, which means that sleep.S (containing
cpu_resume) is not built.

With this patch, ld reports the following error:
arch/arm/mach-imx/resume-imx6.o: in function `v7_cpu_resume':
(.text+0x8): undefined reference to `cpu_resume'

Best regards,
Clemens

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

* Re: [PATCH] ARM: imx: build v7_cpu_resume() unconditionally
@ 2020-03-22 18:50   ` Clemens Gruber
  0 siblings, 0 replies; 14+ messages in thread
From: Clemens Gruber @ 2020-03-22 18:50 UTC (permalink / raw)
  To: Rouven Czerwinski
  Cc: Ahmad Fatoum, festevam, s.hauer, linux-kernel, stable, linux-imx,
	kernel, Greg Kroah-Hartman, shawnguo, linux-arm-kernel,
	Lucas Stach

Hi,

On Thu, Jan 16, 2020 at 03:18:49PM +0100, Rouven Czerwinski wrote:
> From: Ahmad Fatoum <a.fatoum@pengutronix.de>
> 
> This function is not only needed by the platform suspend code, but is also
> reused as the CPU resume function when the ARM cores can be powered down
> completely in deep idle, which is the case on i.MX6SX and i.MX6UL(L).
> 
> Providing the static inline stub whenever CONFIG_SUSPEND is disabled means
> that those platforms will hang on resume from cpuidle if suspend is disabled.
> 
> So there are two problems:
> 
>   - The static inline stub masks the linker error
>   - The function is not available where needed
> 
> Fix both by just building the function unconditionally, when
> CONFIG_SOC_IMX6 is enabled. The actual code is three instructions long,
> so it's arguably ok to just leave it in for all i.MX6 kernel configurations.
> 
> Fixes: 05136f0897b5 ("ARM: imx: support arm power off in cpuidle for i.mx6sx")
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> Signed-off-by: Rouven Czerwinski <r.czerwinski@pengutronix.de>
> ---
>  arch/arm/mach-imx/Makefile       |  2 ++
>  arch/arm/mach-imx/common.h       |  4 ++--
>  arch/arm/mach-imx/resume-imx6.S  | 24 ++++++++++++++++++++++++
>  arch/arm/mach-imx/suspend-imx6.S | 14 --------------
>  4 files changed, 28 insertions(+), 16 deletions(-)
>  create mode 100644 arch/arm/mach-imx/resume-imx6.S
> 
> diff --git a/arch/arm/mach-imx/Makefile b/arch/arm/mach-imx/Makefile
> index 35ff620537e6..03506ce46149 100644
> --- a/arch/arm/mach-imx/Makefile
> +++ b/arch/arm/mach-imx/Makefile
> @@ -91,6 +91,8 @@ AFLAGS_suspend-imx6.o :=-Wa,-march=armv7-a
>  obj-$(CONFIG_SOC_IMX6) += suspend-imx6.o
>  obj-$(CONFIG_SOC_IMX53) += suspend-imx53.o
>  endif
> +AFLAGS_resume-imx6.o :=-Wa,-march=armv7-a
> +obj-$(CONFIG_SOC_IMX6) += resume-imx6.o
>  obj-$(CONFIG_SOC_IMX6) += pm-imx6.o
>  
>  obj-$(CONFIG_SOC_IMX1) += mach-imx1.o
> diff --git a/arch/arm/mach-imx/common.h b/arch/arm/mach-imx/common.h
> index 912aeceb4ff8..5aa5796cff0e 100644
> --- a/arch/arm/mach-imx/common.h
> +++ b/arch/arm/mach-imx/common.h
> @@ -109,17 +109,17 @@ void imx_cpu_die(unsigned int cpu);
>  int imx_cpu_kill(unsigned int cpu);
>  
>  #ifdef CONFIG_SUSPEND
> -void v7_cpu_resume(void);
>  void imx53_suspend(void __iomem *ocram_vbase);
>  extern const u32 imx53_suspend_sz;
>  void imx6_suspend(void __iomem *ocram_vbase);
>  #else
> -static inline void v7_cpu_resume(void) {}
>  static inline void imx53_suspend(void __iomem *ocram_vbase) {}
>  static const u32 imx53_suspend_sz;
>  static inline void imx6_suspend(void __iomem *ocram_vbase) {}
>  #endif
>  
> +void v7_cpu_resume(void);
> +
>  void imx6_pm_ccm_init(const char *ccm_compat);
>  void imx6q_pm_init(void);
>  void imx6dl_pm_init(void);
> diff --git a/arch/arm/mach-imx/resume-imx6.S b/arch/arm/mach-imx/resume-imx6.S
> new file mode 100644
> index 000000000000..5bd1ba7ef15b
> --- /dev/null
> +++ b/arch/arm/mach-imx/resume-imx6.S
> @@ -0,0 +1,24 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright 2014 Freescale Semiconductor, Inc.
> + */
> +
> +#include <linux/linkage.h>
> +#include <asm/assembler.h>
> +#include <asm/asm-offsets.h>
> +#include <asm/hardware/cache-l2x0.h>
> +#include "hardware.h"
> +
> +/*
> + * The following code must assume it is running from physical address
> + * where absolute virtual addresses to the data section have to be
> + * turned into relative ones.
> + */
> +
> +ENTRY(v7_cpu_resume)
> +	bl	v7_invalidate_l1
> +#ifdef CONFIG_CACHE_L2X0
> +	bl	l2c310_early_resume
> +#endif
> +	b	cpu_resume
> +ENDPROC(v7_cpu_resume)
> diff --git a/arch/arm/mach-imx/suspend-imx6.S b/arch/arm/mach-imx/suspend-imx6.S
> index 062391ff13da..1eabf2d2834b 100644
> --- a/arch/arm/mach-imx/suspend-imx6.S
> +++ b/arch/arm/mach-imx/suspend-imx6.S
> @@ -327,17 +327,3 @@ resume:
>  
>  	ret	lr
>  ENDPROC(imx6_suspend)
> -
> -/*
> - * The following code must assume it is running from physical address
> - * where absolute virtual addresses to the data section have to be
> - * turned into relative ones.
> - */
> -
> -ENTRY(v7_cpu_resume)
> -	bl	v7_invalidate_l1
> -#ifdef CONFIG_CACHE_L2X0
> -	bl	l2c310_early_resume
> -#endif
> -	b	cpu_resume
> -ENDPROC(v7_cpu_resume)
> -- 
> 2.25.0

This patch broke the build for our i.MX6 kernel.

I am referring to commits 512a928aff in mainline and
7199cb65bb in linux-stable.

In our kernel, neither CONFIG_PM nor CONFIG_SUSPEND are set. Therefore,
ARM_CPU_SUSPEND is also unset, which means that sleep.S (containing
cpu_resume) is not built.

With this patch, ld reports the following error:
arch/arm/mach-imx/resume-imx6.o: in function `v7_cpu_resume':
(.text+0x8): undefined reference to `cpu_resume'

Best regards,
Clemens

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH] ARM: imx: build v7_cpu_resume() unconditionally
  2020-03-22 18:50   ` Clemens Gruber
@ 2020-03-22 20:22     ` Sasha Levin
  -1 siblings, 0 replies; 14+ messages in thread
From: Sasha Levin @ 2020-03-22 20:22 UTC (permalink / raw)
  To: 20200116141849.73955-1-r.czerwinski
  Cc: Rouven Czerwinski, s.hauer, shawnguo, Ahmad Fatoum, linux-kernel,
	linux-imx, kernel, festevam, linux-arm-kernel, Lucas Stach,
	stable, Greg Kroah-Hartman

On Sun, Mar 22, 2020 at 07:50:22PM +0100, Clemens Gruber wrote:
>Hi,
>
>On Thu, Jan 16, 2020 at 03:18:49PM +0100, Rouven Czerwinski wrote:
>> From: Ahmad Fatoum <a.fatoum@pengutronix.de>
>>
>> This function is not only needed by the platform suspend code, but is also
>> reused as the CPU resume function when the ARM cores can be powered down
>> completely in deep idle, which is the case on i.MX6SX and i.MX6UL(L).
>>
>> Providing the static inline stub whenever CONFIG_SUSPEND is disabled means
>> that those platforms will hang on resume from cpuidle if suspend is disabled.
>>
>> So there are two problems:
>>
>>   - The static inline stub masks the linker error
>>   - The function is not available where needed
>>
>> Fix both by just building the function unconditionally, when
>> CONFIG_SOC_IMX6 is enabled. The actual code is three instructions long,
>> so it's arguably ok to just leave it in for all i.MX6 kernel configurations.
>>
>> Fixes: 05136f0897b5 ("ARM: imx: support arm power off in cpuidle for i.mx6sx")
>> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>> Signed-off-by: Rouven Czerwinski <r.czerwinski@pengutronix.de>
>> ---
>>  arch/arm/mach-imx/Makefile       |  2 ++
>>  arch/arm/mach-imx/common.h       |  4 ++--
>>  arch/arm/mach-imx/resume-imx6.S  | 24 ++++++++++++++++++++++++
>>  arch/arm/mach-imx/suspend-imx6.S | 14 --------------
>>  4 files changed, 28 insertions(+), 16 deletions(-)
>>  create mode 100644 arch/arm/mach-imx/resume-imx6.S
>>
>> diff --git a/arch/arm/mach-imx/Makefile b/arch/arm/mach-imx/Makefile
>> index 35ff620537e6..03506ce46149 100644
>> --- a/arch/arm/mach-imx/Makefile
>> +++ b/arch/arm/mach-imx/Makefile
>> @@ -91,6 +91,8 @@ AFLAGS_suspend-imx6.o :=-Wa,-march=armv7-a
>>  obj-$(CONFIG_SOC_IMX6) += suspend-imx6.o
>>  obj-$(CONFIG_SOC_IMX53) += suspend-imx53.o
>>  endif
>> +AFLAGS_resume-imx6.o :=-Wa,-march=armv7-a
>> +obj-$(CONFIG_SOC_IMX6) += resume-imx6.o
>>  obj-$(CONFIG_SOC_IMX6) += pm-imx6.o
>>
>>  obj-$(CONFIG_SOC_IMX1) += mach-imx1.o
>> diff --git a/arch/arm/mach-imx/common.h b/arch/arm/mach-imx/common.h
>> index 912aeceb4ff8..5aa5796cff0e 100644
>> --- a/arch/arm/mach-imx/common.h
>> +++ b/arch/arm/mach-imx/common.h
>> @@ -109,17 +109,17 @@ void imx_cpu_die(unsigned int cpu);
>>  int imx_cpu_kill(unsigned int cpu);
>>
>>  #ifdef CONFIG_SUSPEND
>> -void v7_cpu_resume(void);
>>  void imx53_suspend(void __iomem *ocram_vbase);
>>  extern const u32 imx53_suspend_sz;
>>  void imx6_suspend(void __iomem *ocram_vbase);
>>  #else
>> -static inline void v7_cpu_resume(void) {}
>>  static inline void imx53_suspend(void __iomem *ocram_vbase) {}
>>  static const u32 imx53_suspend_sz;
>>  static inline void imx6_suspend(void __iomem *ocram_vbase) {}
>>  #endif
>>
>> +void v7_cpu_resume(void);
>> +
>>  void imx6_pm_ccm_init(const char *ccm_compat);
>>  void imx6q_pm_init(void);
>>  void imx6dl_pm_init(void);
>> diff --git a/arch/arm/mach-imx/resume-imx6.S b/arch/arm/mach-imx/resume-imx6.S
>> new file mode 100644
>> index 000000000000..5bd1ba7ef15b
>> --- /dev/null
>> +++ b/arch/arm/mach-imx/resume-imx6.S
>> @@ -0,0 +1,24 @@
>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
>> +/*
>> + * Copyright 2014 Freescale Semiconductor, Inc.
>> + */
>> +
>> +#include <linux/linkage.h>
>> +#include <asm/assembler.h>
>> +#include <asm/asm-offsets.h>
>> +#include <asm/hardware/cache-l2x0.h>
>> +#include "hardware.h"
>> +
>> +/*
>> + * The following code must assume it is running from physical address
>> + * where absolute virtual addresses to the data section have to be
>> + * turned into relative ones.
>> + */
>> +
>> +ENTRY(v7_cpu_resume)
>> +	bl	v7_invalidate_l1
>> +#ifdef CONFIG_CACHE_L2X0
>> +	bl	l2c310_early_resume
>> +#endif
>> +	b	cpu_resume
>> +ENDPROC(v7_cpu_resume)
>> diff --git a/arch/arm/mach-imx/suspend-imx6.S b/arch/arm/mach-imx/suspend-imx6.S
>> index 062391ff13da..1eabf2d2834b 100644
>> --- a/arch/arm/mach-imx/suspend-imx6.S
>> +++ b/arch/arm/mach-imx/suspend-imx6.S
>> @@ -327,17 +327,3 @@ resume:
>>
>>  	ret	lr
>>  ENDPROC(imx6_suspend)
>> -
>> -/*
>> - * The following code must assume it is running from physical address
>> - * where absolute virtual addresses to the data section have to be
>> - * turned into relative ones.
>> - */
>> -
>> -ENTRY(v7_cpu_resume)
>> -	bl	v7_invalidate_l1
>> -#ifdef CONFIG_CACHE_L2X0
>> -	bl	l2c310_early_resume
>> -#endif
>> -	b	cpu_resume
>> -ENDPROC(v7_cpu_resume)
>> --
>> 2.25.0
>
>This patch broke the build for our i.MX6 kernel.
>
>I am referring to commits 512a928aff in mainline and
>7199cb65bb in linux-stable.
>
>In our kernel, neither CONFIG_PM nor CONFIG_SUSPEND are set. Therefore,
>ARM_CPU_SUSPEND is also unset, which means that sleep.S (containing
>cpu_resume) is not built.
>
>With this patch, ld reports the following error:
>arch/arm/mach-imx/resume-imx6.o: in function `v7_cpu_resume':
>(.text+0x8): undefined reference to `cpu_resume'

Is this a problem with Linus's tree as well?

-- 
Thanks,
Sasha

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

* Re: [PATCH] ARM: imx: build v7_cpu_resume() unconditionally
@ 2020-03-22 20:22     ` Sasha Levin
  0 siblings, 0 replies; 14+ messages in thread
From: Sasha Levin @ 2020-03-22 20:22 UTC (permalink / raw)
  To: 20200116141849.73955-1-r.czerwinski
  Cc: Ahmad Fatoum, festevam, s.hauer, linux-kernel, stable, linux-imx,
	kernel, Greg Kroah-Hartman, Rouven Czerwinski, shawnguo,
	linux-arm-kernel, Lucas Stach

On Sun, Mar 22, 2020 at 07:50:22PM +0100, Clemens Gruber wrote:
>Hi,
>
>On Thu, Jan 16, 2020 at 03:18:49PM +0100, Rouven Czerwinski wrote:
>> From: Ahmad Fatoum <a.fatoum@pengutronix.de>
>>
>> This function is not only needed by the platform suspend code, but is also
>> reused as the CPU resume function when the ARM cores can be powered down
>> completely in deep idle, which is the case on i.MX6SX and i.MX6UL(L).
>>
>> Providing the static inline stub whenever CONFIG_SUSPEND is disabled means
>> that those platforms will hang on resume from cpuidle if suspend is disabled.
>>
>> So there are two problems:
>>
>>   - The static inline stub masks the linker error
>>   - The function is not available where needed
>>
>> Fix both by just building the function unconditionally, when
>> CONFIG_SOC_IMX6 is enabled. The actual code is three instructions long,
>> so it's arguably ok to just leave it in for all i.MX6 kernel configurations.
>>
>> Fixes: 05136f0897b5 ("ARM: imx: support arm power off in cpuidle for i.mx6sx")
>> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>> Signed-off-by: Rouven Czerwinski <r.czerwinski@pengutronix.de>
>> ---
>>  arch/arm/mach-imx/Makefile       |  2 ++
>>  arch/arm/mach-imx/common.h       |  4 ++--
>>  arch/arm/mach-imx/resume-imx6.S  | 24 ++++++++++++++++++++++++
>>  arch/arm/mach-imx/suspend-imx6.S | 14 --------------
>>  4 files changed, 28 insertions(+), 16 deletions(-)
>>  create mode 100644 arch/arm/mach-imx/resume-imx6.S
>>
>> diff --git a/arch/arm/mach-imx/Makefile b/arch/arm/mach-imx/Makefile
>> index 35ff620537e6..03506ce46149 100644
>> --- a/arch/arm/mach-imx/Makefile
>> +++ b/arch/arm/mach-imx/Makefile
>> @@ -91,6 +91,8 @@ AFLAGS_suspend-imx6.o :=-Wa,-march=armv7-a
>>  obj-$(CONFIG_SOC_IMX6) += suspend-imx6.o
>>  obj-$(CONFIG_SOC_IMX53) += suspend-imx53.o
>>  endif
>> +AFLAGS_resume-imx6.o :=-Wa,-march=armv7-a
>> +obj-$(CONFIG_SOC_IMX6) += resume-imx6.o
>>  obj-$(CONFIG_SOC_IMX6) += pm-imx6.o
>>
>>  obj-$(CONFIG_SOC_IMX1) += mach-imx1.o
>> diff --git a/arch/arm/mach-imx/common.h b/arch/arm/mach-imx/common.h
>> index 912aeceb4ff8..5aa5796cff0e 100644
>> --- a/arch/arm/mach-imx/common.h
>> +++ b/arch/arm/mach-imx/common.h
>> @@ -109,17 +109,17 @@ void imx_cpu_die(unsigned int cpu);
>>  int imx_cpu_kill(unsigned int cpu);
>>
>>  #ifdef CONFIG_SUSPEND
>> -void v7_cpu_resume(void);
>>  void imx53_suspend(void __iomem *ocram_vbase);
>>  extern const u32 imx53_suspend_sz;
>>  void imx6_suspend(void __iomem *ocram_vbase);
>>  #else
>> -static inline void v7_cpu_resume(void) {}
>>  static inline void imx53_suspend(void __iomem *ocram_vbase) {}
>>  static const u32 imx53_suspend_sz;
>>  static inline void imx6_suspend(void __iomem *ocram_vbase) {}
>>  #endif
>>
>> +void v7_cpu_resume(void);
>> +
>>  void imx6_pm_ccm_init(const char *ccm_compat);
>>  void imx6q_pm_init(void);
>>  void imx6dl_pm_init(void);
>> diff --git a/arch/arm/mach-imx/resume-imx6.S b/arch/arm/mach-imx/resume-imx6.S
>> new file mode 100644
>> index 000000000000..5bd1ba7ef15b
>> --- /dev/null
>> +++ b/arch/arm/mach-imx/resume-imx6.S
>> @@ -0,0 +1,24 @@
>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
>> +/*
>> + * Copyright 2014 Freescale Semiconductor, Inc.
>> + */
>> +
>> +#include <linux/linkage.h>
>> +#include <asm/assembler.h>
>> +#include <asm/asm-offsets.h>
>> +#include <asm/hardware/cache-l2x0.h>
>> +#include "hardware.h"
>> +
>> +/*
>> + * The following code must assume it is running from physical address
>> + * where absolute virtual addresses to the data section have to be
>> + * turned into relative ones.
>> + */
>> +
>> +ENTRY(v7_cpu_resume)
>> +	bl	v7_invalidate_l1
>> +#ifdef CONFIG_CACHE_L2X0
>> +	bl	l2c310_early_resume
>> +#endif
>> +	b	cpu_resume
>> +ENDPROC(v7_cpu_resume)
>> diff --git a/arch/arm/mach-imx/suspend-imx6.S b/arch/arm/mach-imx/suspend-imx6.S
>> index 062391ff13da..1eabf2d2834b 100644
>> --- a/arch/arm/mach-imx/suspend-imx6.S
>> +++ b/arch/arm/mach-imx/suspend-imx6.S
>> @@ -327,17 +327,3 @@ resume:
>>
>>  	ret	lr
>>  ENDPROC(imx6_suspend)
>> -
>> -/*
>> - * The following code must assume it is running from physical address
>> - * where absolute virtual addresses to the data section have to be
>> - * turned into relative ones.
>> - */
>> -
>> -ENTRY(v7_cpu_resume)
>> -	bl	v7_invalidate_l1
>> -#ifdef CONFIG_CACHE_L2X0
>> -	bl	l2c310_early_resume
>> -#endif
>> -	b	cpu_resume
>> -ENDPROC(v7_cpu_resume)
>> --
>> 2.25.0
>
>This patch broke the build for our i.MX6 kernel.
>
>I am referring to commits 512a928aff in mainline and
>7199cb65bb in linux-stable.
>
>In our kernel, neither CONFIG_PM nor CONFIG_SUSPEND are set. Therefore,
>ARM_CPU_SUSPEND is also unset, which means that sleep.S (containing
>cpu_resume) is not built.
>
>With this patch, ld reports the following error:
>arch/arm/mach-imx/resume-imx6.o: in function `v7_cpu_resume':
>(.text+0x8): undefined reference to `cpu_resume'

Is this a problem with Linus's tree as well?

-- 
Thanks,
Sasha

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH] ARM: imx: build v7_cpu_resume() unconditionally
  2020-03-22 20:22     ` Sasha Levin
@ 2020-03-22 23:09       ` Clemens Gruber
  -1 siblings, 0 replies; 14+ messages in thread
From: Clemens Gruber @ 2020-03-22 23:09 UTC (permalink / raw)
  To: Sasha Levin
  Cc: Ahmad Fatoum, festevam, s.hauer, linux-kernel, stable, linux-imx,
	kernel, Greg Kroah-Hartman, Rouven Czerwinski, shawnguo,
	linux-arm-kernel, Lucas Stach, Clemens Gruber

On Sun, Mar 22, 2020 at 04:22:24PM -0400, Sasha Levin wrote:
> On Sun, Mar 22, 2020 at 07:50:22PM +0100, Clemens Gruber wrote:
> > Hi,
> > 
> > On Thu, Jan 16, 2020 at 03:18:49PM +0100, Rouven Czerwinski wrote:
> > > From: Ahmad Fatoum <a.fatoum@pengutronix.de>
> > > 
> > > This function is not only needed by the platform suspend code, but is also
> > > reused as the CPU resume function when the ARM cores can be powered down
> > > completely in deep idle, which is the case on i.MX6SX and i.MX6UL(L).
> > > 
> > > Providing the static inline stub whenever CONFIG_SUSPEND is disabled means
> > > that those platforms will hang on resume from cpuidle if suspend is disabled.
> > > 
> > > So there are two problems:
> > > 
> > >   - The static inline stub masks the linker error
> > >   - The function is not available where needed
> > > 
> > > Fix both by just building the function unconditionally, when
> > > CONFIG_SOC_IMX6 is enabled. The actual code is three instructions long,
> > > so it's arguably ok to just leave it in for all i.MX6 kernel configurations.
> > > 
> > > Fixes: 05136f0897b5 ("ARM: imx: support arm power off in cpuidle for i.mx6sx")
> > > Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> > > Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> > > Signed-off-by: Rouven Czerwinski <r.czerwinski@pengutronix.de>
> > > ---
> > >  arch/arm/mach-imx/Makefile       |  2 ++
> > >  arch/arm/mach-imx/common.h       |  4 ++--
> > >  arch/arm/mach-imx/resume-imx6.S  | 24 ++++++++++++++++++++++++
> > >  arch/arm/mach-imx/suspend-imx6.S | 14 --------------
> > >  4 files changed, 28 insertions(+), 16 deletions(-)
> > >  create mode 100644 arch/arm/mach-imx/resume-imx6.S
> > > 
> > > diff --git a/arch/arm/mach-imx/Makefile b/arch/arm/mach-imx/Makefile
> > > index 35ff620537e6..03506ce46149 100644
> > > --- a/arch/arm/mach-imx/Makefile
> > > +++ b/arch/arm/mach-imx/Makefile
> > > @@ -91,6 +91,8 @@ AFLAGS_suspend-imx6.o :=-Wa,-march=armv7-a
> > >  obj-$(CONFIG_SOC_IMX6) += suspend-imx6.o
> > >  obj-$(CONFIG_SOC_IMX53) += suspend-imx53.o
> > >  endif
> > > +AFLAGS_resume-imx6.o :=-Wa,-march=armv7-a
> > > +obj-$(CONFIG_SOC_IMX6) += resume-imx6.o
> > >  obj-$(CONFIG_SOC_IMX6) += pm-imx6.o
> > > 
> > >  obj-$(CONFIG_SOC_IMX1) += mach-imx1.o
> > > diff --git a/arch/arm/mach-imx/common.h b/arch/arm/mach-imx/common.h
> > > index 912aeceb4ff8..5aa5796cff0e 100644
> > > --- a/arch/arm/mach-imx/common.h
> > > +++ b/arch/arm/mach-imx/common.h
> > > @@ -109,17 +109,17 @@ void imx_cpu_die(unsigned int cpu);
> > >  int imx_cpu_kill(unsigned int cpu);
> > > 
> > >  #ifdef CONFIG_SUSPEND
> > > -void v7_cpu_resume(void);
> > >  void imx53_suspend(void __iomem *ocram_vbase);
> > >  extern const u32 imx53_suspend_sz;
> > >  void imx6_suspend(void __iomem *ocram_vbase);
> > >  #else
> > > -static inline void v7_cpu_resume(void) {}
> > >  static inline void imx53_suspend(void __iomem *ocram_vbase) {}
> > >  static const u32 imx53_suspend_sz;
> > >  static inline void imx6_suspend(void __iomem *ocram_vbase) {}
> > >  #endif
> > > 
> > > +void v7_cpu_resume(void);
> > > +
> > >  void imx6_pm_ccm_init(const char *ccm_compat);
> > >  void imx6q_pm_init(void);
> > >  void imx6dl_pm_init(void);
> > > diff --git a/arch/arm/mach-imx/resume-imx6.S b/arch/arm/mach-imx/resume-imx6.S
> > > new file mode 100644
> > > index 000000000000..5bd1ba7ef15b
> > > --- /dev/null
> > > +++ b/arch/arm/mach-imx/resume-imx6.S
> > > @@ -0,0 +1,24 @@
> > > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > > +/*
> > > + * Copyright 2014 Freescale Semiconductor, Inc.
> > > + */
> > > +
> > > +#include <linux/linkage.h>
> > > +#include <asm/assembler.h>
> > > +#include <asm/asm-offsets.h>
> > > +#include <asm/hardware/cache-l2x0.h>
> > > +#include "hardware.h"
> > > +
> > > +/*
> > > + * The following code must assume it is running from physical address
> > > + * where absolute virtual addresses to the data section have to be
> > > + * turned into relative ones.
> > > + */
> > > +
> > > +ENTRY(v7_cpu_resume)
> > > +	bl	v7_invalidate_l1
> > > +#ifdef CONFIG_CACHE_L2X0
> > > +	bl	l2c310_early_resume
> > > +#endif
> > > +	b	cpu_resume
> > > +ENDPROC(v7_cpu_resume)
> > > diff --git a/arch/arm/mach-imx/suspend-imx6.S b/arch/arm/mach-imx/suspend-imx6.S
> > > index 062391ff13da..1eabf2d2834b 100644
> > > --- a/arch/arm/mach-imx/suspend-imx6.S
> > > +++ b/arch/arm/mach-imx/suspend-imx6.S
> > > @@ -327,17 +327,3 @@ resume:
> > > 
> > >  	ret	lr
> > >  ENDPROC(imx6_suspend)
> > > -
> > > -/*
> > > - * The following code must assume it is running from physical address
> > > - * where absolute virtual addresses to the data section have to be
> > > - * turned into relative ones.
> > > - */
> > > -
> > > -ENTRY(v7_cpu_resume)
> > > -	bl	v7_invalidate_l1
> > > -#ifdef CONFIG_CACHE_L2X0
> > > -	bl	l2c310_early_resume
> > > -#endif
> > > -	b	cpu_resume
> > > -ENDPROC(v7_cpu_resume)
> > > --
> > > 2.25.0
> > 
> > This patch broke the build for our i.MX6 kernel.
> > 
> > I am referring to commits 512a928aff in mainline and
> > 7199cb65bb in linux-stable.
> > 
> > In our kernel, neither CONFIG_PM nor CONFIG_SUSPEND are set. Therefore,
> > ARM_CPU_SUSPEND is also unset, which means that sleep.S (containing
> > cpu_resume) is not built.
> > 
> > With this patch, ld reports the following error:
> > arch/arm/mach-imx/resume-imx6.o: in function `v7_cpu_resume':
> > (.text+0x8): undefined reference to `cpu_resume'
> 
> Is this a problem with Linus's tree as well?

Yes, it affects Linus's tree and linux-stable.

Reverting 512a928aff in mainline (7199cb65bb in linux-stable) fixes the
build error for us.

Thanks,
Clemens

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

* Re: [PATCH] ARM: imx: build v7_cpu_resume() unconditionally
@ 2020-03-22 23:09       ` Clemens Gruber
  0 siblings, 0 replies; 14+ messages in thread
From: Clemens Gruber @ 2020-03-22 23:09 UTC (permalink / raw)
  To: Sasha Levin
  Cc: Clemens Gruber, Ahmad Fatoum, Greg Kroah-Hartman, s.hauer,
	linux-kernel, stable, linux-imx, kernel, Rouven Czerwinski,
	festevam, shawnguo, linux-arm-kernel, Lucas Stach

On Sun, Mar 22, 2020 at 04:22:24PM -0400, Sasha Levin wrote:
> On Sun, Mar 22, 2020 at 07:50:22PM +0100, Clemens Gruber wrote:
> > Hi,
> > 
> > On Thu, Jan 16, 2020 at 03:18:49PM +0100, Rouven Czerwinski wrote:
> > > From: Ahmad Fatoum <a.fatoum@pengutronix.de>
> > > 
> > > This function is not only needed by the platform suspend code, but is also
> > > reused as the CPU resume function when the ARM cores can be powered down
> > > completely in deep idle, which is the case on i.MX6SX and i.MX6UL(L).
> > > 
> > > Providing the static inline stub whenever CONFIG_SUSPEND is disabled means
> > > that those platforms will hang on resume from cpuidle if suspend is disabled.
> > > 
> > > So there are two problems:
> > > 
> > >   - The static inline stub masks the linker error
> > >   - The function is not available where needed
> > > 
> > > Fix both by just building the function unconditionally, when
> > > CONFIG_SOC_IMX6 is enabled. The actual code is three instructions long,
> > > so it's arguably ok to just leave it in for all i.MX6 kernel configurations.
> > > 
> > > Fixes: 05136f0897b5 ("ARM: imx: support arm power off in cpuidle for i.mx6sx")
> > > Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> > > Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> > > Signed-off-by: Rouven Czerwinski <r.czerwinski@pengutronix.de>
> > > ---
> > >  arch/arm/mach-imx/Makefile       |  2 ++
> > >  arch/arm/mach-imx/common.h       |  4 ++--
> > >  arch/arm/mach-imx/resume-imx6.S  | 24 ++++++++++++++++++++++++
> > >  arch/arm/mach-imx/suspend-imx6.S | 14 --------------
> > >  4 files changed, 28 insertions(+), 16 deletions(-)
> > >  create mode 100644 arch/arm/mach-imx/resume-imx6.S
> > > 
> > > diff --git a/arch/arm/mach-imx/Makefile b/arch/arm/mach-imx/Makefile
> > > index 35ff620537e6..03506ce46149 100644
> > > --- a/arch/arm/mach-imx/Makefile
> > > +++ b/arch/arm/mach-imx/Makefile
> > > @@ -91,6 +91,8 @@ AFLAGS_suspend-imx6.o :=-Wa,-march=armv7-a
> > >  obj-$(CONFIG_SOC_IMX6) += suspend-imx6.o
> > >  obj-$(CONFIG_SOC_IMX53) += suspend-imx53.o
> > >  endif
> > > +AFLAGS_resume-imx6.o :=-Wa,-march=armv7-a
> > > +obj-$(CONFIG_SOC_IMX6) += resume-imx6.o
> > >  obj-$(CONFIG_SOC_IMX6) += pm-imx6.o
> > > 
> > >  obj-$(CONFIG_SOC_IMX1) += mach-imx1.o
> > > diff --git a/arch/arm/mach-imx/common.h b/arch/arm/mach-imx/common.h
> > > index 912aeceb4ff8..5aa5796cff0e 100644
> > > --- a/arch/arm/mach-imx/common.h
> > > +++ b/arch/arm/mach-imx/common.h
> > > @@ -109,17 +109,17 @@ void imx_cpu_die(unsigned int cpu);
> > >  int imx_cpu_kill(unsigned int cpu);
> > > 
> > >  #ifdef CONFIG_SUSPEND
> > > -void v7_cpu_resume(void);
> > >  void imx53_suspend(void __iomem *ocram_vbase);
> > >  extern const u32 imx53_suspend_sz;
> > >  void imx6_suspend(void __iomem *ocram_vbase);
> > >  #else
> > > -static inline void v7_cpu_resume(void) {}
> > >  static inline void imx53_suspend(void __iomem *ocram_vbase) {}
> > >  static const u32 imx53_suspend_sz;
> > >  static inline void imx6_suspend(void __iomem *ocram_vbase) {}
> > >  #endif
> > > 
> > > +void v7_cpu_resume(void);
> > > +
> > >  void imx6_pm_ccm_init(const char *ccm_compat);
> > >  void imx6q_pm_init(void);
> > >  void imx6dl_pm_init(void);
> > > diff --git a/arch/arm/mach-imx/resume-imx6.S b/arch/arm/mach-imx/resume-imx6.S
> > > new file mode 100644
> > > index 000000000000..5bd1ba7ef15b
> > > --- /dev/null
> > > +++ b/arch/arm/mach-imx/resume-imx6.S
> > > @@ -0,0 +1,24 @@
> > > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > > +/*
> > > + * Copyright 2014 Freescale Semiconductor, Inc.
> > > + */
> > > +
> > > +#include <linux/linkage.h>
> > > +#include <asm/assembler.h>
> > > +#include <asm/asm-offsets.h>
> > > +#include <asm/hardware/cache-l2x0.h>
> > > +#include "hardware.h"
> > > +
> > > +/*
> > > + * The following code must assume it is running from physical address
> > > + * where absolute virtual addresses to the data section have to be
> > > + * turned into relative ones.
> > > + */
> > > +
> > > +ENTRY(v7_cpu_resume)
> > > +	bl	v7_invalidate_l1
> > > +#ifdef CONFIG_CACHE_L2X0
> > > +	bl	l2c310_early_resume
> > > +#endif
> > > +	b	cpu_resume
> > > +ENDPROC(v7_cpu_resume)
> > > diff --git a/arch/arm/mach-imx/suspend-imx6.S b/arch/arm/mach-imx/suspend-imx6.S
> > > index 062391ff13da..1eabf2d2834b 100644
> > > --- a/arch/arm/mach-imx/suspend-imx6.S
> > > +++ b/arch/arm/mach-imx/suspend-imx6.S
> > > @@ -327,17 +327,3 @@ resume:
> > > 
> > >  	ret	lr
> > >  ENDPROC(imx6_suspend)
> > > -
> > > -/*
> > > - * The following code must assume it is running from physical address
> > > - * where absolute virtual addresses to the data section have to be
> > > - * turned into relative ones.
> > > - */
> > > -
> > > -ENTRY(v7_cpu_resume)
> > > -	bl	v7_invalidate_l1
> > > -#ifdef CONFIG_CACHE_L2X0
> > > -	bl	l2c310_early_resume
> > > -#endif
> > > -	b	cpu_resume
> > > -ENDPROC(v7_cpu_resume)
> > > --
> > > 2.25.0
> > 
> > This patch broke the build for our i.MX6 kernel.
> > 
> > I am referring to commits 512a928aff in mainline and
> > 7199cb65bb in linux-stable.
> > 
> > In our kernel, neither CONFIG_PM nor CONFIG_SUSPEND are set. Therefore,
> > ARM_CPU_SUSPEND is also unset, which means that sleep.S (containing
> > cpu_resume) is not built.
> > 
> > With this patch, ld reports the following error:
> > arch/arm/mach-imx/resume-imx6.o: in function `v7_cpu_resume':
> > (.text+0x8): undefined reference to `cpu_resume'
> 
> Is this a problem with Linus's tree as well?

Yes, it affects Linus's tree and linux-stable.

Reverting 512a928aff in mainline (7199cb65bb in linux-stable) fixes the
build error for us.

Thanks,
Clemens

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH] ARM: imx: build v7_cpu_resume() unconditionally
  2020-03-22 23:09       ` Clemens Gruber
@ 2020-03-23  8:22         ` Ahmad Fatoum
  -1 siblings, 0 replies; 14+ messages in thread
From: Ahmad Fatoum @ 2020-03-23  8:22 UTC (permalink / raw)
  To: Sasha Levin
  Cc: festevam, s.hauer, linux-kernel, stable, linux-imx, kernel,
	Greg Kroah-Hartman, Rouven Czerwinski, shawnguo,
	linux-arm-kernel, Lucas Stach, Clemens Gruber

Hello Clemens

On 3/23/20 12:09 AM, Clemens Gruber wrote:
>> Is this a problem with Linus's tree as well?
> 
> Yes, it affects Linus's tree and linux-stable.
> 
> Reverting 512a928aff in mainline (7199cb65bb in linux-stable) fixes the
> build error for us.

Thanks for the report and sorry for the inconvenience.
I just sent out a patch[1] to fix the linker error.

[1]: https://lore.kernel.org/linux-arm-kernel/20200323081933.31497-1-a.fatoum@pengutronix.de/T/#u

Cheers
Ahmad

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

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

* Re: [PATCH] ARM: imx: build v7_cpu_resume() unconditionally
@ 2020-03-23  8:22         ` Ahmad Fatoum
  0 siblings, 0 replies; 14+ messages in thread
From: Ahmad Fatoum @ 2020-03-23  8:22 UTC (permalink / raw)
  To: Sasha Levin
  Cc: Clemens Gruber, Rouven Czerwinski, Greg Kroah-Hartman, s.hauer,
	linux-kernel, stable, linux-imx, kernel, shawnguo, festevam,
	linux-arm-kernel, Lucas Stach

Hello Clemens

On 3/23/20 12:09 AM, Clemens Gruber wrote:
>> Is this a problem with Linus's tree as well?
> 
> Yes, it affects Linus's tree and linux-stable.
> 
> Reverting 512a928aff in mainline (7199cb65bb in linux-stable) fixes the
> build error for us.

Thanks for the report and sorry for the inconvenience.
I just sent out a patch[1] to fix the linker error.

[1]: https://lore.kernel.org/linux-arm-kernel/20200323081933.31497-1-a.fatoum@pengutronix.de/T/#u

Cheers
Ahmad

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

end of thread, other threads:[~2020-03-23  8:23 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-16 14:18 [PATCH] ARM: imx: build v7_cpu_resume() unconditionally Rouven Czerwinski
2020-01-16 14:18 ` Rouven Czerwinski
2020-02-17  7:41 ` Rouven Czerwinski
2020-02-17  7:41   ` Rouven Czerwinski
2020-02-17  7:52 ` Shawn Guo
2020-02-17  7:52   ` Shawn Guo
2020-03-22 18:50 ` Clemens Gruber
2020-03-22 18:50   ` Clemens Gruber
2020-03-22 20:22   ` Sasha Levin
2020-03-22 20:22     ` Sasha Levin
2020-03-22 23:09     ` Clemens Gruber
2020-03-22 23:09       ` Clemens Gruber
2020-03-23  8:22       ` Ahmad Fatoum
2020-03-23  8:22         ` Ahmad Fatoum

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.