IOPortBase is exported.  While I agree that compiler.h and most of the bus 
support in xf86 should die in a fire, you really shouldn't change the symbol 
name for no good reason.

1) I would prefer to see a fix for siliconmotion driver to have it "do the 
right thing" by not calling into this broken API.

2) You should use void * instead of char * for generic pointers.



On Oct 14, 2011, at 5:10 AM, Matt Kraai wrote:

> Change IOPortBase to ioBase and set it properly on MIPS systems.
> 
> Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=41038
> Signed-off-by: Matt Kraai <[email protected]>
> ---
> hw/xfree86/common/compiler.h            |   14 +++++++-------
> hw/xfree86/os-support/bsd/arm_video.c   |   20 ++++++++++----------
> hw/xfree86/os-support/linux/lnx_video.c |    9 +++++++--
> 3 files changed, 24 insertions(+), 19 deletions(-)
> 
> diff --git a/hw/xfree86/common/compiler.h b/hw/xfree86/common/compiler.h
> index 9e00d75..61af0c4 100644
> --- a/hw/xfree86/common/compiler.h
> +++ b/hw/xfree86/common/compiler.h
> @@ -714,42 +714,42 @@ xf86WriteMmio32LeNB(__volatile__ void *base, const 
> unsigned long offset,
> #     define PORT_SIZE short
> #    endif
> 
> -_X_EXPORT unsigned int IOPortBase;  /* Memory mapped I/O port area */
> +_X_EXPORT unsigned char *ioBase;
> 
> static __inline__ void
> outb(unsigned PORT_SIZE port, unsigned char val)
> {
> -     *(volatile unsigned char*)(((unsigned PORT_SIZE)(port))+IOPortBase) = 
> val;
> +     *(volatile unsigned char*)(((unsigned PORT_SIZE)(port))+ioBase) = val;
> }
> 
> static __inline__ void
> outw(unsigned PORT_SIZE port, unsigned short val)
> {
> -     *(volatile unsigned short*)(((unsigned PORT_SIZE)(port))+IOPortBase) = 
> val;
> +     *(volatile unsigned short*)(((unsigned PORT_SIZE)(port))+ioBase) = val;
> }
> 
> static __inline__ void
> outl(unsigned PORT_SIZE port, unsigned int val)
> {
> -     *(volatile unsigned int*)(((unsigned PORT_SIZE)(port))+IOPortBase) = 
> val;
> +     *(volatile unsigned int*)(((unsigned PORT_SIZE)(port))+ioBase) = val;
> }
> 
> static __inline__ unsigned int
> inb(unsigned PORT_SIZE port)
> {
> -     return *(volatile unsigned char*)(((unsigned 
> PORT_SIZE)(port))+IOPortBase);
> +     return *(volatile unsigned char*)(((unsigned PORT_SIZE)(port))+ioBase);
> }
> 
> static __inline__ unsigned int
> inw(unsigned PORT_SIZE port)
> {
> -     return *(volatile unsigned short*)(((unsigned 
> PORT_SIZE)(port))+IOPortBase);
> +     return *(volatile unsigned short*)(((unsigned PORT_SIZE)(port))+ioBase);
> }
> 
> static __inline__ unsigned int
> inl(unsigned PORT_SIZE port)
> {
> -     return *(volatile unsigned int*)(((unsigned 
> PORT_SIZE)(port))+IOPortBase);
> +     return *(volatile unsigned int*)(((unsigned PORT_SIZE)(port))+ioBase);
> }
> 
> 
> diff --git a/hw/xfree86/os-support/bsd/arm_video.c 
> b/hw/xfree86/os-support/bsd/arm_video.c
> index eb631a7..2bc6d3b 100644
> --- a/hw/xfree86/os-support/bsd/arm_video.c
> +++ b/hw/xfree86/os-support/bsd/arm_video.c
> @@ -495,7 +495,7 @@ xf86EnableIO()
>                               MAP_FLAGS, fd, (off_t)0x0000);
> 
>               if (base != (pointer)-1) {
> -                     IOPortBase = base;
> +                     ioBase = base;
>               }
>               else {
>                       xf86Msg(X_WARNING,"EnableIO: failed to mmap %s (%s)\n",
> @@ -562,7 +562,7 @@ int ScreenNum;
>                               MAP_FLAGS, fd, (off_t)0x0000);
> 
>               if (base != (pointer)-1) {
> -                     IOPortBase = base;
> +                     ioBase = base;
>               }
>               else {
>                       xf86Msg(X_ERROR,
> @@ -577,20 +577,20 @@ int ScreenNum;
> #endif
> 
> #ifdef __arm32__
> -     IOPortBase = (unsigned int)-1;
> +     ioBase = (unsigned char *)-1;
> 
>       if((memInfoP = checkMapInfo(TRUE, MMIO_REGION)) != NULL)
>       {
>           /* 
>            * xf86MapInfoMap maps an offset from the start of video IO
> -          * space (e.g. 0x3B0), but IOPortBase is expected to map to
> +          * space (e.g. 0x3B0), but ioBase is expected to map to
>            * physical address 0x000, so subtract the start of video I/O
>            * space from the result.  This is safe for now becase we
>            * actually mmap the start of the page, then the start of video
>            * I/O space is added as an internal offset.
>            */
> -         IOPortBase = (unsigned int)xf86MapInfoMap(memInfoP,
> -                                                   (caddr_t)0x0, 0L) 
> +         ioBase = (unsigned char *)xf86MapInfoMap(memInfoP,
> +                                                  (caddr_t)0x0, 0L)
>               - memInfoP->memInfo.u.map_info_mmap.internal_offset;
>           ExtendedEnabled = TRUE;
>           return TRUE;
> @@ -604,10 +604,10 @@ int ScreenNum;
>                                MAP_FLAGS, devMemFd, (off_t)DEV_MEM_IOBASE);
> 
>           if (base != (pointer)-1)
> -             IOPortBase = (unsigned int)base;
> +             ioBase = base;
>       }
> 
> -        if (IOPortBase == (unsigned int)-1)
> +        if (ioBase == (unsigned char *)-1)
>       {       
>           xf86Msg(X_WARNING,"xf86EnableIOPorts: failed to open mem device or 
> map IO base. \n\
> Make sure you have the Aperture Driver installed, or a kernel built with the 
> INSECURE option\n");
> @@ -652,8 +652,8 @@ int ScreenNum;
>               if (ScreenEnabled[i])
>                       return;
> 
> -     munmap((caddr_t)IOPortBase, 0x400);
> -     IOPortBase = (unsigned int)-1;
> +     munmap((caddr_t)ioBase, 0x400);
> +     ioBase = (unsigned char *)-1;
>       ExtendedEnabled = FALSE;
> #endif
> 
> diff --git a/hw/xfree86/os-support/linux/lnx_video.c 
> b/hw/xfree86/os-support/linux/lnx_video.c
> index 3d45511..48a3f76 100644
> --- a/hw/xfree86/os-support/linux/lnx_video.c
> +++ b/hw/xfree86/os-support/linux/lnx_video.c
> @@ -493,7 +493,7 @@ volatile unsigned char *ioBase = NULL;
> Bool
> xf86EnableIO(void)
> {
> -#if defined(__powerpc__)
> +#if defined(__mips__) || defined(__powerpc__)
>       int fd;
>       unsigned int ioBase_phys;
> #endif
> @@ -501,8 +501,13 @@ xf86EnableIO(void)
>       if (ExtendedEnabled)
>               return TRUE;
> 
> -#if defined(__powerpc__)
> +#if defined(__mips__) || defined(__powerpc__)
> +
> +# if defined(__mips__)
> +     ioBase_phys = 0x1fd00000;
> +# elif defined(__powerpc__)
>       ioBase_phys = syscall(__NR_pciconfig_iobase, 2, 0, 0);
> +# endif
> 
>       fd = open("/dev/mem", O_RDWR);
>       if (ioBase == NULL) {
> -- 
> 1.7.7
> 
> _______________________________________________
> [email protected]: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel

_______________________________________________
[email protected]: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Reply via email to