All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Morton <akpm@linux-foundation.org>
To: olof@lixom.net (Olof Johansson)
Cc: linuxppc-dev@ozlabs.org, linux-pcmcia@lists.infradead.org,
	paulus@samba.org, linux-kernel@vger.kernel.org,
	Dominik Brodowski <linux@dominikbrodowski.net>
Subject: Re: [PATCH] pcmcia: ppc64 needs 64-bit ioaddr_t
Date: Mon, 14 May 2007 15:08:02 -0700	[thread overview]
Message-ID: <20070514150802.b9120e9f.akpm@linux-foundation.org> (raw)
In-Reply-To: <20070512143105.GA12890@lixom.net>

On Sat, 12 May 2007 09:31:05 -0500
olof@lixom.net (Olof Johansson) wrote:

> ppc64 really needs ioaddr_t to be 64-bit, since I/O addresses really
> are MMIO addresses, and remapped to a high range.
> 
> While the type is exported to userspace, there hasn't been any platforms
> with PCMCIA on 64-bit powerpc until now, so changing it won't regress
> any existing users.
> 
> 
> Signed-off-by: Olof Johansson <olof@lixom.net>
> 
> Index: 2.6.21/include/pcmcia/cs_types.h
> ===================================================================
> --- 2.6.21.orig/include/pcmcia/cs_types.h
> +++ 2.6.21/include/pcmcia/cs_types.h
> @@ -21,12 +21,17 @@
>  #include <sys/types.h>
>  #endif
>  
> +#if defined(__powerpc64__)
> +/* I/O addresses are really MMIO addresses on PPC, and can thus be 64 bits */
> +typedef unsigned long ioaddr_t;
> +#else
>  #if defined(__arm__) || defined(__mips__)
>  /* This (ioaddr_t) is exposed to userspace & hence cannot be changed. */
>  typedef u_int   ioaddr_t;
>  #else
>  typedef u_short	ioaddr_t;
>  #endif
> +#endif
>  typedef unsigned long kio_addr_t;
>  
>  typedef u_short	socket_t;

Well that's some pretty sad code you've found there.  The kernel surely has
some appropriate type to use here without us having to invent a new one. 
But I suspect if we were to rationalise things in there it will get messy.

I think your patch can be cast more neatly if we use #elif:

--- a/include/pcmcia/cs_types.h~pcmcia-ppc64-needs-64-bit-ioaddr_t
+++ a/include/pcmcia/cs_types.h
@@ -21,12 +21,16 @@
 #include <sys/types.h>
 #endif
 
-#if defined(__arm__) || defined(__mips__)
+#if defined(__powerpc64__)
+/* I/O addresses are really MMIO addresses on PPC, and can thus be 64 bits */
+typedef unsigned long ioaddr_t;
+#elif defined(__arm__) || defined(__mips__)
 /* This (ioaddr_t) is exposed to userspace & hence cannot be changed. */
 typedef u_int   ioaddr_t;
 #else
 typedef u_short	ioaddr_t;
 #endif
+
 typedef unsigned long kio_addr_t;
 
 typedef u_short	socket_t;
_

Also, I wonder if `unsigned long' is the correct type to use here.  32-bit
userspace will treat it as 32-bit and 64-bit userspace will treat it as
64-bit.

Would it be better to use uint64_t here?

WARNING: multiple messages have this Message-ID (diff)
From: Andrew Morton <akpm@linux-foundation.org>
To: olof@lixom.net (Olof Johansson)
Cc: paulus@samba.org, linuxppc-dev@ozlabs.org,
	linux-kernel@vger.kernel.org, linux-pcmcia@lists.infradead.org,
	Dominik Brodowski <linux@dominikbrodowski.net>
Subject: Re: [PATCH] pcmcia: ppc64 needs 64-bit ioaddr_t
Date: Mon, 14 May 2007 15:08:02 -0700	[thread overview]
Message-ID: <20070514150802.b9120e9f.akpm@linux-foundation.org> (raw)
In-Reply-To: <20070512143105.GA12890@lixom.net>

On Sat, 12 May 2007 09:31:05 -0500
olof@lixom.net (Olof Johansson) wrote:

> ppc64 really needs ioaddr_t to be 64-bit, since I/O addresses really
> are MMIO addresses, and remapped to a high range.
> 
> While the type is exported to userspace, there hasn't been any platforms
> with PCMCIA on 64-bit powerpc until now, so changing it won't regress
> any existing users.
> 
> 
> Signed-off-by: Olof Johansson <olof@lixom.net>
> 
> Index: 2.6.21/include/pcmcia/cs_types.h
> ===================================================================
> --- 2.6.21.orig/include/pcmcia/cs_types.h
> +++ 2.6.21/include/pcmcia/cs_types.h
> @@ -21,12 +21,17 @@
>  #include <sys/types.h>
>  #endif
>  
> +#if defined(__powerpc64__)
> +/* I/O addresses are really MMIO addresses on PPC, and can thus be 64 bits */
> +typedef unsigned long ioaddr_t;
> +#else
>  #if defined(__arm__) || defined(__mips__)
>  /* This (ioaddr_t) is exposed to userspace & hence cannot be changed. */
>  typedef u_int   ioaddr_t;
>  #else
>  typedef u_short	ioaddr_t;
>  #endif
> +#endif
>  typedef unsigned long kio_addr_t;
>  
>  typedef u_short	socket_t;

Well that's some pretty sad code you've found there.  The kernel surely has
some appropriate type to use here without us having to invent a new one. 
But I suspect if we were to rationalise things in there it will get messy.

I think your patch can be cast more neatly if we use #elif:

--- a/include/pcmcia/cs_types.h~pcmcia-ppc64-needs-64-bit-ioaddr_t
+++ a/include/pcmcia/cs_types.h
@@ -21,12 +21,16 @@
 #include <sys/types.h>
 #endif
 
-#if defined(__arm__) || defined(__mips__)
+#if defined(__powerpc64__)
+/* I/O addresses are really MMIO addresses on PPC, and can thus be 64 bits */
+typedef unsigned long ioaddr_t;
+#elif defined(__arm__) || defined(__mips__)
 /* This (ioaddr_t) is exposed to userspace & hence cannot be changed. */
 typedef u_int   ioaddr_t;
 #else
 typedef u_short	ioaddr_t;
 #endif
+
 typedef unsigned long kio_addr_t;
 
 typedef u_short	socket_t;
_

Also, I wonder if `unsigned long' is the correct type to use here.  32-bit
userspace will treat it as 32-bit and 64-bit userspace will treat it as
64-bit.

Would it be better to use uint64_t here?


  parent reply	other threads:[~2007-05-14 22:08 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-05-12 14:31 [PATCH] pcmcia: ppc64 needs 64-bit ioaddr_t Olof Johansson
2007-05-12 14:31 ` Olof Johansson
2007-05-13 21:22 ` Olof Johansson
2007-05-13 21:22   ` Olof Johansson
2007-05-13 21:46 ` Christoph Hellwig
2007-05-13 21:46   ` Christoph Hellwig
2007-05-14 22:57   ` Olof Johansson
2007-05-14 22:57     ` Olof Johansson
2007-05-15  5:52     ` Christoph Hellwig
2007-05-15  5:52       ` Christoph Hellwig
2007-05-14 22:08 ` Andrew Morton [this message]
2007-05-14 22:08   ` Andrew Morton
2007-05-14 22:47   ` Benjamin Herrenschmidt
2007-05-14 22:47     ` Benjamin Herrenschmidt
2007-05-14 22:59     ` Olof Johansson
2007-05-14 22:59       ` Olof Johansson
2007-05-15  4:33 ` [PATCH v2] pcmcia: ppc64 needs 32-bit ioaddr_t Olof Johansson
2007-05-15  4:33   ` Olof Johansson
2007-05-15  7:35   ` Benjamin Herrenschmidt
2007-05-15  7:35     ` Benjamin Herrenschmidt

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=20070514150802.b9120e9f.akpm@linux-foundation.org \
    --to=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pcmcia@lists.infradead.org \
    --cc=linux@dominikbrodowski.net \
    --cc=linuxppc-dev@ozlabs.org \
    --cc=olof@lixom.net \
    --cc=paulus@samba.org \
    /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.