[RFC] [PATCH] PNP: request ioport and iomem resources used by active devices - Kernel

This is a discussion on [RFC] [PATCH] PNP: request ioport and iomem resources used by active devices - Kernel ; Reserve resources used by active PNP devices to prevent those resources from being assigned to other devices. This can be turned off with the "pnp=no_reservations" flag. If you need to use it, please report it, because it indicates a potential ...

+ Reply to Thread
Results 1 to 5 of 5

Thread: [RFC] [PATCH] PNP: request ioport and iomem resources used by active devices

  1. [RFC] [PATCH] PNP: request ioport and iomem resources used by active devices

    Reserve resources used by active PNP devices to prevent those resources
    from being assigned to other devices.

    This can be turned off with the "pnp=no_reservations" flag. If you need to
    use it, please report it, because it indicates a potential problem, like a
    driver that requests more resources than it needs.

    This is similar to request_standard_resources(). That function requests
    resources for a compiled-in list of standard PC devices such as DMA
    controllers, PICs, timers, and keyboard, and marks them "busy." This
    patch requests resources for devices that are actually present but does
    not mark them "busy."

    Sometimes the built-in standard resources are larger than what PNP reports,
    or they combine things that PNP reports separately, which makes things look
    a little strange, e.g.,

    0000-001f : dma1 <-- built-in resource includes 2 controllers
    0000-000f : 00:02 <-- PNP reports only one DMA controller
    0020-0021 : pic1
    002e-002f : 00:06
    0040-0043 : timer0
    0050-0053 : timer1
    0060-006f : keyboard <-- built-in resource groups several things
    0060-0060 : 00:04 <-- PNP reports 8042 controller data register
    0061-0061 : 00:03 <-- PNP reports AT-style speaker
    0064-0064 : 00:04 <-- PNP reports 8042 controller status register
    0070-0073 : 00:06
    0070-0071 : rtc

    Signed-off-by: Bjorn Helgaas

    Index: w/Documentation/kernel-parameters.txt
    ================================================== =================
    --- w.orig/Documentation/kernel-parameters.txt 2007-10-29 15:00:46.000000000 -0600
    +++ w/Documentation/kernel-parameters.txt 2007-10-29 15:02:32.000000000 -0600
    @@ -1410,6 +1410,14 @@
    Format: { parport | timid | 0 }
    See also Documentation/parport.txt.

    + pnp= [PNP] PNP subsystem options:
    + no_reservations Don't reserve resources used by PNP devices
    + This is for working around problems where a
    + driver uses a PNP device without using the PNP
    + core. Typical symptom is that the driver can't
    + reserve the resources it needs. Please report
    + these issues.
    +
    pnpacpi= [ACPI]
    { off }

    Index: w/drivers/pnp/base.h
    ================================================== =================
    --- w.orig/drivers/pnp/base.h 2007-10-29 15:00:45.000000000 -0600
    +++ w/drivers/pnp/base.h 2007-10-29 15:00:58.000000000 -0600
    @@ -5,6 +5,8 @@
    void pnp_free_option(struct pnp_option *option);
    int __pnp_add_device(struct pnp_dev *dev);
    void __pnp_remove_device(struct pnp_dev *dev);
    +int pnp_request_resources(struct pnp_dev *dev);
    +int pnp_release_resources(struct pnp_dev *dev);

    int pnp_check_port(struct pnp_dev * dev, int idx);
    int pnp_check_mem(struct pnp_dev * dev, int idx);
    Index: w/drivers/pnp/core.c
    ================================================== =================
    --- w.orig/drivers/pnp/core.c 2007-10-29 15:00:45.000000000 -0600
    +++ w/drivers/pnp/core.c 2007-10-29 15:00:58.000000000 -0600
    @@ -124,6 +124,9 @@
    list_add_tail(&dev->protocol_list, &dev->protocol->devices);
    spin_unlock(&pnp_lock);

    + if (dev->active)
    + pnp_request_resources(dev);
    +
    ret = device_register(&dev->dev);
    if (ret)
    return ret;
    Index: w/drivers/pnp/manager.c
    ================================================== =================
    --- w.orig/drivers/pnp/manager.c 2007-10-29 15:00:45.000000000 -0600
    +++ w/drivers/pnp/manager.c 2007-10-29 15:00:58.000000000 -0600
    @@ -391,9 +391,11 @@

    if (!pnp_can_configure(dev))
    return -ENODEV;
    +
    bak = pnp_alloc(sizeof(struct pnp_resource_table));
    if (!bak)
    return -ENOMEM;
    +
    *bak = dev->res;

    down(&pnp_res_mutex);
    @@ -463,7 +465,7 @@
    * pnp_start_dev - low-level start of the PnP device
    * @dev: pointer to the desired device
    *
    - * assumes that resources have already been allocated
    + * assumes that resources have already been assigned to the device
    */
    int pnp_start_dev(struct pnp_dev *dev)
    {
    @@ -472,6 +474,9 @@
    return -EINVAL;
    }

    + if (pnp_request_resources(dev))
    + dev_err(&dev->dev, "could not allocate resources\n");
    +
    if (dev->protocol->set(dev, &dev->res) < 0) {
    dev_err(&dev->dev, "activation failed\n");
    return -EIO;
    @@ -484,8 +489,6 @@
    /**
    * pnp_stop_dev - low-level disable of the PnP device
    * @dev: pointer to the desired device
    - *
    - * does not free resources
    */
    int pnp_stop_dev(struct pnp_dev *dev)
    {
    @@ -493,11 +496,14 @@
    dev_dbg(&dev->dev, "disabling not supported\n");
    return -EINVAL;
    }
    +
    if (dev->protocol->disable(dev) < 0) {
    dev_err(&dev->dev, "disable failed\n");
    return -EIO;
    }

    + pnp_release_resources(dev);
    +
    dev_info(&dev->dev, "disabled\n");
    return 0;
    }
    Index: w/drivers/pnp/resource.c
    ================================================== =================
    --- w.orig/drivers/pnp/resource.c 2007-10-29 15:00:45.000000000 -0600
    +++ w/drivers/pnp/resource.c 2007-10-29 15:00:58.000000000 -0600
    @@ -23,6 +23,7 @@
    static int pnp_reserve_dma[8] = {[0 ... 7] = -1 }; /* reserve (don't use) some DMA */
    static int pnp_reserve_io[16] = {[0 ... 15] = -1 }; /* reserve (don't use) some I/O region */
    static int pnp_reserve_mem[16] = {[0 ... 15] = -1 }; /* reserve (don't use) some memory region */
    +static int pnp_no_reservations;

    /*
    * option registration
    @@ -459,6 +460,98 @@
    #endif
    }

    +int pnp_request_resources(struct pnp_dev *dev)
    +{
    + int i, ret;
    + struct resource *res;
    +
    + if (pnp_no_reservations) {
    + dev_warn(&dev->dev, "not reserving active resources because "
    + "\"pnp=no_reservations\"\n");
    + for (i = 0; i < PNP_MAX_PORT; i++) {
    + if (pnp_port_valid(dev, i)) {
    + res = &dev->res.port_resource[i];
    + dev_warn(&dev->dev, " ports 0x%llx-0x%llx\n",
    + (unsigned long long) res->start,
    + (unsigned long long) res->end);
    + }
    + }
    + for (i = 0; i < PNP_MAX_MEM; i++) {
    + if (pnp_mem_valid(dev, i)) {
    + res = &dev->res.mem_resource[i];
    + dev_warn(&dev->dev, " MMIO 0x%llx-0x%llx\n",
    + (unsigned long long) res->start,
    + (unsigned long long) res->end);
    + }
    + }
    + return 0;
    + }
    +
    + /*
    + * We use insert_resource() rather than request_resource() because
    + * request_standard_resources() has already requested some standard
    + * areas that are described by PNP.
    + */
    + for (i = 0; i < PNP_MAX_PORT; i++) {
    + if (pnp_port_valid(dev, i)) {
    + res = &dev->res.port_resource[i];
    + res->name = dev->dev.bus_id;
    + ret = insert_resource(&ioport_resource, res);
    + if (ret)
    + dev_warn(&dev->dev,
    + "can't allocate I/O ports at 0x%llx\n",
    + (unsigned long long) res->start);
    + }
    + }
    +
    + for (i = 0; i < PNP_MAX_MEM; i++) {
    + if (pnp_mem_valid(dev, i)) {
    + res = &dev->res.mem_resource[i];
    + res->name = dev->dev.bus_id;
    + ret = insert_resource(&iomem_resource, res);
    + if (ret)
    + dev_warn(&dev->dev,
    + "can't allocate MMIO space at 0x%llx\n",
    + (unsigned long long) res->start);
    + }
    + }
    +
    + return 0;
    +}
    +
    +int pnp_release_resources(struct pnp_dev *dev)
    +{
    + int i, ret;
    + struct resource *res;
    +
    + if (pnp_no_reservations)
    + return 0;
    +
    + for (i = 0; i < PNP_MAX_PORT; i++) {
    + if (pnp_port_valid(dev, i)) {
    + res = &dev->res.port_resource[i];
    + ret = release_resource(res);
    + if (ret)
    + dev_warn(&dev->dev,
    + "can't release I/O ports at 0x%llx\n",
    + (unsigned long long) res->start);
    + }
    + }
    +
    + for (i = 0; i < PNP_MAX_MEM; i++) {
    + if (pnp_mem_valid(dev, i)) {
    + res = &dev->res.mem_resource[i];
    + ret = release_resource(res);
    + if (ret)
    + dev_warn(&dev->dev,
    + "can't release MMIO space at 0x%llx\n",
    + (unsigned long long) res->start);
    + }
    + }
    +
    + return 0;
    +}
    +
    /* format is: pnp_reserve_irq=irq1[,irq2] .... */
    static int __init pnp_setup_reserve_irq(char *str)
    {
    @@ -510,3 +603,13 @@
    }

    __setup("pnp_reserve_mem=", pnp_setup_reserve_mem);
    +
    +static int __init pnp_setup(char *str)
    +{
    + if (!strcmp(str, "no_reservations"))
    + pnp_no_reservations = 1;
    +
    + return 1;
    +}
    +
    +__setup("pnp=", pnp_setup);
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  2. Re: [RFC] [PATCH] PNP: request ioport and iomem resources used by active devices

    On Mon, Oct 29, 2007 at 03:25:31PM -0600, Bjorn Helgaas wrote:
    > Reserve resources used by active PNP devices to prevent those resources
    > from being assigned to other devices.


    Yes, I think this is probably a safe approach to take.

    --
    Matthew Garrett | mjg59@srcf.ucam.org
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  3. Re: [RFC] [PATCH] PNP: request ioport and iomem resources used by active devices

    On Mon, 29 Oct 2007, Bjorn Helgaas wrote:

    > 0000-001f : dma1 <-- built-in resource includes 2 controllers
    > 0000-000f : 00:02 <-- PNP reports only one DMA controller

    [...]
    > 0060-006f : keyboard <-- built-in resource groups several things
    > 0060-0060 : 00:04 <-- PNP reports 8042 controller data register
    > 0061-0061 : 00:03 <-- PNP reports AT-style speaker
    > 0064-0064 : 00:04 <-- PNP reports 8042 controller status register


    Note that the built-in resources are requested according to how the PC/AT
    architecture defined address decoding for the relevant subsystems. In
    particular, there was always a single DMA controller at 0x0000 and the
    8042 controller was decoded at 0x0060. No full decoding was done and
    unused locations aliased to the corresponding ones with the don't-care
    bits set to the other value. I do not recall all the details of the port
    B (named after the original from the 8255 as used in the PC/XT) at 0x0061
    as implemented in the PC/AT and I do not have a reference handy, but it is
    a bunch of GPIO bits, some being r/o and some r/w, possibly with
    side-effects. I believe it was a set of additional latches sub-decoded
    from the 8042 controller range for compatibility with how the 8255 was
    wired and programmed.

    Anyway, PNP may provide a more accurate picture of the address ranges in
    a particular system... if it gets it right! Which is a lost hope, I
    suppose, as even the above is questionable -- "AT-style speaker" is a
    misnomer as most of the GPIO bits involved deal with the handling of
    system error interrupts normally routed to NMI.

    The patch itself is useful and makes sense though. Have you passed it
    through `scripts/checkpatch.pl'?

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

  4. Re: [RFC] [PATCH] PNP: request ioport and iomem resources used by active devices

    On Wednesday 07 November 2007 06:22:48 am Maciej W. Rozycki wrote:
    > On Mon, 29 Oct 2007, Bjorn Helgaas wrote:
    >
    > > 0000-001f : dma1 <-- built-in resource includes 2 controllers
    > > 0000-000f : 00:02 <-- PNP reports only one DMA controller

    > [...]
    > > 0060-006f : keyboard <-- built-in resource groups several things
    > > 0060-0060 : 00:04 <-- PNP reports 8042 controller data register
    > > 0061-0061 : 00:03 <-- PNP reports AT-style speaker
    > > 0064-0064 : 00:04 <-- PNP reports 8042 controller status register

    >
    > Note that the built-in resources are requested according to how the PC/AT
    > architecture defined address decoding for the relevant subsystems. In
    > particular, there was always a single DMA controller at 0x0000 and the
    > 8042 controller was decoded at 0x0060. No full decoding was done and
    > unused locations aliased to the corresponding ones with the don't-care
    > bits set to the other value. I do not recall all the details of the port
    > B (named after the original from the 8255 as used in the PC/XT) at 0x0061
    > as implemented in the PC/AT and I do not have a reference handy, but it is
    > a bunch of GPIO bits, some being r/o and some r/w, possibly with
    > side-effects. I believe it was a set of additional latches sub-decoded
    > from the 8042 controller range for compatibility with how the 8255 was
    > wired and programmed.
    >
    > Anyway, PNP may provide a more accurate picture of the address ranges in
    > a particular system... if it gets it right! Which is a lost hope, I
    > suppose, as even the above is questionable -- "AT-style speaker" is a
    > misnomer as most of the GPIO bits involved deal with the handling of
    > system error interrupts normally routed to NMI.


    Thanks for the background and the comments.

    There will undoubtedly be errors in what the firmware reports. But if
    firmware tells us about some non PC/AT device, we should believe it at
    least enough to avoid placing some other device on top of it.

    If we have PNPBIOS or ACPI, the built-in stuff reserved in
    request_standard_resources() should be superfluous. In theory.
    But I doubt there's any benefit in skipping request_standard_resources().
    Possibly it could be done *after* the PNP reservations, so the
    hierarchy would make more sense, but I don't know if even that
    is worth the trouble.

    > The patch itself is useful and makes sense though. Have you passed it
    > through `scripts/checkpatch.pl'?


    Yes. The patch was added to -mm on Nov 1 as
    pnp-request-ioport-and-iomem-resources-used-by-active-devices.patch.

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

  5. Re: [RFC] [PATCH] PNP: request ioport and iomem resources used by active devices

    On Wed, 7 Nov 2007, Bjorn Helgaas wrote:

    > There will undoubtedly be errors in what the firmware reports. But if
    > firmware tells us about some non PC/AT device, we should believe it at
    > least enough to avoid placing some other device on top of it.
    >
    > If we have PNPBIOS or ACPI, the built-in stuff reserved in
    > request_standard_resources() should be superfluous. In theory.
    > But I doubt there's any benefit in skipping request_standard_resources().
    > Possibly it could be done *after* the PNP reservations, so the
    > hierarchy would make more sense, but I don't know if even that
    > is worth the trouble.


    In practice it is resources above 0x00ff that really matter here as using
    the motherboard range for device allocation is unsafe anyway (the whole
    range may be actively decoded by the south bridge). So whatever is
    reported between 0x0000 and 0x00ff is mostly informational; with some
    protection against driver bugs perhaps (that assuming they have got error
    handling right for reservations). I second it is not worth the hassle.

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

+ Reply to Thread