[U-Boot] common: image-fit: add FIT data-position & data-offset property support

Message ID 1526448939-23869-1-git-send-email-keguang.zhang@gmail.com
State Superseded
Delegated to: Tom Rini
Headers show
Series
  • [U-Boot] common: image-fit: add FIT data-position & data-offset property support
Related show

Commit Message

Kelvin Cheung May 16, 2018, 5:35 a.m.
Add FIT data-position & data-offset property support for bootm,
which are already supported in SPL.

Signed-off-by: Kelvin Cheung <keguang.zhang@gmail.com>
---

 common/image-fit.c | 82 +++++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 78 insertions(+), 4 deletions(-)

Comments

Simon Glass May 16, 2018, 3:40 p.m. | #1
Hi Kevin,

On 16 May 2018 at 15:35, Kelvin Cheung <keguang.zhang@gmail.com> wrote:
> Add FIT data-position & data-offset property support for bootm,
> which are already supported in SPL.
>
> Signed-off-by: Kelvin Cheung <keguang.zhang@gmail.com>
> ---
>
>  common/image-fit.c | 82 +++++++++++++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 78 insertions(+), 4 deletions(-)
>
> diff --git a/common/image-fit.c b/common/image-fit.c
> index 5b93dce..f21db4b 100644
> --- a/common/image-fit.c
> +++ b/common/image-fit.c
> @@ -359,11 +359,14 @@ void fit_image_print(const void *fit, int image_noffset, const char *p)
>  {
>         char *desc;
>         uint8_t type, arch, os, comp;
> -       size_t size;
> +       size_t size = 0;
>         ulong load, entry;
>         const void *data;
>         int noffset;
>         int ndepth;
> +       bool external_data = false;
> +       int offset;
> +       int len;
>         int ret;
>
>         /* Mandatory properties */
> @@ -391,7 +394,27 @@ void fit_image_print(const void *fit, int image_noffset, const char *p)
>         fit_image_get_comp(fit, image_noffset, &comp);
>         printf("%s  Compression:  %s\n", p, genimg_get_comp_name(comp));
>
> -       ret = fit_image_get_data(fit, image_noffset, &data, &size);
> +       if (!fit_image_get_data_position(fit, image_noffset, &offset)) {
> +               external_data = true;
> +       } else if (!fit_image_get_data_offset(fit, image_noffset, &offset)) {
> +               external_data = true;
> +               /*
> +                * For FIT with external data, figure out where
> +                * the external images start. This is the base
> +                * for the data-offset properties in each image.
> +                */
> +               size = (fdt_totalsize(fit) + 3) & ~3;
> +               offset += size;
> +       }
> +
> +       if (external_data) {
> +               printf("%s  External Data\n", p);
> +               ret = fit_image_get_data_size(fit, image_noffset, &len);
> +               data = fit + offset;
> +               size = len;
> +       } else {
> +               ret = fit_image_get_data(fit, image_noffset, &data, &size);
> +       }

It looks like you should create which does all of the above, that can
be called in the three places where you repeat this code.

Perhaps fit_image_get_data_buf_size()?

>
>  #ifndef USE_HOSTCC
>         printf("%s  Data Start:   ", p);
> @@ -1151,9 +1174,35 @@ int fit_image_verify(const void *fit, int image_noffset)
>         size_t          size;
>         int             noffset = 0;
>         char            *err_msg = "";
> +       bool external_data = false;
> +       int offset;
> +       int len;
> +       int ret;
>
>         /* Get image data and data length */
> -       if (fit_image_get_data(fit, image_noffset, &data, &size)) {
> +       if (!fit_image_get_data_position(fit, image_noffset, &offset)) {
> +               external_data = true;
> +       } else if (!fit_image_get_data_offset(fit, image_noffset, &offset)) {
> +               external_data = true;
> +               /*
> +                * For FIT with external data, figure out where
> +                * the external images start. This is the base
> +                * for the data-offset properties in each image.
> +                */
> +               size = (fdt_totalsize(fit) + 3) & ~3;
> +               offset += size;
> +       }
> +
> +       if (external_data) {
> +               debug("External Data\n");
> +               ret = fit_image_get_data_size(fit, image_noffset, &len);
> +               data = fit + offset;
> +               size = len;
> +       } else {
> +               ret = fit_image_get_data(fit, image_noffset, &data, &size);
> +       }
> +
> +       if (ret) {
>                 err_msg = "Can't get image data/size";
>                 printf("error!\n%s for '%s' hash node in '%s' image node\n",
>                        err_msg, fit_get_name(fit, noffset, NULL),
> @@ -1750,6 +1799,9 @@ int fit_image_load(bootm_headers_t *images, ulong addr,
>         uint8_t os_arch;
>  #endif
>         const char *prop_name;
> +       bool external_data = false;
> +       int offset;
> +       int data_len;
>         int ret;
>
>         fit = map_sysmem(addr, 0);
> @@ -1875,7 +1927,29 @@ int fit_image_load(bootm_headers_t *images, ulong addr,
>         bootstage_mark(bootstage_id + BOOTSTAGE_SUB_CHECK_ALL_OK);
>
>         /* get image data address and length */
> -       if (fit_image_get_data(fit, noffset, &buf, &size)) {
> +       if (!fit_image_get_data_position(fit, noffset, &offset)) {
> +               external_data = true;
> +       } else if (!fit_image_get_data_offset(fit, noffset, &offset)) {
> +               external_data = true;
> +               /*
> +                * For FIT with external data, figure out where
> +                * the external images start. This is the base
> +                * for the data-offset properties in each image.
> +                */
> +               size = (fdt_totalsize(fit) + 3) & ~3;
> +               offset += size;
> +       }
> +
> +       if (external_data) {
> +               debug("External Data\n");
> +               ret = fit_image_get_data_size(fit, noffset, &data_len);
> +               buf = fit + offset;
> +               size = data_len;
> +       } else {
> +               ret = fit_image_get_data(fit, noffset, &buf, &size);
> +       }
> +
> +       if (ret) {
>                 printf("Could not find %s subimage data!\n", prop_name);
>                 bootstage_error(bootstage_id + BOOTSTAGE_SUB_GET_DATA);
>                 return -ENOENT;
> --
> 1.9.1
>

Regards,
Simon
Kelvin Cheung May 17, 2018, 2:15 a.m. | #2
Hi Simon,
You are right.
I will update the patch soon.
Thanks very much!

2018-05-16 23:40 GMT+08:00 Simon Glass <sjg@chromium.org>:

> Hi Kevin,
>
> On 16 May 2018 at 15:35, Kelvin Cheung <keguang.zhang@gmail.com> wrote:
> > Add FIT data-position & data-offset property support for bootm,
> > which are already supported in SPL.
> >
> > Signed-off-by: Kelvin Cheung <keguang.zhang@gmail.com>
> > ---
> >
> >  common/image-fit.c | 82 ++++++++++++++++++++++++++++++
> +++++++++++++++++++++---
> >  1 file changed, 78 insertions(+), 4 deletions(-)
> >
> > diff --git a/common/image-fit.c b/common/image-fit.c
> > index 5b93dce..f21db4b 100644
> > --- a/common/image-fit.c
> > +++ b/common/image-fit.c
> > @@ -359,11 +359,14 @@ void fit_image_print(const void *fit, int
> image_noffset, const char *p)
> >  {
> >         char *desc;
> >         uint8_t type, arch, os, comp;
> > -       size_t size;
> > +       size_t size = 0;
> >         ulong load, entry;
> >         const void *data;
> >         int noffset;
> >         int ndepth;
> > +       bool external_data = false;
> > +       int offset;
> > +       int len;
> >         int ret;
> >
> >         /* Mandatory properties */
> > @@ -391,7 +394,27 @@ void fit_image_print(const void *fit, int
> image_noffset, const char *p)
> >         fit_image_get_comp(fit, image_noffset, &comp);
> >         printf("%s  Compression:  %s\n", p, genimg_get_comp_name(comp));
> >
> > -       ret = fit_image_get_data(fit, image_noffset, &data, &size);
> > +       if (!fit_image_get_data_position(fit, image_noffset, &offset)) {
> > +               external_data = true;
> > +       } else if (!fit_image_get_data_offset(fit, image_noffset,
> &offset)) {
> > +               external_data = true;
> > +               /*
> > +                * For FIT with external data, figure out where
> > +                * the external images start. This is the base
> > +                * for the data-offset properties in each image.
> > +                */
> > +               size = (fdt_totalsize(fit) + 3) & ~3;
> > +               offset += size;
> > +       }
> > +
> > +       if (external_data) {
> > +               printf("%s  External Data\n", p);
> > +               ret = fit_image_get_data_size(fit, image_noffset, &len);
> > +               data = fit + offset;
> > +               size = len;
> > +       } else {
> > +               ret = fit_image_get_data(fit, image_noffset, &data,
> &size);
> > +       }
>
> It looks like you should create which does all of the above, that can
> be called in the three places where you repeat this code.
>
> Perhaps fit_image_get_data_buf_size()?
>
> >
> >  #ifndef USE_HOSTCC
> >         printf("%s  Data Start:   ", p);
> > @@ -1151,9 +1174,35 @@ int fit_image_verify(const void *fit, int
> image_noffset)
> >         size_t          size;
> >         int             noffset = 0;
> >         char            *err_msg = "";
> > +       bool external_data = false;
> > +       int offset;
> > +       int len;
> > +       int ret;
> >
> >         /* Get image data and data length */
> > -       if (fit_image_get_data(fit, image_noffset, &data, &size)) {
> > +       if (!fit_image_get_data_position(fit, image_noffset, &offset)) {
> > +               external_data = true;
> > +       } else if (!fit_image_get_data_offset(fit, image_noffset,
> &offset)) {
> > +               external_data = true;
> > +               /*
> > +                * For FIT with external data, figure out where
> > +                * the external images start. This is the base
> > +                * for the data-offset properties in each image.
> > +                */
> > +               size = (fdt_totalsize(fit) + 3) & ~3;
> > +               offset += size;
> > +       }
> > +
> > +       if (external_data) {
> > +               debug("External Data\n");
> > +               ret = fit_image_get_data_size(fit, image_noffset, &len);
> > +               data = fit + offset;
> > +               size = len;
> > +       } else {
> > +               ret = fit_image_get_data(fit, image_noffset, &data,
> &size);
> > +       }
> > +
> > +       if (ret) {
> >                 err_msg = "Can't get image data/size";
> >                 printf("error!\n%s for '%s' hash node in '%s' image
> node\n",
> >                        err_msg, fit_get_name(fit, noffset, NULL),
> > @@ -1750,6 +1799,9 @@ int fit_image_load(bootm_headers_t *images, ulong
> addr,
> >         uint8_t os_arch;
> >  #endif
> >         const char *prop_name;
> > +       bool external_data = false;
> > +       int offset;
> > +       int data_len;
> >         int ret;
> >
> >         fit = map_sysmem(addr, 0);
> > @@ -1875,7 +1927,29 @@ int fit_image_load(bootm_headers_t *images, ulong
> addr,
> >         bootstage_mark(bootstage_id + BOOTSTAGE_SUB_CHECK_ALL_OK);
> >
> >         /* get image data address and length */
> > -       if (fit_image_get_data(fit, noffset, &buf, &size)) {
> > +       if (!fit_image_get_data_position(fit, noffset, &offset)) {
> > +               external_data = true;
> > +       } else if (!fit_image_get_data_offset(fit, noffset, &offset)) {
> > +               external_data = true;
> > +               /*
> > +                * For FIT with external data, figure out where
> > +                * the external images start. This is the base
> > +                * for the data-offset properties in each image.
> > +                */
> > +               size = (fdt_totalsize(fit) + 3) & ~3;
> > +               offset += size;
> > +       }
> > +
> > +       if (external_data) {
> > +               debug("External Data\n");
> > +               ret = fit_image_get_data_size(fit, noffset, &data_len);
> > +               buf = fit + offset;
> > +               size = data_len;
> > +       } else {
> > +               ret = fit_image_get_data(fit, noffset, &buf, &size);
> > +       }
> > +
> > +       if (ret) {
> >                 printf("Could not find %s subimage data!\n", prop_name);
> >                 bootstage_error(bootstage_id + BOOTSTAGE_SUB_GET_DATA);
> >                 return -ENOENT;
> > --
> > 1.9.1
> >
>
> Regards,
> Simon
>

Patch

diff --git a/common/image-fit.c b/common/image-fit.c
index 5b93dce..f21db4b 100644
--- a/common/image-fit.c
+++ b/common/image-fit.c
@@ -359,11 +359,14 @@  void fit_image_print(const void *fit, int image_noffset, const char *p)
 {
 	char *desc;
 	uint8_t type, arch, os, comp;
-	size_t size;
+	size_t size = 0;
 	ulong load, entry;
 	const void *data;
 	int noffset;
 	int ndepth;
+	bool external_data = false;
+	int offset;
+	int len;
 	int ret;
 
 	/* Mandatory properties */
@@ -391,7 +394,27 @@  void fit_image_print(const void *fit, int image_noffset, const char *p)
 	fit_image_get_comp(fit, image_noffset, &comp);
 	printf("%s  Compression:  %s\n", p, genimg_get_comp_name(comp));
 
-	ret = fit_image_get_data(fit, image_noffset, &data, &size);
+	if (!fit_image_get_data_position(fit, image_noffset, &offset)) {
+		external_data = true;
+	} else if (!fit_image_get_data_offset(fit, image_noffset, &offset)) {
+		external_data = true;
+		/*
+		 * For FIT with external data, figure out where
+		 * the external images start. This is the base
+		 * for the data-offset properties in each image.
+		 */
+		size = (fdt_totalsize(fit) + 3) & ~3;
+		offset += size;
+	}
+
+	if (external_data) {
+		printf("%s  External Data\n", p);
+		ret = fit_image_get_data_size(fit, image_noffset, &len);
+		data = fit + offset;
+		size = len;
+	} else {
+		ret = fit_image_get_data(fit, image_noffset, &data, &size);
+	}
 
 #ifndef USE_HOSTCC
 	printf("%s  Data Start:   ", p);
@@ -1151,9 +1174,35 @@  int fit_image_verify(const void *fit, int image_noffset)
 	size_t		size;
 	int		noffset = 0;
 	char		*err_msg = "";
+	bool external_data = false;
+	int offset;
+	int len;
+	int ret;
 
 	/* Get image data and data length */
-	if (fit_image_get_data(fit, image_noffset, &data, &size)) {
+	if (!fit_image_get_data_position(fit, image_noffset, &offset)) {
+		external_data = true;
+	} else if (!fit_image_get_data_offset(fit, image_noffset, &offset)) {
+		external_data = true;
+		/*
+		 * For FIT with external data, figure out where
+		 * the external images start. This is the base
+		 * for the data-offset properties in each image.
+		 */
+		size = (fdt_totalsize(fit) + 3) & ~3;
+		offset += size;
+	}
+
+	if (external_data) {
+		debug("External Data\n");
+		ret = fit_image_get_data_size(fit, image_noffset, &len);
+		data = fit + offset;
+		size = len;
+	} else {
+		ret = fit_image_get_data(fit, image_noffset, &data, &size);
+	}
+
+	if (ret) {
 		err_msg = "Can't get image data/size";
 		printf("error!\n%s for '%s' hash node in '%s' image node\n",
 		       err_msg, fit_get_name(fit, noffset, NULL),
@@ -1750,6 +1799,9 @@  int fit_image_load(bootm_headers_t *images, ulong addr,
 	uint8_t os_arch;
 #endif
 	const char *prop_name;
+	bool external_data = false;
+	int offset;
+	int data_len;
 	int ret;
 
 	fit = map_sysmem(addr, 0);
@@ -1875,7 +1927,29 @@  int fit_image_load(bootm_headers_t *images, ulong addr,
 	bootstage_mark(bootstage_id + BOOTSTAGE_SUB_CHECK_ALL_OK);
 
 	/* get image data address and length */
-	if (fit_image_get_data(fit, noffset, &buf, &size)) {
+	if (!fit_image_get_data_position(fit, noffset, &offset)) {
+		external_data = true;
+	} else if (!fit_image_get_data_offset(fit, noffset, &offset)) {
+		external_data = true;
+		/*
+		 * For FIT with external data, figure out where
+		 * the external images start. This is the base
+		 * for the data-offset properties in each image.
+		 */
+		size = (fdt_totalsize(fit) + 3) & ~3;
+		offset += size;
+	}
+
+	if (external_data) {
+		debug("External Data\n");
+		ret = fit_image_get_data_size(fit, noffset, &data_len);
+		buf = fit + offset;
+		size = data_len;
+	} else {
+		ret = fit_image_get_data(fit, noffset, &buf, &size);
+	}
+
+	if (ret) {
 		printf("Could not find %s subimage data!\n", prop_name);
 		bootstage_error(bootstage_id + BOOTSTAGE_SUB_GET_DATA);
 		return -ENOENT;