2015-05-18 19:14:57

by Jagan Teki

[permalink] [raw]
Subject: [PATCH] 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(-)

diff --git a/drivers/block/mg_disk.c b/drivers/block/mg_disk.c
index 145ce2a..c26d430 100644
--- a/drivers/block/mg_disk.c
+++ b/drivers/block/mg_disk.c
@@ -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)
diff --git a/drivers/block/mtip32xx/mtip32xx.c b/drivers/block/mtip32xx/mtip32xx.c
index 3bd7ca9..5224bab 100644
--- a/drivers/block/mtip32xx/mtip32xx.c
+++ b/drivers/block/mtip32xx/mtip32xx.c
@@ -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);
diff --git a/drivers/block/ps3vram.c b/drivers/block/ps3vram.c
index ef45cfb..b621e34 100644
--- a/drivers/block/ps3vram.c
+++ b/drivers/block/ps3vram.c
@@ -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);
diff --git a/drivers/block/skd_main.c b/drivers/block/skd_main.c
index 1e46eb2..100ba71 100644
--- a/drivers/block/skd_main.c
+++ b/drivers/block/skd_main.c
@@ -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
diff --git a/drivers/block/skd_s1120.h b/drivers/block/skd_s1120.h
index 61c757f..80c633f 100644
--- a/drivers/block/skd_s1120.h
+++ b/drivers/block/skd_s1120.h
@@ -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)
diff --git a/drivers/block/sx8.c b/drivers/block/sx8.c
index 5d55285..4a364a8 100644
--- a/drivers/block/sx8.c
+++ b/drivers/block/sx8.c
@@ -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",
--
1.9.1


2015-05-20 18:44:07

by Jagan Teki

[permalink] [raw]
Subject: Re: [PATCH] block: Use BIT macro from include/linux/bitops.h

Ping!

On 19 May 2015 at 00:44, Jagan Teki <[email protected]> wrote:
> 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(-)
>
> diff --git a/drivers/block/mg_disk.c b/drivers/block/mg_disk.c
> index 145ce2a..c26d430 100644
> --- a/drivers/block/mg_disk.c
> +++ b/drivers/block/mg_disk.c
> @@ -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)
> diff --git a/drivers/block/mtip32xx/mtip32xx.c b/drivers/block/mtip32xx/mtip32xx.c
> index 3bd7ca9..5224bab 100644
> --- a/drivers/block/mtip32xx/mtip32xx.c
> +++ b/drivers/block/mtip32xx/mtip32xx.c
> @@ -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);
> diff --git a/drivers/block/ps3vram.c b/drivers/block/ps3vram.c
> index ef45cfb..b621e34 100644
> --- a/drivers/block/ps3vram.c
> +++ b/drivers/block/ps3vram.c
> @@ -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);
> diff --git a/drivers/block/skd_main.c b/drivers/block/skd_main.c
> index 1e46eb2..100ba71 100644
> --- a/drivers/block/skd_main.c
> +++ b/drivers/block/skd_main.c
> @@ -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
> diff --git a/drivers/block/skd_s1120.h b/drivers/block/skd_s1120.h
> index 61c757f..80c633f 100644
> --- a/drivers/block/skd_s1120.h
> +++ b/drivers/block/skd_s1120.h
> @@ -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)
> diff --git a/drivers/block/sx8.c b/drivers/block/sx8.c
> index 5d55285..4a364a8 100644
> --- a/drivers/block/sx8.c
> +++ b/drivers/block/sx8.c
> @@ -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",
> --
> 1.9.1
>

2015-05-20 19:22:55

by Jens Axboe

[permalink] [raw]
Subject: Re: [PATCH] block: Use BIT macro from include/linux/bitops.h

On 05/18/2015 01:14 PM, Jagan Teki wrote:
> Replace (1 << nr) to BIT(nr) where nr = 0, 1, 2 .... 31

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

--
Jens Axboe

2015-05-20 19:41:44

by Jagan Teki

[permalink] [raw]
Subject: Re: [PATCH] block: Use BIT macro from include/linux/bitops.h

On 21 May 2015 at 00:52, Jens Axboe <[email protected]> wrote:
> On 05/18/2015 01:14 PM, Jagan Teki wrote:
>>
>> 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!
--
Jagan Teki,
Openedev.

2015-05-20 19:43:07

by Jens Axboe

[permalink] [raw]
Subject: Re: [PATCH] block: Use BIT macro from include/linux/bitops.h

On 05/20/2015 01:41 PM, Jagan Teki wrote:
> On 21 May 2015 at 00:52, Jens Axboe <[email protected]> wrote:
>> On 05/18/2015 01:14 PM, Jagan Teki wrote:
>>>
>>> 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.

--
Jens Axboe

2015-05-20 19:50:52

by Jagan Teki

[permalink] [raw]
Subject: Re: [PATCH] block: Use BIT macro from include/linux/bitops.h

On 21 May 2015 at 01:13, Jens Axboe <[email protected]> wrote:
> On 05/20/2015 01:41 PM, Jagan Teki wrote:
>>
>> On 21 May 2015 at 00:52, Jens Axboe <[email protected]> wrote:
>>>
>>> On 05/18/2015 01:14 PM, Jagan Teki wrote:
>>>>
>>>>
>>>> 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.

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.

--
Jagan.

2015-05-20 19:53:00

by Jens Axboe

[permalink] [raw]
Subject: Re: [PATCH] block: Use BIT macro from include/linux/bitops.h

On 05/20/2015 01:50 PM, Jagan Teki wrote:
> On 21 May 2015 at 01:13, Jens Axboe <[email protected]> wrote:
>> On 05/20/2015 01:41 PM, Jagan Teki wrote:
>>>
>>> On 21 May 2015 at 00:52, Jens Axboe <[email protected]> wrote:
>>>>
>>>> On 05/18/2015 01:14 PM, Jagan Teki wrote:
>>>>>
>>>>>
>>>>> 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.
>
> 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.


--
Jens Axboe