Re: [PATCH] fbdev: suppress warning when assigning vga-save/restore base

[Ondrej Zajicek <santiago@crfreenet.org>]

On Sun, Aug 04, 2013 at 06:51:46PM -0700, David Miller wrote:
> From: "H. Peter Anvin" <hpa@zytor.com>
> Date: Sun, 04 Aug 2013 10:33:46 -0700
> 
> > Anyone who can dig backwards and summarize?  In other words:
> > 
> > Where in the current code do we stuff a physical address in a pointer,
> > or a virtual address into a non-pointer?
> 
> The VGA register accessors try to accomodate iomem and ioport
> accesses.
> 
> If they are given a non-NULL iomem pointer 'regbase' they use
> iomem accesses, otherwise they do direct ISA port poking.
> 
> And yes the drivers in question are making some brash assumptions.
> I suspect they should be using ioremap() or similar.

Well, these drivers were written without MMIO (port IO only with NULL
instead of 'vgabase' for VGA register accessors). They were converted in
patches 94c322c30bd14ae6cdd369cb4a1f94c5c3809ac9,
f8645933513c65ac55f23c63b2649097289795c6 and a few others (from David
Miller) to potentially use MMIO by using 'vgabase' instead of NULL:

 pcibios_bus_to_resource(dev, &vga_res, &bus_reg);
 par->state.vgabase = (void __iomem *) vga_res.start;

How this could even work? AFAIK these cards have to be explicitly programmed
to enable MMIO (which was not done in the patches). These patches claim that
it is for multi-domain PCI. I would guess that vgabase is NULL in common
configurations but if it is non-NULL, it probably wouldn't work, unless
there is some hardware magic that transparently converts MMIO (from CPU PoV)
to port IO (from card/PCI PoV).

Note that there are some later patches (86c0f043a737dadf034a4e6f29aefb074f4a1146)
from Ondrej Zary that independently enable MMIO, but they use par->mmio
field instead of par->state.vgabase .

-- 
Elen sila lumenn' omentielvo

Ondrej 'SanTiago' Zajicek (email: santiago@crfreenet.org)
OpenPGP encrypted e-mails preferred (KeyID 0x11DEADC3, wwwkeys.pgp.net)
"To err is human -- to blame it on a computer is even more so."