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

[Kelvin Cheung]

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 at chromium.org>:

> Hi Kevin,
>
> On 16 May 2018 at 15:35, Kelvin Cheung <keguang.zhang at 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 at 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
>



-- 
Best regards,

Kelvin Cheung