Re: [PM-WIP-OPP][PATCH] OPP: Introduces enum for addressing different OPP types

Kevin Hilman
2010-01-13T14:43:52Z

Romit Dasgupta <romit@ti.com> writes:

> Menon, Nishanth wrote:
>>
[...]

>>> -static int __deprecated freq_to_opp(u8 *opp_id, struct omap_opp *opps,
>>> +static int __deprecated freq_to_opp(u8 *opp_id, enum opp_t opp_t,
>> Re: enum type and variable have the same name :( mebbe a rename of variable is
>> appropriate
>>> unsigned long freq)
>>> {
>>> struct omap_opp *opp;
>>>
>>> - BUG_ON(!opp_id || !opps);
>>> - opp = opp_find_freq_ceil(opps, &freq);
>>> + BUG_ON(opp_t == OPP_NONE || opp_t > OPP_TYPES);
>>> + opp = opp_find_freq_ceil(opp_t, &freq);
>>> if (IS_ERR(opp))
>>> return -EINVAL;
>>> *opp_id = opp_get_opp_id(opp);
>>> @@ -218,9 +218,6 @@ void init_opp(struct shared_resource *resp)
>>> u8 opp_id;
>>> resp->no_of_users = 0;
>>>
>>> - if (!mpu_opps || !dsp_opps || !l3_opps)
>>> - return;
>>> -
>> the original intent of this check is lost here - if the initializations did not
>> take place, we will not proceed. An equivalent check might be good to maintain
>> at this point.
>
> You are partially correct. I took off the checks because we have a BUG_ON() call
> in the beginning of the boot code right after we initialize the OPP tables. So
> we should not hit this check.

Please do not code things based on the the existence of BUG_ON().

All of these BUG* points need to be removed as we none of these are fatal. We need
better error recovery, most likely using WARN* and returning error codes.

Kevin