From mboxrd@z Thu Jan 1 00:00:00 1970 From: Guido Guenther Subject: Re: Re: [PATCH] fbdev: Fix IO access in rivafb Date: Fri, 12 Nov 2004 20:18:52 +0100 Message-ID: <20041112191852.GA4536@bogon.ms20.nix> References: <200411080521.iA85LbG6025914@hera.kernel.org> <200411090402.22696.adaplas@hotpop.com> <200411090608.02759.adaplas@hotpop.com> <20041112125125.GA3613@bogon.ms20.nix> Reply-To: linux-fbdev-devel@lists.sourceforge.net Mime-Version: 1.0 Return-path: Received: from sc8-sf-mx2-b.sourceforge.net ([10.3.1.12] helo=sc8-sf-mx2.sourceforge.net) by sc8-sf-list1.sourceforge.net with esmtp (Exim 4.30) id 1CSgyr-0001yg-UE for linux-fbdev-devel@lists.sourceforge.net; Fri, 12 Nov 2004 11:21:09 -0800 Received: from honk1.physik.uni-konstanz.de ([134.34.140.224]) by sc8-sf-mx2.sourceforge.net with esmtp (TLSv1:DES-CBC3-SHA:168) (Exim 4.41) id 1CSgyq-0000yN-6m for linux-fbdev-devel@lists.sourceforge.net; Fri, 12 Nov 2004 11:21:09 -0800 Content-Disposition: inline In-Reply-To: Sender: linux-fbdev-devel-admin@lists.sourceforge.net Errors-To: linux-fbdev-devel-admin@lists.sourceforge.net List-Unsubscribe: , List-Id: List-Post: List-Help: List-Subscribe: , List-Archive: Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Linus Torvalds Cc: adaplas@pol.net, Linux Fbdev development list , Benjamin Herrenschmidt , Linux Kernel list , Andrew Morton On Fri, Nov 12, 2004 at 08:01:00AM -0800, Linus Torvalds wrote: > On Fri, 12 Nov 2004, Guido Guenther wrote: > > > > Doesn't work for me. This one works: > > > > diff -u -u linux-2.6.10-rc1-mm5.orig/drivers/video/riva/riva_hw.h linux-2.6.10-rc1-mm5/drivers/video/riva/riva_hw.h > > --- linux-2.6.10-rc1-mm5.orig/drivers/video/riva/riva_hw.h 2004-11-12 13:42:54.000000000 +0100 > > +++ linux-2.6.10-rc1-mm5/drivers/video/riva/riva_hw.h 2004-11-12 13:47:29.400807920 +0100 > > @@ -75,12 +75,12 @@ > > */ > > #include > > > > -#define NV_WR08(p,i,d) (__raw_writeb((d), (void __iomem *)(p) + (i))) > > -#define NV_RD08(p,i) (__raw_readb((void __iomem *)(p) + (i))) > > -#define NV_WR16(p,i,d) (__raw_writew((d), (void __iomem *)(p) + (i))) > > -#define NV_RD16(p,i) (__raw_readw((void __iomem *)(p) + (i))) > > -#define NV_WR32(p,i,d) (__raw_writel((d), (void __iomem *)(p) + (i))) > > -#define NV_RD32(p,i) (__raw_readl((void __iomem *)(p) + (i))) > > +#define NV_WR08(p,i,d) (writeb((d), (u8 __iomem *)(p) + (i))) > > +#define NV_RD08(p,i) (readb((u8 __iomem *)(p) + (i))) > > +#define NV_WR16(p,i,d) (__raw_writew((d), (u16 __iomem *)(p) + (i)/2)) > > +#define NV_RD16(p,i) (__raw_readw((u16 __iomem *)(p) + (i)/2)) > > +#define NV_WR32(p,i,d) (__raw_writel((d), (u32 __iomem *)(p) + (i)/4)) > > +#define NV_RD32(p,i) (__raw_readl((u32 __iomem *)(p) + (i)/4)) > > Can you please try without the broken "u16" and "(i/2)" things (and u32 > and i/4)? They really should not be needed, unless something is > _seriously_ broken. The only thing they do is the automatically align the > reference - and quite frankly, it looks like anybody passing an unaligned > register offset is _quite_ buggy. O.k., it was the __raw_{write,read}b which broke things, not the "alignment". This one works: diff -u -u linux-2.6.10-rc1-mm5.orig/drivers/video/riva/riva_hw.h linux-2.6.10-rc1-mm5/drivers/video/riva/riva_hw.h --- linux-2.6.10-rc1-mm5.orig/drivers/video/riva/riva_hw.h 2004-11-12 13:42:54.000000000 +0100 +++ linux-2.6.10-rc1-mm5/drivers/video/riva/riva_hw.h 2004-11-12 17:39:22.000000000 +0100 @@ -75,8 +75,8 @@ */ #include -#define NV_WR08(p,i,d) (__raw_writeb((d), (void __iomem *)(p) + (i))) -#define NV_RD08(p,i) (__raw_readb((void __iomem *)(p) + (i))) +#define NV_WR08(p,i,d) (writeb((d), (void __iomem *)(p) + (i))) +#define NV_RD08(p,i) (readb((void __iomem *)(p) + (i))) #define NV_WR16(p,i,d) (__raw_writew((d), (void __iomem *)(p) + (i))) #define NV_RD16(p,i) (__raw_readw((void __iomem *)(p) + (i))) #define NV_WR32(p,i,d) (__raw_writel((d), (void __iomem *)(p) + (i))) Signed-off-by: Guido Guenther > IOW, it would seem that the real difference is the "__raw_writeb" vs > "writeb". Can you test just that change? > > > #define VGA_WR08(p,i,d) NV_WR08(p,i,d) > > #define VGA_RD08(p,i) NV_RD08(p,i) > > > > Interesting enough this one doesn't (only differenc in NV_{WR,RW}08: > > > > I don't see the difference to your other patch. Did you put in the wrong > patch? There aren't any, I actually attached the wrong patch. The non-working version has __raw_{read,write}b8 instead of {read,write}b8. In 2.6.9 riva_hw.h used {in,out}_8 for NV_{RD,WR}08 so using the "non-raw" writeb/readb now looks correct since these map to {in,out}_8 now. Cheers, -- Guido ------------------------------------------------------- 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