On 08/20/22 at 11:53pm, Christoph Hellwig wrote: > On Sat, Aug 20, 2022 at 08:31:15AM +0800, Baoquan He wrote: > > +void __iomem *arch_ioremap(phys_addr_t phys_addr, size_t size, unsigned long prot); > > Please avoid the overly long lines. Thanks for reviewing. Will break the line. > > I also wonder if we just want a common definition with a __weak default > instead of duplicating it in many arch headers. Seems __weak symbol is not suggested any more in kernel. Please see below thread. [PATCH] kexec_file: Drop weak attribute from arch_kexec_apply_relocations[_add] https://lore.kernel.org/all/20220518181828.645877-1-naveen.n.rao@xxxxxxxxxxxxxxxxxx/T/#u > > > + ioaddr = arch_ioremap(phys_addr, size, prot); > > + if (IS_ERR(ioaddr)) > > + return NULL; > > + else if (ioaddr) > > + return ioaddr; > > No need for the else here. Do you mean changing it like this? It's fine to me if I get it correctly. ioaddr = arch_ioremap(phys_addr, size, prot); if (IS_ERR(ioaddr)) return NULL; if (ioaddr) return ioaddr; Thanks Baoquan