Skip to contentSkip to conversation

ARM: AM43xx: PRM: fix reset handling issues

PATCH New Tero Kristo 4 participants 8 messages May 5, 2015 – May 8, 2015

Subsystems (5)
lore
Share & tools
lore
#0 lore raw

cover ARM: AM43xx: PRM: fix reset handling issues

Hi,

The work done for consolidating PRM code introduced a reset issue on AM43xx which is visible at least with wkup-m3, and prevents taking the wkup-m3 core out from reset. Fixed by passing all the relevant PRCM data from hwmod data, which was previously partly omitted on OMAP4+ case.

This set also removes some redundant AM33xx hwmod code and just re-uses the OMAP4+ version of the same.

Thanks to Dave Gerlach and Anna Suman for reporting this. Dave, care to give this patch set a try to see if it fixes the problem you see?

Testing done:
- am335x-bone: boot
- am437x-gp-evm: boot
- omap4-panda-es: boot, suspend/resume, cpuidle

-Tero

#1 lore raw

PATCH 1/2 ARM: OMAP4+: PRM: add support for passing status register/bit info to reset

AM43xx has slightly different reset register layout compared to OMAP4+, with varying status bit shifts and status register offsets. Current code assumes static offsets and identical status / reset control bit shifts, which is wrong. This patch adds PRM core support for passing the actual implementations from hwmod code. AM43xx mappings will be fixed in subsequent patch.

---
 arch/arm/mach-omap2/omap_hwmod.c  |   12 ++++++++++--
 arch/arm/mach-omap2/prminst44xx.c |   20 +++++++-------------
 2 files changed, 17 insertions(+), 15 deletions(-)
arch/arm/mach-omap2/omap_hwmod.c +10 -2
index 355b089..e482562 100644
@@ -171,6 +171,12 @@
  */
 #define LINKS_PER_OCP_IF		2
 
+/*
+ * Address offset (in bytes) between the reset control and the reset
+ * status registers: 4 bytes on OMAP4
+ */
+#define OMAP4_RST_CTRL_ST_OFFSET	4
+
 /**
  * struct omap_hwmod_soc_ops - fn ptrs for some SoC-specific operations
  * @enable_module: function to enable a module (via MODULEMODE)
@@ -3016,10 +3022,12 @@ static int _omap4_deassert_hardreset(struct omap_hwmod *oh,
 	if (ohri->st_shift)
 		pr_err("omap_hwmod: %s: %s: hwmod data error: OMAP4 does not support st_shift\n",
 		       oh->name, ohri->name);
-	return omap_prm_deassert_hardreset(ohri->rst_shift, 0,
+	return omap_prm_deassert_hardreset(ohri->rst_shift, ohri->rst_shift,
 					   oh->clkdm->pwrdm.ptr->prcm_partition,
 					   oh->clkdm->pwrdm.ptr->prcm_offs,
-					   oh->prcm.omap4.rstctrl_offs, 0);
+					   oh->prcm.omap4.rstctrl_offs,
+					   oh->prcm.omap4.rstctrl_offs +
+					   OMAP4_RST_CTRL_ST_OFFSET);
 }
 
 /**
arch/arm/mach-omap2/prminst44xx.c +7 -13
index c4859c4..d0b15db 100644
@@ -87,12 +87,6 @@ u32 omap4_prminst_rmw_inst_reg_bits(u32 mask, u32 bits, u8 part, s16 inst,
 	return v;
 }
 
-/*
- * Address offset (in bytes) between the reset control and the reset
- * status registers: 4 bytes on OMAP4
- */
-#define OMAP4_RST_CTRL_ST_OFFSET		4
-
 /**
  * omap4_prminst_is_hardreset_asserted - read the HW reset line state of
  * submodules contained in the hwmod module
@@ -141,11 +135,11 @@ int omap4_prminst_assert_hardreset(u8 shift, u8 part, s16 inst,
  * omap4_prminst_deassert_hardreset - deassert a submodule hardreset line and
  * wait
  * @shift: register bit shift corresponding to the reset line to deassert
- * @st_shift: status bit offset, not used for OMAP4+
+ * @st_shift: status bit offset corresponding to the reset line
  * @part: PRM partition
  * @inst: PRM instance offset
  * @rstctrl_offs: reset register offset
- * @st_offs: reset status register offset, not used for OMAP4+
+ * @rstst_offs: reset status register offset
  *
  * Some IPs like dsp, ipu or iva contain processors that require an HW
  * reset line to be asserted / deasserted in order to fully enable the
@@ -157,11 +151,11 @@ int omap4_prminst_assert_hardreset(u8 shift, u8 part, s16 inst,
  * of reset, or -EBUSY if the submodule did not exit reset promptly.
  */
 int omap4_prminst_deassert_hardreset(u8 shift, u8 st_shift, u8 part, s16 inst,
-				     u16 rstctrl_offs, u16 st_offs)
+				     u16 rstctrl_offs, u16 rstst_offs)
 {
 	int c;
 	u32 mask = 1 << shift;
-	u16 rstst_offs = rstctrl_offs + OMAP4_RST_CTRL_ST_OFFSET;
+	u32 st_mask = 1 << st_shift;
 
 	/* Check the current status to avoid de-asserting the line twice */
 	if (omap4_prminst_is_hardreset_asserted(shift, part, inst,
@@ -169,13 +163,13 @@ int omap4_prminst_deassert_hardreset(u8 shift, u8 st_shift, u8 part, s16 inst,
 		return -EEXIST;
 
 	/* Clear the reset status by writing 1 to the status bit */
-	omap4_prminst_rmw_inst_reg_bits(0xffffffff, mask, part, inst,
+	omap4_prminst_rmw_inst_reg_bits(0xffffffff, st_mask, part, inst,
 					rstst_offs);
 	/* de-assert the reset control line */
 	omap4_prminst_rmw_inst_reg_bits(mask, 0, part, inst, rstctrl_offs);
 	/* wait the status to be set */
-	omap_test_timeout(omap4_prminst_is_hardreset_asserted(shift, part, inst,
-							      rstst_offs),
+	omap_test_timeout(omap4_prminst_is_hardreset_asserted(st_shift, part,
+							      inst, rstst_offs),
 			  MAX_MODULE_HARDRESET_WAIT, c);
 
 	return (c == MAX_MODULE_HARDRESET_WAIT) ? -EBUSY : 0;
Signature · 1 line
1.7.9.5
#2 lore raw

PATCH 1/2 ARM: OMAP4+: PRM: add support for passing status register/bit info to reset

Quoted 10 lines · Tero Kristo

AM43xx has slightly different reset register layout compared to OMAP4+, with varying status bit shifts and status register offsets. Current code assumes static offsets and identical status / reset control bit shifts, which is wrong. This patch adds PRM core support for passing the actual implementations from hwmod code. AM43xx mappings will be fixed in subsequent patch.

Looks good to me, please correct the signature to Suman Anna <[email protected]> on this patch and next patch as well.

regards
Suman

Quoted 100 lines
---
 arch/arm/mach-omap2/omap_hwmod.c  |   12 ++++++++++--
 arch/arm/mach-omap2/prminst44xx.c |   20 +++++++-------------
 2 files changed, 17 insertions(+), 15 deletions(-)
arch/arm/mach-omap2/omap_hwmod.c +10 -2
index 355b089..e482562 100644
@@ -171,6 +171,12 @@
  */
 #define LINKS_PER_OCP_IF		2
 
+/*
+ * Address offset (in bytes) between the reset control and the reset
+ * status registers: 4 bytes on OMAP4
+ */
+#define OMAP4_RST_CTRL_ST_OFFSET	4
+
 /**
  * struct omap_hwmod_soc_ops - fn ptrs for some SoC-specific operations
  * @enable_module: function to enable a module (via MODULEMODE)
@@ -3016,10 +3022,12 @@ static int _omap4_deassert_hardreset(struct omap_hwmod *oh,
 	if (ohri->st_shift)
 		pr_err("omap_hwmod: %s: %s: hwmod data error: OMAP4 does not support st_shift\n",
 		       oh->name, ohri->name);
-	return omap_prm_deassert_hardreset(ohri->rst_shift, 0,
+	return omap_prm_deassert_hardreset(ohri->rst_shift, ohri->rst_shift,
 					   oh->clkdm->pwrdm.ptr->prcm_partition,
 					   oh->clkdm->pwrdm.ptr->prcm_offs,
-					   oh->prcm.omap4.rstctrl_offs, 0);
+					   oh->prcm.omap4.rstctrl_offs,
+					   oh->prcm.omap4.rstctrl_offs +
+					   OMAP4_RST_CTRL_ST_OFFSET);
 }
 
 /**
arch/arm/mach-omap2/prminst44xx.c +7 -13
index c4859c4..d0b15db 100644
@@ -87,12 +87,6 @@ u32 omap4_prminst_rmw_inst_reg_bits(u32 mask, u32 bits, u8 part, s16 inst,
 	return v;
 }
 
-/*
- * Address offset (in bytes) between the reset control and the reset
- * status registers: 4 bytes on OMAP4
- */
-#define OMAP4_RST_CTRL_ST_OFFSET		4
-
 /**
  * omap4_prminst_is_hardreset_asserted - read the HW reset line state of
  * submodules contained in the hwmod module
@@ -141,11 +135,11 @@ int omap4_prminst_assert_hardreset(u8 shift, u8 part, s16 inst,
  * omap4_prminst_deassert_hardreset - deassert a submodule hardreset line and
  * wait
  * @shift: register bit shift corresponding to the reset line to deassert
- * @st_shift: status bit offset, not used for OMAP4+
+ * @st_shift: status bit offset corresponding to the reset line
  * @part: PRM partition
  * @inst: PRM instance offset
  * @rstctrl_offs: reset register offset
- * @st_offs: reset status register offset, not used for OMAP4+
+ * @rstst_offs: reset status register offset
  *
  * Some IPs like dsp, ipu or iva contain processors that require an HW
  * reset line to be asserted / deasserted in order to fully enable the
@@ -157,11 +151,11 @@ int omap4_prminst_assert_hardreset(u8 shift, u8 part, s16 inst,
  * of reset, or -EBUSY if the submodule did not exit reset promptly.
  */
 int omap4_prminst_deassert_hardreset(u8 shift, u8 st_shift, u8 part, s16 inst,
-				     u16 rstctrl_offs, u16 st_offs)
+				     u16 rstctrl_offs, u16 rstst_offs)
 {
 	int c;
 	u32 mask = 1 << shift;
-	u16 rstst_offs = rstctrl_offs + OMAP4_RST_CTRL_ST_OFFSET;
+	u32 st_mask = 1 << st_shift;
 
 	/* Check the current status to avoid de-asserting the line twice */
 	if (omap4_prminst_is_hardreset_asserted(shift, part, inst,
@@ -169,13 +163,13 @@ int omap4_prminst_deassert_hardreset(u8 shift, u8 st_shift, u8 part, s16 inst,
 		return -EEXIST;
 
 	/* Clear the reset status by writing 1 to the status bit */
-	omap4_prminst_rmw_inst_reg_bits(0xffffffff, mask, part, inst,
+	omap4_prminst_rmw_inst_reg_bits(0xffffffff, st_mask, part, inst,
 					rstst_offs);
 	/* de-assert the reset control line */
 	omap4_prminst_rmw_inst_reg_bits(mask, 0, part, inst, rstctrl_offs);
 	/* wait the status to be set */
-	omap_test_timeout(omap4_prminst_is_hardreset_asserted(shift, part, inst,
-							      rstst_offs),
+	omap_test_timeout(omap4_prminst_is_hardreset_asserted(st_shift, part,
+							      inst, rstst_offs),
 			  MAX_MODULE_HARDRESET_WAIT, c);
 
 	return (c == MAX_MODULE_HARDRESET_WAIT) ? -EBUSY : 0;
maintainer
#3 lore raw

PATCH 1/2 ARM: OMAP4+: PRM: add support for passing status register/bit info to reset

Quoted 8 lines · Suman Anna
Quoted 7 lines · Tero Kristo

AM43xx has slightly different reset register layout compared to OMAP4+, with varying status bit shifts and status register offsets. Current code assumes static offsets and identical status / reset control bit shifts, which is wrong. This patch adds PRM core support for passing the actual implementations from hwmod code. AM43xx mappings will be fixed in subsequent patch.

Quoted 6 lines
Quoted 3 lines

Looks good to me, please correct the signature to Suman Anna <[email protected]> on this patch and next patch as well.

Fixed in the local patches here.

- Paul

maintainer
#4 lore raw

PATCH 1/2 ARM: OMAP4+: PRM: add support for passing status register/bit info to reset

In reply to Tero Kristo
Quoted 10 lines · Tero Kristo

AM43xx has slightly different reset register layout compared to OMAP4+, with varying status bit shifts and status register offsets. Current code assumes static offsets and identical status / reset control bit shifts, which is wrong. This patch adds PRM core support for passing the actual implementations from hwmod code. AM43xx mappings will be fixed in subsequent patch.

Thanks, queued for v4.1-rc fixes.

- Paul

#5 lore raw

PATCH 2/2 ARM: AM33xx+: hwmod: re-use omap4 implementations for reset functionality

In reply to Tero Kristo

The reset code functionality is mostly a copy paste between OMAP4+ and AM33xx+. Re-use the omap4 code where possible, and just keep the special implementation for de-asserting the hardreset lines for AM33xx, as AM33xx+ devices have slightly different register layouts compared to OMAP4+. This patch also fixes the hardreset issues faced on AM43xx.

---
 arch/arm/mach-omap2/omap_hwmod.c |   56 +++-----------------------------------
 1 file changed, 4 insertions(+), 52 deletions(-)
arch/arm/mach-omap2/omap_hwmod.c +4 -52
index e482562..752969f 100644
@@ -3056,27 +3056,6 @@ static int _omap4_is_hardreset_asserted(struct omap_hwmod *oh,
 }
 
 /**
- * _am33xx_assert_hardreset - call AM33XX PRM hardreset fn with hwmod args
- * @oh: struct omap_hwmod * to assert hardreset
- * @ohri: hardreset line data
- *
- * Call am33xx_prminst_assert_hardreset() with parameters extracted
- * from the hwmod @oh and the hardreset line data @ohri.  Only
- * intended for use as an soc_ops function pointer.  Passes along the
- * return value from am33xx_prminst_assert_hardreset().  XXX This
- * function is scheduled for removal when the PRM code is moved into
- * drivers/.
- */
-static int _am33xx_assert_hardreset(struct omap_hwmod *oh,
-				   struct omap_hwmod_rst_info *ohri)
-
-{
-	return omap_prm_assert_hardreset(ohri->rst_shift, 0,
-					 oh->clkdm->pwrdm.ptr->prcm_offs,
-					 oh->prcm.omap4.rstctrl_offs);
-}
-
-/**
  * _am33xx_deassert_hardreset - call AM33XX PRM hardreset fn with hwmod args
  * @oh: struct omap_hwmod * to deassert hardreset
  * @ohri: hardreset line data
@@ -3091,32 +3070,13 @@ static int _am33xx_assert_hardreset(struct omap_hwmod *oh,
 static int _am33xx_deassert_hardreset(struct omap_hwmod *oh,
 				     struct omap_hwmod_rst_info *ohri)
 {
-	return omap_prm_deassert_hardreset(ohri->rst_shift, ohri->st_shift, 0,
+	return omap_prm_deassert_hardreset(ohri->rst_shift, ohri->st_shift,
+					   oh->clkdm->pwrdm.ptr->prcm_partition,
 					   oh->clkdm->pwrdm.ptr->prcm_offs,
 					   oh->prcm.omap4.rstctrl_offs,
 					   oh->prcm.omap4.rstst_offs);
 }
 
-/**
- * _am33xx_is_hardreset_asserted - call AM33XX PRM hardreset fn with hwmod args
- * @oh: struct omap_hwmod * to test hardreset
- * @ohri: hardreset line data
- *
- * Call am33xx_prminst_is_hardreset_asserted() with parameters
- * extracted from the hwmod @oh and the hardreset line data @ohri.
- * Only intended for use as an soc_ops function pointer.  Passes along
- * the return value from am33xx_prminst_is_hardreset_asserted().  XXX
- * This function is scheduled for removal when the PRM code is moved
- * into drivers/.
- */
-static int _am33xx_is_hardreset_asserted(struct omap_hwmod *oh,
-					struct omap_hwmod_rst_info *ohri)
-{
-	return omap_prm_is_hardreset_asserted(ohri->rst_shift, 0,
-					      oh->clkdm->pwrdm.ptr->prcm_offs,
-					      oh->prcm.omap4.rstctrl_offs);
-}
-
 /* Public functions */
 
 u32 omap_hwmod_read(struct omap_hwmod *oh, u16 reg_offs)
@@ -3916,21 +3876,13 @@ void __init omap_hwmod_init(void)
 		soc_ops.init_clkdm = _init_clkdm;
 		soc_ops.update_context_lost = _omap4_update_context_lost;
 		soc_ops.get_context_lost = _omap4_get_context_lost;
-	} else if (soc_is_am43xx()) {
+	} else if (cpu_is_ti816x() || soc_is_am33xx() || soc_is_am43xx()) {
 		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.init_clkdm = _init_clkdm;
-	} else if (cpu_is_ti816x() || soc_is_am33xx()) {
-		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 = _am33xx_assert_hardreset;
 		soc_ops.deassert_hardreset = _am33xx_deassert_hardreset;
-		soc_ops.is_hardreset_asserted = _am33xx_is_hardreset_asserted;
+		soc_ops.is_hardreset_asserted = _omap4_is_hardreset_asserted;
 		soc_ops.init_clkdm = _init_clkdm;
 	} else {
 		WARN(1, "omap_hwmod: unknown SoC type\n");
Signature · 1 line
1.7.9.5
maintainer
#6 lore raw

PATCH 2/2 ARM: AM33xx+: hwmod: re-use omap4 implementations for reset functionality

Quoted 9 lines · Tero Kristo

The reset code functionality is mostly a copy paste between OMAP4+ and AM33xx+. Re-use the omap4 code where possible, and just keep the special implementation for de-asserting the hardreset lines for AM33xx, as AM33xx+ devices have slightly different register layouts compared to OMAP4+. This patch also fixes the hardreset issues faced on AM43xx.

Thanks, queued for v4.1-rc fixes.

- Paul

#7 lore raw

cover ARM: AM43xx: PRM: fix reset handling issues

In reply to Tero Kristo

Tero,

Quoted 8 lines · Tero Kristo

Hi,

The work done for consolidating PRM code introduced a reset issue on AM43xx which is visible at least with wkup-m3, and prevents taking the wkup-m3 core out from reset. Fixed by passing all the relevant PRCM data from hwmod data, which was previously partly omitted on OMAP4+ case.

Quoted 6 lines

This set also removes some redundant AM33xx hwmod code and just re-uses the OMAP4+ version of the same.

Thanks to Dave Gerlach and Anna Suman for reporting this. Dave, care to give this patch set a try to see if it fixes the problem you see?

Thanks for fixing this up, I swapped these patches for my reset patch on my suspend branch [1] and tested the following:

am335x-gp-evm: boot and firmware loaded, suspend/resume multiple cycles
am437x-gp-evm: boot and firmware loaded, suspend/resume multiple cycles

Patches look fine to me.

Regards,
Dave

[1] https://github.com/dgerlach/linux-pm/tree/pm-ds0-v4.1-rc1-WIP

Quoted 7 lines

Testing done:
- am335x-bone: boot
- am437x-gp-evm: boot
- omap4-panda-es: boot, suspend/resume, cpuidle

-Tero