Todd,
On Fri, Jul 29, 2011 at 9:59 AM, Todd Poynor <toddpoynor@xxxxxxxxxx> wrote:
> On Thu, Jul 28, 2011 at 10:30:15AM +0200, jean.pihet@xxxxxxxxxxxxxx wrote:
> ...
>> +int pwrdm_set_wkup_lat_constraint(struct powerdomain *pwrdm, void *cookie,
>> + long min_latency)
>> +{
>> + struct pwrdm_wkup_constraints_entry *user = NULL;
>> + struct pwrdm_wkup_constraints_entry *tmp_user, *new_user;
>> + int ret = 0, free_new_user = 0, free_node = 0;
>> + long value = 0;
>> + unsigned long flags;
>> +
>> + pr_debug("powerdomain: %s: pwrdm %s, cookie=0x%p, min_latency=%ld\n",
>> + __func__, pwrdm->name, cookie, min_latency);
>> +
>> + new_user = kzalloc(sizeof(struct pwrdm_wkup_constraints_entry),
>> + GFP_KERNEL);
>> + if (!new_user) {
>> + pr_err("%s: FATAL ERROR: kzalloc failed\n", __func__);
>> + return -ENOMEM;
>> + }
>> +
>> + spin_lock_irqsave(&pwrdm->wkup_lat_plist_lock, flags);
>> +
>> + /* Check if there already is a constraint for cookie */
>> + plist_for_each_entry(tmp_user, &pwrdm->wkup_lat_plist_head, node) {
>> + if (tmp_user->cookie == cookie) {
>> + user = tmp_user;
>> + free_new_user = 1;
>> + break;
>> + }
>> + }
>> +
>> + if (min_latency != PM_QOS_DEV_LAT_DEFAULT_VALUE) {
>> + /* If nothing to update, job done */
>> + if (user && (user->node.prio == min_latency))
>> + goto exit_ok;
>> +
>> + if (!user) {
>> + /* Add new entry to the list */
>> + user = new_user;
>> + user->cookie = cookie;
>> + } else {
>> + /* Update existing entry */
>> + plist_del(&user->node, &pwrdm->wkup_lat_plist_head);
>> + }
>> +
>> + plist_node_init(&user->node, min_latency);
>> + plist_add(&user->node, &pwrdm->wkup_lat_plist_head);
>> + } else {
>> + /* Remove the constraint from the list */
>> + if (!user) {
>> + pr_err("%s: Error: no prior constraint to release\n",
>> + __func__);
>> + ret = -EINVAL;
>> + goto exit_error;
>> + }
>> +
>> + plist_del(&user->node, &pwrdm->wkup_lat_plist_head);
>> + free_node = 1;
>
> All min_latency != PM_QOS_DEV_LAT_DEFAULT_VALUE paths need
> free_new_user = 1.
free_new_user = 1 is only needed if no existing constraint has been
found, i.e. user stays at NULL. This is implemented in the check for
an existing constraint (plist_for_each_entry(...)).
>(Or maybe change the logic to check user !=
> new_user and free new_user if so.)
That could be done as well.
>
>> + }
>> +
>> +exit_ok:
>> + /* Find the strongest constraint from the list */
>> + if (!plist_head_empty(&pwrdm->wkup_lat_plist_head))
>> + value = plist_first(&pwrdm->wkup_lat_plist_head)->prio;
>> +
>> + spin_unlock_irqrestore(&pwrdm->wkup_lat_plist_lock, flags);
>> +
>> + if (free_node)
>> + kfree(user);
>> +
>> + if (free_new_user)
>> + kfree(new_user);
>> +
>> + /* Apply the constraint to the pwrdm */
>> + pr_debug("powerdomain: %s: pwrdm %s, value=%ld\n",
>> + __func__, pwrdm->name, value);
>> + pwrdm_wakeuplat_update_pwrst(pwrdm, value);
>> +
>> + return 0;
>> +
>> +exit_error:
>> + spin_unlock_irqrestore(&pwrdm->wkup_lat_plist_lock, flags);
>
> Need:
> kfree(new_user);
Correct!
BTW I have a new version (patch here below) that improves the cases
with latency = PM_QOS_DEV_LAT_DEFAULT_VALUE. This will come in v4
after testing.
>
>> + return ret;
>> +}
>
>
> Todd
>
Thanks for reviewing!
Regards,
Jean
---
Patch for cases with latency = PM_QOS_DEV_LAT_DEFAULT_VALUE:
diff --git a/arch/arm/mach-omap2/powerdomain.c
b/arch/arm/mach-omap2/powerdomain.c
index c0f48ab..2e9379b 100644
--- a/arch/arm/mach-omap2/powerdomain.c
+++ b/arch/arm/mach-omap2/powerdomain.c
@@ -206,7 +206,7 @@ static int _pwrdm_post_transition_cb(struct
powerdomain *pwrdm, void *unused)
* pwrdm_wakeuplat_update_pwrst - Update power domain power state if needed
* @pwrdm: struct powerdomain * to which requesting device belongs to.
* @min_latency: the allowed wake-up latency for the given power domain. A
- * value of 0 means 'no constraint' on the pwrdm.
+ * value of PM_QOS_DEV_LAT_DEFAULT_VALUE means 'no constraint' on the pwrdm.
*
* Finds the power domain next power state that fulfills the constraint.
* Programs a new target state if it is different from current power state.
@@ -232,7 +232,7 @@ static int pwrdm_wakeuplat_update_pwrst(struct
powerdomain *pwrdm,
/* Find power state with wakeup latency < minimum constraint */
for (new_state = 0x0; new_state < PWRDM_MAX_PWRSTS; new_state++) {
- if (min_latency == 0 ||
+ if (min_latency == PM_QOS_DEV_LAT_DEFAULT_VALUE ||
pwrdm->wakeup_lat[new_state] <= min_latency)
break;
}
@@ -1018,8 +1018,8 @@ int pwrdm_post_transition(void)
* @pwrdm: struct powerdomain * which the constraint applies to
* @cookie: constraint identifier, used for tracking.
* @min_latency: minimum wakeup latency constraint (in microseconds) for
- * the given pwrdm. The value of PM_QOS_DEV_WAKEUP_LAT_DEFAULT_VALUE
- * removes the constraint.
+ * the given pwrdm. The value of PM_QOS_DEV_LAT_DEFAULT_VALUE removes
+ * the constraint.
*
* Tracks the constraints by @cookie.
* Constraint set/update: Adds a new entry to powerdomain's wake-up latency
@@ -1042,7 +1042,7 @@ int pwrdm_set_wkup_lat_constraint(struct
powerdomain *pwrdm, void *cookie,
struct pwrdm_wkup_constraints_entry *user = NULL;
struct pwrdm_wkup_constraints_entry *tmp_user, *new_user;
int ret = 0, free_new_user = 0, free_node = 0;
- long value = 0;
+ long value = PM_QOS_DEV_LAT_DEFAULT_VALUE;
unsigned long flags;
pr_debug("powerdomain: %s: pwrdm %s, cookie=0x%p, min_latency=%ld\n",
@@ -1083,16 +1083,17 @@ int pwrdm_set_wkup_lat_constraint(struct
powerdomain *pwrdm, void *cookie,
plist_node_init(&user->node, min_latency);
plist_add(&user->node, &pwrdm->wkup_lat_plist_head);
} else {
- /* Remove the constraint from the list */
- if (!user) {
- pr_err("%s: Error: no prior constraint to release\n",
- __func__);
+ if (user) {
+ /* Remove the constraint from the list */
+ plist_del(&user->node, &pwrdm->wkup_lat_plist_head);
+ free_node = 1;
+ } else {
+ /* Constraint not existing or list empty, do nothing */
+ free_new_user = 1;
ret = -EINVAL;
goto exit_error;
}
- plist_del(&user->node, &pwrdm->wkup_lat_plist_head);
- free_node = 1;
}
exit_ok:
@@ -1117,6 +1118,10 @@ exit_ok:
exit_error:
spin_unlock_irqrestore(&pwrdm->wkup_lat_plist_lock, flags);
+
+ if (free_new_user)
+ kfree(new_user);
+
return ret;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
