Hi Simon,
On 05/15/2015 09:57 AM, Simon Glass wrote:
Hi Karl,
On 13 May 2015 at 06:53, Karl Apsite Karl.Apsite@dornerworks.com wrote:
From: Karl Apsite karl.apsite@dornerworks.com
Added a trimmed down instance of boot_get_<thing>() to satisfy the minimum requierments of the added feature. The function follows the normal patterns set by other boot_get<thing>'s, which should make it a bit easier to combine them all together into one boot_get_image() function in a later refactor.
Documentation for the new function can be found in source: common/image.c
Signed-off-by: Karl Apsite Karl.Apsite@dornerworks.com
common/bootm.c | 22 +++++++++++++ common/image-fit.c | 8 ++++- common/image.c | 93 +++++++++++++++++++++++++++++++++++++++++++++++++++++ include/bootstage.h | 1 + include/image.h | 5 ++- 5 files changed, 127 insertions(+), 2 deletions(-)
This looks fine to me. Can you please add documentation - doc/uImage.FIT
This will be included in one of the missing commits
A few bits below.
diff --git a/common/bootm.c b/common/bootm.c index 6842029..f04e49b 100644 --- a/common/bootm.c +++ b/common/bootm.c @@ -240,6 +240,23 @@ static int bootm_find_fdt(int flag, int argc, char * const argv[]) } #endif
+#if defined(CONFIG_FIT) +static int bootm_find_loadables(int flag, int argc, char * const argv[]) +{
int ret;/* find all of the loadables */ret = boot_get_loadable(argc, argv, &images, IH_ARCH_DEFAULT,NULL, NULL);if (ret) {puts("Loadable(s) is corrupt or invalid\n");printf() here (we are trying to avoid using puts() so that one day we can make it compatible with the C library puts in that it automatically outputs \n).
I will convert puts to printf in all 3 cases.
return 1;}return 0;+} +#endif
int bootm_find_ramdisk_fdt(int flag, int argc, char * const argv[]) { if (bootm_find_ramdisk(flag, argc, argv)) @@ -250,6 +267,11 @@ int bootm_find_ramdisk_fdt(int flag, int argc, char * const argv[]) return 1; #endif
+#if defined(CONFIG_FIT)
if (bootm_find_loadables(flag, argc, argv))return 1;+#endif
return 0;}
diff --git a/common/image-fit.c b/common/image-fit.c index 245264d..fe23c13 100644 --- a/common/image-fit.c +++ b/common/image-fit.c @@ -1544,6 +1544,8 @@ static const char *fit_get_image_type_property(int type) return FIT_RAMDISK_PROP; case IH_TYPE_X86_SETUP: return FIT_SETUP_PROP;
case IH_TYPE_LOADABLE:return FIT_LOADABLE_PROP; } return "unknown";@@ -1661,7 +1663,11 @@ int fit_image_load(bootm_headers_t *images, ulong addr, os_ok = image_type == IH_TYPE_FLATDT || fit_image_check_os(fit, noffset, IH_OS_LINUX) || fit_image_check_os(fit, noffset, IH_OS_OPENRTOS);
if (!type_ok || !os_ok) {
/* If either of the checks fail, we should report an error, but/*
- If either ...
- what it is
*/
I'll correct the comment
* if the image type is coming from the "loadables" field, we* don't care about what it is */if ((!type_ok || !os_ok) && image_type != IH_TYPE_LOADABLE) { fit_image_get_os(fit, noffset, &os); printf("No %s %s %s Image\n", genimg_get_os_name(os),diff --git a/common/image.c b/common/image.c index fdec496..dc7e795 100644 --- a/common/image.c +++ b/common/image.c @@ -1165,6 +1165,99 @@ int boot_get_setup(bootm_headers_t *images, uint8_t arch, #endif }
+/**
- boot_get_loadable - routine to load a list of binaries to memory
- @argc: Ignored Argument
- @argv: Ignored Argument
- @images: pointer to the bootm images structure
- @arch: expected architecture for the image
- @ld_start: Ignored Argument
- @ld_len: Ignored Argument
- boot_get_loadable() will take the given FIT configuration, and look
- for a field named "loadables". Loadables, is a list of elements in
- the FIT given as strings. exe:
- loadables = "linux_kernel@1", "fdt@2";
- this function will attempt to parse each string, and load the
- corresponding element from the FIT into memory. Once placed,
- no aditional actions are taken.
- returns:
@return
I did not see @return in the linked documentation: http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Document...
I will change it to @return
0, if only valid images or no images are found
error code, if an error occurs during fit_image_loadGreat docs.
- */
+#if defined(CONFIG_FIT) +int boot_get_loadable(int argc, char * const argv[], bootm_headers_t *images,
uint8_t arch, const ulong *ld_start, ulong * const ld_len)+{
/** These variabels are used to hold the current image location* in system memory.*/
s/variabels/variables
ulong tmp_img_addr;/* These are not used */
I will make this comment a bit more clear
ulong img_data, img_len;void *buf;char *uname;char *uname_end;int len, fit_img_result;blank line
I looked, and didn't see a blank line, so I take this to mean you WANT a line here.
/* Check to see if the images struct has a FIT configuration */if (genimg_has_config(images)) {Can we do...
if (!genimg_has_config(images)) { debug("## FIT configuration was not specified\n"); return 0; }
just to limit the indent level?
Totally!
/** Obtain the os FIT header from the images struct* copy from dataflash if needed*/tmp_img_addr = map_to_sysmem(images->fit_hdr_os);tmp_img_addr = genimg_get_image(tmp_img_addr);buf = map_sysmem(tmp_img_addr, 0);/** Check image type. For FIT images get FIT node* and attempt to locate a generic binary.*/switch (genimg_get_format(buf)) {case IMAGE_FORMAT_FIT:/** Try to obtain the 'loadables' field from the* configuration If it exists, then try and load* all of the listed strings** This logic assumes a character is one byte*/uname = (char *)fdt_getprop(buf,do you need the cast?
It is a (const void *) without the cast. An error is tossed if I don't have the cast because I am discarding the ‘const’ qualifier. Probably a moot point.
fit_conf_get_node(buf, images->fit_uname_cfg),FIT_LOADABLE_PROP, &len);uname_end = uname + len;if (NULL != uname) {if (uname)
but perhaps you should use:
for (index = 0; !fdt_get_string_index(...index...); index++) { process string
Sure!
[snip] +#endif
#ifdef CONFIG_SYS_BOOT_GET_CMDLINE /**
- boot_get_cmdline - allocate and initialize kernel cmdline
diff --git a/include/bootstage.h b/include/bootstage.h index be44014..4e2e0fb 100644 --- a/include/bootstage.h +++ b/include/bootstage.h @@ -168,6 +168,7 @@ enum bootstage_id { BOOTSTAGE_ID_NAND_FIT_READ = 150, BOOTSTAGE_ID_NAND_FIT_READ_OK,
BOOTSTAGE_ID_FIT_LOADS_START = 160, /* for Loadable Images */FIT_LOADABLE_START?
The name is getting a bit long, but I can change it.
/* * These boot stages are new, higher level, and not directly related * to the old boot progress numbers. They are useful for recordingdiff --git a/include/image.h b/include/image.h index 97b96b3..0b0b55b 100644 --- a/include/image.h +++ b/include/image.h @@ -244,6 +244,7 @@ struct lmb; #define IH_TYPE_SOCFPGAIMAGE 19 /* Altera SOCFPGA Preloader */ #define IH_TYPE_X86_SETUP 20 /* x86 setup.bin Image */ #define IH_TYPE_LPC32XXIMAGE 21 /* x86 setup.bin Image */ +#define IH_TYPE_LOADABLE 22 /* A list of typeless images */
/*
- Compression Types
@@ -455,7 +456,9 @@ ulong genimg_get_image(ulong img_addr);
int boot_get_ramdisk(int argc, char * const argv[], bootm_headers_t *images, uint8_t arch, ulong *rd_start, ulong *rd_end); -#endif +int boot_get_loadable(int argc, char * const argv[], bootm_headers_t *images,
uint8_t arch, const ulong *ld_start, ulong * const ld_len);Function comment should go here since this is the API definition (move it from the C file).
I agree, however by that logic, ALL of the comments on non-static functions in common/image.c should move. If that's correct, then it might be a good idea to defer this until someone can get around to moving the lot of them together, to maintain consistency.
Current examples: http://git.denx.de/?p=u-boot.git;a=blob;f=common/image.c;h=fdec496c4bf0e936f...
http://git.denx.de/?p=u-boot.git;a=blob;f=common/image-fdt.c;h=7e2da7b3b7218...
+#endif /* !USE_HOSTCC */
int boot_get_setup_fit(bootm_headers_t *images, uint8_t arch, ulong *setup_start, ulong *setup_len); -- 2.3.7
Regards, Simon