[PATCH 0/3] Some omap_device/hwmod/pwrdomain patches

[PATCH 0/3] Some omap_device/hwmod/pwrdomain patches

[Tomi Valkeinen]

I came up with these patches while implementing pm runtime adaptation for DSS
driver. I'm not quite sure on who's turf they belong, or do they even belong
together, but here they are anyway.

get_context_loss_count patch was discussed on l-o with Kevin.

The omap_device_reset patch I made as some parts of DSS currently presume that
the HW module is reset when it is enabled, and the reset is better handled in
hwmod code.

can_ever_lose_context code I didn't strictly need, but as there's such a
function I added that to the context save code in DSS while rewriting the code.

 Tomi

Tomi Valkeinen (3):
  OMAP: change get_context_loss_count ret value to int
  OMAP: add omap_device_reset()
  OMAP: Add (omap_device|omap_hwmod)_can_ever_lose_context functions

 arch/arm/mach-omap2/omap_hwmod.c              |   24 ++++++++++++-
 arch/arm/mach-omap2/powerdomain.c             |   14 +++++--
 arch/arm/mach-omap2/powerdomain.h             |    2 +-
 arch/arm/plat-omap/include/plat/omap-pm.h     |    4 +-
 arch/arm/plat-omap/include/plat/omap_device.h |    4 ++-
 arch/arm/plat-omap/include/plat/omap_hwmod.h  |    3 +-
 arch/arm/plat-omap/omap-pm-noop.c             |   24 +++++++++----
 arch/arm/plat-omap/omap_device.c              |   48 ++++++++++++++++++++++++-
 8 files changed, 105 insertions(+), 18 deletions(-)

-- 
1.7.4.1

[PATCH 1/3] OMAP: change get_context_loss_count ret value to int

[Tomi Valkeinen]

get_context_loss_count functions return context loss count as u32, and
zero means an error. However, zero is also returned when context has
never been lost and could also be returned when the context loss count
has wrapped and goes to zero.

Change the functions to return an int, with negative value meaning an
error.

OMAP HSMMC code uses omap_pm_get_dev_context_loss_count(), but as the
hsmmc code handles the returned value as an int, with negative value
meaning an error, this patch actually fixes hsmmc code also.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod.c              |    2 +-
 arch/arm/mach-omap2/powerdomain.c             |   14 ++++++++++----
 arch/arm/mach-omap2/powerdomain.h             |    2 +-
 arch/arm/plat-omap/include/plat/omap-pm.h     |    4 ++--
 arch/arm/plat-omap/include/plat/omap_device.h |    2 +-
 arch/arm/plat-omap/include/plat/omap_hwmod.h  |    2 +-
 arch/arm/plat-omap/omap-pm-noop.c             |   24 +++++++++++++++++-------
 arch/arm/plat-omap/omap_device.c              |    2 +-
 8 files changed, 34 insertions(+), 18 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index e034294..4f0d554 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -2332,7 +2332,7 @@ ohsps_unlock:
  * Returns the context loss count of the powerdomain assocated with @oh
  * upon success, or zero if no powerdomain exists for @oh.
  */
-u32 omap_hwmod_get_context_loss_count(struct omap_hwmod *oh)
+int omap_hwmod_get_context_loss_count(struct omap_hwmod *oh)
 {
 	struct powerdomain *pwrdm;
 	int ret = 0;
diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c
index 9af0847..9281481 100644
--- a/arch/arm/mach-omap2/powerdomain.c
+++ b/arch/arm/mach-omap2/powerdomain.c
@@ -935,16 +935,16 @@ int pwrdm_post_transition(void)
  * @pwrdm: struct powerdomain * to wait for
  *
  * Context loss count is the sum of powerdomain off-mode counter, the
- * logic off counter and the per-bank memory off counter.  Returns 0
+ * logic off counter and the per-bank memory off counter.  Returns negative
  * (and WARNs) upon error, otherwise, returns the context loss count.
  */
-u32 pwrdm_get_context_loss_count(struct powerdomain *pwrdm)
+int pwrdm_get_context_loss_count(struct powerdomain *pwrdm)
 {
 	int i, count;
 
 	if (!pwrdm) {
 		WARN(1, "powerdomain: %s: pwrdm is null\n", __func__);
-		return 0;
+		return -ENODEV;
 	}
 
 	count = pwrdm->state_counter[PWRDM_POWER_OFF];
@@ -953,7 +953,13 @@ u32 pwrdm_get_context_loss_count(struct powerdomain *pwrdm)
 	for (i = 0; i < pwrdm->banks; i++)
 		count += pwrdm->ret_mem_off_counter[i];
 
-	pr_debug("powerdomain: %s: context loss count = %u\n",
+	/*
+	 * Context loss count has to be a non-negative value. Clear the sign
+	 * bit to get a value range from 0 to INT_MAX.
+	 */
+	count &= ~(1 << 31);
+
+	pr_debug("powerdomain: %s: context loss count = %d\n",
 		 pwrdm->name, count);
 
 	return count;
diff --git a/arch/arm/mach-omap2/powerdomain.h b/arch/arm/mach-omap2/powerdomain.h
index d23d979..012827f 100644
--- a/arch/arm/mach-omap2/powerdomain.h
+++ b/arch/arm/mach-omap2/powerdomain.h
@@ -207,7 +207,7 @@ int pwrdm_clkdm_state_switch(struct clockdomain *clkdm);
 int pwrdm_pre_transition(void);
 int pwrdm_post_transition(void);
 int pwrdm_set_lowpwrstchange(struct powerdomain *pwrdm);
-u32 pwrdm_get_context_loss_count(struct powerdomain *pwrdm);
+int pwrdm_get_context_loss_count(struct powerdomain *pwrdm);
 bool pwrdm_can_ever_lose_context(struct powerdomain *pwrdm);
 
 extern void omap2xxx_powerdomains_init(void);
diff --git a/arch/arm/plat-omap/include/plat/omap-pm.h b/arch/arm/plat-omap/include/plat/omap-pm.h
index c0a7520..68df031 100644
--- a/arch/arm/plat-omap/include/plat/omap-pm.h
+++ b/arch/arm/plat-omap/include/plat/omap-pm.h
@@ -350,9 +350,9 @@ unsigned long omap_pm_cpu_get_freq(void);
  * driver must restore device context.   If the number of context losses
  * exceeds the maximum positive integer, the function will wrap to 0 and
  * continue counting.  Returns the number of context losses for this device,
- * or zero upon error.
+ * or negative value upon error.
  */
-u32 omap_pm_get_dev_context_loss_count(struct device *dev);
+int omap_pm_get_dev_context_loss_count(struct device *dev);
 
 void omap_pm_enable_off_mode(void);
 void omap_pm_disable_off_mode(void);
diff --git a/arch/arm/plat-omap/include/plat/omap_device.h b/arch/arm/plat-omap/include/plat/omap_device.h
index e4c349f..70d31d0 100644
--- a/arch/arm/plat-omap/include/plat/omap_device.h
+++ b/arch/arm/plat-omap/include/plat/omap_device.h
@@ -107,7 +107,7 @@ void __iomem *omap_device_get_rt_va(struct omap_device *od);
 int omap_device_align_pm_lat(struct platform_device *pdev,
 			     u32 new_wakeup_lat_limit);
 struct powerdomain *omap_device_get_pwrdm(struct omap_device *od);
-u32 omap_device_get_context_loss_count(struct platform_device *pdev);
+int omap_device_get_context_loss_count(struct platform_device *pdev);
 
 /* Other */
 
diff --git a/arch/arm/plat-omap/include/plat/omap_hwmod.h b/arch/arm/plat-omap/include/plat/omap_hwmod.h
index 1adea9c..8658e2d 100644
--- a/arch/arm/plat-omap/include/plat/omap_hwmod.h
+++ b/arch/arm/plat-omap/include/plat/omap_hwmod.h
@@ -598,7 +598,7 @@ int omap_hwmod_for_each_by_class(const char *classname,
 				 void *user);
 
 int omap_hwmod_set_postsetup_state(struct omap_hwmod *oh, u8 state);
-u32 omap_hwmod_get_context_loss_count(struct omap_hwmod *oh);
+int omap_hwmod_get_context_loss_count(struct omap_hwmod *oh);
 
 int omap_hwmod_no_setup_reset(struct omap_hwmod *oh);
 
diff --git a/arch/arm/plat-omap/omap-pm-noop.c b/arch/arm/plat-omap/omap-pm-noop.c
index b0471bb2..477e3aa 100644
--- a/arch/arm/plat-omap/omap-pm-noop.c
+++ b/arch/arm/plat-omap/omap-pm-noop.c
@@ -27,7 +27,7 @@
 #include <plat/omap_device.h>
 
 static bool off_mode_enabled;
-static u32 dummy_context_loss_counter;
+static int dummy_context_loss_counter;
 
 /*
  * Device-driver-originated constraints (via board-*.c files)
@@ -311,22 +311,32 @@ void omap_pm_disable_off_mode(void)
 
 #ifdef CONFIG_ARCH_OMAP2PLUS
 
-u32 omap_pm_get_dev_context_loss_count(struct device *dev)
+int omap_pm_get_dev_context_loss_count(struct device *dev)
 {
 	struct platform_device *pdev = to_platform_device(dev);
-	u32 count;
+	int count;
 
 	if (WARN_ON(!dev))
-		return 0;
+		return -ENODEV;
 
 	if (dev->parent == &omap_device_parent) {
 		count = omap_device_get_context_loss_count(pdev);
 	} else {
 		WARN_ONCE(off_mode_enabled, "omap_pm: using dummy context loss counter; device %s should be converted to omap_device",
 			  dev_name(dev));
-		if (off_mode_enabled)
-			dummy_context_loss_counter++;
+
 		count = dummy_context_loss_counter;
+
+		if (off_mode_enabled) {
+			count++;
+			/*
+			 * Context loss count has to be a non-negative value.
+			 * Clear the sign bit to get a value range from 0 to
+			 * INT_MAX.
+			 */
+			count &= ~(1 << 31);
+			dummy_context_loss_counter = count;
+		}
 	}
 
 	pr_debug("OMAP PM: context loss count for dev %s = %d\n",
@@ -337,7 +347,7 @@ u32 omap_pm_get_dev_context_loss_count(struct device *dev)
 
 #else
 
-u32 omap_pm_get_dev_context_loss_count(struct device *dev)
+int omap_pm_get_dev_context_loss_count(struct device *dev)
 {
 	return dummy_context_loss_counter;
 }
diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c
index 9bbda9a..9753f71 100644
--- a/arch/arm/plat-omap/omap_device.c
+++ b/arch/arm/plat-omap/omap_device.c
@@ -310,7 +310,7 @@ static void _add_optional_clock_clkdev(struct omap_device *od,
  * return the context loss counter for that hwmod, otherwise return
  * zero.
  */
-u32 omap_device_get_context_loss_count(struct platform_device *pdev)
+int omap_device_get_context_loss_count(struct platform_device *pdev)
 {
 	struct omap_device *od;
 	u32 ret = 0;
-- 
1.7.4.1

[PATCH 2/3] OMAP: add omap_device_reset()

[Tomi Valkeinen]

Add omap_device_reset() function which can be used to reset the hwmods
associated with the given platform device.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 arch/arm/plat-omap/include/plat/omap_device.h |    1 +
 arch/arm/plat-omap/omap_device.c              |   23 +++++++++++++++++++++++
 2 files changed, 24 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 70d31d0..bcf1b35 100644
--- a/arch/arm/plat-omap/include/plat/omap_device.h
+++ b/arch/arm/plat-omap/include/plat/omap_device.h
@@ -108,6 +108,7 @@ int omap_device_align_pm_lat(struct platform_device *pdev,
 			     u32 new_wakeup_lat_limit);
 struct powerdomain *omap_device_get_pwrdm(struct omap_device *od);
 int omap_device_get_context_loss_count(struct platform_device *pdev);
+int omap_device_reset(struct platform_device *pdev);
 
 /* Other */
 
diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c
index 9753f71..4e6fc1b 100644
--- a/arch/arm/plat-omap/omap_device.c
+++ b/arch/arm/plat-omap/omap_device.c
@@ -324,6 +324,29 @@ int omap_device_get_context_loss_count(struct platform_device *pdev)
 }
 
 /**
+ * omap_device_reset - reset hwmods of given device
+ * @pdev: struct platform_device *
+ *
+ * Reset all hwmods associated with the given device. If a reset of a hwmod
+ * fails the rest of the hwmods are skipped and the error is returned.
+ */
+int omap_device_reset(struct platform_device *pdev)
+{
+	struct omap_device *od;
+	int i, r;
+
+	od = _find_by_pdev(pdev);
+
+	for (i = 0; i < od->hwmods_cnt; i++) {
+		r = omap_hwmod_reset(od->hwmods[i]);
+		if (r)
+			return r;
+	}
+
+	return 0;
+}
+
+/**
  * omap_device_count_resources - count number of struct resource entries needed
  * @od: struct omap_device *
  *
-- 
1.7.4.1

[PATCH 3/3] OMAP: Add (omap_device|omap_hwmod)_can_ever_lose_context functions

[Tomi Valkeinen]

Add hwmod and omap_device versions of can_ever_lose_context functions
so that drivers are able to use it.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod.c              |   22 ++++++++++++++++++++++
 arch/arm/plat-omap/include/plat/omap_device.h |    1 +
 arch/arm/plat-omap/include/plat/omap_hwmod.h  |    1 +
 arch/arm/plat-omap/omap_device.c              |   23 +++++++++++++++++++++++
 4 files changed, 47 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index 4f0d554..210280e 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -2369,3 +2369,25 @@ int omap_hwmod_no_setup_reset(struct omap_hwmod *oh)
 
 	return 0;
 }
+
+/**
+ * omap_hwmod_can_ever_lose_context - can this hwmod ever lose context?
+ * @oh: struct omap_hwmod *
+ *
+ * Finds the powerdomain associated with the given hwmod, and calls
+ * pwrdm_can_ever_lose_context for the powerdomain.
+ *
+ * Returns the return value from pwrdm_can_ever_lose_context, or true if no
+ * powerdomain is associated with the hwmod.
+ */
+bool omap_hwmod_can_ever_lose_context(struct omap_hwmod *oh)
+{
+	struct powerdomain *pwrdm;
+	int ret = true;
+
+	pwrdm = omap_hwmod_get_pwrdm(oh);
+	if (pwrdm)
+		ret = pwrdm_can_ever_lose_context(pwrdm);
+
+	return ret;
+}
diff --git a/arch/arm/plat-omap/include/plat/omap_device.h b/arch/arm/plat-omap/include/plat/omap_device.h
index bcf1b35..640c2eb 100644
--- a/arch/arm/plat-omap/include/plat/omap_device.h
+++ b/arch/arm/plat-omap/include/plat/omap_device.h
@@ -109,6 +109,7 @@ int omap_device_align_pm_lat(struct platform_device *pdev,
 struct powerdomain *omap_device_get_pwrdm(struct omap_device *od);
 int omap_device_get_context_loss_count(struct platform_device *pdev);
 int omap_device_reset(struct platform_device *pdev);
+bool omap_device_can_ever_lose_context(struct platform_device *pdev);
 
 /* Other */
 
diff --git a/arch/arm/plat-omap/include/plat/omap_hwmod.h b/arch/arm/plat-omap/include/plat/omap_hwmod.h
index 8658e2d..0d7435e 100644
--- a/arch/arm/plat-omap/include/plat/omap_hwmod.h
+++ b/arch/arm/plat-omap/include/plat/omap_hwmod.h
@@ -601,6 +601,7 @@ int omap_hwmod_set_postsetup_state(struct omap_hwmod *oh, u8 state);
 int omap_hwmod_get_context_loss_count(struct omap_hwmod *oh);
 
 int omap_hwmod_no_setup_reset(struct omap_hwmod *oh);
+bool omap_hwmod_can_ever_lose_context(struct omap_hwmod *oh);
 
 /*
  * Chip variant-specific hwmod init routines - XXX should be converted
diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c
index 4e6fc1b..bcee8d3 100644
--- a/arch/arm/plat-omap/omap_device.c
+++ b/arch/arm/plat-omap/omap_device.c
@@ -347,6 +347,29 @@ int omap_device_reset(struct platform_device *pdev)
 }
 
 /**
+ * omap_device_can_ever_lose_context - can this device ever lose context?
+ * @pdev: struct platform_device *
+ *
+ * Calls omap_hwmod_can_ever_lose_context for all hwmods associated with the
+ * given device, and returns true if one of the hwmods returns true. Otherwise
+ * returns false.
+ */
+bool omap_device_can_ever_lose_context(struct platform_device *pdev)
+{
+	struct omap_device *od;
+	int i;
+
+	od = _find_by_pdev(pdev);
+
+	for (i = 0; i < od->hwmods_cnt; i++) {
+		if (omap_hwmod_can_ever_lose_context(od->hwmods[i]))
+			return true;
+	}
+
+	return false;
+}
+
+/**
  * omap_device_count_resources - count number of struct resource entries needed
  * @od: struct omap_device *
  *
-- 
1.7.4.1

Re: [PATCH 1/3] OMAP: change get_context_loss_count ret value to int

[Kevin Hilman]

Tomi Valkeinen <tomi.valkeinen@ti.com> writes:

> get_context_loss_count functions return context loss count as u32, and
> zero means an error. However, zero is also returned when context has
> never been lost and could also be returned when the context loss count
> has wrapped and goes to zero.
>
> Change the functions to return an int, with negative value meaning an
> error.
>
> OMAP HSMMC code uses omap_pm_get_dev_context_loss_count(), but as the
> hsmmc code handles the returned value as an int, with negative value
> meaning an error, this patch actually fixes hsmmc code also.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>

Acked-by: Kevin Hilman <khilman@ti.com>

Re: [PATCH 0/3] Some omap_device/hwmod/pwrdomain patches

[Kevin Hilman]

Tomi Valkeinen <tomi.valkeinen@ti.com> writes:

> I came up with these patches while implementing pm runtime adaptation for DSS
> driver. I'm not quite sure on who's turf they belong, or do they even belong
> together, but here they are anyway.
>
> get_context_loss_count patch was discussed on l-o with Kevin.
>
> The omap_device_reset patch I made as some parts of DSS currently presume that
> the HW module is reset when it is enabled, and the reset is better handled in
> hwmod code.
>
> can_ever_lose_context code I didn't strictly need, but as there's such a
> function I added that to the context save code in DSS while rewriting the code.

Are any of the DSS blocks in power domains that can't lose context
(WKUP?)

This isn't something in general that drivers should be aware of, so I'd
rather not see this exposed to drivers (unless there's a real need.)

As soon as I finish the move to device power domains (hopefully for
2.6.41), the driver's callbacks will only be called if the device has
lost context, so checking for context loss will not be needed at all at
the driver level.

Kevin

>
>  Tomi
>
> Tomi Valkeinen (3):
>   OMAP: change get_context_loss_count ret value to int
>   OMAP: add omap_device_reset()
>   OMAP: Add (omap_device|omap_hwmod)_can_ever_lose_context functions
>
>  arch/arm/mach-omap2/omap_hwmod.c              |   24 ++++++++++++-
>  arch/arm/mach-omap2/powerdomain.c             |   14 +++++--
>  arch/arm/mach-omap2/powerdomain.h             |    2 +-
>  arch/arm/plat-omap/include/plat/omap-pm.h     |    4 +-
>  arch/arm/plat-omap/include/plat/omap_device.h |    4 ++-
>  arch/arm/plat-omap/include/plat/omap_hwmod.h  |    3 +-
>  arch/arm/plat-omap/omap-pm-noop.c             |   24 +++++++++----
>  arch/arm/plat-omap/omap_device.c              |   48 ++++++++++++++++++++++++-
>  8 files changed, 105 insertions(+), 18 deletions(-)

Re: [PATCH 1/3] OMAP: change get_context_loss_count ret value to int

[Paul Walmsley]

Hello Tomi, Kevin,

On Thu, 26 May 2011, Tomi Valkeinen wrote:

> get_context_loss_count functions return context loss count as u32, and
> zero means an error. However, zero is also returned when context has
> never been lost and could also be returned when the context loss count
> has wrapped and goes to zero.
> 
> Change the functions to return an int, with negative value meaning an
> error.
> 
> OMAP HSMMC code uses omap_pm_get_dev_context_loss_count(), but as the
> hsmmc code handles the returned value as an int, with negative value
> meaning an error, this patch actually fixes hsmmc code also.

Thanks, I agree this makes more sense than the previous arrangement.

One minor comment:

> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
>  arch/arm/mach-omap2/omap_hwmod.c              |    2 +-
>  arch/arm/mach-omap2/powerdomain.c             |   14 ++++++++++----
>  arch/arm/mach-omap2/powerdomain.h             |    2 +-
>  arch/arm/plat-omap/include/plat/omap-pm.h     |    4 ++--
>  arch/arm/plat-omap/include/plat/omap_device.h |    2 +-
>  arch/arm/plat-omap/include/plat/omap_hwmod.h  |    2 +-
>  arch/arm/plat-omap/omap-pm-noop.c             |   24 +++++++++++++++++-------
>  arch/arm/plat-omap/omap_device.c              |    2 +-
>  8 files changed, 34 insertions(+), 18 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
> index e034294..4f0d554 100644
> --- a/arch/arm/mach-omap2/omap_hwmod.c
> +++ b/arch/arm/mach-omap2/omap_hwmod.c
> @@ -2332,7 +2332,7 @@ ohsps_unlock:
>   * Returns the context loss count of the powerdomain assocated with @oh
>   * upon success, or zero if no powerdomain exists for @oh.
>   */
> -u32 omap_hwmod_get_context_loss_count(struct omap_hwmod *oh)
> +int omap_hwmod_get_context_loss_count(struct omap_hwmod *oh)
>  {
>  	struct powerdomain *pwrdm;
>  	int ret = 0;
> diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c
> index 9af0847..9281481 100644
> --- a/arch/arm/mach-omap2/powerdomain.c
> +++ b/arch/arm/mach-omap2/powerdomain.c
> @@ -935,16 +935,16 @@ int pwrdm_post_transition(void)
>   * @pwrdm: struct powerdomain * to wait for
>   *
>   * Context loss count is the sum of powerdomain off-mode counter, the
> - * logic off counter and the per-bank memory off counter.  Returns 0
> + * logic off counter and the per-bank memory off counter.  Returns negative
>   * (and WARNs) upon error, otherwise, returns the context loss count.
>   */
> -u32 pwrdm_get_context_loss_count(struct powerdomain *pwrdm)
> +int pwrdm_get_context_loss_count(struct powerdomain *pwrdm)
>  {
>  	int i, count;
>  
>  	if (!pwrdm) {
>  		WARN(1, "powerdomain: %s: pwrdm is null\n", __func__);
> -		return 0;
> +		return -ENODEV;
>  	}
>  
>  	count = pwrdm->state_counter[PWRDM_POWER_OFF];
> @@ -953,7 +953,13 @@ u32 pwrdm_get_context_loss_count(struct powerdomain *pwrdm)
>  	for (i = 0; i < pwrdm->banks; i++)
>  		count += pwrdm->ret_mem_off_counter[i];
>  
> -	pr_debug("powerdomain: %s: context loss count = %u\n",
> +	/*
> +	 * Context loss count has to be a non-negative value. Clear the sign
> +	 * bit to get a value range from 0 to INT_MAX.
> +	 */
> +	count &= ~(1 << 31);

Could you use INT_MAX here?  It seems best not to have the implicit 
dependency on 32-bit ints, if it is not too inconvenient.

> +
> +	pr_debug("powerdomain: %s: context loss count = %d\n",
>  		 pwrdm->name, count);
>  
>  	return count;
> diff --git a/arch/arm/mach-omap2/powerdomain.h b/arch/arm/mach-omap2/powerdomain.h
> index d23d979..012827f 100644
> --- a/arch/arm/mach-omap2/powerdomain.h
> +++ b/arch/arm/mach-omap2/powerdomain.h
> @@ -207,7 +207,7 @@ int pwrdm_clkdm_state_switch(struct clockdomain *clkdm);
>  int pwrdm_pre_transition(void);
>  int pwrdm_post_transition(void);
>  int pwrdm_set_lowpwrstchange(struct powerdomain *pwrdm);
> -u32 pwrdm_get_context_loss_count(struct powerdomain *pwrdm);
> +int pwrdm_get_context_loss_count(struct powerdomain *pwrdm);
>  bool pwrdm_can_ever_lose_context(struct powerdomain *pwrdm);
>  
>  extern void omap2xxx_powerdomains_init(void);
> diff --git a/arch/arm/plat-omap/include/plat/omap-pm.h b/arch/arm/plat-omap/include/plat/omap-pm.h
> index c0a7520..68df031 100644
> --- a/arch/arm/plat-omap/include/plat/omap-pm.h
> +++ b/arch/arm/plat-omap/include/plat/omap-pm.h
> @@ -350,9 +350,9 @@ unsigned long omap_pm_cpu_get_freq(void);
>   * driver must restore device context.   If the number of context losses
>   * exceeds the maximum positive integer, the function will wrap to 0 and
>   * continue counting.  Returns the number of context losses for this device,
> - * or zero upon error.
> + * or negative value upon error.
>   */
> -u32 omap_pm_get_dev_context_loss_count(struct device *dev);
> +int omap_pm_get_dev_context_loss_count(struct device *dev);
>  
>  void omap_pm_enable_off_mode(void);
>  void omap_pm_disable_off_mode(void);
> diff --git a/arch/arm/plat-omap/include/plat/omap_device.h b/arch/arm/plat-omap/include/plat/omap_device.h
> index e4c349f..70d31d0 100644
> --- a/arch/arm/plat-omap/include/plat/omap_device.h
> +++ b/arch/arm/plat-omap/include/plat/omap_device.h
> @@ -107,7 +107,7 @@ void __iomem *omap_device_get_rt_va(struct omap_device *od);
>  int omap_device_align_pm_lat(struct platform_device *pdev,
>  			     u32 new_wakeup_lat_limit);
>  struct powerdomain *omap_device_get_pwrdm(struct omap_device *od);
> -u32 omap_device_get_context_loss_count(struct platform_device *pdev);
> +int omap_device_get_context_loss_count(struct platform_device *pdev);
>  
>  /* Other */
>  
> diff --git a/arch/arm/plat-omap/include/plat/omap_hwmod.h b/arch/arm/plat-omap/include/plat/omap_hwmod.h
> index 1adea9c..8658e2d 100644
> --- a/arch/arm/plat-omap/include/plat/omap_hwmod.h
> +++ b/arch/arm/plat-omap/include/plat/omap_hwmod.h
> @@ -598,7 +598,7 @@ int omap_hwmod_for_each_by_class(const char *classname,
>  				 void *user);
>  
>  int omap_hwmod_set_postsetup_state(struct omap_hwmod *oh, u8 state);
> -u32 omap_hwmod_get_context_loss_count(struct omap_hwmod *oh);
> +int omap_hwmod_get_context_loss_count(struct omap_hwmod *oh);
>  
>  int omap_hwmod_no_setup_reset(struct omap_hwmod *oh);
>  
> diff --git a/arch/arm/plat-omap/omap-pm-noop.c b/arch/arm/plat-omap/omap-pm-noop.c
> index b0471bb2..477e3aa 100644
> --- a/arch/arm/plat-omap/omap-pm-noop.c
> +++ b/arch/arm/plat-omap/omap-pm-noop.c
> @@ -27,7 +27,7 @@
>  #include <plat/omap_device.h>
>  
>  static bool off_mode_enabled;
> -static u32 dummy_context_loss_counter;
> +static int dummy_context_loss_counter;
>  
>  /*
>   * Device-driver-originated constraints (via board-*.c files)
> @@ -311,22 +311,32 @@ void omap_pm_disable_off_mode(void)
>  
>  #ifdef CONFIG_ARCH_OMAP2PLUS
>  
> -u32 omap_pm_get_dev_context_loss_count(struct device *dev)
> +int omap_pm_get_dev_context_loss_count(struct device *dev)
>  {
>  	struct platform_device *pdev = to_platform_device(dev);
> -	u32 count;
> +	int count;
>  
>  	if (WARN_ON(!dev))
> -		return 0;
> +		return -ENODEV;
>  
>  	if (dev->parent == &omap_device_parent) {
>  		count = omap_device_get_context_loss_count(pdev);
>  	} else {
>  		WARN_ONCE(off_mode_enabled, "omap_pm: using dummy context loss counter; device %s should be converted to omap_device",
>  			  dev_name(dev));
> -		if (off_mode_enabled)
> -			dummy_context_loss_counter++;
> +
>  		count = dummy_context_loss_counter;
> +
> +		if (off_mode_enabled) {
> +			count++;
> +			/*
> +			 * Context loss count has to be a non-negative value.
> +			 * Clear the sign bit to get a value range from 0 to
> +			 * INT_MAX.
> +			 */
> +			count &= ~(1 << 31);

And likewise here.

> +			dummy_context_loss_counter = count;
> +		}
>  	}
>  
>  	pr_debug("OMAP PM: context loss count for dev %s = %d\n",
> @@ -337,7 +347,7 @@ u32 omap_pm_get_dev_context_loss_count(struct device *dev)
>  
>  #else
>  
> -u32 omap_pm_get_dev_context_loss_count(struct device *dev)
> +int omap_pm_get_dev_context_loss_count(struct device *dev)
>  {
>  	return dummy_context_loss_counter;
>  }
> diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c
> index 9bbda9a..9753f71 100644
> --- a/arch/arm/plat-omap/omap_device.c
> +++ b/arch/arm/plat-omap/omap_device.c
> @@ -310,7 +310,7 @@ static void _add_optional_clock_clkdev(struct omap_device *od,
>   * return the context loss counter for that hwmod, otherwise return
>   * zero.
>   */
> -u32 omap_device_get_context_loss_count(struct platform_device *pdev)
> +int omap_device_get_context_loss_count(struct platform_device *pdev)
>  {
>  	struct omap_device *od;
>  	u32 ret = 0;
> -- 
> 1.7.4.1
> 


- Paul

Re: [PATCH 1/3] OMAP: change get_context_loss_count ret value to int

[Tomi Valkeinen]

On Thu, 2011-05-26 at 12:37 -0600, Paul Walmsley wrote:
> Hello Tomi, Kevin,
> 
> On Thu, 26 May 2011, Tomi Valkeinen wrote:
> 
> > get_context_loss_count functions return context loss count as u32, and
> > zero means an error. However, zero is also returned when context has
> > never been lost and could also be returned when the context loss count
> > has wrapped and goes to zero.
> > 
> > Change the functions to return an int, with negative value meaning an
> > error.
> > 
> > OMAP HSMMC code uses omap_pm_get_dev_context_loss_count(), but as the
> > hsmmc code handles the returned value as an int, with negative value
> > meaning an error, this patch actually fixes hsmmc code also.
> 
> Thanks, I agree this makes more sense than the previous arrangement.
> 
> One minor comment:

<snip>

> > @@ -953,7 +953,13 @@ u32 pwrdm_get_context_loss_count(struct powerdomain *pwrdm)
> >  	for (i = 0; i < pwrdm->banks; i++)
> >  		count += pwrdm->ret_mem_off_counter[i];
> >  
> > -	pr_debug("powerdomain: %s: context loss count = %u\n",
> > +	/*
> > +	 * Context loss count has to be a non-negative value. Clear the sign
> > +	 * bit to get a value range from 0 to INT_MAX.
> > +	 */
> > +	count &= ~(1 << 31);
> 
> Could you use INT_MAX here?  It seems best not to have the implicit 
> dependency on 32-bit ints, if it is not too inconvenient.

Heh, Kevin made the same comment.

I didn't want to use INT_MAX because I feel INT_MAX is just a number,
not a mask. But as that's just a gut feeling, I'm fine with changing it
to INT_MAX.

 Tomi

Re: [PATCH 0/3] Some omap_device/hwmod/pwrdomain patches

[Tomi Valkeinen]

On Thu, 2011-05-26 at 10:30 -0700, Kevin Hilman wrote:
> Tomi Valkeinen <tomi.valkeinen@ti.com> writes:
> 
> > I came up with these patches while implementing pm runtime adaptation for DSS
> > driver. I'm not quite sure on who's turf they belong, or do they even belong
> > together, but here they are anyway.
> >
> > get_context_loss_count patch was discussed on l-o with Kevin.
> >
> > The omap_device_reset patch I made as some parts of DSS currently presume that
> > the HW module is reset when it is enabled, and the reset is better handled in
> > hwmod code.
> >
> > can_ever_lose_context code I didn't strictly need, but as there's such a
> > function I added that to the context save code in DSS while rewriting the code.
> 
> Are any of the DSS blocks in power domains that can't lose context
> (WKUP?)

Probably not. I have to say I don't know when can_ever_lose_context
returns false.

I had some old code in DSS's context save functions which disabled
context saving for OMAP2. I don't remember why that was put there, but
probably either 1) OMAP2's DSS can't ever lose context 2) OMAP2's DSS
couldn't lose context at the time the code was written.

I guess 2) is more likely, but nevertheless when I noticed
can_ever_lose_context I thought it'd be good to have that in the context
save code.

> This isn't something in general that drivers should be aware of, so I'd
> rather not see this exposed to drivers (unless there's a real need.)

Ok, I'll drop the patch. I don't think there's any need for this in DSS.

> As soon as I finish the move to device power domains (hopefully for
> 2.6.41), the driver's callbacks will only be called if the device has
> lost context, so checking for context loss will not be needed at all at
> the driver level.

This sounds good. Runtime PM's suspend & resume callbacks or something
else? 

 Tomi

Re: [PATCH 0/3] Some omap_device/hwmod/pwrdomain patches

[Kevin Hilman]

Tomi Valkeinen <tomi.valkeinen@ti.com> writes:

[...]

>
>> This isn't something in general that drivers should be aware of, so I'd
>> rather not see this exposed to drivers (unless there's a real need.)
>
> Ok, I'll drop the patch. I don't think there's any need for this in DSS.
>
>> As soon as I finish the move to device power domains (hopefully for
>> 2.6.41), the driver's callbacks will only be called if the device has
>> lost context, so checking for context loss will not be needed at all at
>> the driver level.
>
> This sounds good. Runtime PM's suspend & resume callbacks or something
> else? 

Runtime PM callbacks.

Kevin