On Fri, Feb 7, 2014 at 11:19 PM, Thomas Abraham <ta.omasab@xxxxxxxxx> wrote:
>>> --- a/drivers/cpufreq/cpufreq-cpu0.c
>>> +++ b/drivers/cpufreq/cpufreq-cpu0.c
>>> @@ -195,6 +195,9 @@ static int cpu0_cpufreq_probe(struct platform_device *pdev)
>>> transition_latency += ret * 1000;
>>> }
>>>
>>> + if (of_find_property(cpu_dev->of_node, "boost-frequency", NULL))
>>> + cpu0_cpufreq_driver.boost_supported = true;
>>> +
>> might as well hide that under the opp api which returns a bool that
>> says if boost is supported or not. that allows the parse to be
>> isolated to a single location.
>
> This is specific to the cpufreq-cpu0 driver. Other cpufreq drivers
> might want to use other methods and additional constraints to
> determine if boost has to be enabled.
struct cpufreq_driver::boost_supported is nothing CPU0 specifc - i am
just saying that the same property "boost-frequency" parsed by in two
places (dev_pm_opp_init_cpufreq_table and cpu0_cpufreq_probe) is a
very bad idea.
you could instead make
int dev_pm_opp_init_cpufreq_table(struct device *dev,
struct cpufreq_frequency_table **table)
into:
int dev_pm_opp_init_cpufreq_table(struct device *dev,
struct cpufreq_frequency_table **table,
bool *boost_supported);
Then use boost_supported for anything of your driver's choice
(including updating cpufreq_driver::boost_supported).
That allows the property to be isolated into a single function, who
can be modified without impacting other drivers at a later point in
time.
Regards,
Nishanth Menon
--
To unsubscribe from this list: send the line "unsubscribe cpufreq" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
