On 1/8/2016 1:31 PM, Boris Brezillon wrote:
On Fri, 8 Jan 2016 12:03:25 +0530
Archit Taneja <architt@xxxxxxxxxxxxxx> wrote:
Hi,
On 1/6/2016 10:35 PM, Boris Brezillon wrote:
Hi Archit,
On Tue, 5 Jan 2016 10:55:00 +0530
Archit Taneja <architt@xxxxxxxxxxxxxx> wrote:
The Qualcomm NAND controller is found in SoCs like IPQ806x, MSM7xx,
MDM9x15 series.
It exists as a sub block inside the IPs EBI2 (External Bus Interface 2)
and QPIC (Qualcomm Parallel Interface Controller). These IPs provide a
broader interface for external slow peripheral devices such as LCD and
NAND/NOR flash memory or SRAM like interfaces.
We add support for the NAND controller found within EBI2. For the SoCs
of our interest, we only use the NAND controller within EBI2. Therefore,
it's safe for us to assume that the NAND controller is a standalone block
within the SoC.
The controller supports 512B, 2kB, 4kB and 8kB page 8-bit and 16-bit NAND
flash devices. It contains a HW ECC block that supports BCH ECC (4, 8 and
16 bit correction/step) and RS ECC(4 bit correction/step) that covers main
and spare data. The controller contains an internal 512 byte page buffer
to which we read/write via DMA. The EBI2 type NAND controller uses ADM DMA
for register read/write and data transfers. The controller performs page
reads and writes at a codeword/step level of 512 bytes. It can support up
to 2 external chips of different configurations.
The driver prepares register read and write configuration descriptors for
each codeword, followed by data descriptors to read or write data from the
controller's internal buffer. It uses a single ADM DMA channel that we get
via dmaengine API. The controller requires 2 ADM CRCIs for command and
data flow control. These are passed via DT.
The ecc layout used by the controller is syndrome like, but we can't use
the standard syndrome ecc ops because of several reasons. First, the amount
of data bytes covered by ecc isn't same in each step. Second, writing to
free oob space requires us writing to the entire step in which the oob
lies. This forces us to create our own ecc ops.
One more difference is how the controller accesses the bad block marker.
The controller ignores reading the marker when ECC is enabled. ECC needs
to be explicity disabled to read or write to the bad block marker. The
nand_bbt helpers library hence can't access BBMs for the controller.
For now, we skip the creation of BBT and populate chip->block_bad and
chip->block_markbad helpers instead.
Reviewed-by: Andy Gross <agross@xxxxxxxxxxxxxx>
Reviewed-by: Stephen Boyd <sboyd@xxxxxxxxxxxxxx>
Signed-off-by: Stephen Boyd <sboyd@xxxxxxxxxxxxxx>
Signed-off-by: Archit Taneja <architt@xxxxxxxxxxxxxx>
---
v5:
- split chip/controller structs
- simplify layout by considering reserved bytes as part of ECC
- create ecc layouts automatically
- implement block_bad and block_markbad chip ops instead of
- read_oob_raw/write_oob_raw ecc ops to access BBMs.
- Add NAND_SKIP_BBTSCAN flag until we get badblockbits support.
- misc clean ups
v4:
- Shrink submit_descs
- add desc list node at the end of dma_prep_desc
- Endianness and warning fixes
- Add Stephen's Signed-off since he provided a patch to fix
endianness problems
v3:
- Refactor dma functions for maximum reuse
- Use dma_slave_confing on stack
- optimize and clean upempty_page_fixup using memchr_inv
- ensure portability with dma register reads using le32_* funcs
- use NAND_USE_BOUNCE_BUFFER instead of doing it ourselves
- fix handling of return values of dmaengine funcs
- constify wherever possible
- Remove dependency on ADM DMA in Kconfig
- Misc fixes and clean ups
v2:
- Use new BBT flag that allows us to read BBM in raw mode
- reduce memcpy-s in the driver
- some refactor and clean ups because of above changes
drivers/mtd/nand/Kconfig | 7 +
drivers/mtd/nand/Makefile | 1 +
drivers/mtd/nand/qcom_nandc.c | 1981 +++++++++++++++++++++++++++++++++++++++++
3 files changed, 1989 insertions(+)
create mode 100644 drivers/mtd/nand/qcom_nandc.c
diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index 95b8d2b..2fccdfb 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -546,4 +546,11 @@ config MTD_NAND_HISI504
help
Enables support for NAND controller on Hisilicon SoC Hip04.
+config MTD_NAND_QCOM
+ tristate "Support for NAND on QCOM SoCs"
+ depends on ARCH_QCOM
+ help
+ Enables support for NAND flash chips on SoCs containing the EBI2 NAND
+ controller. This controller is found on IPQ806x SoC.
+
endif # MTD_NAND
diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
index 2c7f014..9450cdc 100644
--- a/drivers/mtd/nand/Makefile
+++ b/drivers/mtd/nand/Makefile
@@ -55,5 +55,6 @@ obj-$(CONFIG_MTD_NAND_BCM47XXNFLASH) += bcm47xxnflash/
obj-$(CONFIG_MTD_NAND_SUNXI) += sunxi_nand.o
obj-$(CONFIG_MTD_NAND_HISI504) += hisi504_nand.o
obj-$(CONFIG_MTD_NAND_BRCMNAND) += brcmnand/
+obj-$(CONFIG_MTD_NAND_QCOM) += qcom_nandc.o
nand-objs := nand_base.o nand_bbt.o nand_timings.o
diff --git a/drivers/mtd/nand/qcom_nandc.c b/drivers/mtd/nand/qcom_nandc.c
new file mode 100644
index 0000000..a4dd922
--- /dev/null
+++ b/drivers/mtd/nand/qcom_nandc.c
[...]
+
+/*
+ * when using RS ECC, the NAND controller flags an error when reading an
+ * erased page. however, there are special characters at certain offsets when
+ * we read the erased page. we check here if the page is really empty. if so,
+ * we replace the magic characters with 0xffs
+ */
+static bool empty_page_fixup(struct qcom_nand_host *host, u8 *data_buf)
+{
+ struct qcom_nand_controller *nandc = host->nandc;
+ struct nand_chip *chip = &host->chip;
+ struct mtd_info *mtd = nand_to_mtd(chip);
+ int cwperpage = chip->ecc.steps;
+ u8 orig1[MAX_NUM_STEPS], orig2[MAX_NUM_STEPS];
+ int i, j;
+
+ /* if BCH is enabled, HW will take care of detecting erased pages */
+ if (host->bch_enabled || !host->use_ecc)
+ return false;
+
+ for (i = 0; i < cwperpage; i++) {
+ u8 *empty1, *empty2;
+ u32 flash_status = le32_to_cpu(nandc->reg_read_buf[3 * i]);
+
+ /*
+ * an erased page flags an error in NAND_FLASH_STATUS, check if
+ * the page is erased by looking for 0x54s at offsets 3 and 175
+ * from the beginning of each codeword
+ */
+ if (!(flash_status & FS_OP_ERR))
+ break;
+
+ empty1 = &data_buf[3 + i * host->cw_data];
+ empty2 = &data_buf[175 + i * host->cw_data];
+
+ /*
+ * if the error wasn't because of an erased page, bail out and
+ * and let someone else do the error checking
+ */
+ if ((*empty1 == 0x54 && *empty2 == 0xff) ||
+ (*empty1 == 0xff && *empty2 == 0x54)) {
+ orig1[i] = *empty1;
+ orig2[i] = *empty2;
+
+ *empty1 = 0xff;
+ *empty2 = 0xff;
+ } else {
+ break;
+ }
+ }
+
+ if (i < cwperpage || memchr_inv(data_buf, 0xff, mtd->writesize))
+ goto not_empty;
+
+ /*
+ * tell the caller that the page was empty and is fixed up, so that
+ * parse_read_errors() doesn't think it's an error
+ */
+ return true;
+
+not_empty:
+ /* restore original values if not empty*/
+ for (j = 0; j < i; j++) {
+ data_buf[3 + j * host->cw_data] = orig1[j];
+ data_buf[175 + j * host->cw_data] = orig2[j];
+ }
+
+ return false;
+}
This empty page detection seems a bit complicated to me. Could you
consider using nand_check_erased_ecc_chunk() to check is the chunk is
containing 0xff data instead of implementing your own logic?
We wouldn't be able to use nand_check_erased_ecc_chunk directly because
there are certain bytes in each chunk that are intentionally not 0xffs.
But I could make it a two step process, where I first override those
magic bytes with 0xffs, and then use nand_check_erased_ecc_chunk. That
may not reduce tons of code, but would atleast consider possibility
of bitflips in erased pages, which the driver currently doesn't do.
Too bad your ECC engines decides to fix some bits even when it cannot
fix all of them. Are you sure there's no flag to disable this behavior?
Usually ECC engines just flag the data chunk as uncorrectable and leave
its data untouched.
I don't think the engine is actually writing to the chunk at any
point when reading an erased chunk.
This whole method adopted by the engine is just a way of flagging
to us that it read an erased chunk. Once it reads an erased chunk, it
flags an error in one of the controller registers, and replaces
offsets at 3 and 175 in its internal buffer.
I am not sure why the HW didn't just provide another bit field which
notifies that it it read an erased page. This is what is done in the
future version of this controller, and we don't need to any of this
stuff.
I don't think this behaviour can be disabled as such.
Anyway, are you sure your heuristic to detect erased pages is 100%
sure. What if you really have a lot of bitflips but the values @3 and
@175 are still 0x54 and 0xff because those are the bitflips the engine
decided to fix?
As pointed above, I don't think 0x54 and 0xff are explicitly written
to the nand flash chip.
Actually, I don't know if you have any other option but to re-read the
page in raw mode and use nand_check_erased_ecc_chunk(). It adds a
considerable overhead, but at least you're sure to detect real empty
pages without any false positive.
I am not sure why this is needed. The driver code does parse the entire
page and reports that it isn't an erased page if all of them (apart
from the given offsets) weren't 0xffs.
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
