[PATCH v5 0/2] Meson: R/W support for pages used by boot ROM

[PATCH v5 0/2] Meson: R/W support for pages used by boot ROM

[Arseniy Krasnov]

Amlogic's boot ROM code needs that some pages on NAND must be written
in special "short" ECC mode with scrambling enabled. Such pages:
1) Contain some metadata about hardware.
2) Located with some interval starting from 0 offset, until some
   specified offset. Interval and second offset are set in the
   device tree.

This patchset adds R/W support for such pages. To enable it we can setup
it in dts:

    nand-is-boot-medium;
    amlogic,boot-pages = <1024>;
    amlogic,boot-page-step = <128>;

It means that each 128th page in range 0 to 1024 pages will be accessed
in special mode ("short" ECC + scrambling). In practice this feature is
needed when we want to update first block of NAND - driver will enable
required mode by itself using value from device tree.

Changelog:
 v1 -> v2:
  * Rename 'meson,boot-page-XXX' -> 'amlogic,boot-page-XXX'.
  * Add words that 'amlogic,boot-page-step' is measured in pages.
  * Remove words that 'amlogic,boot-page-XXX' depends on 'nand-is-boot-medium'.
  * Make both 'amlogic,boot-page-XXX' depend on each other also, in
    addition to 'nand-is-boot-medium' dependency.
 v2 -> v3:
  * Add quotes to 0001 in dependencies. This fixes 'make dt_binding_check'
    warning.
 v3 -> v4:
  * Rename 'amlogic,boot-page-last' to 'amlogic,boot-pages'.
 v4 -> v5:
  * Update 'description' fields in bindings.

Arseniy Krasnov (2):
  dt-bindings: mtd: amlogic,meson-nand: support fields for boot ROM code
  mtd: rawnand: meson: support R/W mode for boot ROM

 .../bindings/mtd/amlogic,meson-nand.yaml      | 14 +++
 drivers/mtd/nand/raw/meson_nand.c             | 88 +++++++++++++------
 2 files changed, 73 insertions(+), 29 deletions(-)

-- 
2.35.0

[PATCH v5 1/2] dt-bindings: mtd: amlogic,meson-nand: support fields for boot ROM code

[Arseniy Krasnov]

Boot ROM code on Meson requires that some pages on NAND must be written
in special mode: "short" ECC mode where each block is 384 bytes and
scrambling mode is on. Such pages located with the specified interval
within specified offset. Both interval and offset are located in the
device tree and used by driver if 'nand-is-boot-medium' is set for
NAND chip.

Signed-off-by: Arseniy Krasnov <avkrasnov@salutedevices.com>
---
 .../bindings/mtd/amlogic,meson-nand.yaml           | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml b/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml
index 57b6957c8415..67b2f7c1259c 100644
--- a/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml
+++ b/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml
@@ -64,11 +64,25 @@ patternProperties:
         items:
           maximum: 0
 
+      amlogic,boot-pages:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        description:
+          Number of pages starting from 0, where special ECC
+          algorithm will be used by the driver.
+
+      amlogic,boot-page-step:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        description:
+          Interval between pages, read/written by the driver with
+          special ECC algorithm.
+
     unevaluatedProperties: false
 
     dependencies:
       nand-ecc-strength: [nand-ecc-step-size]
       nand-ecc-step-size: [nand-ecc-strength]
+      amlogic,boot-pages: [nand-is-boot-medium, "amlogic,boot-page-step"]
+      amlogic,boot-page-step: [nand-is-boot-medium, "amlogic,boot-pages"]
 
 
 required:
-- 
2.35.0

[PATCH v5 2/2] mtd: rawnand: meson: support R/W mode for boot ROM

[Arseniy Krasnov]

Boot ROM code on Meson requires that some pages on NAND must be written
in special mode: "short" ECC mode where each block is 384 bytes and
scrambling mode is on. Such pages located with the specified interval
within specified offset. Both interval and offset are located in the
device tree and used by driver if 'nand-is-boot-medium' is set for
NAND chip.

Signed-off-by: Arseniy Krasnov <avkrasnov@salutedevices.com>
---
 drivers/mtd/nand/raw/meson_nand.c | 88 +++++++++++++++++++++----------
 1 file changed, 59 insertions(+), 29 deletions(-)

diff --git a/drivers/mtd/nand/raw/meson_nand.c b/drivers/mtd/nand/raw/meson_nand.c
index 00ce0e5bb970..9ee11243b257 100644
--- a/drivers/mtd/nand/raw/meson_nand.c
+++ b/drivers/mtd/nand/raw/meson_nand.c
@@ -35,6 +35,7 @@
 #define NFC_CMD_RB		BIT(20)
 #define NFC_CMD_SCRAMBLER_ENABLE	BIT(19)
 #define NFC_CMD_SCRAMBLER_DISABLE	0
+#define NFC_CMD_SHORTMODE_ENABLE	1
 #define NFC_CMD_SHORTMODE_DISABLE	0
 #define NFC_CMD_RB_INT		BIT(14)
 #define NFC_CMD_RB_INT_NO_PIN	((0xb << 10) | BIT(18) | BIT(16))
@@ -78,6 +79,8 @@
 #define DMA_DIR(dir)		((dir) ? NFC_CMD_N2M : NFC_CMD_M2N)
 #define DMA_ADDR_ALIGN		8
 
+#define NFC_SHORT_MODE_ECC_SZ	384
+
 #define ECC_CHECK_RETURN_FF	(-1)
 
 #define NAND_CE0		(0xe << 10)
@@ -125,6 +128,8 @@ struct meson_nfc_nand_chip {
 	u32 twb;
 	u32 tadl;
 	u32 tbers_max;
+	u32 boot_pages;
+	u32 boot_page_step;
 
 	u32 bch_mode;
 	u8 *data_buf;
@@ -298,28 +303,49 @@ static void meson_nfc_cmd_seed(struct meson_nfc *nfc, u32 seed)
 	       nfc->reg_base + NFC_REG_CMD);
 }
 
-static void meson_nfc_cmd_access(struct nand_chip *nand, int raw, bool dir,
-				 int scrambler)
+static int meson_nfc_page_is_boot(struct nand_chip *nand, int page)
+{
+	const struct meson_nfc_nand_chip *meson_chip = to_meson_nand(nand);
+
+	return (nand->options & NAND_IS_BOOT_MEDIUM) &&
+	       !(page % meson_chip->boot_page_step) &&
+	       (page < meson_chip->boot_pages);
+}
+
+static void meson_nfc_cmd_access(struct nand_chip *nand, bool raw, bool dir, int page)
 {
+	const struct meson_nfc_nand_chip *meson_chip = to_meson_nand(nand);
 	struct mtd_info *mtd = nand_to_mtd(nand);
 	struct meson_nfc *nfc = nand_get_controller_data(mtd_to_nand(mtd));
-	struct meson_nfc_nand_chip *meson_chip = to_meson_nand(nand);
-	u32 bch = meson_chip->bch_mode, cmd;
 	int len = mtd->writesize, pagesize, pages;
+	int scrambler;
+	u32 cmd;
 
-	pagesize = nand->ecc.size;
+	if (nand->options & NAND_NEED_SCRAMBLING)
+		scrambler = NFC_CMD_SCRAMBLER_ENABLE;
+	else
+		scrambler = NFC_CMD_SCRAMBLER_DISABLE;
 
 	if (raw) {
 		len = mtd->writesize + mtd->oobsize;
 		cmd = len | scrambler | DMA_DIR(dir);
-		writel(cmd, nfc->reg_base + NFC_REG_CMD);
-		return;
-	}
+	} else if (meson_nfc_page_is_boot(nand, page)) {
+		pagesize = NFC_SHORT_MODE_ECC_SZ >> 3;
+		pages = mtd->writesize / 512;
+
+		scrambler = NFC_CMD_SCRAMBLER_ENABLE;
+		cmd = CMDRWGEN(DMA_DIR(dir), scrambler, NFC_ECC_BCH8_1K,
+			       NFC_CMD_SHORTMODE_ENABLE, pagesize, pages);
+	} else {
+		pagesize = nand->ecc.size >> 3;
+		pages = len / nand->ecc.size;
 
-	pages = len / nand->ecc.size;
+		cmd = CMDRWGEN(DMA_DIR(dir), scrambler, meson_chip->bch_mode,
+			       NFC_CMD_SHORTMODE_DISABLE, pagesize, pages);
+	}
 
-	cmd = CMDRWGEN(DMA_DIR(dir), scrambler, bch,
-		       NFC_CMD_SHORTMODE_DISABLE, pagesize, pages);
+	if (scrambler == NFC_CMD_SCRAMBLER_ENABLE)
+		meson_nfc_cmd_seed(nfc, page);
 
 	writel(cmd, nfc->reg_base + NFC_REG_CMD);
 }
@@ -743,15 +769,7 @@ static int meson_nfc_write_page_sub(struct nand_chip *nand,
 	if (ret)
 		return ret;
 
-	if (nand->options & NAND_NEED_SCRAMBLING) {
-		meson_nfc_cmd_seed(nfc, page);
-		meson_nfc_cmd_access(nand, raw, DIRWRITE,
-				     NFC_CMD_SCRAMBLER_ENABLE);
-	} else {
-		meson_nfc_cmd_access(nand, raw, DIRWRITE,
-				     NFC_CMD_SCRAMBLER_DISABLE);
-	}
-
+	meson_nfc_cmd_access(nand, raw, DIRWRITE, page);
 	cmd = nfc->param.chip_select | NFC_CMD_CLE | NAND_CMD_PAGEPROG;
 	writel(cmd, nfc->reg_base + NFC_REG_CMD);
 	meson_nfc_queue_rb(nand, PSEC_TO_MSEC(sdr->tPROG_max), false);
@@ -829,15 +847,7 @@ static int meson_nfc_read_page_sub(struct nand_chip *nand,
 	if (ret)
 		return ret;
 
-	if (nand->options & NAND_NEED_SCRAMBLING) {
-		meson_nfc_cmd_seed(nfc, page);
-		meson_nfc_cmd_access(nand, raw, DIRREAD,
-				     NFC_CMD_SCRAMBLER_ENABLE);
-	} else {
-		meson_nfc_cmd_access(nand, raw, DIRREAD,
-				     NFC_CMD_SCRAMBLER_DISABLE);
-	}
-
+	meson_nfc_cmd_access(nand, raw, DIRREAD, page);
 	ret = meson_nfc_wait_dma_finish(nfc);
 	meson_nfc_check_ecc_pages_valid(nfc, nand, raw);
 
@@ -1436,6 +1446,26 @@ meson_nfc_nand_chip_init(struct device *dev,
 	if (ret)
 		return ret;
 
+	if (nand->options & NAND_IS_BOOT_MEDIUM) {
+		ret = of_property_read_u32(np, "amlogic,boot-pages",
+					   &meson_chip->boot_pages);
+		if (ret) {
+			dev_err(dev, "could not retrieve 'amlogic,boot-pages' property: %d",
+				ret);
+			nand_cleanup(nand);
+			return ret;
+		}
+
+		ret = of_property_read_u32(np, "amlogic,boot-page-step",
+					   &meson_chip->boot_page_step);
+		if (ret) {
+			dev_err(dev, "could not retrieve 'amlogic,boot-page-step' property: %d",
+				ret);
+			nand_cleanup(nand);
+			return ret;
+		}
+	}
+
 	ret = mtd_device_register(mtd, NULL, 0);
 	if (ret) {
 		dev_err(dev, "failed to register MTD device: %d\n", ret);
-- 
2.35.0

Re: [PATCH v5 1/2] dt-bindings: mtd: amlogic,meson-nand: support fields for boot ROM code

[Rob Herring]

On Tue, 16 Apr 2024 11:51:00 +0300, Arseniy Krasnov wrote:
> Boot ROM code on Meson requires that some pages on NAND must be written
> in special mode: "short" ECC mode where each block is 384 bytes and
> scrambling mode is on. Such pages located with the specified interval
> within specified offset. Both interval and offset are located in the
> device tree and used by driver if 'nand-is-boot-medium' is set for
> NAND chip.
> 
> Signed-off-by: Arseniy Krasnov <avkrasnov@salutedevices.com>
> ---
>  .../bindings/mtd/amlogic,meson-nand.yaml           | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>

Re: [PATCH v5 2/2] mtd: rawnand: meson: support R/W mode for boot ROM

[Arseniy Krasnov]

Hello, sorry, pls ping, 2 weeks :)

Thanks, Arseniy

On 16.04.2024 11:51, Arseniy Krasnov wrote:
> Boot ROM code on Meson requires that some pages on NAND must be written
> in special mode: "short" ECC mode where each block is 384 bytes and
> scrambling mode is on. Such pages located with the specified interval
> within specified offset. Both interval and offset are located in the
> device tree and used by driver if 'nand-is-boot-medium' is set for
> NAND chip.
> 
> Signed-off-by: Arseniy Krasnov <avkrasnov@salutedevices.com>
> ---
>  drivers/mtd/nand/raw/meson_nand.c | 88 +++++++++++++++++++++----------
>  1 file changed, 59 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/meson_nand.c b/drivers/mtd/nand/raw/meson_nand.c
> index 00ce0e5bb970..9ee11243b257 100644
> --- a/drivers/mtd/nand/raw/meson_nand.c
> +++ b/drivers/mtd/nand/raw/meson_nand.c
> @@ -35,6 +35,7 @@
>  #define NFC_CMD_RB		BIT(20)
>  #define NFC_CMD_SCRAMBLER_ENABLE	BIT(19)
>  #define NFC_CMD_SCRAMBLER_DISABLE	0
> +#define NFC_CMD_SHORTMODE_ENABLE	1
>  #define NFC_CMD_SHORTMODE_DISABLE	0
>  #define NFC_CMD_RB_INT		BIT(14)
>  #define NFC_CMD_RB_INT_NO_PIN	((0xb << 10) | BIT(18) | BIT(16))
> @@ -78,6 +79,8 @@
>  #define DMA_DIR(dir)		((dir) ? NFC_CMD_N2M : NFC_CMD_M2N)
>  #define DMA_ADDR_ALIGN		8
>  
> +#define NFC_SHORT_MODE_ECC_SZ	384
> +
>  #define ECC_CHECK_RETURN_FF	(-1)
>  
>  #define NAND_CE0		(0xe << 10)
> @@ -125,6 +128,8 @@ struct meson_nfc_nand_chip {
>  	u32 twb;
>  	u32 tadl;
>  	u32 tbers_max;
> +	u32 boot_pages;
> +	u32 boot_page_step;
>  
>  	u32 bch_mode;
>  	u8 *data_buf;
> @@ -298,28 +303,49 @@ static void meson_nfc_cmd_seed(struct meson_nfc *nfc, u32 seed)
>  	       nfc->reg_base + NFC_REG_CMD);
>  }
>  
> -static void meson_nfc_cmd_access(struct nand_chip *nand, int raw, bool dir,
> -				 int scrambler)
> +static int meson_nfc_page_is_boot(struct nand_chip *nand, int page)
> +{
> +	const struct meson_nfc_nand_chip *meson_chip = to_meson_nand(nand);
> +
> +	return (nand->options & NAND_IS_BOOT_MEDIUM) &&
> +	       !(page % meson_chip->boot_page_step) &&
> +	       (page < meson_chip->boot_pages);
> +}
> +
> +static void meson_nfc_cmd_access(struct nand_chip *nand, bool raw, bool dir, int page)
>  {
> +	const struct meson_nfc_nand_chip *meson_chip = to_meson_nand(nand);
>  	struct mtd_info *mtd = nand_to_mtd(nand);
>  	struct meson_nfc *nfc = nand_get_controller_data(mtd_to_nand(mtd));
> -	struct meson_nfc_nand_chip *meson_chip = to_meson_nand(nand);
> -	u32 bch = meson_chip->bch_mode, cmd;
>  	int len = mtd->writesize, pagesize, pages;
> +	int scrambler;
> +	u32 cmd;
>  
> -	pagesize = nand->ecc.size;
> +	if (nand->options & NAND_NEED_SCRAMBLING)
> +		scrambler = NFC_CMD_SCRAMBLER_ENABLE;
> +	else
> +		scrambler = NFC_CMD_SCRAMBLER_DISABLE;
>  
>  	if (raw) {
>  		len = mtd->writesize + mtd->oobsize;
>  		cmd = len | scrambler | DMA_DIR(dir);
> -		writel(cmd, nfc->reg_base + NFC_REG_CMD);
> -		return;
> -	}
> +	} else if (meson_nfc_page_is_boot(nand, page)) {
> +		pagesize = NFC_SHORT_MODE_ECC_SZ >> 3;
> +		pages = mtd->writesize / 512;
> +
> +		scrambler = NFC_CMD_SCRAMBLER_ENABLE;
> +		cmd = CMDRWGEN(DMA_DIR(dir), scrambler, NFC_ECC_BCH8_1K,
> +			       NFC_CMD_SHORTMODE_ENABLE, pagesize, pages);
> +	} else {
> +		pagesize = nand->ecc.size >> 3;
> +		pages = len / nand->ecc.size;
>  
> -	pages = len / nand->ecc.size;
> +		cmd = CMDRWGEN(DMA_DIR(dir), scrambler, meson_chip->bch_mode,
> +			       NFC_CMD_SHORTMODE_DISABLE, pagesize, pages);
> +	}
>  
> -	cmd = CMDRWGEN(DMA_DIR(dir), scrambler, bch,
> -		       NFC_CMD_SHORTMODE_DISABLE, pagesize, pages);
> +	if (scrambler == NFC_CMD_SCRAMBLER_ENABLE)
> +		meson_nfc_cmd_seed(nfc, page);
>  
>  	writel(cmd, nfc->reg_base + NFC_REG_CMD);
>  }
> @@ -743,15 +769,7 @@ static int meson_nfc_write_page_sub(struct nand_chip *nand,
>  	if (ret)
>  		return ret;
>  
> -	if (nand->options & NAND_NEED_SCRAMBLING) {
> -		meson_nfc_cmd_seed(nfc, page);
> -		meson_nfc_cmd_access(nand, raw, DIRWRITE,
> -				     NFC_CMD_SCRAMBLER_ENABLE);
> -	} else {
> -		meson_nfc_cmd_access(nand, raw, DIRWRITE,
> -				     NFC_CMD_SCRAMBLER_DISABLE);
> -	}
> -
> +	meson_nfc_cmd_access(nand, raw, DIRWRITE, page);
>  	cmd = nfc->param.chip_select | NFC_CMD_CLE | NAND_CMD_PAGEPROG;
>  	writel(cmd, nfc->reg_base + NFC_REG_CMD);
>  	meson_nfc_queue_rb(nand, PSEC_TO_MSEC(sdr->tPROG_max), false);
> @@ -829,15 +847,7 @@ static int meson_nfc_read_page_sub(struct nand_chip *nand,
>  	if (ret)
>  		return ret;
>  
> -	if (nand->options & NAND_NEED_SCRAMBLING) {
> -		meson_nfc_cmd_seed(nfc, page);
> -		meson_nfc_cmd_access(nand, raw, DIRREAD,
> -				     NFC_CMD_SCRAMBLER_ENABLE);
> -	} else {
> -		meson_nfc_cmd_access(nand, raw, DIRREAD,
> -				     NFC_CMD_SCRAMBLER_DISABLE);
> -	}
> -
> +	meson_nfc_cmd_access(nand, raw, DIRREAD, page);
>  	ret = meson_nfc_wait_dma_finish(nfc);
>  	meson_nfc_check_ecc_pages_valid(nfc, nand, raw);
>  
> @@ -1436,6 +1446,26 @@ meson_nfc_nand_chip_init(struct device *dev,
>  	if (ret)
>  		return ret;
>  
> +	if (nand->options & NAND_IS_BOOT_MEDIUM) {
> +		ret = of_property_read_u32(np, "amlogic,boot-pages",
> +					   &meson_chip->boot_pages);
> +		if (ret) {
> +			dev_err(dev, "could not retrieve 'amlogic,boot-pages' property: %d",
> +				ret);
> +			nand_cleanup(nand);
> +			return ret;
> +		}
> +
> +		ret = of_property_read_u32(np, "amlogic,boot-page-step",
> +					   &meson_chip->boot_page_step);
> +		if (ret) {
> +			dev_err(dev, "could not retrieve 'amlogic,boot-page-step' property: %d",
> +				ret);
> +			nand_cleanup(nand);
> +			return ret;
> +		}
> +	}
> +
>  	ret = mtd_device_register(mtd, NULL, 0);
>  	if (ret) {
>  		dev_err(dev, "failed to register MTD device: %d\n", ret);

Re: [PATCH v5 2/2] mtd: rawnand: meson: support R/W mode for boot ROM

[Arseniy Krasnov]

On 06.05.2024 11:47, Miquel Raynal wrote:
> Hi Arseniy,
> 
> avkrasnov@salutedevices.com wrote on Thu, 2 May 2024 08:09:34 +0300:
> 
>> Hello, sorry, pls ping, 2 weeks :)
> 
> I was on vacation, I am currently late by 30+ patches, yours is on the
> list ;)
> 

Hi! Ah , ok, no problem! Thanks! :)

Thanks, Arseniy

> Thanks,
> Miquèl

Re: [PATCH v5 2/2] mtd: rawnand: meson: support R/W mode for boot ROM

[Miquel Raynal]

Hi Arseniy,

avkrasnov@salutedevices.com wrote on Thu, 2 May 2024 08:09:34 +0300:

> Hello, sorry, pls ping, 2 weeks :)

I was on vacation, I am currently late by 30+ patches, yours is on the
list ;)

Thanks,
Miquèl

Re: [PATCH v5 1/2] dt-bindings: mtd: amlogic,meson-nand: support fields for boot ROM code

[Miquel Raynal]

Hi Arseniy,

avkrasnov@salutedevices.com wrote on Tue, 16 Apr 2024 11:51:00 +0300:

> Boot ROM code on Meson requires that some pages on NAND must be written
> in special mode: "short" ECC mode where each block is 384 bytes and
> scrambling mode is on.

Ok

> Such pages located with the specified interval within specified offset.

I'm sorry I don't get that sentence.

> Both interval and offset are located in the
> device tree and used by driver if 'nand-is-boot-medium' is set for
> NAND chip.

This sentence is probably not needed.

> 
> Signed-off-by: Arseniy Krasnov <avkrasnov@salutedevices.com>
> ---
>  .../bindings/mtd/amlogic,meson-nand.yaml           | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml b/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml
> index 57b6957c8415..67b2f7c1259c 100644
> --- a/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml
> +++ b/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml
> @@ -64,11 +64,25 @@ patternProperties:
>          items:
>            maximum: 0
>  
> +      amlogic,boot-pages:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description:
> +          Number of pages starting from 0, where special ECC

from *offset* 0 I guess?

> +          algorithm will be used by the driver.

"where a special ECC configuration must be used because it is accessed
by the ROM code"? Maybe you can even detail what are these values if
they are fixed.

You should probably inform that scrambling shall be on as well.
 
> +
> +      amlogic,boot-page-step:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description:
> +          Interval between pages, read/written by the driver with
> +          special ECC algorithm.

I'm not sure I fully get the description. What is the unit here? can
you draw a small ascii-art diagram?

> +
>      unevaluatedProperties: false
>  
>      dependencies:
>        nand-ecc-strength: [nand-ecc-step-size]
>        nand-ecc-step-size: [nand-ecc-strength]
> +      amlogic,boot-pages: [nand-is-boot-medium, "amlogic,boot-page-step"]
> +      amlogic,boot-page-step: [nand-is-boot-medium, "amlogic,boot-pages"]
>  
>  
>  required:


Thanks,
Miquèl

Re: [PATCH v5 2/2] mtd: rawnand: meson: support R/W mode for boot ROM

[Miquel Raynal]

Hi Arseniy,

avkrasnov@salutedevices.com wrote on Tue, 16 Apr 2024 11:51:01 +0300:

> Boot ROM code on Meson requires that some pages on NAND must be written
> in special mode: "short" ECC mode where each block is 384 bytes and
> scrambling mode is on. Such pages located with the specified interval
> within specified offset. Both interval and offset are located in the
> device tree and used by driver if 'nand-is-boot-medium' is set for
> NAND chip.
> 
> Signed-off-by: Arseniy Krasnov <avkrasnov@salutedevices.com>
> ---
>  drivers/mtd/nand/raw/meson_nand.c | 88 +++++++++++++++++++++----------
>  1 file changed, 59 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/meson_nand.c b/drivers/mtd/nand/raw/meson_nand.c
> index 00ce0e5bb970..9ee11243b257 100644
> --- a/drivers/mtd/nand/raw/meson_nand.c
> +++ b/drivers/mtd/nand/raw/meson_nand.c
> @@ -35,6 +35,7 @@
>  #define NFC_CMD_RB		BIT(20)
>  #define NFC_CMD_SCRAMBLER_ENABLE	BIT(19)
>  #define NFC_CMD_SCRAMBLER_DISABLE	0
> +#define NFC_CMD_SHORTMODE_ENABLE	1
>  #define NFC_CMD_SHORTMODE_DISABLE	0
>  #define NFC_CMD_RB_INT		BIT(14)
>  #define NFC_CMD_RB_INT_NO_PIN	((0xb << 10) | BIT(18) | BIT(16))
> @@ -78,6 +79,8 @@
>  #define DMA_DIR(dir)		((dir) ? NFC_CMD_N2M : NFC_CMD_M2N)
>  #define DMA_ADDR_ALIGN		8
>  
> +#define NFC_SHORT_MODE_ECC_SZ	384
> +
>  #define ECC_CHECK_RETURN_FF	(-1)
>  
>  #define NAND_CE0		(0xe << 10)
> @@ -125,6 +128,8 @@ struct meson_nfc_nand_chip {
>  	u32 twb;
>  	u32 tadl;
>  	u32 tbers_max;
> +	u32 boot_pages;
> +	u32 boot_page_step;
>  
>  	u32 bch_mode;
>  	u8 *data_buf;
> @@ -298,28 +303,49 @@ static void meson_nfc_cmd_seed(struct meson_nfc *nfc, u32 seed)
>  	       nfc->reg_base + NFC_REG_CMD);
>  }
>  
> -static void meson_nfc_cmd_access(struct nand_chip *nand, int raw, bool dir,
> -				 int scrambler)
> +static int meson_nfc_page_is_boot(struct nand_chip *nand, int page)

meson_nfc_is_boot_page() is easier to read

> +{
> +	const struct meson_nfc_nand_chip *meson_chip = to_meson_nand(nand);
> +
> +	return (nand->options & NAND_IS_BOOT_MEDIUM) &&
> +	       !(page % meson_chip->boot_page_step) &&

I would dedicate all the space below ->boot_pages to the bootrom, no?
Using space in between sounds silly.

> +	       (page < meson_chip->boot_pages);
> +}
> +
> +static void meson_nfc_cmd_access(struct nand_chip *nand, bool raw, bool dir, int page)
>  {
> +	const struct meson_nfc_nand_chip *meson_chip = to_meson_nand(nand);
>  	struct mtd_info *mtd = nand_to_mtd(nand);
>  	struct meson_nfc *nfc = nand_get_controller_data(mtd_to_nand(mtd));
> -	struct meson_nfc_nand_chip *meson_chip = to_meson_nand(nand);
> -	u32 bch = meson_chip->bch_mode, cmd;
>  	int len = mtd->writesize, pagesize, pages;
> +	int scrambler;
> +	u32 cmd;
>  
> -	pagesize = nand->ecc.size;
> +	if (nand->options & NAND_NEED_SCRAMBLING)
> +		scrambler = NFC_CMD_SCRAMBLER_ENABLE;
> +	else
> +		scrambler = NFC_CMD_SCRAMBLER_DISABLE;

That is a separate feature?

>  
>  	if (raw) {
>  		len = mtd->writesize + mtd->oobsize;
>  		cmd = len | scrambler | DMA_DIR(dir);
> -		writel(cmd, nfc->reg_base + NFC_REG_CMD);
> -		return;
> -	}
> +	} else if (meson_nfc_page_is_boot(nand, page)) {
> +		pagesize = NFC_SHORT_MODE_ECC_SZ >> 3;
> +		pages = mtd->writesize / 512;
> +
> +		scrambler = NFC_CMD_SCRAMBLER_ENABLE;
> +		cmd = CMDRWGEN(DMA_DIR(dir), scrambler, NFC_ECC_BCH8_1K,
> +			       NFC_CMD_SHORTMODE_ENABLE, pagesize, pages);
> +	} else {
> +		pagesize = nand->ecc.size >> 3;
> +		pages = len / nand->ecc.size;
>  
> -	pages = len / nand->ecc.size;
> +		cmd = CMDRWGEN(DMA_DIR(dir), scrambler, meson_chip->bch_mode,
> +			       NFC_CMD_SHORTMODE_DISABLE, pagesize, pages);
> +	}
>  
> -	cmd = CMDRWGEN(DMA_DIR(dir), scrambler, bch,
> -		       NFC_CMD_SHORTMODE_DISABLE, pagesize, pages);
> +	if (scrambler == NFC_CMD_SCRAMBLER_ENABLE)
> +		meson_nfc_cmd_seed(nfc, page);
>  
>  	writel(cmd, nfc->reg_base + NFC_REG_CMD);
>  }
> @@ -743,15 +769,7 @@ static int meson_nfc_write_page_sub(struct nand_chip *nand,
>  	if (ret)
>  		return ret;
>  
> -	if (nand->options & NAND_NEED_SCRAMBLING) {
> -		meson_nfc_cmd_seed(nfc, page);
> -		meson_nfc_cmd_access(nand, raw, DIRWRITE,
> -				     NFC_CMD_SCRAMBLER_ENABLE);
> -	} else {
> -		meson_nfc_cmd_access(nand, raw, DIRWRITE,
> -				     NFC_CMD_SCRAMBLER_DISABLE);
> -	}
> -

Ok I get it, the feature already exist but is handled differently.
Please split this patch:
- improve scrambler handling to facilitate boot page support
- add boot pages support

> +	meson_nfc_cmd_access(nand, raw, DIRWRITE, page);
>  	cmd = nfc->param.chip_select | NFC_CMD_CLE | NAND_CMD_PAGEPROG;
>  	writel(cmd, nfc->reg_base + NFC_REG_CMD);
>  	meson_nfc_queue_rb(nand, PSEC_TO_MSEC(sdr->tPROG_max), false);
> @@ -829,15 +847,7 @@ static int meson_nfc_read_page_sub(struct nand_chip *nand,
>  	if (ret)
>  		return ret;
>  
> -	if (nand->options & NAND_NEED_SCRAMBLING) {
> -		meson_nfc_cmd_seed(nfc, page);
> -		meson_nfc_cmd_access(nand, raw, DIRREAD,
> -				     NFC_CMD_SCRAMBLER_ENABLE);
> -	} else {
> -		meson_nfc_cmd_access(nand, raw, DIRREAD,
> -				     NFC_CMD_SCRAMBLER_DISABLE);
> -	}
> -
> +	meson_nfc_cmd_access(nand, raw, DIRREAD, page);
>  	ret = meson_nfc_wait_dma_finish(nfc);
>  	meson_nfc_check_ecc_pages_valid(nfc, nand, raw);
>  
> @@ -1436,6 +1446,26 @@ meson_nfc_nand_chip_init(struct device *dev,
>  	if (ret)
>  		return ret;
>  
> +	if (nand->options & NAND_IS_BOOT_MEDIUM) {
> +		ret = of_property_read_u32(np, "amlogic,boot-pages",
> +					   &meson_chip->boot_pages);
> +		if (ret) {
> +			dev_err(dev, "could not retrieve 'amlogic,boot-pages' property: %d",
> +				ret);
> +			nand_cleanup(nand);
> +			return ret;
> +		}
> +
> +		ret = of_property_read_u32(np, "amlogic,boot-page-step",
> +					   &meson_chip->boot_page_step);
> +		if (ret) {
> +			dev_err(dev, "could not retrieve 'amlogic,boot-page-step' property: %d",
> +				ret);
> +			nand_cleanup(nand);
> +			return ret;
> +		}
> +	}
> +
>  	ret = mtd_device_register(mtd, NULL, 0);
>  	if (ret) {
>  		dev_err(dev, "failed to register MTD device: %d\n", ret);


Thanks,
Miquèl

Re: [PATCH v5 1/2] dt-bindings: mtd: amlogic,meson-nand: support fields for boot ROM code

[Arseniy Krasnov]

On 06.05.2024 16:48, Miquel Raynal wrote:
> Hi Arseniy,
> 
> avkrasnov@salutedevices.com wrote on Tue, 16 Apr 2024 11:51:00 +0300:
> 
>> Boot ROM code on Meson requires that some pages on NAND must be written
>> in special mode: "short" ECC mode where each block is 384 bytes and
>> scrambling mode is on.
> 
> Ok
> 
>> Such pages located with the specified interval within specified offset.
> 
> I'm sorry I don't get that sentence.

Sorry, I mean this (let me draw :) ) :

[ page 0 ][ page 1 ][ page 2 ][ page 3 ][ page 4 ][ page 5 ][ page 6 ][ page 7 ][ page 8 ][ page 9 ]

For example, we have 10 pages starting from the beginning of the chip - this is "within specified offset",
e.g. offset is 10. BootROM on axg needs that (for example) every third page must be written in "special"
mode: scrambling is on and ECC is 384 bytes. Such pages are 0, 2, 4, 6, 8. E.g. "specified interval" will
be 3.

So:

amlogic,boot-pages: 10
amlogic,boot-page-step: 3


> 
>> Both interval and offset are located in the
>> device tree and used by driver if 'nand-is-boot-medium' is set for
>> NAND chip.
> 
> This sentence is probably not needed.

Ok

> 
>>
>> Signed-off-by: Arseniy Krasnov <avkrasnov@salutedevices.com>
>> ---
>>  .../bindings/mtd/amlogic,meson-nand.yaml           | 14 ++++++++++++++
>>  1 file changed, 14 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml b/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml
>> index 57b6957c8415..67b2f7c1259c 100644
>> --- a/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml
>> +++ b/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml
>> @@ -64,11 +64,25 @@ patternProperties:
>>          items:
>>            maximum: 0
>>  
>> +      amlogic,boot-pages:
>> +        $ref: /schemas/types.yaml#/definitions/uint32
>> +        description:
>> +          Number of pages starting from 0, where special ECC
> 
> from *offset* 0 I guess?

Yes, or may be better "from beginning of the chip" ?

May be i can place here ascii picture from above, with reduced number of pages?
And also place this picture in the commit message ?

> 
>> +          algorithm will be used by the driver.
> 
> "where a special ECC configuration must be used because it is accessed
> by the ROM code"? Maybe you can even detail what are these values if
> they are fixed.
> 
> You should probably inform that scrambling shall be on as well.
>  
>> +
>> +      amlogic,boot-page-step:
>> +        $ref: /schemas/types.yaml#/definitions/uint32
>> +        description:
>> +          Interval between pages, read/written by the driver with
>> +          special ECC algorithm.
> 
> I'm not sure I fully get the description. What is the unit here? can
> you draw a small ascii-art diagram?

May be i can place here "please see diagram above" ? And in 'amlogic,boot-pages' there
will be diagram.


> 
>> +
>>      unevaluatedProperties: false
>>  
>>      dependencies:
>>        nand-ecc-strength: [nand-ecc-step-size]
>>        nand-ecc-step-size: [nand-ecc-strength]
>> +      amlogic,boot-pages: [nand-is-boot-medium, "amlogic,boot-page-step"]
>> +      amlogic,boot-page-step: [nand-is-boot-medium, "amlogic,boot-pages"]
>>  
>>  
>>  required:
> 
> 
> Thanks,
> Miquèl

Thanks, Arseniy

Re: [PATCH v5 1/2] dt-bindings: mtd: amlogic,meson-nand: support fields for boot ROM code

[Miquel Raynal]

Hi Arseniy,

avkrasnov@salutedevices.com wrote on Tue, 7 May 2024 09:53:06 +0300:

> On 06.05.2024 16:48, Miquel Raynal wrote:
> > Hi Arseniy,
> > 
> > avkrasnov@salutedevices.com wrote on Tue, 16 Apr 2024 11:51:00 +0300:
> >   
> >> Boot ROM code on Meson requires that some pages on NAND must be written
> >> in special mode: "short" ECC mode where each block is 384 bytes and
> >> scrambling mode is on.  
> > 
> > Ok
> >   
> >> Such pages located with the specified interval within specified offset.  
> > 
> > I'm sorry I don't get that sentence.  
> 
> Sorry, I mean this (let me draw :) ) :
> 
> [ page 0 ][ page 1 ][ page 2 ][ page 3 ][ page 4 ][ page 5 ][ page 6 ][ page 7 ][ page 8 ][ page 9 ]
> 
> For example, we have 10 pages starting from the beginning of the chip - this is "within specified offset",
> e.g. offset is 10. BootROM on axg needs that (for example) every third page must be written in "special"
> mode: scrambling is on and ECC is 384 bytes. Such pages are 0, 2, 4, 6, 8. E.g. "specified interval" will
> be 3.

Shall be 2, no?

> 
> So:
> 
> amlogic,boot-pages: 10
> amlogic,boot-page-step: 3

Ok I get it. Thanks for the explanation. I don't really understand the
logic behind it though. Do you know why the bootROM would access only
one page over 2 or 3? Is there a default value? Is this configurable?

Thanks,
Miquèl

Re: [PATCH v5 1/2] dt-bindings: mtd: amlogic,meson-nand: support fields for boot ROM code

[Arseniy Krasnov]

On 07.05.2024 10:27, Miquel Raynal wrote:
> Hi Arseniy,
> 
> avkrasnov@salutedevices.com wrote on Tue, 7 May 2024 09:53:06 +0300:
> 
>> On 06.05.2024 16:48, Miquel Raynal wrote:
>>> Hi Arseniy,
>>>
>>> avkrasnov@salutedevices.com wrote on Tue, 16 Apr 2024 11:51:00 +0300:
>>>   
>>>> Boot ROM code on Meson requires that some pages on NAND must be written
>>>> in special mode: "short" ECC mode where each block is 384 bytes and
>>>> scrambling mode is on.  
>>>
>>> Ok
>>>   
>>>> Such pages located with the specified interval within specified offset.  
>>>
>>> I'm sorry I don't get that sentence.  
>>
>> Sorry, I mean this (let me draw :) ) :
>>
>> [ page 0 ][ page 1 ][ page 2 ][ page 3 ][ page 4 ][ page 5 ][ page 6 ][ page 7 ][ page 8 ][ page 9 ]
>>
>> For example, we have 10 pages starting from the beginning of the chip - this is "within specified offset",
>> e.g. offset is 10. BootROM on axg needs that (for example) every third page must be written in "special"
>> mode: scrambling is on and ECC is 384 bytes. Such pages are 0, 2, 4, 6, 8. E.g. "specified interval" will
>> be 3.
> 
> Shall be 2, no?

yes, starting from 0 - then 2. e.g.
if (!(page_num % 2))
    boot ROM need this page

> 
>>
>> So:
>>
>> amlogic,boot-pages: 10
>> amlogic,boot-page-step: 3
> 
> Ok I get it. Thanks for the explanation. I don't really understand the
> logic behind it though. Do you know why the bootROM would access only
> one page over 2 or 3? Is there a default value? Is this configurable?

No, boot rom source is closed, I don't have access to it. I get this logic
from old version of vendor's uboot - in practice they use non 2 or 3, they
use hardcoded 128 step value. And amlogic,boot-pages is 1024

Thanks, Arseniy

> 
> Thanks,
> Miquèl

Re: [PATCH v5 1/2] dt-bindings: mtd: amlogic,meson-nand: support fields for boot ROM code

[Miquel Raynal]

Hi Arseniy,

avkrasnov@salutedevices.com wrote on Tue, 7 May 2024 10:35:51 +0300:

> On 07.05.2024 10:27, Miquel Raynal wrote:
> > Hi Arseniy,
> > 
> > avkrasnov@salutedevices.com wrote on Tue, 7 May 2024 09:53:06 +0300:
> >   
> >> On 06.05.2024 16:48, Miquel Raynal wrote:  
> >>> Hi Arseniy,
> >>>
> >>> avkrasnov@salutedevices.com wrote on Tue, 16 Apr 2024 11:51:00 +0300:
> >>>     
> >>>> Boot ROM code on Meson requires that some pages on NAND must be written
> >>>> in special mode: "short" ECC mode where each block is 384 bytes and
> >>>> scrambling mode is on.    
> >>>
> >>> Ok
> >>>     
> >>>> Such pages located with the specified interval within specified offset.    
> >>>
> >>> I'm sorry I don't get that sentence.    
> >>
> >> Sorry, I mean this (let me draw :) ) :
> >>
> >> [ page 0 ][ page 1 ][ page 2 ][ page 3 ][ page 4 ][ page 5 ][ page 6 ][ page 7 ][ page 8 ][ page 9 ]
> >>
> >> For example, we have 10 pages starting from the beginning of the chip - this is "within specified offset",
> >> e.g. offset is 10. BootROM on axg needs that (for example) every third page must be written in "special"
> >> mode: scrambling is on and ECC is 384 bytes. Such pages are 0, 2, 4, 6, 8. E.g. "specified interval" will
> >> be 3.  
> > 
> > Shall be 2, no?  
> 
> yes, starting from 0 - then 2. e.g.
> if (!(page_num % 2))
>     boot ROM need this page
> 
> >   
> >>
> >> So:
> >>
> >> amlogic,boot-pages: 10
> >> amlogic,boot-page-step: 3  
> > 
> > Ok I get it. Thanks for the explanation. I don't really understand the
> > logic behind it though. Do you know why the bootROM would access only
> > one page over 2 or 3? Is there a default value? Is this configurable?  
> 
> No, boot rom source is closed, I don't have access to it. I get this logic
> from old version of vendor's uboot - in practice they use non 2 or 3, they
> use hardcoded 128 step value. And amlogic,boot-pages is 1024

Feels like they are trying to use only the first page of each block, no?

That's very weird but I understand better.

Thanks,
Miquèl

Re: [PATCH v5 1/2] dt-bindings: mtd: amlogic,meson-nand: support fields for boot ROM code

[Arseniy Krasnov]

On 07.05.2024 11:05, Miquel Raynal wrote:
> Hi Arseniy,
> 
> avkrasnov@salutedevices.com wrote on Tue, 7 May 2024 10:35:51 +0300:
> 
>> On 07.05.2024 10:27, Miquel Raynal wrote:
>>> Hi Arseniy,
>>>
>>> avkrasnov@salutedevices.com wrote on Tue, 7 May 2024 09:53:06 +0300:
>>>   
>>>> On 06.05.2024 16:48, Miquel Raynal wrote:  
>>>>> Hi Arseniy,
>>>>>
>>>>> avkrasnov@salutedevices.com wrote on Tue, 16 Apr 2024 11:51:00 +0300:
>>>>>     
>>>>>> Boot ROM code on Meson requires that some pages on NAND must be written
>>>>>> in special mode: "short" ECC mode where each block is 384 bytes and
>>>>>> scrambling mode is on.    
>>>>>
>>>>> Ok
>>>>>     
>>>>>> Such pages located with the specified interval within specified offset.    
>>>>>
>>>>> I'm sorry I don't get that sentence.    
>>>>
>>>> Sorry, I mean this (let me draw :) ) :
>>>>
>>>> [ page 0 ][ page 1 ][ page 2 ][ page 3 ][ page 4 ][ page 5 ][ page 6 ][ page 7 ][ page 8 ][ page 9 ]
>>>>
>>>> For example, we have 10 pages starting from the beginning of the chip - this is "within specified offset",
>>>> e.g. offset is 10. BootROM on axg needs that (for example) every third page must be written in "special"
>>>> mode: scrambling is on and ECC is 384 bytes. Such pages are 0, 2, 4, 6, 8. E.g. "specified interval" will
>>>> be 3.  
>>>
>>> Shall be 2, no?  
>>
>> yes, starting from 0 - then 2. e.g.
>> if (!(page_num % 2))
>>     boot ROM need this page
>>
>>>   
>>>>
>>>> So:
>>>>
>>>> amlogic,boot-pages: 10
>>>> amlogic,boot-page-step: 3  
>>>
>>> Ok I get it. Thanks for the explanation. I don't really understand the
>>> logic behind it though. Do you know why the bootROM would access only
>>> one page over 2 or 3? Is there a default value? Is this configurable?  
>>
>> No, boot rom source is closed, I don't have access to it. I get this logic
>> from old version of vendor's uboot - in practice they use non 2 or 3, they
>> use hardcoded 128 step value. And amlogic,boot-pages is 1024
> 
> Feels like they are trying to use only the first page of each block, no?
> 
> That's very weird but I understand better.

A little bit no, they use every 128 page in range [0, 1024] pages. E.g. there will
be 8 such pages:
0
128
256
512
640
768
896
1024

They write some metadata about SoC to such pages.

Thanks, Arseniy

> 
> Thanks,
> Miquèl