Re: [RFC: PATCH] OMAP: hwmod: New API to modify the autoidle bits of sysconfig register

[kishon <a0393678@ti.com>]

On Tuesday 31 August 2010 09:46 PM, Cousson, Benoit wrote:
> On 8/31/2010 4:41 PM, ABRAHAM, KISHON VIJAY wrote:
>    
>>
>> On Tuesday 31 August 2010 01:43 PM, Felipe Balbi wrote:
>>      
>>> On Tue, 31 Aug 2010 10:53:36 +0530, kishon<a0393678@ti.com>    wrote:
>>>
>>>        
>>>>         Though driver shouldn't be using hwmod directly, there is no
>>>> corresponding API in omap_device to do the same. So we are planning to
>>>>         store the omap_hwmod structure in platform_data during
>>>> arch_initcall (in the callback to omap_hwmod_for_each_by_class). So
>>>> whenever the
>>>>         AUTOIDLE bits need to be reset or set, we pass the stored
>>>> omap_hwmod structure to this API. Currently, the functions that needs
>>>> AUTOIDLE
>>>>         bit to be modified (omap_st_on, omap_st_off) resides in plat-omap.
>>>>
>>>>          
>>> couldn't your API instead be something like:
>>>
>>> int omap_hwmod_set_autoidle(struct device *dev, u8 autoidle)
>>> {
>>> 	struct omap_hwmod *oh = dev_to_hwmod(dev);
>>>
>>> 	if (!oh)
>>> 		return -ENODEV;
>>> 	[...]
>>>
>>> 	return 0;
>>> }
>>>
>>>        
>>        Balbi,
>>            I couldn't find "dev_to_hwmod()" function. Do you actually mean
>> implementing dev_to_hwmod() function?
>>        I created this API based on "omap_hwmod_set_slave_idlemode()"
>> present in lo (omap_hwmod.c) for changing the smart idle bit
>>        of SYSCONFIG register.
>>
>>        -Kishon
>>      
> That API does not exist. And since a device can contains several hwmods,
> like in your case, this API cannot exist in that form.
>
> The real issue is that we do not want to expose any API relative to PRCM
> management to the driver. Even if you hide that behind some pdata, it is
> still the driver that will call it.
>
> I know that some IPs like MUSB or McBSP requires some unusual idle mode
> change due to HW bugs, but in general, you should try to find a better
> way to handle that in the core framework if possible.
>
>    
     Benoit,

     I see the trend of using specific flags in hwmod which gets set
     in pm_runtime_get_sync() for scenarios like this where auto idle
     bits or smart idle bits need to be set/reset at runtime. The problem
     in the case of sidetone in MCBSP is that we don't call get_sync or
     put_sync for sidetone separately.
     There is only two options we can think of for sidetone
         1) Always disable autoidle for sidetone (I'm not sure if this will
     have some power implications).
         2) Use of the above API.

     Would be helpful if you can also suggest us some other options as well.

     -Kishon
> Regards,
> Benoit
>