[U-Boot] [PATCH 1/2 v2] fit: Support compression for non-kernel components (e.g. FDT)

Julius Werner jwerner at chromium.org
Fri May 3 21:27:04 UTC 2019


> Oh, ok. Guess I was just surprised that it says "can't extract compat"
> when it tries to check "comp"...

Well, what I'm trying to say in that error is "can't extract the
compatible string *because* the FDT is compressed", but without making
it three lines long.

> We should get a test together for this. There is an existing
> test_fit.py which might be expanded, or perhaps create a new one and
> share some code.

Oh, neat, I didn't know that exists there! Added a test case with
compressed images.

> > @@ -1795,11 +1803,12 @@ int fit_image_load(bootm_headers_t *images, ulong addr,
> >         const char *fit_uname_config;
> >         const char *fit_base_uname_config;
> >         const void *fit;
> > -       const void *buf;
> > +       void *buf;
> > +       void *loadbuf;
> >         size_t size;
> >         int type_ok, os_ok;
> > -       ulong load, data, len;
> > -       uint8_t os;
> > +       ulong load, load_end, data, len;
> > +       uint8_t os, comp;
> >  #ifndef USE_HOSTCC
> >         uint8_t os_arch;
> >  #endif
> > @@ -1895,12 +1904,6 @@ int fit_image_load(bootm_headers_t *images, ulong addr,
> >         images->os.arch = os_arch;
> >  #endif
> >
> > -       if (image_type == IH_TYPE_FLATDT &&
> > -           !fit_image_check_comp(fit, noffset, IH_COMP_NONE)) {
> > -               puts("FDT image is compressed");
> > -               return -EPROTONOSUPPORT;
> > -       }
> > -
> >         bootstage_mark(bootstage_id + BOOTSTAGE_SUB_CHECK_ALL);
> >         type_ok = fit_image_check_type(fit, noffset, image_type) ||
> >                   fit_image_check_type(fit, noffset, IH_TYPE_FIRMWARE) ||
> > @@ -1931,7 +1934,8 @@ 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_and_size(fit, noffset, &buf, &size)) {
> > +       if (fit_image_get_data_and_size(fit, noffset,
> > +                                       (const void **)&buf, &size)) {
> >                 printf("Could not find %s subimage data!\n", prop_name);
> >                 bootstage_error(bootstage_id + BOOTSTAGE_SUB_GET_DATA);
> >                 return -ENOENT;
> > @@ -1939,30 +1943,15 @@ int fit_image_load(bootm_headers_t *images, ulong addr,
> >
> >  #if !defined(USE_HOSTCC) && defined(CONFIG_FIT_IMAGE_POST_PROCESS)
> >         /* perform any post-processing on the image data */
> > -       board_fit_image_post_process((void **)&buf, &size);
> > +       board_fit_image_post_process(&buf, &size);
> >  #endif
> >
> >         len = (ulong)size;
> >
> > -       /* verify that image data is a proper FDT blob */
> > -       if (image_type == IH_TYPE_FLATDT && fdt_check_header(buf)) {
> > -               puts("Subimage data is not a FDT");
> > -               return -ENOEXEC;
> > -       }
> > -
> >         bootstage_mark(bootstage_id + BOOTSTAGE_SUB_GET_DATA_OK);
> >
> > -       /*
> > -        * Work-around for eldk-4.2 which gives this warning if we try to
> > -        * cast in the unmap_sysmem() call:
> > -        * warning: initialization discards qualifiers from pointer target type
> > -        */
> > -       {
> > -               void *vbuf = (void *)buf;
> > -
> > -               data = map_to_sysmem(vbuf);
> > -       }
> > -
>
> We don't support gcc 4.2 now so this code can be simplified to the
> line you have below, but perhaps this should be in a separate patch?

Well, I just rearranged the whole thing by removing the const
qualifier from 'buf' itself, which I think makes this all way less
confusing. So this block should become obsolete regardless of compiler
version. That's somewhat intertwined with changing around all the load
address handling so I hope keeping it all in one patch is okay.

> > +       comp = IH_COMP_NONE;
> > +       loadbuf = buf;
> > +       /* Kernel images get decompressed later in bootm_host_load_image(). */
>
> But only for the host, right?

Oh, right, good catch, looked up the wrong function name. The normal
case goes through bootm_load_os().


More information about the U-Boot mailing list