On Mon, Aug 01, 2022 at 10:40:20PM +0800, Baoquan He wrote: > This is a preparation patch, no functionality change. There is, please see below. > @@ -3,11 +3,17 @@ > #include <linux/mm.h> > #include <linux/io.h> > > -void __iomem *ioremap_allowed(phys_addr_t phys_addr, size_t size, unsigned long prot) > +void __iomem * > +ioremap_allowed(phys_addr_t *paddr, size_t size, unsigned long *prot_val) > { > - unsigned long last_addr = phys_addr + size - 1; > + unsigned long last_addr, offset, phys_addr = *paddr; > int ret = -EINVAL; > > + offset = phys_addr & (~PAGE_MASK); > + phys_addr -= offset; FWIW, phys_addr &= PAGE_MASK looks much more usual. > @@ -11,13 +11,20 @@ > #include <linux/io.h> > #include <linux/export.h> > > -void __iomem *ioremap_prot(phys_addr_t phys_addr, size_t size, > +void __iomem *ioremap_prot(phys_addr_t paddr, size_t size, > unsigned long prot) > { > unsigned long offset, vaddr; > - phys_addr_t last_addr; > + phys_addr_t last_addr, phys_addr = paddr; > struct vm_struct *area; > void __iomem *base; > + unsigned long prot_val = prot; Why prot_val is needed? > + base = ioremap_allowed(&phys_addr, size, &prot_val); > + if (IS_ERR(base)) > + return NULL; > + else if (base) > + return base; By moving ioremap_allowed() here you allow it to be called before the wrap-around check, including architectures that do not do fixups. And now ioremap_allowed() semantics, prototype and name turn less than obvious. Why not introduce a separate fixup callback? > /* Disallow wrap-around or zero size */ > last_addr = phys_addr + size - 1; > @@ -29,12 +36,6 @@ void __iomem *ioremap_prot(phys_addr_t phys_addr, size_t size, > phys_addr -= offset; > size = PAGE_ALIGN(size + offset); > > - base = ioremap_allowed(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)