On Tue, Nov 15, 2011 at 11:27 PM, Omar Ramirez Luna <or.rmz1@xxxxxxxxx> wrote: > Hi Tarun, > > On Tue, Sep 20, 2011 at 6:30 AM, Tarun Kanti DebBarma > <tarun.kanti@xxxxxx> wrote: >> Clock is enabled only when timer is started and disabled when the the timer >> is stopped. Therefore before accessing registers in functions clock is enabled >> and then disabled back at the end of access. Context save is done dynamically >> whenever the registers are modified. Context restore is called when context is >> lost. > > Now that handling of the clock has been decoupled from > omap_dm_timer_request_specific/free and placed in start/stop > functions, there is a "possible irq lock inversion dependency > detected" for any attempt to enable the clock from soft|hard irq. > > My use case is as follows: > > DSP sends a mailbox message to ARM, this triggers a tasklet in mailbox > for processing the message, when it reaches tidspbridge it turns out > that the DSP wants to enable a gpt timer; however, locks involved > "clocks_mutex" and "dm_timer_lock" now could cause a deadlock as they > have been called from unsafe contexts in the past > (http://pastebin.com/7Dtz8t0f). > > Is the intention to restrict enabling the dmtimer clocks from hard|soft irqs? The aim is to prevent client drivers perform clock enable/disable independently. Instead just use the request/start/stop APIs. In that way we can make clock enable/disable functions static in the future. > > ... >> @@ -303,6 +342,22 @@ void omap_dm_timer_stop(struct omap_dm_timer *timer) >> rate = clk_get_rate(timer->fclk); >> >> __omap_dm_timer_stop(timer, timer->posted, rate, is_omap2); >> + >> + if (timer->loses_context) { >> + if (timer->get_context_loss_count) >> + timer->ctx_loss_count = >> + timer->get_context_loss_count(&timer->pdev->dev); >> + } >> + >> + /* >> + * Since the register values are computed and written within >> + * __omap_dm_timer_stop, we need to use read to retrieve the >> + * context. >> + */ >> + timer->context.tclr = >> + omap_dm_timer_read_reg(timer, OMAP_TIMER_CTRL_REG); >> + timer->context.tisr = __raw_readl(timer->irq_stat); >> + omap_dm_timer_disable(timer); >> } >> EXPORT_SYMBOL_GPL(omap_dm_timer_stop); >> > > I didn't review the whole patch, but at least this part is not in > mainline, it seems like the patch modified by Tony and the v16 were > not the same. > > I could diff and include the missing parts, if someone can confirm > that we need v16, which I believe so, since current stop function > doesn't call omap_dm_timer_disable. You are right. This change some how got missed in mainline. I have posted a patch for this already. -- Tarun > > Regards, > > Omar > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
