[PATCH v8 2/3] mtd: nand: Qualcomm NAND controller driver
Archit Taneja
architt at codeaurora.org
Sat Mar 19 03:14:51 PDT 2016
On 03/18/2016 10:18 PM, Boris Brezillon wrote:
> On Fri, 18 Mar 2016 16:49:04 +0100
> Boris Brezillon <boris.brezillon at free-electrons.com> wrote:
>
>> Hi Archit,
>>
>> On Wed, 3 Feb 2016 14:29:50 +0530
>> Archit Taneja <architt at codeaurora.org> wrote:
>>
>>> +/*
>>> + * NAND controller page layout info
>>> + *
>>> + * Layout with ECC enabled:
>>> + *
>>> + * |----------------------| |---------------------------------|
>>> + * | xx.......yy| | *********xx.......yy|
>>> + * | DATA xx..ECC..yy| | DATA **SPARE**xx..ECC..yy|
>>> + * | (516) xx.......yy| | (516-n*4) **(n*4)**xx.......yy|
>>> + * | xx.......yy| | *********xx.......yy|
>>> + * |----------------------| |---------------------------------|
>>> + * codeword 1,2..n-1 codeword n
>>> + * <---(528/532 Bytes)--> <-------(528/532 Bytes)--------->
>>> + *
>>> + * n = Number of codewords in the page
>>> + * . = ECC bytes
>>> + * * = Spare/free bytes
>>> + * x = Unused byte(s)
>>> + * y = Reserved byte(s)
>>> + *
>>> + * 2K page: n = 4, spare = 16 bytes
>>> + * 4K page: n = 8, spare = 32 bytes
>>> + * 8K page: n = 16, spare = 64 bytes
>>> + *
>>> + * the qcom nand controller operates at a sub page/codeword level. each
>>> + * codeword is 528 and 532 bytes for 4 bit and 8 bit ECC modes respectively.
>>> + * the number of ECC bytes vary based on the ECC strength and the bus width.
>>> + *
>>> + * the first n - 1 codewords contains 516 bytes of user data, the remaining
>>> + * 12/16 bytes consist of ECC and reserved data. The nth codeword contains
>>> + * both user data and spare(oobavail) bytes that sum up to 516 bytes.
>>> + *
>>> + * When we access a page with ECC enabled, the reserved bytes(s) are not
>>> + * accessible at all. When reading, we fill up these unreadable positions
>>> + * with 0xffs. When writing, the controller skips writing the inaccessible
>>> + * bytes.
>>> + *
>>> + * Layout with ECC disabled:
>>> + *
>>> + * |------------------------------| |---------------------------------------|
>>> + * | yy xx.......| | bb *********xx.......|
>>> + * | DATA1 yy DATA2 xx..ECC..| | DATA1 bb DATA2 **SPARE**xx..ECC..|
>>> + * | (size1) yy (size2) xx.......| | (size1) bb (size2) **(n*4)**xx.......|
>>> + * | yy xx.......| | bb *********xx.......|
>>> + * |------------------------------| |---------------------------------------|
>>> + * codeword 1,2..n-1 codeword n
>>> + * <-------(528/532 Bytes)------> <-----------(528/532 Bytes)----------->
>>> + *
>>> + * n = Number of codewords in the page
>>> + * . = ECC bytes
>>> + * * = Spare/free bytes
>>> + * x = Unused byte(s)
>>> + * y = Dummy Bad Bock byte(s)
>>> + * b = Real Bad Block byte(s)
>>> + * size1/size2 = function of codeword size and 'n'
>>> + *
>>> + * when the ECC block is disabled, one reserved byte (or two for 16 bit bus
>>> + * width) is now accessible. For the first n - 1 codewords, these are dummy Bad
>>> + * Block Markers. In the last codeword, this position contains the real BBM
>>> + *
>>> + * In order to have a consistent layout between RAW and ECC modes, we assume
>>> + * the following OOB layout arrangement:
>>> + *
>>> + * |-----------| |--------------------|
>>> + * |yyxx.......| |bb*********xx.......|
>>> + * |yyxx..ECC..| |bb*FREEOOB*xx..ECC..|
>>> + * |yyxx.......| |bb*********xx.......|
>>> + * |yyxx.......| |bb*********xx.......|
>>> + * |-----------| |--------------------|
>>> + * first n - 1 nth OOB region
>>> + * OOB regions
>>> + *
>>> + * n = Number of codewords in the page
>>> + * . = ECC bytes
>>> + * * = FREE OOB bytes
>>> + * y = Dummy bad block byte(s) (inaccessible when ECC enabled)
>>> + * x = Unused byte(s)
>>> + * b = Real bad block byte(s) (inaccessible when ECC enabled)
>>> + *
>>> + * This layout is read as is when ECC is disabled. When ECC is enabled, the
>>> + * inaccessible Bad Block byte(s) are ignored when we write to a page/oob,
>>> + * and assumed as 0xffs when we read a page/oob. The ECC, unused and
>>> + * dummy/real bad block bytes are grouped as ecc bytes in nand_ecclayout (i.e,
>>> + * ecc->bytes is the sum of the three).
>>> + */
>>> +
>>> +static struct nand_ecclayout *
>>> +qcom_nand_create_layout(struct qcom_nand_host *host)
>>> +{
>>> + struct nand_chip *chip = &host->chip;
>>> + struct mtd_info *mtd = nand_to_mtd(chip);
>>> + struct qcom_nand_controller *nandc = get_qcom_nand_controller(chip);
>>> + struct nand_ecc_ctrl *ecc = &chip->ecc;
>>> + struct nand_ecclayout *layout;
>>> + int i, j, steps, pos = 0, shift = 0;
>>> +
>>> + layout = devm_kzalloc(nandc->dev, sizeof(*layout), GFP_KERNEL);
>>> + if (!layout)
>>> + return NULL;
>>> +
>>> + steps = mtd->writesize / ecc->size;
>>> + layout->eccbytes = steps * ecc->bytes;
>>> +
>>> + layout->oobfree[0].offset = (steps - 1) * ecc->bytes + host->bbm_size;
>>> + layout->oobfree[0].length = steps << 2;
>>> + layout->oobavail = steps << 2;
>>> +
>>> + /*
>>> + * the oob bytes in the first n - 1 codewords are all grouped together
>>> + * in the format:
>>> + * DUMMY_BBM + UNUSED + ECC
>>> + */
>>> + for (i = 0; i < steps - 1; i++) {
>>> + for (j = 0; j < ecc->bytes; j++)
>>> + layout->eccpos[pos++] = i * ecc->bytes + j;
>>> + }
>>> +
>>> + /*
>>> + * the oob bytes in the last codeword are grouped in the format:
>>> + * BBM + FREE OOB + UNUSED + ECC
>>> + */
>>> +
>>> + /* fill up the bbm positions */
>>> + for (j = 0; j < host->bbm_size; j++)
>>> + layout->eccpos[pos++] = i * ecc->bytes + j;
>>> +
>>> + /*
>>> + * fill up the ecc and reserved positions, their indices are offseted
>>> + * by the free oob region
>>> + */
>>> + shift = layout->oobfree[0].length + host->bbm_size;
>>> +
>>> + for (j = 0; j < (host->ecc_bytes_hw + host->spare_bytes); j++)
>>> + layout->eccpos[pos++] = i * ecc->bytes + shift + j;
>>> +
>>> + return layout;
>>> +}
>>
>> I'm trying to move this layout definition to the mtd_ooblayout_ops
>> approach, and I wonder why you decided to take such a complicated
>> representation.
>> AFAIU, in each ECC step you have 512 bytes of data, X ECC+reserved
>> bytes (you decided to consider all of them as ECC bytes, which is fine
>> by me) and 4 usable/free bytes. Am I correct?
>>
>> If that's the case, then why not exposing the following layout.
>>
>> eccregion[i] = {
>> .offset = i * (ecc->bytes + 4);
>> .length = ecc->bytes;
>> }
>>
>> oobfreeregion[i] = {
>> .offset = (i * (ecc->bytes + 4)) + ecc->bytes;
>> .length = 4;
>> }
>>
>> Are there any userspace tools relying on the ooblayout you're currently
>> exposing (remember that the exposed OOB layout is not necessarily
>> what you see on the media)?
>
> Okay, I think we already had this discussion :).
> I'm still not happy with the exposed layout (it would be much easier to
> reserve 4 free bytes per chunk, and declare each chunk as containing 512
> data bytes + 4 oob bytes + X ECC/reserved bytes), but IIRC, your ROM
> code (and/or bootloader) is already using this layout :-(.
Sadly, yeah. The reason given for this was that if a filesystem only
wanted to read the 16 oob bytes off the page, it could just read the
last subpage instead of going through all pages. The optimization
clearly doesn't seem worth the software overhead.
Is this something that's blocking your mtd_ooblayout_ops work?
Archit
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum, hosted by The Linux Foundation
More information about the linux-mtd
mailing list