I suggest to get rid of the bogus readX/writeX stuff in drivers that 
know what they are doing, until 2.3, as follows:

#if defined(__i386__)   /* i386 implements full FLAT memory/MMIO model */

#undef the stuff that does not work for __i386__ with PCI MMIO regions  
       mapped in high address space and ioremapped, then:

#define readb(a)        (*(volatile unsigned char *) (a))
#define readw(a)        (*(volatile unsigned short *) (a))
#define readl(a)        (*(volatile unsigned int *) (a))
#define writeb(b,a)     ((*(volatile unsigned char *) (a)) = (b))
#define writew(b,a)     ((*(volatile unsigned short *) (a)) = (b))
#define writel(b,a)     ((*(volatile unsigned int *) (a)) = (b))

#else etc ...

And obviously to _not_ use phys_to_virt() that seems to only make sense 
on a single O/S.

BTW, already done in the development 896 driver.

   Gerard.

On Tue, 26 Jan 1999, Linus Torvalds wrote:

> 
> 
> On Tue, 26 Jan 1999, Leonard N. Zubkoff wrote:
> > 
> > (1) The identity phys_to_virt(virt_to_phys(x)) == x is violated.
> 
> Not for the default setup. Guess why the default setup is the default
> setup?
> 
> > (2) The readb/writeb/readw/writew/readl/writel macros are completely incorrect
> >     since they are supposed to be applied to memory mapped I/O regions
> >     allocated with ioremap, but ioremap returns a kernel virtual address
> >     and hence no further mapping is appropriate.
> 
> We also currently accept things like
> 
>       readb(0xA0000);
> 
> ie the "legacy ISA region" is currently (for backwards compatibility
> reasons) not required to be io-remapped.
> 
> Look at various network drivers for example - your patches probably break
> a noticeable number of them. 
> 
> > The BusLogic driver ran into (1) and the DAC960 driver ran into (2).  With the
> > patch enclosed below, both drivers work fine when PAGE_OFFSET is changed. 
> 
> With the patch enclosed below, tons of random device drivers may break. 
> Not something to be done lightly. 
> 
> >                                                              The
> > change to traps.c is also necessary as readl was incorrect usage, even though
> > the modified code is now uglier.
> 
> The modified code in your patch is completely wrong, and is MUCH more
> incorrect than the old one. 
> 
> The old one made the assumption that the legacy ISA region doesn't need to
> be IO-remapped and can be directly accessed with read[bwl]/write[bwl] -
> which is a painful assumption, but it's a legacy one, and breaking that
> assumption means that _tons_ of drivers will have to be checked.
> 
> The new code makes the assumption that the legacy ISA region is mapped in
> the kernel virtual address space, and that phys_to_virt() has any meaning
> for IO addresses. That happens to be the case right now, but it's a
> completely invalid assumption that does not work on many other
> architectures. 
> 
> So the old code was at least technically correct. The new one isn't.
> 
> In short, I will accept this kind of patch for 2.3 (fixed up properly),
> but I hope you see why I would never consider it for 2.2.x. 
> 
> The _proper_ fixup is to make sure that every access to IO memory is
> preceded by a "ioremap()" to make sure the address is mapped. The
> "phys_to_virt()" function has absolutely nothing at all to do with IO
> memory, and anything that uses it is buggy.
> 
> NOW do you understand why I use binary and/or and force the power-of-two
> rule?
> 
>               Linus
> 
> -
> Linux SMP list: FIRST see FAQ at http://www.irisa.fr/prive/mentre/smp-faq/
> To Unsubscribe: send "unsubscribe linux-smp" to [EMAIL PROTECTED]
> 
> 

-
Linux SMP list: FIRST see FAQ at http://www.irisa.fr/prive/mentre/smp-faq/
To Unsubscribe: send "unsubscribe linux-smp" to [EMAIL PROTECTED]

Reply via email to