Messages in this thread | | | Date | Tue, 26 Jan 1999 23:02:30 +0100 (MET) | From | Gerard Roudier <> | Subject | Re: Big Fix for 2.2.1 |
| |
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 majordomo@vger.rutgers.edu > >
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.rutgers.edu Please read the FAQ at http://www.tux.org/lkml/
| |