[PATCH v5 09/11] spl: Convert semihosting to spl_load

[Heinrich Schuchardt]

On 03.08.23 10: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.anderson at 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 ?
>
>> -	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;
>
> (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 ?)

The return type used should match the implementation on the host side.
The ARM documentation explicitly refers to register R0 which will match
the length of long (or unsigned long).

For smh_seek, smh_read(), smh_write()  we should investigate if the type
should not better be unsigned long to handle a number of bytes exceeding
2^31 on a 32bit emulation or system. But that is beyond the scope of
this patch series.

Best regards

Heinrich