[PATCH v1 0/5] Fixes for Rockchip NAND controller driver

[PATCH v1 0/5] Fixes for Rockchip NAND controller driver

[Johan Jonker]

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 default timing.
  Fix oobfree layout.
  Add missing chip ID.

Johan Jonker (4):
  mtd: nand: raw: rockchip-nand-controller: copy hwecc PA data to
    oob_poi buffer
  mtd: nand: raw: rockchip-nand-controller: add skipbbt option
  mtd: nand: raw: rockchip-nand-controller: fix nand timing default
  mtd: nand: raw: rockchip-nand-controller: fix oobfree offset and
    description

Paweł Jarosz (1):
  mtd: nand: add support for the Sandisk SDTNQGAMA chip

 drivers/mtd/nand/raw/nand_ids.c               |  3 +
 .../mtd/nand/raw/rockchip-nand-controller.c   | 59 ++++++++++++-------
 2 files changed, 40 insertions(+), 22 deletions(-)

--
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v1 1/5] mtd: nand: raw: rockchip-nand-controller: copy hwecc PA data to oob_poi buffer

[Johan Jonker]

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 last 4 bytes in the
chip->oob_poi data layout.

Signed-off-by: Johan Jonker <jbx6244@gmail.com>
---
 .../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 2312e2736..cafccc324 100644
--- a/drivers/mtd/nand/raw/rockchip-nand-controller.c
+++ b/drivers/mtd/nand/raw/rockchip-nand-controller.c
@@ -597,7 +597,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);
@@ -624,6 +624,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)) &&
@@ -634,21 +641,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,
@@ -811,12 +814,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-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v1 2/5] mtd: nand: raw: rockchip-nand-controller: add skipbbt option

[Johan Jonker]

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 we can run it manually.
The NAND_NO_BBM_QUIRK option allows us to erase bad blocks with
the nand_erase_nand() function and the flash_erase command.

Signed-off-by: Johan Jonker <jbx6244@gmail.com>
---
 drivers/mtd/nand/raw/rockchip-nand-controller.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/mtd/nand/raw/rockchip-nand-controller.c b/drivers/mtd/nand/raw/rockchip-nand-controller.c
index cafccc324..f56430f6c 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 the NAND chip contains data 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);
@@ -1156,6 +1160,10 @@ static int rk_nfc_nand_chip_init(struct device *dev, struct rk_nfc *nfc,

 	nand_set_controller_data(chip, nfc);

+	/* Skip the automatic BBT scan so we can run it manually. */
+	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-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 2/5] mtd: nand: raw: rockchip-nand-controller: add skipbbt option

[Miquel Raynal]

Hi Johan,

jbx6244@gmail.com wrote on Thu, 8 Jun 2023 18:30:27 +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 we can run it manually.

How do you run it manually?

> The NAND_NO_BBM_QUIRK option allows us to erase bad blocks with
> the nand_erase_nand() function and the flash_erase command.

For erasure you now have access to a debugfs entry for
experts/forensics which allows you to bypass all bad block checks.

> 
> Signed-off-by: Johan Jonker <jbx6244@gmail.com>
> ---
>  drivers/mtd/nand/raw/rockchip-nand-controller.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/mtd/nand/raw/rockchip-nand-controller.c b/drivers/mtd/nand/raw/rockchip-nand-controller.c
> index cafccc324..f56430f6c 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 the NAND chip contains data not in MTD format.");

I highly dislike this.

It's not a module parameter that you need, it is related to a partition.

> +
>  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);
> @@ -1156,6 +1160,10 @@ static int rk_nfc_nand_chip_init(struct device *dev, struct rk_nfc *nfc,
> 
>  	nand_set_controller_data(chip, nfc);
> 
> +	/* Skip the automatic BBT scan so we can run it manually. */
> +	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-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v1 3/5] mtd: nand: raw: rockchip-nand-controller: fix nand timing default

[Johan Jonker]

Somehow not all NAND chips give a valid timing setting with the
nand_get_sdr_timings() function. Don't consider it as an error,
but fall back to the default value in order to continue to use
the driver.

Signed-off-by: Johan Jonker <jbx6244@gmail.com>
---
 drivers/mtd/nand/raw/rockchip-nand-controller.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/nand/raw/rockchip-nand-controller.c b/drivers/mtd/nand/raw/rockchip-nand-controller.c
index f56430f6c..e39431cfa 100644
--- a/drivers/mtd/nand/raw/rockchip-nand-controller.c
+++ b/drivers/mtd/nand/raw/rockchip-nand-controller.c
@@ -429,8 +429,10 @@ static int rk_nfc_setup_interface(struct nand_chip *chip, int target,
 		return 0;

 	timings = nand_get_sdr_timings(conf);
-	if (IS_ERR(timings))
-		return -EOPNOTSUPP;
+	if (IS_ERR(timings)) {
+		rknand->timing = 0x1081;
+		return 0;
+	}

 	if (IS_ERR(nfc->nfc_clk))
 		rate = clk_get_rate(nfc->ahb_clk);
--
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 3/5] mtd: nand: raw: rockchip-nand-controller: fix nand timing default

[Miquel Raynal]

Hi Johan,

jbx6244@gmail.com wrote on Thu, 8 Jun 2023 18:30:40 +0200:

> Somehow not all NAND chips give a valid timing setting with the
> nand_get_sdr_timings() function.

nand_get_sdr_timings() is a core function and is not particularly
clever. All chips are supposed to support SDR mode 0 so if you chip
does not advertises that the chip is broken, not the controller. This
must be fixed in the chip manufacturer driver, not in the controller
driver.

> Don't consider it as an error,
> but fall back to the default value in order to continue to use
> the driver.
> 
> Signed-off-by: Johan Jonker <jbx6244@gmail.com>
> ---
>  drivers/mtd/nand/raw/rockchip-nand-controller.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/rockchip-nand-controller.c b/drivers/mtd/nand/raw/rockchip-nand-controller.c
> index f56430f6c..e39431cfa 100644
> --- a/drivers/mtd/nand/raw/rockchip-nand-controller.c
> +++ b/drivers/mtd/nand/raw/rockchip-nand-controller.c
> @@ -429,8 +429,10 @@ static int rk_nfc_setup_interface(struct nand_chip *chip, int target,
>  		return 0;
> 
>  	timings = nand_get_sdr_timings(conf);
> -	if (IS_ERR(timings))
> -		return -EOPNOTSUPP;
> +	if (IS_ERR(timings)) {
> +		rknand->timing = 0x1081;

This is way to magical anyway :)

> +		return 0;
> +	}
> 
>  	if (IS_ERR(nfc->nfc_clk))
>  		rate = clk_get_rate(nfc->ahb_clk);
> --
> 2.30.2
> 


Thanks,
Miquèl

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v1 4/5] mtd: nand: raw: rockchip-nand-controller: fix oobfree offset and description

[Johan Jonker]

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. The first free OOB byte is therefore OOB2 at offset 2.
Page address(PA) bytes are moved to the last 4 positions before
ECC. Update the description for Linux.

Signed-off-by: Johan Jonker <jbx6244@gmail.com>
---
 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 e39431cfa..cf0fe502f 100644
--- a/drivers/mtd/nand/raw/rockchip-nand-controller.c
+++ b/drivers/mtd/nand/raw/rockchip-nand-controller.c
@@ -568,9 +568,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),
@@ -947,12 +948,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-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v1 5/5] mtd: nand: add support for the Sandisk SDTNQGAMA chip

[Johan Jonker]

Sandisk SDTNQGAMA is a 8GB size, 3.3V 8 bit chip with 16KB page size,
1KB write size and 40 bit ecc support

Signed-off-by: Paweł Jarosz <paweljarosz3691@gmail.com>
Signed-off-by: Johan Jonker <jbx6244@gmail.com>
---
 drivers/mtd/nand/raw/nand_ids.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/mtd/nand/raw/nand_ids.c b/drivers/mtd/nand/raw/nand_ids.c
index dacc5529b..53c4118de 100644
--- a/drivers/mtd/nand/raw/nand_ids.c
+++ b/drivers/mtd/nand/raw/nand_ids.c
@@ -44,6 +44,9 @@ struct nand_flash_dev nand_flash_ids[] = {
 	{"TC58NVG6D2 64G 3.3V 8-bit",
 		{ .id = {0x98, 0xde, 0x94, 0x82, 0x76, 0x56, 0x04, 0x20} },
 		  SZ_8K, SZ_8K, SZ_2M, 0, 8, 640, NAND_ECC_INFO(40, SZ_1K) },
+	{"SDTNQGAMA 64G 3.3V 8-bit",
+		{ .id = {0x45, 0xde, 0x94, 0x93, 0x76, 0x57} },
+		  SZ_16K, SZ_8K, SZ_4M, 0, 6, 1280, NAND_ECC_INFO(40, SZ_1K) },
 	{"SDTNRGAMA 64G 3.3V 8-bit",
 		{ .id = {0x45, 0xde, 0x94, 0x93, 0x76, 0x50} },
 		  SZ_16K, SZ_8K, SZ_4M, 0, 6, 1280, NAND_ECC_INFO(40, SZ_1K) },
--
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 5/5] mtd: nand: add support for the Sandisk SDTNQGAMA chip

[Miquel Raynal]

Hi Johan,

jbx6244@gmail.com wrote on Thu, 8 Jun 2023 18:31:04 +0200:

> Sandisk SDTNQGAMA is a 8GB size, 3.3V 8 bit chip with 16KB page size,
> 1KB write size and 40 bit ecc support
> 
> Signed-off-by: Paweł Jarosz <paweljarosz3691@gmail.com>

Pawel needs to be author of the patch or credited with a
co-developped-by+SoB otherwise. The current SoB lines are invalid.

> Signed-off-by: Johan Jonker <jbx6244@gmail.com>
> ---
>  drivers/mtd/nand/raw/nand_ids.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/mtd/nand/raw/nand_ids.c b/drivers/mtd/nand/raw/nand_ids.c
> index dacc5529b..53c4118de 100644
> --- a/drivers/mtd/nand/raw/nand_ids.c
> +++ b/drivers/mtd/nand/raw/nand_ids.c
> @@ -44,6 +44,9 @@ struct nand_flash_dev nand_flash_ids[] = {
>  	{"TC58NVG6D2 64G 3.3V 8-bit",
>  		{ .id = {0x98, 0xde, 0x94, 0x82, 0x76, 0x56, 0x04, 0x20} },
>  		  SZ_8K, SZ_8K, SZ_2M, 0, 8, 640, NAND_ECC_INFO(40, SZ_1K) },
> +	{"SDTNQGAMA 64G 3.3V 8-bit",
> +		{ .id = {0x45, 0xde, 0x94, 0x93, 0x76, 0x57} },
> +		  SZ_16K, SZ_8K, SZ_4M, 0, 6, 1280, NAND_ECC_INFO(40, SZ_1K) },

I guess you'll need a sandisk driver to reflect the missing timings?

>  	{"SDTNRGAMA 64G 3.3V 8-bit",
>  		{ .id = {0x45, 0xde, 0x94, 0x93, 0x76, 0x50} },
>  		  SZ_16K, SZ_8K, SZ_4M, 0, 6, 1280, NAND_ECC_INFO(40, SZ_1K) },
> --
> 2.30.2
> 


Thanks,
Miquèl

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel