Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Message-ID: <feae94cd-8474-73e2-2d64-520cec51eb3e@arm.com>
Date:   Mon, 23 May 2022 09:27:57 +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 v3 4/6] mm: ioremap: Add arch_ioremap/iounmap()
Content-Language: en-US
To:     Andrew Morton <akpm@linux-foundation.org>,
        Kefeng Wang <wangkefeng.wang@huawei.com>
Cc:     catalin.marinas@arm.com, will@kernel.org,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
        linux-mm@kvack.org, hch@infradead.org, arnd@arndb.de
References: <20220519082552.117736-1-wangkefeng.wang@huawei.com>
 <20220519082552.117736-5-wangkefeng.wang@huawei.com>
 <20220519115210.a6e41d5a997921354400557e@linux-foundation.org>
From:   Anshuman Khandual <anshuman.khandual@arm.com>
In-Reply-To: <20220519115210.a6e41d5a997921354400557e@linux-foundation.org>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 5/20/22 00:22, Andrew Morton wrote:
> On Thu, 19 May 2022 16:25:50 +0800 Kefeng Wang <wangkefeng.wang@huawei.com> wrote:
> 
>> Add special hook for architecture to verify or setup addr, size
>> or prot when ioremap() or iounmap(), which will make the generic
>> ioremap more useful.
>>
>>   arch_ioremap() return a pointer,
>>     - 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
>>
>> ...
>>
>> --- a/include/asm-generic/io.h
>> +++ b/include/asm-generic/io.h
>> @@ -964,6 +964,30 @@ static inline void iounmap(volatile void __iomem *addr)
>>  #elif defined(CONFIG_GENERIC_IOREMAP)
>>  #include <linux/pgtable.h>
>>  
>> +/*
>> + * Arch code can implement the following two special hooks when using GENERIC_IOREMAP
>> + * arch_ioremap() return a pointer,
>> + *   - IS_ERR means return an error
>> + *   - NULL means continue to remap
>> + *   - a non-NULL, non-IS_ERR pointer is returned directly
>> + * arch_iounmap() return a int,
>> + *   - 0 means continue to vunmap
>> + *   - error code means skip vunmap and return directly
>> + */
>> +#ifndef arch_ioremap
>> +static inline void __iomem *arch_ioremap(phys_addr_t phys_addr, size_t size, unsigned long prot)
>> +{
>> +	return NULL;
>> +}
> 
> Maybe should do
> 
> 	#define arch_ioremap arch_ioremap
> 
> here
> 
>> +#endif
>> +
>> +#ifndef arch_iounmap
>> +static inline int arch_iounmap(void __iomem *addr)
>> +{
>> +	return 0;
>> +}
> 
> and here.
> 
> It shouldn't matter a lot because this file has inclusion guards. 
> However it seems tidier and perhaps other code will want to know
> whether this was defined.  Dunno.

+1, agreed.