* [PATCH 05/11] OMAPDSS: add clk_prepare and clk_unprepare
[not found] <1340372890-10091-1-git-send-email-rnayak@ti.com>
@ 2012-06-22 13:51 ` Rajendra Nayak
2012-06-25 6:07 ` Tomi Valkeinen
0 siblings, 1 reply; 13+ messages in thread
From: Rajendra Nayak @ 2012-06-22 13:51 UTC (permalink / raw)
To: linux-arm-kernel
In preparation of OMAP moving to Common Clk Framework(CCF) add clk_prepare()
and clk_unprepare() for the omapdss clocks.
Signed-off-by: Rajendra Nayak <rnayak@ti.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: <linux-fbdev@vger.kernel.org>
---
drivers/video/omap2/dss/dsi.c | 5 ++++-
drivers/video/omap2/dss/hdmi.c | 5 ++++-
drivers/video/omap2/dss/venc.c | 5 ++++-
3 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c
index ca8382d..81b474d 100644
--- a/drivers/video/omap2/dss/dsi.c
+++ b/drivers/video/omap2/dss/dsi.c
@@ -4776,6 +4776,7 @@ static int dsi_get_clocks(struct platform_device *dsidev)
}
dsi->sys_clk = clk;
+ clk_prepare(dsi->sys_clk);
return 0;
}
@@ -4786,8 +4787,10 @@ static void dsi_put_clocks(struct platform_device *dsidev)
if (dsi->dss_clk)
clk_put(dsi->dss_clk);
- if (dsi->sys_clk)
+ if (dsi->sys_clk) {
+ clk_unprepare(dsi->sys_clk);
clk_put(dsi->sys_clk);
+ }
}
static void __init dsi_probe_pdata(struct platform_device *dsidev)
diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c
index 8195c71..0b7ded3 100644
--- a/drivers/video/omap2/dss/hdmi.c
+++ b/drivers/video/omap2/dss/hdmi.c
@@ -544,14 +544,17 @@ static int hdmi_get_clocks(struct platform_device *pdev)
}
hdmi.sys_clk = clk;
+ clk_prepare(hdmi.sys_clk);
return 0;
}
static void hdmi_put_clocks(void)
{
- if (hdmi.sys_clk)
+ if (hdmi.sys_clk) {
+ clk_unprepare(hdmi.sys_clk);
clk_put(hdmi.sys_clk);
+ }
}
#if defined(CONFIG_OMAP4_DSS_HDMI_AUDIO)
diff --git a/drivers/video/omap2/dss/venc.c b/drivers/video/omap2/dss/venc.c
index 2b89739..62f395d 100644
--- a/drivers/video/omap2/dss/venc.c
+++ b/drivers/video/omap2/dss/venc.c
@@ -818,14 +818,17 @@ static int venc_get_clocks(struct platform_device *pdev)
}
venc.tv_dac_clk = clk;
+ clk_prepare(venc.tv_dac_clk);
return 0;
}
static void venc_put_clocks(void)
{
- if (venc.tv_dac_clk)
+ if (venc.tv_dac_clk) {
+ clk_unprepare(venc.tv_dac_clk);
clk_put(venc.tv_dac_clk);
+ }
}
static void __init venc_probe_pdata(struct platform_device *pdev)
--
1.7.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 05/11] OMAPDSS: add clk_prepare and clk_unprepare
2012-06-22 13:51 ` [PATCH 05/11] OMAPDSS: add clk_prepare and clk_unprepare Rajendra Nayak
@ 2012-06-25 6:07 ` Tomi Valkeinen
2012-06-25 7:11 ` Rajendra Nayak
2012-06-25 11:22 ` Russell King - ARM Linux
0 siblings, 2 replies; 13+ messages in thread
From: Tomi Valkeinen @ 2012-06-25 6:07 UTC (permalink / raw)
To: linux-arm-kernel
[-- Attachment #1: Type: text/plain, Size: 652 bytes --]
Hi,
On Fri, 2012-06-22 at 19:18 +0530, Rajendra Nayak wrote:
> In preparation of OMAP moving to Common Clk Framework(CCF) add clk_prepare()
> and clk_unprepare() for the omapdss clocks.
You used clk_prepare and clk_unprepare instead of clk_prepare_enable and
clk_disable_unprepare. I didn't check the dss driver yet, but my hunch
is that the clocks are normally not enabled/disabled from atomic
context.
What does the prepare/unprepare actually do? Is there any benefit in
delaying preparing, i.e. is there a difference between prepare right
after clk_get, or prepare right before clk_enable? (And similarly for
unprepare)
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 05/11] OMAPDSS: add clk_prepare and clk_unprepare
2012-06-25 6:07 ` Tomi Valkeinen
@ 2012-06-25 7:11 ` Rajendra Nayak
2012-06-25 7:58 ` Tomi Valkeinen
2012-06-25 11:22 ` Russell King - ARM Linux
1 sibling, 1 reply; 13+ messages in thread
From: Rajendra Nayak @ 2012-06-25 7:11 UTC (permalink / raw)
To: linux-arm-kernel
On Monday 25 June 2012 11:37 AM, Tomi Valkeinen wrote:
> Hi,
>
> On Fri, 2012-06-22 at 19:18 +0530, Rajendra Nayak wrote:
>> In preparation of OMAP moving to Common Clk Framework(CCF) add clk_prepare()
>> and clk_unprepare() for the omapdss clocks.
>
> You used clk_prepare and clk_unprepare instead of clk_prepare_enable and
> clk_disable_unprepare. I didn't check the dss driver yet, but my hunch
> is that the clocks are normally not enabled/disabled from atomic
> context.
>
> What does the prepare/unprepare actually do? Is there any benefit in
> delaying preparing, i.e. is there a difference between prepare right
> after clk_get, or prepare right before clk_enable? (And similarly for
> unprepare)
clk_prepare/unprepare are useful for clocks which need the 'enable'
logic to be implemented as a slow part (which can sleep) and a fast part
(which does not sleep). For all the dss clocks in question we don't need
a slow part and hence they do not have a .clk_prepare/unprepare
platform hook.
The framework however still does prepare usecounting (it has a prepare
count and an enable count, and prepare count is expected to be non-zero
while the clock is being enabled) and uses a mutex around to guard it.
So while the dss driver would do multiple clk_enable/disable while its
active, it seems fair to just prepare/unprepare the clocks once just
after clk_get() and before clk_put() in this particular case.
>
> Tom
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 05/11] OMAPDSS: add clk_prepare and clk_unprepare
2012-06-25 7:11 ` Rajendra Nayak
@ 2012-06-25 7:58 ` Tomi Valkeinen
2012-06-25 11:52 ` Rajendra Nayak
0 siblings, 1 reply; 13+ messages in thread
From: Tomi Valkeinen @ 2012-06-25 7:58 UTC (permalink / raw)
To: linux-arm-kernel
[-- Attachment #1: Type: text/plain, Size: 1970 bytes --]
On Mon, 2012-06-25 at 12:29 +0530, Rajendra Nayak wrote:
> On Monday 25 June 2012 11:37 AM, Tomi Valkeinen wrote:
> > Hi,
> >
> > On Fri, 2012-06-22 at 19:18 +0530, Rajendra Nayak wrote:
> >> In preparation of OMAP moving to Common Clk Framework(CCF) add clk_prepare()
> >> and clk_unprepare() for the omapdss clocks.
> >
> > You used clk_prepare and clk_unprepare instead of clk_prepare_enable and
> > clk_disable_unprepare. I didn't check the dss driver yet, but my hunch
> > is that the clocks are normally not enabled/disabled from atomic
> > context.
> >
> > What does the prepare/unprepare actually do? Is there any benefit in
> > delaying preparing, i.e. is there a difference between prepare right
> > after clk_get, or prepare right before clk_enable? (And similarly for
> > unprepare)
>
> clk_prepare/unprepare are useful for clocks which need the 'enable'
> logic to be implemented as a slow part (which can sleep) and a fast part
> (which does not sleep). For all the dss clocks in question we don't need
> a slow part and hence they do not have a .clk_prepare/unprepare
> platform hook.
>
> The framework however still does prepare usecounting (it has a prepare
> count and an enable count, and prepare count is expected to be non-zero
> while the clock is being enabled) and uses a mutex around to guard it.
> So while the dss driver would do multiple clk_enable/disable while its
> active, it seems fair to just prepare/unprepare the clocks once just
> after clk_get() and before clk_put() in this particular case.
But the driver should not presume anything special about the clocks. In
this case the dss driver would presume that the clocks it uses do not
have prepare and unprepare hooks.
If the generally proper way to use prepare/unprepare is in combination
of enable/disable, then I think we should try to do that.
I'll check if any of the dss clocks are enabled or disabled in atomic
context.
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 05/11] OMAPDSS: add clk_prepare and clk_unprepare
2012-06-25 6:07 ` Tomi Valkeinen
2012-06-25 7:11 ` Rajendra Nayak
@ 2012-06-25 11:22 ` Russell King - ARM Linux
1 sibling, 0 replies; 13+ messages in thread
From: Russell King - ARM Linux @ 2012-06-25 11:22 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Jun 25, 2012 at 09:07:58AM +0300, Tomi Valkeinen wrote:
> On Fri, 2012-06-22 at 19:18 +0530, Rajendra Nayak wrote:
> > In preparation of OMAP moving to Common Clk Framework(CCF) add clk_prepare()
> > and clk_unprepare() for the omapdss clocks.
>
> You used clk_prepare and clk_unprepare instead of clk_prepare_enable and
> clk_disable_unprepare. I didn't check the dss driver yet, but my hunch
> is that the clocks are normally not enabled/disabled from atomic
> context.
>
> What does the prepare/unprepare actually do? Is there any benefit in
> delaying preparing, i.e. is there a difference between prepare right
> after clk_get, or prepare right before clk_enable? (And similarly for
> unprepare)
I think you're looking at this the wrong way.
Think of clk_prepare() as the _sleepable_ part of clk_enable(). So, think
of clk_prepare() as ideally being placed just before clk_enable() with the
exception that if that's not possible (because you're in a non-atomic
context) it should be placed in a position as close to clk_enable() as
possible which is atomic.
Don't think of it as an extra step to be done after clk_get() but as an
extra step needing to be done before clk_enable().
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 05/11] OMAPDSS: add clk_prepare and clk_unprepare
2012-06-25 7:58 ` Tomi Valkeinen
@ 2012-06-25 11:52 ` Rajendra Nayak
2012-06-25 13:14 ` Tomi Valkeinen
0 siblings, 1 reply; 13+ messages in thread
From: Rajendra Nayak @ 2012-06-25 11:52 UTC (permalink / raw)
To: linux-arm-kernel
On Monday 25 June 2012 01:28 PM, Tomi Valkeinen wrote:
> On Mon, 2012-06-25 at 12:29 +0530, Rajendra Nayak wrote:
>> On Monday 25 June 2012 11:37 AM, Tomi Valkeinen wrote:
>>> Hi,
>>>
>>> On Fri, 2012-06-22 at 19:18 +0530, Rajendra Nayak wrote:
>>>> In preparation of OMAP moving to Common Clk Framework(CCF) add clk_prepare()
>>>> and clk_unprepare() for the omapdss clocks.
>>>
>>> You used clk_prepare and clk_unprepare instead of clk_prepare_enable and
>>> clk_disable_unprepare. I didn't check the dss driver yet, but my hunch
>>> is that the clocks are normally not enabled/disabled from atomic
>>> context.
>>>
>>> What does the prepare/unprepare actually do? Is there any benefit in
>>> delaying preparing, i.e. is there a difference between prepare right
>>> after clk_get, or prepare right before clk_enable? (And similarly for
>>> unprepare)
>>
>> clk_prepare/unprepare are useful for clocks which need the 'enable'
>> logic to be implemented as a slow part (which can sleep) and a fast part
>> (which does not sleep). For all the dss clocks in question we don't need
>> a slow part and hence they do not have a .clk_prepare/unprepare
>> platform hook.
>>
>> The framework however still does prepare usecounting (it has a prepare
>> count and an enable count, and prepare count is expected to be non-zero
>> while the clock is being enabled) and uses a mutex around to guard it.
>> So while the dss driver would do multiple clk_enable/disable while its
>> active, it seems fair to just prepare/unprepare the clocks once just
>> after clk_get() and before clk_put() in this particular case.
>
> But the driver should not presume anything special about the clocks. In
> this case the dss driver would presume that the clocks it uses do not
> have prepare and unprepare hooks.
>
> If the generally proper way to use prepare/unprepare is in combination
> of enable/disable, then I think we should try to do that.
makes sense. Lets see if any of the clk_enable/disable happen in atomic
context, if not it would be just a matter of replacing all with a
clk_prepare_enable/disable_unprepare. Else we might have to find a safe
place sometime before clk_enable to prepare the clk and after
clk_disable to unprepare it.
>
> I'll check if any of the dss clocks are enabled or disabled in atomic
> context.
>
> Tomi
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 05/11] OMAPDSS: add clk_prepare and clk_unprepare
2012-06-25 11:52 ` Rajendra Nayak
@ 2012-06-25 13:14 ` Tomi Valkeinen
2012-06-26 5:12 ` Rajendra Nayak
2012-06-27 0:47 ` Mike Turquette
0 siblings, 2 replies; 13+ messages in thread
From: Tomi Valkeinen @ 2012-06-25 13:14 UTC (permalink / raw)
To: linux-arm-kernel
[-- Attachment #1: Type: text/plain, Size: 3644 bytes --]
On Mon, 2012-06-25 at 17:18 +0530, Rajendra Nayak wrote:
> On Monday 25 June 2012 01:28 PM, Tomi Valkeinen wrote:
> > On Mon, 2012-06-25 at 12:29 +0530, Rajendra Nayak wrote:
> >> On Monday 25 June 2012 11:37 AM, Tomi Valkeinen wrote:
> >>> Hi,
> >>>
> >>> On Fri, 2012-06-22 at 19:18 +0530, Rajendra Nayak wrote:
> >>>> In preparation of OMAP moving to Common Clk Framework(CCF) add clk_prepare()
> >>>> and clk_unprepare() for the omapdss clocks.
> >>>
> >>> You used clk_prepare and clk_unprepare instead of clk_prepare_enable and
> >>> clk_disable_unprepare. I didn't check the dss driver yet, but my hunch
> >>> is that the clocks are normally not enabled/disabled from atomic
> >>> context.
> >>>
> >>> What does the prepare/unprepare actually do? Is there any benefit in
> >>> delaying preparing, i.e. is there a difference between prepare right
> >>> after clk_get, or prepare right before clk_enable? (And similarly for
> >>> unprepare)
> >>
> >> clk_prepare/unprepare are useful for clocks which need the 'enable'
> >> logic to be implemented as a slow part (which can sleep) and a fast part
> >> (which does not sleep). For all the dss clocks in question we don't need
> >> a slow part and hence they do not have a .clk_prepare/unprepare
> >> platform hook.
> >>
> >> The framework however still does prepare usecounting (it has a prepare
> >> count and an enable count, and prepare count is expected to be non-zero
> >> while the clock is being enabled) and uses a mutex around to guard it.
> >> So while the dss driver would do multiple clk_enable/disable while its
> >> active, it seems fair to just prepare/unprepare the clocks once just
> >> after clk_get() and before clk_put() in this particular case.
> >
> > But the driver should not presume anything special about the clocks. In
> > this case the dss driver would presume that the clocks it uses do not
> > have prepare and unprepare hooks.
> >
> > If the generally proper way to use prepare/unprepare is in combination
> > of enable/disable, then I think we should try to do that.
>
> makes sense. Lets see if any of the clk_enable/disable happen in atomic
> context, if not it would be just a matter of replacing all with a
> clk_prepare_enable/disable_unprepare. Else we might have to find a safe
> place sometime before clk_enable to prepare the clk and after
> clk_disable to unprepare it.
>
> >
> > I'll check if any of the dss clocks are enabled or disabled in atomic
> > context.
venc and hdmi use clk_enable/disable in runtime PM callbacks (suspend &
resume). If I understand correctly, the callbacks are not called in
atomic context if pm_runtime_irq_safe() has not been used. And it is not
used in omapdss.
dsi uses clk_enable/disable in a different manner, but not in atomic
context.
So as far as I see, clocks are never handled in atomic context. Is
everything related to the base clk stuff already in mainline? Can I take
the clk_prepare/unprepare patch into my omapdss tree?
A question about clk_prepare/unprepare, not directly related: let's say
I have a driver for some HW block. The driver doesn't use clk functions,
but uses runtime PM. The driver also sets pm_runtime_irq_safe().
Now, the driver can call pm_runtime_get_sync() in an atomic context, and
this would lead to the underlying framework (hwmod, omap_device, I don't
know who =) enabling the func clock for that HW. But this would happen
in atomic context, so the underlying framework can't use clk_prepare.
How does the underlying framework handle that case? (sorry if that's a
stupid question =).
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 05/11] OMAPDSS: add clk_prepare and clk_unprepare
2012-06-25 13:14 ` Tomi Valkeinen
@ 2012-06-26 5:12 ` Rajendra Nayak
2012-06-26 6:55 ` Tomi Valkeinen
2012-06-27 0:47 ` Mike Turquette
1 sibling, 1 reply; 13+ messages in thread
From: Rajendra Nayak @ 2012-06-26 5:12 UTC (permalink / raw)
To: linux-arm-kernel
On Monday 25 June 2012 06:44 PM, Tomi Valkeinen wrote:
> venc and hdmi use clk_enable/disable in runtime PM callbacks (suspend&
> resume). If I understand correctly, the callbacks are not called in
> atomic context if pm_runtime_irq_safe() has not been used. And it is not
> used in omapdss.
>
> dsi uses clk_enable/disable in a different manner, but not in atomic
> context.
>
> So as far as I see, clocks are never handled in atomic context. Is
> everything related to the base clk stuff already in mainline? Can I take
> the clk_prepare/unprepare patch into my omapdss tree?
Well the Common Clk framework is already in mainline, but we still don;t
have CONFIG_COMMON_CLK enabled for our builds yet. So until we do so,
clk_prepare/unprepare will just be stubs which do nothing.
I will repost the patch getting rid of the clk_prepare/unprepare and
adding clk_prepare_enable/disable_unprepare instead.
>
>
> A question about clk_prepare/unprepare, not directly related: let's say
> I have a driver for some HW block. The driver doesn't use clk functions,
> but uses runtime PM. The driver also sets pm_runtime_irq_safe().
>
> Now, the driver can call pm_runtime_get_sync() in an atomic context, and
> this would lead to the underlying framework (hwmod, omap_device, I don't
> know who =) enabling the func clock for that HW. But this would happen
> in atomic context, so the underlying framework can't use clk_prepare.
>
> How does the underlying framework handle that case? (sorry if that's a
> stupid question =).
No, its not a stupid question at all. I have been thinking about this
too to figure out whats the best way to handle this. For now, the
patches I posted, do an early clk_prepare (like I did for dss) for all
hwmod clocks as I have no way to know which ones will have their
_enable, _idle etc called in atomic context. Maybe I should see if there
is a way I can do so only for those devices which end up calling a
pm_runtime_irq_safe() and not do it early for all.
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 05/11] OMAPDSS: add clk_prepare and clk_unprepare
2012-06-26 5:12 ` Rajendra Nayak
@ 2012-06-26 6:55 ` Tomi Valkeinen
2012-06-26 7:48 ` Rajendra Nayak
0 siblings, 1 reply; 13+ messages in thread
From: Tomi Valkeinen @ 2012-06-26 6:55 UTC (permalink / raw)
To: linux-arm-kernel
[-- Attachment #1: Type: text/plain, Size: 788 bytes --]
On Tue, 2012-06-26 at 10:30 +0530, Rajendra Nayak wrote:
> > So as far as I see, clocks are never handled in atomic context. Is
> > everything related to the base clk stuff already in mainline? Can I take
> > the clk_prepare/unprepare patch into my omapdss tree?
>
> Well the Common Clk framework is already in mainline, but we still don;t
> have CONFIG_COMMON_CLK enabled for our builds yet. So until we do so,
> clk_prepare/unprepare will just be stubs which do nothing.
But if I understood correctly, clk_prepare and clk_unprepare are anyway
no-ops with dss clocks, even when CONFIG_COMMON_CLK is enabled?
My point was only to understand if I can safely take the patch into
omapdss tree, instead of it going through l-o, to avoid any possible
conflicts.
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 05/11] OMAPDSS: add clk_prepare and clk_unprepare
2012-06-26 6:55 ` Tomi Valkeinen
@ 2012-06-26 7:48 ` Rajendra Nayak
0 siblings, 0 replies; 13+ messages in thread
From: Rajendra Nayak @ 2012-06-26 7:48 UTC (permalink / raw)
To: linux-arm-kernel
On Tuesday 26 June 2012 12:25 PM, Tomi Valkeinen wrote:
> On Tue, 2012-06-26 at 10:30 +0530, Rajendra Nayak wrote:
>
>>> So as far as I see, clocks are never handled in atomic context. Is
>>> everything related to the base clk stuff already in mainline? Can I take
>>> the clk_prepare/unprepare patch into my omapdss tree?
>>
>> Well the Common Clk framework is already in mainline, but we still don;t
>> have CONFIG_COMMON_CLK enabled for our builds yet. So until we do so,
>> clk_prepare/unprepare will just be stubs which do nothing.
>
> But if I understood correctly, clk_prepare and clk_unprepare are anyway
> no-ops with dss clocks, even when CONFIG_COMMON_CLK is enabled?
With CONFIG_COMMON_CLK enabled, they will do prepare use-counting with
a mutex lock/unlock around it.
>
> My point was only to understand if I can safely take the patch into
> omapdss tree, instead of it going through l-o, to avoid any possible
> conflicts.
yes, I don't see any issues with it going through dss tree.
>
> Tomi
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 05/11] OMAPDSS: add clk_prepare and clk_unprepare
2012-06-25 13:14 ` Tomi Valkeinen
2012-06-26 5:12 ` Rajendra Nayak
@ 2012-06-27 0:47 ` Mike Turquette
2012-06-27 4:19 ` Tomi Valkeinen
1 sibling, 1 reply; 13+ messages in thread
From: Mike Turquette @ 2012-06-27 0:47 UTC (permalink / raw)
To: linux-arm-kernel
On 20120625-16:14, Tomi Valkeinen wrote:
> A question about clk_prepare/unprepare, not directly related: let's say
> I have a driver for some HW block. The driver doesn't use clk functions,
> but uses runtime PM. The driver also sets pm_runtime_irq_safe().
>
> Now, the driver can call pm_runtime_get_sync() in an atomic context, and
> this would lead to the underlying framework (hwmod, omap_device, I don't
> know who =) enabling the func clock for that HW. But this would happen
> in atomic context, so the underlying framework can't use clk_prepare.
>
> How does the underlying framework handle that case? (sorry if that's a
> stupid question =).
>
I think it's a good question!
If we're going to call clk_prepare_enable from within a runtime pm
callback then I think we'll need to check if _irq_safe() is set and
conditionally call only clk_enable in such a case.
I'm not a runtime pm expert, but if the driver owns the responsibility
of calling pm_runtime_irq_safe then the driver has the proper context
to know that it should call clk_prepare BEFORE calling
pm_runtime_get_sync.
However if some other framework (outside of the driver) calls
pm_runtime_irq_safe then it will be difficult for the driver to know
what to do...
Regards,
Mike
> Tomi
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 05/11] OMAPDSS: add clk_prepare and clk_unprepare
2012-06-27 0:47 ` Mike Turquette
@ 2012-06-27 4:19 ` Tomi Valkeinen
2012-06-27 5:31 ` Rajendra Nayak
0 siblings, 1 reply; 13+ messages in thread
From: Tomi Valkeinen @ 2012-06-27 4:19 UTC (permalink / raw)
To: linux-arm-kernel
[-- Attachment #1: Type: text/plain, Size: 1617 bytes --]
On Tue, 2012-06-26 at 17:47 -0700, Mike Turquette wrote:
> On 20120625-16:14, Tomi Valkeinen wrote:
> > A question about clk_prepare/unprepare, not directly related: let's say
> > I have a driver for some HW block. The driver doesn't use clk functions,
> > but uses runtime PM. The driver also sets pm_runtime_irq_safe().
> >
> > Now, the driver can call pm_runtime_get_sync() in an atomic context, and
> > this would lead to the underlying framework (hwmod, omap_device, I don't
> > know who =) enabling the func clock for that HW. But this would happen
> > in atomic context, so the underlying framework can't use clk_prepare.
> >
> > How does the underlying framework handle that case? (sorry if that's a
> > stupid question =).
> >
>
> I think it's a good question!
>
> If we're going to call clk_prepare_enable from within a runtime pm
> callback then I think we'll need to check if _irq_safe() is set and
> conditionally call only clk_enable in such a case.
>
> I'm not a runtime pm expert, but if the driver owns the responsibility
> of calling pm_runtime_irq_safe then the driver has the proper context
> to know that it should call clk_prepare BEFORE calling
> pm_runtime_get_sync.
That's not quite what I meant. If it's the driver that does clk_enable,
be it in runtime PM callback or not, it's driver's responsibility.
But some clocks are not handled by the driver, but the hwmod/omap_device
framework. Mainly I think this is for the functional and interface
clocks. The driver has no visibility to those, they are implicitly
enabled via pm_runtime_get.
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 05/11] OMAPDSS: add clk_prepare and clk_unprepare
2012-06-27 4:19 ` Tomi Valkeinen
@ 2012-06-27 5:31 ` Rajendra Nayak
0 siblings, 0 replies; 13+ messages in thread
From: Rajendra Nayak @ 2012-06-27 5:31 UTC (permalink / raw)
To: linux-arm-kernel
On Wednesday 27 June 2012 09:49 AM, Tomi Valkeinen wrote:
> On Tue, 2012-06-26 at 17:47 -0700, Mike Turquette wrote:
>> On 20120625-16:14, Tomi Valkeinen wrote:
>>> A question about clk_prepare/unprepare, not directly related: let's say
>>> I have a driver for some HW block. The driver doesn't use clk functions,
>>> but uses runtime PM. The driver also sets pm_runtime_irq_safe().
>>>
>>> Now, the driver can call pm_runtime_get_sync() in an atomic context, and
>>> this would lead to the underlying framework (hwmod, omap_device, I don't
>>> know who =) enabling the func clock for that HW. But this would happen
>>> in atomic context, so the underlying framework can't use clk_prepare.
>>>
>>> How does the underlying framework handle that case? (sorry if that's a
>>> stupid question =).
>>>
>>
>> I think it's a good question!
>>
>> If we're going to call clk_prepare_enable from within a runtime pm
>> callback then I think we'll need to check if _irq_safe() is set and
>> conditionally call only clk_enable in such a case.
>>
>> I'm not a runtime pm expert, but if the driver owns the responsibility
>> of calling pm_runtime_irq_safe then the driver has the proper context
>> to know that it should call clk_prepare BEFORE calling
>> pm_runtime_get_sync.
>
> That's not quite what I meant. If it's the driver that does clk_enable,
> be it in runtime PM callback or not, it's driver's responsibility.
>
> But some clocks are not handled by the driver, but the hwmod/omap_device
> framework. Mainly I think this is for the functional and interface
> clocks. The driver has no visibility to those, they are implicitly
> enabled via pm_runtime_get.
yes, thats the tricky part on how would hwmod/omap_device know if the
driver would use runtime pm within atomic context or non-atomic context.
The driver does inform the runtime pm framework about this by calling a
pm_runtime_irq_safe(), which is then used to set the .irq_safe flag
telling it to leave the interrupts disabled during callbacks.
This information however does not flow down to hwmod/omap_device in any
way. So the way its currently handled is to do an early prepare of all
hwmod controlled clocks.
I have copied Kevin in case he has any better ideas on how this should
be handled.
>
> Tomi
>
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2012-06-27 5:31 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1340372890-10091-1-git-send-email-rnayak@ti.com>
2012-06-22 13:51 ` [PATCH 05/11] OMAPDSS: add clk_prepare and clk_unprepare Rajendra Nayak
2012-06-25 6:07 ` Tomi Valkeinen
2012-06-25 7:11 ` Rajendra Nayak
2012-06-25 7:58 ` Tomi Valkeinen
2012-06-25 11:52 ` Rajendra Nayak
2012-06-25 13:14 ` Tomi Valkeinen
2012-06-26 5:12 ` Rajendra Nayak
2012-06-26 6:55 ` Tomi Valkeinen
2012-06-26 7:48 ` Rajendra Nayak
2012-06-27 0:47 ` Mike Turquette
2012-06-27 4:19 ` Tomi Valkeinen
2012-06-27 5:31 ` Rajendra Nayak
2012-06-25 11:22 ` Russell King - ARM Linux
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).