On Wed, Sep 14, 2011 at 4:45 AM, Tony Lindgren <tony@xxxxxxxxxxx> wrote:
> * Tarun Kanti DebBarma <tarun.kanti@xxxxxx> [110908 13:36]:
>> Since the exported APIs can be called from interrupt context
>> extend spinlock protection to some more relevant APIs to avoid
>> race condition.
>
> We should have locking for requesting and releasing a timer etc,
> but I don't see need for that for the timer specific functions.
Alright... I will remove locking from timer specific functions.
In that case we have extension of locks to just following two functions:
omap_dm_timer_request()
omap_dm_timer_request_specific()
So, I can modify the patch subject and description accordingly.
>
>> @@ -317,9 +317,11 @@ EXPORT_SYMBOL_GPL(omap_dm_timer_trigger);
>> void omap_dm_timer_start(struct omap_dm_timer *timer)
>> {
>> u32 l;
>> + unsigned long flags;
>>
>> omap_dm_timer_enable(timer);
>>
>> + spin_lock_irqsave(&dm_timer_lock, flags);
>> if (timer->loses_context) {
>> u32 ctx_loss_cnt_after =
>> timer->get_context_loss_count(&timer->pdev->dev);
>
> Here the caller already owns the timer being started. So there
> should never be multiple users for a single timer. If there are,
> then the caller should take care of locking.
Ok.
>
>> void omap_dm_timer_stop(struct omap_dm_timer *timer)
>> {
>> - unsigned long rate = 0;
>> + unsigned long rate = 0, flags;
>> struct dmtimer_platform_data *pdata = timer->pdev->dev.platform_data;
>> bool is_omap2 = true;
>>
>> + spin_lock_irqsave(&dm_timer_lock, flags);
>> if (pdata->needs_manual_reset)
>> is_omap2 = false;
>> else
>
> Here too.
Right.
>
>> @@ -389,8 +394,10 @@ void omap_dm_timer_set_load(struct omap_dm_timer *timer, int autoreload,
>> unsigned int load)
>> {
>> u32 l;
>> + unsigned long flags;
>>
>> omap_dm_timer_enable(timer);
>> + spin_lock_irqsave(&dm_timer_lock, flags);
>> l = omap_dm_timer_read_reg(timer, OMAP_TIMER_CTRL_REG);
>> if (autoreload)
>> l |= OMAP_TIMER_CTRL_AR;
>
> And here.
Ok.
>
>> @@ -412,9 +420,11 @@ void omap_dm_timer_set_load_start(struct omap_dm_timer *timer, int autoreload,
>> unsigned int load)
>> {
>> u32 l;
>> + unsigned long flags;
>>
>> omap_dm_timer_enable(timer);
>>
>> + spin_lock_irqsave(&dm_timer_lock, flags);
>> if (timer->loses_context) {
>> u32 ctx_loss_cnt_after =
>> timer->get_context_loss_count(&timer->pdev->dev);
>
> Not needed here either. And that's the case for all the other functions
> too.
Sure.
--
Tarun
>
> Regards,
>
> Tony
>
--
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
