On Tue, Aug 23, 2011 at 3:19 AM, Turquette, Mike <mturquette@xxxxxx> wrote:
> On Fri, Aug 19, 2011 at 1:30 AM, MyungJoo Ham <myungjoo.ham@xxxxxxxxxxx> wrote:
>> +
>> + if (jiffies_passed == 0)
>> + jiffies_passed = 1;
>> + if (jiffies_passed < 0) /* "Infinite Timeout" */
>> + jiffies_passed = INT_MAX;
>
> This doesn't account for jiffies rollover (~49 days on architectures
> with HZ == 1000). At rollover-time some devices might incorrectly get
> marked as having an infinite timeout.
Yes, that is to be removed and jiffies_passed appears to be better
being unsigned long, not int.
>
>> +
>> + mutex_lock(&devfreq_list_lock);
>> +
>> + list_for_each_entry_safe(devfreq, tmp, &devfreq_list, node) {
>> + /* Reflect the changes in profile->polling_ms */
>> + if (devfreq->polling_ms != devfreq->profile->polling_ms) {
>> + devfreq->polling_ms = devfreq->profile->polling_ms;
>> + devfreq->polling_jiffies = msecs_to_jiffies(
>> + devfreq->polling_ms);
>
> Does struct devfreq need ->polling_ms? It seems like useless storage
> since we're really interested in ->polling_jiffies.
>
> How about removing devfreq->polling_ms and then just doing the
> conversion whenever the sysfs file is written to?
It we assume that profile->polling_ms is never changed by either the
driver or the governor, we can remove it in devfreq.
It was added to struct devfreq in order to see if the value has been
changed and get polling_jiffies updated accordingly.
Although there is no "store" function for polling_ms in sysfs, the
drivers and governors are able to update the value at any time.
>
> Regards,
> Mike
>
I'll send "v7" patch with some minor updates soon. It appears that
devfreq.polling_ms would be better unsigned along with jiffies_passed.
Thanks
MyungJoo
--
MyungJoo Ham (함명주), Ph.D.
Mobile Software Platform Lab,
Digital Media and Communications (DMC) Business
Samsung Electronics
cell: 82-10-6714-2858
_______________________________________________
linux-pm mailing list
linux-pm@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linux-foundation.org/mailman/listinfo/linux-pm
