[PATCH] staging: octeon-usb: Prefer using BIT macro

[PATCH] staging: octeon-usb: Prefer using BIT macro

[Namrata A Shettar]

Replace bit shifting on 1 with BIT(i) macro. Done using Coccinelle:
@@ int i; @@

- (1 << i)
+ BIT(i)

Signed-off-by: Namrata A Shettar <namrataashettar@gmail.com>
---
 drivers/staging/octeon-usb/octeon-hcd.c   | 22 +++++++++++-----------
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/octeon-usb/octeon-hcd.c b/drivers/staging/octeon-usb/octeon-hcd.c
index 17442b3..0d1a7d8 100644
--- a/drivers/staging/octeon-usb/octeon-hcd.c
+++ b/drivers/staging/octeon-usb/octeon-hcd.c
@@ -187,14 +187,14 @@ struct cvmx_usb_iso_packet {
  *					      data transfer use for the USB
  */
 enum cvmx_usb_initialize_flags {
-	CVMX_USB_INITIALIZE_FLAGS_CLOCK_XO_XI		= 1 << 0,
-	CVMX_USB_INITIALIZE_FLAGS_CLOCK_XO_GND		= 1 << 1,
+	CVMX_USB_INITIALIZE_FLAGS_CLOCK_XO_XI		= BIT(0),
+	CVMX_USB_INITIALIZE_FLAGS_CLOCK_XO_GND		= BIT(1),
 	CVMX_USB_INITIALIZE_FLAGS_CLOCK_MHZ_MASK	= 3 << 3,
-	CVMX_USB_INITIALIZE_FLAGS_CLOCK_12MHZ		= 1 << 3,
+	CVMX_USB_INITIALIZE_FLAGS_CLOCK_12MHZ		= BIT(3),
 	CVMX_USB_INITIALIZE_FLAGS_CLOCK_24MHZ		= 2 << 3,
 	CVMX_USB_INITIALIZE_FLAGS_CLOCK_48MHZ		= 3 << 3,
 	/* Bits 3-4 used to encode the clock frequency */
-	CVMX_USB_INITIALIZE_FLAGS_NO_DMA		= 1 << 5,
+	CVMX_USB_INITIALIZE_FLAGS_NO_DMA		= BIT(5),
 };
 
 /**
@@ -206,8 +206,8 @@ enum cvmx_usb_initialize_flags {
  *				   pipe is in the ping state.
  */
 enum cvmx_usb_pipe_flags {
-	CVMX_USB_PIPE_FLAGS_SCHEDULED	= 1 << 17,
-	CVMX_USB_PIPE_FLAGS_NEED_PING	= 1 << 18,
+	CVMX_USB_PIPE_FLAGS_SCHEDULED	= BIT(17),
+	CVMX_USB_PIPE_FLAGS_NEED_PING	= BIT(18),
 };
 
 /* Maximum number of times to retry failed transactions */
@@ -1505,7 +1505,7 @@ static void cvmx_usb_start_channel(struct octeon_hcd *usb, int channel,
 	pipe->flags |= CVMX_USB_PIPE_FLAGS_SCHEDULED;
 
 	/* Mark this channel as in use */
-	usb->idle_hardware_channels &= ~(1 << channel);
+	usb->idle_hardware_channels &= ~BIT(channel);
 
 	/* Enable the channel interrupt bits */
 	{
@@ -1551,7 +1551,7 @@ static void cvmx_usb_start_channel(struct octeon_hcd *usb, int channel,
 		/* Enable the channel interrupt to propagate */
 		usbc_haintmsk.u32 = cvmx_usb_read_csr32(usb,
 					CVMX_USBCX_HAINTMSK(usb->index));
-		usbc_haintmsk.s.haintmsk |= 1 << channel;
+		usbc_haintmsk.s.haintmsk |= BIT(channel);
 		cvmx_usb_write_csr32(usb, CVMX_USBCX_HAINTMSK(usb->index),
 				     usbc_haintmsk.u32);
 	}
@@ -2665,7 +2665,7 @@ static int cvmx_usb_poll_channel(struct octeon_hcd *usb, int channel)
 
 	/* Disable the channel interrupts now that it is done */
 	cvmx_usb_write_csr32(usb, CVMX_USBCX_HCINTMSKX(channel, usb->index), 0);
-	usb->idle_hardware_channels |= (1 << channel);
+	usb->idle_hardware_channels |= BIT(channel);
 
 	/* Make sure this channel is tied to a valid pipe */
 	pipe = usb->pipe_for_channel[channel];
@@ -3017,7 +3017,7 @@ static int cvmx_usb_poll(struct octeon_hcd *usb)
 
 			channel = __fls(usbc_haint.u32);
 			cvmx_usb_poll_channel(usb, channel);
-			usbc_haint.u32 ^= 1 << channel;
+			usbc_haint.u32 ^= BIT(channel);
 		}
 	}
 
@@ -3595,7 +3595,7 @@ static int octeon_usb_probe(struct platform_device *pdev)
 
 	if (irq < 0) {
 		/* Defective device tree, but we know how to fix it. */
-		irq_hw_number_t hwirq = usb_num ? (1 << 6) + 17 : 56;
+		irq_hw_number_t hwirq = usb_num ? BIT(6) + 17 : 56;
 
 		irq = irq_create_mapping(NULL, hwirq);
 	}

-- 
2.7.4

Re: [PATCH] staging: octeon-usb: Prefer using BIT macro

[Greg Kroah-Hartman]

On Mon, Oct 03, 2016 at 03:03:13AM +0300, Aaro Koskinen wrote:
> Hi,
> 
> On Mon, Oct 03, 2016 at 12:03:25AM +0530, Namrata A Shettar wrote:
> > -	CVMX_USB_INITIALIZE_FLAGS_CLOCK_XO_XI		= 1 << 0,
> > -	CVMX_USB_INITIALIZE_FLAGS_CLOCK_XO_GND		= 1 << 1,
> > +	CVMX_USB_INITIALIZE_FLAGS_CLOCK_XO_XI		= BIT(0),
> > +	CVMX_USB_INITIALIZE_FLAGS_CLOCK_XO_GND		= BIT(1),
> >  	CVMX_USB_INITIALIZE_FLAGS_CLOCK_MHZ_MASK	= 3 << 3,
> > -	CVMX_USB_INITIALIZE_FLAGS_CLOCK_12MHZ		= 1 << 3,
> > +	CVMX_USB_INITIALIZE_FLAGS_CLOCK_12MHZ		= BIT(3),
> >  	CVMX_USB_INITIALIZE_FLAGS_CLOCK_24MHZ		= 2 << 3,
> >  	CVMX_USB_INITIALIZE_FLAGS_CLOCK_48MHZ		= 3 << 3,
> >  	/* Bits 3-4 used to encode the clock frequency */
> > -	CVMX_USB_INITIALIZE_FLAGS_NO_DMA		= 1 << 5,
> > +	CVMX_USB_INITIALIZE_FLAGS_NO_DMA		= BIT(5),
> 
> All entries should be converted to use BIT.

The ones that could be, were, right?  What else should be done here?

Well, there are bit mask macros now in the kernel tree, perhaps those
should be used, but I don't see how BIT() could be used for "3 << 3", do
you?

thanks,

gre gk-h

Re: [PATCH] staging: octeon-usb: Prefer using BIT macro

[Greg Kroah-Hartman]

On Mon, Oct 03, 2016 at 11:20:49AM +0300, Aaro Koskinen wrote:
> Hi,
> 
> On Mon, Oct 03, 2016 at 09:01:36AM +0200, Greg Kroah-Hartman wrote:
> > On Mon, Oct 03, 2016 at 03:03:13AM +0300, Aaro Koskinen wrote:
> > > On Mon, Oct 03, 2016 at 12:03:25AM +0530, Namrata A Shettar wrote:
> > > > -	CVMX_USB_INITIALIZE_FLAGS_CLOCK_XO_XI		= 1 << 0,
> > > > -	CVMX_USB_INITIALIZE_FLAGS_CLOCK_XO_GND		= 1 << 1,
> > > > +	CVMX_USB_INITIALIZE_FLAGS_CLOCK_XO_XI		= BIT(0),
> > > > +	CVMX_USB_INITIALIZE_FLAGS_CLOCK_XO_GND		= BIT(1),
> > > >  	CVMX_USB_INITIALIZE_FLAGS_CLOCK_MHZ_MASK	= 3 << 3,
> > > > -	CVMX_USB_INITIALIZE_FLAGS_CLOCK_12MHZ		= 1 << 3,
> > > > +	CVMX_USB_INITIALIZE_FLAGS_CLOCK_12MHZ		= BIT(3),
> > > >  	CVMX_USB_INITIALIZE_FLAGS_CLOCK_24MHZ		= 2 << 3,
> > > >  	CVMX_USB_INITIALIZE_FLAGS_CLOCK_48MHZ		= 3 << 3,
> > > >  	/* Bits 3-4 used to encode the clock frequency */
> > > > -	CVMX_USB_INITIALIZE_FLAGS_NO_DMA		= 1 << 5,
> > > > +	CVMX_USB_INITIALIZE_FLAGS_NO_DMA		= BIT(5),
> > > 
> > > All entries should be converted to use BIT.
> > 
> > The ones that could be, were, right?  What else should be done here?
> 
> 24MHZ is BIT(4).

Oh yeah, nice catch.

> > Well, there are bit mask macros now in the kernel tree, perhaps those
> > should be used, but I don't see how BIT() could be used for "3 << 3", do
> > you?
> 
> BIT(3) | BIT(4)?

Maybe, but it's not an automatic conversion :)

thanks,

greg k-h

Re: [Outreachy kernel] Re: [PATCH] staging: octeon-usb: Prefer using BIT macro

[Greg Kroah-Hartman]

On Mon, Oct 03, 2016 at 11:25:27AM +0200, Greg Kroah-Hartman wrote:
> On Mon, Oct 03, 2016 at 11:20:49AM +0300, Aaro Koskinen wrote:
> > Hi,
> > 
> > On Mon, Oct 03, 2016 at 09:01:36AM +0200, Greg Kroah-Hartman wrote:
> > > On Mon, Oct 03, 2016 at 03:03:13AM +0300, Aaro Koskinen wrote:
> > > > On Mon, Oct 03, 2016 at 12:03:25AM +0530, Namrata A Shettar wrote:
> > > > > -	CVMX_USB_INITIALIZE_FLAGS_CLOCK_XO_XI		= 1 << 0,
> > > > > -	CVMX_USB_INITIALIZE_FLAGS_CLOCK_XO_GND		= 1 << 1,
> > > > > +	CVMX_USB_INITIALIZE_FLAGS_CLOCK_XO_XI		= BIT(0),
> > > > > +	CVMX_USB_INITIALIZE_FLAGS_CLOCK_XO_GND		= BIT(1),
> > > > >  	CVMX_USB_INITIALIZE_FLAGS_CLOCK_MHZ_MASK	= 3 << 3,
> > > > > -	CVMX_USB_INITIALIZE_FLAGS_CLOCK_12MHZ		= 1 << 3,
> > > > > +	CVMX_USB_INITIALIZE_FLAGS_CLOCK_12MHZ		= BIT(3),
> > > > >  	CVMX_USB_INITIALIZE_FLAGS_CLOCK_24MHZ		= 2 << 3,
> > > > >  	CVMX_USB_INITIALIZE_FLAGS_CLOCK_48MHZ		= 3 << 3,
> > > > >  	/* Bits 3-4 used to encode the clock frequency */
> > > > > -	CVMX_USB_INITIALIZE_FLAGS_NO_DMA		= 1 << 5,
> > > > > +	CVMX_USB_INITIALIZE_FLAGS_NO_DMA		= BIT(5),
> > > > 
> > > > All entries should be converted to use BIT.
> > > 
> > > The ones that could be, were, right?  What else should be done here?
> > 
> > 24MHZ is BIT(4).
> 
> Oh yeah, nice catch.
> 
> > > Well, there are bit mask macros now in the kernel tree, perhaps those
> > > should be used, but I don't see how BIT() could be used for "3 << 3", do
> > > you?
> > 
> > BIT(3) | BIT(4)?
> 
> Maybe, but it's not an automatic conversion :)

But it could be, if you use GENMASK() instead, I'm sure you can write up
a transformation rule to do that in an automated way so that you know it
works properly...

thanks,

greg k-h

Re: [Outreachy kernel] Re: [PATCH] staging: octeon-usb: Prefer using BIT macro

[Namrata A Shettar]

On Mon, Oct 03, 2016 at 12:13:12PM +0200, Greg Kroah-Hartman wrote:
> On Mon, Oct 03, 2016 at 11:25:27AM +0200, Greg Kroah-Hartman wrote:
> > On Mon, Oct 03, 2016 at 11:20:49AM +0300, Aaro Koskinen wrote:
> > > Hi,
> > > 
> > > On Mon, Oct 03, 2016 at 09:01:36AM +0200, Greg Kroah-Hartman wrote:
> > > > On Mon, Oct 03, 2016 at 03:03:13AM +0300, Aaro Koskinen wrote:
> > > > > On Mon, Oct 03, 2016 at 12:03:25AM +0530, Namrata A Shettar wrote:
> > > > > > -	CVMX_USB_INITIALIZE_FLAGS_CLOCK_XO_XI		= 1 << 0,
> > > > > > -	CVMX_USB_INITIALIZE_FLAGS_CLOCK_XO_GND		= 1 << 1,
> > > > > > +	CVMX_USB_INITIALIZE_FLAGS_CLOCK_XO_XI		= BIT(0),
> > > > > > +	CVMX_USB_INITIALIZE_FLAGS_CLOCK_XO_GND		= BIT(1),
> > > > > >  	CVMX_USB_INITIALIZE_FLAGS_CLOCK_MHZ_MASK	= 3 << 3,
> > > > > > -	CVMX_USB_INITIALIZE_FLAGS_CLOCK_12MHZ		= 1 << 3,
> > > > > > +	CVMX_USB_INITIALIZE_FLAGS_CLOCK_12MHZ		= BIT(3),
> > > > > >  	CVMX_USB_INITIALIZE_FLAGS_CLOCK_24MHZ		= 2 << 3,
> > > > > >  	CVMX_USB_INITIALIZE_FLAGS_CLOCK_48MHZ		= 3 << 3,
> > > > > >  	/* Bits 3-4 used to encode the clock frequency */
> > > > > > -	CVMX_USB_INITIALIZE_FLAGS_NO_DMA		= 1 << 5,
> > > > > > +	CVMX_USB_INITIALIZE_FLAGS_NO_DMA		= BIT(5),
> > > > > 
> > > > > All entries should be converted to use BIT.
> > > > 
> > > > The ones that could be, were, right?  What else should be done here?
> > > 
> > > 24MHZ is BIT(4).
> > 
> > Oh yeah, nice catch.
> > 
> > > > Well, there are bit mask macros now in the kernel tree, perhaps those
> > > > should be used, but I don't see how BIT() could be used for "3 << 3", do
> > > > you?
> > > 
> > > BIT(3) | BIT(4)?
> > 
> > Maybe, but it's not an automatic conversion :)
> 
> But it could be, if you use GENMASK() instead, I'm sure you can write up
> a transformation rule to do that in an automated way so that you know it
> works properly...
> 
> thanks,
> 
> greg k-h


Yes, that can be done! I'll try doing that and send a revised patch.
Thanks for pointing that out Aaro! and for the help Greg :).

namrata