* [PATCH v3 0/3] Fixes for Rockchip NAND controller driver @ 2023-06-15 17:32 Johan Jonker 2023-06-15 17:34 ` [PATCH v3 1/3] mtd: rawnand: rockchip-nand-controller: fix oobfree offset and description Johan Jonker ` (2 more replies) 0 siblings, 3 replies; 11+ messages in thread From: Johan Jonker @ 2023-06-15 17:32 UTC (permalink / raw) To: miquel.raynal Cc: richard, vigneshr, heiko, linux-mtd, linux-kernel, linux-arm-kernel, linux-rockchip, yifeng.zhao This serie contains various fixes for the Rockchip NAND controller driver that showed up while testing boot block writing. Fixed are: Always copy hwecc PA data to/from oob_poi buffer in order to be able to read/write the various boot block layouts. Add option to safely probe the driver on a NAND with unknown data layout. Fix oobfree layout. Changed V3: Change patch order, layout fixes first Change prefixes Reword State that patches break all existing jffs2 users Changed V2: Add tag Add manufacturer ops Reword Johan Jonker (3): mtd: rawnand: rockchip-nand-controller: fix oobfree offset and description mtd: rawnand: rockchip-nand-controller: copy hwecc PA data to oob_poi buffer mtd: rawnand: rockchip-nand-controller: add skipbbt option .../mtd/nand/raw/rockchip-nand-controller.c | 52 ++++++++++++------- 1 file changed, 32 insertions(+), 20 deletions(-) -- 2.30.2 ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v3 1/3] mtd: rawnand: rockchip-nand-controller: fix oobfree offset and description 2023-06-15 17:32 [PATCH v3 0/3] Fixes for Rockchip NAND controller driver Johan Jonker @ 2023-06-15 17:34 ` Johan Jonker 2023-07-04 15:03 ` Miquel Raynal 2023-06-15 17:34 ` [PATCH v3 2/3] mtd: rawnand: rockchip-nand-controller: copy hwecc PA data to oob_poi buffer Johan Jonker 2023-06-15 17:34 ` [PATCH v3 3/3] mtd: rawnand: rockchip-nand-controller: add skipbbt option Johan Jonker 2 siblings, 1 reply; 11+ messages in thread From: Johan Jonker @ 2023-06-15 17:34 UTC (permalink / raw) To: miquel.raynal Cc: richard, vigneshr, heiko, linux-mtd, linux-kernel, linux-arm-kernel, linux-rockchip, yifeng.zhao The MTD framework reserves 1 or 2 bytes for the bad block marker depending on the bus size. The rockchip-nand-controller driver currently only supports a 8 bit bus, but reserves standard 2 bytes for the BBM in the chip->oob_poi buffer. The first free OOB byte is therefore OOB2 at offset 2. Page Address (PA) bytes are located at the last 4 positions before ECC. The current advertised free OOB area has an offset that starts at OOB6 and a length that overlaps with the space reserved for the PA bytes. Writing unrelated data to a reserved space with a specific task can corrupt our boot block page read order. Fix by changing the free OOB offset to 2. This change breaks existing jffs2 users. Signed-off-by: Johan Jonker <jbx6244@gmail.com> --- Changed V3: Change prefixes Reword State break existing users. --- Example: Wrong free OOB offset starts at OOB6: oob_region->offset = NFC_SYS_DATA_SIZE + 2; = 4 + 2 = 6 oob_region->length = rknand->metadata_size - NFC_SYS_DATA_SIZE - 2; = 32 - 4 - 2 = 26 Together with this length above it overlaps a reserved space for the boot blocks Page Address(PA) chip->oob_poi buffer layout for 8 steps: BBM0 BBM1 OOB2 OOB3 | OOB4 OOB5 OOB6 OOB7 OOB8 OOB9 OOB10 OOB11 | OOB12 OOB13 OOB15 OOB15 OOB16 OOB17 OOB18 OOB19 | OOB20 OOB21 OOB22 OOB23 OOB24 OOB25 OOB26 OOB27 | PA0 PA1 PA2 PA3 ECC0 ECC1 ECC2 ECC3 | ... ... ... ... Fix by new offset at OOB2: oob_region->offset = 2; The full range of free OOB with 8 steps runs from OOB2 till/including OOB27. --- drivers/mtd/nand/raw/rockchip-nand-controller.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/drivers/mtd/nand/raw/rockchip-nand-controller.c b/drivers/mtd/nand/raw/rockchip-nand-controller.c index 2312e2736..37fc07ba5 100644 --- a/drivers/mtd/nand/raw/rockchip-nand-controller.c +++ b/drivers/mtd/nand/raw/rockchip-nand-controller.c @@ -562,9 +562,10 @@ static int rk_nfc_write_page_raw(struct nand_chip *chip, const u8 *buf, * BBM OOB1 OOB2 OOB3 |......| PA0 PA1 PA2 PA3 * * The rk_nfc_ooblayout_free() function already has reserved - * these 4 bytes with: + * these 4 bytes together with 2 bytes for BBM + * by reducing it's length: * - * oob_region->offset = NFC_SYS_DATA_SIZE + 2; + * oob_region->length = rknand->metadata_size - NFC_SYS_DATA_SIZE - 2; */ if (!i) memcpy(rk_nfc_oob_ptr(chip, i), @@ -933,12 +934,8 @@ static int rk_nfc_ooblayout_free(struct mtd_info *mtd, int section, if (section) return -ERANGE; - /* - * The beginning of the OOB area stores the reserved data for the NFC, - * the size of the reserved data is NFC_SYS_DATA_SIZE bytes. - */ oob_region->length = rknand->metadata_size - NFC_SYS_DATA_SIZE - 2; - oob_region->offset = NFC_SYS_DATA_SIZE + 2; + oob_region->offset = 2; return 0; } -- 2.30.2 ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v3 1/3] mtd: rawnand: rockchip-nand-controller: fix oobfree offset and description 2023-06-15 17:34 ` [PATCH v3 1/3] mtd: rawnand: rockchip-nand-controller: fix oobfree offset and description Johan Jonker @ 2023-07-04 15:03 ` Miquel Raynal 0 siblings, 0 replies; 11+ messages in thread From: Miquel Raynal @ 2023-07-04 15:03 UTC (permalink / raw) To: Johan Jonker Cc: richard, vigneshr, heiko, linux-mtd, linux-kernel, linux-arm-kernel, linux-rockchip, yifeng.zhao Hi Johan, Would you mind reducing the subjets with s/rockchip-nand-controller/rockchip/? jbx6244@gmail.com wrote on Thu, 15 Jun 2023 19:34:01 +0200: > The MTD framework reserves 1 or 2 bytes for the bad block marker > depending on the bus size. The rockchip-nand-controller driver > currently only supports a 8 bit bus, but reserves standard 2 bytes > for the BBM in the chip->oob_poi buffer. The first free OOB byte is > therefore OOB2 at offset 2. Again, I don't think it's ever been a single byte. Please drop this paragraph, it does not justify anything below. What is important here is the PA thing I guess. > Page Address (PA) bytes are located at the Maybe you can briefly explain what the PA bytes are above. > last 4 positions before ECC. The current advertised free OOB area has > an offset that starts at OOB6 and a length that overlaps with the space Let me try to rephrase this: " The currently advertised free OOB area starts at offset 6, like if 4 PA bytes were located right after the BBM. This is wrong as the PA bytes are located right before the ECC bytes. Fix the layout by allowing access to all bytes between the BBM and the PA bytes instead of reserving 4 bytes right after the BBM. > reserved for the PA bytes. Writing unrelated data to a reserved space > with a specific task can corrupt our boot block page read order. "our"? > Fix by changing the free OOB offset to 2. > > This change breaks existing jffs2 users. > > Signed-off-by: Johan Jonker <jbx6244@gmail.com> Please add a Fixes tag, now that I look at it again, it looks like the original author did try to protect these PA bytes, but failed. > --- > > Changed V3: > Change prefixes > Reword > State break existing users. > > --- > > Example: > > Wrong free OOB offset starts at OOB6: > oob_region->offset = NFC_SYS_DATA_SIZE + 2; > = 4 + 2 > = 6 > > oob_region->length = rknand->metadata_size - NFC_SYS_DATA_SIZE - 2; > = 32 - 4 - 2 > = 26 > > Together with this length above it overlaps a reserved space for the > boot blocks Page Address(PA) > > chip->oob_poi buffer layout for 8 steps: > > BBM0 BBM1 OOB2 OOB3 | OOB4 OOB5 OOB6 OOB7 > > OOB8 OOB9 OOB10 OOB11 | OOB12 OOB13 OOB15 OOB15 > OOB16 OOB17 OOB18 OOB19 | OOB20 OOB21 OOB22 OOB23 > > OOB24 OOB25 OOB26 OOB27 | PA0 PA1 PA2 PA3 > > ECC0 ECC1 ECC2 ECC3 | ... ... ... ... > > Fix by new offset at OOB2: > oob_region->offset = 2; > > The full range of free OOB with 8 steps runs from OOB2 > till/including OOB27. > --- > drivers/mtd/nand/raw/rockchip-nand-controller.c | 11 ++++------- > 1 file changed, 4 insertions(+), 7 deletions(-) > > diff --git a/drivers/mtd/nand/raw/rockchip-nand-controller.c b/drivers/mtd/nand/raw/rockchip-nand-controller.c > index 2312e2736..37fc07ba5 100644 > --- a/drivers/mtd/nand/raw/rockchip-nand-controller.c > +++ b/drivers/mtd/nand/raw/rockchip-nand-controller.c > @@ -562,9 +562,10 @@ static int rk_nfc_write_page_raw(struct nand_chip *chip, const u8 *buf, > * BBM OOB1 OOB2 OOB3 |......| PA0 PA1 PA2 PA3 > * > * The rk_nfc_ooblayout_free() function already has reserved > - * these 4 bytes with: > + * these 4 bytes together with 2 bytes for BBM > + * by reducing it's length: > * > - * oob_region->offset = NFC_SYS_DATA_SIZE + 2; > + * oob_region->length = rknand->metadata_size - NFC_SYS_DATA_SIZE - 2; > */ > if (!i) > memcpy(rk_nfc_oob_ptr(chip, i), > @@ -933,12 +934,8 @@ static int rk_nfc_ooblayout_free(struct mtd_info *mtd, int section, > if (section) > return -ERANGE; > > - /* > - * The beginning of the OOB area stores the reserved data for the NFC, > - * the size of the reserved data is NFC_SYS_DATA_SIZE bytes. > - */ > oob_region->length = rknand->metadata_size - NFC_SYS_DATA_SIZE - 2; > - oob_region->offset = NFC_SYS_DATA_SIZE + 2; > + oob_region->offset = 2; > > return 0; > } > -- > 2.30.2 > Thanks, Miquèl ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v3 2/3] mtd: rawnand: rockchip-nand-controller: copy hwecc PA data to oob_poi buffer 2023-06-15 17:32 [PATCH v3 0/3] Fixes for Rockchip NAND controller driver Johan Jonker 2023-06-15 17:34 ` [PATCH v3 1/3] mtd: rawnand: rockchip-nand-controller: fix oobfree offset and description Johan Jonker @ 2023-06-15 17:34 ` Johan Jonker 2023-07-04 15:08 ` Miquel Raynal 2023-06-15 17:34 ` [PATCH v3 3/3] mtd: rawnand: rockchip-nand-controller: add skipbbt option Johan Jonker 2 siblings, 1 reply; 11+ messages in thread From: Johan Jonker @ 2023-06-15 17:34 UTC (permalink / raw) To: miquel.raynal Cc: richard, vigneshr, heiko, linux-mtd, linux-kernel, linux-arm-kernel, linux-rockchip, yifeng.zhao Rockchip boot blocks are written per 4 x 512 byte sectors per page. Each page must have a page address (PA) pointer in OOB to the next page. Pages are written in a pattern depending on the NAND chip ID. This logic used to build a page pattern table is not fully disclosed and is not easy to fit in the MTD framework. The formula in rk_nfc_write_page_hwecc() function is not correct. Make hwecc and raw behavior identical. Generate boot block page address and pattern for hwecc in user space and copy PA data to/from the already reserved last 4 bytes before EEC in the chip->oob_poi data layout. This patch breaks all existing jffs2 users that have free OOB overlap with the reserved space for PA data. Signed-off-by: Johan Jonker <jbx6244@gmail.com> --- Changed V3: Change prefixes Reword --- .../mtd/nand/raw/rockchip-nand-controller.c | 34 ++++++++++++------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/drivers/mtd/nand/raw/rockchip-nand-controller.c b/drivers/mtd/nand/raw/rockchip-nand-controller.c index 37fc07ba5..5a0468034 100644 --- a/drivers/mtd/nand/raw/rockchip-nand-controller.c +++ b/drivers/mtd/nand/raw/rockchip-nand-controller.c @@ -598,7 +598,7 @@ static int rk_nfc_write_page_hwecc(struct nand_chip *chip, const u8 *buf, int pages_per_blk = mtd->erasesize / mtd->writesize; int ret = 0, i, boot_rom_mode = 0; dma_addr_t dma_data, dma_oob; - u32 reg; + u32 tmp; u8 *oob; nand_prog_page_begin_op(chip, page, 0, NULL, 0); @@ -625,6 +625,13 @@ static int rk_nfc_write_page_hwecc(struct nand_chip *chip, const u8 *buf, * * 0xFF 0xFF 0xFF 0xFF | BBM OOB1 OOB2 OOB3 | ... * + * The code here just swaps the first 4 bytes with the last + * 4 bytes without losing any data. + * + * The chip->oob_poi data layout: + * + * BBM OOB1 OOB2 OOB3 |......| PA0 PA1 PA2 PA3 + * * Configure the ECC algorithm supported by the boot ROM. */ if ((page < (pages_per_blk * rknand->boot_blks)) && @@ -635,21 +642,17 @@ static int rk_nfc_write_page_hwecc(struct nand_chip *chip, const u8 *buf, } for (i = 0; i < ecc->steps; i++) { - if (!i) { - reg = 0xFFFFFFFF; - } else { + if (!i) + oob = chip->oob_poi + (ecc->steps - 1) * NFC_SYS_DATA_SIZE; + else oob = chip->oob_poi + (i - 1) * NFC_SYS_DATA_SIZE; - reg = oob[0] | oob[1] << 8 | oob[2] << 16 | - oob[3] << 24; - } - if (!i && boot_rom_mode) - reg = (page & (pages_per_blk - 1)) * 4; + tmp = oob[0] | oob[1] << 8 | oob[2] << 16 | oob[3] << 24; if (nfc->cfg->type == NFC_V9) - nfc->oob_buf[i] = reg; + nfc->oob_buf[i] = tmp; else - nfc->oob_buf[i * (oob_step / 4)] = reg; + nfc->oob_buf[i * (oob_step / 4)] = tmp; } dma_data = dma_map_single(nfc->dev, (void *)nfc->page_buf, @@ -812,12 +815,17 @@ static int rk_nfc_read_page_hwecc(struct nand_chip *chip, u8 *buf, int oob_on, goto timeout_err; } - for (i = 1; i < ecc->steps; i++) { - oob = chip->oob_poi + (i - 1) * NFC_SYS_DATA_SIZE; + for (i = 0; i < ecc->steps; i++) { + if (!i) + oob = chip->oob_poi + (ecc->steps - 1) * NFC_SYS_DATA_SIZE; + else + oob = chip->oob_poi + (i - 1) * NFC_SYS_DATA_SIZE; + if (nfc->cfg->type == NFC_V9) tmp = nfc->oob_buf[i]; else tmp = nfc->oob_buf[i * (oob_step / 4)]; + *oob++ = (u8)tmp; *oob++ = (u8)(tmp >> 8); *oob++ = (u8)(tmp >> 16); -- 2.30.2 ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v3 2/3] mtd: rawnand: rockchip-nand-controller: copy hwecc PA data to oob_poi buffer 2023-06-15 17:34 ` [PATCH v3 2/3] mtd: rawnand: rockchip-nand-controller: copy hwecc PA data to oob_poi buffer Johan Jonker @ 2023-07-04 15:08 ` Miquel Raynal 2023-07-05 18:20 ` Johan Jonker 0 siblings, 1 reply; 11+ messages in thread From: Miquel Raynal @ 2023-07-04 15:08 UTC (permalink / raw) To: Johan Jonker Cc: richard, vigneshr, heiko, linux-mtd, linux-kernel, linux-arm-kernel, linux-rockchip, yifeng.zhao Hi Johan, jbx6244@gmail.com wrote on Thu, 15 Jun 2023 19:34:13 +0200: > Rockchip boot blocks are written per 4 x 512 byte sectors per page. > Each page must have a page address (PA) pointer in OOB to the next page. Only when used as boot device I guess? It's a BootROM limitation. > Pages are written in a pattern depending on the NAND chip ID. > This logic used to build a page pattern table is not fully disclosed and > is not easy to fit in the MTD framework. > The formula in rk_nfc_write_page_hwecc() function is not correct. > Make hwecc and raw behavior identical. So this is a fix as well, deserves a tag. Whatever the reason why you need this, the issue you are solving is: write_page_hwecc and write_page_raw are not aligned. > Generate boot block page address and pattern for hwecc in user space > and copy PA data to/from the already reserved last 4 bytes before EEC ECC > in the chip->oob_poi data layout. > > This patch breaks all existing jffs2 users that have free OOB overlap > with the reserved space for PA data. > > Signed-off-by: Johan Jonker <jbx6244@gmail.com> > --- > > Changed V3: > Change prefixes > Reword > --- > .../mtd/nand/raw/rockchip-nand-controller.c | 34 ++++++++++++------- > 1 file changed, 21 insertions(+), 13 deletions(-) > > diff --git a/drivers/mtd/nand/raw/rockchip-nand-controller.c b/drivers/mtd/nand/raw/rockchip-nand-controller.c > index 37fc07ba5..5a0468034 100644 > --- a/drivers/mtd/nand/raw/rockchip-nand-controller.c > +++ b/drivers/mtd/nand/raw/rockchip-nand-controller.c > @@ -598,7 +598,7 @@ static int rk_nfc_write_page_hwecc(struct nand_chip *chip, const u8 *buf, > int pages_per_blk = mtd->erasesize / mtd->writesize; > int ret = 0, i, boot_rom_mode = 0; > dma_addr_t dma_data, dma_oob; > - u32 reg; > + u32 tmp; > u8 *oob; > > nand_prog_page_begin_op(chip, page, 0, NULL, 0); > @@ -625,6 +625,13 @@ static int rk_nfc_write_page_hwecc(struct nand_chip *chip, const u8 *buf, > * > * 0xFF 0xFF 0xFF 0xFF | BBM OOB1 OOB2 OOB3 | ... > * > + * The code here just swaps the first 4 bytes with the last > + * 4 bytes without losing any data. > + * > + * The chip->oob_poi data layout: > + * > + * BBM OOB1 OOB2 OOB3 |......| PA0 PA1 PA2 PA3 > + * > * Configure the ECC algorithm supported by the boot ROM. > */ > if ((page < (pages_per_blk * rknand->boot_blks)) && > @@ -635,21 +642,17 @@ static int rk_nfc_write_page_hwecc(struct nand_chip *chip, const u8 *buf, > } > > for (i = 0; i < ecc->steps; i++) { > - if (!i) { > - reg = 0xFFFFFFFF; > - } else { > + if (!i) > + oob = chip->oob_poi + (ecc->steps - 1) * NFC_SYS_DATA_SIZE; > + else > oob = chip->oob_poi + (i - 1) * NFC_SYS_DATA_SIZE; > - reg = oob[0] | oob[1] << 8 | oob[2] << 16 | > - oob[3] << 24; > - } > > - if (!i && boot_rom_mode) So we no longer need boot_rom_mode? Or do we? > - reg = (page & (pages_per_blk - 1)) * 4; > + tmp = oob[0] | oob[1] << 8 | oob[2] << 16 | oob[3] << 24; > > if (nfc->cfg->type == NFC_V9) > - nfc->oob_buf[i] = reg; > + nfc->oob_buf[i] = tmp; > else > - nfc->oob_buf[i * (oob_step / 4)] = reg; > + nfc->oob_buf[i * (oob_step / 4)] = tmp; > } > > dma_data = dma_map_single(nfc->dev, (void *)nfc->page_buf, > @@ -812,12 +815,17 @@ static int rk_nfc_read_page_hwecc(struct nand_chip *chip, u8 *buf, int oob_on, > goto timeout_err; > } > > - for (i = 1; i < ecc->steps; i++) { > - oob = chip->oob_poi + (i - 1) * NFC_SYS_DATA_SIZE; > + for (i = 0; i < ecc->steps; i++) { > + if (!i) > + oob = chip->oob_poi + (ecc->steps - 1) * NFC_SYS_DATA_SIZE; > + else > + oob = chip->oob_poi + (i - 1) * NFC_SYS_DATA_SIZE; > + > if (nfc->cfg->type == NFC_V9) > tmp = nfc->oob_buf[i]; > else > tmp = nfc->oob_buf[i * (oob_step / 4)]; > + > *oob++ = (u8)tmp; > *oob++ = (u8)(tmp >> 8); > *oob++ = (u8)(tmp >> 16); > -- > 2.30.2 > Thanks, Miquèl ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 2/3] mtd: rawnand: rockchip-nand-controller: copy hwecc PA data to oob_poi buffer 2023-07-04 15:08 ` Miquel Raynal @ 2023-07-05 18:20 ` Johan Jonker 0 siblings, 0 replies; 11+ messages in thread From: Johan Jonker @ 2023-07-05 18:20 UTC (permalink / raw) To: Miquel Raynal Cc: richard, vigneshr, heiko, linux-mtd, linux-kernel, linux-arm-kernel, linux-rockchip, yifeng.zhao On 7/4/23 17:08, Miquel Raynal wrote: > Hi Johan, > > jbx6244@gmail.com wrote on Thu, 15 Jun 2023 19:34:13 +0200: > >> Rockchip boot blocks are written per 4 x 512 byte sectors per page. >> Each page must have a page address (PA) pointer in OOB to the next page. > > Only when used as boot device I guess? It's a BootROM limitation. Yes, required by the BootROM. > >> Pages are written in a pattern depending on the NAND chip ID. >> This logic used to build a page pattern table is not fully disclosed and >> is not easy to fit in the MTD framework. >> The formula in rk_nfc_write_page_hwecc() function is not correct. >> Make hwecc and raw behavior identical. > > So this is a fix as well, deserves a tag. Whatever the reason why you > need this, the issue you are solving is: write_page_hwecc and > write_page_raw are not aligned. Fixes: 058e0e847d54 ("mtd: rawnand: rockchip: NFC driver for RK3308, RK2928 and others") > >> Generate boot block page address and pattern for hwecc in user space >> and copy PA data to/from the already reserved last 4 bytes before EEC > > ECC OK > >> in the chip->oob_poi data layout. >> >> This patch breaks all existing jffs2 users that have free OOB overlap >> with the reserved space for PA data. >> >> Signed-off-by: Johan Jonker <jbx6244@gmail.com> >> --- >> >> Changed V3: >> Change prefixes >> Reword >> --- >> .../mtd/nand/raw/rockchip-nand-controller.c | 34 ++++++++++++------- >> 1 file changed, 21 insertions(+), 13 deletions(-) >> >> diff --git a/drivers/mtd/nand/raw/rockchip-nand-controller.c b/drivers/mtd/nand/raw/rockchip-nand-controller.c >> index 37fc07ba5..5a0468034 100644 >> --- a/drivers/mtd/nand/raw/rockchip-nand-controller.c >> +++ b/drivers/mtd/nand/raw/rockchip-nand-controller.c >> @@ -598,7 +598,7 @@ static int rk_nfc_write_page_hwecc(struct nand_chip *chip, const u8 *buf, >> int pages_per_blk = mtd->erasesize / mtd->writesize; >> int ret = 0, i, boot_rom_mode = 0; >> dma_addr_t dma_data, dma_oob; >> - u32 reg; >> + u32 tmp; >> u8 *oob; >> >> nand_prog_page_begin_op(chip, page, 0, NULL, 0); >> @@ -625,6 +625,13 @@ static int rk_nfc_write_page_hwecc(struct nand_chip *chip, const u8 *buf, >> * >> * 0xFF 0xFF 0xFF 0xFF | BBM OOB1 OOB2 OOB3 | ... >> * >> + * The code here just swaps the first 4 bytes with the last >> + * 4 bytes without losing any data. >> + * >> + * The chip->oob_poi data layout: >> + * >> + * BBM OOB1 OOB2 OOB3 |......| PA0 PA1 PA2 PA3 >> + * >> * Configure the ECC algorithm supported by the boot ROM. >> */ >> if ((page < (pages_per_blk * rknand->boot_blks)) && >> @@ -635,21 +642,17 @@ static int rk_nfc_write_page_hwecc(struct nand_chip *chip, const u8 *buf, >> } >> >> for (i = 0; i < ecc->steps; i++) { >> - if (!i) { >> - reg = 0xFFFFFFFF; >> - } else { >> + if (!i) >> + oob = chip->oob_poi + (ecc->steps - 1) * NFC_SYS_DATA_SIZE; >> + else >> oob = chip->oob_poi + (i - 1) * NFC_SYS_DATA_SIZE; >> - reg = oob[0] | oob[1] << 8 | oob[2] << 16 | >> - oob[3] << 24; >> - } >> >> - if (!i && boot_rom_mode) > > So we no longer need boot_rom_mode? Or do we? boot_rom_mode is still in use for ecc switching. > >> - reg = (page & (pages_per_blk - 1)) * 4; >> + tmp = oob[0] | oob[1] << 8 | oob[2] << 16 | oob[3] << 24; >> >> if (nfc->cfg->type == NFC_V9) >> - nfc->oob_buf[i] = reg; >> + nfc->oob_buf[i] = tmp; >> else >> - nfc->oob_buf[i * (oob_step / 4)] = reg; >> + nfc->oob_buf[i * (oob_step / 4)] = tmp; >> } >> >> dma_data = dma_map_single(nfc->dev, (void *)nfc->page_buf, >> @@ -812,12 +815,17 @@ static int rk_nfc_read_page_hwecc(struct nand_chip *chip, u8 *buf, int oob_on, >> goto timeout_err; >> } >> >> - for (i = 1; i < ecc->steps; i++) { >> - oob = chip->oob_poi + (i - 1) * NFC_SYS_DATA_SIZE; >> + for (i = 0; i < ecc->steps; i++) { >> + if (!i) >> + oob = chip->oob_poi + (ecc->steps - 1) * NFC_SYS_DATA_SIZE; >> + else >> + oob = chip->oob_poi + (i - 1) * NFC_SYS_DATA_SIZE; >> + >> if (nfc->cfg->type == NFC_V9) >> tmp = nfc->oob_buf[i]; >> else >> tmp = nfc->oob_buf[i * (oob_step / 4)]; >> + >> *oob++ = (u8)tmp; >> *oob++ = (u8)(tmp >> 8); >> *oob++ = (u8)(tmp >> 16); >> -- >> 2.30.2 >> > > > Thanks, > Miquèl ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v3 3/3] mtd: rawnand: rockchip-nand-controller: add skipbbt option 2023-06-15 17:32 [PATCH v3 0/3] Fixes for Rockchip NAND controller driver Johan Jonker 2023-06-15 17:34 ` [PATCH v3 1/3] mtd: rawnand: rockchip-nand-controller: fix oobfree offset and description Johan Jonker 2023-06-15 17:34 ` [PATCH v3 2/3] mtd: rawnand: rockchip-nand-controller: copy hwecc PA data to oob_poi buffer Johan Jonker @ 2023-06-15 17:34 ` Johan Jonker 2023-07-04 15:13 ` Miquel Raynal 2 siblings, 1 reply; 11+ messages in thread From: Johan Jonker @ 2023-06-15 17:34 UTC (permalink / raw) To: miquel.raynal Cc: richard, vigneshr, heiko, linux-mtd, linux-kernel, linux-arm-kernel, linux-rockchip, yifeng.zhao On Rockchip SoCs the first boot stages are written on NAND with help of manufacturer software that uses a different format then the MTD framework. Skip the automatic BBT scan with the NAND_SKIP_BBTSCAN option so that the original content is unchanged during the driver probe. The NAND_NO_BBM_QUIRK option allows us to erase bad blocks with the nand_erase_nand() function and the flash_erase command. With these options the user has the "freedom of choice" by neutral access mode to read and write in whatever format is needed. Signed-off-by: Johan Jonker <jbx6244@gmail.com> --- Changes V3: Change prefixes Changed V2: reword --- I'm aware that the maintainer finds it "awful", but it's absolute necessary to: 1: read/write boot blocks in user space without touching original content 2: format a NAND for MTD either with built in or external driver module So we keep it include in this serie. --- drivers/mtd/nand/raw/rockchip-nand-controller.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/drivers/mtd/nand/raw/rockchip-nand-controller.c b/drivers/mtd/nand/raw/rockchip-nand-controller.c index 5a0468034..fcda4c760 100644 --- a/drivers/mtd/nand/raw/rockchip-nand-controller.c +++ b/drivers/mtd/nand/raw/rockchip-nand-controller.c @@ -188,6 +188,10 @@ struct rk_nfc { unsigned long assigned_cs; }; +static int skipbbt; +module_param(skipbbt, int, 0644); +MODULE_PARM_DESC(skipbbt, "Skip BBT scan if data on the NAND chip is not in MTD format."); + static inline struct rk_nfc_nand_chip *rk_nfc_to_rknand(struct nand_chip *chip) { return container_of(chip, struct rk_nfc_nand_chip, chip); @@ -1153,6 +1157,9 @@ static int rk_nfc_nand_chip_init(struct device *dev, struct rk_nfc *nfc, nand_set_controller_data(chip, nfc); + if (skipbbt) + chip->options |= NAND_SKIP_BBTSCAN | NAND_NO_BBM_QUIRK; + chip->options |= NAND_USES_DMA | NAND_NO_SUBPAGE_WRITE; chip->bbt_options = NAND_BBT_USE_FLASH | NAND_BBT_NO_OOB; -- 2.30.2 ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v3 3/3] mtd: rawnand: rockchip-nand-controller: add skipbbt option 2023-06-15 17:34 ` [PATCH v3 3/3] mtd: rawnand: rockchip-nand-controller: add skipbbt option Johan Jonker @ 2023-07-04 15:13 ` Miquel Raynal 2023-07-07 15:27 ` Johan Jonker 0 siblings, 1 reply; 11+ messages in thread From: Miquel Raynal @ 2023-07-04 15:13 UTC (permalink / raw) To: Johan Jonker Cc: richard, vigneshr, heiko, linux-mtd, linux-kernel, linux-arm-kernel, linux-rockchip, yifeng.zhao Hi Johan, jbx6244@gmail.com wrote on Thu, 15 Jun 2023 19:34:26 +0200: > On Rockchip SoCs the first boot stages are written on NAND > with help of manufacturer software that uses a different format > then the MTD framework. Skip the automatic BBT scan with the > NAND_SKIP_BBTSCAN option so that the original content is unchanged > during the driver probe. > The NAND_NO_BBM_QUIRK option allows us to erase bad blocks with > the nand_erase_nand() function and the flash_erase command. > With these options the user has the "freedom of choice" by neutral > access mode to read and write in whatever format is needed. Can the boot_rom_mode thing help? For writing this part you can disable the bad block protection using debugfs and then write an externally processed file in raw mode I guess. For the boot I think I proposed a DT property. I don't remember how far the discussion went. > > Signed-off-by: Johan Jonker <jbx6244@gmail.com> > --- > > Changes V3: > Change prefixes > > Changed V2: > reword > > --- > > I'm aware that the maintainer finds it "awful", > but it's absolute necessary to: > 1: read/write boot blocks in user space without touching original content > 2: format a NAND for MTD either with built in or external driver module > > So we keep it include in this serie. > --- > drivers/mtd/nand/raw/rockchip-nand-controller.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/mtd/nand/raw/rockchip-nand-controller.c b/drivers/mtd/nand/raw/rockchip-nand-controller.c > index 5a0468034..fcda4c760 100644 > --- a/drivers/mtd/nand/raw/rockchip-nand-controller.c > +++ b/drivers/mtd/nand/raw/rockchip-nand-controller.c > @@ -188,6 +188,10 @@ struct rk_nfc { > unsigned long assigned_cs; > }; > > +static int skipbbt; > +module_param(skipbbt, int, 0644); > +MODULE_PARM_DESC(skipbbt, "Skip BBT scan if data on the NAND chip is not in MTD format."); > + > static inline struct rk_nfc_nand_chip *rk_nfc_to_rknand(struct nand_chip *chip) > { > return container_of(chip, struct rk_nfc_nand_chip, chip); > @@ -1153,6 +1157,9 @@ static int rk_nfc_nand_chip_init(struct device *dev, struct rk_nfc *nfc, > > nand_set_controller_data(chip, nfc); > > + if (skipbbt) > + chip->options |= NAND_SKIP_BBTSCAN | NAND_NO_BBM_QUIRK; > + > chip->options |= NAND_USES_DMA | NAND_NO_SUBPAGE_WRITE; > chip->bbt_options = NAND_BBT_USE_FLASH | NAND_BBT_NO_OOB; > > -- > 2.30.2 > Thanks, Miquèl ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 3/3] mtd: rawnand: rockchip-nand-controller: add skipbbt option 2023-07-04 15:13 ` Miquel Raynal @ 2023-07-07 15:27 ` Johan Jonker 2023-07-12 13:57 ` Miquel Raynal 0 siblings, 1 reply; 11+ messages in thread From: Johan Jonker @ 2023-07-07 15:27 UTC (permalink / raw) To: Miquel Raynal Cc: richard, vigneshr, heiko, linux-mtd, linux-kernel, linux-arm-kernel, linux-rockchip, yifeng.zhao Hi Miquel, Some comments/questions below, On 7/4/23 17:13, Miquel Raynal wrote: > Hi Johan, > > jbx6244@gmail.com wrote on Thu, 15 Jun 2023 19:34:26 +0200: > >> On Rockchip SoCs the first boot stages are written on NAND >> with help of manufacturer software that uses a different format >> then the MTD framework. Skip the automatic BBT scan with the Not only about the boot blocks. The rest of the Nand is formatted as well by closed source. It formats the location at the new BBT pages as well. >> NAND_SKIP_BBTSCAN option so that the original content is unchanged >> during the driver probe. >> The NAND_NO_BBM_QUIRK option allows us to erase bad blocks with >> the nand_erase_nand() function and the flash_erase command. >> With these options the user has the "freedom of choice" by neutral >> access mode to read and write in whatever format is needed. > > Can the boot_rom_mode thing help? This boot_rom_mode thing is for switching ECC mode in boot block pages and is not related to BBT pages. > > For writing this part you can disable the bad block protection using > debugfs and then write an externally processed file in raw mode I guess. The use of debugfs only might make sense when the driver can pass the probe function without errors. It does however not save guard existing data when it creates and writes to a new BBT page location. When the data at the new BBT pages previously is written with a different ECC or data/OOB format the hardware driver probe fails. Your suggestion does not work for the Rockchip case. Please advise what to do with this patch. === [ 2148.026403] calling rk_nfc_driver_init+0x0/0x1000 [rockchip_nand_controller] @ 2062 [ 2148.039156] rockchip-nfc 10500000.nand-controller: timing 11e1 [ 2148.056891] nand: device found, Manufacturer ID: 0xad, Chip ID: 0xde [ 2148.067700] nand: Hynix H27UCG8T2ATR-BC 64G 3.3V 8-bit [ 2148.076717] nand: 8192 MiB, MLC, erase size: 2048 KiB, page size: 8192, OOB size: 640 [ 2148.089022] rockchip-nfc 10500000.nand-controller: hynix_nand_init [ 2148.099317] rockchip-nfc 10500000.nand-controller: h27ucg8t2atrbc_choose_interface_config [ 2148.111779] rockchip-nfc 10500000.nand-controller: timing 11e1 [ 2148.129836] rockchip-nfc 10500000.nand-controller: rd hw page: 1048575 [ 2148.149364] rockchip-nfc 10500000.nand-controller: read page: fffff ecc error! [ 2148.160742] rockchip-nfc 10500000.nand-controller: rd hw page: 1048319 [ 2148.180164] rockchip-nfc 10500000.nand-controller: read page: ffeff ecc error! [ 2148.191413] rockchip-nfc 10500000.nand-controller: rd hw page: 1048063 [..] Check for existing BBT pages. [ 2148.339836] rockchip-nfc 10500000.nand-controller: read page: ffdff ecc error! [ 2148.350864] rockchip-nfc 10500000.nand-controller: rd hw page: 1047807 [ 2148.369835] rockchip-nfc 10500000.nand-controller: read page: ffcff ecc error! [ 2148.380597] Bad block table not found for chip 0 [ 2148.388479] Scanning device for bad blocks [ 2148.395591] rockchip-nfc 10500000.nand-controller: rd hw page: 255 [ 2148.414087] rockchip-nfc 10500000.nand-controller: read page: max_bitflips: 0 [..] Check all pages to create a new BBT. [ 2258.693279] rockchip-nfc 10500000.nand-controller: rd hw page: 1030143 [ 2258.710970] rockchip-nfc 10500000.nand-controller: read page: max_bitflips: 0 [ 2258.720804] rockchip-nfc 10500000.nand-controller: rd hw page: 1030399 [ 2258.738394] rockchip-nfc 10500000.nand-controller: read page: fb8ff ecc error! [ 2258.748229] Bad eraseblock 4024 at 0x0001f7000000 [..] All BBT pages are marked bad, so it thinks there's no space left [ 2261.403708] rockchip-nfc 10500000.nand-controller: rd hw page: 1048575 [ 2261.422750] rockchip-nfc 10500000.nand-controller: read page: fffff ecc error! [ 2261.433634] Bad eraseblock 4095 at 0x0001ffe00000 [ 2261.441723] No space left to write bad block table This is not done. A Nand disk should not be altered if it's formatted in a different format unless an explicit command is given. [ 2261.449860] nand_bbt: error while writing bad block table -28 [ 2261.467156] rockchip-nfc 10500000.nand-controller: failed to init NAND chips [ 2261.477917] rockchip-nfc: probe of 10500000.nand-controller failed with error -28 [ 2261.497011] probe of 10500000.nand-controller returned 28 after 113456649 usecs [ 2261.508273] initcall rk_nfc_driver_init+0x0/0x1000 [rockchip_nand_controller] returned 0 after 113468040 usecs Probe failed, but there nothing wrong with hardware. MTDx partitions are not created. User space commands that rely strings like "/dev/mtd0" don't work. === Application Kernel Drivers Hardware Why do we fail a hardware driver probe when the level above doesn't deal with all real world situations. Shouldn't that be more separated in MTD levels. === > > For the boot I think I proposed a DT property. I don't remember how far > the discussion went. Is there a web link of that discussion? How do I link 'compatible' to '/dev/mtd0' for example. In a '/proc/mtd' entry? See /drivers/mtd/spi-nor/debugfs.c In order to write boot blocks in a pattern it needs to know the chip ID as used in nand_flash_ids[]. How can we export this ID to userspace? Also in a '/proc/mtd' entry? === partitions { compatible = "fixed-partitions"; #address-cells = <2>; #size-cells = <2>; partition@0 { reg = <0x0 0x0 0x0 0x02000000>; compatible = "rockchip,boot-blk"; label = "boot-blk"; }; partition@2000000 { reg = <0x0 0x02000000 0x1 0xfa000000>; label = "rootfs"; }; partition@1fc000000 { reg = <0x1 0xfc000000 0x0 0x04000000>; compatible = "mtd,bbt" label = "bbt"; }; }; How many erase blocks is MTD using for BBT pages? 4 or 8 or ? BBT pages are not clearly defined in the DT. They are somehow marked bad in the overlapping partition. How are suppose to erase BBT pages in a automated way? Is there a need for a new BBT compatible? What is your idea about compatibles suggestion: rockchip,boot-blk mtd,bbt === Johan > >> >> Signed-off-by: Johan Jonker <jbx6244@gmail.com> >> --- >> >> Changes V3: >> Change prefixes >> >> Changed V2: >> reword >> >> --- >> >> I'm aware that the maintainer finds it "awful", >> but it's absolute necessary to: >> 1: read/write boot blocks in user space without touching original content >> 2: format a NAND for MTD either with built in or external driver module >> >> So we keep it include in this serie. >> --- >> drivers/mtd/nand/raw/rockchip-nand-controller.c | 7 +++++++ >> 1 file changed, 7 insertions(+) >> >> diff --git a/drivers/mtd/nand/raw/rockchip-nand-controller.c b/drivers/mtd/nand/raw/rockchip-nand-controller.c >> index 5a0468034..fcda4c760 100644 >> --- a/drivers/mtd/nand/raw/rockchip-nand-controller.c >> +++ b/drivers/mtd/nand/raw/rockchip-nand-controller.c >> @@ -188,6 +188,10 @@ struct rk_nfc { >> unsigned long assigned_cs; >> }; >> >> +static int skipbbt; >> +module_param(skipbbt, int, 0644); >> +MODULE_PARM_DESC(skipbbt, "Skip BBT scan if data on the NAND chip is not in MTD format."); >> + >> static inline struct rk_nfc_nand_chip *rk_nfc_to_rknand(struct nand_chip *chip) >> { >> return container_of(chip, struct rk_nfc_nand_chip, chip); >> @@ -1153,6 +1157,9 @@ static int rk_nfc_nand_chip_init(struct device *dev, struct rk_nfc *nfc, >> >> nand_set_controller_data(chip, nfc); >> >> + if (skipbbt) >> + chip->options |= NAND_SKIP_BBTSCAN | NAND_NO_BBM_QUIRK; >> + >> chip->options |= NAND_USES_DMA | NAND_NO_SUBPAGE_WRITE; >> chip->bbt_options = NAND_BBT_USE_FLASH | NAND_BBT_NO_OOB; >> >> -- >> 2.30.2 >> > > > Thanks, > Miquèl ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 3/3] mtd: rawnand: rockchip-nand-controller: add skipbbt option 2023-07-07 15:27 ` Johan Jonker @ 2023-07-12 13:57 ` Miquel Raynal 2023-07-13 15:17 ` Richard Weinberger 0 siblings, 1 reply; 11+ messages in thread From: Miquel Raynal @ 2023-07-12 13:57 UTC (permalink / raw) To: Johan Jonker Cc: richard, vigneshr, heiko, linux-mtd, linux-kernel, linux-arm-kernel, linux-rockchip, yifeng.zhao Hi Johan, Richard, a question for you below :-) jbx6244@gmail.com wrote on Fri, 7 Jul 2023 17:27:56 +0200: > Hi Miquel, > > Some comments/questions below, There are a lot, I've selected the ones which appear the most relevant to me right now. > On 7/4/23 17:13, Miquel Raynal wrote: > > Hi Johan, > > > > jbx6244@gmail.com wrote on Thu, 15 Jun 2023 19:34:26 +0200: > > > >> On Rockchip SoCs the first boot stages are written on NAND > > >> with help of manufacturer software that uses a different format > >> then the MTD framework. Skip the automatic BBT scan with the > > Not only about the boot blocks. > The rest of the Nand is formatted as well by closed source. > It formats the location at the new BBT pages as well. > > >> NAND_SKIP_BBTSCAN option so that the original content is unchanged > >> during the driver probe. > >> The NAND_NO_BBM_QUIRK option allows us to erase bad blocks with > >> the nand_erase_nand() function and the flash_erase command. > >> With these options the user has the "freedom of choice" by neutral > >> access mode to read and write in whatever format is needed. > > > > > Can the boot_rom_mode thing help? > > This boot_rom_mode thing is for switching ECC mode in boot block pages and is not related to BBT pages. > > > > > For writing this part you can disable the bad block protection using > > debugfs and then write an externally processed file in raw mode I guess. > > The use of debugfs only might make sense when the driver can pass the probe function without errors. > It does however not save guard existing data when it creates and writes to a new BBT page location. > When the data at the new BBT pages previously is written with a different ECC or data/OOB format the hardware driver probe fails. > > Your suggestion does not work for the Rockchip case. > Please advise what to do with this patch. > > === > > [ 2148.026403] calling rk_nfc_driver_init+0x0/0x1000 [rockchip_nand_controller] @ 2062 > [ 2148.039156] rockchip-nfc 10500000.nand-controller: timing 11e1 > [ 2148.056891] nand: device found, Manufacturer ID: 0xad, Chip ID: 0xde > [ 2148.067700] nand: Hynix H27UCG8T2ATR-BC 64G 3.3V 8-bit > [ 2148.076717] nand: 8192 MiB, MLC, erase size: 2048 KiB, page size: 8192, OOB size: 640 > [ 2148.089022] rockchip-nfc 10500000.nand-controller: hynix_nand_init > [ 2148.099317] rockchip-nfc 10500000.nand-controller: h27ucg8t2atrbc_choose_interface_config > [ 2148.111779] rockchip-nfc 10500000.nand-controller: timing 11e1 > [ 2148.129836] rockchip-nfc 10500000.nand-controller: rd hw page: 1048575 > [ 2148.149364] rockchip-nfc 10500000.nand-controller: read page: fffff ecc error! > [ 2148.160742] rockchip-nfc 10500000.nand-controller: rd hw page: 1048319 > [ 2148.180164] rockchip-nfc 10500000.nand-controller: read page: ffeff ecc error! > [ 2148.191413] rockchip-nfc 10500000.nand-controller: rd hw page: 1048063 > > [..] > > Check for existing BBT pages. > > [ 2148.339836] rockchip-nfc 10500000.nand-controller: read page: ffdff ecc error! > [ 2148.350864] rockchip-nfc 10500000.nand-controller: rd hw page: 1047807 > [ 2148.369835] rockchip-nfc 10500000.nand-controller: read page: ffcff ecc error! > [ 2148.380597] Bad block table not found for chip 0 > [ 2148.388479] Scanning device for bad blocks > [ 2148.395591] rockchip-nfc 10500000.nand-controller: rd hw page: 255 > [ 2148.414087] rockchip-nfc 10500000.nand-controller: read page: max_bitflips: 0 > > [..] > > Check all pages to create a new BBT. > > [ 2258.693279] rockchip-nfc 10500000.nand-controller: rd hw page: 1030143 > [ 2258.710970] rockchip-nfc 10500000.nand-controller: read page: max_bitflips: 0 > [ 2258.720804] rockchip-nfc 10500000.nand-controller: rd hw page: 1030399 > [ 2258.738394] rockchip-nfc 10500000.nand-controller: read page: fb8ff ecc error! > [ 2258.748229] Bad eraseblock 4024 at 0x0001f7000000 > > [..] > > All BBT pages are marked bad, so it thinks there's no space left > > [ 2261.403708] rockchip-nfc 10500000.nand-controller: rd hw page: 1048575 > [ 2261.422750] rockchip-nfc 10500000.nand-controller: read page: fffff ecc error! > [ 2261.433634] Bad eraseblock 4095 at 0x0001ffe00000 > > [ 2261.441723] No space left to write bad block table > > This is not done. > A Nand disk should not be altered if it's formatted in a different format unless an explicit command is given. > > [ 2261.449860] nand_bbt: error while writing bad block table -28 > [ 2261.467156] rockchip-nfc 10500000.nand-controller: failed to init NAND chips > [ 2261.477917] rockchip-nfc: probe of 10500000.nand-controller failed with error -28 > [ 2261.497011] probe of 10500000.nand-controller returned 28 after 113456649 usecs > [ 2261.508273] initcall rk_nfc_driver_init+0x0/0x1000 [rockchip_nand_controller] returned 0 after 113468040 usecs > > Probe failed, but there nothing wrong with hardware. > MTDx partitions are not created. > User space commands that rely strings like "/dev/mtd0" don't work. > > === > > Application > Kernel > Drivers > Hardware > > Why do we fail a hardware driver probe when the level above doesn't deal with all real world situations. > Shouldn't that be more separated in MTD levels. > > === > > > > > For the boot I think I proposed a DT property. I don't remember how far > > the discussion went. > > Is there a web link of that discussion? https://lore.kernel.org/linux-mtd/20230609104426.3901df54@xps-13/ Maybe the term "DT property" did not appear but that's what I had in mind :-) I don't know if it must be a chip or a partition property. Richard, here is where I would like your feedback, I am kind of opposed to a "skip_bbt" module parameter. It's not a strong opinion, if you think it's fine I'm open. I would rather prefer a DT property which says "do not use the standard pattern". Johan, how are bad blocks supposed to be tracked if the bad block tables are written in a way which does not allow Linux to read it? > How do I link 'compatible' to '/dev/mtd0' for example. > In a '/proc/mtd' entry? > See /drivers/mtd/spi-nor/debugfs.c > > > In order to write boot blocks in a pattern it needs to know the chip ID as used in nand_flash_ids[]. > How can we export this ID to userspace? There was an attempt a few days back, it's not upstream, I can't find it back ATM. I'll send it once I find it back. > Also in a '/proc/mtd' entry? > > === > > partitions { > compatible = "fixed-partitions"; > #address-cells = <2>; > #size-cells = <2>; > > partition@0 { > reg = <0x0 0x0 0x0 0x02000000>; > compatible = "rockchip,boot-blk"; > label = "boot-blk"; > }; > > partition@2000000 { > reg = <0x0 0x02000000 0x1 0xfa000000>; > label = "rootfs"; > }; > > partition@1fc000000 { > reg = <0x1 0xfc000000 0x0 0x04000000>; > compatible = "mtd,bbt" > label = "bbt"; This does not work, it would take me hours to explain why, it's all about: - stability - how mtd has been implemented > }; > }; > > How many erase blocks is MTD using for BBT pages? 4 or 8 or ? As many blocks are needed to cover the size of the device, times 2. > BBT pages are not clearly defined in the DT. No, because this is software, not hardware. MTD is a Linux thing, which gives you access to an mtd device through a number of operations. It handles bad blocks. > They are somehow marked bad in the overlapping partition. Not sure what "overlapping partition" means. > How are suppose to erase BBT pages in a automated way? > Is there a need for a new BBT compatible? No, Linux has run with this standard pattern for 20 years, downstream kernels sometimes make silly design decisions, we do not want to support them. > What is your idea about compatibles suggestion: > rockchip,boot-blk > mtd,bbt > > === > > Johan > > > > >> > >> Signed-off-by: Johan Jonker <jbx6244@gmail.com> > >> --- > >> > >> Changes V3: > >> Change prefixes > >> > >> Changed V2: > >> reword > >> > >> --- > >> > >> I'm aware that the maintainer finds it "awful", > >> but it's absolute necessary to: > >> 1: read/write boot blocks in user space without touching original content > >> 2: format a NAND for MTD either with built in or external driver module > >> > >> So we keep it include in this serie. > >> --- > >> drivers/mtd/nand/raw/rockchip-nand-controller.c | 7 +++++++ > >> 1 file changed, 7 insertions(+) > >> > >> diff --git a/drivers/mtd/nand/raw/rockchip-nand-controller.c b/drivers/mtd/nand/raw/rockchip-nand-controller.c > >> index 5a0468034..fcda4c760 100644 > >> --- a/drivers/mtd/nand/raw/rockchip-nand-controller.c > >> +++ b/drivers/mtd/nand/raw/rockchip-nand-controller.c > >> @@ -188,6 +188,10 @@ struct rk_nfc { > >> unsigned long assigned_cs; > >> }; > >> > >> +static int skipbbt; > >> +module_param(skipbbt, int, 0644); > >> +MODULE_PARM_DESC(skipbbt, "Skip BBT scan if data on the NAND chip is not in MTD format."); > >> + > >> static inline struct rk_nfc_nand_chip *rk_nfc_to_rknand(struct nand_chip *chip) > >> { > >> return container_of(chip, struct rk_nfc_nand_chip, chip); > >> @@ -1153,6 +1157,9 @@ static int rk_nfc_nand_chip_init(struct device *dev, struct rk_nfc *nfc, > >> > >> nand_set_controller_data(chip, nfc); > >> > >> + if (skipbbt) > >> + chip->options |= NAND_SKIP_BBTSCAN | NAND_NO_BBM_QUIRK; > >> + > >> chip->options |= NAND_USES_DMA | NAND_NO_SUBPAGE_WRITE; > >> chip->bbt_options = NAND_BBT_USE_FLASH | NAND_BBT_NO_OOB; > >> > >> -- > >> 2.30.2 > >> > > > > > > Thanks, > > Miquèl Thanks, Miquèl ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 3/3] mtd: rawnand: rockchip-nand-controller: add skipbbt option 2023-07-12 13:57 ` Miquel Raynal @ 2023-07-13 15:17 ` Richard Weinberger 0 siblings, 0 replies; 11+ messages in thread From: Richard Weinberger @ 2023-07-13 15:17 UTC (permalink / raw) To: Miquel Raynal Cc: Johan Jonker, Vignesh Raghavendra, heiko, linux-mtd, linux-kernel, linux-arm-kernel, linux-rockchip, yifeng zhao ----- Ursprüngliche Mail ----- > >> > For the boot I think I proposed a DT property. I don't remember how far >> > the discussion went. >> >> Is there a web link of that discussion? > > https://lore.kernel.org/linux-mtd/20230609104426.3901df54@xps-13/ > > Maybe the term "DT property" did not appear but that's what I had in > mind :-) I don't know if it must be a chip or a partition property. > > Richard, here is where I would like your feedback, I am kind of opposed > to a "skip_bbt" module parameter. It's not a strong opinion, if you > think it's fine I'm open. > > I would rather prefer a DT property which says "do not use the > standard pattern". Yes, please no new module parameters. A module parameter should only affect the behavior of the driver itself (e.g. debug mode) but not be device specific. We made this mistake in the past too often. :-( A single driver can serve more devices. Therefore IMHO controlling this via DT makes perfectly sense. Thanks, //richard ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2023-07-13 15:17 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-06-15 17:32 [PATCH v3 0/3] Fixes for Rockchip NAND controller driver Johan Jonker 2023-06-15 17:34 ` [PATCH v3 1/3] mtd: rawnand: rockchip-nand-controller: fix oobfree offset and description Johan Jonker 2023-07-04 15:03 ` Miquel Raynal 2023-06-15 17:34 ` [PATCH v3 2/3] mtd: rawnand: rockchip-nand-controller: copy hwecc PA data to oob_poi buffer Johan Jonker 2023-07-04 15:08 ` Miquel Raynal 2023-07-05 18:20 ` Johan Jonker 2023-06-15 17:34 ` [PATCH v3 3/3] mtd: rawnand: rockchip-nand-controller: add skipbbt option Johan Jonker 2023-07-04 15:13 ` Miquel Raynal 2023-07-07 15:27 ` Johan Jonker 2023-07-12 13:57 ` Miquel Raynal 2023-07-13 15:17 ` Richard Weinberger
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).