On 01/27/2015 02:35 AM, Kevin Cernekee wrote:
On Wed, Jan 21, 2015 at 10:36 PM, Archit Taneja <architt@xxxxxxxxxxxxxx> wrote:
On 01/21/2015 06:24 AM, Daniel Ehrenberg wrote:
On Fri, Jan 16, 2015 at 6:48 AM, Archit Taneja <architt@xxxxxxxxxxxxxx>
wrote:
+/*
+ * the bad block marker is readable only when we read the page with ECC
+ * disabled. all the read/write commands like NAND_CMD_READOOB,
NAND_CMD_READ0
+ * and NAND_CMD_PAGEPROG are executed in the driver with ECC enabled.
therefore,
+ * the default nand helper functions to detect a bad block or mark a bad
block
+ * can't be used.
+ */
+static int qcom_nandc_block_bad(struct mtd_info *mtd, loff_t ofs, int
getchip)
+{
+ int page, r, mark, bad = 0;
+ struct nand_chip *chip = mtd->priv;
+ struct nand_ecc_ctrl *ecc = &chip->ecc;
+ int cwperpage = ecc->steps;
+ struct qcom_nandc_data *this = chip->priv;
+ u32 flash_status;
+
+ pre_command(this, NAND_CMD_NONE);
+
+ page = (int)(ofs >> chip->page_shift) & chip->pagemask;
+
+ /*
+ * configure registers for a raw page read, the address is set to
the
+ * beginning of the last codeword
+ */
+ this->use_ecc = false;
+ set_address(this, this->cw_size * (cwperpage - 1), page);
+
+ /* we just read one codeword that contains the bad block marker
*/
+ update_rw_regs(this, 1, true);
+
+ read_cw(this, this->cw_size, 0);
+
+ r = submit_descs(this);
+ if (r) {
+ dev_err(this->dev, "error submitting descs\n");
+ goto err;
+ }
+
+ flash_status = this->reg_read_buf[0];
+
+ /*
+ * unable to read the marker successfully, is that sufficient to
report
+ * the block as bad?
+ */
+ if (flash_status & (FS_OP_ERR | FS_MPU_ERR)) {
+ dev_warn(this->dev, "error while reading bad block
mark\n");
+ goto err;
+ }
+
+ mark = mtd->writesize - (this->cw_size * (cwperpage - 1));
+
+ if (chip->options & NAND_BUSWIDTH_16)
+ bad = this->data_buffer[mark] != 0xff ||
+ this->data_buffer[mark + 1] != 0xff;
+
+ bad = this->data_buffer[mark] != 0xff;
+err:
+ free_descs(this);
+
+ return bad;
+}
+
+static int qcom_nandc_block_markbad(struct mtd_info *mtd, loff_t ofs)
+{
+ int page, r;
+ struct nand_chip *chip = mtd->priv;
+ struct nand_ecc_ctrl *ecc = &chip->ecc;
+ int cwperpage = ecc->steps;
+ struct qcom_nandc_data *this = chip->priv;
+ u32 flash_status;
+
+ pre_command(this, NAND_CMD_NONE);
+
+ /* fill our internal buffer's oob area with 0's */
+ memset(this->data_buffer, 0x00, mtd->writesize + mtd->oobsize);
+
+ page = (int)(ofs >> chip->page_shift) & chip->pagemask;
+
+ this->use_ecc = false;
+ set_address(this, this->cw_size * (cwperpage - 1), page);
+
+ /* we just write to one codeword that contains the bad block
marker*/
+ update_rw_regs(this, 1, false);
+
+ /*
+ * overwrite the last codeword with 0s, this will result in
setting
+ * the bad block marker to 0 too
+ */
+ write_cw(this, this->cw_size, 0);
+
+ r = submit_descs(this);
+ if (r) {
+ dev_err(this->dev, "error submitting descs\n");
+ r = -EIO;
+ goto err;
+ }
+
+ flash_status = this->reg_read_buf[0];
+
+ if (flash_status & (FS_OP_ERR | FS_MPU_ERR))
+ r = -EIO;
+
+err:
+ free_descs(this);
+
+ return r;
+}
Would it be possible to refactor this code so that it implements
generic read_oob_raw() and write_oob_raw() callbacks, which can
read/write arbitrary uncorrected OOB bytes instead of just the BBI?
Or is the controller limited in which OOB bytes can be directly
accessed?
When accessing the page with ECC disabled, there doesn't seem to be any
limitation.
We have 528/32 bytes per codeword, and we can access all of it in raw mode.
One advantage of implementing the generic raw read/write callbacks is
that the MTD subsystem already has logic to handle old NAND chips with
different BBI positions, bitflips in the BBI, and other corner cases.
Looks like this code marks block bad and reads bad block information
based on information in the OOB area. And in qcom_nandc_init,
NAND_SKIP_BBTSCAN is set, meaning that this is the code used in
practice on the chip in the mtd_block_isbad path. Can this driver be
used with an on-flash OOB table by editing the init function's chip
flags, or would it need more significant changes to allow that?
The code doesn't exactly read the OOB area. When the ECC HW block is
enabled, the bad block isn't in either oob or data area! We can access it
only when ECC is disabled. With ECC disabled, the bad block marker lies in
the last codeword somewhere in the middle of the user data. For the
mtd_read_oob()/write_oob() functions, we have the ECC always enabled, hence,
we never access the bad block marker through them at all.
Creating an on-flash bad block table won't work right now. The reason is
that the nand_bbt library assumes that it can find the bad block marker by
reading oob. While creating a bbt in memory, it scans the device for bad
blocks using the function scan_block_fast(). This would currently result in
not reading the bad block marker, and therefore break things.
I'm trying to find out if there is a way by which the controller can access
the bad block marker with ECC HW enabled. If that works, we can use the
nand_bbt helper as is. For now, I wanted to get the driver upstream without
the BBT functionality.
So, back in commit a7e68834fc2739 ("mtd: nand: use ECC, if present,
when scanning OOB"), the BBT code was changed to use MTD_OPS_PLACE_OOB
instead of MTD_OPS_RAW, because some controllers offer the ability to
provide corrected OOB data and we wanted to use it if possible. The
changelog notes that many existing drivers still disabled ECC when
reading OOB in MTD_OPS_PLACE_OOB mode.
Now it sounds like the qcom_nandc controller can either provide access
to some corrected OOB bytes (not including the BBI) when
MTD_OPS_PLACE_OOB is used, or all (?) uncorrected OOB bytes (including
the BBI) when MTD_OPS_RAW is used.
That's right.
Two possible options are:
1) qcom_nandc should just disable ECC when performing OOB reads/writes
in MTD_OPS_PLACE_OOB mode
What prevented me from doing this was how the
chip->ecc.read_page/write_page funcs are defined. They take
the param called oob_required.
If oob_required is set, we would need to read/write page contents with
ECC disabled too. Or, perform 2 rounds of acceses. One for the page
contents with ECC enabled, and other for oob with ECC disabled.
I don't know how often oob_required is set by the mtd subsystem, if it's
a path that is never exercised, we could try this option.
2) MTD should be made aware of whether the controller can access the
BBI in MTD_OPS_PLACE_OOB mode, and fall back to MTD_OPS_RAW mode if
the hardware cannot do so
That sounds like a good option. A NAND chip option mentioning this
capability could be helpful here.
Thanks,
Archit
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
