linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 1/5] mtd: rawnand: qcom: Implement exec_op()
@ 2023-05-31 12:49 Md Sadre Alam
  2023-05-31 12:49 ` [PATCH v3 2/5] mtd: rawnand: qcom: Add support for reset, readid, status exec_op Md Sadre Alam
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Md Sadre Alam @ 2023-05-31 12:49 UTC (permalink / raw)
  To: mani, miquel.raynal, richard, vigneshr, linux-mtd, linux-arm-msm,
	linux-kernel
  Cc: quic_srichara, quic_mdalam

Implement exec_op() so we can later get rid of the legacy interface
implementation.

Co-developed-by: Sricharan Ramabadhran <quic_srichara@quicinc.com>
Signed-off-by: Sricharan Ramabadhran <quic_srichara@quicinc.com>
Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
---
Change in [v3]

* Removed NAND_CMD_STATUS check in pre_command and move
  it to status exec_op.

* Removed min() , since this check not needed

* Removed all the dummy APIs of exec_ops, and added it
  into same patch where its getting added.

* Added qcom_check_op() API to check for unsupported feature
  by controller in check_only path.

Change in [v2]

* Missed to post Cover-letter, so posting v2 patch with cover-letter

Change in [v1]

* Added initial support for exec_ops.
   
 drivers/mtd/nand/raw/qcom_nandc.c | 159 ++++++++++++++++++++++++++++++
 1 file changed, 159 insertions(+)

diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
index 72d6168d8a1b..d9c4c9fe2fe8 100644
--- a/drivers/mtd/nand/raw/qcom_nandc.c
+++ b/drivers/mtd/nand/raw/qcom_nandc.c
@@ -157,6 +157,7 @@
 #define	OP_PAGE_PROGRAM_WITH_ECC	0x7
 #define	OP_PROGRAM_PAGE_SPARE		0x9
 #define	OP_BLOCK_ERASE			0xa
+#define	OP_CHECK_STATUS			0xc
 #define	OP_FETCH_ID			0xb
 #define	OP_RESET_DEVICE			0xd
 
@@ -235,6 +236,8 @@ nandc_set_reg(chip, reg,			\
  */
 #define NAND_ERASED_CW_SET		BIT(4)
 
+#define MAX_ADDRESS_CYCLE		5
+#define MAX_CHUNK_SIZE			SZ_8K
 /*
  * This data type corresponds to the BAM transaction which will be used for all
  * NAND transfers.
@@ -447,6 +450,29 @@ struct qcom_nand_boot_partition {
 	u32 page_size;
 };
 
+/*
+ * Qcom op for each exec_op transfer
+ *
+ * @data_instr:			data instruction pointer
+ * @data_instr_idx:		data instruction index
+ * @rdy_timeout_ms:		wait ready timeout in ms
+ * @rdy_delay_ns:		Additional delay in ns
+ * @addr1_reg:			Address1 register value
+ * @addr2_reg:			Address2 register value
+ * @cmd_reg:			CMD register value
+ * @flag:			flag for misc instruction
+ */
+struct qcom_op {
+	const struct nand_op_instr *data_instr;
+	unsigned int data_instr_idx;
+	unsigned int rdy_timeout_ms;
+	unsigned int rdy_delay_ns;
+	u32 addr1_reg;
+	u32 addr2_reg;
+	u32 cmd_reg;
+	u8 flag;
+};
+
 /*
  * NAND chip structure
  *
@@ -2867,8 +2893,141 @@ static int qcom_nand_attach_chip(struct nand_chip *chip)
 	return 0;
 }
 
+static int qcom_op_cmd_mapping(struct qcom_nand_controller *nandc, u8 cmd,
+			       struct qcom_op *q_op)
+{
+	int ret;
+
+	switch (cmd) {
+	case NAND_CMD_RESET:
+		ret = OP_RESET_DEVICE;
+		break;
+	case NAND_CMD_READID:
+		ret = OP_FETCH_ID;
+		break;
+	case NAND_CMD_PARAM:
+		if (nandc->props->qpic_v2)
+			ret = OP_PAGE_READ_ONFI_READ;
+		else
+			ret = OP_PAGE_READ;
+		break;
+	case NAND_CMD_ERASE1:
+	case NAND_CMD_ERASE2:
+		ret = OP_BLOCK_ERASE;
+		break;
+	case NAND_CMD_STATUS:
+		ret = OP_CHECK_STATUS;
+		break;
+	case NAND_CMD_PAGEPROG:
+		ret = OP_PROGRAM_PAGE;
+		break;
+	default:
+		break;
+	}
+
+	return ret;
+}
+
+/* NAND framework ->exec_op() hooks and related helpers */
+static void qcom_parse_instructions(struct nand_chip *chip,
+				    const struct nand_subop *subop,
+					struct qcom_op *q_op)
+{
+	struct qcom_nand_controller *nandc = get_qcom_nand_controller(chip);
+	const struct nand_op_instr *instr = NULL;
+	unsigned int op_id;
+	int i;
+
+	memset(q_op, 0, sizeof(*q_op));
+
+	for (op_id = 0; op_id < subop->ninstrs; op_id++) {
+		unsigned int offset, naddrs;
+		const u8 *addrs;
+
+		instr = &subop->instrs[op_id];
+
+		switch (instr->type) {
+		case NAND_OP_CMD_INSTR:
+			q_op->cmd_reg = qcom_op_cmd_mapping(nandc, instr->ctx.cmd.opcode, q_op);
+			q_op->rdy_delay_ns = instr->delay_ns;
+			break;
+
+		case NAND_OP_ADDR_INSTR:
+			offset = nand_subop_get_addr_start_off(subop, op_id);
+			naddrs = nand_subop_get_num_addr_cyc(subop, op_id);
+			addrs = &instr->ctx.addr.addrs[offset];
+			for (i = 0; i < MAX_ADDRESS_CYCLE; i++) {
+				if (i < 4)
+					q_op->addr1_reg |= (u32)addrs[i] << i * 8;
+				else
+					q_op->addr2_reg |= addrs[i];
+			}
+			q_op->rdy_delay_ns = instr->delay_ns;
+			break;
+
+		case NAND_OP_DATA_IN_INSTR:
+			q_op->data_instr = instr;
+			q_op->data_instr_idx = op_id;
+			q_op->rdy_delay_ns = instr->delay_ns;
+			fallthrough;
+		case NAND_OP_DATA_OUT_INSTR:
+			q_op->rdy_delay_ns = instr->delay_ns;
+			break;
+
+		case NAND_OP_WAITRDY_INSTR:
+			q_op->rdy_timeout_ms = instr->ctx.waitrdy.timeout_ms;
+			q_op->rdy_delay_ns = instr->delay_ns;
+			break;
+		}
+	}
+}
+
+static int qcom_check_op(struct nand_chip *chip,
+			 const struct nand_operation *op)
+{
+	const struct nand_op_instr *instr;
+	int op_id;
+
+	for (op_id = 0; op_id < op->ninstrs; op_id++) {
+		instr = &op->instrs[op_id];
+
+		switch (instr->type) {
+		case NAND_OP_CMD_INSTR:
+			if (instr->ctx.cmd.opcode == NAND_CMD_READCACHESEQ ||
+			    instr->ctx.cmd.opcode == NAND_CMD_READCACHEEND)
+				return -ENOTSUPP;
+			break;
+		case NAND_OP_ADDR_INSTR:
+			if (instr->ctx.addr.naddrs > MAX_ADDRESS_CYCLE)
+				return -ENOTSUPP;
+
+			break;
+		case NAND_OP_DATA_IN_INSTR:
+		case NAND_OP_DATA_OUT_INSTR:
+			if (instr->ctx.data.len > MAX_CHUNK_SIZE)
+				return -ENOTSUPP;
+			break;
+		default:
+			break;
+		}
+	}
+
+	return 0;
+}
+
+static int qcom_nand_exec_op(struct nand_chip *chip,
+			     const struct nand_operation *op,
+			bool check_only)
+{
+	if (check_only)
+		return qcom_check_op(chip, op);
+
+	return 0;
+}
+
 static const struct nand_controller_ops qcom_nandc_ops = {
 	.attach_chip = qcom_nand_attach_chip,
+	.exec_op = qcom_nand_exec_op,
 };
 
 static void qcom_nandc_unalloc(struct qcom_nand_controller *nandc)
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v3 2/5] mtd: rawnand: qcom: Add support for reset, readid, status exec_op
  2023-05-31 12:49 [PATCH v3 1/5] mtd: rawnand: qcom: Implement exec_op() Md Sadre Alam
@ 2023-05-31 12:49 ` Md Sadre Alam
  2023-06-08 13:58   ` Miquel Raynal
  2023-05-31 12:49 ` [PATCH v3 3/5] mtd: rawnand: qcom: Add support for param_page read exec_ops Md Sadre Alam
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Md Sadre Alam @ 2023-05-31 12:49 UTC (permalink / raw)
  To: mani, miquel.raynal, richard, vigneshr, linux-mtd, linux-arm-msm,
	linux-kernel
  Cc: quic_srichara, quic_mdalam

This change will add exec_ops support for RESET , READ_ID, STATUS
command.

Co-developed-by: Sricharan Ramabadhran <quic_srichara@quicinc.com>
Signed-off-by: Sricharan Ramabadhran <quic_srichara@quicinc.com>
Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
---
Change in [v3]

* Updated q_op->flag in qcom_op_cmd_mapping() with OP_PROGRAM_PAGE
  instead NAND_CMD_PAGEPROG

* Change the if condition check for exec_opwrite into single line check.

* Added error check

* Removed check for NAND_CMD_RESET, NAND_CMD_READID from pre_command.

Change in [v2]

* Missed to post Cover-letter, so posting v2 patch with cover-letter

Change in [v1]

* Added initial support for exec_ops.
   
 drivers/mtd/nand/raw/qcom_nandc.c | 218 +++++++++++++++++++++++++++++-
 1 file changed, 215 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
index 6a83cd86a12d..659ba923b68a 100644
--- a/drivers/mtd/nand/raw/qcom_nandc.c
+++ b/drivers/mtd/nand/raw/qcom_nandc.c
@@ -385,6 +385,9 @@ struct nandc_regs {
  * @reg_read_pos:		marker for data read in reg_read_buf
  *
  * @cmd1/vld:			some fixed controller register values
+ *
+ * @exec_opwrite:		flag to select correct number of code word
+ *				while reading status
  */
 struct qcom_nand_controller {
 	struct device *dev;
@@ -435,6 +438,7 @@ struct qcom_nand_controller {
 	int reg_read_pos;
 
 	u32 cmd1, vld;
+	bool exec_opwrite;
 };
 
 /*
@@ -1542,8 +1546,7 @@ static void pre_command(struct qcom_nand_host *host, int command)
 
 	clear_read_regs(nandc);
 
-	if (command == NAND_CMD_RESET || command == NAND_CMD_READID ||
-	    command == NAND_CMD_PARAM || command == NAND_CMD_ERASE1)
+	if (command == NAND_CMD_PARAM || command == NAND_CMD_ERASE1)
 		clear_bam_transaction(nandc);
 }
 
@@ -2920,6 +2923,8 @@ static int qcom_op_cmd_mapping(struct qcom_nand_controller *nandc, u8 cmd,
 		break;
 	case NAND_CMD_PAGEPROG:
 		ret = OP_PROGRAM_PAGE;
+		q_op->flag = OP_PROGRAM_PAGE;
+		nandc->exec_opwrite = true;
 		break;
 	default:
 		break;
@@ -2982,6 +2987,212 @@ static void qcom_parse_instructions(struct nand_chip *chip,
 	}
 }
 
+static void qcom_delay_ns(unsigned int ns)
+{
+	if (!ns)
+		return;
+
+	if (ns < 10000)
+		ndelay(ns);
+	else
+		udelay(DIV_ROUND_UP(ns, 1000));
+}
+
+static int qcom_wait_rdy_poll(struct nand_chip *chip, unsigned int time_ms)
+{
+	struct qcom_nand_controller *nandc = get_qcom_nand_controller(chip);
+	unsigned long start = jiffies + msecs_to_jiffies(time_ms);
+	u32 flash;
+
+	nandc_read_buffer_sync(nandc, true);
+
+	do {
+		flash = le32_to_cpu(nandc->reg_read_buf[0]);
+		if (flash & FS_READY_BSY_N)
+			return 0;
+		cpu_relax();
+	} while (time_after(start, jiffies));
+
+	dev_err(nandc->dev, "Timeout waiting for device to be ready:0x%08x\n", flash);
+
+	return -ETIMEDOUT;
+}
+
+static int qcom_read_status_exec(struct nand_chip *chip,
+				 const struct nand_subop *subop)
+{
+	struct qcom_nand_host *host = to_qcom_nand_host(chip);
+	struct qcom_nand_controller *nandc = get_qcom_nand_controller(chip);
+	struct nand_ecc_ctrl *ecc = &chip->ecc;
+	struct qcom_op q_op;
+	const struct nand_op_instr *instr = NULL;
+	unsigned int op_id = 0;
+	unsigned int len = 0;
+	int ret = 0, num_cw, i;
+	u32 flash_status;
+
+	host->status = NAND_STATUS_READY | NAND_STATUS_WP;
+
+	qcom_parse_instructions(chip, subop, &q_op);
+
+	num_cw = nandc->exec_opwrite ? ecc->steps : 1;
+	nandc->exec_opwrite = false;
+
+	nandc->buf_count = 0;
+	nandc->buf_start = 0;
+	host->use_ecc = false;
+
+	clear_read_regs(nandc);
+	clear_bam_transaction(nandc);
+
+	nandc_set_reg(chip, NAND_FLASH_CMD, q_op.cmd_reg);
+	nandc_set_reg(chip, NAND_EXEC_CMD, 1);
+
+	write_reg_dma(nandc, NAND_FLASH_CMD, 1, NAND_BAM_NEXT_SGL);
+	write_reg_dma(nandc, NAND_EXEC_CMD, 1, NAND_BAM_NEXT_SGL);
+	read_reg_dma(nandc, NAND_FLASH_STATUS, 1, NAND_BAM_NEXT_SGL);
+
+	ret = submit_descs(nandc);
+	if (ret) {
+		dev_err(nandc->dev, "failure in sbumitting status descriptor\n");
+		free_descs(nandc);
+		goto err_out;
+	}
+	free_descs(nandc);
+
+	nandc_read_buffer_sync(nandc, true);
+
+	for (i = 0; i < num_cw; i++) {
+		flash_status = le32_to_cpu(nandc->reg_read_buf[i]);
+
+	if (flash_status & FS_MPU_ERR)
+		host->status &= ~NAND_STATUS_WP;
+
+	if (flash_status & FS_OP_ERR || (i == (num_cw - 1) &&
+					 (flash_status & FS_DEVICE_STS_ERR)))
+		host->status |= NAND_STATUS_FAIL;
+	}
+
+	flash_status = host->status;
+	instr = q_op.data_instr;
+	op_id = q_op.data_instr_idx;
+	len = nand_subop_get_data_len(subop, op_id);
+	memcpy(instr->ctx.data.buf.in, &flash_status, len);
+
+err_out:
+	return ret;
+}
+
+static int qcom_read_id_type_exec(struct nand_chip *chip, const struct nand_subop *subop)
+{
+	struct qcom_nand_controller *nandc = get_qcom_nand_controller(chip);
+	struct qcom_nand_host *host = to_qcom_nand_host(chip);
+	struct qcom_op q_op;
+	const struct nand_op_instr *instr = NULL;
+	unsigned int op_id = 0;
+	unsigned int len = 0;
+	int ret = 0;
+
+	qcom_parse_instructions(chip, subop, &q_op);
+
+	nandc->buf_count = 0;
+	nandc->buf_start = 0;
+	host->use_ecc = false;
+
+	clear_read_regs(nandc);
+	clear_bam_transaction(nandc);
+
+	nandc_set_reg(chip, NAND_FLASH_CMD, q_op.cmd_reg);
+	nandc_set_reg(chip, NAND_ADDR0, q_op.addr1_reg);
+	nandc_set_reg(chip, NAND_ADDR1, q_op.addr2_reg);
+	nandc_set_reg(chip, NAND_FLASH_CHIP_SELECT,
+		      nandc->props->is_bam ? 0 : DM_EN);
+
+	nandc_set_reg(chip, NAND_EXEC_CMD, 1);
+
+	write_reg_dma(nandc, NAND_FLASH_CMD, 4, NAND_BAM_NEXT_SGL);
+	write_reg_dma(nandc, NAND_EXEC_CMD, 1, NAND_BAM_NEXT_SGL);
+
+	read_reg_dma(nandc, NAND_READ_ID, 1, NAND_BAM_NEXT_SGL);
+
+	ret = submit_descs(nandc);
+	if (ret) {
+		dev_err(nandc->dev, "failure in sbumitting read id descriptor\n");
+		free_descs(nandc);
+		goto err_out;
+	}
+	free_descs(nandc);
+
+	instr = q_op.data_instr;
+	op_id = q_op.data_instr_idx;
+	len = nand_subop_get_data_len(subop, op_id);
+
+	nandc_read_buffer_sync(nandc, true);
+	memcpy(instr->ctx.data.buf.in, nandc->reg_read_buf, len);
+
+err_out:
+	return ret;
+}
+
+static int qcom_misc_cmd_type_exec(struct nand_chip *chip, const struct nand_subop *subop)
+{
+	struct qcom_nand_controller *nandc = get_qcom_nand_controller(chip);
+	struct qcom_nand_host *host = to_qcom_nand_host(chip);
+	struct qcom_op q_op;
+	int ret = 0;
+
+	qcom_parse_instructions(chip, subop, &q_op);
+
+	if (q_op.flag == OP_PROGRAM_PAGE)
+		goto wait_rdy;
+
+	nandc->buf_count = 0;
+	nandc->buf_start = 0;
+	host->use_ecc = false;
+
+	clear_read_regs(nandc);
+	clear_bam_transaction(nandc);
+
+	nandc_set_reg(chip, NAND_FLASH_CMD, q_op.cmd_reg);
+	nandc_set_reg(chip, NAND_EXEC_CMD, 1);
+
+	write_reg_dma(nandc, NAND_FLASH_CMD, 1, NAND_BAM_NEXT_SGL);
+	write_reg_dma(nandc, NAND_EXEC_CMD, 1, NAND_BAM_NEXT_SGL);
+
+	read_reg_dma(nandc, NAND_FLASH_STATUS, 1, NAND_BAM_NEXT_SGL);
+
+	ret = submit_descs(nandc);
+	if (ret) {
+		dev_err(nandc->dev, "failure in sbumitting misc descriptor\n");
+		free_descs(nandc);
+		goto err_out;
+	}
+	free_descs(nandc);
+
+wait_rdy:
+	qcom_delay_ns(q_op.rdy_delay_ns);
+	ret = qcom_wait_rdy_poll(chip, q_op.rdy_timeout_ms);
+
+err_out:
+	return ret;
+}
+
+static const struct nand_op_parser qcom_op_parser = NAND_OP_PARSER(
+		NAND_OP_PARSER_PATTERN(
+			qcom_misc_cmd_type_exec,
+			NAND_OP_PARSER_PAT_CMD_ELEM(false),
+			NAND_OP_PARSER_PAT_WAITRDY_ELEM(false)),
+		NAND_OP_PARSER_PATTERN(
+			qcom_read_id_type_exec,
+			NAND_OP_PARSER_PAT_CMD_ELEM(false),
+			NAND_OP_PARSER_PAT_ADDR_ELEM(false, MAX_ADDRESS_CYCLE),
+			NAND_OP_PARSER_PAT_DATA_IN_ELEM(false, 8)),
+		NAND_OP_PARSER_PATTERN(
+			qcom_read_status_exec,
+			NAND_OP_PARSER_PAT_CMD_ELEM(false),
+			NAND_OP_PARSER_PAT_DATA_IN_ELEM(false, 1)),
+		);
+
 static int qcom_check_op(struct nand_chip *chip,
 			const struct nand_operation *op)
 {
@@ -3022,7 +3233,8 @@ static int qcom_nand_exec_op(struct nand_chip *chip,
 	if (check_only)
 		return qcom_check_op(chip, op);
 
-	return 0;
+	return nand_op_parser_exec_op(chip, &qcom_op_parser,
+			op, check_only);
 }
 
 static const struct nand_controller_ops qcom_nandc_ops = {
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v3 3/5] mtd: rawnand: qcom: Add support for param_page read exec_ops
  2023-05-31 12:49 [PATCH v3 1/5] mtd: rawnand: qcom: Implement exec_op() Md Sadre Alam
  2023-05-31 12:49 ` [PATCH v3 2/5] mtd: rawnand: qcom: Add support for reset, readid, status exec_op Md Sadre Alam
@ 2023-05-31 12:49 ` Md Sadre Alam
  2023-05-31 12:49 ` [PATCH v3 4/5] mtd: rawnand: qcom: Add support for read, write, erase exec_ops Md Sadre Alam
  2023-05-31 12:49 ` [PATCH v3 5/5] mtd: rawnand: qcom: Remove legacy interface Md Sadre Alam
  3 siblings, 0 replies; 9+ messages in thread
From: Md Sadre Alam @ 2023-05-31 12:49 UTC (permalink / raw)
  To: mani, miquel.raynal, richard, vigneshr, linux-mtd, linux-arm-msm,
	linux-kernel
  Cc: quic_srichara, quic_mdalam

This change will add exec_ops for PARAM_PAGE_READ command.

Co-developed-by: Sricharan Ramabadhran <quic_srichara@quicinc.com>
Signed-off-by: Sricharan Ramabadhran <quic_srichara@quicinc.com>
Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
---
Change in [v3]

* Removed hard coded value in param_page exec_op API, and used whatever
  passed by core.

* Removed nandc->props->qpic_v2 condition , since its not needed at all.

* Updated nandc->buf_count = len , len provided by core.

Change in [v2]

* Missed to post Cover-letter, so posting v2 patch with cover-letter

Change in [v1]

* Added initial support for exec_ops.

 drivers/mtd/nand/raw/qcom_nandc.c | 105 +++++++++++++++++++++++++++++-
 1 file changed, 104 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
index 659ba923b68a..b4823b72fe73 100644
--- a/drivers/mtd/nand/raw/qcom_nandc.c
+++ b/drivers/mtd/nand/raw/qcom_nandc.c
@@ -1546,7 +1546,7 @@ static void pre_command(struct qcom_nand_host *host, int command)
 
 	clear_read_regs(nandc);
 
-	if (command == NAND_CMD_PARAM || command == NAND_CMD_ERASE1)
+	if (command == NAND_CMD_ERASE1)
 		clear_bam_transaction(nandc);
 }
 
@@ -3177,6 +3177,103 @@ static int qcom_misc_cmd_type_exec(struct nand_chip *chip, const struct nand_sub
 	return ret;
 }
 
+static int qcom_param_page_type_exec(struct nand_chip *chip,  const struct nand_subop *subop)
+{
+	struct qcom_nand_host *host = to_qcom_nand_host(chip);
+	struct qcom_nand_controller *nandc = get_qcom_nand_controller(chip);
+	struct qcom_op q_op;
+	const struct nand_op_instr *instr = NULL;
+	unsigned int op_id = 0;
+	unsigned int len = 0;
+	int ret = 0;
+
+	qcom_parse_instructions(chip, subop, &q_op);
+
+	q_op.cmd_reg |= PAGE_ACC | LAST_PAGE;
+
+	nandc->buf_count = 0;
+	nandc->buf_start = 0;
+	host->use_ecc = false;
+	clear_read_regs(nandc);
+	clear_bam_transaction(nandc);
+
+	nandc_set_reg(chip, NAND_FLASH_CMD, q_op.cmd_reg);
+
+	nandc_set_reg(chip, NAND_ADDR0, 0);
+	nandc_set_reg(chip, NAND_ADDR1, 0);
+	nandc_set_reg(chip, NAND_DEV0_CFG0, 0 << CW_PER_PAGE
+					| 512 << UD_SIZE_BYTES
+					| 5 << NUM_ADDR_CYCLES
+					| 0 << SPARE_SIZE_BYTES);
+	nandc_set_reg(chip, NAND_DEV0_CFG1, 7 << NAND_RECOVERY_CYCLES
+					| 0 << CS_ACTIVE_BSY
+					| 17 << BAD_BLOCK_BYTE_NUM
+					| 1 << BAD_BLOCK_IN_SPARE_AREA
+					| 2 << WR_RD_BSY_GAP
+					| 0 << WIDE_FLASH
+					| 1 << DEV0_CFG1_ECC_DISABLE);
+	if (!nandc->props->qpic_v2)
+		nandc_set_reg(chip, NAND_EBI2_ECC_BUF_CFG, 1 << ECC_CFG_ECC_DISABLE);
+
+	/* configure CMD1 and VLD for ONFI param probing in QPIC v1 */
+	if (!nandc->props->qpic_v2) {
+		nandc_set_reg(chip, NAND_DEV_CMD_VLD,
+			      (nandc->vld & ~READ_START_VLD));
+		nandc_set_reg(chip, NAND_DEV_CMD1,
+			      (nandc->cmd1 & ~(0xFF << READ_ADDR))
+			      | NAND_CMD_PARAM << READ_ADDR);
+	}
+
+	nandc_set_reg(chip, NAND_EXEC_CMD, 1);
+
+	if (!nandc->props->qpic_v2) {
+		nandc_set_reg(chip, NAND_DEV_CMD1_RESTORE, nandc->cmd1);
+		nandc_set_reg(chip, NAND_DEV_CMD_VLD_RESTORE, nandc->vld);
+	}
+
+	instr = q_op.data_instr;
+	op_id = q_op.data_instr_idx;
+	len = nand_subop_get_data_len(subop, op_id);
+
+	nandc_set_read_loc(chip, 0, 0, 0, len, 1);
+
+	if (!nandc->props->qpic_v2) {
+		write_reg_dma(nandc, NAND_DEV_CMD_VLD, 1, 0);
+		write_reg_dma(nandc, NAND_DEV_CMD1, 1, NAND_BAM_NEXT_SGL);
+	}
+
+	nandc->buf_count = len;
+	memset(nandc->data_buffer, 0xff, nandc->buf_count);
+
+	config_nand_single_cw_page_read(chip, false, 0);
+
+	read_data_dma(nandc, FLASH_BUF_ACC, nandc->data_buffer,
+		      nandc->buf_count, 0);
+
+	/* restore CMD1 and VLD regs */
+	if (!nandc->props->qpic_v2) {
+		write_reg_dma(nandc, NAND_DEV_CMD1_RESTORE, 1, 0);
+		write_reg_dma(nandc, NAND_DEV_CMD_VLD_RESTORE, 1, NAND_BAM_NEXT_SGL);
+	}
+
+	ret = submit_descs(nandc);
+	if (ret) {
+		dev_err(nandc->dev, "failure in sbumitting param page descriptor\n");
+		free_descs(nandc);
+		goto err_out;
+	}
+	free_descs(nandc);
+
+	ret = qcom_wait_rdy_poll(chip, q_op.rdy_timeout_ms);
+	if (ret)
+		goto err_out;
+
+	memcpy(instr->ctx.data.buf.in, nandc->data_buffer, len);
+
+err_out:
+	return ret;
+}
+
 static const struct nand_op_parser qcom_op_parser = NAND_OP_PARSER(
 		NAND_OP_PARSER_PATTERN(
 			qcom_misc_cmd_type_exec,
@@ -3191,6 +3288,12 @@ static const struct nand_op_parser qcom_op_parser = NAND_OP_PARSER(
 			qcom_read_status_exec,
 			NAND_OP_PARSER_PAT_CMD_ELEM(false),
 			NAND_OP_PARSER_PAT_DATA_IN_ELEM(false, 1)),
+		NAND_OP_PARSER_PATTERN(
+			qcom_param_page_type_exec,
+			NAND_OP_PARSER_PAT_CMD_ELEM(false),
+			NAND_OP_PARSER_PAT_ADDR_ELEM(false, MAX_ADDRESS_CYCLE),
+			NAND_OP_PARSER_PAT_WAITRDY_ELEM(true),
+			NAND_OP_PARSER_PAT_DATA_IN_ELEM(false, 512)),
 		);
 
 static int qcom_check_op(struct nand_chip *chip,
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v3 4/5] mtd: rawnand: qcom: Add support for read, write, erase exec_ops
  2023-05-31 12:49 [PATCH v3 1/5] mtd: rawnand: qcom: Implement exec_op() Md Sadre Alam
  2023-05-31 12:49 ` [PATCH v3 2/5] mtd: rawnand: qcom: Add support for reset, readid, status exec_op Md Sadre Alam
  2023-05-31 12:49 ` [PATCH v3 3/5] mtd: rawnand: qcom: Add support for param_page read exec_ops Md Sadre Alam
@ 2023-05-31 12:49 ` Md Sadre Alam
  2023-06-08 14:03   ` Miquel Raynal
  2023-05-31 12:49 ` [PATCH v3 5/5] mtd: rawnand: qcom: Remove legacy interface Md Sadre Alam
  3 siblings, 1 reply; 9+ messages in thread
From: Md Sadre Alam @ 2023-05-31 12:49 UTC (permalink / raw)
  To: mani, miquel.raynal, richard, vigneshr, linux-mtd, linux-arm-msm,
	linux-kernel
  Cc: quic_srichara, quic_mdalam

This change will add exec_ops support for READ, WRITE, and ERASE
command.

Co-developed-by: Sricharan Ramabadhran <quic_srichara@quicinc.com>
Signed-off-by: Sricharan Ramabadhran <quic_srichara@quicinc.com>
Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
---
Change in [v3]

* Removed chip->cont_read.ongoing flag.

* Removed pre_command from erase_etype_exec_ops.

Change in [v2]

* Missed to post Cover-letter, so posting v2 patch with cover-letter.

Change in [v1]

* Added initial support for exec_ops.

 drivers/mtd/nand/raw/qcom_nandc.c | 97 +++++++++++++++++++++++++++++--
 1 file changed, 93 insertions(+), 4 deletions(-)

diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
index b4823b72fe73..7dc769f9e797 100644
--- a/drivers/mtd/nand/raw/qcom_nandc.c
+++ b/drivers/mtd/nand/raw/qcom_nandc.c
@@ -1546,8 +1546,7 @@ static void pre_command(struct qcom_nand_host *host, int command)
 
 	clear_read_regs(nandc);
 
-	if (command == NAND_CMD_ERASE1)
-		clear_bam_transaction(nandc);
+	clear_bam_transaction(nandc);
 }
 
 /*
@@ -1764,7 +1763,7 @@ qcom_nandc_read_cw_raw(struct mtd_info *mtd, struct nand_chip *chip,
 	int ret, reg_off = FLASH_BUF_ACC, read_loc = 0;
 	int raw_cw = cw;
 
-	nand_read_page_op(chip, page, 0, NULL, 0);
+	nand_read_page_op(chip, page, 0, data_buf, mtd->writesize);
 	host->use_ecc = false;
 
 	if (nandc->props->qpic_v2)
@@ -2181,14 +2180,23 @@ static void qcom_nandc_codeword_fixup(struct qcom_nand_host *host, int page)
 static int qcom_nandc_read_page(struct nand_chip *chip, uint8_t *buf,
 				int oob_required, int page)
 {
+	struct mtd_info *mtd = nand_to_mtd(chip);
 	struct qcom_nand_host *host = to_qcom_nand_host(chip);
 	struct qcom_nand_controller *nandc = get_qcom_nand_controller(chip);
+	struct nand_ecc_ctrl *ecc = &chip->ecc;
 	u8 *data_buf, *oob_buf = NULL;
 
 	if (host->nr_boot_partitions)
 		qcom_nandc_codeword_fixup(host, page);
 
-	nand_read_page_op(chip, page, 0, NULL, 0);
+	nand_read_page_op(chip, page, 0, buf, mtd->writesize);
+	nandc->buf_count = 0;
+	nandc->buf_start = 0;
+	host->use_ecc = true;
+	clear_read_regs(nandc);
+	set_address(host, 0, page);
+	update_rw_regs(host, ecc->steps, true, 0);
+
 	data_buf = buf;
 	oob_buf = oob_required ? chip->oob_poi : NULL;
 
@@ -2258,6 +2266,9 @@ static int qcom_nandc_write_page(struct nand_chip *chip, const uint8_t *buf,
 
 	nand_prog_page_begin_op(chip, page, 0, NULL, 0);
 
+	set_address(host, 0, page);
+	nandc->buf_count = 0;
+	nandc->buf_start = 0;
 	clear_read_regs(nandc);
 	clear_bam_transaction(nandc);
 
@@ -3274,6 +3285,67 @@ static int qcom_param_page_type_exec(struct nand_chip *chip,  const struct nand_
 	return ret;
 }
 
+static int qcom_erase_cmd_type_exec(struct nand_chip *chip, const struct nand_subop *subop)
+{
+	struct qcom_nand_host *host = to_qcom_nand_host(chip);
+	struct qcom_nand_controller *nandc = get_qcom_nand_controller(chip);
+	struct qcom_op q_op;
+	int ret = 0;
+
+	qcom_parse_instructions(chip, subop, &q_op);
+
+	q_op.cmd_reg |= PAGE_ACC | LAST_PAGE;
+
+	nandc->buf_count = 0;
+	nandc->buf_start = 0;
+	host->use_ecc = false;
+	clear_read_regs(nandc);
+	clear_bam_transaction(nandc);
+
+	nandc_set_reg(chip, NAND_FLASH_CMD, q_op.cmd_reg);
+	nandc_set_reg(chip, NAND_ADDR0, q_op.addr1_reg);
+	nandc_set_reg(chip, NAND_ADDR1, q_op.addr2_reg);
+	nandc_set_reg(chip, NAND_DEV0_CFG0,
+		      host->cfg0_raw & ~(7 << CW_PER_PAGE));
+	nandc_set_reg(chip, NAND_DEV0_CFG1, host->cfg1_raw);
+	nandc_set_reg(chip, NAND_EXEC_CMD, 1);
+
+	write_reg_dma(nandc, NAND_FLASH_CMD, 3, NAND_BAM_NEXT_SGL);
+	write_reg_dma(nandc, NAND_DEV0_CFG0, 2, NAND_BAM_NEXT_SGL);
+	write_reg_dma(nandc, NAND_EXEC_CMD, 1, NAND_BAM_NEXT_SGL);
+
+	ret = submit_descs(nandc);
+	if (ret) {
+		dev_err(nandc->dev, "failure in sbumitting erase descriptor\n");
+		free_descs(nandc);
+		goto err_out;
+	}
+	free_descs(nandc);
+
+	ret = qcom_wait_rdy_poll(chip, q_op.rdy_timeout_ms);
+	if (ret)
+		goto err_out;
+
+err_out:
+	return ret;
+}
+
+static int qcom_data_read_type_exec(struct nand_chip *chip, const struct nand_subop *subop)
+{
+	/* currently read_exec_op() return 0 , and all the read operation handle in
+	 * actual API itself
+	 */
+	return 0;
+}
+
+static int qcom_data_write_type_exec(struct nand_chip *chip, const struct nand_subop *subop)
+{
+	/* currently write_exec_op() return 0, and all the write operation handle in
+	 * actual API itself
+	 */
+	return 0;
+}
+
 static const struct nand_op_parser qcom_op_parser = NAND_OP_PARSER(
 		NAND_OP_PARSER_PATTERN(
 			qcom_misc_cmd_type_exec,
@@ -3294,6 +3366,23 @@ static const struct nand_op_parser qcom_op_parser = NAND_OP_PARSER(
 			NAND_OP_PARSER_PAT_ADDR_ELEM(false, MAX_ADDRESS_CYCLE),
 			NAND_OP_PARSER_PAT_WAITRDY_ELEM(true),
 			NAND_OP_PARSER_PAT_DATA_IN_ELEM(false, 512)),
+		NAND_OP_PARSER_PATTERN(
+			qcom_erase_cmd_type_exec,
+			NAND_OP_PARSER_PAT_CMD_ELEM(false),
+			NAND_OP_PARSER_PAT_ADDR_ELEM(false, MAX_ADDRESS_CYCLE),
+			NAND_OP_PARSER_PAT_CMD_ELEM(false),
+			NAND_OP_PARSER_PAT_WAITRDY_ELEM(false)),
+		NAND_OP_PARSER_PATTERN(
+			qcom_data_read_type_exec,
+			NAND_OP_PARSER_PAT_CMD_ELEM(false),
+			NAND_OP_PARSER_PAT_ADDR_ELEM(false, MAX_ADDRESS_CYCLE),
+			NAND_OP_PARSER_PAT_CMD_ELEM(false),
+			NAND_OP_PARSER_PAT_WAITRDY_ELEM(true),
+			NAND_OP_PARSER_PAT_DATA_IN_ELEM(false, 2048)),
+		NAND_OP_PARSER_PATTERN(
+			qcom_data_write_type_exec,
+			NAND_OP_PARSER_PAT_CMD_ELEM(true),
+			NAND_OP_PARSER_PAT_ADDR_ELEM(true, MAX_ADDRESS_CYCLE)),
 		);
 
 static int qcom_check_op(struct nand_chip *chip,
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v3 5/5] mtd: rawnand: qcom: Remove legacy interface
  2023-05-31 12:49 [PATCH v3 1/5] mtd: rawnand: qcom: Implement exec_op() Md Sadre Alam
                   ` (2 preceding siblings ...)
  2023-05-31 12:49 ` [PATCH v3 4/5] mtd: rawnand: qcom: Add support for read, write, erase exec_ops Md Sadre Alam
@ 2023-05-31 12:49 ` Md Sadre Alam
  3 siblings, 0 replies; 9+ messages in thread
From: Md Sadre Alam @ 2023-05-31 12:49 UTC (permalink / raw)
  To: mani, miquel.raynal, richard, vigneshr, linux-mtd, linux-arm-msm,
	linux-kernel
  Cc: quic_srichara, quic_mdalam

This change will remove legacy interface implementation

Co-developed-by: Sricharan Ramabadhran <quic_srichara@quicinc.com>
Signed-off-by: Sricharan Ramabadhran <quic_srichara@quicinc.com>
Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
---

Change in [v3]

* Removed pre_command() API definition completely.

Change in [v2]

* Missed to post Cover-letter, so posting v2 patch with cover-letter

Change in [v1]

* Added initial support for exec_ops.

 drivers/mtd/nand/raw/qcom_nandc.c | 359 ------------------------------
 1 file changed, 359 deletions(-)

diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
index 7dc769f9e797..34448492b308 100644
--- a/drivers/mtd/nand/raw/qcom_nandc.c
+++ b/drivers/mtd/nand/raw/qcom_nandc.c
@@ -1303,155 +1303,6 @@ static void config_nand_cw_write(struct nand_chip *chip)
 	write_reg_dma(nandc, NAND_READ_STATUS, 1, NAND_BAM_NEXT_SGL);
 }
 
-/*
- * the following functions are used within chip->legacy.cmdfunc() to
- * perform different NAND_CMD_* commands
- */
-
-/* sets up descriptors for NAND_CMD_PARAM */
-static int nandc_param(struct qcom_nand_host *host)
-{
-	struct nand_chip *chip = &host->chip;
-	struct qcom_nand_controller *nandc = get_qcom_nand_controller(chip);
-
-	/*
-	 * NAND_CMD_PARAM is called before we know much about the FLASH chip
-	 * in use. we configure the controller to perform a raw read of 512
-	 * bytes to read onfi params
-	 */
-	if (nandc->props->qpic_v2)
-		nandc_set_reg(chip, NAND_FLASH_CMD, OP_PAGE_READ_ONFI_READ |
-			      PAGE_ACC | LAST_PAGE);
-	else
-		nandc_set_reg(chip, NAND_FLASH_CMD, OP_PAGE_READ |
-			      PAGE_ACC | LAST_PAGE);
-
-	nandc_set_reg(chip, NAND_ADDR0, 0);
-	nandc_set_reg(chip, NAND_ADDR1, 0);
-	nandc_set_reg(chip, NAND_DEV0_CFG0, 0 << CW_PER_PAGE
-					| 512 << UD_SIZE_BYTES
-					| 5 << NUM_ADDR_CYCLES
-					| 0 << SPARE_SIZE_BYTES);
-	nandc_set_reg(chip, NAND_DEV0_CFG1, 7 << NAND_RECOVERY_CYCLES
-					| 0 << CS_ACTIVE_BSY
-					| 17 << BAD_BLOCK_BYTE_NUM
-					| 1 << BAD_BLOCK_IN_SPARE_AREA
-					| 2 << WR_RD_BSY_GAP
-					| 0 << WIDE_FLASH
-					| 1 << DEV0_CFG1_ECC_DISABLE);
-	if (!nandc->props->qpic_v2)
-		nandc_set_reg(chip, NAND_EBI2_ECC_BUF_CFG, 1 << ECC_CFG_ECC_DISABLE);
-
-	/* configure CMD1 and VLD for ONFI param probing in QPIC v1 */
-	if (!nandc->props->qpic_v2) {
-		nandc_set_reg(chip, NAND_DEV_CMD_VLD,
-			      (nandc->vld & ~READ_START_VLD));
-		nandc_set_reg(chip, NAND_DEV_CMD1,
-			      (nandc->cmd1 & ~(0xFF << READ_ADDR))
-			      | NAND_CMD_PARAM << READ_ADDR);
-	}
-
-	nandc_set_reg(chip, NAND_EXEC_CMD, 1);
-
-	if (!nandc->props->qpic_v2) {
-		nandc_set_reg(chip, NAND_DEV_CMD1_RESTORE, nandc->cmd1);
-		nandc_set_reg(chip, NAND_DEV_CMD_VLD_RESTORE, nandc->vld);
-	}
-
-	nandc_set_read_loc(chip, 0, 0, 0, 512, 1);
-
-	if (!nandc->props->qpic_v2) {
-		write_reg_dma(nandc, NAND_DEV_CMD_VLD, 1, 0);
-		write_reg_dma(nandc, NAND_DEV_CMD1, 1, NAND_BAM_NEXT_SGL);
-	}
-
-	nandc->buf_count = 512;
-	memset(nandc->data_buffer, 0xff, nandc->buf_count);
-
-	config_nand_single_cw_page_read(chip, false, 0);
-
-	read_data_dma(nandc, FLASH_BUF_ACC, nandc->data_buffer,
-		      nandc->buf_count, 0);
-
-	/* restore CMD1 and VLD regs */
-	if (!nandc->props->qpic_v2) {
-		write_reg_dma(nandc, NAND_DEV_CMD1_RESTORE, 1, 0);
-		write_reg_dma(nandc, NAND_DEV_CMD_VLD_RESTORE, 1, NAND_BAM_NEXT_SGL);
-	}
-
-	return 0;
-}
-
-/* sets up descriptors for NAND_CMD_ERASE1 */
-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);
-
-	nandc_set_reg(chip, NAND_FLASH_CMD,
-		      OP_BLOCK_ERASE | PAGE_ACC | LAST_PAGE);
-	nandc_set_reg(chip, NAND_ADDR0, page_addr);
-	nandc_set_reg(chip, NAND_ADDR1, 0);
-	nandc_set_reg(chip, NAND_DEV0_CFG0,
-		      host->cfg0_raw & ~(7 << CW_PER_PAGE));
-	nandc_set_reg(chip, NAND_DEV0_CFG1, host->cfg1_raw);
-	nandc_set_reg(chip, NAND_EXEC_CMD, 1);
-	nandc_set_reg(chip, NAND_FLASH_STATUS, host->clrflashstatus);
-	nandc_set_reg(chip, NAND_READ_STATUS, host->clrreadstatus);
-
-	write_reg_dma(nandc, NAND_FLASH_CMD, 3, NAND_BAM_NEXT_SGL);
-	write_reg_dma(nandc, NAND_DEV0_CFG0, 2, NAND_BAM_NEXT_SGL);
-	write_reg_dma(nandc, NAND_EXEC_CMD, 1, NAND_BAM_NEXT_SGL);
-
-	read_reg_dma(nandc, NAND_FLASH_STATUS, 1, NAND_BAM_NEXT_SGL);
-
-	write_reg_dma(nandc, NAND_FLASH_STATUS, 1, 0);
-	write_reg_dma(nandc, NAND_READ_STATUS, 1, NAND_BAM_NEXT_SGL);
-
-	return 0;
-}
-
-/* sets up descriptors for NAND_CMD_READID */
-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);
-
-	if (column == -1)
-		return 0;
-
-	nandc_set_reg(chip, NAND_FLASH_CMD, OP_FETCH_ID);
-	nandc_set_reg(chip, NAND_ADDR0, column);
-	nandc_set_reg(chip, NAND_ADDR1, 0);
-	nandc_set_reg(chip, NAND_FLASH_CHIP_SELECT,
-		      nandc->props->is_bam ? 0 : DM_EN);
-	nandc_set_reg(chip, NAND_EXEC_CMD, 1);
-
-	write_reg_dma(nandc, NAND_FLASH_CMD, 4, NAND_BAM_NEXT_SGL);
-	write_reg_dma(nandc, NAND_EXEC_CMD, 1, NAND_BAM_NEXT_SGL);
-
-	read_reg_dma(nandc, NAND_READ_ID, 1, NAND_BAM_NEXT_SGL);
-
-	return 0;
-}
-
-/* sets up descriptors for NAND_CMD_RESET */
-static int reset(struct qcom_nand_host *host)
-{
-	struct nand_chip *chip = &host->chip;
-	struct qcom_nand_controller *nandc = get_qcom_nand_controller(chip);
-
-	nandc_set_reg(chip, NAND_FLASH_CMD, OP_RESET_DEVICE);
-	nandc_set_reg(chip, NAND_EXEC_CMD, 1);
-
-	write_reg_dma(nandc, NAND_FLASH_CMD, 1, NAND_BAM_NEXT_SGL);
-	write_reg_dma(nandc, NAND_EXEC_CMD, 1, NAND_BAM_NEXT_SGL);
-
-	read_reg_dma(nandc, NAND_FLASH_STATUS, 1, NAND_BAM_NEXT_SGL);
-
-	return 0;
-}
-
 /* helpers to submit/free our list of dma descriptors */
 static int submit_descs(struct qcom_nand_controller *nandc)
 {
@@ -1534,150 +1385,6 @@ static void clear_read_regs(struct qcom_nand_controller *nandc)
 	nandc_read_buffer_sync(nandc, false);
 }
 
-static void pre_command(struct qcom_nand_host *host, int command)
-{
-	struct nand_chip *chip = &host->chip;
-	struct qcom_nand_controller *nandc = get_qcom_nand_controller(chip);
-
-	nandc->buf_count = 0;
-	nandc->buf_start = 0;
-	host->use_ecc = false;
-	host->last_command = command;
-
-	clear_read_regs(nandc);
-
-	clear_bam_transaction(nandc);
-}
-
-/*
- * this is called after NAND_CMD_PAGEPROG and NAND_CMD_ERASE1 to set our
- * privately maintained status byte, this status byte can be read after
- * NAND_CMD_STATUS is called
- */
-static void parse_erase_write_errors(struct qcom_nand_host *host, int command)
-{
-	struct nand_chip *chip = &host->chip;
-	struct qcom_nand_controller *nandc = get_qcom_nand_controller(chip);
-	struct nand_ecc_ctrl *ecc = &chip->ecc;
-	int num_cw;
-	int i;
-
-	num_cw = command == NAND_CMD_PAGEPROG ? ecc->steps : 1;
-	nandc_read_buffer_sync(nandc, true);
-
-	for (i = 0; i < num_cw; i++) {
-		u32 flash_status = le32_to_cpu(nandc->reg_read_buf[i]);
-
-		if (flash_status & FS_MPU_ERR)
-			host->status &= ~NAND_STATUS_WP;
-
-		if (flash_status & FS_OP_ERR || (i == (num_cw - 1) &&
-						 (flash_status &
-						  FS_DEVICE_STS_ERR)))
-			host->status |= NAND_STATUS_FAIL;
-	}
-}
-
-static void post_command(struct qcom_nand_host *host, int command)
-{
-	struct nand_chip *chip = &host->chip;
-	struct qcom_nand_controller *nandc = get_qcom_nand_controller(chip);
-
-	switch (command) {
-	case NAND_CMD_READID:
-		nandc_read_buffer_sync(nandc, true);
-		memcpy(nandc->data_buffer, nandc->reg_read_buf,
-		       nandc->buf_count);
-		break;
-	case NAND_CMD_PAGEPROG:
-	case NAND_CMD_ERASE1:
-		parse_erase_write_errors(host, command);
-		break;
-	default:
-		break;
-	}
-}
-
-/*
- * Implements chip->legacy.cmdfunc. It's  only used for a limited set of
- * commands. The rest of the commands wouldn't be called by upper layers.
- * For example, NAND_CMD_READOOB would never be called because we have our own
- * versions of read_oob ops for nand_ecc_ctrl.
- */
-static void qcom_nandc_command(struct nand_chip *chip, unsigned int command,
-			       int column, int page_addr)
-{
-	struct qcom_nand_host *host = to_qcom_nand_host(chip);
-	struct nand_ecc_ctrl *ecc = &chip->ecc;
-	struct qcom_nand_controller *nandc = get_qcom_nand_controller(chip);
-	bool wait = false;
-	int ret = 0;
-
-	pre_command(host, command);
-
-	switch (command) {
-	case NAND_CMD_RESET:
-		ret = reset(host);
-		wait = true;
-		break;
-
-	case NAND_CMD_READID:
-		nandc->buf_count = 4;
-		ret = read_id(host, column);
-		wait = true;
-		break;
-
-	case NAND_CMD_PARAM:
-		ret = nandc_param(host);
-		wait = true;
-		break;
-
-	case NAND_CMD_ERASE1:
-		ret = erase_block(host, page_addr);
-		wait = true;
-		break;
-
-	case NAND_CMD_READ0:
-		/* we read the entire page for now */
-		WARN_ON(column != 0);
-
-		host->use_ecc = true;
-		set_address(host, 0, page_addr);
-		update_rw_regs(host, ecc->steps, true, 0);
-		break;
-
-	case NAND_CMD_SEQIN:
-		WARN_ON(column != 0);
-		set_address(host, 0, page_addr);
-		break;
-
-	case NAND_CMD_PAGEPROG:
-	case NAND_CMD_STATUS:
-	case NAND_CMD_NONE:
-	default:
-		break;
-	}
-
-	if (ret) {
-		dev_err(nandc->dev, "failure executing command %d\n",
-			command);
-		free_descs(nandc);
-		return;
-	}
-
-	if (wait) {
-		ret = submit_descs(nandc);
-		if (ret)
-			dev_err(nandc->dev,
-				"failure submitting descs for command %d\n",
-				command);
-	}
-
-	free_descs(nandc);
-
-	post_command(host, command);
-}
-
 /*
  * when using BCH ECC, the HW flags an error in NAND_FLASH_STATUS if it read
  * an erased CW, and reports an erased CW in NAND_ERASED_CW_DETECT_STATUS.
@@ -2534,64 +2241,6 @@ static int qcom_nandc_block_markbad(struct nand_chip *chip, loff_t ofs)
 	return nand_prog_page_end_op(chip);
 }
 
-/*
- * the three functions below implement chip->legacy.read_byte(),
- * chip->legacy.read_buf() and chip->legacy.write_buf() respectively. these
- * aren't used for reading/writing page data, they are used for smaller data
- * like reading	id, status etc
- */
-static uint8_t qcom_nandc_read_byte(struct nand_chip *chip)
-{
-	struct qcom_nand_host *host = to_qcom_nand_host(chip);
-	struct qcom_nand_controller *nandc = get_qcom_nand_controller(chip);
-	u8 *buf = nandc->data_buffer;
-	u8 ret = 0x0;
-
-	if (host->last_command == NAND_CMD_STATUS) {
-		ret = host->status;
-
-		host->status = NAND_STATUS_READY | NAND_STATUS_WP;
-
-		return ret;
-	}
-
-	if (nandc->buf_start < nandc->buf_count)
-		ret = buf[nandc->buf_start++];
-
-	return ret;
-}
-
-static void qcom_nandc_read_buf(struct nand_chip *chip, uint8_t *buf, int len)
-{
-	struct qcom_nand_controller *nandc = get_qcom_nand_controller(chip);
-	int real_len = min_t(size_t, len, nandc->buf_count - nandc->buf_start);
-
-	memcpy(buf, nandc->data_buffer + nandc->buf_start, real_len);
-	nandc->buf_start += real_len;
-}
-
-static void qcom_nandc_write_buf(struct nand_chip *chip, const uint8_t *buf,
-				 int len)
-{
-	struct qcom_nand_controller *nandc = get_qcom_nand_controller(chip);
-	int real_len = min_t(size_t, len, nandc->buf_count - nandc->buf_start);
-
-	memcpy(nandc->data_buffer + nandc->buf_start, buf, real_len);
-
-	nandc->buf_start += real_len;
-}
-
-/* we support only one external chip for now */
-static void qcom_nandc_select_chip(struct nand_chip *chip, int chipnr)
-{
-	struct qcom_nand_controller *nandc = get_qcom_nand_controller(chip);
-
-	if (chipnr <= 0)
-		return;
-
-	dev_warn(nandc->dev, "invalid chip select\n");
-}
-
 /*
  * NAND controller page layout info
  *
@@ -3698,14 +3347,6 @@ static int qcom_nand_host_init_and_register(struct qcom_nand_controller *nandc,
 	mtd->owner = THIS_MODULE;
 	mtd->dev.parent = dev;
 
-	chip->legacy.cmdfunc	= qcom_nandc_command;
-	chip->legacy.select_chip	= qcom_nandc_select_chip;
-	chip->legacy.read_byte	= qcom_nandc_read_byte;
-	chip->legacy.read_buf	= qcom_nandc_read_buf;
-	chip->legacy.write_buf	= qcom_nandc_write_buf;
-	chip->legacy.set_features	= nand_get_set_features_notsupp;
-	chip->legacy.get_features	= nand_get_set_features_notsupp;
-
 	/*
 	 * the bad block marker is readable only when we read the last codeword
 	 * of a page with ECC disabled. currently, the nand_base and nand_bbt
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 2/5] mtd: rawnand: qcom: Add support for reset, readid, status exec_op
  2023-05-31 12:49 ` [PATCH v3 2/5] mtd: rawnand: qcom: Add support for reset, readid, status exec_op Md Sadre Alam
@ 2023-06-08 13:58   ` Miquel Raynal
  2023-06-14 14:01     ` Md Sadre Alam
  0 siblings, 1 reply; 9+ messages in thread
From: Miquel Raynal @ 2023-06-08 13:58 UTC (permalink / raw)
  To: Md Sadre Alam
  Cc: mani, richard, vigneshr, linux-mtd, linux-arm-msm, linux-kernel,
	quic_srichara

Hello,

quic_mdalam@quicinc.com wrote on Wed, 31 May 2023 18:19:50 +0530:

> This change will add exec_ops support for RESET , READ_ID, STATUS
> command.

Imperative form here and after in your commit log please.

Add support for...

Minor comments below, otherwise looks good.

> 
> Co-developed-by: Sricharan Ramabadhran <quic_srichara@quicinc.com>
> Signed-off-by: Sricharan Ramabadhran <quic_srichara@quicinc.com>
> Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
> ---
> Change in [v3]
> 
> * Updated q_op->flag in qcom_op_cmd_mapping() with OP_PROGRAM_PAGE
>   instead NAND_CMD_PAGEPROG
> 
> * Change the if condition check for exec_opwrite into single line check.
> 
> * Added error check
> 
> * Removed check for NAND_CMD_RESET, NAND_CMD_READID from pre_command.
> 
> Change in [v2]
> 
> * Missed to post Cover-letter, so posting v2 patch with cover-letter
> 
> Change in [v1]
> 
> * Added initial support for exec_ops.
>    
>  drivers/mtd/nand/raw/qcom_nandc.c | 218 +++++++++++++++++++++++++++++-
>  1 file changed, 215 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
> index 6a83cd86a12d..659ba923b68a 100644
> --- a/drivers/mtd/nand/raw/qcom_nandc.c
> +++ b/drivers/mtd/nand/raw/qcom_nandc.c
> @@ -385,6 +385,9 @@ struct nandc_regs {
>   * @reg_read_pos:		marker for data read in reg_read_buf
>   *
>   * @cmd1/vld:			some fixed controller register values
> + *
> + * @exec_opwrite:		flag to select correct number of code word
> + *				while reading status
>   */
>  struct qcom_nand_controller {
>  	struct device *dev;
> @@ -435,6 +438,7 @@ struct qcom_nand_controller {
>  	int reg_read_pos;
>  
>  	u32 cmd1, vld;
> +	bool exec_opwrite;
>  };
>  
>  /*
> @@ -1542,8 +1546,7 @@ static void pre_command(struct qcom_nand_host *host, int command)
>  
>  	clear_read_regs(nandc);
>  
> -	if (command == NAND_CMD_RESET || command == NAND_CMD_READID ||
> -	    command == NAND_CMD_PARAM || command == NAND_CMD_ERASE1)
> +	if (command == NAND_CMD_PARAM || command == NAND_CMD_ERASE1)
>  		clear_bam_transaction(nandc);
>  }
>  
> @@ -2920,6 +2923,8 @@ static int qcom_op_cmd_mapping(struct qcom_nand_controller *nandc, u8 cmd,
>  		break;
>  	case NAND_CMD_PAGEPROG:
>  		ret = OP_PROGRAM_PAGE;
> +		q_op->flag = OP_PROGRAM_PAGE;
> +		nandc->exec_opwrite = true;
>  		break;
>  	default:
>  		break;
> @@ -2982,6 +2987,212 @@ static void qcom_parse_instructions(struct nand_chip *chip,
>  	}
>  }
>  
> +static void qcom_delay_ns(unsigned int ns)
> +{
> +	if (!ns)
> +		return;
> +
> +	if (ns < 10000)
> +		ndelay(ns);
> +	else
> +		udelay(DIV_ROUND_UP(ns, 1000));
> +}
> +
> +static int qcom_wait_rdy_poll(struct nand_chip *chip, unsigned int time_ms)
> +{
> +	struct qcom_nand_controller *nandc = get_qcom_nand_controller(chip);
> +	unsigned long start = jiffies + msecs_to_jiffies(time_ms);
> +	u32 flash;
> +
> +	nandc_read_buffer_sync(nandc, true);
> +
> +	do {
> +		flash = le32_to_cpu(nandc->reg_read_buf[0]);
> +		if (flash & FS_READY_BSY_N)
> +			return 0;
> +		cpu_relax();
> +	} while (time_after(start, jiffies));
> +
> +	dev_err(nandc->dev, "Timeout waiting for device to be ready:0x%08x\n", flash);
> +
> +	return -ETIMEDOUT;
> +}
> +
> +static int qcom_read_status_exec(struct nand_chip *chip,
> +				 const struct nand_subop *subop)
> +{
> +	struct qcom_nand_host *host = to_qcom_nand_host(chip);
> +	struct qcom_nand_controller *nandc = get_qcom_nand_controller(chip);
> +	struct nand_ecc_ctrl *ecc = &chip->ecc;
> +	struct qcom_op q_op;
> +	const struct nand_op_instr *instr = NULL;
> +	unsigned int op_id = 0;
> +	unsigned int len = 0;
> +	int ret = 0, num_cw, i;
> +	u32 flash_status;
> +
> +	host->status = NAND_STATUS_READY | NAND_STATUS_WP;
> +
> +	qcom_parse_instructions(chip, subop, &q_op);
> +
> +	num_cw = nandc->exec_opwrite ? ecc->steps : 1;
> +	nandc->exec_opwrite = false;
> +
> +	nandc->buf_count = 0;
> +	nandc->buf_start = 0;
> +	host->use_ecc = false;
> +
> +	clear_read_regs(nandc);
> +	clear_bam_transaction(nandc);
> +
> +	nandc_set_reg(chip, NAND_FLASH_CMD, q_op.cmd_reg);
> +	nandc_set_reg(chip, NAND_EXEC_CMD, 1);
> +
> +	write_reg_dma(nandc, NAND_FLASH_CMD, 1, NAND_BAM_NEXT_SGL);
> +	write_reg_dma(nandc, NAND_EXEC_CMD, 1, NAND_BAM_NEXT_SGL);
> +	read_reg_dma(nandc, NAND_FLASH_STATUS, 1, NAND_BAM_NEXT_SGL);
> +
> +	ret = submit_descs(nandc);
> +	if (ret) {
> +		dev_err(nandc->dev, "failure in sbumitting status descriptor\n");

                                                 ^

Please run checkpatch.pl --strict

> +		free_descs(nandc);
> +		goto err_out;
> +	}
> +	free_descs(nandc);
> +
> +	nandc_read_buffer_sync(nandc, true);
> +
> +	for (i = 0; i < num_cw; i++) {
> +		flash_status = le32_to_cpu(nandc->reg_read_buf[i]);
> +
> +	if (flash_status & FS_MPU_ERR)
> +		host->status &= ~NAND_STATUS_WP;
> +
> +	if (flash_status & FS_OP_ERR || (i == (num_cw - 1) &&
> +					 (flash_status & FS_DEVICE_STS_ERR)))

  	if (flash_status & FS_OP_ERR ||
	    (i == (num_cw - 1) && (flash_status & FS_DEVICE_STS_ERR)))

looks more readable?

> +		host->status |= NAND_STATUS_FAIL;
> +	}
> +
> +	flash_status = host->status;
> +	instr = q_op.data_instr;
> +	op_id = q_op.data_instr_idx;
> +	len = nand_subop_get_data_len(subop, op_id);
> +	memcpy(instr->ctx.data.buf.in, &flash_status, len);
> +
> +err_out:
> +	return ret;
> +}
> +
> +static int qcom_read_id_type_exec(struct nand_chip *chip, const struct nand_subop *subop)
> +{
> +	struct qcom_nand_controller *nandc = get_qcom_nand_controller(chip);
> +	struct qcom_nand_host *host = to_qcom_nand_host(chip);
> +	struct qcom_op q_op;
> +	const struct nand_op_instr *instr = NULL;
> +	unsigned int op_id = 0;
> +	unsigned int len = 0;
> +	int ret = 0;
> +
> +	qcom_parse_instructions(chip, subop, &q_op);
> +
> +	nandc->buf_count = 0;
> +	nandc->buf_start = 0;
> +	host->use_ecc = false;
> +
> +	clear_read_regs(nandc);
> +	clear_bam_transaction(nandc);
> +
> +	nandc_set_reg(chip, NAND_FLASH_CMD, q_op.cmd_reg);
> +	nandc_set_reg(chip, NAND_ADDR0, q_op.addr1_reg);
> +	nandc_set_reg(chip, NAND_ADDR1, q_op.addr2_reg);
> +	nandc_set_reg(chip, NAND_FLASH_CHIP_SELECT,
> +		      nandc->props->is_bam ? 0 : DM_EN);
> +
> +	nandc_set_reg(chip, NAND_EXEC_CMD, 1);
> +
> +	write_reg_dma(nandc, NAND_FLASH_CMD, 4, NAND_BAM_NEXT_SGL);
> +	write_reg_dma(nandc, NAND_EXEC_CMD, 1, NAND_BAM_NEXT_SGL);
> +
> +	read_reg_dma(nandc, NAND_READ_ID, 1, NAND_BAM_NEXT_SGL);
> +
> +	ret = submit_descs(nandc);
> +	if (ret) {
> +		dev_err(nandc->dev, "failure in sbumitting read id descriptor\n");
> +		free_descs(nandc);
> +		goto err_out;
> +	}
> +	free_descs(nandc);
> +
> +	instr = q_op.data_instr;
> +	op_id = q_op.data_instr_idx;
> +	len = nand_subop_get_data_len(subop, op_id);
> +
> +	nandc_read_buffer_sync(nandc, true);
> +	memcpy(instr->ctx.data.buf.in, nandc->reg_read_buf, len);
> +
> +err_out:
> +	return ret;
> +}
> +
> +static int qcom_misc_cmd_type_exec(struct nand_chip *chip, const struct nand_subop *subop)
> +{
> +	struct qcom_nand_controller *nandc = get_qcom_nand_controller(chip);
> +	struct qcom_nand_host *host = to_qcom_nand_host(chip);
> +	struct qcom_op q_op;
> +	int ret = 0;
> +
> +	qcom_parse_instructions(chip, subop, &q_op);
> +
> +	if (q_op.flag == OP_PROGRAM_PAGE)
> +		goto wait_rdy;
> +
> +	nandc->buf_count = 0;
> +	nandc->buf_start = 0;
> +	host->use_ecc = false;
> +
> +	clear_read_regs(nandc);
> +	clear_bam_transaction(nandc);
> +
> +	nandc_set_reg(chip, NAND_FLASH_CMD, q_op.cmd_reg);
> +	nandc_set_reg(chip, NAND_EXEC_CMD, 1);
> +
> +	write_reg_dma(nandc, NAND_FLASH_CMD, 1, NAND_BAM_NEXT_SGL);
> +	write_reg_dma(nandc, NAND_EXEC_CMD, 1, NAND_BAM_NEXT_SGL);
> +
> +	read_reg_dma(nandc, NAND_FLASH_STATUS, 1, NAND_BAM_NEXT_SGL);
> +
> +	ret = submit_descs(nandc);
> +	if (ret) {
> +		dev_err(nandc->dev, "failure in sbumitting misc descriptor\n");
> +		free_descs(nandc);
> +		goto err_out;
> +	}
> +	free_descs(nandc);
> +
> +wait_rdy:
> +	qcom_delay_ns(q_op.rdy_delay_ns);
> +	ret = qcom_wait_rdy_poll(chip, q_op.rdy_timeout_ms);
> +
> +err_out:
> +	return ret;
> +}
> +
> +static const struct nand_op_parser qcom_op_parser = NAND_OP_PARSER(
> +		NAND_OP_PARSER_PATTERN(
> +			qcom_misc_cmd_type_exec,
> +			NAND_OP_PARSER_PAT_CMD_ELEM(false),
> +			NAND_OP_PARSER_PAT_WAITRDY_ELEM(false)),
> +		NAND_OP_PARSER_PATTERN(
> +			qcom_read_id_type_exec,
> +			NAND_OP_PARSER_PAT_CMD_ELEM(false),
> +			NAND_OP_PARSER_PAT_ADDR_ELEM(false, MAX_ADDRESS_CYCLE),
> +			NAND_OP_PARSER_PAT_DATA_IN_ELEM(false, 8)),
> +		NAND_OP_PARSER_PATTERN(
> +			qcom_read_status_exec,
> +			NAND_OP_PARSER_PAT_CMD_ELEM(false),
> +			NAND_OP_PARSER_PAT_DATA_IN_ELEM(false, 1)),
> +		);
> +
>  static int qcom_check_op(struct nand_chip *chip,
>  			const struct nand_operation *op)
>  {
> @@ -3022,7 +3233,8 @@ static int qcom_nand_exec_op(struct nand_chip *chip,
>  	if (check_only)
>  		return qcom_check_op(chip, op);
>  
> -	return 0;
> +	return nand_op_parser_exec_op(chip, &qcom_op_parser,
> +			op, check_only);
>  }
>  
>  static const struct nand_controller_ops qcom_nandc_ops = {


Thanks,
Miquèl

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 4/5] mtd: rawnand: qcom: Add support for read, write, erase exec_ops
  2023-05-31 12:49 ` [PATCH v3 4/5] mtd: rawnand: qcom: Add support for read, write, erase exec_ops Md Sadre Alam
@ 2023-06-08 14:03   ` Miquel Raynal
  2023-06-14 15:19     ` Md Sadre Alam
  0 siblings, 1 reply; 9+ messages in thread
From: Miquel Raynal @ 2023-06-08 14:03 UTC (permalink / raw)
  To: Md Sadre Alam
  Cc: mani, richard, vigneshr, linux-mtd, linux-arm-msm, linux-kernel,
	quic_srichara

Hi Md,

quic_mdalam@quicinc.com wrote on Wed, 31 May 2023 18:19:52 +0530:

> This change will add exec_ops support for READ, WRITE, and ERASE
> command.
> 
> Co-developed-by: Sricharan Ramabadhran <quic_srichara@quicinc.com>
> Signed-off-by: Sricharan Ramabadhran <quic_srichara@quicinc.com>
> Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
> ---
> Change in [v3]
> 
> * Removed chip->cont_read.ongoing flag.
> 
> * Removed pre_command from erase_etype_exec_ops.
> 
> Change in [v2]
> 
> * Missed to post Cover-letter, so posting v2 patch with cover-letter.
> 
> Change in [v1]
> 
> * Added initial support for exec_ops.
> 
>  drivers/mtd/nand/raw/qcom_nandc.c | 97 +++++++++++++++++++++++++++++--
>  1 file changed, 93 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
> index b4823b72fe73..7dc769f9e797 100644
> --- a/drivers/mtd/nand/raw/qcom_nandc.c
> +++ b/drivers/mtd/nand/raw/qcom_nandc.c
> @@ -1546,8 +1546,7 @@ static void pre_command(struct qcom_nand_host *host, int command)
>  
>  	clear_read_regs(nandc);
>  
> -	if (command == NAND_CMD_ERASE1)
> -		clear_bam_transaction(nandc);
> +	clear_bam_transaction(nandc);
>  }
>  
>  /*
> @@ -1764,7 +1763,7 @@ qcom_nandc_read_cw_raw(struct mtd_info *mtd, struct nand_chip *chip,
>  	int ret, reg_off = FLASH_BUF_ACC, read_loc = 0;
>  	int raw_cw = cw;
>  
> -	nand_read_page_op(chip, page, 0, NULL, 0);
> +	nand_read_page_op(chip, page, 0, data_buf, mtd->writesize);
>  	host->use_ecc = false;
>  
>  	if (nandc->props->qpic_v2)
> @@ -2181,14 +2180,23 @@ static void qcom_nandc_codeword_fixup(struct qcom_nand_host *host, int page)
>  static int qcom_nandc_read_page(struct nand_chip *chip, uint8_t *buf,
>  				int oob_required, int page)
>  {
> +	struct mtd_info *mtd = nand_to_mtd(chip);
>  	struct qcom_nand_host *host = to_qcom_nand_host(chip);
>  	struct qcom_nand_controller *nandc = get_qcom_nand_controller(chip);
> +	struct nand_ecc_ctrl *ecc = &chip->ecc;
>  	u8 *data_buf, *oob_buf = NULL;
>  
>  	if (host->nr_boot_partitions)
>  		qcom_nandc_codeword_fixup(host, page);
>  
> -	nand_read_page_op(chip, page, 0, NULL, 0);
> +	nand_read_page_op(chip, page, 0, buf, mtd->writesize);
> +	nandc->buf_count = 0;
> +	nandc->buf_start = 0;
> +	host->use_ecc = true;
> +	clear_read_regs(nandc);
> +	set_address(host, 0, page);
> +	update_rw_regs(host, ecc->steps, true, 0);
> +
>  	data_buf = buf;
>  	oob_buf = oob_required ? chip->oob_poi : NULL;
>  
> @@ -2258,6 +2266,9 @@ static int qcom_nandc_write_page(struct nand_chip *chip, const uint8_t *buf,
>  
>  	nand_prog_page_begin_op(chip, page, 0, NULL, 0);
>  
> +	set_address(host, 0, page);
> +	nandc->buf_count = 0;
> +	nandc->buf_start = 0;
>  	clear_read_regs(nandc);
>  	clear_bam_transaction(nandc);
>  
> @@ -3274,6 +3285,67 @@ static int qcom_param_page_type_exec(struct nand_chip *chip,  const struct nand_
>  	return ret;
>  }
>  
> +static int qcom_erase_cmd_type_exec(struct nand_chip *chip, const struct nand_subop *subop)
> +{
> +	struct qcom_nand_host *host = to_qcom_nand_host(chip);
> +	struct qcom_nand_controller *nandc = get_qcom_nand_controller(chip);
> +	struct qcom_op q_op;
> +	int ret = 0;
> +
> +	qcom_parse_instructions(chip, subop, &q_op);
> +
> +	q_op.cmd_reg |= PAGE_ACC | LAST_PAGE;
> +
> +	nandc->buf_count = 0;
> +	nandc->buf_start = 0;
> +	host->use_ecc = false;
> +	clear_read_regs(nandc);
> +	clear_bam_transaction(nandc);
> +
> +	nandc_set_reg(chip, NAND_FLASH_CMD, q_op.cmd_reg);
> +	nandc_set_reg(chip, NAND_ADDR0, q_op.addr1_reg);
> +	nandc_set_reg(chip, NAND_ADDR1, q_op.addr2_reg);
> +	nandc_set_reg(chip, NAND_DEV0_CFG0,
> +		      host->cfg0_raw & ~(7 << CW_PER_PAGE));
> +	nandc_set_reg(chip, NAND_DEV0_CFG1, host->cfg1_raw);
> +	nandc_set_reg(chip, NAND_EXEC_CMD, 1);
> +
> +	write_reg_dma(nandc, NAND_FLASH_CMD, 3, NAND_BAM_NEXT_SGL);
> +	write_reg_dma(nandc, NAND_DEV0_CFG0, 2, NAND_BAM_NEXT_SGL);
> +	write_reg_dma(nandc, NAND_EXEC_CMD, 1, NAND_BAM_NEXT_SGL);
> +
> +	ret = submit_descs(nandc);
> +	if (ret) {
> +		dev_err(nandc->dev, "failure in sbumitting erase descriptor\n");

                                                :)

> +		free_descs(nandc);
> +		goto err_out;
> +	}
> +	free_descs(nandc);
> +
> +	ret = qcom_wait_rdy_poll(chip, q_op.rdy_timeout_ms);
> +	if (ret)
> +		goto err_out;
> +
> +err_out:
> +	return ret;
> +}
> +
> +static int qcom_data_read_type_exec(struct nand_chip *chip, const struct nand_subop *subop)
> +{
> +	/* currently read_exec_op() return 0 , and all the read operation handle in
> +	 * actual API itself
> +	 */
> +	return 0;

Mmmh, I don't think this is gonna work. I don't understand what you're
doing here. What is "actual API itself"? What is "read_exec_op"? I
doubt I am going to like what all this means. Please don't make any
assumptions on what could come next. The core asks you to do something,
just do it. If you can't then the parsing will fail. If the core has a
fallback it's fine. If the core does not, we can discuss it. But please
don't do any guesses like that, this is *exactly* why we introduced
exec_op in the first place: you have access to the whole operation, so
please handle it correctly.

> +}
> +
> +static int qcom_data_write_type_exec(struct nand_chip *chip, const struct nand_subop *subop)
> +{
> +	/* currently write_exec_op() return 0, and all the write operation handle in
> +	 * actual API itself
> +	 */
> +	return 0;
> +}
> +
>  static const struct nand_op_parser qcom_op_parser = NAND_OP_PARSER(
>  		NAND_OP_PARSER_PATTERN(
>  			qcom_misc_cmd_type_exec,
> @@ -3294,6 +3366,23 @@ static const struct nand_op_parser qcom_op_parser = NAND_OP_PARSER(
>  			NAND_OP_PARSER_PAT_ADDR_ELEM(false, MAX_ADDRESS_CYCLE),
>  			NAND_OP_PARSER_PAT_WAITRDY_ELEM(true),
>  			NAND_OP_PARSER_PAT_DATA_IN_ELEM(false, 512)),
> +		NAND_OP_PARSER_PATTERN(
> +			qcom_erase_cmd_type_exec,
> +			NAND_OP_PARSER_PAT_CMD_ELEM(false),
> +			NAND_OP_PARSER_PAT_ADDR_ELEM(false, MAX_ADDRESS_CYCLE),
> +			NAND_OP_PARSER_PAT_CMD_ELEM(false),
> +			NAND_OP_PARSER_PAT_WAITRDY_ELEM(false)),
> +		NAND_OP_PARSER_PATTERN(
> +			qcom_data_read_type_exec,
> +			NAND_OP_PARSER_PAT_CMD_ELEM(false),
> +			NAND_OP_PARSER_PAT_ADDR_ELEM(false, MAX_ADDRESS_CYCLE),
> +			NAND_OP_PARSER_PAT_CMD_ELEM(false),
> +			NAND_OP_PARSER_PAT_WAITRDY_ELEM(true),
> +			NAND_OP_PARSER_PAT_DATA_IN_ELEM(false, 2048)),
> +		NAND_OP_PARSER_PATTERN(
> +			qcom_data_write_type_exec,
> +			NAND_OP_PARSER_PAT_CMD_ELEM(true),
> +			NAND_OP_PARSER_PAT_ADDR_ELEM(true, MAX_ADDRESS_CYCLE)),
>  		);
>  
>  static int qcom_check_op(struct nand_chip *chip,


Thanks,
Miquèl

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 2/5] mtd: rawnand: qcom: Add support for reset, readid, status exec_op
  2023-06-08 13:58   ` Miquel Raynal
@ 2023-06-14 14:01     ` Md Sadre Alam
  0 siblings, 0 replies; 9+ messages in thread
From: Md Sadre Alam @ 2023-06-14 14:01 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: mani, richard, vigneshr, linux-mtd, linux-arm-msm, linux-kernel,
	quic_srichara



On 6/8/2023 7:28 PM, Miquel Raynal wrote:
> Hello,
> 
> quic_mdalam@quicinc.com wrote on Wed, 31 May 2023 18:19:50 +0530:
> 
>> This change will add exec_ops support for RESET , READ_ID, STATUS
>> command.
> 
> Imperative form here and after in your commit log please.
> 
> Add support for...
> 
> Minor comments below, otherwise looks good.

   Thanks, I will update commit message in all the patches in next
   patch V4.

> 
>>
>> Co-developed-by: Sricharan Ramabadhran <quic_srichara@quicinc.com>
>> Signed-off-by: Sricharan Ramabadhran <quic_srichara@quicinc.com>
>> Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
>> ---
>> Change in [v3]
>>
>> * Updated q_op->flag in qcom_op_cmd_mapping() with OP_PROGRAM_PAGE
>>    instead NAND_CMD_PAGEPROG
>>
>> * Change the if condition check for exec_opwrite into single line check.
>>
>> * Added error check
>>
>> * Removed check for NAND_CMD_RESET, NAND_CMD_READID from pre_command.
>>
>> Change in [v2]
>>
>> * Missed to post Cover-letter, so posting v2 patch with cover-letter
>>
>> Change in [v1]
>>
>> * Added initial support for exec_ops.
>>     
>>   drivers/mtd/nand/raw/qcom_nandc.c | 218 +++++++++++++++++++++++++++++-
>>   1 file changed, 215 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
>> index 6a83cd86a12d..659ba923b68a 100644
>> --- a/drivers/mtd/nand/raw/qcom_nandc.c
>> +++ b/drivers/mtd/nand/raw/qcom_nandc.c
>> @@ -385,6 +385,9 @@ struct nandc_regs {
>>    * @reg_read_pos:		marker for data read in reg_read_buf
>>    *
>>    * @cmd1/vld:			some fixed controller register values
>> + *
>> + * @exec_opwrite:		flag to select correct number of code word
>> + *				while reading status
>>    */
>>   struct qcom_nand_controller {
>>   	struct device *dev;
>> @@ -435,6 +438,7 @@ struct qcom_nand_controller {
>>   	int reg_read_pos;
>>   
>>   	u32 cmd1, vld;
>> +	bool exec_opwrite;
>>   };
>>   
>>   /*
>> @@ -1542,8 +1546,7 @@ static void pre_command(struct qcom_nand_host *host, int command)
>>   
>>   	clear_read_regs(nandc);
>>   
>> -	if (command == NAND_CMD_RESET || command == NAND_CMD_READID ||
>> -	    command == NAND_CMD_PARAM || command == NAND_CMD_ERASE1)
>> +	if (command == NAND_CMD_PARAM || command == NAND_CMD_ERASE1)
>>   		clear_bam_transaction(nandc);
>>   }
>>   
>> @@ -2920,6 +2923,8 @@ static int qcom_op_cmd_mapping(struct qcom_nand_controller *nandc, u8 cmd,
>>   		break;
>>   	case NAND_CMD_PAGEPROG:
>>   		ret = OP_PROGRAM_PAGE;
>> +		q_op->flag = OP_PROGRAM_PAGE;
>> +		nandc->exec_opwrite = true;
>>   		break;
>>   	default:
>>   		break;
>> @@ -2982,6 +2987,212 @@ static void qcom_parse_instructions(struct nand_chip *chip,
>>   	}
>>   }
>>   
>> +static void qcom_delay_ns(unsigned int ns)
>> +{
>> +	if (!ns)
>> +		return;
>> +
>> +	if (ns < 10000)
>> +		ndelay(ns);
>> +	else
>> +		udelay(DIV_ROUND_UP(ns, 1000));
>> +}
>> +
>> +static int qcom_wait_rdy_poll(struct nand_chip *chip, unsigned int time_ms)
>> +{
>> +	struct qcom_nand_controller *nandc = get_qcom_nand_controller(chip);
>> +	unsigned long start = jiffies + msecs_to_jiffies(time_ms);
>> +	u32 flash;
>> +
>> +	nandc_read_buffer_sync(nandc, true);
>> +
>> +	do {
>> +		flash = le32_to_cpu(nandc->reg_read_buf[0]);
>> +		if (flash & FS_READY_BSY_N)
>> +			return 0;
>> +		cpu_relax();
>> +	} while (time_after(start, jiffies));
>> +
>> +	dev_err(nandc->dev, "Timeout waiting for device to be ready:0x%08x\n", flash);
>> +
>> +	return -ETIMEDOUT;
>> +}
>> +
>> +static int qcom_read_status_exec(struct nand_chip *chip,
>> +				 const struct nand_subop *subop)
>> +{
>> +	struct qcom_nand_host *host = to_qcom_nand_host(chip);
>> +	struct qcom_nand_controller *nandc = get_qcom_nand_controller(chip);
>> +	struct nand_ecc_ctrl *ecc = &chip->ecc;
>> +	struct qcom_op q_op;
>> +	const struct nand_op_instr *instr = NULL;
>> +	unsigned int op_id = 0;
>> +	unsigned int len = 0;
>> +	int ret = 0, num_cw, i;
>> +	u32 flash_status;
>> +
>> +	host->status = NAND_STATUS_READY | NAND_STATUS_WP;
>> +
>> +	qcom_parse_instructions(chip, subop, &q_op);
>> +
>> +	num_cw = nandc->exec_opwrite ? ecc->steps : 1;
>> +	nandc->exec_opwrite = false;
>> +
>> +	nandc->buf_count = 0;
>> +	nandc->buf_start = 0;
>> +	host->use_ecc = false;
>> +
>> +	clear_read_regs(nandc);
>> +	clear_bam_transaction(nandc);
>> +
>> +	nandc_set_reg(chip, NAND_FLASH_CMD, q_op.cmd_reg);
>> +	nandc_set_reg(chip, NAND_EXEC_CMD, 1);
>> +
>> +	write_reg_dma(nandc, NAND_FLASH_CMD, 1, NAND_BAM_NEXT_SGL);
>> +	write_reg_dma(nandc, NAND_EXEC_CMD, 1, NAND_BAM_NEXT_SGL);
>> +	read_reg_dma(nandc, NAND_FLASH_STATUS, 1, NAND_BAM_NEXT_SGL);
>> +
>> +	ret = submit_descs(nandc);
>> +	if (ret) {
>> +		dev_err(nandc->dev, "failure in sbumitting status descriptor\n");
> 
>                                                   ^
> 
> Please run checkpatch.pl --strict

   Thanks for pointing this, its spelling mistake. I will fix this in next patch V4.
> 
>> +		free_descs(nandc);
>> +		goto err_out;
>> +	}
>> +	free_descs(nandc);
>> +
>> +	nandc_read_buffer_sync(nandc, true);
>> +
>> +	for (i = 0; i < num_cw; i++) {
>> +		flash_status = le32_to_cpu(nandc->reg_read_buf[i]);
>> +
>> +	if (flash_status & FS_MPU_ERR)
>> +		host->status &= ~NAND_STATUS_WP;
>> +
>> +	if (flash_status & FS_OP_ERR || (i == (num_cw - 1) &&
>> +					 (flash_status & FS_DEVICE_STS_ERR)))
> 
>    	if (flash_status & FS_OP_ERR ||
> 	    (i == (num_cw - 1) && (flash_status & FS_DEVICE_STS_ERR)))
> 
> looks more readable?

   Yes you are right. I will fix this in next patch V4.

> 
>> +		host->status |= NAND_STATUS_FAIL;
>> +	}
>> +
>> +	flash_status = host->status;
>> +	instr = q_op.data_instr;
>> +	op_id = q_op.data_instr_idx;
>> +	len = nand_subop_get_data_len(subop, op_id);
>> +	memcpy(instr->ctx.data.buf.in, &flash_status, len);
>> +
>> +err_out:
>> +	return ret;
>> +}
>> +
>> +static int qcom_read_id_type_exec(struct nand_chip *chip, const struct nand_subop *subop)
>> +{
>> +	struct qcom_nand_controller *nandc = get_qcom_nand_controller(chip);
>> +	struct qcom_nand_host *host = to_qcom_nand_host(chip);
>> +	struct qcom_op q_op;
>> +	const struct nand_op_instr *instr = NULL;
>> +	unsigned int op_id = 0;
>> +	unsigned int len = 0;
>> +	int ret = 0;
>> +
>> +	qcom_parse_instructions(chip, subop, &q_op);
>> +
>> +	nandc->buf_count = 0;
>> +	nandc->buf_start = 0;
>> +	host->use_ecc = false;
>> +
>> +	clear_read_regs(nandc);
>> +	clear_bam_transaction(nandc);
>> +
>> +	nandc_set_reg(chip, NAND_FLASH_CMD, q_op.cmd_reg);
>> +	nandc_set_reg(chip, NAND_ADDR0, q_op.addr1_reg);
>> +	nandc_set_reg(chip, NAND_ADDR1, q_op.addr2_reg);
>> +	nandc_set_reg(chip, NAND_FLASH_CHIP_SELECT,
>> +		      nandc->props->is_bam ? 0 : DM_EN);
>> +
>> +	nandc_set_reg(chip, NAND_EXEC_CMD, 1);
>> +
>> +	write_reg_dma(nandc, NAND_FLASH_CMD, 4, NAND_BAM_NEXT_SGL);
>> +	write_reg_dma(nandc, NAND_EXEC_CMD, 1, NAND_BAM_NEXT_SGL);
>> +
>> +	read_reg_dma(nandc, NAND_READ_ID, 1, NAND_BAM_NEXT_SGL);
>> +
>> +	ret = submit_descs(nandc);
>> +	if (ret) {
>> +		dev_err(nandc->dev, "failure in sbumitting read id descriptor\n");
>> +		free_descs(nandc);
>> +		goto err_out;
>> +	}
>> +	free_descs(nandc);
>> +
>> +	instr = q_op.data_instr;
>> +	op_id = q_op.data_instr_idx;
>> +	len = nand_subop_get_data_len(subop, op_id);
>> +
>> +	nandc_read_buffer_sync(nandc, true);
>> +	memcpy(instr->ctx.data.buf.in, nandc->reg_read_buf, len);
>> +
>> +err_out:
>> +	return ret;
>> +}
>> +
>> +static int qcom_misc_cmd_type_exec(struct nand_chip *chip, const struct nand_subop *subop)
>> +{
>> +	struct qcom_nand_controller *nandc = get_qcom_nand_controller(chip);
>> +	struct qcom_nand_host *host = to_qcom_nand_host(chip);
>> +	struct qcom_op q_op;
>> +	int ret = 0;
>> +
>> +	qcom_parse_instructions(chip, subop, &q_op);
>> +
>> +	if (q_op.flag == OP_PROGRAM_PAGE)
>> +		goto wait_rdy;
>> +
>> +	nandc->buf_count = 0;
>> +	nandc->buf_start = 0;
>> +	host->use_ecc = false;
>> +
>> +	clear_read_regs(nandc);
>> +	clear_bam_transaction(nandc);
>> +
>> +	nandc_set_reg(chip, NAND_FLASH_CMD, q_op.cmd_reg);
>> +	nandc_set_reg(chip, NAND_EXEC_CMD, 1);
>> +
>> +	write_reg_dma(nandc, NAND_FLASH_CMD, 1, NAND_BAM_NEXT_SGL);
>> +	write_reg_dma(nandc, NAND_EXEC_CMD, 1, NAND_BAM_NEXT_SGL);
>> +
>> +	read_reg_dma(nandc, NAND_FLASH_STATUS, 1, NAND_BAM_NEXT_SGL);
>> +
>> +	ret = submit_descs(nandc);
>> +	if (ret) {
>> +		dev_err(nandc->dev, "failure in sbumitting misc descriptor\n");
>> +		free_descs(nandc);
>> +		goto err_out;
>> +	}
>> +	free_descs(nandc);
>> +
>> +wait_rdy:
>> +	qcom_delay_ns(q_op.rdy_delay_ns);
>> +	ret = qcom_wait_rdy_poll(chip, q_op.rdy_timeout_ms);
>> +
>> +err_out:
>> +	return ret;
>> +}
>> +
>> +static const struct nand_op_parser qcom_op_parser = NAND_OP_PARSER(
>> +		NAND_OP_PARSER_PATTERN(
>> +			qcom_misc_cmd_type_exec,
>> +			NAND_OP_PARSER_PAT_CMD_ELEM(false),
>> +			NAND_OP_PARSER_PAT_WAITRDY_ELEM(false)),
>> +		NAND_OP_PARSER_PATTERN(
>> +			qcom_read_id_type_exec,
>> +			NAND_OP_PARSER_PAT_CMD_ELEM(false),
>> +			NAND_OP_PARSER_PAT_ADDR_ELEM(false, MAX_ADDRESS_CYCLE),
>> +			NAND_OP_PARSER_PAT_DATA_IN_ELEM(false, 8)),
>> +		NAND_OP_PARSER_PATTERN(
>> +			qcom_read_status_exec,
>> +			NAND_OP_PARSER_PAT_CMD_ELEM(false),
>> +			NAND_OP_PARSER_PAT_DATA_IN_ELEM(false, 1)),
>> +		);
>> +
>>   static int qcom_check_op(struct nand_chip *chip,
>>   			const struct nand_operation *op)
>>   {
>> @@ -3022,7 +3233,8 @@ static int qcom_nand_exec_op(struct nand_chip *chip,
>>   	if (check_only)
>>   		return qcom_check_op(chip, op);
>>   
>> -	return 0;
>> +	return nand_op_parser_exec_op(chip, &qcom_op_parser,
>> +			op, check_only);
>>   }
>>   
>>   static const struct nand_controller_ops qcom_nandc_ops = {
> 
> 
> Thanks,
> Miquèl

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 4/5] mtd: rawnand: qcom: Add support for read, write, erase exec_ops
  2023-06-08 14:03   ` Miquel Raynal
@ 2023-06-14 15:19     ` Md Sadre Alam
  0 siblings, 0 replies; 9+ messages in thread
From: Md Sadre Alam @ 2023-06-14 15:19 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: mani, richard, vigneshr, linux-mtd, linux-arm-msm, linux-kernel,
	quic_srichara



On 6/8/2023 7:33 PM, Miquel Raynal wrote:
> Hi Md,
> 
> quic_mdalam@quicinc.com wrote on Wed, 31 May 2023 18:19:52 +0530:
> 
>> This change will add exec_ops support for READ, WRITE, and ERASE
>> command.
>>
>> Co-developed-by: Sricharan Ramabadhran <quic_srichara@quicinc.com>
>> Signed-off-by: Sricharan Ramabadhran <quic_srichara@quicinc.com>
>> Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
>> ---
>> Change in [v3]
>>
>> * Removed chip->cont_read.ongoing flag.
>>
>> * Removed pre_command from erase_etype_exec_ops.
>>
>> Change in [v2]
>>
>> * Missed to post Cover-letter, so posting v2 patch with cover-letter.
>>
>> Change in [v1]
>>
>> * Added initial support for exec_ops.
>>
>>   drivers/mtd/nand/raw/qcom_nandc.c | 97 +++++++++++++++++++++++++++++--
>>   1 file changed, 93 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
>> index b4823b72fe73..7dc769f9e797 100644
>> --- a/drivers/mtd/nand/raw/qcom_nandc.c
>> +++ b/drivers/mtd/nand/raw/qcom_nandc.c
>> @@ -1546,8 +1546,7 @@ static void pre_command(struct qcom_nand_host *host, int command)
>>   
>>   	clear_read_regs(nandc);
>>   
>> -	if (command == NAND_CMD_ERASE1)
>> -		clear_bam_transaction(nandc);
>> +	clear_bam_transaction(nandc);
>>   }
>>   
>>   /*
>> @@ -1764,7 +1763,7 @@ qcom_nandc_read_cw_raw(struct mtd_info *mtd, struct nand_chip *chip,
>>   	int ret, reg_off = FLASH_BUF_ACC, read_loc = 0;
>>   	int raw_cw = cw;
>>   
>> -	nand_read_page_op(chip, page, 0, NULL, 0);
>> +	nand_read_page_op(chip, page, 0, data_buf, mtd->writesize);
>>   	host->use_ecc = false;
>>   
>>   	if (nandc->props->qpic_v2)
>> @@ -2181,14 +2180,23 @@ static void qcom_nandc_codeword_fixup(struct qcom_nand_host *host, int page)
>>   static int qcom_nandc_read_page(struct nand_chip *chip, uint8_t *buf,
>>   				int oob_required, int page)
>>   {
>> +	struct mtd_info *mtd = nand_to_mtd(chip);
>>   	struct qcom_nand_host *host = to_qcom_nand_host(chip);
>>   	struct qcom_nand_controller *nandc = get_qcom_nand_controller(chip);
>> +	struct nand_ecc_ctrl *ecc = &chip->ecc;
>>   	u8 *data_buf, *oob_buf = NULL;
>>   
>>   	if (host->nr_boot_partitions)
>>   		qcom_nandc_codeword_fixup(host, page);
>>   
>> -	nand_read_page_op(chip, page, 0, NULL, 0);
>> +	nand_read_page_op(chip, page, 0, buf, mtd->writesize);
>> +	nandc->buf_count = 0;
>> +	nandc->buf_start = 0;
>> +	host->use_ecc = true;
>> +	clear_read_regs(nandc);
>> +	set_address(host, 0, page);
>> +	update_rw_regs(host, ecc->steps, true, 0);
>> +
>>   	data_buf = buf;
>>   	oob_buf = oob_required ? chip->oob_poi : NULL;
>>   
>> @@ -2258,6 +2266,9 @@ static int qcom_nandc_write_page(struct nand_chip *chip, const uint8_t *buf,
>>   
>>   	nand_prog_page_begin_op(chip, page, 0, NULL, 0);
>>   
>> +	set_address(host, 0, page);
>> +	nandc->buf_count = 0;
>> +	nandc->buf_start = 0;
>>   	clear_read_regs(nandc);
>>   	clear_bam_transaction(nandc);
>>   
>> @@ -3274,6 +3285,67 @@ static int qcom_param_page_type_exec(struct nand_chip *chip,  const struct nand_
>>   	return ret;
>>   }
>>   
>> +static int qcom_erase_cmd_type_exec(struct nand_chip *chip, const struct nand_subop *subop)
>> +{
>> +	struct qcom_nand_host *host = to_qcom_nand_host(chip);
>> +	struct qcom_nand_controller *nandc = get_qcom_nand_controller(chip);
>> +	struct qcom_op q_op;
>> +	int ret = 0;
>> +
>> +	qcom_parse_instructions(chip, subop, &q_op);
>> +
>> +	q_op.cmd_reg |= PAGE_ACC | LAST_PAGE;
>> +
>> +	nandc->buf_count = 0;
>> +	nandc->buf_start = 0;
>> +	host->use_ecc = false;
>> +	clear_read_regs(nandc);
>> +	clear_bam_transaction(nandc);
>> +
>> +	nandc_set_reg(chip, NAND_FLASH_CMD, q_op.cmd_reg);
>> +	nandc_set_reg(chip, NAND_ADDR0, q_op.addr1_reg);
>> +	nandc_set_reg(chip, NAND_ADDR1, q_op.addr2_reg);
>> +	nandc_set_reg(chip, NAND_DEV0_CFG0,
>> +		      host->cfg0_raw & ~(7 << CW_PER_PAGE));
>> +	nandc_set_reg(chip, NAND_DEV0_CFG1, host->cfg1_raw);
>> +	nandc_set_reg(chip, NAND_EXEC_CMD, 1);
>> +
>> +	write_reg_dma(nandc, NAND_FLASH_CMD, 3, NAND_BAM_NEXT_SGL);
>> +	write_reg_dma(nandc, NAND_DEV0_CFG0, 2, NAND_BAM_NEXT_SGL);
>> +	write_reg_dma(nandc, NAND_EXEC_CMD, 1, NAND_BAM_NEXT_SGL);
>> +
>> +	ret = submit_descs(nandc);
>> +	if (ret) {
>> +		dev_err(nandc->dev, "failure in sbumitting erase descriptor\n");
> 
>                                                  :)

      Its spelling mistake. Will fix this in next patch V4.

> 
>> +		free_descs(nandc);
>> +		goto err_out;
>> +	}
>> +	free_descs(nandc);
>> +
>> +	ret = qcom_wait_rdy_poll(chip, q_op.rdy_timeout_ms);
>> +	if (ret)
>> +		goto err_out;
>> +
>> +err_out:
>> +	return ret;
>> +}
>> +
>> +static int qcom_data_read_type_exec(struct nand_chip *chip, const struct nand_subop *subop)
>> +{
>> +	/* currently read_exec_op() return 0 , and all the read operation handle in
>> +	 * actual API itself
>> +	 */
>> +	return 0;
> 
> Mmmh, I don't think this is gonna work. I don't understand what you're
> doing here. What is "actual API itself"? What is "read_exec_op"? I
> doubt I am going to like what all this means. Please don't make any
> assumptions on what could come next. The core asks you to do something,
> just do it. If you can't then the parsing will fail. If the core has a
> fallback it's fine. If the core does not, we can discuss it. But please
> don't do any guesses like that, this is *exactly* why we introduced
> exec_op in the first place: you have access to the whole operation, so
> please handle it correctly.


   Thanks, When Initailly I was started exec_op() implementation I thought
   that nand_read_page_op() will fail, so that I added the dummy
   qcom_data_read_type_exec() and qcom_data_write_type_exec() APIs for read/write.
   But as you said in above comment , we have to follow whatever core ask. So I
   removed these APIs and tested read/write working fine. So now these
   dummy APIs not at all needed.I think I misunderstood. I will keep
   nand_read_page_op() API in original state nand_read_page_op(chip, page, 0, NULL, 0);
   Also removed the pattern for read_page() and write_page() since its not at all
   needed.

   Will fix these all in patch V4.


> 
>> +}
>> +
>> +static int qcom_data_write_type_exec(struct nand_chip *chip, const struct nand_subop *subop)
>> +{
>> +	/* currently write_exec_op() return 0, and all the write operation handle in
>> +	 * actual API itself
>> +	 */
>> +	return 0;
>> +}
>> +
>>   static const struct nand_op_parser qcom_op_parser = NAND_OP_PARSER(
>>   		NAND_OP_PARSER_PATTERN(
>>   			qcom_misc_cmd_type_exec,
>> @@ -3294,6 +3366,23 @@ static const struct nand_op_parser qcom_op_parser = NAND_OP_PARSER(
>>   			NAND_OP_PARSER_PAT_ADDR_ELEM(false, MAX_ADDRESS_CYCLE),
>>   			NAND_OP_PARSER_PAT_WAITRDY_ELEM(true),
>>   			NAND_OP_PARSER_PAT_DATA_IN_ELEM(false, 512)),
>> +		NAND_OP_PARSER_PATTERN(
>> +			qcom_erase_cmd_type_exec,
>> +			NAND_OP_PARSER_PAT_CMD_ELEM(false),
>> +			NAND_OP_PARSER_PAT_ADDR_ELEM(false, MAX_ADDRESS_CYCLE),
>> +			NAND_OP_PARSER_PAT_CMD_ELEM(false),
>> +			NAND_OP_PARSER_PAT_WAITRDY_ELEM(false)),
>> +		NAND_OP_PARSER_PATTERN(
>> +			qcom_data_read_type_exec,
>> +			NAND_OP_PARSER_PAT_CMD_ELEM(false),
>> +			NAND_OP_PARSER_PAT_ADDR_ELEM(false, MAX_ADDRESS_CYCLE),
>> +			NAND_OP_PARSER_PAT_CMD_ELEM(false),
>> +			NAND_OP_PARSER_PAT_WAITRDY_ELEM(true),
>> +			NAND_OP_PARSER_PAT_DATA_IN_ELEM(false, 2048)),
>> +		NAND_OP_PARSER_PATTERN(
>> +			qcom_data_write_type_exec,
>> +			NAND_OP_PARSER_PAT_CMD_ELEM(true),
>> +			NAND_OP_PARSER_PAT_ADDR_ELEM(true, MAX_ADDRESS_CYCLE)),
>>   		);
>>   
>>   static int qcom_check_op(struct nand_chip *chip,
> 
> 
> Thanks,
> Miquèl

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2023-06-14 15:20 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-31 12:49 [PATCH v3 1/5] mtd: rawnand: qcom: Implement exec_op() Md Sadre Alam
2023-05-31 12:49 ` [PATCH v3 2/5] mtd: rawnand: qcom: Add support for reset, readid, status exec_op Md Sadre Alam
2023-06-08 13:58   ` Miquel Raynal
2023-06-14 14:01     ` Md Sadre Alam
2023-05-31 12:49 ` [PATCH v3 3/5] mtd: rawnand: qcom: Add support for param_page read exec_ops Md Sadre Alam
2023-05-31 12:49 ` [PATCH v3 4/5] mtd: rawnand: qcom: Add support for read, write, erase exec_ops Md Sadre Alam
2023-06-08 14:03   ` Miquel Raynal
2023-06-14 15:19     ` Md Sadre Alam
2023-05-31 12:49 ` [PATCH v3 5/5] mtd: rawnand: qcom: Remove legacy interface Md Sadre Alam

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).