Re: [PATCH 6/6] staging: sm750fb: Spinlock and unlock in the same block

From: Dan Carpenter
Date: Wed Mar 11 2015 - 05:09:39 EST


On Wed, Mar 11, 2015 at 01:28:45AM +0000, Lorenzo Stoakes wrote:
> -static inline void myspin_lock(spinlock_t * sl){
> - struct lynx_share * share;
> - share = container_of(sl,struct lynx_share,slock);
> - if(share->dual){
> - spin_lock(sl);
> - }
> -}

Yes, good. We all hate locking wrappers but these are worse than
normal.

> + /* if not use spin_lock,system will die if user load driver
> + * and immediatly unload driver frequently (dual)*/
> + if (share->dual) {
> + spin_lock(&share->slock);
> + share->accel.de_fillrect(&share->accel,
> + base,pitch,Bpp,
> + region->dx,region->dy,
> + region->width,region->height,
> + color,rop);
> + spin_unlock(&share->slock);
> + } else
> + share->accel.de_fillrect(&share->accel,
> + base,pitch,Bpp,
> + region->dx,region->dy,
> + region->width,region->height,
> + color,rop);
> }

No. You've made the code uglier to work around Sparse stupidness. Also
the braces are not according to kernel style.

if (share->dual)
spin_lock(&share->slock);

share->accel.de_fillrect(&share->accel,
base,pitch,Bpp,
region->dx,region->dy,
region->width,region->height,
color,rop);

if (share->dual)
spin_unlock(&share->slock);

Sparse will still complain but no one cares.

regards,
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/