Nishanth Menon <nm@xxxxxx> writes:
> Kevin Hilman wrote, on 03/03/2011 05:38 AM:
>> Nishanth Menon<nm@xxxxxx> 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@xxxxxx>
>>
>> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
