[U-Boot] Problem with attaching UBI partition

[Bakhvalov, Denis (Nokia - PL/Wroclaw)]

Hi Jagan, Heiko,

> Did you enable CONFIG_SPI_FLASH_SPANSION on your config, becuase
> S25FL512S is already been added u-boot. Pls- check the same and let me
> know if you find any issues while detecting the flash.

> Whether the driver detecting flash or not with RDID, please define
> DEBUG on drivers/mtd/spi/spi_flash.c

I had no problems with detecting the flash. It was detected like this:
SF: Detected S25FL512S_256K with page size 512 Bytes, erase size 256 KiB, total 64 MiB

Yes, I have CONFIG_SPI_FLASH_SPANSION enabled in configs/socfpga_arria5_dbrrh_defconfig (dbrrh - is my custom board).

Actually, today I solved the problem, although in not very clean way, I think.

As I mentioned in previous mail I had problems with reading from the flash. Some bytes were read incorrectly.
I'm migrating from U-Boot 2013 to mainline U-Boot. And on previous version there were no such issue.
I found out that there was some workaround introduced in order to fix this.

So, I just apply some part of that workaround to mainline U-Boot and now it works.

Here is the problem in more detail:

On previous version (2013):
U-BOOT # sf read 0x1B000000 0x03800000 4
     cadence_qspi_apb_indirect_read_setup. addr_value = 0x3800000
     cadence_qspi_apb_indirect_read_setup. rd_reg = 0x800000b
     cadence_qspi_apb_indirect_read_setup. device size = 0x101003

U-BOOT # md 0x1B000000 1
1b000000: 55424923

But on mainline U-Boot(2016.03-rc1) I had:
U-BOOT # sf read 0x1B000000 0x03800000 4
     cadence_qspi_apb_indirect_read_setup. addr_value = 0x800000
     cadence_qspi_apb_indirect_read_setup. rd_reg = 0x0b
     cadence_qspi_apb_indirect_read_setup. device size = 0x101002

U-BOOT # md 0x1B000000 1
1b000000: 554249ff

You can see the difference in the register values that were written in QSPI registers.
In case with mainline U-Boot I had address value not properly set.

Here is the workaround that I've made:

Index: drivers/mtd/spi/spi_flash.c
===================================================================
--- drivers/mtd/spi/spi_flash.c	(revision 608)
+++ drivers/mtd/spi/spi_flash.c	(revision 609)
@@ -29,6 +29,17 @@
 	cmd[3] = addr >> 0;
 }
 
+#ifdef CONFIG_DBRRH_WORKAROUND                
+static void spi_flash_addr32(u32 addr, u8 *cmd)
+{
+	/* cmd[0] is actual command */
+	cmd[1] = (addr >> 24) & 0xFF;
+	cmd[2] = (addr >> 16) & 0xFF;
+	cmd[3] = (addr >> 8) & 0xFF;
+	cmd[4] = (addr >> 0) & 0xFF;
+}
+#endif
+
 static int read_sr(struct spi_flash *flash, u8 *rs)
 {
 	int ret;
@@ -510,8 +521,14 @@
 		else
 			read_len = remain_len;
 
+#ifdef CONFIG_DBRRH_WORKAROUND                
+        		spi_flash_addr32(read_addr, cmd);
+#else
 		spi_flash_addr(read_addr, cmd);
+#endif
 
		ret = spi_flash_read_common(flash, cmd, cmdsz, data, read_len);
 		if (ret < 0) {
 			debug("SF: read failed\n");

Index: drivers/spi/cadence_qspi_apb.c
===================================================================
--- drivers/spi/cadence_qspi_apb.c	(revision 608)
+++ drivers/spi/cadence_qspi_apb.c	(revision 609)
@@ -30,6 +30,10 @@
 #include <asm/errno.h>
 #include "cadence_qspi.h"
 
+#ifdef CONFIG_DBRRH_WORKAROUND
+	#define CMD_OPTION_DUMMY_CYCLES		0x7F	/* Dummy Cycles for Read Command */
+#endif
+
@@ -706,9 +710,25 @@
 #endif
 
 	/* Get address */
+#ifdef CONFIG_DBRRH_WORKAROUND
+	addr_value = cadence_qspi_apb_cmd2addr(&cmdbuf[1], 4);
+#else
 	addr_value = cadence_qspi_apb_cmd2addr(&cmdbuf[1], addr_bytes);
+#endif

+#ifdef CONFIG_DBRRH_WORKAROUND
+	/* Setting Dummy Clock Cycle */
+	dummy_clk = (0x08 & CMD_OPTION_DUMMY_CYCLES);
+	if (dummy_clk) 
+	{
+		rd_reg |= (dummy_clk & CQSPI_REG_RD_INSTR_DUMMY_MASK)
+			<< CQSPI_REG_RD_INSTR_DUMMY_LSB;
+	}
+#else
+
 	/* The remaining lenght is dummy bytes. */
 	dummy_bytes = cmdlen - addr_bytes - 1;
 	if (dummy_bytes) {
@@ -731,7 +751,9 @@
 			rd_reg |= (dummy_clk & CQSPI_REG_RD_INSTR_DUMMY_MASK)
 				<< CQSPI_REG_RD_INSTR_DUMMY_LSB;
 	}
+#endif


With this correction I can read contents of the flash properly.
However, I'm a bit surprised that I was forced to make such correction like storing 4 bytes of address (see spi_flash_addr32() above).
On the other hand I haven't found any switch that could be turned on to fix my problem in a clean and nice way.

With 24 bytes we can address only 16 MB. How cadence driver is supposed to work for larger spaces?
Is this 4th byte comes from somewhere else?

Jagan, Heiko, please evaluate my correction.

Best regards,
Denis Bakhvalov