Re: [PATCH 4/4] PM/Devfreq: Add Exynos5-bus devfreq driver for Exynos5250

[Abhilash Kesavan]

Hi Rajagopal,

Thanks for the review. Sorry for the late response, have been a little
busy with other things,

[...]
>> +#include "../governor.h"
>
> This header file is meant for governors use. What's the need of it here?
I was using a custom monitor function and needed update_devfreq for
it. Will change this in the next version.
[...]
>> +       stat->current_frequency = data->curr_freq;
>> +       stat->busy_time = data->busy;
>> +       stat->total_time = 100;
>
> How is busy_time is relative to total_time here? busy_time <=
> total_time is guaranteed?
Will modify the get_dev_status function to do PPMU reads and populate
the device status structure.
>
[...]
> Again, update_devfreq() is meant for devfreq governors use. Why is the devfreq
> driver is doing devfreq governor job? any specific reason? The devfreq device
> load monitoring is done by governors.
>
Same as above.
[...]
>> +       .polling_ms             = 0,
>
> why is polling_ms is set to zero? It defeats the purpose of devfreq driver.
I bypassed the devfreq_monitor with the new custom function created. Am fixing
this.

I have now done this on the lines of the exynos4 driver. Will re-post in a bit.

Regards,
Abhilash
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/