Re: [PATCH v3 1/5] pwms: pwm-ti*: Remove support for local clock gating

[Franklin S Cooper Jr.]

On 02/29/2016 05:20 PM, Tony Lindgren wrote:
> * Franklin S Cooper Jr. <fcooper@xxxxxx> [160229 15:12]:
>>
>> On 02/29/2016 04:55 PM, Tony Lindgren wrote:
>>> * Franklin S Cooper Jr. <fcooper@xxxxxx> [160229 14:31]:
>>>> On 02/29/2016 04:04 PM, Tony Lindgren wrote:
>>>>> Hmm but why are you also removing the pm_runtime calls? Those
>>>>> actually do take care of gating the clocks via the interconnect
>>>>> level code that is hwmod in this case.
>>>> I removed all PM runtime calls that revolved around
>>>> pwmss_submodule_state_change. Originally the driver would do
>>>> a pm_runtime_get_sync then call pwmss_submodule_state_change
>>>> and then immediately call pm_runtime_put_sync. Without
>>>> pwmss_submodule_state_change those calls would be
>>>> meaningless.  I also removed pm_runtime calls in error paths
>>>> that no longer existed.
>>> Typically the interconnect level code can gate the clkctrl bit
>>> for the module with PM runtime even with no other driver specific
>>> registers. If you remove the pm_runtime calls, that does not
>>> happen.
>> So the clocks should be unlocked when ever the IP registers are
>> being read/written or if the peripheral is being used for
>> example
>> the pwm signal is being generated. All these cases are already
>> being handled.
>>
>> Using ecap driver as an example.
>>
>> Pm_runtime_get_sync is called within ecap_pwm_enable when
>> the pwm signal is to be generated. Pm_runtime_put_sync is called
>> when the pwm signal is to be stopped.
>>
>> When either the pwm signal polarity is set or pwm
>> configuration is made
>> then a pm_runtime_get_sync and pm_runtime_put_sync are
>> called within
>> the same function surrounding calls to the IP's registers.
>>
>> Probe is calling pm_runtime_enable while remove is calling
>> pm_runtime_disable.
> OK good to hear you have considered this. The above answers
> my questions then thanks.
>
>> So the correct pm_runtime calls are being made from what I
>> can see.
>> I'm not sure I understand the concern since removing those
>> calls aren't
>> creating any kind of imbalance.
> OK thanks for checking.
>
>> If I'm not addressing your concern please give me an example
>> of where
>> you see a possible issue.
> No that's fine. I thought you're ripping out all of the the
> pm_runtime based on just looking at the patch :)
>
>>> Also, how do you know this change does not affect the other
>>> SoC variants using the same driver?
>> I've tested these changes on AM335x GP and AM437x GP evms.
>> AM335x
>> and AM437x were the only other users of this driver. Sorry 
>> I should of
>> documented this in my cover-letter.
> OK good to hear.
>
> Thanks,
>
> Tony

I know there are some comments regarding other patches in
this patchset but this patch is unrelated and can be pulled
in separately. Any objections to this or should I just
resubmit this patch by itself?