pcmcia: ppc64 needs 64-bit ioaddr_t

PATCH Superseded Olof Johansson 4 participants 10 messages May 12, 2007 – May 15, 2007 Pcmcia

Share & tools

Olof Johansson

author

2007-05-12 14:28 UTC #0 lore raw

PATCH 1/1 pcmcia: ppc64 needs 64-bit ioaddr_t

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 <[email protected]>

include/pcmcia/cs_types.h +5 -0

Index: 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;

Olof Johansson

author

2007-05-13 21:20 UTC #1 lore raw

Quoted 6 lines · 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.

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

Christoph Hellwig

2007-05-13 21:46 UTC #2 lore raw

In reply to Olof Johansson

Quoted 6 lines · 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.

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?)

Olof Johansson

author

2007-05-14 22:54 UTC #3 lore raw

Quoted 8 lines · Christoph Hellwig

Quoted 6 lines · 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.

Quoted 7 lines

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

Christoph Hellwig

2007-05-15 05:53 UTC #4 lore raw

Quoted 12 lines · Olof Johansson

Quoted 11 lines · Christoph Hellwig
Quoted 6 lines · 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.
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

Quoted 8 lines

Quoted 4 lines
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)

Andrew Morton

2007-05-14 22:08 UTC #5 lore raw

In reply to Olof Johansson

Quoted 22 lines · On Sat, 12 May 2007 09:31:05 -0500 [email protected] (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 <[email protected]>
Index: 2.6.21/include/pcmcia/cs_types.h
===================================================================
include/pcmcia/cs_types.h +4 -0
@@ -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

include/pcmcia/cs_types.h line 37

Quoted 10 lines

include/pcmcia/cs_types.h +1 -0

 #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:

include/pcmcia/cs_types.h renamed from include/pcmcia/cs_types.h~pcmcia-ppc64-needs-64-bit-ioaddr_t +5 -1

@@ -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?

Benjamin Herrenschmidt

2007-05-14 22:48 UTC #6 lore raw

Quoted 7 lines · Andrew Morton

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.

Olof Johansson

author

2007-05-14 22:56 UTC #7 lore raw

Quoted 9 lines · Benjamin Herrenschmidt

Quoted 7 lines · Andrew Morton
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.

Quoted 5 lines

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

Olof Johansson

author

2007-05-15 04:30 UTC #8 lore raw

PATCH v2 1/1 pcmcia: ppc64 needs 32-bit ioaddr_t

In reply to 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 <[email protected]>

include/pcmcia/cs_types.h +1 -1

Index: 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

Benjamin Herrenschmidt

Acked-by on v2 1/1

2007-05-15 07:36 UTC #9 lore raw

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

Quoted 10 lines · 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 <[email protected]>

Acked-by Benjamin Herrenschmidt <[email protected]>

Quoted 17 lines

Index: 2.6.21/include/pcmcia/cs_types.h
===================================================================
include/pcmcia/cs_types.h +1 -1
@@ -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
[email protected]
https://ozlabs.org/mailman/listinfo/linuxppc-dev