[RTEMS Project] #2128: [Patch] io.h: Use uint32_t and co. instead of "unsigned int"

RTEMS trac trac at rtems.org
Sun Nov 23 00:16:37 UTC 2014


#2128: [Patch] io.h: Use uint32_t and co. instead of "unsigned int"
--------------------------+----------------------------
 Reporter:  nick.withers  |       Owner:  joel.sherrill
     Type:  defect        |      Status:  new
 Priority:  normal        |   Milestone:  4.11
Component:  cpukit        |     Version:  HEAD
 Severity:  trivial       |  Resolution:
 Keywords:                |
--------------------------+----------------------------

Old description:

> In at least the mvme3100's libcpu/io.h, inputs and outputs for in_be16()
> and co. are declared as unsigned ints, shorts and so forth.
>
> This can lead to spurious warnings during application compile if the same
> types are not used (and they shouldn't be, IMHO); e.g.:
> ____
>
> powerpc-rtems4.11-gcc --pipe -std=c99 -g -Wall -mcpu=powerpc -msoft-float
> -D__ppc_generic -I. -I/home/nick/rtems/rtems-4.11/powerpc-
> rtems4.11/mvme3100/lib/include -I/home/nick/rtems/RTEMS_gdb_stub_1_5 -O0
> -DDEBUG=1 -DDEBUG_GDB_STUB=1 -DDEBUG_INIT_DONE_RESET=0
> -DDEBUG_HALT_ON_FATAL=0 -c -o modules/v785.o modules/v785.c
> modules/v785.c: In function 'v785_read_data':
> modules/v785.c:200:2: warning: passing argument 1 of 'in_be32' from
> incompatible pointer type [enabled by default]
>   if (((output_buffer_word = in_be32((volatile uint32_t *) ((uintptr_t)
> (card->a24_addr ? card->a24_addr : card->a16_addr) + (uintptr_t)
> v785_addr_output_buffer))) | v785_output_buffer_header_type_mask) ==
> v785_output_buffer_header_type_invalid)
>   ^
> In file included from modules/v785.c:20:0:
> /home/nick/rtems/rtems-4.11/powerpc-
> rtems4.11/mvme3100/lib/include/libcpu/io.h:118:24: note: expected
> 'volatile unsigned int *' but argument is of type 'volatile uint32_t *'
>  static inline unsigned in_be32(volatile unsigned *addr)
>                         ^
> ____
>
> This patch (only touching the MVME3100's io.h) changes the parameter
> types to uint8_t (or similar) and return types to uint_least8_t (or
> similar):
> ____
>
> diff --git a/c/src/lib/libcpu/powerpc/shared/include/io.h
> b/c/src/lib/libcpu/powerpc/shared/include/io.h
> index ebc405d..9deb61d 100644
> --- a/c/src/lib/libcpu/powerpc/shared/include/io.h
> +++ b/c/src/lib/libcpu/powerpc/shared/include/io.h
> @@ -31,18 +31,20 @@
>
>  #include <bsp.h>               /* for _IO_BASE & friends */
>
> +#include <stdint.h>            /* for uint8_t & friends */
> +
>  /* NOTE: The use of these macros is DISCOURAGED.
>   *       you should consider e.g. using in_xxx / out_xxx
>   *       with a device specific base address that is
>   *       defined by the BSP. This makes drivers easier
>   *       to port.
>   */
> -#define inb(port)              in_8((unsigned char *)((port)+_IO_BASE))
> -#define outb(val, port)                out_8((unsigned char
> *)((port)+_IO_BASE), (val))
> -#define inw(port)              in_le16((unsigned short
> *)((port)+_IO_BASE))
> -#define outw(val, port)                out_le16((unsigned short
> *)((port)+_IO_BASE), (val))
> -#define inl(port)              in_le32((unsigned *)((port)+_IO_BASE))
> -#define outl(val, port)                out_le32((unsigned
> *)((port)+_IO_BASE), (val))
> +#define inb(port)              in_8((uint8_t *)((port)+_IO_BASE))
> +#define outb(val, port)                out_8((uint8_t
> *)((port)+_IO_BASE), (val))
> +#define inw(port)              in_le16((uint16_t *)((port)+_IO_BASE))
> +#define outw(val, port)                out_le16((uint16_t
> *)((port)+_IO_BASE), (val))
> +#define inl(port)              in_le32((uint32_t *)((port)+_IO_BASE))
> +#define outl(val, port)                out_le32((uint32_t
> *)((port)+_IO_BASE), (val))
>
>  /*
>   * Enforce In-order Execution of I/O:
> @@ -65,7 +67,7 @@ static inline void eieio(void)
>  /*
>   * 8, 16 and 32 bit, big and little endian I/O operations, with barrier.
>   */
> -static inline int in_8(volatile unsigned char *addr)
> +static inline uint_least8_t in_8(volatile uint8_t *addr)
>  {
>         int ret;
>
> @@ -73,12 +75,12 @@ static inline int in_8(volatile unsigned char *addr)
>         return ret;
>  }
>
> -static inline void out_8(volatile unsigned char *addr, int val)
> +static inline void out_8(volatile uint8_t *addr, uint_least8_t val)
>  {
>         __asm__ __volatile__("stb%U0%X0 %1,%0; eieio" : "=m" (*addr) :
> "r" (val));
>  }
>
> -static inline int in_le16(volatile unsigned short *addr)
> +static inline uint_least16_t in_le16(volatile uint16_t *addr)
>  {
>         int ret;
>
> @@ -87,7 +89,7 @@ static inline int in_le16(volatile unsigned short
> *addr)
>         return ret;
>  }
>
> -static inline int in_be16(volatile unsigned short *addr)
> +static inline uint_least16_t in_be16(volatile uint16_t *addr)
>  {
>         int ret;
>
> @@ -95,18 +97,18 @@ static inline int in_be16(volatile unsigned short
> *addr)
>         return ret;
>  }
>
> -static inline void out_le16(volatile unsigned short *addr, int val)
> +static inline void out_le16(volatile uint16_t *addr, uint_least16_t val)
>  {
>         __asm__ __volatile__("sthbrx %1,0,%2; eieio" : "=m" (*addr) :
>                               "r" (val), "r" (addr));
>  }
>
> -static inline void out_be16(volatile unsigned short *addr, int val)
> +static inline void out_be16(volatile uint16_t *addr, uint_least16_t val)
>  {
>         __asm__ __volatile__("sth%U0%X0 %1,%0; eieio" : "=m" (*addr) :
> "r" (val));
>  }
>
> -static inline unsigned in_le32(volatile unsigned *addr)
> +static inline uint_least32_t in_le32(volatile uint32_t *addr)
>  {
>         unsigned ret;
>
> @@ -115,7 +117,7 @@ static inline unsigned in_le32(volatile unsigned
> *addr)
>         return ret;
>  }
>
> -static inline unsigned in_be32(volatile unsigned *addr)
> +static inline uint_least32_t in_be32(volatile uint32_t *addr)
>  {
>         unsigned ret;
>
> @@ -123,13 +125,13 @@ static inline unsigned in_be32(volatile unsigned
> *addr)
>         return ret;
>  }
>
> -static inline void out_le32(volatile unsigned *addr, int val)
> +static inline void out_le32(volatile uint32_t *addr, uint_least32_t val)
>  {
>         __asm__ __volatile__("stwbrx %1,0,%2; eieio" : "=m" (*addr) :
>                              "r" (val), "r" (addr));
>  }
>
> -static inline void out_be32(volatile unsigned *addr, int val)
> +static inline void out_be32(volatile uint32_t *addr, uint_least32_t val)
>  {
>         __asm__ __volatile__("stw%U0%X0 %1,%0; eieio" : "=m" (*addr) :
> "r" (val));
>  }
> ____

New description:

 In at least the mvme3100's libcpu/io.h, inputs and outputs for in_be16()
 and co. are declared as unsigned ints, shorts and so forth.

 This can lead to spurious warnings during application compile if the same
 types are not used (and they shouldn't be, IMHO); e.g.:
 ____

 powerpc-rtems4.11-gcc --pipe -std=c99 -g -Wall -mcpu=powerpc -msoft-float
 -D__ppc_generic -I. -I/home/nick/rtems/rtems-4.11/powerpc-
 rtems4.11/mvme3100/lib/include -I/home/nick/rtems/RTEMS_gdb_stub_1_5 -O0
 -DDEBUG=1 -DDEBUG_GDB_STUB=1 -DDEBUG_INIT_DONE_RESET=0
 -DDEBUG_HALT_ON_FATAL=0 -c -o modules/v785.o modules/v785.c
 modules/v785.c: In function 'v785_read_data':
 modules/v785.c:200:2: warning: passing argument 1 of 'in_be32' from
 incompatible pointer type [enabled by default]
   if (((output_buffer_word = in_be32((volatile uint32_t *) ((uintptr_t)
 (card->a24_addr ? card->a24_addr : card->a16_addr) + (uintptr_t)
 v785_addr_output_buffer))) | v785_output_buffer_header_type_mask) ==
 v785_output_buffer_header_type_invalid)
   ^
 In file included from modules/v785.c:20:0:
 /home/nick/rtems/rtems-4.11/powerpc-
 rtems4.11/mvme3100/lib/include/libcpu/io.h:118:24: note: expected
 'volatile unsigned int *' but argument is of type 'volatile uint32_t *'
  static inline unsigned in_be32(volatile unsigned *addr)
                         ^
 ____

 This patch (only touching the MVME3100's io.h) changes the parameter types
 to uint8_t (or similar) and return types to uint_least8_t (or similar):
 ____

 diff --git a/c/src/lib/libcpu/powerpc/shared/include/io.h
 b/c/src/lib/libcpu/powerpc/shared/include/io.h
 index ebc405d..9deb61d 100644
 --- a/c/src/lib/libcpu/powerpc/shared/include/io.h
 +++ b/c/src/lib/libcpu/powerpc/shared/include/io.h
 @@ -31,18 +31,20 @@

  #include <bsp.h>               /* for _IO_BASE & friends */

 +#include <stdint.h>            /* for uint8_t & friends */
 +
  /* NOTE: The use of these macros is DISCOURAGED.
   *       you should consider e.g. using in_xxx / out_xxx
   *       with a device specific base address that is
   *       defined by the BSP. This makes drivers easier
   *       to port.
   */
 -#define inb(port)              in_8((unsigned char *)((port)+_IO_BASE))
 -#define outb(val, port)                out_8((unsigned char
 *)((port)+_IO_BASE), (val))
 -#define inw(port)              in_le16((unsigned short
 *)((port)+_IO_BASE))
 -#define outw(val, port)                out_le16((unsigned short
 *)((port)+_IO_BASE), (val))
 -#define inl(port)              in_le32((unsigned *)((port)+_IO_BASE))
 -#define outl(val, port)                out_le32((unsigned
 *)((port)+_IO_BASE), (val))
 +#define inb(port)              in_8((uint8_t *)((port)+_IO_BASE))
 +#define outb(val, port)                out_8((uint8_t
 *)((port)+_IO_BASE), (val))
 +#define inw(port)              in_le16((uint16_t *)((port)+_IO_BASE))
 +#define outw(val, port)                out_le16((uint16_t
 *)((port)+_IO_BASE), (val))
 +#define inl(port)              in_le32((uint32_t *)((port)+_IO_BASE))
 +#define outl(val, port)                out_le32((uint32_t
 *)((port)+_IO_BASE), (val))

  /*
   * Enforce In-order Execution of I/O:
 @@ -65,7 +67,7 @@ static inline void eieio(void)
  /*
   * 8, 16 and 32 bit, big and little endian I/O operations, with barrier.
   */
 -static inline int in_8(volatile unsigned char *addr)
 +static inline uint_least8_t in_8(volatile uint8_t *addr)
  {
         int ret;

 @@ -73,12 +75,12 @@ static inline int in_8(volatile unsigned char *addr)
         return ret;
  }

 -static inline void out_8(volatile unsigned char *addr, int val)
 +static inline void out_8(volatile uint8_t *addr, uint_least8_t val)
  {
         __asm__ __volatile__("stb%U0%X0 %1,%0; eieio" : "=m" (*addr) : "r"
 (val));
  }

 -static inline int in_le16(volatile unsigned short *addr)
 +static inline uint_least16_t in_le16(volatile uint16_t *addr)
  {
         int ret;

 @@ -87,7 +89,7 @@ static inline int in_le16(volatile unsigned short *addr)
         return ret;
  }

 -static inline int in_be16(volatile unsigned short *addr)
 +static inline uint_least16_t in_be16(volatile uint16_t *addr)
  {
         int ret;

 @@ -95,18 +97,18 @@ static inline int in_be16(volatile unsigned short
 *addr)
         return ret;
  }

 -static inline void out_le16(volatile unsigned short *addr, int val)
 +static inline void out_le16(volatile uint16_t *addr, uint_least16_t val)
  {
         __asm__ __volatile__("sthbrx %1,0,%2; eieio" : "=m" (*addr) :
                               "r" (val), "r" (addr));
  }

 -static inline void out_be16(volatile unsigned short *addr, int val)
 +static inline void out_be16(volatile uint16_t *addr, uint_least16_t val)
  {
         __asm__ __volatile__("sth%U0%X0 %1,%0; eieio" : "=m" (*addr) : "r"
 (val));
  }

 -static inline unsigned in_le32(volatile unsigned *addr)
 +static inline uint_least32_t in_le32(volatile uint32_t *addr)
  {
         unsigned ret;

 @@ -115,7 +117,7 @@ static inline unsigned in_le32(volatile unsigned
 *addr)
         return ret;
  }

 -static inline unsigned in_be32(volatile unsigned *addr)
 +static inline uint_least32_t in_be32(volatile uint32_t *addr)
  {
         unsigned ret;

 @@ -123,13 +125,13 @@ static inline unsigned in_be32(volatile unsigned
 *addr)
         return ret;
  }

 -static inline void out_le32(volatile unsigned *addr, int val)
 +static inline void out_le32(volatile uint32_t *addr, uint_least32_t val)
  {
         __asm__ __volatile__("stwbrx %1,0,%2; eieio" : "=m" (*addr) :
                              "r" (val), "r" (addr));
  }

 -static inline void out_be32(volatile unsigned *addr, int val)
 +static inline void out_be32(volatile uint32_t *addr, uint_least32_t val)
  {
         __asm__ __volatile__("stw%U0%X0 %1,%0; eieio" : "=m" (*addr) : "r"
 (val));
  }
 ____

--

Comment (by joel.sherrill):

 Nick... is this still an issue? I don't think the mvme3100 has any
 warnings when building so I am prone to say you should stick to the types
 the method requires.

 We are trying to close tickets so close it or let's discuss this.

 Thanks.

--
Ticket URL: <http://devel.rtems.org/ticket/2128#comment:4>
RTEMS Project <http://www.rtems.org/>
RTEMS Project


More information about the bugs mailing list