From: Steve Rae <srae@broadcom.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 2/3] arm: semihosting: fix up compile bugs
Date: Fri, 12 Dec 2014 10:03:01 -0800 [thread overview]
Message-ID: <548B2DD5.30703@broadcom.com> (raw)
In-Reply-To: <1416479123-27814-1-git-send-email-linus.walleij@linaro.org>
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>
> }
>
prev parent reply other threads:[~2014-12-12 18:03 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
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 message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=548B2DD5.30703@broadcom.com \
--to=srae@broadcom.com \
--cc=u-boot@lists.denx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.