Re: [PATCH 09/19] omap3+: sr: introduce class init,deinit and priv data

[Kevin Hilman <khilman@ti.com>]

Nishanth Menon <nm@ti.com> writes:

> Kevin Hilman wrote, on 03/03/2011 05:38 AM:
>> Nishanth Menon<nm@ti.com>  writes:
>>
>>> Certain class drivers such as class 1.5 drivers, will need specific
>>> notification that they have to be started up or stopped independent
>>> of smart reflex operation. They also may need private data to be
>>> used for operations of their own, provide the same.
>>>
>>> Signed-off-by: Nishanth Menon<nm@ti.com>
>>
>> Basic principle looks fine, but some naming comments below...
> k, thx.
>
>>
>> [...]
>>
>>> diff --git a/arch/arm/plat-omap/include/plat/smartreflex.h b/arch/arm/plat-omap/include/plat/smartreflex.h
>>> index 6568c88..8b6ecd9 100644
>>> --- a/arch/arm/plat-omap/include/plat/smartreflex.h
>>> +++ b/arch/arm/plat-omap/include/plat/smartreflex.h
>>> @@ -167,6 +167,8 @@ struct omap_sr_pmic_data {
>>>    *
>>>    * @enable:		API to enable a particular class smaartreflex.
>>>    * @disable:		API to disable a particular class smartreflex.
>>> + * @class_init:		API to do class specific initialization (optional)
>>> + * @class_deinit:	API to do class specific initialization (optional)
>>
>> The 'class_' prefix here is not needed.
> ack.
>
>>
>> The changelog uses 'start' and 'stop' instead of init&  deinit.  I
>> prefer those names to [de]init.
>
> Would'nt start and stop cause confusion when mixed with existing
> enable/disable? does enable/start actually start the SR? intent here
> with init/deinit is to do class specific initialization or
> deinitialization.

Well, one way or another, make the changelog consistent with the
function names.

To me though, start/stop has more meaning.  They are called from
sr_[start|stop]_vddautocomp, so using start/stop is consistent there.
Also, init is usually something done once (and doesn't have a de-init
counterpart) but here we're talking about something done for each 
sr_[start|stop]_vddautocomp()

Kevin