From: sh_def <[email protected]> > [email protected] writes: > > diff --git a/drivers/staging/fbtft/fbtft-core.c > > b/drivers/staging/fbtft/fbtft-core.c > > index ca0c38221c16..193643d0329d 100644 > > --- a/drivers/staging/fbtft/fbtft-core.c > > +++ b/drivers/staging/fbtft/fbtft-core.c > > @@ -298,14 +298,20 @@ static void fbtft_mkdirty(struct fb_info *info, int > > y, int height) > > { > > struct fbtft_par *par = info->par; > > struct fb_deferred_io *fbdefio = info->fbdefio; > > + unsigned long flags; > > > > /* Mark display lines/area as dirty */ > > - spin_lock(&par->dirty_lock); > > + /* > > + * fbcon takes dirty_lock while holding console_owner. Disable local > > + * interrupts here so a printk hardirq cannot acquire console_owner > > + * while dirty_lock is held and create the inverse lock ordering. > > + */ > > Beside that reason, we also need spin_lock_irqsave() because > fbtft_mkdirty() can be called in both task context and hardirq > context. And this reason alone suffices and usually is why > spin_lock_irqsave() is used, so I think a comment is not necessary. >
Thanks. I will drop it in v2. > But I am fine with it either way. > > > spin_lock_irqsave(&par->dirty_lock, flags); > > if (y < par->dirty_lines_start) > > par->dirty_lines_start = y; > > if (y + height - 1 > par->dirty_lines_end) > > par->dirty_lines_end = y + height - 1; > > - spin_unlock(&par->dirty_lock); > > + spin_unlock_irqrestore(&par->dirty_lock, flags); > > > > /* Schedule deferred_io to update display (no-op if already on queue)*/ > > schedule_delayed_work(&info->deferred_work, fbdefio->delay); > > @@ -317,14 +323,15 @@ static void fbtft_deferred_io(struct fb_info *info, > > struct list_head *pagereflis > > unsigned int dirty_lines_start, dirty_lines_end; > > struct fb_deferred_io_pageref *pageref; > > unsigned int y_low = 0, y_high = 0; > > + unsigned long flags; > > > > - spin_lock(&par->dirty_lock); > > + spin_lock_irqsave(&par->dirty_lock, flags); > > dirty_lines_start = par->dirty_lines_start; > > dirty_lines_end = par->dirty_lines_end; > > /* set display line markers as clean */ > > par->dirty_lines_start = par->info->var.yres - 1; > > par->dirty_lines_end = 0; > > - spin_unlock(&par->dirty_lock); > > + spin_unlock_irqrestore(&par->dirty_lock, flags); > > fbtft_deferred_io() is executed in workqueue with interrupt enabled. So > it can use spin_lock_irq() instead of spin_lock_irqsave(), right? Yes, agreed. I will use spin_lock_irq() here in v2. Thanks for the review. > > Nam
