[PATCH 0/2] Discover current address mode by CRC

[PATCH 0/2] Discover current address mode by CRC

[tkuw584924]

From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>

Hello,

I introduced this series based on the discussion [0].

Patch#1 is needed to use READ_4B(13h) for offline CRC calculation. This
can replace spi_nor_read_raw() in SFDP, if it is OK.

Path#2 implements address mode discovery by:
  - determine factory default from BFPT DWORD16
  - read 4 bytes from address 0 by 13h opcode
  - calculate CRC from 4 byte data by offline
  - perform data integrity CRC op
  - read data integrity CRC register by Read Any Reg with 3- and/or 4-byte
    address bytes to determine the current address mode

[0] https://patchwork.ozlabs.org/project/linux-mtd/patch/e47ed0aa3e1a6fdca7689434ce7dea99ff4826e7.1659764848.git.Takahiro.Kuwano@infineon.com/

Takahiro Kuwano (2):
  mtd: spi-nor: core: Add a helper to read with alternative method
  mtd: spi-nor: spansion: Discover current address mode by CRC

 drivers/mtd/spi-nor/core.c     |  48 ++++++++++++
 drivers/mtd/spi-nor/core.h     |   2 +
 drivers/mtd/spi-nor/sfdp.h     |   1 +
 drivers/mtd/spi-nor/spansion.c | 130 +++++++++++++++++++++++++++++++++
 4 files changed, 181 insertions(+)

-- 
2.25.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 1/2] mtd: spi-nor: core: Add a helper to read with alternative method

[tkuw584924]

From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>

The existing spi_nor_read_data() uses preset opcode, number of address
bytes, and dummy cycles. During SFDP parse and corresponding fixups we
need to perform read op with different opcode, number of address, and/or
dummy cycles from preset ones. The spi_nor_alt_read() helps that by
backup - read op - restore the preset opcode, etc.

Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
---
 drivers/mtd/spi-nor/core.c | 48 ++++++++++++++++++++++++++++++++++++++
 drivers/mtd/spi-nor/core.h |  2 ++
 2 files changed, 50 insertions(+)

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index f2c64006f8d7..ae98b4ad9b19 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -247,6 +247,54 @@ ssize_t spi_nor_read_data(struct spi_nor *nor, loff_t from, size_t len, u8 *buf)
 	return nor->controller_ops->read(nor, from, len, buf);
 }
 
+/**
+ * spi_nor_alt_read() - read data from flash memory with alternative opcode,
+ *                      number of address bytes, and dummy cycles.
+ * @nor:		pointer to 'struct spi_nor'
+ * @from:		offset to read from
+ * @len:		number of bytes to read
+ * @buf:		pointer to dst buffer
+ * @read_opcode:	read opcode to issue
+ * @addr_nbytes:	number of address bytes to send
+ * @read_dummy:		number of dummy cycles needed to read
+ *
+ * Return: 0 on success, -errno otherwise
+ */
+int spi_nor_alt_read(struct spi_nor *nor, u32 addr, size_t len, u8 *buf,
+		     u8 read_opcode, u8 addr_nbytes, u8 read_dummy)
+{
+	u8 bak_read_opcode = nor->read_opcode;
+	u8 bak_addr_nbytes = nor->addr_nbytes;
+	u8 bak_read_dummy = nor->read_dummy;
+	ssize_t ret;
+
+	nor->read_opcode = read_opcode;
+	nor->addr_nbytes = addr_nbytes;
+	nor->read_dummy = read_dummy;
+
+	while (len) {
+		ret = spi_nor_read_data(nor, addr, len, buf);
+		if (ret < 0)
+			goto out;
+		if (!ret || ret > len) {
+			ret = -EIO;
+			goto out;
+		}
+
+		buf += ret;
+		addr += ret;
+		len -= ret;
+	}
+	ret = 0;
+
+out:
+	nor->read_opcode = bak_read_opcode;
+	nor->addr_nbytes = bak_addr_nbytes;
+	nor->read_dummy = bak_read_dummy;
+
+	return ret;
+}
+
 /**
  * spi_nor_spimem_write_data() - write data to flash memory via
  *                               spi-mem
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index 85b0cf254e97..4f1e636f9362 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -656,6 +656,8 @@ ssize_t spi_nor_read_data(struct spi_nor *nor, loff_t from, size_t len,
 			  u8 *buf);
 ssize_t spi_nor_write_data(struct spi_nor *nor, loff_t to, size_t len,
 			   const u8 *buf);
+int spi_nor_alt_read(struct spi_nor *nor, u32 addr, size_t len, u8 *buf,
+		     u8 read_opcode, u8 addr_nbytes, u8 read_dummy);
 int spi_nor_read_any_reg(struct spi_nor *nor, struct spi_mem_op *op,
 			 enum spi_nor_protocol proto);
 int spi_nor_write_any_volatile_reg(struct spi_nor *nor, struct spi_mem_op *op,
-- 
2.25.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 2/2] mtd: spi-nor: spansion: Discover current address mode by CRC

[tkuw584924]

From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>

In some Infineon devices, the address mode (3- or 4-byte) is configurable
in both volatile and non-volatile registers. Determining current address
mode prior to SMPT parse is crucial because Read Any Reg op used in SMPT
takes 3- or 4-byte address depending on current address mode. The current
address mode is found in CFR2V[7] but CFR2V can be read by Read Any Reg
op (chicken-and-egg). This patch introduce a way to discover the current
address mode by using on-die CRC feature in Infineon SEMPER family.

The data integrity CRC (on-die CRC) calculation is conducted by issuing
a specific command followed by start and end addresses on which CRC is
calculated. The flash becomes busy state during calculation so we need to
poll status and wait for the completion. The calculated CRC value is
stored in Data Integrity Check CRC registers (DCRV) and can be read by
Read Any Reg op. We can try with 3- and 4-byte addresses to read DCRV then
compare it with expected CRC value calculated by offline. Both CRC should
match only when we perform Read Any Reg op with correct number of address
bytes.

The offline CRC calculation is done by CRC32C algorithm with 0x1edc6f41
polynomial. The data is read by READ_4B (13h) opcode.

BFPT DWORD16 is used to determine factory default of address mode,
assuming factory default is 3-byte in case power cycle exits 4-byte
address mode.

Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
---
 drivers/mtd/spi-nor/sfdp.h     |   1 +
 drivers/mtd/spi-nor/spansion.c | 130 +++++++++++++++++++++++++++++++++
 2 files changed, 131 insertions(+)

diff --git a/drivers/mtd/spi-nor/sfdp.h b/drivers/mtd/spi-nor/sfdp.h
index bbf80d2990ab..d7e1620bd870 100644
--- a/drivers/mtd/spi-nor/sfdp.h
+++ b/drivers/mtd/spi-nor/sfdp.h
@@ -90,6 +90,7 @@ struct sfdp_bfpt {
 #define BFPT_DWORD15_QER_SR2_BIT1_NO_RD		(0x4UL << 20)
 #define BFPT_DWORD15_QER_SR2_BIT1		(0x5UL << 20) /* Spansion */
 
+#define BFPT_DWORD16_EX4B_PWRCYC		BIT(21)
 #define BFPT_DWORD16_SWRST_EN_RST		BIT(12)
 
 #define BFPT_DWORD18_CMD_EXT_MASK		GENMASK(30, 29)
diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c
index 676ffd6d12ec..8ed4b2d53403 100644
--- a/drivers/mtd/spi-nor/spansion.c
+++ b/drivers/mtd/spi-nor/spansion.c
@@ -23,7 +23,9 @@
 #define SPINOR_REG_CYPRESS_CFR5V		0x00800006
 #define SPINOR_REG_CYPRESS_CFR5V_OCT_DTR_EN	0x3
 #define SPINOR_REG_CYPRESS_CFR5V_OCT_DTR_DS	0
+#define SPINOR_REG_CYPRESS_DCRV			0x00800095
 #define SPINOR_OP_CYPRESS_RD_FAST		0xee
+#define SPINOR_OP_CYPRESS_DI_CRC		0x5b
 
 /* Cypress SPI NOR flash operations. */
 #define CYPRESS_NOR_WR_ANY_REG_OP(naddr, addr, ndata, buf)		\
@@ -213,11 +215,139 @@ static int cypress_nor_set_page_size(struct spi_nor *nor)
 	return 0;
 }
 
+#define CRC32C_POLY	0x1edc6f41
+static u32 cypress_nor_calc_crc32c(u32 len, u8 *buf)
+{
+	u32 crc = 0;
+	int i;
+
+	while (len--) {
+		crc ^= *buf++ << 24;
+		for (i = 0; i < 8; i++)
+			crc = (crc << 1) ^ ((crc & 0x80000000) ? CRC32C_POLY :
+					    0);
+	}
+
+	return crc;
+}
+
+static int cypress_nor_data_integrity_crc(struct spi_nor *nor, u32 addr,
+					  u32 len)
+{
+	struct spi_mem_op op =
+			SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_CYPRESS_DI_CRC, 0),
+				   SPI_MEM_OP_ADDR(4, addr, 0),
+				   SPI_MEM_OP_NO_DUMMY,
+				   SPI_MEM_OP_DATA_OUT(4, nor->bouncebuf, 0));
+	u32 end_addr = addr + len - 1;
+	int ret;
+
+	nor->bouncebuf[0] = (end_addr >> 24) & 0xff;
+	nor->bouncebuf[1] = (end_addr >> 16) & 0xff;
+	nor->bouncebuf[2] = (end_addr >> 8) & 0xff;
+	nor->bouncebuf[3] = end_addr & 0xff;
+
+	spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
+
+	ret = spi_mem_exec_op(nor->spimem, &op);
+	if (ret)
+		return ret;
+
+	return spi_nor_wait_till_ready(nor);
+}
+
+/**
+ * cypress_nor_verify_crc() - Compare CRC and data integrity CRC registers.
+ * @nor:		pointer to 'struct spi_nor'.
+ * @addr_nbytes:	number of address bytes used in Read Any Reg op
+ * @crc:		CRC value to compare with registers
+ *
+ * Return: 1 if match, 0 if not match, -errno on errors.
+ */
+static int cypress_nor_verify_crc(struct spi_nor *nor, u8 addr_nbytes, u32 crc)
+{
+	struct spi_mem_op op =
+			CYPRESS_NOR_RD_ANY_REG_OP(addr_nbytes,
+						  SPINOR_REG_CYPRESS_DCRV,
+						  nor->bouncebuf);
+	int i, ret;
+
+	for (i = 0; i < 4; i++) {
+		ret = spi_nor_read_any_reg(nor, &op, nor->reg_proto);
+		if (ret)
+			return ret;
+
+		if ((crc & 0xff) != nor->bouncebuf[0])
+			return 0;
+
+		crc >>= 8;
+		op.addr.val++;
+	}
+
+	return 1;
+}
+
+static int cypress_nor_discover_addr_mode(struct spi_nor *nor)
+{
+	u32 addr = 0;
+	u32 len = 4;
+	u32 crc;
+	u8 addr_nbytes;
+	int ret;
+
+	/* Read flash memory array */
+	ret = spi_nor_alt_read(nor, addr, len, nor->bouncebuf,
+			       SPINOR_OP_READ_4B, 4, 0);
+	if (ret)
+		return ret;
+
+	/* Calculate CRC32C of read data */
+	crc = cypress_nor_calc_crc32c(len, nor->bouncebuf);
+
+	/* Perform on-die CRC calculation */
+	ret = cypress_nor_data_integrity_crc(nor, addr, len);
+	if (ret)
+		return ret;
+
+	/* Read and verify CRC registers with current addr_mode_nbytes */
+	ret = cypress_nor_verify_crc(nor, nor->params->addr_mode_nbytes, crc);
+	if (ret < 0)
+		return ret;
+	if (ret)
+		return 0;
+
+	/* Read and verify CRC registers with another number of address bytes */
+	addr_nbytes = nor->params->addr_mode_nbytes == 3 ? 4 : 3;
+	ret = cypress_nor_verify_crc(nor, addr_nbytes, crc);
+	if (ret < 0)
+		return ret;
+	if (ret)
+		nor->params->addr_mode_nbytes = addr_nbytes;
+	else
+		dev_warn(nor->dev, "Failed to discover current address mode. Assuming default %d-byte.\n",
+			 nor->params->addr_mode_nbytes);
+
+	return 0;
+}
+
 static int
 s25hx_t_post_bfpt_fixup(struct spi_nor *nor,
 			const struct sfdp_parameter_header *bfpt_header,
 			const struct sfdp_bfpt *bfpt)
 {
+	int ret;
+
+	/*
+	 * Discover flash's current address mode. Determine factory default
+	 * address mode in advance by checking if power cycle exits 4-byte
+	 * address mode, meaning factory default is 3-byte.
+	 */
+	nor->params->addr_mode_nbytes =
+		bfpt->dwords[BFPT_DWORD(16)] & BFPT_DWORD16_EX4B_PWRCYC ? 3 : 4;
+	ret = cypress_nor_discover_addr_mode(nor);
+	if (ret)
+		return ret;
+
 	/* Replace Quad Enable with volatile version */
 	nor->params->quad_enable = cypress_nor_quad_enable_volatile;
 
-- 
2.25.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 0/2] Discover current address mode by CRC

[Takahiro Kuwano]

Hi Tudor,

Have you had a chance to take a look?

Thanks,
Takahiro

On 8/12/2022 5:06 PM, tkuw584924@gmail.com wrote:
> From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
> 
> Hello,
> 
> I introduced this series based on the discussion [0].
> 
> Patch#1 is needed to use READ_4B(13h) for offline CRC calculation. This
> can replace spi_nor_read_raw() in SFDP, if it is OK.
> 
> Path#2 implements address mode discovery by:
>   - determine factory default from BFPT DWORD16
>   - read 4 bytes from address 0 by 13h opcode
>   - calculate CRC from 4 byte data by offline
>   - perform data integrity CRC op
>   - read data integrity CRC register by Read Any Reg with 3- and/or 4-byte
>     address bytes to determine the current address mode
> 
> [0] https://patchwork.ozlabs.org/project/linux-mtd/patch/e47ed0aa3e1a6fdca7689434ce7dea99ff4826e7.1659764848.git.Takahiro.Kuwano@infineon.com/
> 
> Takahiro Kuwano (2):
>   mtd: spi-nor: core: Add a helper to read with alternative method
>   mtd: spi-nor: spansion: Discover current address mode by CRC
> 
>  drivers/mtd/spi-nor/core.c     |  48 ++++++++++++
>  drivers/mtd/spi-nor/core.h     |   2 +
>  drivers/mtd/spi-nor/sfdp.h     |   1 +
>  drivers/mtd/spi-nor/spansion.c | 130 +++++++++++++++++++++++++++++++++
>  4 files changed, 181 insertions(+)
> 

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 2/2] mtd: spi-nor: spansion: Discover current address mode by CRC

[Tudor.Ambarus]

Hi, Takahiro,

On 8/12/22 11:06, tkuw584924@gmail.com wrote:
> +#define CRC32C_POLY    0x1edc6f41
> +static u32 cypress_nor_calc_crc32c(u32 len, u8 *buf)
> +{
> +       u32 crc = 0;
> +       int i;
> +
> +       while (len--) {
> +               crc ^= *buf++ << 24;
> +               for (i = 0; i < 8; i++)
> +                       crc = (crc << 1) ^ ((crc & 0x80000000) ? CRC32C_POLY :
> +                                           0);
> +       }
> +
> +       return crc;
> +}

I haven't gone through the details of the patch set, but I like the idea.
I stumbled on your crc implementation. Can't we use #include <linux/crc32.h>
instead?

-- 
Cheers,
ta
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 2/2] mtd: spi-nor: spansion: Discover current address mode by CRC

[Takahiro Kuwano]

On 10/5/2022 4:10 PM, Tudor.Ambarus@microchip.com wrote:
> Hi, Takahiro,
> 
> On 8/12/22 11:06, tkuw584924@gmail.com wrote:
>> +#define CRC32C_POLY    0x1edc6f41
>> +static u32 cypress_nor_calc_crc32c(u32 len, u8 *buf)
>> +{
>> +       u32 crc = 0;
>> +       int i;
>> +
>> +       while (len--) {
>> +               crc ^= *buf++ << 24;
>> +               for (i = 0; i < 8; i++)
>> +                       crc = (crc << 1) ^ ((crc & 0x80000000) ? CRC32C_POLY :
>> +                                           0);
>> +       }
>> +
>> +       return crc;
>> +}
> 
> I haven't gone through the details of the patch set, but I like the idea.
> I stumbled on your crc implementation. Can't we use #include <linux/crc32.h>
> instead?
> 
CRC32C BE is a missing piece in lib/crc32 so I added this local helper.

I have created following patch that adds CRC32C BE into lib/crc32.
Do you agree to submit this? 
We also need to add test case to crc32test.c.


diff --git a/include/linux/crc32.h b/include/linux/crc32.h
index 9e8a032c1788..adbfd886ccf8 100644
--- a/include/linux/crc32.h
+++ b/include/linux/crc32.h
@@ -37,6 +37,7 @@ static inline u32 crc32_le_combine(u32 crc1, u32 crc2, size_t len2)
 }
 
 u32 __pure __crc32c_le(u32 crc, unsigned char const *p, size_t len);
+u32 __pure __crc32c_be(u32 crc, unsigned char const *p, size_t len);
 
 /**
  * __crc32c_le_combine - Combine two crc32c check values into one. For two
diff --git a/include/linux/crc32poly.h b/include/linux/crc32poly.h
index 62c4b7790a28..ed5d81f61d4e 100644
--- a/include/linux/crc32poly.h
+++ b/include/linux/crc32poly.h
@@ -16,5 +16,6 @@
  * x^8+x^6+x^0
  */
 #define CRC32C_POLY_LE 0x82F63B78
+#define CRC32C_POLY_BE 0x1EDC6F41
 
 #endif /* _LINUX_CRC32_POLY_H */
diff --git a/lib/crc32.c b/lib/crc32.c
index 5649847d0a8d..15dc6f99ff46 100644
--- a/lib/crc32.c
+++ b/lib/crc32.c
@@ -335,10 +335,19 @@ u32 __pure __weak crc32_be(u32 crc, unsigned char const *p, size_t len)
 {
 	return crc32_be_generic(crc, p, len, NULL, CRC32_POLY_BE);
 }
+u32 __pure __weak __crc32c_be(u32 crc, unsigned char const *p, size_t len)
+{
+	return crc32_be_generic(crc, p, len, NULL, CRC32C_POLY_BE);
+}
 #else
 u32 __pure __weak crc32_be(u32 crc, unsigned char const *p, size_t len)
 {
 	return crc32_be_generic(crc, p, len, crc32table_be, CRC32_POLY_BE);
 }
+u32 __pure __weak __crc32c_be(u32 crc, unsigned char const *p, size_t len)
+{
+	return crc32_be_generic(crc, p, len, crc32ctable_be, CRC32C_POLY_BE);
+}
 #endif
 EXPORT_SYMBOL(crc32_be);
+EXPORT_SYMBOL(__crc32c_be);
diff --git a/lib/gen_crc32table.c b/lib/gen_crc32table.c
index f755b997b967..fcb90c496e48 100644
--- a/lib/gen_crc32table.c
+++ b/lib/gen_crc32table.c
@@ -26,6 +26,7 @@
 static uint32_t crc32table_le[LE_TABLE_ROWS][256];
 static uint32_t crc32table_be[BE_TABLE_ROWS][256];
 static uint32_t crc32ctable_le[LE_TABLE_ROWS][256];
+static uint32_t crc32ctable_be[BE_TABLE_ROWS][256];
 
 /**
  * crc32init_le() - allocate and initialize LE table data
@@ -67,29 +68,40 @@ static void crc32cinit_le(void)
 }
 
 /**
- * crc32init_be() - allocate and initialize BE table data
+ * crc32init_be_generic() - allocate and initialize BE table data
  */
-static void crc32init_be(void)
+static void crc32init_be_generic(const uint32_t polynomial,
+				 uint32_t (*tab)[256])
 {
 	unsigned i, j;
 	uint32_t crc = 0x80000000;
 
-	crc32table_be[0][0] = 0;
+	tab[0][0] = 0;
 
 	for (i = 1; i < BE_TABLE_SIZE; i <<= 1) {
-		crc = (crc << 1) ^ ((crc & 0x80000000) ? CRC32_POLY_BE : 0);
+		crc = (crc << 1) ^ ((crc & 0x80000000) ? polynomial : 0);
 		for (j = 0; j < i; j++)
-			crc32table_be[0][i + j] = crc ^ crc32table_be[0][j];
+			tab[0][i + j] = crc ^ tab[0][j];
 	}
 	for (i = 0; i < BE_TABLE_SIZE; i++) {
-		crc = crc32table_be[0][i];
+		crc = tab[0][i];
 		for (j = 1; j < BE_TABLE_ROWS; j++) {
-			crc = crc32table_be[0][(crc >> 24) & 0xff] ^ (crc << 8);
-			crc32table_be[j][i] = crc;
+			crc = tab[0][(crc >> 24) & 0xff] ^ (crc << 8);
+			tab[j][i] = crc;
 		}
 	}
 }
 
+static void crc32init_be(void)
+{
+	crc32init_be_generic(CRC32_POLY_BE, crc32table_be);
+}
+
+static void crc32cinit_be(void)
+{
+	crc32init_be_generic(CRC32C_POLY_BE, crc32ctable_be);
+}
+
 static void output_table(uint32_t (*table)[256], int rows, int len, char *trans)
 {
 	int i, j;
@@ -137,6 +149,15 @@ int main(int argc, char** argv)
 			     LE_TABLE_SIZE, "tole");
 		printf("};\n");
 	}
+	if (CRC_BE_BITS > 1) {
+		crc32cinit_be();
+		printf("static const u32 ____cacheline_aligned "
+		       "crc32ctable_be[%d][%d] = {",
+		       BE_TABLE_ROWS, BE_TABLE_SIZE);
+		output_table(crc32ctable_be, BE_TABLE_ROWS,
+			     BE_TABLE_SIZE, "tobe");
+		printf("};\n");
+	}
 
 	return 0;
 }

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/