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
>>
