[PATCH 0/8] omap_hwmod/omap_device prep work

[PATCH 0/8] omap_hwmod/omap_device prep work

[Kevin Hilman]

In converting the UART drivers over to omap_hwmod/omap_device, I
found a few issues. 

Kevin Hilman (7):
  OMAP3: clock: add clockdomains for UART1 & 2
  OMAP: hwmod: warn on missing clockdomain
  OMAP: omap_device: deactivate latency typo
  OMAP: omap_device: add usecounting
  OMAP: omap_device: use read_persistent_clock() instead of
    getnstimeofday()
  OMAP: omap_device: warn about expected latencies only if non-zero
  OMAP: omap_device: use UINT_MAX for default wakeup latency limit

Paul Walmsley (1):
  OMAP3 hwmod: Add automatic OCP_SYSCONFIG AUTOIDLE handling

 arch/arm/mach-omap2/clock34xx.h               |    2 +
 arch/arm/mach-omap2/omap_hwmod.c              |   53 ++++++++++++++++++++++--
 arch/arm/plat-omap/include/plat/omap_device.h |    1 +
 arch/arm/plat-omap/include/plat/omap_hwmod.h  |    8 +++-
 arch/arm/plat-omap/omap_device.c              |   43 +++++++++++++-------
 5 files changed, 86 insertions(+), 21 deletions(-)

[PATCH 1/8] OMAP3: clock: add clockdomains for UART1 & 2

[Kevin Hilman]

UART1 & 2 were missing clockdomains resulting in broken omap_hwmod
init for these devices.

Signed-off-by: Kevin Hilman <khilman@deeprootsystems.com>
---
 arch/arm/mach-omap2/clock34xx.h |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/clock34xx.h b/arch/arm/mach-omap2/clock34xx.h
index a1b3de7..cbc3d8a 100644
--- a/arch/arm/mach-omap2/clock34xx.h
+++ b/arch/arm/mach-omap2/clock34xx.h
@@ -1507,6 +1507,7 @@ static struct clk uart2_fck = {
 	.parent		= &core_48m_fck,
 	.enable_reg	= OMAP_CM_REGADDR(CORE_MOD, CM_FCLKEN1),
 	.enable_bit	= OMAP3430_EN_UART2_SHIFT,
+	.clkdm_name	= "core_l4_clkdm",
 	.recalc		= &followparent_recalc,
 };
 
@@ -1516,6 +1517,7 @@ static struct clk uart1_fck = {
 	.parent		= &core_48m_fck,
 	.enable_reg	= OMAP_CM_REGADDR(CORE_MOD, CM_FCLKEN1),
 	.enable_bit	= OMAP3430_EN_UART1_SHIFT,
+	.clkdm_name	= "core_l4_clkdm",
 	.recalc		= &followparent_recalc,
 };
 
-- 
1.6.5.1

[PATCH 2/8] OMAP: hwmod: warn on missing clockdomain

[Kevin Hilman]

WARN if a clock/hwmod is missing a clockdomain association since
resulting hwmod will not be able to correctly enable/disable clocks.

Signed-off-by: Kevin Hilman <khilman@deeprootsystems.com>
---
 arch/arm/mach-omap2/omap_hwmod.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index 633b216..7d7b3b8 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -326,6 +326,9 @@ static int _init_main_clk(struct omap_hwmod *oh)
 		ret = -EINVAL;
 	oh->_clk = c;
 
+	WARN(!c->clkdm, "omap_hwmod: %s: missing clockdomain for %s.\n",
+	     oh->clkdev_con_id, c->name);
+
 	return ret;
 }
 
-- 
1.6.5.1

[PATCH 3/8] OMAP3 hwmod: Add automatic OCP_SYSCONFIG AUTOIDLE handling

[Kevin Hilman]

From: Paul Walmsley <paul@pwsan.com>

This patch fills in the OCP_SYSCONFIG.AUTOIDLE handling in the OMAP
hwmod code.

After this patch, the hwmod code will set the module AUTOIDLE bit (generally
<module>.OCP_SYSCONFIG.AUTOIDLE) to 1 by default upon enable, and 1 upon
disable.  If the hwmod flag HWMOD_NO_AUTOIDLE is set, AUTOIDLE will be
set to 0 upon enable.

Enabling module autoidle should save some power.  The only reason to
not set the OCP_SYSCONFIG.AUTOIDLE bit is if there is a bug in the
module RTL, e.g., the MPUINTC block on OMAP3.

Comments from Kevin Hilman <khilman@deeprootsystems.com> inspired this patch.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Tested-by: Kevin Hilman <khilman@deeprootsystems.com>
---
 arch/arm/mach-omap2/omap_hwmod.c             |   50 +++++++++++++++++++++++---
 arch/arm/plat-omap/include/plat/omap_hwmod.h |    8 ++++-
 2 files changed, 52 insertions(+), 6 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index 7d7b3b8..25d9484 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -210,6 +210,29 @@ static int _set_softreset(struct omap_hwmod *oh, u32 *v)
 }
 
 /**
+ * _set_module_autoidle: set the OCP_SYSCONFIG AUTOIDLE field in @v
+ * @oh: struct omap_hwmod *
+ * @autoidle: desired AUTOIDLE bitfield value (0 or 1)
+ * @v: pointer to register contents to modify
+ *
+ * Update the module autoidle mode bit in @v to be @autoidle for the
+ * @oh hwmod.  Does not write to the hardware.  Returns -EINVAL upon
+ * error or 0 upon success.
+ */
+static int _set_module_autoidle(struct omap_hwmod *oh, u8 autoidle,
+				u32 *v)
+{
+	if (!oh->sysconfig ||
+	    !(oh->sysconfig->sysc_flags & SYSC_HAS_AUTOIDLE))
+		return -EINVAL;
+
+	*v &= ~SYSC_AUTOIDLE_MASK;
+	*v |= autoidle << SYSC_AUTOIDLE_SHIFT;
+
+	return 0;
+}
+
+/**
  * _enable_wakeup: set OCP_SYSCONFIG.ENAWAKEUP bit in the hardware
  * @oh: struct omap_hwmod *
  *
@@ -560,7 +583,13 @@ static void _sysc_enable(struct omap_hwmod *oh)
 		_set_master_standbymode(oh, idlemode, &v);
 	}
 
-	/* XXX OCP AUTOIDLE bit? */
+	if (oh->sysconfig->sysc_flags & SYSC_HAS_AUTOIDLE) {
+		idlemode = (oh->flags & HWMOD_NO_AUTOIDLE) ?
+			0 : 1;
+		_set_module_autoidle(oh, idlemode, &v);
+	}
+
+	/* XXX OCP ENAWAKEUP bit? */
 
 	if (oh->flags & HWMOD_SET_DEFAULT_CLOCKACT &&
 	    oh->sysconfig->sysc_flags & SYSC_HAS_CLOCKACTIVITY)
@@ -625,7 +654,8 @@ static void _sysc_shutdown(struct omap_hwmod *oh)
 	if (oh->sysconfig->sysc_flags & SYSC_HAS_MIDLEMODE)
 		_set_master_standbymode(oh, HWMOD_IDLEMODE_FORCE, &v);
 
-	/* XXX clear OCP AUTOIDLE bit? */
+	if (oh->sysconfig->sysc_flags & SYSC_HAS_AUTOIDLE)
+		_set_module_autoidle(oh, 1, &v);
 
 	_write_sysconfig(v, oh);
 }
@@ -951,11 +981,21 @@ static int _setup(struct omap_hwmod *oh)
 
 	_enable(oh);
 
-	if (!(oh->flags & HWMOD_INIT_NO_RESET))
+	if (!(oh->flags & HWMOD_INIT_NO_RESET)) {
 		_reset(oh);
 
-	/* XXX OCP AUTOIDLE bit? */
-	/* XXX OCP ENAWAKEUP bit? */
+		/*
+		 * XXX Do the OCP_SYSCONFIG bits need to be
+		 * reprogrammed after a reset?  If not, then this can
+		 * be removed.  If it does, then probably the
+		 * _enable() function should be split to avoid the
+		 * rewrite of the OCP_SYSCONFIG register.
+		 */
+		if (oh->sysconfig) {
+			_update_sysc_cache(oh);
+			_sysc_enable(oh);
+		}
+	}
 
 	if (!(oh->flags & HWMOD_INIT_NO_IDLE))
 		_idle(oh);
diff --git a/arch/arm/plat-omap/include/plat/omap_hwmod.h b/arch/arm/plat-omap/include/plat/omap_hwmod.h
index dbdd123..ec140b0 100644
--- a/arch/arm/plat-omap/include/plat/omap_hwmod.h
+++ b/arch/arm/plat-omap/include/plat/omap_hwmod.h
@@ -50,6 +50,8 @@ struct omap_device;
 #define SYSC_ENAWAKEUP_MASK		(1 << SYSC_ENAWAKEUP_SHIFT)
 #define SYSC_SOFTRESET_SHIFT		1
 #define SYSC_SOFTRESET_MASK		(1 << SYSC_SOFTRESET_SHIFT)
+#define SYSC_AUTOIDLE_SHIFT		0
+#define SYSC_AUTOIDLE_MASK		(1 << SYSC_AUTOIDLE_SHIFT)
 
 /* OCP SYSSTATUS bit shifts/masks */
 #define SYSS_RESETDONE_SHIFT		0
@@ -294,13 +296,17 @@ struct omap_hwmod_omap4_prcm {
  *     SDRAM controller, etc.
  * HWMOD_INIT_NO_IDLE: don't idle this module at boot - important for SDRAM
  *     controller, etc.
+ * HWMOD_NO_AUTOIDLE: disable module autoidle (OCP_SYSCONFIG.AUTOIDLE)
+ *     when module is enabled, rather than the default, which is to
+ *     enable autoidle
  * HWMOD_SET_DEFAULT_CLOCKACT: program CLOCKACTIVITY bits at startup
  */
 #define HWMOD_SWSUP_SIDLE			(1 << 0)
 #define HWMOD_SWSUP_MSTANDBY			(1 << 1)
 #define HWMOD_INIT_NO_RESET			(1 << 2)
 #define HWMOD_INIT_NO_IDLE			(1 << 3)
-#define HWMOD_SET_DEFAULT_CLOCKACT		(1 << 4)
+#define HWMOD_NO_AUTOIDLE			(1 << 4)
+#define HWMOD_SET_DEFAULT_CLOCKACT		(1 << 5)
 
 /*
  * omap_hwmod._int_flags definitions
-- 
1.6.5.1

[PATCH 4/8] OMAP: omap_device: deactivate latency typo

[Kevin Hilman]

The deactivate hook should use 'deactivate_lat' instead of
'activate_lat' when doing checking on expected latency values.

Signed-off-by: Kevin Hilman <khilman@deeprootsystems.com>
---
 arch/arm/plat-omap/omap_device.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c
index bb16e62..da649f2 100644
--- a/arch/arm/plat-omap/omap_device.c
+++ b/arch/arm/plat-omap/omap_device.c
@@ -186,7 +186,7 @@ static int _omap_device_deactivate(struct omap_device *od, u8 ignore_lat)
 		odpl = od->pm_lats + od->pm_lat_level;
 
 		if (!ignore_lat &&
-		    ((od->dev_wakeup_lat + odpl->activate_lat) >
+		    ((od->dev_wakeup_lat + odpl->deactivate_lat) >
 		     od->_dev_wakeup_lat_limit))
 			break;
 
@@ -209,7 +209,7 @@ static int _omap_device_deactivate(struct omap_device *od, u8 ignore_lat)
 		     "(%llu > %d)\n", od->pdev.name, od->pdev.id,
 		     od->pm_lat_level, deact_lat, odpl->deactivate_lat);
 
-		od->dev_wakeup_lat += odpl->activate_lat;
+		od->dev_wakeup_lat += odpl->deactivate_lat;
 
 		od->pm_lat_level++;
 	}
-- 
1.6.5.1

[PATCH 5/8] OMAP: omap_device: add usecounting

[Kevin Hilman]

Add use counters to omap_device to enable multiple potential users of
an omap_device.  This is needed if ever there are multiple users or
multiple instances of a driver with a single omap_device.

Without usecounting, with multiple users, the first one to call idle
may forcibly idle the device while other users are still active.

Signed-off-by: Kevin Hilman <khilman@deeprootsystems.com>
---
 arch/arm/plat-omap/include/plat/omap_device.h |    1 +
 arch/arm/plat-omap/omap_device.c              |    9 +++++++++
 2 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/arch/arm/plat-omap/include/plat/omap_device.h b/arch/arm/plat-omap/include/plat/omap_device.h
index d939246..c504780 100644
--- a/arch/arm/plat-omap/include/plat/omap_device.h
+++ b/arch/arm/plat-omap/include/plat/omap_device.h
@@ -71,6 +71,7 @@ struct omap_device {
 	s8				pm_lat_level;
 	u8				hwmods_cnt;
 	u8				_state;
+	u32                             _usecount;
 };
 
 /* Device driver interface (call via platform_data fn ptrs) */
diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c
index da649f2..6a8b0ce 100644
--- a/arch/arm/plat-omap/omap_device.c
+++ b/arch/arm/plat-omap/omap_device.c
@@ -388,6 +388,8 @@ struct omap_device *omap_device_build_ss(const char *pdev_name, int pdev_id,
 	od->pm_lats = pm_lats;
 	od->pm_lats_cnt = pm_lats_cnt;
 
+	od->_usecount = 0;
+
 	ret = omap_device_register(od);
 	if (ret)
 		goto odbs_exit4;
@@ -446,6 +448,9 @@ int omap_device_enable(struct platform_device *pdev)
 
 	od = _find_by_pdev(pdev);
 
+	if (od->_usecount++)
+		return 0;
+
 	if (od->_state == OMAP_DEVICE_STATE_ENABLED) {
 		WARN(1, "omap_device: %s.%d: omap_device_enable() called from "
 		     "invalid state\n", od->pdev.name, od->pdev.id);
@@ -485,6 +490,9 @@ int omap_device_idle(struct platform_device *pdev)
 
 	od = _find_by_pdev(pdev);
 
+	if (--od->_usecount)
+		return 0;
+
 	if (od->_state != OMAP_DEVICE_STATE_ENABLED) {
 		WARN(1, "omap_device: %s.%d: omap_device_idle() called from "
 		     "invalid state\n", od->pdev.name, od->pdev.id);
@@ -530,6 +538,7 @@ int omap_device_shutdown(struct platform_device *pdev)
 		omap_hwmod_shutdown(oh);
 
 	od->_state = OMAP_DEVICE_STATE_SHUTDOWN;
+	od->_usecount = 0;
 
 	return ret;
 }
-- 
1.6.5.1

[PATCH 6/8] OMAP: omap_device: use read_persistent_clock() instead of getnstimeofday()

[Kevin Hilman]

During suspend and resume, when omap_device deactivation and
activation is happening, the timekeeping subsystem has likely already
been suspended.  Thus getnstimeofday() will fail and trigger a WARN().

Use read_persistent_clock() instead of getnstimeofday() to avoid this.

Signed-off-by: Kevin Hilman <khilman@deeprootsystems.com>
---
 arch/arm/plat-omap/omap_device.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c
index 6a8b0ce..f6cdf1a 100644
--- a/arch/arm/plat-omap/omap_device.c
+++ b/arch/arm/plat-omap/omap_device.c
@@ -134,12 +134,12 @@ static int _omap_device_activate(struct omap_device *od, u8 ignore_lat)
 		    (od->dev_wakeup_lat <= od->_dev_wakeup_lat_limit))
 			break;
 
-		getnstimeofday(&a);
+		read_persistent_clock(&a);
 
 		/* XXX check return code */
 		odpl->activate_func(od);
 
-		getnstimeofday(&b);
+		read_persistent_clock(&b);
 
 		c = timespec_sub(b, a);
 		act_lat = timespec_to_ns(&c) * NSEC_PER_USEC;
@@ -190,12 +190,12 @@ static int _omap_device_deactivate(struct omap_device *od, u8 ignore_lat)
 		     od->_dev_wakeup_lat_limit))
 			break;
 
-		getnstimeofday(&a);
+		read_persistent_clock(&a);
 
 		/* XXX check return code */
 		odpl->deactivate_func(od);
 
-		getnstimeofday(&b);
+		read_persistent_clock(&b);
 
 		c = timespec_sub(b, a);
 		deact_lat = timespec_to_ns(&c) * NSEC_PER_USEC;
-- 
1.6.5.1

[PATCH 7/8] OMAP: omap_device: warn about expected latencies only if non-zero

[Kevin Hilman]

When checking measured [de]activate latencies against expected
latencies, only print warning if driver has provided a non-zero
latency.

This allows drivers to set their [de]activate latencies to zero when
they are not known, or are don't care, and the omap_device core will
not complain about unexpected latencies.

RFC: Maybe the measuring of latency should also avoided all together
     in this case?

Signed-off-by: Kevin Hilman <khilman@deeprootsystems.com>
---
 arch/arm/plat-omap/omap_device.c |   20 ++++++++++++--------
 1 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c
index f6cdf1a..83bee1c 100644
--- a/arch/arm/plat-omap/omap_device.c
+++ b/arch/arm/plat-omap/omap_device.c
@@ -148,10 +148,12 @@ static int _omap_device_activate(struct omap_device *od, u8 ignore_lat)
 			 "%llu usec\n", od->pdev.name, od->pm_lat_level,
 			 act_lat);
 
-		WARN(act_lat > odpl->activate_lat, "omap_device: %s.%d: "
-		     "activate step %d took longer than expected (%llu > %d)\n",
-		     od->pdev.name, od->pdev.id, od->pm_lat_level,
-		     act_lat, odpl->activate_lat);
+		if (odpl->activate_lat)
+			WARN(act_lat > odpl->activate_lat,
+			     "omap_device: %s.%d: activate step %d "
+			     "took longer than expected (%llu > %d)\n",
+			     od->pdev.name, od->pdev.id, od->pm_lat_level,
+			     act_lat, odpl->activate_lat);
 
 		od->dev_wakeup_lat -= odpl->activate_lat;
 	}
@@ -204,10 +206,12 @@ static int _omap_device_deactivate(struct omap_device *od, u8 ignore_lat)
 			 "%llu usec\n", od->pdev.name, od->pm_lat_level,
 			 deact_lat);
 
-		WARN(deact_lat > odpl->deactivate_lat, "omap_device: %s.%d: "
-		     "deactivate step %d took longer than expected "
-		     "(%llu > %d)\n", od->pdev.name, od->pdev.id,
-		     od->pm_lat_level, deact_lat, odpl->deactivate_lat);
+		if (odpl->deactivate_lat)
+			WARN(deact_lat > odpl->deactivate_lat,
+			     "omap_device: %s.%d: "
+			     "deactivate step %d took longer than expected "
+			     "(%llu > %d)\n", od->pdev.name, od->pdev.id,
+			     od->pm_lat_level, deact_lat, odpl->deactivate_lat);
 
 		od->dev_wakeup_lat += odpl->deactivate_lat;
 
-- 
1.6.5.1

[PATCH 8/8] OMAP: omap_device: use UINT_MAX for default wakeup latency limit

[Kevin Hilman]

The _dev_wakeup_lat_limit field of struct omap_device is u32, so use
UINT_MAX instead of INT_MAX for the default maximum.

Signed-off-by: Kevin Hilman <khilman@deeprootsystems.com>
---
 arch/arm/plat-omap/omap_device.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c
index 83bee1c..9ec3735 100644
--- a/arch/arm/plat-omap/omap_device.c
+++ b/arch/arm/plat-omap/omap_device.c
@@ -468,7 +468,7 @@ int omap_device_enable(struct platform_device *pdev)
 	ret = _omap_device_activate(od, IGNORE_WAKEUP_LAT);
 
 	od->dev_wakeup_lat = 0;
-	od->_dev_wakeup_lat_limit = INT_MAX;
+	od->_dev_wakeup_lat_limit = UINT_MAX;
 	od->_state = OMAP_DEVICE_STATE_ENABLED;
 
 	return ret;
-- 
1.6.5.1

Re: [PATCH 3/8] OMAP3 hwmod: Add automatic OCP_SYSCONFIG AUTOIDLE handling

[Vimal Singh]

On Wed, Nov 18, 2009 at 6:35 AM, Kevin Hilman
<khilman@deeprootsystems.com> wrote:
> From: Paul Walmsley <paul@pwsan.com>
>
> This patch fills in the OCP_SYSCONFIG.AUTOIDLE handling in the OMAP
> hwmod code.
>
> After this patch, the hwmod code will set the module AUTOIDLE bit (generally
> <module>.OCP_SYSCONFIG.AUTOIDLE) to 1 by default upon enable, and 1 upon
> disable.

You wanted to say:  "0 upon disable", isn't it?

-vimal

> If the hwmod flag HWMOD_NO_AUTOIDLE is set, AUTOIDLE will be
> set to 0 upon enable.
>
> Enabling module autoidle should save some power.  The only reason to
> not set the OCP_SYSCONFIG.AUTOIDLE bit is if there is a bug in the
> module RTL, e.g., the MPUINTC block on OMAP3.
>
> Comments from Kevin Hilman <khilman@deeprootsystems.com> inspired this patch.
>
> Signed-off-by: Paul Walmsley <paul@pwsan.com>
> Tested-by: Kevin Hilman <khilman@deeprootsystems.com>
> ---
>  arch/arm/mach-omap2/omap_hwmod.c             |   50 +++++++++++++++++++++++---
>  arch/arm/plat-omap/include/plat/omap_hwmod.h |    8 ++++-
>  2 files changed, 52 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
> index 7d7b3b8..25d9484 100644
> --- a/arch/arm/mach-omap2/omap_hwmod.c
> +++ b/arch/arm/mach-omap2/omap_hwmod.c
> @@ -210,6 +210,29 @@ static int _set_softreset(struct omap_hwmod *oh, u32 *v)
>  }
>
>  /**
> + * _set_module_autoidle: set the OCP_SYSCONFIG AUTOIDLE field in @v
> + * @oh: struct omap_hwmod *
> + * @autoidle: desired AUTOIDLE bitfield value (0 or 1)
> + * @v: pointer to register contents to modify
> + *
> + * Update the module autoidle mode bit in @v to be @autoidle for the
> + * @oh hwmod.  Does not write to the hardware.  Returns -EINVAL upon
> + * error or 0 upon success.
> + */
> +static int _set_module_autoidle(struct omap_hwmod *oh, u8 autoidle,
> +                               u32 *v)
> +{
> +       if (!oh->sysconfig ||
> +           !(oh->sysconfig->sysc_flags & SYSC_HAS_AUTOIDLE))
> +               return -EINVAL;
> +
> +       *v &= ~SYSC_AUTOIDLE_MASK;
> +       *v |= autoidle << SYSC_AUTOIDLE_SHIFT;
> +
> +       return 0;
> +}
> +
> +/**
>  * _enable_wakeup: set OCP_SYSCONFIG.ENAWAKEUP bit in the hardware
>  * @oh: struct omap_hwmod *
>  *
> @@ -560,7 +583,13 @@ static void _sysc_enable(struct omap_hwmod *oh)
>                _set_master_standbymode(oh, idlemode, &v);
>        }
>
> -       /* XXX OCP AUTOIDLE bit? */
> +       if (oh->sysconfig->sysc_flags & SYSC_HAS_AUTOIDLE) {
> +               idlemode = (oh->flags & HWMOD_NO_AUTOIDLE) ?
> +                       0 : 1;
> +               _set_module_autoidle(oh, idlemode, &v);
> +       }
> +
> +       /* XXX OCP ENAWAKEUP bit? */
>
>        if (oh->flags & HWMOD_SET_DEFAULT_CLOCKACT &&
>            oh->sysconfig->sysc_flags & SYSC_HAS_CLOCKACTIVITY)
> @@ -625,7 +654,8 @@ static void _sysc_shutdown(struct omap_hwmod *oh)
>        if (oh->sysconfig->sysc_flags & SYSC_HAS_MIDLEMODE)
>                _set_master_standbymode(oh, HWMOD_IDLEMODE_FORCE, &v);
>
> -       /* XXX clear OCP AUTOIDLE bit? */
> +       if (oh->sysconfig->sysc_flags & SYSC_HAS_AUTOIDLE)
> +               _set_module_autoidle(oh, 1, &v);
>
>        _write_sysconfig(v, oh);
>  }
> @@ -951,11 +981,21 @@ static int _setup(struct omap_hwmod *oh)
>
>        _enable(oh);
>
> -       if (!(oh->flags & HWMOD_INIT_NO_RESET))
> +       if (!(oh->flags & HWMOD_INIT_NO_RESET)) {
>                _reset(oh);
>
> -       /* XXX OCP AUTOIDLE bit? */
> -       /* XXX OCP ENAWAKEUP bit? */
> +               /*
> +                * XXX Do the OCP_SYSCONFIG bits need to be
> +                * reprogrammed after a reset?  If not, then this can
> +                * be removed.  If it does, then probably the
> +                * _enable() function should be split to avoid the
> +                * rewrite of the OCP_SYSCONFIG register.
> +                */
> +               if (oh->sysconfig) {
> +                       _update_sysc_cache(oh);
> +                       _sysc_enable(oh);
> +               }
> +       }
>
>        if (!(oh->flags & HWMOD_INIT_NO_IDLE))
>                _idle(oh);
> diff --git a/arch/arm/plat-omap/include/plat/omap_hwmod.h b/arch/arm/plat-omap/include/plat/omap_hwmod.h
> index dbdd123..ec140b0 100644
> --- a/arch/arm/plat-omap/include/plat/omap_hwmod.h
> +++ b/arch/arm/plat-omap/include/plat/omap_hwmod.h
> @@ -50,6 +50,8 @@ struct omap_device;
>  #define SYSC_ENAWAKEUP_MASK            (1 << SYSC_ENAWAKEUP_SHIFT)
>  #define SYSC_SOFTRESET_SHIFT           1
>  #define SYSC_SOFTRESET_MASK            (1 << SYSC_SOFTRESET_SHIFT)
> +#define SYSC_AUTOIDLE_SHIFT            0
> +#define SYSC_AUTOIDLE_MASK             (1 << SYSC_AUTOIDLE_SHIFT)
>
>  /* OCP SYSSTATUS bit shifts/masks */
>  #define SYSS_RESETDONE_SHIFT           0
> @@ -294,13 +296,17 @@ struct omap_hwmod_omap4_prcm {
>  *     SDRAM controller, etc.
>  * HWMOD_INIT_NO_IDLE: don't idle this module at boot - important for SDRAM
>  *     controller, etc.
> + * HWMOD_NO_AUTOIDLE: disable module autoidle (OCP_SYSCONFIG.AUTOIDLE)
> + *     when module is enabled, rather than the default, which is to
> + *     enable autoidle
>  * HWMOD_SET_DEFAULT_CLOCKACT: program CLOCKACTIVITY bits at startup
>  */
>  #define HWMOD_SWSUP_SIDLE                      (1 << 0)
>  #define HWMOD_SWSUP_MSTANDBY                   (1 << 1)
>  #define HWMOD_INIT_NO_RESET                    (1 << 2)
>  #define HWMOD_INIT_NO_IDLE                     (1 << 3)
> -#define HWMOD_SET_DEFAULT_CLOCKACT             (1 << 4)
> +#define HWMOD_NO_AUTOIDLE                      (1 << 4)
> +#define HWMOD_SET_DEFAULT_CLOCKACT             (1 << 5)
>
>  /*
>  * omap_hwmod._int_flags definitions
> --
> 1.6.5.1
>
> --
> 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
>
--
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/8] OMAP3 hwmod: Add automatic OCP_SYSCONFIG AUTOIDLE handling

[Paul Walmsley]

On Wed, 18 Nov 2009, Vimal Singh wrote:

> On Wed, Nov 18, 2009 at 6:35 AM, Kevin Hilman
> <khilman@deeprootsystems.com> wrote:
> > From: Paul Walmsley <paul@pwsan.com>
> >
> > This patch fills in the OCP_SYSCONFIG.AUTOIDLE handling in the OMAP
> > hwmod code.
> >
> > After this patch, the hwmod code will set the module AUTOIDLE bit (generally
> > <module>.OCP_SYSCONFIG.AUTOIDLE) to 1 by default upon enable, and 1 upon
> > disable.
> 
> You wanted to say:  "0 upon disable", isn't it?

No, the original wording was intended, although it is somewhat confusing.

This patch wasn't intended to be posted publicly; it combines two 
unrelated changes that should be split.


- Paul

Re: [PATCH 5/8] OMAP: omap_device: add usecounting

[Paul Walmsley]

Hello Kevin,

On Tue, 17 Nov 2009, Kevin Hilman wrote:

> Add use counters to omap_device to enable multiple potential users of
> an omap_device.  This is needed if ever there are multiple users or
> multiple instances of a driver with a single omap_device.
> 
> Without usecounting, with multiple users, the first one to call idle
> may forcibly idle the device while other users are still active.

Could you please send along an example of the use case for this?  I would 
prefer not to add usecounting at this layer unless it is absolutely 
necessary, since only one driver should be bound to a device at a time.


- Paul

Re: [PATCH 4/8] OMAP: omap_device: deactivate latency typo

[Paul Walmsley]

Hello Kevin,

On Tue, 17 Nov 2009, Kevin Hilman wrote:

> The deactivate hook should use 'deactivate_lat' instead of
> 'activate_lat' when doing checking on expected latency values.

Why ?

- Paul

> 
> Signed-off-by: Kevin Hilman <khilman@deeprootsystems.com>
> ---
>  arch/arm/plat-omap/omap_device.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c
> index bb16e62..da649f2 100644
> --- a/arch/arm/plat-omap/omap_device.c
> +++ b/arch/arm/plat-omap/omap_device.c
> @@ -186,7 +186,7 @@ static int _omap_device_deactivate(struct omap_device *od, u8 ignore_lat)
>  		odpl = od->pm_lats + od->pm_lat_level;
>  
>  		if (!ignore_lat &&
> -		    ((od->dev_wakeup_lat + odpl->activate_lat) >
> +		    ((od->dev_wakeup_lat + odpl->deactivate_lat) >
>  		     od->_dev_wakeup_lat_limit))
>  			break;
>  
> @@ -209,7 +209,7 @@ static int _omap_device_deactivate(struct omap_device *od, u8 ignore_lat)
>  		     "(%llu > %d)\n", od->pdev.name, od->pdev.id,
>  		     od->pm_lat_level, deact_lat, odpl->deactivate_lat);
>  
> -		od->dev_wakeup_lat += odpl->activate_lat;
> +		od->dev_wakeup_lat += odpl->deactivate_lat;
>  
>  		od->pm_lat_level++;
>  	}
> -- 
> 1.6.5.1
> 
> --
> 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/8] OMAP3 hwmod: Add automatic OCP_SYSCONFIG AUTOIDLE handling

[Paul Walmsley]

On Tue, 17 Nov 2009, Kevin Hilman wrote:

> From: Paul Walmsley <paul@pwsan.com>
> 
> This patch fills in the OCP_SYSCONFIG.AUTOIDLE handling in the OMAP
> hwmod code.

This patch actually combines two unrelated changes.  I've got those split 
into two patches here, will post them.

- Paul

Re: [PATCH 8/8] OMAP: omap_device: use UINT_MAX for default wakeup latency limit

[Paul Walmsley]

Hi Kevin,

On Tue, 17 Nov 2009, Kevin Hilman wrote:

> The _dev_wakeup_lat_limit field of struct omap_device is u32, so use
> UINT_MAX instead of INT_MAX for the default maximum.
> 
> Signed-off-by: Kevin Hilman <khilman@deeprootsystems.com>

Indeed, thanks.  Will add this to the second version of the fixes series.


- Paul

> ---
>  arch/arm/plat-omap/omap_device.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c
> index 83bee1c..9ec3735 100644
> --- a/arch/arm/plat-omap/omap_device.c
> +++ b/arch/arm/plat-omap/omap_device.c
> @@ -468,7 +468,7 @@ int omap_device_enable(struct platform_device *pdev)
>  	ret = _omap_device_activate(od, IGNORE_WAKEUP_LAT);
>  
>  	od->dev_wakeup_lat = 0;
> -	od->_dev_wakeup_lat_limit = INT_MAX;
> +	od->_dev_wakeup_lat_limit = UINT_MAX;
>  	od->_state = OMAP_DEVICE_STATE_ENABLED;
>  
>  	return ret;
> -- 
> 1.6.5.1
> 
> --
> 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
> 


- Paul

Re: [PATCH 1/8] OMAP3: clock: add clockdomains for UART1 & 2

[Paul Walmsley]

Hi Kevin,

On Tue, 17 Nov 2009, Kevin Hilman wrote:

> UART1 & 2 were missing clockdomains resulting in broken omap_hwmod
> init for these devices.

also looks good, will add this to the fixes series.

- Paul

> 
> Signed-off-by: Kevin Hilman <khilman@deeprootsystems.com>
> ---
>  arch/arm/mach-omap2/clock34xx.h |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/clock34xx.h b/arch/arm/mach-omap2/clock34xx.h
> index a1b3de7..cbc3d8a 100644
> --- a/arch/arm/mach-omap2/clock34xx.h
> +++ b/arch/arm/mach-omap2/clock34xx.h
> @@ -1507,6 +1507,7 @@ static struct clk uart2_fck = {
>  	.parent		= &core_48m_fck,
>  	.enable_reg	= OMAP_CM_REGADDR(CORE_MOD, CM_FCLKEN1),
>  	.enable_bit	= OMAP3430_EN_UART2_SHIFT,
> +	.clkdm_name	= "core_l4_clkdm",
>  	.recalc		= &followparent_recalc,
>  };
>  
> @@ -1516,6 +1517,7 @@ static struct clk uart1_fck = {
>  	.parent		= &core_48m_fck,
>  	.enable_reg	= OMAP_CM_REGADDR(CORE_MOD, CM_FCLKEN1),
>  	.enable_bit	= OMAP3430_EN_UART1_SHIFT,
> +	.clkdm_name	= "core_l4_clkdm",
>  	.recalc		= &followparent_recalc,
>  };
>  
> -- 
> 1.6.5.1
> 
> --
> 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 6/8] OMAP: omap_device: use read_persistent_clock() instead of getnstimeofday()

[Paul Walmsley]

Hi, 

On Tue, 17 Nov 2009, Kevin Hilman wrote:

> During suspend and resume, when omap_device deactivation and
> activation is happening, the timekeeping subsystem has likely already
> been suspended.  Thus getnstimeofday() will fail and trigger a WARN().
> 
> Use read_persistent_clock() instead of getnstimeofday() to avoid this.

Thanks, will queue in fixes series.

- Paul

> 
> Signed-off-by: Kevin Hilman <khilman@deeprootsystems.com>
> ---
>  arch/arm/plat-omap/omap_device.c |    8 ++++----
>  1 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c
> index 6a8b0ce..f6cdf1a 100644
> --- a/arch/arm/plat-omap/omap_device.c
> +++ b/arch/arm/plat-omap/omap_device.c
> @@ -134,12 +134,12 @@ static int _omap_device_activate(struct omap_device *od, u8 ignore_lat)
>  		    (od->dev_wakeup_lat <= od->_dev_wakeup_lat_limit))
>  			break;
>  
> -		getnstimeofday(&a);
> +		read_persistent_clock(&a);
>  
>  		/* XXX check return code */
>  		odpl->activate_func(od);
>  
> -		getnstimeofday(&b);
> +		read_persistent_clock(&b);
>  
>  		c = timespec_sub(b, a);
>  		act_lat = timespec_to_ns(&c) * NSEC_PER_USEC;
> @@ -190,12 +190,12 @@ static int _omap_device_deactivate(struct omap_device *od, u8 ignore_lat)
>  		     od->_dev_wakeup_lat_limit))
>  			break;
>  
> -		getnstimeofday(&a);
> +		read_persistent_clock(&a);
>  
>  		/* XXX check return code */
>  		odpl->deactivate_func(od);
>  
> -		getnstimeofday(&b);
> +		read_persistent_clock(&b);
>  
>  		c = timespec_sub(b, a);
>  		deact_lat = timespec_to_ns(&c) * NSEC_PER_USEC;
> -- 
> 1.6.5.1
> 
> --
> 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
> 


- Paul

Re: [PATCH 2/8] OMAP: hwmod: warn on missing clockdomain

[Paul Walmsley]

Hi Kevin

On Tue, 17 Nov 2009, Kevin Hilman wrote:

> WARN if a clock/hwmod is missing a clockdomain association since
> resulting hwmod will not be able to correctly enable/disable clocks.

Wouldn't this check be best placed in the clock code?  

> 
> Signed-off-by: Kevin Hilman <khilman@deeprootsystems.com>
> ---
>  arch/arm/mach-omap2/omap_hwmod.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
> index 633b216..7d7b3b8 100644
> --- a/arch/arm/mach-omap2/omap_hwmod.c
> +++ b/arch/arm/mach-omap2/omap_hwmod.c
> @@ -326,6 +326,9 @@ static int _init_main_clk(struct omap_hwmod *oh)
>  		ret = -EINVAL;
>  	oh->_clk = c;
>  
> +	WARN(!c->clkdm, "omap_hwmod: %s: missing clockdomain for %s.\n",
> +	     oh->clkdev_con_id, c->name);
> +
>  	return ret;
>  }
>  
> -- 
> 1.6.5.1
> 
> --
> 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
> 


- Paul

Re: [PATCH 7/8] OMAP: omap_device: warn about expected latencies only if non-zero

[Paul Walmsley]

Hi,

On Tue, 17 Nov 2009, Kevin Hilman wrote:

> When checking measured [de]activate latencies against expected
> latencies, only print warning if driver has provided a non-zero
> latency.
> 
> This allows drivers to set their [de]activate latencies to zero when
> they are not known, or are don't care, and the omap_device core will
> not complain about unexpected latencies.

I'm concerned that this will effectively mean that no driver maintainers 
will bother to measure these latencies.   These are necessary for 
a reasonable implementation of omap_pm_set_max_dev_wakeup_lat().
They should not be difficult to measure: boot at the lowest-performance 
OPPs with the latencies uninitialized; then these warnings should indicate 
numbers that can be plugged back into the omap_device structure.  But 
perhaps I am misunderstanding the point you are making?


- Paul

Re: [PATCH 5/8] OMAP: omap_device: add usecounting

[Kevin Hilman]

Paul Walmsley <paul@pwsan.com> writes:

> Hello Kevin,
>
> On Tue, 17 Nov 2009, Kevin Hilman wrote:
>
>> Add use counters to omap_device to enable multiple potential users of
>> an omap_device.  This is needed if ever there are multiple users or
>> multiple instances of a driver with a single omap_device.
>> 
>> Without usecounting, with multiple users, the first one to call idle
>> may forcibly idle the device while other users are still active.
>
> Could you please send along an example of the use case for this?  

The current use case is the serial driver (not yet ready for posting.)

While there is only a single driver bound to the omap_device, there
are effectivily multiple instances of the driver.  One for initial
init/probe requriring and early _enable, followed by _idle in the
late_init hook.  Another when the actual serial driver gets started
and the inactivity timers etc. start firing.  These usages overlap
slightly and the easiest way I saw to handle this was with
usecounting.

> I would prefer not to add usecounting at this layer unless it is
> absolutely necessary, since only one driver should be bound to a
> device at a time.

What about mulitple instances of the same driver?

Or, what about drivers like i2c which might do _enable/_idle on a
per-transfer basis and there might be multiple transfers pending at
any given time.

We already have use-counting in the clock framework for this same
purpose.  I'm essentially proposing the usecounting for the same reason,
but it also need to protect enable/idle parts of hwmod as well.

Maybe usecounting at the hwmod level is more appropriate since that's
where the problem lies.  I'm fine with that.

Kevin

Re: [PATCH 4/8] OMAP: omap_device: deactivate latency typo

[Kevin Hilman]

Paul Walmsley <paul@pwsan.com> writes:

> Hello Kevin,
>
> On Tue, 17 Nov 2009, Kevin Hilman wrote:
>
>> The deactivate hook should use 'deactivate_lat' instead of
>> 'activate_lat' when doing checking on expected latency values.
>
> Why ?
>

Hmm, looking closer, I think misunderstood the use of 'activate_lat' in the
deactivate hook.

I'll drop this one.

Kevin

Re: [PATCH 2/8] OMAP: hwmod: warn on missing clockdomain

[Kevin Hilman]

Paul Walmsley <paul@pwsan.com> writes:

> Hi Kevin
>
> On Tue, 17 Nov 2009, Kevin Hilman wrote:
>
>> WARN if a clock/hwmod is missing a clockdomain association since
>> resulting hwmod will not be able to correctly enable/disable clocks.
>
> Wouldn't this check be best placed in the clock code?  

Possibly.  The idea here was to flag an issue that will prevent the
hwmod code from crashing.

Doing something like below in the clock code may be the right
place, but it creates lots of noise that didn't help debug the hwmod
problem.

clock omap_32k_fck is missing clockdomain
clock virt_12m_ck is missing clockdomain
clock virt_13m_ck is missing clockdomain
clock virt_16_8m_ck is missing clockdomain
clock virt_19_2m_ck is missing clockdomain
clock virt_26m_ck is missing clockdomain
clock virt_38_4m_ck is missing clockdomain
clock osc_sys_ck is missing clockdomain
clock sys_ck is missing clockdomain
clock sys_altclk is missing clockdomain
clock mcbsp_clks is missing clockdomain
clock sys_clkout1 is missing clockdomain
clock core_ck is missing clockdomain
clock omap_96m_alwon_fck is missing clockdomain
clock omap_96m_fck is missing clockdomain
clock cm_96m_fck is missing clockdomain
clock omap_54m_fck is missing clockdomain
clock omap_48m_fck is missing clockdomain
clock omap_12m_fck is missing clockdomain
clock sys_clkout2 is missing clockdomain
clock corex2_fck is missing clockdomain
clock dpll1_fck is missing clockdomain
clock emu_mpu_alwon_ck is missing clockdomain
clock dpll2_fck is missing clockdomain
clock rm_ick is missing clockdomain
clock cpefuse_fck is missing clockdomain
clock ts_fck is missing clockdomain
clock usbtll_fck is missing clockdomain
clock mcspi_fck is missing clockdomain
clock mcspi_fck is missing clockdomain
clock mcspi_fck is missing clockdomain
clock mcspi_fck is missing clockdomain
clock hdq_fck is missing clockdomain
clock ssi_sst_fck is missing clockdomain
clock security_l3_ick is missing clockdomain
clock pka_ick is missing clockdomain
clock omapctrl_ick is missing clockdomain
clock security_l4_ick2 is missing clockdomain
clock aes1_ick is missing clockdomain
clock rng_ick is missing clockdomain
clock sha11_ick is missing clockdomain
clock des1_ick is missing clockdomain
clock usim_fck is missing clockdomain
clock sr1_fck is missing clockdomain
clock sr2_fck is missing clockdomain
clock secure_32k_fck is missing clockdomain
clock gpt12_fck is missing clockdomain
clock wdt1_fck is missing clockdomain

diff --git a/arch/arm/mach-omap2/clock.c b/arch/arm/mach-omap2/clock.c
index 4716206..9022858 100644
--- a/arch/arm/mach-omap2/clock.c
+++ b/arch/arm/mach-omap2/clock.c
@@ -153,8 +153,10 @@ void omap2_init_clk_clkdm(struct clk *clk)
 {
 	struct clockdomain *clkdm;
 
-	if (!clk->clkdm_name)
+	if (!clk->clkdm_name) {
+		pr_debug("clock %s is missing clockdomain\n", clk->name);
 		return;
+	}
 
 	clkdm = clkdm_lookup(clk->clkdm_name);
 	if (clkdm) {
k

Re: [PATCH 3/8] OMAP3 hwmod: Add automatic OCP_SYSCONFIG AUTOIDLE handling

[Kevin Hilman]

Paul Walmsley <paul@pwsan.com> writes:

> On Wed, 18 Nov 2009, Vimal Singh wrote:
>
>> On Wed, Nov 18, 2009 at 6:35 AM, Kevin Hilman
>> <khilman@deeprootsystems.com> wrote:
>> > From: Paul Walmsley <paul@pwsan.com>
>> >
>> > This patch fills in the OCP_SYSCONFIG.AUTOIDLE handling in the OMAP
>> > hwmod code.
>> >
>> > After this patch, the hwmod code will set the module AUTOIDLE bit (generally
>> > <module>.OCP_SYSCONFIG.AUTOIDLE) to 1 by default upon enable, and 1 upon
>> > disable.
>> 
>> You wanted to say:  "0 upon disable", isn't it?
>
> No, the original wording was intended, although it is somewhat confusing.
>
> This patch wasn't intended to be posted publicly; it combines two 
> unrelated changes that should be split.

Paul, sorry for posting this one.  I didn't mean to include it in this
series.

Kevin

Re: [PATCH 3/8] OMAP3 hwmod: Add automatic OCP_SYSCONFIG AUTOIDLE handling

[Paul Walmsley]

On Wed, 18 Nov 2009, Kevin Hilman wrote:

> Paul, sorry for posting this one.  I didn't mean to include it in this
> series.

No worries, it was an effective motivation for me to repost the fixed 
split :-)

Also I will take Vimal's feedback and update the commit message for the 
second patch.

Anyway, the revised split are the first two patches posted in

http://www.mail-archive.com/linux-omap@vger.kernel.org/msg19497.html


- Paul

Re: [PATCH 7/8] OMAP: omap_device: warn about expected latencies only if non-zero

[Kevin Hilman]

Paul Walmsley <paul@pwsan.com> writes:

> Hi,
>
> On Tue, 17 Nov 2009, Kevin Hilman wrote:
>
>> When checking measured [de]activate latencies against expected
>> latencies, only print warning if driver has provided a non-zero
>> latency.
>> 
>> This allows drivers to set their [de]activate latencies to zero when
>> they are not known, or are don't care, and the omap_device core will
>> not complain about unexpected latencies.
>
> I'm concerned that this will effectively mean that no driver maintainers 
> will bother to measure these latencies.   
>
> These are necessary for a reasonable implementation of
> omap_pm_set_max_dev_wakeup_lat().  They should not be difficult to
> measure: boot at the lowest-performance OPPs with the latencies
> uninitialized; then these warnings should indicate numbers that can
> be plugged back into the omap_device structure.  But perhaps I am
> misunderstanding the point you are making?

Part of the issue is being able to do a quick conversion, without
needing to measure latencies (which I'm guilty of.)  

The other part was that I was thinking (the RFC part) was that it
might be useful for performance reasons to not do do any measuring at
all.  Of course, this would preclude a useful max_dev_wakeup_lat() but
we could warn about that.

But, for now I agree. I think getting these latencies in place is more
important than having an optimization for avoiding measurement for now.

Kevin

Re: [PATCH 5/8] OMAP: omap_device: add usecounting

[Kevin Hilman]

On Wed, Nov 18, 2009 at 2:51 AM, Paul Walmsley <paul@pwsan.com> wrote:
> Hello Kevin,
>
> On Tue, 17 Nov 2009, Kevin Hilman wrote:
>
>> Add use counters to omap_device to enable multiple potential users of
>> an omap_device.  This is needed if ever there are multiple users or
>> multiple instances of a driver with a single omap_device.
>>
>> Without usecounting, with multiple users, the first one to call idle
>> may forcibly idle the device while other users are still active.
>
> Could you please send along an example of the use case for this?  I would
> prefer not to add usecounting at this layer unless it is absolutely
> necessary, since only one driver should be bound to a device at a time.

OK, after some more wrangling with the UART layer, I don't think I
will need this patch.

If an omap_device is bound to a single driver, the driver itself can
do the usecounting and will be more effecient when doing so.

Note however that this will prevent any direct replacement of
clk_enable/disable with omap_device_enable/disable since some drivers
are built on the assumption that the clk API does usecounting.

Kevin
--
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 2/8] OMAP: hwmod: warn on missing clockdomain

[Paul Walmsley]

On Wed, 18 Nov 2009, Kevin Hilman wrote:

> Paul Walmsley <paul@pwsan.com> writes:
> 
> > Hi Kevin
> >
> > On Tue, 17 Nov 2009, Kevin Hilman wrote:
> >
> >> WARN if a clock/hwmod is missing a clockdomain association since
> >> resulting hwmod will not be able to correctly enable/disable clocks.
> >
> > Wouldn't this check be best placed in the clock code?  
> 
> Possibly.  The idea here was to flag an issue that will prevent the
> hwmod code from crashing.

You've convinced me :-)  We should wait to move it into the clock code 
until all clocks are associated with clockdomains.  Until then, let's take 
your patch.


- Paul

Re: [PATCH 1/8] OMAP3: clock: add clockdomains for UART1 & 2

[Paul Walmsley]

Hi Kevin,

On Wed, 18 Nov 2009, Paul Walmsley wrote:

> On Tue, 17 Nov 2009, Kevin Hilman wrote:
> 
> > UART1 & 2 were missing clockdomains resulting in broken omap_hwmod
> > init for these devices.
> 
> also looks good, will add this to the fixes series.

My apologies, looks like I missed this one.  Will requeue for .33-rc2.


- Paul