From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 77AD3C433EF for ; Thu, 19 May 2022 04:47:04 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 99E756B0072; Thu, 19 May 2022 00:47:03 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 927EC6B0073; Thu, 19 May 2022 00:47:03 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 7C8726B0074; Thu, 19 May 2022 00:47:03 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 681F26B0072 for ; Thu, 19 May 2022 00:47:03 -0400 (EDT) Received: from smtpin15.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 43F6031FD5 for ; Thu, 19 May 2022 04:47:03 +0000 (UTC) X-FDA: 79481257926.15.CEA47DD Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by imf24.hostedemail.com (Postfix) with ESMTP id D766C180003 for ; Thu, 19 May 2022 04:46:51 +0000 (UTC) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 75F141042; Wed, 18 May 2022 21:47:01 -0700 (PDT) Received: from [10.162.42.8] (unknown [10.162.42.8]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 750223F73D; Wed, 18 May 2022 21:46:58 -0700 (PDT) Message-ID: <871657f3-9c26-a56a-03d2-29b1915001c9@arm.com> Date: Thu, 19 May 2022 10:16:55 +0530 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.5.0 Subject: Re: [PATCH v2 3/5] mm: ioremap: Add arch_ioremap/iounmap() Content-Language: en-US To: Kefeng Wang , catalin.marinas@arm.com, will@kernel.org, akpm@linux-foundation.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Cc: linux-mm@kvack.org, hch@infradead.org, arnd@arndb.de References: <20220429103225.75121-1-wangkefeng.wang@huawei.com> <20220429103225.75121-4-wangkefeng.wang@huawei.com> From: Anshuman Khandual In-Reply-To: <20220429103225.75121-4-wangkefeng.wang@huawei.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Rspamd-Server: rspam03 X-Rspamd-Queue-Id: D766C180003 X-Stat-Signature: jkdz3j8i1k3n5sk6ryznryekk8xmx4mw Authentication-Results: imf24.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=arm.com; spf=pass (imf24.hostedemail.com: domain of anshuman.khandual@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=anshuman.khandual@arm.com X-Rspam-User: X-HE-Tag: 1652935611-289871 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: Hi Kefeng, On 4/29/22 16:02, Kefeng Wang wrote: > Add special hook for architecture to verify addr, size and prot > or setup when ioremap() or iounmap(), which will make the generic > ioremap more useful. > > arch_ioremap() return a 'void __iomem *', > - IS_ERR means return an error > - NULL means continue to remap > - a non-NULL, non-IS_ERR pointer is directly returned > arch_iounmap() return a int value, > - 0 means continue to vunmap > - error code means skip vunmap and return directly Should not these comments be also included as in-code documentation, possibly near generic fall back stubs for arch_ioremap()/arch_iounmap() in the header include/asm-generic/io.h ? > > Signed-off-by: Kefeng Wang > --- > include/asm-generic/io.h | 14 ++++++++++++++ > mm/ioremap.c | 14 +++++++++++++- > 2 files changed, 27 insertions(+), 1 deletion(-) > > diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h > index e6ffa2519f08..f2f9aeedb5e8 100644 > --- a/include/asm-generic/io.h > +++ b/arch_iounmap > @@ -964,6 +964,20 @@ static inline void iounmap(volatile void __iomem *addr) > #elif defined(CONFIG_GENERIC_IOREMAP) > #include > > +#ifndef arch_ioremap > +static inline void __iomem *arch_ioremap(phys_addr_t phys_addr, size_t size, unsigned long prot) > +{ > + return NULL; > +} > +#endif > + > +#ifndef arch_iounmap > +static inline int arch_iounmap(void __iomem *addr) > +{ > + return 0; > +} > +#endif There is a function in arch/arm/ with exact same name although the platform does not enable GENERIC_IOREMAP. That function would require renaming for these arch callbacks to be added here in GENERIC_IOREMAP path. Otherwise, it might be just confusing later. git grep "arch_iounmap" arch/arm/ arch/arm/include/asm/io.h:extern void (*arch_iounmap)(volatile void __iomem *); arch/arm/mm/ioremap.c:void (*arch_iounmap)(volatile void __iomem *) = __iounmap; arch/arm/mm/ioremap.c: arch_iounmap(cookie); arch/arm/mm/nommu.c:void (*arch_iounmap)(volatile void __iomem *); > + > void __iomem *ioremap_prot(phys_addr_t phys_addr, size_t size, unsigned long prot); > void iounmap(volatile void __iomem *addr); > > diff --git a/mm/ioremap.c b/mm/ioremap.c > index 7cb9996b0c12..de5a2e899e14 100644 > --- a/mm/ioremap.c > +++ b/mm/ioremap.c > @@ -16,6 +16,7 @@ void __iomem *ioremap_prot(phys_addr_t phys_addr, size_t size, unsigned long pro > unsigned long offset, vaddr; > phys_addr_t last_addr; > struct vm_struct *area; > + void __iomem *base; > > /* Disallow wrap-around or zero size */ > last_addr = phys_addr + size - 1; > @@ -27,6 +28,12 @@ void __iomem *ioremap_prot(phys_addr_t phys_addr, size_t size, unsigned long pro > phys_addr -= offset; > size = PAGE_ALIGN(size + offset); > > + base = arch_ioremap(phys_addr, size, prot); > + if (IS_ERR(base)) > + return NULL; > + else if (base) > + return base; > + > area = get_vm_area_caller(size, VM_IOREMAP, > __builtin_return_address(0)); > if (!area) > @@ -45,6 +52,11 @@ EXPORT_SYMBOL(ioremap_prot); > > void iounmap(volatile void __iomem *addr) > { > - vunmap((void *)((unsigned long)addr & PAGE_MASK)); > + void *vaddr = (void *)((unsigned long)addr & PAGE_MASK); Should not this variable be 'void __iomem *vaddr' instead, like above in ioremap_prot(). Because arch_iounmap() takes 'void __iomem *' instead. > + > + if (arch_iounmap(vaddr)) > + return; > + > + vunmap(vaddr); > } > EXPORT_SYMBOL(iounmap); - Anshuman