PROCESSOR-SDK-AM62X: Reading a file from UBIFS makes every mtd partitions shifted 2 bytes

Hi Dung,

What interface is the device of the MTD on, OSPI or GPMC, or something else?

We're using OSPI for MTD.

Hi Dung,

I am routing your query to our OSPI expert for comments.

Hi Dung,

can you see if you can re-create this on a TI board (SK-AM62)?

Regards, Andreas

Hi Andreas,

I created a thread at: https://e2e.ti.com/support/processors-group/processors/f/processors-forum/1392015/sk-am62-reading-a-file-from-ubifs-makes-every-mtd-partitions-shifted-2-bytes

Regards,

Hi Dung,

I meant re-creating the issue, NOT filing a new E2E thread. What you filed looks like a duplicate of this E2E thread here, with no new information. What I'd like you to do is to see if you can re-create this on a TI SK-AM62 board/hardware.

Regards, Andreas

We have tested on the TI SK-AM62 dev-kit and above issue was not happen.

Hi Dung,

thanks for running this test. Can we try to summarize all potentially relevant differences of your design vs. the experiment you run on SK-AM62 from a HW and SW perspective? Such as...

Then, can you try to reduce the OSPI module clock frequency from 160MHz (assuming that's what you use) to something slower like 80MHz (should be possible to use this frequency w/ the internal integer clock dividers that are implemented in HW) to see if there's perhaps a speed dependency of this issue occurring? See here how to change that frequency, and double-check the changes at runtime through `k3conf`: https://e2e.ti.com/support/processors-group/processors/f/processors-forum/1223290/am625-set-ospi0-clock-setting-to-160mhz/4746677#4746677

Regards, Andreas

Hi, Andreas

We have summarize relevant differences of us design as below:

I have seen the guideline to change frequency, but I see not clock ID for 80MHz, can you guideline detail to configuration with 80MHz of OSPI?

I tried change frequency to 166MHz, The result is still fail.

Regards, Huy

Huy Khuat said:

I have seen the guideline to change frequency, but I see not clock ID for 80MHz, can you guideline detail to configuration with 80MHz of OSPI?

Can you try the below? I haven't tried it myself (just an edit/mock-up), but I think it should work. Same diff as from the other E2E thread I referenced, but plugging in 80MHz instead of 160MHz.

$ git diff
diff --git a/arch/arm64/boot/dts/ti/k3-am62-main.dtsi b/arch/arm64/boot/dts/ti/k3-am62-main.dtsi
index 510775e9a88f..8100879cdaa1 100644
--- a/arch/arm64/boot/dts/ti/k3-am62-main.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am62-main.dtsi
@@ -628,8 +628,8 @@ ospi0: spi@fc40000 {
                        cdns,trigger-address = <0x0>;
                        clocks = <&k3_clks 75 7>;
                        assigned-clocks = <&k3_clks 75 7>;
-                       assigned-clock-parents = <&k3_clks 75 8>;
-                       assigned-clock-rates = <166666666>;
+                       assigned-clock-parents = <&k3_clks 75 9>; /* Select PARENT_POSTDIV1_16FFT_MAIN_1_HSDIVOUT5_CLK */
+                       assigned-clock-rates = <80000000>;
                        power-domains = <&k3_pds 75 TI_SCI_PD_EXCLUSIVE>;
                        #address-cells = <1>;
                        #size-cells = <0>;

Regards, Andreas

Thanks for reply quick of you

I tried, but not work.

Regards, Huy

Huy Khuat said:

Thanks for reply quick of you

I tried, but not work.

What does that mean? You still see the "byte shift" issue, or something else happened?

Then, can you run the below command to make sure the 80MHz setting is active, and provide the output here?

$ k3conf dump clock 75

Regards, Andreas

The Nor s28hs02gt ID value is read incorrectly which makes nor init not successful., and I checked k3conf below, 80MHz setting  is active.

Regards, Huy

Huy Khuat said:

The Nor s28hs02gt ID value is read incorrectly which makes nor init not successful.

Ok looks like now it doesn't work at all; looks like reducing the frequency like we did probably was not a valid thing to do.

Also for testing purposes, can you please try using the kernel from our current AM62x SDK v9.2? I see there are quite a few commits/fixes on the OSPI driver (drivers/spi/spi-cadence-quadspi.c), so it'll be a good experiment to see if this new Kernel has an impact on the behavior. Should the new Kernel "solve" the issue we could then look more specifically as to what fixes it and what to do about fixing your system.

SDK: https://www.ti.com/tool/download/PROCESSOR-SDK-LINUX-AM62X/09.02.01.10
But from the SDK you really only need this: https://git.ti.com/cgit/ti-linux-kernel/ti-linux-kernel/log/?h=ti-linux-6.1.y

Regards, Andreas

1. I tried use AM62x SDK v9.2 with update device driver of S28HS02GT on our sample-board, and I do not see error shifted 2 bytes.
2. befor, We have update DAC mode read in file (drivers/spi/spi-cadence-quadspi.c), I reverted this change and retest, I also don't see this error.
so this change in spi-cadence-quadspi.c may be the cause of this error. I attached file this diff file, can you help me check this changes.

From e09c3479ca952e59ad362ca0a7d299b4b20ebf2c Mon Sep 17 00:00:00 2001
From: huy <huykv1@fpt.com>
Date: Thu, 1 Aug 2024 18:17:37 +0700
Subject: [PATCH] s28hs02gt DAC read mode

---
 drivers/spi/spi-cadence-quadspi.c | 31 ++++++++++++++++++++++++++++++-
 1 file changed, 30 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
index cb9ab1ba67..ce71e56198 100644
--- a/drivers/spi/spi-cadence-quadspi.c
+++ b/drivers/spi/spi-cadence-quadspi.c
@@ -39,6 +39,7 @@
 
 /* Capabilities */
 #define CQSPI_SUPPORTS_OCTAL		BIT(0)
+#define CQSPI_STIG_DATA_LEN_MAX		 8
 
 struct cqspi_st;
 
@@ -172,6 +173,8 @@ struct cqspi_driver_platdata {
 #define CQSPI_REG_SIZE_ADDRESS_MASK		0xF
 #define CQSPI_REG_SIZE_PAGE_MASK		0xFFF
 #define CQSPI_REG_SIZE_BLOCK_MASK		0x3F
+#define CQSPI_REG_SIZE_MEM_SIZE_ON_C0           (0x2<<21)
+#define CQSPI_REG_SIZE_MEM_SIZE_ON_C0_MASK      (0x3<<21)
 
 #define CQSPI_REG_SRAMPARTITION			0x18
 #define CQSPI_REG_INDIRECTTRIGGER		0x1C
@@ -215,6 +218,7 @@ struct cqspi_driver_platdata {
 #define CQSPI_REG_CMDCTRL_ADD_BYTES_LSB		16
 #define CQSPI_REG_CMDCTRL_ADDR_EN_LSB		19
 #define CQSPI_REG_CMDCTRL_RD_BYTES_LSB		20
+#define	CQSPI_REG_CMDCTRL_ADDR_EN		BIT(19)
 #define CQSPI_REG_CMDCTRL_RD_EN_LSB		23
 #define CQSPI_REG_CMDCTRL_OPCODE_LSB		24
 #define CQSPI_REG_CMDCTRL_WR_BYTES_MASK		0x7
@@ -1146,6 +1150,19 @@ static int cqspi_command_read(struct cqspi_flash_pdata *f_pdata,
 	/* 0 means 1 byte. */
 	reg |= (((n_rx - 1) & CQSPI_REG_CMDCTRL_RD_BYTES_MASK)
 		<< CQSPI_REG_CMDCTRL_RD_BYTES_LSB);
+
+	/* setup ADDR BIT field */
+	if (op->addr.nbytes) {
+		writel(op->addr.val, reg_base + CQSPI_REG_CMDADDRESS);
+		/*
+		 * address bytes are zero indexed
+		 */
+		reg |= (((op->addr.nbytes - 1) &
+			  CQSPI_REG_CMDCTRL_ADD_BYTES_MASK) <<
+			  CQSPI_REG_CMDCTRL_ADD_BYTES_LSB);
+		reg |= CQSPI_REG_CMDCTRL_ADDR_EN;
+	}
+
 	status = cqspi_exec_flash_cmd(cqspi, reg);
 	if (status)
 		return status;
@@ -1275,6 +1292,8 @@ static int cqspi_read_setup(struct cqspi_flash_pdata *f_pdata,
 	reg = readl(reg_base + CQSPI_REG_SIZE);
 	reg &= ~CQSPI_REG_SIZE_ADDRESS_MASK;
 	reg |= (op->addr.nbytes - 1);
+	reg &= ~CQSPI_REG_SIZE_MEM_SIZE_ON_C0_MASK;
+	reg |= CQSPI_REG_SIZE_MEM_SIZE_ON_C0;
 	writel(reg, reg_base + CQSPI_REG_SIZE);
 	readl(reg_base + CQSPI_REG_SIZE); /* Flush posted write. */
 	return 0;
@@ -1418,6 +1437,8 @@ static int cqspi_write_setup(struct cqspi_flash_pdata *f_pdata,
 	reg = readl(reg_base + CQSPI_REG_SIZE);
 	reg &= ~CQSPI_REG_SIZE_ADDRESS_MASK;
 	reg |= (op->addr.nbytes - 1);
+	reg &= ~CQSPI_REG_SIZE_MEM_SIZE_ON_C0_MASK;
+	reg |= CQSPI_REG_SIZE_MEM_SIZE_ON_C0;
 	writel(reg, reg_base + CQSPI_REG_SIZE);
 	readl(reg_base + CQSPI_REG_SIZE); /* Flush posted write. */
 	return 0;
@@ -1993,7 +2014,14 @@ static int cqspi_mem_process(struct spi_mem *mem, const struct spi_mem_op *op)
 	cqspi_configure(f_pdata, mem->spi->max_speed_hz);
 
 	if (op->data.dir == SPI_MEM_DATA_IN && op->data.buf.in) {
-		if (!op->addr.nbytes)
+		// if (!op->addr.nbytes)
+					/*
+	 * Performing reads in DAC mode forces to read minimum 4 bytes
+	 * which is unsupported on some flash devices during register
+	 * reads, prefer STIG mode for such small reads.
+	 */
+		if (!op->addr.nbytes ||
+		    op->data.nbytes < CQSPI_STIG_DATA_LEN_MAX)
 			return cqspi_command_read(f_pdata, op);
 
 		return cqspi_read(f_pdata, op);
@@ -2514,3 +2542,4 @@ MODULE_AUTHOR("Graham Moore <grmoore@opensource.altera.com>");
 MODULE_AUTHOR("Vadivel Murugan R <vadivel.muruganx.ramuthevar@intel.com>");
 MODULE_AUTHOR("Vignesh Raghavendra <vigneshr@ti.com>");
 MODULE_AUTHOR("Pratyush Yadav <p.yadav@ti.com>");
+
-- 
2.25.1

Regards, Huy

Hi Andreas, 

 I have new update, with our kernel, I tried comment out as below.  above issue was not happen., but in kernel in SDK v9.2 not change comment as below, it still works fine.

Regards, Huy

Hi Huy,

Andreas is currently out of office so please expect delay in responses.

Regards,
Krunal

Huy,

thanks for the updates/findings, this is great to see because it looks like this will help solving your concern.

Huy Khuat said:

 I have new update, with our kernel, I tried comment out as below.  above issue was not happen

Ok so you found a way making it work with Kernel v8.x when what looks like avoiding/disabling DAC mode in certain scenarios.

Huy Khuat said:

K v9.2 not change comment as below, it still works fine.

And you confirmed with kernel v9.2 the issue doesn't happen.

Let me look at the commit differences/logs between the Kernels and see if this behavior can be attributed to a specific change/commit and get back to you.

Regards, Andreas

Hi Andreas,

I applied this patch, but above issue was not happen.

I compared SDK v8.6 and SDK v9.2 kernel version, and change code difference between 2 kernel version. It work fine

diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
index ddb53f3c9a6fc8b508eefddfb90200f312a30369..ea4ec953d06aa3e927efed4074203507635c4d29 100644
--- a/drivers/spi/spi-cadence-quadspi.c
+++ b/drivers/spi/spi-cadence-quadspi.c
@@ -39,7 +39,6 @@
 
 /* Capabilities */
 #define CQSPI_SUPPORTS_OCTAL		BIT(0)
-#define CQSPI_STIG_DATA_LEN_MAX		 8
 
 struct cqspi_st;
 
@@ -861,7 +860,7 @@ static bool cqspi_is_idle(struct cqspi_st *cqspi)
 {
 	u32 reg = readl(cqspi->iobase + CQSPI_REG_CONFIG);
 
-	return reg & (1 << CQSPI_REG_CONFIG_IDLE_LSB);
+	return reg & (1UL << CQSPI_REG_CONFIG_IDLE_LSB);
 }
 
 static u32 cqspi_get_rd_sram_level(struct cqspi_st *cqspi)
@@ -906,6 +905,9 @@ static unsigned int cqspi_calc_dummy(const struct spi_mem_op *op, bool dtr)
 {
 	unsigned int dummy_clk;
 
+	if (!op->dummy.nbytes)
+		return 0;
+
 	dummy_clk = op->dummy.nbytes * (8 / op->dummy.buswidth);
 	if (dtr)
 		dummy_clk /= 2;
@@ -1153,14 +1155,12 @@ static int cqspi_command_read(struct cqspi_flash_pdata *f_pdata,
 
 	/* setup ADDR BIT field */
 	if (op->addr.nbytes) {
+		reg |= (0x1 << CQSPI_REG_CMDCTRL_ADDR_EN_LSB);
+		reg |= ((op->addr.nbytes - 1) &
+			CQSPI_REG_CMDCTRL_ADD_BYTES_MASK)
+			<< CQSPI_REG_CMDCTRL_ADD_BYTES_LSB;
+
 		writel(op->addr.val, reg_base + CQSPI_REG_CMDADDRESS);
-		/*
-		 * address bytes are zero indexed
-		 */
-		reg |= (((op->addr.nbytes - 1) &
-			  CQSPI_REG_CMDCTRL_ADD_BYTES_MASK) <<
-			  CQSPI_REG_CMDCTRL_ADD_BYTES_LSB);
-		reg |= CQSPI_REG_CMDCTRL_ADDR_EN;
 	}
 
 	status = cqspi_exec_flash_cmd(cqspi, reg);
@@ -1181,6 +1181,9 @@ static int cqspi_command_read(struct cqspi_flash_pdata *f_pdata,
 		memcpy(rxbuf, &reg, read_len);
 	}
 
+	/* Reset CMD_CTRL Reg once command read completes */
+	writel(0, reg_base + CQSPI_REG_CMDCTRL);
+
 	return 0;
 }
 
@@ -1935,7 +1938,7 @@ static int cqspi_direct_read_execute(struct cqspi_flash_pdata *f_pdata,
 	u_char *buf = op->data.buf.in;
 	int ret;
 
-	if (!cqspi->rx_chan || !virt_addr_valid(buf)) {
+	if (!cqspi->rx_chan || !virt_addr_valid(buf) || len <= 16) {
 		cqspi_memcpy_fromio(op, buf, cqspi->ahb_base + from, len);
 		return 0;
 	}
@@ -2014,14 +2017,13 @@ static int cqspi_mem_process(struct spi_mem *mem, const struct spi_mem_op *op)
 	cqspi_configure(f_pdata, mem->spi->max_speed_hz);
 
 	if (op->data.dir == SPI_MEM_DATA_IN && op->data.buf.in) {
-		// if (!op->addr.nbytes)
-					/*
+	/*
 	 * Performing reads in DAC mode forces to read minimum 4 bytes
 	 * which is unsupported on some flash devices during register
 	 * reads, prefer STIG mode for such small reads.
 	 */
 		if (!op->addr.nbytes ||
-		    op->data.nbytes < CQSPI_STIG_DATA_LEN_MAX)
+		    op->data.nbytes <= CQSPI_STIG_DATA_LEN_MAX)
 			return cqspi_command_read(f_pdata, op);
 
 		return cqspi_read(f_pdata, op);

I think this issue resolved

Thank you for your support

Huy

Huy,

glad to hear, thanks for the feedback.

Regards, Andreas