Re: [PATCH] Don't call request_region() for 3C90x chips
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
Re: [PATCH] Don't call request_region() for 3C90x chips
Sergei Shtylyov wrote:
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.
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().
Jeff
-
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
Re: [PATCH] Don't call request_region() for 3C90x chips
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).
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...
This bug you have found was probably a missed detail during the
conversion to the iomap API.
Well, not only that: it was wrong once the driver started using MMIO
(which is of course a preference).
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
Re: [PATCH] Don't call request_region() for 3C90x chips
Sergei Shtylylov 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.
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.
This bug you have found was probably a missed detail during the
conversion to the iomap API.
Jeff
-
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
