Re: Re: [PATCH] fbdev: Fix IO access in rivafb

On Monday 08 November 2004 13:57, Benjamin Herrenschmidt wrote:

quoted

diff -Nru a/drivers/video/riva/riva_hw.h b/drivers/video/riva/riva_hw.h

--- a/drivers/video/riva/riva_hw.h	2004-11-07 21:21:47 -08:00
+++ b/drivers/video/riva/riva_hw.h	2004-11-07 21:21:47 -08:00

@@ -78,13 +78,13 @@
 #define NV_WR08(p,i,d)	out_8(p+i, d)
 #define NV_RD08(p,i)	in_8(p+i)
 #else
-#define NV_WR08(p,i,d)  (((U008 *)(p))[i]=(d))
-#define NV_RD08(p,i)    (((U008 *)(p))[i])
+#define NV_WR08(p,i,d)  (writeb((d), (u8 __iomem *)(p) + (i)))
+#define NV_RD08(p,i)    (readb((u8 __iomem *)(p) + (i)))
 #endif
-#define NV_WR16(p,i,d)  (((U016 *)(p))[(i)/2]=(d))
-#define NV_RD16(p,i)    (((U016 *)(p))[(i)/2])
-#define NV_WR32(p,i,d)  (((U032 *)(p))[(i)/4]=(d))
-#define NV_RD32(p,i)    (((U032 *)(p))[(i)/4])
+#define NV_WR16(p,i,d)  (writew((d), (u16 __iomem *)(p) + (i)/2))
+#define NV_RD16(p,i)    (readw((u16 __iomem *)(p) + (i)/2))
+#define NV_WR32(p,i,d)  (writel((d), (u32 __iomem *)(p) + (i)/4))
+#define NV_RD32(p,i)    (readl((u32 __iomem *)(p) + (i)/4))
 #define VGA_WR08(p,i,d) NV_WR08(p,i,d)
 #define VGA_RD08(p,i)   NV_RD08(p,i)

You probably broke ppc versions here. The driver enables "big endian"
register access, but readw/writew/l functions do byteswap, which will
lead to incorrect results.

The fix would be to probably just remove the code that puts the chip
registers into big endian mode, this isn't necessary nor a very good
idea actually.

How about this patch?  This is almost the original macro in riva_hw.h,
with the __force annotation.

Signed-off-by: Antonino Daplas <redacted>
---

diff -Nru a/drivers/video/riva/riva_hw.h b/drivers/video/riva/riva_hw.h

--- a/drivers/video/riva/riva_hw.h	2004-11-06 12:42:19 +08:00
+++ b/drivers/video/riva/riva_hw.h	2004-11-08 16:59:34 +08:00

@@ -77,14 +77,18 @@
 #include <asm/io.h>
 #define NV_WR08(p,i,d)	out_8(p+i, d)
 #define NV_RD08(p,i)	in_8(p+i)
+#define NV_WR16(p,i,d)  (((volatile u16 __force *)(p))[(i)/2]=(d))
+#define NV_RD16(p,i)    (((volatile u16 __force *)(p))[(i)/2])
+#define NV_WR32(p,i,d)  (((volatile u32 __force *)(p))[(i)/4]=(d))
+#define NV_RD32(p,i)    (((volatile u32 __force *)(p))[(i)/4])
 #else
 #define NV_WR08(p,i,d)  (writeb((d), (u8 __iomem *)(p) + (i)))
 #define NV_RD08(p,i)    (readb((u8 __iomem *)(p) + (i)))
-#endif
 #define NV_WR16(p,i,d)  (writew((d), (u16 __iomem *)(p) + (i)/2))
 #define NV_RD16(p,i)    (readw((u16 __iomem *)(p) + (i)/2))
 #define NV_WR32(p,i,d)  (writel((d), (u32 __iomem *)(p) + (i)/4))
 #define NV_RD32(p,i)    (readl((u32 __iomem *)(p) + (i)/4))
+#endif
 #define VGA_WR08(p,i,d) NV_WR08(p,i,d)
 #define VGA_RD08(p,i)   NV_RD08(p,i)
 




-------------------------------------------------------

This SF.Net email is sponsored by:
Sybase ASE Linux Express Edition - download now for FREE
LinuxWorld Reader's Choice Award Winner for best database on Linux.
http://ads.osdn.com/?ad_id=5588&alloc_id=12065&op=click