[PATCH v16 09/12] OMAP: dmtimer: low-power mode support

[DebBarma, Tarun Kanti]

On Tue, Nov 15, 2011 at 11:27 PM, Omar Ramirez Luna <or.rmz1 at gmail.com> wrote:
> Hi Tarun,
>
> On Tue, Sep 20, 2011 at 6:30 AM, Tarun Kanti DebBarma
> <tarun.kanti at ti.com> 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
>