nvme: add wrapper for nvme ctrl register r/w

PATCH New weiping zhang 4 participants 7 messages Jun 29, 2017 – Jul 14, 2017 NVMe

lore

Share & tools

lore

weiping zhang

author

2017-06-29 17:26 UTC #0 lore raw

PATCH 1/1 nvme: add wrapper for nvme ctrl register r/w

Simplify nvme ctrl register r/w code.

Signed-off-by weiping zhang <zhangweiping at didichuxing.com>

---
 drivers/nvme/host/core.c | 14 +++++++-------
 drivers/nvme/host/nvme.h | 22 ++++++++++++++++++++--
 2 files changed, 27 insertions(+), 9 deletions(-)

drivers/nvme/host/core.c +7 -7

index 903d581..f73fe65 100644
@@ -1205,7 +1205,7 @@ static int nvme_wait_ready(struct nvme_ctrl *ctrl, u64 cap, bool enabled)
 	u32 csts, bit = enabled ? NVME_CSTS_RDY : 0;
 	int ret;
 
-	while ((ret = ctrl->ops->reg_read32(ctrl, NVME_REG_CSTS, &csts)) == 0) {
+	while ((ret = nvme_ctrl_reg_read32(ctrl, NVME_REG_CSTS, &csts)) == 0) {
 		if (csts == ~0)
 			return -ENODEV;
 		if ((csts & NVME_CSTS_RDY) == bit)

@@ -1238,7 +1238,7 @@ int nvme_disable_ctrl(struct nvme_ctrl *ctrl, u64 cap)
 	ctrl->ctrl_config &= ~NVME_CC_SHN_MASK;
 	ctrl->ctrl_config &= ~NVME_CC_ENABLE;
 
-	ret = ctrl->ops->reg_write32(ctrl, NVME_REG_CC, ctrl->ctrl_config);
+	ret = nvme_ctrl_reg_write32(ctrl, NVME_REG_CC, ctrl->ctrl_config);
 	if (ret)
 		return ret;

@@ -1274,7 +1274,7 @@ int nvme_enable_ctrl(struct nvme_ctrl *ctrl, u64 cap)
 	ctrl->ctrl_config |= NVME_CC_IOSQES | NVME_CC_IOCQES;
 	ctrl->ctrl_config |= NVME_CC_ENABLE;
 
-	ret = ctrl->ops->reg_write32(ctrl, NVME_REG_CC, ctrl->ctrl_config);
+	ret = nvme_ctrl_reg_write32(ctrl, NVME_REG_CC, ctrl->ctrl_config);
 	if (ret)
 		return ret;
 	return nvme_wait_ready(ctrl, cap, true);

@@ -1290,11 +1290,11 @@ int nvme_shutdown_ctrl(struct nvme_ctrl *ctrl)
 	ctrl->ctrl_config &= ~NVME_CC_SHN_MASK;
 	ctrl->ctrl_config |= NVME_CC_SHN_NORMAL;
 
-	ret = ctrl->ops->reg_write32(ctrl, NVME_REG_CC, ctrl->ctrl_config);
+	ret = nvme_ctrl_reg_write32(ctrl, NVME_REG_CC, ctrl->ctrl_config);
 	if (ret)
 		return ret;
 
-	while ((ret = ctrl->ops->reg_read32(ctrl, NVME_REG_CSTS, &csts)) == 0) {
+	while ((ret = nvme_ctrl_reg_read32(ctrl, NVME_REG_CSTS, &csts)) == 0) {
 		if ((csts & NVME_CSTS_SHST_MASK) == NVME_CSTS_SHST_CMPLT)
 			break;

@@ -1541,13 +1541,13 @@ int nvme_init_identify(struct nvme_ctrl *ctrl)
 	u32 max_hw_sectors;
 	u8 prev_apsta;
 
-	ret = ctrl->ops->reg_read32(ctrl, NVME_REG_VS, &ctrl->vs);
+	ret = nvme_ctrl_reg_read32(ctrl, NVME_REG_VS, &ctrl->vs);
 	if (ret) {
 		dev_err(ctrl->device, "Reading VS failed (%d)\n", ret);
 		return ret;
 	}
 
-	ret = ctrl->ops->reg_read64(ctrl, NVME_REG_CAP, &cap);
+	ret = nvme_ctrl_reg_read64(ctrl, NVME_REG_CAP, &cap);
 	if (ret) {
 		dev_err(ctrl->device, "Reading CAP failed (%d)\n", ret);
 		return ret;

drivers/nvme/host/nvme.h +20 -2

index 9d6a070..c6b6ced 100644
@@ -222,11 +222,29 @@ struct nvme_ctrl_ops {
 	int (*get_address)(struct nvme_ctrl *ctrl, char *buf, int size);
 };
 
+static inline int nvme_ctrl_reg_read32(struct nvme_ctrl *ctrl, u32 off,
+			u32 *val)
+{
+	return ctrl->ops->reg_read32(ctrl, off, val);
+}
+
+static inline int nvme_ctrl_reg_write32(struct nvme_ctrl *ctrl, u32 off,
+			u32 val)
+{
+	return ctrl->ops->reg_write32(ctrl, off, val);
+}
+
+static inline int nvme_ctrl_reg_read64(struct nvme_ctrl *ctrl, u32 off,
+			u64 *val)
+{
+	return ctrl->ops->reg_read64(ctrl, off, val);
+}
+
 static inline bool nvme_ctrl_ready(struct nvme_ctrl *ctrl)
 {
 	u32 val = 0;
 
-	if (ctrl->ops->reg_read32(ctrl, NVME_REG_CSTS, &val))
+	if (nvme_ctrl_reg_read32(ctrl, NVME_REG_CSTS, &val))
 		return false;
 	return val & NVME_CSTS_RDY;
 }

@@ -235,7 +253,7 @@ static inline int nvme_reset_subsystem(struct nvme_ctrl *ctrl)
 {
 	if (!ctrl->subsystem)
 		return -ENOTTY;
-	return ctrl->ops->reg_write32(ctrl, NVME_REG_NSSR, 0x4E564D65);
+	return nvme_ctrl_reg_write32(ctrl, NVME_REG_NSSR, 0x4E564D65);
 }
 
 static inline u64 nvme_block_nr(struct nvme_ns *ns, sector_t sector)

Signature · 1 line

2.9.4

Christoph Hellwig

maintainer

2017-07-12 07:20 UTC #1 lore raw

PATCH 1/1 nvme: add wrapper for nvme ctrl register r/w

Quoted 3 lines · weiping zhang

Simplify nvme ctrl register r/w code.
Signed-off-by weiping zhang <zhangweiping at didichuxing.com>

This seems generally useful, especially with more consolidation of code to the core. Sagi - do you want to integrate this into your consolidation series and fixup any clashes?

Sagi Grimberg

maintainer

2017-07-12 10:34 UTC #2 lore raw

PATCH 1/1 nvme: add wrapper for nvme ctrl register r/w

Quoted 8 lines

Quoted 4 lines
Simplify nvme ctrl register r/w code.
Signed-off-by weiping zhang <zhangweiping at didichuxing.com>
This seems generally useful, especially with more consolidation of code to the core. Sagi - do you want to integrate this into your consolidation series and fixup any clashes?

Not sure whats special about register read/write that they deserve a wrapper, but its easy enough to get in before my patches if I'll see a review on it.

Christoph Hellwig

maintainer

2017-07-12 11:55 UTC #3 lore raw

PATCH 1/1 nvme: add wrapper for nvme ctrl register r/w

Quoted 3 lines · Sagi Grimberg

Not sure whats special about register read/write that they deserve a wrapper, but its easy enough to get in before my patches if I'll see a review on it.

Mostly because it's a little shorter and we're only going to get more and more of these calls. In the end I don't feel strongly and we can just skip the patch for now and reconsider it once we have more register read/write calls.

Johannes Thumshirn

2017-07-12 12:01 UTC #4 lore raw

PATCH 1/1 nvme: add wrapper for nvme ctrl register r/w

Quoted 4 lines · Christoph Hellwig

Mostly because it's a little shorter and we're only going to get more and more of these calls. In the end I don't feel strongly and we can just skip the patch for now and reconsider it once we have more register read/write calls.

I'm in favour of the patch as well. It hides the transport specific implementation details quite good.

Reviewed-by Johannes Thumshirn <jthumshirn at suse.de>

Signature · 6 lines

Johannes Thumshirn                                          Storage
jthumshirn at suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg
GF: Felix Imend?rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N?rnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

weiping zhang

author

2017-07-12 13:25 UTC #5 lore raw

PATCH 1/1 nvme: add wrapper for nvme ctrl register r/w

Quoted 9 lines · Johannes Thumshirn

Quoted 4 lines · Christoph Hellwig
Mostly because it's a little shorter and we're only going to get more and more of these calls. In the end I don't feel strongly and we can just skip the patch for now and reconsider it once we have more register read/write calls.
I'm in favour of the patch as well. It hides the transport specific implementation details quite good.

Thanks all of your suggestions, I just want add a common interface of register read/write. If someday we change the path of read/write register, we only need modify nvme_ctrl_reg_xxx than every ctrl->ops->xxx.

weiping zhang

author

2017-07-14 09:57 UTC #6 lore raw

PATCH 1/1 nvme: add wrapper for nvme ctrl register r/w

In reply to Johannes Thumshirn

Quoted 6 lines · Johannes Thumshirn

Quoted 4 lines · Christoph Hellwig
Mostly because it's a little shorter and we're only going to get more and more of these calls. In the end I don't feel strongly and we can just skip the patch for now and reconsider it once we have more register read/write calls.

Quoted 7 lines

I'm in favour of the patch as well. It hides the transport specific implementation details quite good.
Reviewed-by Johannes Thumshirn <jthumshirn at suse.de>
Signature · 2 lines
Johannes Thumshirn                                          Storage
jthumshirn at suse.de                                +49 911 74053 689

Hi Sagi,

Johannes has reviewed this patch, do you want include this patch ?

--
Weiping