[PATCH 0/5] mtd: rawnand: qcom: Implement exec_op()

[PATCH 0/5] mtd: rawnand: qcom: Implement exec_op()

[Md Sadre Alam]

These series of patches will add exec_ops support in qpic nand
driver. Earlier posted patches V1,V2 will be discontinued ,after
addrissing Mani and Miquel comment. These all patches are split
of single patche posted earlier for exec_ops.


Md Sadre Alam (5):
  mtd: rawnand: qcom: Implement exec_op()
  mtd: rawnand: qcom: Add support for reset, readid, status exec_op
  mtd: rawnand: qcom: Add support for param_page read exec_ops
  mtd: rawnand: qcom: Add support for read, write, erase exec_ops
  mtd: rawnand: qcom: Remove legacy interface implementation.

 drivers/mtd/nand/raw/qcom_nandc.c | 857 ++++++++++++++++++------------
 1 file changed, 510 insertions(+), 347 deletions(-)

-- 
2.17.1

[PATCH v2 1/5] mtd: rawnand: qcom: Implement exec_op()

[Md Sadre Alam]

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 [v2]

* Missed to post Cover-letter, so posting v2 patch with cover-letter
 
 drivers/mtd/nand/raw/qcom_nandc.c | 214 +++++++++++++++++++++++++++++-
 1 file changed, 213 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
index 72d6168d8a1b..dae460e2aa0b 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,7 @@ nandc_set_reg(chip, reg,			\
  */
 #define NAND_ERASED_CW_SET		BIT(4)
 
+#define MAX_ADDRESS_CYCLE		5
 /*
  * This data type corresponds to the BAM transaction which will be used for all
  * NAND transfers.
@@ -447,6 +449,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
  *
@@ -1517,7 +1542,8 @@ 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)
+	    command == NAND_CMD_PARAM || command == NAND_CMD_ERASE1 ||
+	    command == NAND_CMD_STATUS)
 		clear_bam_transaction(nandc);
 }
 
@@ -2867,8 +2893,194 @@ 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 = 0;
+
+	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 < min(5U, naddrs); 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_read_status_exec(struct nand_chip *chip,
+				 const struct nand_subop *subop)
+{
+	return 0;
+}
+
+static int qcom_erase_cmd_type_exec(struct nand_chip *chip, const struct nand_subop *subop)
+{
+	return 0;
+}
+
+static int qcom_param_page_type_exec(struct nand_chip *chip,  const struct nand_subop *subop)
+{
+	return 0;
+}
+
+static int qcom_read_id_type_exec(struct nand_chip *chip, const struct nand_subop *subop)
+{
+	return 0;
+}
+
+static int qcom_misc_cmd_type_exec(struct nand_chip *chip, const struct nand_subop *subop)
+{
+	return 0;
+}
+
+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
+	 */
+	struct qcom_op q_op;
+
+	qcom_parse_instructions(chip, subop, &q_op);
+
+	return 0;
+}
+
+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_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)),
+		NAND_OP_PARSER_PATTERN(
+			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_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_nand_exec_op(struct nand_chip *chip,
+			     const struct nand_operation *op,
+			bool check_only)
+{
+	if (check_only)
+		return 0;
+
+	return nand_op_parser_exec_op(chip, &qcom_op_parser,
+			op, check_only);
+}
+
 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

Re: [PATCH v2 1/5] mtd: rawnand: qcom: Implement exec_op()

[Miquel Raynal]

Hello,

quic_mdalam@quicinc.com wrote on Thu, 11 May 2023 19:00:13 +0530:

> 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 [v2]
> 
> * Missed to post Cover-letter, so posting v2 patch with cover-letter
>  
>  drivers/mtd/nand/raw/qcom_nandc.c | 214 +++++++++++++++++++++++++++++-
>  1 file changed, 213 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
> index 72d6168d8a1b..dae460e2aa0b 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,7 @@ nandc_set_reg(chip, reg,			\
>   */
>  #define NAND_ERASED_CW_SET		BIT(4)
>  
> +#define MAX_ADDRESS_CYCLE		5
>  /*
>   * This data type corresponds to the BAM transaction which will be used for all
>   * NAND transfers.
> @@ -447,6 +449,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
>   *
> @@ -1517,7 +1542,8 @@ 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)
> +	    command == NAND_CMD_PARAM || command == NAND_CMD_ERASE1 ||
> +	    command == NAND_CMD_STATUS)

I don't like this much, is there another way to derive whether
clear_bam_transaction() is needed? What is the rationale behind it?

>  		clear_bam_transaction(nandc);
>  }
>  
> @@ -2867,8 +2893,194 @@ 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 = 0;
> +
> +	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:

This should error out and the error be catch in the check_only path.

> +		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 < min(5U, naddrs); i++) {

Is this min() useful? You already limit the number of cycles to 5,
otherwise the pattern won't match, right?

> +				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_read_status_exec(struct nand_chip *chip,
> +				 const struct nand_subop *subop)
> +{
> +	return 0;
> +}
> +
> +static int qcom_erase_cmd_type_exec(struct nand_chip *chip, const struct nand_subop *subop)
> +{
> +	return 0;
> +}
> +
> +static int qcom_param_page_type_exec(struct nand_chip *chip,  const struct nand_subop *subop)
> +{
> +	return 0;
> +}
> +
> +static int qcom_read_id_type_exec(struct nand_chip *chip, const struct nand_subop *subop)
> +{
> +	return 0;
> +}
> +
> +static int qcom_misc_cmd_type_exec(struct nand_chip *chip, const struct nand_subop *subop)
> +{
> +	return 0;
> +}
> +
> +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;

Please make all exec_op additions in the same patch, unless you're
truly adding a feature, in this case it can be split, but no pattern
should match what's unsupported by ->exec_op(). This way we avoid these
very strange (and wrong) empty functions).

> +}
> +
> +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
> +	 */
> +	struct qcom_op q_op;
> +
> +	qcom_parse_instructions(chip, subop, &q_op);
> +
> +	return 0;
> +}
> +
> +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_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)),
> +		NAND_OP_PARSER_PATTERN(
> +			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_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_nand_exec_op(struct nand_chip *chip,
> +			     const struct nand_operation *op,
> +			bool check_only)
> +{
> +	if (check_only)
> +		return 0;

This is wrong, you cannot blindly return 0 if check_only is true.

> +	return nand_op_parser_exec_op(chip, &qcom_op_parser,
> +			op, check_only);
> +}
> +
>  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)


Thanks,
Miquèl

Re: [PATCH v2 1/5] mtd: rawnand: qcom: Implement exec_op()

[Md Sadre Alam]

On 5/22/2023 7:05 PM, Miquel Raynal wrote:
> Hello,
> 
> quic_mdalam@quicinc.com wrote on Thu, 11 May 2023 19:00:13 +0530:
> 
>> 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 [v2]
>>
>> * Missed to post Cover-letter, so posting v2 patch with cover-letter
>>   
>>   drivers/mtd/nand/raw/qcom_nandc.c | 214 +++++++++++++++++++++++++++++-
>>   1 file changed, 213 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
>> index 72d6168d8a1b..dae460e2aa0b 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,7 @@ nandc_set_reg(chip, reg,			\
>>    */
>>   #define NAND_ERASED_CW_SET		BIT(4)
>>   
>> +#define MAX_ADDRESS_CYCLE		5
>>   /*
>>    * This data type corresponds to the BAM transaction which will be used for all
>>    * NAND transfers.
>> @@ -447,6 +449,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
>>    *
>> @@ -1517,7 +1542,8 @@ 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)
>> +	    command == NAND_CMD_PARAM || command == NAND_CMD_ERASE1 ||
>> +	    command == NAND_CMD_STATUS)
> 
> I don't like this much, is there another way to derive whether
> clear_bam_transaction() is needed? What is the rationale behind it?

   clear_bam_transcation() is resting all the bam realted counter to 0 before starting new transcation.
   I will move these all condition check to exec_ops() specific API , and remove pre_command itself.
   Will fix this in next patch V3.
> 
>>   		clear_bam_transaction(nandc);
>>   }
>>   
>> @@ -2867,8 +2893,194 @@ 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 = 0;
>> +
>> +	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:
> 
> This should error out and the error be catch in the check_only path.

   Will fix it in next patch V3.
> 
>> +		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 < min(5U, naddrs); i++) {
> 
> Is this min() useful? You already limit the number of cycles to 5,
> otherwise the pattern won't match, right?

   Yeah you are right. If address cycle is fixed to 5 , then this min not required.
   will fix this in next v3 patch.
> 
>> +				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_read_status_exec(struct nand_chip *chip,
>> +				 const struct nand_subop *subop)
>> +{
>> +	return 0;
>> +}
>> +
>> +static int qcom_erase_cmd_type_exec(struct nand_chip *chip, const struct nand_subop *subop)
>> +{
>> +	return 0;
>> +}
>> +
>> +static int qcom_param_page_type_exec(struct nand_chip *chip,  const struct nand_subop *subop)
>> +{
>> +	return 0;
>> +}
>> +
>> +static int qcom_read_id_type_exec(struct nand_chip *chip, const struct nand_subop *subop)
>> +{
>> +	return 0;
>> +}
>> +
>> +static int qcom_misc_cmd_type_exec(struct nand_chip *chip, const struct nand_subop *subop)
>> +{
>> +	return 0;
>> +}
>> +
>> +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;
> 
> Please make all exec_op additions in the same patch, unless you're
> truly adding a feature, in this case it can be split, but no pattern
> should match what's unsupported by ->exec_op(). This way we avoid these
> very strange (and wrong) empty functions).

   Sure, will take care this in patch V3.
> 
>> +}
>> +
>> +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
>> +	 */
>> +	struct qcom_op q_op;
>> +
>> +	qcom_parse_instructions(chip, subop, &q_op);
>> +
>> +	return 0;
>> +}
>> +
>> +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_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)),
>> +		NAND_OP_PARSER_PATTERN(
>> +			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_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_nand_exec_op(struct nand_chip *chip,
>> +			     const struct nand_operation *op,
>> +			bool check_only)
>> +{
>> +	if (check_only)
>> +		return 0;
> 
> This is wrong, you cannot blindly return 0 if check_only is true.

   Will fix this in next patch V3.
> 
>> +	return nand_op_parser_exec_op(chip, &qcom_op_parser,
>> +			op, check_only);
>> +}
>> +
>>   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)
> 
> 
> Thanks,
> Miquèl

[PATCH v2 2/5] mtd: rawnand: qcom: Add support for reset, readid, status exec_op

[Md Sadre Alam]

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 [v2]

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

 drivers/mtd/nand/raw/qcom_nandc.c | 166 +++++++++++++++++++++++++++++-
 1 file changed, 163 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
index dae460e2aa0b..d2f2a8971907 100644
--- a/drivers/mtd/nand/raw/qcom_nandc.c
+++ b/drivers/mtd/nand/raw/qcom_nandc.c
@@ -384,6 +384,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;
@@ -434,6 +437,7 @@ struct qcom_nand_controller {
 	int reg_read_pos;
 
 	u32 cmd1, vld;
+	bool exec_opwrite;
 };
 
 /*
@@ -2920,6 +2924,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 = NAND_CMD_PAGEPROG;
+		nandc->exec_opwrite = true;
 		break;
 	default:
 		break;
@@ -2982,10 +2988,95 @@ 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)
 {
-	return 0;
+	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 = 1, i;
+	u32 flash_status;
+
+	host->status = NAND_STATUS_READY | NAND_STATUS_WP;
+
+	qcom_parse_instructions(chip, subop, &q_op);
+
+	if (nandc->exec_opwrite) {
+		num_cw = ecc->steps;
+		nandc->exec_opwrite = false;
+	}
+
+	pre_command(host, NAND_CMD_STATUS);
+
+	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);
+
+	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);
+
+	return ret;
 }
 
 static int qcom_erase_cmd_type_exec(struct nand_chip *chip, const struct nand_subop *subop)
@@ -3000,12 +3091,81 @@ static int qcom_param_page_type_exec(struct nand_chip *chip,  const struct nand_
 
 static int qcom_read_id_type_exec(struct nand_chip *chip, const struct nand_subop *subop)
 {
-	return 0;
+	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);
+
+	pre_command(host, NAND_CMD_READID);
+
+	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);
+
+	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);
+
+	return ret;
 }
 
 static int qcom_misc_cmd_type_exec(struct nand_chip *chip, const struct nand_subop *subop)
 {
-	return 0;
+	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 == NAND_CMD_PAGEPROG)
+		goto wait_rdy;
+
+	pre_command(host, NAND_CMD_RESET);
+
+	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);
+
+wait_rdy:
+	qcom_delay_ns(q_op.rdy_delay_ns);
+
+	ret = qcom_wait_rdy_poll(chip, q_op.rdy_timeout_ms);
+
+	return ret;
 }
 
 static int qcom_data_read_type_exec(struct nand_chip *chip, const struct nand_subop *subop)
-- 
2.17.1

Re: [PATCH v2 2/5] mtd: rawnand: qcom: Add support for reset, readid, status exec_op

[Miquel Raynal]

Hi Md,

quic_mdalam@quicinc.com wrote on Thu, 11 May 2023 19:00:14 +0530:

> 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 [v2]
> 
> * Missed to post Cover-letter, so posting v2 patch with cover-letter
> 
>  drivers/mtd/nand/raw/qcom_nandc.c | 166 +++++++++++++++++++++++++++++-
>  1 file changed, 163 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
> index dae460e2aa0b..d2f2a8971907 100644
> --- a/drivers/mtd/nand/raw/qcom_nandc.c
> +++ b/drivers/mtd/nand/raw/qcom_nandc.c
> @@ -384,6 +384,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;
> @@ -434,6 +437,7 @@ struct qcom_nand_controller {
>  	int reg_read_pos;
>  
>  	u32 cmd1, vld;
> +	bool exec_opwrite;
>  };
>  
>  /*
> @@ -2920,6 +2924,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 = NAND_CMD_PAGEPROG;

Just use the instruction value?

> +		nandc->exec_opwrite = true;
>  		break;
>  	default:
>  		break;
> @@ -2982,10 +2988,95 @@ 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)
>  {
> -	return 0;
> +	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 = 1, i;
> +	u32 flash_status;
> +
> +	host->status = NAND_STATUS_READY | NAND_STATUS_WP;
> +
> +	qcom_parse_instructions(chip, subop, &q_op);
> +
> +	if (nandc->exec_opwrite) {

I definitely don't understand this flag at all.

> +		num_cw = ecc->steps;
> +		nandc->exec_opwrite = false;
> +	}
> +
> +	pre_command(host, NAND_CMD_STATUS);
> +
> +	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);
> +
> +	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;

If there is a failure detected, error out (everywhere).

> +	}
> +
> +	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);
> +
> +	return ret;
>  }
>  
>  static int qcom_erase_cmd_type_exec(struct nand_chip *chip, const struct nand_subop *subop)
> @@ -3000,12 +3091,81 @@ static int qcom_param_page_type_exec(struct nand_chip *chip,  const struct nand_
>  
>  static int qcom_read_id_type_exec(struct nand_chip *chip, const struct nand_subop *subop)
>  {
> -	return 0;
> +	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);
> +
> +	pre_command(host, NAND_CMD_READID);
> +
> +	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);
> +
> +	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);
> +
> +	return ret;
>  }
>  
>  static int qcom_misc_cmd_type_exec(struct nand_chip *chip, const struct nand_subop *subop)
>  {
> -	return 0;
> +	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 == NAND_CMD_PAGEPROG)
> +		goto wait_rdy;
> +
> +	pre_command(host, NAND_CMD_RESET);

???

> +
> +	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");

Typo                                             ^

Same above.

You should error out immediately when something wrong happens.

> +
> +	free_descs(nandc);
> +
> +wait_rdy:
> +	qcom_delay_ns(q_op.rdy_delay_ns);
> +
> +	ret = qcom_wait_rdy_poll(chip, q_op.rdy_timeout_ms);
> +
> +	return ret;
>  }
>  
>  static int qcom_data_read_type_exec(struct nand_chip *chip, const struct nand_subop *subop)


Thanks,
Miquèl

Re: [PATCH v2 2/5] mtd: rawnand: qcom: Add support for reset, readid, status exec_op

[Md Sadre Alam]

On 5/22/2023 7:15 PM, Miquel Raynal wrote:
> Hi Md,
> 
> quic_mdalam@quicinc.com wrote on Thu, 11 May 2023 19:00:14 +0530:
> 
>> 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 [v2]
>>
>> * Missed to post Cover-letter, so posting v2 patch with cover-letter
>>
>>   drivers/mtd/nand/raw/qcom_nandc.c | 166 +++++++++++++++++++++++++++++-
>>   1 file changed, 163 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
>> index dae460e2aa0b..d2f2a8971907 100644
>> --- a/drivers/mtd/nand/raw/qcom_nandc.c
>> +++ b/drivers/mtd/nand/raw/qcom_nandc.c
>> @@ -384,6 +384,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;
>> @@ -434,6 +437,7 @@ struct qcom_nand_controller {
>>   	int reg_read_pos;
>>   
>>   	u32 cmd1, vld;
>> +	bool exec_opwrite;
>>   };
>>   
>>   /*
>> @@ -2920,6 +2924,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 = NAND_CMD_PAGEPROG;
> 
> Just use the instruction value?

   Sure , will fix this in next patch V3.
> 
>> +		nandc->exec_opwrite = true;
>>   		break;
>>   	default:
>>   		break;
>> @@ -2982,10 +2988,95 @@ 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)
>>   {
>> -	return 0;
>> +	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 = 1, i;
>> +	u32 flash_status;
>> +
>> +	host->status = NAND_STATUS_READY | NAND_STATUS_WP;
>> +
>> +	qcom_parse_instructions(chip, subop, &q_op);
>> +
>> +	if (nandc->exec_opwrite) {
> 
> I definitely don't understand this flag at all.

   This flag is to get the status for all code word in case of program page operation.
   Since this read status is common for reading status for all kind of operation.
   so in page program operation it needs to get status for all code word i.e 4 in 2K page.
   but for normal operation number of code word will be 1.
> 
>> +		num_cw = ecc->steps;
>> +		nandc->exec_opwrite = false;
>> +	}
>> +
>> +	pre_command(host, NAND_CMD_STATUS);
>> +
>> +	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);
>> +
>> +	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;
> 
> If there is a failure detected, error out (everywhere).

   Sure, will fix it in next patch V3.
> 
>> +	}
>> +
>> +	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);
>> +
>> +	return ret;
>>   }
>>   
>>   static int qcom_erase_cmd_type_exec(struct nand_chip *chip, const struct nand_subop *subop)
>> @@ -3000,12 +3091,81 @@ static int qcom_param_page_type_exec(struct nand_chip *chip,  const struct nand_
>>   
>>   static int qcom_read_id_type_exec(struct nand_chip *chip, const struct nand_subop *subop)
>>   {
>> -	return 0;
>> +	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);
>> +
>> +	pre_command(host, NAND_CMD_READID);
>> +
>> +	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);
>> +
>> +	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);
>> +
>> +	return ret;
>>   }
>>   
>>   static int qcom_misc_cmd_type_exec(struct nand_chip *chip, const struct nand_subop *subop)
>>   {
>> -	return 0;
>> +	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 == NAND_CMD_PAGEPROG)
>> +		goto wait_rdy;
>> +
>> +	pre_command(host, NAND_CMD_RESET);
> 
> ???

   Will fix it in next patch V3.
> 
>> +
>> +	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");
> 
> Typo                                             ^
> 
> Same above.
> 
> You should error out immediately when something wrong happens.

   Sure, will fix it in next patch V3.
> 
>> +
>> +	free_descs(nandc);
>> +
>> +wait_rdy:
>> +	qcom_delay_ns(q_op.rdy_delay_ns);
>> +
>> +	ret = qcom_wait_rdy_poll(chip, q_op.rdy_timeout_ms);
>> +
>> +	return ret;
>>   }
>>   
>>   static int qcom_data_read_type_exec(struct nand_chip *chip, const struct nand_subop *subop)
> 
> 
> Thanks,
> Miquèl

Re: [PATCH v2 2/5] mtd: rawnand: qcom: Add support for reset, readid, status exec_op

[Miquel Raynal]

Hi Md,

quic_mdalam@quicinc.com wrote on Wed, 24 May 2023 14:54:34 +0530:

> On 5/22/2023 7:15 PM, Miquel Raynal wrote:
> > Hi Md,
> > 
> > quic_mdalam@quicinc.com wrote on Thu, 11 May 2023 19:00:14 +0530:
> >   
> >> 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 [v2]
> >>
> >> * Missed to post Cover-letter, so posting v2 patch with cover-letter
> >>
> >>   drivers/mtd/nand/raw/qcom_nandc.c | 166 +++++++++++++++++++++++++++++-
> >>   1 file changed, 163 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
> >> index dae460e2aa0b..d2f2a8971907 100644
> >> --- a/drivers/mtd/nand/raw/qcom_nandc.c
> >> +++ b/drivers/mtd/nand/raw/qcom_nandc.c
> >> @@ -384,6 +384,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;
> >> @@ -434,6 +437,7 @@ struct qcom_nand_controller {
> >>   	int reg_read_pos;  
> >>   >>   	u32 cmd1, vld;  
> >> +	bool exec_opwrite;
> >>   };  
> >>   >>   /*  
> >> @@ -2920,6 +2924,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 = NAND_CMD_PAGEPROG;  
> > 
> > Just use the instruction value?  
> 
>    Sure , will fix this in next patch V3.
> >   
> >> +		nandc->exec_opwrite = true;
> >>   		break;
> >>   	default:
> >>   		break;
> >> @@ -2982,10 +2988,95 @@ 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)
> >>   {
> >> -	return 0;
> >> +	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 = 1, i;
> >> +	u32 flash_status;
> >> +
> >> +	host->status = NAND_STATUS_READY | NAND_STATUS_WP;
> >> +
> >> +	qcom_parse_instructions(chip, subop, &q_op);
> >> +
> >> +	if (nandc->exec_opwrite) {  
> > 
> > I definitely don't understand this flag at all.  
> 
>    This flag is to get the status for all code word in case of program page operation.
>    Since this read status is common for reading status for all kind of operation.
>    so in page program operation it needs to get status for all code word i.e 4 in 2K page.
>    but for normal operation number of code word will be 1.

Then you don't need that dark flag, just ask for a number of CW to
check. It will always be 1 unless you're in a page helper and want as
many CW as chunks.

> >   
> >> +		num_cw = ecc->steps;
> >> +		nandc->exec_opwrite = false;
> >> +	}
> >> +

Thanks,
Miquèl

[PATCH v2 3/5] mtd: rawnand: qcom: Add support for param_page read exec_ops

[Md Sadre Alam]

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 [v2]

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

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

diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
index d2f2a8971907..8717d5086f80 100644
--- a/drivers/mtd/nand/raw/qcom_nandc.c
+++ b/drivers/mtd/nand/raw/qcom_nandc.c
@@ -3086,7 +3086,96 @@ static int qcom_erase_cmd_type_exec(struct nand_chip *chip, const struct nand_su
 
 static int qcom_param_page_type_exec(struct nand_chip *chip,  const struct nand_subop *subop)
 {
-	return 0;
+	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;
+
+	pre_command(host, NAND_CMD_PARAM);
+	/*
+	 * 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, q_op.cmd_reg);
+	else
+		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);
+	}
+
+	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);
+	}
+
+	ret = submit_descs(nandc);
+	if (ret)
+		dev_err(nandc->dev, "failure in sbumitting param page descriptor\n");
+
+	free_descs(nandc);
+
+	ret = qcom_wait_rdy_poll(chip, q_op.rdy_timeout_ms);
+
+	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, nandc->data_buffer, len);
+
+	return ret;
 }
 
 static int qcom_read_id_type_exec(struct nand_chip *chip, const struct nand_subop *subop)
-- 
2.17.1

Re: [PATCH v2 3/5] mtd: rawnand: qcom: Add support for param_page read exec_ops

[Miquel Raynal]

Hi Md,

quic_mdalam@quicinc.com wrote on Thu, 11 May 2023 19:00:15 +0530:

> 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 [v2]
> 
> * Missed to post Cover-letter, so posting v2 patch with cover-letter
> 
>  drivers/mtd/nand/raw/qcom_nandc.c | 91 ++++++++++++++++++++++++++++++-
>  1 file changed, 90 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
> index d2f2a8971907..8717d5086f80 100644
> --- a/drivers/mtd/nand/raw/qcom_nandc.c
> +++ b/drivers/mtd/nand/raw/qcom_nandc.c
> @@ -3086,7 +3086,96 @@ static int qcom_erase_cmd_type_exec(struct nand_chip *chip, const struct nand_su
>  
>  static int qcom_param_page_type_exec(struct nand_chip *chip,  const struct nand_subop *subop)
>  {
> -	return 0;
> +	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;
> +
> +	pre_command(host, NAND_CMD_PARAM);
> +	/*
> +	 * 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

There is no guess to do, just follow what the core asks.

> +	 */
> +	if (nandc->props->qpic_v2)
> +		nandc_set_reg(chip, NAND_FLASH_CMD, q_op.cmd_reg);
> +	else
> +		nandc_set_reg(chip, NAND_FLASH_CMD, q_op.cmd_reg);

There is something wrong here.

> +
> +	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;

The length is provided by the instruction.

> +	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);
> +
> +	ret = qcom_wait_rdy_poll(chip, q_op.rdy_timeout_ms);
> +
> +	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, nandc->data_buffer, len);
> +
> +	return ret;
>  }
>  
>  static int qcom_read_id_type_exec(struct nand_chip *chip, const struct nand_subop *subop)


Thanks,
Miquèl

Re: [PATCH v2 3/5] mtd: rawnand: qcom: Add support for param_page read exec_ops

[Md Sadre Alam]

On 5/22/2023 7:19 PM, Miquel Raynal wrote:
> Hi Md,
> 
> quic_mdalam@quicinc.com wrote on Thu, 11 May 2023 19:00:15 +0530:
> 
>> 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 [v2]
>>
>> * Missed to post Cover-letter, so posting v2 patch with cover-letter
>>
>>   drivers/mtd/nand/raw/qcom_nandc.c | 91 ++++++++++++++++++++++++++++++-
>>   1 file changed, 90 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
>> index d2f2a8971907..8717d5086f80 100644
>> --- a/drivers/mtd/nand/raw/qcom_nandc.c
>> +++ b/drivers/mtd/nand/raw/qcom_nandc.c
>> @@ -3086,7 +3086,96 @@ static int qcom_erase_cmd_type_exec(struct nand_chip *chip, const struct nand_su
>>   
>>   static int qcom_param_page_type_exec(struct nand_chip *chip,  const struct nand_subop *subop)
>>   {
>> -	return 0;
>> +	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;
>> +
>> +	pre_command(host, NAND_CMD_PARAM);
>> +	/*
>> +	 * 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
> 
> There is no guess to do, just follow what the core asks.

   Sure, will fix this in next patch V3.
> 
>> +	 */
>> +	if (nandc->props->qpic_v2)
>> +		nandc_set_reg(chip, NAND_FLASH_CMD, q_op.cmd_reg);
>> +	else
>> +		nandc_set_reg(chip, NAND_FLASH_CMD, q_op.cmd_reg);
> 
> There is something wrong here.
> 
   Will fix this in next patch V3.

>> +
>> +	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;
> 
> The length is provided by the instruction.

    Will fix this in next patch V3.
> 
>> +	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);
>> +
>> +	ret = qcom_wait_rdy_poll(chip, q_op.rdy_timeout_ms);
>> +
>> +	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, nandc->data_buffer, len);
>> +
>> +	return ret;
>>   }
>>   
>>   static int qcom_read_id_type_exec(struct nand_chip *chip, const struct nand_subop *subop)
> 
> 
> Thanks,
> Miquèl

[PATCH v2 4/5] mtd: rawnand: qcom: Add support for read, write, erase exec_ops

[Md Sadre Alam]

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 [v2]

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

 drivers/mtd/nand/raw/qcom_nandc.c | 52 +++++++++++++++++++++++++++++--
 1 file changed, 49 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
index 8717d5086f80..14ab21a4771b 100644
--- a/drivers/mtd/nand/raw/qcom_nandc.c
+++ b/drivers/mtd/nand/raw/qcom_nandc.c
@@ -1765,7 +1765,8 @@ 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);
+	chip->cont_read.ongoing = false;
+	nand_read_page_op(chip, page, 0, data_buf, mtd->writesize);
 	host->use_ecc = false;
 
 	if (nandc->props->qpic_v2)
@@ -2182,14 +2183,24 @@ 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);
+	chip->cont_read.ongoing = false;
+	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;
 
@@ -2259,6 +2270,10 @@ 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);
 
@@ -3081,7 +3096,38 @@ static int qcom_read_status_exec(struct nand_chip *chip,
 
 static int qcom_erase_cmd_type_exec(struct nand_chip *chip, const struct nand_subop *subop)
 {
-	return 0;
+	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;
+
+	pre_command(host, NAND_CMD_ERASE1);
+
+	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 reset descriptor\n");
+
+	free_descs(nandc);
+
+	ret = qcom_wait_rdy_poll(chip, q_op.rdy_timeout_ms);
+
+	return ret;
 }
 
 static int qcom_param_page_type_exec(struct nand_chip *chip,  const struct nand_subop *subop)
-- 
2.17.1

Re: [PATCH v2 4/5] mtd: rawnand: qcom: Add support for read, write, erase exec_ops

[Miquel Raynal]

quic_mdalam@quicinc.com wrote on Thu, 11 May 2023 19:00:16 +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 [v2]
> 
> * Missed to post Cover-letter, so posting v2 patch with cover-letter
> 
>  drivers/mtd/nand/raw/qcom_nandc.c | 52 +++++++++++++++++++++++++++++--
>  1 file changed, 49 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
> index 8717d5086f80..14ab21a4771b 100644
> --- a/drivers/mtd/nand/raw/qcom_nandc.c
> +++ b/drivers/mtd/nand/raw/qcom_nandc.c
> @@ -1765,7 +1765,8 @@ 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);
> +	chip->cont_read.ongoing = false;

This should be checked once for all by the core at startup, that's when
you can tell the core continuous read is not supported by the
controller.

> +	nand_read_page_op(chip, page, 0, data_buf, mtd->writesize);
>  	host->use_ecc = false;
>  
>  	if (nandc->props->qpic_v2)
> @@ -2182,14 +2183,24 @@ 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);
> +	chip->cont_read.ongoing = false;
> +	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;
>  
> @@ -2259,6 +2270,10 @@ 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);
>  
> @@ -3081,7 +3096,38 @@ static int qcom_read_status_exec(struct nand_chip *chip,
>  
>  static int qcom_erase_cmd_type_exec(struct nand_chip *chip, const struct nand_subop *subop)
>  {
> -	return 0;
> +	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;
> +
> +	pre_command(host, NAND_CMD_ERASE1);

The instruction is up to the caller, not to the driver. If no other
instruction rather than NAND_CMD_ERASE1 can be used with this pattern,
then it should be properly described (see the Arasan controller,
anfc_check_op()).

> +
> +	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 reset descriptor\n");
> +
> +	free_descs(nandc);
> +
> +	ret = qcom_wait_rdy_poll(chip, q_op.rdy_timeout_ms);
> +
> +	return ret;
>  }
>  
>  static int qcom_param_page_type_exec(struct nand_chip *chip,  const struct nand_subop *subop)


Thanks,
Miquèl

Re: [PATCH v2 4/5] mtd: rawnand: qcom: Add support for read, write, erase exec_ops

[Md Sadre Alam]

On 5/22/2023 7:23 PM, Miquel Raynal wrote:
> 
> quic_mdalam@quicinc.com wrote on Thu, 11 May 2023 19:00:16 +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 [v2]
>>
>> * Missed to post Cover-letter, so posting v2 patch with cover-letter
>>
>>   drivers/mtd/nand/raw/qcom_nandc.c | 52 +++++++++++++++++++++++++++++--
>>   1 file changed, 49 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
>> index 8717d5086f80..14ab21a4771b 100644
>> --- a/drivers/mtd/nand/raw/qcom_nandc.c
>> +++ b/drivers/mtd/nand/raw/qcom_nandc.c
>> @@ -1765,7 +1765,8 @@ 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);
>> +	chip->cont_read.ongoing = false;
> 
> This should be checked once for all by the core at startup, that's when
> you can tell the core continuous read is not supported by the
> controller.

   Sure, Will fix this in next patch V3.
> 
>> +	nand_read_page_op(chip, page, 0, data_buf, mtd->writesize);
>>   	host->use_ecc = false;
>>   
>>   	if (nandc->props->qpic_v2)
>> @@ -2182,14 +2183,24 @@ 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);
>> +	chip->cont_read.ongoing = false;
>> +	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;
>>   
>> @@ -2259,6 +2270,10 @@ 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);
>>   
>> @@ -3081,7 +3096,38 @@ static int qcom_read_status_exec(struct nand_chip *chip,
>>   
>>   static int qcom_erase_cmd_type_exec(struct nand_chip *chip, const struct nand_subop *subop)
>>   {
>> -	return 0;
>> +	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;
>> +
>> +	pre_command(host, NAND_CMD_ERASE1);
> 
> The instruction is up to the caller, not to the driver. If no other
> instruction rather than NAND_CMD_ERASE1 can be used with this pattern,
> then it should be properly described (see the Arasan controller,
> anfc_check_op()).
> 
   Thanks, will fix this in next patch V3.

>> +
>> +	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 reset descriptor\n");
>> +
>> +	free_descs(nandc);
>> +
>> +	ret = qcom_wait_rdy_poll(chip, q_op.rdy_timeout_ms);
>> +
>> +	return ret;
>>   }
>>   
>>   static int qcom_param_page_type_exec(struct nand_chip *chip,  const struct nand_subop *subop)
> 
> 
> Thanks,
> Miquèl

[PATCH v2 5/5] mtd: rawnand: qcom: Remove legacy interface implementation.

[Md Sadre Alam]

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 [v2]

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

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

diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
index 14ab21a4771b..e689809cd4df 100644
--- a/drivers/mtd/nand/raw/qcom_nandc.c
+++ b/drivers/mtd/nand/raw/qcom_nandc.c
@@ -1302,155 +1302,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)
 {
@@ -1551,135 +1402,6 @@ static void pre_command(struct qcom_nand_host *host, int command)
 		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.
@@ -2539,64 +2261,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
  *
@@ -3642,14 +3306,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