Re: [Powertop] [PATCH] devfreq: add devfreq devices stats support

[Sanjay Singh Rawat <sanjay.rawat at linaro.org>]

[-- Attachment #1: Type: text/plain, Size: 1876 bytes --]

thanks for the review Sergey

On Wednesday 01 October 2014 08:42 PM, Sergey Senozhatsky wrote:
> On (09/30/14 12:04), Sanjay Singh Rawat wrote:
>> add window to show frequency stats for devfreq devices
>>
>> Signed-off-by: Rajagopal Venkat <rajagopal.venkat(a)gmail.com>
>> Signed-off-by: Sanjay Singh Rawat <sanjay.rawat(a)linaro.org>
>> ---
>>
>> v2 - Show devfreq window on support basis. Check for empty devfreq
>>       directory.
>>     - Free the open dirp while exiting.
>> ---
>>   src/Makefile.am           |   1 +
>>   src/devices/devfreq.cpp   | 367 ++++++++++++++++++++++++++++++++++++++++++++++
>>   src/devices/devfreq.h     |  75 ++++++++++
>>   src/main.cpp              |   9 ++
[...]
>> +	file.close();
>> +}
>> +
>> +void devfreq::start_measurement(void)
>> +{
>> +	unsigned int i;
>> +	ifstream file;
> not needed.

ack
>
[...]
>> +		is_enabled = false;
>> +		closedir(dir);
>> +		return;
>> +	}
> who close the dir if `num != 2'?

the dir is closed in the clear_all_devfreq() function while exiting
>
>> +	callback fn = &devfreq_dev_callback;
>> +	process_directory(p.c_str(), fn);
>> +}
>> +
>> +void initialize_devfreq(void)
[...]
>> +
>> +void clear_all_devfreq()
>> +{
>> +	unsigned int i, j;
>> +
>> +	for (i=0; i < all_devfreq.size(); i++) {
>> +		class devfreq *df = all_devfreq[i];
>> +
>> +		for(j=0; j < df->dstates.size(); j++)
>> +			delete df->dstates[j];
>> +
>> +		df->dstates.resize(0);
> resize not needed, I think

ack
>
>> +		delete df;
>> +	}
>> +	all_devfreq.clear();
>> +	/* close /sys/class/devfreq */
[...]
>> +
>> +class devfreq: public device {
>> +	char dir_name[4096];
> hm. so this is 2 pages for each class instance.
> can you

will resize to 128 bytes
> 	const char *dir_name;
> 	
> and then
> 	dir_name = strdup(dname); ?
>
>

-- 
sanjay


[-- Attachment #2: attachment.html --]
[-- Type: text/html, Size: 3818 bytes --]