Re: [PATCH 01/11] mm/ioremap: change the return value of io[re|un]map_allowed
From: Alexander Gordeev <agordeev@linux.ibm.com>
Date: 2022-08-04 15:43:38
Also in:
linux-mm, lkml
On Mon, Aug 01, 2022 at 10:40:19PM +0800, Baoquan He wrote: Hi Baoquan,
quoted hunk ↗ jump to hunk
--- a/arch/arm64/mm/ioremap.c +++ b/arch/arm64/mm/ioremap.c@@ -3,19 +3,20 @@ #include <linux/mm.h> #include <linux/io.h> -bool ioremap_allowed(phys_addr_t phys_addr, size_t size, unsigned long prot) +void __iomem *ioremap_allowed(phys_addr_t phys_addr, size_t size, unsigned long prot) { unsigned long last_addr = phys_addr + size - 1; + int ret = -EINVAL;
If ret variable is really needed?
quoted hunk ↗ jump to hunk
diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h index 72974cb81343..d72eb310fb3c 100644 --- a/include/asm-generic/io.h +++ b/include/asm-generic/io.h@@ -967,26 +967,27 @@ static inline void iounmap(volatile void __iomem *addr) /* * Arch code can implement the following two hooks when using GENERIC_IOREMAP * ioremap_allowed() return a bool, - * - true means continue to remap - * - false means skip remap and return directly + * - IS_ERR means return an error + * - NULL means continue to remap + * - a non-NULL, non-IS_ERR pointer is returned directly
If ioremap_allowed() returns a valid pointer, then the function name is not as precise anymore.
quoted hunk ↗ jump to hunk
@@ -28,8 +29,11 @@ void __iomem *ioremap_prot(phys_addr_t phys_addr, size_t size, phys_addr -= offset; size = PAGE_ALIGN(size + offset); - if (!ioremap_allowed(phys_addr, size, prot)) + base = ioremap_allowed(phys_addr, size, prot); + if (IS_ERR(base)) return NULL; + else if (base) + return base;
It is probably just me, but the base name bit misleading here.
quoted hunk ↗ jump to hunk
@@ -50,9 +54,9 @@ EXPORT_SYMBOL(ioremap_prot); void iounmap(volatile void __iomem *addr) { - void *vaddr = (void *)((unsigned long)addr & PAGE_MASK); + void __iomem *vaddr = (void __iomem *)((unsigned long)addr & PAGE_MASK); - if (!iounmap_allowed(vaddr)) + if (iounmap_allowed(vaddr))
I guess, iounmap_allowed() should accept void __iomem *, not void *. Then addr needs to be passed to iounmap_allowed() not vaddr.
return;
_______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel