Re: [PATCH 1/6] OMAP2+: powerdomain: control power domains next state

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Linux ACPI]     [Netdev]     [Ethernet Bridging]     [Linux Wireless]     [CPU Freq]     [Kernel Newbies]     [Fedora Kernel]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]

  Powered by Linux