by Miquel Raynal

[permalink] [raw]

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

Hi Md,

[email protected] wrote on Wed, 24 May 2023 14:54:34 +0530:

> On 5/22/2023 7:15 PM, Miquel Raynal wrote:
> > Hi Md,
> >
> > [email protected] 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 <[email protected]>
> >> Signed-off-by: Sricharan Ramabadhran <[email protected]>
> >> Signed-off-by: Md Sadre Alam <[email protected]>
> >> ---
> >> 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