* [U-Boot] [PATCH 2/3] arm: semihosting: fix up compile bugs
@ 2014-11-20 10:25 Linus Walleij
2014-12-12 18:03 ` Steve Rae
0 siblings, 1 reply; 2+ messages in thread
From: Linus Walleij @ 2014-11-20 10:25 UTC (permalink / raw)
To: u-boot
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;
}
--
1.9.3
^ permalink raw reply related [flat|nested] 2+ messages in thread
* [U-Boot] [PATCH 2/3] arm: semihosting: fix up compile bugs
2014-11-20 10:25 [U-Boot] [PATCH 2/3] arm: semihosting: fix up compile bugs Linus Walleij
@ 2014-12-12 18:03 ` Steve Rae
0 siblings, 0 replies; 2+ messages in thread
From: Steve Rae @ 2014-12-12 18:03 UTC (permalink / raw)
To: u-boot
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,
> + 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;
unnecessary cast ???
otherwise
Acked-by: Steve Rae <srae@broadcom.com>
> }
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2014-12-12 18:03 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-11-20 10:25 [U-Boot] [PATCH 2/3] arm: semihosting: fix up compile bugs Linus Walleij
2014-12-12 18:03 ` Steve Rae
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox