| Message ID | 20200410110431.12256-1-sr@denx.de |
|---|---|
| State | New |
| Headers | show |
| Series | [RFC,1/3] spl: spl_nor: Move legacy image loading into spl_legacy.c | expand |
Am 10.04.20 um 13:04 schrieb Stefan Roese: > Move the legacy image loading into spl_legacy.c. This makes it easier > to extend the legacy image handling with new features that other > SPL loaders might use (e.g. spl_spi.c etc). > > No functional change intended. > > Signed-off-by: Stefan Roese <sr at denx.de> > Cc: Weijie Gao <weijie.gao at mediatek.com> > Cc: Daniel Schwierzeck <daniel.schwierzeck at gmail.com> > Cc: Simon Goldschmidt <simon.k.r.goldschmidt at gmail.com> > --- > RFC comment: I'm sendig these 3 patches as RFC and once we've come to an > agreement on these (Acked etc), I'll integrate them into the mtmips SPL > patchset from Weijie and will send v7. > > Thanks, > Stefan > > common/spl/spl_legacy.c | 20 ++++++++++++++++++++ > common/spl/spl_nor.c | 27 +++++++++++++-------------- > include/spl.h | 13 +++++++++++++ > 3 files changed, 46 insertions(+), 14 deletions(-) Reviewed-by: Daniel Schwierzeck <daniel.schwierzeck at gmail.com> nits below > > diff --git a/common/spl/spl_legacy.c b/common/spl/spl_legacy.c > index 772135193e..7f00fc8885 100644 > --- a/common/spl/spl_legacy.c > +++ b/common/spl/spl_legacy.c > @@ -51,3 +51,23 @@ int spl_parse_legacy_header(struct spl_image_info *spl_image, > > return 0; > } > + > +int spl_load_legacy_img(struct spl_image_info *spl_image, > + struct spl_load_info *load, ulong header) > +{ > + struct image_header hdr; > + int ret; > + > + /* Read header into local struct */ > + load->read(load, header, sizeof(hdr), &hdr); > + > + ret = spl_parse_image_header(spl_image, &hdr); > + if (ret) > + return ret; > + > + /* Read image */ > + load->read(load, header + sizeof(hdr), spl_image->size, > + (void *)(unsigned long)spl_image->load_addr); > + > + return 0; > +} > diff --git a/common/spl/spl_nor.c b/common/spl/spl_nor.c > index b1e79b9ded..3f03ffe6a3 100644 > --- a/common/spl/spl_nor.c > +++ b/common/spl/spl_nor.c > @@ -24,7 +24,6 @@ unsigned long __weak spl_nor_get_uboot_base(void) > static int spl_nor_load_image(struct spl_image_info *spl_image, > struct spl_boot_device *bootdev) > { > - int ret; > __maybe_unused const struct image_header *header; > __maybe_unused struct spl_load_info load; > > @@ -43,6 +42,8 @@ static int spl_nor_load_image(struct spl_image_info *spl_image, > header = (const struct image_header *)CONFIG_SYS_OS_BASE; > #ifdef CONFIG_SPL_LOAD_FIT > if (image_get_magic(header) == FDT_MAGIC) { > + int ret; > + > debug("Found FIT\n"); > load.bl_len = 1; > load.read = spl_nor_load_read; > @@ -61,6 +62,7 @@ static int spl_nor_load_image(struct spl_image_info *spl_image, > #endif > if (image_get_os(header) == IH_OS_LINUX) { > /* happy - was a Linux */ > + int ret; > > ret = spl_parse_image_header(spl_image, header); > if (ret) > @@ -93,11 +95,9 @@ static int spl_nor_load_image(struct spl_image_info *spl_image, > debug("Found FIT format U-Boot\n"); > load.bl_len = 1; > load.read = spl_nor_load_read; > - ret = spl_load_simple_fit(spl_image, &load, > - spl_nor_get_uboot_base(), > - (void *)header); > - > - return ret; > + return spl_load_simple_fit(spl_image, &load, > + spl_nor_get_uboot_base(), > + (void *)header); looks like an unrelated optimisation > } > #endif > if (IS_ENABLED(CONFIG_SPL_LOAD_IMX_CONTAINER)) { > @@ -107,14 +107,13 @@ static int spl_nor_load_image(struct spl_image_info *spl_image, > spl_nor_get_uboot_base()); > } > > - ret = spl_parse_image_header(spl_image, > - (const struct image_header *)spl_nor_get_uboot_base()); > - if (ret) > - return ret; > - > - memcpy((void *)(unsigned long)spl_image->load_addr, > - (void *)(spl_nor_get_uboot_base() + sizeof(struct image_header)), > - spl_image->size); > + /* Legacy image handling */ > + if (IS_ENABLED(CONFIG_SPL_LEGACY_IMAGE_SUPPORT)) { > + load.bl_len = 1; > + load.read = spl_nor_load_read; > + return spl_load_legacy_img(spl_image, &load, > + spl_nor_get_uboot_base()); > + } > > return 0; > } > diff --git a/include/spl.h b/include/spl.h > index 6087cd793c..c6c64b6a72 100644 > --- a/include/spl.h > +++ b/include/spl.h > @@ -219,6 +219,19 @@ int spl_load_simple_fit(struct spl_image_info *spl_image, > #define SPL_COPY_PAYLOAD_ONLY 1 > #define SPL_FIT_FOUND 2 > > +/** > + * spl_load_legacy_img() - Loads a legacy image from a device. > + * @spl_image: Image description to set up > + * @load: Structure containing the information required to load data. > + * @header: Pointer to image header (including appended image) > + * > + * Reads an legacy image from the device. Loads u-boot image to > + * specified load address. > + * Returns 0 on success. > + */ > +int spl_load_legacy_img(struct spl_image_info *spl_image, > + struct spl_load_info *load, ulong header); > + > /** > * spl_load_imx_container() - Loads a imx container image from a device. > * @spl_image: Image description to set up >
On 15.04.20 13:46, Daniel Schwierzeck wrote: > > > Am 10.04.20 um 13:04 schrieb Stefan Roese: >> Move the legacy image loading into spl_legacy.c. This makes it easier >> to extend the legacy image handling with new features that other >> SPL loaders might use (e.g. spl_spi.c etc). >> >> No functional change intended. >> >> Signed-off-by: Stefan Roese <sr at denx.de> >> Cc: Weijie Gao <weijie.gao at mediatek.com> >> Cc: Daniel Schwierzeck <daniel.schwierzeck at gmail.com> >> Cc: Simon Goldschmidt <simon.k.r.goldschmidt at gmail.com> >> --- >> RFC comment: I'm sendig these 3 patches as RFC and once we've come to an >> agreement on these (Acked etc), I'll integrate them into the mtmips SPL >> patchset from Weijie and will send v7. >> >> Thanks, >> Stefan >> >> common/spl/spl_legacy.c | 20 ++++++++++++++++++++ >> common/spl/spl_nor.c | 27 +++++++++++++-------------- >> include/spl.h | 13 +++++++++++++ >> 3 files changed, 46 insertions(+), 14 deletions(-) > > Reviewed-by: Daniel Schwierzeck <daniel.schwierzeck at gmail.com> > > nits below > >> >> diff --git a/common/spl/spl_legacy.c b/common/spl/spl_legacy.c >> index 772135193e..7f00fc8885 100644 >> --- a/common/spl/spl_legacy.c >> +++ b/common/spl/spl_legacy.c >> @@ -51,3 +51,23 @@ int spl_parse_legacy_header(struct spl_image_info *spl_image, >> >> return 0; >> } >> + >> +int spl_load_legacy_img(struct spl_image_info *spl_image, >> + struct spl_load_info *load, ulong header) >> +{ >> + struct image_header hdr; >> + int ret; >> + >> + /* Read header into local struct */ >> + load->read(load, header, sizeof(hdr), &hdr); >> + >> + ret = spl_parse_image_header(spl_image, &hdr); >> + if (ret) >> + return ret; >> + >> + /* Read image */ >> + load->read(load, header + sizeof(hdr), spl_image->size, >> + (void *)(unsigned long)spl_image->load_addr); >> + >> + return 0; >> +} >> diff --git a/common/spl/spl_nor.c b/common/spl/spl_nor.c >> index b1e79b9ded..3f03ffe6a3 100644 >> --- a/common/spl/spl_nor.c >> +++ b/common/spl/spl_nor.c >> @@ -24,7 +24,6 @@ unsigned long __weak spl_nor_get_uboot_base(void) >> static int spl_nor_load_image(struct spl_image_info *spl_image, >> struct spl_boot_device *bootdev) >> { >> - int ret; >> __maybe_unused const struct image_header *header; >> __maybe_unused struct spl_load_info load; >> >> @@ -43,6 +42,8 @@ static int spl_nor_load_image(struct spl_image_info *spl_image, >> header = (const struct image_header *)CONFIG_SYS_OS_BASE; >> #ifdef CONFIG_SPL_LOAD_FIT >> if (image_get_magic(header) == FDT_MAGIC) { >> + int ret; >> + >> debug("Found FIT\n"); >> load.bl_len = 1; >> load.read = spl_nor_load_read; >> @@ -61,6 +62,7 @@ static int spl_nor_load_image(struct spl_image_info *spl_image, >> #endif >> if (image_get_os(header) == IH_OS_LINUX) { >> /* happy - was a Linux */ >> + int ret; >> >> ret = spl_parse_image_header(spl_image, header); >> if (ret) >> @@ -93,11 +95,9 @@ static int spl_nor_load_image(struct spl_image_info *spl_image, >> debug("Found FIT format U-Boot\n"); >> load.bl_len = 1; >> load.read = spl_nor_load_read; >> - ret = spl_load_simple_fit(spl_image, &load, >> - spl_nor_get_uboot_base(), >> - (void *)header); >> - >> - return ret; >> + return spl_load_simple_fit(spl_image, &load, >> + spl_nor_get_uboot_base(), >> + (void *)header); > > looks like an unrelated optimisation It looks this way. But its related: This is needed, as the if statement below now results in a warning when the usage of the "ret" variable is not changed: common/spl/spl_nor.c:27:6: warning: unused variable 'ret' [-Wunused-variable] That's why I removed "ret" completely in this patch. I could either generate a small patch "before" this one to remove "ret" or mention this removal in the commit text of this patch. Thanks, Stefan >> } >> #endif >> if (IS_ENABLED(CONFIG_SPL_LOAD_IMX_CONTAINER)) { >> @@ -107,14 +107,13 @@ static int spl_nor_load_image(struct spl_image_info *spl_image, >> spl_nor_get_uboot_base()); >> } >> >> - ret = spl_parse_image_header(spl_image, >> - (const struct image_header *)spl_nor_get_uboot_base()); >> - if (ret) >> - return ret; >> - >> - memcpy((void *)(unsigned long)spl_image->load_addr, >> - (void *)(spl_nor_get_uboot_base() + sizeof(struct image_header)), >> - spl_image->size); >> + /* Legacy image handling */ >> + if (IS_ENABLED(CONFIG_SPL_LEGACY_IMAGE_SUPPORT)) { >> + load.bl_len = 1; >> + load.read = spl_nor_load_read; >> + return spl_load_legacy_img(spl_image, &load, >> + spl_nor_get_uboot_base()); >> + } >> >> return 0; >> } >> diff --git a/include/spl.h b/include/spl.h >> index 6087cd793c..c6c64b6a72 100644 >> --- a/include/spl.h >> +++ b/include/spl.h >> @@ -219,6 +219,19 @@ int spl_load_simple_fit(struct spl_image_info *spl_image, >> #define SPL_COPY_PAYLOAD_ONLY 1 >> #define SPL_FIT_FOUND 2 >> >> +/** >> + * spl_load_legacy_img() - Loads a legacy image from a device. >> + * @spl_image: Image description to set up >> + * @load: Structure containing the information required to load data. >> + * @header: Pointer to image header (including appended image) >> + * >> + * Reads an legacy image from the device. Loads u-boot image to >> + * specified load address. >> + * Returns 0 on success. >> + */ >> +int spl_load_legacy_img(struct spl_image_info *spl_image, >> + struct spl_load_info *load, ulong header); >> + >> /** >> * spl_load_imx_container() - Loads a imx container image from a device. >> * @spl_image: Image description to set up >> > Viele Gr??e, Stefan
diff --git a/common/spl/spl_legacy.c b/common/spl/spl_legacy.c index 772135193e..7f00fc8885 100644 --- a/common/spl/spl_legacy.c +++ b/common/spl/spl_legacy.c @@ -51,3 +51,23 @@ int spl_parse_legacy_header(struct spl_image_info *spl_image, return 0; } + +int spl_load_legacy_img(struct spl_image_info *spl_image, + struct spl_load_info *load, ulong header) +{ + struct image_header hdr; + int ret; + + /* Read header into local struct */ + load->read(load, header, sizeof(hdr), &hdr); + + ret = spl_parse_image_header(spl_image, &hdr); + if (ret) + return ret; + + /* Read image */ + load->read(load, header + sizeof(hdr), spl_image->size, + (void *)(unsigned long)spl_image->load_addr); + + return 0; +} diff --git a/common/spl/spl_nor.c b/common/spl/spl_nor.c index b1e79b9ded..3f03ffe6a3 100644 --- a/common/spl/spl_nor.c +++ b/common/spl/spl_nor.c @@ -24,7 +24,6 @@ unsigned long __weak spl_nor_get_uboot_base(void) static int spl_nor_load_image(struct spl_image_info *spl_image, struct spl_boot_device *bootdev) { - int ret; __maybe_unused const struct image_header *header; __maybe_unused struct spl_load_info load; @@ -43,6 +42,8 @@ static int spl_nor_load_image(struct spl_image_info *spl_image, header = (const struct image_header *)CONFIG_SYS_OS_BASE; #ifdef CONFIG_SPL_LOAD_FIT if (image_get_magic(header) == FDT_MAGIC) { + int ret; + debug("Found FIT\n"); load.bl_len = 1; load.read = spl_nor_load_read; @@ -61,6 +62,7 @@ static int spl_nor_load_image(struct spl_image_info *spl_image, #endif if (image_get_os(header) == IH_OS_LINUX) { /* happy - was a Linux */ + int ret; ret = spl_parse_image_header(spl_image, header); if (ret) @@ -93,11 +95,9 @@ static int spl_nor_load_image(struct spl_image_info *spl_image, debug("Found FIT format U-Boot\n"); load.bl_len = 1; load.read = spl_nor_load_read; - ret = spl_load_simple_fit(spl_image, &load, - spl_nor_get_uboot_base(), - (void *)header); - - return ret; + return spl_load_simple_fit(spl_image, &load, + spl_nor_get_uboot_base(), + (void *)header); } #endif if (IS_ENABLED(CONFIG_SPL_LOAD_IMX_CONTAINER)) { @@ -107,14 +107,13 @@ static int spl_nor_load_image(struct spl_image_info *spl_image, spl_nor_get_uboot_base()); } - ret = spl_parse_image_header(spl_image, - (const struct image_header *)spl_nor_get_uboot_base()); - if (ret) - return ret; - - memcpy((void *)(unsigned long)spl_image->load_addr, - (void *)(spl_nor_get_uboot_base() + sizeof(struct image_header)), - spl_image->size); + /* Legacy image handling */ + if (IS_ENABLED(CONFIG_SPL_LEGACY_IMAGE_SUPPORT)) { + load.bl_len = 1; + load.read = spl_nor_load_read; + return spl_load_legacy_img(spl_image, &load, + spl_nor_get_uboot_base()); + } return 0; } diff --git a/include/spl.h b/include/spl.h index 6087cd793c..c6c64b6a72 100644 --- a/include/spl.h +++ b/include/spl.h @@ -219,6 +219,19 @@ int spl_load_simple_fit(struct spl_image_info *spl_image, #define SPL_COPY_PAYLOAD_ONLY 1 #define SPL_FIT_FOUND 2 +/** + * spl_load_legacy_img() - Loads a legacy image from a device. + * @spl_image: Image description to set up + * @load: Structure containing the information required to load data. + * @header: Pointer to image header (including appended image) + * + * Reads an legacy image from the device. Loads u-boot image to + * specified load address. + * Returns 0 on success. + */ +int spl_load_legacy_img(struct spl_image_info *spl_image, + struct spl_load_info *load, ulong header); + /** * spl_load_imx_container() - Loads a imx container image from a device. * @spl_image: Image description to set up
Move the legacy image loading into spl_legacy.c. This makes it easier to extend the legacy image handling with new features that other SPL loaders might use (e.g. spl_spi.c etc). No functional change intended. Signed-off-by: Stefan Roese <sr at denx.de> Cc: Weijie Gao <weijie.gao at mediatek.com> Cc: Daniel Schwierzeck <daniel.schwierzeck at gmail.com> Cc: Simon Goldschmidt <simon.k.r.goldschmidt at gmail.com> --- RFC comment: I'm sendig these 3 patches as RFC and once we've come to an agreement on these (Acked etc), I'll integrate them into the mtmips SPL patchset from Weijie and will send v7. Thanks, Stefan common/spl/spl_legacy.c | 20 ++++++++++++++++++++ common/spl/spl_nor.c | 27 +++++++++++++-------------- include/spl.h | 13 +++++++++++++ 3 files changed, 46 insertions(+), 14 deletions(-)