[08/13] OMAP2+: powerdomain: control power domains next state

Todd,

On Fri, Jul 29, 2011 at 9:59 AM, Todd Poynor <toddpoynor@google.com> wrote:
> On Thu, Jul 28, 2011 at 10:30:15AM +0200, jean.pihet@newoldbits.com 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:

  * 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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[08/13] OMAP2+: powerdomain: control power domains next state

Commit Message

Comments

Patch