Re: [PATCH v3 1/3] omap: opp: add OMAP3 OPP table data and common init

Nishanth Menon
2010-11-16T11:55:02Z

Thomas Petazzoni wrote, on 11/16/2010 05:21 AM:
> Hello,
>
> On Mon, 15 Nov 2010 13:27:39 -0600
> Nishanth Menon<nm@ti.com> wrote:
>
>> + /*
>> + * Allow multiple calls, but initialize only if not already initalized
>
> Minor: s/initalized/initialized/.
aah thanks :)

>
>> + * even if the previous call failed, coz, no reason we'd succeed again
>> + */
>> + if (omap_table_init)
>> + return 0;
>> + omap_table_init = 1;
>
> Do we really need this ? I personaly don't really like this quite of
> "Hey, I'm already initialized, let's do nothing silently then". Unless
> there are strong reasons for which this function could be called twice,
> I'd rather not have this, or turn this into a BUG_ON(omap_table_init ==
> 1).
Yes, it is needed. The intent here is different. See the documentation
that I put along with this patch - At times, board files may need to do
customization to opps - like enable 1GHz on that platform alone -> it
can do it *only if* the defaults are registered, following which it can
call opp_enable. when device_initcall follows this at a later point, it
is still valid.

btw, BUG_ON is a strict NO NO for me here - if I dont have OPP table, ok
fine, system can still survive without cpufreq, no need to stop system
operations because of that.

--
Regards,
Nishanth Menon