From: "Franklin S Cooper Jr." <fcooper@ti.com>
To: Tony Lindgren <tony@atomide.com>
Cc: <paul@pwsan.com>, <t-kristo@ti.com>, <vigneshr@ti.com>,
<linux-pwm@vger.kernel.org>, <devicetree@vger.kernel.org>,
<linux-kernel@vger.kernel.org>, <linux-omap@vger.kernel.org>,
<linux-arm-kernel@lists.infradead.org>,
<linux-clk@vger.kernel.org>,
Thierry Reding <thierry.reding@gmail.com>
Subject: Re: [PATCH v3 1/5] pwms: pwm-ti*: Remove support for local clock gating
Date: Wed, 2 Mar 2016 13:41:26 -0600 [thread overview]
Message-ID: <56D741E6.3000708@ti.com> (raw)
In-Reply-To: <20160229232011.GU13417@atomide.com>
On 02/29/2016 05:20 PM, Tony Lindgren wrote:
> * Franklin S Cooper Jr. <fcooper@ti.com> [160229 15:12]:
>>
>> On 02/29/2016 04:55 PM, Tony Lindgren wrote:
>>> * Franklin S Cooper Jr. <fcooper@ti.com> [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?
WARNING: multiple messages have this Message-ID (diff)
From: "Franklin S Cooper Jr." <fcooper@ti.com>
To: Tony Lindgren <tony@atomide.com>
Cc: paul@pwsan.com, t-kristo@ti.com, vigneshr@ti.com,
linux-pwm@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-omap@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, linux-clk@vger.kernel.org,
Thierry Reding <thierry.reding@gmail.com>
Subject: Re: [PATCH v3 1/5] pwms: pwm-ti*: Remove support for local clock gating
Date: Wed, 2 Mar 2016 13:41:26 -0600 [thread overview]
Message-ID: <56D741E6.3000708@ti.com> (raw)
In-Reply-To: <20160229232011.GU13417@atomide.com>
On 02/29/2016 05:20 PM, Tony Lindgren wrote:
> * Franklin S Cooper Jr. <fcooper@ti.com> [160229 15:12]:
>>
>> On 02/29/2016 04:55 PM, Tony Lindgren wrote:
>>> * Franklin S Cooper Jr. <fcooper@ti.com> [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?
WARNING: multiple messages have this Message-ID (diff)
From: fcooper@ti.com (Franklin S Cooper Jr.)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v3 1/5] pwms: pwm-ti*: Remove support for local clock gating
Date: Wed, 2 Mar 2016 13:41:26 -0600 [thread overview]
Message-ID: <56D741E6.3000708@ti.com> (raw)
In-Reply-To: <20160229232011.GU13417@atomide.com>
On 02/29/2016 05:20 PM, Tony Lindgren wrote:
> * Franklin S Cooper Jr. <fcooper@ti.com> [160229 15:12]:
>>
>> On 02/29/2016 04:55 PM, Tony Lindgren wrote:
>>> * Franklin S Cooper Jr. <fcooper@ti.com> [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?
next prev parent reply other threads:[~2016-03-02 19:41 UTC|newest]
Thread overview: 86+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-02-25 22:36 [PATCH v3 0/5] Add support for PWMSS on DRA7 Franklin S Cooper Jr
2016-02-25 22:36 ` Franklin S Cooper Jr
2016-02-25 22:36 ` Franklin S Cooper Jr
2016-02-25 22:36 ` [PATCH v3 1/5] pwms: pwm-ti*: Remove support for local clock gating Franklin S Cooper Jr
2016-02-25 22:36 ` Franklin S Cooper Jr
2016-02-26 10:27 ` Sekhar Nori
2016-02-26 10:27 ` Sekhar Nori
2016-02-26 10:27 ` Sekhar Nori
2016-02-26 19:14 ` Tony Lindgren
2016-02-26 19:14 ` Tony Lindgren
2016-02-29 22:04 ` Tony Lindgren
2016-02-29 22:04 ` Tony Lindgren
2016-02-29 22:04 ` Tony Lindgren
2016-02-29 22:30 ` Franklin S Cooper Jr.
2016-02-29 22:30 ` Franklin S Cooper Jr.
2016-02-29 22:30 ` Franklin S Cooper Jr.
2016-02-29 22:55 ` Tony Lindgren
2016-02-29 22:55 ` Tony Lindgren
2016-02-29 23:11 ` Franklin S Cooper Jr.
2016-02-29 23:11 ` Franklin S Cooper Jr.
2016-02-29 23:11 ` Franklin S Cooper Jr.
2016-02-29 23:20 ` Tony Lindgren
2016-02-29 23:20 ` Tony Lindgren
2016-03-02 19:41 ` Franklin S Cooper Jr. [this message]
2016-03-02 19:41 ` Franklin S Cooper Jr.
2016-03-02 19:41 ` Franklin S Cooper Jr.
2016-03-02 22:54 ` Tony Lindgren
2016-03-02 22:54 ` Tony Lindgren
2016-02-25 22:36 ` [PATCH v3 2/5] ARM: OMAP2+: DRA7: Add hwmod entries for PWMSS Franklin S Cooper Jr
2016-02-25 22:36 ` Franklin S Cooper Jr
2016-03-01 18:07 ` Paul Walmsley
2016-03-01 18:07 ` Paul Walmsley
2016-03-01 18:07 ` Paul Walmsley
2016-03-01 18:11 ` Tony Lindgren
2016-03-01 18:11 ` Tony Lindgren
2016-03-01 18:59 ` Paul Walmsley
2016-03-01 18:59 ` Paul Walmsley
2016-03-01 18:59 ` Paul Walmsley
2016-03-01 20:50 ` Tony Lindgren
2016-03-01 20:50 ` Tony Lindgren
2016-03-02 16:22 ` Franklin S Cooper Jr.
2016-03-02 16:22 ` Franklin S Cooper Jr.
2016-03-02 16:22 ` Franklin S Cooper Jr.
2016-03-04 2:07 ` Franklin S Cooper Jr.
2016-03-04 2:07 ` Franklin S Cooper Jr.
2016-03-04 2:07 ` Franklin S Cooper Jr.
2016-03-04 6:25 ` Paul Walmsley
2016-03-04 6:25 ` Paul Walmsley
2016-03-04 12:23 ` Franklin S Cooper Jr.
2016-03-04 12:23 ` Franklin S Cooper Jr.
2016-03-04 12:23 ` Franklin S Cooper Jr.
2016-03-04 16:32 ` Paul Walmsley
2016-03-04 16:32 ` Paul Walmsley
2016-02-25 22:36 ` [PATCH v3 3/5] ARM: dts: DRA7: Add TBCLK " Franklin S Cooper Jr
2016-02-25 22:36 ` Franklin S Cooper Jr
2016-02-29 23:23 ` Tony Lindgren
2016-02-29 23:23 ` Tony Lindgren
2016-03-01 12:54 ` Tero Kristo
2016-03-01 12:54 ` Tero Kristo
2016-03-01 12:54 ` Tero Kristo
2016-03-01 18:05 ` Tony Lindgren
2016-03-01 18:05 ` Tony Lindgren
2016-02-25 22:36 ` [PATCH v3 4/5] clk: ti: DRA7: Add tbclk data for ehrpwm Franklin S Cooper Jr
2016-02-25 22:36 ` Franklin S Cooper Jr
2016-02-26 19:16 ` Tony Lindgren
2016-02-26 19:16 ` Tony Lindgren
2016-02-26 19:17 ` Tony Lindgren
2016-02-26 19:17 ` Tony Lindgren
2016-02-25 22:36 ` [PATCH v3 5/5] ARM: dts: DRA7: Add dt nodes for PWMSS Franklin S Cooper Jr
2016-02-25 22:36 ` Franklin S Cooper Jr
2016-02-26 19:18 ` Tony Lindgren
2016-02-26 19:18 ` Tony Lindgren
2016-02-26 19:43 ` Franklin S Cooper Jr
2016-02-26 19:43 ` Franklin S Cooper Jr
2016-02-26 19:43 ` Franklin S Cooper Jr
2016-02-29 23:24 ` Tony Lindgren
2016-02-29 23:24 ` Tony Lindgren
2016-03-01 21:00 ` Tony Lindgren
2016-03-01 21:00 ` Tony Lindgren
2016-03-02 18:26 ` Rob Herring
2016-03-02 18:26 ` Rob Herring
2016-03-04 1:39 ` Franklin S Cooper Jr.
2016-03-04 1:39 ` Franklin S Cooper Jr.
2016-03-04 1:39 ` Franklin S Cooper Jr.
2016-03-04 14:52 ` Rob Herring
2016-03-04 14:52 ` Rob Herring
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=56D741E6.3000708@ti.com \
--to=fcooper@ti.com \
--cc=devicetree@vger.kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-clk@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-omap@vger.kernel.org \
--cc=linux-pwm@vger.kernel.org \
--cc=paul@pwsan.com \
--cc=t-kristo@ti.com \
--cc=thierry.reding@gmail.com \
--cc=tony@atomide.com \
--cc=vigneshr@ti.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.