Re: [PATCH] drivers: staging: vme: Changed (1 << n) to BIT(n)
From: Greg Kroah-Hartman
Date: Tue Nov 24 2015 - 18:28:15 EST
On Sun, Nov 08, 2015 at 11:43:28PM +0200, Andy Shevchenko wrote:
> On Sun, Nov 8, 2015 at 10:39 PM, Egor Uleyskiy <egor.ulieiskii@xxxxxxxxx> wrote:
> > From: Egor Uleyskiy <egor.ulieiskii@xxxxxxxxx>
> >
> > Signed-off-by: Egor Uleyskiy <egor.ulieiskii@xxxxxxxxx>
> > ---
> > drivers/staging/vme/devices/vme_pio2.h | 93 ++++++++++++++++------------------
> > 1 file changed, 45 insertions(+), 48 deletions(-)
> >
> > diff --git a/drivers/staging/vme/devices/vme_pio2.h b/drivers/staging/vme/devices/vme_pio2.h
> > index d5d94c4..29d7a58 100644
> > --- a/drivers/staging/vme/devices/vme_pio2.h
> > +++ b/drivers/staging/vme/devices/vme_pio2.h
> > @@ -1,6 +1,8 @@
> > #ifndef _VME_PIO2_H_
> > #define _VME_PIO2_H_
> >
> > +#include <linux/bitops.h>
> > +
> > #define PIO2_CARDS_MAX 32
> >
> > #define PIO2_VARIANT_LENGTH 5
> > @@ -48,8 +50,6 @@ static const int PIO2_REGS_INT_MASK[8] = { PIO2_REGS_INT_MASK0,
> > PIO2_REGS_INT_MASK6,
> > PIO2_REGS_INT_MASK7 };
> >
> > -
> > -
> > #define PIO2_REGS_CTRL 0x18
> > #define PIO2_REGS_VME_VECTOR 0x19
> > #define PIO2_REGS_CNTR0 0x20
> > @@ -63,7 +63,6 @@ static const int PIO2_REGS_INT_MASK[8] = { PIO2_REGS_INT_MASK0,
> >
> > #define PIO2_REGS_ID 0x30
> >
> > -
> > /* PIO2_REGS_DATAx (0x0 - 0x3) */
> >
> > static const int PIO2_CHANNEL_BANK[32] = { 0, 0, 0, 0, 0, 0, 0, 0,
> > @@ -71,38 +70,38 @@ static const int PIO2_CHANNEL_BANK[32] = { 0, 0, 0, 0, 0, 0, 0, 0,
> > 2, 2, 2, 2, 2, 2, 2, 2,
> > 3, 3, 3, 3, 3, 3, 3, 3 };
> >
> > -#define PIO2_CHANNEL0_BIT (1 << 0)
> > -#define PIO2_CHANNEL1_BIT (1 << 1)
> > -#define PIO2_CHANNEL2_BIT (1 << 2)
> > -#define PIO2_CHANNEL3_BIT (1 << 3)
> > -#define PIO2_CHANNEL4_BIT (1 << 4)
> > -#define PIO2_CHANNEL5_BIT (1 << 5)
> > -#define PIO2_CHANNEL6_BIT (1 << 6)
> > -#define PIO2_CHANNEL7_BIT (1 << 7)
> > -#define PIO2_CHANNEL8_BIT (1 << 0)
> > -#define PIO2_CHANNEL9_BIT (1 << 1)
> > -#define PIO2_CHANNEL10_BIT (1 << 2)
> > -#define PIO2_CHANNEL11_BIT (1 << 3)
> > -#define PIO2_CHANNEL12_BIT (1 << 4)
> > -#define PIO2_CHANNEL13_BIT (1 << 5)
> > -#define PIO2_CHANNEL14_BIT (1 << 6)
> > -#define PIO2_CHANNEL15_BIT (1 << 7)
> > -#define PIO2_CHANNEL16_BIT (1 << 0)
> > -#define PIO2_CHANNEL17_BIT (1 << 1)
> > -#define PIO2_CHANNEL18_BIT (1 << 2)
> > -#define PIO2_CHANNEL19_BIT (1 << 3)
> > -#define PIO2_CHANNEL20_BIT (1 << 4)
> > -#define PIO2_CHANNEL21_BIT (1 << 5)
> > -#define PIO2_CHANNEL22_BIT (1 << 6)
> > -#define PIO2_CHANNEL23_BIT (1 << 7)
> > -#define PIO2_CHANNEL24_BIT (1 << 0)
> > -#define PIO2_CHANNEL25_BIT (1 << 1)
> > -#define PIO2_CHANNEL26_BIT (1 << 2)
> > -#define PIO2_CHANNEL27_BIT (1 << 3)
> > -#define PIO2_CHANNEL28_BIT (1 << 4)
> > -#define PIO2_CHANNEL29_BIT (1 << 5)
> > -#define PIO2_CHANNEL30_BIT (1 << 6)
> > -#define PIO2_CHANNEL31_BIT (1 << 7)
> > +#define PIO2_CHANNEL0_BIT BIT(0)
> > +#define PIO2_CHANNEL1_BIT BIT(1)
> > +#define PIO2_CHANNEL2_BIT BIT(2)
> > +#define PIO2_CHANNEL3_BIT BIT(3)
> > +#define PIO2_CHANNEL4_BIT BIT(4)
> > +#define PIO2_CHANNEL5_BIT BIT(5)
> > +#define PIO2_CHANNEL6_BIT BIT(6)
> > +#define PIO2_CHANNEL7_BIT BIT(7)
> > +#define PIO2_CHANNEL8_BIT BIT(0)
> > +#define PIO2_CHANNEL9_BIT BIT(1)
> > +#define PIO2_CHANNEL10_BIT BIT(2)
> > +#define PIO2_CHANNEL11_BIT BIT(3)
> > +#define PIO2_CHANNEL12_BIT BIT(4)
> > +#define PIO2_CHANNEL13_BIT BIT(5)
> > +#define PIO2_CHANNEL14_BIT BIT(6)
> > +#define PIO2_CHANNEL15_BIT BIT(7)
> > +#define PIO2_CHANNEL16_BIT BIT(0)
> > +#define PIO2_CHANNEL17_BIT BIT(1)
> > +#define PIO2_CHANNEL18_BIT BIT(2)
> > +#define PIO2_CHANNEL19_BIT BIT(3)
> > +#define PIO2_CHANNEL20_BIT BIT(4)
> > +#define PIO2_CHANNEL21_BIT BIT(5)
> > +#define PIO2_CHANNEL22_BIT BIT(6)
> > +#define PIO2_CHANNEL23_BIT BIT(7)
> > +#define PIO2_CHANNEL24_BIT BIT(0)
> > +#define PIO2_CHANNEL25_BIT BIT(1)
> > +#define PIO2_CHANNEL26_BIT BIT(2)
> > +#define PIO2_CHANNEL27_BIT BIT(3)
> > +#define PIO2_CHANNEL28_BIT BIT(4)
> > +#define PIO2_CHANNEL29_BIT BIT(5)
> > +#define PIO2_CHANNEL30_BIT BIT(6)
> > +#define PIO2_CHANNEL31_BIT BIT(7)
> >
> > static const int PIO2_CHANNEL_BIT[32] = { PIO2_CHANNEL0_BIT, PIO2_CHANNEL1_BIT,
> > PIO2_CHANNEL2_BIT, PIO2_CHANNEL3_BIT,
> > @@ -123,12 +122,12 @@ static const int PIO2_CHANNEL_BIT[32] = { PIO2_CHANNEL0_BIT, PIO2_CHANNEL1_BIT,
> > };
> >
> > /* PIO2_REGS_INT_STAT_CNTR (0xc) */
> > -#define PIO2_COUNTER0 (1 << 0)
> > -#define PIO2_COUNTER1 (1 << 1)
> > -#define PIO2_COUNTER2 (1 << 2)
> > -#define PIO2_COUNTER3 (1 << 3)
> > -#define PIO2_COUNTER4 (1 << 4)
> > -#define PIO2_COUNTER5 (1 << 5)
> > +#define PIO2_COUNTER0 BIT(0)
> > +#define PIO2_COUNTER1 BIT(1)
> > +#define PIO2_COUNTER2 BIT(2)
> > +#define PIO2_COUNTER3 BIT(3)
> > +#define PIO2_COUNTER4 BIT(4)
> > +#define PIO2_COUNTER5 BIT(5)
> >
> > static const int PIO2_COUNTER[6] = { PIO2_COUNTER0, PIO2_COUNTER1,
> > PIO2_COUNTER2, PIO2_COUNTER3,
> > @@ -136,8 +135,8 @@ static const int PIO2_COUNTER[6] = { PIO2_COUNTER0, PIO2_COUNTER1,
> >
> > /* PIO2_REGS_CTRL (0x18) */
> > #define PIO2_VME_INT_MASK 0x7
> > -#define PIO2_LED (1 << 6)
> > -#define PIO2_LOOP (1 << 7)
> > +#define PIO2_LED BIT(6)
> > +#define PIO2_LOOP BIT(7)
> >
> > /* PIO2_REGS_VME_VECTOR (0x19) */
> > #define PIO2_VME_VECTOR_SPUR 0x0
> > @@ -182,7 +181,7 @@ static const int PIO2_CNTR_CTRL[6] = { PIO2_REGS_CTRL_WRD0,
> > PIO2_REGS_CTRL_WRD1 };
> >
> > #define PIO2_CNTR_SC_DEV0 0
> > -#define PIO2_CNTR_SC_DEV1 (1 << 6)
> > +#define PIO2_CNTR_SC_DEV1 BIT(6)
> > #define PIO2_CNTR_SC_DEV2 (2 << 6)
> > #define PIO2_CNTR_SC_RDBACK (3 << 6)
>
> With the first parts which are an excellent clean up, this one makes
> two styles out of one.
> Greg, what would you suggest to do? For my opinion in such cases
> direct values or previous syntax looks better.
Yes, this should not be changed.
thanks,
greg k-h
--
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/