[PATCH] pcmcia: ppc64 needs 64-bit ioaddr_t

[PATCH] pcmcia: ppc64 needs 64-bit ioaddr_t

[Olof Johansson]

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;

Re: [PATCH] pcmcia: ppc64 needs 64-bit ioaddr_t

[Olof Johansson]

On Sat, May 12, 2007 at 09:31:05AM -0500, 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.

Hold off on this one, with the current discussions going on it seems likely
that we'll be fine with 32 bits, just like arm/mips.

I'll repost within a day or two if needed.


-Olof

Re: [PATCH] pcmcia: ppc64 needs 64-bit ioaddr_t

[Christoph Hellwig]

On Sat, May 12, 2007 at 09:31:05AM -0500, 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.

In fact the only use of the type should be in the obsolete ioctl-based user
interface.  So instead of changing the size of the type you should

 a) make sure you're not using cardmgr
 b) fix up those places that still use ioaddr_t where they shouldn't
    and switch the to kio_addr_t (why not just unsigned long like all
    other busses?) 

Re: [PATCH] pcmcia: ppc64 needs 64-bit ioaddr_t

[Olof Johansson]

On Sun, May 13, 2007 at 11:46:08PM +0200, Christoph Hellwig wrote:
> On Sat, May 12, 2007 at 09:31:05AM -0500, 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.
> 
> In fact the only use of the type should be in the obsolete ioctl-based user
> interface.  So instead of changing the size of the type you should
> 
>  a) make sure you're not using cardmgr
>  b) fix up those places that still use ioaddr_t where they shouldn't
>     and switch the to kio_addr_t (why not just unsigned long like all
>     other busses?) 

I just noticed that feature-removal-schedule.txt says November 2005 for
the removal of the pcmcia ioctl's, but they're still there. I'm missing
the history here, any reason they weren't removed yet?


-Olof

Re: [PATCH] pcmcia: ppc64 needs 64-bit ioaddr_t

[Christoph Hellwig]

On Mon, May 14, 2007 at 05:57:31PM -0500, Olof Johansson wrote:
> On Sun, May 13, 2007 at 11:46:08PM +0200, Christoph Hellwig wrote:
> > On Sat, May 12, 2007 at 09:31:05AM -0500, 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.
> > 
> > In fact the only use of the type should be in the obsolete ioctl-based user
> > interface.  So instead of changing the size of the type you should
> > 
> >  a) make sure you're not using cardmgr
> >  b) fix up those places that still use ioaddr_t where they shouldn't
> >     and switch the to kio_addr_t (why not just unsigned long like all
> >     other busses?) 
> 
> I just noticed that feature-removal-schedule.txt says November 2005 for
> the removal of the pcmcia ioctl's, but they're still there. I'm missing
> the history here, any reason they weren't removed yet?

See the discussions on lkml.  It seems like people haven't really
noticed the deprecation messages and are still using it.  Still not
a good reason to use them on a new port (at least new in terms of
pcmcia support)

Re: [PATCH] pcmcia: ppc64 needs 64-bit ioaddr_t

[Andrew Morton]

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?

Re: [PATCH] pcmcia: ppc64 needs 64-bit ioaddr_t

[Benjamin Herrenschmidt]

On Mon, 2007-05-14 at 15:08 -0700, Andrew Morton wrote:
> 
> 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.

There is more sad stuff involved with drivers assuming IO ports fit in
int. A proper fix for 2.6.23 will be the rework of PIO allocation I'm
doing. A temporary fix for 2.6.22 would be for Olof to use
reserve_phb_iospace() to make sure his PIO gets in the low 31 bits. We
need to add a spinlock to it though.

Ben.

Re: [PATCH] pcmcia: ppc64 needs 64-bit ioaddr_t

[Olof Johansson]

On Tue, May 15, 2007 at 08:47:04AM +1000, Benjamin Herrenschmidt wrote:
> On Mon, 2007-05-14 at 15:08 -0700, Andrew Morton wrote:
> > 
> > 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.
> 
> There is more sad stuff involved with drivers assuming IO ports fit in
> int. A proper fix for 2.6.23 will be the rework of PIO allocation I'm
> doing. A temporary fix for 2.6.22 would be for Olof to use
> reserve_phb_iospace() to make sure his PIO gets in the low 31 bits. We
> need to add a spinlock to it though.

Yes, we should do fine with just a 32-bit type and my CF driver being
fixed. I've been travelling and catching up with other things today,
I should have something to post tomorrow.

Andrew, you can drop this patch for now. Sorry for the churn.


-Olof

[PATCH v2] pcmcia: ppc64 needs 32-bit ioaddr_t

[Olof Johansson]

ppc64 really needs ioaddr_t to be 32-bit, since I/O beyond the
first PCI bus might be mapped at a higher 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. Besides, those interfaces are overdue for removal
already.


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,7 +21,7 @@
 #include <sys/types.h>
 #endif
 
-#if defined(__arm__) || defined(__mips__)
+#if defined(__arm__) || defined(__mips__) || defined(__powerpc64__)
 /* This (ioaddr_t) is exposed to userspace & hence cannot be changed. */
 typedef u_int   ioaddr_t;
 #else

Re: [PATCH v2] pcmcia: ppc64 needs 32-bit ioaddr_t

[Benjamin Herrenschmidt]

On Mon, 2007-05-14 at 23:33 -0500, Olof Johansson wrote:
> ppc64 really needs ioaddr_t to be 32-bit, since I/O beyond the
> first PCI bus might be mapped at a higher 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. Besides, those interfaces are overdue for removal
> already.
> 
> 
> Signed-off-by: Olof Johansson <olof@lixom.net>

Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>

> 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,7 +21,7 @@
>  #include <sys/types.h>
>  #endif
>  
> -#if defined(__arm__) || defined(__mips__)
> +#if defined(__arm__) || defined(__mips__) || defined(__powerpc64__)
>  /* This (ioaddr_t) is exposed to userspace & hence cannot be changed. */
>  typedef u_int   ioaddr_t;
>  #else
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev