From: Boris Brezillon <boris.brezillon@bootlin.com>
To: Miquel Raynal <miquel.raynal@bootlin.com>
Cc: Abhishek Sahu <absahu@codeaurora.org>,
David Woodhouse <dwmw2@infradead.org>,
Brian Norris <computersforpeace@gmail.com>,
Marek Vasut <marek.vasut@gmail.com>,
Richard Weinberger <richard@nod.at>,
linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-mtd@lists.infradead.org, Andy Gross <andy.gross@linaro.org>
Subject: Re: [PATCH 2/5] mtd: rawnand: qcom: remove driver specific block_markbad function
Date: Wed, 18 Jul 2018 23:43:58 +0200 [thread overview]
Message-ID: <20180718234358.6bb5e8a0@bbrezillon> (raw)
In-Reply-To: <20180718232350.3eaade9a@xps13>
On Wed, 18 Jul 2018 23:23:50 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> Boris,
>
> Can you please check the change in qcom_nandc_write_oob() is
> valid? I think it is but as this is a bit of a hack I prefer double checking.
Indeed, it's hack-ish.
>
> Thanks,
> Miquèl
>
>
> Abhishek Sahu <absahu@codeaurora.org> wrote on Fri, 6 Jul 2018
> 13:21:56 +0530:
>
> > The NAND base layer calls write_oob() by setting bytes at
> > chip->badblockpos with value non 0xFF for updating bad block status.
> > The QCOM NAND controller skips the bad block bytes while doing normal
> > write with ECC enabled. When initial support for this driver was
> > added, the driver specific function was added temporarily for
> > block_markbad() with assumption to change for raw read in NAND base
> > layer. Moving to raw read for block_markbad() seems to take more time
> > so this patch removes driver specific block_markbad() function by
> > using following HACK in write_oob() function.
> >
> > Check for BBM bytes in OOB and accordingly do raw write for updating
> > BBM bytes in NAND flash or normal write for updating available OOB
> > bytes.
Why don't we change that instead of patching the qcom driver to guess
when the core tries to mark a block bad? If you're afraid of breaking
existing drivers that might rely on the "write/read BBM in non-raw
mode" solution (I'm sure some drivers are), you can always add a new
flag in chip->options (NAND_ACCESS_BBM_IN_RAW_MODE) and only use raw
accessors when this flag is set.
> >
> > Signed-off-by: Abhishek Sahu <absahu@codeaurora.org>
> > ---
> > drivers/mtd/nand/raw/qcom_nandc.c | 103 +++++++++++++++-----------------------
> > 1 file changed, 40 insertions(+), 63 deletions(-)
> >
> > diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
> > index ea253ac..df12cf3 100644
> > --- a/drivers/mtd/nand/raw/qcom_nandc.c
> > +++ b/drivers/mtd/nand/raw/qcom_nandc.c
> > @@ -2138,28 +2138,57 @@ static int qcom_nandc_write_oob(struct mtd_info *mtd, struct nand_chip *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 *oob = chip->oob_poi;
> > - int data_size, oob_size;
> > + u8 *oob = chip->oob_poi, bbm_byte;
> > + int data_size, oob_size, bbm_offset, write_size;
> > int ret;
> >
> > - host->use_ecc = true;
> > clear_bam_transaction(nandc);
> >
> > - /* calculate the data and oob size for the last codeword/step */
> > - data_size = ecc->size - ((ecc->steps - 1) << 2);
> > - oob_size = mtd->oobavail;
> > + /*
> > + * The NAND base layer calls ecc->write_oob() by setting bytes at
> > + * chip->badblockpos (chip->badblockpos will be 0 for QCOM NAND
> > + * controller layout) in OOB buffer with value other that 0xFF
> > + * for updating bad block status. QCOM NAND controller skips
> > + * BBM bytes while writing with ECC, so following HACK has been
> > + * added in this function for using generic block_markbad() function.
> > + *
> > + * Check for BBM bytes in OOB and accordingly do raw write for
> > + * updating BBM bytes in NAND flash or normal write with ECC for
> > + * updating available OOB bytes.
> > + */
> > + bbm_byte = oob[0];
> > + if (chip->options & NAND_BUSWIDTH_16)
> > + bbm_byte &= oob[1];
> >
> > - memset(nandc->data_buffer, 0xff, host->cw_data);
> > - /* override new oob content to last codeword */
> > - mtd_ooblayout_get_databytes(mtd, nandc->data_buffer + data_size, oob,
> > - 0, mtd->oobavail);
> > + /* Write BBM bytes by doing raw write. */
> > + if (bbm_byte != 0xff) {
> > + host->use_ecc = false;
> > + memset(nandc->data_buffer, 0xff, host->cw_size);
> > + /* Determine the BBM bytes position and update the same */
> > + bbm_offset = mtd->writesize - host->cw_size * (ecc->steps - 1);
> > + memcpy(nandc->data_buffer + bbm_offset, oob, host->bbm_size);
> > + write_size = host->cw_size;
> > + /* Write OOB bytes by doing normal write with ECC */
> > + } else {
> > + host->use_ecc = true;
> > + /* calculate the data and oob size for the last codeword/step */
> > + data_size = ecc->size - ((ecc->steps - 1) << 2);
> > + oob_size = mtd->oobavail;
> > +
> > + memset(nandc->data_buffer, 0xff, host->cw_data);
> > + /* override new oob content to last codeword */
> > + mtd_ooblayout_get_databytes(mtd, nandc->data_buffer + data_size,
> > + oob, 0, mtd->oobavail);
> > +
> > + write_size = data_size + oob_size;
> > + }
> >
> > set_address(host, host->cw_size * (ecc->steps - 1), page);
> > update_rw_regs(host, 1, false);
> >
> > config_nand_page_write(nandc);
> > write_data_dma(nandc, FLASH_BUF_ACC,
> > - nandc->data_buffer, data_size + oob_size, 0);
> > + nandc->data_buffer, write_size, 0);
> > config_nand_cw_write(nandc);
> >
> > ret = submit_descs(nandc);
> > @@ -2174,48 +2203,6 @@ static int qcom_nandc_write_oob(struct mtd_info *mtd, struct nand_chip *chip,
> > return nand_prog_page_end_op(chip);
> > }
> >
> > -static int qcom_nandc_block_markbad(struct mtd_info *mtd, loff_t ofs)
> > -{
> > - struct nand_chip *chip = mtd_to_nand(mtd);
> > - 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 page, ret;
> > -
> > - clear_read_regs(nandc);
> > - clear_bam_transaction(nandc);
> > -
> > - /*
> > - * to mark the BBM as bad, we flash the entire last codeword with 0s.
> > - * we don't care about the rest of the content in the codeword since
> > - * we aren't going to use this block again
> > - */
> > - memset(nandc->data_buffer, 0x00, host->cw_size);
> > -
> > - page = (int)(ofs >> chip->page_shift) & chip->pagemask;
> > -
> > - /* prepare write */
> > - host->use_ecc = false;
> > - set_address(host, host->cw_size * (ecc->steps - 1), page);
> > - update_rw_regs(host, 1, false);
> > -
> > - config_nand_page_write(nandc);
> > - write_data_dma(nandc, FLASH_BUF_ACC,
> > - nandc->data_buffer, host->cw_size, 0);
> > - config_nand_cw_write(nandc);
> > -
> > - ret = submit_descs(nandc);
> > -
> > - free_descs(nandc);
> > -
> > - if (ret) {
> > - dev_err(nandc->dev, "failure to update BBM\n");
> > - return -EIO;
> > - }
> > -
> > - return nand_prog_page_end_op(chip);
> > -}
> > -
> > /*
> > * the three functions below implement chip->read_byte(), chip->read_buf()
> > * and chip->write_buf() respectively. these aren't used for
> > @@ -2757,16 +2744,6 @@ static int qcom_nand_host_init(struct qcom_nand_controller *nandc,
> > chip->set_features = nand_get_set_features_notsupp;
> > chip->get_features = nand_get_set_features_notsupp;
> >
> > - /*
> > - * the bad block marker is writable only when we write the last codeword
> > - * of a page with ECC disabled. currently, the nand_base and nand_bbt
> > - * helpers don't allow us to write BB from a nand chip with ECC
> > - * disabled (MTD_OPS_PLACE_OOB is set by default). use the block_markbad
> > - * helpers until we permanently switch to using
> > - * MTD_OPS_RAW for all drivers (with the help of badblockbits)
> > - */
> > - chip->block_markbad = qcom_nandc_block_markbad;
> > -
> > chip->controller = &nandc->controller;
> > chip->options |= NAND_NO_SUBPAGE_WRITE | NAND_USE_BOUNCE_BUFFER |
> > NAND_SKIP_BBTSCAN;
>
>
>
next prev parent reply other threads:[~2018-07-18 21:44 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-07-06 7:51 [PATCH 0/5] Update for removing driver specific BBM functions Abhishek Sahu
2018-07-06 7:51 ` [PATCH 1/5] mtd: rawnand: qcom: remove driver specific bad block check function Abhishek Sahu
2018-07-06 7:51 ` [PATCH 2/5] mtd: rawnand: qcom: remove driver specific block_markbad function Abhishek Sahu
2018-07-18 21:23 ` Miquel Raynal
2018-07-18 21:43 ` Boris Brezillon [this message]
2018-07-20 12:16 ` Abhishek Sahu
2018-07-20 13:03 ` Boris Brezillon
2018-11-04 15:56 ` Boris Brezillon
2018-11-09 6:18 ` Abhishek Sahu
2018-07-06 7:51 ` [PATCH 3/5] mtd: rawnand: qcom: fix NAND register write errors Abhishek Sahu
2018-07-06 7:51 ` [PATCH 4/5] mtd: rawnand: qcom: update BBT related flags Abhishek Sahu
2018-07-18 21:15 ` Miquel Raynal
2018-07-18 21:36 ` Boris Brezillon
2018-07-18 21:41 ` Miquel Raynal
2018-07-18 21:42 ` Miquel Raynal
2018-07-20 7:14 ` Abhishek Sahu
2018-07-06 7:51 ` [PATCH 5/5] mtd: rawnand: qcom: reorganization by removing read/write helpers Abhishek Sahu
2018-07-18 21:29 ` Miquel Raynal
2018-07-18 21:54 ` Boris Brezillon
2018-07-28 10:20 ` Abhishek Sahu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180718234358.6bb5e8a0@bbrezillon \
--to=boris.brezillon@bootlin.com \
--cc=absahu@codeaurora.org \
--cc=andy.gross@linaro.org \
--cc=computersforpeace@gmail.com \
--cc=dwmw2@infradead.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mtd@lists.infradead.org \
--cc=marek.vasut@gmail.com \
--cc=miquel.raynal@bootlin.com \
--cc=richard@nod.at \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).