drivers: nxp: flexspi: fix hyper flash hang issue

CS hold time parameter is not correct which may cause bus fault
randomly.
System hang during status register reading after flash progromming which
is caused by parameter accessing in XIP mode.
Add dummy delay for READ command according the flash datasheet which is
required for SDR mode.
Use FlexSPI internal divider for clock updating instead of register in
CCM to avoid potential risk caused by flash access during clock updating
procedure.

Signed-off-by: Raymond Lei <raymond.lei@nxp.com>

boards/nxp/mimxrt1050_evk/mimxrt1050_evk_mimxrt1052_hyperflash.dts

@@ -34,7 +34,7 @@
 		word-addressable;
 		cs-interval-unit = <1>;
 		cs-interval = <2>;
-		cs-hold-time = <0>;
+		cs-hold-time = <3>;
 		cs-setup-time = <3>;
 		data-valid-time = <1>;
 		column-space = <3>;

boards/nxp/mimxrt1060_evk/mimxrt1060_evk_mimxrt1062_hyperflash.dts

@@ -32,7 +32,7 @@
 		word-addressable;
 		cs-interval-unit = <1>;
 		cs-interval = <2>;
-		cs-hold-time = <0>;
+		cs-hold-time = <3>;
 		cs-setup-time = <3>;
 		data-valid-time = <1>;
 		column-space = <3>;

boards/nxp/mimxrt1062_fmurt6/mimxrt1062_fmurt6.dts

@@ -193,7 +193,7 @@
 		word-addressable;
 		cs-interval-unit = <1>;
 		cs-interval = <2>;
-		cs-hold-time = <0>;
+		cs-hold-time = <3>;
 		cs-setup-time = <3>;
 		data-valid-time = <1>;
 		column-space = <3>;

drivers/flash/flash_mcux_flexspi_hyperflash.c

@@ -33,7 +33,6 @@ LOG_MODULE_REGISTER(flexspi_hyperflash, CONFIG_FLASH_LOG_LEVEL);
 #endif
 
 #include <zephyr/sys/util.h>
-
 #include "memc_mcux_flexspi.h"
 
 #define SPI_HYPERFLASH_SECTOR_SIZE              (0x40000U)
@@ -41,6 +40,17 @@ LOG_MODULE_REGISTER(flexspi_hyperflash, CONFIG_FLASH_LOG_LEVEL);
 
 #define HYPERFLASH_ERASE_VALUE                  (0xFF)
 
+/* Hyper flash support SDR and DDR, from the FlexSPI controller point of view,
+ * if DDR enabled, commands in LUT need to be DDR command and root clock is
+ * double of Serial clock (clock output to flash).
+ * if DDR mode enabled, set it to 2
+ * if SDR mode enabled, set it to 1
+ */
+#define MCUX_FLEXSPI_HYPERFLASH_DDR_SDR_MODE     2
+
+/* Some hyper flashs require a lower frequency when doing writing operation. */
+#define FREQ_FOR_HYPERFLASH_WRITE               (MHZ(42) * MCUX_FLEXSPI_HYPERFLASH_DDR_SDR_MODE)
+
 #ifdef CONFIG_FLASH_MCUX_FLEXSPI_HYPERFLASH_WRITE_BUFFER
 static uint8_t hyperflash_write_buf[SPI_HYPERFLASH_PAGE_SIZE];
 #endif
@@ -65,7 +75,10 @@ static const uint32_t flash_flexspi_hyperflash_lut[CUSTOM_LUT_LENGTH] = {
 				kFLEXSPI_Command_RADDR_DDR, kFLEXSPI_8PAD, 0x18),
 	[4 * READ_DATA + 1] =
 		FLEXSPI_LUT_SEQ(kFLEXSPI_Command_CADDR_DDR, kFLEXSPI_8PAD, 0x10,
-				kFLEXSPI_Command_READ_DDR,  kFLEXSPI_8PAD, 0x04),
+				kFLEXSPI_Command_DUMMY_RWDS_DDR, kFLEXSPI_8PAD, 0xC),
+	[4 * READ_DATA + 2] =
+		FLEXSPI_LUT_SEQ(kFLEXSPI_Command_READ_DDR,  kFLEXSPI_8PAD, 0x04,
+				kFLEXSPI_Command_STOP,     kFLEXSPI_1PAD, 0x0),
 	/* Write Data */
 	[4 * WRITE_DATA] =
 		FLEXSPI_LUT_SEQ(kFLEXSPI_Command_DDR,       kFLEXSPI_8PAD, 0x20,
@@ -256,9 +269,9 @@ struct flash_flexspi_hyperflash_data {
 	struct flash_parameters flash_parameters;
 };
 
-static int flash_flexspi_hyperflash_wait_bus_busy(const struct device *dev)
+/* Make sure all parameters accessed by this function are in RAM when XIP is enabled. */
+static int flash_flexspi_hyperflash_wait_bus_busy(const struct flash_flexspi_hyperflash_data *data)
 {
-	struct flash_flexspi_hyperflash_data *data = dev->data;
 	flexspi_transfer_t transfer;
 	int ret;
 	bool is_busy;
@@ -289,9 +302,10 @@ static int flash_flexspi_hyperflash_wait_bus_busy(const struct device *dev)
 	return ret;
 }
 
-static int flash_flexspi_hyperflash_write_enable(const struct device *dev, uint32_t address)
+/* Make sure all parameters accessed by this function are in RAM when XIP is enabled. */
+static int flash_flexspi_hyperflash_write_enable(const struct flash_flexspi_hyperflash_data *data,
+				uint32_t address)
 {
-	struct flash_flexspi_hyperflash_data *data = dev->data;
 	flexspi_transfer_t transfer;
 	int ret;
 
@@ -370,11 +384,10 @@ static int flash_flexspi_hyperflash_check_vendor_id(const struct device *dev)
 	return ret;
 }
 
-static int flash_flexspi_hyperflash_page_program(const struct device *dev, off_t
-		offset, const void *buffer, size_t len)
+/* Make sure all parameters accessed by this function are in RAM when XIP is enabled. */
+static int flash_flexspi_hyperflash_page_program(const struct flash_flexspi_hyperflash_data *data,
+				off_t offset, const void *buffer, size_t len)
 {
-	struct flash_flexspi_hyperflash_data *data = dev->data;
-
 	flexspi_transfer_t transfer = {
 		.deviceAddress = offset,
 		.port = data->port,
@@ -442,9 +455,12 @@ static int flash_flexspi_hyperflash_write(const struct device *dev, off_t offset
 		memc_flexspi_wait_bus_idle(&data->controller);
 	}
 
-	/* Clock FlexSPI at 84 MHZ (42MHz SCLK in DDR mode) */
-	(void)memc_flexspi_update_clock(&data->controller, &data->config,
-					data->port, MHZ(84));
+	/* Update clock to a freq which usually lower than normal working freq. */
+	if (memc_flexspi_update_clock(&data->controller, &data->config,
+					data->port, FREQ_FOR_HYPERFLASH_WRITE)) {
+		ret = -ENOTSUP;
+		goto __exit;
+	}
 
 	while (len) {
 		/* Writing between two page sizes crashes the platform so we
@@ -457,30 +473,24 @@ static int flash_flexspi_hyperflash_write(const struct device *dev, off_t offset
 		for (j = 0; j < i; j++) {
 			hyperflash_write_buf[j] = src[j];
 		}
-		/* As memcpy could cause an XIP access,
-		 * we need to wait for XIP prefetch to be finished again
-		 */
-		if (memc_flexspi_is_running_xip(&data->controller)) {
-			memc_flexspi_wait_bus_idle(&data->controller);
-		}
 #endif
-		ret = flash_flexspi_hyperflash_write_enable(dev, offset);
+		ret = flash_flexspi_hyperflash_write_enable(data, offset);
 		if (ret != 0) {
 			LOG_ERR("failed to enable write");
 			break;
 		}
 #ifdef CONFIG_FLASH_MCUX_FLEXSPI_HYPERFLASH_WRITE_BUFFER
-		ret = flash_flexspi_hyperflash_page_program(dev, offset,
+		ret = flash_flexspi_hyperflash_page_program(data, offset,
 				hyperflash_write_buf, i);
 #else
-		ret = flash_flexspi_hyperflash_page_program(dev, offset, src, i);
+		ret = flash_flexspi_hyperflash_page_program(data, offset, src, i);
 #endif
 		if (ret != 0) {
 			LOG_ERR("failed to write");
 			break;
 		}
 
-		ret = flash_flexspi_hyperflash_wait_bus_busy(dev);
+		ret = flash_flexspi_hyperflash_wait_bus_busy(data);
 		if (ret != 0) {
 			LOG_ERR("failed to wait bus busy");
 			break;
@@ -493,14 +503,18 @@ static int flash_flexspi_hyperflash_write(const struct device *dev, off_t offset
 		len -= i;
 	}
 
-	/* Clock FlexSPI at 332 MHZ (166 MHz SCLK in DDR mode) */
-	(void)memc_flexspi_update_clock(&data->controller, &data->config,
-					data->port, MHZ(332));
+	/* Clock FlexSPI at max freq flash support. */
+	if (memc_flexspi_update_clock(&data->controller, &data->config,
+					data->port, data->config.flexspiRootClk)) {
+		ret = -ENOTSUP;
+		goto __exit;
+	}
 
 #ifdef CONFIG_HAS_MCUX_CACHE
 	DCACHE_InvalidateByRange((uint32_t) dst, size);
 #endif
 
+__exit:
 	if (memc_flexspi_is_running_xip(&data->controller)) {
 		/* ==== EXIT CRITICAL SECTION ==== */
 		irq_unlock(key);
@@ -546,7 +560,7 @@ static int flash_flexspi_hyperflash_erase(const struct device *dev, off_t offset
 	}
 
 	for (i = 0; i < num_sectors; i++) {
-		ret = flash_flexspi_hyperflash_write_enable(dev, offset);
+		ret = flash_flexspi_hyperflash_write_enable(data, offset);
 		if (ret != 0) {
 			LOG_ERR("failed to write_enable");
 			break;
@@ -567,7 +581,7 @@ static int flash_flexspi_hyperflash_erase(const struct device *dev, off_t offset
 		}
 
 		/* wait bus busy */
-		ret = flash_flexspi_hyperflash_wait_bus_busy(dev);
+		ret = flash_flexspi_hyperflash_wait_bus_busy(data);
 		if (ret != 0) {
 			LOG_ERR("failed to wait bus busy");
 			break;
@@ -627,10 +641,6 @@ static int flash_flexspi_hyperflash_init(const struct device *dev)
 
 	memc_flexspi_wait_bus_idle(&data->controller);
 
-	if (memc_flexspi_is_running_xip(&data->controller)) {
-		/* Wait for bus idle before configuring */
-		memc_flexspi_wait_bus_idle(&data->controller);
-	}
 	if (memc_flexspi_set_device_config(&data->controller, &data->config,
 	    (const uint32_t *)flash_flexspi_hyperflash_lut,
 	    sizeof(flash_flexspi_hyperflash_lut) / MEMC_FLEXSPI_CMD_SIZE,
@@ -669,7 +679,8 @@ static DEVICE_API(flash, flash_flexspi_hyperflash_api) = {
 
 #define FLASH_FLEXSPI_DEVICE_CONFIG(n)					\
 	{								\
-		.flexspiRootClk = MHZ(42),				\
+		.flexspiRootClk = DT_INST_PROP(n, spi_max_frequency) * \
+		MCUX_FLEXSPI_HYPERFLASH_DDR_SDR_MODE, \
 		.flashSize = DT_INST_PROP(n, size) / 8 / KB(1),		\
 		.CSIntervalUnit =					\
 			CS_INTERVAL_UNIT(				\

drivers/memc/memc_mcux_flexspi.c

@@ -90,9 +90,26 @@ int memc_flexspi_update_clock(const struct device *dev,
 		flexspi_port_t port, uint32_t freq_hz)
 {
 	struct memc_flexspi_data *data = dev->data;
-	uint32_t rate;
 	uint32_t key;
-	int ret;
+	uint32_t divider, actual_freq, flexspiRootClk_copy, ccm_clock;
+
+	int ret = clock_control_get_rate(data->clock_dev, data->clock_subsys, &ccm_clock);
+
+	if (ret < 0) {
+		LOG_ERR("memc flexspi get root clock error: %d", ret);
+		return ret;
+	}
+
+	/* Freq required shall not exceed the max freq flash can support. */
+	freq_hz = MIN(freq_hz, device_config->flexspiRootClk);
+
+	/* Get the real freq on going. */
+	divider = (data->base->MCR0 & FLEXSPI_MCR0_SERCLKDIV_MASK) >> FLEXSPI_MCR0_SERCLKDIV_SHIFT;
+	actual_freq = ccm_clock / (divider + 1);
+	if (freq_hz ==  actual_freq) {
+		return 0;
+	}
+
 
 	/* To reclock the FlexSPI, we should:
 	 * - disable the module
@@ -103,39 +120,34 @@ int memc_flexspi_update_clock(const struct device *dev,
 	 */
 	key = irq_lock();
 	memc_flexspi_wait_bus_idle(dev);
-
-	ret = clock_control_set_rate(data->clock_dev, data->clock_subsys,
-				(clock_control_subsys_rate_t)freq_hz);
-	if (ret < 0) {
-		irq_unlock(key);
-		return ret;
-	}
+	FLEXSPI_Enable(data->base, false);
+
+	 /* Select a divider based on root frequency.
+	  * if we can't get an exact divider, round down
+	  */
+	divider = ((ccm_clock + (freq_hz - 1)) / freq_hz) - 1;
+	/* Cap divider to max value */
+	divider = MIN(divider, FLEXSPI_MCR0_SERCLKDIV_MASK >> FLEXSPI_MCR0_SERCLKDIV_SHIFT);
+	/* Update the internal divider*/
+	data->base->MCR0 &= ~FLEXSPI_MCR0_SERCLKDIV_MASK;
+	data->base->MCR0 |= FLEXSPI_MCR0_SERCLKDIV(divider);
 
 	/*
-	 * We need to update the DLL value before we call clock_control_get_rate,
-	 * because this will cause XIP (flash reads) to occur. Although the
-	 * true flash clock is not known, assume the set_rate function programmed
-	 * a value close to what we requested.
+	 * We don't want to modify the root clock variable, but we have to use this
+	 * parameter for DLL updating purpose(FLEXSPI_CalculateDll use flexspiRootClk as
+	 * the real frequency of FlexSPI serial clock). Back up it and restore it after DLL
+	 * Updating.
 	 */
-	device_config->flexspiRootClk = freq_hz;
-	FLEXSPI_UpdateDllValue(data->base, device_config, port);
-	memc_flexspi_reset(dev);
-
-	memc_flexspi_wait_bus_idle(dev);
-	ret = clock_control_get_rate(data->clock_dev, data->clock_subsys, &rate);
-	if (ret < 0) {
-		irq_unlock(key);
-		return ret;
-	}
-
-
-	device_config->flexspiRootClk = rate;
+	flexspiRootClk_copy = device_config->flexspiRootClk;
+	device_config->flexspiRootClk = ccm_clock/(divider + 1);
 	FLEXSPI_UpdateDllValue(data->base, device_config, port);
+	/* Restore root clock */
+	device_config->flexspiRootClk = flexspiRootClk_copy;
 
+	FLEXSPI_Enable(data->base, true);
 	memc_flexspi_reset(dev);
 
 	irq_unlock(key);
-
 	return 0;
 }
 
@@ -151,6 +163,8 @@ int memc_flexspi_set_device_config(const struct device *dev,
 	const uint32_t *lut_ptr = lut_array;
 	uint8_t lut_used = 0U;
 	unsigned int key = 0;
+	int ret;
+	uint32_t divider;
 
 	if (port >= kFLEXSPI_PortCount) {
 		LOG_ERR("Invalid port number");
@@ -196,9 +210,28 @@ int memc_flexspi_set_device_config(const struct device *dev,
 	tmp_config.ARDSeqIndex += data->port_luts[port].lut_offset / MEMC_FLEXSPI_CMD_PER_SEQ;
 	tmp_config.AWRSeqIndex += data->port_luts[port].lut_offset / MEMC_FLEXSPI_CMD_PER_SEQ;
 
+	/* Set FlexSPI clock to the max frequency flash can support.
+	 * FLEXSPI_SetFlashConfig only update DLL but not the freq divider.
+	 */
+	ret = memc_flexspi_update_clock(dev, &tmp_config,
+					port, device_config->flexspiRootClk);
+	if (ret < 0) {
+		LOG_ERR("memc flexspi update clock error: %d", ret);
+		return ret;
+	}
+
+	/* Get the real clock for DLL updating. */
+	ret = clock_control_get_rate(data->clock_dev, data->clock_subsys,
+				&tmp_config.flexspiRootClk);
+	if (ret < 0) {
+		LOG_ERR("memc flexspi get root clock error: %d", ret);
+		return ret;
+	}
+	divider = (data->base->MCR0 & FLEXSPI_MCR0_SERCLKDIV_MASK) >> FLEXSPI_MCR0_SERCLKDIV_SHIFT;
+	tmp_config.flexspiRootClk /= (divider + 1);
+
 	/* Lock IRQs before reconfiguring FlexSPI, to prevent XIP */
 	key = irq_lock();
-
 	FLEXSPI_SetFlashConfig(data->base, &tmp_config, port);
 	FLEXSPI_UpdateLUT(data->base, data->port_luts[port].lut_offset,
 			  lut_ptr, lut_count);

soc/nxp/imxrt/imxrt10xx/flexspi.c

@@ -11,13 +11,15 @@
 #include <zephyr/irq.h>
 #include <zephyr/dt-bindings/clock/imx_ccm.h>
 
+/* Use FlexSPi internal divider for clock modification.
+ * This function only updates the freq, but does not update DLL.
+ * Use function memc_flexspi_update_clock to update both of them.
+ */
 uint32_t flexspi_clock_set_freq(uint32_t clock_name, uint32_t rate)
 {
 	uint8_t divider;
 	uint32_t root_rate;
 	FLEXSPI_Type *flexspi;
-	clock_div_t div_sel;
-	clock_ip_name_t clk_name;
 
 	switch (clock_name) {
 	case IMX_CCM_FLEXSPI_CLK:
@@ -25,8 +27,6 @@ uint32_t flexspi_clock_set_freq(uint32_t clock_name, uint32_t rate)
 		root_rate = CLOCK_GetClockRootFreq(kCLOCK_FlexspiClkRoot) *
 					(CLOCK_GetDiv(kCLOCK_FlexspiDiv) + 1);
 		flexspi = (FLEXSPI_Type *)DT_REG_ADDR(DT_NODELABEL(flexspi));
-		div_sel = kCLOCK_FlexspiDiv;
-		clk_name = kCLOCK_FlexSpi;
 		break;
 #if DT_NODE_HAS_STATUS_OKAY(DT_NODELABEL(flexspi2))
 	case IMX_CCM_FLEXSPI2_CLK:
@@ -34,13 +34,12 @@ uint32_t flexspi_clock_set_freq(uint32_t clock_name, uint32_t rate)
 		root_rate = CLOCK_GetClockRootFreq(kCLOCK_Flexspi2ClkRoot) *
 					(CLOCK_GetDiv(kCLOCK_Flexspi2Div) + 1);
 		flexspi = (FLEXSPI_Type *)DT_REG_ADDR(DT_NODELABEL(flexspi2));
-		div_sel = kCLOCK_Flexspi2Div;
-		clk_name = kCLOCK_FlexSpi2;
 		break;
 #endif
 	default:
 		return -ENOTSUP;
 	}
+
 	/* Select a divider based on root frequency.
 	 * if we can't get an exact divider, round down
 	 */
@@ -52,13 +51,9 @@ uint32_t flexspi_clock_set_freq(uint32_t clock_name, uint32_t rate)
 		/* Spin */
 	}
 	FLEXSPI_Enable(flexspi, false);
-
-	CLOCK_DisableClock(clk_name);
-
-	CLOCK_SetDiv(div_sel, divider);
-
-	CLOCK_EnableClock(clk_name);
-
+	/* Update the internal divider*/
+	flexspi->MCR0 &= ~FLEXSPI_MCR0_SERCLKDIV_MASK;
+	flexspi->MCR0 |= FLEXSPI_MCR0_SERCLKDIV(divider);
 	FLEXSPI_Enable(flexspi, true);
 
 	FLEXSPI_SoftwareReset(flexspi);