[PATCH 0/3] OMAP: hwmod: Add hardreset and softreset management

[PATCH 0/3] OMAP: hwmod: Add hardreset and softreset management

[Benoit Cousson]

Hi Paul & Kevin,

Here are a reset management series.

- The first patch will be removed as soon as we will have the proper
  OMAP4 support for the prm_xxx accessors.
- The second one is adding hardreset support in order to allow
  syslink driver to manage properly the DSP and IPU processor
  reset.
- The last one is forcing a sofreset after the first init. 
  Some IP might require sofreset after each wakeup
  from power domain OFF mode. That still needs to be confirm.


Tested on PAB board using OMAP4 ES1.0 GP device

Patches are based on lo/for-next + for-next-fixes and are available here:
git://dev.omapzoom.org/pub/scm/swarch/linux-omap-adv.git pm-wip/hwmods-reset

Regards,
Benoit


Benoit Cousson (3):
  OMAP4: prcm: Add temporarily helper functions for rmw and read inside
    the PRM
  OMAP: hwmod: Add hardreset management support
  OMAP: hwmod: Force a softreset during _setup

 arch/arm/mach-omap2/omap_hwmod.c             |  197 ++++++++++++++++++++++++--
 arch/arm/mach-omap2/prcm.c                   |   24 +++
 arch/arm/plat-omap/include/plat/omap_hwmod.h |   21 +++
 arch/arm/plat-omap/include/plat/prcm.h       |    2 +
 4 files changed, 235 insertions(+), 9 deletions(-)

[PATCH 1/3] OMAP4: prcm: Add temporarily helper functions for rmw and read inside the PRM

[Benoit Cousson]

Since OMAP4 is using an absolute address, the current PRM accessors
are not useable.
OMAP4 adaptation for these API are currently ongoing, so define temp
version until the proper ones are defined.

Signed-off-by: Benoit Cousson <b-cousson@ti.com>
Cc: Paul Walmsley <paul@pwsan.com>
Cc: Kevin Hilman <khilman@deeprootsystems.com>
---
 arch/arm/mach-omap2/prcm.c             |   24 ++++++++++++++++++++++++
 arch/arm/plat-omap/include/plat/prcm.h |    2 ++
 2 files changed, 26 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/prcm.c b/arch/arm/mach-omap2/prcm.c
index 96f4616..d4388d3 100644
--- a/arch/arm/mach-omap2/prcm.c
+++ b/arch/arm/mach-omap2/prcm.c
@@ -216,6 +216,30 @@ u32 prm_read_mod_bits_shift(s16 domain, s16 idx, u32 mask)
 	return v;
 }
 
+/* Read a PRM register, AND it, and shift the result down to bit 0 */
+u32 omap4_prm_read_bits_shift(void __iomem *reg, u32 mask)
+{
+	u32 v;
+
+	v = __raw_readl(reg);
+	v &= mask;
+	v >>= __ffs(mask);
+
+	return v;
+}
+
+/* Read-modify-write a register in a PRM module. Caller must lock */
+u32 omap4_prm_rmw_reg_bits(u32 mask, u32 bits, void __iomem *reg)
+{
+	u32 v;
+
+	v = __raw_readl(reg);
+	v &= ~mask;
+	v |= bits;
+	__raw_writel(v, reg);
+
+	return v;
+}
 /* Read a register in a CM module */
 u32 cm_read_mod_reg(s16 module, u16 idx)
 {
diff --git a/arch/arm/plat-omap/include/plat/prcm.h b/arch/arm/plat-omap/include/plat/prcm.h
index 9fbd914..ab77442 100644
--- a/arch/arm/plat-omap/include/plat/prcm.h
+++ b/arch/arm/plat-omap/include/plat/prcm.h
@@ -38,6 +38,8 @@ u32 prm_read_mod_reg(s16 module, u16 idx);
 void prm_write_mod_reg(u32 val, s16 module, u16 idx);
 u32 prm_rmw_mod_reg_bits(u32 mask, u32 bits, s16 module, s16 idx);
 u32 prm_read_mod_bits_shift(s16 domain, s16 idx, u32 mask);
+u32 omap4_prm_read_bits_shift(void __iomem *reg, u32 mask);
+u32 omap4_prm_rmw_reg_bits(u32 mask, u32 bits, void __iomem *reg);
 u32 cm_read_mod_reg(s16 module, u16 idx);
 void cm_write_mod_reg(u32 val, s16 module, u16 idx);
 u32 cm_rmw_mod_reg_bits(u32 mask, u32 bits, s16 module, s16 idx);
-- 
1.6.1.3

[PATCH 2/3] OMAP: hwmod: Add hardreset management support

[Benoit Cousson]

Most processor IPs does have a hardreset signal controlled by the PRM.
This is different of the softreset used for local IP reset from the
SYSCONFIG register.
The granularity can be much finer than orginal HWMOD, for ex, the IVA
hwmod contains 3 reset lines, the IPU 3 as well, the DSP 2...
Since this granularity is needed by the driver, we have to ensure
than one hwmod exist for each hardreset line.
We do expose as well an hardreset API at hwmod in order to assert / deassert
all the reset individual lines that belong to an hwmod.

- Store reset lines as hwmod resources that a driver can query by name like
  an irq or sdma line.

- Add two APIs for asserting / deasserting reset lines in hwmods
  processor that require manual reset control.
- Add one API to get the current reset state.
- If an hwmod contains only one line, an automatic assertion / de-assertion
  is done.
  -> de-assert the hardreset line only during enable from disable transition
  -> assert the hardreset line only during shutdown

Note: The hwmods with hardreset line and HWMOD_INIT_NO_RESET flag must be
kept in INITIALIZED state.
They can be properly enabled only if the hardreset line is de-asserted
before.

For information here is the list of IPs with HW reset control
on an OMAP4430 device:

RM_DSP_RSTCTRL
  1,1,'RST2','RW','1','DSP - MMU, cache and slave interface reset control'
  0,0,'RST1','RW','1','DSP - DSP reset control'

RM_IVA_RSTCTRL
  2,2,'RST3','RW','1','IVA logic and SL2 reset control'
  1,1,'RST2','RW','1','IVA Sequencer2 reset control'
  0,0,'RST1','RW','1','IVA sequencer1 reset control'

RM_IPU_RSTCTRL
  2,2,'RST3','RW','1','IPU MMU and CACHE interface reset control.'
  1,1,'RST2','RW','1','IPU Cortex M3 CPU2  reset control.'
  0,0,'RST1','RW','1','IPU Cortex M3 CPU1  reset control.'

PRM_RSTCTRL
  1,1,'RST_GLOBAL_COLD_SW','RW','0','Global COLD software reset control.'
  0,0,'RST_GLOBAL_WARM_SW','RW','0','Global WARM software reset control.'

RM_CPU0_CPU0_RSTCTRL
RM_CPU1_CPU1_RSTCTRL
  0,0,'RST','RW','0','Cortex A9 CPU0&1 warm local reset control'

Signed-off-by: Benoit Cousson <b-cousson@ti.com>
Cc: Paul Walmsley <paul@pwsan.com>
Cc: Kevin Hilman <khilman@deeprootsystems.com>
Cc: Rajendra Nayak <rnayak@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod.c             |  180 ++++++++++++++++++++++++++
 arch/arm/plat-omap/include/plat/omap_hwmod.h |   21 +++
 2 files changed, 201 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index 100115f..53b08e3 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -50,6 +50,7 @@
 #include <plat/powerdomain.h>
 #include <plat/clock.h>
 #include <plat/omap_hwmod.h>
+#include <plat/prcm.h>
 
 #include "cm.h"
 
@@ -909,6 +910,15 @@ int _omap_hwmod_enable(struct omap_hwmod *oh)
 
 	pr_debug("omap_hwmod: %s: enabling\n", oh->name);
 
+	/*
+	 * If an IP contains only one HW reset line, then de-assert it in order
+	 * to allow to enable the clocks. Otherwise the PRCM will return
+	 * Intransition status, and the init will failed.
+	 */
+	if ((oh->_state == _HWMOD_STATE_INITIALIZED ||
+	     oh->_state == _HWMOD_STATE_DISABLED) && oh->rst_lines_cnt == 1)
+		omap_hwmod_hardreset_deassert(oh, oh->rst_lines[0].name);
+
 	/* XXX mux balls */
 
 	_add_initiator_dep(oh, mpu_oh);
@@ -982,6 +992,12 @@ static int _shutdown(struct omap_hwmod *oh)
 
 	if (oh->class->sysc)
 		_sysc_shutdown(oh);
+	/*
+	 * If an IP contains only one HW reset line, then assert it
+	 * before disabling the clocks and shutting down the IP.
+	 */
+	if (oh->rst_lines_cnt == 1)
+		omap_hwmod_hardreset_assert(oh, oh->rst_lines[0].name);
 
 	/* clocks and deps are already disabled in idle */
 	if (oh->_state == _HWMOD_STATE_ENABLED) {
@@ -1040,6 +1056,16 @@ static int _setup(struct omap_hwmod *oh, void *data)
 
 	oh->_state = _HWMOD_STATE_INITIALIZED;
 
+	/*
+	 * In the case of hwmod with hardreset that should not be
+	 * de-assert at boot time, we have to keep the module
+	 * initialized, because we cannot enable it properly with the
+	 * reset asserted. Exit without warning because that behavior is
+	 * expected.
+	 */
+	if ((oh->flags & HWMOD_INIT_NO_RESET) && oh->rst_lines_cnt == 1)
+		return 0;
+
 	r = _omap_hwmod_enable(oh);
 	if (r) {
 		pr_warning("omap_hwmod: %s: cannot be enabled (%d)\n",
@@ -1450,6 +1476,160 @@ int omap_hwmod_reset(struct omap_hwmod *oh)
 	return r;
 }
 
+/* offset between the reset control and the reset status registers */
+#define RST_CTRL_ST_OFFSET 4
+
+/**
+ * _lookup_reset - reset the hwmod
+ * @oh: struct omap_hwmod *
+ * @name: name of the reset line in the context of this hwmod
+ *
+ * Return the bit position of the reset line that match the
+ * input name. Return -ENOENT if not found.
+ */
+u8 _lookup_reset(struct omap_hwmod *oh, const char *name)
+{
+	int i;
+
+	for (i = 0; i < oh->rst_lines_cnt; i++) {
+		const char *rst_line = oh->rst_lines[i].name;
+		if (!strcmp(rst_line, name)) {
+			u8 shift = oh->rst_lines[i].rst_shift;
+			pr_debug("omap_hwmod: %s: _lookup_reset: %s: %d\n",
+				 oh->name, rst_line, shift);
+
+			return shift;
+		}
+	}
+
+	return -ENOENT;
+}
+
+/**
+ * omap_hwmod_reset_assert - assert the HW reset line of submodules
+ * contained in the hwmod module.
+ * @oh: struct omap_hwmod *
+ * @name: name of the reset line to lookup and assert
+ *
+ * Some IP like dsp, ipu or iva contain processor that require
+ * an HW reset line to be assert / deassert in order to enable fully
+ * the IP.
+ */
+int omap_hwmod_hardreset_assert(struct omap_hwmod *oh, const char *name)
+{
+	u8 shift;
+	u32 mask;
+
+	if (!oh)
+		return -EINVAL;
+
+	/* XXX For the moment only OMAP4 is supported */
+	if (!cpu_is_omap44xx()) {
+		pr_warning("%s only supported on OMAP4\n", __func__);
+		return -EINVAL;
+	}
+
+	shift = _lookup_reset(oh, name);
+	if (IS_ERR_VALUE(shift))
+		return shift;
+
+	mask = 1 << shift;
+	omap4_prm_rmw_reg_bits(mask, mask, oh->prcm.omap4.rstctrl_reg);
+
+	return 0;
+}
+
+/**
+ * omap_hwmod_hardreset_deassert - deassert the HW reset line of submodules
+ * contained in the hwmod module.
+ * @oh: struct omap_hwmod *
+ * @name: name of the reset line to look up and deassert
+ *
+ * Some IP like dsp, ipu or iva contain processor that require
+ * an HW reset line to be assert / deassert in order to enable fully
+ * the IP.
+ */
+int omap_hwmod_hardreset_deassert(struct omap_hwmod *oh, const char *name)
+{
+	u8 shift;
+	u32 mask;
+	int c = 0;
+
+	if (!oh)
+		return -EINVAL;
+
+	/* XXX For the moment only OMAP4 is supported */
+	if (!cpu_is_omap44xx()) {
+		pr_warning("%s only supported on OMAP4\n", __func__);
+		return -EINVAL;
+	}
+
+	shift = _lookup_reset(oh, name);
+	if (IS_ERR_VALUE(shift))
+		return shift;
+
+	mask = 1 << shift;
+
+	/* Check the current status to avoid de-asserting the line twice */
+	if (omap4_prm_read_bits_shift(oh->prcm.omap4.rstctrl_reg, mask) == 0) {
+		pr_warning("omap_hwmod: %s: reset already de-asserted\n",
+			   oh->name);
+		return 0;
+	}
+
+	/* Clear the reset status by writing 1 to the status bit */
+	omap4_prm_rmw_reg_bits(0xffffffff, mask, oh->prcm.omap4.rstctrl_reg
+						 + RST_CTRL_ST_OFFSET);
+	/* de-assert the reset control line */
+	omap4_prm_rmw_reg_bits(mask, 0, oh->prcm.omap4.rstctrl_reg);
+	/* wait the status to be set */
+	omap_test_timeout(omap4_prm_read_bits_shift(oh->prcm.omap4.rstctrl_reg
+						    + RST_CTRL_ST_OFFSET, mask),
+			  MAX_MODULE_RESET_WAIT, c);
+
+	if (c == MAX_MODULE_RESET_WAIT) {
+		pr_warning("omap_hwmod: %s: failed to reset in %d usec\n",
+			   oh->name, MAX_MODULE_RESET_WAIT);
+		return -EBUSY;
+	} else {
+		pr_debug("omap_hwmod: %s: reset %s in %d usec\n",
+			 oh->name, name, c);
+	}
+
+	return 0;
+}
+
+/**
+ * omap_hwmod_hw_reset_state - read the HW reset line state of submodules
+ * contained in the hwmod module
+ * @oh: struct omap_hwmod *
+ * @name: name of the reset line to look up and read
+ *
+ * Return the state of the reset line.
+ */
+int omap_hwmod_hardreset_state(struct omap_hwmod *oh, const char *name)
+{
+	u8 shift;
+	u32 mask;
+
+	if (!oh)
+		return -EINVAL;
+
+	/* XXX For the moment only OMAP4 is supported */
+	if (!cpu_is_omap44xx()) {
+		pr_warning("%s only supported on OMAP4\n", __func__);
+		return -EINVAL;
+	}
+
+	shift = _lookup_reset(oh, name);
+	if (IS_ERR_VALUE(shift))
+		return shift;
+
+	mask = 1 << shift;
+
+	return omap4_prm_read_bits_shift(oh->prcm.omap4.rstctrl_reg, mask);
+}
+
 /**
  * omap_hwmod_count_resources - count number of struct resources needed by hwmod
  * @oh: struct omap_hwmod *
diff --git a/arch/arm/plat-omap/include/plat/omap_hwmod.h b/arch/arm/plat-omap/include/plat/omap_hwmod.h
index 5506d80..5941183 100644
--- a/arch/arm/plat-omap/include/plat/omap_hwmod.h
+++ b/arch/arm/plat-omap/include/plat/omap_hwmod.h
@@ -108,6 +108,19 @@ struct omap_hwmod_dma_info {
 };
 
 /**
+ * struct omap_hwmod_rst_info - IPs reset lines use by hwmod
+ * @name: name of the reset line (module local name)
+ * @rst_shift: Offset of the reset bit
+ *
+ * @name should be something short, e.g., "cpu0" or "rst". It is defined
+ * locally to the hwmod.
+ */
+struct omap_hwmod_rst_info {
+	const char	*name;
+	u8		rst_shift;
+};
+
+/**
  * struct omap_hwmod_opt_clk - optional clocks used by this hwmod
  * @role: "sys", "32k", "tv", etc -- for use in clk_get()
  * @clk: opt clock: OMAP clock name
@@ -327,10 +340,12 @@ struct omap_hwmod_omap2_prcm {
 /**
  * struct omap_hwmod_omap4_prcm - OMAP4-specific PRCM data
  * @clkctrl_reg: PRCM address of the clock control register
+ * @rstctrl_reg: adress of the XXX_RSTCTRL register located in the PRM
  * @submodule_wkdep_bit: bit shift of the WKDEP range
  */
 struct omap_hwmod_omap4_prcm {
 	void __iomem	*clkctrl_reg;
+	void __iomem	*rstctrl_reg;
 	u8		submodule_wkdep_bit;
 };
 
@@ -449,6 +464,7 @@ struct omap_hwmod {
 	struct omap_device		*od;
 	struct omap_hwmod_irq_info	*mpu_irqs;
 	struct omap_hwmod_dma_info	*sdma_reqs;
+	struct omap_hwmod_rst_info	*rst_lines;
 	union {
 		struct omap_hwmod_omap2_prcm omap2;
 		struct omap_hwmod_omap4_prcm omap4;
@@ -469,6 +485,7 @@ struct omap_hwmod {
 	u8				response_lat;
 	u8				mpu_irqs_cnt;
 	u8				sdma_reqs_cnt;
+	u8				rst_lines_cnt;
 	u8				opt_clks_cnt;
 	u8				masters_cnt;
 	u8				slaves_cnt;
@@ -495,6 +512,10 @@ int omap_hwmod_shutdown(struct omap_hwmod *oh);
 int omap_hwmod_enable_clocks(struct omap_hwmod *oh);
 int omap_hwmod_disable_clocks(struct omap_hwmod *oh);
 
+int omap_hwmod_hardreset_assert(struct omap_hwmod *oh, const char *name);
+int omap_hwmod_hardreset_deassert(struct omap_hwmod *oh, const char *name);
+int omap_hwmod_hardreset_state(struct omap_hwmod *oh, const char *name);
+
 int omap_hwmod_set_slave_idlemode(struct omap_hwmod *oh, u8 idlemode);
 
 int omap_hwmod_reset(struct omap_hwmod *oh);
-- 
1.6.1.3

[PATCH 3/3] OMAP: hwmod: Force a softreset during _setup

[Benoit Cousson]

Force the softreset of every IPs during the _setup phase.
IPs that cannot support softreset or that should not
be reset must set the HWMOD_INIT_NO_RESET flag in the
hwmod struct.

Signed-off-by: Benoit Cousson <b-cousson@ti.com>
Cc: Paul Walmsley <paul@pwsan.com>
Cc: Kevin Hilman <khilman@deeprootsystems.com>
---
 arch/arm/mach-omap2/omap_hwmod.c |   17 ++++++++---------
 1 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index 53b08e3..91ad9c6 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -856,8 +856,8 @@ static int _reset(struct omap_hwmod *oh)
 
 	/* clocks must be on for this operation */
 	if (oh->_state != _HWMOD_STATE_ENABLED) {
-		WARN(1, "omap_hwmod: %s: reset can only be entered from "
-		     "enabled state\n", oh->name);
+		pr_warning("omap_hwmod: %s: reset can only be entered from "
+			   "enabled state\n", oh->name);
 		return -EINVAL;
 	}
 
@@ -874,8 +874,8 @@ static int _reset(struct omap_hwmod *oh)
 			  MAX_MODULE_RESET_WAIT, c);
 
 	if (c == MAX_MODULE_RESET_WAIT)
-		WARN(1, "omap_hwmod: %s: failed to reset in %d usec\n",
-		     oh->name, MAX_MODULE_RESET_WAIT);
+		pr_warning("omap_hwmod: %s: failed to reset in %d usec\n",
+			   oh->name, MAX_MODULE_RESET_WAIT);
 	else
 		pr_debug("omap_hwmod: %s: reset in %d usec\n", oh->name, c);
 
@@ -1074,12 +1074,11 @@ static int _setup(struct omap_hwmod *oh, void *data)
 	}
 
 	if (!(oh->flags & HWMOD_INIT_NO_RESET)) {
+		_reset(oh);
 		/*
-		 * XXX Do the OCP_SYSCONFIG bits need to be
-		 * reprogrammed after a reset?  If not, then this can
-		 * be removed.  If they do, then probably the
-		 * _omap_hwmod_enable() function should be split to avoid the
-		 * rewrite of the OCP_SYSCONFIG register.
+		 * OCP_SYSCONFIG bits need to be reprogrammed after a softreset.
+		 * The _omap_hwmod_enable() function should be split to
+		 * avoid the rewrite of the OCP_SYSCONFIG register.
 		 */
 		if (oh->class->sysc) {
 			_update_sysc_cache(oh);
-- 
1.6.1.3

Re: [PATCH 1/3] OMAP4: prcm: Add temporarily helper functions for rmw and read inside the PRM

[Kevin Hilman]

Benoit Cousson <b-cousson@ti.com> writes:

> Since OMAP4 is using an absolute address, the current PRM accessors
> are not useable.
> OMAP4 adaptation for these API are currently ongoing, so define temp
> version until the proper ones are defined.

Curious what we're waiting for for the final versions of these?

Kevin

> Signed-off-by: Benoit Cousson <b-cousson@ti.com>
> Cc: Paul Walmsley <paul@pwsan.com>
> Cc: Kevin Hilman <khilman@deeprootsystems.com>
> ---
>  arch/arm/mach-omap2/prcm.c             |   24 ++++++++++++++++++++++++
>  arch/arm/plat-omap/include/plat/prcm.h |    2 ++
>  2 files changed, 26 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/prcm.c b/arch/arm/mach-omap2/prcm.c
> index 96f4616..d4388d3 100644
> --- a/arch/arm/mach-omap2/prcm.c
> +++ b/arch/arm/mach-omap2/prcm.c
> @@ -216,6 +216,30 @@ u32 prm_read_mod_bits_shift(s16 domain, s16 idx, u32 mask)
>  	return v;
>  }
>  
> +/* Read a PRM register, AND it, and shift the result down to bit 0 */
> +u32 omap4_prm_read_bits_shift(void __iomem *reg, u32 mask)
> +{
> +	u32 v;
> +
> +	v = __raw_readl(reg);
> +	v &= mask;
> +	v >>= __ffs(mask);
> +
> +	return v;
> +}
> +
> +/* Read-modify-write a register in a PRM module. Caller must lock */
> +u32 omap4_prm_rmw_reg_bits(u32 mask, u32 bits, void __iomem *reg)
> +{
> +	u32 v;
> +
> +	v = __raw_readl(reg);
> +	v &= ~mask;
> +	v |= bits;
> +	__raw_writel(v, reg);
> +
> +	return v;
> +}
>  /* Read a register in a CM module */
>  u32 cm_read_mod_reg(s16 module, u16 idx)
>  {
> diff --git a/arch/arm/plat-omap/include/plat/prcm.h b/arch/arm/plat-omap/include/plat/prcm.h
> index 9fbd914..ab77442 100644
> --- a/arch/arm/plat-omap/include/plat/prcm.h
> +++ b/arch/arm/plat-omap/include/plat/prcm.h
> @@ -38,6 +38,8 @@ u32 prm_read_mod_reg(s16 module, u16 idx);
>  void prm_write_mod_reg(u32 val, s16 module, u16 idx);
>  u32 prm_rmw_mod_reg_bits(u32 mask, u32 bits, s16 module, s16 idx);
>  u32 prm_read_mod_bits_shift(s16 domain, s16 idx, u32 mask);
> +u32 omap4_prm_read_bits_shift(void __iomem *reg, u32 mask);
> +u32 omap4_prm_rmw_reg_bits(u32 mask, u32 bits, void __iomem *reg);
>  u32 cm_read_mod_reg(s16 module, u16 idx);
>  void cm_write_mod_reg(u32 val, s16 module, u16 idx);
>  u32 cm_rmw_mod_reg_bits(u32 mask, u32 bits, s16 module, s16 idx);

Re: [PATCH 1/3] OMAP4: prcm: Add temporarily helper functions for rmw and read inside the PRM

[Cousson, Benoit]

On 8/9/2010 7:30 PM, Kevin Hilman wrote:
> Benoit Cousson<b-cousson@ti.com>  writes:
>
>> Since OMAP4 is using an absolute address, the current PRM accessors
>> are not useable.
>> OMAP4 adaptation for these API are currently ongoing, so define temp
>> version until the proper ones are defined.
>
> Curious what we're waiting for for the final versions of these?

That should be soon... Rajendra is working on that... among hundreds 
other stuff.

Benoit

> Kevin
>
>> Signed-off-by: Benoit Cousson<b-cousson@ti.com>
>> Cc: Paul Walmsley<paul@pwsan.com>
>> Cc: Kevin Hilman<khilman@deeprootsystems.com>
>> ---
>>   arch/arm/mach-omap2/prcm.c             |   24 ++++++++++++++++++++++++
>>   arch/arm/plat-omap/include/plat/prcm.h |    2 ++
>>   2 files changed, 26 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/prcm.c b/arch/arm/mach-omap2/prcm.c
>> index 96f4616..d4388d3 100644
>> --- a/arch/arm/mach-omap2/prcm.c
>> +++ b/arch/arm/mach-omap2/prcm.c
>> @@ -216,6 +216,30 @@ u32 prm_read_mod_bits_shift(s16 domain, s16 idx, u32 mask)
>>   	return v;
>>   }
>>
>> +/* Read a PRM register, AND it, and shift the result down to bit 0 */
>> +u32 omap4_prm_read_bits_shift(void __iomem *reg, u32 mask)
>> +{
>> +	u32 v;
>> +
>> +	v = __raw_readl(reg);
>> +	v&= mask;
>> +	v>>= __ffs(mask);
>> +
>> +	return v;
>> +}
>> +
>> +/* Read-modify-write a register in a PRM module. Caller must lock */
>> +u32 omap4_prm_rmw_reg_bits(u32 mask, u32 bits, void __iomem *reg)
>> +{
>> +	u32 v;
>> +
>> +	v = __raw_readl(reg);
>> +	v&= ~mask;
>> +	v |= bits;
>> +	__raw_writel(v, reg);
>> +
>> +	return v;
>> +}
>>   /* Read a register in a CM module */
>>   u32 cm_read_mod_reg(s16 module, u16 idx)
>>   {
>> diff --git a/arch/arm/plat-omap/include/plat/prcm.h b/arch/arm/plat-omap/include/plat/prcm.h
>> index 9fbd914..ab77442 100644
>> --- a/arch/arm/plat-omap/include/plat/prcm.h
>> +++ b/arch/arm/plat-omap/include/plat/prcm.h
>> @@ -38,6 +38,8 @@ u32 prm_read_mod_reg(s16 module, u16 idx);
>>   void prm_write_mod_reg(u32 val, s16 module, u16 idx);
>>   u32 prm_rmw_mod_reg_bits(u32 mask, u32 bits, s16 module, s16 idx);
>>   u32 prm_read_mod_bits_shift(s16 domain, s16 idx, u32 mask);
>> +u32 omap4_prm_read_bits_shift(void __iomem *reg, u32 mask);
>> +u32 omap4_prm_rmw_reg_bits(u32 mask, u32 bits, void __iomem *reg);
>>   u32 cm_read_mod_reg(s16 module, u16 idx);
>>   void cm_write_mod_reg(u32 val, s16 module, u16 idx);
>>   u32 cm_rmw_mod_reg_bits(u32 mask, u32 bits, s16 module, s16 idx);

RE: [PATCH 1/3] OMAP4: prcm: Add temporarily helper functions for rmw and read inside the PRM

[Nayak, Rajendra]

 

> -----Original Message-----
> From: Cousson, Benoit 
> Sent: Monday, August 09, 2010 11:04 PM
> To: Kevin Hilman
> Cc: linux-omap@vger.kernel.org; paul@pwsan.com; Nayak, 
> Rajendra; Shilimkar, Santosh
> Subject: Re: [PATCH 1/3] OMAP4: prcm: Add temporarily helper 
> functions for rmw and read inside the PRM
> 
> On 8/9/2010 7:30 PM, Kevin Hilman wrote:
> > Benoit Cousson<b-cousson@ti.com>  writes:
> >
> >> Since OMAP4 is using an absolute address, the current PRM accessors
> >> are not useable.
> >> OMAP4 adaptation for these API are currently ongoing, so
> define temp
> >> version until the proper ones are defined.
> >
> > Curious what we're waiting for for the final versions of these?
> 
> That should be soon... Rajendra is working on that... among hundreds 
> other stuff.
> 
> Benoit

I just posted a couple of RFC patches for these now which
can be found here
https://patchwork.kernel.org/patch/118566/
https://patchwork.kernel.org/patch/118568/

> 
> > Kevin
> >
> >> Signed-off-by: Benoit Cousson<b-cousson@ti.com>
> >> Cc: Paul Walmsley<paul@pwsan.com>
> >> Cc: Kevin Hilman<khilman@deeprootsystems.com>
> >> ---
> >>   arch/arm/mach-omap2/prcm.c             |   24 
> ++++++++++++++++++++++++
> >>   arch/arm/plat-omap/include/plat/prcm.h |    2 ++
> >>   2 files changed, 26 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/arch/arm/mach-omap2/prcm.c 
> b/arch/arm/mach-omap2/prcm.c
> >> index 96f4616..d4388d3 100644
> >> --- a/arch/arm/mach-omap2/prcm.c
> >> +++ b/arch/arm/mach-omap2/prcm.c
> >> @@ -216,6 +216,30 @@ u32 prm_read_mod_bits_shift(s16 
> domain, s16 idx, u32 mask)
> >>   	return v;
> >>   }
> >>
> >> +/* Read a PRM register, AND it, and shift the result down 
> to bit 0 */
> >> +u32 omap4_prm_read_bits_shift(void __iomem *reg, u32 mask)
> >> +{
> >> +	u32 v;
> >> +
> >> +	v = __raw_readl(reg);
> >> +	v&= mask;
> >> +	v>>= __ffs(mask);
> >> +
> >> +	return v;
> >> +}
> >> +
> >> +/* Read-modify-write a register in a PRM module. Caller 
> must lock */
> >> +u32 omap4_prm_rmw_reg_bits(u32 mask, u32 bits, void __iomem *reg)
> >> +{
> >> +	u32 v;
> >> +
> >> +	v = __raw_readl(reg);
> >> +	v&= ~mask;
> >> +	v |= bits;
> >> +	__raw_writel(v, reg);
> >> +
> >> +	return v;
> >> +}
> >>   /* Read a register in a CM module */
> >>   u32 cm_read_mod_reg(s16 module, u16 idx)
> >>   {
> >> diff --git a/arch/arm/plat-omap/include/plat/prcm.h 
> b/arch/arm/plat-omap/include/plat/prcm.h
> >> index 9fbd914..ab77442 100644
> >> --- a/arch/arm/plat-omap/include/plat/prcm.h
> >> +++ b/arch/arm/plat-omap/include/plat/prcm.h
> >> @@ -38,6 +38,8 @@ u32 prm_read_mod_reg(s16 module, u16 idx);
> >>   void prm_write_mod_reg(u32 val, s16 module, u16 idx);
> >>   u32 prm_rmw_mod_reg_bits(u32 mask, u32 bits, s16 module, 
> s16 idx);
> >>   u32 prm_read_mod_bits_shift(s16 domain, s16 idx, u32 mask);
> >> +u32 omap4_prm_read_bits_shift(void __iomem *reg, u32 mask);
> >> +u32 omap4_prm_rmw_reg_bits(u32 mask, u32 bits, void __iomem *reg);
> >>   u32 cm_read_mod_reg(s16 module, u16 idx);
> >>   void cm_write_mod_reg(u32 val, s16 module, u16 idx);
> >>   u32 cm_rmw_mod_reg_bits(u32 mask, u32 bits, s16 module, s16 idx);
> 
> 

RE: [PATCH 3/3] OMAP: hwmod: Force a softreset during _setup

[Basak, Partha]

> -----Original Message-----
> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
> owner@vger.kernel.org] On Behalf Of Cousson, Benoit
> Sent: Thursday, August 05, 2010 3:43 AM
> To: linux-omap@vger.kernel.org; khilman@deeprootsystems.com;
> paul@pwsan.com
> Cc: Nayak, Rajendra; Shilimkar, Santosh; Cousson, Benoit
> Subject: [PATCH 3/3] OMAP: hwmod: Force a softreset during _setup
> 
> Force the softreset of every IPs during the _setup phase.
> IPs that cannot support softreset or that should not
> be reset must set the HWMOD_INIT_NO_RESET flag in the
> hwmod struct.
> 
> Signed-off-by: Benoit Cousson <b-cousson@ti.com>
> Cc: Paul Walmsley <paul@pwsan.com>
> Cc: Kevin Hilman <khilman@deeprootsystems.com>
> ---
>  arch/arm/mach-omap2/omap_hwmod.c |   17 ++++++++---------
>  1 files changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-
> omap2/omap_hwmod.c
> index 53b08e3..91ad9c6 100644
> --- a/arch/arm/mach-omap2/omap_hwmod.c
> +++ b/arch/arm/mach-omap2/omap_hwmod.c
> @@ -856,8 +856,8 @@ static int _reset(struct omap_hwmod *oh)
> 
>  	/* clocks must be on for this operation */
>  	if (oh->_state != _HWMOD_STATE_ENABLED) {
> -		WARN(1, "omap_hwmod: %s: reset can only be entered from "
> -		     "enabled state\n", oh->name);
> +		pr_warning("omap_hwmod: %s: reset can only be entered from "
> +			   "enabled state\n", oh->name);
>  		return -EINVAL;
>  	}
> 
> @@ -874,8 +874,8 @@ static int _reset(struct omap_hwmod *oh)
>  			  MAX_MODULE_RESET_WAIT, c);
> 
>  	if (c == MAX_MODULE_RESET_WAIT)
> -		WARN(1, "omap_hwmod: %s: failed to reset in %d usec\n",
> -		     oh->name, MAX_MODULE_RESET_WAIT);
> +		pr_warning("omap_hwmod: %s: failed to reset in %d usec\n",
> +			   oh->name, MAX_MODULE_RESET_WAIT);

http://focus.ti.com/pdfs/wtbu/SWPU177B_FinalEPDF_12_04_2009.pdf

This is leading to the above warning for DSS on OMAP3430/3630. Referring to the reference manual (7.5.1 Display Subsystem Reset), successful reset of DSS would need PRCM.CM_FCLKEN_DSS[2] EN_TV bit set to 1. For DSS, even though TV clock is an optional clock, this is mandatory for successful DSS reset. We could ignore this warning, or possibly WA it.
One way could be:

1. In the database, have HWMOD_INIT_NO_RESET flag set so that _setup() skips reset.
 
2. After omap device build of DSS is done, lookup the opt-clock and enable it (using clock framework).

3. Then do DSS reset again calling omap_device_reset().

All IPs that potentially have any special soft-reset sequence will need customized handling. Should we keep the framework light and handle such special cases in the drivers? In that case, the framework need not throw up a warning. Please comment.

>  	else
>  		pr_debug("omap_hwmod: %s: reset in %d usec\n", oh->name, c);
> 
> @@ -1074,12 +1074,11 @@ static int _setup(struct omap_hwmod *oh, void
> *data)
>  	}
> 
>  	if (!(oh->flags & HWMOD_INIT_NO_RESET)) {
> +		_reset(oh);
>  		/*
> -		 * XXX Do the OCP_SYSCONFIG bits need to be
> -		 * reprogrammed after a reset?  If not, then this can
> -		 * be removed.  If they do, then probably the
> -		 * _omap_hwmod_enable() function should be split to avoid the
> -		 * rewrite of the OCP_SYSCONFIG register.
> +		 * OCP_SYSCONFIG bits need to be reprogrammed after a
> softreset.
> +		 * The _omap_hwmod_enable() function should be split to
> +		 * avoid the rewrite of the OCP_SYSCONFIG register.
>  		 */
>  		if (oh->class->sysc) {
>  			_update_sysc_cache(oh);
> --
> 1.6.1.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/3] OMAP: hwmod: Force a softreset during _setup

[Cousson, Benoit]

Hi Partha,

On 8/17/2010 2:46 PM, Basak, Partha wrote:
>
>> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
>> owner@vger.kernel.org] On Behalf Of Cousson, Benoit
>> Sent: Thursday, August 05, 2010 3:43 AM
>>
>> Force the softreset of every IPs during the _setup phase.
>> IPs that cannot support softreset or that should not
>> be reset must set the HWMOD_INIT_NO_RESET flag in the
>> hwmod struct.
>>
>> Signed-off-by: Benoit Cousson<b-cousson@ti.com>
>> Cc: Paul Walmsley<paul@pwsan.com>
>> Cc: Kevin Hilman<khilman@deeprootsystems.com>
>> ---
>>   arch/arm/mach-omap2/omap_hwmod.c |   17 ++++++++---------
>>   1 files changed, 8 insertions(+), 9 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-
>> omap2/omap_hwmod.c
>> index 53b08e3..91ad9c6 100644
>> --- a/arch/arm/mach-omap2/omap_hwmod.c
>> +++ b/arch/arm/mach-omap2/omap_hwmod.c
>> @@ -856,8 +856,8 @@ static int _reset(struct omap_hwmod *oh)
>>
>>   	/* clocks must be on for this operation */
>>   	if (oh->_state != _HWMOD_STATE_ENABLED) {
>> -		WARN(1, "omap_hwmod: %s: reset can only be entered from "
>> -		     "enabled state\n", oh->name);
>> +		pr_warning("omap_hwmod: %s: reset can only be entered from "
>> +			   "enabled state\n", oh->name);
>>   		return -EINVAL;
>>   	}
>>
>> @@ -874,8 +874,8 @@ static int _reset(struct omap_hwmod *oh)
>>   			  MAX_MODULE_RESET_WAIT, c);
>>
>>   	if (c == MAX_MODULE_RESET_WAIT)
>> -		WARN(1, "omap_hwmod: %s: failed to reset in %d usec\n",
>> -		     oh->name, MAX_MODULE_RESET_WAIT);
>> +		pr_warning("omap_hwmod: %s: failed to reset in %d usec\n",
>> +			   oh->name, MAX_MODULE_RESET_WAIT);
>
> http://focus.ti.com/pdfs/wtbu/SWPU177B_FinalEPDF_12_04_2009.pdf
>
> This is leading to the above warning for DSS on OMAP3430/3630. Referring to the reference manual (7.5.1 Display Subsystem Reset), successful reset of DSS would need PRCM.CM_FCLKEN_DSS[2] EN_TV bit set to 1. For DSS, even though TV clock is an optional clock, this is mandatory for successful DSS reset. We could ignore this warning, or possibly WA it.
> One way could be:
>
> 1. In the database, have HWMOD_INIT_NO_RESET flag set so that _setup() skips reset.
>
> 2. After omap device build of DSS is done, lookup the opt-clock and enable it (using clock framework).
>
> 3. Then do DSS reset again calling omap_device_reset().
>
> All IPs that potentially have any special soft-reset sequence will need customized handling. Should we keep the framework light and handle such special cases in the drivers? In that case, the framework need not throw up a warning. Please comment.

If the softreset is not mandatory for an IP like DSS, you just have to 
set this HWMOD_INIT_NO_RESET flag.
There is no need to use the softreset for every IP, the PRCM already 
resets every IPs each time the power domain is powered on.
softreset is useful if you need to recover from an HW error.

Regards,
Benoit

RE: [PATCH 3/3] OMAP: hwmod: Force a softreset during _setup

[Basak, Partha]

> -----Original Message-----
> From: Cousson, Benoit
> Sent: Wednesday, August 18, 2010 8:31 PM
> To: Basak, Partha
> Cc: Nayak, Rajendra; Shilimkar, Santosh; linux-omap@vger.kernel.org;
> khilman@deeprootsystems.com; paul@pwsan.com
> Subject: Re: [PATCH 3/3] OMAP: hwmod: Force a softreset during _setup
> 
> Hi Partha,
> 
> On 8/17/2010 2:46 PM, Basak, Partha wrote:
> >
> >> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
> >> owner@vger.kernel.org] On Behalf Of Cousson, Benoit
> >> Sent: Thursday, August 05, 2010 3:43 AM
> >>
> >> Force the softreset of every IPs during the _setup phase.
> >> IPs that cannot support softreset or that should not
> >> be reset must set the HWMOD_INIT_NO_RESET flag in the
> >> hwmod struct.
> >>
> >> Signed-off-by: Benoit Cousson<b-cousson@ti.com>
> >> Cc: Paul Walmsley<paul@pwsan.com>
> >> Cc: Kevin Hilman<khilman@deeprootsystems.com>
> >> ---
> >>   arch/arm/mach-omap2/omap_hwmod.c |   17 ++++++++---------
> >>   1 files changed, 8 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-
> >> omap2/omap_hwmod.c
> >> index 53b08e3..91ad9c6 100644
> >> --- a/arch/arm/mach-omap2/omap_hwmod.c
> >> +++ b/arch/arm/mach-omap2/omap_hwmod.c
> >> @@ -856,8 +856,8 @@ static int _reset(struct omap_hwmod *oh)
> >>
> >>   	/* clocks must be on for this operation */
> >>   	if (oh->_state != _HWMOD_STATE_ENABLED) {
> >> -		WARN(1, "omap_hwmod: %s: reset can only be entered from "
> >> -		     "enabled state\n", oh->name);
> >> +		pr_warning("omap_hwmod: %s: reset can only be entered from "
> >> +			   "enabled state\n", oh->name);
> >>   		return -EINVAL;
> >>   	}
> >>
> >> @@ -874,8 +874,8 @@ static int _reset(struct omap_hwmod *oh)
> >>   			  MAX_MODULE_RESET_WAIT, c);
> >>
> >>   	if (c == MAX_MODULE_RESET_WAIT)
> >> -		WARN(1, "omap_hwmod: %s: failed to reset in %d usec\n",
> >> -		     oh->name, MAX_MODULE_RESET_WAIT);
> >> +		pr_warning("omap_hwmod: %s: failed to reset in %d usec\n",
> >> +			   oh->name, MAX_MODULE_RESET_WAIT);
> >
> > http://focus.ti.com/pdfs/wtbu/SWPU177B_FinalEPDF_12_04_2009.pdf
> >
> > This is leading to the above warning for DSS on OMAP3430/3630. Referring
> to the reference manual (7.5.1 Display Subsystem Reset), successful reset
> of DSS would need PRCM.CM_FCLKEN_DSS[2] EN_TV bit set to 1. For DSS, even
> though TV clock is an optional clock, this is mandatory for successful DSS
> reset. We could ignore this warning, or possibly WA it.
> > One way could be:
> >
> > 1. In the database, have HWMOD_INIT_NO_RESET flag set so that _setup()
> skips reset.
> >
> > 2. After omap device build of DSS is done, lookup the opt-clock and
> enable it (using clock framework).
> >
>>
> > 4. Then do DSS reset again calling omap_device_reset().
> >
> > All IPs that potentially have any special soft-reset sequence will need
> customized handling. Should we keep the framework light and handle such
> special cases in the drivers? In that case, the framework need not throw
> up a warning. Please comment.
> 
> If the softreset is not mandatory for an IP like DSS, you just have to
> set this HWMOD_INIT_NO_RESET flag.
> There is no need to use the softreset for every IP, the PRCM already
> resets every IPs each time the power domain is powered on.
> softreset is useful if you need to recover from an HW error.
> 

I agree, however without doing soft-reset, we have observed DSI malfunction. Senthil can provide more details. As DSS needs TV_clk for successful reset, I doubt whether PRCM can reset DSS once we merely turn on the DSS power domain.
So, it really depends on the IP in question. If it is necessary, we will do a omap_device_reset()in the driver. Correct?

> Regards,
> Benoit

Re: [PATCH 3/3] OMAP: hwmod: Force a softreset during _setup

[Cousson, Benoit]

On 8/19/2010 1:57 PM, Basak, Partha wrote:
>> -----Original Message-----
>> From: Cousson, Benoit
>> Sent: Wednesday, August 18, 2010 8:31 PM
>> To: Basak, Partha
>> Cc: Nayak, Rajendra; Shilimkar, Santosh; linux-omap@vger.kernel.org;
>> khilman@deeprootsystems.com; paul@pwsan.com
>> Subject: Re: [PATCH 3/3] OMAP: hwmod: Force a softreset during _setup
>>
>> Hi Partha,
>>
>> On 8/17/2010 2:46 PM, Basak, Partha wrote:
>>>
>>>> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
>>>> owner@vger.kernel.org] On Behalf Of Cousson, Benoit
>>>> Sent: Thursday, August 05, 2010 3:43 AM
>>>>
>>>> Force the softreset of every IPs during the _setup phase.
>>>> IPs that cannot support softreset or that should not
>>>> be reset must set the HWMOD_INIT_NO_RESET flag in the
>>>> hwmod struct.
>>>>
>>>> Signed-off-by: Benoit Cousson<b-cousson@ti.com>
>>>> Cc: Paul Walmsley<paul@pwsan.com>
>>>> Cc: Kevin Hilman<khilman@deeprootsystems.com>
>>>> ---
>>>>    arch/arm/mach-omap2/omap_hwmod.c |   17 ++++++++---------
>>>>    1 files changed, 8 insertions(+), 9 deletions(-)
>>>>
>>>> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-
>>>> omap2/omap_hwmod.c
>>>> index 53b08e3..91ad9c6 100644
>>>> --- a/arch/arm/mach-omap2/omap_hwmod.c
>>>> +++ b/arch/arm/mach-omap2/omap_hwmod.c
>>>> @@ -856,8 +856,8 @@ static int _reset(struct omap_hwmod *oh)
>>>>
>>>>    	/* clocks must be on for this operation */
>>>>    	if (oh->_state != _HWMOD_STATE_ENABLED) {
>>>> -		WARN(1, "omap_hwmod: %s: reset can only be entered from "
>>>> -		     "enabled state\n", oh->name);
>>>> +		pr_warning("omap_hwmod: %s: reset can only be entered from "
>>>> +			   "enabled state\n", oh->name);
>>>>    		return -EINVAL;
>>>>    	}
>>>>
>>>> @@ -874,8 +874,8 @@ static int _reset(struct omap_hwmod *oh)
>>>>    			  MAX_MODULE_RESET_WAIT, c);
>>>>
>>>>    	if (c == MAX_MODULE_RESET_WAIT)
>>>> -		WARN(1, "omap_hwmod: %s: failed to reset in %d usec\n",
>>>> -		     oh->name, MAX_MODULE_RESET_WAIT);
>>>> +		pr_warning("omap_hwmod: %s: failed to reset in %d usec\n",
>>>> +			   oh->name, MAX_MODULE_RESET_WAIT);
>>>
>>> http://focus.ti.com/pdfs/wtbu/SWPU177B_FinalEPDF_12_04_2009.pdf
>>>
>>> This is leading to the above warning for DSS on OMAP3430/3630. Referring
>> to the reference manual (7.5.1 Display Subsystem Reset), successful reset
>> of DSS would need PRCM.CM_FCLKEN_DSS[2] EN_TV bit set to 1. For DSS, even
>> though TV clock is an optional clock, this is mandatory for successful DSS
>> reset. We could ignore this warning, or possibly WA it.
>>> One way could be:
>>>
>>> 1. In the database, have HWMOD_INIT_NO_RESET flag set so that _setup()
>> skips reset.
>>>
>>> 2. After omap device build of DSS is done, lookup the opt-clock and
>> enable it (using clock framework).
>>>
>>>
>>> 4. Then do DSS reset again calling omap_device_reset().
>>>
>>> All IPs that potentially have any special soft-reset sequence will need
>> customized handling. Should we keep the framework light and handle such
>> special cases in the drivers? In that case, the framework need not throw
>> up a warning. Please comment.
>>
>> If the softreset is not mandatory for an IP like DSS, you just have to
>> set this HWMOD_INIT_NO_RESET flag.
>> There is no need to use the softreset for every IP, the PRCM already
>> resets every IPs each time the power domain is powered on.
>> softreset is useful if you need to recover from an HW error.
>>
>
> I agree, however without doing soft-reset, we have observed DSI malfunction. Senthil can provide more details. As DSS needs TV_clk for successful reset, I doubt whether PRCM can reset DSS once we merely turn on the DSS power domain.

After a check with Jean, The DSS will propagate the reset to sub-IPs and 
only the one that are properly clocked will complete the reset. So the 
DSS reset status cannot transition until all sub-IPs have completed the 
reset. The issue should not exist on OMAP4 because the sys_clk is 
available for all DSS sub-IPs.

> So, it really depends on the IP in question. If it is necessary, we will do a omap_device_reset()in the driver. Correct?

We should avoid that. We can either allow a custom hooks to allow IPs 
with specifics reset / sysconfig management to change the default 
behavior. Or we can find a generic way to handle all these specifics cases.

I know that Paul have some ideas on that.

Regards,
Benoit

Re: [PATCH 3/3] OMAP: hwmod: Force a softreset during _setup

[Cousson, Benoit]

Hi Partha,

Here is a small patch that should fix that issue:
The approach is quite basic, I'm just enabling all the optional clocks 
if the flag is set.

Could you give it a try?

Thanks,
Benoit


---
 From 37cda332362d58e584cf3df190a9dceb9c9db8ab Mon Sep 17 00:00:00 2001
From: Benoit Cousson <b-cousson@ti.com>
Date: Mon, 23 Aug 2010 15:45:24 +0200
Subject: [PATCH] OMAP: hwmod: Fix softreset for modules with optional clocks

Some modules (like GPIO, DSS...) require optionals clock to be enabled
in order to complete the sofreset properly.
Add a HWMOD_CONTROL_OPT_CLKS_IN_RESET flag to force all optional clocks
to be enabled before reset. Disabled them once the reset is done.

Reported-by: Partha Basak <p-basak2@ti.com>
Signed-off-by: Benoit Cousson <b-cousson@ti.com>
Cc: Paul Walmsley <paul@pwsan.com>
Cc: Kevin Hilman <khilman@deeprootsystems.com>
---
  arch/arm/mach-omap2/omap_hwmod.c |   53 
+++++++++++++++++++++++++++++++++----
  1 files changed, 47 insertions(+), 6 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod.c 
b/arch/arm/mach-omap2/omap_hwmod.c
index 9bd99ad..5466c30 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -545,6 +545,36 @@ static int _disable_clocks(struct omap_hwmod *oh)
  	return 0;
  }

+static void _enable_optional_clocks(struct omap_hwmod *oh)
+{
+	struct omap_hwmod_opt_clk *oc;
+	int i;
+
+	pr_debug("omap_hwmod: %s: enabling optional clocks\n", oh->name);
+
+	for (i = oh->opt_clks_cnt, oc = oh->opt_clks; i > 0; i--, oc++)
+		if (oc->_clk) {
+			pr_debug("omap_hwmod: enable %s:%s\n", oc->role,
+				   oc->_clk->name);
+			clk_enable(oc->_clk);
+		}
+}
+
+static void _disable_optional_clocks(struct omap_hwmod *oh)
+{
+	struct omap_hwmod_opt_clk *oc;
+	int i;
+
+	pr_debug("omap_hwmod: %s: disabling optional clocks\n", oh->name);
+
+	for (i = oh->opt_clks_cnt, oc = oh->opt_clks; i > 0; i--, oc++)
+		if (oc->_clk) {
+			pr_debug("omap_hwmod: disable %s:%s\n", oc->role,
+				   oc->_clk->name);
+			clk_disable(oc->_clk);
+		}
+}
+
  /**
   * _find_mpu_port_index - find hwmod OCP slave port ID intended for 
MPU use
   * @oh: struct omap_hwmod *
@@ -845,8 +875,9 @@ static int _wait_target_ready(struct omap_hwmod *oh)
   */
  static int _reset(struct omap_hwmod *oh)
  {
-	u32 r, v;
+	u32 v;
  	int c = 0;
+	int ret = 0;

  	if (!oh->class->sysc ||
  	    !(oh->class->sysc->sysc_flags & SYSC_HAS_SOFTRESET) ||
@@ -860,12 +891,16 @@ static int _reset(struct omap_hwmod *oh)
  		return -EINVAL;
  	}

-	pr_debug("omap_hwmod: %s: resetting\n", oh->name);
+	/* For some modules, all optionnal clocks need to be enabled as well */
+	if (oh->flags & HWMOD_CONTROL_OPT_CLKS_IN_RESET)
+		_enable_optional_clocks(oh);
+
+	pr_debug("omap_hwmod: %s: resetting\n", oh->name);

  	v = oh->_sysc_cache;
-	r = _set_softreset(oh, &v);
-	if (r)
-		return r;
+	ret = _set_softreset(oh, &v);
+	if (ret)
+		goto dis_opt_clks;
  	_write_sysconfig(v, oh);

  	omap_test_timeout((omap_hwmod_readl(oh, oh->class->sysc->syss_offs) &
@@ -883,7 +918,13 @@ static int _reset(struct omap_hwmod *oh)
  	 * _wait_target_ready() or _reset()
  	 */

-	return (c == MAX_MODULE_RESET_WAIT) ? -ETIMEDOUT : 0;
+	ret = (c == MAX_MODULE_RESET_WAIT) ? -ETIMEDOUT : 0;
+
+dis_opt_clks:
+	if (oh->flags & HWMOD_CONTROL_OPT_CLKS_IN_RESET)
+		_disable_optional_clocks(oh);
+
+	return ret;
  }

  /**
-- 
1.6.0.4


On 8/19/2010 1:57 PM, Basak, Partha wrote:
>> From: Cousson, Benoit
>> Sent: Wednesday, August 18, 2010 8:31 PM
>>
>> Hi Partha,
>>
>> On 8/17/2010 2:46 PM, Basak, Partha wrote:
>>>
>>>> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
>>>> owner@vger.kernel.org] On Behalf Of Cousson, Benoit
>>>> Sent: Thursday, August 05, 2010 3:43 AM
>>>>
>>>> Force the softreset of every IPs during the _setup phase.
>>>> IPs that cannot support softreset or that should not
>>>> be reset must set the HWMOD_INIT_NO_RESET flag in the
>>>> hwmod struct.
>>>>
>>>> Signed-off-by: Benoit Cousson<b-cousson@ti.com>
>>>> Cc: Paul Walmsley<paul@pwsan.com>
>>>> Cc: Kevin Hilman<khilman@deeprootsystems.com>
>>>> ---
>>>>    arch/arm/mach-omap2/omap_hwmod.c |   17 ++++++++---------
>>>>    1 files changed, 8 insertions(+), 9 deletions(-)
>>>>
>>>> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-
>>>> omap2/omap_hwmod.c
>>>> index 53b08e3..91ad9c6 100644
>>>> --- a/arch/arm/mach-omap2/omap_hwmod.c
>>>> +++ b/arch/arm/mach-omap2/omap_hwmod.c
>>>> @@ -856,8 +856,8 @@ static int _reset(struct omap_hwmod *oh)
>>>>
>>>>    	/* clocks must be on for this operation */
>>>>    	if (oh->_state != _HWMOD_STATE_ENABLED) {
>>>> -		WARN(1, "omap_hwmod: %s: reset can only be entered from "
>>>> -		     "enabled state\n", oh->name);
>>>> +		pr_warning("omap_hwmod: %s: reset can only be entered from "
>>>> +			   "enabled state\n", oh->name);
>>>>    		return -EINVAL;
>>>>    	}
>>>>
>>>> @@ -874,8 +874,8 @@ static int _reset(struct omap_hwmod *oh)
>>>>    			  MAX_MODULE_RESET_WAIT, c);
>>>>
>>>>    	if (c == MAX_MODULE_RESET_WAIT)
>>>> -		WARN(1, "omap_hwmod: %s: failed to reset in %d usec\n",
>>>> -		     oh->name, MAX_MODULE_RESET_WAIT);
>>>> +		pr_warning("omap_hwmod: %s: failed to reset in %d usec\n",
>>>> +			   oh->name, MAX_MODULE_RESET_WAIT);
>>>
>>> http://focus.ti.com/pdfs/wtbu/SWPU177B_FinalEPDF_12_04_2009.pdf
>>>
>>> This is leading to the above warning for DSS on OMAP3430/3630. Referring
>> to the reference manual (7.5.1 Display Subsystem Reset), successful reset
>> of DSS would need PRCM.CM_FCLKEN_DSS[2] EN_TV bit set to 1. For DSS, even
>> though TV clock is an optional clock, this is mandatory for successful DSS
>> reset. We could ignore this warning, or possibly WA it.
>>> One way could be:
>>>
>>> 1. In the database, have HWMOD_INIT_NO_RESET flag set so that _setup()
>> skips reset.
>>>
>>> 2. After omap device build of DSS is done, lookup the opt-clock and
>> enable it (using clock framework).
>>>
>>>
>>> 4. Then do DSS reset again calling omap_device_reset().
>>>
>>> All IPs that potentially have any special soft-reset sequence will need
>> customized handling. Should we keep the framework light and handle such
>> special cases in the drivers? In that case, the framework need not throw
>> up a warning. Please comment.
>>
>> If the softreset is not mandatory for an IP like DSS, you just have to
>> set this HWMOD_INIT_NO_RESET flag.
>> There is no need to use the softreset for every IP, the PRCM already
>> resets every IPs each time the power domain is powered on.
>> softreset is useful if you need to recover from an HW error.
>>
>
> I agree, however without doing soft-reset, we have observed DSI malfunction. Senthil can provide more details. As DSS needs TV_clk for successful reset, I doubt whether PRCM can reset DSS once we merely turn on the DSS power domain.
> So, it really depends on the IP in question. If it is necessary, we will do a omap_device_reset()in the driver. Correct?
>
>> Regards,
>> Benoit

Re: [PATCH 3/3] OMAP: hwmod: Force a softreset during _setup

[Paul Walmsley]

On Mon, 23 Aug 2010, Cousson, Benoit wrote:

> Here is a small patch that should fix that issue:
> The approach is quite basic, I'm just enabling all the optional clocks if the
> flag is set.
> 
> Could you give it a try?

This approach seems good.  Maybe we should add a flags field in 
struct omap_hwmod_opt_clks to indicate that a particular clock needs to be 
enabled during the reset process?  That way the specific clocks that are 
necessary can be enabled, rather than all of the clocks.

One other general comment on an earlier part of this discussion: the goal 
is to reset almost every module during boot: this is to clear any bogus 
configurations that the bootloader may have written to the hardware.  The 
goal is to have a consistent, known hardware configuration in the kernel, 
independent of the bootloader.

Partha, does this patch work for you?

- Paul

Re: [PATCH 0/3] OMAP: hwmod: Add hardreset and softreset management

[Paul Walmsley]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1160 bytes --]

Hi Benoît,

I regret the delay -

On Thu, 5 Aug 2010, Benoit Cousson wrote:

> Here are a reset management series.
> 
> - The first patch will be removed as soon as we will have the proper
>   OMAP4 support for the prm_xxx accessors.
> - The second one is adding hardreset support in order to allow
>   syslink driver to manage properly the DSP and IPU processor
>   reset.
> - The last one is forcing a sofreset after the first init. 
>   Some IP might require sofreset after each wakeup
>   from power domain OFF mode. That still needs to be confirm.

I've reviewed these patches and made a few changes.  The PRM register 
twiddling has been moved out into a prm44xx.c file.  I also added OMAP2/3 
hardreset support, based on your code.  Could you take a quick look at the 
updated series (in subsequent E-mails, or the git branch below) and let me 
know if you are okay with it?  I took some liberties in preserving your 
changelogs and Signed-off-by's across the changes, so please let me know 
if you'd like any changes in that regard.

The git branch is hosted at:

git://git.pwsan.com/linux-2.6 hwmod_hardreset_dev


- Paul

Re: [PATCH 0/3] OMAP: hwmod: Add hardreset and softreset management

[Cousson, Benoit]

Hi Paul,

On 9/21/2010 7:03 AM, Paul Walmsley wrote:
> Hi Benoît,
>
> I regret the delay -

Considering the job you did on that reset series, you are already 
forgiven :-)

>
> On Thu, 5 Aug 2010, Benoit Cousson wrote:
>
>> Here are a reset management series.
>>
>> - The first patch will be removed as soon as we will have the proper
>>    OMAP4 support for the prm_xxx accessors.
>> - The second one is adding hardreset support in order to allow
>>    syslink driver to manage properly the DSP and IPU processor
>>    reset.
>> - The last one is forcing a sofreset after the first init.
>>    Some IP might require sofreset after each wakeup
>>    from power domain OFF mode. That still needs to be confirm.
>
> I've reviewed these patches and made a few changes.  The PRM register
> twiddling has been moved out into a prm44xx.c file.  I also added OMAP2/3
> hardreset support, based on your code.  Could you take a quick look at the
> updated series (in subsequent E-mails, or the git branch below) and let me
> know if you are okay with it?  I took some liberties in preserving your
> changelogs and Signed-off-by's across the changes, so please let me know
> if you'd like any changes in that regard.

That's really good. The low level reset does indeed belong to PRM more 
than the hwmod core code.
Thanks for the OMAP2/3 stuff as well, IVA people were starting to ping 
me for that support.

I've just rebased my OMAP4 hwmods series on top on this one, and it work 
fine.

Are you targeting that series for 2.6.37?
I have a couple of patches on top of that code that will fix the 
softreset with optional clocks. This code is fixing the GPIO reset issue 
that I observed in the past.

I'll send you that soon.

Thanks,
Benoit

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/3] OMAP: hwmod: Add hardreset and softreset management

[Paul Walmsley]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1004 bytes --]

Hi Benoît,

On Tue, 21 Sep 2010, Cousson, Benoit wrote:

> That's really good. The low level reset does indeed belong to PRM more than
> the hwmod core code.
> Thanks for the OMAP2/3 stuff as well, IVA people were starting to ping me for
> that support.
> 
> I've just rebased my OMAP4 hwmods series on top on this one, and it work fine.

Cool.

> Are you targeting that series for 2.6.37?

Yep, I'll send those patches to the linux-arm-kernel mailing list today, 
along with the other hwmod base patches; I'd like to get those in for 
2.6.37.

> I have a couple of patches on top of that code that will fix the softreset
> with optional clocks. This code is fixing the GPIO reset issue that I observed
> in the past.
> 
> I'll send you that soon.

OK, sounds good.  If you cc the linux-arm-kernel mailing list when you 
send them out, that will make it easier to add those to the 2.6.37 queue 
if they arrive on time.

Thanks for the review and for the comments,


- Paul