On Mon, Mar 13, 2017 at 2:41 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.
Some style comments.
> v2: rebase on changes in rbar support
This kind of comments usually goes after --- delimiter below.
> Signed-off-by: Christian König <christian.koenig@xxxxxxx>
> ---
...here.
> +int pci_reassign_bridge_resources(struct pci_dev *bridge, unsigned long type)
> +{
> + const unsigned long type_mask = IORESOURCE_IO | IORESOURCE_MEM |
> + IORESOURCE_PREFETCH | IORESOURCE_MEM_64;
> +
> + struct resource saved;
> + LIST_HEAD(add_list);
> + LIST_HEAD(fail_head);
> + struct pci_dev_resource *fail_res;
Perhaps move before definition of 'saved'?
> + unsigned i;
> + int ret = 0;
> +
> + /* Release all children from the matching bridge resource */
> + for (i = PCI_BRIDGE_RESOURCES; i < PCI_BRIDGE_RESOURCE_END; ++i) {
> + struct resource *res = &bridge->resource[i];
> +
> + if ((res->flags & type_mask) != (type & type_mask))
IIUC it would be
if ((res->flags ^ type) & type_mask)
(I consider 'diff' as XOR operation is more understandable, but it's up to you)
> + continue;
> + /* restore size and flags */
> + list_for_each_entry(fail_res, &fail_head, list) {
> + struct resource *res = fail_res->res;
> +
> + res->start = fail_res->start;
> + res->end = fail_res->end;
> + res->flags = fail_res->flags;
> + }
> +
> + /* Revert to the old configuration */
> + if (!list_empty(&fail_head)) {
> + struct resource *res = &bridge->resource[i];
> +
> + res->start = saved.start;
> + res->end = saved.end;
> + res->flags = saved.flags;
Would
*res = saved;
work?
> +int pci_resize_resource(struct pci_dev *dev, int resno, int size)
> +{
> + struct resource *res = dev->resource + resno;
> + u32 sizes = pci_rbar_get_possible_sizes(dev, resno);
> + int old = pci_rbar_get_current_size(dev, resno);
> + u64 bytes = 1ULL << (size + 20);
> + int ret = 0;
> +
I would put
sizes = pci_rbar_get_possible_sizes(dev, resno);
here
> + if (!sizes)
> + return -ENOTSUPP;
> +
> + if (!(sizes & (1 << size)))
BIT(size) ?
> + return -EINVAL;
> +
and
old = pci_rbar_get_current_size(dev, resno);
here
> + if (old < 0)
> + return old;
> +error_resize:
> + pci_rbar_set_size(dev, resno, old);
> + res->end = res->start + (1ULL << (old + 20)) - 1;
BIT_ULL(old + 20) ?
--
With Best Regards,
Andy Shevchenko
