From mboxrd@z Thu Jan 1 00:00:00 1970 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 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 From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andy Shevchenko Subject: Re: [PATCH 2/4] PCI: add functionality for resizing resources v2 Date: Mon, 13 Mar 2017 18:43:42 +0200 Message-ID: References: <1489408896-25039-1-git-send-email-deathsimple@vodafone.de> <1489408896-25039-3-git-send-email-deathsimple@vodafone.de> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <1489408896-25039-3-git-send-email-deathsimple@vodafone.de> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: =?UTF-8?Q?Christian_K=C3=B6nig?= Cc: "linux-pci@vger.kernel.org" , "linux-kernel@vger.kernel.org" , amd-gfx@lists.freedesktop.org, Platform Driver , helgaas@kernel.org, dri-devel@lists.freedesktop.org List-Id: platform-driver-x86.vger.kernel.org T24gTW9uLCBNYXIgMTMsIDIwMTcgYXQgMjo0MSBQTSwgQ2hyaXN0aWFuIEvDtm5pZwo8ZGVhdGhz aW1wbGVAdm9kYWZvbmUuZGU+IHdyb3RlOgo+IEZyb206IENocmlzdGlhbiBLw7ZuaWcgPGNocmlz dGlhbi5rb2VuaWdAYW1kLmNvbT4KPgo+IFRoaXMgYWxsb3dzIGRldmljZSBkcml2ZXJzIHRvIHJl cXVlc3QgcmVzaXppbmcgdGhlaXIgQkFScy4KPgo+IFRoZSBmdW5jdGlvbiBvbmx5IHRyaWVzIHRv IHJlcHJvZ3JhbSB0aGUgd2luZG93cyBvZiB0aGUgYnJpZGdlIGRpcmVjdGx5IGFib3ZlCj4gdGhl IHJlcXVlc3RpbmcgZGV2aWNlIGFuZCBvbmx5IHRoZSBCQVIgb2YgdGhlIHNhbWUgdHlwZSAodXN1 YWxseSBtZW0sIDY0Yml0LAo+IHByZWZldGNoYWJsZSkuIFRoaXMgaXMgZG9uZSB0byBtYWtlIHN1 cmUgbm90IHRvIGRpc3R1cmIgb3RoZXIgZHJpdmVycyBieQo+IGNoYW5naW5nIHRoZSBCQVJzIG9m IHRoZWlyIGRldmljZXMuCj4KPiBJZiByZXByb2dyYW1taW5nIHRoZSBicmlkZ2UgQkFSIGZhaWxz IHRoZSBvbGQgc3RhdHVzIGlzIHJlc3RvcmVkIGFuZCAtRU5PU1BDCj4gcmV0dXJuZWQgdG8gdGhl IGNhbGxpbmcgZGV2aWNlIGRyaXZlci4KClNvbWUgc3R5bGUgY29tbWVudHMuCgo+IHYyOiByZWJh c2Ugb24gY2hhbmdlcyBpbiByYmFyIHN1cHBvcnQKClRoaXMga2luZCBvZiBjb21tZW50cyB1c3Vh bGx5IGdvZXMgYWZ0ZXIgLS0tIGRlbGltaXRlciBiZWxvdy4KCj4gU2lnbmVkLW9mZi1ieTogQ2hy aXN0aWFuIEvDtm5pZyA8Y2hyaXN0aWFuLmtvZW5pZ0BhbWQuY29tPgo+IC0tLQoKLi4uaGVyZS4K Cj4gK2ludCBwY2lfcmVhc3NpZ25fYnJpZGdlX3Jlc291cmNlcyhzdHJ1Y3QgcGNpX2RldiAqYnJp ZGdlLCB1bnNpZ25lZCBsb25nIHR5cGUpCj4gK3sKPiArICAgICAgIGNvbnN0IHVuc2lnbmVkIGxv bmcgdHlwZV9tYXNrID0gSU9SRVNPVVJDRV9JTyB8IElPUkVTT1VSQ0VfTUVNIHwKPiArICAgICAg ICAgICAgICAgSU9SRVNPVVJDRV9QUkVGRVRDSCB8IElPUkVTT1VSQ0VfTUVNXzY0Owo+ICsKPiAr ICAgICAgIHN0cnVjdCByZXNvdXJjZSBzYXZlZDsKPiArICAgICAgIExJU1RfSEVBRChhZGRfbGlz dCk7Cj4gKyAgICAgICBMSVNUX0hFQUQoZmFpbF9oZWFkKTsKCj4gKyAgICAgICBzdHJ1Y3QgcGNp X2Rldl9yZXNvdXJjZSAqZmFpbF9yZXM7CgpQZXJoYXBzIG1vdmUgYmVmb3JlIGRlZmluaXRpb24g b2YgJ3NhdmVkJz8KCj4gKyAgICAgICB1bnNpZ25lZCBpOwo+ICsgICAgICAgaW50IHJldCA9IDA7 Cj4gKwo+ICsgICAgICAgLyogUmVsZWFzZSBhbGwgY2hpbGRyZW4gZnJvbSB0aGUgbWF0Y2hpbmcg YnJpZGdlIHJlc291cmNlICovCj4gKyAgICAgICBmb3IgKGkgPSBQQ0lfQlJJREdFX1JFU09VUkNF UzsgaSA8IFBDSV9CUklER0VfUkVTT1VSQ0VfRU5EOyArK2kpIHsKPiArICAgICAgICAgICAgICAg c3RydWN0IHJlc291cmNlICpyZXMgPSAmYnJpZGdlLT5yZXNvdXJjZVtpXTsKPiArCgoKPiArICAg ICAgICAgICAgICAgaWYgKChyZXMtPmZsYWdzICYgdHlwZV9tYXNrKSAhPSAodHlwZSAmIHR5cGVf bWFzaykpCgpJSVVDIGl0IHdvdWxkIGJlCmlmICgocmVzLT5mbGFncyBeIHR5cGUpICYgdHlwZV9t YXNrKQoKKEkgY29uc2lkZXIgJ2RpZmYnIGFzIFhPUiBvcGVyYXRpb24gaXMgbW9yZSB1bmRlcnN0 YW5kYWJsZSwgYnV0IGl0J3MgdXAgdG8geW91KQoKPiArICAgICAgICAgICAgICAgICAgICAgICBj b250aW51ZTsKCj4gKyAgICAgICAvKiByZXN0b3JlIHNpemUgYW5kIGZsYWdzICovCj4gKyAgICAg ICBsaXN0X2Zvcl9lYWNoX2VudHJ5KGZhaWxfcmVzLCAmZmFpbF9oZWFkLCBsaXN0KSB7Cj4gKyAg ICAgICAgICAgICAgIHN0cnVjdCByZXNvdXJjZSAqcmVzID0gZmFpbF9yZXMtPnJlczsKPiArCj4g KyAgICAgICAgICAgICAgIHJlcy0+c3RhcnQgPSBmYWlsX3Jlcy0+c3RhcnQ7Cj4gKyAgICAgICAg ICAgICAgIHJlcy0+ZW5kID0gZmFpbF9yZXMtPmVuZDsKPiArICAgICAgICAgICAgICAgcmVzLT5m bGFncyA9IGZhaWxfcmVzLT5mbGFnczsKPiArICAgICAgIH0KPiArCj4gKyAgICAgICAvKiBSZXZl cnQgdG8gdGhlIG9sZCBjb25maWd1cmF0aW9uICovCj4gKyAgICAgICBpZiAoIWxpc3RfZW1wdHko JmZhaWxfaGVhZCkpIHsKPiArICAgICAgICAgICAgICAgc3RydWN0IHJlc291cmNlICpyZXMgPSAm YnJpZGdlLT5yZXNvdXJjZVtpXTsKPiArCgo+ICsgICAgICAgICAgICAgICByZXMtPnN0YXJ0ID0g c2F2ZWQuc3RhcnQ7Cj4gKyAgICAgICAgICAgICAgIHJlcy0+ZW5kID0gc2F2ZWQuZW5kOwo+ICsg ICAgICAgICAgICAgICByZXMtPmZsYWdzID0gc2F2ZWQuZmxhZ3M7CgpXb3VsZAogKnJlcyA9IHNh dmVkOwp3b3JrPwoKPiAraW50IHBjaV9yZXNpemVfcmVzb3VyY2Uoc3RydWN0IHBjaV9kZXYgKmRl diwgaW50IHJlc25vLCBpbnQgc2l6ZSkKPiArewo+ICsgICAgICAgc3RydWN0IHJlc291cmNlICpy ZXMgPSBkZXYtPnJlc291cmNlICsgcmVzbm87Cj4gKyAgICAgICB1MzIgc2l6ZXMgPSBwY2lfcmJh cl9nZXRfcG9zc2libGVfc2l6ZXMoZGV2LCByZXNubyk7Cj4gKyAgICAgICBpbnQgb2xkID0gcGNp X3JiYXJfZ2V0X2N1cnJlbnRfc2l6ZShkZXYsIHJlc25vKTsKPiArICAgICAgIHU2NCBieXRlcyA9 IDFVTEwgPDwgKHNpemUgKyAyMCk7Cj4gKyAgICAgICBpbnQgcmV0ID0gMDsKPiArCgpJIHdvdWxk IHB1dAogc2l6ZXMgPSBwY2lfcmJhcl9nZXRfcG9zc2libGVfc2l6ZXMoZGV2LCByZXNubyk7Cmhl cmUKCj4gKyAgICAgICBpZiAoIXNpemVzKQo+ICsgICAgICAgICAgICAgICByZXR1cm4gLUVOT1RT VVBQOwo+ICsKPiArICAgICAgIGlmICghKHNpemVzICYgKDEgPDwgc2l6ZSkpKQoKQklUKHNpemUp ID8KCj4gKyAgICAgICAgICAgICAgIHJldHVybiAtRUlOVkFMOwo+ICsKCmFuZAogb2xkID0gcGNp X3JiYXJfZ2V0X2N1cnJlbnRfc2l6ZShkZXYsIHJlc25vKTsKaGVyZQoKPiArICAgICAgIGlmIChv bGQgPCAwKQo+ICsgICAgICAgICAgICAgICByZXR1cm4gb2xkOwoKPiArZXJyb3JfcmVzaXplOgo+ ICsgICAgICAgcGNpX3JiYXJfc2V0X3NpemUoZGV2LCByZXNubywgb2xkKTsKPiArICAgICAgIHJl cy0+ZW5kID0gcmVzLT5zdGFydCArICgxVUxMIDw8IChvbGQgKyAyMCkpIC0gMTsKCkJJVF9VTEwo b2xkICsgMjApID8KCi0tIApXaXRoIEJlc3QgUmVnYXJkcywKQW5keSBTaGV2Y2hlbmtvCl9fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCmRyaS1kZXZlbCBtYWls aW5nIGxpc3QKZHJpLWRldmVsQGxpc3RzLmZyZWVkZXNrdG9wLm9yZwpodHRwczovL2xpc3RzLmZy ZWVkZXNrdG9wLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2RyaS1kZXZlbAo= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Return-Path: 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 List-ID: On Mon, Mar 13, 2017 at 2:41 PM, Christian K=C3=B6nig wrote: > From: Christian K=C3=B6nig > > This allows device drivers to request resizing their BARs. > > The function only tries to reprogram the windows of the bridge directly a= bove > the requesting device and only the BAR of the same type (usually mem, 64b= it, > 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 -ENO= SPC > 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=C3=B6nig > --- ...here. > +int pci_reassign_bridge_resources(struct pci_dev *bridge, unsigned long = type) > +{ > + const unsigned long type_mask =3D 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 =3D 0; > + > + /* Release all children from the matching bridge resource */ > + for (i =3D PCI_BRIDGE_RESOURCES; i < PCI_BRIDGE_RESOURCE_END; ++i= ) { > + struct resource *res =3D &bridge->resource[i]; > + > + if ((res->flags & type_mask) !=3D (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 =3D fail_res->res; > + > + res->start =3D fail_res->start; > + res->end =3D fail_res->end; > + res->flags =3D fail_res->flags; > + } > + > + /* Revert to the old configuration */ > + if (!list_empty(&fail_head)) { > + struct resource *res =3D &bridge->resource[i]; > + > + res->start =3D saved.start; > + res->end =3D saved.end; > + res->flags =3D saved.flags; Would *res =3D saved; work? > +int pci_resize_resource(struct pci_dev *dev, int resno, int size) > +{ > + struct resource *res =3D dev->resource + resno; > + u32 sizes =3D pci_rbar_get_possible_sizes(dev, resno); > + int old =3D pci_rbar_get_current_size(dev, resno); > + u64 bytes =3D 1ULL << (size + 20); > + int ret =3D 0; > + I would put sizes =3D pci_rbar_get_possible_sizes(dev, resno); here > + if (!sizes) > + return -ENOTSUPP; > + > + if (!(sizes & (1 << size))) BIT(size) ? > + return -EINVAL; > + and old =3D pci_rbar_get_current_size(dev, resno); here > + if (old < 0) > + return old; > +error_resize: > + pci_rbar_set_size(dev, resno, old); > + res->end =3D res->start + (1ULL << (old + 20)) - 1; BIT_ULL(old + 20) ? --=20 With Best Regards, Andy Shevchenko