On Fri, 2008-06-06 at 22:35 +0200, Krzysztof Helt wrote:

On Thu, 05 Jun 2008 17:26:54 -0500
James Bottomley [off-list ref] wrote:

quoted

I'm really not sure this is such a good idea.  pci_map_rom() can do an
ioremap() on the region.  In that case, gsc_readl which punches through
our virtual memory into physical I/O space will fail.  What assurance do
we have that all STI roms are correctly set up so pci_map_rom() isn't
inclined to ioremap them?

I haven't thought about this. The ROM the Helge tested was obviously
set up correctly, otherwise reading the ROM signature would be impossible.

I suspect Helge only tested GSC based cards, which will work just fine
because they use the old logic.

What is you advise for this patch James?

The more I look at this the more I doubt that it's a good thing.  STI is
really a platform driver; it has to function for both GSC and PCI
busses.  pci_map_rom() seems to be a bit of a minefield designed for x86
centric video cards, which STI definitely isn't.  The sticore driver is
also designed to operate on unmapped ranges ... at least this is the way
it's designed for the GSC, so the PCI part has to follow suit.

Our simple base get and enabling transparently the driver currently has
says what we're doing; I don't really see why it needs to switch to
pci_map_rom.  Additionally, I fear that switching to pci_map_rom will
cause us to break later on down the road.

James