From: arnd@arndb.de (Arnd Bergmann)
To: linux-arm-kernel@lists.infradead.org
Subject: Integrator PCI base dilemma
Date: Fri, 22 Mar 2013 09:48:27 +0000 [thread overview]
Message-ID: <201303220948.28092.arnd@arndb.de> (raw)
In-Reply-To: <20130321234004.GV4977@n2100.arm.linux.org.uk>
On Thursday 21 March 2013, Russell King - ARM Linux wrote:
> Gah. Wrap failure.
>
> On Thu, Mar 21, 2013 at 01:02:18PM +0000, Arnd Bergmann wrote:
> > Alpha, IA64, PPC64, and Tile use ioremap there, which seems reasonable
> > especially given that the at least the vga16fb assumes it can iounmap
> > the pointer returned by VGA_MAP_MEM when unloading.
>
> Be. Very. Careful. with that.
>
> Look at vgacon. vgacon gets used on ARM. vga16fb does not. vgacon
> does not use VGA_MAP_MEM() in a way compatible with our ioremap() -
> things like the font ops will end up creating multiple mappings which
> will eventually full the ioremap() space if you do go down that route.
Ah, so we have two drivers using the same interface based on conflicting
assumptions, nice.
I guess ioremap would still work on ARM if we already have a static
mapping, but in that case there is no need to use ioremap in the first
place.
To fix this properly, we could add a matching VGA_UNMAP_MEM() interface
and use it consistently in both drivers. I don't think anyone has the
energy to do that and get it merged, but I would review patches ;-)
> > Again, I'd assume that tile is incorrectly copied from
> > one of the others, but it might actually work. All but ia64 here rely
> > on the VGA registers and memory and ROM being mapped into physical
> > addresses 0xA0000-0xC7fff as they are on PCs.
>
> That's because it's pretty much built into the way VGA crud works.
> VGA BIOSes hard code these addresses into themselves. Really, so does
> the kernel, but non-native architectures are given the chance to offset
> this appropriately - and remember that non-native architectures will
> run the VGA BIOS via an x86 emulator, which you pretty much have to do
> to get VGA cards to initialize properly.
Unfortunately, the offsetting also means that you have a non-zero mem_offset
value for the pci host controller, which breaks code that reads the PCI BARs
and uses those as physical addresses. I believe XFree86 traditionally did
this. I normally recommend to do PCI host bridge drivers with mem_offset=0,
meaning you have a window of valid PCI memory space addresses that match
the physical address at which they are visible to the CPU. This of course
means that VGA_MAP_MEM() cannot work, because the ISA compatible memory
space addresses end up outside of the window.
> > arch/arm/mach-dove/pcie.c: vga_base = DOVE_PCIE0_MEM_PHYS_BASE;
> > arch/arm/mach-footbridge/dc21285.c: vga_base = PCIMEM_BASE;
> > arch/arm/mach-integrator/integrator_ap.c: vga_base = PCI_MEMORY_VADDR;
> > arch/arm/mach-kirkwood/pcie.c: vga_base = KIRKWOOD_PCIE_MEM_PHYS_BASE;
> > arch/arm/mach-mv78xx0/pcie.c: vga_base = MV78XX0_PCIE_MEM_PHYS_BASE;
> > arch/arm/mach-orion5x/pci.c: vga_base = ORION5X_PCIE_MEM_PHYS_BASE;
> > arch/arm/mach-shark/pci.c: vga_base = 0xe8000000;
> >
> > Dove/integrator/kirkwood/mv78xx0/orion5x all put a *physical* address in here,
> > which cannot work, since that is not mapped anywhere.
>
> Erm, no. Look again at Integrator. PCI_MEMORY_*V*ADDR.
> arch/arm/mach-integrator/include/mach/platform.h:#define PCI_MEMORY_VADDR IOMEM(0xe8000000)
Right. My brain was thinking correctly, but my fingers typed it out wrong ;-)
> > Footbridge and integrator get it right, presumably because Russell
> > was actually using that code ;-)
>
> ... and for Footbridge, still do. Remember, though that this whole
> vga_base thing is a relatively recent addition due to the single zImage
> project, so any mistakes with the above need proper research to see
> whether they're bugs introduced by that activity.
Yes. It took me a while yesterday to go through the history for shark, as
I feared we broke VGA_MAP_MEM during the conversion, but I'm pretty sure it
was already broken before.
> > Now what does this all tell us? First of all I think you are free to change
> > it in any way that does not break footbridge, since everything else is already
> > broken.
>
> Footbridge and Integrator/AP.
Yes, I was assuming that Linus would keep Integrator/AP working since that is
what he uses.
> > I think the most logical way forward would be to use the same method
> > as ia64 and use ioremap() with a platform-specific offset, but keeping the
> > static mapping would probably be easier to do.
>
> No, our ioremap() will definitely break here.
Ok, so let's keep the static mapping method.
Arnd
next prev parent reply other threads:[~2013-03-22 9:48 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-03-20 23:15 Integrator PCI base dilemma Linus Walleij
2013-03-20 23:54 ` Rob Herring
2013-03-21 9:16 ` Linus Walleij
2013-03-21 13:22 ` Rob Herring
2013-03-21 16:17 ` Arnd Bergmann
2013-03-22 11:05 ` Russell King - ARM Linux
2013-03-21 13:02 ` Arnd Bergmann
2013-03-21 23:40 ` Russell King - ARM Linux
2013-03-22 9:48 ` Arnd Bergmann [this message]
2013-03-22 10:37 ` Russell King - ARM Linux
2013-03-22 11:52 ` Arnd Bergmann
2013-03-22 12:12 ` Russell King - ARM Linux
2013-03-22 12:34 ` Arnd Bergmann
2013-03-22 18:33 ` Jason Gunthorpe
2013-03-22 18:35 ` Russell King - ARM Linux
2013-03-22 19:22 ` Linus Walleij
2013-03-22 20:05 ` Arnd Bergmann
2013-03-22 21:13 ` Wolfgang Denk
2013-03-22 22:35 ` Linus Walleij
2013-03-22 23:48 ` Russell King - ARM Linux
2013-03-23 0:19 ` Wolfgang Denk
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=201303220948.28092.arnd@arndb.de \
--to=arnd@arndb.de \
--cc=linux-arm-kernel@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).