Re: [PATCH v3 03/11] mm/ioremap: change the return value of io[re|un]map_allowed and rename

[Kefeng Wang]

On 2022/10/10 8:25, Baoquan He wrote:
> On 10/09/22 at 07:13pm, Kefeng Wang wrote:
> > On 2022/10/9 18:31, Baoquan He wrote:
> > > Currently, hooks ioremap_allowed() and iounmap_allowed() are used to
> > > check if it's qualified to do ioremap, and now this is done on ARM64.
> > > However, in oder to convert more architectures to take GENERIC_IOREMAP
> > > method, several more things need be done in those two hooks:
> > >    1) The io address mapping need be handled specifically on architectures,
> > >       e.g arc, ia64, s390;
> > >    2) The original physical address passed into ioremap_prot() need be
> > >       fixed up, e.g arc;
> > >    3) The 'prot' passed into ioremap_prot() need be adjusted, e.g on arc
> > >       and xtensa.
> > >
> > > To handle these three issues,
> > >
> > >    1) Rename ioremap_allowed() and iounmap_allowed() to arch_ioremap()
> > >       and arch_iounmap() since the old name can't reflect their
> > >       functionality after change;
> > >    2) Change the return value of arch_ioremap() so that arch can add
> > >       specifical io address mapping handling inside and return the maped
> > >       address. Now their returned value means:
> > >       ===
> > >       arch_ioremap() return a bool,
> > pointer?
> Right, I forgot fixing it again. Thanks.
>
> > >         - IS_ERR means return an error
> > >         - 0 means continue to remap
> > >         - a non-zero, non-IS_ERR pointer is returned directly
> > >       arch_iounmap() return a bool,
> > >         - true means continue to vunmap
> > >         - false means skip vunmap and return directly
> > ...
> > >    /*
> > > diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
> > > index a68f8fbf423b..2ae16906f3be 100644
> > > --- a/include/asm-generic/io.h
> > > +++ b/include/asm-generic/io.h
> > > @@ -1049,25 +1049,26 @@ 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
> > > - * iounmap_allowed() return a bool,
> > > + * arch_ioremap() return a bool,
> > ditto...
> Will change.
>
> > >    	area = get_vm_area_caller(size, VM_IOREMAP,
> > >    			__builtin_return_address(0));
> > >    	if (!area)
> > > @@ -52,7 +57,7 @@ void iounmap(volatile void __iomem *addr)
> > >    {
> > >    	void *vaddr = (void *)((unsigned long)addr & PAGE_MASK);
> > > -	if (!iounmap_allowed(vaddr))
> > > +	if (!arch_iounmap((void __iomem *)addr))
> > vaddr?
> No, it's intentional. Alexander suggested this, both of you discussed
> this in v1, see below thread.
ok, please ignore it.
> https://lore.kernel.org/all/Yu4mYxpV0GWRTjQp@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/T/#u
>
> > >    		return;
> > >    	if (is_vmalloc_addr(vaddr))
>
> .