Re: [PATCH v5] devres: combine function devm_ioremap*

[Yisheng Xie]

On 2018/1/23 16:42, Greg KH wrote:
> On Tue, Jan 16, 2018 at 08:03:41PM +0800, Yisheng Xie wrote:
>> When I tried to use devm_ioremap function and review related
>> code, I found devm_ioremap_* almost have the similar realize
>> with each other, which can be combined.
>>
>> In the former version, I have tried to kill ioremap_cache to
>> reduce the size of devres, which can not work for ioremap is
>> not the same as ioremap_nocache in some ARCHs likes ia64.
>> Therefore, as the suggestion of Christophe, I introduce a help
>> function __devm_ioremap, let devm_ioremap* inline and call
>> __devm_ioremap with different devm_ioremap_type.
>>
>> After apply the patch, the size of devres.o can be reduce from
>> 8216 Bytes to 7352Bytes in my compile environment.
>>
>> Suggested-by: Christophe LEROY <christophe.leroy@xxxxxx>
>> Signed-off-by: Yisheng Xie <xieyisheng1@xxxxxxxxxx>
>> ---
>> v2:
>>  - use MARCO for ioremap
>> v3:
>>  - kill dev_ioremap_nocache
>> v4:
>>  - combine function devm_ioremap*
>> v5:
>>  - fix code style.
>>
>>  include/linux/io.h | 61 +++++++++++++++++++++++++++++++++++----
>>  lib/devres.c       | 84 ++++++++++--------------------------------------------
>>  2 files changed, 70 insertions(+), 75 deletions(-)
>>
>> diff --git a/include/linux/io.h b/include/linux/io.h
>> index 32e30e8..4d0a640 100644
>> --- a/include/linux/io.h
>> +++ b/include/linux/io.h
>> @@ -73,12 +73,61 @@ static inline void devm_ioport_unmap(struct device *dev, void __iomem *addr)
>>  
>>  #define IOMEM_ERR_PTR(err) (__force void __iomem *)ERR_PTR(err)
>>  
>> -void __iomem *devm_ioremap(struct device *dev, resource_size_t offset,
>> -			   resource_size_t size);
>> -void __iomem *devm_ioremap_nocache(struct device *dev, resource_size_t offset,
>> -				   resource_size_t size);
>> -void __iomem *devm_ioremap_wc(struct device *dev, resource_size_t offset,
>> -				   resource_size_t size);
>> +enum devm_ioremap_type {
>> +	DEVM_IOREMAP = 0,
>> +	DEVM_IOREMAP_NC,
>> +	DEVM_IOREMAP_WC,
>> +};
> 
> Why do these types need to be in a public .h file?
> 
> Why not just keep the .h file as-is and then just put the cleanup in the
> .c file like you did?
> 
Right. I was just trying to inline these functions. Anyway, I
will follow your suggestion. Sorry for sending so many versions.

Thanks
Yisheng

> thanks,
> 
> greg k-h
> 
> .
>