ARM: OMAP2+: hwmod: Change hardreset soc_ops for AM43XX

PATCH New Dave Gerlach 2 participants 6 messages Jun 9, 2014 – Jun 16, 2014

Subsystems (4)

Arm (32-bit) Arm sub-architectures OMAP2+ support OMAP hwmod support

lore

Share & tools

lore

Dave Gerlach

author

2014-06-09 20:59 UTC #0 lore raw

PATCH 1/1 ARM: OMAP2+: hwmod: Change hardreset soc_ops for AM43XX

am43xx reset register layout is more similar to am33xx than omap4 so use the am33xx functions for hwmod hardreset soc_ops rather than the currently used omap4 functions. Without this, assert_hardreset and deassert_hardreset will not work on am43xx.

Signed-off-by Dave Gerlach <[email protected]>

---
 arch/arm/mach-omap2/omap_hwmod.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

arch/arm/mach-omap2/omap_hwmod.c +3 -3

index 66c60fe..1c0885b 100644
@@ -4251,9 +4251,9 @@ void __init omap_hwmod_init(void)
 		soc_ops.enable_module = _omap4_enable_module;
 		soc_ops.disable_module = _omap4_disable_module;
 		soc_ops.wait_target_ready = _omap4_wait_target_ready;
-		soc_ops.assert_hardreset = _omap4_assert_hardreset;
-		soc_ops.deassert_hardreset = _omap4_deassert_hardreset;
-		soc_ops.is_hardreset_asserted = _omap4_is_hardreset_asserted;
+		soc_ops.assert_hardreset = _am33xx_assert_hardreset;
+		soc_ops.deassert_hardreset = _am33xx_deassert_hardreset;
+		soc_ops.is_hardreset_asserted = _am33xx_is_hardreset_asserted;
 		soc_ops.init_clkdm = _init_clkdm;
 	} else if (soc_is_am33xx()) {
 		soc_ops.enable_module = _am33xx_enable_module;

Signature · 1 line

1.9.0

Paul Walmsley

maintainer

2014-06-13 19:40 UTC #1 lore raw

PATCH 1/1 ARM: OMAP2+: hwmod: Change hardreset soc_ops for AM43XX

Quoted 6 lines · Dave Gerlach

am43xx reset register layout is more similar to am33xx than omap4 so use the am33xx functions for hwmod hardreset soc_ops rather than the currently used omap4 functions. Without this, assert_hardreset and deassert_hardreset will not work on am43xx.
Signed-off-by Dave Gerlach <[email protected]>

Makes sense to me; queued for v3.16-rc.

- Paul

Paul Walmsley

maintainer

2014-06-16 01:56 UTC #2 lore raw

PATCH 1/1 ARM: OMAP2+: hwmod: Change hardreset soc_ops for AM43XX

Quoted 10 lines · Paul Walmsley

Quoted 6 lines · Dave Gerlach
am43xx reset register layout is more similar to am33xx than omap4 so use the am33xx functions for hwmod hardreset soc_ops rather than the currently used omap4 functions. Without this, assert_hardreset and deassert_hardreset will not work on am43xx.
Signed-off-by Dave Gerlach <[email protected]>
Makes sense to me; queued for v3.16-rc.

The patch failed a build test for an AM43xx-only config:

----
arch/arm/mach-omap2/built-in.o: In function `_am33xx_is_hardreset_asserted':
arch/arm/mach-omap2/omap_hwmod.c:3223: undefined reference to am33xx_prm_is_hardreset_asserted' arch/arm/mach-omap2/built-in.o: In function _am33xx_deassert_hardreset':
arch/arm/mach-omap2/omap_hwmod.c:3201: undefined reference to am33xx_prm_deassert_hardreset' arch/arm/mach-omap2/built-in.o: In function _am33xx_assert_hardreset':
arch/arm/mach-omap2/omap_hwmod.c:3181:
undefined reference to `am33xx_prm_assert_hardreset'
make: *** [vmlinux] Error 1
----

I went ahead and modified the patch to build (updated patch below), but it would be helpful if you could also build-test your AM43xx-specific patches with an AM43xx-specific .config (along with the usual omap2plus_defconfig).

- Paul

From: Dave Gerlach <[email protected]>
Date: Sun, 15 Jun 2014 16:02:17 -0600
Subject: [PATCH 1/2] ARM: OMAP2+: hwmod: Change hardreset soc_ops for AM43XX

Signed-off-by Dave Gerlach <[email protected]>

[paul at pwsan.com: fixed build errors for an AM43xx-only Kconfig]

Signed-off-by Paul Walmsley <[email protected]>

---
 arch/arm/mach-omap2/Makefile     | 6 ++++--
 arch/arm/mach-omap2/cm33xx.h     | 2 +-
 arch/arm/mach-omap2/omap_hwmod.c | 6 +++---
 3 files changed, 8 insertions(+), 6 deletions(-)

arch/arm/mach-omap2/Makefile +4 -2

index 8421f38cf445..7a695362aee7 100644
@@ -110,14 +110,16 @@ obj-y					+= prm_common.o cm_common.o
 obj-$(CONFIG_ARCH_OMAP2)		+= prm2xxx_3xxx.o prm2xxx.o cm2xxx.o
 obj-$(CONFIG_ARCH_OMAP3)		+= prm2xxx_3xxx.o prm3xxx.o cm3xxx.o
 obj-$(CONFIG_ARCH_OMAP3)		+= vc3xxx_data.o vp3xxx_data.o
-obj-$(CONFIG_SOC_AM33XX)		+= prm33xx.o cm33xx.o
 omap-prcm-4-5-common			=  cminst44xx.o cm44xx.o prm44xx.o \
 					   prcm_mpu44xx.o prminst44xx.o \
 					   vc44xx_data.o vp44xx_data.o
 obj-$(CONFIG_ARCH_OMAP4)		+= $(omap-prcm-4-5-common)
 obj-$(CONFIG_SOC_OMAP5)			+= $(omap-prcm-4-5-common)
 obj-$(CONFIG_SOC_DRA7XX)		+= $(omap-prcm-4-5-common)
-obj-$(CONFIG_SOC_AM43XX)		+= $(omap-prcm-4-5-common)
+am33xx-43xx-prcm-common			+= prm33xx.o
+obj-$(CONFIG_SOC_AM33XX)		+= $(am33xx-43xx-prcm-common) cm33xx.o
+obj-$(CONFIG_SOC_AM43XX)		+= $(omap-prcm-4-5-common) \
+					   $(am33xx-43xx-prcm-common)
 
 # OMAP voltage domains
 voltagedomain-common			:= voltage.o vc.o vp.o

arch/arm/mach-omap2/cm33xx.h +1 -1

index 15a778ce7707..bd2441790779 100644
@@ -380,7 +380,7 @@ void am33xx_cm_clkdm_disable_hwsup(u16 inst, u16 cdoffs);
 void am33xx_cm_clkdm_force_sleep(u16 inst, u16 cdoffs);
 void am33xx_cm_clkdm_force_wakeup(u16 inst, u16 cdoffs);
 
-#ifdef CONFIG_SOC_AM33XX
+#if defined(CONFIG_SOC_AM33XX) || defined(CONFIG_SOC_AM43XX)
 extern int am33xx_cm_wait_module_idle(u16 inst, s16 cdoffs,
 					u16 clkctrl_offs);
 extern void am33xx_cm_module_enable(u8 mode, u16 inst, s16 cdoffs,

arch/arm/mach-omap2/omap_hwmod.c +3 -3

index f7bb435bb543..6c074f37cdd2 100644
@@ -4251,9 +4251,9 @@ void __init omap_hwmod_init(void)
 		soc_ops.enable_module = _omap4_enable_module;
 		soc_ops.disable_module = _omap4_disable_module;
 		soc_ops.wait_target_ready = _omap4_wait_target_ready;
-		soc_ops.assert_hardreset = _omap4_assert_hardreset;
-		soc_ops.deassert_hardreset = _omap4_deassert_hardreset;
-		soc_ops.is_hardreset_asserted = _omap4_is_hardreset_asserted;
+		soc_ops.assert_hardreset = _am33xx_assert_hardreset;
+		soc_ops.deassert_hardreset = _am33xx_deassert_hardreset;
+		soc_ops.is_hardreset_asserted = _am33xx_is_hardreset_asserted;
 		soc_ops.init_clkdm = _init_clkdm;
 	} else if (soc_is_am33xx()) {
 		soc_ops.enable_module = _am33xx_enable_module;

Signature · 1 line

2.0.0

Dave Gerlach

author

2014-06-16 20:11 UTC #3 lore raw

PATCH 1/1 ARM: OMAP2+: hwmod: Change hardreset soc_ops for AM43XX

Quoted 26 lines · Paul Walmsley

Quoted 10 lines · Paul Walmsley
Quoted 6 lines · Dave Gerlach
am43xx reset register layout is more similar to am33xx than omap4 so use the am33xx functions for hwmod hardreset soc_ops rather than the currently used omap4 functions. Without this, assert_hardreset and deassert_hardreset will not work on am43xx.
Signed-off-by Dave Gerlach <[email protected]>
Makes sense to me; queued for v3.16-rc.
The patch failed a build test for an AM43xx-only config:
----
arch/arm/mach-omap2/built-in.o: In function `_am33xx_is_hardreset_asserted':
arch/arm/mach-omap2/omap_hwmod.c:3223: undefined reference to am33xx_prm_is_hardreset_asserted' arch/arm/mach-omap2/built-in.o: In function _am33xx_deassert_hardreset':
arch/arm/mach-omap2/omap_hwmod.c:3201: undefined reference to am33xx_prm_deassert_hardreset' arch/arm/mach-omap2/built-in.o: In function _am33xx_assert_hardreset':
arch/arm/mach-omap2/omap_hwmod.c:3181:
undefined reference to `am33xx_prm_assert_hardreset'
make: *** [vmlinux] Error 1
----

Quoted 5 lines

I went ahead and modified the patch to build (updated patch below), but it would be helpful if you could also build-test your AM43xx-specific patches with an AM43xx-specific .config (along with the usual omap2plus_defconfig).

Sorry about that, I definitely should have tried that, thanks for the fixup!

Regards,
Dave

Quoted 75 lines

- Paul

From: Dave Gerlach <[email protected]>
Date: Sun, 15 Jun 2014 16:02:17 -0600
Subject: [PATCH 1/2] ARM: OMAP2+: hwmod: Change hardreset soc_ops for AM43XX

Signed-off-by Dave Gerlach <[email protected]>

[paul at pwsan.com: fixed build errors for an AM43xx-only Kconfig]

Signed-off-by Paul Walmsley <[email protected]>

---
  arch/arm/mach-omap2/Makefile     | 6 ++++--
  arch/arm/mach-omap2/cm33xx.h     | 2 +-
  arch/arm/mach-omap2/omap_hwmod.c | 6 +++---
  3 files changed, 8 insertions(+), 6 deletions(-)

arch/arm/mach-omap2/Makefile +4 -2

index 8421f38cf445..7a695362aee7 100644
@@ -110,14 +110,16 @@ obj-y					+= prm_common.o cm_common.o
  obj-$(CONFIG_ARCH_OMAP2)		+= prm2xxx_3xxx.o prm2xxx.o cm2xxx.o
  obj-$(CONFIG_ARCH_OMAP3)		+= prm2xxx_3xxx.o prm3xxx.o cm3xxx.o
  obj-$(CONFIG_ARCH_OMAP3)		+= vc3xxx_data.o vp3xxx_data.o
-obj-$(CONFIG_SOC_AM33XX)		+= prm33xx.o cm33xx.o
  omap-prcm-4-5-common			=  cminst44xx.o cm44xx.o prm44xx.o \
  					   prcm_mpu44xx.o prminst44xx.o \
  					   vc44xx_data.o vp44xx_data.o
  obj-$(CONFIG_ARCH_OMAP4)		+= $(omap-prcm-4-5-common)
  obj-$(CONFIG_SOC_OMAP5)			+= $(omap-prcm-4-5-common)
  obj-$(CONFIG_SOC_DRA7XX)		+= $(omap-prcm-4-5-common)
-obj-$(CONFIG_SOC_AM43XX)		+= $(omap-prcm-4-5-common)
+am33xx-43xx-prcm-common			+= prm33xx.o
+obj-$(CONFIG_SOC_AM33XX)		+= $(am33xx-43xx-prcm-common) cm33xx.o
+obj-$(CONFIG_SOC_AM43XX)		+= $(omap-prcm-4-5-common) \
+					   $(am33xx-43xx-prcm-common)
 
  # OMAP voltage domains
  voltagedomain-common			:= voltage.o vc.o vp.o

arch/arm/mach-omap2/cm33xx.h +1 -1

index 15a778ce7707..bd2441790779 100644
@@ -380,7 +380,7 @@ void am33xx_cm_clkdm_disable_hwsup(u16 inst, u16 cdoffs);
  void am33xx_cm_clkdm_force_sleep(u16 inst, u16 cdoffs);
  void am33xx_cm_clkdm_force_wakeup(u16 inst, u16 cdoffs);
 
-#ifdef CONFIG_SOC_AM33XX
+#if defined(CONFIG_SOC_AM33XX) || defined(CONFIG_SOC_AM43XX)
  extern int am33xx_cm_wait_module_idle(u16 inst, s16 cdoffs,
  					u16 clkctrl_offs);
  extern void am33xx_cm_module_enable(u8 mode, u16 inst, s16 cdoffs,

arch/arm/mach-omap2/omap_hwmod.c +3 -3

index f7bb435bb543..6c074f37cdd2 100644
@@ -4251,9 +4251,9 @@ void __init omap_hwmod_init(void)
  		soc_ops.enable_module = _omap4_enable_module;
  		soc_ops.disable_module = _omap4_disable_module;
  		soc_ops.wait_target_ready = _omap4_wait_target_ready;
-		soc_ops.assert_hardreset = _omap4_assert_hardreset;
-		soc_ops.deassert_hardreset = _omap4_deassert_hardreset;
-		soc_ops.is_hardreset_asserted = _omap4_is_hardreset_asserted;
+		soc_ops.assert_hardreset = _am33xx_assert_hardreset;
+		soc_ops.deassert_hardreset = _am33xx_deassert_hardreset;
+		soc_ops.is_hardreset_asserted = _am33xx_is_hardreset_asserted;
  		soc_ops.init_clkdm = _init_clkdm;
  	} else if (soc_is_am33xx()) {
  		soc_ops.enable_module = _am33xx_enable_module;

Paul Walmsley

maintainer

2014-06-16 04:43 UTC #4 lore raw

PATCH 1/1 ARM: OMAP2+: hwmod: Change hardreset soc_ops for AM43XX

In reply to Dave Gerlach

Dave,

Quoted 17 lines · Dave Gerlach

am43xx reset register layout is more similar to am33xx than omap4 so use the am33xx functions for hwmod hardreset soc_ops rather than the currently used omap4 functions. Without this, assert_hardreset and deassert_hardreset will not work on am43xx.
Signed-off-by Dave Gerlach <[email protected]>
---
 arch/arm/mach-omap2/omap_hwmod.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
arch/arm/mach-omap2/omap_hwmod.c +0 -0
index 66c60fe..1c0885b 100644
@@ -4251,9 +4251,9 @@ void __init omap_hwmod_init(void)
 		soc_ops.enable_module = _omap4_enable_module;
 		soc_ops.disable_module = _omap4_disable_module;

arch/arm/mach-omap2/omap_hwmod.c void __init omap_hwmod_init(void) · line 4259

Quoted 10 lines

arch/arm/mach-omap2/omap_hwmod.c +3 -3

 		soc_ops.wait_target_ready = _omap4_wait_target_ready;
-		soc_ops.assert_hardreset = _omap4_assert_hardreset;
-		soc_ops.deassert_hardreset = _omap4_deassert_hardreset;
-		soc_ops.is_hardreset_asserted = _omap4_is_hardreset_asserted;
+		soc_ops.assert_hardreset = _am33xx_assert_hardreset;
+		soc_ops.deassert_hardreset = _am33xx_deassert_hardreset;
+		soc_ops.is_hardreset_asserted = _am33xx_is_hardreset_asserted;
 		soc_ops.init_clkdm = _init_clkdm;
 	} else if (soc_is_am33xx()) {
 		soc_ops.enable_module = _am33xx_enable_module;

Should AM43XX be using the _am33xx_{enable,disable}_module and wait_target_ready functions?

- Paul

Dave Gerlach

author

2014-06-16 20:19 UTC #5 lore raw

PATCH 1/1 ARM: OMAP2+: hwmod: Change hardreset soc_ops for AM43XX

Paul,

Quoted 26 lines · Paul Walmsley

Dave,
Quoted 22 lines · Dave Gerlach
am43xx reset register layout is more similar to am33xx than omap4 so use the am33xx functions for hwmod hardreset soc_ops rather than the currently used omap4 functions. Without this, assert_hardreset and deassert_hardreset will not work on am43xx.
Signed-off-by Dave Gerlach <[email protected]>
---
  arch/arm/mach-omap2/omap_hwmod.c | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)
arch/arm/mach-omap2/omap_hwmod.c +1 -3
index 66c60fe..1c0885b 100644
@@ -4251,9 +4251,9 @@ void __init omap_hwmod_init(void)
  		soc_ops.enable_module = _omap4_enable_module;
  		soc_ops.disable_module = _omap4_disable_module;
  		soc_ops.wait_target_ready = _omap4_wait_target_ready;
-		soc_ops.assert_hardreset = _omap4_assert_hardreset;
-		soc_ops.deassert_hardreset = _omap4_deassert_hardreset;
-		soc_ops.is_hardreset_asserted = _omap4_is_hardreset_asserted;
+		soc_ops.assert_hardreset = _am33xx_assert_hardreset;

Quoted 8 lines

arch/arm/mach-omap2/omap_hwmod.c void __init omap_hwmod_init(void) · line 4259
Quoted 5 lines
arch/arm/mach-omap2/omap_hwmod.c +2 -0
+		soc_ops.deassert_hardreset = _am33xx_deassert_hardreset;
+		soc_ops.is_hardreset_asserted = _am33xx_is_hardreset_asserted;
  		soc_ops.init_clkdm = _init_clkdm;
  	} else if (soc_is_am33xx()) {
  		soc_ops.enable_module = _am33xx_enable_module;
Should AM43XX be using the _am33xx_{enable,disable}_module and wait_target_ready functions?

AM43xx has been using the _omap4_{enable,disable}_module functions without issue. Nothing is currently using the hardreset ops for am43xx which is why it went unnoticed but I saw the issue when I tried to use them for the wkup_m3 hwmod and they failed during internal development.

With that said, am43xx clockdomains have prcm_partition defined as part of their data which is only used by the _omap4_{enable,disable}_module functions and not the _am33xx functions so as the current configuration stands the _omap4 variants are the right choice.

Regards,
Dave

Quoted 4 lines

- Paul