Skip to contentSkip to conversation

OMAP2+: powerdomain: add API to get context loss count

PATCH In review Kevin Hilman 3 participants 12 messages Dec 10, 2010 – Dec 15, 2010

Subsystems (5)
Arm (32-bit) Arm sub-architectures OMAP2+ support OMAP power management support +1 more subsystems
lore
Share & tools
lore

Kevin Hilman

author
#0 lore raw

PATCH 1/2 OMAP2+: powerdomain: add API to get context loss count

Add new powerdomain API

int pwrdm_get_context_loss_count(struct powerdomain *pwrdm)

for checking how many times the powerdomain has lost context. The loss count is the sum sum of the powerdomain off-mode counter, the logic off counter and the per-bank memory off counter.

Cc Paul Walmsley <[email protected]>
Signed-off-by Kevin Hilman <[email protected]>
---
 arch/arm/mach-omap2/powerdomain.c |   23 +++++++++++++++++++++++
 arch/arm/mach-omap2/powerdomain.h |    1 +
 2 files changed, 24 insertions(+), 0 deletions(-)
arch/arm/mach-omap2/powerdomain.c +23 -0
index 06ef60e..78e7d22 100644
@@ -909,3 +909,26 @@ int pwrdm_post_transition(void)
 	pwrdm_for_each(_pwrdm_post_transition_cb, NULL);
 	return 0;
 }
+
+/**
+ * pwrdm_get_context_loss_count - get powerdomain's context loss count
+ * @pwrdm: struct powerdomain * to wait for
+ *
+ * Context loss count is a sum of powerdomain off-mode counter,
+ * the logic off counter and the per-bank memory off counter.
+ */
+int pwrdm_get_context_loss_count(struct powerdomain *pwrdm)
+{
+	int i, count;
+
+	count = pwrdm->state_counter[PWRDM_POWER_OFF];
+	count += pwrdm->ret_logic_off_counter;
+
+	for (i = 0; i < pwrdm->banks; i++)
+		count += pwrdm->ret_mem_off_counter[i];
+
+	pr_debug("powerdomain: %s: context loss count = %u\n",
+		 pwrdm->name, count);
+
+	return count;
+}
arch/arm/mach-omap2/powerdomain.h +1 -0
index 35b5b48..d269eff 100644
@@ -211,6 +211,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);
+int pwrdm_get_context_loss_count(struct powerdomain *pwrdm);
 
 extern void omap2xxx_powerdomains_init(void);
 extern void omap3xxx_powerdomains_init(void);
Signature · 1 line
1.7.2.1
author
#1 lore raw

PATCH 2/2 OMAP: PM: implement context loss count APIs

Implement OMAP PM layer omap_pm_get_dev_context_loss_count() API by creating similar APIs at the omap_device and omap_hwmod levels. The omap_hwmod level call is the layer with access to the powerdomain core, so it is the place where the powerdomain is queried to get the context loss count.

NOTE: only works for devices which have been converted to use
omap_device/omap_hwmod.

Longer term, we could possibly remove this API from the OMAP PM layer, and instead directly use the omap_device level API.

Cc Paul Walmsley <[email protected]>
Signed-off-by Kevin Hilman <[email protected]>
---
 arch/arm/mach-omap2/omap_hwmod.c              |   18 ++++++++++++++++++
 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-pm-noop.c             |   17 +++++++++--------
 arch/arm/plat-omap/omap_device.c              |   20 ++++++++++++++++++++
 5 files changed, 49 insertions(+), 8 deletions(-)
arch/arm/mach-omap2/omap_hwmod.c +18 -0
index 81c1097..0ec9c70 100644
@@ -2220,3 +2220,21 @@ ohsps_unlock:
 
 	return ret;
 }
+
+/**
+ * omap_hwmod_get_context_loss_count - get lost context count
+ * @oh: struct omap_hwmod *
+ *
+ * Query the powerdomain of of @oh to get the context loss
+ * count for this device.
+ */
+int omap_hwmod_get_context_loss_count(struct omap_hwmod *oh)
+{
+	struct powerdomain *pwrdm;
+
+	pwrdm = omap_hwmod_get_pwrdm(oh);
+	if (!pwrdm)
+		return -ENODEV;
+
+	return pwrdm_get_context_loss_count(pwrdm);
+}
arch/arm/plat-omap/include/plat/omap_device.h +1 -0
index 28e2d1a..70d31d0 100644
@@ -107,6 +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);
+int omap_device_get_context_loss_count(struct platform_device *pdev);
 
 /* Other */
arch/arm/plat-omap/include/plat/omap_hwmod.h +1 -0
index 62bdb23..5a96ac5 100644
@@ -568,6 +568,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);
+int omap_hwmod_get_context_loss_count(struct omap_hwmod *oh);
 
 /*
  * Chip variant-specific hwmod init routines - XXX should be converted
arch/arm/plat-omap/omap-pm-noop.c +9 -8
index 7578366..5a58f97 100644
@@ -20,9 +20,11 @@
 #include <linux/init.h>
 #include <linux/cpufreq.h>
 #include <linux/device.h>
+#include <linux/platform_device.h>
 
 /* Interface documentation is in mach/omap-pm.h */
 #include <plat/omap-pm.h>
+#include <plat/omap_device.h>
 
 struct omap_opp *dsp_opps;
 struct omap_opp *mpu_opps;

@@ -288,20 +290,19 @@ unsigned long omap_pm_cpu_get_freq(void)
 
 int omap_pm_get_dev_context_loss_count(struct device *dev)
 {
+	struct platform_device *pdev = to_platform_device(dev);
+	int count;
+
 	if (!dev) {
 		WARN_ON(1);
 		return -EINVAL;
 	};
 
-	pr_debug("OMAP PM: returning context loss count for dev %s\n",
-		 dev_name(dev));
+	count = omap_device_get_context_loss_count(pdev);
+	pr_debug("OMAP PM: context loss count for dev %s = %d\n",
+		 dev_name(dev), count);
 
-	/*
-	 * Map the device to the powerdomain.  Return the powerdomain
-	 * off counter.
-	 */
-
-	return 0;
+	return count;
 }
arch/arm/plat-omap/omap_device.c +20 -0
index abe933c..e29c2d6 100644
@@ -280,6 +280,26 @@ static void _add_optional_clock_alias(struct omap_device *od,
 /* Public functions for use by core code */
 
 /**
+ * omap_device_get_context_loss_count - get lost context count
+ * @od: struct omap_device *
+ *
+ * Using the primary hwmod for this device, query the context loss
+ * count for this device.
+ */
+int omap_device_get_context_loss_count(struct platform_device *pdev)
+{
+	struct omap_device *od;
+	int ret = -ENODEV;
+
+	od = _find_by_pdev(pdev);
+
+	if (od->hwmods_cnt)
+		omap_hwmod_get_context_loss_count(od->hwmods[0]);
+
+	return ret;
+}
+
+/**
  * omap_device_count_resources - count number of struct resource entries needed
  * @od: struct omap_device *
  *
Signature · 1 line
1.7.2.1
#2 lore raw

RE: [PATCH/RFC 2/2] OMAP: PM: implement context loss count APIs

Kevin,

Quoted 116 lines

-----Original Message-----
From: [email protected] [mailto:linux-omap- [email protected]] On Behalf Of Kevin Hilman
Sent: Friday, December 10, 2010 5:39 AM
To: [email protected]
Cc: Paul Walmsely
Subject: [PATCH/RFC 2/2] OMAP: PM: implement context loss count APIs

Implement OMAP PM layer omap_pm_get_dev_context_loss_count() API by
creating similar APIs at the omap_device and omap_hwmod levels. The omap_hwmod level call is the layer with access to the powerdomain core, so it is the place where the powerdomain is queried to get the context loss count.

NOTE: only works for devices which have been converted to use
omap_device/omap_hwmod.

Longer term, we could possibly remove this API from the OMAP PM layer, and instead directly use the omap_device level API.

Cc Paul Walmsley <[email protected]>
Signed-off-by Kevin Hilman <[email protected]>
---
 arch/arm/mach-omap2/omap_hwmod.c              |   18
++++++++++++++++++
 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-pm-noop.c             |   17 +++++++++-----
---
 arch/arm/plat-omap/omap_device.c              |   20
++++++++++++++++++++
 5 files changed, 49 insertions(+), 8 deletions(-)
arch/arm/mach- renamed from arch/arm/mach-omap2/omap_hwmod.c +0 -0

omap2/omap_hwmod.c
index 81c1097..0ec9c70 100644

arch/arm/mach-omap2/omap_hwmod.c +18 -0
@@ -2220,3 +2220,21 @@ ohsps_unlock:
 
 	return ret;
 }
+
+/**
+ * omap_hwmod_get_context_loss_count - get lost context count
+ * @oh: struct omap_hwmod *
+ *
+ * Query the powerdomain of of @oh to get the context loss
+ * count for this device.
+ */
+int omap_hwmod_get_context_loss_count(struct omap_hwmod *oh)
+{
+	struct powerdomain *pwrdm;
+
+	pwrdm = omap_hwmod_get_pwrdm(oh);
+	if (!pwrdm)
+		return -ENODEV;
+
+	return pwrdm_get_context_loss_count(pwrdm);
+}
arch/arm/plat-omap/include/plat/omap_device.h +1 -0
index 28e2d1a..70d31d0 100644
@@ -107,6 +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);
+int omap_device_get_context_loss_count(struct platform_device
 *pdev);

/* Other */

arch/arm/plat-omap/include/plat/omap_hwmod.h +1 -0
index 62bdb23..5a96ac5 100644
@@ -568,6 +568,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);
+int omap_hwmod_get_context_loss_count(struct omap_hwmod *oh);

/*
* Chip variant-specific hwmod init routines - XXX should be converted

arch/arm/plat- renamed from arch/arm/plat-omap/omap-pm-noop.c +0 -0

omap/omap-pm-noop.c
index 7578366..5a58f97 100644

arch/arm/plat-omap/omap-pm-noop.c +5 -0
@@ -20,9 +20,11 @@
 #include <linux/init.h>
 #include <linux/cpufreq.h>
 #include <linux/device.h>
+#include <linux/platform_device.h>
 
 /* Interface documentation is in mach/omap-pm.h */
 #include <plat/omap-pm.h>
+#include <plat/omap_device.h>
 
 struct omap_opp *dsp_opps;
 struct omap_opp *mpu_opps;

@@ -288,20 +290,19 @@ unsigned long omap_pm_cpu_get_freq(void)
 
 int omap_pm_get_dev_context_loss_count(struct device *dev)
 {
+	struct platform_device *pdev = to_platform_device(dev);
+	int count;
+

arch/arm/plat-omap/omap-pm-noop.c unsigned long omap_pm_cpu_get_freq(void) · line 303

Quoted 10 lines
arch/arm/plat-omap/omap-pm-noop.c +3 -2
 	if (!dev) {
 		WARN_ON(1);
 		return -EINVAL;
 	};
 
-	pr_debug("OMAP PM: returning context loss count for dev %s\n",
-		 dev_name(dev));
+	count = omap_device_get_context_loss_count(pdev);
+	pr_debug("OMAP PM: context loss count for dev %s = %d\n",
+		 dev_name(dev), count);

Shouldn't this implementation be part of omap-pm.c where all the OMAP PM functions are to be implemented? I thought omap-pm-noop.c should have dummy implementation.

Vishwa

Quoted 53 lines
arch/arm/plat-omap/omap-pm-noop.c +1 -6
-	/*
-	 * Map the device to the powerdomain.  Return the powerdomain
-	 * off counter.
-	 */
-
-	return 0;
+	return count;
 }
arch/arm/plat- renamed from arch/arm/plat-omap/omap_device.c +0 -0

omap/omap_device.c
index abe933c..e29c2d6 100644

arch/arm/plat-omap/omap_device.c +20 -0
@@ -280,6 +280,26 @@ static void _add_optional_clock_alias(struct
 omap_device *od,
 /* Public functions for use by core code */
 
 /**
+ * omap_device_get_context_loss_count - get lost context count
+ * @od: struct omap_device *
+ *
+ * Using the primary hwmod for this device, query the context loss
+ * count for this device.
+ */
+int omap_device_get_context_loss_count(struct platform_device
 *pdev)
+{
+	struct omap_device *od;
+	int ret = -ENODEV;
+
+	od = _find_by_pdev(pdev);
+
+	if (od->hwmods_cnt)
+		omap_hwmod_get_context_loss_count(od->hwmods[0]);
+
+	return ret;
+}
+
+/**
  * omap_device_count_resources - count number of struct resource

entries needed
* @od: struct omap_device *
*
--
1.7.2.1

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

author
#3 lore raw

Re: [PATCH/RFC 2/2] OMAP: PM: implement context loss count APIs

Quoted 1 line · Vishwanath Sripathy

Kevin,

[...]

Quoted 12 lines
Quoted 12 lines
arch/arm/plat-omap/omap-pm-noop.c +3 -0
@@ -288,20 +290,19 @@ unsigned long omap_pm_cpu_get_freq(void)
 
 int omap_pm_get_dev_context_loss_count(struct device *dev)
 {
+	struct platform_device *pdev = to_platform_device(dev);
+	int count;
+
 	if (!dev) {
 		WARN_ON(1);
 		return -EINVAL;
 	};
Quoted 8 lines

arch/arm/plat-omap/omap-pm-noop.c unsigned long omap_pm_cpu_get_freq(void) · line 303

Quoted 5 lines
arch/arm/plat-omap/omap-pm-noop.c +3 -2
-	pr_debug("OMAP PM: returning context loss count for dev %s\n",
-		 dev_name(dev));
+	count = omap_device_get_context_loss_count(pdev);
+	pr_debug("OMAP PM: context loss count for dev %s = %d\n",
+		 dev_name(dev), count);

Shouldn't this implementation be part of omap-pm.c where all the OMAP PM functions are to be implemented?

Where is omap-pm.c?

Kevin

#4 lore raw

RE: [PATCH/RFC 2/2] OMAP: PM: implement context loss count APIs

Kevin,

Quoted 33 lines

-----Original Message-----
From: Kevin Hilman [mailto:[email protected]]
Sent: Saturday, December 11, 2010 6:14 AM
To: Vishwanath Sripathy
Cc: [email protected]; Paul Walmsely
Subject: Re: [PATCH/RFC 2/2] OMAP: PM: implement context loss count APIs

Quoted 1 line · Vishwanath Sripathy

Kevin,

[...]

Quoted 1 line

Review unsigned long

Quoted 1 line
(quoted diff) +0 -0
@@ -288,20 +290,19 @@ unsigned long

omap_pm_cpu_get_freq(void)

Quoted 11 lines
Quoted 11 lines
arch/arm/plat-omap/omap-pm-noop.c +3 -0
 int omap_pm_get_dev_context_loss_count(struct device *dev)
 {
+	struct platform_device *pdev = to_platform_device(dev);
+	int count;
+
 	if (!dev) {
 		WARN_ON(1);
 		return -EINVAL;
 	};
Quoted 7 lines

arch/arm/plat-omap/omap-pm-noop.c unsigned long omap_pm_cpu_get_freq(void) · line 303

Quoted 5 lines
arch/arm/plat-omap/omap-pm-noop.c +3 -2
-	pr_debug("OMAP PM: returning context loss count for dev %s\n",
-		 dev_name(dev));
+	count = omap_device_get_context_loss_count(pdev);
+	pr_debug("OMAP PM: context loss count for dev %s = %d\n",
+		 dev_name(dev), count);

Shouldn't this implementation be part of omap-pm.c where all the

Quoted 4 lines

OMAP PM

Quoted 1 line

functions are to be implemented?

Where is omap-pm.c?

It's not present and needs to be added. This file is anyway required for adding device latency constraints as well.

Vishwa

Quoted 2 lines

Kevin

maintainer
#5 lore raw

RE: [PATCH/RFC 2/2] OMAP: PM: implement context loss count APIs

In reply to Vishwanath Sripathy

Hello Vishwa,

Quoted 6 lines · Vishwanath Sripathy

arch/arm/plat-omap/omap-pm-noop.c unsigned long omap_pm_cpu_get_freq(void) · line 303

Quoted 3 lines
arch/arm/plat-omap/omap-pm-noop.c +3 -0
+	count = omap_device_get_context_loss_count(pdev);
+	pr_debug("OMAP PM: context loss count for dev %s = %d\n",
+		 dev_name(dev), count);

Shouldn't this implementation be part of omap-pm.c where all the OMAP PM functions are to be implemented? I thought omap-pm-noop.c should have dummy implementation.

In general, yes. But we also want the code in omap-pm-noop.c not to cause additional breakage. Unlike most of the other functions in this file, if the context loss count function doesn't do something minimally useful, then the system is going to break badly. You've probably seen this thread:

http://www.mail-archive.com/[email protected]/msg40079.html

(By the way, the reason why I think we shouldn't use the approach described in

http://www.mail-archive.com/[email protected]/msg40101.html

is because I suspect it is going to seriously damage retention idle performance. For example, the HSMMC driver resets its entire IP block in its context restore function...)

But to confirm your general point, yes, in general, further functional development of the OMAP PM code should take place outside the no-op file. Hopefully, at some point, we'll be able to drop the no-op file. Once there is a useful replacement, we should be able to switch to it as a default.

- Paul

#6 lore raw

RE: [PATCH/RFC 2/2] OMAP: PM: implement context loss count APIs

Paul,

Quoted 16 lines

-----Original Message-----
From: Paul Walmsley [mailto:[email protected]]
Sent: Monday, December 13, 2010 4:13 PM
To: Vishwanath Sripathy
Cc: Kevin Hilman; [email protected]
Subject: RE: [PATCH/RFC 2/2] OMAP: PM: implement context loss count APIs

Hello Vishwa,

Quoted 4 lines · Vishwanath Sripathy

arch/arm/plat-omap/omap-pm-noop.c unsigned long omap_pm_cpu_get_freq(void) · line 303

Quoted 3 lines
arch/arm/plat-omap/omap-pm-noop.c +3 -0
+	count = omap_device_get_context_loss_count(pdev);
+	pr_debug("OMAP PM: context loss count for dev %s = %d\n",
+		 dev_name(dev), count);

Shouldn't this implementation be part of omap-pm.c where all the

Quoted 8 lines

OMAP PM

Quoted 1 line

functions are to be implemented? I thought omap-pm-noop.c should

have

Quoted 1 line

dummy implementation.

In general, yes. But we also want the code in omap-pm-noop.c not to cause
additional breakage. Unlike most of the other functions in this file,

if

Quoted 7 lines

the context loss count function doesn't do something minimally useful, then
the system is going to break badly. You've probably seen this thread:

http://www.mail-archive.com/linux-
[email protected]/msg40079.html

Quoted 8 lines

(By the way, the reason why I think we shouldn't use the approach described in

http://www.mail-archive.com/linux-
[email protected]/msg40101.html

is because I suspect it is going to seriously damage retention idle performance. For example, the HSMMC driver resets its entire IP block

in

Quoted 8 lines

its context restore function...)

But to confirm your general point, yes, in general, further functional development of the OMAP PM code should take place outside the no-op file.
Hopefully, at some point, we'll be able to drop the no-op file. Once there is a useful replacement, we should be able to switch to it as a default.

I have no issues with the implementation and I agree. All I am saying is that why can't this implementation be added in omap-pm.c and compile this file instead of omap-pm-noop.c when OMAP_PM is enabled. I believe this function is useful when off mode is enabled which means OMAP_PM is enabled.

Vishwa

Quoted 3 lines

- Paul

maintainer
#7 lore raw

RE: [PATCH/RFC 2/2] OMAP: PM: implement context loss count APIs

Vishwa

Quoted 3 lines · Vishwanath Sripathy

I have no issues with the implementation and I agree. All I am saying is that why can't this implementation be added in omap-pm.c and compile this file instead of omap-pm-noop.c

1. because omap-pm-noop.c needs to not actively break anything when it is used, and this function, unlike most of the other functions in omap-pm-noop.c, has no valid 'no-op' failure mode; and

2. because no one has yet posted a complete, clean, working, SRF-free OMAP PM layer to the lists. It was the hope several years ago when this interface was created that someone else would be capable of doing this.

Quoted 1 line

when OMAP_PM is enabled.

What is this OMAP_PM that you are referring to?

Quoted 2 lines

I believe this function is useful when off mode is enabled which means OMAP_PM is enabled.

Huh? Mainline Linux on OMAP can enter off-mode with omap2plus_defconfig right now via a runtime debugfs setting. There is no KConfig setting to enable.

- Paul

maintainer
#8 lore raw

Re: [PATCH/RFC 2/2] OMAP: PM: implement context loss count APIs

In reply to Kevin Hilman

Hi Kevin

a few comments here

Quoted 27 lines · Kevin Hilman

Implement OMAP PM layer omap_pm_get_dev_context_loss_count() API by creating similar APIs at the omap_device and omap_hwmod levels. The omap_hwmod level call is the layer with access to the powerdomain core, so it is the place where the powerdomain is queried to get the context loss count.

NOTE: only works for devices which have been converted to use
omap_device/omap_hwmod.

Longer term, we could possibly remove this API from the OMAP PM layer, and instead directly use the omap_device level API.

Cc Paul Walmsley <[email protected]>
Signed-off-by Kevin Hilman <[email protected]>
---
 arch/arm/mach-omap2/omap_hwmod.c              |   18 ++++++++++++++++++
 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-pm-noop.c             |   17 +++++++++--------
 arch/arm/plat-omap/omap_device.c              |   20 ++++++++++++++++++++
 5 files changed, 49 insertions(+), 8 deletions(-)
arch/arm/mach-omap2/omap_hwmod.c +0 -0
index 81c1097..0ec9c70 100644

arch/arm/mach-omap2/omap_hwmod.c ohsps_unlock: · line 2229

Quoted 10 lines
arch/arm/mach-omap2/omap_hwmod.c +7 -0
@@ -2220,3 +2220,21 @@ ohsps_unlock:
 
 	return ret;
 }
+
+/**
+ * omap_hwmod_get_context_loss_count - get lost context count
+ * @oh: struct omap_hwmod *
+ *
+ * Query the powerdomain of of @oh to get the context loss
+ * count for this device.

Might want to document the possible return values of this function, and suggest that its callers consider the context to be lost whenever a successful call to this function results in any different value than the caller got the last time it called this code.

arch/arm/mach-omap2/omap_hwmod.c ohsps_unlock: · line 2234

Quoted 5 lines
arch/arm/mach-omap2/omap_hwmod.c +5 -0
+ */
+int omap_hwmod_get_context_loss_count(struct omap_hwmod *oh)
+{
+	struct powerdomain *pwrdm;
+

Please test to see if oh is null here...

Quoted 79 lines
arch/arm/mach-omap2/omap_hwmod.c +6 -0
+	pwrdm = omap_hwmod_get_pwrdm(oh);
+	if (!pwrdm)
+		return -ENODEV;
+
+	return pwrdm_get_context_loss_count(pwrdm);
+}
arch/arm/plat-omap/include/plat/omap_device.h +1 -0
index 28e2d1a..70d31d0 100644
@@ -107,6 +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);
+int omap_device_get_context_loss_count(struct platform_device *pdev);
 
 /* Other */
arch/arm/plat-omap/include/plat/omap_hwmod.h +1 -0
index 62bdb23..5a96ac5 100644
@@ -568,6 +568,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);
+int omap_hwmod_get_context_loss_count(struct omap_hwmod *oh);
 
 /*
  * Chip variant-specific hwmod init routines - XXX should be converted
arch/arm/plat-omap/omap-pm-noop.c +9 -8
index 7578366..5a58f97 100644
@@ -20,9 +20,11 @@
 #include <linux/init.h>
 #include <linux/cpufreq.h>
 #include <linux/device.h>
+#include <linux/platform_device.h>
 
 /* Interface documentation is in mach/omap-pm.h */
 #include <plat/omap-pm.h>
+#include <plat/omap_device.h>
 
 struct omap_opp *dsp_opps;
 struct omap_opp *mpu_opps;

@@ -288,20 +290,19 @@ unsigned long omap_pm_cpu_get_freq(void)
 
 int omap_pm_get_dev_context_loss_count(struct device *dev)
 {
+	struct platform_device *pdev = to_platform_device(dev);
+	int count;
+
 	if (!dev) {
 		WARN_ON(1);
 		return -EINVAL;
 	};
 
-	pr_debug("OMAP PM: returning context loss count for dev %s\n",
-		 dev_name(dev));
+	count = omap_device_get_context_loss_count(pdev);
+	pr_debug("OMAP PM: context loss count for dev %s = %d\n",
+		 dev_name(dev), count);
 
-	/*
-	 * Map the device to the powerdomain.  Return the powerdomain
-	 * off counter.
-	 */
-
-	return 0;
+	return count;
 }
arch/arm/plat-omap/omap_device.c +0 -0
index abe933c..e29c2d6 100644

arch/arm/plat-omap/omap_device.c static void _add_optional_clock_alias(struct omap_device *od, · line 287

Quoted 8 lines
arch/arm/plat-omap/omap_device.c +5 -0
@@ -280,6 +280,26 @@ static void _add_optional_clock_alias(struct omap_device *od,
 /* Public functions for use by core code */
 
 /**
+ * omap_device_get_context_loss_count - get lost context count
+ * @od: struct omap_device *
+ *
+ * Using the primary hwmod for this device, query the context loss
+ * count for this device.

Please document the possible return values here (as above).

Quoted 8 lines
arch/arm/plat-omap/omap_device.c +8 -0
+ */
+int omap_device_get_context_loss_count(struct platform_device *pdev)
+{
+	struct omap_device *od;
+	int ret = -ENODEV;
+
+	od = _find_by_pdev(pdev);
+

arch/arm/plat-omap/omap_device.c static void _add_optional_clock_alias(struct omap_device *od, · line 305

Quoted 10 lines
arch/arm/plat-omap/omap_device.c +7 -0
+	if (od->hwmods_cnt)
+		omap_hwmod_get_context_loss_count(od->hwmods[0]);
+
+	return ret;
+}
+
+/**
  * omap_device_count_resources - count number of struct resource entries needed
  * @od: struct omap_device *
  *

- Paul

author
#9 lore raw

[PATCH/RFC 3/2] OMAP: PM noop: implement context loss count for non-omap_devices

In reply to Kevin Hilman

For devices which have not (yet) been converted to use omap_device, implement the context loss counter using the "brutal method" as originally proposed by Paul Walmsley[1].

The dummy context loss counter is incremented every time it is checked, but only when off-mode is enabled. When off-mode is disabled, the dummy counter stops incrementing.

Tested on 36xx/Zoom3 using MMC driver, which is currently the only in-tree user of this API.

This patch should be reverted after all devices are converted to using omap_device.

[1] http://marc.info/?l=linux-omap&m=129176260000626&w=2

Cc Paul Walmsley <[email protected]>
Signed-off-by Kevin Hilman <[email protected]>
---
 arch/arm/mach-omap2/pm-debug.c            |    2 ++
 arch/arm/plat-omap/include/plat/omap-pm.h |    1 +
 arch/arm/plat-omap/omap-pm-noop.c         |   19 ++++++++++++++++++-
 3 files changed, 21 insertions(+), 1 deletions(-)
arch/arm/mach-omap2/pm-debug.c +2 -0
index e535082..af66acb 100644
@@ -32,6 +32,7 @@
 #include "powerdomain.h"
 #include "clockdomain.h"
 #include <plat/dmtimer.h>
+#include <plat/omap-pm.h>
 
 #include "cm2xxx_3xxx.h"
 #include "prm2xxx_3xxx.h"

@@ -581,6 +582,7 @@ static int option_set(void *data, u64 val)
 	*option = val;
 
 	if (option == &enable_off_mode) {
+		omap_pm_enable_off_mode(val ? true : false);
 		if (cpu_is_omap34xx())
 			omap3_pm_off_mode_enable(val);
 	}
arch/arm/plat-omap/include/plat/omap-pm.h +1 -0
index 9870b4f..cfa5e44 100644
@@ -372,5 +372,6 @@ unsigned long omap_pm_cpu_get_freq(void);
  */
 int omap_pm_get_dev_context_loss_count(struct device *dev);
 
+void omap_pm_enable_off_mode(bool enable);
 
 #endif
arch/arm/plat-omap/omap-pm-noop.c +18 -1
index 5a58f97..32f2818 100644
@@ -30,6 +30,9 @@ struct omap_opp *dsp_opps;
 struct omap_opp *mpu_opps;
 struct omap_opp *l3_opps;
 
+static bool off_mode_enabled;
+static u32 dummy_context_loss_counter;
+
 /*
  * Device-driver-originated constraints (via board-*.c files)
  */

@@ -287,6 +290,10 @@ unsigned long omap_pm_cpu_get_freq(void)
 /*
  * Device context loss tracking
  */
+void omap_pm_enable_off_mode(bool enable)
+{
+	off_mode_enabled = enable;
+}
 
 int omap_pm_get_dev_context_loss_count(struct device *dev)
 {

@@ -298,7 +305,17 @@ int omap_pm_get_dev_context_loss_count(struct device *dev)
 		return -EINVAL;
 	};
 
-	count = omap_device_get_context_loss_count(pdev);
+	if (dev->parent == &omap_device_parent) {
+		count = omap_device_get_context_loss_count(pdev);
+	} else {
+		WARN_ONCE(off_mode_enabled, "using dummy context loss counter, "
+			  "device %s should be converted to omap_device.",
+			  __func__, dev_name(dev));
+		if (off_mode_enabled)
+			dummy_context_loss_counter++;
+		count = dummy_context_loss_counter;
+	}
+
 	pr_debug("OMAP PM: context loss count for dev %s = %d\n",
 		 dev_name(dev), count);
Signature · 1 line
1.7.2.1
maintainer
#10 lore raw

Re: [PATCH/RFC 3/2] OMAP: PM noop: implement context loss count for non-omap_devices

Hi

a few more minor comments

Quoted 70 lines · Kevin Hilman

For devices which have not (yet) been converted to use omap_device, implement the context loss counter using the "brutal method" as originally proposed by Paul Walmsley[1].

The dummy context loss counter is incremented every time it is checked, but only when off-mode is enabled. When off-mode is disabled, the dummy counter stops incrementing.

Tested on 36xx/Zoom3 using MMC driver, which is currently the only in-tree user of this API.

This patch should be reverted after all devices are converted to using omap_device.

[1] http://marc.info/?l=linux-omap&m=129176260000626&w=2

Cc Paul Walmsley <[email protected]>
Signed-off-by Kevin Hilman <[email protected]>
---
 arch/arm/mach-omap2/pm-debug.c            |    2 ++
 arch/arm/plat-omap/include/plat/omap-pm.h |    1 +
 arch/arm/plat-omap/omap-pm-noop.c         |   19 ++++++++++++++++++-
 3 files changed, 21 insertions(+), 1 deletions(-)
arch/arm/mach-omap2/pm-debug.c +2 -0
index e535082..af66acb 100644
@@ -32,6 +32,7 @@
 #include "powerdomain.h"
 #include "clockdomain.h"
 #include <plat/dmtimer.h>
+#include <plat/omap-pm.h>
 
 #include "cm2xxx_3xxx.h"
 #include "prm2xxx_3xxx.h"

@@ -581,6 +582,7 @@ static int option_set(void *data, u64 val)
 	*option = val;
 
 	if (option == &enable_off_mode) {
+		omap_pm_enable_off_mode(val ? true : false);
 		if (cpu_is_omap34xx())
 			omap3_pm_off_mode_enable(val);
 	}
arch/arm/plat-omap/include/plat/omap-pm.h +1 -0
index 9870b4f..cfa5e44 100644
@@ -372,5 +372,6 @@ unsigned long omap_pm_cpu_get_freq(void);
  */
 int omap_pm_get_dev_context_loss_count(struct device *dev);
 
+void omap_pm_enable_off_mode(bool enable);
 
 #endif
arch/arm/plat-omap/omap-pm-noop.c +3 -0
index 5a58f97..32f2818 100644
@@ -30,6 +30,9 @@ struct omap_opp *dsp_opps;
 struct omap_opp *mpu_opps;
 struct omap_opp *l3_opps;
 
+static bool off_mode_enabled;
+static u32 dummy_context_loss_counter;
+
 /*
  * Device-driver-originated constraints (via board-*.c files)
  */

arch/arm/plat-omap/omap-pm-noop.c unsigned long omap_pm_cpu_get_freq(void) · line 292

Quoted 3 lines
arch/arm/plat-omap/omap-pm-noop.c +0 -0
@@ -287,6 +290,10 @@ unsigned long omap_pm_cpu_get_freq(void)
 /*
  * Device context loss tracking
  */

Please add some basic kerneldoc here -- maybe just document that it's meant to be called from the core OMAP PM or PM debug code whenever off-mode is enabled or disabled...

arch/arm/plat-omap/omap-pm-noop.c unsigned long omap_pm_cpu_get_freq(void) · line 296

Quoted 4 lines
arch/arm/plat-omap/omap-pm-noop.c +4 -0
+void omap_pm_enable_off_mode(bool enable)
+{
+	off_mode_enabled = enable;
+}

Personal preference: please split this into two functions, omap_pm_{enable,disable}_off_mode(void). That way, when I read it in other code, I don't have to try to remember what the argument means :-)

Quoted 8 lines
arch/arm/plat-omap/omap-pm-noop.c +1 -1
 int omap_pm_get_dev_context_loss_count(struct device *dev)
 {

@@ -298,7 +305,17 @@ int omap_pm_get_dev_context_loss_count(struct device *dev)
 		return -EINVAL;
 	};
 
-	count = omap_device_get_context_loss_count(pdev);
+	if (dev->parent == &omap_device_parent) {

arch/arm/plat-omap/omap-pm-noop.c int omap_pm_get_dev_context_loss_count(struct device *dev) · line 318

Quoted 10 lines
arch/arm/plat-omap/omap-pm-noop.c +10 -0
+		count = omap_device_get_context_loss_count(pdev);
+	} else {
+		WARN_ONCE(off_mode_enabled, "using dummy context loss counter, "
+			  "device %s should be converted to omap_device.",
+			  __func__, dev_name(dev));
+		if (off_mode_enabled)
+			dummy_context_loss_counter++;
+		count = dummy_context_loss_counter;
+	}
+

This part looks fine to me, thanks for working on this stuff. Once you fix the arithmetic rollover stuff, I think it will be good for 2.6.38.

- Paul

maintainer
#11 lore raw
In reply to Kevin Hilman

Hi Kevin,

working my way through these...

Quoted 24 lines · Kevin Hilman

Add new powerdomain API

int pwrdm_get_context_loss_count(struct powerdomain *pwrdm)

for checking how many times the powerdomain has lost context. The loss count is the sum sum of the powerdomain off-mode counter, the logic off counter and the per-bank memory off counter.

Cc Paul Walmsley <[email protected]>
Signed-off-by Kevin Hilman <[email protected]>
---
 arch/arm/mach-omap2/powerdomain.c |   23 +++++++++++++++++++++++
 arch/arm/mach-omap2/powerdomain.h |    1 +
 2 files changed, 24 insertions(+), 0 deletions(-)
arch/arm/mach-omap2/powerdomain.c +1 -0
index 06ef60e..78e7d22 100644
@@ -909,3 +909,26 @@ int pwrdm_post_transition(void)
 	pwrdm_for_each(_pwrdm_post_transition_cb, NULL);
 	return 0;
 }
+

arch/arm/mach-omap2/powerdomain.c int pwrdm_post_transition(void) · line 922

Quoted 10 lines
arch/arm/mach-omap2/powerdomain.c +10 -0
+/**
+ * pwrdm_get_context_loss_count - get powerdomain's context loss count
+ * @pwrdm: struct powerdomain * to wait for
+ *
+ * Context loss count is a sum of powerdomain off-mode counter,
+ * the logic off counter and the per-bank memory off counter.
+ */
+int pwrdm_get_context_loss_count(struct powerdomain *pwrdm)
+{
+	int i, count;

There should be a test here to prevent a null pointer deref if pwrdm is NULL.

arch/arm/mach-omap2/powerdomain.c int pwrdm_post_transition(void) · line 928

Quoted 6 lines
arch/arm/mach-omap2/powerdomain.c +6 -0
+
+	count = pwrdm->state_counter[PWRDM_POWER_OFF];
+	count += pwrdm->ret_logic_off_counter;
+
+	for (i = 0; i < pwrdm->banks; i++)
+		count += pwrdm->ret_mem_off_counter[i];

Looks like these state counters are unsigned ints. So they could easily overflow count when they are summed. This is probably not a major problem as far as this function is concerned, but the next patch can return -ENODEV upon error, which, if this function were really unlucky, it could effectively return -ENODEV.

It would be good to constrain the minimum successful return value of this function to 0. This will require some creativity since the function shouldn't just return 0 for all sums that wrap around and wind up negative; the return value should still continue to differ from the value when it was called previously. Otherwise devices might not restore their context when they should.

Quoted 11 lines
arch/arm/mach-omap2/powerdomain.c +6 -0
+
+	pr_debug("powerdomain: %s: context loss count = %u\n",
+		 pwrdm->name, count);
+
+	return count;
+}
arch/arm/mach-omap2/powerdomain.h +0 -0
index 35b5b48..d269eff 100644

arch/arm/mach-omap2/powerdomain.h int pwrdm_clkdm_state_switch(struct clockdomain *clkdm); · line 217

Quoted 7 lines
arch/arm/mach-omap2/powerdomain.h +1 -0
@@ -211,6 +211,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);
+int pwrdm_get_context_loss_count(struct powerdomain *pwrdm);
 
 extern void omap2xxx_powerdomains_init(void);
 extern void omap3xxx_powerdomains_init(void);

Also, as we discussed privately, there is at least one bug in the counters where the logic-off and membank-off counters don't increment correctly when the entire powerdomain is off. But that problem does not have to be dealt with as part of this series...

- Paul