Hello.
Jeff Garzik wrote:
It's generally not a good idea to call request_region() on an
address returned by pci_iomap(), even less so on a MMIO address. And
there was absolutely no point in claiming the region already claimed
by the PCI core, especially with the same PCI generic owner's name.
As this is the only case of the must_free_region flag being set,
this flag may go away as well...
Signed-off-by: Sergei Shtylyov <[EMAIL PROTECTED]>
I agree you have identified a bug, but this is not a solution.
The current driver bug is that it calls request_region() potentially
on an MMIO address, but the solution is _not_ to completely avoid
reserving the resource.
It's not even a MMIO/PIO address anymore after pci_iomap() -- it
either went thru ioremap() or ioport_map() which both change the
mapping from the physical to the virtual address (or some equivalent
of it for I/O ports).
Yes. _Obviously_ you must reserve the resource passed to
pci_iomap/ioremap, not the cookie returned by such.
What somewhat puzzled me is the words "Cardbus drivers already allocate
for us" in the current driver's vortex_probe1(). What extra drivers, and why
should they call request_region() for us? :-/
The region registered with the PCI core, but _not_ claimed by anyone.
Someone still needs to either call pci_{request,release}_regions() or
request_[mem_]region() to indicate that the resource is reserved.
Sigh, it seems I've missed that difference. So, I'll recast...
IMO it would be easiest to do pci_{request,release}_regions() in the
PCI-only code. I believe this matches up well with the existing
EISA-specific code, which also performs request_region().
Ugh, I've looked at vortex_remove_one() and found another buglet: they're
trying to reset the chip there... after calling pci_disable_device(). Guess
whether they really reset anything. I wonder whether it's accpetable for this
fix to be put in the same patch...
Jeff
WBR, Sergei
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
