From fc8d861892d8626967dbeea9ba2d321f0482a5e8 Mon Sep 17 00:00:00 2001 From: Jim Harris Date: Tue, 8 Jun 2021 22:15:28 +0000 Subject: [PATCH] nvme: add new SET_EN_0 state for ctrlr initialization This removes some code that was duplicated in the CHECK_EN and DISABLE_WAIT_FOR_READY_1 states. Signed-off-by: Jim Harris Signed-off-by: Konrad Sztyber Change-Id: Ie5d175540f71c692f7784c7ff22a48f34b9b7082 Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/8614 Tested-by: SPDK CI Jenkins Community-CI: Broadcom CI Community-CI: Mellanox Build Bot Reviewed-by: Shuhei Matsumoto Reviewed-by: Aleksey Marchuk --- lib/nvme/nvme_ctrlr.c | 76 ++++++++----------- lib/nvme/nvme_internal.h | 5 ++ .../lib/nvme/nvme_ctrlr.c/nvme_ctrlr_ut.c | 4 +- 3 files changed, 41 insertions(+), 44 deletions(-) diff --git a/lib/nvme/nvme_ctrlr.c b/lib/nvme/nvme_ctrlr.c index f34b07daa..3ad96d8dd 100644 --- a/lib/nvme/nvme_ctrlr.c +++ b/lib/nvme/nvme_ctrlr.c @@ -1237,6 +1237,8 @@ nvme_ctrlr_state_string(enum nvme_ctrlr_state state) return "check en"; case NVME_CTRLR_STATE_DISABLE_WAIT_FOR_READY_1: return "disable and wait for CSTS.RDY = 1"; + case NVME_CTRLR_STATE_SET_EN_0: + return "set CC.EN = 0"; case NVME_CTRLR_STATE_DISABLE_WAIT_FOR_READY_0: return "disable and wait for CSTS.RDY = 0"; case NVME_CTRLR_STATE_ENABLE: @@ -3462,56 +3464,44 @@ nvme_ctrlr_process_init(struct spdk_nvme_ctrlr *ctrlr) /* Begin the hardware initialization by making sure the controller is disabled. */ if (cc.bits.en) { NVME_CTRLR_DEBUGLOG(ctrlr, "CC.EN = 1\n"); - /* - * Controller is currently enabled. We need to disable it to cause a reset. - * - * If CC.EN = 1 && CSTS.RDY = 0, the controller is in the process of becoming ready. - * Wait for the ready bit to be 1 before disabling the controller. - */ - if (csts.bits.rdy == 0) { - NVME_CTRLR_DEBUGLOG(ctrlr, "CC.EN = 1 && CSTS.RDY = 0 - waiting for reset to complete\n"); - nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_DISABLE_WAIT_FOR_READY_1, ready_timeout_in_ms); - return 0; - } - - /* CC.EN = 1 && CSTS.RDY == 1, so we can immediately disable the controller. */ - NVME_CTRLR_DEBUGLOG(ctrlr, "Setting CC.EN = 0\n"); - cc.bits.en = 0; - if (nvme_ctrlr_set_cc(ctrlr, &cc)) { - NVME_CTRLR_ERRLOG(ctrlr, "set_cc() failed\n"); - return -EIO; - } - nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_DISABLE_WAIT_FOR_READY_0, ready_timeout_in_ms); - - /* - * Wait 2.5 seconds before accessing PCI registers. - * Not using sleep() to avoid blocking other controller's initialization. - */ - if (ctrlr->quirks & NVME_QUIRK_DELAY_BEFORE_CHK_RDY) { - NVME_CTRLR_DEBUGLOG(ctrlr, "Applying quirk: delay 2.5 seconds before reading registers\n"); - ctrlr->sleep_timeout_tsc = ticks + (2500 * spdk_get_ticks_hz() / 1000); - } - return 0; + nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_DISABLE_WAIT_FOR_READY_1, ready_timeout_in_ms); } else { nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_DISABLE_WAIT_FOR_READY_0, ready_timeout_in_ms); - return 0; } - break; + return 0; case NVME_CTRLR_STATE_DISABLE_WAIT_FOR_READY_1: + /* + * Controller is currently enabled. We need to disable it to cause a reset. + * + * If CC.EN = 1 && CSTS.RDY = 0, the controller is in the process of becoming ready. + * Wait for the ready bit to be 1 before disabling the controller. + */ if (csts.bits.rdy == 1) { - NVME_CTRLR_DEBUGLOG(ctrlr, "CC.EN = 1 && CSTS.RDY = 1 - disabling controller\n"); - /* CC.EN = 1 && CSTS.RDY = 1, so we can set CC.EN = 0 now. */ - NVME_CTRLR_DEBUGLOG(ctrlr, "Setting CC.EN = 0\n"); - cc.bits.en = 0; - if (nvme_ctrlr_set_cc(ctrlr, &cc)) { - NVME_CTRLR_ERRLOG(ctrlr, "set_cc() failed\n"); - return -EIO; - } - nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_DISABLE_WAIT_FOR_READY_0, ready_timeout_in_ms); - return 0; + nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_SET_EN_0, ready_timeout_in_ms); + } else { + NVME_CTRLR_DEBUGLOG(ctrlr, "CC.EN = 1 && CSTS.RDY = 0 - waiting for reset to complete\n"); } - break; + return 0; + + case NVME_CTRLR_STATE_SET_EN_0: + NVME_CTRLR_DEBUGLOG(ctrlr, "Setting CC.EN = 0\n"); + cc.bits.en = 0; + if (nvme_ctrlr_set_cc(ctrlr, &cc)) { + NVME_CTRLR_ERRLOG(ctrlr, "set_cc() failed\n"); + return -EIO; + } + nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_DISABLE_WAIT_FOR_READY_0, ready_timeout_in_ms); + + /* + * Wait 2.5 seconds before accessing PCI registers. + * Not using sleep() to avoid blocking other controller's initialization. + */ + if (ctrlr->quirks & NVME_QUIRK_DELAY_BEFORE_CHK_RDY) { + NVME_CTRLR_DEBUGLOG(ctrlr, "Applying quirk: delay 2.5 seconds before reading registers\n"); + ctrlr->sleep_timeout_tsc = ticks + (2500 * spdk_get_ticks_hz() / 1000); + } + return 0; case NVME_CTRLR_STATE_DISABLE_WAIT_FOR_READY_0: if (csts.bits.rdy == 0) { diff --git a/lib/nvme/nvme_internal.h b/lib/nvme/nvme_internal.h index 721c6b942..fb9319e69 100644 --- a/lib/nvme/nvme_internal.h +++ b/lib/nvme/nvme_internal.h @@ -578,6 +578,11 @@ enum nvme_ctrlr_state { */ NVME_CTRLR_STATE_DISABLE_WAIT_FOR_READY_1, + /** + * Disabling the controller by setting CC.EN to 0. + */ + NVME_CTRLR_STATE_SET_EN_0, + /** * Waiting for CSTS.RDY to transition from 1 to 0 so that CC.EN may be set to 1. */ diff --git a/test/unit/lib/nvme/nvme_ctrlr.c/nvme_ctrlr_ut.c b/test/unit/lib/nvme/nvme_ctrlr.c/nvme_ctrlr_ut.c index 28ea74909..40a475e82 100644 --- a/test/unit/lib/nvme/nvme_ctrlr.c/nvme_ctrlr_ut.c +++ b/test/unit/lib/nvme/nvme_ctrlr.c/nvme_ctrlr_ut.c @@ -724,6 +724,8 @@ test_nvme_ctrlr_init_en_1_rdy_0(void) */ g_ut_nvme_regs.csts.bits.rdy = 1; CU_ASSERT(nvme_ctrlr_process_init(&ctrlr) == 0); + CU_ASSERT(ctrlr.state == NVME_CTRLR_STATE_SET_EN_0); + CU_ASSERT(nvme_ctrlr_process_init(&ctrlr) == 0); CU_ASSERT(ctrlr.state == NVME_CTRLR_STATE_DISABLE_WAIT_FOR_READY_0); CU_ASSERT(g_ut_nvme_regs.cc.bits.en == 0); @@ -777,7 +779,7 @@ test_nvme_ctrlr_init_en_1_rdy_1(void) ctrlr.cdata.nn = 1; ctrlr.page_size = 0x1000; CU_ASSERT(ctrlr.state == NVME_CTRLR_STATE_INIT); - while (ctrlr.state != NVME_CTRLR_STATE_CHECK_EN) { + while (ctrlr.state != NVME_CTRLR_STATE_SET_EN_0) { CU_ASSERT(nvme_ctrlr_process_init(&ctrlr) == 0); } CU_ASSERT(nvme_ctrlr_process_init(&ctrlr) == 0);