Re: [Outreachy kernel] Re: [PATCH 2/2] Staging: media: davinci_vpfe: Remove RESETBIT macro and use BIT macro

[Greg KH <gregkh@linuxfoundation.org>]

On Fri, Mar 22, 2019 at 07:31:01AM +0100, Julia Lawall wrote:
> 
> 
> On Fri, 22 Mar 2019, Greg KH wrote:
> 
> > On Thu, Mar 21, 2019 at 08:26:06PM -0500, Madhumitha Prabakaran wrote:
> > > Remove RESETBIT macro and use BIT(x) instead of (1 << x) to improve
> > > codebase.
> > >
> > > Signed-off-by: Madhumitha Prabakaran <madhumithabiw@gmail.com>
> > > ---
> > >  drivers/staging/media/davinci_vpfe/dm365_ipipeif.c | 8 ++++----
> > >  drivers/staging/media/davinci_vpfe/dm365_ipipeif.h | 1 -
> > >  2 files changed, 4 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/staging/media/davinci_vpfe/dm365_ipipeif.c b/drivers/staging/media/davinci_vpfe/dm365_ipipeif.c
> > > index 382d88bee5a7..4f18db127caf 100644
> > > --- a/drivers/staging/media/davinci_vpfe/dm365_ipipeif.c
> > > +++ b/drivers/staging/media/davinci_vpfe/dm365_ipipeif.c
> > > @@ -296,14 +296,14 @@ static int ipipeif_hw_setup(struct v4l2_subdev *sd)
> > >  		case MEDIA_BUS_FMT_YUYV8_1X16:
> > >  		case MEDIA_BUS_FMT_UYVY8_2X8:
> > >  		case MEDIA_BUS_FMT_Y8_1X8:
> > > -			RESETBIT(val, IPIPEIF_CFG2_YUV8_SHIFT);
> > > +			val &= ~BIT(IPIPEIF_CFG2_YUV8_SHIFT);
> >
> > clear_bit()?
> >
> > >  			val |= BIT(IPIPEIF_CFG2_YUV16_SHIFT);
> >
> > set_bit()?
> 
> OK, I see that the definition of these things is much more complicated
> than what I was looking for.  These are atomic operations.  There is also
> __set_bit, which is not atomic.  But set_bit does seem to be commonly
> used.

Only use __set_bit() if you know you are safe doing the operation in an
"unordered" fashion.  Given that these originally were "open coded",
it's probably safe, but it's simpler to use the ordered calls for now.

thanks,

greg k-h