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

From: Md Sadre Alam
Date: Wed Jun 14 2023 - 11:20:32 EST

Next message: Peter Xu: "Re: [PATCH 6/7] mm/gup: Accelerate thp gup even for "pages != NULL""
Previous message: Trond Myklebust: "Re: [PATCH] nfsroot: Fix missing check for return value of strsep()"
In reply to: Miquel Raynal: "Re: [PATCH v3 4/5] mtd: rawnand: qcom: Add support for read, write, erase exec_ops"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On 6/8/2023 7:33 PM, Miquel Raynal wrote:

Hi Md,

quic_mdalam@xxxxxxxxxxx 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@xxxxxxxxxxx>
Signed-off-by: Sricharan Ramabadhran <quic_srichara@xxxxxxxxxxx>
Signed-off-by: Md Sadre Alam <quic_mdalam@xxxxxxxxxxx>
---
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

Next message: Peter Xu: "Re: [PATCH 6/7] mm/gup: Accelerate thp gup even for "pages != NULL""
Previous message: Trond Myklebust: "Re: [PATCH] nfsroot: Fix missing check for return value of strsep()"
In reply to: Miquel Raynal: "Re: [PATCH v3 4/5] mtd: rawnand: qcom: Add support for read, write, erase exec_ops"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]