Hi Archit,
On Wed, 3 Feb 2016 14:29:50 +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>
> Signed-off-by: Stephen Boyd <sboyd@xxxxxxxxxxxxxx>
> Signed-off-by: Archit Taneja <architt@xxxxxxxxxxxxxx>
> ---
> v8:
> - Use nand_check_erased_ecc_chunk in the right manner as
> suggested by Boris (place the check when we see uncorrectable
> errors).
> - Rewrite the empty_page_fixup code such that we check for a
> codeword being erased rather than the entire page. This simplifies
> the code and we can now place it in parse_read_errors().
> - Introduce raw page access functions. This results in making some
> modifications in the existing ECC page access ops too, since the
> layout now also considers the real/dummy bad block markers. Explained
> in comments.
>
> v7:
> - Incorporated missing/new comments by Boris
> - Cleaned up some strict checkpatch warnings
>
> v6:
> - Fix up erased page parsing. Use nand_check_erased_ecc_chunk to
> return corrected bitflips in an erased page.
> - Fix whitespace issues
> - Update compatible tring to something more specific
>
> 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 | 2224 +++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 2232 insertions(+)
> create mode 100644 drivers/mtd/nand/qcom_nandc.c
>
[...]
> +static int qcom_nand_host_setup(struct qcom_nand_host *host)
> +{
> + struct nand_chip *chip = &host->chip;
> + struct mtd_info *mtd = nand_to_mtd(chip);
> + struct nand_ecc_ctrl *ecc = &chip->ecc;
> + struct qcom_nand_controller *nandc = get_qcom_nand_controller(chip);
> + int cwperpage, bad_block_byte;
> + bool wide_bus;
> + int ecc_mode = 1;
> +
> + /*
> + * the controller requires each step consists of 512 bytes of data.
> + * bail out if DT has populated a wrong step size.
> + */
> + if (ecc->size != NANDC_STEP_SIZE) {
> + dev_err(nandc->dev, "invalid ecc size\n");
> + return -EINVAL;
> + }
> +
> + wide_bus = chip->options & NAND_BUSWIDTH_16 ? true : false;
> +
> + if (ecc->strength >= 8) {
> + /* 8 bit ECC defaults to BCH ECC on all platforms */
> + host->bch_enabled = true;
> + ecc_mode = 1;
> +
> + if (wide_bus) {
> + host->ecc_bytes_hw = 14;
> + host->spare_bytes = 0;
> + host->bbm_size = 2;
> + } else {
> + host->ecc_bytes_hw = 13;
> + host->spare_bytes = 2;
> + host->bbm_size = 1;
> + }
> + } else {
> + /*
> + * if the controller supports BCH for 4 bit ECC, the controller
> + * uses lesser bytes for ECC. If RS is used, the ECC bytes is
> + * always 10 bytes
> + */
> + if (nandc->ecc_modes & ECC_BCH_4BIT) {
> + /* BCH */
> + host->bch_enabled = true;
> + ecc_mode = 0;
> +
> + if (wide_bus) {
> + host->ecc_bytes_hw = 8;
> + host->spare_bytes = 2;
> + host->bbm_size = 2;
> + } else {
> + host->ecc_bytes_hw = 7;
> + host->spare_bytes = 4;
> + host->bbm_size = 1;
> + }
> + } else {
> + /* RS */
> + host->ecc_bytes_hw = 10;
> +
> + if (wide_bus) {
> + host->spare_bytes = 0;
> + host->bbm_size = 2;
> + } else {
> + host->spare_bytes = 1;
> + host->bbm_size = 1;
> + }
> + }
> + }
> +
> + /*
> + * we consider ecc->bytes as the sum of all the non-data content in a
> + * step. It gives us a clean representation of the oob area (even if
> + * all the bytes aren't used for ECC).It is always 16 bytes for 8 bit
> + * ECC and 12 bytes for 4 bit ECC
> + */
> + ecc->bytes = host->ecc_bytes_hw + host->spare_bytes + host->bbm_size;
> +
> + ecc->read_page = qcom_nandc_read_page;
> + ecc->read_page_raw = qcom_nandc_read_page_raw;
> + ecc->read_oob = qcom_nandc_read_oob;
> + ecc->write_page = qcom_nandc_write_page;
> + ecc->write_page_raw = qcom_nandc_write_page_raw;
> + ecc->write_oob = qcom_nandc_write_oob;
You should probably also implement ->{read, write}_oob_raw(), otherwise
the core set them to ecc->{read, write}_oob(), which is not exactly the
same. Anyway, let's keep that as things that as future improvements.
The rest looks good to me.
Reviewed-by: Boris Brezillon <boris.brezillon@xxxxxxxxxxxxxxxxxx>
Thanks for your patience, and all the reworks you've done.
Best Regards,
Boris
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
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
