lkml.org 
[lkml]   [1999]   [Aug]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: New resources - pls, explain :-(


On Sat, 7 Aug 1999, Martin Mares wrote:
>
> In case of usual PCI devices using functions properly allocating
> all the regions is just enough. For the other cases, drivers should
> be allowed to allocate the resources on their own. Okay?

Yes. But they shouldn't really need to "allocate" any regions, they should
just need to mark them busy. I think the allocation should be done
separately, because it is _not_ true that you want to necessarily enable
every single region the card can enable.

So there is also need for other helper functions to do the actual enabling
of the IO ports/memory that were not enabled at boot time.

I know your patches try to enable some regions automatically (like the
ROMs etc), but I suspect that we really want to leave that to the device
driver. For example, multiple PCI devices can be accessed with either the
memory interface or the IO interface, and it may not be even worthwhile to
enable both (it just eats up resource space, which is not necessarily
always infinite ;)

But leaving that to the driver means that we want to have a simple way to
do it, something like

pci_enable_rom(struct pci_dev *dev, unsigned int offset);
pci_enable_region(struct pci_dev *dev, struct resource *res, unsigned int offset);

which will try to enable the region specified at "offset" (this is for
drivers like the PIIX4 that have "extra" regions in addition to the
standard ones, so you can't just hardcode the standard offsets, and for
drivers that want to have their rom region mapped in order to read
information from them), as well as

unsigned int pci_enable_io(struct pci_dev *dev);
unsigned long pci_enable_mem(struct pci_dev *dev);

which would just enable and mark as busy the first IO or MEM port of the
resources that it finds and in the mem case it would actually do the
ioremap() too (remember: this is a _helper_ function to cover 99% of the
common cases).

Not doing the enable automatically at boot-time also means that any broken
devices can be fixed up later, possibly by a driver that _knows_ about
their brokenness. The boot sequence should try to avoid fixing up anything
but the most egregious problems that cause major havoc...

So I agree about the need for these kinds of helper functions, it's just
that I want to make sure they stay _helper_, and are not invoced
automatically. And I really think that they shouldn't try to be too
generic - they should be specific in ways that most drivers need them to
be (and then more complex drivers with multiple IO regions etc might as
well just do the work by hand).

For example, with the above setup it should be trivial for most PCI-aware
drivers to just with a few small lines get the information most of them
want. You could have

pci_dev = NULL;
while ((pci_dev = pci_find_device(vendor, device, pci_dev)) != NULL) {
unsigned long iobase = pci_enable_mem(pci_dev);
/* Failed to map due to being busy or something */
if (!iobase)
continue;
..save away the IO address..
}
because _all_ the common things (search for which entry in the device is
actually the memory one / busy check / ioremap) would have been done by
the helper routine.

That's why I think that the "enable and allocate ALL ioregions" approach
is wrong: because it's really not necessarily what most device drivers
really want, I think.

Linus


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2005-03-22 13:53    [from the cache]
©2003-2014 Jasper Spaans. Advertise on this site