mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH v4] FIT: Parse `load` and `entry` addresses.
@ 2020-07-14  9:11 Christian Mauderer
  2020-07-14  9:15 ` Ahmad Fatoum
  2020-07-14 18:09 ` Sascha Hauer
  0 siblings, 2 replies; 5+ messages in thread
From: Christian Mauderer @ 2020-07-14  9:11 UTC (permalink / raw)
  To: barebox; +Cc: a.fatoum

According to the U-Boot documentation for the FIT file format, the load
and entry have to be allways defined for a "kernel" or "standalone".
But Barebox ignored the parameters. That changes with this patch.

For backward compatibility the default address is still used for images
without `load` or `entry`.

Signed-off-by: Christian Mauderer <christian.mauderer@embedded-brains.de>
---
 arch/arm/mach-layerscape/ppa.c |  3 +-
 common/bootm.c                 |  7 +++--
 common/image-fit.c             | 50 ++++++++++++++++++++++++++++++++--
 include/image-fit.h            |  3 +-
 4 files changed, 56 insertions(+), 7 deletions(-)

diff --git a/arch/arm/mach-layerscape/ppa.c b/arch/arm/mach-layerscape/ppa.c
index 477e89478..3eb7ab641 100644
--- a/arch/arm/mach-layerscape/ppa.c
+++ b/arch/arm/mach-layerscape/ppa.c
@@ -82,7 +82,8 @@ static int ppa_init(void *ppa, size_t ppa_size, void *sec_firmware_addr)
 	}
 
 
-	ret = fit_open_image(fit, conf, "firmware", &buf, &firmware_size);
+	ret = fit_open_image(fit, conf, "firmware", &buf, &firmware_size,
+			     NULL, NULL);
 	if (ret) {
 		pr_err("Cannot open firmware image in ppa FIT image: %s\n",
 		       strerror(-ret));
diff --git a/common/bootm.c b/common/bootm.c
index 366f31455..94600cae3 100644
--- a/common/bootm.c
+++ b/common/bootm.c
@@ -222,7 +222,7 @@ int bootm_load_initrd(struct image_data *data, unsigned long load_address)
 		unsigned long initrd_size;
 
 		ret = fit_open_image(data->os_fit, data->fit_config, "ramdisk",
-				     &initrd, &initrd_size);
+				     &initrd, &initrd_size, NULL, NULL);
 
 		data->initrd_res = request_sdram_region("initrd",
 				load_address,
@@ -348,7 +348,7 @@ void *bootm_get_devicetree(struct image_data *data)
 		unsigned long of_size;
 
 		ret = fit_open_image(data->os_fit, data->fit_config, "fdt",
-				     &of_tree, &of_size);
+				     &of_tree, &of_size, NULL, NULL);
 		if (ret)
 			return ERR_PTR(ret);
 
@@ -622,7 +622,8 @@ int bootm_boot(struct bootm_data *bootm_data)
 		}
 
 		ret = fit_open_image(data->os_fit, data->fit_config, "kernel",
-				     &data->fit_kernel, &data->fit_kernel_size);
+				     &data->fit_kernel, &data->fit_kernel_size,
+				     &data->os_address, &data->os_entry);
 		if (ret)
 			goto err_out;
 	}
diff --git a/common/image-fit.c b/common/image-fit.c
index 2681d62a9..6dd541634 100644
--- a/common/image-fit.c
+++ b/common/image-fit.c
@@ -517,12 +517,30 @@ int fit_has_image(struct fit_handle *handle, void *configuration,
 	return 1;
 }
 
+static int fit_get_address(struct device_node *image, const char *property,
+			   unsigned long *addr)
+{
+	const __be32 *cell;
+	int len = 0;
+
+	cell = of_get_property(image, property, &len);
+	if (!cell)
+		return -EINVAL;
+	if (len > sizeof(*addr))
+		return -ENOTSUPP;
+
+	*addr = (unsigned long)of_read_number(cell, len / sizeof(*cell));
+	return 0;
+}
+
 /**
  * fit_open_image - Open an image in a FIT image
  * @handle: The FIT image handle
  * @name: The name of the image to open
  * @outdata: The returned image
  * @outsize: Size of the returned image
+ * @load: The load address given by the image
+ * @entry: The entry address given by the image
  *
  * Open an image in a FIT image. The returned image is freed during fit_close().
  * @configuration holds the cookie returned from fit_open_configuration() if
@@ -532,11 +550,16 @@ int fit_has_image(struct fit_handle *handle, void *configuration,
  * then only the hash is checked (because opening the configuration already
  * checks the RSA signature of all involved nodes).
  *
+ * The load address and entry point of the image description in the FIT will be
+ * parsed if they exist and if the @load and @entry parameters are not NULL.
+ * Otherwise @load and @entry won't be changed.
+ *
  * Return: 0 for success, negative error code otherwise
  */
 int fit_open_image(struct fit_handle *handle, void *configuration,
 		   const char *name, const void **outdata,
-		   unsigned long *outsize)
+		   unsigned long *outsize, unsigned long *load,
+		   unsigned long *entry)
 {
 	struct device_node *image;
 	const char *unit, *type = NULL, *desc= "(no description)";
@@ -559,7 +582,30 @@ int fit_open_image(struct fit_handle *handle, void *configuration,
 		return -ENOENT;
 
 	of_property_read_string(image, "description", &desc);
-	pr_info("image '%s': '%s'\n", unit, desc);
+	pr_info("image '%s': '%s'", unit, desc);
+
+	if (load) {
+		ret = fit_get_address(image, "load", load);
+		if (ret < 0)
+			pr_info("; Couldn't get load address in %s. Use default.\n",
+				image->full_name);
+		else
+			pr_cont("; load: 0x%lx", *load);
+	}
+
+	if (load && entry) {
+		ret = fit_get_address(image, "entry", entry);
+		if (ret < 0) {
+			pr_info("Couldn't get entry point in %s. Use default.\n",
+				image->full_name);
+		} else {
+			/* Barebox uses an entry relative to load but the FIT
+			 * images assume an absolute entry. */
+			*entry -= *load;
+			pr_cont("; entry (relative to load): 0x%lx", *entry);
+		}
+	}
+	pr_cont("\n");
 
 	of_property_read_string(image, "type", &type);
 	if (!type) {
diff --git a/include/image-fit.h b/include/image-fit.h
index 27c9e8351..038732d0d 100644
--- a/include/image-fit.h
+++ b/include/image-fit.h
@@ -31,7 +31,8 @@ int fit_has_image(struct fit_handle *handle, void *configuration,
 		  const char *name);
 int fit_open_image(struct fit_handle *handle, void *configuration,
 		   const char *name, const void **outdata,
-		   unsigned long *outsize);
+		   unsigned long *outsize, unsigned long *load,
+		   unsigned long *entry);
 
 void fit_close(struct fit_handle *handle);
 
-- 
2.26.2


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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v4] FIT: Parse `load` and `entry` addresses.
  2020-07-14  9:11 [PATCH v4] FIT: Parse `load` and `entry` addresses Christian Mauderer
@ 2020-07-14  9:15 ` Ahmad Fatoum
  2020-07-14  9:15   ` Christian Mauderer
  2020-07-14 18:09 ` Sascha Hauer
  1 sibling, 1 reply; 5+ messages in thread
From: Ahmad Fatoum @ 2020-07-14  9:15 UTC (permalink / raw)
  To: Christian Mauderer, barebox

On 7/14/20 11:11 AM, Christian Mauderer wrote:
> According to the U-Boot documentation for the FIT file format, the load
> and entry have to be allways defined for a "kernel" or "standalone".
> But Barebox ignored the parameters. That changes with this patch.
> 
> For backward compatibility the default address is still used for images
> without `load` or `entry`.
> 
> Signed-off-by: Christian Mauderer <christian.mauderer@embedded-brains.de>

Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

Thanks
Ahmad

> ---
>  arch/arm/mach-layerscape/ppa.c |  3 +-
>  common/bootm.c                 |  7 +++--
>  common/image-fit.c             | 50 ++++++++++++++++++++++++++++++++--
>  include/image-fit.h            |  3 +-
>  4 files changed, 56 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/arm/mach-layerscape/ppa.c b/arch/arm/mach-layerscape/ppa.c
> index 477e89478..3eb7ab641 100644
> --- a/arch/arm/mach-layerscape/ppa.c
> +++ b/arch/arm/mach-layerscape/ppa.c
> @@ -82,7 +82,8 @@ static int ppa_init(void *ppa, size_t ppa_size, void *sec_firmware_addr)
>  	}
>  
>  
> -	ret = fit_open_image(fit, conf, "firmware", &buf, &firmware_size);
> +	ret = fit_open_image(fit, conf, "firmware", &buf, &firmware_size,
> +			     NULL, NULL);
>  	if (ret) {
>  		pr_err("Cannot open firmware image in ppa FIT image: %s\n",
>  		       strerror(-ret));
> diff --git a/common/bootm.c b/common/bootm.c
> index 366f31455..94600cae3 100644
> --- a/common/bootm.c
> +++ b/common/bootm.c
> @@ -222,7 +222,7 @@ int bootm_load_initrd(struct image_data *data, unsigned long load_address)
>  		unsigned long initrd_size;
>  
>  		ret = fit_open_image(data->os_fit, data->fit_config, "ramdisk",
> -				     &initrd, &initrd_size);
> +				     &initrd, &initrd_size, NULL, NULL);
>  
>  		data->initrd_res = request_sdram_region("initrd",
>  				load_address,
> @@ -348,7 +348,7 @@ void *bootm_get_devicetree(struct image_data *data)
>  		unsigned long of_size;
>  
>  		ret = fit_open_image(data->os_fit, data->fit_config, "fdt",
> -				     &of_tree, &of_size);
> +				     &of_tree, &of_size, NULL, NULL);
>  		if (ret)
>  			return ERR_PTR(ret);
>  
> @@ -622,7 +622,8 @@ int bootm_boot(struct bootm_data *bootm_data)
>  		}
>  
>  		ret = fit_open_image(data->os_fit, data->fit_config, "kernel",
> -				     &data->fit_kernel, &data->fit_kernel_size);
> +				     &data->fit_kernel, &data->fit_kernel_size,
> +				     &data->os_address, &data->os_entry);
>  		if (ret)
>  			goto err_out;
>  	}
> diff --git a/common/image-fit.c b/common/image-fit.c
> index 2681d62a9..6dd541634 100644
> --- a/common/image-fit.c
> +++ b/common/image-fit.c
> @@ -517,12 +517,30 @@ int fit_has_image(struct fit_handle *handle, void *configuration,
>  	return 1;
>  }
>  
> +static int fit_get_address(struct device_node *image, const char *property,
> +			   unsigned long *addr)
> +{
> +	const __be32 *cell;
> +	int len = 0;
> +
> +	cell = of_get_property(image, property, &len);
> +	if (!cell)
> +		return -EINVAL;
> +	if (len > sizeof(*addr))
> +		return -ENOTSUPP;
> +
> +	*addr = (unsigned long)of_read_number(cell, len / sizeof(*cell));
> +	return 0;
> +}
> +
>  /**
>   * fit_open_image - Open an image in a FIT image
>   * @handle: The FIT image handle
>   * @name: The name of the image to open
>   * @outdata: The returned image
>   * @outsize: Size of the returned image
> + * @load: The load address given by the image
> + * @entry: The entry address given by the image
>   *
>   * Open an image in a FIT image. The returned image is freed during fit_close().
>   * @configuration holds the cookie returned from fit_open_configuration() if
> @@ -532,11 +550,16 @@ int fit_has_image(struct fit_handle *handle, void *configuration,
>   * then only the hash is checked (because opening the configuration already
>   * checks the RSA signature of all involved nodes).
>   *
> + * The load address and entry point of the image description in the FIT will be
> + * parsed if they exist and if the @load and @entry parameters are not NULL.
> + * Otherwise @load and @entry won't be changed.
> + *
>   * Return: 0 for success, negative error code otherwise
>   */
>  int fit_open_image(struct fit_handle *handle, void *configuration,
>  		   const char *name, const void **outdata,
> -		   unsigned long *outsize)
> +		   unsigned long *outsize, unsigned long *load,
> +		   unsigned long *entry)
>  {
>  	struct device_node *image;
>  	const char *unit, *type = NULL, *desc= "(no description)";
> @@ -559,7 +582,30 @@ int fit_open_image(struct fit_handle *handle, void *configuration,
>  		return -ENOENT;
>  
>  	of_property_read_string(image, "description", &desc);
> -	pr_info("image '%s': '%s'\n", unit, desc);
> +	pr_info("image '%s': '%s'", unit, desc);
> +
> +	if (load) {
> +		ret = fit_get_address(image, "load", load);
> +		if (ret < 0)
> +			pr_info("; Couldn't get load address in %s. Use default.\n",
> +				image->full_name);
> +		else
> +			pr_cont("; load: 0x%lx", *load);
> +	}
> +
> +	if (load && entry) {
> +		ret = fit_get_address(image, "entry", entry);
> +		if (ret < 0) {
> +			pr_info("Couldn't get entry point in %s. Use default.\n",
> +				image->full_name);
> +		} else {
> +			/* Barebox uses an entry relative to load but the FIT
> +			 * images assume an absolute entry. */
> +			*entry -= *load;
> +			pr_cont("; entry (relative to load): 0x%lx", *entry);
> +		}
> +	}
> +	pr_cont("\n");
>  
>  	of_property_read_string(image, "type", &type);
>  	if (!type) {
> diff --git a/include/image-fit.h b/include/image-fit.h
> index 27c9e8351..038732d0d 100644
> --- a/include/image-fit.h
> +++ b/include/image-fit.h
> @@ -31,7 +31,8 @@ int fit_has_image(struct fit_handle *handle, void *configuration,
>  		  const char *name);
>  int fit_open_image(struct fit_handle *handle, void *configuration,
>  		   const char *name, const void **outdata,
> -		   unsigned long *outsize);
> +		   unsigned long *outsize, unsigned long *load,
> +		   unsigned long *entry);
>  
>  void fit_close(struct fit_handle *handle);
>  
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
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] 5+ messages in thread

* Re: [PATCH v4] FIT: Parse `load` and `entry` addresses.
  2020-07-14  9:15 ` Ahmad Fatoum
@ 2020-07-14  9:15   ` Christian Mauderer
  0 siblings, 0 replies; 5+ messages in thread
From: Christian Mauderer @ 2020-07-14  9:15 UTC (permalink / raw)
  To: Ahmad Fatoum, barebox

On 14/07/2020 11:15, Ahmad Fatoum wrote:
> On 7/14/20 11:11 AM, Christian Mauderer wrote:
>> According to the U-Boot documentation for the FIT file format, the load
>> and entry have to be allways defined for a "kernel" or "standalone".
>> But Barebox ignored the parameters. That changes with this patch.
>>
>> For backward compatibility the default address is still used for images
>> without `load` or `entry`.
>>
>> Signed-off-by: Christian Mauderer <christian.mauderer@embedded-brains.de>
> 
> Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> 
> Thanks
> Ahmad

Thanks for the review and for your patience.

Best regards

Christian

> 
>> ---
>>  arch/arm/mach-layerscape/ppa.c |  3 +-
>>  common/bootm.c                 |  7 +++--
>>  common/image-fit.c             | 50 ++++++++++++++++++++++++++++++++--
>>  include/image-fit.h            |  3 +-
>>  4 files changed, 56 insertions(+), 7 deletions(-)
>>
>> diff --git a/arch/arm/mach-layerscape/ppa.c b/arch/arm/mach-layerscape/ppa.c
>> index 477e89478..3eb7ab641 100644
>> --- a/arch/arm/mach-layerscape/ppa.c
>> +++ b/arch/arm/mach-layerscape/ppa.c
>> @@ -82,7 +82,8 @@ static int ppa_init(void *ppa, size_t ppa_size, void *sec_firmware_addr)
>>  	}
>>  
>>  
>> -	ret = fit_open_image(fit, conf, "firmware", &buf, &firmware_size);
>> +	ret = fit_open_image(fit, conf, "firmware", &buf, &firmware_size,
>> +			     NULL, NULL);
>>  	if (ret) {
>>  		pr_err("Cannot open firmware image in ppa FIT image: %s\n",
>>  		       strerror(-ret));
>> diff --git a/common/bootm.c b/common/bootm.c
>> index 366f31455..94600cae3 100644
>> --- a/common/bootm.c
>> +++ b/common/bootm.c
>> @@ -222,7 +222,7 @@ int bootm_load_initrd(struct image_data *data, unsigned long load_address)
>>  		unsigned long initrd_size;
>>  
>>  		ret = fit_open_image(data->os_fit, data->fit_config, "ramdisk",
>> -				     &initrd, &initrd_size);
>> +				     &initrd, &initrd_size, NULL, NULL);
>>  
>>  		data->initrd_res = request_sdram_region("initrd",
>>  				load_address,
>> @@ -348,7 +348,7 @@ void *bootm_get_devicetree(struct image_data *data)
>>  		unsigned long of_size;
>>  
>>  		ret = fit_open_image(data->os_fit, data->fit_config, "fdt",
>> -				     &of_tree, &of_size);
>> +				     &of_tree, &of_size, NULL, NULL);
>>  		if (ret)
>>  			return ERR_PTR(ret);
>>  
>> @@ -622,7 +622,8 @@ int bootm_boot(struct bootm_data *bootm_data)
>>  		}
>>  
>>  		ret = fit_open_image(data->os_fit, data->fit_config, "kernel",
>> -				     &data->fit_kernel, &data->fit_kernel_size);
>> +				     &data->fit_kernel, &data->fit_kernel_size,
>> +				     &data->os_address, &data->os_entry);
>>  		if (ret)
>>  			goto err_out;
>>  	}
>> diff --git a/common/image-fit.c b/common/image-fit.c
>> index 2681d62a9..6dd541634 100644
>> --- a/common/image-fit.c
>> +++ b/common/image-fit.c
>> @@ -517,12 +517,30 @@ int fit_has_image(struct fit_handle *handle, void *configuration,
>>  	return 1;
>>  }
>>  
>> +static int fit_get_address(struct device_node *image, const char *property,
>> +			   unsigned long *addr)
>> +{
>> +	const __be32 *cell;
>> +	int len = 0;
>> +
>> +	cell = of_get_property(image, property, &len);
>> +	if (!cell)
>> +		return -EINVAL;
>> +	if (len > sizeof(*addr))
>> +		return -ENOTSUPP;
>> +
>> +	*addr = (unsigned long)of_read_number(cell, len / sizeof(*cell));
>> +	return 0;
>> +}
>> +
>>  /**
>>   * fit_open_image - Open an image in a FIT image
>>   * @handle: The FIT image handle
>>   * @name: The name of the image to open
>>   * @outdata: The returned image
>>   * @outsize: Size of the returned image
>> + * @load: The load address given by the image
>> + * @entry: The entry address given by the image
>>   *
>>   * Open an image in a FIT image. The returned image is freed during fit_close().
>>   * @configuration holds the cookie returned from fit_open_configuration() if
>> @@ -532,11 +550,16 @@ int fit_has_image(struct fit_handle *handle, void *configuration,
>>   * then only the hash is checked (because opening the configuration already
>>   * checks the RSA signature of all involved nodes).
>>   *
>> + * The load address and entry point of the image description in the FIT will be
>> + * parsed if they exist and if the @load and @entry parameters are not NULL.
>> + * Otherwise @load and @entry won't be changed.
>> + *
>>   * Return: 0 for success, negative error code otherwise
>>   */
>>  int fit_open_image(struct fit_handle *handle, void *configuration,
>>  		   const char *name, const void **outdata,
>> -		   unsigned long *outsize)
>> +		   unsigned long *outsize, unsigned long *load,
>> +		   unsigned long *entry)
>>  {
>>  	struct device_node *image;
>>  	const char *unit, *type = NULL, *desc= "(no description)";
>> @@ -559,7 +582,30 @@ int fit_open_image(struct fit_handle *handle, void *configuration,
>>  		return -ENOENT;
>>  
>>  	of_property_read_string(image, "description", &desc);
>> -	pr_info("image '%s': '%s'\n", unit, desc);
>> +	pr_info("image '%s': '%s'", unit, desc);
>> +
>> +	if (load) {
>> +		ret = fit_get_address(image, "load", load);
>> +		if (ret < 0)
>> +			pr_info("; Couldn't get load address in %s. Use default.\n",
>> +				image->full_name);
>> +		else
>> +			pr_cont("; load: 0x%lx", *load);
>> +	}
>> +
>> +	if (load && entry) {
>> +		ret = fit_get_address(image, "entry", entry);
>> +		if (ret < 0) {
>> +			pr_info("Couldn't get entry point in %s. Use default.\n",
>> +				image->full_name);
>> +		} else {
>> +			/* Barebox uses an entry relative to load but the FIT
>> +			 * images assume an absolute entry. */
>> +			*entry -= *load;
>> +			pr_cont("; entry (relative to load): 0x%lx", *entry);
>> +		}
>> +	}
>> +	pr_cont("\n");
>>  
>>  	of_property_read_string(image, "type", &type);
>>  	if (!type) {
>> diff --git a/include/image-fit.h b/include/image-fit.h
>> index 27c9e8351..038732d0d 100644
>> --- a/include/image-fit.h
>> +++ b/include/image-fit.h
>> @@ -31,7 +31,8 @@ int fit_has_image(struct fit_handle *handle, void *configuration,
>>  		  const char *name);
>>  int fit_open_image(struct fit_handle *handle, void *configuration,
>>  		   const char *name, const void **outdata,
>> -		   unsigned long *outsize);
>> +		   unsigned long *outsize, unsigned long *load,
>> +		   unsigned long *entry);
>>  
>>  void fit_close(struct fit_handle *handle);
>>  
>>
> 

-- 
--------------------------------------------
embedded brains GmbH
Herr Christian Mauderer
Dornierstr. 4
D-82178 Puchheim
Germany
email: christian.mauderer@embedded-brains.de
Phone: +49-89-18 94 741 - 18
Fax:   +49-89-18 94 741 - 08
PGP: Public key available on request.

Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v4] FIT: Parse `load` and `entry` addresses.
  2020-07-14  9:11 [PATCH v4] FIT: Parse `load` and `entry` addresses Christian Mauderer
  2020-07-14  9:15 ` Ahmad Fatoum
@ 2020-07-14 18:09 ` Sascha Hauer
  2020-07-15  9:30   ` Christian Mauderer
  1 sibling, 1 reply; 5+ messages in thread
From: Sascha Hauer @ 2020-07-14 18:09 UTC (permalink / raw)
  To: Christian Mauderer; +Cc: barebox, a.fatoum

Hi Christian,

On Tue, Jul 14, 2020 at 11:11:39AM +0200, Christian Mauderer wrote:
> @@ -622,7 +622,8 @@ int bootm_boot(struct bootm_data *bootm_data)
>  		}
>  
>  		ret = fit_open_image(data->os_fit, data->fit_config, "kernel",
> -				     &data->fit_kernel, &data->fit_kernel_size);
> +				     &data->fit_kernel, &data->fit_kernel_size,
> +				     &data->os_address, &data->os_entry);

bootm has -a and -e options to specify the load address and the entry
point. If given, these should take precedence over the addresses given
in the FIT image.

>  int fit_open_image(struct fit_handle *handle, void *configuration,
>  		   const char *name, const void **outdata,
> -		   unsigned long *outsize)
> +		   unsigned long *outsize, unsigned long *load,
> +		   unsigned long *entry)
>  {

fit_open_image() has many parameters already and the two new ones are
not always needed or used. I think it's better to make getting the
addresses a separate function, fit_get_load_address() and
fit_get_entry() maybe?

Regards,
  Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
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] 5+ messages in thread

* Re: [PATCH v4] FIT: Parse `load` and `entry` addresses.
  2020-07-14 18:09 ` Sascha Hauer
@ 2020-07-15  9:30   ` Christian Mauderer
  0 siblings, 0 replies; 5+ messages in thread
From: Christian Mauderer @ 2020-07-15  9:30 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: barebox, a.fatoum

Hello Sascha,

thanks for the review. It was a bit more effort and especially the
extra fit_get_image_address (I used one instead of separate ones for
load and entry) moved quite some code around. But I posted a v5 that
should fulfill the asked changes. The output for the four cases (load,
entry set or not set) now looks like the following:

barebox@sometarget:/mnt/mmc bootm -v -d fallback.itb
FIT: Opened FIT image: Some Image
FIT: No match found. Trying default.
FIT: configuration 'conf-1': Boot Application with FDT blob
FIT: image 'kernel': 'RTEMS Application'
FIT: /images/kernel/hash-1: hash OK
FIT: /images/kernel/load: 0x80500000
FIT: /images/kernel/entry: 0x80500000

Loading open firmware Device Tree flattened Binary 'fallback.itb'
OS image not yet relocated
Passing control to FIT image handler
no initrd load address, defaulting to 0x82abb000
FIT: image 'fdt-1': 'Flattened Device Tree blob'
FIT: /images/fdt-1/hash-1: hash OK
commandline:  console=ttymxc0,115200n8

Starting kernel at 0x80500000, oftree at 0x82abb000...
Starting kernel in secure mode
Dryrun. Aborted
barebox@sometarget:/mnt/mmc bootm -v -d -e 0x10 fallback.itb
FIT: Opened FIT image: Some Image
FIT: No match found. Trying default.
FIT: configuration 'conf-1': Boot Application with FDT blob
FIT: image 'kernel': 'RTEMS Application'
FIT: /images/kernel/hash-1: hash OK
FIT: /images/kernel/load: 0x80500000

Loading open firmware Device Tree flattened Binary 'fallback.itb'
OS image not yet relocated
Passing control to FIT image handler
no initrd load address, defaulting to 0x82abb000
FIT: image 'fdt-1': 'Flattened Device Tree blob'
FIT: /images/fdt-1/hash-1: hash OK
commandline:  console=ttymxc0,115200n8

Starting kernel at 0x80500010, oftree at 0x82abb000...
Starting kernel in secure mode
Dryrun. Aborted
barebox@sometarget:/mnt/mmc bootm -v -d -a 0x10 fallback.itb
FIT: Opened FIT image: Some Image
FIT: No match found. Trying default.
FIT: configuration 'conf-1': Boot Application with FDT blob
FIT: image 'kernel': 'RTEMS Application'
FIT: /images/kernel/hash-1: hash OK
FIT: /images/kernel/entry: 0x80500000

Loading open firmware Device Tree flattened Binary 'fallback.itb'
OS image not yet relocated
Passing control to FIT image handler
Dryrun. Aborted
handler failed with: Out of memory
barebox@sometarget:/mnt/mmc bootm -v -d -a 0x10 -e 0x10 fallback.itb
FIT: Opened FIT image: Some Image
FIT: No match found. Trying default.
FIT: configuration 'conf-1': Boot Application with FDT blob
FIT: image 'kernel': 'RTEMS Application'
FIT: /images/kernel/hash-1: hash OK

Loading open firmware Device Tree flattened Binary 'fallback.itb'
OS image not yet relocated
Passing control to FIT image handler
Dryrun. Aborted
handler failed with: Out of memory
barebox@sometarget:/mnt/mmc

Best regards

Christian

On 14/07/2020 20:09, Sascha Hauer wrote:
> Hi Christian,
>
> On Tue, Jul 14, 2020 at 11:11:39AM +0200, Christian Mauderer wrote:
>> @@ -622,7 +622,8 @@ int bootm_boot(struct bootm_data *bootm_data)
>>  		}
>>
>>  		ret = fit_open_image(data->os_fit, data->fit_config, "kernel",
>> -				     &data->fit_kernel, &data->fit_kernel_size);
>> +				     &data->fit_kernel, &data->fit_kernel_size,
>> +				     &data->os_address, &data->os_entry);
>
> bootm has -a and -e options to specify the load address and the entry
> point. If given, these should take precedence over the addresses given
> in the FIT image.
>
>>  int fit_open_image(struct fit_handle *handle, void *configuration,
>>  		   const char *name, const void **outdata,
>> -		   unsigned long *outsize)
>> +		   unsigned long *outsize, unsigned long *load,
>> +		   unsigned long *entry)
>>  {
>
> fit_open_image() has many parameters already and the two new ones are
> not always needed or used. I think it's better to make getting the
> addresses a separate function, fit_get_load_address() and
> fit_get_entry() maybe?
>
> Regards,
>   Sascha
>

-- 
--------------------------------------------
embedded brains GmbH
Herr Christian Mauderer
Dornierstr. 4
D-82178 Puchheim
Germany
email: christian.mauderer@embedded-brains.de
Phone: +49-89-18 94 741 - 18
Fax:   +49-89-18 94 741 - 08
PGP: Public key available on request.

Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-07-15  9:30 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-14  9:11 [PATCH v4] FIT: Parse `load` and `entry` addresses Christian Mauderer
2020-07-14  9:15 ` Ahmad Fatoum
2020-07-14  9:15   ` Christian Mauderer
2020-07-14 18:09 ` Sascha Hauer
2020-07-15  9:30   ` Christian Mauderer

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