Re: [PATCH 3/5] PCI: add functionality for resizing resources v4

[Andy Shevchenko]

On Thu, May 4, 2017 at 12:31 PM, Christian KÃnig
<deathsimple@xxxxxxxxxxx> wrote:
> From: Christian KÃnig <christian.koenig@xxxxxxx>
>
> This allows device drivers to request resizing their BARs.
>
> The function only tries to reprogram the windows of the bridge directly above
> the requesting device and only the BAR of the same type (usually mem, 64bit,
> prefetchable). This is done to make sure not to disturb other drivers by
> changing the BARs of their devices.
>
> If reprogramming the bridge BAR fails the old status is restored and -ENOSPC
> returned to the calling device driver.

My comments below.

> +int pci_reassign_bridge_resources(struct pci_dev *bridge, unsigned long type)
> +{
> +       struct pci_dev_resource *dev_res;
> +       LIST_HEAD(saved);
> +       LIST_HEAD(added);
> +       LIST_HEAD(failed);
> +       unsigned int i;
> +       int ret;
> +
> +       /* Walk to the root BUS, releasing bridge BARs when possible */
> +       while (1) {
> +               for (i = PCI_BRIDGE_RESOURCES; i < PCI_BRIDGE_RESOURCE_END;
> +                    i++) {
> +                       struct resource *res = &bridge->resource[i];
> +
> +                       if ((res->flags ^ type) & PCI_RES_TYPE_MASK)
> +                               continue;
> +
> +                       /* Ignore BARs which are still in use */
> +                       if (res->child)
> +                               continue;
> +
> +                       ret = add_to_list(&saved, bridge, res, 0, 0);
> +                       if (ret)
> +                               goto cleanup;
> +
> +                       dev_info(&bridge->dev, "BAR %d: releasing %pR\n",
> +                                i, res);
> +
> +                       if (res->parent)
> +                               release_resource(res);
> +                       res->start = 0;
> +                       res->end = 0;
> +                       break;
> +               }
> +               if (i == PCI_BRIDGE_RESOURCE_END)
> +                       break;
> +
> +               if (!bridge->bus || !bridge->bus->self)
> +                       break;
> +
> +               bridge = bridge->bus->self;
> +       }
> +

Please, consider to refactor as I proposed the other day.

> +}

> +int pci_resize_resource(struct pci_dev *dev, int resno, int size)
> +{
> +       struct resource *res = dev->resource + resno;

> +       int old, ret = 0;

Redundant assignment.

> +       u32 sizes;
> +       u16 cmd;
> +

> +       ret = pci_rbar_set_size(dev, resno, size);
> +       if (ret)
> +               goto error_reassign;
> +

> +       return 0;
> +
> +error_resize:
> +       pci_rbar_set_size(dev, resno, old);
> +       res->end = res->start + pci_rbar_size_to_bytes(old) - 1;
> +
> +error_reassign:
> +       pci_assign_unassigned_bus_resources(dev->bus);
> +       return ret;
> +}

-- 
With Best Regards,
Andy Shevchenko