Re: [PATCH v15 11/12] OMAP: dmtimer: extend spinlock to exported APIs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

> @@ -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.

>  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.

> @@ -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.

> @@ -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.

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


[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux