[PATCH 0/2] ARM: AM43xx: PRM: fix reset handling issues

[PATCH 0/2] ARM: AM43xx: PRM: fix reset handling issues

[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.

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

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

[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.

Signed-off-by: Tero Kristo <t-kristo@ti.com>
Reported-by: Dave Gerlach <d-gerlach@ti.com>
Reported-by: Anna Suman <s-anna@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod.c  |   12 ++++++++++--
 arch/arm/mach-omap2/prminst44xx.c |   20 +++++++-------------
 2 files changed, 17 insertions(+), 15 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index 355b089..e482562 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -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);
 }
 
 /**
diff --git a/arch/arm/mach-omap2/prminst44xx.c b/arch/arm/mach-omap2/prminst44xx.c
index c4859c4..d0b15db 100644
--- a/arch/arm/mach-omap2/prminst44xx.c
+++ b/arch/arm/mach-omap2/prminst44xx.c
@@ -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;
-- 
1.7.9.5

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

[Suman Anna]

On 05/05/2015 08:33 AM, Tero Kristo wrote:
> 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.
> 
> Signed-off-by: Tero Kristo <t-kristo@ti.com>
> Reported-by: Dave Gerlach <d-gerlach@ti.com>
> Reported-by: Anna Suman <s-anna@ti.com>

Looks good to me, please correct the signature to
Suman Anna <s-anna@ti.com> on this patch and next patch as well.

regards
Suman

> ---
>  arch/arm/mach-omap2/omap_hwmod.c  |   12 ++++++++++--
>  arch/arm/mach-omap2/prminst44xx.c |   20 +++++++-------------
>  2 files changed, 17 insertions(+), 15 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
> index 355b089..e482562 100644
> --- a/arch/arm/mach-omap2/omap_hwmod.c
> +++ b/arch/arm/mach-omap2/omap_hwmod.c
> @@ -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);
>  }
>  
>  /**
> diff --git a/arch/arm/mach-omap2/prminst44xx.c b/arch/arm/mach-omap2/prminst44xx.c
> index c4859c4..d0b15db 100644
> --- a/arch/arm/mach-omap2/prminst44xx.c
> +++ b/arch/arm/mach-omap2/prminst44xx.c
> @@ -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;
> 

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

[Paul Walmsley]

On Tue, 5 May 2015, Suman Anna wrote:

> On 05/05/2015 08:33 AM, Tero Kristo wrote:
> > 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.
> > 
> > Signed-off-by: Tero Kristo <t-kristo@ti.com>
> > Reported-by: Dave Gerlach <d-gerlach@ti.com>
> > Reported-by: Anna Suman <s-anna@ti.com>
> 
> Looks good to me, please correct the signature to
> Suman Anna <s-anna@ti.com> on this patch and next patch as well.

Fixed in the local patches here.


- Paul

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

[Paul Walmsley]

On Tue, 5 May 2015, Tero Kristo wrote:

> 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.
> 
> Signed-off-by: Tero Kristo <t-kristo@ti.com>
> Reported-by: Dave Gerlach <d-gerlach@ti.com>
> Reported-by: Anna Suman <s-anna@ti.com>

Thanks, queued for v4.1-rc fixes.


- Paul

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

[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.

Signed-off-by: Tero Kristo <t-kristo@ti.com>
Reported-by: Dave Gerlach <d-gerlach@ti.com>
Reported-by: Anna Suman <s-anna@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod.c |   56 +++-----------------------------------
 1 file changed, 4 insertions(+), 52 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index e482562..752969f 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -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");
-- 
1.7.9.5

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

[Paul Walmsley]

On Tue, 5 May 2015, Tero Kristo wrote:

> 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.
> 
> Signed-off-by: Tero Kristo <t-kristo@ti.com>
> Reported-by: Dave Gerlach <d-gerlach@ti.com>
> Reported-by: Anna Suman <s-anna@ti.com>

Thanks, queued for v4.1-rc fixes.


- Paul

[PATCH 0/2] ARM: AM43xx: PRM: fix reset handling issues

[Dave Gerlach]

Tero,
On 05/05/2015 08:33 AM, Tero Kristo wrote:
> 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?
> 

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

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