[PATCH 00/22] i.MX BBU improvements and bugfixes

[PATCH 00/22] i.MX BBU improvements and bugfixes

[Andrey Smirnov]

Everyone:

This series is the result simplification, bugfixing and adding
features to some of the code in BBU subsytem:

  - Patch 2 "filetype: Add code to detect i.MX image v1" is not really
    used anywhere in the series and is provided for the sake of
    completness. It should probably dropeed but I thought I'd include
    it just in case

  - Patches 1, 3 - 12 are a number of code simplifications intended to
    minimize amount of duplicated code as well as generic improvements
    (IMHO)

  - Patches 13 - 17 are i.MX8MQ and VF610 related changes

  - Patches 18 - 22 are a result of discovering that a number of error
    cases were silently ignored and not reported as update
    failures. Patch 20 is technically not related to BBU at all, but
    since I discovered the problem while working on this series, I
    kept it as a part of this series

This patchset was tested of following platforms:

  - i.MX51 ZII RDU1 (SPI-NOR and eMMC)
  - i.MX6Q ZII RDU2 (SPI-NOR and eMMC)
  - VF610 ZII Vybrid Dev Board Rev C. (SPI-NOR)
  - i.MX8MQ NXP EVK (eMMC)
  - i.MX8MQ ZII RDU3 (not supported by upstream Barebox)

Feedback is wellcome!

Thanks,
Andrey Smirnov

Andrey Smirnov (22):
  ARM: i.MX: bbu: Remove unused define
  filetype: Add code to detect i.MX image v1
  filetype: Add code to detect i.MX image v2
  ARM: i.MX: bbu: Move inner-image type check
  ARM: i.MX: bbu: Drop IMX_INTERNAL_FLAG_NAND
  ARM: i.MX: bbu: Consolidate vairous update helpers
  ARM: i.MX: bbu: Simplify imx53_bbu_internal_nand_register_handler()
  ARM: i.MX: bbu: Constify all 'devicefile' arguments
  ARM: i.MX: bbu: Detect which platforms need v2 i.MX header
  ARM: i.MX: bbu: Alias imx5*_bbu_internal_mmc_register_handler()
  ARM: i.MX: bbu: Alias imx5*_bbu_internal_spi_i2c_register_handler()
  ARM: i.MX: bbu: Move protect code into a separate routine
  ARM: i.MX: bbu: Adjust FLASH_HEADER_OFFSET_MMC for i.MX8MQ
  ARM: i.MX: bbu: Add support for SPI/I2C on VFxxx
  ARM: i.MX: zii-vf610-dev-rev-b/c: Add support for BBU on SPI-NOR
  ARM: i.MX: bbu: Add support for MMC on i.MX8MQ
  ARM: nxp-imx8mq-evk: Add eMMC BBU configuration
  ARM: i.MX: bbu: Adjust error code check for pwrite()
  bbu: Remove logical negation in barebox_update_handler_exists()
  block: Do not ignore error in blk->ops->write()
  bbu: Report update failures explicitly
  bbu: command: Make sure specified update handler exists

 arch/arm/boards/nxp-imx8mq-evk/board.c |   3 +
 arch/arm/boards/zii-vf610-dev/board.c  |  19 ++
 arch/arm/dts/vf610-zii-dev-rev-b.dts   |  11 +
 arch/arm/mach-imx/imx-bbu-internal.c   | 339 ++++++++++++++-----------
 arch/arm/mach-imx/include/mach/bbu.h   |  60 +++--
 commands/barebox-update.c              |  15 +-
 common/bbu.c                           |  14 +-
 common/block.c                         |  17 +-
 common/filetype.c                      |  11 +
 include/filetype.h                     |   2 +
 10 files changed, 307 insertions(+), 184 deletions(-)

-- 
2.17.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 01/22] ARM: i.MX: bbu: Remove unused define

[Andrey Smirnov]

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 arch/arm/mach-imx/imx-bbu-internal.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/arm/mach-imx/imx-bbu-internal.c b/arch/arm/mach-imx/imx-bbu-internal.c
index 55e344cff..67ae2961c 100644
--- a/arch/arm/mach-imx/imx-bbu-internal.c
+++ b/arch/arm/mach-imx/imx-bbu-internal.c
@@ -16,8 +16,6 @@
  * GNU General Public License for more details.
  */
 
-#define IMX_INTERNAL_NAND_BBU
-
 #include <common.h>
 #include <malloc.h>
 #include <bbu.h>
-- 
2.17.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 02/22] filetype: Add code to detect i.MX image v1

[Andrey Smirnov]

Modify file_detect_type() and add code needed to be able to detect
i.MX boot images with v1 header.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 common/filetype.c  | 7 +++++++
 include/filetype.h | 1 +
 2 files changed, 8 insertions(+)

diff --git a/common/filetype.c b/common/filetype.c
index c5f2384a6..f68a83bec 100644
--- a/common/filetype.c
+++ b/common/filetype.c
@@ -29,6 +29,8 @@
 #include <image-sparse.h>
 #include <elf.h>
 
+#include <../mach-imx/include/mach/imx-header.h>
+
 struct filetype_str {
 	const char *name;	/* human readable filetype */
 	const char *shortname;	/* short string without spaces for shell scripts */
@@ -71,6 +73,7 @@ static const struct filetype_str filetype_str[] = {
 	[filetype_android_sparse] = { "Android sparse image", "sparse" },
 	[filetype_arm64_linux_image] = { "ARM aarch64 Linux image", "aarch64-linux" },
 	[filetype_elf] = { "ELF", "elf" },
+	[filetype_imx_image_v1] = { "i.MX image (v1)", "imx-image-v1" },
 };
 
 const char *file_type_to_string(enum filetype f)
@@ -250,6 +253,7 @@ enum filetype file_detect_type(const void *_buf, size_t bufsize)
 	const u64 *buf64 = _buf;
 	const u8 *buf8 = _buf;
 	const u16 *buf16 = _buf;
+	const struct imx_flash_header *imx_flash_header = _buf;
 	enum filetype type;
 
 	if (bufsize < 9)
@@ -361,6 +365,9 @@ enum filetype file_detect_type(const void *_buf, size_t bufsize)
 	if (strncmp(buf8, ELFMAG, 4) == 0)
 		return filetype_elf;
 
+	if (imx_flash_header->dcd_barker == DCD_BARKER)
+		return filetype_imx_image_v1;
+
 	return filetype_unknown;
 }
 
diff --git a/include/filetype.h b/include/filetype.h
index 237ed3fbe..e2df5fabf 100644
--- a/include/filetype.h
+++ b/include/filetype.h
@@ -43,6 +43,7 @@ enum filetype {
 	filetype_android_sparse,
 	filetype_arm64_linux_image,
 	filetype_elf,
+	filetype_imx_image_v1,
 	filetype_max,
 };
 
-- 
2.17.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 02/22] filetype: Add code to detect i.MX image v1

[Roland Hieber]

Hello Andrey,

On Mon, Aug 20, 2018 at 11:25:43PM -0700, Andrey Smirnov wrote:
> Modify file_detect_type() and add code needed to be able to detect
> i.MX boot images with v1 header.
> 
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> ---
>  common/filetype.c  | 7 +++++++
>  include/filetype.h | 1 +
>  2 files changed, 8 insertions(+)
> 
> diff --git a/common/filetype.c b/common/filetype.c
> index c5f2384a6..f68a83bec 100644
> --- a/common/filetype.c
> +++ b/common/filetype.c
> @@ -29,6 +29,8 @@
>  #include <image-sparse.h>
>  #include <elf.h>
>  
> +#include <../mach-imx/include/mach/imx-header.h>

This fails at least on ARCH=sandbox:

  common/filetype.c:32:10: fatal error: ../mach-imx/include/mach/imx-header.h: No such file or directory
   #include <../mach-imx/include/mach/imx-header.h>
            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
I guess it could be solved with an #ifdef ARCH_IMX.

 - Roland

> +
>  struct filetype_str {
>  	const char *name;	/* human readable filetype */
>  	const char *shortname;	/* short string without spaces for shell scripts */
> @@ -71,6 +73,7 @@ static const struct filetype_str filetype_str[] = {
>  	[filetype_android_sparse] = { "Android sparse image", "sparse" },
>  	[filetype_arm64_linux_image] = { "ARM aarch64 Linux image", "aarch64-linux" },
>  	[filetype_elf] = { "ELF", "elf" },
> +	[filetype_imx_image_v1] = { "i.MX image (v1)", "imx-image-v1" },
>  };
>  
>  const char *file_type_to_string(enum filetype f)
> @@ -250,6 +253,7 @@ enum filetype file_detect_type(const void *_buf, size_t bufsize)
>  	const u64 *buf64 = _buf;
>  	const u8 *buf8 = _buf;
>  	const u16 *buf16 = _buf;
> +	const struct imx_flash_header *imx_flash_header = _buf;
>  	enum filetype type;
>  
>  	if (bufsize < 9)
> @@ -361,6 +365,9 @@ enum filetype file_detect_type(const void *_buf, size_t bufsize)
>  	if (strncmp(buf8, ELFMAG, 4) == 0)
>  		return filetype_elf;
>  
> +	if (imx_flash_header->dcd_barker == DCD_BARKER)
> +		return filetype_imx_image_v1;
> +
>  	return filetype_unknown;
>  }
>  
> diff --git a/include/filetype.h b/include/filetype.h
> index 237ed3fbe..e2df5fabf 100644
> --- a/include/filetype.h
> +++ b/include/filetype.h
> @@ -43,6 +43,7 @@ enum filetype {
>  	filetype_android_sparse,
>  	filetype_arm64_linux_image,
>  	filetype_elf,
> +	filetype_imx_image_v1,
>  	filetype_max,
>  };
>  
> -- 
> 2.17.1
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox
> 

-- 
Roland Hieber                     | r.hieber@pengutronix.de     |
Pengutronix e.K.                  | https://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim | Phone: +49-5121-206917-5086 |
Amtsgericht Hildesheim, HRA 2686  | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 02/22] filetype: Add code to detect i.MX image v1

[Andrey Smirnov]

On Tue, Aug 21, 2018 at 3:07 AM Roland Hieber <r.hieber@pengutronix.de> wrote:
>
> Hello Andrey,
>
> On Mon, Aug 20, 2018 at 11:25:43PM -0700, Andrey Smirnov wrote:
> > Modify file_detect_type() and add code needed to be able to detect
> > i.MX boot images with v1 header.
> >
> > Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> > ---
> >  common/filetype.c  | 7 +++++++
> >  include/filetype.h | 1 +
> >  2 files changed, 8 insertions(+)
> >
> > diff --git a/common/filetype.c b/common/filetype.c
> > index c5f2384a6..f68a83bec 100644
> > --- a/common/filetype.c
> > +++ b/common/filetype.c
> > @@ -29,6 +29,8 @@
> >  #include <image-sparse.h>
> >  #include <elf.h>
> >
> > +#include <../mach-imx/include/mach/imx-header.h>
>
> This fails at least on ARCH=sandbox:
>
>   common/filetype.c:32:10: fatal error: ../mach-imx/include/mach/imx-header.h: No such file or directory
>    #include <../mach-imx/include/mach/imx-header.h>
>             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> I guess it could be solved with an #ifdef ARCH_IMX.
>

Good catch! I'll make sure to test against ARCH=sandbox when preparing
v2 of the set.

Thanks,
Andrey Smirnov

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 02/22] filetype: Add code to detect i.MX image v1

[Roland Hieber]

On Tue, Aug 21, 2018 at 01:23:28PM -0700, Andrey Smirnov wrote:
> On Tue, Aug 21, 2018 at 3:07 AM Roland Hieber <r.hieber@pengutronix.de> wrote:
> >
> > Hello Andrey,
> >
> > On Mon, Aug 20, 2018 at 11:25:43PM -0700, Andrey Smirnov wrote:
> > > Modify file_detect_type() and add code needed to be able to detect
> > > i.MX boot images with v1 header.
> > >
> > > Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> > > ---
> > >  common/filetype.c  | 7 +++++++
> > >  include/filetype.h | 1 +
> > >  2 files changed, 8 insertions(+)
> > >
> > > diff --git a/common/filetype.c b/common/filetype.c
> > > index c5f2384a6..f68a83bec 100644
> > > --- a/common/filetype.c
> > > +++ b/common/filetype.c
> > > @@ -29,6 +29,8 @@
> > >  #include <image-sparse.h>
> > >  #include <elf.h>
> > >
> > > +#include <../mach-imx/include/mach/imx-header.h>
> >
> > This fails at least on ARCH=sandbox:
> >
> >   common/filetype.c:32:10: fatal error: ../mach-imx/include/mach/imx-header.h: No such file or directory
> >    #include <../mach-imx/include/mach/imx-header.h>
> >             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > I guess it could be solved with an #ifdef ARCH_IMX.
> >
> 
> Good catch! I'll make sure to test against ARCH=sandbox when preparing
> v2 of the set.

An include path of the form <../mach-imx/> strikes me as a rather odd
thing to have in code that should be machine-independent. Maybe it is
better to isolate the respective symbols from imx-header.h into a
generic header file that can live in the same path as e.g. <elf.h>. This
way we could also be able to detect IMX images on ARCH=sandbox :)

 - Roland


-- 
Roland Hieber                     | r.hieber@pengutronix.de     |
Pengutronix e.K.                  | https://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim | Phone: +49-5121-206917-5086 |
Amtsgericht Hildesheim, HRA 2686  | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 02/22] filetype: Add code to detect i.MX image v1

[Andrey Smirnov]

On Thu, Aug 23, 2018 at 2:33 AM Roland Hieber <r.hieber@pengutronix.de> wrote:
>
> On Tue, Aug 21, 2018 at 01:23:28PM -0700, Andrey Smirnov wrote:
> > On Tue, Aug 21, 2018 at 3:07 AM Roland Hieber <r.hieber@pengutronix.de> wrote:
> > >
> > > Hello Andrey,
> > >
> > > On Mon, Aug 20, 2018 at 11:25:43PM -0700, Andrey Smirnov wrote:
> > > > Modify file_detect_type() and add code needed to be able to detect
> > > > i.MX boot images with v1 header.
> > > >
> > > > Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> > > > ---
> > > >  common/filetype.c  | 7 +++++++
> > > >  include/filetype.h | 1 +
> > > >  2 files changed, 8 insertions(+)
> > > >
> > > > diff --git a/common/filetype.c b/common/filetype.c
> > > > index c5f2384a6..f68a83bec 100644
> > > > --- a/common/filetype.c
> > > > +++ b/common/filetype.c
> > > > @@ -29,6 +29,8 @@
> > > >  #include <image-sparse.h>
> > > >  #include <elf.h>
> > > >
> > > > +#include <../mach-imx/include/mach/imx-header.h>
> > >
> > > This fails at least on ARCH=sandbox:
> > >
> > >   common/filetype.c:32:10: fatal error: ../mach-imx/include/mach/imx-header.h: No such file or directory
> > >    #include <../mach-imx/include/mach/imx-header.h>
> > >             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > I guess it could be solved with an #ifdef ARCH_IMX.
> > >
> >
> > Good catch! I'll make sure to test against ARCH=sandbox when preparing
> > v2 of the set.
>
> An include path of the form <../mach-imx/> strikes me as a rather odd
> thing to have in code that should be machine-independent. Maybe it is
> better to isolate the respective symbols from imx-header.h into a
> generic header file that can live in the same path as e.g. <elf.h>. This
> way we could also be able to detect IMX images on ARCH=sandbox :)
>

I don't think that file is really machine independent. There's already
code to detect "filetype_mxs_bootstream", "filetype_socfpga_xload",
"filetype_kwbimage_v0", etc. which are as machine dependent as it
could be. It just those codepaths use magic numbers directly and don't
bother with any includes.

As for being able to detect IMX images on ARCH=sandbox, I was planning
on trying to resolve compilation issue (be it with adding a -I or
creating a shared header) without resorting to #ifdef ARCH_IMX a bunch
of code first and only doing that if I can find no other way of fixing
it.

Thanks,
Andrey Smirnov

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 03/22] filetype: Add code to detect i.MX image v2

[Andrey Smirnov]

Modify file_detect_type() and add code needed to be able to detect
i.MX boot images with v2 header.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 common/filetype.c  | 4 ++++
 include/filetype.h | 1 +
 2 files changed, 5 insertions(+)

diff --git a/common/filetype.c b/common/filetype.c
index f68a83bec..aaeea96d9 100644
--- a/common/filetype.c
+++ b/common/filetype.c
@@ -74,6 +74,7 @@ static const struct filetype_str filetype_str[] = {
 	[filetype_arm64_linux_image] = { "ARM aarch64 Linux image", "aarch64-linux" },
 	[filetype_elf] = { "ELF", "elf" },
 	[filetype_imx_image_v1] = { "i.MX image (v1)", "imx-image-v1" },
+	[filetype_imx_image_v2] = { "i.MX image (v2)", "imx-image-v2" },
 };
 
 const char *file_type_to_string(enum filetype f)
@@ -368,6 +369,9 @@ enum filetype file_detect_type(const void *_buf, size_t bufsize)
 	if (imx_flash_header->dcd_barker == DCD_BARKER)
 		return filetype_imx_image_v1;
 
+	if (is_imx_flash_header_v2(_buf))
+		return filetype_imx_image_v2;
+
 	return filetype_unknown;
 }
 
diff --git a/include/filetype.h b/include/filetype.h
index e2df5fabf..395053dd5 100644
--- a/include/filetype.h
+++ b/include/filetype.h
@@ -44,6 +44,7 @@ enum filetype {
 	filetype_arm64_linux_image,
 	filetype_elf,
 	filetype_imx_image_v1,
+	filetype_imx_image_v2,
 	filetype_max,
 };
 
-- 
2.17.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 04/22] ARM: i.MX: bbu: Move inner-image type check

[Andrey Smirnov]

Since imx_bbu_check_prereq() already uses file_detect_type() and we've
extended it to understand i.MX boot image file type, we can simplify a
bunch of repetitive code as follows:

    1. Convert all checks from IVT_BARKER to filetype_imx_image_v2
       check

    2. Move all of the checking to be a part of imx_bbu_check_prereq()

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 arch/arm/mach-imx/imx-bbu-internal.c | 64 +++++++++++++++++-----------
 1 file changed, 39 insertions(+), 25 deletions(-)

diff --git a/arch/arm/mach-imx/imx-bbu-internal.c b/arch/arm/mach-imx/imx-bbu-internal.c
index 67ae2961c..ea57b2772 100644
--- a/arch/arm/mach-imx/imx-bbu-internal.c
+++ b/arch/arm/mach-imx/imx-bbu-internal.c
@@ -106,11 +106,39 @@ err_close:
 	return ret;
 }
 
-static int imx_bbu_check_prereq(const char *devicefile, struct bbu_data *data)
+static int imx_bbu_check_prereq(struct imx_internal_bbu_handler *imx_handler,
+				const char *devicefile, struct bbu_data *data,
+				enum filetype expected_type)
 {
 	int ret;
-
-	if (file_detect_type(data->image, data->len) != filetype_arm_barebox) {
+	const void *blob;
+	size_t len;
+	enum filetype type;
+
+	type = file_detect_type(data->image, data->len);
+
+	switch (type) {
+	case filetype_arm_barebox:
+		/*
+		 * Specifying expected_type as unknow will disable the
+		 * inner image type check
+		 */
+		if (expected_type == filetype_unknown)
+			break;
+
+		blob = data->image + imx_handler->flash_header_offset;
+		len  = data->len   - imx_handler->flash_header_offset;
+		type = file_detect_type(blob, len);
+
+		if (type != expected_type) {
+			pr_err("Expected image type: %s, "
+			       "detected image type: %s\n",
+			       file_type_to_string(expected_type),
+			       file_type_to_string(type));
+			return -EINVAL;
+		}
+		break;
+	default:
 		if (!bbu_force(data, "Not an ARM barebox image"))
 			return -EINVAL;
 	}
@@ -137,7 +165,8 @@ static int imx_bbu_internal_v1_update(struct bbu_handler *handler, struct bbu_da
 		container_of(handler, struct imx_internal_bbu_handler, handler);
 	int ret;
 
-	ret = imx_bbu_check_prereq(data->devicefile, data);
+	ret = imx_bbu_check_prereq(imx_handler, data->devicefile, data,
+				   filetype_unknown);
 	if (ret)
 		return ret;
 
@@ -319,8 +348,6 @@ out:
 	return ret;
 }
 
-#define IVT_BARKER		0x402000d1
-
 /*
  * Update barebox on a v2 type internal boot (i.MX53)
  *
@@ -333,19 +360,12 @@ static int imx_bbu_internal_v2_update(struct bbu_handler *handler, struct bbu_da
 	struct imx_internal_bbu_handler *imx_handler =
 		container_of(handler, struct imx_internal_bbu_handler, handler);
 	int ret;
-	const uint32_t *barker;
 
-	ret = imx_bbu_check_prereq(data->devicefile, data);
+	ret = imx_bbu_check_prereq(imx_handler, data->devicefile, data,
+				   filetype_imx_image_v2);
 	if (ret)
 		return ret;
 
-	barker = data->image + imx_handler->flash_header_offset;
-
-	if (*barker != IVT_BARKER) {
-		pr_err("Board does not provide DCD data and this image is no imximage\n");
-		return -EINVAL;
-	}
-
 	if (imx_handler->handler.flags & IMX_INTERNAL_FLAG_NAND)
 		ret = imx_bbu_internal_v2_write_nand_dbbt(imx_handler, data);
 	else
@@ -361,18 +381,10 @@ static int imx_bbu_internal_v2_mmcboot_update(struct bbu_handler *handler,
 	struct imx_internal_bbu_handler *imx_handler =
 		container_of(handler, struct imx_internal_bbu_handler, handler);
 	int ret;
-	const uint32_t *barker;
 	char *bootpartvar;
 	const char *bootpart;
 	char *devicefile;
 
-	barker = data->image + imx_handler->flash_header_offset;
-
-	if (*barker != IVT_BARKER) {
-		pr_err("Board does not provide DCD data and this image is no imximage\n");
-		return -EINVAL;
-	}
-
 	ret = asprintf(&bootpartvar, "%s.boot", data->devicefile);
 	if (ret < 0)
 		return ret;
@@ -389,7 +401,8 @@ static int imx_bbu_internal_v2_mmcboot_update(struct bbu_handler *handler,
 	if (ret < 0)
 		goto free_bootpartvar;
 
-	ret = imx_bbu_check_prereq(devicefile, data);
+	ret = imx_bbu_check_prereq(imx_handler, devicefile, data,
+				   filetype_imx_image_v2);
 	if (ret)
 		goto free_devicefile;
 
@@ -414,7 +427,8 @@ static int imx_bbu_external_update(struct bbu_handler *handler, struct bbu_data
 		container_of(handler, struct imx_internal_bbu_handler, handler);
 	int ret;
 
-	ret = imx_bbu_check_prereq(data->devicefile, data);
+	ret = imx_bbu_check_prereq(imx_handler, data->devicefile, data,
+				   filetype_unknown);
 	if (ret)
 		return ret;
 
-- 
2.17.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 04/22] ARM: i.MX: bbu: Move inner-image type check

[Sascha Hauer]

On Mon, Aug 20, 2018 at 11:25:45PM -0700, Andrey Smirnov wrote:
> Since imx_bbu_check_prereq() already uses file_detect_type() and we've
> extended it to understand i.MX boot image file type, we can simplify a
> bunch of repetitive code as follows:
> 
>     1. Convert all checks from IVT_BARKER to filetype_imx_image_v2
>        check
> 
>     2. Move all of the checking to be a part of imx_bbu_check_prereq()
> 
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> ---
>  arch/arm/mach-imx/imx-bbu-internal.c | 64 +++++++++++++++++-----------
>  1 file changed, 39 insertions(+), 25 deletions(-)
> 
> diff --git a/arch/arm/mach-imx/imx-bbu-internal.c b/arch/arm/mach-imx/imx-bbu-internal.c
> index 67ae2961c..ea57b2772 100644
> --- a/arch/arm/mach-imx/imx-bbu-internal.c
> +++ b/arch/arm/mach-imx/imx-bbu-internal.c
> @@ -106,11 +106,39 @@ err_close:
>  	return ret;
>  }
>  
> -static int imx_bbu_check_prereq(const char *devicefile, struct bbu_data *data)
> +static int imx_bbu_check_prereq(struct imx_internal_bbu_handler *imx_handler,
> +				const char *devicefile, struct bbu_data *data,
> +				enum filetype expected_type)
>  {
>  	int ret;
> -
> -	if (file_detect_type(data->image, data->len) != filetype_arm_barebox) {
> +	const void *blob;
> +	size_t len;
> +	enum filetype type;
> +
> +	type = file_detect_type(data->image, data->len);
> +
> +	switch (type) {
> +	case filetype_arm_barebox:
> +		/*
> +		 * Specifying expected_type as unknow will disable the

s/unknow/unknown/

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 04/22] ARM: i.MX: bbu: Move inner-image type check

[Sascha Hauer]

On Mon, Aug 20, 2018 at 11:25:45PM -0700, Andrey Smirnov wrote:
> Since imx_bbu_check_prereq() already uses file_detect_type() and we've
> extended it to understand i.MX boot image file type, we can simplify a
> bunch of repetitive code as follows:
> 
>     1. Convert all checks from IVT_BARKER to filetype_imx_image_v2
>        check
> 
>     2. Move all of the checking to be a part of imx_bbu_check_prereq()
> 
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> ---
>  arch/arm/mach-imx/imx-bbu-internal.c | 64 +++++++++++++++++-----------
>  1 file changed, 39 insertions(+), 25 deletions(-)
> 
> diff --git a/arch/arm/mach-imx/imx-bbu-internal.c b/arch/arm/mach-imx/imx-bbu-internal.c
> index 67ae2961c..ea57b2772 100644
> --- a/arch/arm/mach-imx/imx-bbu-internal.c
> +++ b/arch/arm/mach-imx/imx-bbu-internal.c
> @@ -106,11 +106,39 @@ err_close:
>  	return ret;
>  }
>  
> -static int imx_bbu_check_prereq(const char *devicefile, struct bbu_data *data)
> +static int imx_bbu_check_prereq(struct imx_internal_bbu_handler *imx_handler,
> +				const char *devicefile, struct bbu_data *data,
> +				enum filetype expected_type)
>  {
>  	int ret;
> -
> -	if (file_detect_type(data->image, data->len) != filetype_arm_barebox) {
> +	const void *blob;
> +	size_t len;
> +	enum filetype type;
> +
> +	type = file_detect_type(data->image, data->len);
> +
> +	switch (type) {
> +	case filetype_arm_barebox:
> +		/*
> +		 * Specifying expected_type as unknow will disable the
> +		 * inner image type check
> +		 */
> +		if (expected_type == filetype_unknown)
> +			break;
> +
> +		blob = data->image + imx_handler->flash_header_offset;
> +		len  = data->len   - imx_handler->flash_header_offset;
> +		type = file_detect_type(blob, len);
> +
> +		if (type != expected_type) {
> +			pr_err("Expected image type: %s, "
> +			       "detected image type: %s\n",
> +			       file_type_to_string(expected_type),
> +			       file_type_to_string(type));
> +			return -EINVAL;
> +		}
> +		break;
> +	default:
>  		if (!bbu_force(data, "Not an ARM barebox image"))
>  			return -EINVAL;
>  	}
> @@ -137,7 +165,8 @@ static int imx_bbu_internal_v1_update(struct bbu_handler *handler, struct bbu_da
>  		container_of(handler, struct imx_internal_bbu_handler, handler);
>  	int ret;
>  
> -	ret = imx_bbu_check_prereq(data->devicefile, data);
> +	ret = imx_bbu_check_prereq(imx_handler, data->devicefile, data,
> +				   filetype_unknown);

Why filetype_unknown here? in the v2 version we have
filetype_imx_image_v2. I would expect filetype_imx_image_v1 here.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 04/22] ARM: i.MX: bbu: Move inner-image type check

[Andrey Smirnov]

On Tue, Aug 21, 2018 at 11:52 PM Sascha Hauer <s.hauer@pengutronix.de> wrote:
>
> On Mon, Aug 20, 2018 at 11:25:45PM -0700, Andrey Smirnov wrote:
> > Since imx_bbu_check_prereq() already uses file_detect_type() and we've
> > extended it to understand i.MX boot image file type, we can simplify a
> > bunch of repetitive code as follows:
> >
> >     1. Convert all checks from IVT_BARKER to filetype_imx_image_v2
> >        check
> >
> >     2. Move all of the checking to be a part of imx_bbu_check_prereq()
> >
> > Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> > ---
> >  arch/arm/mach-imx/imx-bbu-internal.c | 64 +++++++++++++++++-----------
> >  1 file changed, 39 insertions(+), 25 deletions(-)
> >
> > diff --git a/arch/arm/mach-imx/imx-bbu-internal.c b/arch/arm/mach-imx/imx-bbu-internal.c
> > index 67ae2961c..ea57b2772 100644
> > --- a/arch/arm/mach-imx/imx-bbu-internal.c
> > +++ b/arch/arm/mach-imx/imx-bbu-internal.c
> > @@ -106,11 +106,39 @@ err_close:
> >       return ret;
> >  }
> >
> > -static int imx_bbu_check_prereq(const char *devicefile, struct bbu_data *data)
> > +static int imx_bbu_check_prereq(struct imx_internal_bbu_handler *imx_handler,
> > +                             const char *devicefile, struct bbu_data *data,
> > +                             enum filetype expected_type)
> >  {
> >       int ret;
> > -
> > -     if (file_detect_type(data->image, data->len) != filetype_arm_barebox) {
> > +     const void *blob;
> > +     size_t len;
> > +     enum filetype type;
> > +
> > +     type = file_detect_type(data->image, data->len);
> > +
> > +     switch (type) {
> > +     case filetype_arm_barebox:
> > +             /*
> > +              * Specifying expected_type as unknow will disable the
> > +              * inner image type check
> > +              */
> > +             if (expected_type == filetype_unknown)
> > +                     break;
> > +
> > +             blob = data->image + imx_handler->flash_header_offset;
> > +             len  = data->len   - imx_handler->flash_header_offset;
> > +             type = file_detect_type(blob, len);
> > +
> > +             if (type != expected_type) {
> > +                     pr_err("Expected image type: %s, "
> > +                            "detected image type: %s\n",
> > +                            file_type_to_string(expected_type),
> > +                            file_type_to_string(type));
> > +                     return -EINVAL;
> > +             }
> > +             break;
> > +     default:
> >               if (!bbu_force(data, "Not an ARM barebox image"))
> >                       return -EINVAL;
> >       }
> > @@ -137,7 +165,8 @@ static int imx_bbu_internal_v1_update(struct bbu_handler *handler, struct bbu_da
> >               container_of(handler, struct imx_internal_bbu_handler, handler);
> >       int ret;
> >
> > -     ret = imx_bbu_check_prereq(data->devicefile, data);
> > +     ret = imx_bbu_check_prereq(imx_handler, data->devicefile, data,
> > +                                filetype_unknown);
>
> Why filetype_unknown here? in the v2 version we have
> filetype_imx_image_v2. I would expect filetype_imx_image_v1 here.
>

Purely because original code didn't do any type checking of "inner"
image, so I specified filetype_unknown and and added special handling
for it to preserve the status quo. It sounds like you think that it
would be better to change the original behavior such that there _is_
an "inner" image type check for v1 one header. That would be my
preference as well, since that'll allow me to get rid of special
filetype_unknown case, so that's what I'll do in v2.

Thanks,
Andrey Smirnov

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 04/22] ARM: i.MX: bbu: Move inner-image type check

[Sascha Hauer]

On Wed, Aug 22, 2018 at 05:06:55PM -0700, Andrey Smirnov wrote:
> On Tue, Aug 21, 2018 at 11:52 PM Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > > @@ -137,7 +165,8 @@ static int imx_bbu_internal_v1_update(struct bbu_handler *handler, struct bbu_da
> > >               container_of(handler, struct imx_internal_bbu_handler, handler);
> > >       int ret;
> > >
> > > -     ret = imx_bbu_check_prereq(data->devicefile, data);
> > > +     ret = imx_bbu_check_prereq(imx_handler, data->devicefile, data,
> > > +                                filetype_unknown);
> >
> > Why filetype_unknown here? in the v2 version we have
> > filetype_imx_image_v2. I would expect filetype_imx_image_v1 here.
> >
> 
> Purely because original code didn't do any type checking of "inner"
> image, so I specified filetype_unknown and and added special handling
> for it to preserve the status quo. It sounds like you think that it
> would be better to change the original behavior such that there _is_
> an "inner" image type check for v1 one header. That would be my
> preference as well, since that'll allow me to get rid of special
> filetype_unknown case, so that's what I'll do in v2.

Not sure why the v1 code is different here, it probably just grew like
that and I never noticed.

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 05/22] ARM: i.MX: bbu: Drop IMX_INTERNAL_FLAG_NAND

[Andrey Smirnov]

Replace IMX_INTERNAL_FLAG_NAND with a function pointer that can be
customized by individual registration functions. The change by iteself
doesn't have that much value, however it makes the simplification in
the commit that follows more straightforward.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 arch/arm/mach-imx/imx-bbu-internal.c | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/arch/arm/mach-imx/imx-bbu-internal.c b/arch/arm/mach-imx/imx-bbu-internal.c
index ea57b2772..0e55e4fd1 100644
--- a/arch/arm/mach-imx/imx-bbu-internal.c
+++ b/arch/arm/mach-imx/imx-bbu-internal.c
@@ -32,11 +32,12 @@
 
 #define FLASH_HEADER_OFFSET_MMC		0x400
 
-#define IMX_INTERNAL_FLAG_NAND		BIT(31)
 #define IMX_INTERNAL_FLAG_ERASE		BIT(30)
 
 struct imx_internal_bbu_handler {
 	struct bbu_handler handler;
+	int (*write_device)(struct imx_internal_bbu_handler *,
+			    struct bbu_data *);
 	unsigned long flash_header_offset;
 	size_t device_size;
 };
@@ -106,6 +107,13 @@ err_close:
 	return ret;
 }
 
+static int __imx_bbu_write_device(struct imx_internal_bbu_handler *imx_handler,
+				  struct bbu_data *data)
+{
+	return imx_bbu_write_device(imx_handler, data->devicefile, data,
+				    data->image, data->len);
+}
+
 static int imx_bbu_check_prereq(struct imx_internal_bbu_handler *imx_handler,
 				const char *devicefile, struct bbu_data *data,
 				enum filetype expected_type)
@@ -366,13 +374,7 @@ static int imx_bbu_internal_v2_update(struct bbu_handler *handler, struct bbu_da
 	if (ret)
 		return ret;
 
-	if (imx_handler->handler.flags & IMX_INTERNAL_FLAG_NAND)
-		ret = imx_bbu_internal_v2_write_nand_dbbt(imx_handler, data);
-	else
-		ret = imx_bbu_write_device(imx_handler, data->devicefile, data,
-					   data->image, data->len);
-
-	return ret;
+	return imx_handler->write_device(imx_handler, data);
 }
 
 static int imx_bbu_internal_v2_mmcboot_update(struct bbu_handler *handler,
@@ -448,6 +450,8 @@ static struct imx_internal_bbu_handler *__init_handler(const char *name, char *d
 	handler->name = name;
 	handler->flags = flags;
 
+	imx_handler->write_device = __imx_bbu_write_device;
+
 	return imx_handler;
 }
 
@@ -539,13 +543,13 @@ int imx53_bbu_internal_nand_register_handler(const char *name,
 {
 	struct imx_internal_bbu_handler *imx_handler;
 
-	imx_handler = __init_handler(name, NULL, flags |
-				     IMX_INTERNAL_FLAG_NAND);
+	imx_handler = __init_handler(name, NULL, flags);
 	imx_handler->flash_header_offset = FLASH_HEADER_OFFSET_MMC;
 
 	imx_handler->handler.handler = imx_bbu_internal_v2_update;
 	imx_handler->handler.devicefile = "/dev/nand0";
 	imx_handler->device_size = partition_size;
+	imx_handler->write_device = imx_bbu_internal_v2_write_nand_dbbt;
 
 	return __register_handler(imx_handler);
 }
-- 
2.17.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 06/22] ARM: i.MX: bbu: Consolidate vairous update helpers

[Andrey Smirnov]

All three, imx_bbu_internal_v1_update(), imx_bbu_external_update() and
imx_bbu_internal_v2_update() are doing exactly the same thing. Instead
of having three almost identical functions, convert
imx_bbu_internal_v2_update() (the most versatile of the three) to be a
generic function and use it everywhere.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 arch/arm/mach-imx/imx-bbu-internal.c | 70 +++++-----------------------
 1 file changed, 12 insertions(+), 58 deletions(-)

diff --git a/arch/arm/mach-imx/imx-bbu-internal.c b/arch/arm/mach-imx/imx-bbu-internal.c
index 0e55e4fd1..34c8ea92c 100644
--- a/arch/arm/mach-imx/imx-bbu-internal.c
+++ b/arch/arm/mach-imx/imx-bbu-internal.c
@@ -40,6 +40,7 @@ struct imx_internal_bbu_handler {
 			    struct bbu_data *);
 	unsigned long flash_header_offset;
 	size_t device_size;
+	enum filetype expected_type;
 };
 
 /*
@@ -160,31 +161,6 @@ static int imx_bbu_check_prereq(struct imx_internal_bbu_handler *imx_handler,
 	return 0;
 }
 
-/*
- * Update barebox on a v1 type internal boot (i.MX25, i.MX35, i.MX51)
- *
- * This constructs a DCD header, adds the specific DCD data and writes
- * the resulting image to the device. Currently this handles MMC/SD
- * devices.
- */
-static int imx_bbu_internal_v1_update(struct bbu_handler *handler, struct bbu_data *data)
-{
-	struct imx_internal_bbu_handler *imx_handler =
-		container_of(handler, struct imx_internal_bbu_handler, handler);
-	int ret;
-
-	ret = imx_bbu_check_prereq(imx_handler, data->devicefile, data,
-				   filetype_unknown);
-	if (ret)
-		return ret;
-
-	pr_info("updating to %s\n", data->devicefile);
-
-	ret = imx_bbu_write_device(imx_handler, data->devicefile, data, data->image, data->len);
-
-	return ret;
-}
-
 #define DBBT_MAGIC	0x44424254
 #define FCB_MAGIC	0x20424346
 
@@ -356,21 +332,14 @@ out:
 	return ret;
 }
 
-/*
- * Update barebox on a v2 type internal boot (i.MX53)
- *
- * This constructs a DCD header, adds the specific DCD data and writes
- * the resulting image to the device. Currently this handles MMC/SD
- * and NAND devices.
- */
-static int imx_bbu_internal_v2_update(struct bbu_handler *handler, struct bbu_data *data)
+static int imx_bbu_update(struct bbu_handler *handler, struct bbu_data *data)
 {
 	struct imx_internal_bbu_handler *imx_handler =
 		container_of(handler, struct imx_internal_bbu_handler, handler);
 	int ret;
 
 	ret = imx_bbu_check_prereq(imx_handler, data->devicefile, data,
-				   filetype_imx_image_v2);
+				   imx_handler->expected_type);
 	if (ret)
 		return ret;
 
@@ -423,21 +392,6 @@ free_bootpartvar:
 	return ret;
 }
 
-static int imx_bbu_external_update(struct bbu_handler *handler, struct bbu_data *data)
-{
-	struct imx_internal_bbu_handler *imx_handler =
-		container_of(handler, struct imx_internal_bbu_handler, handler);
-	int ret;
-
-	ret = imx_bbu_check_prereq(imx_handler, data->devicefile, data,
-				   filetype_unknown);
-	if (ret)
-		return ret;
-
-	return imx_bbu_write_device(imx_handler, data->devicefile, data,
-				    data->image, data->len);
-}
-
 static struct imx_internal_bbu_handler *__init_handler(const char *name, char *devicefile,
 		unsigned long flags)
 {
@@ -449,7 +403,9 @@ static struct imx_internal_bbu_handler *__init_handler(const char *name, char *d
 	handler->devicefile = devicefile;
 	handler->name = name;
 	handler->flags = flags;
+	handler->handler = imx_bbu_update;
 
+	imx_handler->expected_type = filetype_unknown;
 	imx_handler->write_device = __imx_bbu_write_device;
 
 	return imx_handler;
@@ -468,7 +424,8 @@ static int __register_handler(struct imx_internal_bbu_handler *imx_handler)
 
 static int
 imx_bbu_internal_mmc_register_handler(const char *name, char *devicefile,
-				      unsigned long flags, void *handler)
+				      unsigned long flags,
+				      enum filetype expected_type)
 {
 	struct imx_internal_bbu_handler *imx_handler;
 
@@ -476,7 +433,7 @@ imx_bbu_internal_mmc_register_handler(const char *name, char *devicefile,
 				     IMX_BBU_FLAG_KEEP_HEAD);
 	imx_handler->flash_header_offset = FLASH_HEADER_OFFSET_MMC;
 
-	imx_handler->handler.handler = handler;
+	imx_handler->expected_type = expected_type;
 
 	return __register_handler(imx_handler);
 }
@@ -490,8 +447,6 @@ int imx51_bbu_internal_spi_i2c_register_handler(const char *name,
 				     IMX_INTERNAL_FLAG_ERASE);
 	imx_handler->flash_header_offset = FLASH_HEADER_OFFSET_MMC;
 
-	imx_handler->handler.handler = imx_bbu_internal_v1_update;
-
 	return __register_handler(imx_handler);
 }
 
@@ -503,7 +458,7 @@ int imx51_bbu_internal_mmc_register_handler(const char *name, char *devicefile,
 {
 
 	return imx_bbu_internal_mmc_register_handler(name, devicefile, flags,
-						  imx_bbu_internal_v1_update);
+						     filetype_unknown);
 }
 
 /*
@@ -513,7 +468,7 @@ int imx53_bbu_internal_mmc_register_handler(const char *name, char *devicefile,
 		unsigned long flags)
 {
 	return imx_bbu_internal_mmc_register_handler(name, devicefile, flags,
-						  imx_bbu_internal_v2_update);
+						     filetype_imx_image_v2);
 }
 
 /*
@@ -530,7 +485,7 @@ int imx53_bbu_internal_spi_i2c_register_handler(const char *name, char *devicefi
 				     IMX_INTERNAL_FLAG_ERASE);
 	imx_handler->flash_header_offset = FLASH_HEADER_OFFSET_MMC;
 
-	imx_handler->handler.handler = imx_bbu_internal_v2_update;
+	imx_handler->expected_type = filetype_imx_image_v2;
 
 	return __register_handler(imx_handler);
 }
@@ -546,7 +501,7 @@ int imx53_bbu_internal_nand_register_handler(const char *name,
 	imx_handler = __init_handler(name, NULL, flags);
 	imx_handler->flash_header_offset = FLASH_HEADER_OFFSET_MMC;
 
-	imx_handler->handler.handler = imx_bbu_internal_v2_update;
+	imx_handler->expected_type = filetype_imx_image_v2;
 	imx_handler->handler.devicefile = "/dev/nand0";
 	imx_handler->device_size = partition_size;
 	imx_handler->write_device = imx_bbu_internal_v2_write_nand_dbbt;
@@ -607,7 +562,6 @@ int imx_bbu_external_nor_register_handler(const char *name, char *devicefile,
 
 	imx_handler = __init_handler(name, devicefile, flags |
 				     IMX_INTERNAL_FLAG_ERASE);
-	imx_handler->handler.handler = imx_bbu_external_update;
 
 	return __register_handler(imx_handler);
 }
-- 
2.17.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 06/22] ARM: i.MX: bbu: Consolidate vairous update helpers

[Sascha Hauer]

In The subject: s/vairous/various/

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 06/22] ARM: i.MX: bbu: Consolidate vairous update helpers

[Andrey Smirnov]

On Tue, Aug 21, 2018 at 11:52 PM Sascha Hauer <s.hauer@pengutronix.de> wrote:
>
> In The subject: s/vairous/various/
>

Will fix in v2.

Thanks,
Andrey Smirnov

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 07/22] ARM: i.MX: bbu: Simplify imx53_bbu_internal_nand_register_handler()

[Andrey Smirnov]

Second parameter of __init_handler() will initialize
handler.devicefile for us, so there's no need to to it explicitly
after.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 arch/arm/mach-imx/imx-bbu-internal.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/arm/mach-imx/imx-bbu-internal.c b/arch/arm/mach-imx/imx-bbu-internal.c
index 34c8ea92c..9aee102cc 100644
--- a/arch/arm/mach-imx/imx-bbu-internal.c
+++ b/arch/arm/mach-imx/imx-bbu-internal.c
@@ -498,11 +498,10 @@ int imx53_bbu_internal_nand_register_handler(const char *name,
 {
 	struct imx_internal_bbu_handler *imx_handler;
 
-	imx_handler = __init_handler(name, NULL, flags);
+	imx_handler = __init_handler(name, "/dev/nand0", flags);
 	imx_handler->flash_header_offset = FLASH_HEADER_OFFSET_MMC;
 
 	imx_handler->expected_type = filetype_imx_image_v2;
-	imx_handler->handler.devicefile = "/dev/nand0";
 	imx_handler->device_size = partition_size;
 	imx_handler->write_device = imx_bbu_internal_v2_write_nand_dbbt;
 
-- 
2.17.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 08/22] ARM: i.MX: bbu: Constify all 'devicefile' arguments

[Andrey Smirnov]

Constify all 'devicefile' arguments since they are treated as so by
all of the users in BBU code.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 arch/arm/mach-imx/imx-bbu-internal.c | 40 +++++++++++++++++-----------
 arch/arm/mach-imx/include/mach/bbu.h | 40 ++++++++++++++--------------
 2 files changed, 44 insertions(+), 36 deletions(-)

diff --git a/arch/arm/mach-imx/imx-bbu-internal.c b/arch/arm/mach-imx/imx-bbu-internal.c
index 9aee102cc..cff4f6802 100644
--- a/arch/arm/mach-imx/imx-bbu-internal.c
+++ b/arch/arm/mach-imx/imx-bbu-internal.c
@@ -392,8 +392,9 @@ free_bootpartvar:
 	return ret;
 }
 
-static struct imx_internal_bbu_handler *__init_handler(const char *name, char *devicefile,
-		unsigned long flags)
+static struct imx_internal_bbu_handler *__init_handler(const char *name,
+						       const char *devicefile,
+						       unsigned long flags)
 {
 	struct imx_internal_bbu_handler *imx_handler;
 	struct bbu_handler *handler;
@@ -423,7 +424,7 @@ static int __register_handler(struct imx_internal_bbu_handler *imx_handler)
 }
 
 static int
-imx_bbu_internal_mmc_register_handler(const char *name, char *devicefile,
+imx_bbu_internal_mmc_register_handler(const char *name, const char *devicefile,
 				      unsigned long flags,
 				      enum filetype expected_type)
 {
@@ -439,7 +440,7 @@ imx_bbu_internal_mmc_register_handler(const char *name, char *devicefile,
 }
 
 int imx51_bbu_internal_spi_i2c_register_handler(const char *name,
-		char *devicefile, unsigned long flags)
+		const char *devicefile, unsigned long flags)
 {
 	struct imx_internal_bbu_handler *imx_handler;
 
@@ -453,8 +454,9 @@ int imx51_bbu_internal_spi_i2c_register_handler(const char *name,
 /*
  * Register an i.MX51 internal boot update handler for MMC/SD
  */
-int imx51_bbu_internal_mmc_register_handler(const char *name, char *devicefile,
-		unsigned long flags)
+int imx51_bbu_internal_mmc_register_handler(const char *name,
+					    const char *devicefile,
+					    unsigned long flags)
 {
 
 	return imx_bbu_internal_mmc_register_handler(name, devicefile, flags,
@@ -464,8 +466,9 @@ int imx51_bbu_internal_mmc_register_handler(const char *name, char *devicefile,
 /*
  * Register an i.MX53 internal boot update handler for MMC/SD
  */
-int imx53_bbu_internal_mmc_register_handler(const char *name, char *devicefile,
-		unsigned long flags)
+int imx53_bbu_internal_mmc_register_handler(const char *name,
+					    const char *devicefile,
+					    unsigned long flags)
 {
 	return imx_bbu_internal_mmc_register_handler(name, devicefile, flags,
 						     filetype_imx_image_v2);
@@ -476,8 +479,9 @@ int imx53_bbu_internal_mmc_register_handler(const char *name, char *devicefile,
  * EEPROMs / flashes. Nearly the same as MMC/SD, but we do not need to
  * keep a partition table. We have to erase the device beforehand though.
  */
-int imx53_bbu_internal_spi_i2c_register_handler(const char *name, char *devicefile,
-		unsigned long flags)
+int imx53_bbu_internal_spi_i2c_register_handler(const char *name,
+						const char *devicefile,
+						unsigned long flags)
 {
 	struct imx_internal_bbu_handler *imx_handler;
 
@@ -511,14 +515,16 @@ int imx53_bbu_internal_nand_register_handler(const char *name,
 /*
  * Register an i.MX6 internal boot update handler for MMC/SD
  */
-int imx6_bbu_internal_mmc_register_handler(const char *name, char *devicefile,
+int imx6_bbu_internal_mmc_register_handler(const char *name,
+					   const char *devicefile,
 					   unsigned long flags)
 	__alias(imx53_bbu_internal_mmc_register_handler);
 
 /*
  * Register an VF610 internal boot update handler for MMC/SD
  */
-int vf610_bbu_internal_mmc_register_handler(const char *name, char *devicefile,
+int vf610_bbu_internal_mmc_register_handler(const char *name,
+					    const char *devicefile,
 					    unsigned long flags)
 	__alias(imx6_bbu_internal_mmc_register_handler);
 
@@ -531,7 +537,8 @@ int vf610_bbu_internal_mmc_register_handler(const char *name, char *devicefile,
  * Note that no further partitioning of the boot partition is supported up to
  * now.
  */
-int imx6_bbu_internal_mmcboot_register_handler(const char *name, char *devicefile,
+int imx6_bbu_internal_mmcboot_register_handler(const char *name,
+					       const char *devicefile,
 					       unsigned long flags)
 {
 	struct imx_internal_bbu_handler *imx_handler;
@@ -550,12 +557,13 @@ int imx6_bbu_internal_mmcboot_register_handler(const char *name, char *devicefil
  * keep a partition table. We have to erase the device beforehand though.
  */
 int imx6_bbu_internal_spi_i2c_register_handler(const char *name,
-					       char *devicefile,
+					       const char *devicefile,
 					       unsigned long flags)
 	__alias(imx53_bbu_internal_spi_i2c_register_handler);
 
-int imx_bbu_external_nor_register_handler(const char *name, char *devicefile,
-		unsigned long flags)
+int imx_bbu_external_nor_register_handler(const char *name,
+					  const char *devicefile,
+					  unsigned long flags)
 {
 	struct imx_internal_bbu_handler *imx_handler;
 
diff --git a/arch/arm/mach-imx/include/mach/bbu.h b/arch/arm/mach-imx/include/mach/bbu.h
index b9b2c5bcb..b16766c75 100644
--- a/arch/arm/mach-imx/include/mach/bbu.h
+++ b/arch/arm/mach-imx/include/mach/bbu.h
@@ -32,57 +32,57 @@ struct imx_dcd_v2_entry;
 
 #ifdef CONFIG_BAREBOX_UPDATE
 
-int imx51_bbu_internal_mmc_register_handler(const char *name, char *devicefile,
+int imx51_bbu_internal_mmc_register_handler(const char *name, const char *devicefile,
 		unsigned long flags);
 
 int imx51_bbu_internal_spi_i2c_register_handler(const char *name,
-		char *devicefile, unsigned long flags);
+		const char *devicefile, unsigned long flags);
 
-int imx53_bbu_internal_mmc_register_handler(const char *name, char *devicefile,
+int imx53_bbu_internal_mmc_register_handler(const char *name, const char *devicefile,
 		unsigned long flags);
 
-int imx53_bbu_internal_spi_i2c_register_handler(const char *name, char *devicefile,
+int imx53_bbu_internal_spi_i2c_register_handler(const char *name, const char *devicefile,
 		unsigned long flags);
 
 int imx53_bbu_internal_nand_register_handler(const char *name,
 		unsigned long flags, int partition_size);
 
-int imx6_bbu_internal_mmc_register_handler(const char *name, char *devicefile,
+int imx6_bbu_internal_mmc_register_handler(const char *name, const char *devicefile,
 		unsigned long flags);
 
-int imx6_bbu_internal_mmcboot_register_handler(const char *name, char *devicefile,
+int imx6_bbu_internal_mmcboot_register_handler(const char *name, const char *devicefile,
 		unsigned long flags);
 
-int imx6_bbu_internal_spi_i2c_register_handler(const char *name, char *devicefile,
+int imx6_bbu_internal_spi_i2c_register_handler(const char *name, const char *devicefile,
 		unsigned long flags);
 
-int vf610_bbu_internal_mmc_register_handler(const char *name, char *devicefile,
+int vf610_bbu_internal_mmc_register_handler(const char *name, const char *devicefile,
 		unsigned long flags);
 
-int imx_bbu_external_nor_register_handler(const char *name, char *devicefile,
+int imx_bbu_external_nor_register_handler(const char *name, const char *devicefile,
 		unsigned long flags);
 
 #else
 
-static inline int imx51_bbu_internal_mmc_register_handler(const char *name, char *devicefile,
+static inline int imx51_bbu_internal_mmc_register_handler(const char *name, const char *devicefile,
 		unsigned long flags)
 {
 	return -ENOSYS;
 }
 
 static inline int imx51_bbu_internal_spi_i2c_register_handler(const char *name,
-		char *devicefile, unsigned long flags)
+		const char *devicefile, unsigned long flags)
 {
 	return -ENOSYS;
 }
 
-static inline int imx53_bbu_internal_mmc_register_handler(const char *name, char *devicefile,
+static inline int imx53_bbu_internal_mmc_register_handler(const char *name, const char *devicefile,
 		unsigned long flags)
 {
 	return -ENOSYS;
 }
 
-static inline int imx53_bbu_internal_spi_i2c_register_handler(const char *name, char *devicefile,
+static inline int imx53_bbu_internal_spi_i2c_register_handler(const char *name, const char *devicefile,
 		unsigned long flags)
 {
 	return -ENOSYS;
@@ -94,32 +94,32 @@ static inline int imx53_bbu_internal_nand_register_handler(const char *name,
 	return -ENOSYS;
 }
 
-static inline int imx6_bbu_internal_mmc_register_handler(const char *name, char *devicefile,
+static inline int imx6_bbu_internal_mmc_register_handler(const char *name, const char *devicefile,
 		unsigned long flags)
 {
 	return -ENOSYS;
 }
 
 static inline int imx6_bbu_internal_mmcboot_register_handler(const char *name,
-							     char *devicefile,
+							     const char *devicefile,
 							     unsigned long flags)
 {
 	return -ENOSYS;
 }
 
-static inline int imx6_bbu_internal_spi_i2c_register_handler(const char *name, char *devicefile,
+static inline int imx6_bbu_internal_spi_i2c_register_handler(const char *name, const char *devicefile,
 		unsigned long flags)
 {
 	return -ENOSYS;
 }
 
-static inline int vf610_bbu_internal_mmc_register_handler(const char *name, char *devicefile,
+static inline int vf610_bbu_internal_mmc_register_handler(const char *name, const char *devicefile,
 					    unsigned long flags)
 {
 	return -ENOSYS;
 }
 
-static inline int imx_bbu_external_nor_register_handler(const char *name, char *devicefile,
+static inline int imx_bbu_external_nor_register_handler(const char *name, const char *devicefile,
 		unsigned long flags)
 {
 	return -ENOSYS;
@@ -127,10 +127,10 @@ static inline int imx_bbu_external_nor_register_handler(const char *name, char *
 #endif
 
 #if defined(CONFIG_BAREBOX_UPDATE_IMX_EXTERNAL_NAND)
-int imx_bbu_external_nand_register_handler(const char *name, char *devicefile,
+int imx_bbu_external_nand_register_handler(const char *name, const char *devicefile,
 		unsigned long flags);
 #else
-static inline int imx_bbu_external_nand_register_handler(const char *name, char *devicefile,
+static inline int imx_bbu_external_nand_register_handler(const char *name, const char *devicefile,
 		unsigned long flags)
 {
 	return -ENOSYS;
-- 
2.17.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 09/22] ARM: i.MX: bbu: Detect which platforms need v2 i.MX header

[Andrey Smirnov]

Detect which platforms need header v2 based on current CPU type and
replace all of the code explicitly specifying that.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 arch/arm/mach-imx/imx-bbu-internal.c | 29 ++++++++++++++++------------
 1 file changed, 17 insertions(+), 12 deletions(-)

diff --git a/arch/arm/mach-imx/imx-bbu-internal.c b/arch/arm/mach-imx/imx-bbu-internal.c
index cff4f6802..3ef49a859 100644
--- a/arch/arm/mach-imx/imx-bbu-internal.c
+++ b/arch/arm/mach-imx/imx-bbu-internal.c
@@ -29,6 +29,7 @@
 #include <ioctl.h>
 #include <environment.h>
 #include <mach/bbu.h>
+#include <mach/generic.h>
 
 #define FLASH_HEADER_OFFSET_MMC		0x400
 
@@ -332,6 +333,18 @@ out:
 	return ret;
 }
 
+static enum filetype imx_bbu_expected_filetype(void)
+{
+	if (cpu_is_mx8mq() ||
+	    cpu_is_mx7()   ||
+	    cpu_is_mx6()   ||
+	    cpu_is_vf610() ||
+	    cpu_is_mx53())
+		return filetype_imx_image_v2;
+
+	return filetype_unknown;
+}
+
 static int imx_bbu_update(struct bbu_handler *handler, struct bbu_data *data)
 {
 	struct imx_internal_bbu_handler *imx_handler =
@@ -406,7 +419,7 @@ static struct imx_internal_bbu_handler *__init_handler(const char *name,
 	handler->flags = flags;
 	handler->handler = imx_bbu_update;
 
-	imx_handler->expected_type = filetype_unknown;
+	imx_handler->expected_type = imx_bbu_expected_filetype();
 	imx_handler->write_device = __imx_bbu_write_device;
 
 	return imx_handler;
@@ -425,8 +438,7 @@ static int __register_handler(struct imx_internal_bbu_handler *imx_handler)
 
 static int
 imx_bbu_internal_mmc_register_handler(const char *name, const char *devicefile,
-				      unsigned long flags,
-				      enum filetype expected_type)
+				      unsigned long flags)
 {
 	struct imx_internal_bbu_handler *imx_handler;
 
@@ -434,8 +446,6 @@ imx_bbu_internal_mmc_register_handler(const char *name, const char *devicefile,
 				     IMX_BBU_FLAG_KEEP_HEAD);
 	imx_handler->flash_header_offset = FLASH_HEADER_OFFSET_MMC;
 
-	imx_handler->expected_type = expected_type;
-
 	return __register_handler(imx_handler);
 }
 
@@ -459,8 +469,7 @@ int imx51_bbu_internal_mmc_register_handler(const char *name,
 					    unsigned long flags)
 {
 
-	return imx_bbu_internal_mmc_register_handler(name, devicefile, flags,
-						     filetype_unknown);
+	return imx_bbu_internal_mmc_register_handler(name, devicefile, flags);
 }
 
 /*
@@ -470,8 +479,7 @@ int imx53_bbu_internal_mmc_register_handler(const char *name,
 					    const char *devicefile,
 					    unsigned long flags)
 {
-	return imx_bbu_internal_mmc_register_handler(name, devicefile, flags,
-						     filetype_imx_image_v2);
+	return imx_bbu_internal_mmc_register_handler(name, devicefile, flags);
 }
 
 /*
@@ -489,8 +497,6 @@ int imx53_bbu_internal_spi_i2c_register_handler(const char *name,
 				     IMX_INTERNAL_FLAG_ERASE);
 	imx_handler->flash_header_offset = FLASH_HEADER_OFFSET_MMC;
 
-	imx_handler->expected_type = filetype_imx_image_v2;
-
 	return __register_handler(imx_handler);
 }
 
@@ -505,7 +511,6 @@ int imx53_bbu_internal_nand_register_handler(const char *name,
 	imx_handler = __init_handler(name, "/dev/nand0", flags);
 	imx_handler->flash_header_offset = FLASH_HEADER_OFFSET_MMC;
 
-	imx_handler->expected_type = filetype_imx_image_v2;
 	imx_handler->device_size = partition_size;
 	imx_handler->write_device = imx_bbu_internal_v2_write_nand_dbbt;
 
-- 
2.17.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 10/22] ARM: i.MX: bbu: Alias imx5*_bbu_internal_mmc_register_handler()

[Andrey Smirnov]

Alias imx5*_bbu_internal_mmc_register_handler() to
imx_bbu_internal_mmc_register_handler()

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 arch/arm/mach-imx/imx-bbu-internal.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/arch/arm/mach-imx/imx-bbu-internal.c b/arch/arm/mach-imx/imx-bbu-internal.c
index 3ef49a859..e7fbe3e16 100644
--- a/arch/arm/mach-imx/imx-bbu-internal.c
+++ b/arch/arm/mach-imx/imx-bbu-internal.c
@@ -467,10 +467,7 @@ int imx51_bbu_internal_spi_i2c_register_handler(const char *name,
 int imx51_bbu_internal_mmc_register_handler(const char *name,
 					    const char *devicefile,
 					    unsigned long flags)
-{
-
-	return imx_bbu_internal_mmc_register_handler(name, devicefile, flags);
-}
+	__alias(imx_bbu_internal_mmc_register_handler);
 
 /*
  * Register an i.MX53 internal boot update handler for MMC/SD
@@ -478,9 +475,7 @@ int imx51_bbu_internal_mmc_register_handler(const char *name,
 int imx53_bbu_internal_mmc_register_handler(const char *name,
 					    const char *devicefile,
 					    unsigned long flags)
-{
-	return imx_bbu_internal_mmc_register_handler(name, devicefile, flags);
-}
+	__alias(imx_bbu_internal_mmc_register_handler);
 
 /*
  * Register an i.MX6 internal boot update handler for i2c/spi
-- 
2.17.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 11/22] ARM: i.MX: bbu: Alias imx5*_bbu_internal_spi_i2c_register_handler()

[Andrey Smirnov]

Alias imx5*_bbu_internal_spi_i2c_register_handler() to a common function.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 arch/arm/mach-imx/imx-bbu-internal.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/arch/arm/mach-imx/imx-bbu-internal.c b/arch/arm/mach-imx/imx-bbu-internal.c
index e7fbe3e16..318efd88e 100644
--- a/arch/arm/mach-imx/imx-bbu-internal.c
+++ b/arch/arm/mach-imx/imx-bbu-internal.c
@@ -449,8 +449,10 @@ imx_bbu_internal_mmc_register_handler(const char *name, const char *devicefile,
 	return __register_handler(imx_handler);
 }
 
-int imx51_bbu_internal_spi_i2c_register_handler(const char *name,
-		const char *devicefile, unsigned long flags)
+static int
+imx_bbu_internal_spi_i2c_register_handler(const char *name,
+					  const char *devicefile,
+					  unsigned long flags)
 {
 	struct imx_internal_bbu_handler *imx_handler;
 
@@ -461,6 +463,11 @@ int imx51_bbu_internal_spi_i2c_register_handler(const char *name,
 	return __register_handler(imx_handler);
 }
 
+int imx51_bbu_internal_spi_i2c_register_handler(const char *name,
+						const char *devicefile,
+						unsigned long flags)
+	__alias(imx_bbu_internal_spi_i2c_register_handler);
+
 /*
  * Register an i.MX51 internal boot update handler for MMC/SD
  */
@@ -485,15 +492,7 @@ int imx53_bbu_internal_mmc_register_handler(const char *name,
 int imx53_bbu_internal_spi_i2c_register_handler(const char *name,
 						const char *devicefile,
 						unsigned long flags)
-{
-	struct imx_internal_bbu_handler *imx_handler;
-
-	imx_handler = __init_handler(name, devicefile, flags |
-				     IMX_INTERNAL_FLAG_ERASE);
-	imx_handler->flash_header_offset = FLASH_HEADER_OFFSET_MMC;
-
-	return __register_handler(imx_handler);
-}
+	__alias(imx_bbu_internal_spi_i2c_register_handler);
 
 /*
  * Register an i.MX53 internal boot update handler for NAND
-- 
2.17.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 12/22] ARM: i.MX: bbu: Move protect code into a separate routine

[Andrey Smirnov]

Both code sections responsible for enabling/disabling underlying
device write protection are almost identical to each other. To avoid
code duplication move them into a separate routine and adjust the rest
of the code accordignly.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 arch/arm/mach-imx/imx-bbu-internal.c | 62 +++++++++++++++++++---------
 1 file changed, 42 insertions(+), 20 deletions(-)

diff --git a/arch/arm/mach-imx/imx-bbu-internal.c b/arch/arm/mach-imx/imx-bbu-internal.c
index 318efd88e..eb1c459b2 100644
--- a/arch/arm/mach-imx/imx-bbu-internal.c
+++ b/arch/arm/mach-imx/imx-bbu-internal.c
@@ -44,6 +44,41 @@ struct imx_internal_bbu_handler {
 	enum filetype expected_type;
 };
 
+static bool
+imx_bbu_erase_required(struct imx_internal_bbu_handler *imx_handler)
+{
+	return imx_handler->handler.flags & IMX_INTERNAL_FLAG_ERASE;
+}
+
+static int imx_bbu_protect(int fd, struct imx_internal_bbu_handler *imx_handler,
+			   const char *devicefile, int offset, int image_len,
+			   int prot)
+{
+	const char *prefix = prot ? "" : "un";
+	int ret;
+
+	if (!imx_bbu_erase_required(imx_handler))
+		return 0;
+
+	pr_debug("%s: %sprotecting %s from 0x%08x to 0x%08x\n", __func__,
+		 prefix, devicefile, offset, image_len);
+
+	ret = protect(fd, image_len, offset, prot);
+	if (ret) {
+		/*
+		 * If protect() is not implemented for this device,
+		 * just report success
+		 */
+		if (ret == -ENOSYS)
+			return 0;
+
+		pr_err("%sprotecting %s failed with %s\n", prefix, devicefile,
+		       strerror(-ret));
+	}
+
+	return ret;
+}
+
 /*
  * Actually write an image to the target device, eventually keeping a
  * DOS partition table on the device
@@ -67,16 +102,12 @@ static int imx_bbu_write_device(struct imx_internal_bbu_handler *imx_handler,
 	if (imx_handler->handler.flags & IMX_BBU_FLAG_KEEP_HEAD)
 		offset += imx_handler->flash_header_offset;
 
-	if (imx_handler->handler.flags & IMX_INTERNAL_FLAG_ERASE) {
-		pr_debug("%s: unprotecting %s from 0x%08x to 0x%08x\n", __func__,
-				devicefile, offset, image_len);
-		ret = protect(fd, image_len, offset, 0);
-		if (ret && ret != -ENOSYS) {
-			pr_err("unprotecting %s failed with %s\n", devicefile,
-					strerror(-ret));
-			goto err_close;
-		}
+	ret = imx_bbu_protect(fd, imx_handler, devicefile, offset,
+			      image_len, 0);
+	if (ret)
+		goto err_close;
 
+	if (imx_bbu_erase_required(imx_handler)) {
 		pr_debug("%s: erasing %s from 0x%08x to 0x%08x\n", __func__,
 				devicefile, offset, image_len);
 		ret = erase(fd, image_len, offset);
@@ -91,17 +122,8 @@ static int imx_bbu_write_device(struct imx_internal_bbu_handler *imx_handler,
 	if (ret < 0)
 		goto err_close;
 
-	if (imx_handler->handler.flags & IMX_INTERNAL_FLAG_ERASE) {
-		pr_debug("%s: protecting %s from 0x%08x to 0x%08x\n", __func__,
-				devicefile, offset, image_len);
-		ret = protect(fd, image_len, offset, 1);
-		if (ret && ret != -ENOSYS) {
-			pr_err("protecting %s failed with %s\n", devicefile,
-					strerror(-ret));
-		}
-	}
-
-	ret = 0;
+	imx_bbu_protect(fd, imx_handler, devicefile, offset,
+			image_len, 1);
 
 err_close:
 	close(fd);
-- 
2.17.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 13/22] ARM: i.MX: bbu: Adjust FLASH_HEADER_OFFSET_MMC for i.MX8MQ

[Andrey Smirnov]

Flash header is located at 33KiB mark on i.MX8MQ, so we need to take
that into account when initializing imx_handler->flash_header_offset.

Convert FLASH_HEADER_OFFSET_MMC into a function call that will check
if Barebox is running on i.MX8MQ CPU and adjust the offset
accordingly.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 arch/arm/mach-imx/imx-bbu-internal.c | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/arch/arm/mach-imx/imx-bbu-internal.c b/arch/arm/mach-imx/imx-bbu-internal.c
index eb1c459b2..9b12c8bdd 100644
--- a/arch/arm/mach-imx/imx-bbu-internal.c
+++ b/arch/arm/mach-imx/imx-bbu-internal.c
@@ -31,8 +31,6 @@
 #include <mach/bbu.h>
 #include <mach/generic.h>
 
-#define FLASH_HEADER_OFFSET_MMC		0x400
-
 #define IMX_INTERNAL_FLAG_ERASE		BIT(30)
 
 struct imx_internal_bbu_handler {
@@ -367,6 +365,20 @@ static enum filetype imx_bbu_expected_filetype(void)
 	return filetype_unknown;
 }
 
+static unsigned long imx_bbu_flash_header_offset_mmc(void)
+{
+	unsigned long offset = SZ_1K;
+
+	/*
+	 * i.MX8MQ moved the header by 32K to accomodate for GPT
+	 * partition tables
+	 */
+	if (cpu_is_mx8mq())
+		offset += SZ_32K;
+
+	return offset;
+}
+
 static int imx_bbu_update(struct bbu_handler *handler, struct bbu_data *data)
 {
 	struct imx_internal_bbu_handler *imx_handler =
@@ -466,7 +478,7 @@ imx_bbu_internal_mmc_register_handler(const char *name, const char *devicefile,
 
 	imx_handler = __init_handler(name, devicefile, flags |
 				     IMX_BBU_FLAG_KEEP_HEAD);
-	imx_handler->flash_header_offset = FLASH_HEADER_OFFSET_MMC;
+	imx_handler->flash_header_offset = imx_bbu_flash_header_offset_mmc();
 
 	return __register_handler(imx_handler);
 }
@@ -480,7 +492,7 @@ imx_bbu_internal_spi_i2c_register_handler(const char *name,
 
 	imx_handler = __init_handler(name, devicefile, flags |
 				     IMX_INTERNAL_FLAG_ERASE);
-	imx_handler->flash_header_offset = FLASH_HEADER_OFFSET_MMC;
+	imx_handler->flash_header_offset = imx_bbu_flash_header_offset_mmc();
 
 	return __register_handler(imx_handler);
 }
@@ -525,7 +537,7 @@ int imx53_bbu_internal_nand_register_handler(const char *name,
 	struct imx_internal_bbu_handler *imx_handler;
 
 	imx_handler = __init_handler(name, "/dev/nand0", flags);
-	imx_handler->flash_header_offset = FLASH_HEADER_OFFSET_MMC;
+	imx_handler->flash_header_offset = imx_bbu_flash_header_offset_mmc();
 
 	imx_handler->device_size = partition_size;
 	imx_handler->write_device = imx_bbu_internal_v2_write_nand_dbbt;
@@ -565,7 +577,7 @@ int imx6_bbu_internal_mmcboot_register_handler(const char *name,
 	struct imx_internal_bbu_handler *imx_handler;
 
 	imx_handler = __init_handler(name, devicefile, flags);
-	imx_handler->flash_header_offset = FLASH_HEADER_OFFSET_MMC;
+	imx_handler->flash_header_offset = imx_bbu_flash_header_offset_mmc();
 
 	imx_handler->handler.handler = imx_bbu_internal_v2_mmcboot_update;
 
-- 
2.17.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 14/22] ARM: i.MX: bbu: Add support for SPI/I2C on VFxxx

[Andrey Smirnov]

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 arch/arm/mach-imx/imx-bbu-internal.c | 10 ++++++++++
 arch/arm/mach-imx/include/mach/bbu.h | 11 +++++++++++
 2 files changed, 21 insertions(+)

diff --git a/arch/arm/mach-imx/imx-bbu-internal.c b/arch/arm/mach-imx/imx-bbu-internal.c
index 9b12c8bdd..dcabb8625 100644
--- a/arch/arm/mach-imx/imx-bbu-internal.c
+++ b/arch/arm/mach-imx/imx-bbu-internal.c
@@ -594,6 +594,16 @@ int imx6_bbu_internal_spi_i2c_register_handler(const char *name,
 					       unsigned long flags)
 	__alias(imx53_bbu_internal_spi_i2c_register_handler);
 
+/*
+ * Register an VFxxx internal boot update handler for i2c/spi
+ * EEPROMs / flashes. Nearly the same as MMC/SD, but we do not need to
+ * keep a partition table. We have to erase the device beforehand though.
+ */
+int vf610_bbu_internal_spi_i2c_register_handler(const char *name,
+						const char *devicefile,
+						unsigned long flags)
+	__alias(imx6_bbu_internal_spi_i2c_register_handler);
+
 int imx_bbu_external_nor_register_handler(const char *name,
 					  const char *devicefile,
 					  unsigned long flags)
diff --git a/arch/arm/mach-imx/include/mach/bbu.h b/arch/arm/mach-imx/include/mach/bbu.h
index b16766c75..d543d0e92 100644
--- a/arch/arm/mach-imx/include/mach/bbu.h
+++ b/arch/arm/mach-imx/include/mach/bbu.h
@@ -59,6 +59,9 @@ int imx6_bbu_internal_spi_i2c_register_handler(const char *name, const char *dev
 int vf610_bbu_internal_mmc_register_handler(const char *name, const char *devicefile,
 		unsigned long flags);
 
+int vf610_bbu_internal_spi_i2c_register_handler(const char *name, const char *devicefile,
+						unsigned long flags);
+
 int imx_bbu_external_nor_register_handler(const char *name, const char *devicefile,
 		unsigned long flags);
 
@@ -124,6 +127,14 @@ static inline int imx_bbu_external_nor_register_handler(const char *name, const
 {
 	return -ENOSYS;
 }
+
+static inline int
+vf610_bbu_internal_spi_i2c_register_handler(const char *name, char *devicefile,
+					    unsigned long flags)
+{
+	return -ENOSYS;
+}
+
 #endif
 
 #if defined(CONFIG_BAREBOX_UPDATE_IMX_EXTERNAL_NAND)
-- 
2.17.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 15/22] ARM: i.MX: zii-vf610-dev-rev-b/c: Add support for BBU on SPI-NOR

[Andrey Smirnov]

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 arch/arm/boards/zii-vf610-dev/board.c | 19 +++++++++++++++++++
 arch/arm/dts/vf610-zii-dev-rev-b.dts  | 11 +++++++++++
 2 files changed, 30 insertions(+)

diff --git a/arch/arm/boards/zii-vf610-dev/board.c b/arch/arm/boards/zii-vf610-dev/board.c
index d88b26164..818044ce6 100644
--- a/arch/arm/boards/zii-vf610-dev/board.c
+++ b/arch/arm/boards/zii-vf610-dev/board.c
@@ -149,6 +149,25 @@ static int zii_vf610_dev_set_hostname(void)
 }
 late_initcall(zii_vf610_dev_set_hostname);
 
+static int zii_vf610_dev_register_bbu(void)
+{
+	int ret;
+	if (!of_machine_is_compatible("zii,vf610dev-c") &&
+	    !of_machine_is_compatible("zii,vf610dev-b"))
+		return 0;
+
+	ret = vf610_bbu_internal_spi_i2c_register_handler("SPI",
+							  "/dev/m25p0.bootloader",
+							  0);
+	if (ret) {
+		pr_err("Failed to register SPI BBU handler");
+		return ret;
+	}
+
+	return 0;
+}
+late_initcall(zii_vf610_dev_register_bbu);
+
 static int zii_vf610_spu3_register_bbu(void)
 {
 	int ret;
diff --git a/arch/arm/dts/vf610-zii-dev-rev-b.dts b/arch/arm/dts/vf610-zii-dev-rev-b.dts
index 1eb01f44a..ac0807c49 100644
--- a/arch/arm/dts/vf610-zii-dev-rev-b.dts
+++ b/arch/arm/dts/vf610-zii-dev-rev-b.dts
@@ -45,3 +45,14 @@
 #include <arm/vf610-zii-dev-rev-b.dts>
 
 #include "vf610-zii-dev.dtsi"
+
+/ {
+	spi0 {
+		m25p128@0 {
+			partition@0 {
+				label = "bootloader";
+				reg = <0x0 0x100000>;
+			};
+		};
+	};
+};
-- 
2.17.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 16/22] ARM: i.MX: bbu: Add support for MMC on i.MX8MQ

[Andrey Smirnov]

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 arch/arm/mach-imx/imx-bbu-internal.c | 8 ++++++++
 arch/arm/mach-imx/include/mach/bbu.h | 9 +++++++++
 2 files changed, 17 insertions(+)

diff --git a/arch/arm/mach-imx/imx-bbu-internal.c b/arch/arm/mach-imx/imx-bbu-internal.c
index dcabb8625..d83eb972c 100644
--- a/arch/arm/mach-imx/imx-bbu-internal.c
+++ b/arch/arm/mach-imx/imx-bbu-internal.c
@@ -561,6 +561,14 @@ int vf610_bbu_internal_mmc_register_handler(const char *name,
 					    unsigned long flags)
 	__alias(imx6_bbu_internal_mmc_register_handler);
 
+/*
+ * Register an i.MX8MQ internal boot update handler for MMC/SD
+ */
+int imx8mq_bbu_internal_mmc_register_handler(const char *name,
+					     const char *devicefile,
+					     unsigned long flags)
+	__alias(imx6_bbu_internal_mmc_register_handler);
+
 /*
  * Register a handler that writes to the non-active boot partition of an mmc
  * medium and on success activates the written-to partition. So the machine can
diff --git a/arch/arm/mach-imx/include/mach/bbu.h b/arch/arm/mach-imx/include/mach/bbu.h
index d543d0e92..b30403328 100644
--- a/arch/arm/mach-imx/include/mach/bbu.h
+++ b/arch/arm/mach-imx/include/mach/bbu.h
@@ -62,6 +62,9 @@ int vf610_bbu_internal_mmc_register_handler(const char *name, const char *device
 int vf610_bbu_internal_spi_i2c_register_handler(const char *name, const char *devicefile,
 						unsigned long flags);
 
+int imx8mq_bbu_internal_mmc_register_handler(const char *name, const char *devicefile,
+					     unsigned long flags);
+
 int imx_bbu_external_nor_register_handler(const char *name, const char *devicefile,
 		unsigned long flags);
 
@@ -122,6 +125,12 @@ static inline int vf610_bbu_internal_mmc_register_handler(const char *name, cons
 	return -ENOSYS;
 }
 
+static inline int imx8mq_bbu_internal_mmc_register_handler(const char *name, const char *devicefile,
+							   unsigned long flags);
+{
+	return -ENOSYS;
+}
+
 static inline int imx_bbu_external_nor_register_handler(const char *name, const char *devicefile,
 		unsigned long flags)
 {
-- 
2.17.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 17/22] ARM: nxp-imx8mq-evk: Add eMMC BBU configuration

[Andrey Smirnov]

Add BBU configuration to allow programming Barebox to eMMC. Normally
factory U-Boot is shipped in mmc0.boot0, so us using user partition
for Barebox should allow those two to coexist.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 arch/arm/boards/nxp-imx8mq-evk/board.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm/boards/nxp-imx8mq-evk/board.c b/arch/arm/boards/nxp-imx8mq-evk/board.c
index d93e21da1..868c25ebb 100644
--- a/arch/arm/boards/nxp-imx8mq-evk/board.c
+++ b/arch/arm/boards/nxp-imx8mq-evk/board.c
@@ -21,6 +21,7 @@
 #include <init.h>
 #include <asm/memory.h>
 #include <linux/sizes.h>
+#include <mach/bbu.h>
 
 static int imx8mq_evk_mem_init(void)
 {
@@ -39,6 +40,8 @@ static int nxp_imx8mq_evk_init(void)
 
 	barebox_set_hostname("imx8mq-evk");
 
+	imx8mq_bbu_internal_mmc_register_handler("eMMC", "/dev/mmc0", 0);
+
 	return 0;
 }
 device_initcall(nxp_imx8mq_evk_init);
-- 
2.17.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 18/22] ARM: i.MX: bbu: Adjust error code check for pwrite()

[Andrey Smirnov]

Pwrite() will return the amount bytes written or negative error code
on success, so we need to do two things with it:

    1. Check it against "image_len" to make sure we actually wrote all
       of the data

    2. Set it to zero in case of success, since that is what code in
       barebox_update() expects to happen

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 arch/arm/mach-imx/imx-bbu-internal.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-imx/imx-bbu-internal.c b/arch/arm/mach-imx/imx-bbu-internal.c
index d83eb972c..70af5ef84 100644
--- a/arch/arm/mach-imx/imx-bbu-internal.c
+++ b/arch/arm/mach-imx/imx-bbu-internal.c
@@ -86,6 +86,7 @@ static int imx_bbu_write_device(struct imx_internal_bbu_handler *imx_handler,
 		const void *buf, int image_len)
 {
 	int fd, ret, offset = 0;
+	bool partial_write;
 
 	fd = open(devicefile, O_RDWR | O_CREAT);
 	if (fd < 0)
@@ -117,8 +118,14 @@ static int imx_bbu_write_device(struct imx_internal_bbu_handler *imx_handler,
 	}
 
 	ret = pwrite(fd, buf, image_len, offset);
-	if (ret < 0)
+	partial_write = ret > 0 && ret != image_len;
+	if (ret < 0 || partial_write) {
+		ret = partial_write ? -EIO : ret;
+
+		pr_err("writing to %s failed with %s\n", devicefile,
+		       strerror(-ret));
 		goto err_close;
+	}
 
 	imx_bbu_protect(fd, imx_handler, devicefile, offset,
 			image_len, 1);
@@ -126,7 +133,7 @@ static int imx_bbu_write_device(struct imx_internal_bbu_handler *imx_handler,
 err_close:
 	close(fd);
 
-	return ret;
+	return ret < 0 ? ret : 0;
 }
 
 static int __imx_bbu_write_device(struct imx_internal_bbu_handler *imx_handler,
-- 
2.17.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 18/22] ARM: i.MX: bbu: Adjust error code check for pwrite()

[Sascha Hauer]

On Mon, Aug 20, 2018 at 11:25:59PM -0700, Andrey Smirnov wrote:
> Pwrite() will return the amount bytes written or negative error code
> on success, so we need to do two things with it:
> 
>     1. Check it against "image_len" to make sure we actually wrote all
>        of the data
> 
>     2. Set it to zero in case of success, since that is what code in
>        barebox_update() expects to happen
> 
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> ---
>  arch/arm/mach-imx/imx-bbu-internal.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/mach-imx/imx-bbu-internal.c b/arch/arm/mach-imx/imx-bbu-internal.c
> index d83eb972c..70af5ef84 100644
> --- a/arch/arm/mach-imx/imx-bbu-internal.c
> +++ b/arch/arm/mach-imx/imx-bbu-internal.c
> @@ -86,6 +86,7 @@ static int imx_bbu_write_device(struct imx_internal_bbu_handler *imx_handler,
>  		const void *buf, int image_len)
>  {
>  	int fd, ret, offset = 0;
> +	bool partial_write;
>  
>  	fd = open(devicefile, O_RDWR | O_CREAT);
>  	if (fd < 0)
> @@ -117,8 +118,14 @@ static int imx_bbu_write_device(struct imx_internal_bbu_handler *imx_handler,
>  	}
>  
>  	ret = pwrite(fd, buf, image_len, offset);
> -	if (ret < 0)
> +	partial_write = ret > 0 && ret != image_len;
> +	if (ret < 0 || partial_write) {
> +		ret = partial_write ? -EIO : ret;
> +
> +		pr_err("writing to %s failed with %s\n", devicefile,
> +		       strerror(-ret));
>  		goto err_close;

Do we need a pwrite_full analog to write_full?

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 18/22] ARM: i.MX: bbu: Adjust error code check for pwrite()

[Andrey Smirnov]

On Wed, Aug 22, 2018 at 12:01 AM Sascha Hauer <s.hauer@pengutronix.de> wrote:
>
> On Mon, Aug 20, 2018 at 11:25:59PM -0700, Andrey Smirnov wrote:
> > Pwrite() will return the amount bytes written or negative error code
> > on success, so we need to do two things with it:
> >
> >     1. Check it against "image_len" to make sure we actually wrote all
> >        of the data
> >
> >     2. Set it to zero in case of success, since that is what code in
> >        barebox_update() expects to happen
> >
> > Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> > ---
> >  arch/arm/mach-imx/imx-bbu-internal.c | 11 +++++++++--
> >  1 file changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm/mach-imx/imx-bbu-internal.c b/arch/arm/mach-imx/imx-bbu-internal.c
> > index d83eb972c..70af5ef84 100644
> > --- a/arch/arm/mach-imx/imx-bbu-internal.c
> > +++ b/arch/arm/mach-imx/imx-bbu-internal.c
> > @@ -86,6 +86,7 @@ static int imx_bbu_write_device(struct imx_internal_bbu_handler *imx_handler,
> >               const void *buf, int image_len)
> >  {
> >       int fd, ret, offset = 0;
> > +     bool partial_write;
> >
> >       fd = open(devicefile, O_RDWR | O_CREAT);
> >       if (fd < 0)
> > @@ -117,8 +118,14 @@ static int imx_bbu_write_device(struct imx_internal_bbu_handler *imx_handler,
> >       }
> >
> >       ret = pwrite(fd, buf, image_len, offset);
> > -     if (ret < 0)
> > +     partial_write = ret > 0 && ret != image_len;
> > +     if (ret < 0 || partial_write) {
> > +             ret = partial_write ? -EIO : ret;
> > +
> > +             pr_err("writing to %s failed with %s\n", devicefile,
> > +                    strerror(-ret));
> >               goto err_close;
>
> Do we need a pwrite_full analog to write_full?
>

Sure, why not? Should allow me to drop that partial_write variable.
Will do in v2.

Thanks,
Andrey Smirnov

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 19/22] bbu: Remove logical negation in barebox_update_handler_exists()

[Andrey Smirnov]

Returning !bbu_find_handler() from barebox_update_handler_exists()
would return the opposite result from what the name of that funciton
implies. Drop the "!" to make it behave as expected.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 common/bbu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/common/bbu.c b/common/bbu.c
index 11e44f4a7..69ccac68a 100644
--- a/common/bbu.c
+++ b/common/bbu.c
@@ -151,7 +151,7 @@ bool barebox_update_handler_exists(struct bbu_data *data)
 	if (!data->handler_name)
 		return false;
 
-	return !bbu_find_handler(data->handler_name);
+	return bbu_find_handler(data->handler_name);
 }
 
 static int bbu_check_of_compat(struct bbu_data *data)
-- 
2.17.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 19/22] bbu: Remove logical negation in barebox_update_handler_exists()

[Sascha Hauer]

On Mon, Aug 20, 2018 at 11:26:00PM -0700, Andrey Smirnov wrote:
> Returning !bbu_find_handler() from barebox_update_handler_exists()
> would return the opposite result from what the name of that funciton
> implies. Drop the "!" to make it behave as expected.
> 
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> ---
>  common/bbu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/common/bbu.c b/common/bbu.c
> index 11e44f4a7..69ccac68a 100644
> --- a/common/bbu.c
> +++ b/common/bbu.c
> @@ -151,7 +151,7 @@ bool barebox_update_handler_exists(struct bbu_data *data)
>  	if (!data->handler_name)
>  		return false;
>  
> -	return !bbu_find_handler(data->handler_name);
> +	return bbu_find_handler(data->handler_name);

As bbu_find_handler() returns a pointer maybe better '!!' or
bbu_find_handler() != NULL?

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 19/22] bbu: Remove logical negation in barebox_update_handler_exists()

[Andrey Smirnov]

On Wed, Aug 22, 2018 at 12:09 AM Sascha Hauer <s.hauer@pengutronix.de> wrote:
>
> On Mon, Aug 20, 2018 at 11:26:00PM -0700, Andrey Smirnov wrote:
> > Returning !bbu_find_handler() from barebox_update_handler_exists()
> > would return the opposite result from what the name of that funciton
> > implies. Drop the "!" to make it behave as expected.
> >
> > Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> > ---
> >  common/bbu.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/common/bbu.c b/common/bbu.c
> > index 11e44f4a7..69ccac68a 100644
> > --- a/common/bbu.c
> > +++ b/common/bbu.c
> > @@ -151,7 +151,7 @@ bool barebox_update_handler_exists(struct bbu_data *data)
> >       if (!data->handler_name)
> >               return false;
> >
> > -     return !bbu_find_handler(data->handler_name);
> > +     return bbu_find_handler(data->handler_name);
>
> As bbu_find_handler() returns a pointer maybe better '!!' or
> bbu_find_handler() != NULL?
>

That shouldn't be necessary since barebox_update_handler_exists()
returns bool(real type name is _Bool) which explicitly specifies that
a cast of any scalar value to it would be normalized to 1 or 0 (as per
C99 standard from whence it came). Otherwise you'd be able to end up
in a situation where bool1 && bool2 && (bool1 != bool2) evaluates to
true.

To give you more concrete example, here's what the last portion of
that function compiles to on AArch64:

    2924: 97ffff5e     bl 269c <bbu_find_handler>
    2928: f100001f   cmp x0, #0x0
    292c: 1a9f07e0  cset w0, ne  // ne = any
    2930: f9400bf3   ldr x19, [sp, #16]
    2934: a8c27bfd  ldp x29, x30, [sp], #32
    2938: d65f03c0  ret
    293c: 52800020 mov w0, #0x1                    // #1
    2940: 17fffffc      b 2930 <barebox_update_handler_exists+0x30>
    2944: 52800000 mov w0, #0x0                    // #0
    2948: 17fffffa      b 2930 <barebox_update_handler_exists+0x30>

and on AArch32 (Thumb):

    18e0: f7ff ff48 bl 1774 <bbu_find_handler>
    18e4: 3000     adds r0, #0
    18e6: bf18      it ne
    18e8: 2001     movne r0, #1
    18ea: bd10     pop {r4, pc}
    18ec: 2001     movs r0, #1
    18ee: e7fc      b.n 18ea <barebox_update_handler_exists+0x1a>

as you can see both cases already have code to explicitly convert the
result of the function to 0/1.

I am more than happy to add ether !! or != NULL if you still think
that'd be better, it just I don't think it will have any practical
effect.

Thanks,
Andrey Smirnov

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 19/22] bbu: Remove logical negation in barebox_update_handler_exists()

[Sam Ravnborg]

Hi Andrey.

> > >
> > > -     return !bbu_find_handler(data->handler_name);
> > > +     return bbu_find_handler(data->handler_name);
> >
> > As bbu_find_handler() returns a pointer maybe better '!!' or
> > bbu_find_handler() != NULL?
> >
> 
> That shouldn't be necessary

At least to me the xxx != NULL would express the intent in a more clear way.
But then if this becomes a common pattern then I will also learn that.
But you got my preference.

	Sam

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 19/22] bbu: Remove logical negation in barebox_update_handler_exists()

[Sascha Hauer]

On Wed, Aug 22, 2018 at 05:01:41PM -0700, Andrey Smirnov wrote:
> On Wed, Aug 22, 2018 at 12:09 AM Sascha Hauer <s.hauer@pengutronix.de> wrote:
> >
> > On Mon, Aug 20, 2018 at 11:26:00PM -0700, Andrey Smirnov wrote:
> > > Returning !bbu_find_handler() from barebox_update_handler_exists()
> > > would return the opposite result from what the name of that funciton
> > > implies. Drop the "!" to make it behave as expected.
> > >
> > > Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> > > ---
> > >  common/bbu.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/common/bbu.c b/common/bbu.c
> > > index 11e44f4a7..69ccac68a 100644
> > > --- a/common/bbu.c
> > > +++ b/common/bbu.c
> > > @@ -151,7 +151,7 @@ bool barebox_update_handler_exists(struct bbu_data *data)
> > >       if (!data->handler_name)
> > >               return false;
> > >
> > > -     return !bbu_find_handler(data->handler_name);
> > > +     return bbu_find_handler(data->handler_name);
> >
> > As bbu_find_handler() returns a pointer maybe better '!!' or
> > bbu_find_handler() != NULL?
> >
> 
> That shouldn't be necessary since barebox_update_handler_exists()
> returns bool(real type name is _Bool) which explicitly specifies that
> a cast of any scalar value to it would be normalized to 1 or 0 (as per
> C99 standard from whence it came). Otherwise you'd be able to end up
> in a situation where bool1 && bool2 && (bool1 != bool2) evaluates to
> true.
> 
> To give you more concrete example, here's what the last portion of
> that function compiles to on AArch64:
> 
>     2924: 97ffff5e     bl 269c <bbu_find_handler>
>     2928: f100001f   cmp x0, #0x0
>     292c: 1a9f07e0  cset w0, ne  // ne = any
>     2930: f9400bf3   ldr x19, [sp, #16]
>     2934: a8c27bfd  ldp x29, x30, [sp], #32
>     2938: d65f03c0  ret
>     293c: 52800020 mov w0, #0x1                    // #1
>     2940: 17fffffc      b 2930 <barebox_update_handler_exists+0x30>
>     2944: 52800000 mov w0, #0x0                    // #0
>     2948: 17fffffa      b 2930 <barebox_update_handler_exists+0x30>
> 
> and on AArch32 (Thumb):
> 
>     18e0: f7ff ff48 bl 1774 <bbu_find_handler>
>     18e4: 3000     adds r0, #0
>     18e6: bf18      it ne
>     18e8: 2001     movne r0, #1
>     18ea: bd10     pop {r4, pc}
>     18ec: 2001     movs r0, #1
>     18ee: e7fc      b.n 18ea <barebox_update_handler_exists+0x1a>
> 
> as you can see both cases already have code to explicitly convert the
> result of the function to 0/1.
> 
> I am more than happy to add ether !! or != NULL if you still think
> that'd be better, it just I don't think it will have any practical
> effect.

For sure it doesn't have a practical effect, it's merely a sign to show
"I know I'm casting a pointer to bool". Probably I'm just used to add
an explicit cast there and it's just a matter of taste.

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 19/22] bbu: Remove logical negation in barebox_update_handler_exists()

[Andrey Smirnov]

On Wed, Aug 22, 2018 at 11:42 PM Sascha Hauer <s.hauer@pengutronix.de> wrote:
>
> On Wed, Aug 22, 2018 at 05:01:41PM -0700, Andrey Smirnov wrote:
> > On Wed, Aug 22, 2018 at 12:09 AM Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > >
> > > On Mon, Aug 20, 2018 at 11:26:00PM -0700, Andrey Smirnov wrote:
> > > > Returning !bbu_find_handler() from barebox_update_handler_exists()
> > > > would return the opposite result from what the name of that funciton
> > > > implies. Drop the "!" to make it behave as expected.
> > > >
> > > > Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> > > > ---
> > > >  common/bbu.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/common/bbu.c b/common/bbu.c
> > > > index 11e44f4a7..69ccac68a 100644
> > > > --- a/common/bbu.c
> > > > +++ b/common/bbu.c
> > > > @@ -151,7 +151,7 @@ bool barebox_update_handler_exists(struct bbu_data *data)
> > > >       if (!data->handler_name)
> > > >               return false;
> > > >
> > > > -     return !bbu_find_handler(data->handler_name);
> > > > +     return bbu_find_handler(data->handler_name);
> > >
> > > As bbu_find_handler() returns a pointer maybe better '!!' or
> > > bbu_find_handler() != NULL?
> > >
> >
> > That shouldn't be necessary since barebox_update_handler_exists()
> > returns bool(real type name is _Bool) which explicitly specifies that
> > a cast of any scalar value to it would be normalized to 1 or 0 (as per
> > C99 standard from whence it came). Otherwise you'd be able to end up
> > in a situation where bool1 && bool2 && (bool1 != bool2) evaluates to
> > true.
> >
> > To give you more concrete example, here's what the last portion of
> > that function compiles to on AArch64:
> >
> >     2924: 97ffff5e     bl 269c <bbu_find_handler>
> >     2928: f100001f   cmp x0, #0x0
> >     292c: 1a9f07e0  cset w0, ne  // ne = any
> >     2930: f9400bf3   ldr x19, [sp, #16]
> >     2934: a8c27bfd  ldp x29, x30, [sp], #32
> >     2938: d65f03c0  ret
> >     293c: 52800020 mov w0, #0x1                    // #1
> >     2940: 17fffffc      b 2930 <barebox_update_handler_exists+0x30>
> >     2944: 52800000 mov w0, #0x0                    // #0
> >     2948: 17fffffa      b 2930 <barebox_update_handler_exists+0x30>
> >
> > and on AArch32 (Thumb):
> >
> >     18e0: f7ff ff48 bl 1774 <bbu_find_handler>
> >     18e4: 3000     adds r0, #0
> >     18e6: bf18      it ne
> >     18e8: 2001     movne r0, #1
> >     18ea: bd10     pop {r4, pc}
> >     18ec: 2001     movs r0, #1
> >     18ee: e7fc      b.n 18ea <barebox_update_handler_exists+0x1a>
> >
> > as you can see both cases already have code to explicitly convert the
> > result of the function to 0/1.
> >
> > I am more than happy to add ether !! or != NULL if you still think
> > that'd be better, it just I don't think it will have any practical
> > effect.
>
> For sure it doesn't have a practical effect, it's merely a sign to show
> "I know I'm casting a pointer to bool". Probably I'm just used to add
> an explicit cast there and it's just a matter of taste.
>

Since I have two people in favor of adding a != NULL there, I'll just
add it in v2 since I don't have a very strong opinion on the subject.

Thanks,
Andrey Smirnov

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 20/22] block: Do not ignore error in blk->ops->write()

[Andrey Smirnov]

Getting a error from blk->ops->write() is not a very unlikely
event (happens quite often during new board bringup), so we need to
catch and propagate them to upper layers so they can be at least
reported properly. Change the code of all of the callers to bail out
as soon as blk->ops->write() fails.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 common/block.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/common/block.c b/common/block.c
index 219b943af..8d0de42d9 100644
--- a/common/block.c
+++ b/common/block.c
@@ -44,13 +44,17 @@ struct chunk {
 static int writebuffer_flush(struct block_device *blk)
 {
 	struct chunk *chunk;
+	int ret;
 
 	if (!IS_ENABLED(CONFIG_BLOCK_WRITE))
 		return 0;
 
 	list_for_each_entry(chunk, &blk->buffered_blocks, list) {
 		if (chunk->dirty) {
-			blk->ops->write(blk, chunk->data, chunk->block_start, blk->rdbufsize);
+			ret = blk->ops->write(blk, chunk->data, chunk->block_start, blk->rdbufsize);
+			if (ret < 0)
+				return ret;
+
 			chunk->dirty = 0;
 		}
 	}
@@ -107,6 +111,7 @@ static void *block_get_cached(struct block_device *blk, int block)
 static struct chunk *get_chunk(struct block_device *blk)
 {
 	struct chunk *chunk;
+	int ret;
 
 	if (list_empty(&blk->idle_blocks)) {
 		/* use last entry which is the most unused */
@@ -114,8 +119,11 @@ static struct chunk *get_chunk(struct block_device *blk)
 		if (chunk->dirty) {
 			size_t num_blocks = min(blk->rdbufsize,
 					blk->num_blocks - chunk->block_start);
-			blk->ops->write(blk, chunk->data, chunk->block_start,
-					num_blocks);
+			ret = blk->ops->write(blk, chunk->data, chunk->block_start,
+					      num_blocks);
+			if (ret < 0)
+				return ERR_PTR(ret);
+
 			chunk->dirty = 0;
 		}
 
@@ -140,6 +148,9 @@ static int block_cache(struct block_device *blk, int block)
 	int ret;
 
 	chunk = get_chunk(blk);
+	if (IS_ERR(chunk))
+		return PTR_ERR(chunk);
+
 	chunk->block_start = block & ~blk->blkmask;
 
 	debug("%s: %d to %d\n", __func__, chunk->block_start,
-- 
2.17.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 21/22] bbu: Report update failures explicitly

[Andrey Smirnov]

Change the code of barebox_update() to explicitly log that update
failed instead of failing silently (unless update was interrupted) and
relying on user checking the return code.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 common/bbu.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/common/bbu.c b/common/bbu.c
index 69ccac68a..690f2a566 100644
--- a/common/bbu.c
+++ b/common/bbu.c
@@ -260,13 +260,13 @@ int barebox_update(struct bbu_data *data)
 		return ret;
 
 	ret = handler->handler(handler, data);
-	if (ret == -EINTR)
-		printf("update aborted\n");
-
-	if (!ret)
-		printf("update succeeded\n");
+	if (ret) {
+		printf("update %s\n", (ret == -EINTR) ? "aborted" : "failed");
+		return ret;
+	}
 
-	return ret;
+	printf("update succeeded\n");
+	return 0;
 }
 
 /*
-- 
2.17.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 22/22] bbu: command: Make sure specified update handler exists

[Andrey Smirnov]

Add code to verify that update handler specified with either -t or of
-d exists before commencing the update procedure.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 commands/barebox-update.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/commands/barebox-update.c b/commands/barebox-update.c
index 84798ab0d..f76aa19a5 100644
--- a/commands/barebox-update.c
+++ b/commands/barebox-update.c
@@ -24,6 +24,12 @@
 #include <bbu.h>
 #include <fs.h>
 
+static void print_handlers_list(void)
+{
+	printf("registered update handlers:\n");
+	bbu_handlers_list();
+}
+
 static int do_barebox_update(int argc, char *argv[])
 {
 	int opt, ret, repair = 0;
@@ -46,8 +52,7 @@ static int do_barebox_update(int argc, char *argv[])
 			data.flags |= BBU_FLAG_YES;
 			break;
 		case 'l':
-			printf("registered update handlers:\n");
-			bbu_handlers_list();
+			print_handlers_list();
 			return 0;
 		case 'r':
 			repair = 1;
@@ -57,6 +62,12 @@ static int do_barebox_update(int argc, char *argv[])
 		}
 	}
 
+	if (!barebox_update_handler_exists(&data)) {
+		printf("handler '%s' does not exist\n", data.handler_name);
+		print_handlers_list();
+		return COMMAND_ERROR;
+	}
+
 	if (argc - optind > 0) {
 		data.imagefile = argv[optind];
 
-- 
2.17.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox