Messages in this thread |  | | Date | Fri, 07 Jun 2013 16:34:26 +0200 | From | Lukasz Majewski <> | Subject | Re: [PATCH 2/5] cpufreq:boost: Add support for software based CPU frequency boost |
| |
Hi Viresh,
> Hi Lukasz, > > On 7 June 2013 18:57, Lukasz Majewski <l.majewski@samsung.com> wrote: > > I hope you agreed to all the other comments I gave as I don't see an > explicit reply below each of these. I have seen people missing these > in past, so what would be better to do is: > - either reply below each one of them and say yes or no.. > - Or write once below many comments and say: All above comments > are accepted. > > So, that Reviewer is assured that you haven't missed anything. >
Thanks for reminding :-).
I've read through all the comments. I'm redesigning now the driver to remove redundant code.
> > I would prefer to have following fields in the cpufreq_boost > > structure: struct cpufreq_boost { > > unsigned int max_boost_freq; /*boost max freq*/ > > unsigned int max_normal_freq; /*max normal freq > > int (*low_level_boost) (int state); > > bool boost_en; /* indicate if boost is enabled */ > > } > > > > The max_{boost|normal}_freq fields will be filed at > > ret = cpufreq_driver->init(policy); > > > > Thanks to them I will avoid calling many times routine, which > > extracts from freq_table maximal boost and normal frequencies. > > > > I could define those variables in the exynos-cpufreq.c driver, but I > > think, that they are more suitable to be embedded at cpufreq_boost > > structure. > > I understand that you need these variables (I will still look how you > are using them in next version). But they are per policy and driver > isn't responsible for maintaining them. If they are required then > cpufreq core must find them out and keep in struct cpufreq_policy (as > they are policy dependent).. > > So, remove this structure from cpufreq_driver and embed variables > directly.
After refactoring the code. I admit, that we shall embed the struct cpu_boost fields directly to the coufreq_driver. There is no point to create structure with 2 fields.
-- Best regards,
Lukasz Majewski
Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
|  |