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

From: Franklin S Cooper Jr.
Date: Mon Feb 29 2016 - 18:12:25 EST




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.

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.

If I'm not addressing your concern please give me an example
of where
you see a possible issue.

> 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.
> Regards,
>
> Tony