Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752882AbdHVVob (ORCPT ); Tue, 22 Aug 2017 17:44:31 -0400 Received: from smtprelay.synopsys.com ([198.182.60.111]:35782 "EHLO smtprelay.synopsys.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752452AbdHVVo3 (ORCPT ); Tue, 22 Aug 2017 17:44:29 -0400 From: Vineet Gupta Subject: Re: [PATCH 1/3 v8] ARC: Set IO-coherency aperture base to LINUX_LINK_BASE To: Eugeniy Paltsev , CC: , Vineet Gupta , Alexey Brodkin , Rob Herring , Newsgroups: gmane.linux.kernel,gmane.linux.kernel.arc,gmane.linux.drivers.devicetree References: <20170712094023.23226-1-Eugeniy.Paltsev@synopsys.com> <20170712094023.23226-2-Eugeniy.Paltsev@synopsys.com> X-Mozilla-News-Host: news://gmane.comp.lib.uclibc.buildroot Message-ID: <5184d3eb-31bc-c456-3e65-6137a3ba72f7@synopsys.com> Date: Tue, 22 Aug 2017 14:44:20 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: <20170712094023.23226-2-Eugeniy.Paltsev@synopsys.com> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Originating-IP: [10.10.161.108] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3137 Lines: 95 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 ! > > /* 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. > * 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) > + > + /* > + * 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); > >