mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH 1/3] ARM: i.MX: bbu-internal: make filename for device to write to adaptable
@ 2017-10-18 13:41 Uwe Kleine-König
  2017-10-18 13:41 ` [PATCH 2/3] ARM: i.MX: bbu-internal: new handler to make use of mmc boot partitions Uwe Kleine-König
  2017-10-18 13:41 ` [PATCH 3/3] ARM: i.MX: bbu-internal: update board files to make use of new bbu handler Uwe Kleine-König
  0 siblings, 2 replies; 8+ messages in thread
From: Uwe Kleine-König @ 2017-10-18 13:41 UTC (permalink / raw)
  To: barebox; +Cc: Uwe Kleine-König

From: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

This is a preparatory for the next patch that autodetects the device to
write to. To not have to modify the callback data for each call, some
static functions get an additional parameter to allow that.

This doesn't affect boards making use of the file's function and doesn't
change behaviour.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 arch/arm/mach-imx/imx-bbu-internal.c | 37 +++++++++++++++++++-----------------
 arch/arm/mach-imx/include/mach/bbu.h | 10 ++++++++++
 2 files changed, 30 insertions(+), 17 deletions(-)

diff --git a/arch/arm/mach-imx/imx-bbu-internal.c b/arch/arm/mach-imx/imx-bbu-internal.c
index 51ec8b8270f9..254ccb6369e6 100644
--- a/arch/arm/mach-imx/imx-bbu-internal.c
+++ b/arch/arm/mach-imx/imx-bbu-internal.c
@@ -49,29 +49,30 @@ struct imx_internal_bbu_handler {
  * DOS partition table on the device
  */
 static int imx_bbu_write_device(struct imx_internal_bbu_handler *imx_handler,
-		struct bbu_data *data, void *buf, int image_len)
+		const char *devicefile, struct bbu_data *data,
+		void *buf, int image_len)
 {
 	int fd, ret;
 
-	fd = open(data->devicefile, O_RDWR | O_CREAT);
+	fd = open(devicefile, O_RDWR | O_CREAT);
 	if (fd < 0)
 		return fd;
 
 	if (imx_handler->flags & IMX_INTERNAL_FLAG_ERASE) {
 		debug("%s: unprotecting %s from 0 to 0x%08x\n", __func__,
-				data->devicefile, image_len);
+				devicefile, image_len);
 		ret = protect(fd, image_len, 0, 0);
 		if (ret && ret != -ENOSYS) {
-			printf("unprotecting %s failed with %s\n", data->devicefile,
+			printf("unprotecting %s failed with %s\n", devicefile,
 					strerror(-ret));
 			goto err_close;
 		}
 
 		debug("%s: erasing %s from 0 to 0x%08x\n", __func__,
-				data->devicefile, image_len);
+				devicefile, image_len);
 		ret = erase(fd, image_len, 0);
 		if (ret) {
-			printf("erasing %s failed with %s\n", data->devicefile,
+			printf("erasing %s failed with %s\n", devicefile,
 					strerror(-ret));
 			goto err_close;
 		}
@@ -102,10 +103,10 @@ static int imx_bbu_write_device(struct imx_internal_bbu_handler *imx_handler,
 
 	if (imx_handler->flags & IMX_INTERNAL_FLAG_ERASE) {
 		debug("%s: protecting %s from 0 to 0x%08x\n", __func__,
-				data->devicefile, image_len);
+				devicefile, image_len);
 		ret = protect(fd, image_len, 0, 1);
 		if (ret && ret != -ENOSYS) {
-			printf("protecting %s failed with %s\n", data->devicefile,
+			printf("protecting %s failed with %s\n", devicefile,
 					strerror(-ret));
 		}
 	}
@@ -118,7 +119,7 @@ err_close:
 	return ret;
 }
 
-static int imx_bbu_check_prereq(struct bbu_data *data)
+static int imx_bbu_check_prereq(const char *devicefile, struct bbu_data *data)
 {
 	int ret;
 
@@ -131,8 +132,8 @@ static int imx_bbu_check_prereq(struct bbu_data *data)
 	if (ret)
 		return ret;
 
-	if (!strncmp(data->devicefile, "/dev/", 5))
-		device_detect_by_name(data->devicefile + 5);
+	if (!strncmp(devicefile, "/dev/", 5))
+		device_detect_by_name(devicefile + 5);
 
 	return 0;
 }
@@ -150,13 +151,13 @@ 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);
+	ret = imx_bbu_check_prereq(data->devicefile, data);
 	if (ret)
 		return ret;
 
 	printf("updating to %s\n", data->devicefile);
 
-	ret = imx_bbu_write_device(imx_handler, data, data->image, data->len);
+	ret = imx_bbu_write_device(imx_handler, data->devicefile, data, data->image, data->len);
 
 	return ret;
 }
@@ -348,7 +349,7 @@ static int imx_bbu_internal_v2_update(struct bbu_handler *handler, struct bbu_da
 	int ret;
 	uint32_t *barker;
 
-	ret = imx_bbu_check_prereq(data);
+	ret = imx_bbu_check_prereq(data->devicefile, data);
 	if (ret)
 		return ret;
 
@@ -362,7 +363,8 @@ static int imx_bbu_internal_v2_update(struct bbu_handler *handler, struct bbu_da
 	if (imx_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, data->image, data->len);
+		ret = imx_bbu_write_device(imx_handler, data->devicefile, data,
+					   data->image, data->len);
 
 	return ret;
 }
@@ -373,11 +375,12 @@ 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);
+	ret = imx_bbu_check_prereq(data->devicefile, data);
 	if (ret)
 		return ret;
 
-	return imx_bbu_write_device(imx_handler, data, data->image, data->len);
+	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,
diff --git a/arch/arm/mach-imx/include/mach/bbu.h b/arch/arm/mach-imx/include/mach/bbu.h
index 8039091395de..15bdbe1bec0d 100644
--- a/arch/arm/mach-imx/include/mach/bbu.h
+++ b/arch/arm/mach-imx/include/mach/bbu.h
@@ -24,6 +24,9 @@ int imx53_bbu_internal_nand_register_handler(const char *name,
 int imx6_bbu_internal_mmc_register_handler(const char *name, char *devicefile,
 		unsigned long flags);
 
+int imx6_bbu_internal_mmcboot_register_handler(const char *name, char *devicefile,
+		unsigned long flags);
+
 int imx6_bbu_internal_spi_i2c_register_handler(const char *name, char *devicefile,
 		unsigned long flags);
 
@@ -62,6 +65,13 @@ static inline int imx6_bbu_internal_mmc_register_handler(const char *name, char
 	return -ENOSYS;
 }
 
+static inline int imx6_bbu_internal_mmcboot_register_handler(const char *name,
+							     char *devicefile,
+							     unsigned long flags)
+{
+	return -ENOSYS;
+}
+
 static inline int imx6_bbu_internal_spi_i2c_register_handler(const char *name, char *devicefile,
 		unsigned long flags)
 {
-- 
2.14.2


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

^ permalink raw reply	[flat|nested] 8+ messages in thread
* [PATCH 2/3] ARM: i.MX: bbu-internal: new handler to make use of mmc boot partitions
  2017-10-18 13:41 [PATCH 1/3] ARM: i.MX: bbu-internal: make filename for device to write to adaptable Uwe Kleine-König
@ 2017-10-18 13:41 ` Uwe Kleine-König
  2017-10-23  7:20   ` Sascha Hauer
  2017-10-18 13:41 ` [PATCH 3/3] ARM: i.MX: bbu-internal: update board files to make use of new bbu handler Uwe Kleine-König
  1 sibling, 1 reply; 8+ messages in thread
From: Uwe Kleine-König @ 2017-10-18 13:41 UTC (permalink / raw)
  To: barebox; +Cc: Uwe Kleine-König

From: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

This handler updates the non-active MMC boot partition and after a
successful update makes the updated partition the active one. This way
the machine should continue to be bootable when the update fails.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 arch/arm/mach-imx/imx-bbu-internal.c | 71 ++++++++++++++++++++++++++++++++++++
 1 file changed, 71 insertions(+)

diff --git a/arch/arm/mach-imx/imx-bbu-internal.c b/arch/arm/mach-imx/imx-bbu-internal.c
index 254ccb6369e6..c8a8b5b689fc 100644
--- a/arch/arm/mach-imx/imx-bbu-internal.c
+++ b/arch/arm/mach-imx/imx-bbu-internal.c
@@ -29,6 +29,7 @@
 #include <linux/mtd/mtd-abi.h>
 #include <linux/stat.h>
 #include <ioctl.h>
+#include <environment.h>
 #include <mach/bbu.h>
 
 #define FLASH_HEADER_OFFSET_MMC		0x400
@@ -369,6 +370,63 @@ static int imx_bbu_internal_v2_update(struct bbu_handler *handler, struct bbu_da
 	return ret;
 }
 
+static int imx_bbu_internal_v2_mmcboot_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;
+	uint32_t *barker;
+	char *bootpartvar;
+	const char *bootpart;
+	char *devicefile;
+
+	barker = data->image + imx_handler->flash_header_offset;
+
+	if (*barker != IVT_BARKER) {
+		printf("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) {
+		printf("Failed to allocate string for boot variable\n");
+		return ret;
+	}
+
+	bootpart = getenv(bootpartvar);
+
+	if (!strcmp(bootpart, "boot0")) {
+		bootpart = "boot1";
+	} else {
+		bootpart = "boot0";
+	}
+
+	ret = asprintf(&devicefile, "/dev/%s.%s", data->devicefile, bootpart);
+	if (ret < 0) {
+		printf("Failed to allocate string for boot partition device file\n");
+		goto free_bootpartvar;
+	}
+
+	ret = imx_bbu_check_prereq(devicefile, data);
+	if (ret)
+		goto free_devicefile;
+
+	ret = imx_bbu_write_device(imx_handler, devicefile, data, data->image, data->len);
+
+	if (!ret)
+		/* on success switch boot source */
+		ret = setenv(bootpartvar, bootpart);
+
+free_devicefile:
+	free(devicefile);
+
+free_bootpartvar:
+	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 =
@@ -498,6 +556,19 @@ int imx6_bbu_internal_mmc_register_handler(const char *name, char *devicefile,
 	return __register_handler(imx_handler);
 }
 
+int imx6_bbu_internal_mmcboot_register_handler(const char *name, char *devicefile,
+					       unsigned long flags)
+{
+	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->handler.handler = imx_bbu_internal_v2_mmcboot_update;
+
+	return __register_handler(imx_handler);
+}
+
 /*
  * Register a i.MX53 internal boot update handler for i2c/spi
  * EEPROMs / flashes. Nearly the same as MMC/SD, but we do not need to
-- 
2.14.2


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

^ permalink raw reply	[flat|nested] 8+ messages in thread
* [PATCH 3/3] ARM: i.MX: bbu-internal: update board files to make use of new bbu handler
  2017-10-18 13:41 [PATCH 1/3] ARM: i.MX: bbu-internal: make filename for device to write to adaptable Uwe Kleine-König
  2017-10-18 13:41 ` [PATCH 2/3] ARM: i.MX: bbu-internal: new handler to make use of mmc boot partitions Uwe Kleine-König
@ 2017-10-18 13:41 ` Uwe Kleine-König
  2017-10-23  7:23   ` Sascha Hauer
  1 sibling, 1 reply; 8+ messages in thread
From: Uwe Kleine-König @ 2017-10-18 13:41 UTC (permalink / raw)
  To: barebox; +Cc: Uwe Kleine-König

From: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
Hello,

this is only compile tested. There are some boards that write to
mmcX.boot0.barebox. On the board I tested this series on I don't have
such a setup, so I didn't touch these. Probably this series needs
adaption to be able to handle this scenario.

Best regards
Uwe

 arch/arm/boards/dfi-fs700-m60/board.c | 3 +--
 arch/arm/boards/guf-santaro/board.c   | 2 +-
 arch/arm/boards/karo-tx6x/board.c     | 4 ++--
 arch/arm/boards/tqma6x/board.c        | 2 +-
 4 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/arch/arm/boards/dfi-fs700-m60/board.c b/arch/arm/boards/dfi-fs700-m60/board.c
index bef4612d9e66..c2c73db8e547 100644
--- a/arch/arm/boards/dfi-fs700-m60/board.c
+++ b/arch/arm/boards/dfi-fs700-m60/board.c
@@ -110,8 +110,7 @@ static int dfi_fs700_m60_init(void)
 	else
 		flag_mmc |= BBU_HANDLER_FLAG_DEFAULT;
 
-	imx6_bbu_internal_mmc_register_handler("mmc", "/dev/mmc3.boot0",
-		flag_mmc);
+	imx6_bbu_internal_mmcboot_register_handler("mmc", "mmc3", flag_mmc);
 	imx6_bbu_internal_spi_i2c_register_handler("spiflash", "/dev/m25p0",
 		flag_spi);
 
diff --git a/arch/arm/boards/guf-santaro/board.c b/arch/arm/boards/guf-santaro/board.c
index e54110886bd9..adf6830dff1d 100644
--- a/arch/arm/boards/guf-santaro/board.c
+++ b/arch/arm/boards/guf-santaro/board.c
@@ -150,7 +150,7 @@ static int santaro_device_init(void)
 	}
 
 	imx6_bbu_internal_mmc_register_handler("sd", "/dev/mmc1", flag_sd);
-	imx6_bbu_internal_mmc_register_handler("emmc", "/dev/mmc3.boot0", flag_emmc);
+	imx6_bbu_internal_mmcboot_register_handler("emmc", "mmc3", flag_emmc);
 
 	return 0;
 }
diff --git a/arch/arm/boards/karo-tx6x/board.c b/arch/arm/boards/karo-tx6x/board.c
index 31c1c3a9ff5d..3e239bb653e0 100644
--- a/arch/arm/boards/karo-tx6x/board.c
+++ b/arch/arm/boards/karo-tx6x/board.c
@@ -123,8 +123,8 @@ static int tx6x_devices_init(void)
 		of_device_enable_and_register_by_name("environment-nand");
 		of_device_enable_and_register_by_name("gpmi-nand@00112000");
 	} else {
-		imx6_bbu_internal_mmc_register_handler("eMMC", "/dev/mmc3.boot0",
-						       BBU_HANDLER_FLAG_DEFAULT);
+		imx6_bbu_internal_mmcboot_register_handler("eMMC", "mmc3",
+							   BBU_HANDLER_FLAG_DEFAULT);
 		of_device_enable_and_register_by_name("environment-emmc");
 		of_device_enable_and_register_by_name("usdhc@0219c000");
 	}
diff --git a/arch/arm/boards/tqma6x/board.c b/arch/arm/boards/tqma6x/board.c
index 8946a27b696a..ecf8fa06af01 100644
--- a/arch/arm/boards/tqma6x/board.c
+++ b/arch/arm/boards/tqma6x/board.c
@@ -105,7 +105,7 @@ static int tqma6x_env_init(void)
 
 	imx6_bbu_internal_spi_i2c_register_handler("spiflash", "/dev/m25p0.barebox",
 		BBU_HANDLER_FLAG_DEFAULT);
-	imx6_bbu_internal_mmc_register_handler("emmc", "/dev/mmc2.boot0", 0);
+	imx6_bbu_internal_mmcboot_register_handler("emmc", "mmc2", 0);
 
 	device_detect_by_name("mmc2");
 
-- 
2.14.2


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

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH 2/3] ARM: i.MX: bbu-internal: new handler to make use of mmc boot partitions
  2017-10-18 13:41 ` [PATCH 2/3] ARM: i.MX: bbu-internal: new handler to make use of mmc boot partitions Uwe Kleine-König
@ 2017-10-23  7:20   ` Sascha Hauer
  2017-10-23  7:54     ` Uwe Kleine-König
  0 siblings, 1 reply; 8+ messages in thread
From: Sascha Hauer @ 2017-10-23  7:20 UTC (permalink / raw)
  To: Uwe Kleine-König; +Cc: barebox, Uwe Kleine-König

On Wed, Oct 18, 2017 at 03:41:16PM +0200, Uwe Kleine-König wrote:
> From: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> 
> This handler updates the non-active MMC boot partition and after a
> successful update makes the updated partition the active one. This way
> the machine should continue to be bootable when the update fails.

Adding something like this as a comment above
imx6_bbu_internal_mmcboot_register_handler() would be nice.

> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
>  arch/arm/mach-imx/imx-bbu-internal.c | 71 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 71 insertions(+)
> 
> diff --git a/arch/arm/mach-imx/imx-bbu-internal.c b/arch/arm/mach-imx/imx-bbu-internal.c
> index 254ccb6369e6..c8a8b5b689fc 100644
> --- a/arch/arm/mach-imx/imx-bbu-internal.c
> +++ b/arch/arm/mach-imx/imx-bbu-internal.c
> @@ -29,6 +29,7 @@
>  #include <linux/mtd/mtd-abi.h>
>  #include <linux/stat.h>
>  #include <ioctl.h>
> +#include <environment.h>
>  #include <mach/bbu.h>
>  
>  #define FLASH_HEADER_OFFSET_MMC		0x400
> @@ -369,6 +370,63 @@ static int imx_bbu_internal_v2_update(struct bbu_handler *handler, struct bbu_da
>  	return ret;
>  }
>  
> +static int imx_bbu_internal_v2_mmcboot_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;
> +	uint32_t *barker;
> +	char *bootpartvar;
> +	const char *bootpart;
> +	char *devicefile;
> +
> +	barker = data->image + imx_handler->flash_header_offset;
> +
> +	if (*barker != IVT_BARKER) {
> +		printf("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) {
> +		printf("Failed to allocate string for boot variable\n");

I think messages for failed memory allocations which eat more space than
the actual allocation should be avoided. Just return -ENOMEM without a
message.

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

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH 3/3] ARM: i.MX: bbu-internal: update board files to make use of new bbu handler
  2017-10-18 13:41 ` [PATCH 3/3] ARM: i.MX: bbu-internal: update board files to make use of new bbu handler Uwe Kleine-König
@ 2017-10-23  7:23   ` Sascha Hauer
  2017-10-23  7:55     ` Uwe Kleine-König
  0 siblings, 1 reply; 8+ messages in thread
From: Sascha Hauer @ 2017-10-23  7:23 UTC (permalink / raw)
  To: Uwe Kleine-König; +Cc: barebox, Uwe Kleine-König

On Wed, Oct 18, 2017 at 03:41:17PM +0200, Uwe Kleine-König wrote:
> From: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
> Hello,
> 
> this is only compile tested. There are some boards that write to
> mmcX.boot0.barebox. On the board I tested this series on I don't have
> such a setup, so I didn't touch these. Probably this series needs
> adaption to be able to handle this scenario.
> 
> Best regards
> Uwe
> 
>  arch/arm/boards/dfi-fs700-m60/board.c | 3 +--
>  arch/arm/boards/guf-santaro/board.c   | 2 +-
>  arch/arm/boards/karo-tx6x/board.c     | 4 ++--
>  arch/arm/boards/tqma6x/board.c        | 2 +-
>  4 files changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm/boards/dfi-fs700-m60/board.c b/arch/arm/boards/dfi-fs700-m60/board.c
> index bef4612d9e66..c2c73db8e547 100644
> --- a/arch/arm/boards/dfi-fs700-m60/board.c
> +++ b/arch/arm/boards/dfi-fs700-m60/board.c
> @@ -110,8 +110,7 @@ static int dfi_fs700_m60_init(void)
>  	else
>  		flag_mmc |= BBU_HANDLER_FLAG_DEFAULT;
>  
> -	imx6_bbu_internal_mmc_register_handler("mmc", "/dev/mmc3.boot0",
> -		flag_mmc);
> +	imx6_bbu_internal_mmcboot_register_handler("mmc", "mmc3", flag_mmc);

This board at least uses the boot1 partition for the environment, so we
can't use your new handler without moving the environment first.

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

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH 2/3] ARM: i.MX: bbu-internal: new handler to make use of mmc boot partitions
  2017-10-23  7:20   ` Sascha Hauer
@ 2017-10-23  7:54     ` Uwe Kleine-König
  0 siblings, 0 replies; 8+ messages in thread
From: Uwe Kleine-König @ 2017-10-23  7:54 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: barebox

On Mon, Oct 23, 2017 at 09:20:36AM +0200, Sascha Hauer wrote:
> On Wed, Oct 18, 2017 at 03:41:16PM +0200, Uwe Kleine-König wrote:
> > From: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > 
> > This handler updates the non-active MMC boot partition and after a
> > successful update makes the updated partition the active one. This way
> > the machine should continue to be bootable when the update fails.
> 
> Adding something like this as a comment above
> imx6_bbu_internal_mmcboot_register_handler() would be nice.

ok.

> > +static int imx_bbu_internal_v2_mmcboot_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;
> > +	uint32_t *barker;
> > +	char *bootpartvar;
> > +	const char *bootpart;
> > +	char *devicefile;
> > +
> > +	barker = data->image + imx_handler->flash_header_offset;
> > +
> > +	if (*barker != IVT_BARKER) {
> > +		printf("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) {
> > +		printf("Failed to allocate string for boot variable\n");
> 
> I think messages for failed memory allocations which eat more space than
> the actual allocation should be avoided. Just return -ENOMEM without a
> message.

In general not giving an error message makes the problem harder to
locate. But maybe if I'm unable to allocate 20 Bytes the problem is
obvious.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

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

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH 3/3] ARM: i.MX: bbu-internal: update board files to make use of new bbu handler
  2017-10-23  7:23   ` Sascha Hauer
@ 2017-10-23  7:55     ` Uwe Kleine-König
  2017-10-23  8:05       ` Sascha Hauer
  0 siblings, 1 reply; 8+ messages in thread
From: Uwe Kleine-König @ 2017-10-23  7:55 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: barebox

Hello Sascha,

On Mon, Oct 23, 2017 at 09:23:56AM +0200, Sascha Hauer wrote:
> On Wed, Oct 18, 2017 at 03:41:17PM +0200, Uwe Kleine-König wrote:
> > diff --git a/arch/arm/boards/dfi-fs700-m60/board.c b/arch/arm/boards/dfi-fs700-m60/board.c
> > index bef4612d9e66..c2c73db8e547 100644
> > --- a/arch/arm/boards/dfi-fs700-m60/board.c
> > +++ b/arch/arm/boards/dfi-fs700-m60/board.c
> > @@ -110,8 +110,7 @@ static int dfi_fs700_m60_init(void)
> >  	else
> >  		flag_mmc |= BBU_HANDLER_FLAG_DEFAULT;
> >  
> > -	imx6_bbu_internal_mmc_register_handler("mmc", "/dev/mmc3.boot0",
> > -		flag_mmc);
> > +	imx6_bbu_internal_mmcboot_register_handler("mmc", "mmc3", flag_mmc);
> 
> This board at least uses the boot1 partition for the environment, so we
> can't use your new handler without moving the environment first.

Good catch, I will drop this board's modifications then.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

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

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH 3/3] ARM: i.MX: bbu-internal: update board files to make use of new bbu handler
  2017-10-23  7:55     ` Uwe Kleine-König
@ 2017-10-23  8:05       ` Sascha Hauer
  0 siblings, 0 replies; 8+ messages in thread
From: Sascha Hauer @ 2017-10-23  8:05 UTC (permalink / raw)
  To: Uwe Kleine-König; +Cc: barebox

On Mon, Oct 23, 2017 at 09:55:20AM +0200, Uwe Kleine-König wrote:
> Hello Sascha,
> 
> On Mon, Oct 23, 2017 at 09:23:56AM +0200, Sascha Hauer wrote:
> > On Wed, Oct 18, 2017 at 03:41:17PM +0200, Uwe Kleine-König wrote:
> > > diff --git a/arch/arm/boards/dfi-fs700-m60/board.c b/arch/arm/boards/dfi-fs700-m60/board.c
> > > index bef4612d9e66..c2c73db8e547 100644
> > > --- a/arch/arm/boards/dfi-fs700-m60/board.c
> > > +++ b/arch/arm/boards/dfi-fs700-m60/board.c
> > > @@ -110,8 +110,7 @@ static int dfi_fs700_m60_init(void)
> > >  	else
> > >  		flag_mmc |= BBU_HANDLER_FLAG_DEFAULT;
> > >  
> > > -	imx6_bbu_internal_mmc_register_handler("mmc", "/dev/mmc3.boot0",
> > > -		flag_mmc);
> > > +	imx6_bbu_internal_mmcboot_register_handler("mmc", "mmc3", flag_mmc);
> > 
> > This board at least uses the boot1 partition for the environment, so we
> > can't use your new handler without moving the environment first.
> 
> Good catch, I will drop this board's modifications then.

I haven't checked the other boards, they may or may not do the same.

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

^ permalink raw reply	[flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-10-23  8:05 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-18 13:41 [PATCH 1/3] ARM: i.MX: bbu-internal: make filename for device to write to adaptable Uwe Kleine-König
2017-10-18 13:41 ` [PATCH 2/3] ARM: i.MX: bbu-internal: new handler to make use of mmc boot partitions Uwe Kleine-König
2017-10-23  7:20   ` Sascha Hauer
2017-10-23  7:54     ` Uwe Kleine-König
2017-10-18 13:41 ` [PATCH 3/3] ARM: i.MX: bbu-internal: update board files to make use of new bbu handler Uwe Kleine-König
2017-10-23  7:23   ` Sascha Hauer
2017-10-23  7:55     ` Uwe Kleine-König
2017-10-23  8:05       ` Sascha Hauer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox