[PATCH] nvme: use BIT_MASK and GENMASK for NVME definitions

[PATCH] nvme: use BIT_MASK and GENMASK for NVME definitions

[Tokunori Ikegami]

Only some definitions used GENMASK so unify to use it.

Signed-off-by: Tokunori Ikegami <ikegami.t@gmail.com>
---
 include/linux/nvme.h | 218 +++++++++++++++++++++----------------------
 1 file changed, 109 insertions(+), 109 deletions(-)

diff --git a/include/linux/nvme.h b/include/linux/nvme.h
index b58d9405d65e..b454e370b8b9 100644
--- a/include/linux/nvme.h
+++ b/include/linux/nvme.h
@@ -177,29 +177,28 @@ enum {
 #define NVME_CRTO_CRWMT(crto)	((crto) & 0xffff)
 
 enum {
-	NVME_CMBSZ_SQS		= 1 << 0,
-	NVME_CMBSZ_CQS		= 1 << 1,
-	NVME_CMBSZ_LISTS	= 1 << 2,
-	NVME_CMBSZ_RDS		= 1 << 3,
-	NVME_CMBSZ_WDS		= 1 << 4,
+	NVME_CMBSZ_SQS		= BIT_MASK(0),
+	NVME_CMBSZ_CQS		= BIT_MASK(1),
+	NVME_CMBSZ_LISTS	= BIT_MASK(2),
+	NVME_CMBSZ_RDS		= BIT_MASK(3),
+	NVME_CMBSZ_WDS		= BIT_MASK(4),
 
 	NVME_CMBSZ_SZ_SHIFT	= 12,
-	NVME_CMBSZ_SZ_MASK	= 0xfffff,
+	NVME_CMBSZ_SZ_MASK	= GENMASK(19, 0),
 
 	NVME_CMBSZ_SZU_SHIFT	= 8,
-	NVME_CMBSZ_SZU_MASK	= 0xf,
+	NVME_CMBSZ_SZU_MASK	= GENMASK(3, 0),
 };
 
 /*
  * Submission and Completion Queue Entry Sizes for the NVM command set.
  * (In bytes and specified as a power of two (2^n)).
  */
-#define NVME_ADM_SQES       6
+#define NVME_ADM_SQES		6
 #define NVME_NVM_IOSQES		6
 #define NVME_NVM_IOCQES		4
 
 enum {
-	NVME_CC_ENABLE		= 1 << 0,
 	NVME_CC_EN_SHIFT	= 0,
 	NVME_CC_CSS_SHIFT	= 4,
 	NVME_CC_MPS_SHIFT	= 7,
@@ -207,45 +206,46 @@ enum {
 	NVME_CC_SHN_SHIFT	= 14,
 	NVME_CC_IOSQES_SHIFT	= 16,
 	NVME_CC_IOCQES_SHIFT	= 20,
-	NVME_CC_CSS_NVM		= 0 << NVME_CC_CSS_SHIFT,
+	NVME_CC_ENABLE		= BIT_MASK(NVME_CC_EN_SHIFT),
+	NVME_CC_CSS_NVM		= 0,
 	NVME_CC_CSS_CSI		= 6 << NVME_CC_CSS_SHIFT,
-	NVME_CC_CSS_MASK	= 7 << NVME_CC_CSS_SHIFT,
-	NVME_CC_AMS_RR		= 0 << NVME_CC_AMS_SHIFT,
-	NVME_CC_AMS_WRRU	= 1 << NVME_CC_AMS_SHIFT,
+	NVME_CC_CSS_MASK	= GENMASK(NVME_CC_CSS_SHIFT + 2, NVME_CC_CSS_SHIFT),
+	NVME_CC_AMS_RR		= 0,
+	NVME_CC_AMS_WRRU	= BIT_MASK(NVME_CC_AMS_SHIFT),
 	NVME_CC_AMS_VS		= 7 << NVME_CC_AMS_SHIFT,
-	NVME_CC_SHN_NONE	= 0 << NVME_CC_SHN_SHIFT,
-	NVME_CC_SHN_NORMAL	= 1 << NVME_CC_SHN_SHIFT,
+	NVME_CC_SHN_NONE	= 0,
+	NVME_CC_SHN_NORMAL	= BIT_MASK(NVME_CC_SHN_SHIFT),
 	NVME_CC_SHN_ABRUPT	= 2 << NVME_CC_SHN_SHIFT,
-	NVME_CC_SHN_MASK	= 3 << NVME_CC_SHN_SHIFT,
+	NVME_CC_SHN_MASK	= GENMASK(NVME_CC_SHN_SHIFT + 1, NVME_CC_SHN_SHIFT),
 	NVME_CC_IOSQES		= NVME_NVM_IOSQES << NVME_CC_IOSQES_SHIFT,
 	NVME_CC_IOCQES		= NVME_NVM_IOCQES << NVME_CC_IOCQES_SHIFT,
-	NVME_CC_CRIME		= 1 << 24,
+	NVME_CC_CRIME		= BIT_MASK(24),
 };
 
 enum {
-	NVME_CSTS_RDY		= 1 << 0,
-	NVME_CSTS_CFS		= 1 << 1,
-	NVME_CSTS_NSSRO		= 1 << 4,
-	NVME_CSTS_PP		= 1 << 5,
-	NVME_CSTS_SHST_NORMAL	= 0 << 2,
-	NVME_CSTS_SHST_OCCUR	= 1 << 2,
+	NVME_CSTS_RDY		= BIT_MASK(0),
+	NVME_CSTS_CFS		= BIT_MASK(1),
+	NVME_CSTS_NSSRO		= BIT_MASK(4),
+	NVME_CSTS_PP		= BIT_MASK(5),
+	NVME_CSTS_SHST_NORMAL	= 0,
+	NVME_CSTS_SHST_OCCUR	= BIT_MASK(2),
 	NVME_CSTS_SHST_CMPLT	= 2 << 2,
-	NVME_CSTS_SHST_MASK	= 3 << 2,
+	NVME_CSTS_SHST_MASK	= GENMASK(3, 2),
 };
 
 enum {
-	NVME_CMBMSC_CRE		= 1 << 0,
-	NVME_CMBMSC_CMSE	= 1 << 1,
+	NVME_CMBMSC_CRE		= BIT_MASK(0),
+	NVME_CMBMSC_CMSE	= BIT_MASK(1),
 };
 
 enum {
-	NVME_CAP_CSS_NVM	= 1 << 0,
-	NVME_CAP_CSS_CSI	= 1 << 6,
+	NVME_CAP_CSS_NVM	= BIT_MASK(0),
+	NVME_CAP_CSS_CSI	= BIT_MASK(6),
 };
 
 enum {
-	NVME_CAP_CRMS_CRWMS	= 1ULL << 59,
-	NVME_CAP_CRMS_CRIMS	= 1ULL << 60,
+	NVME_CAP_CRMS_CRWMS	= BIT_ULL_MASK(59),
+	NVME_CAP_CRMS_CRIMS	= BIT_ULL_MASK(60),
 };
 
 struct nvme_id_power_state {
@@ -267,14 +267,14 @@ struct nvme_id_power_state {
 };
 
 enum {
-	NVME_PS_FLAGS_MAX_POWER_SCALE	= 1 << 0,
-	NVME_PS_FLAGS_NON_OP_STATE	= 1 << 1,
+	NVME_PS_FLAGS_MAX_POWER_SCALE	= BIT_MASK(0),
+	NVME_PS_FLAGS_NON_OP_STATE	= BIT_MASK(1),
 };
 
 enum nvme_ctrl_attr {
-	NVME_CTRL_ATTR_HID_128_BIT	= (1 << 0),
-	NVME_CTRL_ATTR_TBKAS		= (1 << 6),
-	NVME_CTRL_ATTR_ELBAS		= (1 << 15),
+	NVME_CTRL_ATTR_HID_128_BIT	= BIT_MASK(0),
+	NVME_CTRL_ATTR_TBKAS		= BIT_MASK(6),
+	NVME_CTRL_ATTR_ELBAS		= BIT_MASK(15),
 };
 
 struct nvme_id_ctrl {
@@ -365,29 +365,29 @@ struct nvme_id_ctrl {
 };
 
 enum {
-	NVME_CTRL_CMIC_MULTI_PORT		= 1 << 0,
-	NVME_CTRL_CMIC_MULTI_CTRL		= 1 << 1,
-	NVME_CTRL_CMIC_ANA			= 1 << 3,
-	NVME_CTRL_ONCS_COMPARE			= 1 << 0,
-	NVME_CTRL_ONCS_WRITE_UNCORRECTABLE	= 1 << 1,
-	NVME_CTRL_ONCS_DSM			= 1 << 2,
-	NVME_CTRL_ONCS_WRITE_ZEROES		= 1 << 3,
-	NVME_CTRL_ONCS_RESERVATIONS		= 1 << 5,
-	NVME_CTRL_ONCS_TIMESTAMP		= 1 << 6,
-	NVME_CTRL_VWC_PRESENT			= 1 << 0,
-	NVME_CTRL_OACS_SEC_SUPP                 = 1 << 0,
-	NVME_CTRL_OACS_NS_MNGT_SUPP		= 1 << 3,
-	NVME_CTRL_OACS_DIRECTIVES		= 1 << 5,
-	NVME_CTRL_OACS_DBBUF_SUPP		= 1 << 8,
-	NVME_CTRL_LPA_CMD_EFFECTS_LOG		= 1 << 1,
-	NVME_CTRL_CTRATT_128_ID			= 1 << 0,
-	NVME_CTRL_CTRATT_NON_OP_PSP		= 1 << 1,
-	NVME_CTRL_CTRATT_NVM_SETS		= 1 << 2,
-	NVME_CTRL_CTRATT_READ_RECV_LVLS		= 1 << 3,
-	NVME_CTRL_CTRATT_ENDURANCE_GROUPS	= 1 << 4,
-	NVME_CTRL_CTRATT_PREDICTABLE_LAT	= 1 << 5,
-	NVME_CTRL_CTRATT_NAMESPACE_GRANULARITY	= 1 << 7,
-	NVME_CTRL_CTRATT_UUID_LIST		= 1 << 9,
+	NVME_CTRL_CMIC_MULTI_PORT		= BIT_MASK(0),
+	NVME_CTRL_CMIC_MULTI_CTRL		= BIT_MASK(1),
+	NVME_CTRL_CMIC_ANA			= BIT_MASK(3),
+	NVME_CTRL_ONCS_COMPARE			= BIT_MASK(0),
+	NVME_CTRL_ONCS_WRITE_UNCORRECTABLE	= BIT_MASK(1),
+	NVME_CTRL_ONCS_DSM			= BIT_MASK(2),
+	NVME_CTRL_ONCS_WRITE_ZEROES		= BIT_MASK(3),
+	NVME_CTRL_ONCS_RESERVATIONS		= BIT_MASK(5),
+	NVME_CTRL_ONCS_TIMESTAMP		= BIT_MASK(6),
+	NVME_CTRL_VWC_PRESENT			= BIT_MASK(0),
+	NVME_CTRL_OACS_SEC_SUPP			= BIT_MASK(0),
+	NVME_CTRL_OACS_NS_MNGT_SUPP		= BIT_MASK(3),
+	NVME_CTRL_OACS_DIRECTIVES		= BIT_MASK(5),
+	NVME_CTRL_OACS_DBBUF_SUPP		= BIT_MASK(8),
+	NVME_CTRL_LPA_CMD_EFFECTS_LOG		= BIT_MASK(1),
+	NVME_CTRL_CTRATT_128_ID			= BIT_MASK(0),
+	NVME_CTRL_CTRATT_NON_OP_PSP		= BIT_MASK(1),
+	NVME_CTRL_CTRATT_NVM_SETS		= BIT_MASK(2),
+	NVME_CTRL_CTRATT_READ_RECV_LVLS		= BIT_MASK(3),
+	NVME_CTRL_CTRATT_ENDURANCE_GROUPS	= BIT_MASK(4),
+	NVME_CTRL_CTRATT_PREDICTABLE_LAT	= BIT_MASK(5),
+	NVME_CTRL_CTRATT_NAMESPACE_GRANULARITY	= BIT_MASK(7),
+	NVME_CTRL_CTRATT_UUID_LIST		= BIT_MASK(9),
 };
 
 struct nvme_lbaf {
@@ -482,12 +482,12 @@ struct nvme_id_ns_nvm {
 };
 
 enum {
-	NVME_ID_NS_NVM_STS_MASK		= 0x7f,
+	NVME_ID_NS_NVM_STS_MASK		= GENMASK(6, 0),
 	NVME_ID_NS_NVM_GUARD_SHIFT	= 7,
-	NVME_ID_NS_NVM_GUARD_MASK	= 0x3,
+	NVME_ID_NS_NVM_GUARD_MASK	= GENMASK(1, 0),
 	NVME_ID_NS_NVM_QPIF_SHIFT	= 9,
-	NVME_ID_NS_NVM_QPIF_MASK	= 0xf,
-	NVME_ID_NS_NVM_QPIFS		= 1 << 3,
+	NVME_ID_NS_NVM_QPIF_MASK	= GENMASK(3, 0),
+	NVME_ID_NS_NVM_QPIFS		= BIT_MASK(3),
 };
 
 static inline __u8 nvme_elbaf_sts(__u32 elbaf)
@@ -551,33 +551,33 @@ enum {
 };
 
 enum {
-	NVME_NS_FEAT_THIN	= 1 << 0,
-	NVME_NS_FEAT_ATOMICS	= 1 << 1,
-	NVME_NS_FEAT_IO_OPT	= 1 << 4,
-	NVME_NS_ATTR_RO		= 1 << 0,
-	NVME_NS_FLBAS_LBA_MASK	= 0xf,
+	NVME_NS_FEAT_THIN	= BIT_MASK(0),
+	NVME_NS_FEAT_ATOMICS	= BIT_MASK(1),
+	NVME_NS_FEAT_IO_OPT	= BIT_MASK(4),
+	NVME_NS_ATTR_RO		= BIT_MASK(0),
+	NVME_NS_FLBAS_LBA_MASK	= GENMASK(3, 0),
 	NVME_NS_FLBAS_LBA_UMASK	= 0x60,
 	NVME_NS_FLBAS_LBA_SHIFT	= 1,
 	NVME_NS_FLBAS_META_EXT	= 0x10,
-	NVME_NS_NMIC_SHARED	= 1 << 0,
+	NVME_NS_NMIC_SHARED	= BIT_MASK(0),
 	NVME_LBAF_RP_BEST	= 0,
 	NVME_LBAF_RP_BETTER	= 1,
 	NVME_LBAF_RP_GOOD	= 2,
 	NVME_LBAF_RP_DEGRADED	= 3,
-	NVME_NS_DPC_PI_LAST	= 1 << 4,
-	NVME_NS_DPC_PI_FIRST	= 1 << 3,
-	NVME_NS_DPC_PI_TYPE3	= 1 << 2,
-	NVME_NS_DPC_PI_TYPE2	= 1 << 1,
-	NVME_NS_DPC_PI_TYPE1	= 1 << 0,
-	NVME_NS_DPS_PI_FIRST	= 1 << 3,
-	NVME_NS_DPS_PI_MASK	= 0x7,
+	NVME_NS_DPC_PI_LAST	= BIT_MASK(4),
+	NVME_NS_DPC_PI_FIRST	= BIT_MASK(3),
+	NVME_NS_DPC_PI_TYPE3	= BIT_MASK(2),
+	NVME_NS_DPC_PI_TYPE2	= BIT_MASK(1),
+	NVME_NS_DPC_PI_TYPE1	= BIT_MASK(0),
+	NVME_NS_DPS_PI_FIRST	= BIT_MASK(3),
+	NVME_NS_DPS_PI_MASK	= GENMASK(2, 0),
 	NVME_NS_DPS_PI_TYPE1	= 1,
 	NVME_NS_DPS_PI_TYPE2	= 2,
 	NVME_NS_DPS_PI_TYPE3	= 3,
 };
 
 enum {
-	NVME_NSTAT_NRDY		= 1 << 0,
+	NVME_NSTAT_NRDY		= BIT_MASK(0),
 };
 
 enum {
@@ -595,8 +595,8 @@ static inline __u8 nvme_lbaf_index(__u8 flbas)
 
 /* Identify Namespace Metadata Capabilities (MC): */
 enum {
-	NVME_MC_EXTENDED_LBA	= (1 << 0),
-	NVME_MC_METADATA_PTR	= (1 << 1),
+	NVME_MC_EXTENDED_LBA	= BIT_MASK(0),
+	NVME_MC_METADATA_PTR	= BIT_MASK(1),
 };
 
 struct nvme_ns_id_desc {
@@ -653,14 +653,14 @@ struct nvme_fw_slot_info_log {
 };
 
 enum {
-	NVME_CMD_EFFECTS_CSUPP		= 1 << 0,
-	NVME_CMD_EFFECTS_LBCC		= 1 << 1,
-	NVME_CMD_EFFECTS_NCC		= 1 << 2,
-	NVME_CMD_EFFECTS_NIC		= 1 << 3,
-	NVME_CMD_EFFECTS_CCC		= 1 << 4,
+	NVME_CMD_EFFECTS_CSUPP		= BIT_MASK(0),
+	NVME_CMD_EFFECTS_LBCC		= BIT_MASK(1),
+	NVME_CMD_EFFECTS_NCC		= BIT_MASK(2),
+	NVME_CMD_EFFECTS_NIC		= BIT_MASK(3),
+	NVME_CMD_EFFECTS_CCC		= BIT_MASK(4),
 	NVME_CMD_EFFECTS_CSER_MASK	= GENMASK(15, 14),
 	NVME_CMD_EFFECTS_CSE_MASK	= GENMASK(18, 16),
-	NVME_CMD_EFFECTS_UUID_SEL	= 1 << 19,
+	NVME_CMD_EFFECTS_UUID_SEL	= BIT_MASK(19),
 	NVME_CMD_EFFECTS_SCOPE_MASK	= GENMASK(31, 20),
 };
 
@@ -992,9 +992,9 @@ struct nvme_rw_command {
 };
 
 enum {
-	NVME_RW_LR			= 1 << 15,
-	NVME_RW_FUA			= 1 << 14,
-	NVME_RW_APPEND_PIREMAP		= 1 << 9,
+	NVME_RW_LR			= BIT_MASK(15),
+	NVME_RW_FUA			= BIT_MASK(14),
+	NVME_RW_APPEND_PIREMAP		= BIT_MASK(9),
 	NVME_RW_DSM_FREQ_UNSPEC		= 0,
 	NVME_RW_DSM_FREQ_TYPICAL	= 1,
 	NVME_RW_DSM_FREQ_RARE		= 2,
@@ -1004,18 +1004,18 @@ enum {
 	NVME_RW_DSM_FREQ_ONCE		= 6,
 	NVME_RW_DSM_FREQ_PREFETCH	= 7,
 	NVME_RW_DSM_FREQ_TEMP		= 8,
-	NVME_RW_DSM_LATENCY_NONE	= 0 << 4,
-	NVME_RW_DSM_LATENCY_IDLE	= 1 << 4,
+	NVME_RW_DSM_LATENCY_NONE	= 0,
+	NVME_RW_DSM_LATENCY_IDLE	= BIT_MASK(4),
 	NVME_RW_DSM_LATENCY_NORM	= 2 << 4,
 	NVME_RW_DSM_LATENCY_LOW		= 3 << 4,
-	NVME_RW_DSM_SEQ_REQ		= 1 << 6,
-	NVME_RW_DSM_COMPRESSED		= 1 << 7,
-	NVME_RW_PRINFO_PRCHK_REF	= 1 << 10,
-	NVME_RW_PRINFO_PRCHK_APP	= 1 << 11,
-	NVME_RW_PRINFO_PRCHK_GUARD	= 1 << 12,
-	NVME_RW_PRINFO_PRACT		= 1 << 13,
-	NVME_RW_DTYPE_STREAMS		= 1 << 4,
-	NVME_WZ_DEAC			= 1 << 9,
+	NVME_RW_DSM_SEQ_REQ		= BIT_MASK(6),
+	NVME_RW_DSM_COMPRESSED		= BIT_MASK(7),
+	NVME_RW_PRINFO_PRCHK_REF	= BIT_MASK(10),
+	NVME_RW_PRINFO_PRCHK_APP	= BIT_MASK(11),
+	NVME_RW_PRINFO_PRCHK_GUARD	= BIT_MASK(12),
+	NVME_RW_PRINFO_PRACT		= BIT_MASK(13),
+	NVME_RW_DTYPE_STREAMS		= BIT_MASK(4),
+	NVME_WZ_DEAC			= BIT_MASK(9),
 };
 
 struct nvme_dsm_cmd {
@@ -1118,9 +1118,9 @@ enum {
 /* Features */
 
 enum {
-	NVME_TEMP_THRESH_MASK		= 0xffff,
+	NVME_TEMP_THRESH_MASK		= GENMASK(15, 0),
 	NVME_TEMP_THRESH_SELECT_SHIFT	= 16,
-	NVME_TEMP_THRESH_TYPE_UNDER	= 0x100000,
+	NVME_TEMP_THRESH_TYPE_UNDER	= BIT_MASK(20),
 };
 
 struct nvme_feat_auto_pst {
@@ -1128,8 +1128,8 @@ struct nvme_feat_auto_pst {
 };
 
 enum {
-	NVME_HOST_MEM_ENABLE	= (1 << 0),
-	NVME_HOST_MEM_RETURN	= (1 << 1),
+	NVME_HOST_MEM_ENABLE	= BIT_MASK(0),
+	NVME_HOST_MEM_RETURN	= BIT_MASK(1),
 };
 
 struct nvme_feat_host_behavior {
@@ -1209,10 +1209,10 @@ enum nvme_admin_opcode {
 		nvme_admin_opcode_name(nvme_admin_get_lba_status))
 
 enum {
-	NVME_QUEUE_PHYS_CONTIG	= (1 << 0),
-	NVME_CQ_IRQ_ENABLED	= (1 << 1),
-	NVME_SQ_PRIO_URGENT	= (0 << 1),
-	NVME_SQ_PRIO_HIGH	= (1 << 1),
+	NVME_QUEUE_PHYS_CONTIG	= BIT_MASK(0),
+	NVME_CQ_IRQ_ENABLED	= BIT_MASK(1),
+	NVME_SQ_PRIO_URGENT	= 0,
+	NVME_SQ_PRIO_HIGH	= BIT_MASK(1),
 	NVME_SQ_PRIO_MEDIUM	= (2 << 1),
 	NVME_SQ_PRIO_LOW	= (3 << 1),
 	NVME_FEAT_ARBITRATION	= 0x01,
@@ -1256,8 +1256,8 @@ enum {
 	NVME_LOG_ANA		= 0x0c,
 	NVME_LOG_DISC		= 0x70,
 	NVME_LOG_RESERVATION	= 0x80,
-	NVME_FWACT_REPL		= (0 << 3),
-	NVME_FWACT_REPL_ACTV	= (1 << 3),
+	NVME_FWACT_REPL		= 0,
+	NVME_FWACT_REPL_ACTV	= BIT_MASK(3),
 	NVME_FWACT_ACTV		= (2 << 3),
 };
 
@@ -2004,8 +2004,8 @@ enum {
 	NVME_SC_HOST_PATH_ERROR		= 0x370,
 	NVME_SC_HOST_ABORTED_CMD	= 0x371,
 
-	NVME_SC_MASK			= 0x00ff, /* Status Code */
-	NVME_SCT_MASK			= 0x0700, /* Status Code Type */
+	NVME_SC_MASK			= GENMASK(7, 0), /* Status Code */
+	NVME_SCT_MASK			= GENMASK(10, 8), /* Status Code Type */
 	NVME_SCT_SC_MASK		= NVME_SCT_MASK | NVME_SC_MASK,
 
 	NVME_STATUS_CRD			= 0x1800, /* Command Retry Delayed */
-- 
2.45.2

Re: [PATCH] nvme: use BIT_MASK and GENMASK for NVME definitions

[Christoph Hellwig]

> -	NVME_CMBSZ_CQS		= 1 << 1,
> -	NVME_CMBSZ_LISTS	= 1 << 2,
> -	NVME_CMBSZ_RDS		= 1 << 3,
> -	NVME_CMBSZ_WDS		= 1 << 4,
> +	NVME_CMBSZ_SQS		= BIT_MASK(0),
> +	NVME_CMBSZ_CQS		= BIT_MASK(1),
> +	NVME_CMBSZ_LISTS	= BIT_MASK(2),
> +	NVME_CMBSZ_RDS		= BIT_MASK(3),
> +	NVME_CMBSZ_WDS		= BIT_MASK(4),

Nothjing genmask here, and a lot less readable for no good reason at
all.

>  	NVME_CMBSZ_SZ_SHIFT	= 12,
> -	NVME_CMBSZ_SZ_MASK	= 0xfffff,
> +	NVME_CMBSZ_SZ_MASK	= GENMASK(19, 0),

This is using the GENMASK that you mentioned, and now I actually
need to look up what GENMASK does to decipher the previously perfectly
understandable code.

Could people please stop sending or suggesting cleanups that make
the code much worse?

Re: [PATCH] nvme: use BIT_MASK and GENMASK for NVME definitions

[Tokunori Ikegami]

On 2024/11/12 13:26, Christoph Hellwig wrote:
>> -	NVME_CMBSZ_CQS		= 1 << 1,
>> -	NVME_CMBSZ_LISTS	= 1 << 2,
>> -	NVME_CMBSZ_RDS		= 1 << 3,
>> -	NVME_CMBSZ_WDS		= 1 << 4,
>> +	NVME_CMBSZ_SQS		= BIT_MASK(0),
>> +	NVME_CMBSZ_CQS		= BIT_MASK(1),
>> +	NVME_CMBSZ_LISTS	= BIT_MASK(2),
>> +	NVME_CMBSZ_RDS		= BIT_MASK(3),
>> +	NVME_CMBSZ_WDS		= BIT_MASK(4),
> Nothjing genmask here, and a lot less readable for no good reason at
> all.
Is it still no good to change BIT_MASK() to BIT()?
>>   	NVME_CMBSZ_SZ_SHIFT	= 12,
>> -	NVME_CMBSZ_SZ_MASK	= 0xfffff,
>> +	NVME_CMBSZ_SZ_MASK	= GENMASK(19, 0),
> This is using the GENMASK that you mentioned, and now I actually
> need to look up what GENMASK does to decipher the previously perfectly
> understandable code.
-       NVME_CMBSZ_SZ_SHIFT     = 12,
-       NVME_CMBSZ_SZ_MASK      = 0xfffff,
+       NVME_CMBSZ_SZ_MASK      = GENMASK(31, 12),
Is this also still not understandable?
> Could people please stop sending or suggesting cleanups that make
> the code much worse?

Sorry for bothering you. Also the changes were not considered enough but 
still seems better to use the existing bit operation macros instead of 
the current implementation. Also thinking to change to use FIELD_GET() 
and FIELD_PREP() macros. If possible to make sure let me reconfirm your 
opinion for above. Thank you.

Re: [PATCH] nvme: use BIT_MASK and GENMASK for NVME definitions

[Christoph Hellwig]

On Thu, Nov 14, 2024 at 12:14:33AM +0900, Tokunori Ikegami wrote:
> On 2024/11/12 13:26, Christoph Hellwig wrote:
> > > -	NVME_CMBSZ_CQS		= 1 << 1,
> > > -	NVME_CMBSZ_LISTS	= 1 << 2,
> > > -	NVME_CMBSZ_RDS		= 1 << 3,
> > > -	NVME_CMBSZ_WDS		= 1 << 4,
> > > +	NVME_CMBSZ_SQS		= BIT_MASK(0),
> > > +	NVME_CMBSZ_CQS		= BIT_MASK(1),
> > > +	NVME_CMBSZ_LISTS	= BIT_MASK(2),
> > > +	NVME_CMBSZ_RDS		= BIT_MASK(3),
> > > +	NVME_CMBSZ_WDS		= BIT_MASK(4),
> > Nothjing genmask here, and a lot less readable for no good reason at
> > all.
> Is it still no good to change BIT_MASK() to BIT()?

No, why would it be?

Re: [PATCH] nvme: use BIT_MASK and GENMASK for NVME definitions

[Tokunori Ikegami]

On 2024/11/14 14:14, Christoph Hellwig wrote:
> No, why would it be?

Understood. (The reason is BIT() used more than BIT_MASK() in kernel and 
also still I think better to use the macro instead of the shift operator 
as below.) Thank you.
   #define BIT(nr)                     (UL(1) << (nr))
...
-       NVME_CMBSZ_SQS          = 1 << 0,
-       NVME_CMBSZ_CQS          = 1 << 1,
-       NVME_CMBSZ_LISTS        = 1 << 2,
-       NVME_CMBSZ_RDS          = 1 << 3,
-       NVME_CMBSZ_WDS          = 1 << 4,
+       NVME_CMBSZ_SQS          = BIT(0),
+       NVME_CMBSZ_CQS          = BIT(1),
+       NVME_CMBSZ_LISTS        = BIT(2),
+       NVME_CMBSZ_RDS          = BIT(3),
+       NVME_CMBSZ_WDS          = BIT(4),

Re: [PATCH] nvme: use BIT_MASK and GENMASK for NVME definitions

[Christoph Hellwig]

On Thu, Nov 14, 2024 at 11:43:22PM +0900, Tokunori Ikegami wrote:
> On 2024/11/14 14:14, Christoph Hellwig wrote:
> > No, why would it be?
> 
> Understood. (The reason is BIT() used more than BIT_MASK() in kernel and
> also still I think better to use the macro instead of the shift operator as
> below.) Thank you.

And this still make the code less readable.  1 << 2 is totally
obvious to any one with the most basic knowledge of C.  BIT(1) OTOH
requires looking up the definition.