On 8/3/23 04:36, Xavier Drudis Ferran wrote: > El Mon, Jul 31, 2023 at 06:43:01PM -0400, Sean Anderson deia: >> This converts the semihosting load method to use spl_load. As a result, it >> also adds support for LOAD_FIT_FULL and IMX images. >> >> Signed-off-by: Sean Anderson <sean.ander...@seco.com> >> --- >> >> Changes in v5: >> - Rework to load header in spl_load >> >> Changes in v2: >> - New >> >> common/spl/spl_semihosting.c | 43 ++++++++++++------------------------ >> 1 file changed, 14 insertions(+), 29 deletions(-) >> >> diff --git a/common/spl/spl_semihosting.c b/common/spl/spl_semihosting.c >> index 5b5e842a11..e7cb9f4ce6 100644 >> --- a/common/spl/spl_semihosting.c >> +++ b/common/spl/spl_semihosting.c >> @@ -9,16 +9,16 @@ >> #include <semihosting.h> >> #include <spl.h> >> >> -static int smh_read_full(long fd, void *memp, size_t len) >> +static ulong spl_smh_fit_read(struct spl_load_info *load, ulong sector, >> + ulong count, void *buf) >> { >> - long read; >> + int ret, fd = *(int *)load->priv; >> > > should ret be long to hold smh_read() return value ?
Yes. >> - read = smh_read(fd, memp, len); >> - if (read < 0) >> - return read; >> - if (read != len) >> - return -EIO; >> - return 0; >> + if (smh_seek(fd, sector)) >> + return 0; >> + > > > > I never used smh, but why would this not be : > > + ret = smh_seek(fd, sector); > + if (ret) > + return ret; Because we always return the number of bytes read. So by returning 0 we signal an error. > (why does smh_seek(), smh_write(), smh_open(), smh_close() return > long, by the way? the implementations return either 0 or smh_errno(), > so int would do ?) Only smh_seek/smh_close always returns 0 or smh_errno. The rest return values from smh_trap. The reason why we use long instead of int is that the semihosting ABI always uses native-register-sized values (aka long). We could use int instead of long for those two functions, but I don't think its very critical. --Sean >> + ret = smh_read(fd, buf, count); >> + return ret < 0 ? 0 : ret; >> } >> >> static int spl_smh_load_image(struct spl_image_info *spl_image, >> @@ -27,14 +27,17 @@ static int spl_smh_load_image(struct spl_image_info >> *spl_image, >> const char *filename = CONFIG_SPL_FS_LOAD_PAYLOAD_NAME; >> int ret; >> long fd, len; >> - struct legacy_img_hdr *header = >> - spl_get_load_buffer(-sizeof(*header), sizeof(*header)); >> + struct spl_load_info load = { >> + .bl_len = 1, >> + .read = spl_smh_fit_read, >> + }; >> >> fd = smh_open(filename, MODE_READ | MODE_BINARY); >> if (fd < 0) { >> log_debug("could not open %s: %ld\n", filename, fd); >> return fd; >> } >> + load.priv = &fd; >> >> ret = smh_flen(fd); >> if (ret < 0) { >> @@ -43,25 +46,7 @@ static int spl_smh_load_image(struct spl_image_info >> *spl_image, >> } >> len = ret; >> >> - ret = smh_read_full(fd, header, sizeof(struct legacy_img_hdr)); >> - if (ret) { >> - log_debug("could not read image header: %d\n", ret); >> - goto out; >> - } >> - >> - ret = spl_parse_image_header(spl_image, bootdev, header); >> - if (ret) { >> - log_debug("failed to parse image header: %d\n", ret); >> - goto out; >> - } >> - >> - ret = smh_seek(fd, 0); >> - if (ret) { >> - log_debug("could not seek to start of image: %d\n", ret); >> - goto out; >> - } >> - >> - ret = smh_read_full(fd, (void *)spl_image->load_addr, len); >> + ret = spl_load(spl_image, bootdev, &load, len, 0); >> if (ret) >> log_debug("could not read %s: %d\n", filename, ret); >> out: >> -- >> 2.40.1 >>