From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eugeniy.Paltsev@synopsys.com (Eugeniy Paltsev) Date: Fri, 25 Aug 2017 15:57:43 +0000 Subject: [PATCH 1/3 v8] ARC: Set IO-coherency aperture base to LINUX_LINK_BASE In-Reply-To: <5184d3eb-31bc-c456-3e65-6137a3ba72f7@synopsys.com> References: <20170712094023.23226-1-Eugeniy.Paltsev@synopsys.com> <20170712094023.23226-2-Eugeniy.Paltsev@synopsys.com> <5184d3eb-31bc-c456-3e65-6137a3ba72f7@synopsys.com> List-ID: Message-ID: <1503676662.15555.8.camel@synopsys.com> To: linux-snps-arc@lists.infradead.org On Tue, 2017-08-22@14:44 -0700, Vineet Gupta wrote: > On 07/12/2017 02:40 AM, Eugeniy Paltsev wrote: > > Most of the time we indeed use the one and only LINUX_LINK_BASE > > set to 0x8000_0000. But there might be good reasons to move > > the kernel to another location like 0x9z etc. > > And we want IOC aperture to cover entire area used by the kernel, > > so let's make its base matching link base > > How about something below.... > > Currently IOC aperture base is hardcoded to 0x8000_0000 which may not > be true for? > non default values of CONFIG_LINUX_LINK_BASE, so use the config value > > > and add required asserts: > > checking IOC aperture base address and size to be supported by IOC. > > .... And while at it, also add the required asserts expected by the > IOC? > programming model. > > > > > Signed-off-by: Eugeniy Paltsev > > --- > > ? arch/arc/mm/cache.c | 33 ++++++++++++++++++++++++--------- > > ? 1 file changed, 24 insertions(+), 9 deletions(-) > > > > diff --git a/arch/arc/mm/cache.c b/arch/arc/mm/cache.c > > index a867575..383ff77 100644 > > --- a/arch/arc/mm/cache.c > > +++ b/arch/arc/mm/cache.c > > @@ -1083,7 +1083,8 @@ SYSCALL_DEFINE3(cacheflush, uint32_t, start, > > uint32_t, sz, uint32_t, flags) > > ???*/ > > ? noinline void __init arc_ioc_setup(void) > > ? { > > - unsigned int ap_sz; > > + unsigned int ap_base; > > + long ap_size; > > Is there a reason they are different types ? > Also the way you use it below, best to call it mem_size ! Ok, I used?long for ap_size as it simply populated by?arc_get_mem_sz ------------->8---------- ap_size = arc_get_mem_sz(); ------------->8---------- and arc_get_mem_sz return long. Probably I should fix arc_get_mem_sz because it simply return low_mem_sz which is unsigned long! ------------->8---------- static unsigned long low_mem_sz; long __init arc_get_mem_sz(void) { return low_mem_sz; } ------------->8---------- > > ?? > > ?? /* Flush + invalidate + disable L1 dcache */ > > ?? __dc_disable(); > > @@ -1092,18 +1093,32 @@ noinline void __init arc_ioc_setup(void) > > ?? if (read_aux_reg(ARC_REG_SLC_BCR)) > > ?? slc_entire_op(OP_FLUSH_N_INV); > > ?? > > - /* IOC Aperture start: TDB: handle non default > > CONFIG_LINUX_LINK_BASE */ > > - write_aux_reg(ARC_REG_IO_COH_AP0_BASE, 0x80000); > > - > > ?? /* > > - ?* IOC Aperture size: > > - ?*???decoded as 2 ^ (SIZE + 2) KB: so setting 0x11 implies > > 512M > > - ?* TBD: fix for PGU + 1GB of low mem > > + ?* IOC Aperture size is equal to memory size. > > + ?* TBD: fix for PGU + 1GiB of low mem > > Not really averse to this per-se, but conventionally we've not used > KiB or GiB? > etc, so I'd prefer KB, GB... just for consistency and ability to grep > correctly. But we also use KiB, MiB, GiB for ARC: $ grep -r -I -e "MiB" -e "GiB" arch/arc/ reg = <0x0 0x80000000 0x0 0x20000000 /* 512 MiB low mem */ 0x1 0xc0000000 0x0 0x40000000>; /* 1 GiB highmem */ reg = <0x0 0x80000000 0x0 0x20000000 /* 512 MiB low mem */ 0x1 0xc0000000 0x0 0x40000000>; /* 1 GiB highmem */ reg = <0x0 0x80000000 0x0 0x1b000000>; /* (512 - 32) MiB */ reg = <0x80000000 0x20000000>; /* 512MiB */ reg = <0x80000000 0x20000000>; /* 512MiB */ .......... > > ?? ?* TBD: fix for PAE > > ?? ?*/ > > - ap_sz = order_base_2(arc_get_mem_sz()/1024) - 2; > > - write_aux_reg(ARC_REG_IO_COH_AP0_SIZE, ap_sz); > > + ap_size = arc_get_mem_sz(); > > + > > + if (!is_power_of_2(ap_size) || ap_size < 4096) > > + panic("IOC Aperture size must be power of 2 larger > > than 4KiB"); > > + > > + /* > > + ?* IOC Aperture size decoded as 2 ^ (SIZE + 2) KiB, > > + ?* so setting 0x11 implies 512MiB, 0x12 implies 1G... > > + ?*/ > > + write_aux_reg(ARC_REG_IO_COH_AP0_SIZE, > > order_base_2(ap_size / 1024) - 2); > > for (ap_size / 1024) can you use (ap_size >> 10) I absolutely sure what compiler will implement this division? ap_size/1024 as right shift by itself. So lets don't make things look more complicated?than they are :) > > + > > + /* > > + ?* For now we assume IOC aperture to cover all the memory > > used by the > > + ?* kernel. > > + ?*/ > > + ap_base = CONFIG_LINUX_LINK_BASE; > > + > > + if (ap_base % ap_size != 0) > > + panic("IOC Aperture start must be aligned to the > > size of the aperture"); > > This is good. > > > ?? > > + write_aux_reg(ARC_REG_IO_COH_AP0_BASE, ap_base >> 12); > > ?? write_aux_reg(ARC_REG_IO_COH_PARTIAL, 1); > > ?? write_aux_reg(ARC_REG_IO_COH_ENABLE, 1); > > ?? > > > > -- ?Eugeniy Paltsev From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eugeniy Paltsev Subject: Re: [PATCH 1/3 v8] ARC: Set IO-coherency aperture base to LINUX_LINK_BASE Date: Fri, 25 Aug 2017 15:57:43 +0000 Message-ID: <1503676662.15555.8.camel@synopsys.com> References: <20170712094023.23226-1-Eugeniy.Paltsev@synopsys.com> <20170712094023.23226-2-Eugeniy.Paltsev@synopsys.com> <5184d3eb-31bc-c456-3e65-6137a3ba72f7@synopsys.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <5184d3eb-31bc-c456-3e65-6137a3ba72f7-HKixBCOQz3hWk0Htik3J/w@public.gmane.org> Content-Language: en-US Content-ID: <6521BCF736599944ABCA91AEAE720997-z7JfP6tgrtVBCHUSTMH8dZqQE7yCjDx5@public.gmane.org> Sender: devicetree-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: "linux-snps-arc-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org" Cc: "linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , "Vineet.Gupta1-HKixBCOQz3hWk0Htik3J/w@public.gmane.org" , "Alexey.Brodkin-HKixBCOQz3hWk0Htik3J/w@public.gmane.org" , "robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org" , "devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" List-Id: devicetree@vger.kernel.org T24gVHVlLCAyMDE3LTA4LTIyIGF0IDE0OjQ0IC0wNzAwLCBWaW5lZXQgR3VwdGEgd3JvdGU6DQo+ IE9uIDA3LzEyLzIwMTcgMDI6NDAgQU0sIEV1Z2VuaXkgUGFsdHNldiB3cm90ZToNCj4gPiBNb3N0 IG9mIHRoZSB0aW1lIHdlIGluZGVlZCB1c2UgdGhlIG9uZSBhbmQgb25seSBMSU5VWF9MSU5LX0JB U0UNCj4gPiBzZXQgdG8gMHg4MDAwXzAwMDAuIEJ1dCB0aGVyZSBtaWdodCBiZSBnb29kIHJlYXNv bnMgdG8gbW92ZQ0KPiA+IHRoZSBrZXJuZWwgdG8gYW5vdGhlciBsb2NhdGlvbiBsaWtlIDB4OXog ZXRjLg0KPiA+IEFuZCB3ZSB3YW50IElPQyBhcGVydHVyZSB0byBjb3ZlciBlbnRpcmUgYXJlYSB1 c2VkIGJ5IHRoZSBrZXJuZWwsDQo+ID4gc28gbGV0J3MgbWFrZSBpdHMgYmFzZSBtYXRjaGluZyBs aW5rIGJhc2UNCj4gDQo+IEhvdyBhYm91dCBzb21ldGhpbmcgYmVsb3cuLi4uDQo+IA0KPiBDdXJy ZW50bHkgSU9DIGFwZXJ0dXJlIGJhc2UgaXMgaGFyZGNvZGVkIHRvIDB4ODAwMF8wMDAwIHdoaWNo IG1heSBub3QNCj4gYmUgdHJ1ZSBmb3LCoA0KPiBub24gZGVmYXVsdCB2YWx1ZXMgb2YgQ09ORklH X0xJTlVYX0xJTktfQkFTRSwgc28gdXNlIHRoZSBjb25maWcgdmFsdWUNCj4gDQo+ID4gYW5kIGFk ZCByZXF1aXJlZCBhc3NlcnRzOg0KPiA+IGNoZWNraW5nIElPQyBhcGVydHVyZSBiYXNlIGFkZHJl c3MgYW5kIHNpemUgdG8gYmUgc3VwcG9ydGVkIGJ5IElPQy4NCj4gDQo+IC4uLi4gQW5kIHdoaWxl IGF0IGl0LCBhbHNvIGFkZCB0aGUgcmVxdWlyZWQgYXNzZXJ0cyBleHBlY3RlZCBieSB0aGUNCj4g SU9DwqANCj4gcHJvZ3JhbW1pbmcgbW9kZWwuDQo+IA0KPiA+IA0KPiA+IFNpZ25lZC1vZmYtYnk6 IEV1Z2VuaXkgUGFsdHNldiA8RXVnZW5peS5QYWx0c2V2QHN5bm9wc3lzLmNvbT4NCj4gPiAtLS0N Cj4gPiDCoCBhcmNoL2FyYy9tbS9jYWNoZS5jIHwgMzMgKysrKysrKysrKysrKysrKysrKysrKysr LS0tLS0tLS0tDQo+ID4gwqAgMSBmaWxlIGNoYW5nZWQsIDI0IGluc2VydGlvbnMoKyksIDkgZGVs ZXRpb25zKC0pDQo+ID4gDQo+ID4gZGlmZiAtLWdpdCBhL2FyY2gvYXJjL21tL2NhY2hlLmMgYi9h cmNoL2FyYy9tbS9jYWNoZS5jDQo+ID4gaW5kZXggYTg2NzU3NS4uMzgzZmY3NyAxMDA2NDQNCj4g PiAtLS0gYS9hcmNoL2FyYy9tbS9jYWNoZS5jDQo+ID4gKysrIGIvYXJjaC9hcmMvbW0vY2FjaGUu Yw0KPiA+IEBAIC0xMDgzLDcgKzEwODMsOCBAQCBTWVNDQUxMX0RFRklORTMoY2FjaGVmbHVzaCwg dWludDMyX3QsIHN0YXJ0LA0KPiA+IHVpbnQzMl90LCBzeiwgdWludDMyX3QsIGZsYWdzKQ0KPiA+ IMKgwqDCoCovDQo+ID4gwqAgbm9pbmxpbmUgdm9pZCBfX2luaXQgYXJjX2lvY19zZXR1cCh2b2lk KQ0KPiA+IMKgIHsNCj4gPiAtCXVuc2lnbmVkIGludCBhcF9zejsNCj4gPiArCXVuc2lnbmVkIGlu dCBhcF9iYXNlOw0KPiA+ICsJbG9uZyBhcF9zaXplOw0KPiANCj4gSXMgdGhlcmUgYSByZWFzb24g dGhleSBhcmUgZGlmZmVyZW50IHR5cGVzID8NCj4gQWxzbyB0aGUgd2F5IHlvdSB1c2UgaXQgYmVs b3csIGJlc3QgdG8gY2FsbCBpdCBtZW1fc2l6ZSAhDQoNCk9rLCBJIHVzZWTCoGxvbmcgZm9yIGFw X3NpemUgYXMgaXQgc2ltcGx5IHBvcHVsYXRlZCBiecKgYXJjX2dldF9tZW1fc3oNCi0tLS0tLS0t LS0tLS0+OC0tLS0tLS0tLS0NCmFwX3NpemUgPSBhcmNfZ2V0X21lbV9zeigpOw0KLS0tLS0tLS0t LS0tLT44LS0tLS0tLS0tLQ0KYW5kIGFyY19nZXRfbWVtX3N6IHJldHVybiBsb25nLg0KDQoNClBy b2JhYmx5IEkgc2hvdWxkIGZpeCBhcmNfZ2V0X21lbV9zeiBiZWNhdXNlIGl0IHNpbXBseSByZXR1 cm4NCmxvd19tZW1fc3ogd2hpY2ggaXMgdW5zaWduZWQgbG9uZyENCg0KLS0tLS0tLS0tLS0tLT44 LS0tLS0tLS0tLQ0Kc3RhdGljIHVuc2lnbmVkIGxvbmcgbG93X21lbV9zejsNCmxvbmcgX19pbml0 IGFyY19nZXRfbWVtX3N6KHZvaWQpDQp7DQoJcmV0dXJuIGxvd19tZW1fc3o7DQp9DQotLS0tLS0t LS0tLS0tPjgtLS0tLS0tLS0tDQoNCj4gPiDCoMKgDQo+ID4gwqDCoAkvKiBGbHVzaCArIGludmFs aWRhdGUgKyBkaXNhYmxlIEwxIGRjYWNoZSAqLw0KPiA+IMKgwqAJX19kY19kaXNhYmxlKCk7DQo+ ID4gQEAgLTEwOTIsMTggKzEwOTMsMzIgQEAgbm9pbmxpbmUgdm9pZCBfX2luaXQgYXJjX2lvY19z ZXR1cCh2b2lkKQ0KPiA+IMKgwqAJaWYgKHJlYWRfYXV4X3JlZyhBUkNfUkVHX1NMQ19CQ1IpKQ0K PiA+IMKgwqAJCXNsY19lbnRpcmVfb3AoT1BfRkxVU0hfTl9JTlYpOw0KPiA+IMKgwqANCj4gPiAt CS8qIElPQyBBcGVydHVyZSBzdGFydDogVERCOiBoYW5kbGUgbm9uIGRlZmF1bHQNCj4gPiBDT05G SUdfTElOVVhfTElOS19CQVNFICovDQo+ID4gLQl3cml0ZV9hdXhfcmVnKEFSQ19SRUdfSU9fQ09I X0FQMF9CQVNFLCAweDgwMDAwKTsNCj4gPiAtDQo+ID4gwqDCoAkvKg0KPiA+IC0JwqAqIElPQyBB cGVydHVyZSBzaXplOg0KPiA+IC0JwqAqwqDCoMKgZGVjb2RlZCBhcyAyIF4gKFNJWkUgKyAyKSBL Qjogc28gc2V0dGluZyAweDExIGltcGxpZXMNCj4gPiA1MTJNDQo+ID4gLQnCoCogVEJEOiBmaXgg Zm9yIFBHVSArIDFHQiBvZiBsb3cgbWVtDQo+ID4gKwnCoCogSU9DIEFwZXJ0dXJlIHNpemUgaXMg ZXF1YWwgdG8gbWVtb3J5IHNpemUuDQo+ID4gKwnCoCogVEJEOiBmaXggZm9yIFBHVSArIDFHaUIg b2YgbG93IG1lbQ0KPiANCj4gTm90IHJlYWxseSBhdmVyc2UgdG8gdGhpcyBwZXItc2UsIGJ1dCBj b252ZW50aW9uYWxseSB3ZSd2ZSBub3QgdXNlZA0KPiBLaUIgb3IgR2lCwqANCj4gZXRjLCBzbyBJ J2QgcHJlZmVyIEtCLCBHQi4uLiBqdXN0IGZvciBjb25zaXN0ZW5jeSBhbmQgYWJpbGl0eSB0byBn cmVwDQo+IGNvcnJlY3RseS4NCg0KQnV0IHdlIGFsc28gdXNlIEtpQiwgTWlCLCBHaUIgZm9yIEFS QzoNCiQgZ3JlcCAtciAtSSAtZSAiTWlCIiAtZSAiR2lCIiBhcmNoL2FyYy8NCnJlZyA9IDwweDAg MHg4MDAwMDAwMCAweDAgMHgyMDAwMDAwMAkvKiA1MTIgTWlCIGxvdyBtZW0gKi8NCjB4MSAweGMw MDAwMDAwIDB4MCAweDQwMDAwMDAwPjsJLyogMSBHaUIgaGlnaG1lbSAqLw0KcmVnID0gPDB4MCAw eDgwMDAwMDAwIDB4MCAweDIwMDAwMDAwCS8qIDUxMiBNaUIgbG93IG1lbSAqLw0KMHgxIDB4YzAw MDAwMDAgMHgwIDB4NDAwMDAwMDA+OwkvKiAxIEdpQiBoaWdobWVtICovDQpyZWcgPSA8MHgwIDB4 ODAwMDAwMDAgMHgwIDB4MWIwMDAwMDA+OwkvKiAoNTEyIC0gMzIpIE1pQiAqLw0KcmVnID0gPDB4 ODAwMDAwMDAgMHgyMDAwMDAwMD47CS8qIDUxMk1pQiAqLw0KcmVnID0gPDB4ODAwMDAwMDAgMHgy MDAwMDAwMD47CS8qIDUxMk1pQiAqLw0KLi4uLi4uLi4uLg0KDQoNCj4gPiDCoMKgCcKgKiBUQkQ6 IGZpeCBmb3IgUEFFDQo+ID4gwqDCoAnCoCovDQo+ID4gLQlhcF9zeiA9IG9yZGVyX2Jhc2VfMihh cmNfZ2V0X21lbV9zeigpLzEwMjQpIC0gMjsNCj4gPiAtCXdyaXRlX2F1eF9yZWcoQVJDX1JFR19J T19DT0hfQVAwX1NJWkUsIGFwX3N6KTsNCj4gPiArCWFwX3NpemUgPSBhcmNfZ2V0X21lbV9zeigp Ow0KPiA+ICsNCj4gPiArCWlmICghaXNfcG93ZXJfb2ZfMihhcF9zaXplKSB8fCBhcF9zaXplIDwg NDA5NikNCj4gPiArCQlwYW5pYygiSU9DIEFwZXJ0dXJlIHNpemUgbXVzdCBiZSBwb3dlciBvZiAy IGxhcmdlcg0KPiA+IHRoYW4gNEtpQiIpOw0KPiA+ICsNCj4gPiArCS8qDQo+ID4gKwnCoCogSU9D IEFwZXJ0dXJlIHNpemUgZGVjb2RlZCBhcyAyIF4gKFNJWkUgKyAyKSBLaUIsDQo+ID4gKwnCoCog c28gc2V0dGluZyAweDExIGltcGxpZXMgNTEyTWlCLCAweDEyIGltcGxpZXMgMUcuLi4NCj4gPiAr CcKgKi8NCj4gPiArCXdyaXRlX2F1eF9yZWcoQVJDX1JFR19JT19DT0hfQVAwX1NJWkUsDQo+ID4g b3JkZXJfYmFzZV8yKGFwX3NpemUgLyAxMDI0KSAtIDIpOw0KPiANCj4gZm9yIChhcF9zaXplIC8g MTAyNCkgY2FuIHlvdSB1c2UgKGFwX3NpemUgPj4gMTApDQoNCkkgYWJzb2x1dGVseSBzdXJlIHdo YXQgY29tcGlsZXIgd2lsbCBpbXBsZW1lbnQgdGhpcyBkaXZpc2lvbsKgDQphcF9zaXplLzEwMjQg YXMgcmlnaHQgc2hpZnQgYnkgaXRzZWxmLg0KDQpTbyBsZXRzIGRvbid0IG1ha2UgdGhpbmdzIGxv b2sgbW9yZSBjb21wbGljYXRlZMKgdGhhbiB0aGV5IGFyZSA6KQ0KDQo+ID4gKw0KPiA+ICsJLyoN Cj4gPiArCcKgKiBGb3Igbm93IHdlIGFzc3VtZSBJT0MgYXBlcnR1cmUgdG8gY292ZXIgYWxsIHRo ZSBtZW1vcnkNCj4gPiB1c2VkIGJ5IHRoZQ0KPiA+ICsJwqAqIGtlcm5lbC4NCj4gPiArCcKgKi8N Cj4gPiArCWFwX2Jhc2UgPSBDT05GSUdfTElOVVhfTElOS19CQVNFOw0KPiA+ICsNCj4gPiArCWlm IChhcF9iYXNlICUgYXBfc2l6ZSAhPSAwKQ0KPiA+ICsJCXBhbmljKCJJT0MgQXBlcnR1cmUgc3Rh cnQgbXVzdCBiZSBhbGlnbmVkIHRvIHRoZQ0KPiA+IHNpemUgb2YgdGhlIGFwZXJ0dXJlIik7DQo+ IA0KPiBUaGlzIGlzIGdvb2QuDQo+IA0KPiA+IMKgwqANCj4gPiArCXdyaXRlX2F1eF9yZWcoQVJD X1JFR19JT19DT0hfQVAwX0JBU0UsIGFwX2Jhc2UgPj4gMTIpOw0KPiA+IMKgwqAJd3JpdGVfYXV4 X3JlZyhBUkNfUkVHX0lPX0NPSF9QQVJUSUFMLCAxKTsNCj4gPiDCoMKgCXdyaXRlX2F1eF9yZWco QVJDX1JFR19JT19DT0hfRU5BQkxFLCAxKTsNCj4gPiDCoMKgDQo+ID4gDQo+IA0KPiANCi0tIA0K wqBFdWdlbml5IFBhbHRzZXY= -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934026AbdHYP5t (ORCPT ); Fri, 25 Aug 2017 11:57:49 -0400 Received: from smtprelay.synopsys.com ([198.182.60.111]:33325 "EHLO smtprelay.synopsys.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933738AbdHYP5r (ORCPT ); Fri, 25 Aug 2017 11:57:47 -0400 From: Eugeniy Paltsev To: Vineet Gupta , "linux-snps-arc@lists.infradead.org" CC: "linux-kernel@vger.kernel.org" , "Vineet.Gupta1@synopsys.com" , "Alexey.Brodkin@synopsys.com" , "robh+dt@kernel.org" , "devicetree@vger.kernel.org" Subject: Re: [PATCH 1/3 v8] ARC: Set IO-coherency aperture base to LINUX_LINK_BASE Thread-Topic: [PATCH 1/3 v8] ARC: Set IO-coherency aperture base to LINUX_LINK_BASE Thread-Index: AQHS+vLoiujB7L+NHEaVm/F0deVnNqKRCDIAgARWJQA= Date: Fri, 25 Aug 2017 15:57:43 +0000 Message-ID: <1503676662.15555.8.camel@synopsys.com> References: <20170712094023.23226-1-Eugeniy.Paltsev@synopsys.com> <20170712094023.23226-2-Eugeniy.Paltsev@synopsys.com> <5184d3eb-31bc-c456-3e65-6137a3ba72f7@synopsys.com> In-Reply-To: <5184d3eb-31bc-c456-3e65-6137a3ba72f7@synopsys.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.121.8.106] Content-Type: text/plain; charset="utf-8" Content-ID: <6521BCF736599944ABCA91AEAE720997@internal.synopsys.com> MIME-Version: 1.0 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 base64 to 8bit by nfs id v7PFvrMH007221 On Tue, 2017-08-22 at 14:44 -0700, Vineet Gupta wrote: > On 07/12/2017 02:40 AM, Eugeniy Paltsev wrote: > > Most of the time we indeed use the one and only LINUX_LINK_BASE > > set to 0x8000_0000. But there might be good reasons to move > > the kernel to another location like 0x9z etc. > > And we want IOC aperture to cover entire area used by the kernel, > > so let's make its base matching link base > > How about something below.... > > Currently IOC aperture base is hardcoded to 0x8000_0000 which may not > be true for  > non default values of CONFIG_LINUX_LINK_BASE, so use the config value > > > and add required asserts: > > checking IOC aperture base address and size to be supported by IOC. > > .... And while at it, also add the required asserts expected by the > IOC  > programming model. > > > > > Signed-off-by: Eugeniy Paltsev > > --- > >   arch/arc/mm/cache.c | 33 ++++++++++++++++++++++++--------- > >   1 file changed, 24 insertions(+), 9 deletions(-) > > > > diff --git a/arch/arc/mm/cache.c b/arch/arc/mm/cache.c > > index a867575..383ff77 100644 > > --- a/arch/arc/mm/cache.c > > +++ b/arch/arc/mm/cache.c > > @@ -1083,7 +1083,8 @@ SYSCALL_DEFINE3(cacheflush, uint32_t, start, > > uint32_t, sz, uint32_t, flags) > >    */ > >   noinline void __init arc_ioc_setup(void) > >   { > > - unsigned int ap_sz; > > + unsigned int ap_base; > > + long ap_size; > > Is there a reason they are different types ? > Also the way you use it below, best to call it mem_size ! Ok, I used long for ap_size as it simply populated by arc_get_mem_sz ------------->8---------- ap_size = arc_get_mem_sz(); ------------->8---------- and arc_get_mem_sz return long. Probably I should fix arc_get_mem_sz because it simply return low_mem_sz which is unsigned long! ------------->8---------- static unsigned long low_mem_sz; long __init arc_get_mem_sz(void) { return low_mem_sz; } ------------->8---------- > >    > >    /* Flush + invalidate + disable L1 dcache */ > >    __dc_disable(); > > @@ -1092,18 +1093,32 @@ noinline void __init arc_ioc_setup(void) > >    if (read_aux_reg(ARC_REG_SLC_BCR)) > >    slc_entire_op(OP_FLUSH_N_INV); > >    > > - /* IOC Aperture start: TDB: handle non default > > CONFIG_LINUX_LINK_BASE */ > > - write_aux_reg(ARC_REG_IO_COH_AP0_BASE, 0x80000); > > - > >    /* > > -  * IOC Aperture size: > > -  *   decoded as 2 ^ (SIZE + 2) KB: so setting 0x11 implies > > 512M > > -  * TBD: fix for PGU + 1GB of low mem > > +  * IOC Aperture size is equal to memory size. > > +  * TBD: fix for PGU + 1GiB of low mem > > Not really averse to this per-se, but conventionally we've not used > KiB or GiB  > etc, so I'd prefer KB, GB... just for consistency and ability to grep > correctly. But we also use KiB, MiB, GiB for ARC: $ grep -r -I -e "MiB" -e "GiB" arch/arc/ reg = <0x0 0x80000000 0x0 0x20000000 /* 512 MiB low mem */ 0x1 0xc0000000 0x0 0x40000000>; /* 1 GiB highmem */ reg = <0x0 0x80000000 0x0 0x20000000 /* 512 MiB low mem */ 0x1 0xc0000000 0x0 0x40000000>; /* 1 GiB highmem */ reg = <0x0 0x80000000 0x0 0x1b000000>; /* (512 - 32) MiB */ reg = <0x80000000 0x20000000>; /* 512MiB */ reg = <0x80000000 0x20000000>; /* 512MiB */ .......... > >     * TBD: fix for PAE > >     */ > > - ap_sz = order_base_2(arc_get_mem_sz()/1024) - 2; > > - write_aux_reg(ARC_REG_IO_COH_AP0_SIZE, ap_sz); > > + ap_size = arc_get_mem_sz(); > > + > > + if (!is_power_of_2(ap_size) || ap_size < 4096) > > + panic("IOC Aperture size must be power of 2 larger > > than 4KiB"); > > + > > + /* > > +  * IOC Aperture size decoded as 2 ^ (SIZE + 2) KiB, > > +  * so setting 0x11 implies 512MiB, 0x12 implies 1G... > > +  */ > > + write_aux_reg(ARC_REG_IO_COH_AP0_SIZE, > > order_base_2(ap_size / 1024) - 2); > > for (ap_size / 1024) can you use (ap_size >> 10) I absolutely sure what compiler will implement this division  ap_size/1024 as right shift by itself. So lets don't make things look more complicated than they are :) > > + > > + /* > > +  * For now we assume IOC aperture to cover all the memory > > used by the > > +  * kernel. > > +  */ > > + ap_base = CONFIG_LINUX_LINK_BASE; > > + > > + if (ap_base % ap_size != 0) > > + panic("IOC Aperture start must be aligned to the > > size of the aperture"); > > This is good. > > >    > > + write_aux_reg(ARC_REG_IO_COH_AP0_BASE, ap_base >> 12); > >    write_aux_reg(ARC_REG_IO_COH_PARTIAL, 1); > >    write_aux_reg(ARC_REG_IO_COH_ENABLE, 1); > >    > > > > --  Eugeniy Paltsev