block: Use BIT macro from include/linux/bitops.h

PATCH In review Jagan Teki 2 participants 7 messages May 18, 2015 – May 20, 2015

Subsystems (4)

PowerPC Block layer PS3 platform support PS3VRAM driver

lore

Share & tools

lore

Jagan Teki

author

2015-05-18 19:14 UTC #0 lore raw

PATCH 1/1 block: Use BIT macro from include/linux/bitops.h

Replace (1 << nr) to BIT(nr) where nr = 0, 1, 2 .... 31

Signed-off-by Jagan Teki <[email protected]>

Cc Wolfram Sang <[email protected]>

Cc Jens Axboe <[email protected]>

---
 drivers/block/mg_disk.c           | 10 +++++-----
 drivers/block/mtip32xx/mtip32xx.c | 14 +++++++-------
 drivers/block/ps3vram.c           |  4 ++--
 drivers/block/skd_main.c          |  2 +-
 drivers/block/skd_s1120.h         |  4 ++--
 drivers/block/sx8.c               | 34 +++++++++++++++++-----------------
 6 files changed, 34 insertions(+), 34 deletions(-)

drivers/block/mg_disk.c +5 -5

index 145ce2a..c26d430 100644
@@ -76,10 +76,10 @@
 #define MG_CMD_RD_CONF 0x40
 
 /* operation mode */
-#define MG_OP_CASCADE (1 << 0)
-#define MG_OP_CASCADE_SYNC_RD (1 << 1)
-#define MG_OP_CASCADE_SYNC_WR (1 << 2)
-#define MG_OP_INTERLEAVE (1 << 3)
+#define MG_OP_CASCADE BIT(0)
+#define MG_OP_CASCADE_SYNC_RD BIT(1)
+#define MG_OP_CASCADE_SYNC_WR BIT(2)
+#define MG_OP_INTERLEAVE BIT(3)
 
 /* synchronous */
 #define MG_BURST_LAT_4 (3 << 4)

@@ -87,7 +87,7 @@
 #define MG_BURST_LAT_6 (5 << 4)
 #define MG_BURST_LAT_7 (6 << 4)
 #define MG_BURST_LAT_8 (7 << 4)
-#define MG_BURST_LEN_4 (1 << 1)
+#define MG_BURST_LEN_4 BIT(1)
 #define MG_BURST_LEN_8 (2 << 1)
 #define MG_BURST_LEN_16 (3 << 1)
 #define MG_BURST_LEN_32 (4 << 1)

drivers/block/mtip32xx/mtip32xx.c +7 -7

index 3bd7ca9..5224bab 100644
@@ -69,7 +69,7 @@
 #define CMD_DMA_ALLOC_SZ        (AHCI_CMD_TBL_SGL_SZ + AHCI_CMD_TBL_HDR_SZ)
 
 
-#define HOST_CAP_NZDMA		(1 << 19)
+#define HOST_CAP_NZDMA		BIT(19)
 #define HOST_HSORG		0xFC
 #define HSORG_DISABLE_SLOTGRP_INTR (1<<24)
 #define HSORG_DISABLE_SLOTGRP_PXIS (1<<16)

@@ -859,13 +859,13 @@ static inline void mtip_process_errors(struct driver_data *dd, u32 port_stat)
 	if (unlikely(port_stat & PORT_IRQ_CONNECT)) {
 		dev_warn(&dd->pdev->dev,
 			"Clearing PxSERR.DIAG.x\n");
-		writel((1 << 26), dd->port->mmio + PORT_SCR_ERR);
+		writel(BIT(26), dd->port->mmio + PORT_SCR_ERR);
 	}
 
 	if (unlikely(port_stat & PORT_IRQ_PHYRDY)) {
 		dev_warn(&dd->pdev->dev,
 			"Clearing PxSERR.DIAG.n\n");
-		writel((1 << 16), dd->port->mmio + PORT_SCR_ERR);
+		writel(BIT(16), dd->port->mmio + PORT_SCR_ERR);
 	}
 
 	if (unlikely(port_stat & ~PORT_IRQ_HANDLED)) {

@@ -1158,7 +1158,7 @@ static int mtip_exec_internal_command(struct mtip_port *port,
 			__force_bit2int cpu_to_le32((buffer >> 16) >> 16);
 
 		int_cmd->command_header->opts |=
-			__force_bit2int cpu_to_le32((1 << 16));
+			__force_bit2int cpu_to_le32(BIT(16));
 	}
 
 	/* Populate the command header */

@@ -1390,7 +1390,7 @@ static int mtip_get_identify(struct mtip_port *port, void __user *user_buffer)
 
 #ifdef MTIP_TRIM /* Disabling TRIM support temporarily */
 	/* Demux ID.DRAT & ID.RZAT to determine trim support */
-	if (port->identify[69] & (1 << 14) && port->identify[69] & (1 << 5))
+	if (port->identify[69] & BIT(14) && port->identify[69] & BIT(5))
 		port->dd->trim_supp = true;
 	else
 #endif

@@ -4232,8 +4232,8 @@ static void mtip_disable_link_opts(struct driver_data *dd, struct pci_dev *pdev)
 		pci_read_config_word(pdev,
 			pos + PCI_EXP_DEVCTL,
 			&pcie_dev_ctrl);
-		if (pcie_dev_ctrl & (1 << 11) ||
-		    pcie_dev_ctrl & (1 << 4)) {
+		if (pcie_dev_ctrl & BIT(11) ||
+		    pcie_dev_ctrl & BIT(4)) {
 			dev_info(&dd->pdev->dev,
 				"Disabling ERO/No-Snoop on bridge device %04x:%04x\n",
 					pdev->vendor, pdev->device);

drivers/block/ps3vram.c +2 -2

index ef45cfb..b621e34 100644
@@ -261,7 +261,7 @@ static int ps3vram_upload(struct ps3_system_bus_device *dev,
 	ps3vram_out_ring(priv, len);
 	ps3vram_out_ring(priv, len);
 	ps3vram_out_ring(priv, count);
-	ps3vram_out_ring(priv, (1 << 8) | 1);
+	ps3vram_out_ring(priv, BIT(8) | 1);
 	ps3vram_out_ring(priv, 0);
 
 	ps3vram_notifier_reset(dev);

@@ -293,7 +293,7 @@ static int ps3vram_download(struct ps3_system_bus_device *dev,
 	ps3vram_out_ring(priv, len);
 	ps3vram_out_ring(priv, len);
 	ps3vram_out_ring(priv, count);
-	ps3vram_out_ring(priv, (1 << 8) | 1);
+	ps3vram_out_ring(priv, BIT(8) | 1);
 	ps3vram_out_ring(priv, 0);
 
 	ps3vram_notifier_reset(dev);

drivers/block/skd_main.c +1 -1

index 1e46eb2..100ba71 100644
@@ -89,7 +89,7 @@ MODULE_VERSION(DRV_VERSION "-" DRV_BUILD_ID);
 #define PCI_VENDOR_ID_STEC      0x1B39
 #define PCI_DEVICE_ID_S1120     0x0001
 
-#define SKD_FUA_NV		(1 << 1)
+#define SKD_FUA_NV		BIT(1)
 #define SKD_MINORS_PER_DEVICE   16
 
 #define SKD_MAX_QUEUE_DEPTH     200u

drivers/block/skd_s1120.h +2 -2

index 61c757f..80c633f 100644
@@ -47,7 +47,7 @@
 #define FIT_STATUS			0x510u
 #define FIT_SR_DRIVE_STATE_MASK		0x000000FFu
 #define	FIT_SR_SIGNATURE		(0xFF << 8)
-#define	FIT_SR_PIO_DMA			(1 << 16)
+#define	FIT_SR_PIO_DMA			BIT(16)
 #define FIT_SR_DRIVE_OFFLINE		0x00
 #define FIT_SR_DRIVE_INIT		0x01
 /* #define FIT_SR_DRIVE_READY		0x02 */

@@ -70,7 +70,7 @@
  */
 #define FIT_SR_STATE_MASK		(0xFF << 0)
 #define FIT_SR_SIGNATURE		(0xFF << 8)
-#define FIT_SR_PIO_DMA			(1 << 16)
+#define FIT_SR_PIO_DMA			BIT(16)
 
 /*
  * Interrupt status, 32-bit r/w1c (w1c ==> write 1 to clear)

drivers/block/sx8.c +17 ~~-17~~

index 5d55285..4a364a8 100644
@@ -135,10 +135,10 @@ enum {
 
 	/* bits in CARM_INT_{STAT,MASK} */
 	INT_RESERVED		= 0xfffffff0,
-	INT_WATCHDOG		= (1 << 3),	/* watchdog timer */
-	INT_Q_OVERFLOW		= (1 << 2),	/* cmd msg q overflow */
-	INT_Q_AVAILABLE		= (1 << 1),	/* cmd msg q has free space */
-	INT_RESPONSE		= (1 << 0),	/* response msg available */
+	INT_WATCHDOG		= BIT(3),	/* watchdog timer */
+	INT_Q_OVERFLOW		= BIT(2),	/* cmd msg q overflow */
+	INT_Q_AVAILABLE		= BIT(1),	/* cmd msg q has free space */
+	INT_RESPONSE		= BIT(0),	/* response msg available */
 	INT_ACK_MASK		= INT_WATCHDOG | INT_Q_OVERFLOW,
 	INT_DEF_MASK		= INT_RESERVED | INT_Q_OVERFLOW |
 				  INT_RESPONSE,

@@ -153,11 +153,11 @@ enum {
 	CARM_MSG_IOCTL		= 6,
 	CARM_MSG_ARRAY		= 8,
 	CARM_MSG_MISC		= 9,
-	CARM_CME		= (1 << 2),
-	CARM_RME		= (1 << 1),
-	CARM_WZBC		= (1 << 0),
-	CARM_RMI		= (1 << 0),
-	CARM_Q_FULL		= (1 << 3),
+	CARM_CME		= BIT(2),
+	CARM_RME		= BIT(1),
+	CARM_WZBC		= BIT(0),
+	CARM_RMI		= BIT(0),
+	CARM_Q_FULL		= BIT(3),
 	CARM_MSG_SIZE		= 288,
 	CARM_Q_LEN		= 48,

@@ -172,7 +172,7 @@ enum {
 	/* CARM_MSG_ARRAY messages */
 	CARM_ARRAY_INFO		= 0,
 
-	ARRAY_NO_EXIST		= (1 << 31),
+	ARRAY_NO_EXIST		= BIT(31),
 
 	/* response messages */
 	RMSG_SZ			= 8,	/* sizeof(struct carm_response) */

@@ -189,16 +189,16 @@ enum {
 	MISC_SET_TIME		= 5,
 
 	/* MISC_GET_FW_VER feature bits */
-	FW_VER_4PORT		= (1 << 2), /* 1=4 ports, 0=8 ports */
-	FW_VER_NON_RAID		= (1 << 1), /* 1=non-RAID firmware, 0=RAID */
-	FW_VER_ZCR		= (1 << 0), /* zero channel RAID (whatever that is) */
+	FW_VER_4PORT		= BIT(2), /* 1=4 ports, 0=8 ports */
+	FW_VER_NON_RAID		= BIT(1), /* 1=non-RAID firmware, 0=RAID */
+	FW_VER_ZCR		= BIT(0), /* zero channel RAID (whatever that is) */
 
 	/* carm_host flags */
 	FL_NON_RAID		= FW_VER_NON_RAID,
 	FL_4PORT		= FW_VER_4PORT,
 	FL_FW_VER_MASK		= (FW_VER_NON_RAID | FW_VER_4PORT),
-	FL_DAC			= (1 << 16),
-	FL_DYN_MAJOR		= (1 << 17),
+	FL_DAC			= BIT(16),
+	FL_DYN_MAJOR		= BIT(17),
 };
 
 enum {

@@ -1178,14 +1178,14 @@ static inline void carm_handle_responses(struct carm_host *host)
 		}
 
 		/* response to a message we sent */
-		else if ((status & (1 << 31)) == 0) {
+		else if ((status & BIT(31)) == 0) {
 			VPRINTK("handling msg response on index %u\n", idx);
 			carm_handle_resp(host, resp[idx].ret_handle, status);
 			resp[idx].status = cpu_to_le32(0xffffffff);
 		}
 
 		/* asynchronous events the hardware throws our way */
-		else if ((status & 0xff000000) == (1 << 31)) {
+		else if ((status & 0xff000000) == BIT(31)) {
 			u8 *evt_type_ptr = (u8 *) &resp[idx];
 			u8 evt_type = *evt_type_ptr;
 			printk(KERN_WARNING DRV_NAME "(%s): unhandled event type %d\n",

Signature · 1 line

1.9.1

Jens Axboe

maintainer

2015-05-20 19:22 UTC #2 lore raw

In reply to Jagan Teki

Quoted 1 line · Jagan Teki

Replace (1 << nr) to BIT(nr) where nr = 0, 1, 2 .... 31

I don't like it, I think it hurts readability.

Signature · 1 line

Jens Axboe

Jagan Teki

author

2015-05-20 19:41 UTC #3 lore raw

Quoted 6 lines · Jens Axboe

Quoted 2 lines · Jagan Teki
Replace (1 << nr) to BIT(nr) where nr = 0, 1, 2 .... 31
I don't like it, I think it hurts readability.

What do you mean by don't like, using kernel defined macro instead of numerical assignments huts readability?

thanks!

Signature · 2 lines

Jagan Teki,
Openedev.

Jens Axboe

maintainer

2015-05-20 19:43 UTC #4 lore raw

Quoted 10 lines · Jagan Teki

Quoted 6 lines · Jens Axboe
Quoted 2 lines · Jagan Teki
Replace (1 << nr) to BIT(nr) where nr = 0, 1, 2 .... 31
I don't like it, I think it hurts readability.
What do you mean by don't like, using kernel defined macro instead of numerical assignments huts readability?

In the context of the patch, BIT(0) == (1 << 0) is obvious. But if I just came across BIT(7) in the code, I'd have to check, whereas anyone would immediately know that (1 << 7) is the 7th bit set. Hence, readability is worse, and that's important.

Signature · 1 line

Jens Axboe

Jagan Teki

author

2015-05-20 19:50 UTC #5 lore raw

Quoted 14 lines · Jens Axboe

Quoted 13 lines · Jagan Teki
Quoted 9 lines · Jens Axboe
Quoted 3 lines · Jagan Teki
Replace (1 << nr) to BIT(nr) where nr = 0, 1, 2 .... 31
I don't like it, I think it hurts readability.

Quoted 8 lines

Quoted 2 lines
What do you mean by don't like, using kernel defined macro instead of numerical assignments huts readability?
In the context of the patch, BIT(0) == (1 << 0) is obvious. But if I just came across BIT(7) in the code, I'd have to check, whereas anyone would immediately know that (1 << 7) is the 7th bit set. Hence, readability is worse, and that's important.

I don't how that BIT(7) is tricky to understand as BIT(0) implies to be set 0th bit.
If understanding of BIT(0) is same like to be as BIT(7) and these were simplified
macro's used most of the code in kernel.

Signature · 1 line

Jagan.

Jens Axboe

maintainer

2015-05-20 19:53 UTC #6 lore raw

Quoted 24 lines · Jagan Teki

Quoted 14 lines · Jens Axboe
Quoted 13 lines · Jagan Teki
Quoted 9 lines · Jens Axboe
Quoted 3 lines · Jagan Teki
Replace (1 << nr) to BIT(nr) where nr = 0, 1, 2 .... 31
I don't like it, I think it hurts readability.
Quoted 8 lines
Quoted 2 lines
What do you mean by don't like, using kernel defined macro instead of numerical assignments huts readability?
In the context of the patch, BIT(0) == (1 << 0) is obvious. But if I just came across BIT(7) in the code, I'd have to check, whereas anyone would immediately know that (1 << 7) is the 7th bit set. Hence, readability is worse, and that's important.

Quoted 5 lines

I don't how that BIT(7) is tricky to understand as BIT(0) implies to be set 0th bit.
If understanding of BIT(0) is same like to be as BIT(7) and these were simplified
macro's used most of the code in kernel.

Well of course, if you know what BIT(7) is, you know what BIT(0) is. My point is that I don't know what either of them are, I'd have to look it up. Whereas anyone would immediately know what (1 << 7) or (1 << 0) is without having to look further.

Signature · 1 line

Jens Axboe