Skip to contentSkip to conversation

i.MX6 nand bootupdate

PATCH New Sergey Kubushyn 4 participants 6 messages Jun 14, 2016 – Jun 16, 2016

Subsystems (4)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 i.MX6 nand bootupdate

Here is the initial support for writing i.MX6 NAND U-Boot into NAND with all FCB and DBBT stuff as required.

Just build U-Boot for NAND as u-boot.imx, load it somehow in RAM and say "nand bootupdate [addr] [size]". It will create all necessary structures and write everything as needed.

No SPL required, just plain single stage U-Boot. Supposed to write into first NAND partition, recommended size 4MiB. Creates 4 redundant copies of boot structures in first 4 sectors and 2 redundant copies of U-Boot in the remaining space.

Tested on a Variscite VAR-SOM-SOLO based board with i.MX6DL SoC. Will probably post that board support files later, when config is cleaned up so people could have a working NAND boot DCD portion to be able actually use those braindead SOMs.

---

arch/arm/include/asm/imx-common/mxs_nand.h new file +37 -0
new file mode 100644
index 0000000..7826a9f
@@ -0,0 +1,37 @@
+/*
+ * Copyright (C) 2015 PHYTEC Messtechnik GmbH,
+ * Author: Stefan Christ <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef __NAND_MXS_H
+#define __NAND_MXS_H
+
+/*
+ * Functions are definied in drivers/mtd/nand/nand_mxs.c.  They are used to
+ * calculate the ECC Strength, BadBlockMarkerByte and BadBlockMarkerStartBit
+ * which are placed into the FCB structure. The i.MX6 ROM needs these
+ * parameters to read the firmware from NAND.
+ *
+ * The parameters depends on the pagesize and oobsize of NAND chips and are
+ * different for each combination. To avoid placing hardcoded values in the bbu
+ * update handler code, the generic calculation from the driver code is used.
+ */
+
+uint32_t mxs_nand_get_ecc_strength(uint32_t page_data_size,
+						uint32_t page_oob_size);
+
+uint32_t mxs_nand_mark_byte_offset(struct mtd_info *mtd);
+
+uint32_t mxs_nand_mark_bit_offset(struct mtd_info *mtd);
+
+#endif /* __NAND_MXS_H */
cmd/nand.c +61 -0

85 lines not on this page · raw message

drivers/mtd/nand/Makefile +1 -0
index 837d397..36609e1 100644
@@ -66,6 +66,7 @@ obj-$(CONFIG_TEGRA_NAND) += tegra_nand.o
  obj-$(CONFIG_NAND_OMAP_GPMC) += omap_gpmc.o
  obj-$(CONFIG_NAND_OMAP_ELM) += omap_elm.o
  obj-$(CONFIG_NAND_PLAT) += nand_plat.o
+obj-$(CONFIG_CMD_NAND_BOOTUPDATE) += mxs_nand_bootupdate.o
 
  else  # minimal SPL drivers
 
drivers/mtd/nand/mxs_nand.c +4 -3
index 7be1f86..afeec22 100644
@@ -26,6 +26,7 @@
  #include <asm/imx-common/regs-gpmi.h>
  #include <asm/arch/sys_proto.h>
  #include <asm/imx-common/dma.h>
+#include <asm/imx-common/mxs_nand.h>
 
  #define	MXS_NAND_DMA_DESCRIPTOR_COUNT		4
 
@@ -145,7 +146,7 @@ static uint32_t mxs_nand_aux_status_offset(void)
  	return (MXS_NAND_METADATA_SIZE + 0x3) & ~0x3;
  }
 
-static inline uint32_t mxs_nand_get_ecc_strength(uint32_t page_data_size,
+uint32_t mxs_nand_get_ecc_strength(uint32_t page_data_size,
  						uint32_t page_oob_size)
  {
  	int ecc_strength;
@@ -221,14 +222,14 @@ static inline uint32_t mxs_nand_get_mark_offset(uint32_t page_data_size,
  	return block_mark_bit_offset;
  }
 
-static uint32_t mxs_nand_mark_byte_offset(struct mtd_info *mtd)
+uint32_t mxs_nand_mark_byte_offset(struct mtd_info *mtd)
  {
  	uint32_t ecc_strength;
  	ecc_strength = mxs_nand_get_ecc_strength(mtd->writesize, mtd->oobsize);
  	return mxs_nand_get_mark_offset(mtd->writesize, ecc_strength) >> 3;
  }
 
-static uint32_t mxs_nand_mark_bit_offset(struct mtd_info *mtd)
+uint32_t mxs_nand_mark_bit_offset(struct mtd_info *mtd)
  {
  	uint32_t ecc_strength;
  	ecc_strength = mxs_nand_get_ecc_strength(mtd->writesize, mtd->oobsize);
drivers/mtd/nand/mxs_nand_bootupdate.c new file +556 -0

556 lines not on this page · raw message

---
******************************************************************
* KSI at home KOI8 Net < > The impossible we do immediately. *
* Las Vegas NV, USA < > Miracles require 24-hour notice. *
******************************************************************

#1 lore raw

PATCH 1/1 i.MX6 nand bootupdate

Hi Sergey,

Quoted 12 lines · Sergey Kubushyn

Here is the initial support for writing i.MX6 NAND U-Boot into NAND with all FCB and DBBT stuff as required.

Just build U-Boot for NAND as u-boot.imx, load it somehow in RAM and say "nand bootupdate [addr] [size]". It will create all necessary structures and write everything as needed.

No SPL required, just plain single stage U-Boot. Supposed to write into first NAND partition, recommended size 4MiB. Creates 4 redundant copies of boot structures in first 4 sectors and 2 redundant copies of U-Boot in the remaining space.

Quoted 4 lines

Tested on a Variscite VAR-SOM-SOLO based board with i.MX6DL SoC. Will probably post that board support files later, when config is cleaned up so people could have a working NAND boot DCD portion to be able actually use those braindead SOMs.

I would like to test this, but seems this patch is not rebased to lastest U-Boot (:

Thanks,
Peng.

[...]

#2 lore raw

PATCH 1/1 i.MX6 nand bootupdate

In reply to Sergey Kubushyn

Hi Sergey,

Quoted 3 lines · Sergey Kubushyn

Here is the initial support for writing i.MX6 NAND U-Boot into NAND with all FCB and DBBT stuff as required.

This is a very interesting feature missing in U-Boot. Up now we are constrained to update the bootloader from user space with kobs-ng, and this is really an improvement. Thanks for that.

Anyway, as remarked by Peng, please rebase your patch on current tree, else we cannot test it.

Quoted 3 lines

Just build U-Boot for NAND as u-boot.imx, load it somehow in RAM and say "nand bootupdate [addr] [size]". It will create all necessary structures and write everything as needed.

The whole phrase is misleading here. I guess this is the way you have to put on your board, it is not a general case.

I am expecting to have some parameters like in kobs-ng: I set the number of copies with --search_exponent, how is it done here ?

Quoted 2 lines

No SPL required, just plain single stage U-Boot.

Why ? This has nothing to do. We could write SPL into the NAND as first bootloader, and SPL will load the u-boot.img unabh?ngig von FCB.

Quoted 2 lines

Supposed to write into
first NAND partition, recommended size 4MiB.

Where is this size coming ? This depends on the number of copies (can we have configurable as for kobs-ng ?) and the size of U-Boot. If we write SPL, we need less.

Supposed to write into mtd0 works in most cases, yes. But it should be better let it configurable as in kobs-ng.

One reason is to show the user an easy path from kobs-ng to u-boot - and of course, how to change from kobs-ng to this command should be explained into the README.

Quoted 8 lines

Creates 4 redundant copies
of boot structures in first 4 sectors and 2 redundant copies of U-Boot in the remaining space.

Tested on a Variscite VAR-SOM-SOLO based board with i.MX6DL SoC. Will probably post that board support files later, when config is cleaned up so people could have a working NAND boot DCD portion to be able actually use those braindead SOMs.

ok - anyway, it looks like that Peng and me can test on other boards - no worry about it.

Quoted 16 lines

---

arch/arm/include/asm/imx-common/mxs_nand.h new file +5 -0
new file mode 100644
index 0000000..7826a9f
@@ -0,0 +1,37 @@
+/*
+ * Copyright (C) 2015 PHYTEC Messtechnik GmbH,
+ * Author: Stefan Christ <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or

arch/arm/include/asm/imx-common/mxs_nand.h line 15

Quoted 10 lines
arch/arm/include/asm/imx-common/mxs_nand.h new file +10 -0
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+

Copyright is ok, but we have SPDX in U-Boot. Change the license header.

Quoted 75 lines
arch/arm/include/asm/imx-common/mxs_nand.h new file +5 -0
+#ifndef __NAND_MXS_H
+#define __NAND_MXS_H
+
+/*
+ * Functions are definied in drivers/mtd/nand/nand_mxs.c.  They are

used to
+ * calculate the ECC Strength, BadBlockMarkerByte and BadBlockMarkerStartBit
+ * which are placed into the FCB structure. The i.MX6 ROM needs these
+ * parameters to read the firmware from NAND.
+ *
+ * The parameters depends on the pagesize and oobsize of NAND chips and are
+ * different for each combination. To avoid placing hardcoded values in the bbu
+ * update handler code, the generic calculation from the driver code is used.
+ */

arch/arm/include/asm/imx-common/mxs_nand.h new file +9 -0
+
+uint32_t mxs_nand_get_ecc_strength(uint32_t page_data_size,
+                        uint32_t page_oob_size);
+
+uint32_t mxs_nand_mark_byte_offset(struct mtd_info *mtd);
+
+uint32_t mxs_nand_mark_bit_offset(struct mtd_info *mtd);
+
+#endif /* __NAND_MXS_H */
cmd/nand.c +5 -0
index 583a18f..12e38e1 100644
@@ -38,6 +38,11 @@ int find_dev_and_part(const char *id, struct
 mtd_device **dev,
               u8 *part_num, struct part_info **part);
 #endif
 
+#ifdef CONFIG_CMD_NAND_BOOTUPDATE
+/* This comes from a separate file in drivers/mtd/nand */
+int mxs_do_nand_bootupdate(ulong addr, size_t len);
+#endif
+
 static int nand_dump(struct mtd_info *mtd, ulong off, int only_oob,
              int repeat)

{

cmd/nand.c +3 -0
@@ -373,6 +378,9 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int
 argc, char * const argv[])
     loff_t off, size, maxsize;
     char *cmd, *s;
     struct mtd_info *mtd;
+#ifdef CONFIG_CMD_NAND_BOOTUPDATE
+    size_t cnt;
+#endif
 #ifdef CONFIG_SYS_NAND_QUIET
     int quiet = CONFIG_SYS_NAND_QUIET;

#else

cmd/nand.c +15 -0
@@ -745,6 +753,48 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int
 argc, char * const argv[])
     }
 #endif
 
+#ifdef CONFIG_CMD_NAND_BOOTUPDATE
+    if (strncmp(cmd, "bootupdate", 10) == 0) {
+
+        if (argc < 3) {
+            /* All default values */
+            addr = getenv_ulong("fileaddr", 16, 0UL);
+            cnt = getenv_ulong("filesize", 16, 0UL);
+        }
+
+        if (argc == 3) {
+            /* 'addr' only, file size from environment */
+            if ((addr = simple_strtoul(argv[2], NULL, 16)) == 0UL)
+                addr = getenv_ulong("fileaddr", 16, 0UL);
+
+            cnt = getenv_ulong("filesize", 16, 0UL);

cmd/nand.c static int do_nand(cmd_tbl_t *cmdtp, int flag, int · line 781

Quoted 10 lines
cmd/nand.c +10 -0
+        }
+
+        if (argc > 3) {
+            /* 'addr', 'size', and possibly more */
+            if ((addr = simple_strtoul(argv[2], NULL, 16)) == 0UL)
+                addr = getenv_ulong("fileaddr", 16, 0UL);
+
+            if ((cnt = simple_strtoul(argv[3], NULL, 16)) == 0UL)
+                cnt = getenv_ulong("filesize", 16, 0UL);
+        }

Why do we stick with the environment ? Most U-Boot commands do not do it. A user can always write a script as "bootupdate ${loadaddr} ${filesize}" if he wants, the command itself should be unaware of it.

cmd/nand.c static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])

Quoted 7 lines
cmd/nand.c +7 -0
+
+
+        if (addr == 0 || cnt == 0) {
+            puts("Invalid arguments to nand bootupdate!\n");
+            return 1;
+        }
+ +        if (mxs_do_nand_bootupdate(addr, cnt)) {

Name is misleading. The function is not related to the NAND driver (mxs_nand), but it is an implementation of the layout requested by NXP's SOCs. A name related to BCB (Boot Control Block) as in manual is more correct.

So please changes all names from mxs_ in the code if they are not strictly related to the driver, and let mxs_ name just for functions exported by the driver.

Quoted 29 lines
cmd/nand.c +12 -0
+            puts("NAND bootupdate failed!\n");
+            return 1;
+        }
+
+        puts("NAND bootupdate successful\n");
+        return 0;
+    }
+#endif
+
 usage:
     return CMD_RET_USAGE;
 }
@@ -766,6 +816,17 @@ static char nand_help_text[] =
     "    'addr', skipping bad blocks and dropping any pages at the end\n"
     "    of eraseblocks that contain only 0xFF\n"
 #endif
+#ifdef CONFIG_CMD_NAND_BOOTUPDATE
+    "nand bootupdate - [addr] [size]\n"
+    "    write U-Boot into NAND in board/SoC specific manner creating

all\n"

cmd/nand.c +4 -0
+    "    required headers and other bits and pieces as required for the\n"
+    "    system to be able to boot off of NAND. 'addr' is the address\n"
+    "    where U-Boot image has been loaded at, 'size' is its size.\n"
+    "    If any of 'addr'/'size' is missing it is taken from

environment\n"
+ " for the last file loaded. U-Boot image must be of a proper
type\n"
+ " for the target platform (only IMX image supported at the
moment)\n"

cmd/nand.c static char nand_help_text[] = · line 832

Quoted 5 lines
cmd/nand.c +2 -0
+    "    binary without U-Boot image headers (e.g. u-boot.imx file.)\n"
+#endif
     "nand erase[.spread] [clean] off size - erase 'size' bytes "
     "from offset 'off'\n"
     "    With '.spread', erase enough for given file size, otherwise,\n"

A README file in doc/ is also useful.

drivers/mtd/nand/Makefile obj-$(CONFIG_TEGRA_NAND) += tegra_nand.o · line 69

Quoted 9 lines
drivers/mtd/nand/Makefile +1 -0
index 837d397..36609e1 100644
@@ -66,6 +66,7 @@ obj-$(CONFIG_TEGRA_NAND) += tegra_nand.o
 obj-$(CONFIG_NAND_OMAP_GPMC) += omap_gpmc.o
 obj-$(CONFIG_NAND_OMAP_ELM) += omap_elm.o
 obj-$(CONFIG_NAND_PLAT) += nand_plat.o
+obj-$(CONFIG_CMD_NAND_BOOTUPDATE) += mxs_nand_bootupdate.o

It will be nice to set some dependencies between CONFIG_NAND_MXS and CONFIG_CMD_NAND_BOOTUPDATE. CONFIG_CMD_NAND_BOOTUPDATE is not useful if CONFIG_NAND_MXS is not set, right ?

Quoted 43 lines
drivers/mtd/nand/Makefile +0 -0
 else  # minimal SPL drivers
 
drivers/mtd/nand/mxs_nand.c +4 -3
index 7be1f86..afeec22 100644
@@ -26,6 +26,7 @@
 #include <asm/imx-common/regs-gpmi.h>
 #include <asm/arch/sys_proto.h>
 #include <asm/imx-common/dma.h>
+#include <asm/imx-common/mxs_nand.h>
 
 #define    MXS_NAND_DMA_DESCRIPTOR_COUNT        4
 
@@ -145,7 +146,7 @@ static uint32_t mxs_nand_aux_status_offset(void)
     return (MXS_NAND_METADATA_SIZE + 0x3) & ~0x3;
 }
 
-static inline uint32_t mxs_nand_get_ecc_strength(uint32_t page_data_size,
+uint32_t mxs_nand_get_ecc_strength(uint32_t page_data_size,
                         uint32_t page_oob_size)
 {
     int ecc_strength;
@@ -221,14 +222,14 @@ static inline uint32_t
 mxs_nand_get_mark_offset(uint32_t page_data_size,
     return block_mark_bit_offset;
 }
 
-static uint32_t mxs_nand_mark_byte_offset(struct mtd_info *mtd)
+uint32_t mxs_nand_mark_byte_offset(struct mtd_info *mtd)
 {
     uint32_t ecc_strength;
     ecc_strength = mxs_nand_get_ecc_strength(mtd->writesize,
 mtd->oobsize);
     return mxs_nand_get_mark_offset(mtd->writesize, ecc_strength) >> 3;
 }
 
-static uint32_t mxs_nand_mark_bit_offset(struct mtd_info *mtd)
+uint32_t mxs_nand_mark_bit_offset(struct mtd_info *mtd)
 {
     uint32_t ecc_strength;
     ecc_strength = mxs_nand_get_ecc_strength(mtd->writesize,
Quoted 1 line

mtd->oobsize);

Added Scott as MTD maintainer: it is clear to me why, but the driver exports its functions via the struct nand_chip (see board_nand_init).

Is it not better to extend the structure with the functions you want to export ?

drivers/mtd/nand/mxs_nand_bootupdate.c

Quoted 2 lines
drivers/mtd/nand/mxs_nand_bootupdate.c +0 -0

I do not know if this file should be here or should be in imx-common. It is relevant just for i.MX and not for other SOCs.

In barebox it has a better name : imx-bbu-nand-fcb.c. It says much more as mxs_nand_bootupdate.c.

Quoted 24 lines

new file mode 100644
index 0000000..9442553

drivers/mtd/nand/mxs_nand_bootupdate.c new file +17 -0
@@ -0,0 +1,556 @@
+/*
+ * mxs_nand_bootupdate.c - write U-Boot to MXS NAND to make it bootable
+ *
+ * Copyright (c) 2016 Sergey Kubushyn <[email protected]>
+ *
+ * Most of the source shamelesly stolen from barebox.
+ *
+ * Here is the original copyright:
+ *
+ *=== Cut ===
+ * Copyright (C) 2014 Sascha Hauer, Pengutronix
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *

drivers/mtd/nand/mxs_nand_bootupdate.c line 27

Quoted 10 lines
drivers/mtd/nand/mxs_nand_bootupdate.c new file +10 -0
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation.
+ *=== Cut ===
+ */

Let Copyright and add SPDX header.

As you ported code from another Open Spurce project, it will be nice if you write this into the commit message, writing also from which barebox commit you took the code, exactly how we do when we port code from Linux.

Quoted 10 lines
drivers/mtd/nand/mxs_nand_bootupdate.c new file +10 -0
+
+
+#include <common.h>
+#include <linux/sizes.h>
+#include <linux/mtd/mtd.h>
+#include <linux/compat.h>
+#include <command.h>
+#include <console.h>
+#include <malloc.h>
+#include <asm/byteorder.h>

drivers/mtd/nand/mxs_nand_bootupdate.c line 47

Quoted 10 lines
drivers/mtd/nand/mxs_nand_bootupdate.c new file +10 -0
+#include <jffs2/jffs2.h>
+#include <nand.h>
+#include <errno.h>
+#include <asm/imx-common/mxs_nand.h>
+#include <asm/imx-common/imximage.cfg>
+
+
+#ifndef CONFIG_CMD_MTDPARTS
+#error "CONFIG_CMD_MTDPARTS is not set, mtd partition support missing"
+#endif

I am missing because this is important. U-Boot works with addresses, and not with MTDs. The whole nand utilities take an address as parameter. MTD is optional, and can be used or not in nand erase/read/write.

Quoted 141 lines
drivers/mtd/nand/mxs_nand_bootupdate.c new file +6 -0
+
+static const char *uboot_tgt = "nand0,0";
+
+/* partition handling routines */
+int mtdparts_init(void);
+int id_parse(const char *id, const char **ret_id, u8 *dev_type, u8

*dev_num);

drivers/mtd/nand/mxs_nand_bootupdate.c new file +25 -0
+int find_dev_and_part(const char *id, struct mtd_device **dev,
+              u8 *part_num, struct part_info **part);
+
+struct dbbt_block {
+    uint32_t Checksum;    /* reserved on i.MX6 */
+    uint32_t FingerPrint;
+    uint32_t Version;
+    uint32_t numberBB;    /* reserved on i.MX6 */
+    uint32_t DBBTNumOfPages;
+};
+
+struct fcb_block {
+    uint32_t Checksum;        /* First fingerprint in first byte */
+    uint32_t FingerPrint;        /* 2nd fingerprint at byte 4 */
+    uint32_t Version;        /* 3rd fingerprint at byte 8 */
+    uint8_t DataSetup;
+    uint8_t DataHold;
+    uint8_t AddressSetup;
+    uint8_t DSAMPLE_TIME;
+    /* These are for application use only and not for ROM. */
+    uint8_t NandTimingState;
+    uint8_t REA;
+    uint8_t RLOH;
+    uint8_t RHOH;
+    uint32_t PageDataSize;        /* 2048 for 2K pages, 4096 for 4K

pages */
+ uint32_t TotalPageSize; /* 2112 for 2K pages, 4314 for 4K
pages */

drivers/mtd/nand/mxs_nand_bootupdate.c new file +2 -0
+    uint32_t SectorsPerBlock;    /* Number of 2K sections per block */
+    uint32_t NumberOfNANDs;        /* Total Number of NANDs - not used

by ROM */
+ uint32_t TotalInternalDie; /* Number of separate chips in this
NAND */

drivers/mtd/nand/mxs_nand_bootupdate.c new file +2 -0
+    uint32_t CellType;        /* MLC or SLC */
+    uint32_t EccBlockNEccType;    /* Type of ECC, can be one of

BCH-0-20 */

drivers/mtd/nand/mxs_nand_bootupdate.c new file +2 -0
+    uint32_t EccBlock0Size;        /* Number of bytes for Block0 - BCH */
+    uint32_t EccBlockNSize;        /* Block size in bytes for all

blocks other than Block0 - BCH */

drivers/mtd/nand/mxs_nand_bootupdate.c new file +3 -0
+    uint32_t EccBlock0EccType;    /* Ecc level for Block 0 - BCH */
+    uint32_t MetadataBytes;        /* Metadata size - BCH */
+    uint32_t NumEccBlocksPerPage;    /* Number of blocks per page for

ROM use - BCH */
+ uint32_t EccBlockNEccLevelSDK; /* Type of ECC, can be one of
BCH-0-20 */

drivers/mtd/nand/mxs_nand_bootupdate.c new file +2 -0
+    uint32_t EccBlock0SizeSDK;    /* Number of bytes for Block0 - BCH */
+    uint32_t EccBlockNSizeSDK;    /* Block size in bytes for all blocks

other than Block0 - BCH */

drivers/mtd/nand/mxs_nand_bootupdate.c new file +2 -0
+    uint32_t EccBlock0EccLevelSDK;    /* Ecc level for Block 0 - BCH */
+    uint32_t NumEccBlocksPerPageSDK;/* Number of blocks per page for

SDK use - BCH */

drivers/mtd/nand/mxs_nand_bootupdate.c new file +3 -0
+    uint32_t MetadataBytesSDK;    /* Metadata size - BCH */
+    uint32_t EraseThreshold;    /* To set into BCH_MODE register */
+    uint32_t BootPatch;        /* 0 for normal boot and 1 to load patch

starting next to FCB */

drivers/mtd/nand/mxs_nand_bootupdate.c new file +2 -0
+    uint32_t PatchSectors;        /* Size of patch in sectors */
+    uint32_t Firmware1_startingPage;/* Firmware image starts on this

sector */
+ uint32_t Firmware2_startingPage;/* Secondary FW Image starting
Sector */
+ uint32_t PagesInFirmware1; /* Number of sectors in firmware
image */
+ uint32_t PagesInFirmware2; /* Number of sector in secondary FW
image */
+ uint32_t DBBTSearchAreaStartAddress; /* Page address where dbbt
search area begins */
+ uint32_t BadBlockMarkerByte; /* Byte in page data that have
manufacturer marked bad block marker, */
+ /* this will be swapped with metadata[0] to
complete page data. */
+ uint32_t BadBlockMarkerStartBit;/* For BCH ECC sizes other than 8
and 16 the bad block marker does not */
+ /* start at 0th bit of BadBlockMarkerByte. This
field is used to get to */
+ /* the start bit of bad block marker byte with in
BadBlockMarkerByte */
+ uint32_t BBMarkerPhysicalOffset;/* FCB value that gives byte offset
for bad block marker on physical NAND page */

drivers/mtd/nand/mxs_nand_bootupdate.c new file +13 -0
+    uint32_t BCHType;
+
+    uint32_t TMTiming2_ReadLatency;
+    uint32_t TMTiming2_PreambleDelay;
+    uint32_t TMTiming2_CEDelay;
+    uint32_t TMTiming2_PostambleDelay;
+    uint32_t TMTiming2_CmdAddPause;
+    uint32_t TMTiming2_DataPause;
+    uint32_t TMSpeed;
+    uint32_t TMTiming1_BusyTimeout;
+
+    uint32_t DISBBM;    /* the flag to enable (1)/disable(0) bi swap */
+    uint32_t BBMarkerPhysicalOffsetInSpareData; /* The swap position of

main area in spare area */

drivers/mtd/nand/mxs_nand_bootupdate.c new file +19 -0
+};
+
+struct fw_write_data {
+    int    fw1_blk;
+    int    fw2_blk;
+    int    part_blks;
+    void    *buf;
+    size_t    len;
+};
+
+#define BF_VAL(v, bf)    (((v) & bf##_MASK) >> bf##_OFFSET)
+#define GETBIT(v,n)    (((v) >> (n)) & 0x1)
+
+
+static uint8_t calculate_parity_13_8(uint8_t d)
+{
+    uint8_t    p = 0;
+
+    p |= (GETBIT(d, 6) ^ GETBIT(d, 5) ^ GETBIT(d, 3) ^ GETBIT(d,

2)) << 0;
+ p |= (GETBIT(d, 7) ^ GETBIT(d, 5) ^ GETBIT(d, 4) ^ GETBIT(d, 2) ^
GETBIT(d, 1)) << 1;
+ p |= (GETBIT(d, 7) ^ GETBIT(d, 6) ^ GETBIT(d, 5) ^ GETBIT(d, 1) ^
GETBIT(d, 0)) << 2;
+ p |= (GETBIT(d, 7) ^ GETBIT(d, 4) ^ GETBIT(d, 3) ^ GETBIT(d,
0)) << 3;
+ p |= (GETBIT(d, 6) ^ GETBIT(d, 4) ^ GETBIT(d, 3) ^ GETBIT(d, 2) ^
GETBIT(d, 1) ^ GETBIT(d, 0)) << 4;

drivers/mtd/nand/mxs_nand_bootupdate.c new file +14 -0
+    return p;
+}
+
+static void encode_hamming_13_8(void *_src, void *_ecc, size_t size)
+{
+    int    i;
+    uint8_t    *src = _src;
+    uint8_t    *ecc = _ecc;
+
+    for (i = 0; i < size; i++)
+        ecc[i] = calculate_parity_13_8(src[i]);
+}
+
+static uint32_t calc_chksum(void *buf, size_t size)

drivers/mtd/nand/mxs_nand_bootupdate.c line 169

Quoted 10 lines
drivers/mtd/nand/mxs_nand_bootupdate.c new file +10 -0
+{
+    u32    chksum = 0;
+    u8    *bp = buf;
+    size_t    i;
+
+    for (i = 0; i < size; i++)
+        chksum += bp[i];
+
+    return ~chksum;
+}

I understand that it is for debug only - but you have removed debug functions from barebox (dump_fcb for example). Can they be useful here, too ?

Quoted 185 lines
drivers/mtd/nand/mxs_nand_bootupdate.c new file +3 -0
+
+
+static ssize_t raw_write_page(struct mtd_info *mtd, void *buf, loff_t

offset)

drivers/mtd/nand/mxs_nand_bootupdate.c new file +24 -0
+{
+    struct mtd_oob_ops ops;
+    ssize_t ret;
+
+    ops.mode = MTD_OPS_RAW;
+    ops.ooboffs = 0;
+    ops.datbuf = buf;
+    ops.len = mtd->writesize;
+    ops.oobbuf = buf + mtd->writesize;
+    ops.ooblen = mtd->oobsize;
+    ret = mtd_write_oob(mtd, offset, &ops);
+
+        return ret;
+}
+
+static int fcb_create(struct fcb_block *fcb, struct mtd_info *mtd)
+{
+    fcb->FingerPrint = 0x20424346;
+    fcb->Version = 0x01000000;
+    fcb->PageDataSize = mtd->writesize;
+    fcb->TotalPageSize = mtd->writesize + mtd->oobsize;
+    fcb->SectorsPerBlock = mtd->erasesize / mtd->writesize;
+
+    /* Divide ECC strength by two and save the value into FCB

structure. */

drivers/mtd/nand/mxs_nand_bootupdate.c new file +31 -0
+    fcb->EccBlock0EccType =
+        mxs_nand_get_ecc_strength(mtd->writesize, mtd->oobsize) >> 1;
+
+    fcb->EccBlockNEccType = fcb->EccBlock0EccType;
+
+    /* Also hardcoded in kobs-ng */
+    fcb->EccBlock0Size = 0x00000200;
+    fcb->EccBlockNSize = 0x00000200;
+    fcb->DataSetup = 80;
+    fcb->DataHold = 60;
+    fcb->AddressSetup = 25;
+    fcb->DSAMPLE_TIME = 6;
+    fcb->MetadataBytes = 10;
+
+    /* DBBT search area starts at second page on first block */
+    fcb->DBBTSearchAreaStartAddress = 1;
+
+    fcb->BadBlockMarkerByte = mxs_nand_mark_byte_offset(mtd);
+    fcb->BadBlockMarkerStartBit = mxs_nand_mark_bit_offset(mtd);
+
+    fcb->BBMarkerPhysicalOffset = mtd->writesize;
+
+    fcb->NumEccBlocksPerPage = mtd->writesize / fcb->EccBlock0Size - 1;
+
+    fcb->Checksum = calc_chksum((void *)fcb + 4, sizeof(*fcb) - 4);
+
+    return 0;
+}
+
+/* Erase entire U-Boot partition */
+static int mxs_nand_uboot_erase(struct mtd_info *mtd, struct part_info

*part)

drivers/mtd/nand/mxs_nand_bootupdate.c new file +32 -0
+{
+    uint64_t        offset = 0;
+    struct erase_info    erase;
+    int            len = part->size;
+    int            ret;
+
+    while (len > 0) {
+        pr_debug("erasing at 0x%08llx\n", offset);
+        if (mtd_block_isbad(mtd, offset)) {
+            pr_debug("erase skip block @ 0x%08llx\n", offset);
+            offset += mtd->erasesize;
+            continue;
+        }
+
+        memset(&erase, 0, sizeof(erase));
+        erase.addr = offset;
+        erase.len = mtd->erasesize;
+
+        ret = mtd_erase(mtd, &erase);
+        if (ret)
+            return ret;
+
+        offset += mtd->erasesize;
+        len -= mtd->erasesize;
+    }
+
+    return 0;
+}
+
+/* Write the U-Boot proper (2 copies) to where it belongs.   */
+/* This is U-Boot binary image itself, no FCB/DBBT here yet. */
+static int mxs_nand_uboot_write_fw(struct mtd_info *mtd, struct

fw_write_data *fw)

drivers/mtd/nand/mxs_nand_bootupdate.c new file +75 -0
+{
+    uint64_t    offset;
+    int        ret;
+    int        blk;
+    size_t        dummy;
+    size_t        bytes_left;
+    int        chunk;
+    void        *p;
+
+    bytes_left = fw->len;
+    p = fw->buf;
+    blk = fw->fw1_blk;
+    offset = blk * mtd->erasesize;
+
+    while (bytes_left > 0) {
+        chunk = min(bytes_left, mtd->erasesize);
+
+        pr_debug("fw1: writing %p at 0x%08llx, left 0x%08x\n",
+                p, offset, bytes_left);
+
+        if (mtd_block_isbad(mtd, offset)) {
+            pr_debug("fw1: write skip block @ 0x%08llx\n", offset);
+            offset += mtd->erasesize;
+            blk++;
+            continue;
+        }
+
+        if (blk >= fw->fw2_blk)
+            return -ENOSPC;
+
+        ret = mtd_write(mtd, offset, chunk, &dummy, p);
+        if (ret)
+            return ret;
+
+        offset += chunk;
+        bytes_left -= chunk;
+        p += chunk;
+        blk++;
+    }
+
+    bytes_left = fw->len;
+    p = fw->buf;
+    blk = fw->fw2_blk;
+    offset = blk * mtd->erasesize;
+
+    while (bytes_left > 0) {
+        chunk = min(bytes_left, mtd->erasesize);
+
+        pr_debug("fw2: writing %p at 0x%08llx, left 0x%08x\n",
+                p, offset, bytes_left);
+
+        if (mtd_block_isbad(mtd, offset)) {
+            pr_debug("fw2: write skip block @ 0x%08llx\n", offset);
+            offset += mtd->erasesize;
+            blk++;
+            continue;
+        }
+
+        if (blk >= fw->part_blks)
+            return -ENOSPC;
+
+        ret = mtd_write(mtd, offset, chunk, &dummy, p);
+        if (ret)
+            return ret;
+
+        offset += chunk;
+        bytes_left -= chunk;
+        p += chunk;
+        blk++;
+    }
+
+    return 0;
+}
+
+static int dbbt_data_create(struct mtd_info *mtd, void *buf, int

num_blocks)

drivers/mtd/nand/mxs_nand_bootupdate.c new file +15 -0
+{
+    int        n;
+    int        n_bad_blocks = 0;
+    uint32_t    *bb = buf + 0x8;
+    uint32_t    *n_bad_blocksp = buf + 0x4;
+
+    for (n = 0; n < num_blocks; n++) {
+        loff_t offset = n * mtd->erasesize;
+        if (mtd_block_isbad(mtd, offset)) {
+            n_bad_blocks++;
+            *bb = n;
+            bb++;
+        }
+    }
+

drivers/mtd/nand/mxs_nand_bootupdate.c line 359

Quoted 10 lines
drivers/mtd/nand/mxs_nand_bootupdate.c new file +10 -0
+    *n_bad_blocksp = n_bad_blocks;
+
+    return n_bad_blocks;
+}
+
+/********************************************************************/
+/* This is where it is all done. Takes pointer to a U-Boot image in */
+/* RAM and image size, creates FCB/DBBT and writes everything where */
+/* it belongs into NAND. Image must be an IMX image built for NAND. */
+/********************************************************************/

Multiline comment, please fix it.

Quoted 1 line

+static int mxs_nand_uboot_update(const void *img, size_t len)

I would prefer you let the function names as in original code, so we know where they are coming from (imx_bbu_nand_update here).

Quoted 37 lines
drivers/mtd/nand/mxs_nand_bootupdate.c new file +36 -0
+{
+    int             i, ret;
+
+    size_t            dummy;
+    loff_t            offset = 0;
+
+    void             *fcb_raw_page;
+    void            *dbbt_page;
+    void            *dbbt_data_page;
+    void            *ecc;
+
+    uint32_t         num_blocks_fcb_dbbt;
+    uint32_t        num_blocks_fw;
+
+    struct mtd_info        *mtd;
+    struct fcb_block    *fcb;
+    struct dbbt_block    *dbbt;
+
+    struct mtd_device    *dev;
+    struct part_info    *part;
+    u8            pnum;
+
+    struct fw_write_data    fw;
+
+    if ((mtdparts_init() == 0) &&
+            (find_dev_and_part(uboot_tgt, &dev, &pnum, &part) == 0)) {
+        if (dev->id->type != MTD_DEV_TYPE_NAND) {
+            puts("Not a NAND device\n");
+            return -ENODEV;
+        }
+    }
+
+    nand_curr_device = dev->id->num;
+
+#ifdef CONFIG_SYS_NAND_SELECT_DEVICE
+    board_nand_select_device(nand_info[nand_curr_device].priv,

nand_curr_device);

drivers/mtd/nand/mxs_nand_bootupdate.c line 401

Quoted 5 lines
drivers/mtd/nand/mxs_nand_bootupdate.c new file +5 -0
+#endif
+
+    /* Get a pointer to mtd_info for selected device */
+
+    mtd = get_mtd_device_nm("nand0");    /* We always boot off of nand0 */

...and if I have nand1 ?

As I said, U-Boot works with addresses

Quoted 11 lines

+ + if (IS_ERR(mtd)) {

drivers/mtd/nand/mxs_nand_bootupdate.c new file +7 -0
+        /* Should not happen */
+        puts("No nand0 device...\n");
+        return -ENODEV;
+    }
+ +    put_mtd_device(mtd);
+
+    /* Quick and dirty check if we have 2Mbytes of good blocks in

nand0,0 */
+ /* Not sure if it is needed at all but won't hurt so here it
is... */

Yes, it hurts.

drivers/mtd/nand/mxs_nand_bootupdate.c line 427

Quoted 13 lines

+ + i = 0;

drivers/mtd/nand/mxs_nand_bootupdate.c new file +12 -0
+    offset = 0;    /* It is the first partition so it starts at block 0 */
+
+    while (offset < part->size) {
+        if (!mtd_block_isbad(mtd, offset)) {
+            i += mtd->erasesize;
+        }
+        offset += mtd->erasesize;
+    }
+ +    if (i < SZ_2M) {
+        puts("Partition too small for U-Boot!\n");
+        return -EINVAL;
+    }

I try to understand your changes from barebox code - something are clear due to the differences between the two bootloaders, some other ones not.

drivers/mtd/nand/mxs_nand_bootupdate.c line 434

Quoted 7 lines
drivers/mtd/nand/mxs_nand_bootupdate.c new file +7 -0
+
+    /* We will use 4 first blocks for FCB/DBBT copies. */
+    /* The rest of partition is split in half and used */
+    /* for two U-Boot copies. We don't care if those   */
+    /* start on good or bad block - bad blocks will be */
+    /* skipped on write, ROM boot code will also skip  */
+    /* bad blocks on bootup when loading U-Boot image. */

Multiline comment is wrong. I think the original comment in bareboc is much more clear.

drivers/mtd/nand/mxs_nand_bootupdate.c line 447

Quoted 13 lines
drivers/mtd/nand/mxs_nand_bootupdate.c new file +13 -0
+
+    fw.part_blks = part->size / mtd->erasesize;
+    num_blocks_fcb_dbbt = 4;
+    num_blocks_fw = (fw.part_blks - num_blocks_fcb_dbbt) / 2;
+    fw.fw1_blk = num_blocks_fcb_dbbt;
+    fw.fw2_blk = fw.fw1_blk + num_blocks_fw;
+
+    /* OK, now create FCB structure for bootROM NAND boot */
+
+    fcb_raw_page = kzalloc(mtd->writesize + mtd->oobsize, GFP_KERNEL);
+
+    fcb = fcb_raw_page + 12;
+    ecc = fcb_raw_page + 512 + 12;

??

Quoted 114 lines
drivers/mtd/nand/mxs_nand_bootupdate.c new file +9 -0
+
+    dbbt_page = kzalloc(mtd->writesize, GFP_KERNEL);
+    dbbt_data_page = kzalloc(mtd->writesize, GFP_KERNEL);
+    dbbt = dbbt_page;
+
+    /* Write one additional page to make the ROM happy. */
+    /* Maybe the PagesInFirmwarex fields are really the */
+    /* number of pages - 1. kobs-ng does the same.      */
+ +    fw.len = ALIGN(len + FLASH_OFFSET_STANDARD + mtd->writesize,

mtd->writesize);

drivers/mtd/nand/mxs_nand_bootupdate.c new file +17 -0
+    fw.buf = kzalloc(fw.len, GFP_KERNEL);
+    memcpy(fw.buf + FLASH_OFFSET_STANDARD, img, len);
+
+    /* Erase entire partition */
+    ret = mxs_nand_uboot_erase(mtd, part);
+    if (ret)
+        goto out;
+
+    /* Now write 2 copies of the U-Boot proper to where they belong. */
+    /* Headers (FCB, DBBT) will be generated and written after that. */
+    ret = mxs_nand_uboot_write_fw(mtd, &fw);
+    if (ret < 0)
+        goto out;
+
+    /* Create FCB, calculate ECC (we don't/can't use hardware ECC */
+    /* here so we do it ourselves and then write _RAW_ pages.     */
+ +    fcb->Firmware1_startingPage = fw.fw1_blk * mtd->erasesize /

mtd->writesize;
+ fcb->Firmware2_startingPage = fw.fw2_blk * mtd->erasesize /
mtd->writesize;

drivers/mtd/nand/mxs_nand_bootupdate.c new file +2 -0
+    fcb->PagesInFirmware1 =
+        ALIGN(len + FLASH_OFFSET_STANDARD, mtd->writesize) /

mtd->writesize;

drivers/mtd/nand/mxs_nand_bootupdate.c new file +11 -0
+    fcb->PagesInFirmware2 = fcb->PagesInFirmware1;
+
+    fcb_create(fcb, mtd);
+    encode_hamming_13_8(fcb, ecc, 512);
+
+    /*
+     * Set the first and second byte of OOB data to 0xFF, not 0x00. These
+     * bytes are used as the Manufacturers Bad Block Marker (MBBM). Since
+     * the FCB is mostly written to the first page in a block, a scan for
+     * factory bad blocks will detect these blocks as bad, e.g. when
+     * function nand_scan_bbt() is executed to build a new bad block

table.

drivers/mtd/nand/mxs_nand_bootupdate.c new file +67 -1
+     * We will _NOT_ mark a bad block as good -- we skip the bad blocks.
+     */
+    memset(fcb_raw_page + mtd->writesize, 0xff, 2);
+
+    /* Now create DBBT */
+    dbbt->Checksum = 0;
+    dbbt->FingerPrint = 0x54424244;
+    dbbt->Version = 0x01000000;
+
+    if ((ret = dbbt_data_create(mtd, dbbt_data_page, fw.part_blks)) < 0)
+        goto out;
+
+    if (ret > 0)
+        dbbt->DBBTNumOfPages = 1;
+
+    offset = 0;
+
+    if (mtd_block_isbad(mtd, offset)) {
+        puts("Block 0 is bad, NAND unusable\n");
+        ret = -EIO;
+        goto out;
+    }
+
+    /* Write FCB/DBBT to first 4 blocks. Skip bad blocks if any. */
+    /* Less than 4 copies will be written if there were BBs !!!  */
+    for (i = 0; i < 4; i++) {
+
+        if (mtd_block_isbad(mtd, offset)) {
+            pr_err("Block %d is bad, skipped\n", i);
+            continue;
+        }
+
+
+        ret = raw_write_page(mtd, fcb_raw_page, mtd->erasesize * i);
+        if (ret)
+            goto out;
+
+        ret = mtd_write(mtd, mtd->erasesize * i + mtd->writesize,
+                mtd->writesize, &dummy, dbbt_page);
+        if (ret)
+            goto out;
+
+        /* DBBTNumOfPages == 0 if no bad blocks */
+        if (dbbt->DBBTNumOfPages > 0) {
+            ret = mtd_write(mtd, mtd->erasesize * i + mtd->writesize * 5,
+                    mtd->writesize, &dummy, dbbt_data_page);
+            if (ret)
+                goto out;
+        }
+    }
+
+out:
+    kfree(dbbt_page);
+    kfree(dbbt_data_page);
+    kfree(fcb_raw_page);
+    kfree(fw.buf);
+
+    return ret;
+}
+
+
+int mxs_do_nand_bootupdate(ulong addr, size_t len)
+{
+    /* KSI: Unlock NAND first if it is locked... */
+
+    return mxs_nand_uboot_update((const void *)addr, len);
+}
 
---
Quoted 8 lines

******************************************************************
* KSI at home KOI8 Net < > The impossible we do immediately. *
* Las Vegas NV, USA < > Miracles require 24-hour notice. *
******************************************************************
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Best regards,
Stefano Babic

Signature · 5 lines
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================
#3 lore raw

PATCH 1/1 i.MX6 nand bootupdate

On Wed, 15 Jun 2016, Stefano Babic wrote:

Hi,

Quoted 8 lines

Hi Sergey,

Quoted 3 lines · Sergey Kubushyn

Here is the initial support for writing i.MX6 NAND U-Boot into NAND with all FCB and DBBT stuff as required.

Quoted 6 lines

This is a very interesting feature missing in U-Boot. Up now we are constrained to update the bootloader from user space with kobs-ng, and this is really an improvement. Thanks for that.

Anyway, as remarked by Peng, please rebase your patch on current tree, else we cannot test it.

I will when time permits, probably tomorrow. Now let's go over other stuff...

Quoted 6 lines
Quoted 3 lines

Just build U-Boot for NAND as u-boot.imx, load it somehow in RAM and say "nand bootupdate [addr] [size]". It will create all necessary structures and write everything as needed.

The whole phrase is misleading here. I guess this is the way you have to put on your board, it is not a general case.

The very (and only) purpose for this function is to put U-Boot with proper FCB/DBBT header on one's board so i.MX6 bootrom would be able to boot off of it. There is _NO_ general case. One can _NOT_ change the way i.MX6 (or whatever) boots up, it is in the ROM and it always reads FCB starting from sector 0 of NAND 0. BTW, if your particular NAND chip is not supported by that ROM you are out of luck -- it won't boot off of that chip.

Once again -- it is _NOT_ my board, it is how the i.MX6 boot code works.

Quoted 2 lines

I am expecting to have some parameters like in kobs-ng: I set the number of copies with --search_exponent, how is it done here ?

It makes no sense for limited purpose U-Boot command. You need at least 2 copies of FCB/DBBT. That kobs-ng puts FCB and corresponding DBBT in 2 consecutive erase blocks (sectors) thus taking 2 sectors per FCB/DBBT instance. I put DBBT in the same sector with FCB (exactly like BareBox does) so I can squeeze 4 copies in the same 4 sectors kobs-ng takes for 2 copies.

Then, it is a _LIMITED_ use command so it makes no sense to do something with a long command line and its parsing code. If somebody needs more complex/sophisticated utility he can just put U-Boot with this simple command, boot into their favority OS and use a utility whatever complex or big he wants.

That is what U-Boot is about. I do also miss the mkfs.ext4 or fsck or even just an option to delete or rename a file on ext4 but I do understand it is not all that easy and make workarounds to live with what I have.

Quoted 4 lines
Quoted 1 line

No SPL required, just plain single stage U-Boot.

Why ? This has nothing to do. We could write SPL into the NAND as first bootloader, and SPL will load the u-boot.img unabh?ngig von FCB.

"No SPL required" means exactly that :) It does _NOT_ mean "No SPL is possible" -- as a matter of fact neither that added command nor i.MX6 bootrom even care what that binary payload is. As long as FCB is valid and that binary has a proper header nobody cares what it is -- it is simply loaded and ran.

Quoted 6 lines
Quoted 2 lines

Supposed to write into
first NAND partition, recommended size 4MiB.

Where is this size coming ? This depends on the number of copies (can we have configurable as for kobs-ng ?) and the size of U-Boot. If we write SPL, we need less.

It needs at least 4 sectors for 4 copies of FCB/DBBT (this is 2 copies in kbs-ng, nbare minimum) and at least 2 sectors for each copy of the actual binary if the latter fits in one sector (minimal guarantee we can write it if one of the 2 sectors is bad.) In real life you need some more for NAND. Minimum/standard sector is 128K so you need at least 1MiB for a binary that fits in 128KiB (for bootrom you also need leading 1KiB padding and one trailing empty page so binary must fit in 125KiB.)

If the SPL is that small, yes, one can save some space so SPL partition can be smaller. However possible 3MiB saving on a NAND chip seem hardly worth a bother. I can make 2 commands instead of one, say ubootupdate and splupdate but that is hardly worth a bother. What if one wanted to put a non-SPL U-Boot on his NAND later once it's been partitioned for SPL (more on partition later, see below)?

Quoted 2 lines

Supposed to write into mtd0 works in most cases, yes. But it should be better let it configurable as in kobs-ng.

And the reason to do it is...? What are you going to do with a copy on mtd other than 0? Remember, it is not us who decide where to boot from, it is i.MX6 bootrom. It can NOT boot off of anything but nand0. And remember, that FCB page is not even readable with regular mtd read -- it will fail ECC...

The only use case I could imagine would be 2 redundant NANDs with their CS lines swapped as needed by jumpers or whatever for recovery/dualboot purposes but you can use that very switching mechanism when writing U-Boot into that combination...

Quoted 3 lines

One reason is to show the user an easy path from kobs-ng to u-boot - and of course, how to change from kobs-ng to this command should be explained into the README.

There is no path at all. And nothing can be easier than just issue one command -- there are no options to match/convert, no parameters, nothing at all. What path is needed?

Furthermore once this command is in U-Boot 99.9999% of all users would not have to bother with it at all so they won't even know such a thing ever existed.

Quoted 11 lines
Quoted 8 lines

Creates 4 redundant copies
of boot structures in first 4 sectors and 2 redundant copies of U-Boot in the remaining space.

Tested on a Variscite VAR-SOM-SOLO based board with i.MX6DL SoC. Will probably post that board support files later, when config is cleaned up so people could have a working NAND boot DCD portion to be able actually use those braindead SOMs.

ok - anyway, it looks like that Peng and me can test on other boards - no worry about it.

Sure.

Quoted 19 lines
Quoted 19 lines

---

arch/arm/include/asm/imx-common/mxs_nand.h new file +9 -0
new file mode 100644
index 0000000..7826a9f
@@ -0,0 +1,37 @@
+/*
+ * Copyright (C) 2015 PHYTEC Messtechnik GmbH,
+ * Author: Stefan Christ <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
Quoted 8 lines

arch/arm/include/asm/imx-common/mxs_nand.h line 15

Quoted 6 lines
arch/arm/include/asm/imx-common/mxs_nand.h new file +6 -0
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+

Copyright is ok, but we have SPDX in U-Boot. Change the license header.

That's an excerpt from original copyright. I can change it to SPDX, no biggie. Just wanted to include a verbatim copy of original copyright.

Quoted 82 lines
Quoted 82 lines
arch/arm/include/asm/imx-common/mxs_nand.h new file +5 -0
+#ifndef __NAND_MXS_H
+#define __NAND_MXS_H
+
+/*
+ * Functions are definied in drivers/mtd/nand/nand_mxs.c.  They are

used to
+ * calculate the ECC Strength, BadBlockMarkerByte and BadBlockMarkerStartBit
+ * which are placed into the FCB structure. The i.MX6 ROM needs these
+ * parameters to read the firmware from NAND.
+ *
+ * The parameters depends on the pagesize and oobsize of NAND chips and are
+ * different for each combination. To avoid placing hardcoded values in the bbu
+ * update handler code, the generic calculation from the driver code is used.
+ */

arch/arm/include/asm/imx-common/mxs_nand.h new file +9 -0
+
+uint32_t mxs_nand_get_ecc_strength(uint32_t page_data_size,
+                        uint32_t page_oob_size);
+
+uint32_t mxs_nand_mark_byte_offset(struct mtd_info *mtd);
+
+uint32_t mxs_nand_mark_bit_offset(struct mtd_info *mtd);
+
+#endif /* __NAND_MXS_H */
cmd/nand.c +5 -0
index 583a18f..12e38e1 100644
@@ -38,6 +38,11 @@ int find_dev_and_part(const char *id, struct
 mtd_device **dev,
               u8 *part_num, struct part_info **part);
 #endif
 
+#ifdef CONFIG_CMD_NAND_BOOTUPDATE
+/* This comes from a separate file in drivers/mtd/nand */
+int mxs_do_nand_bootupdate(ulong addr, size_t len);
+#endif
+
 static int nand_dump(struct mtd_info *mtd, ulong off, int only_oob,
              int repeat)

{

cmd/nand.c +3 -0
@@ -373,6 +378,9 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int
 argc, char * const argv[])
     loff_t off, size, maxsize;
     char *cmd, *s;
     struct mtd_info *mtd;
+#ifdef CONFIG_CMD_NAND_BOOTUPDATE
+    size_t cnt;
+#endif
 #ifdef CONFIG_SYS_NAND_QUIET
     int quiet = CONFIG_SYS_NAND_QUIET;

#else

cmd/nand.c +21 -0
@@ -745,6 +753,48 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int
 argc, char * const argv[])
     }
 #endif
 
+#ifdef CONFIG_CMD_NAND_BOOTUPDATE
+    if (strncmp(cmd, "bootupdate", 10) == 0) {
+
+        if (argc < 3) {
+            /* All default values */
+            addr = getenv_ulong("fileaddr", 16, 0UL);
+            cnt = getenv_ulong("filesize", 16, 0UL);
+        }
+
+        if (argc == 3) {
+            /* 'addr' only, file size from environment */
+            if ((addr = simple_strtoul(argv[2], NULL, 16)) == 0UL)
+                addr = getenv_ulong("fileaddr", 16, 0UL);
+
+            cnt = getenv_ulong("filesize", 16, 0UL);
+        }
+
+        if (argc > 3) {
+            /* 'addr', 'size', and possibly more */
+            if ((addr = simple_strtoul(argv[2], NULL, 16)) == 0UL)
+                addr = getenv_ulong("fileaddr", 16, 0UL);
Quoted 8 lines

cmd/nand.c static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) · line 780

Quoted 4 lines
cmd/nand.c +4 -0
+
+            if ((cnt = simple_strtoul(argv[3], NULL, 16)) == 0UL)
+                cnt = getenv_ulong("filesize", 16, 0UL);
+        }

Why do we stick with the environment ? Most U-Boot commands do not do it. A user can always write a script as "bootupdate ${loadaddr} ${filesize}" if he wants, the command itself should be unaware of it.

That means you would have to _ALWAYS_ use the full form, with address and size. And the fact that address is always a hex but size shown after file load is in decimal while it is still hex for [almost] any command in U-Boot so it is very confusing and prone to user errors.

And what's wrong with using default environment variable that is _ALWAYS_ set after file load? It is not something a user can set or forget to in his environment, those are "system" variables.

Quoted 14 lines

cmd/nand.c static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])

Quoted 7 lines
cmd/nand.c +7 -0
+
+
+        if (addr == 0 || cnt == 0) {
+            puts("Invalid arguments to nand bootupdate!\n");
+            return 1;
+        }
+ +        if (mxs_do_nand_bootupdate(addr, cnt)) {

Name is misleading. The function is not related to the NAND driver (mxs_nand), but it is an implementation of the layout requested by NXP's SOCs. A name related to BCB (Boot Control Block) as in manual is more correct.

Quoted 3 lines

So please changes all names from mxs_ in the code if they are not strictly related to the driver, and let mxs_ name just for functions exported by the driver.

I can, no problems. But it is only useful for _SOME_ Freescale (the fact NXP bought them recently doesn't change a fact those are Freescale SOCs) with MXS NAND. It might happen this would apply to other devices in the future but for now it applies nothing but those SOCs with MXS NAND.

Quoted 28 lines
Quoted 28 lines
cmd/nand.c +12 -0
+            puts("NAND bootupdate failed!\n");
+            return 1;
+        }
+
+        puts("NAND bootupdate successful\n");
+        return 0;
+    }
+#endif
+
 usage:
     return CMD_RET_USAGE;
 }
@@ -766,6 +816,17 @@ static char nand_help_text[] =
     "    'addr', skipping bad blocks and dropping any pages at the end\n"
     "    of eraseblocks that contain only 0xFF\n"
 #endif
+#ifdef CONFIG_CMD_NAND_BOOTUPDATE
+    "nand bootupdate - [addr] [size]\n"
+    "    write U-Boot into NAND in board/SoC specific manner creating

all\n"

cmd/nand.c +4 -0
+    "    required headers and other bits and pieces as required for the\n"
+    "    system to be able to boot off of NAND. 'addr' is the address\n"
+    "    where U-Boot image has been loaded at, 'size' is its size.\n"
+    "    If any of 'addr'/'size' is missing it is taken from

environment\n"
+ " for the last file loaded. U-Boot image must be of a proper
type\n"
+ " for the target platform (only IMX image supported at the

Quoted 8 lines

cmd/nand.c static char nand_help_text[] = · line 832

Quoted 6 lines

moment)\n"

cmd/nand.c +2 -0
+    "    binary without U-Boot image headers (e.g. u-boot.imx file.)\n"
+#endif
     "nand erase[.spread] [clean] off size - erase 'size' bytes "
     "from offset 'off'\n"
     "    With '.spread', erase enough for given file size, otherwise,\n"

A README file in doc/ is also useful.

Sure. But not everything at once -- it is the first, initial submission that can be changed/ammended/betterized later on if needed. One has to start somewhere, right?

Quoted 5 lines
Quoted 5 lines
drivers/mtd/nand/Makefile +0 -0
index 837d397..36609e1 100644
@@ -66,6 +66,7 @@ obj-$(CONFIG_TEGRA_NAND) += tegra_nand.o
Quoted 8 lines

drivers/mtd/nand/Makefile obj-$(CONFIG_TEGRA_NAND) += tegra_nand.o · line 69

Quoted 4 lines
drivers/mtd/nand/Makefile +1 -0
 obj-$(CONFIG_NAND_OMAP_GPMC) += omap_gpmc.o
 obj-$(CONFIG_NAND_OMAP_ELM) += omap_elm.o
 obj-$(CONFIG_NAND_PLAT) += nand_plat.o
+obj-$(CONFIG_CMD_NAND_BOOTUPDATE) += mxs_nand_bootupdate.o

It will be nice to set some dependencies between CONFIG_NAND_MXS and CONFIG_CMD_NAND_BOOTUPDATE. CONFIG_CMD_NAND_BOOTUPDATE is not useful if CONFIG_NAND_MXS is not set, right ?

Yep. As well as e.g. UBI is not useful without RBTREE/LZO but you'll have to choose those yourself. And NAND_MXS also requires APHB_DMA and some other stuff. That, in turn, require some other stuff and it goes ad infinitum.

BTW, the fact it is not useful without NAND_MXS is the reason why I used "mxs-" in source/functions file names. Do you still want me to change it?

Quoted 47 lines
Quoted 42 lines
drivers/mtd/nand/Makefile +0 -0
 else  # minimal SPL drivers
 
drivers/mtd/nand/mxs_nand.c +4 -3
index 7be1f86..afeec22 100644
@@ -26,6 +26,7 @@
 #include <asm/imx-common/regs-gpmi.h>
 #include <asm/arch/sys_proto.h>
 #include <asm/imx-common/dma.h>
+#include <asm/imx-common/mxs_nand.h>
 
 #define    MXS_NAND_DMA_DESCRIPTOR_COUNT        4
 
@@ -145,7 +146,7 @@ static uint32_t mxs_nand_aux_status_offset(void)
     return (MXS_NAND_METADATA_SIZE + 0x3) & ~0x3;
 }
 
-static inline uint32_t mxs_nand_get_ecc_strength(uint32_t page_data_size,
+uint32_t mxs_nand_get_ecc_strength(uint32_t page_data_size,
                         uint32_t page_oob_size)
 {
     int ecc_strength;
@@ -221,14 +222,14 @@ static inline uint32_t
 mxs_nand_get_mark_offset(uint32_t page_data_size,
     return block_mark_bit_offset;
 }
 
-static uint32_t mxs_nand_mark_byte_offset(struct mtd_info *mtd)
+uint32_t mxs_nand_mark_byte_offset(struct mtd_info *mtd)
 {
     uint32_t ecc_strength;
     ecc_strength = mxs_nand_get_ecc_strength(mtd->writesize,
 mtd->oobsize);
     return mxs_nand_get_mark_offset(mtd->writesize, ecc_strength) >> 3;
 }
 
-static uint32_t mxs_nand_mark_bit_offset(struct mtd_info *mtd)
+uint32_t mxs_nand_mark_bit_offset(struct mtd_info *mtd)
 {
     uint32_t ecc_strength;
     ecc_strength = mxs_nand_get_ecc_strength(mtd->writesize,
Quoted 1 line

mtd->oobsize);

Added Scott as MTD maintainer: it is clear to me why, but the driver exports its functions via the struct nand_chip (see board_nand_init).

Quoted 2 lines

Is it not better to extend the structure with the functions you want to export ?

It might be. I don't care how I access those functions but it is probably up to them to extend the structure...

BTW it is _EXACTLY_ how it is done in barebox. They started with putting all that code into their source but then realized it is already there, just as a file scope in another file so they simply exported it to avoid code duplication.

If MTD guys decided to add it to their structure I will happily take it from there. That would make it impossible to base it on a current master until such structure change is actually applied but there is no rush here; I can wait :)

Quoted 5 lines

drivers/mtd/nand/mxs_nand_bootupdate.c

Quoted 2 lines
drivers/mtd/nand/mxs_nand_bootupdate.c +0 -0

I do not know if this file should be here or should be in imx-common. It is relevant just for i.MX and not for other SOCs.

I do not either :) I considered both places and the current one had been chosen with a coin toss :) If you want me to put it in imx-common I will do it, no problems. Just tell me to.

Quoted 2 lines

In barebox it has a better name : imx-bbu-nand-fcb.c. It says much more as mxs_nand_bootupdate.c.

It it not. The "bbu" part stays for "BareBox Update" and we are not barebox, we are U-Boot. If you can come up with better file name please tell me what it is and I will rename it. I don't care what it is called.

Quoted 37 lines
Quoted 24 lines

new file mode 100644
index 0000000..9442553

drivers/mtd/nand/mxs_nand_bootupdate.c new file +17 -0
@@ -0,0 +1,556 @@
+/*
+ * mxs_nand_bootupdate.c - write U-Boot to MXS NAND to make it bootable
+ *
+ * Copyright (c) 2016 Sergey Kubushyn <[email protected]>
+ *
+ * Most of the source shamelesly stolen from barebox.
+ *
+ * Here is the original copyright:
+ *
+ *=== Cut ===
+ * Copyright (C) 2014 Sascha Hauer, Pengutronix
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *

drivers/mtd/nand/mxs_nand_bootupdate.c line 27

Quoted 10 lines
drivers/mtd/nand/mxs_nand_bootupdate.c new file +10 -0
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation.
+ *=== Cut ===
+ */

Let Copyright and add SPDX header.

Quoted 3 lines

As you ported code from another Open Spurce project, it will be nice if you write this into the commit message, writing also from which barebox commit you took the code, exactly how we do when we port code from Linux.

No problems, just wanted to keep a verbatim copy of original copyright...

Quoted 16 lines
Quoted 16 lines
drivers/mtd/nand/mxs_nand_bootupdate.c new file +16 -0
+
+
+#include <common.h>
+#include <linux/sizes.h>
+#include <linux/mtd/mtd.h>
+#include <linux/compat.h>
+#include <command.h>
+#include <console.h>
+#include <malloc.h>
+#include <asm/byteorder.h>
+#include <jffs2/jffs2.h>
+#include <nand.h>
+#include <errno.h>
+#include <asm/imx-common/mxs_nand.h>
+#include <asm/imx-common/imximage.cfg>
+
Quoted 8 lines

drivers/mtd/nand/mxs_nand_bootupdate.c line 47

Quoted 4 lines
drivers/mtd/nand/mxs_nand_bootupdate.c new file +4 -0
+
+#ifndef CONFIG_CMD_MTDPARTS
+#error "CONFIG_CMD_MTDPARTS is not set, mtd partition support missing"
+#endif

I am missing because this is important. U-Boot works with addresses, and not with MTDs. The whole nand utilities take an address as parameter. MTD is optional, and can be used or not in nand erase/read/write.

Sure but it makes life more difficult. The update needs to know what space it has to plan accordingly and calculate where to put things. That range is also used for DBBT scan -- it doesn't make sense to go beyond that partition (or region) when creating DBBT because it is of now other use, just for bootrom NAND boot.

Then there is other reason for this. If one has NAND it would've been illogical to just put U-Boot there as Variscite guys do. The remaining space can host quite sizable FS on it. The only one currently supported is UBIFS and UBI requires a partition. Once we have partition anyway it raises a question why not to use a partition instead of address range for U-Boot?

Quoted 147 lines
Quoted 147 lines
drivers/mtd/nand/mxs_nand_bootupdate.c new file +6 -0
+
+static const char *uboot_tgt = "nand0,0";
+
+/* partition handling routines */
+int mtdparts_init(void);
+int id_parse(const char *id, const char **ret_id, u8 *dev_type, u8

*dev_num);

drivers/mtd/nand/mxs_nand_bootupdate.c new file +25 -0
+int find_dev_and_part(const char *id, struct mtd_device **dev,
+              u8 *part_num, struct part_info **part);
+
+struct dbbt_block {
+    uint32_t Checksum;    /* reserved on i.MX6 */
+    uint32_t FingerPrint;
+    uint32_t Version;
+    uint32_t numberBB;    /* reserved on i.MX6 */
+    uint32_t DBBTNumOfPages;
+};
+
+struct fcb_block {
+    uint32_t Checksum;        /* First fingerprint in first byte */
+    uint32_t FingerPrint;        /* 2nd fingerprint at byte 4 */
+    uint32_t Version;        /* 3rd fingerprint at byte 8 */
+    uint8_t DataSetup;
+    uint8_t DataHold;
+    uint8_t AddressSetup;
+    uint8_t DSAMPLE_TIME;
+    /* These are for application use only and not for ROM. */
+    uint8_t NandTimingState;
+    uint8_t REA;
+    uint8_t RLOH;
+    uint8_t RHOH;
+    uint32_t PageDataSize;        /* 2048 for 2K pages, 4096 for 4K

pages */
+ uint32_t TotalPageSize; /* 2112 for 2K pages, 4314 for 4K
pages */

drivers/mtd/nand/mxs_nand_bootupdate.c new file +2 -0
+    uint32_t SectorsPerBlock;    /* Number of 2K sections per block */
+    uint32_t NumberOfNANDs;        /* Total Number of NANDs - not used

by ROM */
+ uint32_t TotalInternalDie; /* Number of separate chips in this
NAND */

drivers/mtd/nand/mxs_nand_bootupdate.c new file +2 -0
+    uint32_t CellType;        /* MLC or SLC */
+    uint32_t EccBlockNEccType;    /* Type of ECC, can be one of

BCH-0-20 */

drivers/mtd/nand/mxs_nand_bootupdate.c new file +2 -0
+    uint32_t EccBlock0Size;        /* Number of bytes for Block0 - BCH */
+    uint32_t EccBlockNSize;        /* Block size in bytes for all

blocks other than Block0 - BCH */

drivers/mtd/nand/mxs_nand_bootupdate.c new file +3 -0
+    uint32_t EccBlock0EccType;    /* Ecc level for Block 0 - BCH */
+    uint32_t MetadataBytes;        /* Metadata size - BCH */
+    uint32_t NumEccBlocksPerPage;    /* Number of blocks per page for

ROM use - BCH */
+ uint32_t EccBlockNEccLevelSDK; /* Type of ECC, can be one of
BCH-0-20 */

drivers/mtd/nand/mxs_nand_bootupdate.c new file +2 -0
+    uint32_t EccBlock0SizeSDK;    /* Number of bytes for Block0 - BCH */
+    uint32_t EccBlockNSizeSDK;    /* Block size in bytes for all blocks

other than Block0 - BCH */

drivers/mtd/nand/mxs_nand_bootupdate.c new file +2 -0
+    uint32_t EccBlock0EccLevelSDK;    /* Ecc level for Block 0 - BCH */
+    uint32_t NumEccBlocksPerPageSDK;/* Number of blocks per page for

SDK use - BCH */

drivers/mtd/nand/mxs_nand_bootupdate.c new file +3 -0
+    uint32_t MetadataBytesSDK;    /* Metadata size - BCH */
+    uint32_t EraseThreshold;    /* To set into BCH_MODE register */
+    uint32_t BootPatch;        /* 0 for normal boot and 1 to load patch

starting next to FCB */

drivers/mtd/nand/mxs_nand_bootupdate.c new file +2 -0
+    uint32_t PatchSectors;        /* Size of patch in sectors */
+    uint32_t Firmware1_startingPage;/* Firmware image starts on this

sector */
+ uint32_t Firmware2_startingPage;/* Secondary FW Image starting
Sector */
+ uint32_t PagesInFirmware1; /* Number of sectors in firmware
image */
+ uint32_t PagesInFirmware2; /* Number of sector in secondary FW
image */
+ uint32_t DBBTSearchAreaStartAddress; /* Page address where dbbt
search area begins */
+ uint32_t BadBlockMarkerByte; /* Byte in page data that have
manufacturer marked bad block marker, */
+ /* this will be swapped with metadata[0] to
complete page data. */
+ uint32_t BadBlockMarkerStartBit;/* For BCH ECC sizes other than 8
and 16 the bad block marker does not */
+ /* start at 0th bit of BadBlockMarkerByte. This
field is used to get to */
+ /* the start bit of bad block marker byte with in
BadBlockMarkerByte */
+ uint32_t BBMarkerPhysicalOffset;/* FCB value that gives byte offset
for bad block marker on physical NAND page */

drivers/mtd/nand/mxs_nand_bootupdate.c new file +13 -0
+    uint32_t BCHType;
+
+    uint32_t TMTiming2_ReadLatency;
+    uint32_t TMTiming2_PreambleDelay;
+    uint32_t TMTiming2_CEDelay;
+    uint32_t TMTiming2_PostambleDelay;
+    uint32_t TMTiming2_CmdAddPause;
+    uint32_t TMTiming2_DataPause;
+    uint32_t TMSpeed;
+    uint32_t TMTiming1_BusyTimeout;
+
+    uint32_t DISBBM;    /* the flag to enable (1)/disable(0) bi swap */
+    uint32_t BBMarkerPhysicalOffsetInSpareData; /* The swap position of

main area in spare area */

drivers/mtd/nand/mxs_nand_bootupdate.c new file +19 -0
+};
+
+struct fw_write_data {
+    int    fw1_blk;
+    int    fw2_blk;
+    int    part_blks;
+    void    *buf;
+    size_t    len;
+};
+
+#define BF_VAL(v, bf)    (((v) & bf##_MASK) >> bf##_OFFSET)
+#define GETBIT(v,n)    (((v) >> (n)) & 0x1)
+
+
+static uint8_t calculate_parity_13_8(uint8_t d)
+{
+    uint8_t    p = 0;
+
+    p |= (GETBIT(d, 6) ^ GETBIT(d, 5) ^ GETBIT(d, 3) ^ GETBIT(d,

2)) << 0;
+ p |= (GETBIT(d, 7) ^ GETBIT(d, 5) ^ GETBIT(d, 4) ^ GETBIT(d, 2) ^
GETBIT(d, 1)) << 1;
+ p |= (GETBIT(d, 7) ^ GETBIT(d, 6) ^ GETBIT(d, 5) ^ GETBIT(d, 1) ^
GETBIT(d, 0)) << 2;
+ p |= (GETBIT(d, 7) ^ GETBIT(d, 4) ^ GETBIT(d, 3) ^ GETBIT(d,
0)) << 3;
+ p |= (GETBIT(d, 6) ^ GETBIT(d, 4) ^ GETBIT(d, 3) ^ GETBIT(d, 2) ^
GETBIT(d, 1) ^ GETBIT(d, 0)) << 4;

drivers/mtd/nand/mxs_nand_bootupdate.c new file +20 -0
+    return p;
+}
+
+static void encode_hamming_13_8(void *_src, void *_ecc, size_t size)
+{
+    int    i;
+    uint8_t    *src = _src;
+    uint8_t    *ecc = _ecc;
+
+    for (i = 0; i < size; i++)
+        ecc[i] = calculate_parity_13_8(src[i]);
+}
+
+static uint32_t calc_chksum(void *buf, size_t size)
+{
+    u32    chksum = 0;
+    u8    *bp = buf;
+    size_t    i;
+
+    for (i = 0; i < size; i++)
Quoted 8 lines

drivers/mtd/nand/mxs_nand_bootupdate.c line 169

Quoted 4 lines
drivers/mtd/nand/mxs_nand_bootupdate.c new file +4 -0
+        chksum += bp[i];
+
+    return ~chksum;
+}

I understand that it is for debug only - but you have removed debug functions from barebox (dump_fcb for example). Can they be useful here, too ?

They were useful for debugging, to show what we put into that FCB. Once we are sure it is working correctly there is nothing more to debug.

Quoted 198 lines
Quoted 185 lines
drivers/mtd/nand/mxs_nand_bootupdate.c new file +3 -0
+
+
+static ssize_t raw_write_page(struct mtd_info *mtd, void *buf, loff_t

offset)

drivers/mtd/nand/mxs_nand_bootupdate.c new file +24 -0
+{
+    struct mtd_oob_ops ops;
+    ssize_t ret;
+
+    ops.mode = MTD_OPS_RAW;
+    ops.ooboffs = 0;
+    ops.datbuf = buf;
+    ops.len = mtd->writesize;
+    ops.oobbuf = buf + mtd->writesize;
+    ops.ooblen = mtd->oobsize;
+    ret = mtd_write_oob(mtd, offset, &ops);
+
+        return ret;
+}
+
+static int fcb_create(struct fcb_block *fcb, struct mtd_info *mtd)
+{
+    fcb->FingerPrint = 0x20424346;
+    fcb->Version = 0x01000000;
+    fcb->PageDataSize = mtd->writesize;
+    fcb->TotalPageSize = mtd->writesize + mtd->oobsize;
+    fcb->SectorsPerBlock = mtd->erasesize / mtd->writesize;
+
+    /* Divide ECC strength by two and save the value into FCB

structure. */

drivers/mtd/nand/mxs_nand_bootupdate.c new file +31 -0
+    fcb->EccBlock0EccType =
+        mxs_nand_get_ecc_strength(mtd->writesize, mtd->oobsize) >> 1;
+
+    fcb->EccBlockNEccType = fcb->EccBlock0EccType;
+
+    /* Also hardcoded in kobs-ng */
+    fcb->EccBlock0Size = 0x00000200;
+    fcb->EccBlockNSize = 0x00000200;
+    fcb->DataSetup = 80;
+    fcb->DataHold = 60;
+    fcb->AddressSetup = 25;
+    fcb->DSAMPLE_TIME = 6;
+    fcb->MetadataBytes = 10;
+
+    /* DBBT search area starts at second page on first block */
+    fcb->DBBTSearchAreaStartAddress = 1;
+
+    fcb->BadBlockMarkerByte = mxs_nand_mark_byte_offset(mtd);
+    fcb->BadBlockMarkerStartBit = mxs_nand_mark_bit_offset(mtd);
+
+    fcb->BBMarkerPhysicalOffset = mtd->writesize;
+
+    fcb->NumEccBlocksPerPage = mtd->writesize / fcb->EccBlock0Size - 1;
+
+    fcb->Checksum = calc_chksum((void *)fcb + 4, sizeof(*fcb) - 4);
+
+    return 0;
+}
+
+/* Erase entire U-Boot partition */
+static int mxs_nand_uboot_erase(struct mtd_info *mtd, struct part_info

*part)

drivers/mtd/nand/mxs_nand_bootupdate.c new file +32 -0
+{
+    uint64_t        offset = 0;
+    struct erase_info    erase;
+    int            len = part->size;
+    int            ret;
+
+    while (len > 0) {
+        pr_debug("erasing at 0x%08llx\n", offset);
+        if (mtd_block_isbad(mtd, offset)) {
+            pr_debug("erase skip block @ 0x%08llx\n", offset);
+            offset += mtd->erasesize;
+            continue;
+        }
+
+        memset(&erase, 0, sizeof(erase));
+        erase.addr = offset;
+        erase.len = mtd->erasesize;
+
+        ret = mtd_erase(mtd, &erase);
+        if (ret)
+            return ret;
+
+        offset += mtd->erasesize;
+        len -= mtd->erasesize;
+    }
+
+    return 0;
+}
+
+/* Write the U-Boot proper (2 copies) to where it belongs.   */
+/* This is U-Boot binary image itself, no FCB/DBBT here yet. */
+static int mxs_nand_uboot_write_fw(struct mtd_info *mtd, struct

fw_write_data *fw)

drivers/mtd/nand/mxs_nand_bootupdate.c new file +75 -0
+{
+    uint64_t    offset;
+    int        ret;
+    int        blk;
+    size_t        dummy;
+    size_t        bytes_left;
+    int        chunk;
+    void        *p;
+
+    bytes_left = fw->len;
+    p = fw->buf;
+    blk = fw->fw1_blk;
+    offset = blk * mtd->erasesize;
+
+    while (bytes_left > 0) {
+        chunk = min(bytes_left, mtd->erasesize);
+
+        pr_debug("fw1: writing %p at 0x%08llx, left 0x%08x\n",
+                p, offset, bytes_left);
+
+        if (mtd_block_isbad(mtd, offset)) {
+            pr_debug("fw1: write skip block @ 0x%08llx\n", offset);
+            offset += mtd->erasesize;
+            blk++;
+            continue;
+        }
+
+        if (blk >= fw->fw2_blk)
+            return -ENOSPC;
+
+        ret = mtd_write(mtd, offset, chunk, &dummy, p);
+        if (ret)
+            return ret;
+
+        offset += chunk;
+        bytes_left -= chunk;
+        p += chunk;
+        blk++;
+    }
+
+    bytes_left = fw->len;
+    p = fw->buf;
+    blk = fw->fw2_blk;
+    offset = blk * mtd->erasesize;
+
+    while (bytes_left > 0) {
+        chunk = min(bytes_left, mtd->erasesize);
+
+        pr_debug("fw2: writing %p at 0x%08llx, left 0x%08x\n",
+                p, offset, bytes_left);
+
+        if (mtd_block_isbad(mtd, offset)) {
+            pr_debug("fw2: write skip block @ 0x%08llx\n", offset);
+            offset += mtd->erasesize;
+            blk++;
+            continue;
+        }
+
+        if (blk >= fw->part_blks)
+            return -ENOSPC;
+
+        ret = mtd_write(mtd, offset, chunk, &dummy, p);
+        if (ret)
+            return ret;
+
+        offset += chunk;
+        bytes_left -= chunk;
+        p += chunk;
+        blk++;
+    }
+
+    return 0;
+}
+
+static int dbbt_data_create(struct mtd_info *mtd, void *buf, int

num_blocks)

drivers/mtd/nand/mxs_nand_bootupdate.c new file +15 -0
+{
+    int        n;
+    int        n_bad_blocks = 0;
+    uint32_t    *bb = buf + 0x8;
+    uint32_t    *n_bad_blocksp = buf + 0x4;
+
+    for (n = 0; n < num_blocks; n++) {
+        loff_t offset = n * mtd->erasesize;
+        if (mtd_block_isbad(mtd, offset)) {
+            n_bad_blocks++;
+            *bb = n;
+            bb++;
+        }
+    }
+

drivers/mtd/nand/mxs_nand_bootupdate.c line 359

Quoted 10 lines
drivers/mtd/nand/mxs_nand_bootupdate.c new file +10 -0
+    *n_bad_blocksp = n_bad_blocks;
+
+    return n_bad_blocks;
+}
+
+/********************************************************************/
+/* This is where it is all done. Takes pointer to a U-Boot image in */
+/* RAM and image size, creates FCB/DBBT and writes everything where */
+/* it belongs into NAND. Image must be an IMX image built for NAND. */
+/********************************************************************/

Multiline comment, please fix it.

Quoted 4 lines
Quoted 1 line

+static int mxs_nand_uboot_update(const void *img, size_t len)

I would prefer you let the function names as in original code, so we know where they are coming from (imx_bbu_nand_update here).

As I already said "bbu" stands for "BareBox Update". Do we have to keep it in U-Boot?

If you have a better idea for function mane please go ahead and tell what it is. I don't care what the name is; even xyz123456 would be OK.

Quoted 36 lines
Quoted 36 lines
drivers/mtd/nand/mxs_nand_bootupdate.c new file +36 -0
+{
+    int             i, ret;
+
+    size_t            dummy;
+    loff_t            offset = 0;
+
+    void             *fcb_raw_page;
+    void            *dbbt_page;
+    void            *dbbt_data_page;
+    void            *ecc;
+
+    uint32_t         num_blocks_fcb_dbbt;
+    uint32_t        num_blocks_fw;
+
+    struct mtd_info        *mtd;
+    struct fcb_block    *fcb;
+    struct dbbt_block    *dbbt;
+
+    struct mtd_device    *dev;
+    struct part_info    *part;
+    u8            pnum;
+
+    struct fw_write_data    fw;
+
+    if ((mtdparts_init() == 0) &&
+            (find_dev_and_part(uboot_tgt, &dev, &pnum, &part) == 0)) {
+        if (dev->id->type != MTD_DEV_TYPE_NAND) {
+            puts("Not a NAND device\n");
+            return -ENODEV;
+        }
+    }
+
+    nand_curr_device = dev->id->num;
+
+#ifdef CONFIG_SYS_NAND_SELECT_DEVICE
+    board_nand_select_device(nand_info[nand_curr_device].priv,
Quoted 8 lines

drivers/mtd/nand/mxs_nand_bootupdate.c line 401

Quoted 6 lines

nand_curr_device);

drivers/mtd/nand/mxs_nand_bootupdate.c new file +5 -0
+#endif
+
+    /* Get a pointer to mtd_info for selected device */
+
+    mtd = get_mtd_device_nm("nand0");    /* We always boot off of nand0 */

...and if I have nand1 ?

So keep it for yourself :) i.MX6 bootrom won't boot off of it so no reason to put a bootloader there.

Quoted 1 line

As I said, U-Boot works with addresses

And what?

Quoted 12 lines
Quoted 11 lines

+ + if (IS_ERR(mtd)) {

drivers/mtd/nand/mxs_nand_bootupdate.c new file +7 -0
+        /* Should not happen */
+        puts("No nand0 device...\n");
+        return -ENODEV;
+    }
+ +    put_mtd_device(mtd);
+
+    /* Quick and dirty check if we have 2Mbytes of good blocks in

nand0,0 */
+ /* Not sure if it is needed at all but won't hurt so here it
is... */

Quoted 1 line

Yes, it hurts.

I can remove it, no problems.

Quoted 25 lines

drivers/mtd/nand/mxs_nand_bootupdate.c line 427

Quoted 13 lines

+ + i = 0;

drivers/mtd/nand/mxs_nand_bootupdate.c new file +12 -0
+    offset = 0;    /* It is the first partition so it starts at block 0 */
+
+    while (offset < part->size) {
+        if (!mtd_block_isbad(mtd, offset)) {
+            i += mtd->erasesize;
+        }
+        offset += mtd->erasesize;
+    }
+ +    if (i < SZ_2M) {
+        puts("Partition too small for U-Boot!\n");
+        return -EINVAL;
+    }

I try to understand your changes from barebox code - something are clear due to the differences between the two bootloaders, some other ones not.

drivers/mtd/nand/mxs_nand_bootupdate.c line 434

Quoted 7 lines
drivers/mtd/nand/mxs_nand_bootupdate.c new file +7 -0
+
+    /* We will use 4 first blocks for FCB/DBBT copies. */
+    /* The rest of partition is split in half and used */
+    /* for two U-Boot copies. We don't care if those   */
+    /* start on good or bad block - bad blocks will be */
+    /* skipped on write, ROM boot code will also skip  */
+    /* bad blocks on bootup when loading U-Boot image. */
Quoted 2 lines

Multiline comment is wrong. I think the original comment in bareboc is much more clear.

OK, no problems, will steal that.

Quoted 7 lines
Quoted 7 lines
drivers/mtd/nand/mxs_nand_bootupdate.c new file +7 -0
+
+    fw.part_blks = part->size / mtd->erasesize;
+    num_blocks_fcb_dbbt = 4;
+    num_blocks_fw = (fw.part_blks - num_blocks_fcb_dbbt) / 2;
+    fw.fw1_blk = num_blocks_fcb_dbbt;
+    fw.fw2_blk = fw.fw1_blk + num_blocks_fw;
+
Quoted 8 lines

drivers/mtd/nand/mxs_nand_bootupdate.c line 447

Quoted 6 lines
drivers/mtd/nand/mxs_nand_bootupdate.c new file +6 -0
+    /* OK, now create FCB structure for bootROM NAND boot */
+
+    fcb_raw_page = kzalloc(mtd->writesize + mtd->oobsize, GFP_KERNEL);
+
+    fcb = fcb_raw_page + 12;
+    ecc = fcb_raw_page + 512 + 12;

??

!! :) It is how the FCB page is laid out and it is what makes it fail regular BCH ECC check. It is what i.MX6 bootrom expects.

FCB page is read in the raw. And written too.

Quoted 104 lines
Quoted 104 lines
drivers/mtd/nand/mxs_nand_bootupdate.c new file +9 -0
+
+    dbbt_page = kzalloc(mtd->writesize, GFP_KERNEL);
+    dbbt_data_page = kzalloc(mtd->writesize, GFP_KERNEL);
+    dbbt = dbbt_page;
+
+    /* Write one additional page to make the ROM happy. */
+    /* Maybe the PagesInFirmwarex fields are really the */
+    /* number of pages - 1. kobs-ng does the same.      */
+ +    fw.len = ALIGN(len + FLASH_OFFSET_STANDARD + mtd->writesize,

mtd->writesize);

drivers/mtd/nand/mxs_nand_bootupdate.c new file +17 -0
+    fw.buf = kzalloc(fw.len, GFP_KERNEL);
+    memcpy(fw.buf + FLASH_OFFSET_STANDARD, img, len);
+
+    /* Erase entire partition */
+    ret = mxs_nand_uboot_erase(mtd, part);
+    if (ret)
+        goto out;
+
+    /* Now write 2 copies of the U-Boot proper to where they belong. */
+    /* Headers (FCB, DBBT) will be generated and written after that. */
+    ret = mxs_nand_uboot_write_fw(mtd, &fw);
+    if (ret < 0)
+        goto out;
+
+    /* Create FCB, calculate ECC (we don't/can't use hardware ECC */
+    /* here so we do it ourselves and then write _RAW_ pages.     */
+ +    fcb->Firmware1_startingPage = fw.fw1_blk * mtd->erasesize /

mtd->writesize;
+ fcb->Firmware2_startingPage = fw.fw2_blk * mtd->erasesize /
mtd->writesize;

drivers/mtd/nand/mxs_nand_bootupdate.c new file +2 -0
+    fcb->PagesInFirmware1 =
+        ALIGN(len + FLASH_OFFSET_STANDARD, mtd->writesize) /

mtd->writesize;

drivers/mtd/nand/mxs_nand_bootupdate.c new file +11 -0
+    fcb->PagesInFirmware2 = fcb->PagesInFirmware1;
+
+    fcb_create(fcb, mtd);
+    encode_hamming_13_8(fcb, ecc, 512);
+
+    /*
+     * Set the first and second byte of OOB data to 0xFF, not 0x00. These
+     * bytes are used as the Manufacturers Bad Block Marker (MBBM). Since
+     * the FCB is mostly written to the first page in a block, a scan for
+     * factory bad blocks will detect these blocks as bad, e.g. when
+     * function nand_scan_bbt() is executed to build a new bad block

table.

drivers/mtd/nand/mxs_nand_bootupdate.c new file +59 -0
+     * We will _NOT_ mark a bad block as good -- we skip the bad blocks.
+     */
+    memset(fcb_raw_page + mtd->writesize, 0xff, 2);
+
+    /* Now create DBBT */
+    dbbt->Checksum = 0;
+    dbbt->FingerPrint = 0x54424244;
+    dbbt->Version = 0x01000000;
+
+    if ((ret = dbbt_data_create(mtd, dbbt_data_page, fw.part_blks)) < 0)
+        goto out;
+
+    if (ret > 0)
+        dbbt->DBBTNumOfPages = 1;
+
+    offset = 0;
+
+    if (mtd_block_isbad(mtd, offset)) {
+        puts("Block 0 is bad, NAND unusable\n");
+        ret = -EIO;
+        goto out;
+    }
+
+    /* Write FCB/DBBT to first 4 blocks. Skip bad blocks if any. */
+    /* Less than 4 copies will be written if there were BBs !!!  */
+    for (i = 0; i < 4; i++) {
+
+        if (mtd_block_isbad(mtd, offset)) {
+            pr_err("Block %d is bad, skipped\n", i);
+            continue;
+        }
+
+
+        ret = raw_write_page(mtd, fcb_raw_page, mtd->erasesize * i);
+        if (ret)
+            goto out;
+
+        ret = mtd_write(mtd, mtd->erasesize * i + mtd->writesize,
+                mtd->writesize, &dummy, dbbt_page);
+        if (ret)
+            goto out;
+
+        /* DBBTNumOfPages == 0 if no bad blocks */
+        if (dbbt->DBBTNumOfPages > 0) {
+            ret = mtd_write(mtd, mtd->erasesize * i + mtd->writesize * 5,
+                    mtd->writesize, &dummy, dbbt_data_page);
+            if (ret)
+                goto out;
+        }
+    }
+
+out:
+    kfree(dbbt_page);
+    kfree(dbbt_data_page);
+    kfree(fcb_raw_page);
+    kfree(fw.buf);
+
+    return ret;
+}
Quoted 8 lines

drivers/mtd/nand/mxs_nand_bootupdate.c line 556

Quoted 8 lines
drivers/mtd/nand/mxs_nand_bootupdate.c new file +8 -0
+
+
+int mxs_do_nand_bootupdate(ulong addr, size_t len)
+{
+    /* KSI: Unlock NAND first if it is locked... */
+
+    return mxs_nand_uboot_update((const void *)addr, len);
+}

---
******************************************************************
* KSI at home KOI8 Net < > The impossible we do immediately. *
* Las Vegas NV, USA < > Miracles require 24-hour notice. *
******************************************************************

#4 lore raw

PATCH 1/1 i.MX6 nand bootupdate

Hi Sergey,

Quoted 16 lines · Sergey Kubushyn

On Wed, 15 Jun 2016, Stefano Babic wrote:

Hi,

Quoted 12 lines

Hi Sergey,

Quoted 3 lines · Sergey Kubushyn

Here is the initial support for writing i.MX6 NAND U-Boot into NAND with all FCB and DBBT stuff as required.

This is a very interesting feature missing in U-Boot. Up now we are constrained to update the bootloader from user space with kobs-ng, and this is really an improvement. Thanks for that.

Quoted 6 lines
Quoted 2 lines

Anyway, as remarked by Peng, please rebase your patch on current tree, else we cannot test it.

I will when time permits, probably tomorrow. Now let's go over other stuff...

Right.

Quoted 10 lines
Quoted 6 lines
Quoted 3 lines

Just build U-Boot for NAND as u-boot.imx, load it somehow in RAM and say "nand bootupdate [addr] [size]". It will create all necessary structures and write everything as needed.

The whole phrase is misleading here. I guess this is the way you have to put on your board, it is not a general case.

The very (and only) purpose for this function is to put U-Boot with proper FCB/DBBT header on one's board so i.MX6 bootrom would be able to boot off of it.

Exactly - this is what it is supposed to do. Write this statement. Building a u-boot.imx, loaing in RAM, etc, this is a specialized use case. I could build SPL, downloading a u-boot.img via serial, and I get the same result. Or use a JTAG debugger.

So what you are saying is a use case - correct, but it looks like it is the rule. Drop it and write simply what the patch is doing, that is adding FCB / DDBT.

Quoted 1 line

There is _NO_ general case.

Right, so do not try to explain - a reader can think _IS_ the rule.

Quoted 3 lines

One can _NOT_ change the way
i.MX6 (or whatever) boots up, it is in the ROM and it always reads FCB starting from sector 0 of NAND 0.

Right, but this has nothing to do how a user put the bootloader the first time. It is enough you write that a new command is added, and the syntax for the new command.

Quoted 3 lines

BTW, if your particular NAND chip is
not supported by that ROM you are out of luck -- it won't boot off of that chip.

We know that :-(

Quoted 2 lines

Once again -- it is _NOT_ my board, it is how the i.MX6 boot code works.

Partially - I hope I have better explained.

Quoted 6 lines
Quoted 3 lines

I am expecting to have some parameters like in kobs-ng: I set the number of copies with --search_exponent, how is it done here ?

It makes no sense for limited purpose U-Boot command. You need at least 2 copies of FCB/DBBT.

Right.

Quoted 5 lines

That kobs-ng puts FCB and corresponding DBBT in 2 consecutive erase blocks (sectors) thus taking 2 sectors per FCB/DBBT instance. I put DBBT in the same sector with FCB (exactly like BareBox does) so I can squeeze 4 copies in the same 4 sectors kobs-ng takes for 2 copies.

It makes sense - thanks for clarification. Indeed, it will be nice if you put exactly this sentences in a README file.

Quoted 6 lines

Then, it is a _LIMITED_ use command so it makes no sense to do something with a long command line and its parsing code. If somebody needs more complex/sophisticated utility he can just put U-Boot with this simple command, boot into their favority OS and use a utility whatever complex or big he wants.

ok, fine with me - point is explained.

Quoted 5 lines

That is what U-Boot is about. I do also miss the mkfs.ext4 or fsck or even just an option to delete or rename a file on ext4 but I do understand it is not all that easy and make workarounds to live with what I have.

...this is OT...

Quoted 9 lines
Quoted 5 lines
Quoted 2 lines

No SPL required, just plain single stage U-Boot.

Why ? This has nothing to do. We could write SPL into the NAND as first bootloader, and SPL will load the u-boot.img unabh?ngig von FCB.

"No SPL required" means exactly that :) It does _NOT_ mean "No SPL is possible" -- as a matter of fact neither that added command nor i.MX6 bootrom even care what that binary payload is.

Exactly. IMHO the sentence can confuse and someone can understand that, evenif SPL is not strictly forbidden, it is _better_ to habe just a single stage. It is not, it has nothing to do.

As you have better explained now, it does not mean this. Drop the sentence.

Quoted 3 lines

As long as FCB is valid
and that binary has a proper header nobody cares what it is -- it is simply loaded and ran.

Exactly. This is what your patch is supposed to do. If someone uses it for single stage u-boot, or with SPL, this is his decision. Please just explain the new feature added by the patch, and let the rest outside.

Quoted 9 lines
Quoted 7 lines
Quoted 3 lines

Supposed to write into
first NAND partition, recommended size 4MiB.

Where is this size coming ? This depends on the number of copies (can we have configurable as for kobs-ng ?) and the size of U-Boot. If we write SPL, we need less.

It needs at least 4 sectors for 4 copies of FCB/DBBT

I guess the 4 copies are coming because RBL reads up to 4 copies, if no FCB is found in the first sector. If we have just two copies, RBL stops after the second one and we lose a redundancy feature.

Having 4 copies makes absolutely sense - I want just to understand it.

Quoted 4 lines

(this is 2 copies
in kbs-ng, nbare minimum) and at least 2 sectors for each copy of the actual binary if the latter fits in one sector (minimal guarantee we can write it if one of the 2 sectors is bad.)

ok, or more - I tend to say that this depends on the binary size, taking into account the reserve.

My only remark is to write here a "fix" size, that can depend on the target. We need a minimum number of sectors, and if NAND sector is 256KB everything doubles. I just suggest to drop the "recommended size", and replace that with number of sectors as you explained here. Then it is fine.

Quoted 7 lines

In real life you need some
more for NAND. Minimum/standard sector is 128K so you need at least 1MiB for a binary that fits in 128KiB (for bootrom you also need leading 1KiB padding and one trailing empty page so binary must fit in 125KiB.)

If the SPL is that small, yes, one can save some space so SPL partition can be smaller.

Right, but my concern is just to avoid to confuse user with fix rules. Anyone should check his project and understand how resources are used.

I will avoid to try to explain here all possible cases : let's remain concentrate on what the patch provides, that means adding FCB / DBBT.

Quoted 3 lines

However possible 3MiB saving on a NAND chip seem hardly worth a bother. I can make 2 commands instead of one, say ubootupdate and splupdate but that is hardly worth a bother.

No, why ? We have already explained that the RBL does not care about the payload, if the image is correctly written.

Quoted 3 lines

What if one wanted to
put a non-SPL U-Boot on his NAND later once it's been partitioned for SPL (more on partition later, see below)?

I am not yet at the point - but IMHO any developer decides how the bootloader looks for the board and if it is split into SPL and u-boot.img or simply having a u-boot.imx. And if there should be that case, it is responsibility of the board maintainer / single developer to provide enough space for it. The tool should work in all cases.

Quoted 7 lines
Quoted 3 lines

Supposed to write into mtd0 works in most cases, yes. But it should be better let it configurable as in kobs-ng.

And the reason to do it is...? What are you going to do with a copy on mtd other than 0? Remember, it is not us who decide where to boot from, it is i.MX6 bootrom.

I am not sure, but I think that the order for devices in U-Boot can be changed, meaning that nand0 could be the not booting device. I think I had a project like that in the past.

Of course, hardware decides and boots always from the same nand. But the order in software can be changed.

Quoted 3 lines

It can NOT boot off of anything but nand0. And
remember, that FCB page is not even readable with regular mtd read -- it will fail ECC...

We are talking about two different things. RBL always loads from the same device. It is hardware, it is fixed.

But even if this is a rare case, we could have, in case of multiple nand devices, a different order in U-Boot, depending which device is initialized first.

Quoted 5 lines

The only use case I could imagine would be 2 redundant NANDs with their CS lines swapped as needed by jumpers or whatever for recovery/dualboot purposes but you can use that very switching mechanism when writing U-Boot into that combination...

Understood what you mind, never heard about a project like this.

Quoted 8 lines
Quoted 4 lines

One reason is to show the user an easy path from kobs-ng to u-boot - and of course, how to change from kobs-ng to this command should be explained into the README.

There is no path at all. And nothing can be easier than just issue one command -- there are no options to match/convert, no parameters, nothing at all. What path is needed?

ok - understood. Fine, we do not need any comparison.

Quoted 4 lines

Furthermore once this command is in U-Boot 99.9999% of all users would not have to bother with it at all so they won't even know such a thing ever existed.

Who knows, users are very creative...:-D

Quoted 43 lines
Quoted 5 lines
Quoted 5 lines

Creates 4 redundant copies
of boot structures in first 4 sectors and 2 redundant copies of U-Boot in the remaining space.

Quoted 7 lines
Quoted 4 lines

Tested on a Variscite VAR-SOM-SOLO based board with i.MX6DL SoC. Will probably post that board support files later, when config is cleaned up so people could have a working NAND boot DCD portion to be able actually use those braindead SOMs.

ok - anyway, it looks like that Peng and me can test on other boards - no worry about it.

Sure.

Quoted 19 lines
Quoted 19 lines

---

arch/arm/include/asm/imx-common/mxs_nand.h new file +9 -0
new file mode 100644
index 0000000..7826a9f
@@ -0,0 +1,37 @@
+/*
+ * Copyright (C) 2015 PHYTEC Messtechnik GmbH,
+ * Author: Stefan Christ <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
Quoted 8 lines

arch/arm/include/asm/imx-common/mxs_nand.h line 15

Quoted 6 lines
arch/arm/include/asm/imx-common/mxs_nand.h new file +6 -0
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+

Copyright is ok, but we have SPDX in U-Boot. Change the license header.

Quoted 2 lines

That's an excerpt from original copyright. I can change it to SPDX, no biggie. Just wanted to include a verbatim copy of original copyright.

That's fine, thanks for that. Now we have seen where code is coming, and according to the rule it is enough you restore the Copyright removing the rest of the header.

Quoted 92 lines
Quoted 83 lines
Quoted 83 lines
arch/arm/include/asm/imx-common/mxs_nand.h new file +5 -0
+#ifndef __NAND_MXS_H
+#define __NAND_MXS_H
+
+/*
+ * Functions are definied in drivers/mtd/nand/nand_mxs.c.  They are

used to
+ * calculate the ECC Strength, BadBlockMarkerByte and BadBlockMarkerStartBit
+ * which are placed into the FCB structure. The i.MX6 ROM needs these
+ * parameters to read the firmware from NAND.
+ *
+ * The parameters depends on the pagesize and oobsize of NAND chips and are
+ * different for each combination. To avoid placing hardcoded values in the bbu
+ * update handler code, the generic calculation from the driver code is used.
+ */

arch/arm/include/asm/imx-common/mxs_nand.h new file +9 -0
+
+uint32_t mxs_nand_get_ecc_strength(uint32_t page_data_size,
+                        uint32_t page_oob_size);
+
+uint32_t mxs_nand_mark_byte_offset(struct mtd_info *mtd);
+
+uint32_t mxs_nand_mark_bit_offset(struct mtd_info *mtd);
+
+#endif /* __NAND_MXS_H */
cmd/nand.c +5 -0
index 583a18f..12e38e1 100644
@@ -38,6 +38,11 @@ int find_dev_and_part(const char *id, struct
 mtd_device **dev,
               u8 *part_num, struct part_info **part);
 #endif
 
+#ifdef CONFIG_CMD_NAND_BOOTUPDATE
+/* This comes from a separate file in drivers/mtd/nand */
+int mxs_do_nand_bootupdate(ulong addr, size_t len);
+#endif
+
 static int nand_dump(struct mtd_info *mtd, ulong off, int only_oob,
              int repeat)

{

cmd/nand.c +3 -0
@@ -373,6 +378,9 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int
 argc, char * const argv[])
     loff_t off, size, maxsize;
     char *cmd, *s;
     struct mtd_info *mtd;
+#ifdef CONFIG_CMD_NAND_BOOTUPDATE
+    size_t cnt;
+#endif
 #ifdef CONFIG_SYS_NAND_QUIET
     int quiet = CONFIG_SYS_NAND_QUIET;

#else

cmd/nand.c +21 -0
@@ -745,6 +753,48 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int
 argc, char * const argv[])
     }
 #endif
 
+#ifdef CONFIG_CMD_NAND_BOOTUPDATE
+    if (strncmp(cmd, "bootupdate", 10) == 0) {
+
+        if (argc < 3) {
+            /* All default values */
+            addr = getenv_ulong("fileaddr", 16, 0UL);
+            cnt = getenv_ulong("filesize", 16, 0UL);
+        }
+
+        if (argc == 3) {
+            /* 'addr' only, file size from environment */
+            if ((addr = simple_strtoul(argv[2], NULL, 16)) == 0UL)
+                addr = getenv_ulong("fileaddr", 16, 0UL);
+
+            cnt = getenv_ulong("filesize", 16, 0UL);
+        }
+
+        if (argc > 3) {
+            /* 'addr', 'size', and possibly more */
+            if ((addr = simple_strtoul(argv[2], NULL, 16)) == 0UL)
+                addr = getenv_ulong("fileaddr", 16, 0UL);
Quoted 8 lines

cmd/nand.c static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) · line 780

Quoted 4 lines
cmd/nand.c +4 -0
+
+            if ((cnt = simple_strtoul(argv[3], NULL, 16)) == 0UL)
+                cnt = getenv_ulong("filesize", 16, 0UL);
+        }

Why do we stick with the environment ? Most U-Boot commands do not do it. A user can always write a script as "bootupdate ${loadaddr} ${filesize}" if he wants, the command itself should be unaware of it.

Quoted 2 lines

That means you would have to _ALWAYS_ use the full form, with address and size.

On the other side, if a user do not add them and values are taken from environment, the FCB / DBBT is maybe written where the user does not want. That means it can corrupt the target.

If address / size are not taken from environment, the command simply reports an error without doing anything.

Quoted 3 lines

And the fact that address is always a hex but size shown after file load is in decimal while it is still hex for [almost] any command in U-Boot so it is very confusing and prone to user errors.

You do not understand. The issue is to avoid that code try to better understand what happens in the developer's head, making assumptions that could be wrong.

As I said, user can still use the variable, but he has to explicitely set it. For people who wants your behavior, they add a script like:

setenv bootupd 'bootupdate ${loadaddr} ${filesize}'

and they use bootupd instead of bootupdate. The issue here is to avoid to force in code a behavior. If you see in u-boot code, there are just a couple of examples doing this, and most commands are unaware about the environment, letting the user to decide what he wants.

Quoted 4 lines

And what's wrong with using default environment variable that is _ALWAYS_ set after file load? It is not something a user can set or forget to in his environment, those are "system" variables.

Let user decide - see above.

Quoted 16 lines
Quoted 16 lines

cmd/nand.c static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])

Quoted 9 lines
cmd/nand.c +7 -0
+
+
+        if (addr == 0 || cnt == 0) {
+            puts("Invalid arguments to nand bootupdate!\n");
+            return 1;
+        }
+ +        if (mxs_do_nand_bootupdate(addr, cnt)) {

Name is misleading. The function is not related to the NAND driver (mxs_nand), but it is an implementation of the layout requested by NXP's SOCs. A name related to BCB (Boot Control Block) as in manual is more correct.

Quoted 8 lines
Quoted 3 lines

So please changes all names from mxs_ in the code if they are not strictly related to the driver, and let mxs_ name just for functions exported by the driver.

I can, no problems. But it is only useful for _SOME_ Freescale (the fact NXP bought them recently doesn't change a fact those are Freescale SOCs) with MXS NAND. It might happen this would apply to other devices in the future but for now it applies nothing but those SOCs with MXS NAND.

ok, as far as I know the FCB is used also by i.MX53. This is _NOT_ mxs, and it uses a different driver, not the mxs. And we do not know the future, that means with next generation of SOCs.

I repeat: the patch here has a well known purpose. It adds FCB / DBBT block for SOCs supporting it. It is not important that the _CURRENT_ use case is i.MX6. I tcan change. The tool uses some function from the NAND driver, and if someone else in the future will add the future to a new SOC, he has to provide the same set of functionalities in the NAND driver.

Quoted 42 lines
Quoted 33 lines
Quoted 33 lines
cmd/nand.c +9 -0
+            puts("NAND bootupdate failed!\n");
+            return 1;
+        }
+
+        puts("NAND bootupdate successful\n");
+        return 0;
+    }
+#endif
+
 usage:
     return CMD_RET_USAGE;
 }
@@ -766,6 +816,17 @@ static char nand_help_text[] =
     "    'addr', skipping bad blocks and dropping any pages at the

end\n"

     "    of eraseblocks that contain only 0xFF\n"

#endif

cmd/nand.c +3 -0
+#ifdef CONFIG_CMD_NAND_BOOTUPDATE
+    "nand bootupdate - [addr] [size]\n"
+    "    write U-Boot into NAND in board/SoC specific manner creating

all\n"
+ " required headers and other bits and pieces as required for
the\n"
+ " system to be able to boot off of NAND. 'addr' is the
address\n"

cmd/nand.c +2 -0
+    "    where U-Boot image has been loaded at, 'size' is its size.\n"
+    "    If any of 'addr'/'size' is missing it is taken from

environment\n"
+ " for the last file loaded. U-Boot image must be of a proper
type\n"
+ " for the target platform (only IMX image supported at the
moment)\n"

Quoted 8 lines
Quoted 6 lines
cmd/nand.c +2 -0
+    "    binary without U-Boot image headers (e.g. u-boot.imx file.)\n"
+#endif
     "nand erase[.spread] [clean] off size - erase 'size' bytes "
     "from offset 'off'\n"
     "    With '.spread', erase enough for given file size,

otherwise,\n"

A README file in doc/ is also useful.

Quoted 3 lines

Sure. But not everything at once -- it is the first, initial submission that can be changed/ammended/betterized later on if needed. One has to start somewhere, right?

That is ok - I have written that a README is useful, and if you want to add it, it is appreciated.

Quoted 15 lines
Quoted 14 lines

drivers/mtd/nand/Makefile obj-$(CONFIG_TEGRA_NAND) += tegra_nand.o · line 69

Quoted 10 lines
drivers/mtd/nand/Makefile +1 -0
index 837d397..36609e1 100644
@@ -66,6 +66,7 @@ obj-$(CONFIG_TEGRA_NAND) += tegra_nand.o
 obj-$(CONFIG_NAND_OMAP_GPMC) += omap_gpmc.o
 obj-$(CONFIG_NAND_OMAP_ELM) += omap_elm.o
 obj-$(CONFIG_NAND_PLAT) += nand_plat.o
+obj-$(CONFIG_CMD_NAND_BOOTUPDATE) += mxs_nand_bootupdate.o

It will be nice to set some dependencies between CONFIG_NAND_MXS and CONFIG_CMD_NAND_BOOTUPDATE. CONFIG_CMD_NAND_BOOTUPDATE is not useful if CONFIG_NAND_MXS is not set, right ?

Quoted 4 lines

Yep. As well as e.g. UBI is not useful without RBTREE/LZO but you'll have to choose those yourself. And NAND_MXS also requires APHB_DMA and some other stuff. That, in turn, require some other stuff and it goes ad infinitum.

Sorry, my comment was not complete. I try to explain better.

You add with the patch a new command, but this will be set in the old fashion way - without Kconfig.
If this is put in cmd/Kconfig, a dependency can be created. The was chosen by U-Boot for the future is with Kconfig, that means new commands must folow the new rule.

Quoted 4 lines

BTW, the fact it is not useful without NAND_MXS is the reason why I used "mxs-" in source/functions file names. Do you still want me to change it?

Yes, I have explained before why. That just means that in Kconfig the command has a "depend on NAND_MXS" now, in future who knows.

Quoted 60 lines
Quoted 49 lines
Quoted 44 lines
drivers/mtd/nand/Makefile +0 -0
 else  # minimal SPL drivers
 
drivers/mtd/nand/mxs_nand.c +2 -1
index 7be1f86..afeec22 100644
@@ -26,6 +26,7 @@
 #include <asm/imx-common/regs-gpmi.h>
 #include <asm/arch/sys_proto.h>
 #include <asm/imx-common/dma.h>
+#include <asm/imx-common/mxs_nand.h>
 
 #define    MXS_NAND_DMA_DESCRIPTOR_COUNT        4
 
@@ -145,7 +146,7 @@ static uint32_t mxs_nand_aux_status_offset(void)
     return (MXS_NAND_METADATA_SIZE + 0x3) & ~0x3;
 }
 
-static inline uint32_t mxs_nand_get_ecc_strength(uint32_t
 page_data_size,
+uint32_t mxs_nand_get_ecc_strength(uint32_t page_data_size,
                         uint32_t page_oob_size)
 {
     int ecc_strength;
drivers/mtd/nand/mxs_nand.c +2 -2
@@ -221,14 +222,14 @@ static inline uint32_t
 mxs_nand_get_mark_offset(uint32_t page_data_size,
     return block_mark_bit_offset;
 }
 
-static uint32_t mxs_nand_mark_byte_offset(struct mtd_info *mtd)
+uint32_t mxs_nand_mark_byte_offset(struct mtd_info *mtd)
 {
     uint32_t ecc_strength;
     ecc_strength = mxs_nand_get_ecc_strength(mtd->writesize,
 mtd->oobsize);
     return mxs_nand_get_mark_offset(mtd->writesize, ecc_strength) >> 3;
 }
 
-static uint32_t mxs_nand_mark_bit_offset(struct mtd_info *mtd)
+uint32_t mxs_nand_mark_bit_offset(struct mtd_info *mtd)
 {
     uint32_t ecc_strength;
     ecc_strength = mxs_nand_get_ecc_strength(mtd->writesize,
Quoted 1 line

mtd->oobsize);

Added Scott as MTD maintainer: it is clear to me why, but the driver exports its functions via the struct nand_chip (see board_nand_init).

Quoted 2 lines

Is it not better to extend the structure with the functions you want to export ?

It might be. I don't care how I access those functions but it is probably up to them to extend the structure...

BTW it is _EXACTLY_ how it is done in barebox. They started with putting all that code into their source but then realized it is already there, just as a file scope in another file so they simply exported it to avoid code duplication.

Quoted 4 lines

If MTD guys decided to add it to their structure I will happily take it from there. That would make it impossible to base it on a current master until such structure change is actually applied but there is no rush here; I can wait :)

Waiting from Scott's answer...

Quoted 10 lines
Quoted 6 lines

drivers/mtd/nand/mxs_nand_bootupdate.c

Quoted 3 lines
drivers/mtd/nand/mxs_nand_bootupdate.c +0 -0

I do not know if this file should be here or should be in imx-common. It is relevant just for i.MX and not for other SOCs.

I do not either :) I considered both places and the current one had been chosen with a coin toss :) If you want me to put it in imx-common I will do it, no problems. Just tell me to.

Agree. But there are also a competence area. If put in mtd/nand, Scott has MTD maintainer has to take care of each patch regarding it, but this feature is strictly related to some i.MXes. For this reason, I propose to put it in imx-common.

Quoted 7 lines
Quoted 3 lines

In barebox it has a better name : imx-bbu-nand-fcb.c. It says much more as mxs_nand_bootupdate.c.

It it not. The "bbu" part stays for "BareBox Update" and we are not barebox, we are U-Boot. If you can come up with better file name please tell me what it is and I will rename it. I don't care what it is called.

ok, I express wrongly my thoughts. I mean: the file codes the FCB / DBBT into the nand. Else a generic nand_boot_update, I suggest for correctness something like imx-fcb-dbbt.c (or something in that direction). I mean, just a way to understand that the code refers to FCB / DBBT. Choose then an appropriated name.

Quoted 38 lines
Quoted 38 lines
Quoted 25 lines

new file mode 100644
index 0000000..9442553

drivers/mtd/nand/mxs_nand_bootupdate.c new file +17 -0
@@ -0,0 +1,556 @@
+/*
+ * mxs_nand_bootupdate.c - write U-Boot to MXS NAND to make it bootable
+ *
+ * Copyright (c) 2016 Sergey Kubushyn <[email protected]>
+ *
+ * Most of the source shamelesly stolen from barebox.
+ *
+ * Here is the original copyright:
+ *
+ *=== Cut ===
+ * Copyright (C) 2014 Sascha Hauer, Pengutronix
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *

drivers/mtd/nand/mxs_nand_bootupdate.c line 27

Quoted 10 lines
drivers/mtd/nand/mxs_nand_bootupdate.c new file +10 -0
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation.
+ *=== Cut ===
+ */

Let Copyright and add SPDX header.

Quoted 5 lines
Quoted 3 lines

As you ported code from another Open Spurce project, it will be nice if you write this into the commit message, writing also from which barebox commit you took the code, exactly how we do when we port code from Linux.

No problems, just wanted to keep a verbatim copy of original copyright...

That's fine - now it is clear, and you can switch to SPDX.

Quoted 26 lines
Quoted 17 lines
Quoted 17 lines
drivers/mtd/nand/mxs_nand_bootupdate.c new file +16 -0
+
+
+#include <common.h>
+#include <linux/sizes.h>
+#include <linux/mtd/mtd.h>
+#include <linux/compat.h>
+#include <command.h>
+#include <console.h>
+#include <malloc.h>
+#include <asm/byteorder.h>
+#include <jffs2/jffs2.h>
+#include <nand.h>
+#include <errno.h>
+#include <asm/imx-common/mxs_nand.h>
+#include <asm/imx-common/imximage.cfg>
+
Quoted 8 lines

drivers/mtd/nand/mxs_nand_bootupdate.c line 47

Quoted 4 lines
drivers/mtd/nand/mxs_nand_bootupdate.c new file +4 -0
+
+#ifndef CONFIG_CMD_MTDPARTS
+#error "CONFIG_CMD_MTDPARTS is not set, mtd partition support missing"
+#endif

I am missing because this is important. U-Boot works with addresses, and not with MTDs. The whole nand utilities take an address as parameter. MTD is optional, and can be used or not in nand erase/read/write.

Quoted 5 lines

Sure but it makes life more difficult. The update needs to know what space it has to plan accordingly and calculate where to put things. That range is also used for DBBT scan -- it doesn't make sense to go beyond that partition (or region) when creating DBBT because it is of now other use, just for bootrom NAND boot.

It can be the opposite. We stated that 4 copies are written, that means that the space is calculated and cannot be changed. The command can output the total size used, and it is duty of the board developer to adjust the size of MTDs if they are wrong.

This way means the user must allocate enough space without knowing exactly how much is necessary.

Quoted 5 lines

Then there is other reason for this. If one has NAND it would've been illogical to just put U-Boot there as Variscite guys do. The remaining space can host quite sizable FS on it. The only one currently supported is UBIFS and UBI requires a partition.

I do not get the point. Sure, it makes no sense to use raw NAND without UBI. But u-boot is not in a UBI container.

It makes absolutely sense for safety to have _NO_ partition for u-boot. The kernel does not find it, and the first MTD partition MTD0 starts after u-boot. There is no *normal* way for user space to write into u-boot space - even better as marking the partition as read-only.

Quoted 3 lines

Once we have partition anyway it
raises a question why not to use a partition instead of address range for U-Boot?

Because there are use cases without MTD partition for U-Boot.

Quoted 159 lines
Quoted 150 lines
Quoted 150 lines
drivers/mtd/nand/mxs_nand_bootupdate.c new file +6 -0
+
+static const char *uboot_tgt = "nand0,0";
+
+/* partition handling routines */
+int mtdparts_init(void);
+int id_parse(const char *id, const char **ret_id, u8 *dev_type, u8

*dev_num);

drivers/mtd/nand/mxs_nand_bootupdate.c new file +25 -0
+int find_dev_and_part(const char *id, struct mtd_device **dev,
+              u8 *part_num, struct part_info **part);
+
+struct dbbt_block {
+    uint32_t Checksum;    /* reserved on i.MX6 */
+    uint32_t FingerPrint;
+    uint32_t Version;
+    uint32_t numberBB;    /* reserved on i.MX6 */
+    uint32_t DBBTNumOfPages;
+};
+
+struct fcb_block {
+    uint32_t Checksum;        /* First fingerprint in first byte */
+    uint32_t FingerPrint;        /* 2nd fingerprint at byte 4 */
+    uint32_t Version;        /* 3rd fingerprint at byte 8 */
+    uint8_t DataSetup;
+    uint8_t DataHold;
+    uint8_t AddressSetup;
+    uint8_t DSAMPLE_TIME;
+    /* These are for application use only and not for ROM. */
+    uint8_t NandTimingState;
+    uint8_t REA;
+    uint8_t RLOH;
+    uint8_t RHOH;
+    uint32_t PageDataSize;        /* 2048 for 2K pages, 4096 for 4K

pages */
+ uint32_t TotalPageSize; /* 2112 for 2K pages, 4314 for 4K
pages */

drivers/mtd/nand/mxs_nand_bootupdate.c new file +2 -0
+    uint32_t SectorsPerBlock;    /* Number of 2K sections per block */
+    uint32_t NumberOfNANDs;        /* Total Number of NANDs - not used

by ROM */
+ uint32_t TotalInternalDie; /* Number of separate chips in this
NAND */

drivers/mtd/nand/mxs_nand_bootupdate.c new file +2 -0
+    uint32_t CellType;        /* MLC or SLC */
+    uint32_t EccBlockNEccType;    /* Type of ECC, can be one of

BCH-0-20 */
+ uint32_t EccBlock0Size; /* Number of bytes for Block0 -
BCH */
+ uint32_t EccBlockNSize; /* Block size in bytes for all
blocks other than Block0 - BCH */

drivers/mtd/nand/mxs_nand_bootupdate.c new file +3 -0
+    uint32_t EccBlock0EccType;    /* Ecc level for Block 0 - BCH */
+    uint32_t MetadataBytes;        /* Metadata size - BCH */
+    uint32_t NumEccBlocksPerPage;    /* Number of blocks per page for

ROM use - BCH */
+ uint32_t EccBlockNEccLevelSDK; /* Type of ECC, can be one of
BCH-0-20 */
+ uint32_t EccBlock0SizeSDK; /* Number of bytes for Block0 -
BCH */
+ uint32_t EccBlockNSizeSDK; /* Block size in bytes for all blocks
other than Block0 - BCH */

drivers/mtd/nand/mxs_nand_bootupdate.c new file +2 -0
+    uint32_t EccBlock0EccLevelSDK;    /* Ecc level for Block 0 - BCH */
+    uint32_t NumEccBlocksPerPageSDK;/* Number of blocks per page for

SDK use - BCH */

drivers/mtd/nand/mxs_nand_bootupdate.c new file +3 -0
+    uint32_t MetadataBytesSDK;    /* Metadata size - BCH */
+    uint32_t EraseThreshold;    /* To set into BCH_MODE register */
+    uint32_t BootPatch;        /* 0 for normal boot and 1 to load patch

starting next to FCB */

drivers/mtd/nand/mxs_nand_bootupdate.c new file +2 -0
+    uint32_t PatchSectors;        /* Size of patch in sectors */
+    uint32_t Firmware1_startingPage;/* Firmware image starts on this

sector */
+ uint32_t Firmware2_startingPage;/* Secondary FW Image starting
Sector */
+ uint32_t PagesInFirmware1; /* Number of sectors in firmware
image */
+ uint32_t PagesInFirmware2; /* Number of sector in secondary FW
image */
+ uint32_t DBBTSearchAreaStartAddress; /* Page address where dbbt
search area begins */
+ uint32_t BadBlockMarkerByte; /* Byte in page data that have
manufacturer marked bad block marker, */
+ /* this will be swapped with metadata[0] to
complete page data. */
+ uint32_t BadBlockMarkerStartBit;/* For BCH ECC sizes other than 8
and 16 the bad block marker does not */
+ /* start at 0th bit of BadBlockMarkerByte. This
field is used to get to */
+ /* the start bit of bad block marker byte with in
BadBlockMarkerByte */
+ uint32_t BBMarkerPhysicalOffset;/* FCB value that gives byte offset
for bad block marker on physical NAND page */

drivers/mtd/nand/mxs_nand_bootupdate.c new file +13 -0
+    uint32_t BCHType;
+
+    uint32_t TMTiming2_ReadLatency;
+    uint32_t TMTiming2_PreambleDelay;
+    uint32_t TMTiming2_CEDelay;
+    uint32_t TMTiming2_PostambleDelay;
+    uint32_t TMTiming2_CmdAddPause;
+    uint32_t TMTiming2_DataPause;
+    uint32_t TMSpeed;
+    uint32_t TMTiming1_BusyTimeout;
+
+    uint32_t DISBBM;    /* the flag to enable (1)/disable(0) bi swap */
+    uint32_t BBMarkerPhysicalOffsetInSpareData; /* The swap position of

main area in spare area */

drivers/mtd/nand/mxs_nand_bootupdate.c new file +19 -0
+};
+
+struct fw_write_data {
+    int    fw1_blk;
+    int    fw2_blk;
+    int    part_blks;
+    void    *buf;
+    size_t    len;
+};
+
+#define BF_VAL(v, bf)    (((v) & bf##_MASK) >> bf##_OFFSET)
+#define GETBIT(v,n)    (((v) >> (n)) & 0x1)
+
+
+static uint8_t calculate_parity_13_8(uint8_t d)
+{
+    uint8_t    p = 0;
+
+    p |= (GETBIT(d, 6) ^ GETBIT(d, 5) ^ GETBIT(d, 3) ^ GETBIT(d,

2)) << 0;
+ p |= (GETBIT(d, 7) ^ GETBIT(d, 5) ^ GETBIT(d, 4) ^ GETBIT(d, 2) ^
GETBIT(d, 1)) << 1;
+ p |= (GETBIT(d, 7) ^ GETBIT(d, 6) ^ GETBIT(d, 5) ^ GETBIT(d, 1) ^
GETBIT(d, 0)) << 2;
+ p |= (GETBIT(d, 7) ^ GETBIT(d, 4) ^ GETBIT(d, 3) ^ GETBIT(d,
0)) << 3;
+ p |= (GETBIT(d, 6) ^ GETBIT(d, 4) ^ GETBIT(d, 3) ^ GETBIT(d, 2) ^
GETBIT(d, 1) ^ GETBIT(d, 0)) << 4;

drivers/mtd/nand/mxs_nand_bootupdate.c new file +20 -0
+    return p;
+}
+
+static void encode_hamming_13_8(void *_src, void *_ecc, size_t size)
+{
+    int    i;
+    uint8_t    *src = _src;
+    uint8_t    *ecc = _ecc;
+
+    for (i = 0; i < size; i++)
+        ecc[i] = calculate_parity_13_8(src[i]);
+}
+
+static uint32_t calc_chksum(void *buf, size_t size)
+{
+    u32    chksum = 0;
+    u8    *bp = buf;
+    size_t    i;
+
+    for (i = 0; i < size; i++)
Quoted 8 lines

drivers/mtd/nand/mxs_nand_bootupdate.c line 169

Quoted 4 lines
drivers/mtd/nand/mxs_nand_bootupdate.c new file +4 -0
+        chksum += bp[i];
+
+    return ~chksum;
+}

I understand that it is for debug only - but you have removed debug functions from barebox (dump_fcb for example). Can they be useful here, too ?

Quoted 2 lines

They were useful for debugging, to show what we put into that FCB. Once we are sure it is working correctly there is nothing more to debug.

Fine if you do not want to add them back - anywyay, you know that software is a life system. It is working now, but we do not know what happens in future due to some other changes (in the MTD driver, maybe ?). Maybe it does not work anymore, and we need the debug back.

Quoted 201 lines
Quoted 200 lines
Quoted 186 lines
drivers/mtd/nand/mxs_nand_bootupdate.c new file +3 -0
+
+
+static ssize_t raw_write_page(struct mtd_info *mtd, void *buf, loff_t

offset)

drivers/mtd/nand/mxs_nand_bootupdate.c new file +24 -0
+{
+    struct mtd_oob_ops ops;
+    ssize_t ret;
+
+    ops.mode = MTD_OPS_RAW;
+    ops.ooboffs = 0;
+    ops.datbuf = buf;
+    ops.len = mtd->writesize;
+    ops.oobbuf = buf + mtd->writesize;
+    ops.ooblen = mtd->oobsize;
+    ret = mtd_write_oob(mtd, offset, &ops);
+
+        return ret;
+}
+
+static int fcb_create(struct fcb_block *fcb, struct mtd_info *mtd)
+{
+    fcb->FingerPrint = 0x20424346;
+    fcb->Version = 0x01000000;
+    fcb->PageDataSize = mtd->writesize;
+    fcb->TotalPageSize = mtd->writesize + mtd->oobsize;
+    fcb->SectorsPerBlock = mtd->erasesize / mtd->writesize;
+
+    /* Divide ECC strength by two and save the value into FCB

structure. */

drivers/mtd/nand/mxs_nand_bootupdate.c new file +31 -0
+    fcb->EccBlock0EccType =
+        mxs_nand_get_ecc_strength(mtd->writesize, mtd->oobsize) >> 1;
+
+    fcb->EccBlockNEccType = fcb->EccBlock0EccType;
+
+    /* Also hardcoded in kobs-ng */
+    fcb->EccBlock0Size = 0x00000200;
+    fcb->EccBlockNSize = 0x00000200;
+    fcb->DataSetup = 80;
+    fcb->DataHold = 60;
+    fcb->AddressSetup = 25;
+    fcb->DSAMPLE_TIME = 6;
+    fcb->MetadataBytes = 10;
+
+    /* DBBT search area starts at second page on first block */
+    fcb->DBBTSearchAreaStartAddress = 1;
+
+    fcb->BadBlockMarkerByte = mxs_nand_mark_byte_offset(mtd);
+    fcb->BadBlockMarkerStartBit = mxs_nand_mark_bit_offset(mtd);
+
+    fcb->BBMarkerPhysicalOffset = mtd->writesize;
+
+    fcb->NumEccBlocksPerPage = mtd->writesize / fcb->EccBlock0Size - 1;
+
+    fcb->Checksum = calc_chksum((void *)fcb + 4, sizeof(*fcb) - 4);
+
+    return 0;
+}
+
+/* Erase entire U-Boot partition */
+static int mxs_nand_uboot_erase(struct mtd_info *mtd, struct part_info

*part)

drivers/mtd/nand/mxs_nand_bootupdate.c new file +32 -0
+{
+    uint64_t        offset = 0;
+    struct erase_info    erase;
+    int            len = part->size;
+    int            ret;
+
+    while (len > 0) {
+        pr_debug("erasing at 0x%08llx\n", offset);
+        if (mtd_block_isbad(mtd, offset)) {
+            pr_debug("erase skip block @ 0x%08llx\n", offset);
+            offset += mtd->erasesize;
+            continue;
+        }
+
+        memset(&erase, 0, sizeof(erase));
+        erase.addr = offset;
+        erase.len = mtd->erasesize;
+
+        ret = mtd_erase(mtd, &erase);
+        if (ret)
+            return ret;
+
+        offset += mtd->erasesize;
+        len -= mtd->erasesize;
+    }
+
+    return 0;
+}
+
+/* Write the U-Boot proper (2 copies) to where it belongs.   */
+/* This is U-Boot binary image itself, no FCB/DBBT here yet. */
+static int mxs_nand_uboot_write_fw(struct mtd_info *mtd, struct

fw_write_data *fw)

drivers/mtd/nand/mxs_nand_bootupdate.c new file +75 -0
+{
+    uint64_t    offset;
+    int        ret;
+    int        blk;
+    size_t        dummy;
+    size_t        bytes_left;
+    int        chunk;
+    void        *p;
+
+    bytes_left = fw->len;
+    p = fw->buf;
+    blk = fw->fw1_blk;
+    offset = blk * mtd->erasesize;
+
+    while (bytes_left > 0) {
+        chunk = min(bytes_left, mtd->erasesize);
+
+        pr_debug("fw1: writing %p at 0x%08llx, left 0x%08x\n",
+                p, offset, bytes_left);
+
+        if (mtd_block_isbad(mtd, offset)) {
+            pr_debug("fw1: write skip block @ 0x%08llx\n", offset);
+            offset += mtd->erasesize;
+            blk++;
+            continue;
+        }
+
+        if (blk >= fw->fw2_blk)
+            return -ENOSPC;
+
+        ret = mtd_write(mtd, offset, chunk, &dummy, p);
+        if (ret)
+            return ret;
+
+        offset += chunk;
+        bytes_left -= chunk;
+        p += chunk;
+        blk++;
+    }
+
+    bytes_left = fw->len;
+    p = fw->buf;
+    blk = fw->fw2_blk;
+    offset = blk * mtd->erasesize;
+
+    while (bytes_left > 0) {
+        chunk = min(bytes_left, mtd->erasesize);
+
+        pr_debug("fw2: writing %p at 0x%08llx, left 0x%08x\n",
+                p, offset, bytes_left);
+
+        if (mtd_block_isbad(mtd, offset)) {
+            pr_debug("fw2: write skip block @ 0x%08llx\n", offset);
+            offset += mtd->erasesize;
+            blk++;
+            continue;
+        }
+
+        if (blk >= fw->part_blks)
+            return -ENOSPC;
+
+        ret = mtd_write(mtd, offset, chunk, &dummy, p);
+        if (ret)
+            return ret;
+
+        offset += chunk;
+        bytes_left -= chunk;
+        p += chunk;
+        blk++;
+    }
+
+    return 0;
+}
+
+static int dbbt_data_create(struct mtd_info *mtd, void *buf, int

num_blocks)

drivers/mtd/nand/mxs_nand_bootupdate.c new file +15 -0
+{
+    int        n;
+    int        n_bad_blocks = 0;
+    uint32_t    *bb = buf + 0x8;
+    uint32_t    *n_bad_blocksp = buf + 0x4;
+
+    for (n = 0; n < num_blocks; n++) {
+        loff_t offset = n * mtd->erasesize;
+        if (mtd_block_isbad(mtd, offset)) {
+            n_bad_blocks++;
+            *bb = n;
+            bb++;
+        }
+    }
+

drivers/mtd/nand/mxs_nand_bootupdate.c line 359

Quoted 10 lines
drivers/mtd/nand/mxs_nand_bootupdate.c new file +10 -0
+    *n_bad_blocksp = n_bad_blocks;
+
+    return n_bad_blocks;
+}
+
+/********************************************************************/
+/* This is where it is all done. Takes pointer to a U-Boot image in */
+/* RAM and image size, creates FCB/DBBT and writes everything where */
+/* it belongs into NAND. Image must be an IMX image built for NAND. */
+/********************************************************************/

Multiline comment, please fix it.

Quoted 1 line

+static int mxs_nand_uboot_update(const void *img, size_t len)

Quoted 8 lines
Quoted 2 lines

I would prefer you let the function names as in original code, so we know where they are coming from (imx_bbu_nand_update here).

As I already said "bbu" stands for "BareBox Update". Do we have to keep it in U-Boot?

If you have a better idea for function mane please go ahead and tell what it is. I don't care what the name is; even xyz123456 would be OK.

See above

Quoted 238 lines
Quoted 38 lines
Quoted 38 lines
drivers/mtd/nand/mxs_nand_bootupdate.c new file +36 -0
+{
+    int             i, ret;
+
+    size_t            dummy;
+    loff_t            offset = 0;
+
+    void             *fcb_raw_page;
+    void            *dbbt_page;
+    void            *dbbt_data_page;
+    void            *ecc;
+
+    uint32_t         num_blocks_fcb_dbbt;
+    uint32_t        num_blocks_fw;
+
+    struct mtd_info        *mtd;
+    struct fcb_block    *fcb;
+    struct dbbt_block    *dbbt;
+
+    struct mtd_device    *dev;
+    struct part_info    *part;
+    u8            pnum;
+
+    struct fw_write_data    fw;
+
+    if ((mtdparts_init() == 0) &&
+            (find_dev_and_part(uboot_tgt, &dev, &pnum, &part) == 0)) {
+        if (dev->id->type != MTD_DEV_TYPE_NAND) {
+            puts("Not a NAND device\n");
+            return -ENODEV;
+        }
+    }
+
+    nand_curr_device = dev->id->num;
+
+#ifdef CONFIG_SYS_NAND_SELECT_DEVICE
+    board_nand_select_device(nand_info[nand_curr_device].priv,

nand_curr_device);

Quoted 8 lines
Quoted 6 lines
drivers/mtd/nand/mxs_nand_bootupdate.c new file +5 -0
+#endif
+
+    /* Get a pointer to mtd_info for selected device */
+
+    mtd = get_mtd_device_nm("nand0");    /* We always boot off of

nand0 */

...and if I have nand1 ?

So keep it for yourself :) i.MX6 bootrom won't boot off of it so no reason to put a bootloader there.

Quoted 1 line

As I said, U-Boot works with addresses

And what?

Quoted 12 lines
Quoted 11 lines

+ + if (IS_ERR(mtd)) {

drivers/mtd/nand/mxs_nand_bootupdate.c new file +7 -0
+        /* Should not happen */
+        puts("No nand0 device...\n");
+        return -ENODEV;
+    }
+ +    put_mtd_device(mtd);
+
+    /* Quick and dirty check if we have 2Mbytes of good blocks in

nand0,0 */
+ /* Not sure if it is needed at all but won't hurt so here it
is... */

Quoted 1 line

Yes, it hurts.

I can remove it, no problems.

Quoted 26 lines
Quoted 4 lines

+ + i = 0;
+ offset = 0; /* It is the first partition so it starts at
block 0 */

drivers/mtd/nand/mxs_nand_bootupdate.c new file +1 -0
+

drivers/mtd/nand/mxs_nand_bootupdate.c line 427

Quoted 10 lines
drivers/mtd/nand/mxs_nand_bootupdate.c new file +10 -0
+    while (offset < part->size) {
+        if (!mtd_block_isbad(mtd, offset)) {
+            i += mtd->erasesize;
+        }
+        offset += mtd->erasesize;
+    }
+ +    if (i < SZ_2M) {
+        puts("Partition too small for U-Boot!\n");
+        return -EINVAL;
+    }

I try to understand your changes from barebox code - something are clear due to the differences between the two bootloaders, some other ones not.

drivers/mtd/nand/mxs_nand_bootupdate.c line 434

Quoted 7 lines
drivers/mtd/nand/mxs_nand_bootupdate.c new file +7 -0
+
+    /* We will use 4 first blocks for FCB/DBBT copies. */
+    /* The rest of partition is split in half and used */
+    /* for two U-Boot copies. We don't care if those   */
+    /* start on good or bad block - bad blocks will be */
+    /* skipped on write, ROM boot code will also skip  */
+    /* bad blocks on bootup when loading U-Boot image. */
Quoted 2 lines

Multiline comment is wrong. I think the original comment in bareboc is much more clear.

OK, no problems, will steal that.

Quoted 7 lines
Quoted 7 lines
drivers/mtd/nand/mxs_nand_bootupdate.c new file +7 -0
+
+    fw.part_blks = part->size / mtd->erasesize;
+    num_blocks_fcb_dbbt = 4;
+    num_blocks_fw = (fw.part_blks - num_blocks_fcb_dbbt) / 2;
+    fw.fw1_blk = num_blocks_fcb_dbbt;
+    fw.fw2_blk = fw.fw1_blk + num_blocks_fw;
+
Quoted 8 lines

drivers/mtd/nand/mxs_nand_bootupdate.c line 447

Quoted 6 lines
drivers/mtd/nand/mxs_nand_bootupdate.c new file +6 -0
+    /* OK, now create FCB structure for bootROM NAND boot */
+
+    fcb_raw_page = kzalloc(mtd->writesize + mtd->oobsize, GFP_KERNEL);
+
+    fcb = fcb_raw_page + 12;
+    ecc = fcb_raw_page + 512 + 12;

??

!! :) It is how the FCB page is laid out and it is what makes it fail regular BCH ECC check. It is what i.MX6 bootrom expects.

FCB page is read in the raw. And written too.

Quoted 116 lines
Quoted 116 lines
drivers/mtd/nand/mxs_nand_bootupdate.c new file +9 -0
+
+    dbbt_page = kzalloc(mtd->writesize, GFP_KERNEL);
+    dbbt_data_page = kzalloc(mtd->writesize, GFP_KERNEL);
+    dbbt = dbbt_page;
+
+    /* Write one additional page to make the ROM happy. */
+    /* Maybe the PagesInFirmwarex fields are really the */
+    /* number of pages - 1. kobs-ng does the same.      */
+ +    fw.len = ALIGN(len + FLASH_OFFSET_STANDARD + mtd->writesize,

mtd->writesize);

drivers/mtd/nand/mxs_nand_bootupdate.c new file +17 -0
+    fw.buf = kzalloc(fw.len, GFP_KERNEL);
+    memcpy(fw.buf + FLASH_OFFSET_STANDARD, img, len);
+
+    /* Erase entire partition */
+    ret = mxs_nand_uboot_erase(mtd, part);
+    if (ret)
+        goto out;
+
+    /* Now write 2 copies of the U-Boot proper to where they belong. */
+    /* Headers (FCB, DBBT) will be generated and written after that. */
+    ret = mxs_nand_uboot_write_fw(mtd, &fw);
+    if (ret < 0)
+        goto out;
+
+    /* Create FCB, calculate ECC (we don't/can't use hardware ECC */
+    /* here so we do it ourselves and then write _RAW_ pages.     */
+ +    fcb->Firmware1_startingPage = fw.fw1_blk * mtd->erasesize /

mtd->writesize;
+ fcb->Firmware2_startingPage = fw.fw2_blk * mtd->erasesize /
mtd->writesize;

drivers/mtd/nand/mxs_nand_bootupdate.c new file +2 -0
+    fcb->PagesInFirmware1 =
+        ALIGN(len + FLASH_OFFSET_STANDARD, mtd->writesize) /

mtd->writesize;

drivers/mtd/nand/mxs_nand_bootupdate.c new file +7 -0
+    fcb->PagesInFirmware2 = fcb->PagesInFirmware1;
+
+    fcb_create(fcb, mtd);
+    encode_hamming_13_8(fcb, ecc, 512);
+
+    /*
+     * Set the first and second byte of OOB data to 0xFF, not 0x00.

These
+ * bytes are used as the Manufacturers Bad Block Marker (MBBM).
Since
+ * the FCB is mostly written to the first page in a block, a
scan for

drivers/mtd/nand/mxs_nand_bootupdate.c new file +2 -0
+     * factory bad blocks will detect these blocks as bad, e.g. when
+     * function nand_scan_bbt() is executed to build a new bad block

table.
+ * We will _NOT_ mark a bad block as good -- we skip the bad
blocks.

drivers/mtd/nand/mxs_nand_bootupdate.c new file +9 -0
+     */
+    memset(fcb_raw_page + mtd->writesize, 0xff, 2);
+
+    /* Now create DBBT */
+    dbbt->Checksum = 0;
+    dbbt->FingerPrint = 0x54424244;
+    dbbt->Version = 0x01000000;
+
+    if ((ret = dbbt_data_create(mtd, dbbt_data_page, fw.part_blks))

< 0)

drivers/mtd/nand/mxs_nand_bootupdate.c new file +35 -0
+        goto out;
+
+    if (ret > 0)
+        dbbt->DBBTNumOfPages = 1;
+
+    offset = 0;
+
+    if (mtd_block_isbad(mtd, offset)) {
+        puts("Block 0 is bad, NAND unusable\n");
+        ret = -EIO;
+        goto out;
+    }
+
+    /* Write FCB/DBBT to first 4 blocks. Skip bad blocks if any. */
+    /* Less than 4 copies will be written if there were BBs !!!  */
+    for (i = 0; i < 4; i++) {
+
+        if (mtd_block_isbad(mtd, offset)) {
+            pr_err("Block %d is bad, skipped\n", i);
+            continue;
+        }
+
+
+        ret = raw_write_page(mtd, fcb_raw_page, mtd->erasesize * i);
+        if (ret)
+            goto out;
+
+        ret = mtd_write(mtd, mtd->erasesize * i + mtd->writesize,
+                mtd->writesize, &dummy, dbbt_page);
+        if (ret)
+            goto out;
+
+        /* DBBTNumOfPages == 0 if no bad blocks */
+        if (dbbt->DBBTNumOfPages > 0) {
+            ret = mtd_write(mtd, mtd->erasesize * i + mtd->writesize

* 5,

drivers/mtd/nand/mxs_nand_bootupdate.c new file +20 -0
+                    mtd->writesize, &dummy, dbbt_data_page);
+            if (ret)
+                goto out;
+        }
+    }
+
+out:
+    kfree(dbbt_page);
+    kfree(dbbt_data_page);
+    kfree(fcb_raw_page);
+    kfree(fw.buf);
+
+    return ret;
+}
+
+
+int mxs_do_nand_bootupdate(ulong addr, size_t len)
+{
+    /* KSI: Unlock NAND first if it is locked... */
+
Quoted 8 lines
Quoted 2 lines

drivers/mtd/nand/mxs_nand_bootupdate.c line 556

Quoted 2 lines
drivers/mtd/nand/mxs_nand_bootupdate.c new file +2 -0
+    return mxs_nand_uboot_update((const void *)addr, len);
+}

---
******************************************************************
* KSI at home KOI8 Net < > The impossible we do immediately. *
* Las Vegas NV, USA < > Miracles require 24-hour notice. *
******************************************************************

Best regards,
Stefano Babic

Signature · 5 lines
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================
#5 lore raw

PATCH 1/1 i.MX6 nand bootupdate

In reply to Stefano Babic

Hi Stefano,

Quoted 15 lines · Stefano Babic

Hi Sergey,

Quoted 3 lines · Sergey Kubushyn

Here is the initial support for writing i.MX6 NAND U-Boot into NAND with all FCB and DBBT stuff as required.

This is a very interesting feature missing in U-Boot. Up now we are constrained to update the bootloader from user space with kobs-ng, and this is really an improvement. Thanks for that.

Anyway, as remarked by Peng, please rebase your patch on current tree, else we cannot test it.

Quoted 6 lines
Quoted 3 lines

Just build U-Boot for NAND as u-boot.imx, load it somehow in RAM and say "nand bootupdate [addr] [size]". It will create all necessary structures and write everything as needed.

The whole phrase is misleading here. I guess this is the way you have to put on your board, it is not a general case.

General case is boot ROM specific, see bellow...

Quoted 9 lines

I am expecting to have some parameters like in kobs-ng: I set the number of copies with --search_exponent, how is it done here ?

Quoted 2 lines

No SPL required, just plain single stage U-Boot.

Why ? This has nothing to do. We could write SPL into the NAND as first bootloader, and SPL will load the u-boot.img unabh?ngig von FCB.

Quoted 6 lines
Quoted 2 lines

Supposed to write into
first NAND partition, recommended size 4MiB.

Where is this size coming ? This depends on the number of copies (can we have configurable as for kobs-ng ?) and the size of U-Boot. If we write SPL, we need less.

What boot ROM expects is boot ROM specific. There's no way to change that, so there's no point to make that anyhow configurable. Note that sunxi or TI DM3730 needs something similar, so I send this patchset:
https://patchwork.ozlabs.org/patch/630451/
https://patchwork.ozlabs.org/patch/630452/
https://patchwork.ozlabs.org/patch/630453/
https://patchwork.ozlabs.org/patch/630454/
https://patchwork.ozlabs.org/patch/630456/
https://patchwork.ozlabs.org/patch/630457/
I'd like at least first three patches to go in, then we can talk about implementation details of fifth one (in case you'll findit usefull)

Quoted 6 lines

Supposed to write into mtd0 works in most cases, yes. But it should be better let it configurable as in kobs-ng.

One reason is to show the user an easy path from kobs-ng to u-boot - and of course, how to change from kobs-ng to this command should be explained into the README.

Perhaps the only thing user can decide is whenever to use separate partition for each one of 4 redundant copies or you create just one for all. That default could be also provided from board specific code.

[snip]

Best regards,
ladis