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

[Nishanth Menon <nm@ti.com>]

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