* [PATCH V2] mtd: rawnand: qcom: Implement exec_op()
@ 2023-04-19 9:36 Md Sadre Alam
2023-04-20 3:05 ` kernel test robot
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Md Sadre Alam @ 2023-04-19 9:36 UTC (permalink / raw)
To: mani, miquel.raynal, richard, vigneshr, linux-mtd, linux-arm-msm,
linux-kernel
Cc: quic_srichara, quic_mdalam
Implement exec_op() so we can later get rid of the legacy interface
implementation.
Co-developed-by: Sricharan R <quic_srichara@quicinc.com>
Signed-off-by: Sricharan R <quic_srichara@quicinc.com>
Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
---
Change in [V2]
* Address all Miquel comments from V1, regarding exec_ops design
* Redesign qpic nand driver with exec_ops using exec_ops parser, to
fit in exec_ops framework
drivers/mtd/nand/raw/qcom_nandc.c | 869 ++++++++++++++++++------------
1 file changed, 520 insertions(+), 349 deletions(-)
diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
index 72d6168d8a1b..17279890454d 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.
@@ -432,6 +434,7 @@ struct qcom_nand_controller {
int reg_read_pos;
u32 cmd1, vld;
+ bool exec_opwrite;
};
/*
@@ -447,6 +450,37 @@ struct qcom_nand_boot_partition {
u32 page_size;
};
+/*
+ * QCOM op for each exec_op transfer
+ *
+ * @addr1_reg: Address1 register value
+ *
+ * @addr2_reg: Address2 register value
+ *
+ * @cmd_reg: CMD register value
+ *
+ * @rdy_timeout_ms: wait ready timeout in ms
+ *
+ * @rdy_delay_ns: Additional delay in ns
+ *
+ * @data_instr_idx: data instruction index
+ *
+ * @data_instr: data instruction pointer
+ *
+ * @flag: flag for misc instruction
+ *
+ */
+struct qcom_op {
+ u32 addr1_reg;
+ u32 addr2_reg;
+ u32 cmd_reg;
+ unsigned int rdy_timeout_ms;
+ unsigned int rdy_delay_ns;
+ unsigned int data_instr_idx;
+ const struct nand_op_instr *data_instr;
+ u8 flag;
+};
+
/*
* NAND chip structure
*
@@ -765,7 +799,6 @@ static void set_address(struct qcom_nand_host *host, u16 column, int page)
if (chip->options & NAND_BUSWIDTH_16)
column >>= 1;
-
nandc_set_reg(chip, NAND_ADDR0, page << 16 | column);
nandc_set_reg(chip, NAND_ADDR1, page >> 16 & 0xff);
}
@@ -1273,155 +1306,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)
{
@@ -1517,139 +1401,11 @@ 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);
}
-/*
- * 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.
@@ -1735,7 +1491,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, 2048);
host->use_ecc = false;
if (nandc->props->qpic_v2)
@@ -2047,7 +1804,9 @@ static int read_page_ecc(struct qcom_nand_host *host, u8 *data_buf,
return ret;
}
- return parse_read_errors(host, data_buf_start, oob_buf_start, page);
+ ret = parse_read_errors(host, data_buf_start, oob_buf_start, page);
+
+ return ret;
}
/*
@@ -2154,12 +1913,25 @@ static int qcom_nandc_read_page(struct nand_chip *chip, uint8_t *buf,
{
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;
+ int ret = 0;
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;
+ ret = nand_read_page_op(chip, page, 0, buf, 2048);
+ if (ret)
+ return ret;
+
+ 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;
@@ -2229,6 +2001,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);
@@ -2494,64 +2270,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
*
@@ -2867,8 +2585,469 @@ 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;
+ q_op->flag = NAND_CMD_PAGEPROG;
+ nandc->exec_opwrite = true;
+ 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 void qcom_delay_ns(unsigned int ns)
+{
+ if (!ns)
+ return;
+
+ if (ns < 10000)
+ ndelay(ns);
+ else
+ udelay(DIV_ROUND_UP(ns, 1000));
+}
+
+static int qcom_wait_rdy_poll(struct nand_chip *chip, unsigned int time_ms)
+{
+ struct qcom_nand_controller *nandc = get_qcom_nand_controller(chip);
+ unsigned long start = jiffies + msecs_to_jiffies(time_ms);
+ u32 flash;
+
+ nandc_read_buffer_sync(nandc, true);
+
+ do {
+ flash = le32_to_cpu(nandc->reg_read_buf[0]);
+ if (flash & FS_READY_BSY_N)
+ return 0;
+ cpu_relax();
+ } while (time_after(start, jiffies));
+
+ dev_err(nandc->dev, "Timeout waiting for device to be ready:0x%08x\n", flash);
+
+ return -ETIMEDOUT;
+}
+
+static int qcom_read_status_exec(struct nand_chip *chip,
+ const struct nand_subop *subop)
+{
+ struct qcom_nand_host *host = to_qcom_nand_host(chip);
+ struct qcom_nand_controller *nandc = get_qcom_nand_controller(chip);
+ struct nand_ecc_ctrl *ecc = &chip->ecc;
+ struct qcom_op q_op;
+ const struct nand_op_instr *instr = NULL;
+ unsigned int op_id = 0;
+ unsigned int len = 0;
+ int ret = 0, num_cw = 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)
+{
+ 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)
+{
+ 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)
+{
+ 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)
+{
+ 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)
+{
+ /* 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)
@@ -3135,14 +3314,6 @@ static int qcom_nand_host_init_and_register(struct qcom_nand_controller *nandc,
mtd->owner = THIS_MODULE;
mtd->dev.parent = dev;
- chip->legacy.cmdfunc = qcom_nandc_command;
- chip->legacy.select_chip = qcom_nandc_select_chip;
- chip->legacy.read_byte = qcom_nandc_read_byte;
- chip->legacy.read_buf = qcom_nandc_read_buf;
- chip->legacy.write_buf = qcom_nandc_write_buf;
- chip->legacy.set_features = nand_get_set_features_notsupp;
- chip->legacy.get_features = nand_get_set_features_notsupp;
-
/*
* the bad block marker is readable only when we read the last codeword
* of a page with ECC disabled. currently, the nand_base and nand_bbt
--
2.17.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH V2] mtd: rawnand: qcom: Implement exec_op()
2023-04-19 9:36 [PATCH V2] mtd: rawnand: qcom: Implement exec_op() Md Sadre Alam
@ 2023-04-20 3:05 ` kernel test robot
2023-04-24 7:35 ` kernel test robot
2023-05-09 11:08 ` Manivannan Sadhasivam
2 siblings, 0 replies; 5+ messages in thread
From: kernel test robot @ 2023-04-20 3:05 UTC (permalink / raw)
To: Md Sadre Alam, mani, miquel.raynal, richard, vigneshr, linux-mtd,
linux-arm-msm, linux-kernel
Cc: llvm, oe-kbuild-all, quic_srichara, quic_mdalam
Hi Md,
kernel test robot noticed the following build warnings:
[auto build test WARNING on mtd/nand/next]
[also build test WARNING on linus/master v6.3-rc7 next-20230419]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Md-Sadre-Alam/mtd-rawnand-qcom-Implement-exec_op/20230419-173849
base: https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next
patch link: https://lore.kernel.org/r/20230419093617.27134-1-quic_mdalam%40quicinc.com
patch subject: [PATCH V2] mtd: rawnand: qcom: Implement exec_op()
config: s390-randconfig-r034-20230418 (https://download.01.org/0day-ci/archive/20230420/202304201013.6bradWxp-lkp@intel.com/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project 437b7602e4a998220871de78afcb020b9c14a661)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install s390 cross compiling tool for clang build
# apt-get install binutils-s390x-linux-gnu
# https://github.com/intel-lab-lkp/linux/commit/a2d1599a63e93bde90166287021663cfc13c91b2
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Md-Sadre-Alam/mtd-rawnand-qcom-Implement-exec_op/20230419-173849
git checkout a2d1599a63e93bde90166287021663cfc13c91b2
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=s390 olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=s390 SHELL=/bin/bash drivers/mtd/nand/raw/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202304201013.6bradWxp-lkp@intel.com/
All warnings (new ones prefixed by >>):
In file included from drivers/mtd/nand/raw/qcom_nandc.c:9:
In file included from include/linux/dma-mapping.h:10:
In file included from include/linux/scatterlist.h:9:
In file included from arch/s390/include/asm/io.h:75:
include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __raw_readb(PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/big_endian.h:37:59: note: expanded from macro '__le16_to_cpu'
#define __le16_to_cpu(x) __swab16((__force __u16)(__le16)(x))
^
include/uapi/linux/swab.h:102:54: note: expanded from macro '__swab16'
#define __swab16(x) (__u16)__builtin_bswap16((__u16)(x))
^
In file included from drivers/mtd/nand/raw/qcom_nandc.c:9:
In file included from include/linux/dma-mapping.h:10:
In file included from include/linux/scatterlist.h:9:
In file included from arch/s390/include/asm/io.h:75:
include/asm-generic/io.h:573:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/big_endian.h:35:59: note: expanded from macro '__le32_to_cpu'
#define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
^
include/uapi/linux/swab.h:115:54: note: expanded from macro '__swab32'
#define __swab32(x) (__u32)__builtin_bswap32((__u32)(x))
^
In file included from drivers/mtd/nand/raw/qcom_nandc.c:9:
In file included from include/linux/dma-mapping.h:10:
In file included from include/linux/scatterlist.h:9:
In file included from arch/s390/include/asm/io.h:75:
include/asm-generic/io.h:584:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writeb(value, PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:594:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:604:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:692:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
readsb(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:700:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
readsw(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:708:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
readsl(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:717:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
writesb(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:726:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
writesw(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:735:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
writesl(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
>> drivers/mtd/nand/raw/qcom_nandc.c:2617:2: warning: unannotated fall-through between switch labels [-Wimplicit-fallthrough]
default:
^
drivers/mtd/nand/raw/qcom_nandc.c:2617:2: note: insert 'break;' to avoid fall-through
default:
^
break;
13 warnings generated.
vim +2617 drivers/mtd/nand/raw/qcom_nandc.c
2587
2588 static int qcom_op_cmd_mapping(struct qcom_nand_controller *nandc, u8 cmd,
2589 struct qcom_op *q_op)
2590 {
2591 int ret = 0;
2592
2593 switch (cmd) {
2594 case NAND_CMD_RESET:
2595 ret = OP_RESET_DEVICE;
2596 break;
2597 case NAND_CMD_READID:
2598 ret = OP_FETCH_ID;
2599 break;
2600 case NAND_CMD_PARAM:
2601 if (nandc->props->qpic_v2)
2602 ret = OP_PAGE_READ_ONFI_READ;
2603 else
2604 ret = OP_PAGE_READ;
2605 break;
2606 case NAND_CMD_ERASE1:
2607 case NAND_CMD_ERASE2:
2608 ret = OP_BLOCK_ERASE;
2609 break;
2610 case NAND_CMD_STATUS:
2611 ret = OP_CHECK_STATUS;
2612 break;
2613 case NAND_CMD_PAGEPROG:
2614 ret = OP_PROGRAM_PAGE;
2615 q_op->flag = NAND_CMD_PAGEPROG;
2616 nandc->exec_opwrite = true;
> 2617 default:
2618 break;
2619 }
2620
2621 return ret;
2622 }
2623
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH V2] mtd: rawnand: qcom: Implement exec_op()
2023-04-19 9:36 [PATCH V2] mtd: rawnand: qcom: Implement exec_op() Md Sadre Alam
2023-04-20 3:05 ` kernel test robot
@ 2023-04-24 7:35 ` kernel test robot
2023-05-09 11:08 ` Manivannan Sadhasivam
2 siblings, 0 replies; 5+ messages in thread
From: kernel test robot @ 2023-04-24 7:35 UTC (permalink / raw)
To: Md Sadre Alam, mani, miquel.raynal, richard, vigneshr, linux-mtd,
linux-arm-msm, linux-kernel
Cc: oe-kbuild-all, quic_srichara, quic_mdalam
Hi Md,
kernel test robot noticed the following build warnings:
[auto build test WARNING on mtd/nand/next]
[also build test WARNING on linus/master v6.3 next-20230421]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Md-Sadre-Alam/mtd-rawnand-qcom-Implement-exec_op/20230419-173849
base: https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next
patch link: https://lore.kernel.org/r/20230419093617.27134-1-quic_mdalam%40quicinc.com
patch subject: [PATCH V2] mtd: rawnand: qcom: Implement exec_op()
config: nios2-randconfig-m041-20230423 (https://download.01.org/0day-ci/archive/20230424/202304241556.fafzkLLk-lkp@intel.com/config)
compiler: nios2-linux-gcc (GCC) 12.1.0
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202304241556.fafzkLLk-lkp@intel.com/
smatch warnings:
drivers/mtd/nand/raw/qcom_nandc.c:2751 qcom_read_status_exec() warn: inconsistent indenting
vim +2751 drivers/mtd/nand/raw/qcom_nandc.c
2708
2709 static int qcom_read_status_exec(struct nand_chip *chip,
2710 const struct nand_subop *subop)
2711 {
2712 struct qcom_nand_host *host = to_qcom_nand_host(chip);
2713 struct qcom_nand_controller *nandc = get_qcom_nand_controller(chip);
2714 struct nand_ecc_ctrl *ecc = &chip->ecc;
2715 struct qcom_op q_op;
2716 const struct nand_op_instr *instr = NULL;
2717 unsigned int op_id = 0;
2718 unsigned int len = 0;
2719 int ret = 0, num_cw = 1, i;
2720 u32 flash_status;
2721
2722 host->status = NAND_STATUS_READY | NAND_STATUS_WP;
2723
2724 qcom_parse_instructions(chip, subop, &q_op);
2725
2726 if (nandc->exec_opwrite) {
2727 num_cw = ecc->steps;
2728 nandc->exec_opwrite = false;
2729 }
2730
2731 pre_command(host, NAND_CMD_STATUS);
2732
2733 nandc_set_reg(chip, NAND_FLASH_CMD, q_op.cmd_reg);
2734 nandc_set_reg(chip, NAND_EXEC_CMD, 1);
2735
2736 write_reg_dma(nandc, NAND_FLASH_CMD, 1, NAND_BAM_NEXT_SGL);
2737 write_reg_dma(nandc, NAND_EXEC_CMD, 1, NAND_BAM_NEXT_SGL);
2738
2739 read_reg_dma(nandc, NAND_FLASH_STATUS, 1, NAND_BAM_NEXT_SGL);
2740
2741 ret = submit_descs(nandc);
2742 if (ret)
2743 dev_err(nandc->dev, "failure in sbumitting status descriptor\n");
2744
2745 free_descs(nandc);
2746
2747 nandc_read_buffer_sync(nandc, true);
2748 for (i = 0; i < num_cw; i++) {
2749 flash_status = le32_to_cpu(nandc->reg_read_buf[i]);
2750
> 2751 if (flash_status & FS_MPU_ERR)
2752 host->status &= ~NAND_STATUS_WP;
2753
2754 if (flash_status & FS_OP_ERR || (i == (num_cw - 1) &&
2755 (flash_status & FS_DEVICE_STS_ERR)))
2756 host->status |= NAND_STATUS_FAIL;
2757 }
2758
2759 flash_status = host->status;
2760
2761 instr = q_op.data_instr;
2762 op_id = q_op.data_instr_idx;
2763 len = nand_subop_get_data_len(subop, op_id);
2764 memcpy(instr->ctx.data.buf.in, &flash_status, len);
2765
2766 return ret;
2767 }
2768
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH V2] mtd: rawnand: qcom: Implement exec_op()
2023-04-19 9:36 [PATCH V2] mtd: rawnand: qcom: Implement exec_op() Md Sadre Alam
2023-04-20 3:05 ` kernel test robot
2023-04-24 7:35 ` kernel test robot
@ 2023-05-09 11:08 ` Manivannan Sadhasivam
2023-05-10 9:01 ` Md Sadre Alam
2 siblings, 1 reply; 5+ messages in thread
From: Manivannan Sadhasivam @ 2023-05-09 11:08 UTC (permalink / raw)
To: Md Sadre Alam
Cc: miquel.raynal, richard, vigneshr, linux-mtd, linux-arm-msm,
linux-kernel, quic_srichara
On Wed, Apr 19, 2023 at 03:06:17PM +0530, Md Sadre Alam wrote:
> Implement exec_op() so we can later get rid of the legacy interface
> implementation.
>
It's good that you got the inspiration from previous exec_op() conversion
patches, but this one should've been splitted into atleast two patches as per
your patch description.
One introducing exec_op() and another getting rid of legacy interface.
> Co-developed-by: Sricharan R <quic_srichara@quicinc.com>
> Signed-off-by: Sricharan R <quic_srichara@quicinc.com>
> Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
> ---
> Change in [V2]
>
> * Address all Miquel comments from V1, regarding exec_ops design
>
> * Redesign qpic nand driver with exec_ops using exec_ops parser, to
> fit in exec_ops framework
>
> drivers/mtd/nand/raw/qcom_nandc.c | 869 ++++++++++++++++++------------
> 1 file changed, 520 insertions(+), 349 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
> index 72d6168d8a1b..17279890454d 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.
> @@ -432,6 +434,7 @@ struct qcom_nand_controller {
> int reg_read_pos;
>
> u32 cmd1, vld;
> + bool exec_opwrite;
Missing kdoc.
> };
>
> /*
> @@ -447,6 +450,37 @@ struct qcom_nand_boot_partition {
> u32 page_size;
> };
>
> +/*
> + * QCOM op for each exec_op transfer
What is "QCOM op"? Also please stick to "Qcom" everywhere.
> + *
> + * @addr1_reg: Address1 register value
> + *
> + * @addr2_reg: Address2 register value
> + *
> + * @cmd_reg: CMD register value
> + *
> + * @rdy_timeout_ms: wait ready timeout in ms
> + *
First letter caps for all instances.
> + * @rdy_delay_ns: Additional delay in ns
> + *
> + * @data_instr_idx: data instruction index
> + *
> + * @data_instr: data instruction pointer
> + *
Do not leave this extra newline comment in-between.
> + * @flag: flag for misc instruction
> + *
> + */
> +struct qcom_op {
> + u32 addr1_reg;
> + u32 addr2_reg;
> + u32 cmd_reg;
> + unsigned int rdy_timeout_ms;
> + unsigned int rdy_delay_ns;
> + unsigned int data_instr_idx;
> + const struct nand_op_instr *data_instr;
Move the pointer to start of the struct.
> + u8 flag;
> +};
> +
> /*
> * NAND chip structure
> *
> @@ -765,7 +799,6 @@ static void set_address(struct qcom_nand_host *host, u16 column, int page)
>
> if (chip->options & NAND_BUSWIDTH_16)
> column >>= 1;
> -
Sporadic?
> nandc_set_reg(chip, NAND_ADDR0, page << 16 | column);
> nandc_set_reg(chip, NAND_ADDR1, page >> 16 & 0xff);
> }
> @@ -1273,155 +1306,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)
As I pointed out above, you need to split these removals into a separate patch.
> -{
> - 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;
> -}
[...]
> /*
> * 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.
> @@ -1735,7 +1491,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, 2048);
How is this change related to exec_op() conversion? Looks like this and few
things below should be part of a separate patch.
> host->use_ecc = false;
>
> if (nandc->props->qpic_v2)
> @@ -2047,7 +1804,9 @@ static int read_page_ecc(struct qcom_nand_host *host, u8 *data_buf,
> return ret;
> }
>
> - return parse_read_errors(host, data_buf_start, oob_buf_start, page);
> + ret = parse_read_errors(host, data_buf_start, oob_buf_start, page);
> +
> + return ret;
Why?
> }
>
> /*
> @@ -2154,12 +1913,25 @@ static int qcom_nandc_read_page(struct nand_chip *chip, uint8_t *buf,
> {
> 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;
> + int ret = 0;
>
> 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;
> + ret = nand_read_page_op(chip, page, 0, buf, 2048);
> + if (ret)
> + return ret;
> +
> + 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;
>
> @@ -2229,6 +2001,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;
> +
As said above, you need a separate patch for the above change. I do not get the
context of the above change.
And I stopped reviewing here. It's hard to review ~800 lines of change in one
shot. Please split this patch into sensible chunks so that it makes life easy
for reviewers.
- Mani
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH V2] mtd: rawnand: qcom: Implement exec_op()
2023-05-09 11:08 ` Manivannan Sadhasivam
@ 2023-05-10 9:01 ` Md Sadre Alam
0 siblings, 0 replies; 5+ messages in thread
From: Md Sadre Alam @ 2023-05-10 9:01 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: miquel.raynal, richard, vigneshr, linux-mtd, linux-arm-msm,
linux-kernel, quic_srichara
On 5/9/2023 4:38 PM, Manivannan Sadhasivam wrote:
> On Wed, Apr 19, 2023 at 03:06:17PM +0530, Md Sadre Alam wrote:
>> Implement exec_op() so we can later get rid of the legacy interface
>> implementation.
>>
>
> It's good that you got the inspiration from previous exec_op() conversion
> patches, but this one should've been splitted into atleast two patches as per
> your patch description.
>
> One introducing exec_op() and another getting rid of legacy interface.
Thanks Mani for reviewing. I am going to split the patches as below.
1) Introduce exec_op structure, parser, command mappping.
2) Add support for RESET, READID, STATUS exec_ops
3) Add support for READ_PARAM_PAGE exec_ops
4) Add support for ERASE, READ, WRITE exec_ops
5) Remove legacy interface
>
>> Co-developed-by: Sricharan R <quic_srichara@quicinc.com>
>> Signed-off-by: Sricharan R <quic_srichara@quicinc.com>
>> Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
>> ---
>> Change in [V2]
>>
>> * Address all Miquel comments from V1, regarding exec_ops design
>>
>> * Redesign qpic nand driver with exec_ops using exec_ops parser, to
>> fit in exec_ops framework
>>
>> drivers/mtd/nand/raw/qcom_nandc.c | 869 ++++++++++++++++++------------
>> 1 file changed, 520 insertions(+), 349 deletions(-)
>>
>> diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
>> index 72d6168d8a1b..17279890454d 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.
>> @@ -432,6 +434,7 @@ struct qcom_nand_controller {
>> int reg_read_pos;
>>
>> u32 cmd1, vld;
>> + bool exec_opwrite;
>
> Missing kdoc.
Will add in next patch
>
>> };
>>
>> /*
>> @@ -447,6 +450,37 @@ struct qcom_nand_boot_partition {
>> u32 page_size;
>> };
>>
>> +/*
>> + * QCOM op for each exec_op transfer
>
> What is "QCOM op"? Also please stick to "Qcom" everywhere.
It is exec_op ops for all instruction supported .
Will update in next patch.
>
>> + *
>> + * @addr1_reg: Address1 register value
>> + *
>> + * @addr2_reg: Address2 register value
>> + *
>> + * @cmd_reg: CMD register value
>> + *
>> + * @rdy_timeout_ms: wait ready timeout in ms
>> + *
>
> First letter caps for all instances.
will do in next patch
>
>> + * @rdy_delay_ns: Additional delay in ns
>> + *
>> + * @data_instr_idx: data instruction index
>> + *
>> + * @data_instr: data instruction pointer
>> + *
>
> Do not leave this extra newline comment in-between.
will do in next patch
>
>> + * @flag: flag for misc instruction
>> + *
>> + */
>> +struct qcom_op {
>> + u32 addr1_reg;
>> + u32 addr2_reg;
>> + u32 cmd_reg;
>> + unsigned int rdy_timeout_ms;
>> + unsigned int rdy_delay_ns;
>> + unsigned int data_instr_idx;
>> + const struct nand_op_instr *data_instr;
>
> Move the pointer to start of the struct.
will do in next patch.
>
>> + u8 flag;
>> +};
>> +
>> /*
>> * NAND chip structure
>> *
>> @@ -765,7 +799,6 @@ static void set_address(struct qcom_nand_host *host, u16 column, int page)
>>
>> if (chip->options & NAND_BUSWIDTH_16)
>> column >>= 1;
>> -
>
> Sporadic?
will fix in next patch
>
>> nandc_set_reg(chip, NAND_ADDR0, page << 16 | column);
>> nandc_set_reg(chip, NAND_ADDR1, page >> 16 & 0xff);
>> }
>> @@ -1273,155 +1306,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)
>
> As I pointed out above, you need to split these removals into a separate patch.
will do in next patch.
>
>> -{
>> - 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;
>> -}
>
> [...]
>
>> /*
>> * 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.
>> @@ -1735,7 +1491,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, 2048);
>
> How is this change related to exec_op() conversion? Looks like this and few
> things below should be part of a separate patch.
Without this data_buf and proper length the read exec_op api is not getting called
from nand_base layer. So its needed. As i am separating patches , this change will be part
of read exec_op support patch.
>
>> host->use_ecc = false;
>>
>> if (nandc->props->qpic_v2)
>> @@ -2047,7 +1804,9 @@ static int read_page_ecc(struct qcom_nand_host *host, u8 *data_buf,
>> return ret;
>> }
>>
>> - return parse_read_errors(host, data_buf_start, oob_buf_start, page);
>> + ret = parse_read_errors(host, data_buf_start, oob_buf_start, page);
>> +
>> + return ret;
>
> Why?
will fix in next patch
>
>> }
>>
>> /*
>> @@ -2154,12 +1913,25 @@ static int qcom_nandc_read_page(struct nand_chip *chip, uint8_t *buf,
>> {
>> 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;
>> + int ret = 0;
>>
>> 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;
>> + ret = nand_read_page_op(chip, page, 0, buf, 2048);
>> + if (ret)
>> + return ret;
>> +
>> + 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;
>>
>> @@ -2229,6 +2001,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;
>> +
>
> As said above, you need a separate patch for the above change. I do not get the
> context of the above change.
>
> And I stopped reviewing here. It's hard to review ~800 lines of change in one
> shot. Please split this patch into sensible chunks so that it makes life easy
> for reviewers.
>
> - Mani
Sure will fix all comments in next patch and post it again.
Thanks,
Alam.
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-05-10 9:02 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-19 9:36 [PATCH V2] mtd: rawnand: qcom: Implement exec_op() Md Sadre Alam
2023-04-20 3:05 ` kernel test robot
2023-04-24 7:35 ` kernel test robot
2023-05-09 11:08 ` Manivannan Sadhasivam
2023-05-10 9:01 ` Md Sadre Alam
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).