From mboxrd@z Thu Jan 1 00:00:00 1970 From: heiko@sntech.de (Heiko =?utf-8?q?St=C3=BCbner?=) Date: Wed, 26 Jun 2013 11:18:43 +0200 Subject: [PATCH v2 2/6] misc: sram: add ability to mark sram sections as reserved In-Reply-To: <1372155425.3981.67.camel@pizza.hi.pengutronix.de> References: <201306251046.04873.heiko@sntech.de> <201306251047.12899.heiko@sntech.de> <1372155425.3981.67.camel@pizza.hi.pengutronix.de> Message-ID: <201306261118.44637.heiko@sntech.de> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi Philipp, Am Dienstag, 25. Juni 2013, 12:17:05 schrieb Philipp Zabel: > Hi Heiko, > > Am Dienstag, den 25.06.2013, 10:47 +0200 schrieb Heiko St?bner: > > Some SoCs need parts of their sram for special purposes. So while being > > part of the periphal, it should not be part of the genpool controlling > > the sram. > > > > Threfore add an option mmio-sram-reserved to keep arbitary portions of > > the sram from being part of the pool. > > > > Suggested-by: Rob Herring > > Signed-off-by: Heiko Stuebner > > --- > > > > Documentation/devicetree/bindings/misc/sram.txt | 8 +++ > > drivers/misc/sram.c | 86 > > +++++++++++++++++++++-- 2 files changed, 88 insertions(+), 6 > > deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/misc/sram.txt > > b/Documentation/devicetree/bindings/misc/sram.txt index 4d0a00e..eae080e > > 100644 > > --- a/Documentation/devicetree/bindings/misc/sram.txt > > +++ b/Documentation/devicetree/bindings/misc/sram.txt > > > > @@ -8,9 +8,17 @@ Required properties: > > - reg : SRAM iomem address range > > > > +Optional properties: > > + > > +- mmio-sram-reserved: ordered list of reserved chunks inside the sram > > that + should not become part of the genalloc pool. > > + Format is , , ...; with base being relative to > > the + reg property base. > > + > > the keyword to reserve blocks of ram is /memreserve/ - should this > property name be aligned with that? The mmio-sram-reserved name was suggested by Rob Herring, who I suppose has some slight experience with devicetree :-) . I wasn't able to find real documentation on /memreserve/ but it looks more like it's used to reserve generic memregions, not being node-specific. So reusing this might also cause confusion when the reserve-data now is relative to it's node reg. > > Example: > > > > sram: sram at 5c000000 { > > > > compatible = "mmio-sram"; > > reg = <0x5c000000 0x40000>; /* 256 KiB SRAM at address 0x5c000000 */ > > > > + mmio-sram-reserved = <0x0 0x100>; /* reserve 0x5c000000-0x5c000100 */ > > > > }; > > > > diff --git a/drivers/misc/sram.c b/drivers/misc/sram.c > > index afe66571..5fccbe3 100644 > > --- a/drivers/misc/sram.c > > +++ b/drivers/misc/sram.c > > @@ -42,6 +42,8 @@ static int sram_probe(struct platform_device *pdev) > > > > struct sram_dev *sram; > > struct resource *res; > > unsigned long size; > > > > + const __be32 *reserved_list = NULL; > > + int reserved_size = 0; > > > > int ret; > > > > res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > > > > @@ -65,12 +67,89 @@ static int sram_probe(struct platform_device *pdev) > > > > if (!sram->pool) > > > > return -ENOMEM; > > > > - ret = gen_pool_add_virt(sram->pool, (unsigned long)virt_base, > > - res->start, size, -1); > > - if (ret < 0) { > > - if (sram->clk) > > - clk_disable_unprepare(sram->clk); > > - return ret; > > + if (pdev->dev.of_node) { > > + reserved_list = of_get_property(pdev->dev.of_node, > > + "mmio-sram-reserved", > > + &reserved_size); > > + if (reserved_list) { > > + reserved_size /= sizeof(*reserved_list); > > + if (!reserved_size || reserved_size % 2) { > > + dev_warn(&pdev->dev, "wrong number of arguments in > > mmio-sram-reserved\n"); + reserved_list = NULL; > > + } > > + } > > + } > > + > > + if (!reserved_list) { > > + ret = gen_pool_add_virt(sram->pool, (unsigned long)virt_base, > > + res->start, size, -1); > > + if (ret < 0) { > > + if (sram->clk) > > + clk_disable_unprepare(sram->clk); > > + return ret; > > + } > > Moving the clk_prepare_enable() further down would allow to avoid the > clk_disable_unprepare() in every error path, > > > + } else { > > + unsigned int cur_start = 0; > > + unsigned int cur_size; > > + unsigned int rstart; > > + unsigned int rsize; > > + int i; > > + > > + for (i = 0; i < reserved_size; i += 2) { > > + /* get the next reserved block */ > > + rstart = be32_to_cpu(*reserved_list++); > > + rsize = be32_to_cpu(*reserved_list++); > > + > > + /* catch unsorted list entries */ > > + if (rstart < cur_start) { > > + dev_err(&pdev->dev, "unsorted reserved list (0x%x before current > > 0x%x)\n", + rstart, cur_start); > > + if (sram->clk) > > + clk_disable_unprepare(sram->clk); > > like here > > > + return -EINVAL; > > + } > > + > > + dev_dbg(&pdev->dev, "found reserved block 0x%x-0x%x\n", > > + rstart, rstart + rsize); > > + > > + /* current start is in a reserved block */ > > + if (rstart <= cur_start) { > > + cur_start = rstart + rsize; > > + continue; > > + } > > + > > + /* > > + * allocate the space between the current starting > > + * address and the following reserved block > > + */ > > + cur_size = rstart - cur_start; > > + > > + dev_dbg(&pdev->dev, "adding chunk 0x%x-0x%x\n", > > + cur_start, cur_start + cur_size); > > + ret = gen_pool_add_virt(sram->pool, > > + (unsigned long)virt_base + cur_start, > > + res->start + cur_start, cur_size, -1); > > + if (ret < 0) { > > + if (sram->clk) > > + clk_disable_unprepare(sram->clk); > > and here. > > > + return ret; > > + } > > + > > + /* next allocation after this reserved block */ > > + cur_start = rstart + rsize; > > + } > > + > > + /* allocate the space after the last reserved block */ > > + if (cur_start < size) { > > + cur_size = size - cur_start; > > + > > + dev_dbg(&pdev->dev, "adding chunk 0x%x-0x%x\n", > > + cur_start, cur_start + cur_size); > > + ret = gen_pool_add_virt(sram->pool, > > + (unsigned long)virt_base + cur_start, > > + res->start + cur_start, cur_size, -1); > > + } > > + > > > > } > > > > platform_set_drvdata(pdev, sram); > > Also, I think you could reduce the duplication of gen_pool_add_virt() > function calls, somehow like this: > > unsigned int cur_start = 0; > unsigned int cur_size; > unsigned int rstart; > unsigned int rsize; > int i = 0; > > if (!reserved_list) > reserved_size = 0; > > for (i = 0; i < (reserved_size + 2); i += 2) { > if (i < reserved_size) { > /* get the next reserved block */ > rstart = be32_to_cpu(*reserved_list++); > rsize = be32_to_cpu(*reserved_list++); > > /* catch unsorted list entries */ > if (rstart < cur_start) { > dev_err(&pdev->dev, > "unsorted reserved list (0x%x before current 0x%x)\n", > rstart, cur_start); > return -EINVAL; > } > > dev_dbg(&pdev->dev, > "found reserved block 0x%x-0x%x\n", > rstart, rstart + rsize); > } else { > /* the last chunk extends to the end of the region */ > rstart = size; > } > > /* current start is in a reserved block */ > if (rstart <= cur_start) { > cur_start = rstart + rsize; > continue; > } > > /* > * allocate the space between the current starting > * address and the following reserved block, or the > * end of the region. > */ > cur_size = rstart - cur_start; > > dev_dbg(&pdev->dev, "adding chunk 0x%x-0x%x\n", > cur_start, cur_start + cur_size); > ret = gen_pool_add_virt(sram->pool, > (unsigned long)virt_base + cur_start, > res->start + cur_start, cur_size, -1); > if (ret < 0) > return ret; > } yep, this looks nicer - same for moving the clk_prepare_enable to below this loop to unclutter the error-path. So I will incorporate this in v3. Thanks Heiko From mboxrd@z Thu Jan 1 00:00:00 1970 From: Heiko =?utf-8?q?St=C3=BCbner?= Subject: Re: [PATCH v2 2/6] misc: sram: add ability to mark sram sections as reserved Date: Wed, 26 Jun 2013 11:18:43 +0200 Message-ID: <201306261118.44637.heiko@sntech.de> References: <201306251046.04873.heiko@sntech.de> <201306251047.12899.heiko@sntech.de> <1372155425.3981.67.camel@pizza.hi.pengutronix.de> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <1372155425.3981.67.camel-/rZezPiN1rtR6QfukMTsflXZhhPuCNm+@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: devicetree-discuss-bounces+gldd-devicetree-discuss=m.gmane.org-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org Sender: "devicetree-discuss" To: Philipp Zabel Cc: Russell King , Greg Kroah-Hartman , devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Grant Likely , "linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org" List-Id: devicetree@vger.kernel.org SGkgUGhpbGlwcCwKCkFtIERpZW5zdGFnLCAyNS4gSnVuaSAyMDEzLCAxMjoxNzowNSBzY2hyaWVi IFBoaWxpcHAgWmFiZWw6Cj4gSGkgSGVpa28sCj4gCj4gQW0gRGllbnN0YWcsIGRlbiAyNS4wNi4y MDEzLCAxMDo0NyArMDIwMCBzY2hyaWViIEhlaWtvIFN0w7xibmVyOgo+ID4gU29tZSBTb0NzIG5l ZWQgcGFydHMgb2YgdGhlaXIgc3JhbSBmb3Igc3BlY2lhbCBwdXJwb3Nlcy4gU28gd2hpbGUgYmVp bmcKPiA+IHBhcnQgb2YgdGhlIHBlcmlwaGFsLCBpdCBzaG91bGQgbm90IGJlIHBhcnQgb2YgdGhl IGdlbnBvb2wgY29udHJvbGxpbmcKPiA+IHRoZSBzcmFtLgo+ID4gCj4gPiBUaHJlZm9yZSBhZGQg YW4gb3B0aW9uIG1taW8tc3JhbS1yZXNlcnZlZCB0byBrZWVwIGFyYml0YXJ5IHBvcnRpb25zIG9m Cj4gPiB0aGUgc3JhbSBmcm9tIGJlaW5nIHBhcnQgb2YgdGhlIHBvb2wuCj4gPiAKPiA+IFN1Z2dl c3RlZC1ieTogUm9iIEhlcnJpbmcgPHJvYmhlcnJpbmcyQGdtYWlsLmNvbT4KPiA+IFNpZ25lZC1v ZmYtYnk6IEhlaWtvIFN0dWVibmVyIDxoZWlrb0BzbnRlY2guZGU+Cj4gPiAtLS0KPiA+IAo+ID4g IERvY3VtZW50YXRpb24vZGV2aWNldHJlZS9iaW5kaW5ncy9taXNjL3NyYW0udHh0IHwgICAgOCAr KysKPiA+ICBkcml2ZXJzL21pc2Mvc3JhbS5jICAgICAgICAgICAgICAgICAgICAgICAgICAgICB8 ICAgODYKPiA+ICArKysrKysrKysrKysrKysrKysrKystLSAyIGZpbGVzIGNoYW5nZWQsIDg4IGlu c2VydGlvbnMoKyksIDYKPiA+ICBkZWxldGlvbnMoLSkKPiA+IAo+ID4gZGlmZiAtLWdpdCBhL0Rv Y3VtZW50YXRpb24vZGV2aWNldHJlZS9iaW5kaW5ncy9taXNjL3NyYW0udHh0Cj4gPiBiL0RvY3Vt ZW50YXRpb24vZGV2aWNldHJlZS9iaW5kaW5ncy9taXNjL3NyYW0udHh0IGluZGV4IDRkMGEwMGUu LmVhZTA4MGUKPiA+IDEwMDY0NAo+ID4gLS0tIGEvRG9jdW1lbnRhdGlvbi9kZXZpY2V0cmVlL2Jp bmRpbmdzL21pc2Mvc3JhbS50eHQKPiA+ICsrKyBiL0RvY3VtZW50YXRpb24vZGV2aWNldHJlZS9i aW5kaW5ncy9taXNjL3NyYW0udHh0Cj4gPiAKPiA+IEBAIC04LDkgKzgsMTcgQEAgUmVxdWlyZWQg cHJvcGVydGllczoKPiA+ICAtIHJlZyA6IFNSQU0gaW9tZW0gYWRkcmVzcyByYW5nZQo+ID4gCj4g PiArT3B0aW9uYWwgcHJvcGVydGllczoKPiA+ICsKPiA+ICstIG1taW8tc3JhbS1yZXNlcnZlZDog b3JkZXJlZCBsaXN0IG9mIHJlc2VydmVkIGNodW5rcyBpbnNpZGUgdGhlIHNyYW0KPiA+IHRoYXQg KyAgc2hvdWxkIG5vdCBiZWNvbWUgcGFydCBvZiB0aGUgZ2VuYWxsb2MgcG9vbC4KPiA+ICsgIEZv cm1hdCBpcyA8YmFzZSBzaXplPiwgPGJhc2Ugc2l6ZT4sIC4uLjsgd2l0aCBiYXNlIGJlaW5nIHJl bGF0aXZlIHRvCj4gPiB0aGUgKyAgcmVnIHByb3BlcnR5IGJhc2UuCj4gPiArCj4gCj4gdGhlIGtl eXdvcmQgdG8gcmVzZXJ2ZSBibG9ja3Mgb2YgcmFtIGlzIC9tZW1yZXNlcnZlLyAtIHNob3VsZCB0 aGlzCj4gcHJvcGVydHkgbmFtZSBiZSBhbGlnbmVkIHdpdGggdGhhdD8KClRoZSBtbWlvLXNyYW0t cmVzZXJ2ZWQgbmFtZSB3YXMgc3VnZ2VzdGVkIGJ5IFJvYiBIZXJyaW5nLCB3aG8gSSBzdXBwb3Nl IGhhcyAKc29tZSBzbGlnaHQgZXhwZXJpZW5jZSB3aXRoIGRldmljZXRyZWUgOi0pIC4KCkkgd2Fz bid0IGFibGUgdG8gZmluZCByZWFsIGRvY3VtZW50YXRpb24gb24gL21lbXJlc2VydmUvIGJ1dCBp dCBsb29rcyBtb3JlIApsaWtlIGl0J3MgdXNlZCB0byByZXNlcnZlIGdlbmVyaWMgbWVtcmVnaW9u cywgbm90IGJlaW5nIG5vZGUtc3BlY2lmaWMuClNvIHJldXNpbmcgdGhpcyBtaWdodCBhbHNvIGNh dXNlIGNvbmZ1c2lvbiB3aGVuIHRoZSByZXNlcnZlLWRhdGEgbm93IGlzIApyZWxhdGl2ZSB0byBp dCdzIG5vZGUgcmVnLgoKCj4gPiAgRXhhbXBsZToKPiA+ICAKPiA+ICBzcmFtOiBzcmFtQDVjMDAw MDAwIHsKPiA+ICAKPiA+ICAJY29tcGF0aWJsZSA9ICJtbWlvLXNyYW0iOwo+ID4gIAlyZWcgPSA8 MHg1YzAwMDAwMCAweDQwMDAwPjsgLyogMjU2IEtpQiBTUkFNIGF0IGFkZHJlc3MgMHg1YzAwMDAw MCAqLwo+ID4gCj4gPiArCW1taW8tc3JhbS1yZXNlcnZlZCA9IDwweDAgMHgxMDA+OyAvKiByZXNl cnZlIDB4NWMwMDAwMDAtMHg1YzAwMDEwMCAqLwo+ID4gCj4gPiAgfTsKPiA+IAo+ID4gZGlmZiAt LWdpdCBhL2RyaXZlcnMvbWlzYy9zcmFtLmMgYi9kcml2ZXJzL21pc2Mvc3JhbS5jCj4gPiBpbmRl eCBhZmU2NjU3MS4uNWZjY2JlMyAxMDA2NDQKPiA+IC0tLSBhL2RyaXZlcnMvbWlzYy9zcmFtLmMK PiA+ICsrKyBiL2RyaXZlcnMvbWlzYy9zcmFtLmMKPiA+IEBAIC00Miw2ICs0Miw4IEBAIHN0YXRp YyBpbnQgc3JhbV9wcm9iZShzdHJ1Y3QgcGxhdGZvcm1fZGV2aWNlICpwZGV2KQo+ID4gCj4gPiAg CXN0cnVjdCBzcmFtX2RldiAqc3JhbTsKPiA+ICAJc3RydWN0IHJlc291cmNlICpyZXM7Cj4gPiAg CXVuc2lnbmVkIGxvbmcgc2l6ZTsKPiA+IAo+ID4gKwljb25zdCBfX2JlMzIgKnJlc2VydmVkX2xp c3QgPSBOVUxMOwo+ID4gKwlpbnQgcmVzZXJ2ZWRfc2l6ZSA9IDA7Cj4gPiAKPiA+ICAJaW50IHJl dDsKPiA+ICAJCj4gPiAgCXJlcyA9IHBsYXRmb3JtX2dldF9yZXNvdXJjZShwZGV2LCBJT1JFU09V UkNFX01FTSwgMCk7Cj4gPiAKPiA+IEBAIC02NSwxMiArNjcsODkgQEAgc3RhdGljIGludCBzcmFt X3Byb2JlKHN0cnVjdCBwbGF0Zm9ybV9kZXZpY2UgKnBkZXYpCj4gPiAKPiA+ICAJaWYgKCFzcmFt LT5wb29sKQo+ID4gIAkKPiA+ICAJCXJldHVybiAtRU5PTUVNOwo+ID4gCj4gPiAtCXJldCA9IGdl bl9wb29sX2FkZF92aXJ0KHNyYW0tPnBvb2wsICh1bnNpZ25lZCBsb25nKXZpcnRfYmFzZSwKPiA+ IC0JCQkJcmVzLT5zdGFydCwgc2l6ZSwgLTEpOwo+ID4gLQlpZiAocmV0IDwgMCkgewo+ID4gLQkJ aWYgKHNyYW0tPmNsaykKPiA+IC0JCQljbGtfZGlzYWJsZV91bnByZXBhcmUoc3JhbS0+Y2xrKTsK PiA+IC0JCXJldHVybiByZXQ7Cj4gPiArCWlmIChwZGV2LT5kZXYub2Zfbm9kZSkgewo+ID4gKwkJ cmVzZXJ2ZWRfbGlzdCA9IG9mX2dldF9wcm9wZXJ0eShwZGV2LT5kZXYub2Zfbm9kZSwKPiA+ICsJ CQkJCQkibW1pby1zcmFtLXJlc2VydmVkIiwKPiA+ICsJCQkJCQkmcmVzZXJ2ZWRfc2l6ZSk7Cj4g PiArCQlpZiAocmVzZXJ2ZWRfbGlzdCkgewo+ID4gKwkJCXJlc2VydmVkX3NpemUgLz0gc2l6ZW9m KCpyZXNlcnZlZF9saXN0KTsKPiA+ICsJCQlpZiAoIXJlc2VydmVkX3NpemUgfHwgcmVzZXJ2ZWRf c2l6ZSAlIDIpIHsKPiA+ICsJCQkJZGV2X3dhcm4oJnBkZXYtPmRldiwgIndyb25nIG51bWJlciBv ZiBhcmd1bWVudHMgaW4KPiA+IG1taW8tc3JhbS1yZXNlcnZlZFxuIik7ICsJCQkJcmVzZXJ2ZWRf bGlzdCA9IE5VTEw7Cj4gPiArCQkJfQo+ID4gKwkJfQo+ID4gKwl9Cj4gPiArCj4gPiArCWlmICgh cmVzZXJ2ZWRfbGlzdCkgewo+ID4gKwkJcmV0ID0gZ2VuX3Bvb2xfYWRkX3ZpcnQoc3JhbS0+cG9v bCwgKHVuc2lnbmVkIGxvbmcpdmlydF9iYXNlLAo+ID4gKwkJCQkJcmVzLT5zdGFydCwgc2l6ZSwg LTEpOwo+ID4gKwkJaWYgKHJldCA8IDApIHsKPiA+ICsJCQlpZiAoc3JhbS0+Y2xrKQo+ID4gKwkJ CQljbGtfZGlzYWJsZV91bnByZXBhcmUoc3JhbS0+Y2xrKTsKPiA+ICsJCQlyZXR1cm4gcmV0Owo+ ID4gKwkJfQo+IAo+IE1vdmluZyB0aGUgY2xrX3ByZXBhcmVfZW5hYmxlKCkgZnVydGhlciBkb3du IHdvdWxkIGFsbG93IHRvIGF2b2lkIHRoZQo+IGNsa19kaXNhYmxlX3VucHJlcGFyZSgpIGluIGV2 ZXJ5IGVycm9yIHBhdGgsCj4gCj4gPiArCX0gZWxzZSB7Cj4gPiArCQl1bnNpZ25lZCBpbnQgY3Vy X3N0YXJ0ID0gMDsKPiA+ICsJCXVuc2lnbmVkIGludCBjdXJfc2l6ZTsKPiA+ICsJCXVuc2lnbmVk IGludCByc3RhcnQ7Cj4gPiArCQl1bnNpZ25lZCBpbnQgcnNpemU7Cj4gPiArCQlpbnQgaTsKPiA+ ICsKPiA+ICsJCWZvciAoaSA9IDA7IGkgPCByZXNlcnZlZF9zaXplOyBpICs9IDIpIHsKPiA+ICsJ CQkvKiBnZXQgdGhlIG5leHQgcmVzZXJ2ZWQgYmxvY2sgKi8KPiA+ICsJCQlyc3RhcnQgPSBiZTMy X3RvX2NwdSgqcmVzZXJ2ZWRfbGlzdCsrKTsKPiA+ICsJCQlyc2l6ZSA9IGJlMzJfdG9fY3B1KCpy ZXNlcnZlZF9saXN0KyspOwo+ID4gKwo+ID4gKwkJCS8qIGNhdGNoIHVuc29ydGVkIGxpc3QgZW50 cmllcyAqLwo+ID4gKwkJCWlmIChyc3RhcnQgPCBjdXJfc3RhcnQpIHsKPiA+ICsJCQkJZGV2X2Vy cigmcGRldi0+ZGV2LCAidW5zb3J0ZWQgcmVzZXJ2ZWQgbGlzdCAoMHgleCBiZWZvcmUgCmN1cnJl bnQKPiA+IDB4JXgpXG4iLCArCQkJCQlyc3RhcnQsIGN1cl9zdGFydCk7Cj4gPiArCQkJCWlmIChz cmFtLT5jbGspCj4gPiArCQkJCQljbGtfZGlzYWJsZV91bnByZXBhcmUoc3JhbS0+Y2xrKTsKPiAK PiBsaWtlIGhlcmUKPiAKPiA+ICsJCQkJcmV0dXJuIC1FSU5WQUw7Cj4gPiArCQkJfQo+ID4gKwo+ ID4gKwkJCWRldl9kYmcoJnBkZXYtPmRldiwgImZvdW5kIHJlc2VydmVkIGJsb2NrIDB4JXgtMHgl eFxuIiwKPiA+ICsJCQkJIHJzdGFydCwgcnN0YXJ0ICsgcnNpemUpOwo+ID4gKwo+ID4gKwkJCS8q IGN1cnJlbnQgc3RhcnQgaXMgaW4gYSByZXNlcnZlZCBibG9jayAqLwo+ID4gKwkJCWlmIChyc3Rh cnQgPD0gY3VyX3N0YXJ0KSB7Cj4gPiArCQkJCWN1cl9zdGFydCA9IHJzdGFydCArIHJzaXplOwo+ ID4gKwkJCQljb250aW51ZTsKPiA+ICsJCQl9Cj4gPiArCj4gPiArCQkJLyoKPiA+ICsJCQkgKiBh bGxvY2F0ZSB0aGUgc3BhY2UgYmV0d2VlbiB0aGUgY3VycmVudCBzdGFydGluZwo+ID4gKwkJCSAq IGFkZHJlc3MgYW5kIHRoZSBmb2xsb3dpbmcgcmVzZXJ2ZWQgYmxvY2sKPiA+ICsJCQkgKi8KPiA+ ICsJCQljdXJfc2l6ZSA9IHJzdGFydCAtIGN1cl9zdGFydDsKPiA+ICsKPiA+ICsJCQlkZXZfZGJn KCZwZGV2LT5kZXYsICJhZGRpbmcgY2h1bmsgMHgleC0weCV4XG4iLAo+ID4gKwkJCQkgY3VyX3N0 YXJ0LCBjdXJfc3RhcnQgKyBjdXJfc2l6ZSk7Cj4gPiArCQkJcmV0ID0gZ2VuX3Bvb2xfYWRkX3Zp cnQoc3JhbS0+cG9vbCwKPiA+ICsJCQkJCSh1bnNpZ25lZCBsb25nKXZpcnRfYmFzZSArIGN1cl9z dGFydCwKPiA+ICsJCQkJCXJlcy0+c3RhcnQgKyBjdXJfc3RhcnQsIGN1cl9zaXplLCAtMSk7Cj4g PiArCQkJaWYgKHJldCA8IDApIHsKPiA+ICsJCQkJaWYgKHNyYW0tPmNsaykKPiA+ICsJCQkJCWNs a19kaXNhYmxlX3VucHJlcGFyZShzcmFtLT5jbGspOwo+IAo+IGFuZCBoZXJlLgo+IAo+ID4gKwkJ CQlyZXR1cm4gcmV0Owo+ID4gKwkJCX0KPiA+ICsKPiA+ICsJCQkvKiBuZXh0IGFsbG9jYXRpb24g YWZ0ZXIgdGhpcyByZXNlcnZlZCBibG9jayAqLwo+ID4gKwkJCWN1cl9zdGFydCA9IHJzdGFydCAr IHJzaXplOwo+ID4gKwkJfQo+ID4gKwo+ID4gKwkJLyogYWxsb2NhdGUgdGhlIHNwYWNlIGFmdGVy IHRoZSBsYXN0IHJlc2VydmVkIGJsb2NrICovCj4gPiArCQlpZiAoY3VyX3N0YXJ0IDwgc2l6ZSkg ewo+ID4gKwkJCWN1cl9zaXplID0gc2l6ZSAtIGN1cl9zdGFydDsKPiA+ICsKPiA+ICsJCQlkZXZf ZGJnKCZwZGV2LT5kZXYsICJhZGRpbmcgY2h1bmsgMHgleC0weCV4XG4iLAo+ID4gKwkJCQkgY3Vy X3N0YXJ0LCBjdXJfc3RhcnQgKyBjdXJfc2l6ZSk7Cj4gPiArCQkJcmV0ID0gZ2VuX3Bvb2xfYWRk X3ZpcnQoc3JhbS0+cG9vbCwKPiA+ICsJCQkJCSh1bnNpZ25lZCBsb25nKXZpcnRfYmFzZSArIGN1 cl9zdGFydCwKPiA+ICsJCQkJCXJlcy0+c3RhcnQgKyBjdXJfc3RhcnQsIGN1cl9zaXplLCAtMSk7 Cj4gPiArCQl9Cj4gPiArCj4gPiAKPiA+ICAJfQo+ID4gIAkKPiA+ICAJcGxhdGZvcm1fc2V0X2Ry dmRhdGEocGRldiwgc3JhbSk7Cj4gCj4gQWxzbywgSSB0aGluayB5b3UgY291bGQgcmVkdWNlIHRo ZSBkdXBsaWNhdGlvbiBvZiBnZW5fcG9vbF9hZGRfdmlydCgpCj4gZnVuY3Rpb24gY2FsbHMsIHNv bWVob3cgbGlrZSB0aGlzOgo+IAo+IAl1bnNpZ25lZCBpbnQgY3VyX3N0YXJ0ID0gMDsKPiAJdW5z aWduZWQgaW50IGN1cl9zaXplOwo+IAl1bnNpZ25lZCBpbnQgcnN0YXJ0Owo+IAl1bnNpZ25lZCBp bnQgcnNpemU7Cj4gCWludCBpID0gMDsKPiAKPiAJaWYgKCFyZXNlcnZlZF9saXN0KQo+IAkJcmVz ZXJ2ZWRfc2l6ZSA9IDA7Cj4gCj4gCWZvciAoaSA9IDA7IGkgPCAocmVzZXJ2ZWRfc2l6ZSArIDIp OyBpICs9IDIpIHsKPiAJCWlmIChpIDwgcmVzZXJ2ZWRfc2l6ZSkgewo+IAkJCS8qIGdldCB0aGUg bmV4dCByZXNlcnZlZCBibG9jayAqLwo+IAkJCXJzdGFydCA9IGJlMzJfdG9fY3B1KCpyZXNlcnZl ZF9saXN0KyspOwo+IAkJCXJzaXplID0gYmUzMl90b19jcHUoKnJlc2VydmVkX2xpc3QrKyk7Cj4g Cj4gCQkJLyogY2F0Y2ggdW5zb3J0ZWQgbGlzdCBlbnRyaWVzICovCj4gCQkJaWYgKHJzdGFydCA8 IGN1cl9zdGFydCkgewo+IAkJCQlkZXZfZXJyKCZwZGV2LT5kZXYsCj4gCQkJCQkidW5zb3J0ZWQg cmVzZXJ2ZWQgbGlzdCAoMHgleCBiZWZvcmUgY3VycmVudCAweCV4KVxuIiwKPiAJCQkJCXJzdGFy dCwgY3VyX3N0YXJ0KTsKPiAJCQkJcmV0dXJuIC1FSU5WQUw7Cj4gCQkJfQo+IAo+IAkJCWRldl9k YmcoJnBkZXYtPmRldiwKPiAJCQkJImZvdW5kIHJlc2VydmVkIGJsb2NrIDB4JXgtMHgleFxuIiwK PiAJCQkJcnN0YXJ0LCByc3RhcnQgKyByc2l6ZSk7Cj4gCQl9IGVsc2Ugewo+IAkJCS8qIHRoZSBs YXN0IGNodW5rIGV4dGVuZHMgdG8gdGhlIGVuZCBvZiB0aGUgcmVnaW9uICovCj4gCQkJcnN0YXJ0 ID0gc2l6ZTsKPiAJCX0KPiAKPiAJCS8qIGN1cnJlbnQgc3RhcnQgaXMgaW4gYSByZXNlcnZlZCBi bG9jayAqLwo+IAkJaWYgKHJzdGFydCA8PSBjdXJfc3RhcnQpIHsKPiAJCQljdXJfc3RhcnQgPSBy c3RhcnQgKyByc2l6ZTsKPiAJCQljb250aW51ZTsKPiAJCX0KPiAKPiAJCS8qCj4gCQkgKiBhbGxv Y2F0ZSB0aGUgc3BhY2UgYmV0d2VlbiB0aGUgY3VycmVudCBzdGFydGluZwo+IAkJICogYWRkcmVz cyBhbmQgdGhlIGZvbGxvd2luZyByZXNlcnZlZCBibG9jaywgb3IgdGhlCj4gCQkgKiBlbmQgb2Yg dGhlIHJlZ2lvbi4KPiAJCSAqLwo+IAkJY3VyX3NpemUgPSByc3RhcnQgLSBjdXJfc3RhcnQ7Cj4g Cj4gCQlkZXZfZGJnKCZwZGV2LT5kZXYsICJhZGRpbmcgY2h1bmsgMHgleC0weCV4XG4iLAo+IAkJ CWN1cl9zdGFydCwgY3VyX3N0YXJ0ICsgY3VyX3NpemUpOwo+IAkJcmV0ID0gZ2VuX3Bvb2xfYWRk X3ZpcnQoc3JhbS0+cG9vbCwKPiAJCQkJKHVuc2lnbmVkIGxvbmcpdmlydF9iYXNlICsgY3VyX3N0 YXJ0LAo+IAkJCQlyZXMtPnN0YXJ0ICsgY3VyX3N0YXJ0LCBjdXJfc2l6ZSwgLTEpOwo+IAkJaWYg KHJldCA8IDApCj4gCQkJcmV0dXJuIHJldDsKPiAJfQoKeWVwLCB0aGlzIGxvb2tzIG5pY2VyIC0g c2FtZSBmb3IgbW92aW5nIHRoZSBjbGtfcHJlcGFyZV9lbmFibGUgdG8gYmVsb3cgdGhpcyAKbG9v cCB0byB1bmNsdXR0ZXIgdGhlIGVycm9yLXBhdGguCgpTbyBJIHdpbGwgaW5jb3Jwb3JhdGUgdGhp cyBpbiB2My4KCgpUaGFua3MKSGVpa28KX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX18KZGV2aWNldHJlZS1kaXNjdXNzIG1haWxpbmcgbGlzdApkZXZpY2V0cmVl LWRpc2N1c3NAbGlzdHMub3psYWJzLm9yZwpodHRwczovL2xpc3RzLm96bGFicy5vcmcvbGlzdGlu Zm8vZGV2aWNldHJlZS1kaXNjdXNzCg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752533Ab3FZJTR (ORCPT ); Wed, 26 Jun 2013 05:19:17 -0400 Received: from gloria.sntech.de ([95.129.55.99]:40673 "EHLO gloria.sntech.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751212Ab3FZJTN (ORCPT ); Wed, 26 Jun 2013 05:19:13 -0400 From: Heiko =?utf-8?q?St=C3=BCbner?= To: Philipp Zabel Subject: Re: [PATCH v2 2/6] misc: sram: add ability to mark sram sections as reserved Date: Wed, 26 Jun 2013 11:18:43 +0200 User-Agent: KMail/1.13.7 (Linux/3.2.0-3-686-pae; KDE/4.8.4; i686; ; ) Cc: Arnd Bergmann , Olof Johansson , "linux-arm-kernel@lists.infradead.org" , Grant Likely , Rob Herring , devicetree-discuss@lists.ozlabs.org, Russell King , linux-kernel@vger.kernel.org, "Greg Kroah-Hartman" References: <201306251046.04873.heiko@sntech.de> <201306251047.12899.heiko@sntech.de> <1372155425.3981.67.camel@pizza.hi.pengutronix.de> In-Reply-To: <1372155425.3981.67.camel@pizza.hi.pengutronix.de> MIME-Version: 1.0 Content-Type: Text/Plain; charset="utf-8" Content-Transfer-Encoding: 8bit Message-Id: <201306261118.44637.heiko@sntech.de> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Philipp, Am Dienstag, 25. Juni 2013, 12:17:05 schrieb Philipp Zabel: > Hi Heiko, > > Am Dienstag, den 25.06.2013, 10:47 +0200 schrieb Heiko Stübner: > > Some SoCs need parts of their sram for special purposes. So while being > > part of the periphal, it should not be part of the genpool controlling > > the sram. > > > > Threfore add an option mmio-sram-reserved to keep arbitary portions of > > the sram from being part of the pool. > > > > Suggested-by: Rob Herring > > Signed-off-by: Heiko Stuebner > > --- > > > > Documentation/devicetree/bindings/misc/sram.txt | 8 +++ > > drivers/misc/sram.c | 86 > > +++++++++++++++++++++-- 2 files changed, 88 insertions(+), 6 > > deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/misc/sram.txt > > b/Documentation/devicetree/bindings/misc/sram.txt index 4d0a00e..eae080e > > 100644 > > --- a/Documentation/devicetree/bindings/misc/sram.txt > > +++ b/Documentation/devicetree/bindings/misc/sram.txt > > > > @@ -8,9 +8,17 @@ Required properties: > > - reg : SRAM iomem address range > > > > +Optional properties: > > + > > +- mmio-sram-reserved: ordered list of reserved chunks inside the sram > > that + should not become part of the genalloc pool. > > + Format is , , ...; with base being relative to > > the + reg property base. > > + > > the keyword to reserve blocks of ram is /memreserve/ - should this > property name be aligned with that? The mmio-sram-reserved name was suggested by Rob Herring, who I suppose has some slight experience with devicetree :-) . I wasn't able to find real documentation on /memreserve/ but it looks more like it's used to reserve generic memregions, not being node-specific. So reusing this might also cause confusion when the reserve-data now is relative to it's node reg. > > Example: > > > > sram: sram@5c000000 { > > > > compatible = "mmio-sram"; > > reg = <0x5c000000 0x40000>; /* 256 KiB SRAM at address 0x5c000000 */ > > > > + mmio-sram-reserved = <0x0 0x100>; /* reserve 0x5c000000-0x5c000100 */ > > > > }; > > > > diff --git a/drivers/misc/sram.c b/drivers/misc/sram.c > > index afe66571..5fccbe3 100644 > > --- a/drivers/misc/sram.c > > +++ b/drivers/misc/sram.c > > @@ -42,6 +42,8 @@ static int sram_probe(struct platform_device *pdev) > > > > struct sram_dev *sram; > > struct resource *res; > > unsigned long size; > > > > + const __be32 *reserved_list = NULL; > > + int reserved_size = 0; > > > > int ret; > > > > res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > > > > @@ -65,12 +67,89 @@ static int sram_probe(struct platform_device *pdev) > > > > if (!sram->pool) > > > > return -ENOMEM; > > > > - ret = gen_pool_add_virt(sram->pool, (unsigned long)virt_base, > > - res->start, size, -1); > > - if (ret < 0) { > > - if (sram->clk) > > - clk_disable_unprepare(sram->clk); > > - return ret; > > + if (pdev->dev.of_node) { > > + reserved_list = of_get_property(pdev->dev.of_node, > > + "mmio-sram-reserved", > > + &reserved_size); > > + if (reserved_list) { > > + reserved_size /= sizeof(*reserved_list); > > + if (!reserved_size || reserved_size % 2) { > > + dev_warn(&pdev->dev, "wrong number of arguments in > > mmio-sram-reserved\n"); + reserved_list = NULL; > > + } > > + } > > + } > > + > > + if (!reserved_list) { > > + ret = gen_pool_add_virt(sram->pool, (unsigned long)virt_base, > > + res->start, size, -1); > > + if (ret < 0) { > > + if (sram->clk) > > + clk_disable_unprepare(sram->clk); > > + return ret; > > + } > > Moving the clk_prepare_enable() further down would allow to avoid the > clk_disable_unprepare() in every error path, > > > + } else { > > + unsigned int cur_start = 0; > > + unsigned int cur_size; > > + unsigned int rstart; > > + unsigned int rsize; > > + int i; > > + > > + for (i = 0; i < reserved_size; i += 2) { > > + /* get the next reserved block */ > > + rstart = be32_to_cpu(*reserved_list++); > > + rsize = be32_to_cpu(*reserved_list++); > > + > > + /* catch unsorted list entries */ > > + if (rstart < cur_start) { > > + dev_err(&pdev->dev, "unsorted reserved list (0x%x before current > > 0x%x)\n", + rstart, cur_start); > > + if (sram->clk) > > + clk_disable_unprepare(sram->clk); > > like here > > > + return -EINVAL; > > + } > > + > > + dev_dbg(&pdev->dev, "found reserved block 0x%x-0x%x\n", > > + rstart, rstart + rsize); > > + > > + /* current start is in a reserved block */ > > + if (rstart <= cur_start) { > > + cur_start = rstart + rsize; > > + continue; > > + } > > + > > + /* > > + * allocate the space between the current starting > > + * address and the following reserved block > > + */ > > + cur_size = rstart - cur_start; > > + > > + dev_dbg(&pdev->dev, "adding chunk 0x%x-0x%x\n", > > + cur_start, cur_start + cur_size); > > + ret = gen_pool_add_virt(sram->pool, > > + (unsigned long)virt_base + cur_start, > > + res->start + cur_start, cur_size, -1); > > + if (ret < 0) { > > + if (sram->clk) > > + clk_disable_unprepare(sram->clk); > > and here. > > > + return ret; > > + } > > + > > + /* next allocation after this reserved block */ > > + cur_start = rstart + rsize; > > + } > > + > > + /* allocate the space after the last reserved block */ > > + if (cur_start < size) { > > + cur_size = size - cur_start; > > + > > + dev_dbg(&pdev->dev, "adding chunk 0x%x-0x%x\n", > > + cur_start, cur_start + cur_size); > > + ret = gen_pool_add_virt(sram->pool, > > + (unsigned long)virt_base + cur_start, > > + res->start + cur_start, cur_size, -1); > > + } > > + > > > > } > > > > platform_set_drvdata(pdev, sram); > > Also, I think you could reduce the duplication of gen_pool_add_virt() > function calls, somehow like this: > > unsigned int cur_start = 0; > unsigned int cur_size; > unsigned int rstart; > unsigned int rsize; > int i = 0; > > if (!reserved_list) > reserved_size = 0; > > for (i = 0; i < (reserved_size + 2); i += 2) { > if (i < reserved_size) { > /* get the next reserved block */ > rstart = be32_to_cpu(*reserved_list++); > rsize = be32_to_cpu(*reserved_list++); > > /* catch unsorted list entries */ > if (rstart < cur_start) { > dev_err(&pdev->dev, > "unsorted reserved list (0x%x before current 0x%x)\n", > rstart, cur_start); > return -EINVAL; > } > > dev_dbg(&pdev->dev, > "found reserved block 0x%x-0x%x\n", > rstart, rstart + rsize); > } else { > /* the last chunk extends to the end of the region */ > rstart = size; > } > > /* current start is in a reserved block */ > if (rstart <= cur_start) { > cur_start = rstart + rsize; > continue; > } > > /* > * allocate the space between the current starting > * address and the following reserved block, or the > * end of the region. > */ > cur_size = rstart - cur_start; > > dev_dbg(&pdev->dev, "adding chunk 0x%x-0x%x\n", > cur_start, cur_start + cur_size); > ret = gen_pool_add_virt(sram->pool, > (unsigned long)virt_base + cur_start, > res->start + cur_start, cur_size, -1); > if (ret < 0) > return ret; > } yep, this looks nicer - same for moving the clk_prepare_enable to below this loop to unclutter the error-path. So I will incorporate this in v3. Thanks Heiko