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

[Kevin Hilman <khilman@deeprootsystems.com>]

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