Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932150AbdCMQqu (ORCPT ); Mon, 13 Mar 2017 12:46:50 -0400 Received: from mail-qt0-f196.google.com ([209.85.216.196]:33005 "EHLO mail-qt0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754259AbdCMQoP (ORCPT ); Mon, 13 Mar 2017 12:44:15 -0400 MIME-Version: 1.0 In-Reply-To: <1489408896-25039-3-git-send-email-deathsimple@vodafone.de> References: <1489408896-25039-1-git-send-email-deathsimple@vodafone.de> <1489408896-25039-3-git-send-email-deathsimple@vodafone.de> From: Andy Shevchenko Date: Mon, 13 Mar 2017 18:43:42 +0200 Message-ID: Subject: Re: [PATCH 2/4] PCI: add functionality for resizing resources v2 To: =?UTF-8?Q?Christian_K=C3=B6nig?= Cc: helgaas@kernel.org, "linux-pci@vger.kernel.org" , dri-devel@lists.freedesktop.org, Platform Driver , amd-gfx@lists.freedesktop.org, "linux-kernel@vger.kernel.org" Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from quoted-printable to 8bit by mail.home.local id v2DGnalB005779 Content-Length: 3187 Lines: 117 On Mon, Mar 13, 2017 at 2:41 PM, Christian König wrote: > From: Christian König > > 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 > --- ...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