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?
If the default setup is only correct for PAGE_OFFSET == 0xC0000000, then why
did you permit the comment in page.h telling precisely how to change it to a
value that violated this assumption?
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.
I had assumed that the legacy ISA region was also mapped at PAGE_OFFSET, in
which case the above readb would still work properly. I should have noticed
that the change to traps.c invalidated this assumption.
Despite the fact that it happens to work with the current PAGE_OFFSET, it still
feels conceptually wrong that one takes the result of ioremap, a kernel virtual
address, and then applies __io_virt to it. It may be clever that __io_virt
works for both the legacy ISA I/O and ioremap cases with the default
PAGE_OFFSET, but it's certainly confusing combined with the comments about
changing PAGE_OFFSET. It wouldn't surprise me if other driver authors spend
time researching bug reports when people change PAGE_OFFSET and the driver
breaks.
NOW do you understand why I use binary and/or and force the power-of-two
rule?
Alas, yes. If there is a power of two rule, it should surely be documented in
the code. The existing code in page.h encourages people to make improper
changes. The only other value I know of that works correctly is 0x80000000.
Leonard
-
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/