[U-Boot] [PATCH 2/3] arm: semihosting: fix up compile bugs

There is currently a regression when using newer ARM64 compilers for semihosting: the way long types are inferred from context is no longer the same.
The semihosting runtime uses long and size_t, so use this explicitly in the semihosting code and interface, and voila: the code now works again.
Tested with aarch64-linux-gnu-gcc: Linaro GCC 4.9-2014.09.
Cc: Darwin Rambo drambo@broadcom.com Cc: AKASHI Takahiro takahiro.akashi@linaro.org Cc: Mark Hambleton mark.hambleton@arm.com Cc: Tom Rini trini@ti.com Suggested-by: Mark Hambleton mark.hambleton@arm.com Signed-off-by: Linus Walleij linus.walleij@linaro.org --- arch/arm/include/asm/semihosting.h | 2 +- arch/arm/lib/semihosting.c | 101 +++++++++++++++++++------------------ 2 files changed, 52 insertions(+), 51 deletions(-)
diff --git a/arch/arm/include/asm/semihosting.h b/arch/arm/include/asm/semihosting.h index e59b44ed6068..835ca7e4b683 100644 --- a/arch/arm/include/asm/semihosting.h +++ b/arch/arm/include/asm/semihosting.h @@ -12,6 +12,6 @@ * code for more information. */ int smh_load(const char *fname, void *memp, int avail, int verbose); -int smh_len(const char *fname); +long smh_len(const char *fname);
#endif /* __SEMIHOSTING_H__ */ diff --git a/arch/arm/lib/semihosting.c b/arch/arm/lib/semihosting.c index 92bbabe158fe..6e1b2d182eca 100644 --- a/arch/arm/lib/semihosting.c +++ b/arch/arm/lib/semihosting.c @@ -23,17 +23,17 @@ #define MODE_READ 0x0 #define MODE_READBIN 0x1
-static int smh_read(int fd, void *memp, int len); -static int smh_open(const char *fname, char *modestr); -static int smh_close(int fd); -static int smh_len_fd(int fd); +static long smh_read(long fd, void *memp, size_t len); +static long smh_open(const char *fname, char *modestr); +static long smh_close(long fd); +static long smh_len_fd(long fd);
/* * Call the handler */ -static int smh_trap(unsigned int sysnum, void *addr) +static long smh_trap(unsigned int sysnum, void *addr) { - register int result asm("r0"); + register long result asm("r0"); #if defined(CONFIG_ARM64) asm volatile ("hlt #0xf000" : "=r" (result) : "0"(sysnum), "r"(addr)); #else @@ -51,7 +51,9 @@ static int smh_trap(unsigned int sysnum, void *addr) */ int smh_load(const char *fname, void *memp, int avail, int verbose) { - int ret, fd, len; + long ret; + long fd; + size_t len;
ret = -1;
@@ -61,21 +63,21 @@ int smh_load(const char *fname, void *memp, int avail, int verbose) /* Open the file */ fd = smh_open(fname, "rb"); if (fd == -1) - return ret; + return -1;
/* Get the file length */ ret = smh_len_fd(fd); if (ret == -1) { smh_close(fd); - return ret; + return -1; }
/* Check that the file will fit in the supplied buffer */ if (ret > avail) { - printf("%s: ERROR ret %d, avail %u\n", __func__, ret, + printf("%s: ERROR ret %ld, avail %u\n", __func__, ret, avail); smh_close(fd); - return ret; + return -1; }
len = ret; @@ -87,7 +89,7 @@ int smh_load(const char *fname, void *memp, int avail, int verbose) if (verbose) { printf("\n%s\n", fname); printf(" 0x%8p dest\n", memp); - printf(" 0x%08x size\n", len); + printf(" 0x%08lx size\n", len); printf(" 0x%08x avail\n", avail); } } @@ -101,54 +103,53 @@ int smh_load(const char *fname, void *memp, int avail, int verbose) /* * Read 'len' bytes of file into 'memp'. Returns 0 on success, else failure */ -static int smh_read(int fd, void *memp, int len) +static long smh_read(long fd, void *memp, size_t len) { - int ret; + long ret; struct smh_read_s { - int fd; + long fd; void *memp; - int len; + size_t len; } read;
- debug("%s: fd %d, memp %p, len %d\n", __func__, fd, memp, len); + debug("%s: fd %ld, memp %p, len %lu\n", __func__, fd, memp, len);
read.fd = fd; read.memp = memp; read.len = len;
ret = smh_trap(SYSREAD, &read); - if (ret == 0) { - return 0; - } else { + if (ret < 0) { /* * The ARM handler allows for returning partial lengths, * but in practice this never happens so rather than create * hard to maintain partial read loops and such, just fail * with an error message. */ - printf("%s: ERROR ret %d, fd %d, len %u memp %p\n", + printf("%s: ERROR ret %ld, fd %ld, len %lu memp %p\n", __func__, ret, fd, len, memp); + return -1; } - return ret; + + return 0; }
/* * Open a file on the host. Mode is "r" or "rb" currently. Returns a file * descriptor or -1 on error. */ -static int smh_open(const char *fname, char *modestr) +static long smh_open(const char *fname, char *modestr) { - int ret, fd, mode; + long fd; + unsigned long mode; struct smh_open_s { const char *fname; - unsigned int mode; - unsigned int len; + unsigned long mode; + size_t len; } open;
debug("%s: file '%s', mode '%s'\n", __func__, fname, modestr);
- ret = -1; - /* Check the file mode */ if (!(strcmp(modestr, "r"))) { mode = MODE_READ; @@ -157,7 +158,7 @@ static int smh_open(const char *fname, char *modestr) } else { printf("%s: ERROR mode '%s' not supported\n", __func__, modestr); - return ret; + return -1; }
open.fname = fname; @@ -167,7 +168,7 @@ static int smh_open(const char *fname, char *modestr) /* Open the file on the host */ fd = smh_trap(SYSOPEN, &open); if (fd == -1) - printf("%s: ERROR fd %d for file '%s'\n", __func__, fd, + printf("%s: ERROR fd %ld for file '%s'\n", __func__, fd, fname);
return fd; @@ -176,17 +177,15 @@ static int smh_open(const char *fname, char *modestr) /* * Close the file using the file descriptor */ -static int smh_close(int fd) +static long smh_close(long fd) { - int ret; - long fdlong; + long ret;
- debug("%s: fd %d\n", __func__, fd); + debug("%s: fd %ld\n", __func__, fd);
- fdlong = (long)fd; - ret = smh_trap(SYSCLOSE, &fdlong); + ret = smh_trap(SYSCLOSE, &fd); if (ret == -1) - printf("%s: ERROR fd %d\n", __func__, fd); + printf("%s: ERROR fd %ld\n", __func__, fd);
return ret; } @@ -194,17 +193,15 @@ static int smh_close(int fd) /* * Get the file length from the file descriptor */ -static int smh_len_fd(int fd) +static long smh_len_fd(long fd) { - int ret; - long fdlong; + long ret;
- debug("%s: fd %d\n", __func__, fd); + debug("%s: fd %ld\n", __func__, fd);
- fdlong = (long)fd; - ret = smh_trap(SYSFLEN, &fdlong); + ret = smh_trap(SYSFLEN, &fd); if (ret == -1) - printf("%s: ERROR ret %d\n", __func__, ret); + printf("%s: ERROR ret %ld, fd %ld\n", __func__, ret, fd);
return ret; } @@ -212,27 +209,31 @@ static int smh_len_fd(int fd) /* * Get the file length from the filename */ -int smh_len(const char *fname) +long smh_len(const char *fname) { - int ret, fd, len; + long ret; + long fd; + long len;
debug("%s: file '%s'\n", __func__, fname);
/* Open the file */ fd = smh_open(fname, "rb"); - if (fd == -1) + if (fd < 0) return fd;
/* Get the file length */ len = smh_len_fd(fd); + if (len < 0) + return len;
/* Close the file */ ret = smh_close(fd); - if (ret == -1) + if (ret < 0) return ret;
- debug("%s: returning len %d\n", __func__, len); + debug("%s: returning len %ld\n", __func__, len);
/* Return the file length (or -1 error indication) */ - return len; + return (long)len; }

On 14-11-20 02:25 AM, Linus Walleij wrote:
There is currently a regression when using newer ARM64 compilers for semihosting: the way long types are inferred from context is no longer the same.
The semihosting runtime uses long and size_t, so use this explicitly in the semihosting code and interface, and voila: the code now works again.
Tested with aarch64-linux-gnu-gcc: Linaro GCC 4.9-2014.09.
Cc: Darwin Rambo drambo@broadcom.com Cc: AKASHI Takahiro takahiro.akashi@linaro.org Cc: Mark Hambleton mark.hambleton@arm.com Cc: Tom Rini trini@ti.com Suggested-by: Mark Hambleton mark.hambleton@arm.com Signed-off-by: Linus Walleij linus.walleij@linaro.org
arch/arm/include/asm/semihosting.h | 2 +- arch/arm/lib/semihosting.c | 101 +++++++++++++++++++------------------ 2 files changed, 52 insertions(+), 51 deletions(-)
diff --git a/arch/arm/include/asm/semihosting.h b/arch/arm/include/asm/semihosting.h index e59b44ed6068..835ca7e4b683 100644 --- a/arch/arm/include/asm/semihosting.h +++ b/arch/arm/include/asm/semihosting.h @@ -12,6 +12,6 @@
- code for more information.
*/ int smh_load(const char *fname, void *memp, int avail, int verbose); -int smh_len(const char *fname); +long smh_len(const char *fname);
#endif /* __SEMIHOSTING_H__ */ diff --git a/arch/arm/lib/semihosting.c b/arch/arm/lib/semihosting.c index 92bbabe158fe..6e1b2d182eca 100644 --- a/arch/arm/lib/semihosting.c +++ b/arch/arm/lib/semihosting.c @@ -23,17 +23,17 @@ #define MODE_READ 0x0 #define MODE_READBIN 0x1
-static int smh_read(int fd, void *memp, int len); -static int smh_open(const char *fname, char *modestr); -static int smh_close(int fd); -static int smh_len_fd(int fd); +static long smh_read(long fd, void *memp, size_t len); +static long smh_open(const char *fname, char *modestr); +static long smh_close(long fd); +static long smh_len_fd(long fd);
/*
- Call the handler
*/ -static int smh_trap(unsigned int sysnum, void *addr) +static long smh_trap(unsigned int sysnum, void *addr) {
- register int result asm("r0");
- register long result asm("r0"); #if defined(CONFIG_ARM64) asm volatile ("hlt #0xf000" : "=r" (result) : "0"(sysnum), "r"(addr)); #else
@@ -51,7 +51,9 @@ static int smh_trap(unsigned int sysnum, void *addr) */ int smh_load(const char *fname, void *memp, int avail, int verbose) {
- int ret, fd, len;
long ret;
long fd;
size_t len;
ret = -1;
@@ -61,21 +63,21 @@ int smh_load(const char *fname, void *memp, int avail, int verbose) /* Open the file */ fd = smh_open(fname, "rb"); if (fd == -1)
return ret;
return -1;
/* Get the file length */ ret = smh_len_fd(fd); if (ret == -1) { smh_close(fd);
return ret;
return -1;
}
/* Check that the file will fit in the supplied buffer */ if (ret > avail) {
printf("%s: ERROR ret %d, avail %u\n", __func__, ret,
smh_close(fd);printf("%s: ERROR ret %ld, avail %u\n", __func__, ret, avail);
return ret;
return -1;
}
len = ret;
@@ -87,7 +89,7 @@ int smh_load(const char *fname, void *memp, int avail, int verbose) if (verbose) { printf("\n%s\n", fname); printf(" 0x%8p dest\n", memp);
printf(" 0x%08x size\n", len);
} }printf(" 0x%08lx size\n", len); printf(" 0x%08x avail\n", avail);
@@ -101,54 +103,53 @@ int smh_load(const char *fname, void *memp, int avail, int verbose) /*
- Read 'len' bytes of file into 'memp'. Returns 0 on success, else failure
*/ -static int smh_read(int fd, void *memp, int len) +static long smh_read(long fd, void *memp, size_t len) {
- int ret;
- long ret; struct smh_read_s {
int fd;
void *memp;long fd;
int len;
} read;size_t len;
- debug("%s: fd %d, memp %p, len %d\n", __func__, fd, memp, len);
debug("%s: fd %ld, memp %p, len %lu\n", __func__, fd, memp, len);
read.fd = fd; read.memp = memp; read.len = len;
ret = smh_trap(SYSREAD, &read);
- if (ret == 0) {
return 0;
- } else {
- if (ret < 0) { /*
*/
- The ARM handler allows for returning partial lengths,
- but in practice this never happens so rather than create
- hard to maintain partial read loops and such, just fail
- with an error message.
printf("%s: ERROR ret %d, fd %d, len %u memp %p\n",
printf("%s: ERROR ret %ld, fd %ld, len %lu memp %p\n", __func__, ret, fd, len, memp);
}return -1;
- return ret;
return 0; }
/*
- Open a file on the host. Mode is "r" or "rb" currently. Returns a file
- descriptor or -1 on error.
*/
-static int smh_open(const char *fname, char *modestr) +static long smh_open(const char *fname, char *modestr) {
- int ret, fd, mode;
- long fd;
- unsigned long mode; struct smh_open_s { const char *fname;
unsigned int mode;
unsigned int len;
unsigned long mode;
size_t len;
} open;
debug("%s: file '%s', mode '%s'\n", __func__, fname, modestr);
- ret = -1;
- /* Check the file mode */ if (!(strcmp(modestr, "r"))) { mode = MODE_READ;
@@ -157,7 +158,7 @@ static int smh_open(const char *fname, char *modestr) } else { printf("%s: ERROR mode '%s' not supported\n", __func__, modestr);
return ret;
return -1;
}
open.fname = fname;
@@ -167,7 +168,7 @@ static int smh_open(const char *fname, char *modestr) /* Open the file on the host */ fd = smh_trap(SYSOPEN, &open); if (fd == -1)
printf("%s: ERROR fd %d for file \'%s\'\n", __func__, fd,
printf("%s: ERROR fd %ld for file \'%s\'\n", __func__, fd, fname);
return fd;
@@ -176,17 +177,15 @@ static int smh_open(const char *fname, char *modestr) /*
- Close the file using the file descriptor
*/ -static int smh_close(int fd) +static long smh_close(long fd) {
- int ret;
- long fdlong;
- long ret;
- debug("%s: fd %d\n", __func__, fd);
- debug("%s: fd %ld\n", __func__, fd);
- fdlong = (long)fd;
- ret = smh_trap(SYSCLOSE, &fdlong);
- ret = smh_trap(SYSCLOSE, &fd); if (ret == -1)
printf("%s: ERROR fd %d\n", __func__, fd);
printf("%s: ERROR fd %ld\n", __func__, fd);
return ret; }
@@ -194,17 +193,15 @@ static int smh_close(int fd) /*
- Get the file length from the file descriptor
*/ -static int smh_len_fd(int fd) +static long smh_len_fd(long fd) {
- int ret;
- long fdlong;
- long ret;
- debug("%s: fd %d\n", __func__, fd);
- debug("%s: fd %ld\n", __func__, fd);
- fdlong = (long)fd;
- ret = smh_trap(SYSFLEN, &fdlong);
- ret = smh_trap(SYSFLEN, &fd); if (ret == -1)
printf("%s: ERROR ret %d\n", __func__, ret);
printf("%s: ERROR ret %ld, fd %ld\n", __func__, ret, fd);
return ret; }
@@ -212,27 +209,31 @@ static int smh_len_fd(int fd) /*
- Get the file length from the filename
*/ -int smh_len(const char *fname) +long smh_len(const char *fname) {
- int ret, fd, len;
long ret;
long fd;
long len;
debug("%s: file '%s'\n", __func__, fname);
/* Open the file */ fd = smh_open(fname, "rb");
- if (fd == -1)
if (fd < 0) return fd;
/* Get the file length */ len = smh_len_fd(fd);
if (len < 0)
return len;
/* Close the file */ ret = smh_close(fd);
- if (ret == -1)
- if (ret < 0) return ret;
- debug("%s: returning len %d\n", __func__, len);
debug("%s: returning len %ld\n", __func__, len);
/* Return the file length (or -1 error indication) */
- return len;
- return (long)len;
unnecessary cast ??? otherwise
Acked-by: Steve Rae srae@broadcom.com
}
participants (2)
-
Linus Walleij
-
Steve Rae