Hi Abhishek, On Wed, 4 Apr 2018 18:12:24 +0530, Abhishek Sahu <[email protected]> wrote:
> This patch does minor code reorganization for raw reads. > Currently the raw read is required for complete page but for > subsequent patches related with erased codeword bit flips > detection, only few CW should be read. So, this patch adds > helper function and introduces the read CW bitmask which > specifies which CW reads are required in complete page. I am not sure this is the right approach for subpage reads. If the controller supports it, you should just enable it in chip->options. > > Signed-off-by: Abhishek Sahu <[email protected]> > --- > drivers/mtd/nand/qcom_nandc.c | 186 > +++++++++++++++++++++++++----------------- > 1 file changed, 110 insertions(+), 76 deletions(-) > > diff --git a/drivers/mtd/nand/qcom_nandc.c b/drivers/mtd/nand/qcom_nandc.c > index 40c790e..f5d1fa4 100644 > --- a/drivers/mtd/nand/qcom_nandc.c > +++ b/drivers/mtd/nand/qcom_nandc.c > @@ -1590,6 +1590,114 @@ static int check_flash_errors(struct qcom_nand_host > *host, int cw_cnt) > } > > /* > + * Helper to perform the page raw read operation. The read_cw_mask will be > + * used to specify the codewords for which the data should be read. The > + * single page contains multiple CW. Sometime, only few CW data is required > + * in complete page. Also, start address will be determined with > + * this CW mask to skip unnecessary data copy from NAND flash device. Then, > + * actual data copy from NAND controller to data buffer will be done only > + * for the CWs which have the mask set. > + */ > +static int > +nandc_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip, > + u8 *data_buf, u8 *oob_buf, > + int page, unsigned long read_cw_mask) > +{ > + 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; > + int i, ret; > + int read_loc, start_step, last_step; > + > + nand_read_page_op(chip, page, 0, NULL, 0); > + > + host->use_ecc = false; > + start_step = ffs(read_cw_mask) - 1; > + last_step = fls(read_cw_mask); > + > + clear_bam_transaction(nandc); > + set_address(host, host->cw_size * start_step, page); > + update_rw_regs(host, last_step - start_step, true); > + config_nand_page_read(nandc); > + > + for (i = start_step; i < last_step; i++) { > + int data_size1, data_size2, oob_size1, oob_size2; > + int reg_off = FLASH_BUF_ACC; > + > + data_size1 = mtd->writesize - host->cw_size * (ecc->steps - 1); > + oob_size1 = host->bbm_size; > + > + if (i == (ecc->steps - 1)) { > + data_size2 = ecc->size - data_size1 - > + ((ecc->steps - 1) << 2); > + oob_size2 = (ecc->steps << 2) + host->ecc_bytes_hw + > + host->spare_bytes; > + } else { > + data_size2 = host->cw_data - data_size1; > + oob_size2 = host->ecc_bytes_hw + host->spare_bytes; > + } > + > + /* > + * Don't perform actual data copy from NAND controller to data > + * buffer through DMA for this codeword > + */ > + if (!(read_cw_mask & BIT(i))) { > + if (nandc->props->is_bam) > + nandc_set_read_loc(nandc, 0, 0, 0, 1); > + > + config_nand_cw_read(nandc, false); > + > + data_buf += data_size1 + data_size2; > + oob_buf += oob_size1 + oob_size2; > + > + continue; > + } > + > + if (nandc->props->is_bam) { > + read_loc = 0; > + nandc_set_read_loc(nandc, 0, read_loc, data_size1, 0); > + read_loc += data_size1; > + > + nandc_set_read_loc(nandc, 1, read_loc, oob_size1, 0); > + read_loc += oob_size1; > + > + nandc_set_read_loc(nandc, 2, read_loc, data_size2, 0); > + read_loc += data_size2; > + > + nandc_set_read_loc(nandc, 3, read_loc, oob_size2, 1); > + } > + > + config_nand_cw_read(nandc, false); > + > + read_data_dma(nandc, reg_off, data_buf, data_size1, 0); > + reg_off += data_size1; > + data_buf += data_size1; > + > + read_data_dma(nandc, reg_off, oob_buf, oob_size1, 0); > + reg_off += oob_size1; > + oob_buf += oob_size1; > + > + read_data_dma(nandc, reg_off, data_buf, data_size2, 0); > + reg_off += data_size2; > + data_buf += data_size2; > + > + read_data_dma(nandc, reg_off, oob_buf, oob_size2, 0); > + oob_buf += oob_size2; > + } > + > + ret = submit_descs(nandc); > + if (ret) > + dev_err(nandc->dev, "failure to read raw page\n"); > + > + free_descs(nandc); > + > + if (!ret) > + ret = check_flash_errors(host, last_step - start_step); > + > + return 0; > +} > + > +/* > * reads back status registers set by the controller to notify page read > * errors. this is equivalent to what 'ecc->correct()' would do. > */ > @@ -1839,82 +1947,8 @@ static int qcom_nandc_read_page_raw(struct mtd_info > *mtd, > struct nand_chip *chip, uint8_t *buf, > int oob_required, int page) > { > - struct qcom_nand_host *host = to_qcom_nand_host(chip); > - struct qcom_nand_controller *nandc = get_qcom_nand_controller(chip); > - u8 *data_buf, *oob_buf; > - struct nand_ecc_ctrl *ecc = &chip->ecc; > - int i, ret; > - int read_loc; > - > - nand_read_page_op(chip, page, 0, NULL, 0); > - data_buf = buf; > - oob_buf = chip->oob_poi; > - > - host->use_ecc = false; > - > - clear_bam_transaction(nandc); > - update_rw_regs(host, ecc->steps, true); > - config_nand_page_read(nandc); > - > - for (i = 0; i < ecc->steps; i++) { > - int data_size1, data_size2, oob_size1, oob_size2; > - int reg_off = FLASH_BUF_ACC; > - > - data_size1 = mtd->writesize - host->cw_size * (ecc->steps - 1); > - oob_size1 = host->bbm_size; > - > - if (i == (ecc->steps - 1)) { > - data_size2 = ecc->size - data_size1 - > - ((ecc->steps - 1) << 2); > - oob_size2 = (ecc->steps << 2) + host->ecc_bytes_hw + > - host->spare_bytes; > - } else { > - data_size2 = host->cw_data - data_size1; > - oob_size2 = host->ecc_bytes_hw + host->spare_bytes; > - } > - > - if (nandc->props->is_bam) { > - read_loc = 0; > - nandc_set_read_loc(nandc, 0, read_loc, data_size1, 0); > - read_loc += data_size1; > - > - nandc_set_read_loc(nandc, 1, read_loc, oob_size1, 0); > - read_loc += oob_size1; > - > - nandc_set_read_loc(nandc, 2, read_loc, data_size2, 0); > - read_loc += data_size2; > - > - nandc_set_read_loc(nandc, 3, read_loc, oob_size2, 1); > - } > - > - config_nand_cw_read(nandc, false); > - > - read_data_dma(nandc, reg_off, data_buf, data_size1, 0); > - reg_off += data_size1; > - data_buf += data_size1; > - > - read_data_dma(nandc, reg_off, oob_buf, oob_size1, 0); > - reg_off += oob_size1; > - oob_buf += oob_size1; > - > - read_data_dma(nandc, reg_off, data_buf, data_size2, 0); > - reg_off += data_size2; > - data_buf += data_size2; > - > - read_data_dma(nandc, reg_off, oob_buf, oob_size2, 0); > - oob_buf += oob_size2; > - } > - > - ret = submit_descs(nandc); > - if (ret) > - dev_err(nandc->dev, "failure to read raw page\n"); > - > - free_descs(nandc); > - > - if (!ret) > - ret = check_flash_errors(host, ecc->steps); > - > - return 0; > + return nandc_read_page_raw(mtd, chip, buf, chip->oob_poi, page, > + BIT(chip->ecc.steps) - 1); I don't understand this. chip->ecc.steps is constant, right? So you always ask for the same subpage? > } > > /* implements ecc->read_oob() */ -- Miquel Raynal, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com
