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    [W:0.033 / U:31.876 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site