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:
signature.asc
Description: This is a digitally signed message part
