Re: [U-Boot] [PATCH 5/5] fit: Introduce methods for applying overlays on fit-load

[Lothar Waßmann]

Hi,

On Wed, 5 Jul 2017 00:19:28 +0200 stefan.bru...@rwth-aachen.de wrote:
> On Dienstag, 4. Juli 2017 19:05:25 CEST Pantelis Antoniou wrote:
> > Hi Marek,
> > 
> > On Sat, 2017-07-01 at 16:11 +0200, Marek Vasut wrote:
> > > On 06/30/2017 06:23 PM, Pantelis Antoniou wrote:
> > > > Introduce an overlay based method for constructing a base DT blob
> > > > to pass to the kernel.
> > > > 
> > > > Both canned and runtime feature selection is supported.
> > > > 
> > > > Signed-off-by: Pantelis Antoniou 
> > > 
> > > [...]
> > > 
> > > > @@ -1475,29 +1482,58 @@ int fit_conf_get_node(const void *fit, const
> > > > char *conf_uname)> > 
> > > > debug("Found default configuration: '%s'\n", 
> > > > conf_uname);
> > > > 
> > > > }
> > > > 
> > > > +   s = strchr(conf_uname, '#');
> > > > +   if (s) {
> > > > +   len = s - conf_uname;
> > > > +   conf_uname_copy = malloc(len + 1);
> > > > +   if (!conf_uname_copy) {
> > > > +   debug("Can't allocate uname copy: '%s'\n",
> > > > +   conf_uname);
> > > > +   return -ENOMEM;
> > > > +   }
> > > > +   memcpy(conf_uname_copy, conf_uname, len);
> > > 
> > > Is that like strdup() here ?
> > 
> > No. The space allocated is not the full string, just the span until the
> > next #.
> 
> You could use strndup() to copy only the first n characters.
>  
or strsep() to tokenize (a copy of) the original string.
IMO using strdup() and strsep() makes the intent of this function
clearer, than the current code. 


Lothar Waßmann
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH 5/5] fit: Introduce methods for applying overlays on fit-load

[stefan.bruens]

On Dienstag, 4. Juli 2017 19:05:25 CEST Pantelis Antoniou wrote:
> Hi Marek,
> 
> On Sat, 2017-07-01 at 16:11 +0200, Marek Vasut wrote:
> > On 06/30/2017 06:23 PM, Pantelis Antoniou wrote:
> > > Introduce an overlay based method for constructing a base DT blob
> > > to pass to the kernel.
> > > 
> > > Both canned and runtime feature selection is supported.
> > > 
> > > Signed-off-by: Pantelis Antoniou 
> > 
> > [...]
> > 
> > > @@ -1475,29 +1482,58 @@ int fit_conf_get_node(const void *fit, const
> > > char *conf_uname)> > 
> > >   debug("Found default configuration: '%s'\n", conf_uname);
> > >   
> > >   }
> > > 
> > > + s = strchr(conf_uname, '#');
> > > + if (s) {
> > > + len = s - conf_uname;
> > > + conf_uname_copy = malloc(len + 1);
> > > + if (!conf_uname_copy) {
> > > + debug("Can't allocate uname copy: '%s'\n",
> > > + conf_uname);
> > > + return -ENOMEM;
> > > + }
> > > + memcpy(conf_uname_copy, conf_uname, len);
> > 
> > Is that like strdup() here ?
> 
> No. The space allocated is not the full string, just the span until the
> next #.

You could use strndup() to copy only the first n characters.
 
> > > + conf_uname_copy[len] = '\0';
> > > + conf_uname = conf_uname_copy;
> > > + }
> > > +
> > > 
> > >   noffset = fdt_subnode_offset(fit, confs_noffset, conf_uname);
> > >   if (noffset < 0) {
> > >   
> > >   debug("Can't get node offset for configuration unit name: '%s'
> > >   (%s)\n",
> > >   
> > > conf_uname, fdt_strerror(noffset));
> > >   
> > >   }
> > > 
> > > + if (conf_uname_copy)

No need for a null ptr check, IIRC.

> > > + free(conf_uname_copy);
> > > +
> > > 
> > >   return noffset;
> > >  
> > >  }
> > 
> > [...]
> 
> Regards
> 
> -- Pantelis
> 
> 
> ___
> U-Boot mailing list
> U-Boot@lists.denx.de
> https://lists.denx.de/listinfo/u-boot


___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH 5/5] fit: Introduce methods for applying overlays on fit-load

[Pantelis Antoniou]

Hi Marek,

On Sat, 2017-07-01 at 16:11 +0200, Marek Vasut wrote:
> On 06/30/2017 06:23 PM, Pantelis Antoniou wrote:
> > Introduce an overlay based method for constructing a base DT blob
> > to pass to the kernel.
> > 
> > Both canned and runtime feature selection is supported.
> > 
> > Signed-off-by: Pantelis Antoniou 
> 
> [...]
> 
> > @@ -1475,29 +1482,58 @@ int fit_conf_get_node(const void *fit, const char 
> > *conf_uname)
> > debug("Found default configuration: '%s'\n", conf_uname);
> > }
> >  
> > +   s = strchr(conf_uname, '#');
> > +   if (s) {
> > +   len = s - conf_uname;
> > +   conf_uname_copy = malloc(len + 1);
> > +   if (!conf_uname_copy) {
> > +   debug("Can't allocate uname copy: '%s'\n",
> > +   conf_uname);
> > +   return -ENOMEM;
> > +   }
> > +   memcpy(conf_uname_copy, conf_uname, len);
> 
> Is that like strdup() here ?
> 

No. The space allocated is not the full string, just the span until the
next #.

> > +   conf_uname_copy[len] = '\0';
> > +   conf_uname = conf_uname_copy;
> > +   }
> > +
> > noffset = fdt_subnode_offset(fit, confs_noffset, conf_uname);
> > if (noffset < 0) {
> > debug("Can't get node offset for configuration unit name: '%s' 
> > (%s)\n",
> >   conf_uname, fdt_strerror(noffset));
> > }
> >  
> > +   if (conf_uname_copy)
> > +   free(conf_uname_copy);
> > +
> > return noffset;
> >  }
> 
> [...]
> 
> 

Regards

-- Pantelis


___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH 5/5] fit: Introduce methods for applying overlays on fit-load

[Marek Vasut]

On 06/30/2017 06:23 PM, Pantelis Antoniou wrote:
> Introduce an overlay based method for constructing a base DT blob
> to pass to the kernel.
> 
> Both canned and runtime feature selection is supported.
> 
> Signed-off-by: Pantelis Antoniou 

[...]

> @@ -1475,29 +1482,58 @@ int fit_conf_get_node(const void *fit, const char 
> *conf_uname)
>   debug("Found default configuration: '%s'\n", conf_uname);
>   }
>  
> + s = strchr(conf_uname, '#');
> + if (s) {
> + len = s - conf_uname;
> + conf_uname_copy = malloc(len + 1);
> + if (!conf_uname_copy) {
> + debug("Can't allocate uname copy: '%s'\n",
> + conf_uname);
> + return -ENOMEM;
> + }
> + memcpy(conf_uname_copy, conf_uname, len);

Is that like strdup() here ?

> + conf_uname_copy[len] = '\0';
> + conf_uname = conf_uname_copy;
> + }
> +
>   noffset = fdt_subnode_offset(fit, confs_noffset, conf_uname);
>   if (noffset < 0) {
>   debug("Can't get node offset for configuration unit name: '%s' 
> (%s)\n",
> conf_uname, fdt_strerror(noffset));
>   }
>  
> + if (conf_uname_copy)
> + free(conf_uname_copy);
> +
>   return noffset;
>  }

[...]


-- 
Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH 5/5] fit: Introduce methods for applying overlays on fit-load

[Pantelis Antoniou]

Introduce an overlay based method for constructing a base DT blob
to pass to the kernel.

Both canned and runtime feature selection is supported.

Signed-off-by: Pantelis Antoniou 
---
 common/image-fdt.c   |   7 +-
 common/image-fit.c   | 215 --
 doc/uImage.FIT/command_syntax_extensions.txt |  12 +-
 doc/uImage.FIT/overlay-fdt-boot.txt  | 221 +++
 doc/uImage.FIT/source_file_format.txt|   6 +-
 include/image.h  |  10 ++
 6 files changed, 453 insertions(+), 18 deletions(-)
 create mode 100644 doc/uImage.FIT/overlay-fdt-boot.txt

diff --git a/common/image-fdt.c b/common/image-fdt.c
index c6e8832..a59134c 100644
--- a/common/image-fdt.c
+++ b/common/image-fdt.c
@@ -356,17 +356,16 @@ int boot_get_fdt(int flag, int argc, char * const argv[], 
uint8_t arch,
if (fit_check_format(buf)) {
ulong load, len;
 
-   fdt_noffset = fit_image_load(images,
+   fdt_noffset = boot_get_fdt_fit(images,
fdt_addr, _uname_fdt,
_uname_config,
-   arch, IH_TYPE_FLATDT,
-   BOOTSTAGE_ID_FIT_FDT_START,
-   FIT_LOAD_OPTIONAL, , );
+   arch, , );
 
images->fit_hdr_fdt = map_sysmem(fdt_addr, 0);
images->fit_uname_fdt = fit_uname_fdt;
images->fit_noffset_fdt = fdt_noffset;
fdt_addr = load;
+
break;
} else
 #endif
diff --git a/common/image-fit.c b/common/image-fit.c
index 109ecfa..f78c5cf 100644
--- a/common/image-fit.c
+++ b/common/image-fit.c
@@ -19,6 +19,7 @@
 #include 
 #include 
 #include 
+#include 
 DECLARE_GLOBAL_DATA_PTR;
 #endif /* !USE_HOSTCC*/
 
@@ -434,6 +435,10 @@ void fit_image_print(const void *fit, int image_noffset, 
const char *p)
printf("0x%08lx\n", load);
}
 
+   /* optional load address for FDT */
+   if (type == IH_TYPE_FLATDT && !fit_image_get_load(fit, image_noffset, 
))
+   printf("%s  Load Address: 0x%08lx\n", p, load);
+
if ((type == IH_TYPE_KERNEL) || (type == IH_TYPE_STANDALONE) ||
(type == IH_TYPE_RAMDISK)) {
ret = fit_image_get_entry(fit, image_noffset, );
@@ -1454,6 +1459,8 @@ int fit_conf_get_node(const void *fit, const char 
*conf_uname)
 {
int noffset, confs_noffset;
int len;
+   const char *s;
+   char *conf_uname_copy = NULL;
 
confs_noffset = fdt_path_offset(fit, FIT_CONFS_PATH);
if (confs_noffset < 0) {
@@ -1475,29 +1482,58 @@ int fit_conf_get_node(const void *fit, const char 
*conf_uname)
debug("Found default configuration: '%s'\n", conf_uname);
}
 
+   s = strchr(conf_uname, '#');
+   if (s) {
+   len = s - conf_uname;
+   conf_uname_copy = malloc(len + 1);
+   if (!conf_uname_copy) {
+   debug("Can't allocate uname copy: '%s'\n",
+   conf_uname);
+   return -ENOMEM;
+   }
+   memcpy(conf_uname_copy, conf_uname, len);
+   conf_uname_copy[len] = '\0';
+   conf_uname = conf_uname_copy;
+   }
+
noffset = fdt_subnode_offset(fit, confs_noffset, conf_uname);
if (noffset < 0) {
debug("Can't get node offset for configuration unit name: '%s' 
(%s)\n",
  conf_uname, fdt_strerror(noffset));
}
 
+   if (conf_uname_copy)
+   free(conf_uname_copy);
+
return noffset;
 }
 
-int fit_conf_get_prop_node(const void *fit, int noffset,
+int fit_conf_get_prop_node_count(const void *fit, int noffset,
const char *prop_name)
 {
-   char *uname;
+   return fdt_stringlist_count(fit, noffset, prop_name);
+}
+
+int fit_conf_get_prop_node_index(const void *fit, int noffset,
+   const char *prop_name, int index)
+{
+   const char *uname;
int len;
 
/* get kernel image unit name from configuration kernel property */
-   uname = (char *)fdt_getprop(fit, noffset, prop_name, );
+   uname = fdt_stringlist_get(fit, noffset, prop_name, index, );
if (uname == NULL)
return len;
 
return fit_image_get_node(fit, uname);
 }
 
+int fit_conf_get_prop_node(const void *fit, int noffset,
+   const char *prop_name)
+{
+   return fit_conf_get_prop_node_index(fit, noffset, prop_name, 0);
+}
+
 /**
  * fit_conf_print - prints out the FIT configuration details