Kevin,
On Thu, Nov 17, 2011 at 10:16 PM, Kevin Hilman <khilman@xxxxxx> wrote:
> jean.pihet@xxxxxxxxxxxxxx writes:
>
>> From: Jean Pihet <j-pihet@xxxxxx>
>>
>> When a PM QoS device latency constraint is requested or removed the
>> PM QoS layer notifies the underlying layer with the updated aggregated
>> constraint value. The constraint is stored in the powerdomain constraints
>> list and then applied to the corresponding power domain.
>> The power domains get the next power state programmed directly in the
>> registers via pwrdm_wakeuplat_update_pwrst.
>>
>> Tested on OMAP3 Beagleboard and OMAP4 Pandaboard in RET/OFF using
>> wake-up latency constraints on MPU, CORE and PER.
>>
>> Signed-off-by: Jean Pihet <j-pihet@xxxxxx>
>> ---
...
>> @@ -104,6 +106,12 @@ static int _pwrdm_register(struct powerdomain *pwrdm)
>> for (i = 0; i < pwrdm->banks; i++)
>> pwrdm->ret_mem_off_counter[i] = 0;
>>
>> + /* Initialize the per-od wake-up constraints data */
>
> This comment needs an update (they are not per-od, but per pwrdm), or
> could probably be removed, since it doesn't add any value over the code.
Ok to remove it
>
>> + spin_lock_init(&pwrdm->wkup_lat_plist_lock);
>> + plist_head_init(&pwrdm->wkup_lat_plist_head);
>> + pwrdm->wkup_lat_next_state = PWRDM_POWER_OFF;
>> +
>> + /* Initialize the pwrdm state */
>> pwrdm_wait_transition(pwrdm);
>> pwrdm->state = pwrdm_read_pwrst(pwrdm);
>> pwrdm->state_counter[pwrdm->state] = 1;
...
>> +int pwrdm_set_wkup_lat_constraint(struct powerdomain *pwrdm, void *cookie,
>> + long min_latency)
>> +{
>> + struct pwrdm_wkup_constraints_entry *user = NULL, *new_user = NULL;
>> + int ret = 0, free_new_user = 0, free_node = 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",
>> + __func__, pwrdm->name, cookie, min_latency);
>> +
>> + if (min_latency != PM_QOS_DEV_LAT_DEFAULT_VALUE) {
>> + 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;
>> + }
>> + free_new_user = 1;
>> + }
>> +
>> + spin_lock_irqsave(&pwrdm->wkup_lat_plist_lock, flags);
>> +
>> + /* Manage the constraints list */
>> + ret = _pwrdm_update_wakeuplat_list(pwrdm, cookie, min_latency,
>> + user, new_user,
>> + &free_new_user, &free_node);
>> +
>> + /* Find the aggregated constraint value from the list */
>> + if (!ret)
>> + 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);
>
> The alloc/free of these buffers is not terribly obvious when reading. I
Agreed.
> think the code/changelog needs some comments describing the logic
> behind how/when these nodes are allocated and freed.
Ok I will add it.
...
>
> Kevin
>
Thanks,
Jean
_______________________________________________
linux-pm mailing list
linux-pm@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/linux-pm
