mgmt: ec_host_cmd: shi_npcx: support the enhanced mode

The original SHI module only has one output FIFO buffer. It costs a lot
when the driver has to send/change the protocol control code because it
must fill out all 128 bytes of output FIFO. In npcx4, we introduce
another output buffer in 1-byte depth. These two buffers can switch back
and forth during the transaction. We can use the single-byte buffer
to send the control code and the 128-byte FIFO to send the data payload.
It helps improve the SHI driver's efficiency.

Signed-off-by: Jun Lin <[email protected]>

soc/arm/nuvoton_npcx/common/reg/reg_def.h

@@ -1677,4 +1677,6 @@ struct shi_reg {
 #define NPCX_SHICFG6_EBUFMD              0
 #define NPCX_SHICFG6_OBUF_SL             1
 
+#define IBF_IBHF_EN_MASK                 (BIT(NPCX_EVENABLE_IBFEN) | BIT(NPCX_EVENABLE_IBHFEN))
+
 #endif /* _NUVOTON_NPCX_REG_DEF_H */

subsys/mgmt/ec_host_cmd/backends/Kconfig

@@ -53,7 +53,8 @@ choice EC_HOST_CMD_BACKEND_SHI_DRIVER
 
 config EC_HOST_CMD_BACKEND_SHI_NPCX
 	bool "SHI by Nuvoton"
-	depends on DT_HAS_NUVOTON_NPCX_SHI_ENABLED
+	depends on DT_HAS_NUVOTON_NPCX_SHI_ENABLED || \
+		   DT_HAS_NUVOTON_NPCX_SHI_ENHANCED_ENABLED
 	help
 	  This option enables the driver for SHI backend in the
 	  Nuvoton NPCX chip.
@@ -67,6 +68,13 @@ config EC_HOST_CMD_BACKEND_SHI_ITE
 
 endchoice
 
+config EC_HOST_CMD_BACKEND_SHI_NPCX_ENHANCED_BUF_MODE
+	def_bool DT_HAS_NUVOTON_NPCX_SHI_ENHANCED_ENABLED
+	help
+	  In this mode, besides the original 128-bytes FIFO, an additional
+	  single-byte output buffer can be selected/switched to generate a
+	  response to simultaneous Read/Write transactions.
+
 config EC_HOST_CMD_BACKEND_SHI_MAX_REQUEST
 	int "Max data size for the version 3 request packet"
 	default 544 if EC_HOST_CMD_BACKEND_SHI_NPCX

subsys/mgmt/ec_host_cmd/backends/ec_host_cmd_backend_shi_npcx.c

@@ -4,8 +4,6 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-#define DT_DRV_COMPAT nuvoton_npcx_shi
-
 #include "ec_host_cmd_backend_shi.h"
 
 #include <zephyr/drivers/clock_control.h>
@@ -19,7 +17,14 @@
 
 #include <soc_miwu.h>
 
+#if DT_HAS_COMPAT_STATUS_OKAY(nuvoton_npcx_shi)
+#define DT_DRV_COMPAT nuvoton_npcx_shi
+#elif DT_HAS_COMPAT_STATUS_OKAY(nuvoton_npcx_shi_enhanced)
+#define DT_DRV_COMPAT nuvoton_npcx_shi_enhanced
+#endif
 BUILD_ASSERT(DT_NUM_INST_STATUS_OKAY(DT_DRV_COMPAT) == 1, "Invalid number of NPCX SHI peripherals");
+BUILD_ASSERT(!(DT_HAS_COMPAT_STATUS_OKAY(nuvoton_npcx_shi) &&
+	       DT_HAS_COMPAT_STATUS_OKAY(nuvoton_npcx_shi_enhanced)));
 
 LOG_MODULE_REGISTER(host_cmd_shi_npcx, CONFIG_EC_HC_LOG_LEVEL);
 
@@ -152,15 +157,15 @@ struct ec_host_cmd_shi_npcx_ctx {
 static void shi_npcx_reset_prepare(const struct device *dev);
 
 static void shi_npcx_pm_policy_state_lock_get(struct shi_npcx_data *data,
-					       enum shi_npcx_pm_policy_state_flag flag)
+					      enum shi_npcx_pm_policy_state_flag flag)
 {
 	if (atomic_test_and_set_bit(data->pm_policy_state_flag, flag) == 0) {
 		pm_policy_state_lock_get(PM_STATE_SUSPEND_TO_IDLE, PM_ALL_SUBSTATES);
 	}
 }
 
 static void shi_npcx_pm_policy_state_lock_put(struct shi_npcx_data *data,
-					    enum shi_npcx_pm_policy_state_flag flag)
+					      enum shi_npcx_pm_policy_state_flag flag)
 {
 	if (atomic_test_and_clear_bit(data->pm_policy_state_flag, flag) == 1) {
 		pm_policy_state_lock_put(PM_STATE_SUSPEND_TO_IDLE, PM_ALL_SUBSTATES);
@@ -180,13 +185,40 @@ static uint32_t shi_npcx_read_buf_pointer(struct shi_reg *const inst)
 	return (uint32_t)stat;
 }
 
+/*
+ * Write pointer of output buffer by consecutive reading
+ * Note: this function (OBUFSTAT) should only be usd in Enhanced Buffer Mode.
+ */
+static uint32_t shi_npcx_write_buf_pointer(struct shi_reg *const inst)
+{
+	uint8_t stat;
+
+	/* Wait for two consecutive equal values are read */
+	do {
+		stat = inst->OBUFSTAT;
+	} while (stat != inst->OBUFSTAT);
+
+	return stat;
+}
+
 /*
  * Valid offset of SHI output buffer to write.
- * When SIMUL bit is set, IBUFPTR can be used instead of OBUFPTR
+ * - In Simultaneous Standard FIFO Mode (SIMUL = 1 and EBUFMD = 0):
+ *   OBUFPTR cannot be used. IBUFPTR can be used instead because it points to
+ *   the same location as OBUFPTR.
+ * - In Simultaneous Enhanced FIFO Mode (SIMUL = 1 and EBUFMD = 1):
+ *   IBUFPTR may not point to the same location as OBUFPTR.
+ *   In this case OBUFPTR reflects the 128-byte payload buffer pointer only
+ *   during the SPI transaction.
  */
 static uint32_t shi_npcx_valid_obuf_offset(struct shi_reg *const inst)
 {
-	return (shi_npcx_read_buf_pointer(inst) + EC_SHI_OUT_PREAMBLE_LENGTH) % SHI_OBUF_FULL_SIZE;
+	if (IS_ENABLED(CONFIG_EC_HOST_CMD_BACKEND_SHI_NPCX_ENHANCED_BUF_MODE)) {
+		return shi_npcx_write_buf_pointer(inst) % SHI_OBUF_FULL_SIZE;
+	} else {
+		return (shi_npcx_read_buf_pointer(inst) + EC_SHI_OUT_PREAMBLE_LENGTH) %
+		       SHI_OBUF_FULL_SIZE;
+	}
 }
 
 /*
@@ -246,6 +278,16 @@ static void shi_npcx_fill_out_status(struct shi_reg *const inst, uint8_t status)
 	volatile uint8_t *fill_end;
 	volatile uint8_t *obuf_end;
 
+	if (IS_ENABLED(CONFIG_EC_HOST_CMD_BACKEND_SHI_NPCX_ENHANCED_BUF_MODE)) {
+		/*
+		 * In Enhanced Buffer Mode, SHI module outputs the status code
+		 * in SBOBUF repeatedly.
+		 */
+		inst->SBOBUF = status;
+
+		return;
+	}
+
 	/*
 	 * Disable interrupts in case the interfere by the other interrupts.
 	 * Use __disable_irq/__enable_irq instead of using irq_lock/irq_unlock
@@ -282,6 +324,10 @@ static void shi_npcx_bad_received_data(const struct device *dev)
 	struct shi_npcx_data *data = dev->data;
 	struct shi_reg *const inst = HAL_INSTANCE(dev);
 
+	if (IS_ENABLED(CONFIG_EC_HOST_CMD_BACKEND_SHI_NPCX_ENHANCED_BUF_MODE)) {
+		inst->EVENABLE &= ~IBF_IBHF_EN_MASK;
+	}
+
 	/* State machine mismatch, timeout, or protocol we can't handle. */
 	shi_npcx_fill_out_status(inst, EC_SHI_RX_BAD_DATA);
 	data->state = SHI_STATE_BAD_RECEIVED_DATA;
@@ -367,6 +413,10 @@ static void shi_npcx_handle_host_package(const struct device *dev)
 	data->state = SHI_STATE_PROCESSING;
 	LOG_DBG("PRC-");
 
+	if (IS_ENABLED(CONFIG_EC_HOST_CMD_BACKEND_SHI_NPCX_ENHANCED_BUF_MODE)) {
+		inst->EVENABLE &= ~IBF_IBHF_EN_MASK;
+	}
+
 	/* Fill output buffer to indicate we`re processing request */
 	shi_npcx_fill_out_status(inst, EC_SHI_PROCESSING);
 	data->out_msg[0] = EC_SHI_FRAME_START;
@@ -719,14 +769,20 @@ static void shi_npcx_reset_prepare(const struct device *dev)
 	data->sz_request = 0;
 	data->sz_response = 0;
 
-	/*
-	 * Fill output buffer to indicate we`re
-	 * ready to receive next transaction.
-	 */
-	for (i = 1; i < SHI_OBUF_FULL_SIZE; i++) {
-		inst->OBUF[i] = EC_SHI_RECEIVING;
+	if (IS_ENABLED(CONFIG_EC_HOST_CMD_BACKEND_SHI_NPCX_ENHANCED_BUF_MODE)) {
+		inst->SBOBUF = EC_SHI_RX_READY;
+		inst->SBOBUF = EC_SHI_RECEIVING;
+		inst->EVENABLE |= IBF_IBHF_EN_MASK;
+		inst->EVENABLE &= ~(BIT(NPCX_EVENABLE_OBEEN) | BIT(NPCX_EVENABLE_OBHEEN));
+	} else {
+		/*
+		 * Fill output buffer to indicate we`re ready to receive next transaction.
+		 */
+		for (i = 1; i < SHI_OBUF_FULL_SIZE; i++) {
+			inst->OBUF[i] = EC_SHI_RECEIVING;
+		}
+		inst->OBUF[0] = EC_SHI_RX_READY;
 	}
-	inst->OBUF[0] = EC_SHI_RX_READY;
 
 	/* SHI/Host Write/input buffer wrap-around enable */
 	inst->SHICFG1 = BIT(NPCX_SHICFG1_IWRAP) | BIT(NPCX_SHICFG1_WEN) | BIT(NPCX_SHICFG1_EN);
@@ -861,8 +917,7 @@ static int shi_npcx_init_registers(const struct device *dev)
 	 * [1] - OBHEEN = 0: Output Buffer Half Empty Interrupt Enable
 	 * [0] - OBEEN  = 0: Output Buffer Empty Interrupt Enable
 	 */
-	inst->EVENABLE =
-		BIT(NPCX_EVENABLE_EOREN) | BIT(NPCX_EVENABLE_IBHFEN) | BIT(NPCX_EVENABLE_IBFEN);
+	inst->EVENABLE = BIT(NPCX_EVENABLE_EOREN) | IBF_IBHF_EN_MASK;
 
 	/*
 	 * EVENABLE2 (Event Enable 2) setting
@@ -875,6 +930,10 @@ static int shi_npcx_init_registers(const struct device *dev)
 	/* Clear SHI events status register */
 	inst->EVSTAT = 0xff;
 
+	if (IS_ENABLED(CONFIG_EC_HOST_CMD_BACKEND_SHI_NPCX_ENHANCED_BUF_MODE)) {
+		inst->SHICFG6 |= BIT(NPCX_SHICFG6_EBUFMD);
+	}
+
 	npcx_miwu_interrupt_configure(&config->shi_cs_wui, NPCX_MIWU_MODE_EDGE, NPCX_MIWU_TRIG_LOW);
 
 	/* SHI interrupt installation */
@@ -928,13 +987,15 @@ static int shi_npcx_backend_send(const struct ec_host_cmd_backend *backend)
 	struct shi_npcx_data *data = hc_shi->dev->data;
 	uint8_t *out_buf = data->out_msg + EC_SHI_FRAME_START_LENGTH;
 
-	/*
-	 * Disable interrupts. This routine is not called from interrupt context and buffer
-	 * underrun will likely occur if it is preempted after writing its initial reply byte.
-	 * Also, we must be sure our state doesn't unexpectedly change, in case we're expected
-	 * to take RESP_NOT_RDY actions.
-	 */
-	__disable_irq();
+	if (!IS_ENABLED(CONFIG_EC_HOST_CMD_BACKEND_SHI_NPCX_ENHANCED_BUF_MODE)) {
+		/*
+		 * Disable interrupts. This routine is not called from interrupt context and buffer
+		 * underrun will likely occur if it is preempted after writing its initial reply
+		 * byte. Also, we must be sure our state doesn't unexpectedly change, in case we're
+		 * expected to take RESP_NOT_RDY actions.
+		 */
+		__disable_irq();
+	}
 
 	if (data->state == SHI_STATE_PROCESSING) {
 		/* Append our past-end byte, which we reserved space for. */
@@ -948,6 +1009,17 @@ static int shi_npcx_backend_send(const struct ec_host_cmd_backend *backend)
 
 		/* Transmit the reply */
 		data->state = SHI_STATE_SENDING;
+		if (IS_ENABLED(CONFIG_EC_HOST_CMD_BACKEND_SHI_NPCX_ENHANCED_BUF_MODE)) {
+			struct shi_reg *const inst = HAL_INSTANCE(hc_shi->dev);
+
+			/*
+			 * Enable output buffer half/full empty interrupt and
+			 * switch output mode from the repeated single byte mode
+			 * to FIFO mode.
+			 */
+			inst->EVENABLE |= BIT(NPCX_EVENABLE_OBEEN) | BIT(NPCX_EVENABLE_OBHEEN);
+			inst->SHICFG6 |= BIT(NPCX_SHICFG6_OBUF_SL);
+		}
 		LOG_DBG("SND-");
 	} else if (data->state == SHI_STATE_CNL_RESP_NOT_RDY) {
 		/*
@@ -961,7 +1033,10 @@ static int shi_npcx_backend_send(const struct ec_host_cmd_backend *backend)
 		LOG_ERR("Unexpected state %d in response handler", data->state);
 	}
 
-	__enable_irq();
+	if (!IS_ENABLED(CONFIG_EC_HOST_CMD_BACKEND_SHI_NPCX_ENHANCED_BUF_MODE)) {
+		__enable_irq();
+	}
+
 	return 0;
 }