[PATCH 1/4] OMAP: introduce OPP layer for device-specific OPPs

[roger.quadros@nokia.com (Roger Quadros)]

On 09/16/2010 05:01 PM, ext Nishanth Menon wrote:
> Roger Quadros had written, on 09/16/2010 08:54 AM, the following:
>> Since you are anyways re-sending this you might as well fix these nits.
> thanks for reviewing..
>
>>
> [..]
>
>>> diff --git a/arch/arm/plat-omap/opp.c b/arch/arm/plat-omap/opp.c
>>> new file mode 100644
>>> index 0000000..17f93b2
>>> --- /dev/null
>>> +++ b/arch/arm/plat-omap/opp.c
>>> @@ -0,0 +1,461 @@
>>> +/*
>>> + * OMAP OPP Interface
>>> + *
>>> + * Copyright (C) 2009-2010 Texas Instruments Incorporated.
>>> + *     Nishanth Menon
>>> + *     Romit Dasgupta<romit@ti.com>
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify
>>> + * it under the terms of the GNU General Public License version 2 as
>>> + * published by the Free Software Foundation.
>>> + */
>>> +
>>
>>> +/**
>>> + * opp_get_voltage() - Gets the voltage corresponding to an opp
>>> + * @opp:       opp for which voltage has to be returned for
>>> + *
>>> + * Return voltage in micro volt corresponding to the opp, else
>>> + * return 0
>>> + */
>>> +unsigned long opp_get_voltage(const struct omap_opp *opp)
>>> +{
>>> +       if (unlikely(!opp || IS_ERR(opp)) || !opp->enabled) {
>>
>> If !opp->enabled, then is it really an invalid parameter being passed?
>>
>> I'm not user if people need to know the voltage of a disabled OPP, but
>> do we need to limit it?
>>
>>> +               pr_err("%s: Invalid parameters being passed\n", __func__);
>>> +               return 0;
>>> +       }
>>> +
>>> +       return opp->u_volt;
>>> +}
>>> +
>>> +/**
>>> + * opp_get_freq() - Gets the frequency corresponding to an opp
>>> + * @opp:       opp for which frequency has to be returned for
>>> + *
>>> + * Return frequency in hertz corresponding to the opp, else
>>> + * return 0
>>> + */
>>> +unsigned long opp_get_freq(const struct omap_opp *opp)
>>> +{
>>> +       if (unlikely(!opp || IS_ERR(opp)) || !opp->enabled) {
>>
>> ditto.
>
> Yes, the intent here was for opp operational apis to function ONLY on
> the enabled opps - this helps to pull out any bugs in the users
> who might be unintentionally using bad params.
>
OK.

> do you have any usecase you can think of where we might want to use
> these on a disabled opp?

Not really. Based on what is an OPP enabled/disabled? Is it possible for 
an initially enabled OPP to be disabled at some point in time? What 
triggers this disable?
OR  does an OPP enabled at boot time remain enabled throughout the power 
session?

regards,
-roger