[PATCH 0/5] mtd: rawnand: qcom: Add support for QSPI nand

[PATCH 0/5] mtd: rawnand: qcom: Add support for QSPI nand

[Md Sadre Alam]

QPIC 2.0 supports Serial NAND support in addition to all features and
commands in QPIC 1.0 for parallel NAND. Parallel and Serial NAND cannot
operate simultaneously. QSPI nand devices will connect to QPIC IO_MACRO
block of QPIC controller. There is a separate IO_MACRO clock for IO_MACRO
block. Default IO_MACRO block divide the input clock by 4. so if IO_MACRO
input clock is 320MHz then on bus it will be 80MHz, so QSPI nand device
should also support this frequency.

QPIC provides 4 data pins to QSPI nand. In standard SPI mode (x1 mode) data
transfer will occur on only 2 pins one pin for Serial data in and one for
serial data out. In QUAD SPI mode (x4 mode) data transfer will occur at all
the four data lines. QPIC controller supports command for x1 mode and x4 mode.

Md Sadre Alam (5):
  dt-bindings: qcom_nandc: IPQ5018 QPIC NAND documentation
  mtd: rawnand: qcom: Add initial support for qspi nand
  mtd: rawnand: qcom: Read QPIC version
  mtd: rawnand: qcom: Enable support for erase,read & write for serial
    nand.
  mtd: rawnand: qcom: Add support for serial training.

 .../devicetree/bindings/mtd/qcom_nandc.txt         |   3 +
 drivers/mtd/nand/raw/nand_ids.c                    |  13 +
 drivers/mtd/nand/raw/qcom_nandc.c                  | 502 ++++++++++++++++++++-
 3 files changed, 494 insertions(+), 24 deletions(-)

-- 
2.7.4

[PATCH 1/5] dt-bindings: qcom_nandc: IPQ5018 QPIC NAND documentation

[Md Sadre Alam]

Qualcom IPQ5018 SoC uses QPIC NAND controller version 2.1.1
which uses BAM DMA Engine and QSPI serial nand interface.

Signed-off-by: Md Sadre Alam <mdalam@codeaurora.org>
---
 Documentation/devicetree/bindings/mtd/qcom_nandc.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/mtd/qcom_nandc.txt b/Documentation/devicetree/bindings/mtd/qcom_nandc.txt
index 5c2fba4..0bfa316 100644
--- a/Documentation/devicetree/bindings/mtd/qcom_nandc.txt
+++ b/Documentation/devicetree/bindings/mtd/qcom_nandc.txt
@@ -8,6 +8,9 @@ Required properties:
                             IPQ4019 SoC and it uses BAM DMA
     * "qcom,ipq8074-nand" - for QPIC NAND controller v1.5.0 being used in
                             IPQ8074 SoC and it uses BAM DMA
+    * "qcom,ipq5018-nand" - for QPIC NAND controller v2.1.1 being used in
+                            IPQ5018 SoC and it uses BAM DMA and QSPI serial
+                            nand interface.
 
 - reg:			MMIO address range
 - clocks:		must contain core clock and always on clock
-- 
2.7.4

Re: [PATCH 1/5] dt-bindings: qcom_nandc: IPQ5018 QPIC NAND documentation

[Rob Herring]

On Sat, 10 Oct 2020 11:01:38 +0530, Md Sadre Alam wrote:
> Qualcom IPQ5018 SoC uses QPIC NAND controller version 2.1.1
> which uses BAM DMA Engine and QSPI serial nand interface.
> 
> Signed-off-by: Md Sadre Alam <mdalam@codeaurora.org>
> ---
>  Documentation/devicetree/bindings/mtd/qcom_nandc.txt | 3 +++
>  1 file changed, 3 insertions(+)
> 

Acked-by: Rob Herring <robh@kernel.org>

[PATCH 2/5] mtd: rawnand: qcom: Add initial support for qspi nand

[Md Sadre Alam]

This change will add initial support for qspi (serial nand).

QPIC Version v.2.0 onwards supports serial nand as well so this
change will initialize all required register to enable qspi (serial
nand).

This change is supporting very basic functionality of qspi nand flash.

1. Reset device (Reset QSPI NAND device).

2. Device detection (Read id QSPI NAND device).

Signed-off-by: Md Sadre Alam <mdalam@codeaurora.org>
---
 drivers/mtd/nand/raw/nand_ids.c   |  13 +++
 drivers/mtd/nand/raw/qcom_nandc.c | 168 +++++++++++++++++++++++++++++++++++---
 2 files changed, 171 insertions(+), 10 deletions(-)

diff --git a/drivers/mtd/nand/raw/nand_ids.c b/drivers/mtd/nand/raw/nand_ids.c
index b994579..32bc419 100644
--- a/drivers/mtd/nand/raw/nand_ids.c
+++ b/drivers/mtd/nand/raw/nand_ids.c
@@ -55,6 +55,19 @@ struct nand_flash_dev nand_flash_ids[] = {
 		{ .id = {0x98, 0xdc, 0x91, 0x15, 0x76} },
 		  SZ_2K, SZ_512, SZ_128K, 0, 5, 128, NAND_ECC_INFO(8, SZ_512) },
 
+	{"GD5F1GQ4RE9IG SPI NAND 1G 1.8V 4-bit",
+		{ .id = {0xc8, 0xc1} },
+		  SZ_2K, SZ_128, SZ_128K, 0, 2, 128, NAND_ECC_INFO(8, SZ_512) },
+	{"GD5F1GQ4RE9IH SPI NAND 1G 1.8V 4-bit",
+		{ .id = {0xc8, 0xc9} },
+		SZ_2K, SZ_128, SZ_128K, 0, 2, 64, NAND_ECC_INFO(4, SZ_512) },
+	{"GD5F2GQ5REYIH SPI NAND 2G 4-bit",
+		{ .id = {0xc8, 0x22} },
+		SZ_2K, SZ_256, SZ_128K, 0, 2, 64, NAND_ECC_INFO(4, SZ_512) },
+	{"MT29F1G01ABBFDWB-IT SPI NAND 1G 1.8V 4-bit",
+		{ .id = {0x2c, 0x15} },
+		SZ_2K, SZ_128, SZ_128K, 0, 2, 128, NAND_ECC_INFO(8, SZ_512) },
+
 	LEGACY_ID_NAND("NAND 4MiB 5V 8-bit",   0x6B, 4, SZ_8K, SP_OPTIONS),
 	LEGACY_ID_NAND("NAND 4MiB 3,3V 8-bit", 0xE3, 4, SZ_8K, SP_OPTIONS),
 	LEGACY_ID_NAND("NAND 4MiB 3,3V 8-bit", 0xE5, 4, SZ_8K, SP_OPTIONS),
diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
index bd7a725..f5064ab 100644
--- a/drivers/mtd/nand/raw/qcom_nandc.c
+++ b/drivers/mtd/nand/raw/qcom_nandc.c
@@ -36,18 +36,33 @@
 #define	NAND_DEV_CMD1			0xa4
 #define	NAND_DEV_CMD2			0xa8
 #define	NAND_DEV_CMD_VLD		0xac
+#define NAND_DEV_CMD7			0xb0
+#define NAND_DEV_CMD8			0xb4
+#define NAND_DEV_CMD9			0xb8
+#define NAND_FLASH_SPI_CFG		0xc0
+#define NAND_SPI_NUM_ADDR_CYCLES	0xc4
+#define NAND_SPI_BUSY_CHECK_WAIT_CNT	0xc8
+#define NAND_DEV_CMD3			0xd0
+#define NAND_DEV_CMD4			0xd4
+#define NAND_DEV_CMD5			0xd8
+#define NAND_DEV_CMD6			0xdc
 #define	SFLASHC_BURST_CFG		0xe0
 #define	NAND_ERASED_CW_DETECT_CFG	0xe8
 #define	NAND_ERASED_CW_DETECT_STATUS	0xec
 #define	NAND_EBI2_ECC_BUF_CFG		0xf0
 #define	FLASH_BUF_ACC			0x100
-
 #define	NAND_CTRL			0xf00
 #define	NAND_VERSION			0xf08
 #define	NAND_READ_LOCATION_0		0xf20
 #define	NAND_READ_LOCATION_1		0xf24
 #define	NAND_READ_LOCATION_2		0xf28
 #define	NAND_READ_LOCATION_3		0xf2c
+#define NAND_READ_LOCATION_LAST_CW_0	0xf40
+#define NAND_READ_LOCATION_LAST_CW_1	0xf44
+#define NAND_READ_LOCATION_LAST_CW_2	0xf48
+#define NAND_READ_LOCATION_LAST_CW_3	0xf4c
+#define NAND_QSPI_MSTR_CONFIG		0xf60
+
 
 /* dummy register offsets, used by write_reg_dma */
 #define	NAND_DEV_CMD1_RESTORE		0xdead
@@ -180,6 +195,28 @@
 #define	ECC_BCH_4BIT	BIT(2)
 #define	ECC_BCH_8BIT	BIT(3)
 
+/* QSPI NAND config reg bits */
+#define LOAD_CLK_CNTR_INIT_EN	(1 << 28)
+#define CLK_CNTR_INIT_VAL_VEC	0x924
+#define FEA_STATUS_DEV_ADDR	0xc0
+#define SPI_CFG	(1 << 0)
+
+/* CMD register value for qspi nand */
+#define CMD0_VAL	0x1080D8D8
+#define CMD1_VAL	0xF00F3000
+#define CMD2_VAL	0xF0FF709F
+#define CMD3_VAL	0x3F310015
+#define CMD7_VAL	0x04061F0F
+#define CMD_VLD_VAL	0xd
+#define SPI_NUM_ADDR	0xDA4DB
+#define WAIT_CNT	0x10
+
+/* QSPI NAND CMD reg bits value */
+#define SPI_WP		(1 << 28)
+#define SPI_HOLD	(1 << 27)
+#define SPI_TRANSFER_MODE_x1	(1 << 29)
+#define SPI_TRANSFER_MODE_x4	(3 << 29)
+
 #define nandc_set_read_loc(nandc, reg, offset, size, is_last)	\
 nandc_set_reg(nandc, NAND_READ_LOCATION_##reg,			\
 	      ((offset) << READ_LOCATION_OFFSET) |		\
@@ -315,6 +352,9 @@ struct nandc_regs {
 	__le32 read_location1;
 	__le32 read_location2;
 	__le32 read_location3;
+	__le32 spi_cfg;
+	__le32 num_addr_cycle;
+	__le32 busy_wait_cnt;
 
 	__le32 erased_cw_detect_cfg_clr;
 	__le32 erased_cw_detect_cfg_set;
@@ -368,6 +408,7 @@ struct qcom_nand_controller {
 
 	struct clk *core_clk;
 	struct clk *aon_clk;
+	struct clk *iomacro_clk;
 
 	union {
 		/* will be used only by QPIC for BAM DMA */
@@ -461,12 +502,14 @@ struct qcom_nand_host {
  * @is_bam - whether NAND controller is using BAM
  * @is_qpic - whether NAND CTRL is part of qpic IP
  * @dev_cmd_reg_start - NAND_DEV_CMD_* registers starting offset
+ * @is_serial_nand - QSPI nand flag, whether QPIC support serial nand or not
  */
 struct qcom_nandc_props {
 	u32 ecc_modes;
 	bool is_bam;
 	bool is_qpic;
 	u32 dev_cmd_reg_start;
+	bool is_serial_nand;
 };
 
 /* Frees the BAM transaction memory */
@@ -641,6 +684,12 @@ static __le32 *offset_to_nandc_reg(struct nandc_regs *regs, int offset)
 		return &regs->read_location2;
 	case NAND_READ_LOCATION_3:
 		return &regs->read_location3;
+	case NAND_FLASH_SPI_CFG:
+		return &regs->spi_cfg;
+	case NAND_SPI_NUM_ADDR_CYCLES:
+		return &regs->num_addr_cycle;
+	case NAND_SPI_BUSY_CHECK_WAIT_CNT:
+		return &regs->busy_wait_cnt;
 	default:
 		return NULL;
 	}
@@ -1245,11 +1294,23 @@ static int read_id(struct qcom_nand_host *host, int column)
 {
 	struct nand_chip *chip = &host->chip;
 	struct qcom_nand_controller *nandc = get_qcom_nand_controller(chip);
+	u32 cmd = OP_FETCH_ID;
 
 	if (column == -1)
 		return 0;
 
-	nandc_set_reg(nandc, NAND_FLASH_CMD, OP_FETCH_ID);
+	if (nandc->props->is_serial_nand) {
+		cmd |= (SPI_WP | SPI_HOLD | SPI_TRANSFER_MODE_x1);
+		/* For spi nand read 2-bytes id only
+		 * else if nandc->buf_count == 4; then the id value
+		 * will repeat and the SLC device will be detect as MLC.
+		 * by nand base layer
+		 * so overwrite the nandc->buf_count == 2;
+		 */
+		nandc->buf_count = 2;
+	}
+
+	nandc_set_reg(nandc, NAND_FLASH_CMD, cmd);
 	nandc_set_reg(nandc, NAND_ADDR0, column);
 	nandc_set_reg(nandc, NAND_ADDR1, 0);
 	nandc_set_reg(nandc, NAND_FLASH_CHIP_SELECT,
@@ -1269,8 +1330,13 @@ static int reset(struct qcom_nand_host *host)
 {
 	struct nand_chip *chip = &host->chip;
 	struct qcom_nand_controller *nandc = get_qcom_nand_controller(chip);
+	int cmd_rst;
+
+	cmd_rst = OP_RESET_DEVICE;
+	if (nandc->props->is_serial_nand)
+		cmd_rst |= (SPI_WP | SPI_HOLD | SPI_TRANSFER_MODE_x1);
 
-	nandc_set_reg(nandc, NAND_FLASH_CMD, OP_RESET_DEVICE);
+	nandc_set_reg(nandc, NAND_FLASH_CMD, cmd_rst);
 	nandc_set_reg(nandc, NAND_EXEC_CMD, 1);
 
 	write_reg_dma(nandc, NAND_FLASH_CMD, 1, NAND_BAM_NEXT_SGL);
@@ -2470,6 +2536,8 @@ static int qcom_nand_attach_chip(struct nand_chip *chip)
 	int cwperpage, bad_block_byte, ret;
 	bool wide_bus;
 	int ecc_mode = 1;
+	int num_addr_cycle = 5, dsbl_sts_aftr_write = 0;
+	int wr_rd_bsy_gap = 2, recovery_cycle = 7;
 
 	/* controller only supports 512 bytes data steps */
 	ecc->size = NANDC_STEP_SIZE;
@@ -2571,33 +2639,43 @@ static int qcom_nand_attach_chip(struct nand_chip *chip)
 	host->cw_size = host->cw_data + ecc->bytes;
 	bad_block_byte = mtd->writesize - host->cw_size * (cwperpage - 1) + 1;
 
+	/* For QSPI serial nand QPIC config register value got changed
+	 * so configure the new value for qspi serial nand
+	 */
+	if (nandc->props->is_serial_nand) {
+		num_addr_cycle = 3;
+		dsbl_sts_aftr_write = 1;
+		wr_rd_bsy_gap = 20;
+		recovery_cycle = 0;
+	}
+
 	host->cfg0 = (cwperpage - 1) << CW_PER_PAGE
 				| host->cw_data << UD_SIZE_BYTES
-				| 0 << DISABLE_STATUS_AFTER_WRITE
-				| 5 << NUM_ADDR_CYCLES
+				| dsbl_sts_aftr_write << DISABLE_STATUS_AFTER_WRITE
+				| num_addr_cycle << NUM_ADDR_CYCLES
 				| host->ecc_bytes_hw << ECC_PARITY_SIZE_BYTES_RS
 				| 0 << STATUS_BFR_READ
 				| 1 << SET_RD_MODE_AFTER_STATUS
 				| host->spare_bytes << SPARE_SIZE_BYTES;
 
-	host->cfg1 = 7 << NAND_RECOVERY_CYCLES
+	host->cfg1 = recovery_cycle << NAND_RECOVERY_CYCLES
 				| 0 <<  CS_ACTIVE_BSY
 				| bad_block_byte << BAD_BLOCK_BYTE_NUM
 				| 0 << BAD_BLOCK_IN_SPARE_AREA
-				| 2 << WR_RD_BSY_GAP
+				| wr_rd_bsy_gap << WR_RD_BSY_GAP
 				| wide_bus << WIDE_FLASH
 				| host->bch_enabled << ENABLE_BCH_ECC;
 
 	host->cfg0_raw = (cwperpage - 1) << CW_PER_PAGE
 				| host->cw_size << UD_SIZE_BYTES
-				| 5 << NUM_ADDR_CYCLES
+				| num_addr_cycle << NUM_ADDR_CYCLES
 				| 0 << SPARE_SIZE_BYTES;
 
-	host->cfg1_raw = 7 << NAND_RECOVERY_CYCLES
+	host->cfg1_raw = recovery_cycle << NAND_RECOVERY_CYCLES
 				| 0 << CS_ACTIVE_BSY
 				| 17 << BAD_BLOCK_BYTE_NUM
 				| 1 << BAD_BLOCK_IN_SPARE_AREA
-				| 2 << WR_RD_BSY_GAP
+				| wr_rd_bsy_gap << WR_RD_BSY_GAP
 				| wide_bus << WIDE_FLASH
 				| 1 << DEV0_CFG1_ECC_DISABLE;
 
@@ -2805,6 +2883,47 @@ static int qcom_nandc_setup(struct qcom_nand_controller *nandc)
 	return 0;
 }
 
+static void qspi_write_reg_bam(struct qcom_nand_controller *nandc,
+		unsigned int val, unsigned int reg)
+{
+	int ret;
+
+	clear_bam_transaction(nandc);
+	nandc_set_reg(nandc, reg, val);
+	write_reg_dma(nandc, reg, 1, NAND_BAM_NEXT_SGL);
+
+	ret = submit_descs(nandc);
+	if (ret)
+		dev_err(nandc->dev, "Error in submitting descriptor to write config reg\n");
+	free_descs(nandc);
+}
+
+static void qspi_nand_init(struct qcom_nand_controller *nandc)
+{
+	u32 spi_cfg_val = 0x0;
+	u32 reg = 0x0;
+
+	spi_cfg_val |= (LOAD_CLK_CNTR_INIT_EN | CLK_CNTR_INIT_VAL_VEC
+			| FEA_STATUS_DEV_ADDR | SPI_CFG);
+
+	qspi_write_reg_bam(nandc, 0x0, NAND_FLASH_SPI_CFG);
+	qspi_write_reg_bam(nandc, spi_cfg_val, NAND_FLASH_SPI_CFG);
+	spi_cfg_val &= ~LOAD_CLK_CNTR_INIT_EN;
+	qspi_write_reg_bam(nandc, spi_cfg_val, NAND_FLASH_SPI_CFG);
+
+	reg = dev_cmd_reg_addr(nandc, NAND_DEV_CMD0);
+	nandc_write(nandc, reg, CMD0_VAL);
+	nandc_write(nandc, reg + 4, CMD1_VAL);
+	nandc_write(nandc, reg + 8, CMD2_VAL);
+	nandc_write(nandc, reg + 12, CMD_VLD_VAL);
+	nandc_write(nandc, reg + 16, CMD7_VAL);
+	reg = dev_cmd_reg_addr(nandc, NAND_DEV_CMD3);
+	nandc_write(nandc, reg, CMD3_VAL);
+
+	qspi_write_reg_bam(nandc, SPI_NUM_ADDR, NAND_SPI_NUM_ADDR_CYCLES);
+	qspi_write_reg_bam(nandc, WAIT_CNT, NAND_SPI_BUSY_CHECK_WAIT_CNT);
+}
+
 static int qcom_nand_host_init_and_register(struct qcom_nand_controller *nandc,
 					    struct qcom_nand_host *host,
 					    struct device_node *dn)
@@ -2854,6 +2973,9 @@ static int qcom_nand_host_init_and_register(struct qcom_nand_controller *nandc,
 	/* set up initial status value */
 	host->status = NAND_STATUS_READY | NAND_STATUS_WP;
 
+	if (nandc->props->is_serial_nand)
+		qspi_nand_init(nandc);
+
 	ret = nand_scan(chip, 1);
 	if (ret)
 		return ret;
@@ -2961,6 +3083,12 @@ static int qcom_nandc_probe(struct platform_device *pdev)
 	if (IS_ERR(nandc->aon_clk))
 		return PTR_ERR(nandc->aon_clk);
 
+	if (nandc->props->is_serial_nand) {
+		nandc->iomacro_clk = devm_clk_get(dev, "io_macro");
+		if (IS_ERR(nandc->iomacro_clk))
+			return PTR_ERR(nandc->iomacro_clk);
+	}
+
 	ret = qcom_nandc_parse_dt(pdev);
 	if (ret)
 		return ret;
@@ -2989,6 +3117,12 @@ static int qcom_nandc_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_aon_clk;
 
+	if (nandc->props->is_serial_nand) {
+		ret = clk_prepare_enable(nandc->iomacro_clk);
+		if (ret)
+			goto err_setup;
+	}
+
 	ret = qcom_nandc_setup(nandc);
 	if (ret)
 		goto err_setup;
@@ -3042,6 +3176,7 @@ static const struct qcom_nandc_props ipq806x_nandc_props = {
 	.ecc_modes = (ECC_RS_4BIT | ECC_BCH_8BIT),
 	.is_bam = false,
 	.dev_cmd_reg_start = 0x0,
+	.is_serial_nand = false,
 };
 
 static const struct qcom_nandc_props ipq4019_nandc_props = {
@@ -3049,6 +3184,7 @@ static const struct qcom_nandc_props ipq4019_nandc_props = {
 	.is_bam = true,
 	.is_qpic = true,
 	.dev_cmd_reg_start = 0x0,
+	.is_serial_nand = false,
 };
 
 static const struct qcom_nandc_props ipq8074_nandc_props = {
@@ -3056,6 +3192,14 @@ static const struct qcom_nandc_props ipq8074_nandc_props = {
 	.is_bam = true,
 	.is_qpic = true,
 	.dev_cmd_reg_start = 0x7000,
+	.is_serial_nand = false,
+};
+
+static const struct qcom_nandc_props ipq5018_nandc_props = {
+	.ecc_modes = (ECC_BCH_4BIT | ECC_BCH_8BIT),
+	.is_bam = true,
+	.dev_cmd_reg_start = 0x7000,
+	.is_serial_nand = true,
 };
 
 /*
@@ -3075,6 +3219,10 @@ static const struct of_device_id qcom_nandc_of_match[] = {
 		.compatible = "qcom,ipq8074-nand",
 		.data = &ipq8074_nandc_props,
 	},
+	{
+		.compatible = "qcom,ipq5018-nand",
+		.data = &ipq5018_nandc_props,
+	},
 	{}
 };
 MODULE_DEVICE_TABLE(of, qcom_nandc_of_match);
-- 
2.7.4

Re: [PATCH 2/5] mtd: rawnand: qcom: Add initial support for qspi nand

[Boris Brezillon]

Hello,

On Sat, 10 Oct 2020 11:01:39 +0530
Md Sadre Alam <mdalam@codeaurora.org> wrote:

> This change will add initial support for qspi (serial nand).
> 
> QPIC Version v.2.0 onwards supports serial nand as well so this
> change will initialize all required register to enable qspi (serial
> nand).
> 
> This change is supporting very basic functionality of qspi nand flash.
> 
> 1. Reset device (Reset QSPI NAND device).
> 
> 2. Device detection (Read id QSPI NAND device).

Unfortunately, that's not going to work in the long term. You're
basically hacking the raw NAND framework to make SPI NANDs fit. I do
understand the rationale behind this decision (re-using the code for
ECC and probably other things), but that's not going to work. So I'd
recommend doing the following instead:

1/ implement a SPI-mem controller driver
2/ implement an ECC engine driver so the ECC logic can be shared
   between the SPI controller and raw NAND controller drivers
3/ convert the raw NAND driver to the exec_op() interface (none of
   this hack would have been possible if the driver was using the new
   API)

Regards,

Boris

Re: [PATCH 2/5] mtd: rawnand: qcom: Add initial support for qspi nand

[Md Sadre Alam]

On 10/29/2020 2:37 PM, Boris Brezillon wrote:
> Hello,
>
> On Sat, 10 Oct 2020 11:01:39 +0530
> Md Sadre Alam <mdalam@codeaurora.org> wrote:
>
>> This change will add initial support for qspi (serial nand).
>>
>> QPIC Version v.2.0 onwards supports serial nand as well so this
>> change will initialize all required register to enable qspi (serial
>> nand).
>>
>> This change is supporting very basic functionality of qspi nand flash.
>>
>> 1. Reset device (Reset QSPI NAND device).
>>
>> 2. Device detection (Read id QSPI NAND device).
> Unfortunately, that's not going to work in the long term. You're
> basically hacking the raw NAND framework to make SPI NANDs fit. I do
> understand the rationale behind this decision (re-using the code for
> ECC and probably other things), but that's not going to work. So I'd
> recommend doing the following instead:
>
> 1/ implement a SPI-mem controller driver
> 2/ implement an ECC engine driver so the ECC logic can be shared
>     between the SPI controller and raw NAND controller drivers
> 3/ convert the raw NAND driver to the exec_op() interface (none of
>     this hack would have been possible if the driver was using the new
>     API)
>
> Regards,
>
> Boris
>
    Sorry for late reply, again started working on this feature 
support.  The QPIC v2 on wards there is serial nand support got added , 
its not a standard SPI controller

    its QPIC controller having support for serial nand. All SPI related 
configuration done by QPIC hardware and its not exposed as SPI bus to 
the external world. Only based on

    QPIC_SPI_CFG = 1, serial functionality will get selected. So that no 
need to implement as SPI-mem controller driver, since its not a SPI 
controller.

   Please check the below diagram for top view of QPIC controller.


                                                   QPIC Wrapper

___________________________________________________________________________________________

                     AHB Master IF                        | 
____________________________________ |____________________________|        |

<-----------------------------------------     |             |         
                  |               |                    QPIC IO_MACRO     
        |        |

                                     |                           |     
                         QPIC           |               |             
(control the SPI BUS         |        | 
-------------------------------------------------->  Serial NAND IOs

                                     |                           |     
        QPIC_SPI_CFG = 1       --------------->     |          |         
        internally)                            |         |  
-------------------------------------------------->

                                     |                           |     
            |               |                             |         |

                                     | |                 |               
|_____________________________|      |

                                     |                           |     
            | | |

                                     |                           |     
         |               |  _____________________________|     |

                                     |                           |     
          |               |                                 |        | 
                                       | |                  
|               | Ebi2_ext_if                       |         |

                                     | | | | |         |

                                    |                            |    
          QPIC_SPI_CFG = 0  ----------------------> |           |       
       | |-------------------------------------------------> Parallel 
NAND IOs

                                    |                            |     
             |               |                                  | 
|--------------------------------------------------->

<------------------------------------------ 
|_____________|_____________________________________|_______|______________________________|____|

                            AHB Slave IF


   Regards,

Alam.


>

Re: [PATCH 2/5] mtd: rawnand: qcom: Add initial support for qspi nand

[Md Sadre Alam]

On 3/6/2023 7:45 PM, Md Sadre Alam wrote:
>
> On 10/29/2020 2:37 PM, Boris Brezillon wrote:
>> Hello,
>>
>> On Sat, 10 Oct 2020 11:01:39 +0530
>> Md Sadre Alam <mdalam@codeaurora.org> wrote:
>>
>>> This change will add initial support for qspi (serial nand).
>>>
>>> QPIC Version v.2.0 onwards supports serial nand as well so this
>>> change will initialize all required register to enable qspi (serial
>>> nand).
>>>
>>> This change is supporting very basic functionality of qspi nand flash.
>>>
>>> 1. Reset device (Reset QSPI NAND device).
>>>
>>> 2. Device detection (Read id QSPI NAND device).
>> Unfortunately, that's not going to work in the long term. You're
>> basically hacking the raw NAND framework to make SPI NANDs fit. I do
>> understand the rationale behind this decision (re-using the code for
>> ECC and probably other things), but that's not going to work. So I'd
>> recommend doing the following instead:
>>
>> 1/ implement a SPI-mem controller driver
>> 2/ implement an ECC engine driver so the ECC logic can be shared
>>     between the SPI controller and raw NAND controller drivers
>> 3/ convert the raw NAND driver to the exec_op() interface (none of
>>     this hack would have been possible if the driver was using the new
>>     API)
>>
>> Regards,
>>
>> Boris
>>
>    Sorry for late reply, again started working on this feature 
> support.  The QPIC v2 on wards there is serial nand support got added 
> , its not a standard SPI controller
>
>    its QPIC controller having support for serial nand. All SPI related 
> configuration done by QPIC hardware and its not exposed as SPI bus to 
> the external world. Only based on
>
>    QPIC_SPI_CFG = 1, serial functionality will get selected. So that 
> no need to implement as SPI-mem controller driver, since its not a SPI 
> controller.
>
>   Please check the below diagram for top view of QPIC controller.
>
>
___________________________________________________________________________

| QPIC wrapper                        |

             AHB master IF                                     | 
|_______________________                __________________________     
             |

<-------------------------------------------------     |                 
   |          QPIC                                |             |    
QPIC_IO_MACRO | |--------------------------------------------->Serial 
NAND IOs

|                        |     QPIC_SPI_CFG = 1     ----> |             
| (Control SPI Bus internally) | 
|--------------------------------------------->

| |                                                   | | 
|                 |

| |                                                   | | 
|                  |

| |                                                   | 
|__________________________|                 |

| |                                                    |          _ 
___________________________                 |

| |                                                    | |               
Ebi2_ext_if                        | |

|                        |   QPIC_SPI_CFG=0     -------> | | | 
|------------------------------------------------->

| |                                                     | | | 
|-------------------------------------------------->Parallel NAND IOs

<----------------------------------------------------   |               
    |________________________ |           | __________________________| 
              |

           AHB Slav IF | |

|_____________________________________________________________________________|

>
>
>
>   Regards,
>
> Alam.
>
>
>>

Re: [PATCH 2/5] mtd: rawnand: qcom: Add initial support for qspi nand

[Miquel Raynal]

Hello,

quic_mdalam@quicinc.com wrote on Mon, 6 Mar 2023 19:45:58 +0530:

> On 10/29/2020 2:37 PM, Boris Brezillon wrote:
> > Hello,
> >
> > On Sat, 10 Oct 2020 11:01:39 +0530
> > Md Sadre Alam <mdalam@codeaurora.org> wrote:
> >  
> >> This change will add initial support for qspi (serial nand).
> >>
> >> QPIC Version v.2.0 onwards supports serial nand as well so this
> >> change will initialize all required register to enable qspi (serial
> >> nand).
> >>
> >> This change is supporting very basic functionality of qspi nand flash.
> >>
> >> 1. Reset device (Reset QSPI NAND device).
> >>
> >> 2. Device detection (Read id QSPI NAND device).  
> > Unfortunately, that's not going to work in the long term. You're
> > basically hacking the raw NAND framework to make SPI NANDs fit. I do
> > understand the rationale behind this decision (re-using the code for
> > ECC and probably other things), but that's not going to work. So I'd
> > recommend doing the following instead:
> >
> > 1/ implement a SPI-mem controller driver
> > 2/ implement an ECC engine driver so the ECC logic can be shared
> >     between the SPI controller and raw NAND controller drivers
> > 3/ convert the raw NAND driver to the exec_op() interface (none of
> >     this hack would have been possible if the driver was using the new
> >     API)
> >
> > Regards,
> >
> > Boris
> >  
>     Sorry for late reply, again started working on this feature support.  The QPIC v2 on wards there is serial nand support got added , its not a standard SPI controller
> 
>     its QPIC controller having support for serial nand. All SPI related configuration done by QPIC hardware and its not exposed as SPI bus to the external world. Only based on
> 
>     QPIC_SPI_CFG = 1, serial functionality will get selected. So that no need to implement as SPI-mem controller driver, since its not a SPI controller.
> 
>    Please check the below diagram for top view of QPIC controller.

One of the hard things in the Linux kernel is to make devices fit
frameworks. This feature does not fit the raw NAND framework. It does
not follow any of the conventions taken there. It is not gonna be
accepted there. You need to expose spi-mem functionalities, even if the
spi-proper features are not available. I believe your situation still
fits the spi-mem abstraction.

Thanks,
Miquèl

Re: [PATCH 2/5] mtd: rawnand: qcom: Add initial support for qspi nand

[Md Sadre Alam]

On 3/6/2023 8:08 PM, Miquel Raynal wrote:
> Hello,
> 
> quic_mdalam@quicinc.com wrote on Mon, 6 Mar 2023 19:45:58 +0530:
> 
>> On 10/29/2020 2:37 PM, Boris Brezillon wrote:
>>> Hello,
>>>
>>> On Sat, 10 Oct 2020 11:01:39 +0530
>>> Md Sadre Alam <mdalam@codeaurora.org> wrote:
>>>   
>>>> This change will add initial support for qspi (serial nand).
>>>>
>>>> QPIC Version v.2.0 onwards supports serial nand as well so this
>>>> change will initialize all required register to enable qspi (serial
>>>> nand).
>>>>
>>>> This change is supporting very basic functionality of qspi nand flash.
>>>>
>>>> 1. Reset device (Reset QSPI NAND device).
>>>>
>>>> 2. Device detection (Read id QSPI NAND device).
>>> Unfortunately, that's not going to work in the long term. You're
>>> basically hacking the raw NAND framework to make SPI NANDs fit. I do
>>> understand the rationale behind this decision (re-using the code for
>>> ECC and probably other things), but that's not going to work. So I'd
>>> recommend doing the following instead:
>>>
>>> 1/ implement a SPI-mem controller driver
>>> 2/ implement an ECC engine driver so the ECC logic can be shared
>>>      between the SPI controller and raw NAND controller drivers
>>> 3/ convert the raw NAND driver to the exec_op() interface (none of
>>>      this hack would have been possible if the driver was using the new
>>>      API)
>>>
>>> Regards,
>>>
>>> Boris
>>>   
>>      Sorry for late reply, again started working on this feature support.  The QPIC v2 on wards there is serial nand support got added , its not a standard SPI controller
>>
>>      its QPIC controller having support for serial nand. All SPI related configuration done by QPIC hardware and its not exposed as SPI bus to the external world. Only based on
>>
>>      QPIC_SPI_CFG = 1, serial functionality will get selected. So that no need to implement as SPI-mem controller driver, since its not a SPI controller.
>>
>>     Please check the below diagram for top view of QPIC controller.
> 
> One of the hard things in the Linux kernel is to make devices fit
> frameworks. This feature does not fit the raw NAND framework. It does
> not follow any of the conventions taken there. It is not gonna be
> accepted there. You need to expose spi-mem functionalities, even if the
> spi-proper features are not available. I believe your situation still
> fits the spi-mem abstraction.
> 
> Thanks,
> Miquèl


I have started writing the driver code for SPI NAND. Please check the below design,
is this fine as per Boris suggestion.


           |------------------------|                      |------------------------------|                        |---------------------------------|
           |qcom spi nand driver    |--------------------->|common ECC engine driver      |<-----------------------|qcom raw nand driver             |
           |                        |                      |                              |                        |                                 |
           |                        |                      |drivers/mtd/nand/ecc-qcom.c   |                        |drivers/mtd/nand/raw/qcom_nand.c |
           |                        |                      |                              |                        |                                 |
           |drivers/spi/spi-qpic.c  |                      |------------------------------|                        |                                 |
           |                        |                      |common API file:              |                        |                                 |
           |                        |                      |common API: reset, read id,   |                        |                                 |
           |                        |--------------------->|erase, read page, write page, |<-----------------------|                                 |
           |------------------------|                      |bad block check etc.          |                        |                                 |
                                                           |                              |                        |---------------------------------|
                                                           |drivers/mtd/nand/raw/qpic_comm|
                                                           |    on.c                      |
                                                           |------------------------------|


Here ECC engine driver as separate file under (drivers/mtd/nand/ecc-qcom.c) and all
common APIs like reset, read id, erase, write page, read page, check block bad etc.
as separate file under drivers/mtd/nand/raw/qpic_common.c.APIs under ECC engine drivers
and qpic_common.c will be exported and used by spi-qpic.c driver (Serial NAND) and qcom_nand.c
(raw nand driver).

Thanks,
Alam.

Re: [PATCH 2/5] mtd: rawnand: qcom: Add initial support for qspi nand

[Miquel Raynal]

Hello,

+ Mark

quic_mdalam@quicinc.com wrote on Mon, 27 Mar 2023 19:24:02 +0530:

> On 3/6/2023 8:08 PM, Miquel Raynal wrote:
> > Hello,
> > 
> > quic_mdalam@quicinc.com wrote on Mon, 6 Mar 2023 19:45:58 +0530:
> >   
> >> On 10/29/2020 2:37 PM, Boris Brezillon wrote:  
> >>> Hello,
> >>>
> >>> On Sat, 10 Oct 2020 11:01:39 +0530
> >>> Md Sadre Alam <mdalam@codeaurora.org> wrote:  
> >>>   >>>> This change will add initial support for qspi (serial nand).  
> >>>>
> >>>> QPIC Version v.2.0 onwards supports serial nand as well so this
> >>>> change will initialize all required register to enable qspi (serial
> >>>> nand).
> >>>>
> >>>> This change is supporting very basic functionality of qspi nand flash.
> >>>>
> >>>> 1. Reset device (Reset QSPI NAND device).
> >>>>
> >>>> 2. Device detection (Read id QSPI NAND device).  
> >>> Unfortunately, that's not going to work in the long term. You're
> >>> basically hacking the raw NAND framework to make SPI NANDs fit. I do
> >>> understand the rationale behind this decision (re-using the code for
> >>> ECC and probably other things), but that's not going to work. So I'd
> >>> recommend doing the following instead:
> >>>
> >>> 1/ implement a SPI-mem controller driver
> >>> 2/ implement an ECC engine driver so the ECC logic can be shared
> >>>      between the SPI controller and raw NAND controller drivers
> >>> 3/ convert the raw NAND driver to the exec_op() interface (none of
> >>>      this hack would have been possible if the driver was using the new
> >>>      API)
> >>>
> >>> Regards,
> >>>
> >>> Boris  
> >>>   >>      Sorry for late reply, again started working on this feature support.  The QPIC v2 on wards there is serial nand support got added , its not a standard SPI controller  
> >>
> >>      its QPIC controller having support for serial nand. All SPI related configuration done by QPIC hardware and its not exposed as SPI bus to the external world. Only based on
> >>
> >>      QPIC_SPI_CFG = 1, serial functionality will get selected. So that no need to implement as SPI-mem controller driver, since its not a SPI controller.
> >>
> >>     Please check the below diagram for top view of QPIC controller.  
> > 
> > One of the hard things in the Linux kernel is to make devices fit
> > frameworks. This feature does not fit the raw NAND framework. It does
> > not follow any of the conventions taken there. It is not gonna be
> > accepted there. You need to expose spi-mem functionalities, even if the
> > spi-proper features are not available. I believe your situation still
> > fits the spi-mem abstraction.
> > 
> > Thanks,
> > Miquèl  
> 
> 
> I have started writing the driver code for SPI NAND. Please check the below design,
> is this fine as per Boris suggestion.

Thanks.


>            |------------------------|                      |------------------------------|                        |---------------------------------|
>            |qcom spi nand driver    |--------------------->|common ECC engine driver      |<-----------------------|qcom raw nand driver             |
>            |                        |                      |                              |                        |                                 |
>            |                        |                      |drivers/mtd/nand/ecc-qcom.c   |                        |drivers/mtd/nand/raw/qcom_nand.c |
>            |                        |                      |                              |                        |                                 |
>            |drivers/spi/spi-qpic.c  |                      |------------------------------|                        |                                 |
>            |                        |                      |common API file:              |                        |                                 |
>            |                        |                      |common API: reset, read id,   |                        |                                 |
>            |                        |--------------------->|erase, read page, write page, |<-----------------------|                                 |
>            |------------------------|                      |bad block check etc.          |                        |                                 |
>                                                            |                              |                        |---------------------------------|
>                                                            |drivers/mtd/nand/raw/qpic_comm|
>                                                            |    on.c                      |
>                                                            |------------------------------|
> 
> 
> Here ECC engine driver as separate file under (drivers/mtd/nand/ecc-qcom.c) and all
> common APIs like reset, read id, erase, write page, read page, check block bad etc.

I'm not sure how generic these APIs are, please try to not put anything
raw NAND or SPI-NAND specific in the common file. I don't want to see
"if (raw)" or "if (spi)" conditions there.

> as separate file under drivers/mtd/nand/raw/qpic_common.c.APIs under ECC engine drivers
> and qpic_common.c will be exported and used by spi-qpic.c driver (Serial NAND) and qcom_nand.c
> (raw nand driver).

On my side, I don't have a strong opinion yet but it looks good to me.

Can you confirm you are considering switching to the ->exec_op() API in
the raw NAND driver?

Thanks,
Miquèl

Re: [PATCH 2/5] mtd: rawnand: qcom: Add initial support for qspi nand

[Md Sadre Alam]

On 3/27/2023 8:19 PM, Miquel Raynal wrote:
> Hello,
> 
> + Mark
> 
> quic_mdalam@quicinc.com wrote on Mon, 27 Mar 2023 19:24:02 +0530:
> 
>> On 3/6/2023 8:08 PM, Miquel Raynal wrote:
>>> Hello,
>>>
>>> quic_mdalam@quicinc.com wrote on Mon, 6 Mar 2023 19:45:58 +0530:
>>>    
>>>> On 10/29/2020 2:37 PM, Boris Brezillon wrote:
>>>>> Hello,
>>>>>
>>>>> On Sat, 10 Oct 2020 11:01:39 +0530
>>>>> Md Sadre Alam <mdalam@codeaurora.org> wrote:
>>>>>    >>>> This change will add initial support for qspi (serial nand).
>>>>>>
>>>>>> QPIC Version v.2.0 onwards supports serial nand as well so this
>>>>>> change will initialize all required register to enable qspi (serial
>>>>>> nand).
>>>>>>
>>>>>> This change is supporting very basic functionality of qspi nand flash.
>>>>>>
>>>>>> 1. Reset device (Reset QSPI NAND device).
>>>>>>
>>>>>> 2. Device detection (Read id QSPI NAND device).
>>>>> Unfortunately, that's not going to work in the long term. You're
>>>>> basically hacking the raw NAND framework to make SPI NANDs fit. I do
>>>>> understand the rationale behind this decision (re-using the code for
>>>>> ECC and probably other things), but that's not going to work. So I'd
>>>>> recommend doing the following instead:
>>>>>
>>>>> 1/ implement a SPI-mem controller driver
>>>>> 2/ implement an ECC engine driver so the ECC logic can be shared
>>>>>       between the SPI controller and raw NAND controller drivers
>>>>> 3/ convert the raw NAND driver to the exec_op() interface (none of
>>>>>       this hack would have been possible if the driver was using the new
>>>>>       API)
>>>>>
>>>>> Regards,
>>>>>
>>>>> Boris
>>>>>    >>      Sorry for late reply, again started working on this feature support.  The QPIC v2 on wards there is serial nand support got added , its not a standard SPI controller
>>>>
>>>>       its QPIC controller having support for serial nand. All SPI related configuration done by QPIC hardware and its not exposed as SPI bus to the external world. Only based on
>>>>
>>>>       QPIC_SPI_CFG = 1, serial functionality will get selected. So that no need to implement as SPI-mem controller driver, since its not a SPI controller.
>>>>
>>>>      Please check the below diagram for top view of QPIC controller.
>>>
>>> One of the hard things in the Linux kernel is to make devices fit
>>> frameworks. This feature does not fit the raw NAND framework. It does
>>> not follow any of the conventions taken there. It is not gonna be
>>> accepted there. You need to expose spi-mem functionalities, even if the
>>> spi-proper features are not available. I believe your situation still
>>> fits the spi-mem abstraction.
>>>
>>> Thanks,
>>> Miquèl
>>
>>
>> I have started writing the driver code for SPI NAND. Please check the below design,
>> is this fine as per Boris suggestion.
> 
> Thanks.
> 
> 
>>             |------------------------|                      |------------------------------|                        |---------------------------------|
>>             |qcom spi nand driver    |--------------------->|common ECC engine driver      |<-----------------------|qcom raw nand driver             |
>>             |                        |                      |                              |                        |                                 |
>>             |                        |                      |drivers/mtd/nand/ecc-qcom.c   |                        |drivers/mtd/nand/raw/qcom_nand.c |
>>             |                        |                      |                              |                        |                                 |
>>             |drivers/spi/spi-qpic.c  |                      |------------------------------|                        |                                 |
>>             |                        |                      |common API file:              |                        |                                 |
>>             |                        |                      |common API: reset, read id,   |                        |                                 |
>>             |                        |--------------------->|erase, read page, write page, |<-----------------------|                                 |
>>             |------------------------|                      |bad block check etc.          |                        |                                 |
>>                                                             |                              |                        |---------------------------------|
>>                                                             |drivers/mtd/nand/raw/qpic_comm|
>>                                                             |    on.c                      |
>>                                                             |------------------------------|
>>
>>
>> Here ECC engine driver as separate file under (drivers/mtd/nand/ecc-qcom.c) and all
>> common APIs like reset, read id, erase, write page, read page, check block bad etc.
> 
> I'm not sure how generic these APIs are, please try to not put anything
> raw NAND or SPI-NAND specific in the common file. I don't want to see
> "if (raw)" or "if (spi)" conditions there.

  I agree , I will not try to put any condition like if(raw) or if(spi).

  I am doing in the below sequence.

  1) exec->op() in raw nand driver

  2) Move ecc related code to new ecc engine driver under ./drivers/mtd/nand/ecc-qcom.c
     and make raw nand driver to work with this ecc engine driver.

  3) Write separate SPI-mem driver under ./drivers/spi/spi-qpic.c

  4) Move common code under qpic_common.c under ./driver/mtd/nand/raw/qpic_common.c

> 
>> as separate file under drivers/mtd/nand/raw/qpic_common.c.APIs under ECC engine drivers
>> and qpic_common.c will be exported and used by spi-qpic.c driver (Serial NAND) and qcom_nand.c
>> (raw nand driver).
> 
> On my side, I don't have a strong opinion yet but it looks good to me.
> 
> Can you confirm you are considering switching to the ->exec_op() API in
> the raw NAND driver?

   Yes, first priority is switching to ->exec_op() API in raw nand driver.
   I have already started and sent couple of patches, will address latest comment
   send again.

> 
> Thanks,
> Miquèl

[PATCH 3/5] mtd: rawnand: qcom: Read QPIC version

[Md Sadre Alam]

This change will add support to read QPIC version.
QPIC version V2.0 onwards some new register introduced
in QPIC. So based on hw_version we will update those
register.

Signed-off-by: Md Sadre Alam <mdalam@codeaurora.org>
---
 drivers/mtd/nand/raw/qcom_nandc.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
index f5064ab..eabb803 100644
--- a/drivers/mtd/nand/raw/qcom_nandc.c
+++ b/drivers/mtd/nand/raw/qcom_nandc.c
@@ -443,6 +443,7 @@ struct qcom_nand_controller {
 
 	u32 cmd1, vld;
 	const struct qcom_nandc_props *props;
+	u32 hw_version;
 };
 
 /*
@@ -2538,6 +2539,7 @@ static int qcom_nand_attach_chip(struct nand_chip *chip)
 	int ecc_mode = 1;
 	int num_addr_cycle = 5, dsbl_sts_aftr_write = 0;
 	int wr_rd_bsy_gap = 2, recovery_cycle = 7;
+	u32 version_reg;
 
 	/* controller only supports 512 bytes data steps */
 	ecc->size = NANDC_STEP_SIZE;
@@ -2545,6 +2547,26 @@ static int qcom_nand_attach_chip(struct nand_chip *chip)
 	cwperpage = mtd->writesize / NANDC_STEP_SIZE;
 
 	/*
+	 * Read the required ecc strength from NAND device and overwrite the
+	 * device tree ecc strength
+	 */
+	if (chip->base.eccreq.strength >= 8)
+		ecc->strength = 8;
+
+	/* Read QPIC version register */
+	if (nandc->props->is_serial_nand)
+		version_reg = (NAND_VERSION + 0x4000);
+	else
+		version_reg = NAND_VERSION;
+	nandc->hw_version = nandc_read(nandc, version_reg);
+	pr_debug("QPIC controller hw version Major:%d, Minor:%d\n",
+		((nandc->hw_version & NAND_VERSION_MAJOR_MASK)
+		 >> NAND_VERSION_MAJOR_SHIFT),
+		((nandc->hw_version & NAND_VERSION_MINOR_MASK)
+		 >> NAND_VERSION_MINOR_SHIFT));
+	nandc->hw_version = ((nandc->hw_version & NAND_VERSION_MAJOR_MASK)
+			>> NAND_VERSION_MAJOR_SHIFT);
+	/*
 	 * Each CW has 4 available OOB bytes which will be protected with ECC
 	 * so remaining bytes can be used for ECC.
 	 */
-- 
2.7.4

[PATCH 4/5] mtd: rawnand: qcom: Enable support for erase,read & write for serial nand.

[Md Sadre Alam]

This change will enable support for erase , read & write support for
QSPI serial nand. In QPIC V2.0 onwards, to read last code word new
regiater is introduced. So to read for first three code word we have to
use LOCATION_n register and for last code word we ahve to use LAST_CW_n.

Signed-off-by: Md Sadre Alam <mdalam@codeaurora.org>
---
 drivers/mtd/nand/raw/qcom_nandc.c | 97 +++++++++++++++++++++++++++++++++------
 1 file changed, 83 insertions(+), 14 deletions(-)

diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
index eabb803..4e8e1dc 100644
--- a/drivers/mtd/nand/raw/qcom_nandc.c
+++ b/drivers/mtd/nand/raw/qcom_nandc.c
@@ -216,6 +216,7 @@
 #define SPI_HOLD	(1 << 27)
 #define SPI_TRANSFER_MODE_x1	(1 << 29)
 #define SPI_TRANSFER_MODE_x4	(3 << 29)
+#define QPIC_v2_0	0x2
 
 #define nandc_set_read_loc(nandc, reg, offset, size, is_last)	\
 nandc_set_reg(nandc, NAND_READ_LOCATION_##reg,			\
@@ -223,6 +224,12 @@ nandc_set_reg(nandc, NAND_READ_LOCATION_##reg,			\
 	      ((size) << READ_LOCATION_SIZE) |			\
 	      ((is_last) << READ_LOCATION_LAST))
 
+#define nandc_set_read_loc_last(nandc, reg, offset, size, is_last)	\
+nandc_set_reg(nandc, NAND_READ_LOCATION_LAST_CW_##reg,			\
+	      ((offset) << READ_LOCATION_OFFSET) |		\
+	      ((size) << READ_LOCATION_SIZE) |			\
+	      ((is_last) << READ_LOCATION_LAST))
+
 /*
  * Returns the actual register address for all NAND_DEV_ registers
  * (i.e. NAND_DEV_CMD0, NAND_DEV_CMD1, NAND_DEV_CMD2 and NAND_DEV_CMD_VLD)
@@ -352,6 +359,10 @@ struct nandc_regs {
 	__le32 read_location1;
 	__le32 read_location2;
 	__le32 read_location3;
+	__le32 read_location_last0;
+	__le32 read_location_last1;
+	__le32 read_location_last2;
+	__le32 read_location_last3;
 	__le32 spi_cfg;
 	__le32 num_addr_cycle;
 	__le32 busy_wait_cnt;
@@ -685,6 +696,14 @@ static __le32 *offset_to_nandc_reg(struct nandc_regs *regs, int offset)
 		return &regs->read_location2;
 	case NAND_READ_LOCATION_3:
 		return &regs->read_location3;
+	case NAND_READ_LOCATION_LAST_CW_0:
+		return &regs->read_location_last0;
+	case NAND_READ_LOCATION_LAST_CW_1:
+		return &regs->read_location_last1;
+	case NAND_READ_LOCATION_LAST_CW_2:
+		return &regs->read_location_last2;
+	case NAND_READ_LOCATION_LAST_CW_3:
+		return &regs->read_location_last3;
 	case NAND_FLASH_SPI_CFG:
 		return &regs->spi_cfg;
 	case NAND_SPI_NUM_ADDR_CYCLES:
@@ -734,13 +753,18 @@ static void update_rw_regs(struct qcom_nand_host *host, int num_cw, bool read)
 	struct qcom_nand_controller *nandc = get_qcom_nand_controller(chip);
 	u32 cmd, cfg0, cfg1, ecc_bch_cfg;
 
+	cmd = (PAGE_ACC | LAST_PAGE);
+
+	if (nandc->props->is_serial_nand)
+		cmd |= (SPI_TRANSFER_MODE_x1 | SPI_WP | SPI_HOLD);
+
 	if (read) {
 		if (host->use_ecc)
-			cmd = OP_PAGE_READ_WITH_ECC | PAGE_ACC | LAST_PAGE;
+			cmd |= OP_PAGE_READ_WITH_ECC;
 		else
-			cmd = OP_PAGE_READ | PAGE_ACC | LAST_PAGE;
+			cmd |= OP_PAGE_READ;
 	} else {
-		cmd = OP_PROGRAM_PAGE | PAGE_ACC | LAST_PAGE;
+		cmd |= OP_PROGRAM_PAGE;
 	}
 
 	if (host->use_ecc) {
@@ -766,9 +790,14 @@ static void update_rw_regs(struct qcom_nand_host *host, int num_cw, bool read)
 	nandc_set_reg(nandc, NAND_READ_STATUS, host->clrreadstatus);
 	nandc_set_reg(nandc, NAND_EXEC_CMD, 1);
 
-	if (read)
+	if (read) {
+		if (nandc->hw_version >= QPIC_v2_0)
+			nandc_set_read_loc_last(nandc, 0, 0, host->use_ecc ?
+					host->cw_data : host->cw_size, 1);
+
 		nandc_set_read_loc(nandc, 0, 0, host->use_ecc ?
 				   host->cw_data : host->cw_size, 1);
+	}
 }
 
 /*
@@ -1143,9 +1172,13 @@ static void config_nand_page_read(struct qcom_nand_controller *nandc)
 static void
 config_nand_cw_read(struct qcom_nand_controller *nandc, bool use_ecc)
 {
-	if (nandc->props->is_bam)
+	if (nandc->props->is_bam) {
+		if (nandc->hw_version >= QPIC_v2_0)
+			write_reg_dma(nandc, NAND_READ_LOCATION_LAST_CW_0,
+					4, NAND_BAM_NEXT_SGL);
 		write_reg_dma(nandc, NAND_READ_LOCATION_0, 4,
 			      NAND_BAM_NEXT_SGL);
+	}
 
 	write_reg_dma(nandc, NAND_FLASH_CMD, 1, NAND_BAM_NEXT_SGL);
 	write_reg_dma(nandc, NAND_EXEC_CMD, 1, NAND_BAM_NEXT_SGL);
@@ -1266,9 +1299,13 @@ static int erase_block(struct qcom_nand_host *host, int page_addr)
 {
 	struct nand_chip *chip = &host->chip;
 	struct qcom_nand_controller *nandc = get_qcom_nand_controller(chip);
+	u32 ers_cmd = OP_BLOCK_ERASE | PAGE_ACC | LAST_PAGE;
 
-	nandc_set_reg(nandc, NAND_FLASH_CMD,
-		      OP_BLOCK_ERASE | PAGE_ACC | LAST_PAGE);
+	if (nandc->props->is_serial_nand) {
+		ers_cmd |= (SPI_WP | SPI_HOLD | SPI_TRANSFER_MODE_x1);
+		page_addr <<= 16;
+	}
+	nandc_set_reg(nandc, NAND_FLASH_CMD, ers_cmd);
 	nandc_set_reg(nandc, NAND_ADDR0, page_addr);
 	nandc_set_reg(nandc, NAND_ADDR1, 0);
 	nandc_set_reg(nandc, NAND_DEV0_CFG0,
@@ -1680,16 +1717,32 @@ qcom_nandc_read_cw_raw(struct mtd_info *mtd, struct nand_chip *chip,
 	}
 
 	if (nandc->props->is_bam) {
-		nandc_set_read_loc(nandc, 0, read_loc, data_size1, 0);
+		if ((nandc->hw_version >= QPIC_v2_0) &&
+			(cw == (ecc->steps - 1)))
+			nandc_set_read_loc_last(nandc, 0, read_loc, data_size1, 0);
+		else
+			nandc_set_read_loc(nandc, 0, read_loc, data_size1, 0);
 		read_loc += data_size1;
 
-		nandc_set_read_loc(nandc, 1, read_loc, oob_size1, 0);
+		if ((nandc->hw_version >= QPIC_v2_0) &&
+			(cw == (ecc->steps - 1)))
+			nandc_set_read_loc_last(nandc, 1, read_loc, oob_size1, 0);
+		else
+			nandc_set_read_loc(nandc, 1, read_loc, oob_size1, 0);
 		read_loc += oob_size1;
 
-		nandc_set_read_loc(nandc, 2, read_loc, data_size2, 0);
+		if ((nandc->hw_version >= QPIC_v2_0) &&
+			(cw == (ecc->steps - 1)))
+			nandc_set_read_loc_last(nandc, 2, read_loc, data_size2, 0);
+		else
+			nandc_set_read_loc(nandc, 2, read_loc, data_size2, 0);
 		read_loc += data_size2;
 
-		nandc_set_read_loc(nandc, 3, read_loc, oob_size2, 1);
+		if ((nandc->hw_version >= QPIC_v2_0) &&
+			(cw == (ecc->steps - 1)))
+			nandc_set_read_loc_last(nandc, 3, read_loc, oob_size2, 0);
+		else
+			nandc_set_read_loc(nandc, 3, read_loc, oob_size2, 1);
 	}
 
 	config_nand_cw_read(nandc, false);
@@ -1924,10 +1977,26 @@ static int read_page_ecc(struct qcom_nand_host *host, u8 *data_buf,
 				nandc_set_read_loc(nandc, 1, data_size,
 						   oob_size, 1);
 			} else if (data_buf) {
-				nandc_set_read_loc(nandc, 0, 0, data_size, 1);
+				if (nandc->hw_version >= QPIC_v2_0) {
+					if (i == (ecc->steps - 1))
+						nandc_set_read_loc_last(nandc, 0, 0,
+							data_size, 1);
+					else
+						nandc_set_read_loc(nandc, 0, 0,
+							data_size, 1);
+				} else
+					nandc_set_read_loc(nandc, 0, 0, data_size, 1);
 			} else {
-				nandc_set_read_loc(nandc, 0, data_size,
-						   oob_size, 1);
+				if (nandc->hw_version >= QPIC_v2_0) {
+					if (i == (ecc->steps - 1))
+						nandc_set_read_loc_last(nandc, 0, data_size,
+							oob_size, 1);
+					else
+						nandc_set_read_loc(nandc, 0, data_size,
+							oob_size, 1);
+				} else
+					nandc_set_read_loc(nandc, 0, data_size,
+							oob_size, 1);
 			}
 		}
 
-- 
2.7.4

[PATCH 5/5] mtd: rawnand: qcom: Add support for serial training.

[Md Sadre Alam]

This change will add support for serial training for
QSPI nand in QPIC.

Due to different PNR and PCB delays, serial read data
can come with different delays to QPIC. At high frequency
operations Rx clock should be adjusted according to delays
so that Rx Data can be captured correctly.CLK_CNTR_INIT_VAL_VEC
in NAND_FLASH_SPI_CFG register is a 12-bit vector which is divided
in 4 parts of 3 bits each representing delay of 4 serial input data
lines. Bit [2:0] corresponds to qspi_miso[0], bit [5:3] corresponds
to qspi_miso[1], bit [8:6] corresponds to qspi_miso[2] and bit [11:9]
corresponds to qspi_miso[3]. Delay of each qspi_miso line can be set
from 0 to 7.

For serial training the following rule should be followd.

1. SW should write a page with any known pattern in flash at lower
   frequency.
2. Set the CLK_CNTR_INIT_VAL_VEC for qspi_miso[0] line.
3. Read that page repetitively in high frequency mode until it
   gets data accurately.
4. Repeat above steps for other qspi_miso lines.

Signed-off-by: Md Sadre Alam <mdalam@codeaurora.org>
---
 drivers/mtd/nand/raw/qcom_nandc.c | 219 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 217 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
index 4e8e1dc..fc5e32c 100644
--- a/drivers/mtd/nand/raw/qcom_nandc.c
+++ b/drivers/mtd/nand/raw/qcom_nandc.c
@@ -217,6 +217,10 @@
 #define SPI_TRANSFER_MODE_x1	(1 << 29)
 #define SPI_TRANSFER_MODE_x4	(3 << 29)
 #define QPIC_v2_0	0x2
+#define FEEDBACK_CLK_EN	(1 << 4)
+#define MAX_TRAINING_BLK	8
+#define TRAINING_OFFSET	0x0
+#define TOTAL_NUM_PHASE	7
 
 #define nandc_set_read_loc(nandc, reg, offset, size, is_last)	\
 nandc_set_reg(nandc, NAND_READ_LOCATION_##reg,			\
@@ -267,6 +271,16 @@ nandc_set_reg(nandc, NAND_READ_LOCATION_LAST_CW_##reg,			\
 #define NAND_ERASED_CW_SET		BIT(4)
 
 /*
+ * An array holding the fixed pattern
+ */
+static const u32 qspi_training_block_64[] = {
+	0x0F0F0F0F, 0x0F0F0F0F, 0x0F0F0F0F, 0x0F0F0F0F,
+	0x0F0F0F0F, 0x0F0F0F0F, 0x0F0F0F0F, 0x0F0F0F0F,
+	0x0F0F0F0F, 0x0F0F0F0F, 0x0F0F0F0F, 0x0F0F0F0F,
+	0x0F0F0F0F, 0x0F0F0F0F, 0x0F0F0F0F, 0x0F0F0F0F,
+};
+
+/*
  * This data type corresponds to the BAM transaction which will be used for all
  * NAND transfers.
  * @bam_ce - the array of BAM command elements
@@ -366,6 +380,7 @@ struct nandc_regs {
 	__le32 spi_cfg;
 	__le32 num_addr_cycle;
 	__le32 busy_wait_cnt;
+	__le32 mstr_cfg;
 
 	__le32 erased_cw_detect_cfg_clr;
 	__le32 erased_cw_detect_cfg_set;
@@ -710,6 +725,8 @@ static __le32 *offset_to_nandc_reg(struct nandc_regs *regs, int offset)
 		return &regs->num_addr_cycle;
 	case NAND_SPI_BUSY_CHECK_WAIT_CNT:
 		return &regs->busy_wait_cnt;
+	case NAND_QSPI_MSTR_CONFIG:
+		return &regs->mstr_cfg;
 	default:
 		return NULL;
 	}
@@ -2978,14 +2995,13 @@ static void qspi_write_reg_bam(struct qcom_nand_controller *nandc,
 		unsigned int val, unsigned int reg)
 {
 	int ret;
-
 	clear_bam_transaction(nandc);
 	nandc_set_reg(nandc, reg, val);
 	write_reg_dma(nandc, reg, 1, NAND_BAM_NEXT_SGL);
 
 	ret = submit_descs(nandc);
 	if (ret)
-		dev_err(nandc->dev, "Error in submitting descriptor to write config reg\n");
+		dev_err(nandc->dev, "Error in submitting descriptor to write reg %x\n", reg);
 	free_descs(nandc);
 }
 
@@ -3015,6 +3031,192 @@ static void qspi_nand_init(struct qcom_nand_controller *nandc)
 	qspi_write_reg_bam(nandc, WAIT_CNT, NAND_SPI_BUSY_CHECK_WAIT_CNT);
 }
 
+static void qspi_set_phase(struct qcom_nand_controller *nandc, int phase)
+{
+	u32 qspi_cfg_val = 0x0;
+	int reg = dev_cmd_reg_addr(nandc, NAND_FLASH_SPI_CFG);
+
+	qspi_cfg_val = nandc_read(nandc, reg);
+	qspi_cfg_val |= LOAD_CLK_CNTR_INIT_EN;
+
+	qspi_write_reg_bam(nandc, qspi_cfg_val, NAND_FLASH_SPI_CFG);
+	qspi_cfg_val &= 0xf000ffff;
+
+	/* Write phase value for all the lines */
+	qspi_cfg_val |= ((phase << 16) | (phase << 19) | (phase << 22)
+			| (phase << 25));
+	qspi_write_reg_bam(nandc, qspi_cfg_val, NAND_FLASH_SPI_CFG);
+
+	/* Clear LOAD_CLK_CNTR_INIT_EN bit to load phase value */
+	qspi_cfg_val &= ~LOAD_CLK_CNTR_INIT_EN;
+	qspi_write_reg_bam(nandc, qspi_cfg_val, NAND_FLASH_SPI_CFG);
+}
+
+static int qspi_get_appropriate_phase(struct qcom_nand_controller *nandc, u8 *phase_table,
+		int phase_count)
+{
+	int i, cnt = 0, phase = 0x0;
+	u8 phase_ranges[TOTAL_NUM_PHASE] = {'\0'};
+
+	for (i = 0; i < phase_count; i++) {
+		if ((phase_table[i] + 1 == phase_table[i + 1]) &&
+		(phase_table[i + 1] + 1 == phase_table[i + 2]))
+			phase_ranges[cnt++] = phase_table[i + 1];
+	}
+
+	/* Filter out middle phase */
+	if (!(cnt & 1))
+		phase = phase_ranges[cnt/2 - 1];
+	else
+		phase = phase_ranges[cnt/2];
+
+	return phase;
+}
+
+static int qspi_execute_training(struct qcom_nand_controller *nandc,
+		struct qcom_nand_host *host, struct mtd_info *mtd)
+{
+	u32 pages_per_block = 0, page = 0;
+	int ret = 0, bb_cnt = 0, i, phase_failed = 0;
+	int phase_cnt, phase;
+	u32 training_offset = TRAINING_OFFSET;
+	u8 *training_data = NULL, trained_phase[TOTAL_NUM_PHASE] = {'\0'};
+	struct nand_chip *chip = &host->chip;
+
+	pages_per_block = 1 << (chip->phys_erase_shift - chip->page_shift);
+	page = (training_offset >> chip->page_shift) & chip->pagemask;
+
+	/* Set feedback clk enable bit to do auto adjustment of phase
+	 * at lower frequency
+	 */
+	qspi_write_reg_bam(nandc, (nandc_read(nandc,
+			NAND_QSPI_MSTR_CONFIG) | FEEDBACK_CLK_EN),
+			NAND_QSPI_MSTR_CONFIG);
+
+	/* check for bad block in allocated training blocks
+	 * The training blocks should be continuous good block or
+	 * continuous bad block, it should be not like good,bad,good etc.
+	 * avoid to use this type of block for serial training
+	 */
+	while (qcom_nandc_block_bad(chip, training_offset)) {
+		training_offset += mtd->erasesize;
+		page += pages_per_block;
+		bb_cnt++;
+	}
+
+	if (bb_cnt == MAX_TRAINING_BLK) {
+		dev_dbg(nandc->dev, "All training blocks are bad, skipping serial training");
+		dev_dbg(nandc->dev, "Operatig at lower frequency");
+		ret = -EINVAL;
+		goto trng_err;
+	}
+
+	qcom_nandc_command(chip, NAND_CMD_ERASE1, 0, page);
+
+	/* Allocate memory to hold one NAND page */
+	training_data = kzalloc(mtd->writesize, GFP_KERNEL);
+	if (!training_data) {
+		ret = -ENOMEM;
+		goto trng_err;
+	}
+	memset(training_data, '\0', mtd->writesize);
+
+	for (i = 0; i < mtd->writesize; i += sizeof(qspi_training_block_64))
+		memcpy(training_data + i, qspi_training_block_64,
+			sizeof(qspi_training_block_64));
+
+	/* Write qspi training data to flash */
+	ret = qcom_nandc_write_page(chip, training_data, 0, page);
+	if (ret) {
+		dev_err(nandc->dev, "Error in writing training data");
+		ret = -EINVAL;
+		goto mem_err;
+	}
+
+	/* Read qspi training data @ low freq */
+	memset(training_data, 0xff, mtd->writesize);
+	ret = qcom_nandc_read_page(chip, training_data, 0, page);
+	if (ret) {
+		dev_err(nandc->dev, "Error in reading training data @ low freq");
+		ret = -EINVAL;
+		goto mem_err;
+	}
+
+	/* compare read training data with known pattern */
+	for (i = 0; i <  mtd->writesize; i += sizeof(qspi_training_block_64)) {
+		if (memcmp(training_data + i, qspi_training_block_64,
+				sizeof(qspi_training_block_64))) {
+			dev_err(nandc->dev, "Training data mismatch @ low freq");
+			ret = -EINVAL;
+			goto mem_err;
+		}
+	}
+
+	/* clear feedback clock bit and start training here */
+	qspi_write_reg_bam(nandc, (nandc_read(nandc,
+			NAND_QSPI_MSTR_CONFIG) & ~FEEDBACK_CLK_EN),
+			NAND_QSPI_MSTR_CONFIG);
+	phase = 1;
+	phase_cnt = 0;
+
+	/* set higest clock frequecy for io_macro i.e 320MHz so
+	 * on bus it will be 320/4 = 80MHz.
+	 */
+
+	ret =  clk_set_rate(nandc->iomacro_clk, 320000000);
+	if (ret) {
+		dev_err(nandc->dev, "Setting clk rate to 320000000 MHz failed");
+		goto mem_err;
+	}
+
+	do {
+		qspi_set_phase(nandc, phase);
+
+		/* Prepare clean buffer to read */
+		memset(training_data, 0xff, mtd->writesize);
+		ret = qcom_nandc_read_page(chip, training_data, 0, page);
+		if (ret) {
+			dev_err(nandc->dev, "Error in reading training data @ high freq");
+			ret = -EINVAL;
+			goto mem_err;
+		}
+		/* compare read training data with known pattern */
+		for (i = 0; i <  mtd->writesize; i += sizeof(qspi_training_block_64)) {
+			if (memcmp(training_data + i, qspi_training_block_64,
+					sizeof(qspi_training_block_64))) {
+				phase_failed++;
+				break;
+			}
+		}
+
+		if (i == mtd->writesize)
+			trained_phase[phase_cnt++] = phase;
+
+	} while (phase++ < TOTAL_NUM_PHASE);
+
+	if (phase_cnt) {
+		phase = qspi_get_appropriate_phase(nandc, trained_phase, phase_cnt);
+		qspi_set_phase(nandc, phase);
+	} else {
+		dev_err(nandc->dev, "Serial training failed");
+		dev_err(nandc->dev, "Running @ low freq 50MHz");
+		/* Run @ lower frequency 50Mhz with feedback clk bit enabled  */
+		qspi_write_reg_bam(nandc, (nandc_read(nandc,
+			NAND_QSPI_MSTR_CONFIG) | FEEDBACK_CLK_EN),
+			NAND_QSPI_MSTR_CONFIG);
+		ret =  clk_set_rate(nandc->iomacro_clk, 200000000);
+		if (ret) {
+			dev_err(nandc->dev, "Setting clk rate to 50000000 MHz failed");
+			goto mem_err;
+		}
+	}
+
+mem_err:
+	kfree(training_data);
+trng_err:
+	return ret;
+}
+
 static int qcom_nand_host_init_and_register(struct qcom_nand_controller *nandc,
 					    struct qcom_nand_host *host,
 					    struct device_node *dn)
@@ -3081,6 +3283,15 @@ static int qcom_nand_host_init_and_register(struct qcom_nand_controller *nandc,
 		}
 	}
 
+	/* QSPI serial training is required if io_macro clk frequency
+	 * is more than 50MHz. This is due to different PNR and PCB delays,
+	 * serial read data can come with different delays to QPIC. So
+	 * Rx clock should be adjusted according to delays so that Rx Data
+	 * can be captured correctly.
+	 */
+	if (nandc->props->is_serial_nand)
+		qspi_execute_training(nandc, host, mtd);
+
 	ret = mtd_device_register(mtd, NULL, 0);
 	if (ret)
 		nand_cleanup(chip);
@@ -3178,6 +3389,10 @@ static int qcom_nandc_probe(struct platform_device *pdev)
 		nandc->iomacro_clk = devm_clk_get(dev, "io_macro");
 		if (IS_ERR(nandc->iomacro_clk))
 			return PTR_ERR(nandc->iomacro_clk);
+
+		ret =  clk_set_rate(nandc->iomacro_clk, 200000000);
+		if (ret)
+			return ret;
 	}
 
 	ret = qcom_nandc_parse_dt(pdev);
-- 
2.7.4

Re: [PATCH 0/5] mtd: rawnand: qcom: Add support for QSPI nand

[Miquel Raynal]

Hello,

Md Sadre Alam <mdalam@codeaurora.org> wrote on Sat, 10 Oct 2020
11:01:37 +0530:

> QPIC 2.0 supports Serial NAND support in addition to all features and
> commands in QPIC 1.0 for parallel NAND. Parallel and Serial NAND cannot
> operate simultaneously. QSPI nand devices will connect to QPIC IO_MACRO
> block of QPIC controller. There is a separate IO_MACRO clock for IO_MACRO
> block. Default IO_MACRO block divide the input clock by 4. so if IO_MACRO
> input clock is 320MHz then on bus it will be 80MHz, so QSPI nand device
> should also support this frequency.
> 
> QPIC provides 4 data pins to QSPI nand. In standard SPI mode (x1 mode) data
> transfer will occur on only 2 pins one pin for Serial data in and one for
> serial data out. In QUAD SPI mode (x4 mode) data transfer will occur at all
> the four data lines. QPIC controller supports command for x1 mode and x4 mode.
> 
> Md Sadre Alam (5):
>   dt-bindings: qcom_nandc: IPQ5018 QPIC NAND documentation
>   mtd: rawnand: qcom: Add initial support for qspi nand
>   mtd: rawnand: qcom: Read QPIC version
>   mtd: rawnand: qcom: Enable support for erase,read & write for serial
>     nand.
>   mtd: rawnand: qcom: Add support for serial training.
> 
>  .../devicetree/bindings/mtd/qcom_nandc.txt         |   3 +
>  drivers/mtd/nand/raw/nand_ids.c                    |  13 +
>  drivers/mtd/nand/raw/qcom_nandc.c                  | 502 ++++++++++++++++++++-
>  3 files changed, 494 insertions(+), 24 deletions(-)
> 

I'm sorry but this series clearly breaks the current layering. I cannot
authorize SPI-NAND code to fall into the raw NAND subsystem.

As both typologies cannot be used at the same time, I guess you should
have another driver handling this feature under the spi/ subsystem +
a few declarations in the SPI-NAND devices list.

Thanks,
Miquèl

Re: [PATCH 0/5] mtd: rawnand: qcom: Add support for QSPI nand

[mdalam]

On 2020-10-28 15:18, Miquel Raynal wrote:
> Hello,
> 
> Md Sadre Alam <mdalam@codeaurora.org> wrote on Sat, 10 Oct 2020
> 11:01:37 +0530:
> 
>> QPIC 2.0 supports Serial NAND support in addition to all features and
>> commands in QPIC 1.0 for parallel NAND. Parallel and Serial NAND 
>> cannot
>> operate simultaneously. QSPI nand devices will connect to QPIC 
>> IO_MACRO
>> block of QPIC controller. There is a separate IO_MACRO clock for 
>> IO_MACRO
>> block. Default IO_MACRO block divide the input clock by 4. so if 
>> IO_MACRO
>> input clock is 320MHz then on bus it will be 80MHz, so QSPI nand 
>> device
>> should also support this frequency.
>> 
>> QPIC provides 4 data pins to QSPI nand. In standard SPI mode (x1 mode) 
>> data
>> transfer will occur on only 2 pins one pin for Serial data in and one 
>> for
>> serial data out. In QUAD SPI mode (x4 mode) data transfer will occur 
>> at all
>> the four data lines. QPIC controller supports command for x1 mode and 
>> x4 mode.
>> 
>> Md Sadre Alam (5):
>>   dt-bindings: qcom_nandc: IPQ5018 QPIC NAND documentation
>>   mtd: rawnand: qcom: Add initial support for qspi nand
>>   mtd: rawnand: qcom: Read QPIC version
>>   mtd: rawnand: qcom: Enable support for erase,read & write for serial
>>     nand.
>>   mtd: rawnand: qcom: Add support for serial training.
>> 
>>  .../devicetree/bindings/mtd/qcom_nandc.txt         |   3 +
>>  drivers/mtd/nand/raw/nand_ids.c                    |  13 +
>>  drivers/mtd/nand/raw/qcom_nandc.c                  | 502 
>> ++++++++++++++++++++-
>>  3 files changed, 494 insertions(+), 24 deletions(-)
>> 
> 
> I'm sorry but this series clearly breaks the current layering. I cannot
> authorize SPI-NAND code to fall into the raw NAND subsystem.
> 

I am agree with you, we should not add SPI-NAND changes inside
raw NAND subsystem.

> As both typologies cannot be used at the same time, I guess you should
> have another driver handling this feature under the spi/ subsystem +
> a few declarations in the SPI-NAND devices list.
> 

Initially I was started writing separate driver under SPI-NAND subsystem 
then I
realized that more than 85% of raw/qcom_nand.c code getting duplicated.

That's why I have added this SPI-NAND change in raw/qcom_nand.c since
more than 85% of code will be reused.

If I will add this change inside SPI-NAND subsystem then much of
raw/qcom_nand.c code will get duplicated. Would it be ok ?

> Thanks,
> Miquèl

Re: [PATCH 0/5] mtd: rawnand: qcom: Add support for QSPI nand

[Miquel Raynal]

Hello,

mdalam@codeaurora.org wrote on Wed, 28 Oct 2020 23:54:23 +0530:

> On 2020-10-28 15:18, Miquel Raynal wrote:
> > Hello,
> > 
> > Md Sadre Alam <mdalam@codeaurora.org> wrote on Sat, 10 Oct 2020
> > 11:01:37 +0530:
> >   
> >> QPIC 2.0 supports Serial NAND support in addition to all features and
> >> commands in QPIC 1.0 for parallel NAND. Parallel and Serial NAND >> cannot
> >> operate simultaneously. QSPI nand devices will connect to QPIC >> IO_MACRO
> >> block of QPIC controller. There is a separate IO_MACRO clock for >> IO_MACRO
> >> block. Default IO_MACRO block divide the input clock by 4. so if >> IO_MACRO
> >> input clock is 320MHz then on bus it will be 80MHz, so QSPI nand >> device
> >> should also support this frequency.  
> >> >> QPIC provides 4 data pins to QSPI nand. In standard SPI mode (x1 mode) >> data  
> >> transfer will occur on only 2 pins one pin for Serial data in and one >> for
> >> serial data out. In QUAD SPI mode (x4 mode) data transfer will occur >> at all
> >> the four data lines. QPIC controller supports command for x1 mode and >> x4 mode.  
> >> >> Md Sadre Alam (5):  
> >>   dt-bindings: qcom_nandc: IPQ5018 QPIC NAND documentation
> >>   mtd: rawnand: qcom: Add initial support for qspi nand
> >>   mtd: rawnand: qcom: Read QPIC version
> >>   mtd: rawnand: qcom: Enable support for erase,read & write for serial
> >>     nand.
> >>   mtd: rawnand: qcom: Add support for serial training.  
> >> >>  .../devicetree/bindings/mtd/qcom_nandc.txt         |   3 +  
> >>  drivers/mtd/nand/raw/nand_ids.c                    |  13 +
> >>  drivers/mtd/nand/raw/qcom_nandc.c                  | 502 >> ++++++++++++++++++++-
> >>  3 files changed, 494 insertions(+), 24 deletions(-)  
> >> > > I'm sorry but this series clearly breaks the current layering. I cannot  
> > authorize SPI-NAND code to fall into the raw NAND subsystem.
> >   
> 
> I am agree with you, we should not add SPI-NAND changes inside
> raw NAND subsystem.
> 
> > As both typologies cannot be used at the same time, I guess you should
> > have another driver handling this feature under the spi/ subsystem +
> > a few declarations in the SPI-NAND devices list.
> >   
> 
> Initially I was started writing separate driver under SPI-NAND subsystem then I
> realized that more than 85% of raw/qcom_nand.c code getting duplicated.
> 
> That's why I have added this SPI-NAND change in raw/qcom_nand.c since
> more than 85% of code will be reused.
> 
> If I will add this change inside SPI-NAND subsystem then much of
> raw/qcom_nand.c code will get duplicated. Would it be ok ?

What about moving the generic code to drivers/mtd/nand/common/ and
referring to it from drivers/mtd/nand/raw/qcom_nand.c and
drivers/spi/spi-qcom.c (or such)?

Thanks,
Miquèl

Re: [PATCH 0/5] mtd: rawnand: qcom: Add support for QSPI nand

[Miquel Raynal]

Miquel Raynal <miquel.raynal@bootlin.com> wrote on Thu, 29 Oct 2020
08:53:44 +0100:

> Hello,
> 
> mdalam@codeaurora.org wrote on Wed, 28 Oct 2020 23:54:23 +0530:
> 
> > On 2020-10-28 15:18, Miquel Raynal wrote:  
> > > Hello,
> > > 
> > > Md Sadre Alam <mdalam@codeaurora.org> wrote on Sat, 10 Oct 2020
> > > 11:01:37 +0530:
> > >     
> > >> QPIC 2.0 supports Serial NAND support in addition to all features and
> > >> commands in QPIC 1.0 for parallel NAND. Parallel and Serial NAND >> cannot
> > >> operate simultaneously. QSPI nand devices will connect to QPIC >> IO_MACRO
> > >> block of QPIC controller. There is a separate IO_MACRO clock for >> IO_MACRO
> > >> block. Default IO_MACRO block divide the input clock by 4. so if >> IO_MACRO
> > >> input clock is 320MHz then on bus it will be 80MHz, so QSPI nand >> device
> > >> should also support this frequency.    
> > >> >> QPIC provides 4 data pins to QSPI nand. In standard SPI mode (x1 mode) >> data    
> > >> transfer will occur on only 2 pins one pin for Serial data in and one >> for
> > >> serial data out. In QUAD SPI mode (x4 mode) data transfer will occur >> at all
> > >> the four data lines. QPIC controller supports command for x1 mode and >> x4 mode.    
> > >> >> Md Sadre Alam (5):    
> > >>   dt-bindings: qcom_nandc: IPQ5018 QPIC NAND documentation
> > >>   mtd: rawnand: qcom: Add initial support for qspi nand
> > >>   mtd: rawnand: qcom: Read QPIC version
> > >>   mtd: rawnand: qcom: Enable support for erase,read & write for serial
> > >>     nand.
> > >>   mtd: rawnand: qcom: Add support for serial training.    
> > >> >>  .../devicetree/bindings/mtd/qcom_nandc.txt         |   3 +    
> > >>  drivers/mtd/nand/raw/nand_ids.c                    |  13 +
> > >>  drivers/mtd/nand/raw/qcom_nandc.c                  | 502 >> ++++++++++++++++++++-
> > >>  3 files changed, 494 insertions(+), 24 deletions(-)    
> > >> > > I'm sorry but this series clearly breaks the current layering. I cannot    
> > > authorize SPI-NAND code to fall into the raw NAND subsystem.
> > >     
> > 
> > I am agree with you, we should not add SPI-NAND changes inside
> > raw NAND subsystem.
> >   
> > > As both typologies cannot be used at the same time, I guess you should
> > > have another driver handling this feature under the spi/ subsystem +
> > > a few declarations in the SPI-NAND devices list.
> > >     
> > 
> > Initially I was started writing separate driver under SPI-NAND subsystem then I
> > realized that more than 85% of raw/qcom_nand.c code getting duplicated.
> > 
> > That's why I have added this SPI-NAND change in raw/qcom_nand.c since
> > more than 85% of code will be reused.
> > 
> > If I will add this change inside SPI-NAND subsystem then much of
> > raw/qcom_nand.c code will get duplicated. Would it be ok ?  
> 
> What about moving the generic code to drivers/mtd/nand/common/ and
> referring to it from drivers/mtd/nand/raw/qcom_nand.c and
> drivers/spi/spi-qcom.c (or such)?

Actually, perhaps drivers/memory/ is the right location for the generic
bits.


Thanks,
Miquèl

Re: [PATCH 0/5] mtd: rawnand: qcom: Add support for QSPI nand

[Boris Brezillon]

On Thu, 29 Oct 2020 08:53:44 +0100
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> Hello,
> 
> mdalam@codeaurora.org wrote on Wed, 28 Oct 2020 23:54:23 +0530:
> 
> > On 2020-10-28 15:18, Miquel Raynal wrote:  
> > > Hello,
> > > 
> > > Md Sadre Alam <mdalam@codeaurora.org> wrote on Sat, 10 Oct 2020
> > > 11:01:37 +0530:
> > >     
> > >> QPIC 2.0 supports Serial NAND support in addition to all features and
> > >> commands in QPIC 1.0 for parallel NAND. Parallel and Serial NAND >> cannot
> > >> operate simultaneously. QSPI nand devices will connect to QPIC >> IO_MACRO
> > >> block of QPIC controller. There is a separate IO_MACRO clock for >> IO_MACRO
> > >> block. Default IO_MACRO block divide the input clock by 4. so if >> IO_MACRO
> > >> input clock is 320MHz then on bus it will be 80MHz, so QSPI nand >> device
> > >> should also support this frequency.    
> > >> >> QPIC provides 4 data pins to QSPI nand. In standard SPI mode (x1 mode) >> data    
> > >> transfer will occur on only 2 pins one pin for Serial data in and one >> for
> > >> serial data out. In QUAD SPI mode (x4 mode) data transfer will occur >> at all
> > >> the four data lines. QPIC controller supports command for x1 mode and >> x4 mode.    
> > >> >> Md Sadre Alam (5):    
> > >>   dt-bindings: qcom_nandc: IPQ5018 QPIC NAND documentation
> > >>   mtd: rawnand: qcom: Add initial support for qspi nand
> > >>   mtd: rawnand: qcom: Read QPIC version
> > >>   mtd: rawnand: qcom: Enable support for erase,read & write for serial
> > >>     nand.
> > >>   mtd: rawnand: qcom: Add support for serial training.    
> > >> >>  .../devicetree/bindings/mtd/qcom_nandc.txt         |   3 +    
> > >>  drivers/mtd/nand/raw/nand_ids.c                    |  13 +
> > >>  drivers/mtd/nand/raw/qcom_nandc.c                  | 502 >> ++++++++++++++++++++-
> > >>  3 files changed, 494 insertions(+), 24 deletions(-)    
> > >> > > I'm sorry but this series clearly breaks the current layering. I cannot    
> > > authorize SPI-NAND code to fall into the raw NAND subsystem.
> > >     
> > 
> > I am agree with you, we should not add SPI-NAND changes inside
> > raw NAND subsystem.
> >   
> > > As both typologies cannot be used at the same time, I guess you should
> > > have another driver handling this feature under the spi/ subsystem +
> > > a few declarations in the SPI-NAND devices list.
> > >     
> > 
> > Initially I was started writing separate driver under SPI-NAND subsystem then I
> > realized that more than 85% of raw/qcom_nand.c code getting duplicated.
> > 
> > That's why I have added this SPI-NAND change in raw/qcom_nand.c since
> > more than 85% of code will be reused.
> > 
> > If I will add this change inside SPI-NAND subsystem then much of
> > raw/qcom_nand.c code will get duplicated. Would it be ok ?  
> 
> What about moving the generic code to drivers/mtd/nand/common/ and

Yeah, I don't think drivers/mtd/nand/common/ is the right place to put
this common code TBH, and I don't really see what's to be shared between
the NAND controller and SPI controller drivers. If it's just helpers to
access the registers, those can probably live in
drivers/soc/qcom/qcom_qpic2.c.

> referring to it from drivers/mtd/nand/raw/qcom_nand.c and
> drivers/spi/spi-qcom.c (or such)?
> 
> Thanks,
> Miquèl
> 
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 2/5] mtd: rawnand: qcom: Add initial support for qspi nand

[mdalam]

On 2020-10-29 14:37, Boris Brezillon wrote:
> Hello,
> 
> On Sat, 10 Oct 2020 11:01:39 +0530
> Md Sadre Alam <mdalam@codeaurora.org> wrote:
> 
>> This change will add initial support for qspi (serial nand).
>> 
>> QPIC Version v.2.0 onwards supports serial nand as well so this
>> change will initialize all required register to enable qspi (serial
>> nand).
>> 
>> This change is supporting very basic functionality of qspi nand flash.
>> 
>> 1. Reset device (Reset QSPI NAND device).
>> 
>> 2. Device detection (Read id QSPI NAND device).
> 
> Unfortunately, that's not going to work in the long term. You're
> basically hacking the raw NAND framework to make SPI NANDs fit. I do
> understand the rationale behind this decision (re-using the code for
> ECC and probably other things), but that's not going to work. So I'd
> recommend doing the following instead:
> 
> 1/ implement a SPI-mem controller driver
> 2/ implement an ECC engine driver so the ECC logic can be shared
>    between the SPI controller and raw NAND controller drivers
> 3/ convert the raw NAND driver to the exec_op() interface (none of
>    this hack would have been possible if the driver was using the new
>    API)

Sorry for late reply. I think I mixup the serial nand support and 
QPIC_V2.0 support.
Only patches [2/5] mtd: rawnand: qcom: Add initial support for qspi nand
and [5/5] mtd: rawnand: qcom: Add support for serial training. are for 
serial
nand. Other patches [3/5] mtd: rawnand: qcom: Read QPIC version &
[4/5] mtd: rawnand: qcom: Enable support for erase,read & write for 
serial nand.
are to support QPIC_V2.0. In QPIC_V2.0 onwards some additional registers 
and features
got added. QPIC_NAND_READ_LOCATION_LAST_CW_n register got added to read 
last code word.
Page scope read & multi page read feature got added to read single and 
multiple
pages. QPIC_NAND_AUTO_STATUS_EN register got added to read status in 
page scope read
& multi page read etc.

I will take out QPIC_V2.0 support patches and will push it separately.
For serial nand support few lines of codes are there around 50 lines
to initalize QPIC serial block and serial training code. So can I put 
this
this as a separate file inside drivers/mtd/nand/raw/qpic_serial_nand.c. 
Would it be ok ?
Because there is no dedicated spi controller for serial nand. QPIC 
controller having
one serial interface block to deal with serial nand device.

> 
> Regards,
> 
> Boris