[PATCH sunxi-tools 1/3] nand-image-builder: Fix the copyright header

83 views
Skip to first unread message

Boris Brezillon

unread,
Jun 1, 2016, 5:05:07 AM6/1/16
to Siarhei Siamashka, Bernhard Nortmann, linux...@googlegroups.com, Hans de Goede, Olliver Schinagl, Boris Brezillon
Add NextThing Co. and Free Electrons copyrights and add myself as the
author of the randomizer and image builder implementation.

Remove the lengthy description explaining how the BCH implementation works,
since this is the purpose of this tool is not to expose a BCH library
(which was the case of the original source code I copied from the kernel).

Signed-off-by: Boris Brezillon <boris.b...@free-electrons.com>
---
Hi,

A PR containing those 2 patches has been seent [1], but please review
the changes before merging the PR.

Thanks,

Boris

[1]https://github.com/linux-sunxi/sunxi-tools/pull/55

nand-image-builder.c | 48 ++++++------------------------------------------
1 file changed, 6 insertions(+), 42 deletions(-)

diff --git a/nand-image-builder.c b/nand-image-builder.c
index 645d1cc..34eee4f 100644
--- a/nand-image-builder.c
+++ b/nand-image-builder.c
@@ -14,55 +14,19 @@
* this program; if not, write to the Free Software Foundation, Inc., 51
* Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
+ * For the BCH implementation:
+ *
* Copyright © 2011 Parrot S.A.
*
* Author: Ivan Djelic <ivan....@parrot.com>
*
- * Description:
- *
- * This library provides runtime configurable encoding/decoding of binary
- * Bose-Chaudhuri-Hocquenghem (BCH) codes.
- *
- * Call init_bch to get a pointer to a newly allocated bch_control structure for
- * the given m (Galois field order), t (error correction capability) and
- * (optional) primitive polynomial parameters.
- *
- * Call encode_bch to compute and store ecc parity bytes to a given buffer.
- * Call decode_bch to detect and locate errors in received data.
- *
- * On systems supporting hw BCH features, intermediate results may be provided
- * to decode_bch in order to skip certain steps. See decode_bch() documentation
- * for details.
- *
- * Option CONFIG_BCH_CONST_PARAMS can be used to force fixed values of
- * parameters m and t; thus allowing extra compiler optimizations and providing
- * better (up to 2x) encoding performance. Using this option makes sense when
- * (m,t) are fixed and known in advance, e.g. when using BCH error correction
- * on a particular NAND flash device.
- *
- * Algorithmic details:
- *
- * Encoding is performed by processing 32 input bits in parallel, using 4
- * remainder lookup tables.
+ * For the randomizer and image builder implementation:
*
- * The final stage of decoding involves the following internal steps:
- * a. Syndrome computation
- * b. Error locator polynomial computation using Berlekamp-Massey algorithm
- * c. Error locator root finding (by far the most expensive step)
+ * Copyright © 2016 NextThing Co.
+ * Copyright © 2016 Free Electrons
*
- * In this implementation, step c is not performed using the usual Chien search.
- * Instead, an alternative approach described in [1] is used. It consists in
- * factoring the error locator polynomial using the Berlekamp Trace algorithm
- * (BTA) down to a certain degree (4), after which ad hoc low-degree polynomial
- * solving techniques [2] are used. The resulting algorithm, called BTZ, yields
- * much better performance than Chien search for usual (m,t) values (typically
- * m >= 13, t < 32, see [1]).
+ * Author: Boris Brezillon <boris.b...@free-electrons.com>
*
- * [1] B. Biswas, V. Herbert. Efficient root finding of polynomials over fields
- * of characteristic 2, in: Western European Workshop on Research in Cryptology
- * - WEWoRC 2009, Graz, Austria, LNCS, Springer, July 2009, to appear.
- * [2] [Zin96] V.A. Zinoviev. On the solution of equations of degree 10 over
- * finite fields GF(2^q). In Rapport de recherche INRIA no 2829, 1996.
*/

#include <stdint.h>
--
2.7.4

Bernhard Nortmann

unread,
Jun 1, 2016, 6:07:50 AM6/1/16
to Boris Brezillon, Siarhei Siamashka, linux...@googlegroups.com, Hans de Goede, Olliver Schinagl
Am 01.06.2016 um 11:05 schrieb Boris Brezillon:
> Add NextThing Co. and Free Electrons copyrights and add myself as the
> author of the randomizer and image builder implementation.
>
> Remove the lengthy description explaining how the BCH implementation works,
> since this is the purpose of this tool is not to expose a BCH library
> (which was the case of the original source code I copied from the kernel).
>
> Signed-off-by: Boris Brezillon <boris.b...@free-electrons.com>
> ---
> Hi,
>
> A PR containing those 2 patches has been seent [1], but please review
> the changes before merging the PR.
>
> Thanks,
>
> Boris
>
> [1]https://github.com/linux-sunxi/sunxi-tools/pull/55

To me that looks much better now. I'd also like to suggest keeping a
one-line
reference/link to the original kernel source, e.g.

See also:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/lib/bch.c

(or maybe http://lxr.free-electrons.com/source/lib/bch.c instead).

Boris Brezillon

unread,
Jun 2, 2016, 11:33:17 AM6/2/16
to Siarhei Siamashka, Bernhard Nortmann, linux...@googlegroups.com, Hans de Goede, Olliver Schinagl, Boris Brezillon
Add NextThing Co. and Free Electrons copyrights and add myself as the
author of the randomizer and image builder implementation.

Remove the lengthy description explaining how the BCH implementation works,
since this is the purpose of this tool is not to expose a BCH library
(which was the case of the original source code I copied from the kernel).

Signed-off-by: Boris Brezillon <boris.b...@free-electrons.com>
---
Changes since v1:
- Add a link to the original bch.c source file
---
nand-image-builder.c | 49 ++++++++-----------------------------------------
1 file changed, 8 insertions(+), 41 deletions(-)

diff --git a/nand-image-builder.c b/nand-image-builder.c
index 645d1cc..a5fb3d8 100644
--- a/nand-image-builder.c
+++ b/nand-image-builder.c
@@ -14,55 +14,22 @@
+ * See also:
+ * http://lxr.free-electrons.com/source/lib/bch.c

Boris Brezillon

unread,
Jun 2, 2016, 11:33:19 AM6/2/16
to Siarhei Siamashka, Bernhard Nortmann, linux...@googlegroups.com, Hans de Goede, Olliver Schinagl, Boris Brezillon
Add explanations on where the options to pass to the tool should be
extracted from, and add two examples to illustrate this explanation.

Signed-off-by: Boris Brezillon <boris.b...@free-electrons.com>
---
Changes since v1:
- use shorter option names
- rework the help context
---
nand-image-builder.c | 67 ++++++++++++++++++++++++++++++++++++----------------
1 file changed, 47 insertions(+), 20 deletions(-)

diff --git a/nand-image-builder.c b/nand-image-builder.c
index 465ab36..98c0a82 100644
--- a/nand-image-builder.c
+++ b/nand-image-builder.c
@@ -917,21 +917,48 @@ static void display_help(int status)
{
fprintf(status == EXIT_SUCCESS ? stdout : stderr,
"Usage: sunxi-nand-image-builder [OPTIONS] source-image output-image\n"
+ "\n"
"Creates a raw NAND image that can be read by the sunxi NAND controller.\n"
"\n"
- "-h --help Display this help and exit\n"
- "-c <strength>/<step> --ecc=<strength>/<step> ECC config\n"
- " Valid strengths: 16, 24, 28, 32, 40, 48, 56, 60 and 64\n"
- " Valid steps: 512 and 1024\n"
- "-p <size> --page-size=<size> Page size\n"
- "-o <size> --oob-size=<size> OOB size\n"
- "-u <size> --usable-page-size=<size> Usable page size. Only needed for boot0 mode\n"
- "-e <size> --eraseblock-size=<size> Erase block size\n"
- "-b --boot0 Build a boot0 image.\n"
- "-s --scramble Scramble data\n"
- "-a <offset> --address Where the image will be programmed.\n"
- " This option is only required for non boot0 images that are meant to be programmed at a non eraseblock aligned offset.\n"
- "\n");
+ " -h --help Display this help and exit\n"
+ " -c <strength>/<step> --ecc=<strength>/<step> ECC config\n"
+ " Valid strengths: 16, 24, 28, 32, 40, 48, 56, 60 and 64\n"
+ " Valid steps: 512 and 1024\n"
+ " -p <size> --page=<size> Page size\n"
+ " -o <size> --oob=<size> OOB size\n"
+ " -u <size> --usable=<size> Usable page size. Only needed for boot0 mode\n"
+ " -e <size> --eraseblock=<size> Erase block size\n"
+ " -b --boot0 Build a boot0 image.\n"
+ " -s --scramble Scramble data\n"
+ " -a <offset> --address Where the image will be programmed.\n"
+ "\n"
+ "Notes:\n"
+ " All the information you need to pass to this tool should be part of the NAND datasheet.\n"
+ "\n"
+ " If you are building a boot0 image, you'll have specify extra options.\n"
+ " These options should be chosen based on the layouts described here:\n"
+ " http://linux-sunxi.org/NAND#More_information_on_BROM_NAND\n"
+ "\n"
+ " --usable should be assigned the 'Hardware page' value\n"
+ " --ecc should be assigned the 'ECC capacity'/'ECC page' values\n"
+ " --usable should be smaller than --page\n"
+ "\n"
+ " The --address option is only required for non-boot0 images that are meant to be\n"
+ " programmed at a non eraseblock aligned offset.\n"
+ "\n"
+ "Examples:\n"
+ " The H27UCG8T2BTR-BC NAND exposes\n"
+ " * 16k pages\n"
+ " * 1280 OOB bytes per page\n"
+ " * 4M eraseblocks\n"
+ " * requires data scrambling\n"
+ " * expects a minimum ECC of 40bits/1024bytes\n"
+ "\n"
+ " A boot0 image can be generated with the following command\n"
+ " sunxi-nand-image-builder -p 16384 -o 1280 -e 0x400000 -s -b -u 4096 -c 64/1024\n"
+ " A normal image can be generated with\n"
+ " sunxi-nand-image-builder -p 16384 -o 1280 -e 0x400000 -s -c 40/1024\n"
+ );
exit(status);
}

@@ -942,17 +969,17 @@ static int check_image_info(struct image_info *info)
unsigned i;

if (!info->page_size) {
- fprintf(stderr, "--page-size is missing\n");
+ fprintf(stderr, "--page is missing\n");
return -EINVAL;
}

if (!info->page_size) {
- fprintf(stderr, "--oob-size is missing\n");
+ fprintf(stderr, "--oob is missing\n");
return -EINVAL;
}

if (!info->eraseblock_size) {
- fprintf(stderr, "--eraseblock-size is missing\n");
+ fprintf(stderr, "--eraseblock is missing\n");
return -EINVAL;
}

@@ -1004,10 +1031,10 @@ int main(int argc, char **argv)
static const struct option long_options[] = {
{"help", no_argument, 0, 0},
{"ecc", required_argument, 0, 'c'},
- {"page-size", required_argument, 0, 'p'},
- {"oob-size", required_argument, 0, 'o'},
- {"usable-page-size", required_argument, 0, 'u'},
- {"eraseblock-size", required_argument, 0, 'e'},
+ {"page", required_argument, 0, 'p'},
+ {"oob", required_argument, 0, 'o'},
+ {"usable", required_argument, 0, 'u'},
+ {"eraseblock", required_argument, 0, 'e'},
{"boot0", no_argument, 0, 'b'},
{"scramble", no_argument, 0, 's'},
{"address", required_argument, 0, 'a'},
--
2.7.4

Boris Brezillon

unread,
Jun 2, 2016, 11:33:19 AM6/2/16
to Siarhei Siamashka, Bernhard Nortmann, linux...@googlegroups.com, Hans de Goede, Olliver Schinagl, Boris Brezillon
Add error messages explaining what is wrong or missing in the arguments
passed by to the sunxi-nand-image-builder tool.

Signed-off-by: Boris Brezillon <boris.b...@free-electrons.com>
Acked-by: Bernhard Nortmann <bernhard...@web.de>
---
Changes since v1:
- Drop uneeded braces
---
nand-image-builder.c | 30 +++++++++++++++++++++++++-----
1 file changed, 25 insertions(+), 5 deletions(-)

diff --git a/nand-image-builder.c b/nand-image-builder.c
index a5fb3d8..465ab36 100644
--- a/nand-image-builder.c
+++ b/nand-image-builder.c
@@ -941,20 +941,37 @@ static int check_image_info(struct image_info *info)
int eccbytes, eccsteps;
unsigned i;

- if (!info->page_size || !info->oob_size || !info->eraseblock_size ||
- !info->usable_page_size)
+ if (!info->page_size) {
+ fprintf(stderr, "--page-size is missing\n");
return -EINVAL;
+ }
+
+ if (!info->page_size) {
+ fprintf(stderr, "--oob-size is missing\n");
+ return -EINVAL;
+ }
+
+ if (!info->eraseblock_size) {
+ fprintf(stderr, "--eraseblock-size is missing\n");
+ return -EINVAL;
+ }

- if (info->ecc_step_size != 512 && info->ecc_step_size != 1024)
+ if (info->ecc_step_size != 512 && info->ecc_step_size != 1024) {
+ fprintf(stderr, "Invalid ECC step argument: %d\n",
+ info->ecc_step_size);
return -EINVAL;
+ }

for (i = 0; i < ARRAY_SIZE(valid_ecc_strengths); i++) {
if (valid_ecc_strengths[i] == info->ecc_strength)
break;
}

- if (i == ARRAY_SIZE(valid_ecc_strengths))
+ if (i == ARRAY_SIZE(valid_ecc_strengths)) {
+ fprintf(stderr, "Invalid ECC strength argument: %d\n",
+ info->ecc_strength);
return -EINVAL;
+ }

eccbytes = DIV_ROUND_UP(info->ecc_strength * 14, 8);
if (eccbytes % 2)
@@ -964,8 +981,11 @@ static int check_image_info(struct image_info *info)
eccsteps = info->usable_page_size / info->ecc_step_size;

if (info->page_size + info->oob_size <
- info->usable_page_size + (eccsteps * (eccbytes)))
+ info->usable_page_size + (eccsteps * eccbytes)) {
+ fprintf(stderr,
+ "ECC bytes do not fit in the NAND page, choose a weaker ECC\n");
return -EINVAL;
+ }

return 0;
}
--
2.7.4

Bernhard Nortmann

unread,
Jun 2, 2016, 1:21:50 PM6/2/16
to Boris Brezillon, Siarhei Siamashka, linux...@googlegroups.com, Hans de Goede, Olliver Schinagl
Am 02.06.2016 um 17:33 schrieb Boris Brezillon:
> Add NextThing Co. and Free Electrons copyrights and add myself as the
> author of the randomizer and image builder implementation.
>
> Remove the lengthy description explaining how the BCH implementation works,
> since this is the purpose of this tool is not to expose a BCH library
> (which was the case of the original source code I copied from the kernel).
>
> Signed-off-by: Boris Brezillon <boris.b...@free-electrons.com>
> ---
> Changes since v1:
> - Add a link to the original bch.c source file
> ---
> nand-image-builder.c | 49 ++++++++-----------------------------------------
> 1 file changed, 8 insertions(+), 41 deletions(-)

Looks good to me now.
Acked-by: Bernhard Nortmann <bernhard...@web.de>

Bernhard Nortmann

unread,
Jun 3, 2016, 1:43:45 AM6/3/16
to boris.b...@free-electrons.com, Siarhei Siamashka, linux...@googlegroups.com, Hans de Goede, Olliver Schinagl
Am 02.06.2016 um 17:33 schrieb Boris Brezillon:
> Add explanations on where the options to pass to the tool should be
> extracted from, and add two examples to illustrate this explanation.
>
> Signed-off-by: Boris Brezillon <boris.b...@free-electrons.com>
> ---
> Changes since v1:
> - use shorter option names
> - rework the help context
> ---
> nand-image-builder.c | 67 ++++++++++++++++++++++++++++++++++++----------------
> 1 file changed, 47 insertions(+), 20 deletions(-)

Hi Boris!

I already like this version of the usage help much more than the previous
one - thanks for reworking it. If it was my patch however, I'd still take
it further. Doing away with the TABs and using SPACEs instead, and with
some lines even more terse, it could look like this:

diff --git a/nand-image-builder.c b/nand-image-builder.c
index 98c0a82..043d679 100644
--- a/nand-image-builder.c
+++ b/nand-image-builder.c
@@ -920,44 +920,44 @@ static void display_help(int status)
"\n"
"Creates a raw NAND image that can be read by the sunxi NAND
controller.\n"
"\n"
- " -h --help Display this help and exit\n"
- " -c <strength>/<step> --ecc=<strength>/<step> ECC config\n"
- " Valid strengths: 16, 24, 28, 32, 40,
48, 56, 60 and 64\n"
- " Valid steps: 512 and 1024\n"
- " -p <size> --page=<size> Page size\n"
- " -o <size> --oob=<size> OOB size\n"
- " -u <size> --usable=<size> Usable page size. Only
needed for boot0 mode\n"
- " -e <size> --eraseblock=<size> Erase block size\n"
- " -b --boot0 Build a boot0 image.\n"
- " -s --scramble Scramble data\n"
- " -a <offset> --address Where the image will be
programmed.\n"
+ "-h --help Display this help and
exit\n"
+ "-c <str>/<step> --ecc=<str>/<step> ECC config\n"
+ " Valid strengths: 16, 24, 28, 32, 40,
48, 56, 60 and 64\n"
+ " Valid steps: 512 and 1024\n"
+ "-p <size> --page=<size> Page size\n"
+ "-o <size> --oob=<size> OOB size\n"
+ "-u <size> --usable=<size> Usable page size. Only
needed for --boot0\n"
+ "-e <size> --eraseblock=<size> Erase block size\n"
+ "-b --boot0 Build a boot0 image.\n"
+ "-s --scramble Scramble data\n"
+ "-a <offset> --address=<offset> Where the image will be
programmed.\n"
"\n"
"Notes:\n"
- " All the information you need to pass to this tool should be
part of the NAND datasheet.\n"
+ "All the information you need to pass should be part of the
NAND datasheet.\n"
"\n"
- " If you are building a boot0 image, you'll have specify extra
options.\n"
- " These options should be chosen based on the layouts described
here:\n"
+ "If you are building a boot0 image, you'll have specify extra
options.\n"
+ "These options should be chosen based on the layouts described
here:\n"
" http://linux-sunxi.org/NAND#More_information_on_BROM_NAND\n"
"\n"
- " --usable should be assigned the 'Hardware page' value\n"
- " --ecc should be assigned the 'ECC capacity'/'ECC page'
values\n"
- " --usable should be smaller than --page\n"
+ " --usable should be assigned the 'Hardware page' value\n"
+ " --usable should be smaller than --page\n"
+ " --ecc should be assigned the 'ECC capacity'/'ECC page'
values\n"
"\n"
- " The --address option is only required for non-boot0 images
that are meant to be\n"
- " programmed at a non eraseblock aligned offset.\n"
+ "The --address option is only required for non-boot0 images
that are\n"
+ "meant to be programmed at a non eraseblock aligned offset.\n"
"\n"
"Examples:\n"
- " The H27UCG8T2BTR-BC NAND exposes\n"
+ " The H27UCG8T2BTR-BC NAND exposes\n"
" * 16k pages\n"
" * 1280 OOB bytes per page\n"
" * 4M eraseblocks\n"
" * requires data scrambling\n"
" * expects a minimum ECC of 40bits/1024bytes\n"
"\n"
- " A boot0 image can be generated with the following command\n"
- " sunxi-nand-image-builder -p 16384 -o 1280 -e 0x400000 -s
-b -u 4096 -c 64/1024\n"
- " A normal image can be generated with\n"
- " sunxi-nand-image-builder -p 16384 -o 1280 -e 0x400000 -s
-c 40/1024\n"
+ " A normal image can be generated with\n"
+ " sunxi-nand-image-builder -p 16384 -o 1280 -e 0x400000 -s
-c 40/1024\n"
+ " A boot0 image can be generated with the following command\n"
+ " sunxi-nand-image-builder -p 16384 -o 1280 -e 0x400000 -s
-c 64/1024 -b -u 4096\n"
);
exit(status);
}

The resulting usage help almost everywhere satisfies the 'classic'
80 chars per line limit, with the last line being an exceptions (which
I find acceptable though).

I've also reordered the example invocations: The "normal" invocation now
comes first, and the "boot0" cases has the parameters reordered, to make
it visually clearer where they differ.

Regards, B. Nortmann

Boris Brezillon

unread,
Jun 3, 2016, 9:50:38 AM6/3/16
to Bernhard Nortmann, Siarhei Siamashka, linux...@googlegroups.com, Hans de Goede, Olliver Schinagl
On Fri, 3 Jun 2016 07:43:40 +0200
Bernhard Nortmann <bernhard...@web.de> wrote:

> Am 02.06.2016 um 17:33 schrieb Boris Brezillon:
> > Add explanations on where the options to pass to the tool should be
> > extracted from, and add two examples to illustrate this explanation.
> >
> > Signed-off-by: Boris Brezillon <boris.b...@free-electrons.com>
> > ---
> > Changes since v1:
> > - use shorter option names
> > - rework the help context
> > ---
> > nand-image-builder.c | 67 ++++++++++++++++++++++++++++++++++++----------------
> > 1 file changed, 47 insertions(+), 20 deletions(-)
>
> Hi Boris!
>
> I already like this version of the usage help much more than the previous
> one - thanks for reworking it. If it was my patch however, I'd still take
> it further. Doing away with the TABs and using SPACEs instead, and with
> some lines even more terse, it could look like this:

Agreed. I'll squash your changes into this patch.

Thanks,

Boris
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Boris Brezillon

unread,
Jun 3, 2016, 11:38:46 AM6/3/16
to Siarhei Siamashka, Bernhard Nortmann, linux...@googlegroups.com, Hans de Goede, Olliver Schinagl, Boris Brezillon
Add error messages explaining what is wrong or missing in the arguments
passed by to the sunxi-nand-image-builder tool.

Signed-off-by: Boris Brezillon <boris.b...@free-electrons.com>
Acked-by: Bernhard Nortmann <bernhard...@web.de>
---
Changes since v1:
- Drop uneeded braces
---
nand-image-builder.c | 30 +++++++++++++++++++++++++-----
1 file changed, 25 insertions(+), 5 deletions(-)

diff --git a/nand-image-builder.c b/nand-image-builder.c
index a5fb3d8..465ab36 100644
--- a/nand-image-builder.c
+++ b/nand-image-builder.c

Boris Brezillon

unread,
Jun 3, 2016, 11:38:46 AM6/3/16
to Siarhei Siamashka, Bernhard Nortmann, linux...@googlegroups.com, Hans de Goede, Olliver Schinagl, Boris Brezillon
Add explanations on where the options to pass to the tool should be
extracted from, and add two examples to illustrate this explanation.

Signed-off-by: Boris Brezillon <boris.b...@free-electrons.com>
---
Changes since v2:
- limit line width in the help context

Changes since v1:
- use shorter option names
- rework the help context
---
nand-image-builder.c | 70 +++++++++++++++++++++++++++++++++++++---------------
1 file changed, 50 insertions(+), 20 deletions(-)

diff --git a/nand-image-builder.c b/nand-image-builder.c
index 465ab36..230ed0c 100644
--- a/nand-image-builder.c
+++ b/nand-image-builder.c
@@ -917,21 +917,51 @@ static void display_help(int status)
{
fprintf(status == EXIT_SUCCESS ? stdout : stderr,
"Usage: sunxi-nand-image-builder [OPTIONS] source-image output-image\n"
+ "\n"
"Creates a raw NAND image that can be read by the sunxi NAND controller.\n"
"\n"
- "-h --help Display this help and exit\n"
- "-c <strength>/<step> --ecc=<strength>/<step> ECC config\n"
- " Valid strengths: 16, 24, 28, 32, 40, 48, 56, 60 and 64\n"
- " Valid steps: 512 and 1024\n"
- "-p <size> --page-size=<size> Page size\n"
- "-o <size> --oob-size=<size> OOB size\n"
- "-u <size> --usable-page-size=<size> Usable page size. Only needed for boot0 mode\n"
- "-e <size> --eraseblock-size=<size> Erase block size\n"
- "-b --boot0 Build a boot0 image.\n"
- "-s --scramble Scramble data\n"
- "-a <offset> --address Where the image will be programmed.\n"
- " This option is only required for non boot0 images that are meant to be programmed at a non eraseblock aligned offset.\n"
- "\n");
+ "-h --help Display this help and exit\n"
+ "-c <str>/<step> --ecc=<str>/<step> ECC config (strength/step-size)\n"
+ "-p <size> --page=<size> Page size\n"
+ "-o <size> --oob=<size> OOB size\n"
+ "-u <size> --usable=<size> Usable page size\n"
+ "-e <size> --eraseblock=<size> Erase block size\n"
+ "-b --boot0 Build a boot0 image.\n"
+ "-s --scramble Scramble data\n"
+ "-a <offset> --address Where the image will be programmed.\n"
+ "\n"
+ "Notes:\n"
+ "All the information you need to pass to this tool should be part of\n"
+ "the NAND datasheet.\n"
+ "\n"
+ "The NAND controller only supports the following ECC configs\n"
+ " Valid ECC strengths: 16, 24, 28, 32, 40, 48, 56, 60 and 64\n"
+ " Valid ECC step size: 512 and 1024\n"
+ "\n"
+ "If you are building a boot0 image, you'll have specify extra options.\n"
+ "These options should be chosen based on the layouts described here:\n"
+ " http://linux-sunxi.org/NAND#More_information_on_BROM_NAND\n"
+ "\n"
+ " --usable should be assigned the 'Hardware page' value\n"
+ " --ecc should be assigned the 'ECC capacity'/'ECC page' values\n"
+ " --usable should be smaller than --page\n"
+ "\n"
+ "The --address option is only required for non-boot0 images that are \n"
+ "meant to be programmed at a non eraseblock aligned offset.\n"
+ "\n"
+ "Examples:\n"
+ " The H27UCG8T2BTR-BC NAND exposes\n"
+ " * 16k pages\n"
+ " * 1280 OOB bytes per page\n"
+ " * 4M eraseblocks\n"
+ " * requires data scrambling\n"
+ " * expects a minimum ECC of 40bits/1024bytes\n"
+ "\n"
+ " A normal image can be generated with\n"
+ " sunxi-nand-image-builder -p 16384 -o 1280 -e 0x400000 -s -c 40/1024\n"
+ " A boot0 image can be generated with\n"
+ " sunxi-nand-image-builder -p 16384 -o 1280 -e 0x400000 -s -b -u 4096 -c 64/1024\n"
+ );
exit(status);
}

@@ -942,17 +972,17 @@ static int check_image_info(struct image_info *info)
unsigned i;

if (!info->page_size) {
- fprintf(stderr, "--page-size is missing\n");
+ fprintf(stderr, "--page is missing\n");
return -EINVAL;
}

if (!info->page_size) {
- fprintf(stderr, "--oob-size is missing\n");
+ fprintf(stderr, "--oob is missing\n");
return -EINVAL;
}

if (!info->eraseblock_size) {
- fprintf(stderr, "--eraseblock-size is missing\n");
+ fprintf(stderr, "--eraseblock is missing\n");
return -EINVAL;
}

@@ -1004,10 +1034,10 @@ int main(int argc, char **argv)

Boris Brezillon

unread,
Jun 3, 2016, 11:38:46 AM6/3/16
to Siarhei Siamashka, Bernhard Nortmann, linux...@googlegroups.com, Hans de Goede, Olliver Schinagl, Boris Brezillon
Add NextThing Co. and Free Electrons copyrights and add myself as the
author of the randomizer and image builder implementation.

Remove the lengthy description explaining how the BCH implementation works,
since this is the purpose of this tool is not to expose a BCH library
(which was the case of the original source code I copied from the kernel).

Signed-off-by: Boris Brezillon <boris.b...@free-electrons.com>
---
Changes since v1:
- Add a link to the original bch.c source file
---
nand-image-builder.c | 49 ++++++++-----------------------------------------
1 file changed, 8 insertions(+), 41 deletions(-)

diff --git a/nand-image-builder.c b/nand-image-builder.c
index 645d1cc..a5fb3d8 100644
--- a/nand-image-builder.c
+++ b/nand-image-builder.c

Bernhard Nortmann

unread,
Jun 4, 2016, 1:49:55 PM6/4/16
to Boris Brezillon, Siarhei Siamashka, linux...@googlegroups.com, Hans de Goede, Olliver Schinagl
Hi Boris!
Shouldn't the long option show the parameter too, i.e. "--address=<offset>"?

> + "\n"
> + "Notes:\n"
> + "All the information you need to pass to this tool should be part of\n"
> + "the NAND datasheet.\n"
> + "\n"
> + "The NAND controller only supports the following ECC configs\n"
> + " Valid ECC strengths: 16, 24, 28, 32, 40, 48, 56, 60 and 64\n"
> + " Valid ECC step size: 512 and 1024\n"
> + "\n"
> + "If you are building a boot0 image, you'll have specify extra options.\n"
> + "These options should be chosen based on the layouts described here:\n"
> + " http://linux-sunxi.org/NAND#More_information_on_BROM_NAND\n"

For consistency, either zero or two leading spaces in front of the URL?
After the remarks above are addressed:
Acked-by: Bernhard Nortmann <bernhard...@web.de>

Boris Brezillon

unread,
Jun 6, 2016, 4:26:27 AM6/6/16
to Siarhei Siamashka, Bernhard Nortmann, linux...@googlegroups.com, Hans de Goede, Olliver Schinagl, Boris Brezillon
Add NextThing Co. and Free Electrons copyrights and add myself as the
author of the randomizer and image builder implementation.

Remove the lengthy description explaining how the BCH implementation works,
since this is the purpose of this tool is not to expose a BCH library
(which was the case of the original source code I copied from the kernel).

Signed-off-by: Boris Brezillon <boris.b...@free-electrons.com>
---
Changes since v1:
- Add a link to the original bch.c source file
---
nand-image-builder.c | 49 ++++++++-----------------------------------------
1 file changed, 8 insertions(+), 41 deletions(-)

diff --git a/nand-image-builder.c b/nand-image-builder.c
index 645d1cc..a5fb3d8 100644
--- a/nand-image-builder.c
+++ b/nand-image-builder.c

Boris Brezillon

unread,
Jun 6, 2016, 4:26:28 AM6/6/16
to Siarhei Siamashka, Bernhard Nortmann, linux...@googlegroups.com, Hans de Goede, Olliver Schinagl, Boris Brezillon
Add explanations on where the options to pass to the tool should be
extracted from, and add two examples to illustrate this explanation.

Signed-off-by: Boris Brezillon <boris.b...@free-electrons.com>
Acked-by: Bernhard Nortmann <bernhard...@web.de>
---
Changes since v3:
- cosmetic changes to the help context

Changes since v2:
- limit line width in the help context

Changes since v1:
- use shorter option names
- rework the help context
---
nand-image-builder.c | 70 +++++++++++++++++++++++++++++++++++++---------------
1 file changed, 50 insertions(+), 20 deletions(-)

diff --git a/nand-image-builder.c b/nand-image-builder.c
index 465ab36..41e559f 100644
--- a/nand-image-builder.c
+++ b/nand-image-builder.c
+ "-a <offset> --address=<offset> Where the image will be programmed.\n"
+ "\n"
+ "Notes:\n"
+ "All the information you need to pass to this tool should be part of\n"
+ "the NAND datasheet.\n"
+ "\n"
+ "The NAND controller only supports the following ECC configs\n"
+ " Valid ECC strengths: 16, 24, 28, 32, 40, 48, 56, 60 and 64\n"
+ " Valid ECC step size: 512 and 1024\n"
+ "\n"
+ "If you are building a boot0 image, you'll have specify extra options.\n"
+ "These options should be chosen based on the layouts described here:\n"
+ " http://linux-sunxi.org/NAND#More_information_on_BROM_NAND\n"
--
2.7.4

Boris Brezillon

unread,
Jun 6, 2016, 4:26:28 AM6/6/16
to Siarhei Siamashka, Bernhard Nortmann, linux...@googlegroups.com, Hans de Goede, Olliver Schinagl, Boris Brezillon
Add error messages explaining what is wrong or missing in the arguments
passed by to the sunxi-nand-image-builder tool.

Signed-off-by: Boris Brezillon <boris.b...@free-electrons.com>
Acked-by: Bernhard Nortmann <bernhard...@web.de>
---
Changes since v1:
- Drop uneeded braces
---
nand-image-builder.c | 30 +++++++++++++++++++++++++-----
1 file changed, 25 insertions(+), 5 deletions(-)

diff --git a/nand-image-builder.c b/nand-image-builder.c
index a5fb3d8..465ab36 100644
--- a/nand-image-builder.c
+++ b/nand-image-builder.c

Boris Brezillon

unread,
Jun 6, 2016, 4:26:29 AM6/6/16
to Siarhei Siamashka, Bernhard Nortmann, linux...@googlegroups.com, Hans de Goede, Olliver Schinagl, Boris Brezillon
--help/-h is not working correctly (it's printing the help context on
stderr instead of stdout).
Adding a valid shortcut for --help solves the problem.

Signed-off-by: Boris Brezillon <boris.b...@free-electrons.com>
Acked-by: Bernhard Nortmann <bernhard...@web.de>
---
nand-image-builder.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/nand-image-builder.c b/nand-image-builder.c
index 41e559f..0ca2d14 100644
--- a/nand-image-builder.c
+++ b/nand-image-builder.c
@@ -1032,7 +1032,7 @@ int main(int argc, char **argv)
int option_index = 0;
char *endptr = NULL;
static const struct option long_options[] = {
- {"help", no_argument, 0, 0},
+ {"help", no_argument, 0, 'h'},
{"ecc", required_argument, 0, 'c'},
{"page", required_argument, 0, 'p'},
{"oob", required_argument, 0, 'o'},
@@ -1044,7 +1044,7 @@ int main(int argc, char **argv)
{0, 0, 0, 0},
};

- int c = getopt_long(argc, argv, "c:p:o:u:e:ba:s",
+ int c = getopt_long(argc, argv, "c:p:o:u:e:ba:sh",
long_options, &option_index);
if (c == EOF)
break;
--
2.7.4

Reply all
Reply to author
Forward
0 new messages