* [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 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
* 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
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).